Re: [PATCH] iommu/amd: Modify set_dte_entry() to use 128-bit cmpxchg operation

From: Uros Bizjak
Date: Mon Aug 19 2024 - 14:15:33 EST




On 19. 08. 24 18:18, Suravee Suthikulpanit wrote:
The current implementation does not follow the 128-bit write
requirement to update DTE as specified in the AMD I/O Virtualization
Techonology (IOMMU) Specification.

In addition, the function is used to program several DTE fields
(e.g. stage1 table, stage2 table, domain id, and etc.), which is
difficult to keep track with current implementation.

Therefore, introduce new a new dte256_t data type and a helper function
update_dte_256(), which uses two try_cmpxchg128 operations to update
256-bit DTE.

Also, separate logic for setting up the GCR3 Table Root Pointer, GIOV, GV,
GLX, and GuestPagingMode into another helper function set_dte_gcr3_table().

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>

[...]

+static void update_dte256(struct amd_iommu *iommu, u16 devid, struct dte256 *new)
+{
+ struct dev_table_entry *dev_table = get_dev_table(iommu);
+ struct dte256 *ptr = (struct dte256 *)&dev_table[devid];
+ struct dte256 old = {
+ .qw_lo.data = ptr->qw_lo.data,
+ .qw_hi.data = ptr->qw_hi.data,
+ };
+
+ /* Update qw_lo */
+ if (!try_cmpxchg128(&ptr->qw_lo.data, &old.qw_lo.data, new->qw_lo.data))
+ goto err_out;
+
+ /* Update qw_hi */
+ if (!try_cmpxchg128(&ptr->qw_hi.data, &old.qw_hi.data, new->qw_hi.data)) {
+ /* Restore qw_lo */
+ try_cmpxchg128(&ptr->qw_lo.data, &new->qw_lo.data, old.qw_lo.data);

You should use plain cmpxchg128() when the result is unused.

Uros.