Hi Shameer,

On 2/11/26 3:58 PM, Shameer Kolothum Thodi wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <[email protected]>
>> Sent: 11 February 2026 14:26
>> To: Shameer Kolothum Thodi <[email protected]>; qemu-
>> [email protected]; [email protected]
>> Cc: [email protected]; Nicolin Chen <[email protected]>; Nathan
>> Chen <[email protected]>; Matt Ochs <[email protected]>; Jiandi An
>> <[email protected]>; Jason Gunthorpe <[email protected]>;
>> [email protected]; [email protected];
>> [email protected]; Krishnakant Jaju <[email protected]>
>> Subject: Re: [PATCH v5 5/5] hw/arm/smmuv3-accel: Read and propagate host
>> vIOMMU events
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Shameer,
>>
>> On 2/11/26 9:34 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().
>>>
>>> Tested-by: Nicolin Chen <[email protected]>
>>> Signed-off-by: Shameer Kolothum <[email protected]>
>>> ---
>>>  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 d92fcb1a89..0d5dcef941 100644
>>> --- a/hw/arm/smmuv3-accel.c
>>> +++ b/hw/arm/smmuv3-accel.c
>>> @@ -390,6 +390,48 @@ 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;
>>> +    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: read failed (%m)");
>>> +        return;
>>> +    }
>>> +
>>> +    if (bytes == sizeof(buf.hdr) &&
>>> +        (buf.hdr.flags & IOMMU_VEVENTQ_FLAG_LOST_EVENTS)) {
>>> +        error_report_once("vEVENTQ has lost events");
>> in case in the future we end up with several vEVENTQs, it may be
>> relevant to log the queue id/type
>>
>> After reading the linux uapi again, this seems to record a queue
>> overflow. I would replace the
>>
>> "vEVENTQ has lost events" trace by "queue <type> <id> has overflowed". This
>> would allow differentiation with the case below.
> Ok. Make sense. <id> is also useful with multiple SMMUv3 instances.
>
>>> +        accel->event_start = false;
>>> +        return;
>>> +    }
>>> +    if (bytes < sizeof(buf)) {
>>> +        error_report_once("vEVENTQ: incomplete read (%zd/%zd bytes)",
>>> +                          bytes, sizeof(buf));
>>> +        return;
>>> +    }
>>> +
>>> +    /* Check sequence in hdr for lost events if any */
>>> +    if (accel->event_start && (buf.hdr.sequence - last_seq != 1)) {
>> wonder if we need to handle any hypothetical wrap-up situation?
> This should not be required. The sequence is __u32 and the delta
>  is computed using unsigned arithmetic, so wrap-around is handled
> naturally.
Ah that's right.

With the 1st comment above taken into account, feel free to add my 

Reviewed-by: Eric Auger <[email protected]>

Thanks

Eric
>
> Thanks,
> Shameer


Reply via email to