On Tue, Nov 04, 2025 at 09:55:46AM +0100, Eric Auger wrote: > On 11/3/25 7:51 PM, Nicolin Chen wrote: > > On Mon, Nov 03, 2025 at 10:17:20AM -0800, Shameer Kolothum wrote: > >>>> The general > >>>> idea is, we will pass the errp to accel functions and report or > >>>> propagate from here. > >>> But there is no "errp" in smmuv3_cmdq_consume() to propagate the these > >>> local_errs further? It ends at the error_report_err(). > >>> > >>> If we only get local_err and print them, why not just print them inside > >>> the > >>> _accel functions? > >> Right, we don’t propagate error now. But in future it might come > >> handy. I would personally keep the error propagation facility if possible. > > smmuv3_cmdq_consume() is called in smmu_writel() only. Where do we > > plan to propagate that in the future? > > > >> Also, this was added as per Eric's comment on RFC v3. > >> > >> https://lore.kernel.org/qemu-devel/[email protected]/ > > If only we have a top function that does error_report_err() in one > > place.. Duplicating error_report_err(local_err) doesn't look clean > > to me. > > > > Maybe smmu_writel() could do: > > { > > + Error *errp = NULL; > > > > switch (offset) { > > case A_XXX: > > smmuv3_cmdq_consume(..., errp); > > + return MEMTX_OK; > > - break; > > ... > > case A_YYY: > > smmuv3_cmdq_consume(..., errp); > > + return MEMTX_OK; > > - break; > > } > > + error_report_err(errp); > > + return MEMTX_OK; > > } > > > > Any better idea, Eric? > > Can't we move local_err outside of case block and after the switch, > > if (cmd_error) { > if (local_err) { > error_report_err(local_err); > } > ../..
I see Shameer's vEVENTQ patch (WIP) has errp also that will end up an error_report_err() in the smmu_writel() for A_EVENTQ_BASE. So, it seems to be cleaner to do in the top function? I am fine with adding in cmdq function for now and moving later though. Thanks Nicolin
