Good suggestion, will add:
@@ -5551,6 +5551,11 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error
**errp)
error_setg(errp, "Need to set scalable mode for PASID");
return false;
}
+ if (s->pasid > PCI_EXT_CAP_PASID_MAX_WIDTH) {
+ error_setg(errp, "PASID width %d, exceed Max PASID Width %d allowed "
+ "in PCI spec", s->pasid, PCI_EXT_CAP_PASID_MAX_WIDTH);
+ return false;
+ }
Thanks
Zhenzhong
>-----Original Message-----
>From: Clement Mathieu--Drif <[email protected]>
>Subject: Re: [RFC PATCH 05/14] intel_iommu: Change pasid property from bool to
>uint8
>
>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 <[[clement.mathieu--
>[email protected]](mailto:clement.mathieu--
>[email protected])](mailto:[clement.mathieu--
>[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:dr
>[email protected]))](mailto:[[clement.mathieu--
>[email protected]](mailto:clement.mathieu--
>[email protected])](mailto:[clement.mathieu--
>[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:d
>[email protected]))](mailto:[[[email protected]](mailto:[email protected])](mailto:[dr
>[email protected]](mailto:[email protected])))](mailto:[clement.mathieu--
>> > > >
>> > > >
>> > >
>> > >
>> > >
>[[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:dr
>[email protected]))](mailto:[[clement.mathieu--
>[email protected]](mailto:clement.mathieu--
>[email protected])](mailto:[clement.mathieu--
>[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:[zh
>[email protected]](mailto:[email protected]))](mailto:[[zhenzho
>[email protected]](mailto:[email protected])](mailto:[zhenzhong.duan
>@intel.com](mailto:[email protected])))](mailto:[zhen
>> > >
>[[[email protected]](mailto:[email protected])](mailto:[zhong.duan@in
>tel.com](mailto:[email protected]))](mailto:[[[email protected]](m
>ailto:[email protected])](mailto:[[email protected]](mailto:zh
>[email protected]))))](mailto:[zhen
>> > >
>> > >
>> > > >
>> > > >
>> > > > >
>> > > >
>> > > >
>> > >
>> > >
>> > >
>[[[[email protected]](mailto:[email protected])](mailto:[zhong.duan@i
>ntel.com](mailto:[email protected]))](mailto:[[[email protected]](mailt
>o:[email protected])](mailto:[[email protected]](mailto:zhong.duan@in
>tel.com)))](mailto:[zhenzhong.duan
>> > >
>@intel.com](mailto:[[[email protected]](mailto:[email protected]
>om)](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/20241004155721.2154626-4-
>[email protected]/](https://lore.kernel.org/qemu-
>devel/20241004155721.2154626-4-
>[email protected]/)](https://lore.kernel.org/qemu-
>devel/20241004155721.2154626-4-
>[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
>> > >
>> > >
>> >
>> >
>>
>>
