Skip to content

Commit cfb60d7

Browse files
ssuthiku-amdgregkh
ssuthiku-amd
authored and
gregkh
committed
iommu/amd: Introduce helper function to update 256-bit DTE
[ Upstream commit 8b3f787 ] The current implementation does not follow 128-bit write requirement to update DTE as specified in the AMD I/O Virtualization Techonology (IOMMU) Specification. Therefore, modify the struct dev_table_entry to contain union of u128 data array, and introduce a helper functions update_dte256() to update DTE using two 128-bit cmpxchg operations to update 256-bit DTE with the modified structure, and take into account the DTE[V, GV] bits when programming the DTE to ensure proper order of DTE programming and flushing. In addition, introduce a per-DTE spin_lock struct dev_data.dte_lock to provide synchronization when updating the DTE to prevent cmpxchg128 failure. Suggested-by: Jason Gunthorpe <jgg@nvidia.com> Suggested-by: Uros Bizjak <ubizjak@gmail.com> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Uros Bizjak <ubizjak@gmail.com> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> Link: https://lore.kernel.org/r/20241118054937.5203-5-suravee.suthikulpanit@amd.com Signed-off-by: Joerg Roedel <jroedel@suse.de> Stable-dep-of: faad224 ("iommu/amd: Fix clone_alias() to use the original device's devid") Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 7b3c9d3 commit cfb60d7

2 files changed

Lines changed: 132 additions & 1 deletion

File tree

‎drivers/iommu/amd/amd_iommu_types.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,9 +426,13 @@
426426
#define DTE_GCR3_SHIFT_C 43
427427

428428
#define DTE_GPT_LEVEL_SHIFT 54
429+
#define DTE_GPT_LEVEL_MASK GENMASK_ULL(55, 54)
429430

430431
#define GCR3_VALID 0x01ULL
431432

433+
/* DTE[128:179] | DTE[184:191] */
434+
#define DTE_DATA2_INTR_MASK ~GENMASK_ULL(55, 52)
435+
432436
#define IOMMU_PAGE_MASK (((1ULL << 52) - 1) & ~0xfffULL)
433437
#define IOMMU_PTE_PRESENT(pte) ((pte) & IOMMU_PTE_PR)
434438
#define IOMMU_PTE_DIRTY(pte) ((pte) & IOMMU_PTE_HD)
@@ -839,6 +843,7 @@ struct devid_map {
839843
struct iommu_dev_data {
840844
/*Protect against attach/detach races */
841845
struct mutex mutex;
846+
spinlock_t dte_lock; /* DTE lock for 256-bit access */
842847

843848
struct list_head list; /* For domain->dev_list */
844849
struct llist_node dev_data_list; /* For global dev_data_list */
@@ -889,7 +894,10 @@ extern struct amd_iommu *amd_iommus[MAX_IOMMUS];
889894
* Structure defining one entry in the device table
890895
*/
891896
struct dev_table_entry {
892-
u64 data[4];
897+
union {
898+
u64 data[4];
899+
u128 data128[2];
900+
};
893901
};
894902

895903
/*

‎drivers/iommu/amd/iommu.c‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,125 @@ static void detach_device(struct device *dev);
7777
static void set_dte_entry(struct amd_iommu *iommu,
7878
struct iommu_dev_data *dev_data);
7979

80+
static void iommu_flush_dte_sync(struct amd_iommu *iommu, u16 devid);
81+
8082
/****************************************************************************
8183
*
8284
* Helper functions
8385
*
8486
****************************************************************************/
8587

88+
static __always_inline void amd_iommu_atomic128_set(__int128 *ptr, __int128 val)
89+
{
90+
/*
91+
* Note:
92+
* We use arch_cmpxchg128_local() because:
93+
* - Need cmpxchg16b instruction mainly for 128-bit store to DTE
94+
* (not necessary for cmpxchg since this function is already
95+
* protected by a spin_lock for this DTE).
96+
* - Neither need LOCK_PREFIX nor try loop because of the spin_lock.
97+
*/
98+
arch_cmpxchg128_local(ptr, *ptr, val);
99+
}
100+
101+
static void write_dte_upper128(struct dev_table_entry *ptr, struct dev_table_entry *new)
102+
{
103+
struct dev_table_entry old;
104+
105+
old.data128[1] = ptr->data128[1];
106+
/*
107+
* Preserve DTE_DATA2_INTR_MASK. This needs to be
108+
* done here since it requires to be inside
109+
* spin_lock(&dev_data->dte_lock) context.
110+
*/
111+
new->data[2] &= ~DTE_DATA2_INTR_MASK;
112+
new->data[2] |= old.data[2] & DTE_DATA2_INTR_MASK;
113+
114+
amd_iommu_atomic128_set(&ptr->data128[1], new->data128[1]);
115+
}
116+
117+
static void write_dte_lower128(struct dev_table_entry *ptr, struct dev_table_entry *new)
118+
{
119+
amd_iommu_atomic128_set(&ptr->data128[0], new->data128[0]);
120+
}
121+
122+
/*
123+
* Note:
124+
* IOMMU reads the entire Device Table entry in a single 256-bit transaction
125+
* but the driver is programming DTE using 2 128-bit cmpxchg. So, the driver
126+
* need to ensure the following:
127+
* - DTE[V|GV] bit is being written last when setting.
128+
* - DTE[V|GV] bit is being written first when clearing.
129+
*
130+
* This function is used only by code, which updates DMA translation part of the DTE.
131+
* So, only consider control bits related to DMA when updating the entry.
132+
*/
133+
static void update_dte256(struct amd_iommu *iommu, struct iommu_dev_data *dev_data,
134+
struct dev_table_entry *new)
135+
{
136+
unsigned long flags;
137+
struct dev_table_entry *dev_table = get_dev_table(iommu);
138+
struct dev_table_entry *ptr = &dev_table[dev_data->devid];
139+
140+
spin_lock_irqsave(&dev_data->dte_lock, flags);
141+
142+
if (!(ptr->data[0] & DTE_FLAG_V)) {
143+
/* Existing DTE is not valid. */
144+
write_dte_upper128(ptr, new);
145+
write_dte_lower128(ptr, new);
146+
iommu_flush_dte_sync(iommu, dev_data->devid);
147+
} else if (!(new->data[0] & DTE_FLAG_V)) {
148+
/* Existing DTE is valid. New DTE is not valid. */
149+
write_dte_lower128(ptr, new);
150+
write_dte_upper128(ptr, new);
151+
iommu_flush_dte_sync(iommu, dev_data->devid);
152+
} else if (!FIELD_GET(DTE_FLAG_GV, ptr->data[0])) {
153+
/*
154+
* Both DTEs are valid.
155+
* Existing DTE has no guest page table.
156+
*/
157+
write_dte_upper128(ptr, new);
158+
write_dte_lower128(ptr, new);
159+
iommu_flush_dte_sync(iommu, dev_data->devid);
160+
} else if (!FIELD_GET(DTE_FLAG_GV, new->data[0])) {
161+
/*
162+
* Both DTEs are valid.
163+
* Existing DTE has guest page table,
164+
* new DTE has no guest page table,
165+
*/
166+
write_dte_lower128(ptr, new);
167+
write_dte_upper128(ptr, new);
168+
iommu_flush_dte_sync(iommu, dev_data->devid);
169+
} else if (FIELD_GET(DTE_GPT_LEVEL_MASK, ptr->data[2]) !=
170+
FIELD_GET(DTE_GPT_LEVEL_MASK, new->data[2])) {
171+
/*
172+
* Both DTEs are valid and have guest page table,
173+
* but have different number of levels. So, we need
174+
* to upadte both upper and lower 128-bit value, which
175+
* require disabling and flushing.
176+
*/
177+
struct dev_table_entry clear = {};
178+
179+
/* First disable DTE */
180+
write_dte_lower128(ptr, &clear);
181+
iommu_flush_dte_sync(iommu, dev_data->devid);
182+
183+
/* Then update DTE */
184+
write_dte_upper128(ptr, new);
185+
write_dte_lower128(ptr, new);
186+
iommu_flush_dte_sync(iommu, dev_data->devid);
187+
} else {
188+
/*
189+
* Both DTEs are valid and have guest page table,
190+
* and same number of levels. We just need to only
191+
* update the lower 128-bit. So no need to disable DTE.
192+
*/
193+
write_dte_lower128(ptr, new);
194+
}
195+
196+
spin_unlock_irqrestore(&dev_data->dte_lock, flags);
197+
}
198+
86199
static inline bool pdom_is_v2_pgtbl_mode(struct protection_domain *pdom)
87200
{
88201
return (pdom && (pdom->pd_mode == PD_MODE_V2));
@@ -226,6 +339,7 @@ static struct iommu_dev_data *alloc_dev_data(struct amd_iommu *iommu, u16 devid)
226339
return NULL;
227340

228341
mutex_init(&dev_data->mutex);
342+
spin_lock_init(&dev_data->dte_lock);
229343
dev_data->devid = devid;
230344
ratelimit_default_init(&dev_data->rs);
231345

@@ -1312,6 +1426,15 @@ static int iommu_flush_dte(struct amd_iommu *iommu, u16 devid)
13121426
return iommu_queue_command(iommu, &cmd);
13131427
}
13141428

1429+
static void iommu_flush_dte_sync(struct amd_iommu *iommu, u16 devid)
1430+
{
1431+
int ret;
1432+
1433+
ret = iommu_flush_dte(iommu, devid);
1434+
if (!ret)
1435+
iommu_completion_wait(iommu);
1436+
}
1437+
13151438
static void amd_iommu_flush_dte_all(struct amd_iommu *iommu)
13161439
{
13171440
u32 devid;

0 commit comments

Comments
 (0)