[PATCH v6 0/5] vEVENTQ support for accelerated SMMUv3 devices

[PATCH v6 0/5] vEVENTQ support for accelerated SMMUv3 devices

[Shameer Kolothum]

Hi,

Changes from v5:
 https://lore.kernel.org/qemu-devel/20260211083415.133534-1-skolothumtho@nvidia.com/
 
 -Addressed feedback and picked up tags. Thanks!
 -Decoupled alloc_veventq on smmuv3_cmdq_consume() (patch  #3)
 -Updated error messages (patch #5)

Please take a look and let me know.

Thanks,
Shameer

---
Background(From v2):

When accel=on is enabled for an SMMUv3 instance, the host hardware SMMUv3
may generate Stage-1 (S1) event notifications that are intended for the
vIOMMU instance in userspace.

This series adds QEMU support for receiving such host events through the
vEVENTQ interface and propagating them to the guest. The implementation
uses the vEVENTQ support provided by the IOMMUFD subsystem in the kernel.

I have tested this on a Grace platform using some hacks to generate
events. Further testing and feedback are welcome.
---

Nicolin Chen (2):
  backends/iommufd: Introduce iommufd_backend_alloc_veventq
  hw/arm/smmuv3-accel: Allocate vEVENTQ for accelerated SMMUv3 devices

Shameer Kolothum (3):
  hw/arm/smmuv3-accel: Add viommu free helper
  hw/arm/smmuv3: Introduce a helper function for event propagation
  hw/arm/smmuv3-accel: Read and propagate host vIOMMU events

 backends/iommufd.c       |  31 +++++++++
 backends/trace-events    |   1 +
 hw/arm/smmuv3-accel.c    | 146 ++++++++++++++++++++++++++++++++++++---
 hw/arm/smmuv3-accel.h    |   8 +++
 hw/arm/smmuv3-internal.h |   4 ++
 hw/arm/smmuv3.c          |  38 ++++++----
 hw/arm/trace-events      |   2 +-
 include/system/iommufd.h |  12 ++++
 8 files changed, 220 insertions(+), 22 deletions(-)

-- 
2.43.0

[PATCH v6 1/5] backends/iommufd: Introduce iommufd_backend_alloc_veventq

[Shameer Kolothum]

From: Nicolin Chen <nicolinc@nvidia.com>

Add a new helper for IOMMU_VEVENTQ_ALLOC ioctl to allocate a virtual event
queue (vEVENTQ) for a vIOMMU object.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 backends/iommufd.c       | 31 +++++++++++++++++++++++++++++++
 backends/trace-events    |  1 +
 include/system/iommufd.h | 12 ++++++++++++
 3 files changed, 44 insertions(+)

diff --git a/backends/iommufd.c b/backends/iommufd.c
index 13822df82f..acfab907c0 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -504,6 +504,37 @@ bool iommufd_backend_alloc_vdev(IOMMUFDBackend *be, uint32_t dev_id,
     return true;
 }
 
+bool iommufd_backend_alloc_veventq(IOMMUFDBackend *be, uint32_t viommu_id,
+                                   uint32_t type, uint32_t depth,
+                                   uint32_t *out_veventq_id,
+                                   uint32_t *out_veventq_fd, Error **errp)
+{
+    int ret;
+    struct iommu_veventq_alloc alloc_veventq = {
+        .size = sizeof(alloc_veventq),
+        .flags = 0,
+        .type = type,
+        .veventq_depth = depth,
+        .viommu_id = viommu_id,
+    };
+
+    ret = ioctl(be->fd, IOMMU_VEVENTQ_ALLOC, &alloc_veventq);
+
+    trace_iommufd_viommu_alloc_eventq(be->fd, viommu_id, type,
+                                      alloc_veventq.out_veventq_id,
+                                      alloc_veventq.out_veventq_fd, ret);
+    if (ret) {
+        error_setg_errno(errp, errno, "IOMMU_VEVENTQ_ALLOC failed");
+        return false;
+    }
+
+    g_assert(out_veventq_id);
+    g_assert(out_veventq_fd);
+    *out_veventq_id = alloc_veventq.out_veventq_id;
+    *out_veventq_fd = alloc_veventq.out_veventq_fd;
+    return true;
+}
+
 bool host_iommu_device_iommufd_attach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
                                            uint32_t hwpt_id, Error **errp)
 {
diff --git a/backends/trace-events b/backends/trace-events
index 8dc64a20d3..b9365113e7 100644
--- a/backends/trace-events
+++ b/backends/trace-events
@@ -23,6 +23,7 @@ iommufd_backend_get_dirty_bitmap(int iommufd, uint32_t hwpt_id, uint64_t iova, u
 iommufd_backend_invalidate_cache(int iommufd, uint32_t id, uint32_t data_type, uint32_t entry_len, uint32_t entry_num, uint32_t done_num, uint64_t data_ptr, int ret) " iommufd=%d id=%u data_type=%u entry_len=%u entry_num=%u done_num=%u data_ptr=0x%"PRIx64" (%d)"
 iommufd_backend_alloc_viommu(int iommufd, uint32_t dev_id, uint32_t type, uint32_t hwpt_id, uint32_t viommu_id, int ret) " iommufd=%d type=%u dev_id=%u hwpt_id=%u viommu_id=%u (%d)"
 iommufd_backend_alloc_vdev(int iommufd, uint32_t dev_id, uint32_t viommu_id, uint64_t virt_id, uint32_t vdev_id, int ret) " iommufd=%d dev_id=%u viommu_id=%u virt_id=0x%"PRIx64" vdev_id=%u (%d)"
+iommufd_viommu_alloc_eventq(int iommufd, uint32_t viommu_id, uint32_t type, uint32_t veventq_id, uint32_t veventq_fd, int ret) " iommufd=%d viommu_id=%u type=%u veventq_id=%u veventq_fd=%u (%d)"
 
 # igvm-cfg.c
 igvm_reset_enter(int type) "type=%u"
diff --git a/include/system/iommufd.h b/include/system/iommufd.h
index 80d72469a9..e4ca16da70 100644
--- a/include/system/iommufd.h
+++ b/include/system/iommufd.h
@@ -56,6 +56,13 @@ typedef struct IOMMUFDVdev {
     uint32_t virt_id;  /* virtual device ID */
 } IOMMUFDVdev;
 
+/* Virtual event queue interface for a vIOMMU */
+typedef struct IOMMUFDVeventq {
+    IOMMUFDViommu *viommu;
+    uint32_t veventq_id;
+    uint32_t veventq_fd;
+} IOMMUFDVeventq;
+
 bool iommufd_backend_connect(IOMMUFDBackend *be, Error **errp);
 void iommufd_backend_disconnect(IOMMUFDBackend *be);
 
@@ -86,6 +93,11 @@ bool iommufd_backend_alloc_vdev(IOMMUFDBackend *be, uint32_t dev_id,
                                 uint32_t viommu_id, uint64_t virt_id,
                                 uint32_t *out_vdev_id, Error **errp);
 
+bool iommufd_backend_alloc_veventq(IOMMUFDBackend *be, uint32_t viommu_id,
+                                   uint32_t type, uint32_t depth,
+                                   uint32_t *out_veventq_id,
+                                   uint32_t *out_veventq_fd, Error **errp);
+
 bool iommufd_backend_set_dirty_tracking(IOMMUFDBackend *be, uint32_t hwpt_id,
                                         bool start, Error **errp);
 bool iommufd_backend_get_dirty_bitmap(IOMMUFDBackend *be, uint32_t hwpt_id,
-- 
2.43.0

[PATCH v6 2/5] hw/arm/smmuv3-accel: Add viommu free helper

[Shameer Kolothum]

Move viommu teardown into a helper function and use it from the
last device removal path. No functional change.

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 hw/arm/smmuv3-accel.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index f5cd4df336..c19c526fca 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -390,6 +390,20 @@ bool smmuv3_accel_issue_inv_cmd(SMMUv3State *bs, void *cmd, SMMUDevice *sdev,
                    sizeof(Cmd), &entry_num, cmd, errp);
 }
 
+static void smmuv3_accel_free_viommu(SMMUv3AccelState *accel)
+{
+    IOMMUFDViommu *viommu = accel->viommu;
+
+    if (!viommu) {
+        return;
+    }
+    iommufd_backend_free_id(viommu->iommufd, accel->bypass_hwpt_id);
+    iommufd_backend_free_id(viommu->iommufd, accel->abort_hwpt_id);
+    iommufd_backend_free_id(viommu->iommufd, accel->viommu->viommu_id);
+    g_free(viommu);
+    accel->viommu = NULL;
+}
+
 static bool
 smmuv3_accel_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
                           Error **errp)
@@ -549,12 +563,7 @@ static void smmuv3_accel_unset_iommu_device(PCIBus *bus, void *opaque,
     trace_smmuv3_accel_unset_iommu_device(devfn, idev->devid);
 
     if (QLIST_EMPTY(&accel->device_list)) {
-        iommufd_backend_free_id(accel->viommu->iommufd, accel->bypass_hwpt_id);
-        iommufd_backend_free_id(accel->viommu->iommufd, accel->abort_hwpt_id);
-        iommufd_backend_free_id(accel->viommu->iommufd,
-                                accel->viommu->viommu_id);
-        g_free(accel->viommu);
-        accel->viommu = NULL;
+        smmuv3_accel_free_viommu(accel);
     }
 }
 
-- 
2.43.0

[PATCH v6 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for accelerated SMMUv3 devices

[Shameer Kolothum]

From: Nicolin Chen <nicolinc@nvidia.com>

When the guest enables the Event Queue and a vIOMMU is present, allocate a
vEVENTQ object so that host-side events related to the vIOMMU can be
received and propagated back to the guest.

For cold-plugged devices using SMMUv3 acceleration, the vIOMMU is created
before the guest boots. In this case, the vEVENTQ is allocated when the
guest writes to SMMU_CR0 and sets EVENTQEN = 1.

If no cold-plugged device exists at boot (i.e. no vIOMMU initially), the
vEVENTQ is allocated when a vIOMMU is created, i.e. during the first
device hot-plug.

Also, rename the local error variable and refactor smmu_writel() to use
a single error accumulator with error_propagate().

Event read and propagation will be added in a later patch.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 hw/arm/smmuv3-accel.c | 61 +++++++++++++++++++++++++++++++++++++++++--
 hw/arm/smmuv3-accel.h |  6 +++++
 hw/arm/smmuv3.c       | 17 +++++++-----
 3 files changed, 75 insertions(+), 9 deletions(-)

diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index c19c526fca..d92fcb1a89 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -390,6 +390,19 @@ bool smmuv3_accel_issue_inv_cmd(SMMUv3State *bs, void *cmd, SMMUDevice *sdev,
                    sizeof(Cmd), &entry_num, cmd, errp);
 }
 
+static void smmuv3_accel_free_veventq(SMMUv3AccelState *accel)
+{
+    IOMMUFDVeventq *veventq = accel->veventq;
+
+    if (!veventq) {
+        return;
+    }
+    close(veventq->veventq_fd);
+    iommufd_backend_free_id(accel->viommu->iommufd, veventq->veventq_id);
+    g_free(veventq);
+    accel->veventq = NULL;
+}
+
 static void smmuv3_accel_free_viommu(SMMUv3AccelState *accel)
 {
     IOMMUFDViommu *viommu = accel->viommu;
@@ -397,6 +410,7 @@ static void smmuv3_accel_free_viommu(SMMUv3AccelState *accel)
     if (!viommu) {
         return;
     }
+    smmuv3_accel_free_veventq(accel);
     iommufd_backend_free_id(viommu->iommufd, accel->bypass_hwpt_id);
     iommufd_backend_free_id(viommu->iommufd, accel->abort_hwpt_id);
     iommufd_backend_free_id(viommu->iommufd, accel->viommu->viommu_id);
@@ -404,6 +418,41 @@ static void smmuv3_accel_free_viommu(SMMUv3AccelState *accel)
     accel->viommu = NULL;
 }
 
+bool smmuv3_accel_alloc_veventq(SMMUv3State *s, Error **errp)
+{
+    SMMUv3AccelState *accel = s->s_accel;
+    IOMMUFDVeventq *veventq;
+    uint32_t veventq_id;
+    uint32_t veventq_fd;
+
+    if (!accel || !accel->viommu) {
+        return true;
+    }
+
+    if (accel->veventq) {
+        return true;
+    }
+
+    if (!smmuv3_eventq_enabled(s)) {
+        return true;
+    }
+
+    if (!iommufd_backend_alloc_veventq(accel->viommu->iommufd,
+                                       accel->viommu->viommu_id,
+                                       IOMMU_VEVENTQ_TYPE_ARM_SMMUV3,
+                                       1 << s->eventq.log2size, &veventq_id,
+                                       &veventq_fd, errp)) {
+        return false;
+    }
+
+    veventq = g_new(IOMMUFDVeventq, 1);
+    veventq->veventq_id = veventq_id;
+    veventq->veventq_fd = veventq_fd;
+    veventq->viommu = accel->viommu;
+    accel->veventq = veventq;
+    return true;
+}
+
 static bool
 smmuv3_accel_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
                           Error **errp)
@@ -429,6 +478,7 @@ smmuv3_accel_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
     viommu->viommu_id = viommu_id;
     viommu->s2_hwpt_id = s2_hwpt_id;
     viommu->iommufd = idev->iommufd;
+    accel->viommu = viommu;
 
     /*
      * Pre-allocate HWPTs for S1 bypass and abort cases. These will be attached
@@ -448,14 +498,20 @@ smmuv3_accel_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
         goto free_abort_hwpt;
     }
 
+    /* Allocate a vEVENTQ if guest has enabled event queue */
+    if (!smmuv3_accel_alloc_veventq(s, errp)) {
+        goto free_bypass_hwpt;
+    }
+
     /* Attach a HWPT based on SMMUv3 GBPA.ABORT value */
     hwpt_id = smmuv3_accel_gbpa_hwpt(s, accel);
     if (!host_iommu_device_iommufd_attach_hwpt(idev, hwpt_id, errp)) {
-        goto free_bypass_hwpt;
+        goto free_veventq;
     }
-    accel->viommu = viommu;
     return true;
 
+free_veventq:
+    smmuv3_accel_free_veventq(accel);
 free_bypass_hwpt:
     iommufd_backend_free_id(idev->iommufd, accel->bypass_hwpt_id);
 free_abort_hwpt:
@@ -463,6 +519,7 @@ free_abort_hwpt:
 free_viommu:
     iommufd_backend_free_id(idev->iommufd, viommu->viommu_id);
     g_free(viommu);
+    accel->viommu = NULL;
     return false;
 }
 
diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
index a8a64802ec..dba6c71de5 100644
--- a/hw/arm/smmuv3-accel.h
+++ b/hw/arm/smmuv3-accel.h
@@ -22,6 +22,7 @@
  */
 typedef struct SMMUv3AccelState {
     IOMMUFDViommu *viommu;
+    IOMMUFDVeventq *veventq;
     uint32_t bypass_hwpt_id;
     uint32_t abort_hwpt_id;
     QLIST_HEAD(, SMMUv3AccelDevice) device_list;
@@ -50,6 +51,7 @@ bool smmuv3_accel_attach_gbpa_hwpt(SMMUv3State *s, Error **errp);
 bool smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd, SMMUDevice *sdev,
                                 Error **errp);
 void smmuv3_accel_idr_override(SMMUv3State *s);
+bool smmuv3_accel_alloc_veventq(SMMUv3State *s, Error **errp);
 void smmuv3_accel_reset(SMMUv3State *s);
 #else
 static inline void smmuv3_accel_init(SMMUv3State *s)
@@ -80,6 +82,10 @@ smmuv3_accel_issue_inv_cmd(SMMUv3State *s, void *cmd, SMMUDevice *sdev,
 static inline void smmuv3_accel_idr_override(SMMUv3State *s)
 {
 }
+static inline bool smmuv3_accel_alloc_veventq(SMMUv3State *s, Error **errp)
+{
+    return true;
+}
 static inline void smmuv3_accel_reset(SMMUv3State *s)
 {
 }
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index c08d58c579..f804d3af25 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1597,14 +1597,17 @@ static MemTxResult smmu_writell(SMMUv3State *s, hwaddr offset,
 static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
                                uint64_t data, MemTxAttrs attrs)
 {
-    Error *local_err = NULL;
+    Error *err = NULL, *local_err = NULL;
 
     switch (offset) {
     case A_CR0:
         s->cr[0] = data;
         s->cr0ack = data & ~SMMU_CR0_RESERVED;
         /* in case the command queue has been enabled */
-        smmuv3_cmdq_consume(s, &local_err);
+        smmuv3_cmdq_consume(s, &err);
+        /* Allocate vEVENTQ if EVENTQ is enabled and a vIOMMU is available */
+        smmuv3_accel_alloc_veventq(s, &local_err);
+        error_propagate(&err, local_err);
         break;
     case A_CR1:
         s->cr[1] = data;
@@ -1621,7 +1624,7 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
          * By acknowledging the CMDQ_ERR, SW may notify cmds can
          * be processed again
          */
-        smmuv3_cmdq_consume(s, &local_err);
+        smmuv3_cmdq_consume(s, &err);
         break;
     case A_GERROR_IRQ_CFG0: /* 64b */
         s->gerror_irq_cfg0 = deposit64(s->gerror_irq_cfg0, 0, 32, data);
@@ -1643,7 +1646,7 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
         if (data & R_GBPA_UPDATE_MASK) {
             /* Ignore update bit as write is synchronous. */
             s->gbpa = data & ~R_GBPA_UPDATE_MASK;
-            smmuv3_accel_attach_gbpa_hwpt(s, &local_err);
+            smmuv3_accel_attach_gbpa_hwpt(s, &err);
         }
         break;
     case A_STRTAB_BASE: /* 64b */
@@ -1671,7 +1674,7 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
         break;
     case A_CMDQ_PROD:
         s->cmdq.prod = data;
-        smmuv3_cmdq_consume(s, &local_err);
+        smmuv3_cmdq_consume(s, &err);
         break;
     case A_CMDQ_CONS:
         s->cmdq.cons = data;
@@ -1711,8 +1714,8 @@ static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
         break;
     }
 
-    if (local_err) {
-        error_report_err(local_err);
+    if (err) {
+        error_report_err(err);
     }
     return MEMTX_OK;
 }
-- 
2.43.0

[PATCH v6 4/5] hw/arm/smmuv3: Introduce a helper function for event propagation

[Shameer Kolothum]

Factor out the code that propagates event records to the guest into a
helper function. The accelerated SMMUv3 path can use this to propagate
host events in a subsequent patch.

Since this helper may be called from outside the SMMUv3 core, take the
mutex before accessing the Event Queue.

No functional change intended.

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 hw/arm/smmuv3-internal.h |  4 ++++
 hw/arm/smmuv3.c          | 21 +++++++++++++++------
 hw/arm/trace-events      |  2 +-
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index a6464425ec..b666109ad9 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -352,7 +352,11 @@ typedef struct SMMUEventInfo {
             (x)->word[6] = (uint32_t)(addr & 0xffffffff); \
     } while (0)
 
+#define EVT_GET_TYPE(x)  extract32((x)->word[0], 0, 8)
+#define EVT_GET_SID(x)   ((x)->word[1])
+
 void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *event);
+void smmuv3_propagate_event(SMMUv3State *s, Evt *evt);
 int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event);
 
 static inline int oas2bits(int oas_field)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index f804d3af25..bcb9176c51 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -168,10 +168,23 @@ static MemTxResult smmuv3_write_eventq(SMMUv3State *s, Evt *evt)
     return MEMTX_OK;
 }
 
+void smmuv3_propagate_event(SMMUv3State *s, Evt *evt)
+{
+    MemTxResult r;
+
+    trace_smmuv3_propagate_event(smmu_event_string(EVT_GET_TYPE(evt)),
+                                 EVT_GET_SID(evt));
+    qemu_mutex_lock(&s->mutex);
+    r = smmuv3_write_eventq(s, evt);
+    if (r != MEMTX_OK) {
+        smmuv3_trigger_irq(s, SMMU_IRQ_GERROR, R_GERROR_EVENTQ_ABT_ERR_MASK);
+    }
+    qemu_mutex_unlock(&s->mutex);
+}
+
 void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *info)
 {
     Evt evt = {};
-    MemTxResult r;
 
     if (!smmuv3_eventq_enabled(s)) {
         return;
@@ -251,11 +264,7 @@ void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *info)
         g_assert_not_reached();
     }
 
-    trace_smmuv3_record_event(smmu_event_string(info->type), info->sid);
-    r = smmuv3_write_eventq(s, &evt);
-    if (r != MEMTX_OK) {
-        smmuv3_trigger_irq(s, SMMU_IRQ_GERROR, R_GERROR_EVENTQ_ABT_ERR_MASK);
-    }
+    smmuv3_propagate_event(s, &evt);
     info->recorded = true;
 }
 
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 8135c0c734..3457536fb0 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -40,7 +40,7 @@ smmuv3_cmdq_opcode(const char *opcode) "<--- %s"
 smmuv3_cmdq_consume_out(uint32_t prod, uint32_t cons, uint8_t prod_wrap, uint8_t cons_wrap) "prod:%d, cons:%d, prod_wrap:%d, cons_wrap:%d "
 smmuv3_cmdq_consume_error(const char *cmd_name, uint8_t cmd_error) "Error on %s command execution: %d"
 smmuv3_write_mmio(uint64_t addr, uint64_t val, unsigned size, uint32_t r) "addr: 0x%"PRIx64" val:0x%"PRIx64" size: 0x%x(%d)"
-smmuv3_record_event(const char *type, uint32_t sid) "%s sid=0x%x"
+smmuv3_propagate_event(const char *type, uint32_t sid) "%s sid=0x%x"
 smmuv3_find_ste(uint16_t sid, uint32_t features, uint16_t sid_split) "sid=0x%x features:0x%x, sid_split:0x%x"
 smmuv3_find_ste_2lvl(uint64_t strtab_base, uint64_t l1ptr, int l1_ste_offset, uint64_t l2ptr, int l2_ste_offset, int max_l2_ste) "strtab_base:0x%"PRIx64" l1ptr:0x%"PRIx64" l1_off:0x%x, l2ptr:0x%"PRIx64" l2_off:0x%x max_l2_ste:%d"
 smmuv3_get_ste(uint64_t addr) "STE addr: 0x%"PRIx64
-- 
2.43.0

[PATCH v6 5/5] hw/arm/smmuv3-accel: Read and propagate host vIOMMU events

[Shameer Kolothum]

Install an event handler on the vEVENTQ fd to read and propagate host
generated vIOMMU events to the guest.

The handler runs in QEMU's main loop, using a non-blocking fd registered
via qemu_set_fd_handler().

Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 hw/arm/smmuv3-accel.c | 64 +++++++++++++++++++++++++++++++++++++++++++
 hw/arm/smmuv3-accel.h |  2 ++
 2 files changed, 66 insertions(+)

diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index d92fcb1a89..dac4526a7f 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -390,6 +390,50 @@ bool smmuv3_accel_issue_inv_cmd(SMMUv3State *bs, void *cmd, SMMUDevice *sdev,
                    sizeof(Cmd), &entry_num, cmd, errp);
 }
 
+static void smmuv3_accel_event_read(void *opaque)
+{
+    SMMUv3State *s = opaque;
+    SMMUv3AccelState *accel = s->s_accel;
+    struct {
+        struct iommufd_vevent_header hdr;
+        struct iommu_vevent_arm_smmuv3 vevent;
+    } buf;
+    enum iommu_veventq_type type = IOMMU_VEVENTQ_TYPE_ARM_SMMUV3;
+    uint32_t id = accel->veventq->veventq_id;
+    uint32_t last_seq = accel->last_event_seq;
+    ssize_t bytes;
+
+    bytes = read(accel->veventq->veventq_fd, &buf, sizeof(buf));
+    if (bytes <= 0) {
+        if (errno == EAGAIN || errno == EINTR) {
+            return;
+        }
+        error_report_once("vEVENTQ(type %u id %u): read failed (%m)", type, id);
+        return;
+    }
+
+    if (bytes == sizeof(buf.hdr) &&
+        (buf.hdr.flags & IOMMU_VEVENTQ_FLAG_LOST_EVENTS)) {
+        error_report_once("vEVENTQ(type %u id %u): overflowed", type, id);
+        accel->event_start = false;
+        return;
+    }
+    if (bytes < sizeof(buf)) {
+        error_report_once("vEVENTQ(type %u id %u): short read(%zd/%zd bytes)",
+                          type, id, bytes, sizeof(buf));
+        return;
+    }
+
+    /* Check sequence in hdr for lost events if any */
+    if (accel->event_start && (buf.hdr.sequence - last_seq != 1)) {
+        error_report_once("vEVENTQ(type %u id %u): lost %u event(s)",
+                          type, id, buf.hdr.sequence - last_seq - 1);
+    }
+    accel->last_event_seq = buf.hdr.sequence;
+    accel->event_start = true;
+    smmuv3_propagate_event(s, (Evt *)&buf.vevent);
+}
+
 static void smmuv3_accel_free_veventq(SMMUv3AccelState *accel)
 {
     IOMMUFDVeventq *veventq = accel->veventq;
@@ -397,6 +441,7 @@ static void smmuv3_accel_free_veventq(SMMUv3AccelState *accel)
     if (!veventq) {
         return;
     }
+    qemu_set_fd_handler(veventq->veventq_fd, NULL, NULL, NULL);
     close(veventq->veventq_fd);
     iommufd_backend_free_id(accel->viommu->iommufd, veventq->veventq_id);
     g_free(veventq);
@@ -424,6 +469,7 @@ bool smmuv3_accel_alloc_veventq(SMMUv3State *s, Error **errp)
     IOMMUFDVeventq *veventq;
     uint32_t veventq_id;
     uint32_t veventq_fd;
+    int flags;
 
     if (!accel || !accel->viommu) {
         return true;
@@ -445,12 +491,30 @@ bool smmuv3_accel_alloc_veventq(SMMUv3State *s, Error **errp)
         return false;
     }
 
+    flags = fcntl(veventq_fd, F_GETFL);
+    if (flags < 0) {
+        error_setg_errno(errp, errno, "Failed to get flags for vEVENTQ fd");
+        goto free_veventq;
+    }
+    if (fcntl(veventq_fd, F_SETFL, flags | O_NONBLOCK) < 0) {
+        error_setg_errno(errp, errno, "Failed to set O_NONBLOCK on vEVENTQ fd");
+        goto free_veventq;
+    }
+
     veventq = g_new(IOMMUFDVeventq, 1);
     veventq->veventq_id = veventq_id;
     veventq->veventq_fd = veventq_fd;
     veventq->viommu = accel->viommu;
     accel->veventq = veventq;
+
+    /* Set up event handler for veventq fd */
+    qemu_set_fd_handler(veventq_fd, smmuv3_accel_event_read, NULL, s);
     return true;
+
+free_veventq:
+    close(veventq_fd);
+    iommufd_backend_free_id(accel->viommu->iommufd, veventq_id);
+    return false;
 }
 
 static bool
diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
index dba6c71de5..c9c10e55c3 100644
--- a/hw/arm/smmuv3-accel.h
+++ b/hw/arm/smmuv3-accel.h
@@ -23,6 +23,8 @@
 typedef struct SMMUv3AccelState {
     IOMMUFDViommu *viommu;
     IOMMUFDVeventq *veventq;
+    uint32_t last_event_seq;
+    bool event_start;
     uint32_t bypass_hwpt_id;
     uint32_t abort_hwpt_id;
     QLIST_HEAD(, SMMUv3AccelDevice) device_list;
-- 
2.43.0

Re: [PATCH v6 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for accelerated SMMUv3 devices

[Nicolin Chen]

On Fri, Feb 13, 2026 at 10:39:40AM +0000, Shameer Kolothum wrote:
>  static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
>                                 uint64_t data, MemTxAttrs attrs)
>  {
> -    Error *local_err = NULL;
> +    Error *err = NULL, *local_err = NULL;
>  
>      switch (offset) {
>      case A_CR0:
>          s->cr[0] = data;
>          s->cr0ack = data & ~SMMU_CR0_RESERVED;
>          /* in case the command queue has been enabled */
> -        smmuv3_cmdq_consume(s, &local_err);
> +        smmuv3_cmdq_consume(s, &err);
> +        /* Allocate vEVENTQ if EVENTQ is enabled and a vIOMMU is available */
> +        smmuv3_accel_alloc_veventq(s, &local_err);
> +        error_propagate(&err, local_err);

It would be cleaner to have:

     Error *local_err2 = NULL;
     Error *local_err = NULL;

         smmuv3_cmdq_consume(s, &local_err);
+        /* Allocate vEVENTQ if EVENTQ is enabled and a vIOMMU is available */
+        smmuv3_accel_alloc_veventq(s, &local_err2);
+        error_propagate(&local_err, local_err2);

Otherwise,

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

Re: [PATCH v6 1/5] backends/iommufd: Introduce iommufd_backend_alloc_veventq

[Nicolin Chen]

On Fri, Feb 13, 2026 at 10:39:38AM +0000, Shameer Kolothum wrote:
> From: Nicolin Chen <nicolinc@nvidia.com>
> 
> Add a new helper for IOMMU_VEVENTQ_ALLOC ioctl to allocate a virtual event
> queue (vEVENTQ) for a vIOMMU object.
> 
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

Re: [PATCH v6 5/5] hw/arm/smmuv3-accel: Read and propagate host vIOMMU events

[Nicolin Chen]

On Fri, Feb 13, 2026 at 10:39:42AM +0000, Shameer Kolothum wrote:
>  typedef struct SMMUv3AccelState {
>      IOMMUFDViommu *viommu;
>      IOMMUFDVeventq *veventq;
> +    uint32_t last_event_seq;
> +    bool event_start;

I see tegra241-cmdqv having these two as well. It seems they are
required by all veventq users. So, let's move these two into the
common IOMMUFDVeventq?

Otherwise,

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

Re: [PATCH v6 0/5] vEVENTQ support for accelerated SMMUv3 devices

[Nicolin Chen]

On Fri, Feb 13, 2026 at 10:39:37AM +0000, Shameer Kolothum wrote:
> Hi,
> 
> Changes from v5:
>  https://lore.kernel.org/qemu-devel/20260211083415.133534-1-skolothumtho@nvidia.com/
>  
>  -Addressed feedback and picked up tags. Thanks!
>  -Decoupled alloc_veventq on smmuv3_cmdq_consume() (patch  #3)
>  -Updated error messages (patch #5)

I tested by hacking away the RMR nodes and msi_gpa to trigger some
translation faults at MSI doorbell.

Tested-by: Nicolin Chen <nicolinc@nvidia.com>

RE: [PATCH v6 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for accelerated SMMUv3 devices

[Shameer Kolothum Thodi]

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: 13 February 2026 22:19
> To: Shameer Kolothum Thodi <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Nathan Chen
> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jiandi An
> <jan@nvidia.com>; Jason Gunthorpe <jgg@nvidia.com>;
> jonathan.cameron@huawei.com; zhangfei.gao@linaro.org;
> zhenzhong.duan@intel.com; Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for
> accelerated SMMUv3 devices
> 
> On Fri, Feb 13, 2026 at 10:39:40AM +0000, Shameer Kolothum wrote:
> >  static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
> >                                 uint64_t data, MemTxAttrs attrs)  {
> > -    Error *local_err = NULL;
> > +    Error *err = NULL, *local_err = NULL;
> >
> >      switch (offset) {
> >      case A_CR0:
> >          s->cr[0] = data;
> >          s->cr0ack = data & ~SMMU_CR0_RESERVED;
> >          /* in case the command queue has been enabled */
> > -        smmuv3_cmdq_consume(s, &local_err);
> > +        smmuv3_cmdq_consume(s, &err);
> > +        /* Allocate vEVENTQ if EVENTQ is enabled and a vIOMMU is available
> */
> > +        smmuv3_accel_alloc_veventq(s, &local_err);
> > +        error_propagate(&err, local_err);
> 
> It would be cleaner to have:
> 
>      Error *local_err2 = NULL;
>      Error *local_err = NULL;

Yeah. I had a bit of back and forth on this. My thinking was to use err as
an accumulating error pointer in case additional error paths are added
later, with local_err_x used for intermediate calls.

Either way is fine I guess for now.

Thanks,
Shameer

RE: [PATCH v6 0/5] vEVENTQ support for accelerated SMMUv3 devices

[Shameer Kolothum Thodi]

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: 13 February 2026 22:32
> To: Shameer Kolothum Thodi <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Nathan Chen
> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jiandi An
> <jan@nvidia.com>; Jason Gunthorpe <jgg@nvidia.com>;
> jonathan.cameron@huawei.com; zhangfei.gao@linaro.org;
> zhenzhong.duan@intel.com; Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 0/5] vEVENTQ support for accelerated SMMUv3
> devices
> 
> On Fri, Feb 13, 2026 at 10:39:37AM +0000, Shameer Kolothum wrote:
> > Hi,
> >
> > Changes from v5:
> >  https://lore.kernel.org/qemu-devel/20260211083415.133534-1-
> skolothumtho@nvidia.com/
> >
> >  -Addressed feedback and picked up tags. Thanks!
> >  -Decoupled alloc_veventq on smmuv3_cmdq_consume() (patch  #3)
> >  -Updated error messages (patch #5)
> 
> I tested by hacking away the RMR nodes and msi_gpa to trigger some
> translation faults at MSI doorbell.
> 
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>

Thanks. I also tested, modifying  STE0_S1CDMAX to trigger BAD_CD ones.

Shameer

Re: [PATCH v6 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for accelerated SMMUv3 devices

[Jonathan Cameron via]

On Fri, 13 Feb 2026 10:39:40 +0000
Shameer Kolothum <skolothumtho@nvidia.com> wrote:

> From: Nicolin Chen <nicolinc@nvidia.com>
> 
> When the guest enables the Event Queue and a vIOMMU is present, allocate a
> vEVENTQ object so that host-side events related to the vIOMMU can be
> received and propagated back to the guest.
> 
> For cold-plugged devices using SMMUv3 acceleration, the vIOMMU is created
> before the guest boots. In this case, the vEVENTQ is allocated when the
> guest writes to SMMU_CR0 and sets EVENTQEN = 1.
> 
> If no cold-plugged device exists at boot (i.e. no vIOMMU initially), the
> vEVENTQ is allocated when a vIOMMU is created, i.e. during the first
> device hot-plug.
> 
> Also, rename the local error variable and refactor smmu_writel() to use
> a single error accumulator with error_propagate().

Why not split the rename out as a separate patch?  I don't hugely mind just
feels like some noise in here could have been broken out before the real
change and made it a tiny bit easier to review.

I wouldn't bother unless you are respinning again for other reasons though!

J

> 
> Event read and propagation will be added in a later patch.
> 
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>

Re: [PATCH v6 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for accelerated SMMUv3 devices

[Jonathan Cameron via qemu development]

On Fri, 13 Feb 2026 10:39:40 +0000
Shameer Kolothum <skolothumtho@nvidia.com> wrote:

> From: Nicolin Chen <nicolinc@nvidia.com>
> 
> When the guest enables the Event Queue and a vIOMMU is present, allocate a
> vEVENTQ object so that host-side events related to the vIOMMU can be
> received and propagated back to the guest.
> 
> For cold-plugged devices using SMMUv3 acceleration, the vIOMMU is created
> before the guest boots. In this case, the vEVENTQ is allocated when the
> guest writes to SMMU_CR0 and sets EVENTQEN = 1.
> 
> If no cold-plugged device exists at boot (i.e. no vIOMMU initially), the
> vEVENTQ is allocated when a vIOMMU is created, i.e. during the first
> device hot-plug.
> 
> Also, rename the local error variable and refactor smmu_writel() to use
> a single error accumulator with error_propagate().

Why not split the rename out as a separate patch?  I don't hugely mind just
feels like some noise in here could have been broken out before the real
change and made it a tiny bit easier to review.

I wouldn't bother unless you are respinning again for other reasons though!

J

> 
> Event read and propagation will be added in a later patch.
> 
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>

Re: [PATCH v6 4/5] hw/arm/smmuv3: Introduce a helper function for event propagation

[Jonathan Cameron via]

On Fri, 13 Feb 2026 10:39:41 +0000
Shameer Kolothum <skolothumtho@nvidia.com> wrote:

> Factor out the code that propagates event records to the guest into a
> helper function. The accelerated SMMUv3 path can use this to propagate
> host events in a subsequent patch.
> 
> Since this helper may be called from outside the SMMUv3 core, take the
> mutex before accessing the Event Queue.

Totally trivial but it might be worth stating that / why it is harmless
(or indeed necessary?) to take the lock in existing paths. I was kind
of expecting to see locked helper wrapping an unlocked version that was
used to replace the original code.  So guess I'm missing something.

> 
> No functional change intended.
> 
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>  hw/arm/smmuv3-internal.h |  4 ++++
>  hw/arm/smmuv3.c          | 21 +++++++++++++++------
>  hw/arm/trace-events      |  2 +-
>  3 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index a6464425ec..b666109ad9 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -352,7 +352,11 @@ typedef struct SMMUEventInfo {
>              (x)->word[6] = (uint32_t)(addr & 0xffffffff); \
>      } while (0)
>  
> +#define EVT_GET_TYPE(x)  extract32((x)->word[0], 0, 8)
> +#define EVT_GET_SID(x)   ((x)->word[1])
> +
>  void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *event);
> +void smmuv3_propagate_event(SMMUv3State *s, Evt *evt);
>  int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event);
>  
>  static inline int oas2bits(int oas_field)
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index f804d3af25..bcb9176c51 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -168,10 +168,23 @@ static MemTxResult smmuv3_write_eventq(SMMUv3State *s, Evt *evt)
>      return MEMTX_OK;
>  }
>  
> +void smmuv3_propagate_event(SMMUv3State *s, Evt *evt)
> +{
> +    MemTxResult r;
> +
> +    trace_smmuv3_propagate_event(smmu_event_string(EVT_GET_TYPE(evt)),
> +                                 EVT_GET_SID(evt));
> +    qemu_mutex_lock(&s->mutex);

Could use 
    QEMU_LOCK_GUARD(&s->mutex);

though gain is minor so feel free to not do so.

> +    r = smmuv3_write_eventq(s, evt);
> +    if (r != MEMTX_OK) {
> +        smmuv3_trigger_irq(s, SMMU_IRQ_GERROR, R_GERROR_EVENTQ_ABT_ERR_MASK);
> +    }
> +    qemu_mutex_unlock(&s->mutex);
> +}

Re: [PATCH v6 4/5] hw/arm/smmuv3: Introduce a helper function for event propagation

[Jonathan Cameron via qemu development]

On Fri, 13 Feb 2026 10:39:41 +0000
Shameer Kolothum <skolothumtho@nvidia.com> wrote:

> Factor out the code that propagates event records to the guest into a
> helper function. The accelerated SMMUv3 path can use this to propagate
> host events in a subsequent patch.
> 
> Since this helper may be called from outside the SMMUv3 core, take the
> mutex before accessing the Event Queue.

Totally trivial but it might be worth stating that / why it is harmless
(or indeed necessary?) to take the lock in existing paths. I was kind
of expecting to see locked helper wrapping an unlocked version that was
used to replace the original code.  So guess I'm missing something.

> 
> No functional change intended.
> 
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>  hw/arm/smmuv3-internal.h |  4 ++++
>  hw/arm/smmuv3.c          | 21 +++++++++++++++------
>  hw/arm/trace-events      |  2 +-
>  3 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index a6464425ec..b666109ad9 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -352,7 +352,11 @@ typedef struct SMMUEventInfo {
>              (x)->word[6] = (uint32_t)(addr & 0xffffffff); \
>      } while (0)
>  
> +#define EVT_GET_TYPE(x)  extract32((x)->word[0], 0, 8)
> +#define EVT_GET_SID(x)   ((x)->word[1])
> +
>  void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *event);
> +void smmuv3_propagate_event(SMMUv3State *s, Evt *evt);
>  int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, SMMUEventInfo *event);
>  
>  static inline int oas2bits(int oas_field)
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index f804d3af25..bcb9176c51 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -168,10 +168,23 @@ static MemTxResult smmuv3_write_eventq(SMMUv3State *s, Evt *evt)
>      return MEMTX_OK;
>  }
>  
> +void smmuv3_propagate_event(SMMUv3State *s, Evt *evt)
> +{
> +    MemTxResult r;
> +
> +    trace_smmuv3_propagate_event(smmu_event_string(EVT_GET_TYPE(evt)),
> +                                 EVT_GET_SID(evt));
> +    qemu_mutex_lock(&s->mutex);

Could use 
    QEMU_LOCK_GUARD(&s->mutex);

though gain is minor so feel free to not do so.

> +    r = smmuv3_write_eventq(s, evt);
> +    if (r != MEMTX_OK) {
> +        smmuv3_trigger_irq(s, SMMU_IRQ_GERROR, R_GERROR_EVENTQ_ABT_ERR_MASK);
> +    }
> +    qemu_mutex_unlock(&s->mutex);
> +}

Re: [PATCH v6 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for accelerated SMMUv3 devices

[Peter Maydell]

On Mon, 16 Feb 2026 at 08:58, Shameer Kolothum Thodi
<skolothumtho@nvidia.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Nicolin Chen <nicolinc@nvidia.com>
> > Sent: 13 February 2026 22:19
> > To: Shameer Kolothum Thodi <skolothumtho@nvidia.com>
> > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> > eric.auger@redhat.com; peter.maydell@linaro.org; Nathan Chen
> > <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jiandi An
> > <jan@nvidia.com>; Jason Gunthorpe <jgg@nvidia.com>;
> > jonathan.cameron@huawei.com; zhangfei.gao@linaro.org;
> > zhenzhong.duan@intel.com; Krishnakant Jaju <kjaju@nvidia.com>
> > Subject: Re: [PATCH v6 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for
> > accelerated SMMUv3 devices
> >
> > On Fri, Feb 13, 2026 at 10:39:40AM +0000, Shameer Kolothum wrote:
> > >  static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
> > >                                 uint64_t data, MemTxAttrs attrs)  {
> > > -    Error *local_err = NULL;
> > > +    Error *err = NULL, *local_err = NULL;
> > >
> > >      switch (offset) {
> > >      case A_CR0:
> > >          s->cr[0] = data;
> > >          s->cr0ack = data & ~SMMU_CR0_RESERVED;
> > >          /* in case the command queue has been enabled */
> > > -        smmuv3_cmdq_consume(s, &local_err);
> > > +        smmuv3_cmdq_consume(s, &err);
> > > +        /* Allocate vEVENTQ if EVENTQ is enabled and a vIOMMU is available
> > */
> > > +        smmuv3_accel_alloc_veventq(s, &local_err);
> > > +        error_propagate(&err, local_err);
> >
> > It would be cleaner to have:
> >
> >      Error *local_err2 = NULL;
> >      Error *local_err = NULL;
>
> Yeah. I had a bit of back and forth on this. My thinking was to use err as
> an accumulating error pointer in case additional error paths are added
> later, with local_err_x used for intermediate calls.
>
> Either way is fine I guess for now.

Generally speaking we should try to stick to one of the
defined standard patterns for Error handling in the
big comment in include/qapi/error.h.

Yours is the "receive and accumulate multiple errors" pattern, I think?
That works, but do you really want to throw away the second error
message without reporting it if both calls fail?

Plus, from the commit message:

> Also, rename the local error variable and refactor smmu_writel() to use
> a single error accumulator with error_propagate().

If you find yourself writing "Also, ..." in a commit message,
it's often a sign that you should consider splitting the
commit in two.

thanks
-- PMM

RE: [PATCH v6 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for accelerated SMMUv3 devices

[Shameer Kolothum Thodi]

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: 16 February 2026 16:57
> To: Shameer Kolothum Thodi <skolothumtho@nvidia.com>
> Cc: Nicolin Chen <nicolinc@nvidia.com>; qemu-arm@nongnu.org; qemu-
> devel@nongnu.org; eric.auger@redhat.com; Nathan Chen
> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jiandi An
> <jan@nvidia.com>; Jason Gunthorpe <jgg@nvidia.com>;
> jonathan.cameron@huawei.com; zhangfei.gao@linaro.org;
> zhenzhong.duan@intel.com; Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for
> accelerated SMMUv3 devices
> 
> External email: Use caution opening links or attachments
> 
> 
> On Mon, 16 Feb 2026 at 08:58, Shameer Kolothum Thodi
> <skolothumtho@nvidia.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Nicolin Chen <nicolinc@nvidia.com>
> > > Sent: 13 February 2026 22:19
> > > To: Shameer Kolothum Thodi <skolothumtho@nvidia.com>
> > > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> > > eric.auger@redhat.com; peter.maydell@linaro.org; Nathan Chen
> > > <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jiandi An
> > > <jan@nvidia.com>; Jason Gunthorpe <jgg@nvidia.com>;
> > > jonathan.cameron@huawei.com; zhangfei.gao@linaro.org;
> > > zhenzhong.duan@intel.com; Krishnakant Jaju <kjaju@nvidia.com>
> > > Subject: Re: [PATCH v6 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for
> > > accelerated SMMUv3 devices
> > >
> > > On Fri, Feb 13, 2026 at 10:39:40AM +0000, Shameer Kolothum wrote:
> > > >  static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset,
> > > >                                 uint64_t data, MemTxAttrs attrs)  {
> > > > -    Error *local_err = NULL;
> > > > +    Error *err = NULL, *local_err = NULL;
> > > >
> > > >      switch (offset) {
> > > >      case A_CR0:
> > > >          s->cr[0] = data;
> > > >          s->cr0ack = data & ~SMMU_CR0_RESERVED;
> > > >          /* in case the command queue has been enabled */
> > > > -        smmuv3_cmdq_consume(s, &local_err);
> > > > +        smmuv3_cmdq_consume(s, &err);
> > > > +        /* Allocate vEVENTQ if EVENTQ is enabled and a vIOMMU is
> available
> > > */
> > > > +        smmuv3_accel_alloc_veventq(s, &local_err);
> > > > +        error_propagate(&err, local_err);
> > >
> > > It would be cleaner to have:
> > >
> > >      Error *local_err2 = NULL;
> > >      Error *local_err = NULL;
> >
> > Yeah. I had a bit of back and forth on this. My thinking was to use err as
> > an accumulating error pointer in case additional error paths are added
> > later, with local_err_x used for intermediate calls.
> >
> > Either way is fine I guess for now.
> 
> Generally speaking we should try to stick to one of the
> defined standard patterns for Error handling in the
> big comment in include/qapi/error.h.
> 
> Yours is the "receive and accumulate multiple errors" pattern, I think?

Yes, this currently uses that pattern.

> That works, but do you really want to throw away the second error
> message without reporting it if both calls fail?

This was considered earlier, but switched to the error_propagate()
pattern to follow the accumulation style. Since the two calls are
independent, I agree it is better to report both explicitly. I will
update this in the next revision if there are no further concerns.

> Plus, from the commit message:
> 
> > Also, rename the local error variable and refactor smmu_writel() to use
> > a single error accumulator with error_propagate().
> 
> If you find yourself writing "Also, ..." in a commit message,
> it's often a sign that you should consider splitting the
> commit in two.

Sure. Jonathan also mentioned that. (I was lazy)

Thanks,
Shameer

RE: [PATCH v6 4/5] hw/arm/smmuv3: Introduce a helper function for event propagation

[Shameer Kolothum Thodi]

> -----Original Message-----
> From: Jonathan Cameron <jonathan.cameron@huawei.com>
> Sent: 16 February 2026 16:30
> To: Shameer Kolothum Thodi <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Nicolin Chen
> <nicolinc@nvidia.com>; Nathan Chen <nathanc@nvidia.com>; Matt Ochs
> <mochs@nvidia.com>; Jiandi An <jan@nvidia.com>; Jason Gunthorpe
> <jgg@nvidia.com>; zhangfei.gao@linaro.org; zhenzhong.duan@intel.com;
> Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 4/5] hw/arm/smmuv3: Introduce a helper function for
> event propagation
> 
> External email: Use caution opening links or attachments
> 
> 
> On Fri, 13 Feb 2026 10:39:41 +0000
> Shameer Kolothum <skolothumtho@nvidia.com> wrote:
> 
> > Factor out the code that propagates event records to the guest into a
> > helper function. The accelerated SMMUv3 path can use this to propagate
> > host events in a subsequent patch.
> >
> > Since this helper may be called from outside the SMMUv3 core, take the
> > mutex before accessing the Event Queue.
> 
> Totally trivial but it might be worth stating that / why it is harmless
> (or indeed necessary?) to take the lock in existing paths. I was kind
> of expecting to see locked helper wrapping an unlocked version that was
> used to replace the original code.  So guess I'm missing something.

Until now, event propagation happens only via the
 smmuv3_translate()
  smmuv3_record_event()
  
path, which does not take any explicit lock.

In a subsequent patch, this helper will also be called from the
smmuv3_accel_event_read() path, which may run concurrently due
to host SMMUv3 event notifications. Hence the lock here.

Since both callers need serialized access to the Event Queue, don't
think we need a separate unlocked version.

I will clarify this in the commit message.

Thanks,
Shameer

> >
> > No functional change intended.
> >
> > Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> > Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> > ---
> >  hw/arm/smmuv3-internal.h |  4 ++++
> >  hw/arm/smmuv3.c          | 21 +++++++++++++++------
> >  hw/arm/trace-events      |  2 +-
> >  3 files changed, 20 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> > index a6464425ec..b666109ad9 100644
> > --- a/hw/arm/smmuv3-internal.h
> > +++ b/hw/arm/smmuv3-internal.h
> > @@ -352,7 +352,11 @@ typedef struct SMMUEventInfo {
> >              (x)->word[6] = (uint32_t)(addr & 0xffffffff); \
> >      } while (0)
> >
> > +#define EVT_GET_TYPE(x)  extract32((x)->word[0], 0, 8)
> > +#define EVT_GET_SID(x)   ((x)->word[1])
> > +
> >  void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *event);
> > +void smmuv3_propagate_event(SMMUv3State *s, Evt *evt);
> >  int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,
> SMMUEventInfo *event);
> >
> >  static inline int oas2bits(int oas_field)
> > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> > index f804d3af25..bcb9176c51 100644
> > --- a/hw/arm/smmuv3.c
> > +++ b/hw/arm/smmuv3.c
> > @@ -168,10 +168,23 @@ static MemTxResult
> smmuv3_write_eventq(SMMUv3State *s, Evt *evt)
> >      return MEMTX_OK;
> >  }
> >
> > +void smmuv3_propagate_event(SMMUv3State *s, Evt *evt)
> > +{
> > +    MemTxResult r;
> > +
> > +
> trace_smmuv3_propagate_event(smmu_event_string(EVT_GET_TYPE(evt)),
> > +                                 EVT_GET_SID(evt));
> > +    qemu_mutex_lock(&s->mutex);
> 
> Could use
>     QEMU_LOCK_GUARD(&s->mutex);
> 
> though gain is minor so feel free to not do so.
> 
> > +    r = smmuv3_write_eventq(s, evt);
> > +    if (r != MEMTX_OK) {
> > +        smmuv3_trigger_irq(s, SMMU_IRQ_GERROR,
> R_GERROR_EVENTQ_ABT_ERR_MASK);
> > +    }
> > +    qemu_mutex_unlock(&s->mutex);
> > +}
>