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

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

[Richard Cheng]

On Fri, Jun 19, 2026 at 01:03:56AM +0800, Wei Hou wrote:
> 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
> 
>

Btw,

You missed to sent this one to many people, I'll add them this time for you

--Richard 

[PATCH v2] 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.

Fixes: 8d8081cecfb9 ("cxl: Move mailbox related bits to the same context")
Reviewed-by: Richard Cheng <icheng@nvidia.com>
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.43.0

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

[Li Ming]

On 6/28/2026 11:58 PM, Wei Hou wrote:
> 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.
>
> Fixes: 8d8081cecfb9 ("cxl: Move mailbox related bits to the same context")
> Reviewed-by: Richard Cheng <icheng@nvidia.com>
> Signed-off-by: Wei Hou <wei.hou@scaleflux.com>

Good catch!

Reviewed-by: Li Ming <ming.li@zohomail.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;

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

[Dave Jiang]

On 6/28/26 8:58 AM, Wei Hou wrote:
> 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.
> 
> Fixes: 8d8081cecfb9 ("cxl: Move mailbox related bits to the same context")
> Reviewed-by: Richard Cheng <icheng@nvidia.com>
> Signed-off-by: Wei Hou <wei.hou@scaleflux.com>

Applied to cxl/next
d79b81893d0c

Next time please post v2 as a separate post instead of having it reply to v1. Thanks.

> ---
>  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;