> -----Original Message----- > From: Eric Auger <[email protected]> > Sent: 12 February 2026 17:12 > To: Shameer Kolothum Thodi <[email protected]>; Nicolin Chen > <[email protected]> > Cc: [email protected]; [email protected]; > [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 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for > accelerated SMMUv3 devices > > External email: Use caution opening links or attachments > > > On 2/12/26 5:28 PM, Shameer Kolothum Thodi wrote: > > > >> -----Original Message----- > >> From: Nicolin Chen <[email protected]> > >> Sent: 12 February 2026 06:52 > >> To: Eric Auger <[email protected]> > >> Cc: Shameer Kolothum Thodi <[email protected]>; qemu- > >> [email protected]; [email protected]; [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 3/5] hw/arm/smmuv3-accel: Allocate vEVENTQ for > >> accelerated SMMUv3 devices > >> > > ... > >>> Usually error_append_hint() is used to append a user friendly msg that > >>> helps the user to fix the problem. > >>> In qapi/error.h there is an example that relates to "Receive and > >>> accumulate multiple errors (first one wins):" by using > >>> error_propagate() > >> I didn't look very closely but just run a grep for "append" :) > >> > >> Yea, I think error_propagate() should work better. > > I am not sure error_propagate makes things better though. > > > > If I am not wrong, this is how the change will look like, > > > > static MemTxResult smmu_writel(SMMUv3State *s, hwaddr offset, > > uint64_t data, MemTxAttrs attrs) > > { > > + Error *err = NULL; > > Error *local_err = NULL; > > > > switch (offset) { > > @@ -1614,9 +1616,15 @@ static MemTxResult > smmu_writel(SMMUv3State *s, hwaddr offset, > > s->cr0ack = data & ~SMMU_CR0_RESERVED; > > /* in case the command queue has been enabled */ > > smmuv3_cmdq_consume(s, &local_err); > > + if (local_err) { > > + error_propagate(&err, local_err); > > + local_err = NULL; > > + } > > /* Allocate vEVENTQ if EventQ is enabled and a vIOMMU is available > > */ > > - if (local_err == NULL) { > > - smmuv3_accel_alloc_veventq(s, &local_err); > > + smmuv3_accel_alloc_veventq(s, &local_err); > > + if (local_err) { > > + error_propagate(&err, local_err); > > + local_err = NULL; > > } > > break; > > case A_CR1: > > @@ -1724,8 +1732,8 @@ static MemTxResult smmu_writel(SMMUv3State > *s, hwaddr offset, > > break; > > } > > > > - if (local_err) { > > - error_report_err(local_err); > > + if (err) { > > + error_report_err(err); > > } > > return MEMTX_OK; > > } > > > > And error_propagate() only will keep the first error. > > > > Instead, if we do below: > > > > @@ -1614,10 +1615,12 @@ static MemTxResult > smmu_writel(SMMUv3State *s, hwaddr offset, > > s->cr0ack = data & ~SMMU_CR0_RESERVED; > > /* in case the command queue has been enabled */ > > smmuv3_cmdq_consume(s, &local_err); > > - /* Allocate vEVENTQ if EventQ is enabled and a vIOMMU is available > > */ > > - if (local_err == NULL) { > > - smmuv3_accel_alloc_veventq(s, &local_err); > > + if (local_err) { > > + error_report_err(local_err); > > + local_err = NULL; > > } > > + /* Allocate vEVENTQ if EventQ is enabled and a vIOMMU is available > > */ > > + smmuv3_accel_alloc_veventq(s, &local_err); > > break; > > If you want to accumulate, why don't you use the example in qapi/error.h: > > * Receive and accumulate multiple errors (first one wins): > * Error *err = NULL, *local_err = NULL; > * foo(arg, &err); > * bar(arg, &local_err); > * error_propagate(&err, local_err); > * if (err) { > * handle the error... > * } > *
This is the one more or less I attempted in first above. It will accumulate but only the first error is kept. Since we are treating both smmuv3_cmdq_consume() and smmuv3_accel_alloc_veventq() paths as independent, I thought it would be better to report both errors independently if it happens. May be it doesn't matter that much. I will switch to error_propagate() then. Thanks, Shameer
