> -----Original Message----- > From: Eric Auger <[email protected]> > Sent: 10 December 2025 08:19 > To: Shameer Kolothum <[email protected]>; qemu- > [email protected]; [email protected] > Cc: [email protected]; Nicolin Chen <[email protected]>; Nathan > Chen <[email protected]>; Matt Ochs <[email protected]>; Jason > Gunthorpe <[email protected]>; [email protected]; > [email protected]; [email protected]; Krishnakant Jaju > <[email protected]> > Subject: Re: [PATCH v2 4/4] hw/arm/smmuv3-accel: Read and propagate host > vIOMMU events > > External email: Use caution opening links or attachments > > > Hi Shameer, > > On 12/4/25 10:22 AM, Shameer Kolothum 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(). > is it future proof to do that in the main loop?
The reason I opt for the main loop is, vEVENTQ object is just for event records other than page faults and we are not expecting a large number of them in a normal Guest scenario. And if there are a lot of them, the there is something obviously going wrong. Page fault events are handles in a separate FAULT_QUEUE object. See the discussion here, https://lore.kernel.org/qemu-devel/cabqgh9hzb9ycd_ryjgfx5zc7rx2e2ivu_fzpu2vm-kuf3jf...@mail.gmail.com/ The page fault handling might require a dedicated thread to speed up things, I guess. > > > > Signed-off-by: Shameer Kolothum <[email protected]> > > --- > > hw/arm/smmuv3-accel.c | 58 > +++++++++++++++++++++++++++++++++++++++++++ > > hw/arm/smmuv3-accel.h | 2 ++ > > 2 files changed, 60 insertions(+) > > > > diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c > > index 74f0be3731..d320c62b04 100644 > > --- a/hw/arm/smmuv3-accel.c > > +++ b/hw/arm/smmuv3-accel.c > > @@ -378,6 +378,58 @@ 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) > So if I understand correctly this handler is called for every > header/data. There cannot be several of them to be consumed in the queue? That is my understanding how aio_dispatch_handler() works. I will double check. > > +{ > > + SMMUv3State *s = opaque; > > + SMMUv3AccelState *accel = s->s_accel; > > + struct { > > + struct iommufd_vevent_header hdr; > > + struct iommu_vevent_arm_smmuv3 vevent; > > + } buf; > > + ssize_t readsz = sizeof(buf); > > + uint32_t last_seq = accel->last_event_seq; > > + ssize_t bytes; > > + > > + bytes = read(accel->veventq->veventq_fd, &buf, readsz); > > + if (bytes <= 0) { > > + if (errno == EAGAIN || errno == EINTR) { > > + return; > > + } > > + error_report("vEVENTQ: read failed (%s)", strerror(errno)); > nit you can use %m directly Ok. > > + return; > > + } > > + > > + if (bytes < readsz) { > > + error_report("vEVENTQ: incomplete read (%zd/%zd bytes)", bytes, > readsz); > > + return; > > + } > > + > > + if (buf.hdr.flags & IOMMU_VEVENTQ_FLAG_LOST_EVENTS) { > > + error_report("vEVENTQ has lost events"); > once you get a lost_event, don't you need to reset the last_event_seq, > event_start to be able to consume again? Yes. I think we need to do that. > > + return; > > + } > > + > > + /* Check sequence in hdr for lost events if any */ > > + if (accel->event_start) { > > + uint32_t expected = (last_seq == INT_MAX) ? 0 : last_seq + 1; > > + > > + if (buf.hdr.sequence != expected) { > > + uint32_t delta; > > + > > + if (buf.hdr.sequence >= last_seq) { > > + delta = buf.hdr.sequence - last_seq; > > + } else { > > + /* Handle wraparound from INT_MAX */ > > + delta = (INT_MAX - last_seq) + buf.hdr.sequence + 1; > > + } > > + error_report("vEVENTQ: detected lost %u event(s)", delta - 1); > do we want to report all losses or just warn once? Will change to warn once. Thanks, Shameer
