On Fri, 2026-02-13 at 08:43 +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 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]))>
> >
> > >
> > > > 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:[clement.mathieu--
> > >
> >
> > [[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:[zhen
> > [[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:[zhenzhong.duan
> > @intel.com](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]/)
>
> 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
> >
>
