Hi Eric,
> -----Original Message-----
> From: Eric Auger <[email protected]>
> Sent: 26 May 2026 09:27
> To: Shameer Kolothum Thodi <[email protected]>; qemu-
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; Nicolin
> Chen <[email protected]>; Nathan Chen <[email protected]>; Matt
> Ochs <[email protected]>; Jiandi An <[email protected]>; Jason Gunthorpe
> <[email protected]>; [email protected]; Krishnakant Jaju
> <[email protected]>; [email protected]
> Subject: Re: [PATCH v5 24/32] hw/arm/tegra241-cmdqv: Read and propagate
> Tegra241 CMDQV errors
>
> External email: Use caution opening links or attachments
>
>
> Hi Shameer,
>
> On 5/19/26 12:37 PM, Shameer Kolothum wrote:
> > Install an event handler on the CMDQV vEVENTQ fd to read and propagate
> > host received CMDQV errors to the guest.
> >
> > The handler runs in QEMU's main loop, using a non-blocking fd registered
> > via qemu_set_fd_handler().
> >
> > Reviewed-by: Nicolin Chen <[email protected]>
> > Signed-off-by: Shameer Kolothum <[email protected]>
> > ---
> > hw/arm/tegra241-cmdqv.c | 60
> +++++++++++++++++++++++++++++++++++++++++
> > hw/arm/trace-events | 1 +
> > 2 files changed, 61 insertions(+)
> >
> > diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
> > index 853c548e58..0bba2c1801 100644
> > --- a/hw/arm/tegra241-cmdqv.c
> > +++ b/hw/arm/tegra241-cmdqv.c
> > @@ -14,6 +14,7 @@
> >
> > #include "hw/arm/smmuv3.h"
> > #include "hw/arm/smmuv3-common.h"
> > +#include "hw/core/irq.h"
> > #include "smmuv3-accel.h"
> > #include "tegra241-cmdqv.h"
> > #include "trace.h"
> > @@ -736,6 +737,48 @@ out:
> > trace_tegra241_cmdqv_write_mmio(offset, value, size);
> > }
> >
> > +static void tegra241_cmdqv_event_read(void *opaque)
> > +{
> > + Tegra241CMDQV *cmdqv = opaque;
> > + IOMMUFDVeventq *veventq = cmdqv->veventq;
> > + struct {
> > + struct iommufd_vevent_header hdr;
> > + struct iommu_vevent_tegra241_cmdqv vevent;
> > + } buf;
> > + Error *local_err = NULL;
> > +
> > + if (!smmuv3_accel_event_read_validate(veventq,
> > +
> > IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV,
> > + &buf, sizeof(buf), &local_err)) {
> > + warn_report_err_once(local_err);
> > + return;
> > + }
> > +
> > + if (buf.vevent.lvcmdq_err_map[0] || buf.vevent.lvcmdq_err_map[1]) {
> > + cmdqv->vintf_cmdq_err_map[0] =
> > + extract64(buf.vevent.lvcmdq_err_map[0], 0, 32);
> > + cmdqv->vintf_cmdq_err_map[1] =
> > + extract64(buf.vevent.lvcmdq_err_map[0], 32, 32);
> > + cmdqv->vintf_cmdq_err_map[2] =
> > + extract64(buf.vevent.lvcmdq_err_map[1], 0, 32);
> > + cmdqv->vintf_cmdq_err_map[3] =
> > + extract64(buf.vevent.lvcmdq_err_map[1], 32, 32);
> > + /*
> > + * CMDQV_CMDQ_ERR_MAP and VINTF0_LVCMDQ_ERR_MAP are
> distinct
> > + * registers (different MMIO offsets). With only VINTF0 exposed
> > + * they carry the same data, so mirror.
> > + */
> > + for (int i = 0; i < 4; i++) {
> > + cmdqv->cmdq_err_map[i] = cmdqv->vintf_cmdq_err_map[i];
> > + }
> > + cmdqv->vi_err_map[0] |= 0x1;
> > + qemu_irq_pulse(cmdqv->irq);
> I don't see SMMU_CMDQV_VI_ERR_MAP_0 updated anywhere. If I
> understand
> correctly, if you get an error on any VCMDQ you should update it.
This one above does that:
cmdqv->vi_err_map[0] |= 0x1; it sets bit 0, which is the
error bit for VINTF0 (the only VINTF QEMU exposes).
I will add a comment to make it clear.
Also
> the triggering of the irq should depnd on SMMU_CMDQ_VI_INT_MASK0, no?
Right. will add:
if (!(cmdqv->vi_int_mask[0] & BIT(0))) {
qemu_irq_pulse(cmdqv->irq);
}
Thanks,
Shameer
