>-----Original Message----- >From: Clement Mathieu--Drif <[email protected]> >Subject: Re: [RFC PATCH 05/14] intel_iommu: Change pasid property from bool to >uint8 > > >On Wed, 2026-02-04 at 22:11 -0500, Zhenzhong Duan wrote: >> 'x-pasid-mode' is a bool property, we need an extra 'pss' property to >> represent PASID size supported. Because there is no any device in QEMU >> supporting pasid capability yet, no guest could use the pasid feature >> until now, 'x-pasid-mode' takes no effect. >> >> So instead of an extra 'pss' property we can use a single 'pasid' >> property of uint8 type to represent if pasid is supported and the PASID >> bits size. A value of N > 0 means pasid is supported and N - 1 is the >> value in PSS field in ECAP register. >> >> Signed-off-by: Zhenzhong Duan ><[[email protected]](mailto:[email protected])> >> --- >> hw/i386/intel_iommu_internal.h | 1 - >> include/hw/i386/intel_iommu.h | 2 +- >> hw/i386/intel_iommu.c | 4 ++-- >> 3 files changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h >> index a2ca79f925..71cb3b662e 100644 >> --- a/hw/i386/intel_iommu_internal.h >> +++ b/hw/i386/intel_iommu_internal.h >> @@ -194,7 +194,6 @@ >> #define VTD_ECAP_PRS (1ULL << 29) >> #define VTD_ECAP_MHMV (15ULL << 20) >> #define VTD_ECAP_SRS (1ULL << 31) >> -#define VTD_ECAP_PSS (7ULL << 35) /* limit: MemTxAttrs::pid >> */ >> #define VTD_ECAP_PASID (1ULL << 40) >> #define VTD_ECAP_SMTS (1ULL << 43) >> #define VTD_ECAP_SSTS (1ULL << 46) >> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h >> index 6c61fd39c7..5e5779e460 100644 >> --- a/include/hw/i386/intel_iommu.h >> +++ b/include/hw/i386/intel_iommu.h >> @@ -315,7 +315,7 @@ struct IntelIOMMUState { >> bool buggy_eim; /* Force buggy EIM unless eim=off */ >> uint8_t aw_bits; /* Host/IOVA address width (in bits) */ >> bool dma_drain; /* Whether DMA r/w draining enabled */ >> - bool pasid; /* Whether to support PASID */ >> + uint8_t pasid; /* PASID supported in bits, 0 if not */ >> bool fs1gp; /* First Stage 1-GByte Page Support */ >> >> /* Transient Mapping, Reserved(0) since VTD spec revision 3.2 */ >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> index e8a6f50a5a..916eb0af5a 100644 >> --- a/hw/i386/intel_iommu.c >> +++ b/hw/i386/intel_iommu.c >> @@ -4151,7 +4151,7 @@ static const Property vtd_properties[] = { >> DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode, >FALSE), >> DEFINE_PROP_BOOL("x-flts", IntelIOMMUState, fsts, FALSE), >> DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState, snoop_control, >false), >> - DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false), >> + DEFINE_PROP_UINT8("pasid", IntelIOMMUState, pasid, 0), >> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true), >> DEFINE_PROP_BOOL("stale-tm", IntelIOMMUState, stale_tm, false), >> DEFINE_PROP_BOOL("fs1gp", IntelIOMMUState, fs1gp, true), >> @@ -4981,7 +4981,7 @@ static void vtd_cap_init(IntelIOMMUState *s) >> } >> >> if (s->pasid) { >> - s->ecap |= VTD_ECAP_PASID | VTD_ECAP_PSS; >> + s->ecap = VTD_ECAP_PASID | deposit64(s->ecap, 35, 5, s->pasid - 1); > >I think this is a bit dangerous, the MemTxAttrs only allow up to 8 bits. >We have a comment next to the definition of VTD_ECAP_PSS about this.
Could you elaborate what dangerous do you see if s->pasid > 8? MemTxAttrs only allow up to 8bits, then Max PASID Width in emulated device's PASID capability Register is no more than 8. Emulated device should never send a Request-with-pasid with pasid > 255. Hard code VTD_ECAP_PSS to 8 bits will make us not able to utilize the full pasid space of passthrough device if it has larger Max PASID Width value in pasid capability. Thanks Zhenzhong
