Re: [PATCH v5 04/15] iommupt: Add the AMD IOMMU v1 page table format

Vasant Hegde
2025-10-09T11:44:23Z

Jason,

On 10/8/2025 6:38 PM, Jason Gunthorpe wrote:
> On Wed, Oct 08, 2025 at 03:13:32PM +0530, Vasant Hegde wrote:
>> Jason,
>>
>> On 9/3/2025 11:16 PM, Jason Gunthorpe wrote:
>>> AMD IOMMU v1 is unique in supporting contiguous pages with a variable size
>>> and it can decode the full 64 bit VA space. Unlike other x86 page tables
>>> this explicitly does not do sign extension as part of allowing the entire
>>> 64 bit VA space to be supported.
>>
>> I am still catching up w/ entire series.. But here is few fixes needed to boot
>> this series w/ SME.
>
> I got them all, like this - thanks a lot!

Looks good. Thanks!

-Vasant

>
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 92095fd17b3899..0b97db94c8c4e0 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2054,7 +2054,7 @@ static void set_dte_entry(struct amd_iommu *iommu,
> &pt_info);
> }
>
> - new.data[0] |= pt_info.host_pt_root |
> + new.data[0] |= __sme_set(pt_info.host_pt_root) |
> (pt_info.mode & DEV_ENTRY_MODE_MASK)
> << DEV_ENTRY_MODE_SHIFT;
> }
> @@ -2166,7 +2166,7 @@ static int init_gcr3_table(struct iommu_dev_data *dev_data,
> return ret;
>
> pt_iommu_x86_64_hw_info(&pdom->amdv2, &pt_info);
> - ret = update_gcr3(dev_data, 0, pt_info.gcr3_pt, true);
> + ret = update_gcr3(dev_data, 0, __sme_set(pt_info.gcr3_pt), true);
> if (ret)
> free_gcr3_table(&dev_data->gcr3_info);
>
> diff --git a/drivers/iommu/generic_pt/fmt/amdv1.h b/drivers/iommu/generic_pt/fmt/amdv1.h
> index d7660d4170ef78..26e29b08a9b4ae 100644
> --- a/drivers/iommu/generic_pt/fmt/amdv1.h
> +++ b/drivers/iommu/generic_pt/fmt/amdv1.h
> @@ -73,22 +73,29 @@ enum {
>
> static inline pt_oaddr_t amdv1pt_table_pa(const struct pt_state *pts)
> {
> - return oalog2_mul(FIELD_GET(AMDV1PT_FMT_OA, pts->entry),
> - PT_GRANULE_LG2SZ);
> + u64 entry = pts->entry;
> +
> + if (pts_feature(pts, PT_FEAT_AMDV1_ENCRYPT_TABLES))
> + entry = __sme_clr(entry);
> + return oalog2_mul(FIELD_GET(AMDV1PT_FMT_OA, entry), PT_GRANULE_LG2SZ);
> }
> #define pt_table_pa amdv1pt_table_pa
>
> /* Returns the oa for the start of the contiguous entry */
> static inline pt_oaddr_t amdv1pt_entry_oa(const struct pt_state *pts)
> {
> - pt_oaddr_t oa = FIELD_GET(AMDV1PT_FMT_OA, pts->entry);
> + u64 entry = pts->entry;
> + pt_oaddr_t oa;
>
> - if (FIELD_GET(AMDV1PT_FMT_NEXT_LEVEL, pts->entry) ==
> - AMDV1PT_FMT_NL_SIZE) {
> + if (pts_feature(pts, PT_FEAT_AMDV1_ENCRYPT_TABLES))
> + entry = __sme_clr(entry);
> + oa = FIELD_GET(AMDV1PT_FMT_OA, entry);
> +
> + if (FIELD_GET(AMDV1PT_FMT_NEXT_LEVEL, entry) == AMDV1PT_FMT_NL_SIZE) {
> unsigned int sz_bits = oaffz(oa);
>
> oa = oalog2_set_mod(oa, 0, sz_bits);
> - } else if (PT_WARN_ON(FIELD_GET(AMDV1PT_FMT_NEXT_LEVEL, pts->entry) !=
> + } else if (PT_WARN_ON(FIELD_GET(AMDV1PT_FMT_NEXT_LEVEL, entry) !=
> AMDV1PT_FMT_NL_DEFAULT))
> return 0;
> return oalog2_mul(oa, PT_GRANULE_LG2SZ);
> diff --git a/drivers/iommu/generic_pt/fmt/x86_64.h b/drivers/iommu/generic_pt/fmt/x86_64.h
> index be2a0a770f903f..d33b2fcd865b84 100644
> --- a/drivers/iommu/generic_pt/fmt/x86_64.h
> +++ b/drivers/iommu/generic_pt/fmt/x86_64.h
> @@ -79,14 +79,22 @@ enum {
>
> static inline pt_oaddr_t x86_64_pt_table_pa(const struct pt_state *pts)
> {
> - return oalog2_mul(FIELD_GET(X86_64_FMT_OA, pts->entry),
> + u64 entry = pts->entry;
> +
> + if (pts_feature(pts, PT_FEAT_X86_64_AMD_ENCRYPT_TABLES))
> + entry = __sme_clr(entry);
> + return oalog2_mul(FIELD_GET(X86_64_FMT_OA, entry),
> PT_TABLEMEM_LG2SZ);
> }
> #define pt_table_pa x86_64_pt_table_pa
>
> static inline pt_oaddr_t x86_64_pt_entry_oa(const struct pt_state *pts)
> {
> - return oalog2_mul(FIELD_GET(X86_64_FMT_OA, pts->entry),
> + u64 entry = pts->entry;
> +
> + if (pts_feature(pts, PT_FEAT_X86_64_AMD_ENCRYPT_TABLES))
> + entry = __sme_clr(entry);
> + return oalog2_mul(FIELD_GET(X86_64_FMT_OA, entry),
> PT_GRANULE_LG2SZ);
> }
> #define pt_entry_oa x86_64_pt_entry_oa