1. Patchew
  2. QEMU
  3. [PATCH v7 00/23] intel_iommu: Enable first stage translation for passthrough device
  4. View patch

[PATCH v7 12/23] intel_iommu: Add some macros and inline functions

Zhenzhong Duan posted 23 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v7 12/23] intel_iommu: Add some macros and inline functions
Posted by Zhenzhong Duan 3 months, 2 weeks ago 
Add some macros and inline functions that will be used by following
patch.

This patch also make a cleanup to change macro VTD_SM_PASID_ENTRY_DID
and VTD_SM_PASID_ENTRY_FSPM to use extract64() just like what smmu does,
because they are either used in following patches or used indirectly by
new introduced inline functions. But we doesn't aim to change the huge
amount of bit mask style macro definitions in this patch, that should be
in a separate patch.

Suggested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Reviewed-by: Yi Liu <yi.l.liu@intel.com>
---
 hw/i386/intel_iommu_internal.h |  8 +++++--
 hw/i386/intel_iommu.c          | 38 +++++++++++++++++++++++++++-------
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 09edba81e2..df80af839d 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -642,10 +642,14 @@ typedef struct VTDPASIDCacheInfo {
 #define VTD_SM_PASID_ENTRY_PT          (4ULL << 6)
 
 #define VTD_SM_PASID_ENTRY_AW          7ULL /* Adjusted guest-address-width */
-#define VTD_SM_PASID_ENTRY_DID(val)    ((val) & VTD_DOMAIN_ID_MASK)
+#define VTD_SM_PASID_ENTRY_DID(x)      extract64((x)->val[1], 0, 16)
 
-#define VTD_SM_PASID_ENTRY_FSPM          3ULL
 #define VTD_SM_PASID_ENTRY_FSPTPTR       (~0xfffULL)
+#define VTD_SM_PASID_ENTRY_SRE_BIT(x)    extract64((x)->val[2], 0, 1)
+/* 00: 4-level paging, 01: 5-level paging, 10-11: Reserved */
+#define VTD_SM_PASID_ENTRY_FSPM(x)       extract64((x)->val[2], 2, 2)
+#define VTD_SM_PASID_ENTRY_WPE_BIT(x)    extract64((x)->val[2], 4, 1)
+#define VTD_SM_PASID_ENTRY_EAFE_BIT(x)   extract64((x)->val[2], 7, 1)
 
 /* First Stage Paging Structure */
 /* Masks for First Stage Paging Entry */
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 56abbb991d..871e6aad19 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -52,8 +52,7 @@
 
 /* pe operations */
 #define VTD_PE_GET_TYPE(pe) ((pe)->val[0] & VTD_SM_PASID_ENTRY_PGTT)
-#define VTD_PE_GET_FS_LEVEL(pe) \
-    (4 + (((pe)->val[2] >> 2) & VTD_SM_PASID_ENTRY_FSPM))
+#define VTD_PE_GET_FS_LEVEL(pe) (VTD_SM_PASID_ENTRY_FSPM(pe) + 4)
 #define VTD_PE_GET_SS_LEVEL(pe) \
     (2 + (((pe)->val[0] >> 2) & VTD_SM_PASID_ENTRY_AW))
 
@@ -837,6 +836,31 @@ static inline bool vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
     }
 }
 
+static inline dma_addr_t vtd_pe_get_fspt_base(VTDPASIDEntry *pe)
+{
+    return pe->val[2] & VTD_SM_PASID_ENTRY_FSPTPTR;
+}
+
+/*
+ * First stage IOVA address width: 48 bits for 4-level paging(FSPM=00)
+ *                                 57 bits for 5-level paging(FSPM=01)
+ */
+static inline uint32_t vtd_pe_get_fs_aw(VTDPASIDEntry *pe)
+{
+    return 48 + VTD_SM_PASID_ENTRY_FSPM(pe) * 9;
+}
+
+static inline bool vtd_pe_pgtt_is_pt(VTDPASIDEntry *pe)
+{
+    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_PT);
+}
+
+/* check if pgtt is first stage translation */
+static inline bool vtd_pe_pgtt_is_fst(VTDPASIDEntry *pe)
+{
+    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_FST);
+}
+
 static inline bool vtd_pdire_present(VTDPASIDDirEntry *pdire)
 {
     return pdire->val & 1;
@@ -1625,7 +1649,7 @@ static uint16_t vtd_get_domain_id(IntelIOMMUState *s,
 
     if (s->root_scalable) {
         vtd_ce_get_pasid_entry(s, ce, &pe, pasid);
-        return VTD_SM_PASID_ENTRY_DID(pe.val[1]);
+        return VTD_SM_PASID_ENTRY_DID(&pe);
     }
 
     return VTD_CONTEXT_ENTRY_DID(ce->hi);
@@ -1707,7 +1731,7 @@ static bool vtd_dev_pt_enabled(IntelIOMMUState *s, VTDContextEntry *ce,
              */
             return false;
         }
-        return (VTD_PE_GET_TYPE(&pe) == VTD_SM_PASID_ENTRY_PT);
+        return vtd_pe_pgtt_is_pt(&pe);
     }
 
     return (vtd_ce_get_type(ce) == VTD_CONTEXT_TT_PASS_THROUGH);
@@ -3146,9 +3170,9 @@ static void vtd_pasid_cache_sync_locked(gpointer key, gpointer value,
         /* Fall through */
     case VTD_INV_DESC_PASIDC_G_DSI:
         if (pc_entry->valid) {
-            did = VTD_SM_PASID_ENTRY_DID(pc_entry->pasid_entry.val[1]);
+            did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
         } else {
-            did = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
+            did = VTD_SM_PASID_ENTRY_DID(&pe);
         }
         if (pc_info->did != did) {
             return;
@@ -5267,7 +5291,7 @@ static int vtd_pri_perform_implicit_invalidation(VTDAddressSpace *vtd_as,
         return -EINVAL;
     }
     pgtt = VTD_PE_GET_TYPE(&pe);
-    domain_id = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
+    domain_id = VTD_SM_PASID_ENTRY_DID(&pe);
     ret = 0;
     switch (pgtt) {
     case VTD_SM_PASID_ENTRY_FST:
-- 
2.47.1
Re: [PATCH v7 12/23] intel_iommu: Add some macros and inline functions
Posted by Eric Auger 3 months, 1 week ago 
Hi Zhenzhong,

On 10/24/25 10:43 AM, Zhenzhong Duan wrote:
> Add some macros and inline functions that will be used by following
> patch.
>
> This patch also make a cleanup to change macro VTD_SM_PASID_ENTRY_DID
> and VTD_SM_PASID_ENTRY_FSPM to use extract64() just like what smmu does,
> because they are either used in following patches or used indirectly by
> new introduced inline functions. But we doesn't aim to change the huge
> amount of bit mask style macro definitions in this patch, that should be
> in a separate patch.
>
> Suggested-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
> ---
>  hw/i386/intel_iommu_internal.h |  8 +++++--
>  hw/i386/intel_iommu.c          | 38 +++++++++++++++++++++++++++-------
>  2 files changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 09edba81e2..df80af839d 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -642,10 +642,14 @@ typedef struct VTDPASIDCacheInfo {
>  #define VTD_SM_PASID_ENTRY_PT          (4ULL << 6)
>  
>  #define VTD_SM_PASID_ENTRY_AW          7ULL /* Adjusted guest-address-width */
> -#define VTD_SM_PASID_ENTRY_DID(val)    ((val) & VTD_DOMAIN_ID_MASK)
> +#define VTD_SM_PASID_ENTRY_DID(x)      extract64((x)->val[1], 0, 16)
>  
> -#define VTD_SM_PASID_ENTRY_FSPM          3ULL
>  #define VTD_SM_PASID_ENTRY_FSPTPTR       (~0xfffULL)
> +#define VTD_SM_PASID_ENTRY_SRE_BIT(x)    extract64((x)->val[2], 0, 1)
> +/* 00: 4-level paging, 01: 5-level paging, 10-11: Reserved */
the above comment is not that useful along with bitmask definition. I
would rather move it along with VTD_PE_GET_FS_LEVEL(pe)
> +#define VTD_SM_PASID_ENTRY_FSPM(x)       extract64((x)->val[2], 2, 2)
> +#define VTD_SM_PASID_ENTRY_WPE_BIT(x)    extract64((x)->val[2], 4, 1)
> +#define VTD_SM_PASID_ENTRY_EAFE_BIT(x)   extract64((x)->val[2], 7, 1)
I would get rid of _BIT suffix to make it consistent with other fields.
>  
>  /* First Stage Paging Structure */
>  /* Masks for First Stage Paging Entry */
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 56abbb991d..871e6aad19 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -52,8 +52,7 @@
>  
>  /* pe operations */
>  #define VTD_PE_GET_TYPE(pe) ((pe)->val[0] & VTD_SM_PASID_ENTRY_PGTT)
> -#define VTD_PE_GET_FS_LEVEL(pe) \
> -    (4 + (((pe)->val[2] >> 2) & VTD_SM_PASID_ENTRY_FSPM))
> +#define VTD_PE_GET_FS_LEVEL(pe) (VTD_SM_PASID_ENTRY_FSPM(pe) + 4)
>  #define VTD_PE_GET_SS_LEVEL(pe) \
>      (2 + (((pe)->val[0] >> 2) & VTD_SM_PASID_ENTRY_AW))
>  
> @@ -837,6 +836,31 @@ static inline bool vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
>      }
>  }
>  
> +static inline dma_addr_t vtd_pe_get_fspt_base(VTDPASIDEntry *pe)
> +{
> +    return pe->val[2] & VTD_SM_PASID_ENTRY_FSPTPTR;

also introduce a define for FSPPTR using extract64((x)->val[2]

> +}
> +
> +/*
> + * First stage IOVA address width: 48 bits for 4-level paging(FSPM=00)
> + *                                 57 bits for 5-level paging(FSPM=01)
> + */
> +static inline uint32_t vtd_pe_get_fs_aw(VTDPASIDEntry *pe)
> +{
> +    return 48 + VTD_SM_PASID_ENTRY_FSPM(pe) * 9;
can you add a comment refering to the spec here?
> +}
> +
> +static inline bool vtd_pe_pgtt_is_pt(VTDPASIDEntry *pe)
> +{
> +    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_PT);
> +}
> +
> +/* check if pgtt is first stage translation */
> +static inline bool vtd_pe_pgtt_is_fst(VTDPASIDEntry *pe)
> +{
> +    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_FST);
> +}
> +
>  static inline bool vtd_pdire_present(VTDPASIDDirEntry *pdire)
>  {
>      return pdire->val & 1;
> @@ -1625,7 +1649,7 @@ static uint16_t vtd_get_domain_id(IntelIOMMUState *s,
>  
>      if (s->root_scalable) {
>          vtd_ce_get_pasid_entry(s, ce, &pe, pasid);
> -        return VTD_SM_PASID_ENTRY_DID(pe.val[1]);
> +        return VTD_SM_PASID_ENTRY_DID(&pe);
>      }
>  
>      return VTD_CONTEXT_ENTRY_DID(ce->hi);
> @@ -1707,7 +1731,7 @@ static bool vtd_dev_pt_enabled(IntelIOMMUState *s, VTDContextEntry *ce,
>               */
>              return false;
>          }
> -        return (VTD_PE_GET_TYPE(&pe) == VTD_SM_PASID_ENTRY_PT);
> +        return vtd_pe_pgtt_is_pt(&pe);
>      }
>  
>      return (vtd_ce_get_type(ce) == VTD_CONTEXT_TT_PASS_THROUGH);
> @@ -3146,9 +3170,9 @@ static void vtd_pasid_cache_sync_locked(gpointer key, gpointer value,
>          /* Fall through */
>      case VTD_INV_DESC_PASIDC_G_DSI:
>          if (pc_entry->valid) {
> -            did = VTD_SM_PASID_ENTRY_DID(pc_entry->pasid_entry.val[1]);
> +            did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
>          } else {
> -            did = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
> +            did = VTD_SM_PASID_ENTRY_DID(&pe);
>          }
>          if (pc_info->did != did) {
>              return;
> @@ -5267,7 +5291,7 @@ static int vtd_pri_perform_implicit_invalidation(VTDAddressSpace *vtd_as,
>          return -EINVAL;
>      }
>      pgtt = VTD_PE_GET_TYPE(&pe);
> -    domain_id = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
> +    domain_id = VTD_SM_PASID_ENTRY_DID(&pe);
>      ret = 0;
>      switch (pgtt) {
>      case VTD_SM_PASID_ENTRY_FST:
Otherwise looks good to me

Thanks

Eric
RE: [PATCH v7 12/23] intel_iommu: Add some macros and inline functions
Posted by Duan, Zhenzhong 3 months, 1 week ago 
Hi Eric,

I showed the planed change inline, let me know if I misunderstood.

Thanks
Zhenzhong

>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH v7 12/23] intel_iommu: Add some macros and inline
>functions
>
>Hi Zhenzhong,
>
>On 10/24/25 10:43 AM, Zhenzhong Duan wrote:
>> Add some macros and inline functions that will be used by following
>> patch.
>>
>> This patch also make a cleanup to change macro
>VTD_SM_PASID_ENTRY_DID
>> and VTD_SM_PASID_ENTRY_FSPM to use extract64() just like what smmu
>does,
>> because they are either used in following patches or used indirectly by
>> new introduced inline functions. But we doesn't aim to change the huge
>> amount of bit mask style macro definitions in this patch, that should be
>> in a separate patch.
>>
>> Suggested-by: Eric Auger <eric.auger@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>>  hw/i386/intel_iommu_internal.h |  8 +++++--
>>  hw/i386/intel_iommu.c          | 38
>+++++++++++++++++++++++++++-------
>>  2 files changed, 37 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h
>b/hw/i386/intel_iommu_internal.h
>> index 09edba81e2..df80af839d 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -642,10 +642,14 @@ typedef struct VTDPASIDCacheInfo {
>>  #define VTD_SM_PASID_ENTRY_PT          (4ULL << 6)
>>
>>  #define VTD_SM_PASID_ENTRY_AW          7ULL /* Adjusted
>guest-address-width */
>> -#define VTD_SM_PASID_ENTRY_DID(val)    ((val) &
>VTD_DOMAIN_ID_MASK)
>> +#define VTD_SM_PASID_ENTRY_DID(x)      extract64((x)->val[1], 0, 16)
>>
>> -#define VTD_SM_PASID_ENTRY_FSPM          3ULL
>>  #define VTD_SM_PASID_ENTRY_FSPTPTR       (~0xfffULL)
>> +#define VTD_SM_PASID_ENTRY_SRE_BIT(x)    extract64((x)->val[2], 0, 1)
>> +/* 00: 4-level paging, 01: 5-level paging, 10-11: Reserved */
>the above comment is not that useful along with bitmask definition. I
>would rather move it along with VTD_PE_GET_FS_LEVEL(pe)

OK, will move it like below:

--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -56,6 +56,7 @@

 /* pe operations */
 #define VTD_PE_GET_TYPE(pe) ((pe)->val[0] & VTD_SM_PASID_ENTRY_PGTT)
+/* 4: 4-level paging, 5: 5-level paging */
 #define VTD_PE_GET_FS_LEVEL(pe) (VTD_SM_PASID_ENTRY_FSPM(pe) + 4)
 #define VTD_PE_GET_SS_LEVEL(pe) \
     (2 + (((pe)->val[0] >> 2) & VTD_SM_PASID_ENTRY_AW))


>> +#define VTD_SM_PASID_ENTRY_FSPM(x)       extract64((x)->val[2], 2,
>2)
>> +#define VTD_SM_PASID_ENTRY_WPE_BIT(x)    extract64((x)->val[2], 4,
>1)
>> +#define VTD_SM_PASID_ENTRY_EAFE_BIT(x)   extract64((x)->val[2], 7, 1)
>I would get rid of _BIT suffix to make it consistent with other fields.

OK

>>
>>  /* First Stage Paging Structure */
>>  /* Masks for First Stage Paging Entry */
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 56abbb991d..871e6aad19 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -52,8 +52,7 @@
>>
>>  /* pe operations */
>>  #define VTD_PE_GET_TYPE(pe) ((pe)->val[0] &
>VTD_SM_PASID_ENTRY_PGTT)
>> -#define VTD_PE_GET_FS_LEVEL(pe) \
>> -    (4 + (((pe)->val[2] >> 2) & VTD_SM_PASID_ENTRY_FSPM))
>> +#define VTD_PE_GET_FS_LEVEL(pe) (VTD_SM_PASID_ENTRY_FSPM(pe) +
>4)
>>  #define VTD_PE_GET_SS_LEVEL(pe) \
>>      (2 + (((pe)->val[0] >> 2) & VTD_SM_PASID_ENTRY_AW))
>>
>> @@ -837,6 +836,31 @@ static inline bool
>vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
>>      }
>>  }
>>
>> +static inline dma_addr_t vtd_pe_get_fspt_base(VTDPASIDEntry *pe)
>> +{
>> +    return pe->val[2] & VTD_SM_PASID_ENTRY_FSPTPTR;
>
>also introduce a define for FSPPTR using extract64((x)->val[2]

Will change like below:

--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -650,7 +650,7 @@ typedef struct VTDPIOTLBInvInfo {
 #define VTD_SM_PASID_ENTRY_AW          7ULL /* Adjusted guest-address-width */
 #define VTD_SM_PASID_ENTRY_DID(x)      extract64((x)->val[1], 0, 16)

-#define VTD_SM_PASID_ENTRY_FSPTPTR       (~0xfffULL)
+#define VTD_SM_PASID_ENTRY_FSPTPFN       extract64((x)->val[2], 12, 52)
 #define VTD_SM_PASID_ENTRY_SRE_BIT(x)    extract64((x)->val[2], 0, 1)
 /* 00: 4-level paging, 01: 5-level paging, 10-11: Reserved */
 #define VTD_SM_PASID_ENTRY_FSPM(x)       extract64((x)->val[2], 2, 2)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index a3a4a8a72b..f801458649 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -850,7 +851,7 @@ static inline bool vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)

 static inline dma_addr_t vtd_pe_get_fspt_base(VTDPASIDEntry *pe)
 {
-    return pe->val[2] & VTD_SM_PASID_ENTRY_FSPTPTR;
+    return VTD_SM_PASID_ENTRY_FSPTPFN << 12;
 }


>
>> +}
>> +
>> +/*
>> + * First stage IOVA address width: 48 bits for 4-level paging(FSPM=00)
>> + *                                 57 bits for 5-level
>paging(FSPM=01)
>> + */
>> +static inline uint32_t vtd_pe_get_fs_aw(VTDPASIDEntry *pe)
>> +{
>> +    return 48 + VTD_SM_PASID_ENTRY_FSPM(pe) * 9;
>can you add a comment refering to the spec here?

OK

>> +}
>> +
>> +static inline bool vtd_pe_pgtt_is_pt(VTDPASIDEntry *pe)
>> +{
>> +    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_PT);
>> +}
>> +
>> +/* check if pgtt is first stage translation */
>> +static inline bool vtd_pe_pgtt_is_fst(VTDPASIDEntry *pe)
>> +{
>> +    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_FST);
>> +}
>> +
>>  static inline bool vtd_pdire_present(VTDPASIDDirEntry *pdire)
>>  {
>>      return pdire->val & 1;
>> @@ -1625,7 +1649,7 @@ static uint16_t
>vtd_get_domain_id(IntelIOMMUState *s,
>>
>>      if (s->root_scalable) {
>>          vtd_ce_get_pasid_entry(s, ce, &pe, pasid);
>> -        return VTD_SM_PASID_ENTRY_DID(pe.val[1]);
>> +        return VTD_SM_PASID_ENTRY_DID(&pe);
>>      }
>>
>>      return VTD_CONTEXT_ENTRY_DID(ce->hi);
>> @@ -1707,7 +1731,7 @@ static bool
>vtd_dev_pt_enabled(IntelIOMMUState *s, VTDContextEntry *ce,
>>               */
>>              return false;
>>          }
>> -        return (VTD_PE_GET_TYPE(&pe) == VTD_SM_PASID_ENTRY_PT);
>> +        return vtd_pe_pgtt_is_pt(&pe);
>>      }
>>
>>      return (vtd_ce_get_type(ce) == VTD_CONTEXT_TT_PASS_THROUGH);
>> @@ -3146,9 +3170,9 @@ static void
>vtd_pasid_cache_sync_locked(gpointer key, gpointer value,
>>          /* Fall through */
>>      case VTD_INV_DESC_PASIDC_G_DSI:
>>          if (pc_entry->valid) {
>> -            did =
>VTD_SM_PASID_ENTRY_DID(pc_entry->pasid_entry.val[1]);
>> +            did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
>>          } else {
>> -            did = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
>> +            did = VTD_SM_PASID_ENTRY_DID(&pe);
>>          }
>>          if (pc_info->did != did) {
>>              return;
>> @@ -5267,7 +5291,7 @@ static int
>vtd_pri_perform_implicit_invalidation(VTDAddressSpace *vtd_as,
>>          return -EINVAL;
>>      }
>>      pgtt = VTD_PE_GET_TYPE(&pe);
>> -    domain_id = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
>> +    domain_id = VTD_SM_PASID_ENTRY_DID(&pe);
>>      ret = 0;
>>      switch (pgtt) {
>>      case VTD_SM_PASID_ENTRY_FST:
>Otherwise looks good to me
>
>Thanks
>
>Eric
Re: [PATCH v7 12/23] intel_iommu: Add some macros and inline functions
Posted by Eric Auger 3 months, 1 week ago 
Hi Zhenzhong,

On 11/3/25 4:44 AM, Duan, Zhenzhong wrote:
> Hi Eric,
>
> I showed the planed change inline, let me know if I misunderstood.
>
> Thanks
> Zhenzhong
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Subject: Re: [PATCH v7 12/23] intel_iommu: Add some macros and inline
>> functions
>>
>> Hi Zhenzhong,
>>
>> On 10/24/25 10:43 AM, Zhenzhong Duan wrote:
>>> Add some macros and inline functions that will be used by following
>>> patch.
>>>
>>> This patch also make a cleanup to change macro
>> VTD_SM_PASID_ENTRY_DID
>>> and VTD_SM_PASID_ENTRY_FSPM to use extract64() just like what smmu
>> does,
>>> because they are either used in following patches or used indirectly by
>>> new introduced inline functions. But we doesn't aim to change the huge
>>> amount of bit mask style macro definitions in this patch, that should be
>>> in a separate patch.
>>>
>>> Suggested-by: Eric Auger <eric.auger@redhat.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
>>> ---
>>>  hw/i386/intel_iommu_internal.h |  8 +++++--
>>>  hw/i386/intel_iommu.c          | 38
>> +++++++++++++++++++++++++++-------
>>>  2 files changed, 37 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/hw/i386/intel_iommu_internal.h
>> b/hw/i386/intel_iommu_internal.h
>>> index 09edba81e2..df80af839d 100644
>>> --- a/hw/i386/intel_iommu_internal.h
>>> +++ b/hw/i386/intel_iommu_internal.h
>>> @@ -642,10 +642,14 @@ typedef struct VTDPASIDCacheInfo {
>>>  #define VTD_SM_PASID_ENTRY_PT          (4ULL << 6)
>>>
>>>  #define VTD_SM_PASID_ENTRY_AW          7ULL /* Adjusted
>> guest-address-width */
>>> -#define VTD_SM_PASID_ENTRY_DID(val)    ((val) &
>> VTD_DOMAIN_ID_MASK)
>>> +#define VTD_SM_PASID_ENTRY_DID(x)      extract64((x)->val[1], 0, 16)
>>>
>>> -#define VTD_SM_PASID_ENTRY_FSPM          3ULL
>>>  #define VTD_SM_PASID_ENTRY_FSPTPTR       (~0xfffULL)
>>> +#define VTD_SM_PASID_ENTRY_SRE_BIT(x)    extract64((x)->val[2], 0, 1)
>>> +/* 00: 4-level paging, 01: 5-level paging, 10-11: Reserved */
>> the above comment is not that useful along with bitmask definition. I
>> would rather move it along with VTD_PE_GET_FS_LEVEL(pe)
> OK, will move it like below:
>
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -56,6 +56,7 @@
>
>  /* pe operations */
>  #define VTD_PE_GET_TYPE(pe) ((pe)->val[0] & VTD_SM_PASID_ENTRY_PGTT)
> +/* 4: 4-level paging, 5: 5-level paging */
I think I would rather put quote the spec
paging mode for first-stage translation. • 00: 4-level paging  • 01:
5-level paging

>  #define VTD_PE_GET_FS_LEVEL(pe) (VTD_SM_PASID_ENTRY_FSPM(pe) + 4)
>  #define VTD_PE_GET_SS_LEVEL(pe) \
>      (2 + (((pe)->val[0] >> 2) & VTD_SM_PASID_ENTRY_AW))
>
>
>>> +#define VTD_SM_PASID_ENTRY_FSPM(x)       extract64((x)->val[2], 2,
>> 2)
>>> +#define VTD_SM_PASID_ENTRY_WPE_BIT(x)    extract64((x)->val[2], 4,
>> 1)
>>> +#define VTD_SM_PASID_ENTRY_EAFE_BIT(x)   extract64((x)->val[2], 7, 1)
>> I would get rid of _BIT suffix to make it consistent with other fields.
> OK
>
>>>  /* First Stage Paging Structure */
>>>  /* Masks for First Stage Paging Entry */
>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>> index 56abbb991d..871e6aad19 100644
>>> --- a/hw/i386/intel_iommu.c
>>> +++ b/hw/i386/intel_iommu.c
>>> @@ -52,8 +52,7 @@
>>>
>>>  /* pe operations */
>>>  #define VTD_PE_GET_TYPE(pe) ((pe)->val[0] &
>> VTD_SM_PASID_ENTRY_PGTT)
>>> -#define VTD_PE_GET_FS_LEVEL(pe) \
>>> -    (4 + (((pe)->val[2] >> 2) & VTD_SM_PASID_ENTRY_FSPM))
>>> +#define VTD_PE_GET_FS_LEVEL(pe) (VTD_SM_PASID_ENTRY_FSPM(pe) +
>> 4)
>>>  #define VTD_PE_GET_SS_LEVEL(pe) \
>>>      (2 + (((pe)->val[0] >> 2) & VTD_SM_PASID_ENTRY_AW))
>>>
>>> @@ -837,6 +836,31 @@ static inline bool
>> vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
>>>      }
>>>  }
>>>
>>> +static inline dma_addr_t vtd_pe_get_fspt_base(VTDPASIDEntry *pe)
>>> +{
>>> +    return pe->val[2] & VTD_SM_PASID_ENTRY_FSPTPTR;
>> also introduce a define for FSPPTR using extract64((x)->val[2]
> Will change like below:
>
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -650,7 +650,7 @@ typedef struct VTDPIOTLBInvInfo {
>  #define VTD_SM_PASID_ENTRY_AW          7ULL /* Adjusted guest-address-width */
>  #define VTD_SM_PASID_ENTRY_DID(x)      extract64((x)->val[1], 0, 16)
>
> -#define VTD_SM_PASID_ENTRY_FSPTPTR       (~0xfffULL)
> +#define VTD_SM_PASID_ENTRY_FSPTPFN       extract64((x)->val[2], 12, 52)

VTD_SM_PASID_ENTRY_FSPTPFN(x)?

>  #define VTD_SM_PASID_ENTRY_SRE_BIT(x)    extract64((x)->val[2], 0, 1)
>  /* 00: 4-level paging, 01: 5-level paging, 10-11: Reserved */
>  #define VTD_SM_PASID_ENTRY_FSPM(x)       extract64((x)->val[2], 2, 2)
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index a3a4a8a72b..f801458649 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -850,7 +851,7 @@ static inline bool vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
>
>  static inline dma_addr_t vtd_pe_get_fspt_base(VTDPASIDEntry *pe)
>  {
> -    return pe->val[2] & VTD_SM_PASID_ENTRY_FSPTPTR;
> +    return VTD_SM_PASID_ENTRY_FSPTPFN << 12;
ditto
>  }
>
>
>>> +}
>>> +
>>> +/*
>>> + * First stage IOVA address width: 48 bits for 4-level paging(FSPM=00)
>>> + *                                 57 bits for 5-level
>> paging(FSPM=01)
>>> + */
>>> +static inline uint32_t vtd_pe_get_fs_aw(VTDPASIDEntry *pe)
>>> +{
>>> +    return 48 + VTD_SM_PASID_ENTRY_FSPM(pe) * 9;
>> can you add a comment refering to the spec here?
> OK
>
>>> +}
>>> +
>>> +static inline bool vtd_pe_pgtt_is_pt(VTDPASIDEntry *pe)
>>> +{
>>> +    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_PT);
>>> +}
>>> +
>>> +/* check if pgtt is first stage translation */
>>> +static inline bool vtd_pe_pgtt_is_fst(VTDPASIDEntry *pe)
>>> +{
>>> +    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_FST);
>>> +}
>>> +
>>>  static inline bool vtd_pdire_present(VTDPASIDDirEntry *pdire)
>>>  {
>>>      return pdire->val & 1;
>>> @@ -1625,7 +1649,7 @@ static uint16_t
>> vtd_get_domain_id(IntelIOMMUState *s,
>>>      if (s->root_scalable) {
>>>          vtd_ce_get_pasid_entry(s, ce, &pe, pasid);
>>> -        return VTD_SM_PASID_ENTRY_DID(pe.val[1]);
>>> +        return VTD_SM_PASID_ENTRY_DID(&pe);
>>>      }
>>>
>>>      return VTD_CONTEXT_ENTRY_DID(ce->hi);
>>> @@ -1707,7 +1731,7 @@ static bool
>> vtd_dev_pt_enabled(IntelIOMMUState *s, VTDContextEntry *ce,
>>>               */
>>>              return false;
>>>          }
>>> -        return (VTD_PE_GET_TYPE(&pe) == VTD_SM_PASID_ENTRY_PT);
>>> +        return vtd_pe_pgtt_is_pt(&pe);
>>>      }
>>>
>>>      return (vtd_ce_get_type(ce) == VTD_CONTEXT_TT_PASS_THROUGH);
>>> @@ -3146,9 +3170,9 @@ static void
>> vtd_pasid_cache_sync_locked(gpointer key, gpointer value,
>>>          /* Fall through */
>>>      case VTD_INV_DESC_PASIDC_G_DSI:
>>>          if (pc_entry->valid) {
>>> -            did =
>> VTD_SM_PASID_ENTRY_DID(pc_entry->pasid_entry.val[1]);
>>> +            did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
>>>          } else {
>>> -            did = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
>>> +            did = VTD_SM_PASID_ENTRY_DID(&pe);
>>>          }
>>>          if (pc_info->did != did) {
>>>              return;
>>> @@ -5267,7 +5291,7 @@ static int
>> vtd_pri_perform_implicit_invalidation(VTDAddressSpace *vtd_as,
>>>          return -EINVAL;
>>>      }
>>>      pgtt = VTD_PE_GET_TYPE(&pe);
>>> -    domain_id = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
>>> +    domain_id = VTD_SM_PASID_ENTRY_DID(&pe);
>>>      ret = 0;
>>>      switch (pgtt) {
>>>      case VTD_SM_PASID_ENTRY_FST:
>> Otherwise looks good to me
>>
>> Thanks
>>
>> Eric
RE: [PATCH v7 12/23] intel_iommu: Add some macros and inline functions
Posted by Duan, Zhenzhong 3 months ago 
Hi Eric,

All clear, thanks.
btw: I'm taking vacation this week, I'll reply your comments for other patches a little later.

BRs,
Zhenzhong

>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH v7 12/23] intel_iommu: Add some macros and inline
>functions
>
>Hi Zhenzhong,
>
>On 11/3/25 4:44 AM, Duan, Zhenzhong wrote:
>> Hi Eric,
>>
>> I showed the planed change inline, let me know if I misunderstood.
>>
>> Thanks
>> Zhenzhong
>>
>>> -----Original Message-----
>>> From: Eric Auger <eric.auger@redhat.com>
>>> Subject: Re: [PATCH v7 12/23] intel_iommu: Add some macros and inline
>>> functions
>>>
>>> Hi Zhenzhong,
>>>
>>> On 10/24/25 10:43 AM, Zhenzhong Duan wrote:
>>>> Add some macros and inline functions that will be used by following
>>>> patch.
>>>>
>>>> This patch also make a cleanup to change macro
>>> VTD_SM_PASID_ENTRY_DID
>>>> and VTD_SM_PASID_ENTRY_FSPM to use extract64() just like what smmu
>>> does,
>>>> because they are either used in following patches or used indirectly by
>>>> new introduced inline functions. But we doesn't aim to change the huge
>>>> amount of bit mask style macro definitions in this patch, that should be
>>>> in a separate patch.
>>>>
>>>> Suggested-by: Eric Auger <eric.auger@redhat.com>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>>> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
>>>> ---
>>>>  hw/i386/intel_iommu_internal.h |  8 +++++--
>>>>  hw/i386/intel_iommu.c          | 38
>>> +++++++++++++++++++++++++++-------
>>>>  2 files changed, 37 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/hw/i386/intel_iommu_internal.h
>>> b/hw/i386/intel_iommu_internal.h
>>>> index 09edba81e2..df80af839d 100644
>>>> --- a/hw/i386/intel_iommu_internal.h
>>>> +++ b/hw/i386/intel_iommu_internal.h
>>>> @@ -642,10 +642,14 @@ typedef struct VTDPASIDCacheInfo {
>>>>  #define VTD_SM_PASID_ENTRY_PT          (4ULL << 6)
>>>>
>>>>  #define VTD_SM_PASID_ENTRY_AW          7ULL /* Adjusted
>>> guest-address-width */
>>>> -#define VTD_SM_PASID_ENTRY_DID(val)    ((val) &
>>> VTD_DOMAIN_ID_MASK)
>>>> +#define VTD_SM_PASID_ENTRY_DID(x)      extract64((x)->val[1], 0,
>16)
>>>>
>>>> -#define VTD_SM_PASID_ENTRY_FSPM          3ULL
>>>>  #define VTD_SM_PASID_ENTRY_FSPTPTR       (~0xfffULL)
>>>> +#define VTD_SM_PASID_ENTRY_SRE_BIT(x)    extract64((x)->val[2], 0,
>1)
>>>> +/* 00: 4-level paging, 01: 5-level paging, 10-11: Reserved */
>>> the above comment is not that useful along with bitmask definition. I
>>> would rather move it along with VTD_PE_GET_FS_LEVEL(pe)
>> OK, will move it like below:
>>
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -56,6 +56,7 @@
>>
>>  /* pe operations */
>>  #define VTD_PE_GET_TYPE(pe) ((pe)->val[0] &
>VTD_SM_PASID_ENTRY_PGTT)
>> +/* 4: 4-level paging, 5: 5-level paging */
>I think I would rather put quote the spec
>paging mode for first-stage translation. • 00: 4-level paging  • 01:
>5-level paging
>
>>  #define VTD_PE_GET_FS_LEVEL(pe) (VTD_SM_PASID_ENTRY_FSPM(pe) +
>4)
>>  #define VTD_PE_GET_SS_LEVEL(pe) \
>>      (2 + (((pe)->val[0] >> 2) & VTD_SM_PASID_ENTRY_AW))
>>
>>
>>>> +#define VTD_SM_PASID_ENTRY_FSPM(x)       extract64((x)->val[2],
>2,
>>> 2)
>>>> +#define VTD_SM_PASID_ENTRY_WPE_BIT(x)    extract64((x)->val[2],
>4,
>>> 1)
>>>> +#define VTD_SM_PASID_ENTRY_EAFE_BIT(x)   extract64((x)->val[2], 7,
>1)
>>> I would get rid of _BIT suffix to make it consistent with other fields.
>> OK
>>
>>>>  /* First Stage Paging Structure */
>>>>  /* Masks for First Stage Paging Entry */
>>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>>> index 56abbb991d..871e6aad19 100644
>>>> --- a/hw/i386/intel_iommu.c
>>>> +++ b/hw/i386/intel_iommu.c
>>>> @@ -52,8 +52,7 @@
>>>>
>>>>  /* pe operations */
>>>>  #define VTD_PE_GET_TYPE(pe) ((pe)->val[0] &
>>> VTD_SM_PASID_ENTRY_PGTT)
>>>> -#define VTD_PE_GET_FS_LEVEL(pe) \
>>>> -    (4 + (((pe)->val[2] >> 2) & VTD_SM_PASID_ENTRY_FSPM))
>>>> +#define VTD_PE_GET_FS_LEVEL(pe) (VTD_SM_PASID_ENTRY_FSPM(pe)
>+
>>> 4)
>>>>  #define VTD_PE_GET_SS_LEVEL(pe) \
>>>>      (2 + (((pe)->val[0] >> 2) & VTD_SM_PASID_ENTRY_AW))
>>>>
>>>> @@ -837,6 +836,31 @@ static inline bool
>>> vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
>>>>      }
>>>>  }
>>>>
>>>> +static inline dma_addr_t vtd_pe_get_fspt_base(VTDPASIDEntry *pe)
>>>> +{
>>>> +    return pe->val[2] & VTD_SM_PASID_ENTRY_FSPTPTR;
>>> also introduce a define for FSPPTR using extract64((x)->val[2]
>> Will change like below:
>>
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -650,7 +650,7 @@ typedef struct VTDPIOTLBInvInfo {
>>  #define VTD_SM_PASID_ENTRY_AW          7ULL /* Adjusted
>guest-address-width */
>>  #define VTD_SM_PASID_ENTRY_DID(x)      extract64((x)->val[1], 0, 16)
>>
>> -#define VTD_SM_PASID_ENTRY_FSPTPTR       (~0xfffULL)
>> +#define VTD_SM_PASID_ENTRY_FSPTPFN       extract64((x)->val[2], 12,
>52)
>
>VTD_SM_PASID_ENTRY_FSPTPFN(x)?
>
>>  #define VTD_SM_PASID_ENTRY_SRE_BIT(x)    extract64((x)->val[2], 0,
>1)
>>  /* 00: 4-level paging, 01: 5-level paging, 10-11: Reserved */
>>  #define VTD_SM_PASID_ENTRY_FSPM(x)       extract64((x)->val[2], 2,
>2)
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index a3a4a8a72b..f801458649 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -850,7 +851,7 @@ static inline bool
>vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
>>
>>  static inline dma_addr_t vtd_pe_get_fspt_base(VTDPASIDEntry *pe)
>>  {
>> -    return pe->val[2] & VTD_SM_PASID_ENTRY_FSPTPTR;
>> +    return VTD_SM_PASID_ENTRY_FSPTPFN << 12;
>ditto
>>  }
>>
>>
>>>> +}
>>>> +
>>>> +/*
>>>> + * First stage IOVA address width: 48 bits for 4-level paging(FSPM=00)
>>>> + *                                 57 bits for 5-level
>>> paging(FSPM=01)
>>>> + */
>>>> +static inline uint32_t vtd_pe_get_fs_aw(VTDPASIDEntry *pe)
>>>> +{
>>>> +    return 48 + VTD_SM_PASID_ENTRY_FSPM(pe) * 9;
>>> can you add a comment refering to the spec here?
>> OK
>>
>>>> +}
>>>> +
>>>> +static inline bool vtd_pe_pgtt_is_pt(VTDPASIDEntry *pe)
>>>> +{
>>>> +    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_PT);
>>>> +}
>>>> +
>>>> +/* check if pgtt is first stage translation */
>>>> +static inline bool vtd_pe_pgtt_is_fst(VTDPASIDEntry *pe)
>>>> +{
>>>> +    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_FST);
>>>> +}
>>>> +
>>>>  static inline bool vtd_pdire_present(VTDPASIDDirEntry *pdire)
>>>>  {
>>>>      return pdire->val & 1;
>>>> @@ -1625,7 +1649,7 @@ static uint16_t
>>> vtd_get_domain_id(IntelIOMMUState *s,
>>>>      if (s->root_scalable) {
>>>>          vtd_ce_get_pasid_entry(s, ce, &pe, pasid);
>>>> -        return VTD_SM_PASID_ENTRY_DID(pe.val[1]);
>>>> +        return VTD_SM_PASID_ENTRY_DID(&pe);
>>>>      }
>>>>
>>>>      return VTD_CONTEXT_ENTRY_DID(ce->hi);
>>>> @@ -1707,7 +1731,7 @@ static bool
>>> vtd_dev_pt_enabled(IntelIOMMUState *s, VTDContextEntry *ce,
>>>>               */
>>>>              return false;
>>>>          }
>>>> -        return (VTD_PE_GET_TYPE(&pe) ==
>VTD_SM_PASID_ENTRY_PT);
>>>> +        return vtd_pe_pgtt_is_pt(&pe);
>>>>      }
>>>>
>>>>      return (vtd_ce_get_type(ce) ==
>VTD_CONTEXT_TT_PASS_THROUGH);
>>>> @@ -3146,9 +3170,9 @@ static void
>>> vtd_pasid_cache_sync_locked(gpointer key, gpointer value,
>>>>          /* Fall through */
>>>>      case VTD_INV_DESC_PASIDC_G_DSI:
>>>>          if (pc_entry->valid) {
>>>> -            did =
>>> VTD_SM_PASID_ENTRY_DID(pc_entry->pasid_entry.val[1]);
>>>> +            did =
>VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
>>>>          } else {
>>>> -            did = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
>>>> +            did = VTD_SM_PASID_ENTRY_DID(&pe);
>>>>          }
>>>>          if (pc_info->did != did) {
>>>>              return;
>>>> @@ -5267,7 +5291,7 @@ static int
>>> vtd_pri_perform_implicit_invalidation(VTDAddressSpace *vtd_as,
>>>>          return -EINVAL;
>>>>      }
>>>>      pgtt = VTD_PE_GET_TYPE(&pe);
>>>> -    domain_id = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
>>>> +    domain_id = VTD_SM_PASID_ENTRY_DID(&pe);
>>>>      ret = 0;
>>>>      switch (pgtt) {
>>>>      case VTD_SM_PASID_ENTRY_FST:
>>> Otherwise looks good to me
>>>
>>> Thanks
>>>
>>> Eric
Re: [PATCH v7 12/23] intel_iommu: Add some macros and inline functions
Posted by Cédric Le Goater 3 months, 2 weeks ago 
On 10/24/25 10:43, Zhenzhong Duan wrote:
> Add some macros and inline functions that will be used by following
> patch.
> 
> This patch also make a cleanup to change macro VTD_SM_PASID_ENTRY_DID
> and VTD_SM_PASID_ENTRY_FSPM to use extract64() just like what smmu does,
> because they are either used in following patches or used indirectly by
> new introduced inline functions. But we doesn't aim to change the huge
> amount of bit mask style macro definitions in this patch, that should be
> in a separate patch.
> 
> Suggested-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
> ---
>   hw/i386/intel_iommu_internal.h |  8 +++++--
>   hw/i386/intel_iommu.c          | 38 +++++++++++++++++++++++++++-------
>   2 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 09edba81e2..df80af839d 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -642,10 +642,14 @@ typedef struct VTDPASIDCacheInfo {
>   #define VTD_SM_PASID_ENTRY_PT          (4ULL << 6)
>   
>   #define VTD_SM_PASID_ENTRY_AW          7ULL /* Adjusted guest-address-width */
> -#define VTD_SM_PASID_ENTRY_DID(val)    ((val) & VTD_DOMAIN_ID_MASK)
> +#define VTD_SM_PASID_ENTRY_DID(x)      extract64((x)->val[1], 0, 16)
>   
> -#define VTD_SM_PASID_ENTRY_FSPM          3ULL
>   #define VTD_SM_PASID_ENTRY_FSPTPTR       (~0xfffULL)
> +#define VTD_SM_PASID_ENTRY_SRE_BIT(x)    extract64((x)->val[2], 0, 1)
> +/* 00: 4-level paging, 01: 5-level paging, 10-11: Reserved */
> +#define VTD_SM_PASID_ENTRY_FSPM(x)       extract64((x)->val[2], 2, 2)
> +#define VTD_SM_PASID_ENTRY_WPE_BIT(x)    extract64((x)->val[2], 4, 1)
> +#define VTD_SM_PASID_ENTRY_EAFE_BIT(x)   extract64((x)->val[2], 7, 1)
>   
>   /* First Stage Paging Structure */
>   /* Masks for First Stage Paging Entry */
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 56abbb991d..871e6aad19 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -52,8 +52,7 @@
>   
>   /* pe operations */
>   #define VTD_PE_GET_TYPE(pe) ((pe)->val[0] & VTD_SM_PASID_ENTRY_PGTT)
> -#define VTD_PE_GET_FS_LEVEL(pe) \
> -    (4 + (((pe)->val[2] >> 2) & VTD_SM_PASID_ENTRY_FSPM))
> +#define VTD_PE_GET_FS_LEVEL(pe) (VTD_SM_PASID_ENTRY_FSPM(pe) + 4)
>   #define VTD_PE_GET_SS_LEVEL(pe) \
>       (2 + (((pe)->val[0] >> 2) & VTD_SM_PASID_ENTRY_AW))
>   
> @@ -837,6 +836,31 @@ static inline bool vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
>       }
>   }
>   
> +static inline dma_addr_t vtd_pe_get_fspt_base(VTDPASIDEntry *pe)
> +{
> +    return pe->val[2] & VTD_SM_PASID_ENTRY_FSPTPTR;
> +}
> +
> +/*
> + * First stage IOVA address width: 48 bits for 4-level paging(FSPM=00)
> + *                                 57 bits for 5-level paging(FSPM=01)
> + */
> +static inline uint32_t vtd_pe_get_fs_aw(VTDPASIDEntry *pe)
> +{
> +    return 48 + VTD_SM_PASID_ENTRY_FSPM(pe) * 9;


Can't we use VTD_HOST_AW_48BIT here ?


Thanks,

C.


> +}
> +
> +static inline bool vtd_pe_pgtt_is_pt(VTDPASIDEntry *pe)
> +{
> +    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_PT);
> +}
> +
> +/* check if pgtt is first stage translation */
> +static inline bool vtd_pe_pgtt_is_fst(VTDPASIDEntry *pe)
> +{
> +    return (VTD_PE_GET_TYPE(pe) == VTD_SM_PASID_ENTRY_FST);
> +}
> +
>   static inline bool vtd_pdire_present(VTDPASIDDirEntry *pdire)
>   {
>       return pdire->val & 1;
> @@ -1625,7 +1649,7 @@ static uint16_t vtd_get_domain_id(IntelIOMMUState *s,
>   
>       if (s->root_scalable) {
>           vtd_ce_get_pasid_entry(s, ce, &pe, pasid);
> -        return VTD_SM_PASID_ENTRY_DID(pe.val[1]);
> +        return VTD_SM_PASID_ENTRY_DID(&pe);
>       }
>   
>       return VTD_CONTEXT_ENTRY_DID(ce->hi);
> @@ -1707,7 +1731,7 @@ static bool vtd_dev_pt_enabled(IntelIOMMUState *s, VTDContextEntry *ce,
>                */
>               return false;
>           }
> -        return (VTD_PE_GET_TYPE(&pe) == VTD_SM_PASID_ENTRY_PT);
> +        return vtd_pe_pgtt_is_pt(&pe);
>       }
>   
>       return (vtd_ce_get_type(ce) == VTD_CONTEXT_TT_PASS_THROUGH);
> @@ -3146,9 +3170,9 @@ static void vtd_pasid_cache_sync_locked(gpointer key, gpointer value,
>           /* Fall through */
>       case VTD_INV_DESC_PASIDC_G_DSI:
>           if (pc_entry->valid) {
> -            did = VTD_SM_PASID_ENTRY_DID(pc_entry->pasid_entry.val[1]);
> +            did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
>           } else {
> -            did = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
> +            did = VTD_SM_PASID_ENTRY_DID(&pe);
>           }
>           if (pc_info->did != did) {
>               return;
> @@ -5267,7 +5291,7 @@ static int vtd_pri_perform_implicit_invalidation(VTDAddressSpace *vtd_as,
>           return -EINVAL;
>       }
>       pgtt = VTD_PE_GET_TYPE(&pe);
> -    domain_id = VTD_SM_PASID_ENTRY_DID(pe.val[1]);
> +    domain_id = VTD_SM_PASID_ENTRY_DID(&pe);
>       ret = 0;
>       switch (pgtt) {
>       case VTD_SM_PASID_ENTRY_FST:
RE: [PATCH v7 12/23] intel_iommu: Add some macros and inline functions
Posted by Duan, Zhenzhong 3 months, 2 weeks ago 

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH v7 12/23] intel_iommu: Add some macros and inline
>functions
>
>On 10/24/25 10:43, Zhenzhong Duan wrote:
>> Add some macros and inline functions that will be used by following
>> patch.
>>
>> This patch also make a cleanup to change macro
>VTD_SM_PASID_ENTRY_DID
>> and VTD_SM_PASID_ENTRY_FSPM to use extract64() just like what smmu
>does,
>> because they are either used in following patches or used indirectly by
>> new introduced inline functions. But we doesn't aim to change the huge
>> amount of bit mask style macro definitions in this patch, that should be
>> in a separate patch.
>>
>> Suggested-by: Eric Auger <eric.auger@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>>   hw/i386/intel_iommu_internal.h |  8 +++++--
>>   hw/i386/intel_iommu.c          | 38
>+++++++++++++++++++++++++++-------
>>   2 files changed, 37 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h
>b/hw/i386/intel_iommu_internal.h
>> index 09edba81e2..df80af839d 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -642,10 +642,14 @@ typedef struct VTDPASIDCacheInfo {
>>   #define VTD_SM_PASID_ENTRY_PT          (4ULL << 6)
>>
>>   #define VTD_SM_PASID_ENTRY_AW          7ULL /* Adjusted
>guest-address-width */
>> -#define VTD_SM_PASID_ENTRY_DID(val)    ((val) &
>VTD_DOMAIN_ID_MASK)
>> +#define VTD_SM_PASID_ENTRY_DID(x)      extract64((x)->val[1], 0, 16)
>>
>> -#define VTD_SM_PASID_ENTRY_FSPM          3ULL
>>   #define VTD_SM_PASID_ENTRY_FSPTPTR       (~0xfffULL)
>> +#define VTD_SM_PASID_ENTRY_SRE_BIT(x)    extract64((x)->val[2], 0, 1)
>> +/* 00: 4-level paging, 01: 5-level paging, 10-11: Reserved */
>> +#define VTD_SM_PASID_ENTRY_FSPM(x)       extract64((x)->val[2], 2,
>2)
>> +#define VTD_SM_PASID_ENTRY_WPE_BIT(x)    extract64((x)->val[2], 4,
>1)
>> +#define VTD_SM_PASID_ENTRY_EAFE_BIT(x)   extract64((x)->val[2], 7, 1)
>>
>>   /* First Stage Paging Structure */
>>   /* Masks for First Stage Paging Entry */
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 56abbb991d..871e6aad19 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -52,8 +52,7 @@
>>
>>   /* pe operations */
>>   #define VTD_PE_GET_TYPE(pe) ((pe)->val[0] &
>VTD_SM_PASID_ENTRY_PGTT)
>> -#define VTD_PE_GET_FS_LEVEL(pe) \
>> -    (4 + (((pe)->val[2] >> 2) & VTD_SM_PASID_ENTRY_FSPM))
>> +#define VTD_PE_GET_FS_LEVEL(pe) (VTD_SM_PASID_ENTRY_FSPM(pe) +
>4)
>>   #define VTD_PE_GET_SS_LEVEL(pe) \
>>       (2 + (((pe)->val[0] >> 2) & VTD_SM_PASID_ENTRY_AW))
>>
>> @@ -837,6 +836,31 @@ static inline bool
>vtd_pe_type_check(IntelIOMMUState *s, VTDPASIDEntry *pe)
>>       }
>>   }
>>
>> +static inline dma_addr_t vtd_pe_get_fspt_base(VTDPASIDEntry *pe)
>> +{
>> +    return pe->val[2] & VTD_SM_PASID_ENTRY_FSPTPTR;
>> +}
>> +
>> +/*
>> + * First stage IOVA address width: 48 bits for 4-level paging(FSPM=00)
>> + *                                 57 bits for 5-level
>paging(FSPM=01)
>> + */
>> +static inline uint32_t vtd_pe_get_fs_aw(VTDPASIDEntry *pe)
>> +{
>> +    return 48 + VTD_SM_PASID_ENTRY_FSPM(pe) * 9;
>
>
>Can't we use VTD_HOST_AW_48BIT here ?

Will do.

Thanks
Zhenzhong

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.