>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v1 05/13] intel_iommu: Change pasid property from bool to
>uint8
>
>On 3/6/26 11:43, 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.
>>
>
>should we keep the "x-" prefix since this is new pasid support?
Copied Daniel's comments about "x-" prefix:
The purpose of using an 'x-' prefix for properties in QEMU is to declare
that they are subject to change with no warning, so we are free to change
them without any deprecation
I think uint8 type is large enough as pasid bits size in foreseeable future.
So we will not need to change or extend pasid property, so 'pasid' property looks fine for me.
Thanks
Zhenzhong
>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>> hw/i386/intel_iommu_internal.h | 2 +-
>> include/hw/i386/intel_iommu.h | 2 +-
>> hw/i386/intel_iommu.c | 5 +++--
>> 3 files changed, 5 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 54c2b6b77a..bb957b93e0 100644
>> --- a/include/hw/i386/intel_iommu.h
>> +++ b/include/hw/i386/intel_iommu.h
>> @@ -315,7 +315,7 @@ struct IntelIOMMUState {
>> OnOffAuto intr_eim; /* Toggle for EIM cabability */
>> 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 d24ba989bf..e5b9689fae 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),
>> DEFINE_PROP_BOOL("svm", IntelIOMMUState, svm, false),
>> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
>> DEFINE_PROP_BOOL("stale-tm", IntelIOMMUState, stale_tm, false),
>> @@ -5046,7 +5046,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;
>> }
>> }
>>