Re: [PATCH v10 01/12] cxl: Move HDM decoder programming helpers

[email protected] Tue, 04 Aug 2026 19:46:30 +0000
Newsgroups org.kernel.vger.linux-cxl,org.kernel.vger.linux-pci
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:
- [Low] `resource.o` is incorrectly compiled as a built-in object when `CON=
FIG_CXL_BUS` is a module, causing fatal linker errors and breaking kernel m=
odularity.
--

--- Patch [1]: [PATCH v10 01/12] cxl: Move HDM decoder programming helpers =
---
commit b0d203a772e57924b3e867594bb9a15ce55bf00c
Author: Srirangan Madhavan <[email protected]>

cxl: Move HDM decoder programming helpers

Move the low-level HDM decoder programming helpers from hdm.c into
resource.c so they can be shared by later reset restore code.

> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
[ ... ]
> @@ -243,4 +244,7 @@ config CXL_ATL
>  	depends on CXL_REGION
>  	depends on ACPI_PRMT && AMD_NB
> =20
> +config CXL_HDM
> +	bool
> +
>  endif
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_CXL_BUS) +=3D cxl_core.o
> +obj-$(CONFIG_CXL_HDM) +=3D resource.o

[Severity: Low]
When CONFIG_CXL_BUS is configured as a loadable module (m), CONFIG_CXL_HDM
evaluates to 'y' because it is defined as a bool in the Kconfig.

Does this incorrectly assign resource.o to the built-in obj-y list instead
of appending it to the cxl_core-y list?

[ ... ]
> diff --git a/drivers/cxl/core/resource.c b/drivers/cxl/core/resource.c
> --- /dev/null
> +++ b/drivers/cxl/core/resource.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2026 NVIDIA Corporation & Affiliates */
> +#include <linux/delay.h>
> +#include <linux/bug.h>
> +#include <linux/errno.h>
> +#include <linux/export.h>
> +#include <linux/kernel.h>
> +
> +#include "cxl.h"
> +#include "core.h"
> +
> +struct cxl_rwsem cxl_rwsem =3D {
> +	.region =3D __RWSEM_INITIALIZER(cxl_rwsem.region),
> +	.dpa =3D __RWSEM_INITIALIZER(cxl_rwsem.dpa),
> +};
> +EXPORT_SYMBOL_FOR_MODULES(cxl_rwsem, "cxl_core");

[Severity: Low]
Are these internal locks being unnecessarily exported to the global
symbol table as a workaround for the Makefile defect?

If resource.o is properly compiled as part of the cxl_core module, we
shouldn't need to export them here.

[ ... ]
> +void cxl_setup_hw_decoder(struct cxl_decoder *cxld, void __iomem *hdm)
> +{
> +	int id =3D cxld->id;
> +	u64 base, size;
> +	u32 ctrl;
> +
> +	/* common decoder settings */
> +	ctrl =3D readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));
> +	cxld_set_interleave(cxld, &ctrl);
> +	cxld_set_type(cxld, &ctrl);
> +	base =3D cxld->hpa_range.start;
> +	size =3D range_len(&cxld->hpa_range);
> +
> +	writel(upper_32_bits(base), hdm + CXL_HDM_DECODER0_BASE_HIGH_OFFSET(id)=
);
> +	writel(lower_32_bits(base), hdm + CXL_HDM_DECODER0_BASE_LOW_OFFSET(id));
> +	writel(upper_32_bits(size), hdm + CXL_HDM_DECODER0_SIZE_HIGH_OFFSET(id)=
);
> +	writel(lower_32_bits(size), hdm + CXL_HDM_DECODER0_SIZE_LOW_OFFSET(id));
> +
> +	if (is_switch_decoder(&cxld->dev)) {

[Severity: Low]
Because resource.o may be built-in, won't this call to is_switch_decoder()
cause an undefined reference linker error when it attempts to resolve this
module-bound symbol from cxl_core.ko?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804192958.1823=
[email protected]?part=3D1