Re: [PATCH v5 05/15] intel_iommu: Rename pasid property to "pasid-bits" and define it as type uint8

[Yi Liu <yi.l.liu@intel.com>]

On 5/9/26 12:07, 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 property of
> uint8 type and named 'pasid-bits' 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 <zhenzhong.duan@intel.com>
> Reviewed-by: Clement Mathieu--Drif <clement.mathieu--drif@bull.com>
> Tested-by: Xudong Hao <xudong.hao@intel.com>
> ---

LGTM.

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

>   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..cf275b496e 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-bits", IntelIOMMUState, pasid, 0),
>       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");