Skip to content

Commit 9e249c4

Browse files
ankitvvsonijoergroedel
authored andcommitted
iommu/amd: serialize sequence allocation under concurrent TLB invalidations
With concurrent TLB invalidations, completion wait randomly gets timed out because cmd_sem_val was incremented outside the IOMMU spinlock, allowing CMD_COMPL_WAIT commands to be queued out of sequence and breaking the ordering assumption in wait_on_sem(). Move the cmd_sem_val increment under iommu->lock so completion sequence allocation is serialized with command queuing. And remove the unnecessary return. Fixes: d2a0cac ("iommu/amd: move wait_on_sem() out of spinlock") Tested-by: Srikanth Aithal <sraithal@amd.com> Reported-by: Srikanth Aithal <sraithal@amd.com> Signed-off-by: Ankit Soni <Ankit.Soni@amd.com> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
1 parent 5b0530b commit 9e249c4

3 files changed

Lines changed: 14 additions & 8 deletions

File tree

drivers/iommu/amd/amd_iommu_types.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,7 @@ struct amd_iommu {
752752

753753
u32 flags;
754754
volatile u64 *cmd_sem;
755-
atomic64_t cmd_sem_val;
755+
u64 cmd_sem_val;
756756
/*
757757
* Track physical address to directly use it in build_completion_wait()
758758
* and avoid adding any special checks and handling for kdump.

drivers/iommu/amd/init.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1885,7 +1885,7 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h,
18851885
iommu->pci_seg = pci_seg;
18861886

18871887
raw_spin_lock_init(&iommu->lock);
1888-
atomic64_set(&iommu->cmd_sem_val, 0);
1888+
iommu->cmd_sem_val = 0;
18891889

18901890
/* Add IOMMU to internal data structures */
18911891
list_add_tail(&iommu->list, &amd_iommu_list);

drivers/iommu/amd/iommu.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,6 +1439,12 @@ static int iommu_queue_command(struct amd_iommu *iommu, struct iommu_cmd *cmd)
14391439
return iommu_queue_command_sync(iommu, cmd, true);
14401440
}
14411441

1442+
static u64 get_cmdsem_val(struct amd_iommu *iommu)
1443+
{
1444+
lockdep_assert_held(&iommu->lock);
1445+
return ++iommu->cmd_sem_val;
1446+
}
1447+
14421448
/*
14431449
* This function queues a completion wait command into the command
14441450
* buffer of an IOMMU
@@ -1453,11 +1459,11 @@ static int iommu_completion_wait(struct amd_iommu *iommu)
14531459
if (!iommu->need_sync)
14541460
return 0;
14551461

1456-
data = atomic64_inc_return(&iommu->cmd_sem_val);
1457-
build_completion_wait(&cmd, iommu, data);
1458-
14591462
raw_spin_lock_irqsave(&iommu->lock, flags);
14601463

1464+
data = get_cmdsem_val(iommu);
1465+
build_completion_wait(&cmd, iommu, data);
1466+
14611467
ret = __iommu_queue_command_sync(iommu, &cmd, false);
14621468
raw_spin_unlock_irqrestore(&iommu->lock, flags);
14631469

@@ -3177,10 +3183,11 @@ static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid)
31773183
return;
31783184

31793185
build_inv_irt(&cmd, devid);
3180-
data = atomic64_inc_return(&iommu->cmd_sem_val);
3181-
build_completion_wait(&cmd2, iommu, data);
31823186

31833187
raw_spin_lock_irqsave(&iommu->lock, flags);
3188+
data = get_cmdsem_val(iommu);
3189+
build_completion_wait(&cmd2, iommu, data);
3190+
31843191
ret = __iommu_queue_command_sync(iommu, &cmd, true);
31853192
if (ret)
31863193
goto out_err;
@@ -3194,7 +3201,6 @@ static void iommu_flush_irt_and_complete(struct amd_iommu *iommu, u16 devid)
31943201

31953202
out_err:
31963203
raw_spin_unlock_irqrestore(&iommu->lock, flags);
3197-
return;
31983204
}
31993205

32003206
static inline u8 iommu_get_int_tablen(struct iommu_dev_data *dev_data)

0 commit comments

Comments
 (0)