Re: [PATCH V3 11/19] accel/amdxdna: Decouple AIE4 doorbell and MSI-X notify transport hooks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] cleanup: mixing goto-based error handling with guard()
--

commit 24bec1792936c12968d8086392604ffa71c7cce1
Author: David Zhang <yidong.zhang@amd.com>

accel/amdxdna: Decouple AIE4 doorbell and MSI-X notify transport hooks

This commit separates PCI-specific doorbell and interrupt notification handling
from the transport-neutral context code.

> diff --git a/drivers/accel/amdxdna/aie4_ctx.c b/drivers/accel/amdxdna/aie4_ctx.c
> index 5eb918e1d58c7..5a2fc19bad206 100644
> --- a/drivers/accel/amdxdna/aie4_ctx.c
> +++ b/drivers/accel/amdxdna/aie4_ctx.c
[ ... ]
> @@ -51,32 +42,27 @@ static struct cert_comp *aie4_lookup_cert_comp(struct amdxdna_dev_hdl *ndev, u32
[ ... ]
> -	ret = request_irq(cert_comp->irq, cert_comp_isr, 0, "xdna_hsa", cert_comp);
> +	/* Transport-specific: PCI wires an MSI-X irq, platform an IPI callback. */
> +	ret = aie4_request_notification(cert_comp);
>  	if (ret) {
> -		XDNA_ERR(xdna, "request irq %d failed %d", cert_comp->irq, ret);
> +		XDNA_ERR(xdna, "request notification for msix idx %u failed %d", msix_idx, ret);
>  		goto free_cert_comp;
>  	}
>  
>  	ret = xa_err(xa_store(&ndev->cert_comp_xa, msix_idx, cert_comp, GFP_KERNEL));
>  	if (ret) {
> -		XDNA_ERR(xdna, "store cert_comp for msix idx %d failed %d", msix_idx, ret);
> +		XDNA_ERR(xdna, "store cert_comp for msix idx %u failed %d", msix_idx, ret);
>  		goto free_irq;
>  	}
>  
>  	return cert_comp;
>  
>  free_irq:
> -	free_irq(cert_comp->irq, cert_comp);
> +	aie4_free_notification(cert_comp);
>  free_cert_comp:
>  	kfree(cert_comp);
>  	return NULL;

[Severity: Low]
Is it safe to continue using goto-based error handling here?

Looking earlier in aie4_lookup_cert_comp(), a scope-based lock is declared:

	guard(mutex)(&ndev->cert_comp_lock);

The kernel cleanup subsystem guidelines recommend that goto statements and
scope-based cleanup helpers like guard() should never be mixed in the same
function. Mixing them creates confusing ownership semantics and increases
the risk of double-free bugs or resource leaks during future modifications.

Could we either convert all resources in aie4_lookup_cert_comp() to
scope-based cleanup, or avoid using guard() in this function entirely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008032348.2044667-1-yidong.zhang@amd.com?part=11