Re: [PATCH 22/24] irqchip/gic-v5: Add GICv5 ITS support

[Lorenzo Pieralisi]

On Wed, Apr 09, 2025 at 08:57:02PM +0200, Thomas Gleixner wrote:
> On Tue, Apr 08 2025 at 12:50, Lorenzo Pieralisi wrote:
> > +	guard(mutex)(&its->dev_alloc_lock);
> > +
> > +	its_dev = gicv5_its_find_device(its, device_id);
> > +	if (IS_ERR(its_dev))
> > +		return PTR_ERR(its_dev);
> > +
> > +	ret = gicv5_its_alloc_eventid(its_dev, nr_irqs, &event_id_base);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = iommu_dma_prepare_msi(info->desc,
> > +				    its->its_trans_phys_base);
> > +	if (ret)
> > +		goto out_eventid;
> 
> As 0-day already complained, guard() and goto does not mix. the simple
> solution is to do
> 
> foo()
>         guard()
>         return __foo();
> 
> Then you can put the gotos into __foo()

Yes even though its usage in this function IIUC is technically fine,
there are other kernel functions where the guard(mutex) is at the
function entry with gotos that follow.

Regardless, bad idea to mix them, as documented, thanks.

Side note: the bug the bot caught is because I moved the guard(mutex)
lower in the function and incidentally got past gotos (and gcc did not
catch it, it has to go up to W=2 to barf).

Anyway - as I mentioned in the cover letter, the current IWB driver
design, (patch 23 - where the guard(mutex) bug is and more readl_poll_*
boilerplate :), sorry), is a bit of a question (should I move it to
DOMAIN_BUS_WIRED_TO_MSI with the fixed eventid caveat to solve somehow),
or is it OK "as-is", treating it basically as an ITS hardcoded plugin ?

Thank you very much for having a look.

Lorenzo