On Wed, 5 Nov 2025 15:46:52 +0000 Shameer Kolothum <[email protected]> wrote:
> 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(). > > Signed-off-by: Shameer Kolothum <[email protected]> A few minor suggestions inline. Otherwise set looks good to me, though I'm very far from an expert of this stuff! Jonathan > --- > hw/arm/smmuv3-accel.c | 62 +++++++++++++++++++++++++++++++++++++++++++ > hw/arm/smmuv3-accel.h | 2 ++ > 2 files changed, 64 insertions(+) > > diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c > index 210e7ebf36..e6c81c4786 100644 > --- a/hw/arm/smmuv3-accel.c > +++ b/hw/arm/smmuv3-accel.c > @@ -383,6 +383,62 @@ static SMMUv3AccelDevice *smmuv3_accel_get_dev(SMMUState > *bs, SMMUPciBus *sbus, > return accel_dev; > } > > +static void smmuv3_accel_event_read(void *opaque) > +{ > + SMMUv3State *s = opaque; > + SMMUv3AccelState *s_accel = s->s_accel; > + SMMUViommu *vsmmu = s_accel->vsmmu; > + struct iommu_vevent_arm_smmuv3 *vevent; > + struct iommufd_vevent_header *hdr; > + ssize_t readsz = sizeof(*hdr) + sizeof(*vevent); > + uint8_t buf[sizeof(*hdr) + sizeof(*vevent)]; Could you wrap this up in a structure to make it a tiny bit more obvious what is going on? struct { struct iommufd_vevent_header hdr; struct iommufd_vevent_arm_smmuv3 vevent; } buf; Should allow sizeof(buf); and accessing elements directly without casts. > + uint32_t last_seq = vsmmu->last_event_seq; > + ssize_t bytes; > + Evt evt = {}; Given you copy into this based on sizeof(evt) I can't see why you need to initialize. > + > + bytes = read(vsmmu->veventq->veventq_fd, buf, readsz); > + if (bytes <= 0) { > + if (errno == EAGAIN || errno == EINTR) { > + return; > + } > + error_report("vEVENTQ: read failed (%s)", strerror(errno)); > + return; > + } > + > + if (bytes < readsz) { > + error_report("vEVENTQ: incomplete read (%zd/%zd bytes)", bytes, > readsz); > + return; > + } > + > + hdr = (struct iommufd_vevent_header *)buf; > + if (hdr->flags & IOMMU_VEVENTQ_FLAG_LOST_EVENTS) { > + error_report("vEVENTQ has lost events"); > + return; > + } > + > + vevent = (struct iommu_vevent_arm_smmuv3 *)(buf + sizeof(*hdr)); > + /* Check sequence in hdr for lost events if any */ > + if (vsmmu->event_start) { > + uint32_t expected = (last_seq == INT_MAX) ? 0 : last_seq + 1; > + > + if (hdr->sequence != expected) { > + uint32_t delta; > + > + if (hdr->sequence >= last_seq) { > + delta = hdr->sequence - last_seq; > + } else { > + /* Handle wraparound from INT_MAX */ > + delta = (INT_MAX - last_seq) + hdr->sequence + 1; > + } > + error_report("vEVENTQ: detected lost %u event(s)", delta - 1); > + } > + } > + vsmmu->last_event_seq = hdr->sequence; > + vsmmu->event_start = true; > + memcpy(&evt, vevent, sizeof(evt)); > + smmuv3_propagate_event(s, &evt); Why is the copy needed? Can't you just use the vevent in place? > +}
