Re: [PATCH v4 3/6] iommu/amd: Modify set_dte_entry() to use 256-bit DTE helpers

From: Suthikulpanit, Suravee
Date: Thu Oct 03 2024 - 12:19:26 EST


On 9/27/2024 2:56 AM, Jason Gunthorpe wrote:
On Mon, Sep 16, 2024 at 05:18:02PM +0000, Suravee Suthikulpanit wrote:
Also, the set_dte_entry() 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, 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>
---
drivers/iommu/amd/iommu.c | 117 +++++++++++++++++++++-----------------
1 file changed, 65 insertions(+), 52 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 48a721d10f06..12f27061680d 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1947,17 +1947,58 @@ int amd_iommu_clear_gcr3(struct iommu_dev_data *dev_data, ioasid_t pasid)
return ret;
}
+static void set_dte_gcr3_table(struct amd_iommu *iommu,
+ struct iommu_dev_data *dev_data,
+ struct dev_table_entry *target)
+{
+ struct gcr3_tbl_info *gcr3_info = &dev_data->gcr3_info;
+ u64 tmp, gcr3;
+
+ if (!gcr3_info->gcr3_tbl)
+ return;
+
+ pr_debug("%s: devid=%#x, glx=%#x, gcr3_tbl=%#llx\n",
+ __func__, dev_data->devid, gcr3_info->glx,
+ (unsigned long long)gcr3_info->gcr3_tbl);
+
+ tmp = gcr3_info->glx;
+ target->data[0] |= (tmp & DTE_GLX_MASK) << DTE_GLX_SHIFT;
+ if (pdom_is_v2_pgtbl_mode(dev_data->domain))
+ target->data[0] |= DTE_FLAG_GIOV;

When does this get called to install a gcr3 table without a v2 domain?

The GCR3 table is also used when we setup v2 table for SVA stuff. In such case, we would be setting up w/ PASID. Therefore, the GIOV bit is not needed.

Other than my remark on patch 5 this looks Ok to me and making a
helper function for the gcr3 case is a good step forward.

Suggest you follow up with helper functions for blocking, identity and
v1 as well :) Then it will be really easy to follow.

We will look to simplify the code in the future.

Thanks,
Suravee