>-----Original Message-----
>From: Liu, Yi L <[email protected]>
>Subject: Re: [PATCH v4 05/15] intel_iommu: Change pasid property from bool to
>uint8
>
>On 4/30/26 15:13, 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.
>>
>> PASID bits size should also be no more than 20 bits according to PCI spec.
>>
>> Signed-off-by: Zhenzhong Duan <[email protected]>
>> Reviewed-by: Clement Mathieu--Drif <[email protected]>
>> Tested-by: Xudong Hao <[email protected]>
>> ---
>> hw/i386/intel_iommu_internal.h | 2 +-
>> include/hw/i386/intel_iommu.h | 2 +-
>> hw/i386/intel_iommu.c | 11 +++++++++--
>> 3 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
>> index 11a53aa369..db4f186a3e 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -195,7 +195,7 @@
>> #define VTD_ECAP_MHMV (15ULL << 20)
>> #define VTD_ECAP_SRS (1ULL << 31)
>> #define VTD_ECAP_NWFS (1ULL << 33)
>> -#define VTD_ECAP_PSS (7ULL << 35) /* limit: MemTxAttrs::pid
>> */
>> +#define VTD_ECAP_SET_PSS(x, v) ((x)->ecap = deposit64((x)->ecap, 35,
>> 5, v))
>> #define VTD_ECAP_PASID (1ULL << 40)
>> #define VTD_ECAP_PDS (1ULL << 42)
>> #define VTD_ECAP_SMTS (1ULL << 43)
>> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
>> index e44ce31841..95c76015e4 100644
>> --- a/include/hw/i386/intel_iommu.h
>> +++ b/include/hw/i386/intel_iommu.h
>> @@ -314,7 +314,7 @@ struct IntelIOMMUState {
>> bool intr_eime; /* Extended interrupt mode enabled */
>> OnOffAuto intr_eim; /* Toggle for EIM cabability */
>> uint8_t aw_bits; /* Host/IOVA address width (in bits) */
>> - 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 b784c5f10a..d1946e3a59 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -4203,7 +4203,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),
>
>how about pasid-bits, it suits aw-bits as well.
Yea, sounds good.
>
>> DEFINE_PROP_BOOL("svm", IntelIOMMUState, svm, false),
>> DEFINE_PROP_BOOL("stale-tm", IntelIOMMUState, stale_tm, false),
>> DEFINE_PROP_BOOL("fs1gp", IntelIOMMUState, fs1gp, true),
>> @@ -5045,7 +5045,8 @@ static void vtd_cap_init(IntelIOMMUState *s)
>> }
>>
>> if (s->pasid) {
>> - s->ecap |= VTD_ECAP_PASID | VTD_ECAP_PSS;
>> + VTD_ECAP_SET_PSS(s, s->pasid - 1);
>> + s->ecap |= VTD_ECAP_PASID;
>> }
>> }
>>
>> @@ -5586,6 +5587,12 @@ static bool vtd_decide_config(IntelIOMMUState *s,
>Error **errp)
>> return false;
>> }
>>
>> + if (s->pasid > PCI_EXT_CAP_PASID_MAX_WIDTH) {
>> + error_setg(errp, "PASID width %d exceeds Max PASID Width %d allowed
>> "
>> + "in PCI spec", s->pasid, PCI_EXT_CAP_PASID_MAX_WIDTH);
>> + return false;
>> + }
>> +
>> if (s->svm) {
>> if (!x86_iommu->dt_supported) {
>> error_setg(errp, "Need to set device IOTLB for svm");
