On 3/9/26 20:21, Nathan Chen wrote:
From: Nathan Chen <[email protected]>

Introduce smmuv3_accel_auto_finalise() to resolve properties
that are set to 'auto' for accelerated SMMUv3. This helper
function allows properties such as ATS, RIL, SSIDSIZE, and OAS
support to be resolved from host IOMMU values, while avoiding
triggering auto-resolved values for hot-plugged devices.

Auto mode requires at least one cold-plugged device to retrieve
and finalise these properties, and we fail boot if that is not
the case.

IIUC, QEMU will require a minimum of one PCI device if
arm-smmuv3,accel=on is passed on the command line and if the
smmu properties are set to 'auto'.

I would try to enforce this requirement in the realize routine.
Can't we leverage the supports_address_space() handler ? See :


    static bool smmuv3_accel_supports_as(PCIBus *bus, void *opaque, int devfn,
                                         Error **errp)
    {
        PCIDevice *pdev = pci_find_device(bus, pci_bus_num(bus), devfn);
        bool vfio_pci = false;
if (pdev && !smmuv3_accel_pdev_allowed(pdev, &vfio_pci)) {
            if (vfio_pci) {
                error_setg(errp, "vfio-pci endpoint devices without an iommufd "
                           "backend not allowed when using 
arm-smmuv3,accel=on");
    ...


Subsequent patches will make use of this helper to set the
values when we convert the values to OnOffAuto. New auto_mode
and auto_finalised bool members are added to SMMUv3AccelState.
smmuv3_accel_init() will set auto_mode to true when 'auto' is
detected for the accel SMMUv3 properties.
smmuv3_accel_auto_finalise() will set auto_finalised to true
after all 'auto' properties are resolved, and subsequent
calls to this function will return early if auto_finalised is
set to true.

Suggested-by: Shameer Kolothum <[email protected]>
Signed-off-by: Nathan Chen <[email protected]>
---
  hw/arm/smmuv3-accel.c | 38 +++++++++++++++++++++++++++++++++-----
  hw/arm/smmuv3-accel.h |  2 ++
  2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index 17306cd04b..617629bacd 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -35,11 +35,34 @@ static int smmuv3_oas_bits(uint32_t oas)
      return map[oas];
  }
+static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
+                                       struct iommu_hw_info_arm_smmuv3 *info) {
+    SMMUv3AccelState *accel = s->s_accel;
+
+    /* Return if no auto for any or finalised already */
+    if (!accel->auto_mode || accel->auto_finalised) {
+        return;
+    }
+
+    /* We can't update if device is hotplugged */
+    if (DEVICE(pdev)->hotplugged) {
+        warn_report("arm-smmuv3: 'auto' feature property detected, but host "
+                    "value cannot be applied for hot-plugged device; using "
+                    "existing value");

Please add an 'Error **' parameter instead.

+        return;
+    }
+
+    accel->auto_finalised = true;
+}
+
  static bool
  smmuv3_accel_check_hw_compatible(SMMUv3State *s,
                                   struct iommu_hw_info_arm_smmuv3 *info,
+                                 PCIDevice *pdev,
                                   Error **errp)
  {
+    smmuv3_accel_auto_finalise(s, pdev, info);
+
      /* QEMU SMMUv3 supports both linear and 2-level stream tables */
      if (FIELD_EX32(info->idr[0], IDR0, STLEVEL) !=
                  FIELD_EX32(s->idr[0], IDR0, STLEVEL)) {
@@ -124,7 +147,7 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
static bool
  smmuv3_accel_hw_compatible(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
-                           Error **errp)
+                           PCIDevice *pdev, Error **errp)
  {
      struct iommu_hw_info_arm_smmuv3 info;
      uint32_t data_type;
@@ -142,7 +165,7 @@ smmuv3_accel_hw_compatible(SMMUv3State *s, 
HostIOMMUDeviceIOMMUFD *idev,
          return false;
      }
- if (!smmuv3_accel_check_hw_compatible(s, &info, errp)) {
+    if (!smmuv3_accel_check_hw_compatible(s, &info, pdev, errp)) {
          return false;
      }
      return true;
@@ -595,6 +618,7 @@ static bool smmuv3_accel_set_iommu_device(PCIBus *bus, void 
*opaque, int devfn,
      SMMUv3State *s = ARM_SMMUV3(bs);
      SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
      SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus, bus, devfn);
+    PCIDevice *pdev = pci_find_device(bus, pci_bus_num(bus), devfn);
if (!idev) {
          return true;
@@ -613,7 +637,7 @@ static bool smmuv3_accel_set_iommu_device(PCIBus *bus, void 
*opaque, int devfn,
       * Check the host SMMUv3 associated with the dev is compatible with the
       * QEMU SMMUv3 accel.
       */
-    if (!smmuv3_accel_hw_compatible(s, idev, errp)) {
+    if (!smmuv3_accel_hw_compatible(s, idev, pdev, errp)) {
          return false;
      }
@@ -867,8 +891,12 @@ bool smmuv3_accel_attach_gbpa_hwpt(SMMUv3State *s, Error **errp) void smmuv3_accel_reset(SMMUv3State *s)
  {
-     /* Attach a HWPT based on GBPA reset value */
-     smmuv3_accel_attach_gbpa_hwpt(s, NULL);
+    if (s->s_accel && s->s_accel->auto_mode && !s->s_accel->auto_finalised) {
+        error_report("AUTO mode specified but properties not finalised.");

This is not a very friendly message for the user.


+        exit(1);

QEMU should not exit in the reset phase. Can this check be done during
the realize stage ?

Thanks,

C.

+    }
+    /* Attach a HWPT based on GBPA reset value */
+    smmuv3_accel_attach_gbpa_hwpt(s, NULL);
  }
static void smmuv3_accel_as_init(SMMUv3State *s)
diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
index dba6c71de5..3c1cd55714 100644
--- a/hw/arm/smmuv3-accel.h
+++ b/hw/arm/smmuv3-accel.h
@@ -26,6 +26,8 @@ typedef struct SMMUv3AccelState {
      uint32_t bypass_hwpt_id;
      uint32_t abort_hwpt_id;
      QLIST_HEAD(, SMMUv3AccelDevice) device_list;
+    bool auto_mode;
+    bool auto_finalised;
  } SMMUv3AccelState;
typedef struct SMMUS1Hwpt {


Reply via email to