Per spec 6.5.2.4, PADID-selective PASID-based iotlb invalidation will
flush stage-2 iotlb entries with matching domain id and pasid.
With scalable modern mode introduced, guest could send PASID-selective
PASID-based iotlb invalidation to flush both stage-1 and stage-2 entries.
By this chance, remove old IOTLB related definition.
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
hw/i386/intel_iommu_internal.h | 14 +++---
hw/i386/intel_iommu.c | 81 ++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+), 5 deletions(-)
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 8fa27c7f3b..19e4ed52ca 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -402,11 +402,6 @@ typedef union VTDInvDesc VTDInvDesc;
#define VTD_INV_DESC_IOTLB_AM(val) ((val) & 0x3fULL)
#define VTD_INV_DESC_IOTLB_RSVD_LO 0xffffffff0000ff00ULL
#define VTD_INV_DESC_IOTLB_RSVD_HI 0xf80ULL
-#define VTD_INV_DESC_IOTLB_PASID_PASID (2ULL << 4)
-#define VTD_INV_DESC_IOTLB_PASID_PAGE (3ULL << 4)
-#define VTD_INV_DESC_IOTLB_PASID(val) (((val) >> 32) & VTD_PASID_ID_MASK)
-#define VTD_INV_DESC_IOTLB_PASID_RSVD_LO 0xfff00000000001c0ULL
-#define VTD_INV_DESC_IOTLB_PASID_RSVD_HI 0xf80ULL
/* Mask for Device IOTLB Invalidate Descriptor */
#define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) & 0xfffffffffffff000ULL)
@@ -438,6 +433,15 @@ typedef union VTDInvDesc VTDInvDesc;
(0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
(0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
+/* Masks for PIOTLB Invalidate Descriptor */
+#define VTD_INV_DESC_PIOTLB_G (3ULL << 4)
+#define VTD_INV_DESC_PIOTLB_ALL_IN_PASID (2ULL << 4)
+#define VTD_INV_DESC_PIOTLB_PSI_IN_PASID (3ULL << 4)
+#define VTD_INV_DESC_PIOTLB_DID(val) (((val) >> 16) & VTD_DOMAIN_ID_MASK)
+#define VTD_INV_DESC_PIOTLB_PASID(val) (((val) >> 32) & 0xfffffULL)
+#define VTD_INV_DESC_PIOTLB_RSVD_VAL0 0xfff000000000f1c0ULL
+#define VTD_INV_DESC_PIOTLB_RSVD_VAL1 0xf80ULL
+
/* Information about page-selective IOTLB invalidate */
struct VTDIOTLBPageInvInfo {
uint16_t domain_id;
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index c1382a5651..df591419b7 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2656,6 +2656,83 @@ static bool vtd_process_iotlb_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
return true;
}
+static gboolean vtd_hash_remove_by_pasid(gpointer key, gpointer value,
+ gpointer user_data)
+{
+ VTDIOTLBEntry *entry = (VTDIOTLBEntry *)value;
+ VTDIOTLBPageInvInfo *info = (VTDIOTLBPageInvInfo *)user_data;
+
+ return ((entry->domain_id == info->domain_id) &&
+ (entry->pasid == info->pasid));
+}
+
+static void vtd_piotlb_pasid_invalidate(IntelIOMMUState *s,
+ uint16_t domain_id, uint32_t pasid)
+{
+ VTDIOTLBPageInvInfo info;
+ VTDAddressSpace *vtd_as;
+ VTDContextEntry ce;
+
+ info.domain_id = domain_id;
+ info.pasid = pasid;
+
+ vtd_iommu_lock(s);
+ g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_pasid,
+ &info);
+ vtd_iommu_unlock(s);
+
+ QLIST_FOREACH(vtd_as, &s->vtd_as_with_notifiers, next) {
+ if (!vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
+ vtd_as->devfn, &ce) &&
+ domain_id == vtd_get_domain_id(s, &ce, vtd_as->pasid)) {
+ uint32_t rid2pasid = VTD_CE_GET_RID2PASID(&ce);
+
+ if ((vtd_as->pasid != PCI_NO_PASID || pasid != rid2pasid) &&
+ vtd_as->pasid != pasid) {
+ continue;
+ }
+
+ if (!s->scalable_modern) {
+ vtd_address_space_sync(vtd_as);
+ }
+ }
+ }
+}
+
+static bool vtd_process_piotlb_desc(IntelIOMMUState *s,
+ VTDInvDesc *inv_desc)
+{
+ uint16_t domain_id;
+ uint32_t pasid;
+
+ if ((inv_desc->val[0] & VTD_INV_DESC_PIOTLB_RSVD_VAL0) ||
+ (inv_desc->val[1] & VTD_INV_DESC_PIOTLB_RSVD_VAL1)) {
+ error_report_once("%s: invalid piotlb inv desc hi=0x%"PRIx64
+ " lo=0x%"PRIx64" (reserved bits unzero)",
+ __func__, inv_desc->val[1], inv_desc->val[0]);
+ return false;
+ }
+
+ domain_id = VTD_INV_DESC_PIOTLB_DID(inv_desc->val[0]);
+ pasid = VTD_INV_DESC_PIOTLB_PASID(inv_desc->val[0]);
+ switch (inv_desc->val[0] & VTD_INV_DESC_PIOTLB_G) {
+ case VTD_INV_DESC_PIOTLB_ALL_IN_PASID:
+ vtd_piotlb_pasid_invalidate(s, domain_id, pasid);
+ break;
+
+ case VTD_INV_DESC_PIOTLB_PSI_IN_PASID:
+ break;
+
+ default:
+ error_report_once("%s: invalid piotlb inv desc: hi=0x%"PRIx64
+ ", lo=0x%"PRIx64" (type mismatch: 0x%llx)",
+ __func__, inv_desc->val[1], inv_desc->val[0],
+ inv_desc->val[0] & VTD_INV_DESC_IOTLB_G);
+ return false;
+ }
+ return true;
+}
+
static bool vtd_process_inv_iec_desc(IntelIOMMUState *s,
VTDInvDesc *inv_desc)
{
@@ -2775,6 +2852,10 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
break;
case VTD_INV_DESC_PIOTLB:
+ trace_vtd_inv_desc("p-iotlb", inv_desc.val[1], inv_desc.val[0]);
+ if (!vtd_process_piotlb_desc(s, &inv_desc)) {
+ return false;
+ }
break;
case VTD_INV_DESC_WAIT:
--
2.34.1On 2024/8/5 14:27, Zhenzhong Duan wrote:
> Per spec 6.5.2.4, PADID-selective PASID-based iotlb invalidation will
> flush stage-2 iotlb entries with matching domain id and pasid.
>
> With scalable modern mode introduced, guest could send PASID-selective
> PASID-based iotlb invalidation to flush both stage-1 and stage-2 entries.
I'm not quite sure if this is correct. In the last collumn of the table 21
in 6.5.2.4, the paging structures of SS will not be invalidated. So it's
not quite recommended for software to invalidate the iotlb entries with
PGTT==SS-only by P_IOTLB invalidation, it's more recommended to use the
IOTLB invalidation.
> By this chance, remove old IOTLB related definition.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
> hw/i386/intel_iommu_internal.h | 14 +++---
> hw/i386/intel_iommu.c | 81 ++++++++++++++++++++++++++++++++++
> 2 files changed, 90 insertions(+), 5 deletions(-)
>
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 8fa27c7f3b..19e4ed52ca 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -402,11 +402,6 @@ typedef union VTDInvDesc VTDInvDesc;
> #define VTD_INV_DESC_IOTLB_AM(val) ((val) & 0x3fULL)
> #define VTD_INV_DESC_IOTLB_RSVD_LO 0xffffffff0000ff00ULL
> #define VTD_INV_DESC_IOTLB_RSVD_HI 0xf80ULL
> -#define VTD_INV_DESC_IOTLB_PASID_PASID (2ULL << 4)
> -#define VTD_INV_DESC_IOTLB_PASID_PAGE (3ULL << 4)
> -#define VTD_INV_DESC_IOTLB_PASID(val) (((val) >> 32) & VTD_PASID_ID_MASK)
> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_LO 0xfff00000000001c0ULL
> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_HI 0xf80ULL
>
> /* Mask for Device IOTLB Invalidate Descriptor */
> #define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) & 0xfffffffffffff000ULL)
> @@ -438,6 +433,15 @@ typedef union VTDInvDesc VTDInvDesc;
> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>
> +/* Masks for PIOTLB Invalidate Descriptor */
> +#define VTD_INV_DESC_PIOTLB_G (3ULL << 4)
> +#define VTD_INV_DESC_PIOTLB_ALL_IN_PASID (2ULL << 4)
> +#define VTD_INV_DESC_PIOTLB_PSI_IN_PASID (3ULL << 4)
> +#define VTD_INV_DESC_PIOTLB_DID(val) (((val) >> 16) & VTD_DOMAIN_ID_MASK)
> +#define VTD_INV_DESC_PIOTLB_PASID(val) (((val) >> 32) & 0xfffffULL)
> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL0 0xfff000000000f1c0ULL
> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL1 0xf80ULL
> +
> /* Information about page-selective IOTLB invalidate */
> struct VTDIOTLBPageInvInfo {
> uint16_t domain_id;
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index c1382a5651..df591419b7 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2656,6 +2656,83 @@ static bool vtd_process_iotlb_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
> return true;
> }
>
> +static gboolean vtd_hash_remove_by_pasid(gpointer key, gpointer value,
> + gpointer user_data)
> +{
> + VTDIOTLBEntry *entry = (VTDIOTLBEntry *)value;
> + VTDIOTLBPageInvInfo *info = (VTDIOTLBPageInvInfo *)user_data;
> +
> + return ((entry->domain_id == info->domain_id) &&
> + (entry->pasid == info->pasid));
> +}
> +
> +static void vtd_piotlb_pasid_invalidate(IntelIOMMUState *s,
> + uint16_t domain_id, uint32_t pasid)
> +{
> + VTDIOTLBPageInvInfo info;
> + VTDAddressSpace *vtd_as;
> + VTDContextEntry ce;
> +
> + info.domain_id = domain_id;
> + info.pasid = pasid;
> +
> + vtd_iommu_lock(s);
> + g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_pasid,
> + &info);
> + vtd_iommu_unlock(s);
> +
> + QLIST_FOREACH(vtd_as, &s->vtd_as_with_notifiers, next) {
> + if (!vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
> + vtd_as->devfn, &ce) &&
> + domain_id == vtd_get_domain_id(s, &ce, vtd_as->pasid)) {
> + uint32_t rid2pasid = VTD_CE_GET_RID2PASID(&ce);
> +
> + if ((vtd_as->pasid != PCI_NO_PASID || pasid != rid2pasid) &&
> + vtd_as->pasid != pasid) {
> + continue;
> + }
> +
> + if (!s->scalable_modern) {
> + vtd_address_space_sync(vtd_as);
> + }
> + }
> + }
> +}
> +
> +static bool vtd_process_piotlb_desc(IntelIOMMUState *s,
> + VTDInvDesc *inv_desc)
> +{
> + uint16_t domain_id;
> + uint32_t pasid;
> +
> + if ((inv_desc->val[0] & VTD_INV_DESC_PIOTLB_RSVD_VAL0) ||
> + (inv_desc->val[1] & VTD_INV_DESC_PIOTLB_RSVD_VAL1)) {
> + error_report_once("%s: invalid piotlb inv desc hi=0x%"PRIx64
> + " lo=0x%"PRIx64" (reserved bits unzero)",
> + __func__, inv_desc->val[1], inv_desc->val[0]);
> + return false;
> + }
> +
> + domain_id = VTD_INV_DESC_PIOTLB_DID(inv_desc->val[0]);
> + pasid = VTD_INV_DESC_PIOTLB_PASID(inv_desc->val[0]);
> + switch (inv_desc->val[0] & VTD_INV_DESC_PIOTLB_G) {
> + case VTD_INV_DESC_PIOTLB_ALL_IN_PASID:
> + vtd_piotlb_pasid_invalidate(s, domain_id, pasid);
> + break;
> +
> + case VTD_INV_DESC_PIOTLB_PSI_IN_PASID:
> + break;
> +
> + default:
> + error_report_once("%s: invalid piotlb inv desc: hi=0x%"PRIx64
> + ", lo=0x%"PRIx64" (type mismatch: 0x%llx)",
> + __func__, inv_desc->val[1], inv_desc->val[0],
> + inv_desc->val[0] & VTD_INV_DESC_IOTLB_G);
> + return false;
> + }
> + return true;
> +}
> +
> static bool vtd_process_inv_iec_desc(IntelIOMMUState *s,
> VTDInvDesc *inv_desc)
> {
> @@ -2775,6 +2852,10 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
> break;
>
> case VTD_INV_DESC_PIOTLB:
> + trace_vtd_inv_desc("p-iotlb", inv_desc.val[1], inv_desc.val[0]);
> + if (!vtd_process_piotlb_desc(s, &inv_desc)) {
> + return false;
> + }
> break;
>
> case VTD_INV_DESC_WAIT:
--
Regards,
Yi Liu
>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v2 04/17] intel_iommu: Flush stage-2 cache in PASID-
>selective PASID-based iotlb invalidation
>
>On 2024/8/5 14:27, Zhenzhong Duan wrote:
>> Per spec 6.5.2.4, PADID-selective PASID-based iotlb invalidation will
>> flush stage-2 iotlb entries with matching domain id and pasid.
>>
>> With scalable modern mode introduced, guest could send PASID-selective
>> PASID-based iotlb invalidation to flush both stage-1 and stage-2 entries.
>
>I'm not quite sure if this is correct. In the last collumn of the table 21
>in 6.5.2.4, the paging structures of SS will not be invalidated. So it's
>not quite recommended for software to invalidate the iotlb entries with
>PGTT==SS-only by P_IOTLB invalidation, it's more recommended to use the
>IOTLB invalidation.
Hmm, when pasid is used with SS-only, PASID-based iotlb invalidation can give better granularity, (DID,PASID) vs. (DID) for IOTLB invalidation.
If non-leaf SS-paging entry is updated, IOTLB invalidation should be used as SS-paging structure cache isn't flushed with PASID-selective PASID-based iotlb invalidation.
Thanks
Zhenzhong
>
>> By this chance, remove old IOTLB related definition.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>> hw/i386/intel_iommu_internal.h | 14 +++---
>> hw/i386/intel_iommu.c | 81
>++++++++++++++++++++++++++++++++++
>> 2 files changed, 90 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h
>b/hw/i386/intel_iommu_internal.h
>> index 8fa27c7f3b..19e4ed52ca 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -402,11 +402,6 @@ typedef union VTDInvDesc VTDInvDesc;
>> #define VTD_INV_DESC_IOTLB_AM(val) ((val) & 0x3fULL)
>> #define VTD_INV_DESC_IOTLB_RSVD_LO 0xffffffff0000ff00ULL
>> #define VTD_INV_DESC_IOTLB_RSVD_HI 0xf80ULL
>> -#define VTD_INV_DESC_IOTLB_PASID_PASID (2ULL << 4)
>> -#define VTD_INV_DESC_IOTLB_PASID_PAGE (3ULL << 4)
>> -#define VTD_INV_DESC_IOTLB_PASID(val) (((val) >> 32) &
>VTD_PASID_ID_MASK)
>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_LO
>0xfff00000000001c0ULL
>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_HI 0xf80ULL
>>
>> /* Mask for Device IOTLB Invalidate Descriptor */
>> #define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) &
>0xfffffffffffff000ULL)
>> @@ -438,6 +433,15 @@ typedef union VTDInvDesc VTDInvDesc;
>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM |
>VTD_SL_TM)) : \
>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>>
>> +/* Masks for PIOTLB Invalidate Descriptor */
>> +#define VTD_INV_DESC_PIOTLB_G (3ULL << 4)
>> +#define VTD_INV_DESC_PIOTLB_ALL_IN_PASID (2ULL << 4)
>> +#define VTD_INV_DESC_PIOTLB_PSI_IN_PASID (3ULL << 4)
>> +#define VTD_INV_DESC_PIOTLB_DID(val) (((val) >> 16) &
>VTD_DOMAIN_ID_MASK)
>> +#define VTD_INV_DESC_PIOTLB_PASID(val) (((val) >> 32) & 0xfffffULL)
>> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL0 0xfff000000000f1c0ULL
>> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL1 0xf80ULL
>> +
>> /* Information about page-selective IOTLB invalidate */
>> struct VTDIOTLBPageInvInfo {
>> uint16_t domain_id;
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index c1382a5651..df591419b7 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -2656,6 +2656,83 @@ static bool
>vtd_process_iotlb_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
>> return true;
>> }
>>
>> +static gboolean vtd_hash_remove_by_pasid(gpointer key, gpointer value,
>> + gpointer user_data)
>> +{
>> + VTDIOTLBEntry *entry = (VTDIOTLBEntry *)value;
>> + VTDIOTLBPageInvInfo *info = (VTDIOTLBPageInvInfo *)user_data;
>> +
>> + return ((entry->domain_id == info->domain_id) &&
>> + (entry->pasid == info->pasid));
>> +}
>> +
>> +static void vtd_piotlb_pasid_invalidate(IntelIOMMUState *s,
>> + uint16_t domain_id, uint32_t pasid)
>> +{
>> + VTDIOTLBPageInvInfo info;
>> + VTDAddressSpace *vtd_as;
>> + VTDContextEntry ce;
>> +
>> + info.domain_id = domain_id;
>> + info.pasid = pasid;
>> +
>> + vtd_iommu_lock(s);
>> + g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_pasid,
>> + &info);
>> + vtd_iommu_unlock(s);
>> +
>> + QLIST_FOREACH(vtd_as, &s->vtd_as_with_notifiers, next) {
>> + if (!vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
>> + vtd_as->devfn, &ce) &&
>> + domain_id == vtd_get_domain_id(s, &ce, vtd_as->pasid)) {
>> + uint32_t rid2pasid = VTD_CE_GET_RID2PASID(&ce);
>> +
>> + if ((vtd_as->pasid != PCI_NO_PASID || pasid != rid2pasid) &&
>> + vtd_as->pasid != pasid) {
>> + continue;
>> + }
>> +
>> + if (!s->scalable_modern) {
>> + vtd_address_space_sync(vtd_as);
>> + }
>> + }
>> + }
>> +}
>> +
>> +static bool vtd_process_piotlb_desc(IntelIOMMUState *s,
>> + VTDInvDesc *inv_desc)
>> +{
>> + uint16_t domain_id;
>> + uint32_t pasid;
>> +
>> + if ((inv_desc->val[0] & VTD_INV_DESC_PIOTLB_RSVD_VAL0) ||
>> + (inv_desc->val[1] & VTD_INV_DESC_PIOTLB_RSVD_VAL1)) {
>> + error_report_once("%s: invalid piotlb inv desc hi=0x%"PRIx64
>> + " lo=0x%"PRIx64" (reserved bits unzero)",
>> + __func__, inv_desc->val[1], inv_desc->val[0]);
>> + return false;
>> + }
>> +
>> + domain_id = VTD_INV_DESC_PIOTLB_DID(inv_desc->val[0]);
>> + pasid = VTD_INV_DESC_PIOTLB_PASID(inv_desc->val[0]);
>> + switch (inv_desc->val[0] & VTD_INV_DESC_PIOTLB_G) {
>> + case VTD_INV_DESC_PIOTLB_ALL_IN_PASID:
>> + vtd_piotlb_pasid_invalidate(s, domain_id, pasid);
>> + break;
>> +
>> + case VTD_INV_DESC_PIOTLB_PSI_IN_PASID:
>> + break;
>> +
>> + default:
>> + error_report_once("%s: invalid piotlb inv desc: hi=0x%"PRIx64
>> + ", lo=0x%"PRIx64" (type mismatch: 0x%llx)",
>> + __func__, inv_desc->val[1], inv_desc->val[0],
>> + inv_desc->val[0] & VTD_INV_DESC_IOTLB_G);
>> + return false;
>> + }
>> + return true;
>> +}
>> +
>> static bool vtd_process_inv_iec_desc(IntelIOMMUState *s,
>> VTDInvDesc *inv_desc)
>> {
>> @@ -2775,6 +2852,10 @@ static bool
>vtd_process_inv_desc(IntelIOMMUState *s)
>> break;
>>
>> case VTD_INV_DESC_PIOTLB:
>> + trace_vtd_inv_desc("p-iotlb", inv_desc.val[1], inv_desc.val[0]);
>> + if (!vtd_process_piotlb_desc(s, &inv_desc)) {
>> + return false;
>> + }
>> break;
>>
>> case VTD_INV_DESC_WAIT:
>
>--
>Regards,
>Yi Liu
On 2024/8/15 13:48, Duan, Zhenzhong wrote:
>
>
>> -----Original Message-----
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Subject: Re: [PATCH v2 04/17] intel_iommu: Flush stage-2 cache in PASID-
>> selective PASID-based iotlb invalidation
>>
>> On 2024/8/5 14:27, Zhenzhong Duan wrote:
>>> Per spec 6.5.2.4, PADID-selective PASID-based iotlb invalidation will
>>> flush stage-2 iotlb entries with matching domain id and pasid.
>>>
>>> With scalable modern mode introduced, guest could send PASID-selective
>>> PASID-based iotlb invalidation to flush both stage-1 and stage-2 entries.
>>
>> I'm not quite sure if this is correct. In the last collumn of the table 21
>> in 6.5.2.4, the paging structures of SS will not be invalidated. So it's
>> not quite recommended for software to invalidate the iotlb entries with
>> PGTT==SS-only by P_IOTLB invalidation, it's more recommended to use the
>> IOTLB invalidation.
>
> Hmm, when pasid is used with SS-only, PASID-based iotlb invalidation can
> give better granularity, (DID,PASID) vs. (DID) for IOTLB invalidation.
But you may need to submit multiple PASID-based iotlb invalidations as a SS
page table can be used by multiple pasid entries. From this pesperctive,
issuing a single IOTLB invalidation is simpler for programmer. And this is
also what VT-d spec recommends in
"Table 25. Guidance to Software for Invalidations".
In fact, for leaf modifications, the iommu driver should use the
page-selective granularity instead of pasid-selective. However, the line
3/column 2 of "Table 21. PASID-based-IOTLB Invalidation" says it is NA to
the iotlb entries with PGTT==010b.
>
> If non-leaf SS-paging entry is updated, IOTLB invalidation should be used
> as SS-paging structure cache isn't flushed with PASID-selective PASID-based
> iotlb invalidation.
yes in concept.
>
>>
>>> By this chance, remove old IOTLB related definition.
>>>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>> hw/i386/intel_iommu_internal.h | 14 +++---
>>> hw/i386/intel_iommu.c | 81
>> ++++++++++++++++++++++++++++++++++
>>> 2 files changed, 90 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/i386/intel_iommu_internal.h
>> b/hw/i386/intel_iommu_internal.h
>>> index 8fa27c7f3b..19e4ed52ca 100644
>>> --- a/hw/i386/intel_iommu_internal.h
>>> +++ b/hw/i386/intel_iommu_internal.h
>>> @@ -402,11 +402,6 @@ typedef union VTDInvDesc VTDInvDesc;
>>> #define VTD_INV_DESC_IOTLB_AM(val) ((val) & 0x3fULL)
>>> #define VTD_INV_DESC_IOTLB_RSVD_LO 0xffffffff0000ff00ULL
>>> #define VTD_INV_DESC_IOTLB_RSVD_HI 0xf80ULL
>>> -#define VTD_INV_DESC_IOTLB_PASID_PASID (2ULL << 4)
>>> -#define VTD_INV_DESC_IOTLB_PASID_PAGE (3ULL << 4)
>>> -#define VTD_INV_DESC_IOTLB_PASID(val) (((val) >> 32) &
>> VTD_PASID_ID_MASK)
>>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_LO
>> 0xfff00000000001c0ULL
>>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_HI 0xf80ULL
>>>
>>> /* Mask for Device IOTLB Invalidate Descriptor */
>>> #define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) &
>> 0xfffffffffffff000ULL)
>>> @@ -438,6 +433,15 @@ typedef union VTDInvDesc VTDInvDesc;
>>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM |
>> VTD_SL_TM)) : \
>>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>>>
>>> +/* Masks for PIOTLB Invalidate Descriptor */
>>> +#define VTD_INV_DESC_PIOTLB_G (3ULL << 4)
>>> +#define VTD_INV_DESC_PIOTLB_ALL_IN_PASID (2ULL << 4)
>>> +#define VTD_INV_DESC_PIOTLB_PSI_IN_PASID (3ULL << 4)
>>> +#define VTD_INV_DESC_PIOTLB_DID(val) (((val) >> 16) &
>> VTD_DOMAIN_ID_MASK)
>>> +#define VTD_INV_DESC_PIOTLB_PASID(val) (((val) >> 32) & 0xfffffULL)
>>> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL0 0xfff000000000f1c0ULL
>>> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL1 0xf80ULL
>>> +
>>> /* Information about page-selective IOTLB invalidate */
>>> struct VTDIOTLBPageInvInfo {
>>> uint16_t domain_id;
>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>> index c1382a5651..df591419b7 100644
>>> --- a/hw/i386/intel_iommu.c
>>> +++ b/hw/i386/intel_iommu.c
>>> @@ -2656,6 +2656,83 @@ static bool
>> vtd_process_iotlb_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
>>> return true;
>>> }
>>>
>>> +static gboolean vtd_hash_remove_by_pasid(gpointer key, gpointer value,
>>> + gpointer user_data)
>>> +{
>>> + VTDIOTLBEntry *entry = (VTDIOTLBEntry *)value;
>>> + VTDIOTLBPageInvInfo *info = (VTDIOTLBPageInvInfo *)user_data;
>>> +
>>> + return ((entry->domain_id == info->domain_id) &&
>>> + (entry->pasid == info->pasid));
>>> +}
>>> +
>>> +static void vtd_piotlb_pasid_invalidate(IntelIOMMUState *s,
>>> + uint16_t domain_id, uint32_t pasid)
>>> +{
>>> + VTDIOTLBPageInvInfo info;
>>> + VTDAddressSpace *vtd_as;
>>> + VTDContextEntry ce;
>>> +
>>> + info.domain_id = domain_id;
>>> + info.pasid = pasid;
>>> +
>>> + vtd_iommu_lock(s);
>>> + g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_pasid,
>>> + &info);
>>> + vtd_iommu_unlock(s);
>>> +
>>> + QLIST_FOREACH(vtd_as, &s->vtd_as_with_notifiers, next) {
>>> + if (!vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
>>> + vtd_as->devfn, &ce) &&
>>> + domain_id == vtd_get_domain_id(s, &ce, vtd_as->pasid)) {
>>> + uint32_t rid2pasid = VTD_CE_GET_RID2PASID(&ce);
>>> +
>>> + if ((vtd_as->pasid != PCI_NO_PASID || pasid != rid2pasid) &&
>>> + vtd_as->pasid != pasid) {
>>> + continue;
>>> + }
>>> +
>>> + if (!s->scalable_modern) {
>>> + vtd_address_space_sync(vtd_as);
>>> + }
>>> + }
>>> + }
>>> +}
>>> +
>>> +static bool vtd_process_piotlb_desc(IntelIOMMUState *s,
>>> + VTDInvDesc *inv_desc)
>>> +{
>>> + uint16_t domain_id;
>>> + uint32_t pasid;
>>> +
>>> + if ((inv_desc->val[0] & VTD_INV_DESC_PIOTLB_RSVD_VAL0) ||
>>> + (inv_desc->val[1] & VTD_INV_DESC_PIOTLB_RSVD_VAL1)) {
>>> + error_report_once("%s: invalid piotlb inv desc hi=0x%"PRIx64
>>> + " lo=0x%"PRIx64" (reserved bits unzero)",
>>> + __func__, inv_desc->val[1], inv_desc->val[0]);
>>> + return false;
>>> + }
>>> +
>>> + domain_id = VTD_INV_DESC_PIOTLB_DID(inv_desc->val[0]);
>>> + pasid = VTD_INV_DESC_PIOTLB_PASID(inv_desc->val[0]);
>>> + switch (inv_desc->val[0] & VTD_INV_DESC_PIOTLB_G) {
>>> + case VTD_INV_DESC_PIOTLB_ALL_IN_PASID:
>>> + vtd_piotlb_pasid_invalidate(s, domain_id, pasid);
>>> + break;
>>> +
>>> + case VTD_INV_DESC_PIOTLB_PSI_IN_PASID:
>>> + break;
>>> +
>>> + default:
>>> + error_report_once("%s: invalid piotlb inv desc: hi=0x%"PRIx64
>>> + ", lo=0x%"PRIx64" (type mismatch: 0x%llx)",
>>> + __func__, inv_desc->val[1], inv_desc->val[0],
>>> + inv_desc->val[0] & VTD_INV_DESC_IOTLB_G);
>>> + return false;
>>> + }
>>> + return true;
>>> +}
>>> +
>>> static bool vtd_process_inv_iec_desc(IntelIOMMUState *s,
>>> VTDInvDesc *inv_desc)
>>> {
>>> @@ -2775,6 +2852,10 @@ static bool
>> vtd_process_inv_desc(IntelIOMMUState *s)
>>> break;
>>>
>>> case VTD_INV_DESC_PIOTLB:
>>> + trace_vtd_inv_desc("p-iotlb", inv_desc.val[1], inv_desc.val[0]);
>>> + if (!vtd_process_piotlb_desc(s, &inv_desc)) {
>>> + return false;
>>> + }
>>> break;
>>>
>>> case VTD_INV_DESC_WAIT:
>>
>> --
>> Regards,
>> Yi Liu
--
Regards,
Yi Liu
On 05/08/2024 08:27, Zhenzhong Duan wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
> Per spec 6.5.2.4, PADID-selective PASID-based iotlb invalidation will
> flush stage-2 iotlb entries with matching domain id and pasid.
>
> With scalable modern mode introduced, guest could send PASID-selective
> PASID-based iotlb invalidation to flush both stage-1 and stage-2 entries.
>
> By this chance, remove old IOTLB related definition.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
> hw/i386/intel_iommu_internal.h | 14 +++---
> hw/i386/intel_iommu.c | 81 ++++++++++++++++++++++++++++++++++
> 2 files changed, 90 insertions(+), 5 deletions(-)
>
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 8fa27c7f3b..19e4ed52ca 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -402,11 +402,6 @@ typedef union VTDInvDesc VTDInvDesc;
> #define VTD_INV_DESC_IOTLB_AM(val) ((val) & 0x3fULL)
> #define VTD_INV_DESC_IOTLB_RSVD_LO 0xffffffff0000ff00ULL
> #define VTD_INV_DESC_IOTLB_RSVD_HI 0xf80ULL
> -#define VTD_INV_DESC_IOTLB_PASID_PASID (2ULL << 4)
> -#define VTD_INV_DESC_IOTLB_PASID_PAGE (3ULL << 4)
> -#define VTD_INV_DESC_IOTLB_PASID(val) (((val) >> 32) & VTD_PASID_ID_MASK)
> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_LO 0xfff00000000001c0ULL
> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_HI 0xf80ULL
>
> /* Mask for Device IOTLB Invalidate Descriptor */
> #define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) & 0xfffffffffffff000ULL)
> @@ -438,6 +433,15 @@ typedef union VTDInvDesc VTDInvDesc;
> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \
> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>
> +/* Masks for PIOTLB Invalidate Descriptor */
> +#define VTD_INV_DESC_PIOTLB_G (3ULL << 4)
> +#define VTD_INV_DESC_PIOTLB_ALL_IN_PASID (2ULL << 4)
> +#define VTD_INV_DESC_PIOTLB_PSI_IN_PASID (3ULL << 4)
> +#define VTD_INV_DESC_PIOTLB_DID(val) (((val) >> 16) & VTD_DOMAIN_ID_MASK)
> +#define VTD_INV_DESC_PIOTLB_PASID(val) (((val) >> 32) & 0xfffffULL)
> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL0 0xfff000000000f1c0ULL
Why did this value change since last post? The 'type' field should
always be zero in this desc
> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL1 0xf80ULL
> +
> /* Information about page-selective IOTLB invalidate */
> struct VTDIOTLBPageInvInfo {
> uint16_t domain_id;
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index c1382a5651..df591419b7 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2656,6 +2656,83 @@ static bool vtd_process_iotlb_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
> return true;
> }
>
> +static gboolean vtd_hash_remove_by_pasid(gpointer key, gpointer value,
> + gpointer user_data)
> +{
> + VTDIOTLBEntry *entry = (VTDIOTLBEntry *)value;
> + VTDIOTLBPageInvInfo *info = (VTDIOTLBPageInvInfo *)user_data;
> +
> + return ((entry->domain_id == info->domain_id) &&
> + (entry->pasid == info->pasid));
> +}
> +
> +static void vtd_piotlb_pasid_invalidate(IntelIOMMUState *s,
> + uint16_t domain_id, uint32_t pasid)
> +{
> + VTDIOTLBPageInvInfo info;
> + VTDAddressSpace *vtd_as;
> + VTDContextEntry ce;
> +
> + info.domain_id = domain_id;
> + info.pasid = pasid;
> +
> + vtd_iommu_lock(s);
> + g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_pasid,
> + &info);
> + vtd_iommu_unlock(s);
> +
> + QLIST_FOREACH(vtd_as, &s->vtd_as_with_notifiers, next) {
> + if (!vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
> + vtd_as->devfn, &ce) &&
> + domain_id == vtd_get_domain_id(s, &ce, vtd_as->pasid)) {
> + uint32_t rid2pasid = VTD_CE_GET_RID2PASID(&ce);
> +
> + if ((vtd_as->pasid != PCI_NO_PASID || pasid != rid2pasid) &&
> + vtd_as->pasid != pasid) {
> + continue;
> + }
> +
> + if (!s->scalable_modern) {
> + vtd_address_space_sync(vtd_as);
> + }
> + }
> + }
> +}
> +
> +static bool vtd_process_piotlb_desc(IntelIOMMUState *s,
> + VTDInvDesc *inv_desc)
> +{
> + uint16_t domain_id;
> + uint32_t pasid;
> +
> + if ((inv_desc->val[0] & VTD_INV_DESC_PIOTLB_RSVD_VAL0) ||
> + (inv_desc->val[1] & VTD_INV_DESC_PIOTLB_RSVD_VAL1)) {
> + error_report_once("%s: invalid piotlb inv desc hi=0x%"PRIx64
> + " lo=0x%"PRIx64" (reserved bits unzero)",
> + __func__, inv_desc->val[1], inv_desc->val[0]);
> + return false;
> + }
> +
> + domain_id = VTD_INV_DESC_PIOTLB_DID(inv_desc->val[0]);
> + pasid = VTD_INV_DESC_PIOTLB_PASID(inv_desc->val[0]);
> + switch (inv_desc->val[0] & VTD_INV_DESC_PIOTLB_G) {
> + case VTD_INV_DESC_PIOTLB_ALL_IN_PASID:
> + vtd_piotlb_pasid_invalidate(s, domain_id, pasid);
> + break;
> +
> + case VTD_INV_DESC_PIOTLB_PSI_IN_PASID:
> + break;
> +
> + default:
> + error_report_once("%s: invalid piotlb inv desc: hi=0x%"PRIx64
> + ", lo=0x%"PRIx64" (type mismatch: 0x%llx)",
> + __func__, inv_desc->val[1], inv_desc->val[0],
> + inv_desc->val[0] & VTD_INV_DESC_IOTLB_G);
> + return false;
> + }
> + return true;
> +}
> +
> static bool vtd_process_inv_iec_desc(IntelIOMMUState *s,
> VTDInvDesc *inv_desc)
> {
> @@ -2775,6 +2852,10 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
> break;
>
> case VTD_INV_DESC_PIOTLB:
> + trace_vtd_inv_desc("p-iotlb", inv_desc.val[1], inv_desc.val[0]);
> + if (!vtd_process_piotlb_desc(s, &inv_desc)) {
> + return false;
> + }
> break;
>
> case VTD_INV_DESC_WAIT:
> --
> 2.34.1
>
On 8/6/2024 2:35 PM, CLEMENT MATHIEU--DRIF wrote: > > On 05/08/2024 08:27, Zhenzhong Duan wrote: >> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe. >> >> >> Per spec 6.5.2.4, PADID-selective PASID-based iotlb invalidation will >> flush stage-2 iotlb entries with matching domain id and pasid. >> >> With scalable modern mode introduced, guest could send PASID-selective >> PASID-based iotlb invalidation to flush both stage-1 and stage-2 entries. >> >> By this chance, remove old IOTLB related definition. >> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >> --- >> hw/i386/intel_iommu_internal.h | 14 +++--- >> hw/i386/intel_iommu.c | 81 ++++++++++++++++++++++++++++++++++ >> 2 files changed, 90 insertions(+), 5 deletions(-) >> >> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h >> index 8fa27c7f3b..19e4ed52ca 100644 >> --- a/hw/i386/intel_iommu_internal.h >> +++ b/hw/i386/intel_iommu_internal.h >> @@ -402,11 +402,6 @@ typedef union VTDInvDesc VTDInvDesc; >> #define VTD_INV_DESC_IOTLB_AM(val) ((val) & 0x3fULL) >> #define VTD_INV_DESC_IOTLB_RSVD_LO 0xffffffff0000ff00ULL >> #define VTD_INV_DESC_IOTLB_RSVD_HI 0xf80ULL >> -#define VTD_INV_DESC_IOTLB_PASID_PASID (2ULL << 4) >> -#define VTD_INV_DESC_IOTLB_PASID_PAGE (3ULL << 4) >> -#define VTD_INV_DESC_IOTLB_PASID(val) (((val) >> 32) & VTD_PASID_ID_MASK) >> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_LO 0xfff00000000001c0ULL >> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_HI 0xf80ULL >> >> /* Mask for Device IOTLB Invalidate Descriptor */ >> #define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) & 0xfffffffffffff000ULL) >> @@ -438,6 +433,15 @@ typedef union VTDInvDesc VTDInvDesc; >> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | VTD_SL_TM)) : \ >> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM)) >> >> +/* Masks for PIOTLB Invalidate Descriptor */ >> +#define VTD_INV_DESC_PIOTLB_G (3ULL << 4) >> +#define VTD_INV_DESC_PIOTLB_ALL_IN_PASID (2ULL << 4) >> +#define VTD_INV_DESC_PIOTLB_PSI_IN_PASID (3ULL << 4) >> +#define VTD_INV_DESC_PIOTLB_DID(val) (((val) >> 16) & VTD_DOMAIN_ID_MASK) >> +#define VTD_INV_DESC_PIOTLB_PASID(val) (((val) >> 32) & 0xfffffULL) >> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL0 0xfff000000000f1c0ULL > Why did this value change since last post? The 'type' field should > always be zero in this desc Yes, type[6:4] are all zero for all existing invalidation type. But they are not real reserved bits. So I removed them from VTD_INV_DESC_PIOTLB_RSVD_VAL0. Thanks Zhenzhong
On 08/08/2024 14:40, Duan, Zhenzhong wrote: > Caution: External email. Do not open attachments or click links, > unless this email comes from a known sender and you know the content > is safe. > > > On 8/6/2024 2:35 PM, CLEMENT MATHIEU--DRIF wrote: >> >> On 05/08/2024 08:27, Zhenzhong Duan wrote: >>> Caution: External email. Do not open attachments or click links, >>> unless this email comes from a known sender and you know the content >>> is safe. >>> >>> >>> Per spec 6.5.2.4, PADID-selective PASID-based iotlb invalidation will >>> flush stage-2 iotlb entries with matching domain id and pasid. >>> >>> With scalable modern mode introduced, guest could send PASID-selective >>> PASID-based iotlb invalidation to flush both stage-1 and stage-2 >>> entries. >>> >>> By this chance, remove old IOTLB related definition. >>> >>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >>> --- >>> hw/i386/intel_iommu_internal.h | 14 +++--- >>> hw/i386/intel_iommu.c | 81 >>> ++++++++++++++++++++++++++++++++++ >>> 2 files changed, 90 insertions(+), 5 deletions(-) >>> >>> diff --git a/hw/i386/intel_iommu_internal.h >>> b/hw/i386/intel_iommu_internal.h >>> index 8fa27c7f3b..19e4ed52ca 100644 >>> --- a/hw/i386/intel_iommu_internal.h >>> +++ b/hw/i386/intel_iommu_internal.h >>> @@ -402,11 +402,6 @@ typedef union VTDInvDesc VTDInvDesc; >>> #define VTD_INV_DESC_IOTLB_AM(val) ((val) & 0x3fULL) >>> #define VTD_INV_DESC_IOTLB_RSVD_LO 0xffffffff0000ff00ULL >>> #define VTD_INV_DESC_IOTLB_RSVD_HI 0xf80ULL >>> -#define VTD_INV_DESC_IOTLB_PASID_PASID (2ULL << 4) >>> -#define VTD_INV_DESC_IOTLB_PASID_PAGE (3ULL << 4) >>> -#define VTD_INV_DESC_IOTLB_PASID(val) (((val) >> 32) & >>> VTD_PASID_ID_MASK) >>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_LO 0xfff00000000001c0ULL >>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_HI 0xf80ULL >>> >>> /* Mask for Device IOTLB Invalidate Descriptor */ >>> #define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) & >>> 0xfffffffffffff000ULL) >>> @@ -438,6 +433,15 @@ typedef union VTDInvDesc VTDInvDesc; >>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | >>> VTD_SL_TM)) : \ >>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM)) >>> >>> +/* Masks for PIOTLB Invalidate Descriptor */ >>> +#define VTD_INV_DESC_PIOTLB_G (3ULL << 4) >>> +#define VTD_INV_DESC_PIOTLB_ALL_IN_PASID (2ULL << 4) >>> +#define VTD_INV_DESC_PIOTLB_PSI_IN_PASID (3ULL << 4) >>> +#define VTD_INV_DESC_PIOTLB_DID(val) (((val) >> 16) & >>> VTD_DOMAIN_ID_MASK) >>> +#define VTD_INV_DESC_PIOTLB_PASID(val) (((val) >> 32) & 0xfffffULL) >>> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL0 0xfff000000000f1c0ULL >> Why did this value change since last post? The 'type' field should >> always be zero in this desc > > Yes, type[6:4] are all zero for all existing invalidation type. But they > are not real reserved bits. > > So I removed them from VTD_INV_DESC_PIOTLB_RSVD_VAL0. Other masks consider these zeroes as reserved. I think we should do the same. For instance, context cache invalidation is : #define VTD_INV_DESC_CC_RSVD 0xfffc00000000ffc0ULL > > Thanks > > Zhenzhong >
>-----Original Message----- >From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com> >Subject: Re: [PATCH v2 04/17] intel_iommu: Flush stage-2 cache in PASID- >selective PASID-based iotlb invalidation > > > >On 08/08/2024 14:40, Duan, Zhenzhong wrote: >> Caution: External email. Do not open attachments or click links, >> unless this email comes from a known sender and you know the content >> is safe. >> >> >> On 8/6/2024 2:35 PM, CLEMENT MATHIEU--DRIF wrote: >>> >>> On 05/08/2024 08:27, Zhenzhong Duan wrote: >>>> Caution: External email. Do not open attachments or click links, >>>> unless this email comes from a known sender and you know the content >>>> is safe. >>>> >>>> >>>> Per spec 6.5.2.4, PADID-selective PASID-based iotlb invalidation will >>>> flush stage-2 iotlb entries with matching domain id and pasid. >>>> >>>> With scalable modern mode introduced, guest could send PASID- >selective >>>> PASID-based iotlb invalidation to flush both stage-1 and stage-2 >>>> entries. >>>> >>>> By this chance, remove old IOTLB related definition. >>>> >>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >>>> --- >>>> hw/i386/intel_iommu_internal.h | 14 +++--- >>>> hw/i386/intel_iommu.c | 81 >>>> ++++++++++++++++++++++++++++++++++ >>>> 2 files changed, 90 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/hw/i386/intel_iommu_internal.h >>>> b/hw/i386/intel_iommu_internal.h >>>> index 8fa27c7f3b..19e4ed52ca 100644 >>>> --- a/hw/i386/intel_iommu_internal.h >>>> +++ b/hw/i386/intel_iommu_internal.h >>>> @@ -402,11 +402,6 @@ typedef union VTDInvDesc VTDInvDesc; >>>> #define VTD_INV_DESC_IOTLB_AM(val) ((val) & 0x3fULL) >>>> #define VTD_INV_DESC_IOTLB_RSVD_LO 0xffffffff0000ff00ULL >>>> #define VTD_INV_DESC_IOTLB_RSVD_HI 0xf80ULL >>>> -#define VTD_INV_DESC_IOTLB_PASID_PASID (2ULL << 4) >>>> -#define VTD_INV_DESC_IOTLB_PASID_PAGE (3ULL << 4) >>>> -#define VTD_INV_DESC_IOTLB_PASID(val) (((val) >> 32) & >>>> VTD_PASID_ID_MASK) >>>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_LO >0xfff00000000001c0ULL >>>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_HI 0xf80ULL >>>> >>>> /* Mask for Device IOTLB Invalidate Descriptor */ >>>> #define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) & >>>> 0xfffffffffffff000ULL) >>>> @@ -438,6 +433,15 @@ typedef union VTDInvDesc VTDInvDesc; >>>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | >>>> VTD_SL_TM)) : \ >>>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM)) >>>> >>>> +/* Masks for PIOTLB Invalidate Descriptor */ >>>> +#define VTD_INV_DESC_PIOTLB_G (3ULL << 4) >>>> +#define VTD_INV_DESC_PIOTLB_ALL_IN_PASID (2ULL << 4) >>>> +#define VTD_INV_DESC_PIOTLB_PSI_IN_PASID (3ULL << 4) >>>> +#define VTD_INV_DESC_PIOTLB_DID(val) (((val) >> 16) & >>>> VTD_DOMAIN_ID_MASK) >>>> +#define VTD_INV_DESC_PIOTLB_PASID(val) (((val) >> 32) & 0xfffffULL) >>>> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL0 0xfff000000000f1c0ULL >>> Why did this value change since last post? The 'type' field should >>> always be zero in this desc >> >> Yes, type[6:4] are all zero for all existing invalidation type. But they >> are not real reserved bits. >> >> So I removed them from VTD_INV_DESC_PIOTLB_RSVD_VAL0. >Other masks consider these zeroes as reserved. >I think we should do the same. >For instance, context cache invalidation is : #define >VTD_INV_DESC_CC_RSVD 0xfffc00000000ffc0ULL Yes, I'll make a separate patch to fix it. Thanks Zhenzhong
On 13/08/2024 04:12, Duan, Zhenzhong wrote: > Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe. > > >> -----Original Message----- >> From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com> >> Subject: Re: [PATCH v2 04/17] intel_iommu: Flush stage-2 cache in PASID- >> selective PASID-based iotlb invalidation >> >> >> >> On 08/08/2024 14:40, Duan, Zhenzhong wrote: >>> Caution: External email. Do not open attachments or click links, >>> unless this email comes from a known sender and you know the content >>> is safe. >>> >>> >>> On 8/6/2024 2:35 PM, CLEMENT MATHIEU--DRIF wrote: >>>> On 05/08/2024 08:27, Zhenzhong Duan wrote: >>>>> Caution: External email. Do not open attachments or click links, >>>>> unless this email comes from a known sender and you know the content >>>>> is safe. >>>>> >>>>> >>>>> Per spec 6.5.2.4, PADID-selective PASID-based iotlb invalidation will >>>>> flush stage-2 iotlb entries with matching domain id and pasid. >>>>> >>>>> With scalable modern mode introduced, guest could send PASID- >> selective >>>>> PASID-based iotlb invalidation to flush both stage-1 and stage-2 >>>>> entries. >>>>> >>>>> By this chance, remove old IOTLB related definition. >>>>> >>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >>>>> --- >>>>> hw/i386/intel_iommu_internal.h | 14 +++--- >>>>> hw/i386/intel_iommu.c | 81 >>>>> ++++++++++++++++++++++++++++++++++ >>>>> 2 files changed, 90 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/hw/i386/intel_iommu_internal.h >>>>> b/hw/i386/intel_iommu_internal.h >>>>> index 8fa27c7f3b..19e4ed52ca 100644 >>>>> --- a/hw/i386/intel_iommu_internal.h >>>>> +++ b/hw/i386/intel_iommu_internal.h >>>>> @@ -402,11 +402,6 @@ typedef union VTDInvDesc VTDInvDesc; >>>>> #define VTD_INV_DESC_IOTLB_AM(val) ((val) & 0x3fULL) >>>>> #define VTD_INV_DESC_IOTLB_RSVD_LO 0xffffffff0000ff00ULL >>>>> #define VTD_INV_DESC_IOTLB_RSVD_HI 0xf80ULL >>>>> -#define VTD_INV_DESC_IOTLB_PASID_PASID (2ULL << 4) >>>>> -#define VTD_INV_DESC_IOTLB_PASID_PAGE (3ULL << 4) >>>>> -#define VTD_INV_DESC_IOTLB_PASID(val) (((val) >> 32) & >>>>> VTD_PASID_ID_MASK) >>>>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_LO >> 0xfff00000000001c0ULL >>>>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_HI 0xf80ULL >>>>> >>>>> /* Mask for Device IOTLB Invalidate Descriptor */ >>>>> #define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) & >>>>> 0xfffffffffffff000ULL) >>>>> @@ -438,6 +433,15 @@ typedef union VTDInvDesc VTDInvDesc; >>>>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | >>>>> VTD_SL_TM)) : \ >>>>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM)) >>>>> >>>>> +/* Masks for PIOTLB Invalidate Descriptor */ >>>>> +#define VTD_INV_DESC_PIOTLB_G (3ULL << 4) >>>>> +#define VTD_INV_DESC_PIOTLB_ALL_IN_PASID (2ULL << 4) >>>>> +#define VTD_INV_DESC_PIOTLB_PSI_IN_PASID (3ULL << 4) >>>>> +#define VTD_INV_DESC_PIOTLB_DID(val) (((val) >> 16) & >>>>> VTD_DOMAIN_ID_MASK) >>>>> +#define VTD_INV_DESC_PIOTLB_PASID(val) (((val) >> 32) & 0xfffffULL) >>>>> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL0 0xfff000000000f1c0ULL >>>> Why did this value change since last post? The 'type' field should >>>> always be zero in this desc >>> Yes, type[6:4] are all zero for all existing invalidation type. But they >>> are not real reserved bits. >>> >>> So I removed them from VTD_INV_DESC_PIOTLB_RSVD_VAL0. >> Other masks consider these zeroes as reserved. >> I think we should do the same. >> For instance, context cache invalidation is : #define >> VTD_INV_DESC_CC_RSVD 0xfffc00000000ffc0ULL > Yes, I'll make a separate patch to fix it. Oops, I just saw your patch, sorry for the misunderstanding!! I think we should continue treating these bits as reserved because the descriptor type detection code only checks the 4 LSB. > > Thanks > Zhenzhong
On 13/08/2024 09:13, CLEMENT MATHIEU--DRIF wrote: > > On 13/08/2024 04:12, Duan, Zhenzhong wrote: >> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe. >> >> >>> -----Original Message----- >>> From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com> >>> Subject: Re: [PATCH v2 04/17] intel_iommu: Flush stage-2 cache in PASID- >>> selective PASID-based iotlb invalidation >>> >>> >>> >>> On 08/08/2024 14:40, Duan, Zhenzhong wrote: >>>> Caution: External email. Do not open attachments or click links, >>>> unless this email comes from a known sender and you know the content >>>> is safe. >>>> >>>> >>>> On 8/6/2024 2:35 PM, CLEMENT MATHIEU--DRIF wrote: >>>>> On 05/08/2024 08:27, Zhenzhong Duan wrote: >>>>>> Caution: External email. Do not open attachments or click links, >>>>>> unless this email comes from a known sender and you know the content >>>>>> is safe. >>>>>> >>>>>> >>>>>> Per spec 6.5.2.4, PADID-selective PASID-based iotlb invalidation will >>>>>> flush stage-2 iotlb entries with matching domain id and pasid. >>>>>> >>>>>> With scalable modern mode introduced, guest could send PASID- >>> selective >>>>>> PASID-based iotlb invalidation to flush both stage-1 and stage-2 >>>>>> entries. >>>>>> >>>>>> By this chance, remove old IOTLB related definition. >>>>>> >>>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >>>>>> --- >>>>>> hw/i386/intel_iommu_internal.h | 14 +++--- >>>>>> hw/i386/intel_iommu.c | 81 >>>>>> ++++++++++++++++++++++++++++++++++ >>>>>> 2 files changed, 90 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/hw/i386/intel_iommu_internal.h >>>>>> b/hw/i386/intel_iommu_internal.h >>>>>> index 8fa27c7f3b..19e4ed52ca 100644 >>>>>> --- a/hw/i386/intel_iommu_internal.h >>>>>> +++ b/hw/i386/intel_iommu_internal.h >>>>>> @@ -402,11 +402,6 @@ typedef union VTDInvDesc VTDInvDesc; >>>>>> #define VTD_INV_DESC_IOTLB_AM(val) ((val) & 0x3fULL) >>>>>> #define VTD_INV_DESC_IOTLB_RSVD_LO 0xffffffff0000ff00ULL >>>>>> #define VTD_INV_DESC_IOTLB_RSVD_HI 0xf80ULL >>>>>> -#define VTD_INV_DESC_IOTLB_PASID_PASID (2ULL << 4) >>>>>> -#define VTD_INV_DESC_IOTLB_PASID_PAGE (3ULL << 4) >>>>>> -#define VTD_INV_DESC_IOTLB_PASID(val) (((val) >> 32) & >>>>>> VTD_PASID_ID_MASK) >>>>>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_LO >>> 0xfff00000000001c0ULL >>>>>> -#define VTD_INV_DESC_IOTLB_PASID_RSVD_HI 0xf80ULL >>>>>> >>>>>> /* Mask for Device IOTLB Invalidate Descriptor */ >>>>>> #define VTD_INV_DESC_DEVICE_IOTLB_ADDR(val) ((val) & >>>>>> 0xfffffffffffff000ULL) >>>>>> @@ -438,6 +433,15 @@ typedef union VTDInvDesc VTDInvDesc; >>>>>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM | >>>>>> VTD_SL_TM)) : \ >>>>>> (0x3ffff800ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM)) >>>>>> >>>>>> +/* Masks for PIOTLB Invalidate Descriptor */ >>>>>> +#define VTD_INV_DESC_PIOTLB_G (3ULL << 4) >>>>>> +#define VTD_INV_DESC_PIOTLB_ALL_IN_PASID (2ULL << 4) >>>>>> +#define VTD_INV_DESC_PIOTLB_PSI_IN_PASID (3ULL << 4) >>>>>> +#define VTD_INV_DESC_PIOTLB_DID(val) (((val) >> 16) & >>>>>> VTD_DOMAIN_ID_MASK) >>>>>> +#define VTD_INV_DESC_PIOTLB_PASID(val) (((val) >> 32) & 0xfffffULL) >>>>>> +#define VTD_INV_DESC_PIOTLB_RSVD_VAL0 0xfff000000000f1c0ULL >>>>> Why did this value change since last post? The 'type' field should >>>>> always be zero in this desc >>>> Yes, type[6:4] are all zero for all existing invalidation type. But they >>>> are not real reserved bits. >>>> >>>> So I removed them from VTD_INV_DESC_PIOTLB_RSVD_VAL0. >>> Other masks consider these zeroes as reserved. >>> I think we should do the same. >>> For instance, context cache invalidation is : #define >>> VTD_INV_DESC_CC_RSVD 0xfffc00000000ffc0ULL >> Yes, I'll make a separate patch to fix it. > Oops, I just saw your patch, sorry for the misunderstanding!! > I think we should continue treating these bits as reserved because the > descriptor type detection code only checks the 4 LSB. Oh, you fixed it as well, so, ok I agree with the changes. Sorry for that >> Thanks >> Zhenzhong
© 2016 - 2024 Red Hat, Inc.