Re: [PATCH v17 09/19] cxl: Refresh cached PCI HDM decoder settings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] cxl: split declaration and init of __free() variable in cxl_hdm_refresh_decoder()
- [Low] cxl: unused struct cxl_hdm_info forward declaration in include/cxl/cxl.h
--

commit 0273d4d08ae78949f901788b93c7db64fb203e17
Author: Srirangan Madhavan <smadhavan@nvidia.com>

cxl: Refresh cached PCI HDM decoder settings

This commit refreshes the cached PCI HDM decoder state when hardware HDM
decoders are enumerated, committed, or reset, and validates the enumerated
decoder count against the cached count.

> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 85f9abf58a42b..ec387070d7adb 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -725,6 +725,24 @@ static void cxlsd_set_targets(struct cxl_switch_decoder *cxlsd, u64 *tgt)
>  		*tgt |= FIELD_PREP(GENMASK_ULL(63, 56), t[7]->port_id);
>  }
>  
> +static void cxl_hdm_refresh_decoder(struct cxl_hdm *cxlhdm,
> +				    struct cxl_decoder *cxld)
> +{
> +	void __iomem *hdm = cxlhdm->regs.hdm_decoder;
> +	struct pci_dev *pdev __free(pci_dev_put) = NULL;
> +
> +	if (!hdm)
> +		return;
> +	if (!is_endpoint_decoder(&cxld->dev))
> +		return;
> +
> +	pdev = cxl_port_get_uport_pci_dev(cxlhdm->port);

[Severity: Low]
As reported by Cheatham, Benjamin on v13 [1], this isn't a bug, but in
cxl_hdm_refresh_decoder() in drivers/cxl/core/hdm.c, pdev is declared with
__free(pci_dev_put) = NULL at the top of the function and assigned later after
the !hdm and !is_endpoint_decoder(&cxld->dev) early returns.

Should pdev be defined and initialized in a single statement where
cxl_port_get_uport_pci_dev(cxlhdm->port) is called after the early returns,
matching cxl_hdm_check_decoder_count() in drivers/cxl/core/pci.c and the
kernel Cleanup API conventions?

[ ... ]

> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> index 6d6e564be27a9..341a7c52894ab 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -26,6 +26,7 @@ enum cxl_devtype {
>  };
>  
>  struct cxl_region;
> +struct cxl_hdm_info;

[Severity: Low]
This isn't a bug, but struct cxl_hdm_info is not referenced anywhere in
include/cxl/cxl.h (it is defined in include/cxl/hdm.h and forward-declared in
include/linux/pci.h). Can this forward declaration be dropped from
include/cxl/cxl.h?

[1] https://lore.kernel.org/r/2c37c82b-8db6-44d1-868b-0b98bae57459@amd.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105185917.2667656-1-smadhavan@nvidia.com?part=9