[PATCH] cxl/pci: Remove incorrect mbox.valid check in cxl_pci_type3_init_mailbox()

[PATCH] cxl/pci: Remove incorrect mbox.valid check in cxl_pci_type3_init_mailbox()

[Wei Hou]

The driver's design intent is that missing or malformed component
registers should not prevent mailbox initialization. cxl_pci_probe()
already reflects this: the CXL_REGLOC_RBI_COMPONENT setup path only
emits a dev_warn() and continues when component registers are absent,
rather than returning an error.

The check 'if (!cxlds->reg_map.device_map.mbox.valid)' violates this
intent and is also technically incorrect for two reasons:

1. Wrong struct: the MEMDEV register block is enumerated into a local
   variable 'map', not into 'cxlds->reg_map'. The device_map.mbox.valid
   field inside cxlds->reg_map is never written by the MEMDEV probe and
   will always read as zero regardless of actual hardware capability.

2. Already validated: cxl_pci_setup_regs(CXL_REGLOC_RBI_MEMDEV) calls
   cxl_probe_regs() which explicitly checks mbox.valid and returns
   -ENXIO if the mailbox is absent. If that check passes, the mailbox is
   guaranteed to be present by the time cxl_pci_type3_init_mailbox() is
   called.

The value that the check actually reads is component_map.ras.valid,
which aliases device_map.mbox.valid in the union. This is populated by
the COMPONENT probe, not the MEMDEV probe. On devices where the
component register BAR does not implement a CXL Component Capability
Array (e.g. certain DCD devices), cxl_probe_component_regs() returns
early leaving ras.valid=false. Through the union, this makes mbox.valid
read as false, causing cxl_pci_type3_init_mailbox() to return -ENODEV
(-19) even though the mailbox hardware is fully functional.

Remove the check. Mailbox presence has already been validated by
cxl_pci_setup_regs(CXL_REGLOC_RBI_MEMDEV). The presence or absence of
component registers is irrelevant to mailbox initialization.

Signed-off-by: Wei Hou <wei.hou@scaleflux.com>
---
 drivers/cxl/pci.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index bace662dc988..e44e5d209900 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -691,12 +691,6 @@ static int cxl_pci_type3_init_mailbox(struct cxl_dev_state *cxlds)
 {
 	int rc;
 
-	/*
-	 * Fail the init if there's no mailbox. For a type3 this is out of spec.
-	 */
-	if (!cxlds->reg_map.device_map.mbox.valid)
-		return -ENODEV;
-
 	rc = cxl_mailbox_init(&cxlds->cxl_mbox, cxlds->dev);
 	if (rc)
 		return rc;
-- 
2.24.1

Re: [PATCH] cxl/pci: Remove incorrect mbox.valid check in cxl_pci_type3_init_mailbox()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver swallows -EPROBE_DEFER (and potentially other errors like -ENOMEM) returned from cxl_pci_setup_regs() when initializing component registers. For Restricted CXL Host (RCH) topologies, cxl_pci_setup_regs() explicitly returns -EPROBE_DEFER if the upstream port isn't initialized yet. However, cxl_pci_probe() only logs a warning and falls through, where the rc variable is subsequently overwritten by cxl_pci_type3_init_mailbox(cxlds). As a result, the probe succeeds and the device binds permanently without its component registers, disabling features like RAS.
--

commit 4e1380c792dda7219b704feca6c3a617dfdf2972
Author: Wei Hou <wei.hou@scaleflux.com>

cxl/pci: Remove incorrect mbox.valid check in cxl_pci_type3_init_mailbox()

This commit removes the mailbox validation check from
cxl_pci_type3_init_mailbox() since mailbox presence is already verified
by the MEMDEV probe. It notes that missing component registers should
not prevent initialization.

> The driver's design intent is that missing or malformed component
> registers should not prevent mailbox initialization. cxl_pci_probe()
> already reflects this: the CXL_REGLOC_RBI_COMPONENT setup path only
> emits a dev_warn() and continues when component registers are absent,
> rather than returning an error.

[Severity: High]
This is a pre-existing issue, but does this design intent inadvertently
swallow -EPROBE_DEFER or other errors like -ENOMEM?

If cxl_pci_setup_regs() returns -EPROBE_DEFER (such as for Restricted CXL
Host topologies when the upstream port isn't initialized yet),
cxl_pci_probe() logs a warning and continues. The rc variable is then
immediately overwritten:

drivers/cxl/pci.c:cxl_pci_probe() {
    ...
	rc = cxl_pci_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT,
				&cxlds->reg_map);
	if (rc)
		dev_warn(&pdev->dev, "No component registers (%d)\n", rc);
	else if (!cxlds->reg_map.component_map.ras.valid)
		dev_dbg(&pdev->dev, "RAS registers not found\n");

	rc = cxl_pci_type3_init_mailbox(cxlds);
    ...
}

Will this cause the probe to succeed without component registers and
bypass probe deferral, permanently disabling features like RAS?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618170356.25382-1-wei.hou@scaleflux.com?part=1