[PATCH v4 21/27] hw/arm/smmuv3-accel: Add a property to specify RIL support

Shameer Kolothum posted 27 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v4 21/27] hw/arm/smmuv3-accel: Add a property to specify RIL support
Posted by Shameer Kolothum 1 month, 2 weeks ago
Currently QEMU SMMUv3 has RIL support by default. But if accelerated mode
is enabled, RIL has to be compatible with host SMMUv3 support.

Add a property so that the user can specify this.

Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 hw/arm/smmuv3-accel.c   | 16 ++++++++++++++--
 hw/arm/smmuv3-accel.h   |  4 ++++
 hw/arm/smmuv3.c         | 13 +++++++++++++
 include/hw/arm/smmuv3.h |  1 +
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index 8396053a6c..e8607b253e 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -79,10 +79,10 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
         return false;
     }
 
-    /* QEMU SMMUv3 supports Range Invalidation by default */
+    /* User can override QEMU SMMUv3 Range Invalidation support */
     val = FIELD_EX32(info->idr[3], IDR3, RIL);
     if (val != FIELD_EX32(s->idr[3], IDR3, RIL)) {
-        error_setg(errp, "Host SUMMUv3 deosn't support Range Invalidation");
+        error_setg(errp, "Host SUMMUv3 differs in Range Invalidation support");
         return false;
     }
 
@@ -634,6 +634,18 @@ static const PCIIOMMUOps smmuv3_accel_ops = {
     .get_msi_address_space = smmuv3_accel_find_msi_as,
 };
 
+void smmuv3_accel_idr_override(SMMUv3State *s)
+{
+    if (!s->accel) {
+        return;
+    }
+
+    /* By default QEMU SMMUv3 has RIL. Update IDR3 if user has disabled it */
+    if (!s->ril) {
+        s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 0);
+    }
+}
+
 /*
  * If the guest reboots and devices are configured for S1+S2, Stage1 must
  * be switched to bypass. Otherwise, QEMU/UEFI may fail when accessing a
diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
index 75f858e34a..357d8352c5 100644
--- a/hw/arm/smmuv3-accel.h
+++ b/hw/arm/smmuv3-accel.h
@@ -49,6 +49,7 @@ bool smmuv3_accel_install_nested_ste_range(SMMUv3State *s, SMMUSIDRange *range,
 bool smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd, SMMUDevice *sdev,
                                 Error **errp);
 void smmuv3_accel_attach_bypass_hwpt(SMMUv3State *s);
+void smmuv3_accel_idr_override(SMMUv3State *s);
 #else
 static inline void smmuv3_accel_init(SMMUv3State *s)
 {
@@ -74,6 +75,9 @@ smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd, SMMUDevice *sdev,
 static inline void smmuv3_accel_attach_bypass_hwpt(SMMUv3State *s)
 {
 }
+static inline void smmuv3_accel_idr_override(SMMUv3State *s)
+{
+}
 #endif
 
 #endif /* HW_ARM_SMMUV3_ACCEL_H */
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index a0f704fc35..0f3a61646a 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -300,6 +300,8 @@ static void smmuv3_init_regs(SMMUv3State *s)
     s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN16K, 1);
     s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN64K, 1);
 
+    smmuv3_accel_idr_override(s);
+
     s->cmdq.base = deposit64(s->cmdq.base, 0, 5, SMMU_CMDQS);
     s->cmdq.prod = 0;
     s->cmdq.cons = 0;
@@ -1925,6 +1927,13 @@ static bool smmu_validate_property(SMMUv3State *s, Error **errp)
         return false;
     }
 #endif
+    if (s->accel) {
+        return true;
+    }
+    if (!s->ril) {
+        error_setg(errp, "ril can only be disabled if accel=on");
+        return false;
+    }
     return true;
 }
 
@@ -2047,6 +2056,8 @@ static const Property smmuv3_properties[] = {
      */
     DEFINE_PROP_STRING("stage", SMMUv3State, stage),
     DEFINE_PROP_BOOL("accel", SMMUv3State, accel, false),
+    /* RIL can be turned off for accel cases */
+    DEFINE_PROP_BOOL("ril", SMMUv3State, ril, true),
 };
 
 static void smmuv3_instance_init(Object *obj)
@@ -2074,6 +2085,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
                                           "Enable SMMUv3 accelerator support."
                                           "Allows host SMMUv3 to be configured "
                                           "in nested mode for vfio-pci dev assignment");
+    object_class_property_set_description(klass, "ril",
+        "Enable/disable range invalidation support");
 }
 
 static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index 874cbaaf32..c555ea684e 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -68,6 +68,7 @@ struct SMMUv3State {
     bool accel;
     struct SMMUv3AccelState  *s_accel;
     Error  *migration_blocker;
+    bool ril;
 };
 
 typedef enum {
-- 
2.43.0


Re: [PATCH v4 21/27] hw/arm/smmuv3-accel: Add a property to specify RIL support
Posted by Eric Auger 2 weeks, 4 days ago
Hi Shameer,

On 9/29/25 3:36 PM, Shameer Kolothum wrote:
> Currently QEMU SMMUv3 has RIL support by default. But if accelerated mode
> is enabled, RIL has to be compatible with host SMMUv3 support.

We may have extended this prop to emulation code too.

While checking I noticed AIDR is set to 3.1 in vIOMMU. Not related to
this patch but should we test the host IOMMU has a compatible
architecture too?
>
> Add a property so that the user can specify this.
>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>  hw/arm/smmuv3-accel.c   | 16 ++++++++++++++--
>  hw/arm/smmuv3-accel.h   |  4 ++++
>  hw/arm/smmuv3.c         | 13 +++++++++++++
>  include/hw/arm/smmuv3.h |  1 +
>  4 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 8396053a6c..e8607b253e 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -79,10 +79,10 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
>          return false;
>      }
>  
> -    /* QEMU SMMUv3 supports Range Invalidation by default */
> +    /* User can override QEMU SMMUv3 Range Invalidation support */
>      val = FIELD_EX32(info->idr[3], IDR3, RIL);
>      if (val != FIELD_EX32(s->idr[3], IDR3, RIL)) {
> -        error_setg(errp, "Host SUMMUv3 deosn't support Range Invalidation");
> +        error_setg(errp, "Host SUMMUv3 differs in Range Invalidation support");
>          return false;
>      }
>  
> @@ -634,6 +634,18 @@ static const PCIIOMMUOps smmuv3_accel_ops = {
>      .get_msi_address_space = smmuv3_accel_find_msi_as,
>  };
>  
> +void smmuv3_accel_idr_override(SMMUv3State *s)
> +{
> +    if (!s->accel) {
> +        return;
> +    }
> +
> +    /* By default QEMU SMMUv3 has RIL. Update IDR3 if user has disabled it */
> +    if (!s->ril) {
> +        s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 0);
> +    }
> +}
> +
>  /*
>   * If the guest reboots and devices are configured for S1+S2, Stage1 must
>   * be switched to bypass. Otherwise, QEMU/UEFI may fail when accessing a
> diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
> index 75f858e34a..357d8352c5 100644
> --- a/hw/arm/smmuv3-accel.h
> +++ b/hw/arm/smmuv3-accel.h
> @@ -49,6 +49,7 @@ bool smmuv3_accel_install_nested_ste_range(SMMUv3State *s, SMMUSIDRange *range,
>  bool smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd, SMMUDevice *sdev,
>                                  Error **errp);
>  void smmuv3_accel_attach_bypass_hwpt(SMMUv3State *s);
> +void smmuv3_accel_idr_override(SMMUv3State *s);
>  #else
>  static inline void smmuv3_accel_init(SMMUv3State *s)
>  {
> @@ -74,6 +75,9 @@ smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd, SMMUDevice *sdev,
>  static inline void smmuv3_accel_attach_bypass_hwpt(SMMUv3State *s)
>  {
>  }
> +static inline void smmuv3_accel_idr_override(SMMUv3State *s)
> +{
> +}
>  #endif
>  
>  #endif /* HW_ARM_SMMUV3_ACCEL_H */
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index a0f704fc35..0f3a61646a 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -300,6 +300,8 @@ static void smmuv3_init_regs(SMMUv3State *s)
>      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN16K, 1);
>      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN64K, 1);
>  
> +    smmuv3_accel_idr_override(s);
> +
>      s->cmdq.base = deposit64(s->cmdq.base, 0, 5, SMMU_CMDQS);
>      s->cmdq.prod = 0;
>      s->cmdq.cons = 0;
> @@ -1925,6 +1927,13 @@ static bool smmu_validate_property(SMMUv3State *s, Error **errp)
>          return false;
>      }
>  #endif
> +    if (s->accel) {
> +        return true;
> +    }
> +    if (!s->ril) {
> +        error_setg(errp, "ril can only be disabled if accel=on");
> +        return false;
> +    }
>      return true;
>  }
>  
> @@ -2047,6 +2056,8 @@ static const Property smmuv3_properties[] = {
>       */
>      DEFINE_PROP_STRING("stage", SMMUv3State, stage),
>      DEFINE_PROP_BOOL("accel", SMMUv3State, accel, false),
> +    /* RIL can be turned off for accel cases */
> +    DEFINE_PROP_BOOL("ril", SMMUv3State, ril, true),
>  };
>  
>  static void smmuv3_instance_init(Object *obj)
> @@ -2074,6 +2085,8 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
>                                            "Enable SMMUv3 accelerator support."
>                                            "Allows host SMMUv3 to be configured "
>                                            "in nested mode for vfio-pci dev assignment");
> +    object_class_property_set_description(klass, "ril",
> +        "Enable/disable range invalidation support");
if we decide to enable the prop only along with accel, might be worth to
document it here.
>  }
>  
>  static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index 874cbaaf32..c555ea684e 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -68,6 +68,7 @@ struct SMMUv3State {
>      bool accel;
>      struct SMMUv3AccelState  *s_accel;
>      Error  *migration_blocker;
> +    bool ril;
>  };
>  
>  typedef enum {
Thanks

Eric


Re: [PATCH v4 21/27] hw/arm/smmuv3-accel: Add a property to specify RIL support
Posted by Zhangfei Gao 4 weeks ago
On Mon, 29 Sept 2025 at 21:40, Shameer Kolothum <skolothumtho@nvidia.com> wrote:
>
> Currently QEMU SMMUv3 has RIL support by default. But if accelerated mode
> is enabled, RIL has to be compatible with host SMMUv3 support.
>
> Add a property so that the user can specify this.
>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>

If ril=off is not specified, the guest kernel will not boot up, is
this expected?

Fail with log:
qemu-system-aarch64: -device
vfio-pci,host=0000:75:00.1,bus=pcie.0,iommufd=iommufd0:
 vfio 0000:75:00.1: Failed to set vIOMMU: Host SUMMUv3 differs in
Range Invalidation support

Thanks
RE: [PATCH v4 21/27] hw/arm/smmuv3-accel: Add a property to specify RIL support
Posted by Shameer Kolothum 4 weeks ago

> -----Original Message-----
> From: Zhangfei Gao <zhangfei.gao@linaro.org>
> Sent: 17 October 2025 09:49
> 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>; Nicolin Chen <nicolinc@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;
> zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> shameerkolothum@gmail.com
> Subject: Re: [PATCH v4 21/27] hw/arm/smmuv3-accel: Add a property to
> specify RIL support
> 
> External email: Use caution opening links or attachments
> 
> 
> On Mon, 29 Sept 2025 at 21:40, Shameer Kolothum
> <skolothumtho@nvidia.com> wrote:
> >
> > Currently QEMU SMMUv3 has RIL support by default. But if accelerated
> > mode is enabled, RIL has to be compatible with host SMMUv3 support.
> >
> > Add a property so that the user can specify this.
> >
> > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> 
> If ril=off is not specified, the guest kernel will not boot up, is this expected?
> 
> Fail with log:
> qemu-system-aarch64: -device
> vfio-pci,host=0000:75:00.1,bus=pcie.0,iommufd=iommufd0:
>  vfio 0000:75:00.1: Failed to set vIOMMU: Host SUMMUv3 differs in Range
> Invalidation support

It will, if the host SMMUv3 doesn't have RIL. Please check that.
This is because when a device is attached to vSMMU , a compatibility check
is performed to ensure that the SMUUv3 features visible to guest are compatible
with host SMMUv3 it is tied to. By default, QEMU SMMUV3 reports RIL to Guest.

The "ril" option is provided so that user can specify this in case incompatibility.

Thanks,
Shameer
Re: [PATCH v4 21/27] hw/arm/smmuv3-accel: Add a property to specify RIL support
Posted by Zhangfei Gao 4 weeks ago
On Fri, 17 Oct 2025 at 17:40, Shameer Kolothum <skolothumtho@nvidia.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Zhangfei Gao <zhangfei.gao@linaro.org>
> > Sent: 17 October 2025 09:49
> > 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>; Nicolin Chen <nicolinc@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;
> > zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> > shameerkolothum@gmail.com
> > Subject: Re: [PATCH v4 21/27] hw/arm/smmuv3-accel: Add a property to
> > specify RIL support
> >
> > External email: Use caution opening links or attachments
> >
> >
> > On Mon, 29 Sept 2025 at 21:40, Shameer Kolothum
> > <skolothumtho@nvidia.com> wrote:
> > >
> > > Currently QEMU SMMUv3 has RIL support by default. But if accelerated
> > > mode is enabled, RIL has to be compatible with host SMMUv3 support.
> > >
> > > Add a property so that the user can specify this.
> > >
> > > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> >
> > If ril=off is not specified, the guest kernel will not boot up, is this expected?
> >
> > Fail with log:
> > qemu-system-aarch64: -device
> > vfio-pci,host=0000:75:00.1,bus=pcie.0,iommufd=iommufd0:
> >  vfio 0000:75:00.1: Failed to set vIOMMU: Host SUMMUv3 differs in Range
> > Invalidation support
>
> It will, if the host SMMUv3 doesn't have RIL. Please check that.

Yes, the host SMMUv3 doesn't have RIL in my case.

> This is because when a device is attached to vSMMU , a compatibility check
> is performed to ensure that the SMUUv3 features visible to guest are compatible
> with host SMMUv3 it is tied to. By default, QEMU SMMUV3 reports RIL to Guest.

OK, got it, using ioctl to get host info and check the compatibility.
>
> The "ril" option is provided so that user can specify this in case incompatibility.

OK, Thanks
Re: [PATCH v4 21/27] hw/arm/smmuv3-accel: Add a property to specify RIL support
Posted by Jonathan Cameron via 1 month, 2 weeks ago
On Mon, 29 Sep 2025 14:36:37 +0100
Shameer Kolothum <skolothumtho@nvidia.com> wrote:

> Currently QEMU SMMUv3 has RIL support by default. But if accelerated mode
> is enabled, RIL has to be compatible with host SMMUv3 support.
> 
> Add a property so that the user can specify this.
> 
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>

One trivial comment inline.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> ---
>  hw/arm/smmuv3-accel.c   | 16 ++++++++++++++--
>  hw/arm/smmuv3-accel.h   |  4 ++++
>  hw/arm/smmuv3.c         | 13 +++++++++++++
>  include/hw/arm/smmuv3.h |  1 +
>  4 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 8396053a6c..e8607b253e 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c

> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index a0f704fc35..0f3a61646a 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -300,6 +300,8 @@ static void smmuv3_init_regs(SMMUv3State *s)
>      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN16K, 1);
>      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN64K, 1);
>  
> +    smmuv3_accel_idr_override(s);
> +
>      s->cmdq.base = deposit64(s->cmdq.base, 0, 5, SMMU_CMDQS);
>      s->cmdq.prod = 0;
>      s->cmdq.cons = 0;
> @@ -1925,6 +1927,13 @@ static bool smmu_validate_property(SMMUv3State *s, Error **errp)
>          return false;
>      }
>  #endif
> +    if (s->accel) {
> +        return true;
> +    }

Feels to me that an early exit here is going to be slightly odd if other propoerties are added
later as they'll have to be added at a specific point in the function. Perhaps.

    if (!s->accel) {
        if (!s->ril) {
            ....
        }
   }

   return true;

is going to be easier to extend.

> +    if (!s->ril) {
> +        error_setg(errp, "ril can only be disabled if accel=on");
> +        return false;
> +    }
>      return true;
>  }