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

Shameer Kolothum posted 5 patches 1 month, 3 weeks ago
Maintainers: Yi Liu <yi.l.liu@intel.com>, Eric Auger <eric.auger@redhat.com>, Zhenzhong Duan <zhenzhong.duan@intel.com>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PATCH v6 4/5] hw/arm/smmuv3: Introduce a helper function for event propagation
Posted by Shameer Kolothum 1 month, 3 weeks ago
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
Re: [PATCH v6 4/5] hw/arm/smmuv3: Introduce a helper function for event propagation
Posted by Jonathan Cameron via qemu development 1 month, 3 weeks ago
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
Posted by Shameer Kolothum Thodi 1 month, 3 weeks ago

> -----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);
> > +}
>