Re: [PATCH v9 04/11] cxl: Cache endpoint decoder settings during PCI enumeration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Enabling PCI_COMMAND_MEMORY and mapping unassigned BARs during early PCI enumeration causes severe resource conflicts and system instability.
- [Medium] Mixing goto-based error handling with scope-based cleanup helpers (`scoped_guard()`), leading to a logical fall-through bug.
--

--- Patch [4]: [PATCH v9 04/11] cxl: Cache endpoint decoder settings during PCI enumeration ---
commit 64ea438bc1dd54d340a46889f9f340251bdf644f
Author: Srirangan Madhavan <smadhavan@nvidia.com>

cxl: Cache endpoint decoder settings during PCI enumeration

This patch populates pci_dev->hdm from PCI capability initialization
for CXL.mem functions. It temporarily sets memory space enable to read
HDM MMIO and restores the original configuration value.

> diff --git a/drivers/cxl/core/reset.c b/drivers/cxl/core/reset.c
> --- a/drivers/cxl/core/reset.c
> +++ b/drivers/cxl/core/reset.c

[ ... ]

> +static int __pci_cxl_hdm_init(struct pci_dev *pdev)
> +{

[ ... ]

> +	if (!(command & PCI_COMMAND_MEMORY))
> +		restore_command = true;
> +
> +	if (restore_command) {
> +		rc = pci_write_config_word(pdev, PCI_COMMAND,
> +					   command | PCI_COMMAND_MEMORY);

[Severity: High]
Is it safe to forcefully enable memory decoding before the PCI core has
verified or assigned safe base address registers?

Since this function is called during early enumeration in
pci_init_capabilities():

drivers/pci/probe.c:pci_init_capabilities() {
    ...
    pci_ide_init(dev);
    pci_cxl_hdm_init(dev);
    ...
}

Could uninitialized or overlapping physical addresses cause the endpoint to
decode transactions intended for system memory, leading to resource
conflicts or machine check exceptions?

> +		if (rc)
> +			return pcibios_err_to_errno(rc);
> +	}

[ ... ]

> +	scoped_guard(rwsem_write, &cxl_rwsem.dpa) {
> +		if (pdev->hdm)
> +			goto out_unmap;

[Severity: Medium]
Could this goto inside a scoped_guard() block lead to logical fall-through
bugs?

The cleanup subsystem guidelines advise against mixing goto and scope-based
cleanups in the same function. If pdev->hdm is already set, execution
jumps to out_unmap and then falls through to out_restore_command.

Could this cause the configuration write to be re-executed redundantly
during the error handling fall-through?

> +		pdev->hdm = info;
> +	}
> +	iounmap(hdm);
> +	return 0;

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