Hi,
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);
}
../..
Eric
>
> Nicolin
>
