[PATCH v6 33/33] hw/arm/smmuv3-accel: Add support for PASID enable

Shameer Kolothum posted 33 patches 3 weeks, 2 days ago
[PATCH v6 33/33] hw/arm/smmuv3-accel: Add support for PASID enable
Posted by Shameer Kolothum 3 weeks, 2 days ago
QEMU SMMUv3 currently forces SSID (Substream ID) to zero. One key use case
for accelerated mode is Shared Virtual Addressing (SVA), which requires
SSID support so the guest can maintain multiple context descriptors per
substream ID.

Provide an option for user to enable PASID support. A SSIDSIZE of 16
is currently used as default.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 hw/arm/smmuv3-accel.c    | 23 ++++++++++++++++++++++-
 hw/arm/smmuv3-internal.h |  1 +
 hw/arm/smmuv3.c          | 13 +++++++++++--
 include/hw/arm/smmuv3.h  |  1 +
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index 254d29ee2d..dc0f61e841 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -67,6 +67,12 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
         error_setg(errp, "Host SMMUv3 SIDSIZE not compatible");
         return false;
     }
+    /* If user enables PASID support(pasid=on), QEMU sets SSIDSIZE to 16 */
+    if (FIELD_EX32(info->idr[1], IDR1, SSIDSIZE) <
+                FIELD_EX32(s->idr[1], IDR1, SSIDSIZE)) {
+        error_setg(errp, "Host SMMUv3 SSIDSIZE not compatible");
+        return false;
+    }
 
     /* User can disable QEMU SMMUv3 Range Invalidation support */
     if (FIELD_EX32(info->idr[3], IDR3, RIL) >
@@ -643,7 +649,14 @@ static uint64_t smmuv3_accel_get_viommu_flags(void *opaque)
      * The real HW nested support should be reported from host SMMUv3 and if
      * it doesn't, the nesting parent allocation will fail anyway in VFIO core.
      */
-    return VIOMMU_FLAG_WANT_NESTING_PARENT;
+    uint64_t flags = VIOMMU_FLAG_WANT_NESTING_PARENT;
+    SMMUState *bs = opaque;
+    SMMUv3State *s = ARM_SMMUV3(bs);
+
+    if (s->pasid) {
+        flags |= VIOMMU_FLAG_PASID_SUPPORTED;
+    }
+    return flags;
 }
 
 static const PCIIOMMUOps smmuv3_accel_ops = {
@@ -671,6 +684,14 @@ void smmuv3_accel_idr_override(SMMUv3State *s)
     if (s->oas == 48) {
         s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, SMMU_IDR5_OAS_48);
     }
+
+    /*
+     * By default QEMU SMMUv3 has no PASID(SSID) support. Update IDR1 if user
+     * has enabled it.
+     */
+    if (s->pasid) {
+        s->idr[1] = FIELD_DP32(s->idr[1], IDR1, SSIDSIZE, SMMU_IDR1_SSIDSIZE);
+    }
 }
 
 /* Based on SMUUv3 GPBA.ABORT configuration, attach a corresponding HWPT */
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 0f44a4e1d3..e45aad27f7 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -81,6 +81,7 @@ REG32(IDR1,                0x4)
     FIELD(IDR1, ECMDQ,        31, 1)
 
 #define SMMU_IDR1_SIDSIZE 16
+#define SMMU_IDR1_SSIDSIZE 16
 #define SMMU_CMDQS   19
 #define SMMU_EVENTQS 19
 
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index a7bd4eeb77..763f069a35 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -611,9 +611,11 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
         }
     }
 
-    if (STE_S1CDMAX(ste) != 0) {
+    /* If pasid not enabled, can't support multiple CDs */
+    if (!s->pasid && STE_S1CDMAX(ste) != 0) {
         qemu_log_mask(LOG_UNIMP,
-                      "SMMUv3 does not support multiple context descriptors yet\n");
+              "SMMUv3: multiple S1 context descriptors require PASID support. "
+              "Enable PASID with pasid=on (supported only with accel=on)\n");
         goto bad_ste;
     }
 
@@ -1950,6 +1952,10 @@ static bool smmu_validate_property(SMMUv3State *s, Error **errp)
             error_setg(errp, "OAS can only be set to 44 bits if accel=off");
             return false;
         }
+        if (s->pasid) {
+            error_setg(errp, "pasid can only be enabled if accel=on");
+            return false;
+        }
         return true;
     }
 
@@ -2084,6 +2090,7 @@ static const Property smmuv3_properties[] = {
     DEFINE_PROP_BOOL("ril", SMMUv3State, ril, true),
     DEFINE_PROP_BOOL("ats", SMMUv3State, ats, false),
     DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
+    DEFINE_PROP_BOOL("pasid", SMMUv3State, pasid, false),
 };
 
 static void smmuv3_instance_init(Object *obj)
@@ -2117,6 +2124,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
     object_class_property_set_description(klass, "oas",
         "Specify Output Address Size (for accel =on). Supported values "
         "are 44 or 48 bits. Defaults to 44 bits");
+    object_class_property_set_description(klass, "pasid",
+        "Enable/disable PASID support (for accel=on)");
 }
 
 static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index d488a39cd0..2d4970fe19 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -72,6 +72,7 @@ struct SMMUv3State {
     bool ril;
     bool ats;
     uint8_t oas;
+    bool pasid;
 };
 
 typedef enum {
-- 
2.43.0
Re: [PATCH v6 33/33] hw/arm/smmuv3-accel: Add support for PASID enable
Posted by Nicolin Chen 3 weeks, 2 days ago
On Thu, Nov 20, 2025 at 01:22:13PM +0000, Shameer Kolothum wrote:
> +++ b/hw/arm/smmuv3-accel.c
> @@ -67,6 +67,12 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
>          error_setg(errp, "Host SMMUv3 SIDSIZE not compatible");
>          return false;
>      }
> +    /* If user enables PASID support(pasid=on), QEMU sets SSIDSIZE to 16 */
> +    if (FIELD_EX32(info->idr[1], IDR1, SSIDSIZE) <
> +                FIELD_EX32(s->idr[1], IDR1, SSIDSIZE)) {
> +        error_setg(errp, "Host SMMUv3 SSIDSIZE not compatible");
> +        return false;
> +    }

I think we can print the values: host vs VM. And at SIDSIZE above
as well.

> @@ -2084,6 +2090,7 @@ static const Property smmuv3_properties[] = {
>      DEFINE_PROP_BOOL("ril", SMMUv3State, ril, true),
>      DEFINE_PROP_BOOL("ats", SMMUv3State, ats, false),
>      DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
> +    DEFINE_PROP_BOOL("pasid", SMMUv3State, pasid, false),
>  };

Instead of doing a boolean "pasid", perhaps ssidsize and sidsize
should be configurable. Then, user can follow the not-compatible
print to set correct SSIDSIZE and SIDSIZE.

They can also choose to set a higher value if underlying SMMU HW
supports that.

Otherwise,

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
RE: [PATCH v6 33/33] hw/arm/smmuv3-accel: Add support for PASID enable
Posted by Shameer Kolothum 3 weeks, 1 day ago

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: 20 November 2025 22:10
> To: Shameer Kolothum <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> <jgg@nvidia.com>; ddutile@redhat.com; berrange@redhat.com; Nathan
> Chen <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 33/33] hw/arm/smmuv3-accel: Add support for PASID
> enable
> 
> On Thu, Nov 20, 2025 at 01:22:13PM +0000, Shameer Kolothum wrote:
> > +++ b/hw/arm/smmuv3-accel.c
> > @@ -67,6 +67,12 @@ smmuv3_accel_check_hw_compatible(SMMUv3State
> *s,
> >          error_setg(errp, "Host SMMUv3 SIDSIZE not compatible");
> >          return false;
> >      }
> > +    /* If user enables PASID support(pasid=on), QEMU sets SSIDSIZE to 16 */
> > +    if (FIELD_EX32(info->idr[1], IDR1, SSIDSIZE) <
> > +                FIELD_EX32(s->idr[1], IDR1, SSIDSIZE)) {
> > +        error_setg(errp, "Host SMMUv3 SSIDSIZE not compatible");
> > +        return false;
> > +    }
> 
> I think we can print the values: host vs VM. And at SIDSIZE above
> as well.
> 
> > @@ -2084,6 +2090,7 @@ static const Property smmuv3_properties[] = {
> >      DEFINE_PROP_BOOL("ril", SMMUv3State, ril, true),
> >      DEFINE_PROP_BOOL("ats", SMMUv3State, ats, false),
> >      DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
> > +    DEFINE_PROP_BOOL("pasid", SMMUv3State, pasid, false),
> >  };
> 
> Instead of doing a boolean "pasid", perhaps ssidsize and sidsize
> should be configurable. Then, user can follow the not-compatible
> print to set correct SSIDSIZE and SIDSIZE.

Do we really need that? Currently both are set to 16 which means 64K
values are supported. I think we can make it configurable when any
usecase  with >64K requirement comes up.
 
Thanks,
Shameer
Re: [PATCH v6 33/33] hw/arm/smmuv3-accel: Add support for PASID enable
Posted by Nicolin Chen 3 weeks, 1 day ago
On Fri, Nov 21, 2025 at 02:22:21AM -0800, Shameer Kolothum wrote:
> > > @@ -2084,6 +2090,7 @@ static const Property smmuv3_properties[] = {
> > >      DEFINE_PROP_BOOL("ril", SMMUv3State, ril, true),
> > >      DEFINE_PROP_BOOL("ats", SMMUv3State, ats, false),
> > >      DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
> > > +    DEFINE_PROP_BOOL("pasid", SMMUv3State, pasid, false),
> > >  };
> > 
> > Instead of doing a boolean "pasid", perhaps ssidsize and sidsize
> > should be configurable. Then, user can follow the not-compatible
> > print to set correct SSIDSIZE and SIDSIZE.
> 
> Do we really need that? Currently both are set to 16 which means 64K
> values are supported. I think we can make it configurable when any
> usecase  with >64K requirement comes up.

For upper boundary, we have SoCs with SSIDSIZE=0x14 i.e. 20. I
am not sure how user space would use this range, but I feel it
is better not to cap it. And SIDSIZE=16 is probably way enough
given that QEMU only has one PCI Bus domain.

For lower boundary, SMMUv3 spec defines:
  SSIDSIZE, bits [10:6]
  Max bits of SubstreamID.
  Valid range 0 to 20 inclusive, 0 meaning no substreams are supported.
and
  SIDSIZE, bits [5:0]
  Max bits of StreamID.
  This value is between 0 and 32 inclusive.
  Note: 0 is a legal value. In this case the SMMU supports one stream.

We apply a hard requirement that a host value must >= VM value.
This might not work for hardware that has smaller numbers.

Yes, we may add an SIDSIZE when somebody actually wants it. But
the "bool pasid" would be very useless when we add an SSIDSIZE.

So, I think it's nicer to define "uint32 ssidsize" in the first
place, which also aligns the QEMU parameter with the HW naming.

Nicolin
RE: [PATCH v6 33/33] hw/arm/smmuv3-accel: Add support for PASID enable
Posted by Shameer Kolothum 2 weeks, 5 days ago

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: 21 November 2025 17:51
> To: Shameer Kolothum <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> <jgg@nvidia.com>; ddutile@redhat.com; berrange@redhat.com; Nathan
> Chen <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 33/33] hw/arm/smmuv3-accel: Add support for PASID
> enable
> 
> On Fri, Nov 21, 2025 at 02:22:21AM -0800, Shameer Kolothum wrote:
> > > > @@ -2084,6 +2090,7 @@ static const Property smmuv3_properties[] =
> {
> > > >      DEFINE_PROP_BOOL("ril", SMMUv3State, ril, true),
> > > >      DEFINE_PROP_BOOL("ats", SMMUv3State, ats, false),
> > > >      DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
> > > > +    DEFINE_PROP_BOOL("pasid", SMMUv3State, pasid, false),
> > > >  };
> > >
> > > Instead of doing a boolean "pasid", perhaps ssidsize and sidsize
> > > should be configurable. Then, user can follow the not-compatible
> > > print to set correct SSIDSIZE and SIDSIZE.
> >
> > Do we really need that? Currently both are set to 16 which means 64K
> > values are supported. I think we can make it configurable when any
> > usecase  with >64K requirement comes up.
> 
> For upper boundary, we have SoCs with SSIDSIZE=0x14 i.e. 20. I
> am not sure how user space would use this range, 

Right. My assumption was that nobody uses that higher range per SID
now in real applications even if hardware supports that.

but I feel it
> is better not to cap it. And SIDSIZE=16 is probably way enough
> given that QEMU only has one PCI Bus domain.
>
> For lower boundary, SMMUv3 spec defines:
>   SSIDSIZE, bits [10:6]
>   Max bits of SubstreamID.
>   Valid range 0 to 20 inclusive, 0 meaning no substreams are supported.
> and
>   SIDSIZE, bits [5:0]
>   Max bits of StreamID.
>   This value is between 0 and 32 inclusive.
>   Note: 0 is a legal value. In this case the SMMU supports one stream.
> 
> We apply a hard requirement that a host value must >= VM value.
> This might not work for hardware that has smaller numbers.

I guess 16 is a reasonable low value for all hardware's out there now,
at least, I am not aware of anyone implementing less than that.
 
> Yes, we may add an SIDSIZE when somebody actually wants it. But
> the "bool pasid" would be very useless when we add an SSIDSIZE.

That make sense.

> So, I think it's nicer to define "uint32 ssidsize" in the first
> place, which also aligns the QEMU parameter with the HW naming.

Ok. Jason also made this argument that it is better to align it with IDR 
names here. So I will change this to "ssidsize" so that we have all the 
configurable properties align with ID register field names.

Thanks,
Shameer
Re: [PATCH v6 33/33] hw/arm/smmuv3-accel: Add support for PASID enable
Posted by Jason Gunthorpe 3 weeks, 1 day ago
On Fri, Nov 21, 2025 at 09:50:50AM -0800, Nicolin Chen wrote:
> On Fri, Nov 21, 2025 at 02:22:21AM -0800, Shameer Kolothum wrote:
> > > > @@ -2084,6 +2090,7 @@ static const Property smmuv3_properties[] = {
> > > >      DEFINE_PROP_BOOL("ril", SMMUv3State, ril, true),
> > > >      DEFINE_PROP_BOOL("ats", SMMUv3State, ats, false),
> > > >      DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
> > > > +    DEFINE_PROP_BOOL("pasid", SMMUv3State, pasid, false),
> > > >  };
> > > 
> > > Instead of doing a boolean "pasid", perhaps ssidsize and sidsize
> > > should be configurable. Then, user can follow the not-compatible
> > > print to set correct SSIDSIZE and SIDSIZE.
> > 
> > Do we really need that? Currently both are set to 16 which means 64K
> > values are supported. I think we can make it configurable when any
> > usecase  with >64K requirement comes up.
> 
> For upper boundary, we have SoCs with SSIDSIZE=0x14 i.e. 20. I
> am not sure how user space would use this range, but I feel it
> is better not to cap it. And SIDSIZE=16 is probably way enough
> given that QEMU only has one PCI Bus domain.

Yeah, it should be ssize not pasid.

The use case of these values is exactly defining a SMMU instance type
so that it can migration between different physical HW. So long as
physical can implement the instance type.

Thus you broadly want to make the iidrs configurable in the exact
spec language of the iidrs, IMHO.

Jason
Re: [PATCH v6 33/33] hw/arm/smmuv3-accel: Add support for PASID enable
Posted by Nicolin Chen 3 weeks, 1 day ago
On Fri, Nov 21, 2025 at 09:50:53AM -0800, Nicolin Chen wrote:
> So, I think it's nicer to define "uint32 ssidsize" in the first

Correction: "u8 ssidsize".