Shouldn't we check that the user never asks for more than 20 bits on the cli
btw?
On Fri, 2026-02-13 at 09:59 +0100, Clement Mathieu--Drif wrote:
>
> On Fri, 2026-02-13 at 08:43 +0000, Duan, Zhenzhong wrote:
>
>
> >
> >
> >
> > > -----Original Message-----
> > > From: Clement Mathieu--Drif
> > > <[[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected]))>
> > > Subject: Re: [RFC PATCH 05/14] intel_iommu: Change pasid property from
> > > bool to
> > > uint8
> > >
> > >
> > > On Fri, 2026-02-13 at 01:21 +0000, Duan, Zhenzhong wrote:
> > >
> > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Clement Mathieu--Drif <[clement.mathieu--
> > > >
> > > >
> > >
> > >
> > > [[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected]))](mailto:[[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected])))>
> > >
> > >
> > > >
> > > >
> > > > > Subject: Re: [RFC PATCH 05/14] intel_iommu: Change pasid property
> > > > > from bool
> > > >
> > > >
> > >
> > >
> > > to
> > >
> > >
> > > >
> > > >
> > > > > uint8
> > > > >
> > > > >
> > > > > On Thu, 2026-02-12 at 08:16 +0000, Duan, Zhenzhong wrote:
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Clement Mathieu--Drif <[clement.mathieu--
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > > >
> > > > > [[[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected]))](mailto:[[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected])))](mailto:[clement.mathieu--
> > > >
> > > >
> > >
> > >
> > > [[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected]))](mailto:[[[email protected]](mailto:[email protected])](mailto:[[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]))](mailto:[[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected])))](mailto:[zhen
> > > [[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected]))](mailto:[[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected]))))](mailto:[zhen
> > >
> > >
> > > >
> > > >
> > > > >
> > > >
> > > >
> > >
> > >
> > > [[[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected]))](mailto:[[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected])))](mailto:[zhenzhong.duan
> > > @intel.com](mailto:[[[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?
> > > >
> > > >
> > > >
> > > >
> > > > Not get your concern. Define a hard coded limit looks an overkill.
> > > > How you define too large here, larger than MIN(iommu.VTD_ECAP_PSS,
> > > > device's
> > >
> > >
> > > Max PASID Width)?
> > >
> > > Oh yep, you are righ, my "too large" is not very explicit.
> > > I mean too large to be passed from an emulated device, all the way down
> > > to the
> > > iommu, through MemTxAttrs.
> > >
> > >
> > >
> > > >
> > > > IIUC, kernel should take MIN(iommu.VTD_ECAP_PSS, device's Max PASID
> > > > Width)
> > >
> > >
> > > as the upper limit
> > >
> > >
> > > > of pasid range.
> > >
> > >
> > >
> > > Yes, that's true.
> > > Our current implementation makes sure that this MIN is always smaller
> > > than what
> > > fits in MemTxAttrs.
> > > This is always the case as pss is small enough.
> > >
> > > Increasing pss means that it's now up to devices to know that the attrs
> > > cannot
> > > carry a full-width pasid
> > > and expose a lower pasid size to make sure the kernel never allocates an
> > > out-of-
> > > range pasid.
> >
> >
> >
> > I see your concern.
> > Looked into RISC-V emulation series
> > [[https://lore.kernel.org/qemu-devel/[email protected]/](https://lore.kernel.org/qemu-devel/[email protected]/)](https://lore.kernel.org/qemu-devel/[email protected]/](https://lore.kernel.org/qemu-devel/[email protected]/))
> >
> > which introduced MemTxAttrs.pid, it looks MemTxAttrs.pid is only used by
> > RISC-V,
> > E.g., only riscv_iommu_memory_region_index() returns attrs.pid and pass it
> > to imrc->translate().
> >
> > For VTD and other vIOMMUs, we create different AddressSpace for different
> > pasid,
> > When call imrc->translate() callback, it's enough to go ahead with
> > AddressSpace,
> > we don't pass in pasid through iommu_idx parameter.
> >
> >
> >
> >
> > > This a bit more dagerous for device implementors but I don't see any
> > > other way to
> > > make accel work without this change. I just wanted to make sure that you
> > > noticed
> > > the design shift behind your patch.
> >
> >
> >
> > I think it's not an issue with VTD as explained above.
> >
> >
> >
> >
> > > btw, why do we need to pass the actual value on the command line?
> > > Couldn't we
> > > fetch the pss supported by
> > > the underlying hardware and use the same value?
> >
> >
> >
> > Because we can have heterogeneous IOMMUs on host, PSS can be different
> > between them.
> > Meanwhile, take migration into consideration, it needs to be configurable
> > by user.
>
>
> right
>
> Thanks
> cmd
>
>
>
> > Thanks
> > Zhenzhong
> >
> >
> >
> >
> > > Thanks!
> > > cmd
> > >
> > >
> > >
> > > >
> > > > We also have patch12 to ensure vIOMMU's pasid upper limit not bigger
> > > > than
> > >
> > >
> > > host side:
> > >
> > >
> > > >
> > > > @@ -64,6 +65,13 @@ bool vtd_check_hiod_accel(IntelIOMMUState *s,
> > >
> > >
> > > VTDHostIOMMUDevice *vtd_hiod,
> > >
> > >
> > > > return false;
> > > > }
> > > >
> > > > + /* Only do the check when host device support PASIDs */
> > > > + if (caps->max_pasid_log2 && s->pasid > hpasid) {
> > > > + error_setg(errp, "PASID bits size %d > host IOMMU PASID bits
> > > > size %d",
> > > > + s->pasid, hpasid);
> > > > + return false;
> > > > + }
> > > >
> > > > Thanks
> > > > Zhenzhong
> > >
> > >
> >
> >
>
>
