On Thu, 2026-02-12 at 08:16 +0000, Duan, Zhenzhong wrote: > > > > > -----Original Message----- > > From: Clement Mathieu--Drif > > <[[email protected]](mailto:[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])](mailto:[[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. >
The previous approach was to make sure that the kernel never allocates a too large pasid by limiting the width in the iommu instead of doint it in the devices. Your patch seems to reverse the problem. Is it something we are ready to accept? > Thanks > Zhenzhong
