diff mbox series

[v4,23/23] PCI/AER: Unmask RCEC internal errors to enable RCH downstream port error handling

Message ID 20230523232214.55282-24-terry.bowman@amd.com
State Superseded
Headers show
Series cxl/pci: Add support for RCH RAS error handling | expand

Commit Message

Bowman, Terry May 23, 2023, 11:22 p.m. UTC 
From: Robert Richter <rrichter@amd.com>

AER corrected and uncorrectable internal errors (CIE/UIE) are masked
in their corresponding mask registers per default once in power-up
state. [1][2] Enable internal errors for RCECs to receive CXL
downstream port errors of Restricted CXL Hosts (RCHs).

[1] CXL 3.0 Spec, 12.2.1.1 - RCH Downstream Port Detected Errors
[2] PCIe Base Spec 6.0, 7.8.4.3 Uncorrectable Error Mask Register,
    7.8.4.6 Correctable Error Mask Register

Co-developed-by: Terry Bowman <terry.bowman@amd.com>
Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/pci/pcie/aer.c | 64 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

Comments

Bjorn Helgaas May 24, 2023, 9:45 p.m. UTC | #1 
On Tue, May 23, 2023 at 06:22:14PM -0500, Terry Bowman wrote:
> From: Robert Richter <rrichter@amd.com>
> 
> AER corrected and uncorrectable internal errors (CIE/UIE) are masked
> in their corresponding mask registers per default once in power-up
> state. [1][2] Enable internal errors for RCECs to receive CXL
> downstream port errors of Restricted CXL Hosts (RCHs).
> 
> [1] CXL 3.0 Spec, 12.2.1.1 - RCH Downstream Port Detected Errors
> [2] PCIe Base Spec 6.0, 7.8.4.3 Uncorrectable Error Mask Register,
>     7.8.4.6 Correctable Error Mask Register

I use "r6.0" to make sure it isn't mistaken for a section number.

> Co-developed-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/pci/pcie/aer.c | 64 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 2e3f00b6a5bd..c5076ae4eb58 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -948,6 +948,32 @@ static bool find_source_device(struct pci_dev *parent,
>  
>  #ifdef CONFIG_PCIEAER_CXL
>  
> +static int pci_aer_unmask_internal_errors(struct pci_dev *dev)
> +{
> +	int aer, rc;
> +	u32 mask;
> +
> +	if (!pcie_aer_is_native(dev))
> +		return -EIO;
> +
> +	aer = dev->aer_cap;
> +	rc = pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, &mask);
> +	if (rc)
> +		return rc;

I don't think there's much value in checking all these config accesses
for failure.  A failure return really just means you called it with
invalid parameters; it doesn't tell you whether it was successful on
PCI.

> +	mask &= ~PCI_ERR_UNC_INTN;
> +	rc = pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, mask);
> +	if (rc)
> +		return rc;
> +
> +	rc = pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK, &mask);
> +	if (rc)
> +		return rc;
> +	mask &= ~PCI_ERR_COR_INTERNAL;
> +	rc = pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, mask);
> +
> +	return rc;
> +}
> +
>  static bool is_cxl_mem_dev(struct pci_dev *dev)
>  {
>  	/*
> @@ -1031,7 +1057,44 @@ static void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info)
>  		pcie_walk_rcec(dev, cxl_rch_handle_error_iter, info);
>  }
>  
> +static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
> +{
> +	int *handles_cxl = data;
> +
> +	*handles_cxl = is_cxl_mem_dev(dev) && cxl_error_is_native(dev);

This effectively only looks at the *last* RCiEP associated with this
RCEC.  I would expect a logical OR of all of them.

I see this is another use of is_cxl_mem_dev() and
cxl_error_is_native() that really requires them to be in this file.

> +	return *handles_cxl;
> +}
> +
> +static bool handles_cxl_errors(struct pci_dev *rcec)
> +{
> +	int handles_cxl = 0;
> +
> +	if (pci_pcie_type(rcec) == PCI_EXP_TYPE_RC_EC &&
> +	    pcie_aer_is_native(rcec))
> +		pcie_walk_rcec(rcec, handles_cxl_error_iter, &handles_cxl);
> +
> +	return !!handles_cxl;
> +}
> +
> +static void cxl_rch_enable_rcec(struct pci_dev *rcec)
> +{
> +	if (!handles_cxl_errors(rcec))
> +		return;
> +
> +	/*
> +	 * Internal errors are masked by default, unmask RCEC's here
> +	 * PCI6.0 7.8.4.3 Uncorrectable Error Mask Register (Offset 08h)
> +	 * PCI6.0 7.8.4.6 Correctable Error Mask Register (Offset 14h)

The spec references seem superfluous here.  The PCI_ERR_UNCOR_MASK and
PCI_ERR_COR_MASK in pci_aer_unmask_internal_errors() are pretty good
pointers.

> +	 */
> +	if (pci_aer_unmask_internal_errors(rcec))
> +		pci_err(rcec, "CXL: Failed to unmask internal errors");
> +	else
> +		pci_info(rcec, "CXL: Internal errors unmasked");
> +}
> +
>  #else
> +static inline void cxl_rch_enable_rcec(struct pci_dev *dev) { }
>  static inline void cxl_rch_handle_error(struct pci_dev *dev,
>  					struct aer_err_info *info) { }
>  #endif
> @@ -1432,6 +1495,7 @@ static int aer_probe(struct pcie_device *dev)
>  		return status;
>  	}
>  
> +	cxl_rch_enable_rcec(port);

Could this be done by the driver that claims the CXL RCiEP?  There's
no point in unmasking the errors before there's a driver with
pci_error_handlers that can do something with them anyway.

>  	aer_enable_rootport(rpc);
>  	pci_info(port, "enabled with IRQ %d\n", dev->irq);
>  	return 0;
> -- 
> 2.34.1
>
Robert Richter May 25, 2023, 10:08 p.m. UTC | #2 
On 24.05.23 16:45:06, Bjorn Helgaas wrote:
> On Tue, May 23, 2023 at 06:22:14PM -0500, Terry Bowman wrote:
> > From: Robert Richter <rrichter@amd.com>
> > 
> > AER corrected and uncorrectable internal errors (CIE/UIE) are masked
> > in their corresponding mask registers per default once in power-up
> > state. [1][2] Enable internal errors for RCECs to receive CXL
> > downstream port errors of Restricted CXL Hosts (RCHs).
> > 
> > [1] CXL 3.0 Spec, 12.2.1.1 - RCH Downstream Port Detected Errors
> > [2] PCIe Base Spec 6.0, 7.8.4.3 Uncorrectable Error Mask Register,
> >     7.8.4.6 Correctable Error Mask Register
> 
> I use "r6.0" to make sure it isn't mistaken for a section number.
> 
> > Co-developed-by: Terry Bowman <terry.bowman@amd.com>
> > Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> > Signed-off-by: Robert Richter <rrichter@amd.com>
> > ---
> >  drivers/pci/pcie/aer.c | 64 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> > 
> > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> > index 2e3f00b6a5bd..c5076ae4eb58 100644
> > --- a/drivers/pci/pcie/aer.c
> > +++ b/drivers/pci/pcie/aer.c
> > @@ -948,6 +948,32 @@ static bool find_source_device(struct pci_dev *parent,
> >  
> >  #ifdef CONFIG_PCIEAER_CXL
> >  
> > +static int pci_aer_unmask_internal_errors(struct pci_dev *dev)
> > +{
> > +	int aer, rc;
> > +	u32 mask;
> > +
> > +	if (!pcie_aer_is_native(dev))
> > +		return -EIO;
> > +
> > +	aer = dev->aer_cap;
> > +	rc = pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, &mask);
> > +	if (rc)
> > +		return rc;
> 
> I don't think there's much value in checking all these config accesses
> for failure.  A failure return really just means you called it with
> invalid parameters; it doesn't tell you whether it was successful on
> PCI.
> 
> > +	mask &= ~PCI_ERR_UNC_INTN;
> > +	rc = pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, mask);
> > +	if (rc)
> > +		return rc;
> > +
> > +	rc = pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK, &mask);
> > +	if (rc)
> > +		return rc;
> > +	mask &= ~PCI_ERR_COR_INTERNAL;
> > +	rc = pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, mask);
> > +
> > +	return rc;
> > +}
> > +
> >  static bool is_cxl_mem_dev(struct pci_dev *dev)
> >  {
> >  	/*
> > @@ -1031,7 +1057,44 @@ static void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info)
> >  		pcie_walk_rcec(dev, cxl_rch_handle_error_iter, info);
> >  }
> >  
> > +static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
> > +{
> > +	int *handles_cxl = data;
> > +
> > +	*handles_cxl = is_cxl_mem_dev(dev) && cxl_error_is_native(dev);
> 
> This effectively only looks at the *last* RCiEP associated with this
> RCEC.  I would expect a logical OR of all of them.
> 
> I see this is another use of is_cxl_mem_dev() and
> cxl_error_is_native() that really requires them to be in this file.
> 
> > +	return *handles_cxl;

If this is non-zero, the iteration stops. So as soon we find a cxl
device we can stop the loop. Else, all devices are non-cxl devs and
the last return is zero too.

Now checking the code, pci_walk_bus() works that way, but walk_rcec()
does not break in all cases. I think this function not working as
expected. We would need to check if pci_walk_bus() stopped the
iteration, e.g. with a return code.

Alternatively we could add this check:

	if (!*handles_cxl)
		*handles_cxl = ...

> > +}
> > +
> > +static bool handles_cxl_errors(struct pci_dev *rcec)
> > +{
> > +	int handles_cxl = 0;
> > +
> > +	if (pci_pcie_type(rcec) == PCI_EXP_TYPE_RC_EC &&
> > +	    pcie_aer_is_native(rcec))
> > +		pcie_walk_rcec(rcec, handles_cxl_error_iter, &handles_cxl);
> > +
> > +	return !!handles_cxl;
> > +}
> > +
> > +static void cxl_rch_enable_rcec(struct pci_dev *rcec)
> > +{
> > +	if (!handles_cxl_errors(rcec))
> > +		return;
> > +
> > +	/*
> > +	 * Internal errors are masked by default, unmask RCEC's here
> > +	 * PCI6.0 7.8.4.3 Uncorrectable Error Mask Register (Offset 08h)
> > +	 * PCI6.0 7.8.4.6 Correctable Error Mask Register (Offset 14h)
> 
> The spec references seem superfluous here.  The PCI_ERR_UNCOR_MASK and
> PCI_ERR_COR_MASK in pci_aer_unmask_internal_errors() are pretty good
> pointers.
> 
> > +	 */
> > +	if (pci_aer_unmask_internal_errors(rcec))
> > +		pci_err(rcec, "CXL: Failed to unmask internal errors");
> > +	else
> > +		pci_info(rcec, "CXL: Internal errors unmasked");
> > +}
> > +
> >  #else
> > +static inline void cxl_rch_enable_rcec(struct pci_dev *dev) { }
> >  static inline void cxl_rch_handle_error(struct pci_dev *dev,
> >  					struct aer_err_info *info) { }
> >  #endif
> > @@ -1432,6 +1495,7 @@ static int aer_probe(struct pcie_device *dev)
> >  		return status;
> >  	}
> >  
> > +	cxl_rch_enable_rcec(port);
> 
> Could this be done by the driver that claims the CXL RCiEP?  There's
> no point in unmasking the errors before there's a driver with
> pci_error_handlers that can do something with them anyway.

This sounds reasonable at the first glance. The problem is there could
be many devices associated with the RCEC. Not all of them will be
bound to a driver and handler at the same time. We would need to
refcount it or maintain a list of enabled devices. But there is
already something similar by checking dev->driver. But right, AER
errros could be seen and handled then at least on PCI level. I tent to
permanently enable RCEC AER, but that could cause side-effects. What
do you think?

Thanks,

-Robert

> 
> >  	aer_enable_rootport(rpc);
> >  	pci_info(port, "enabled with IRQ %d\n", dev->irq);
> >  	return 0;
> > -- 
> > 2.34.1
> >
Bjorn Helgaas May 26, 2023, 8:31 p.m. UTC | #3 
On Fri, May 26, 2023 at 12:08:33AM +0200, Robert Richter wrote:
> On 24.05.23 16:45:06, Bjorn Helgaas wrote:
> > On Tue, May 23, 2023 at 06:22:14PM -0500, Terry Bowman wrote:
> > > From: Robert Richter <rrichter@amd.com>
> > > 
> > > AER corrected and uncorrectable internal errors (CIE/UIE) are masked
> > > in their corresponding mask registers per default once in power-up
> > > state. [1][2] Enable internal errors for RCECs to receive CXL
> > > downstream port errors of Restricted CXL Hosts (RCHs).
> > > ...

> > > +static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
> > > +{
> > > +	int *handles_cxl = data;
> > > +
> > > +	*handles_cxl = is_cxl_mem_dev(dev) && cxl_error_is_native(dev);
> > 
> > This effectively only looks at the *last* RCiEP associated with this
> > RCEC.  I would expect a logical OR of all of them.
> > 
> > > +	return *handles_cxl;
> 
> If this is non-zero, the iteration stops. So as soon we find a cxl
> device we can stop the loop. Else, all devices are non-cxl devs and
> the last return is zero too.
> 
> Now checking the code, pci_walk_bus() works that way, but walk_rcec()
> does not break in all cases. I think this function not working as
> expected. We would need to check if pci_walk_bus() stopped the
> iteration, e.g. with a return code.
> 
> Alternatively we could add this check:
> 
> 	if (!*handles_cxl)
> 		*handles_cxl = ...

If handles_cxl_error_iter() returns 1 (device is CXL mem, etc),
pci_walk_bus() will terminate.  And handles_cxl_error_iter() also sets
*userdata to 1, so handles_cxl_errors() will return true.

I think that's all you need in this case: at least one associated
RCiEP might report errors you care about, so you should unmask RCEC
internal errors.  You don't need to look at *all* the RCiEPs to know
that.

In the other case, cxl_rch_handle_error() does need to look at all the
RCiEPs, and cxl_rch_handle_error_iter() always returns 0, so it should
never terminate pci_walk_bus().

So I think I raised a false alarm here, and the current patches work
fine as-is.  But I do think it's a little bit tricky to set
*handles_cxl and also use that as the return value and rely on it
terminating the loop.  Maybe something like this would be more
straightforward?

  static int handles_cxl_error_iter(...)
  {
    ...
    *handles_cxl |= is_cxl_mem_dev(dev) && cxl_error_is_native(dev);
    return 0;
  }

Certainly not as efficient because it looks at more RCiEPs than
strictly necessary.

> > > +static bool handles_cxl_errors(struct pci_dev *rcec)
> > > +{
> > > +	int handles_cxl = 0;
> > > +
> > > +	if (pci_pcie_type(rcec) == PCI_EXP_TYPE_RC_EC &&
> > > +	    pcie_aer_is_native(rcec))
> > > +		pcie_walk_rcec(rcec, handles_cxl_error_iter, &handles_cxl);
> > > +
> > > +	return !!handles_cxl;

> > > @@ -1432,6 +1495,7 @@ static int aer_probe(struct pcie_device *dev)
> > >  		return status;
> > >  	}
> > >  
> > > +	cxl_rch_enable_rcec(port);
> > 
> > Could this be done by the driver that claims the CXL RCiEP?  There's
> > no point in unmasking the errors before there's a driver with
> > pci_error_handlers that can do something with them anyway.
> 
> This sounds reasonable at the first glance. The problem is there could
> be many devices associated with the RCEC. Not all of them will be
> bound to a driver and handler at the same time. We would need to
> refcount it or maintain a list of enabled devices. But there is
> already something similar by checking dev->driver. But right, AER
> errors could be seen and handled then at least on PCI level. I tent to
> permanently enable RCEC AER, but that could cause side-effects. What
> do you think?

IIUC, this really just affects CXL devices, so I think the choice is
(1) always unmask internal errors for RCECs where those CXL devices
report errors (as this patch does), or (2) unmask when first CXL
driver that can handle the errors is loaded and restore previous state
when last one is unloaded.

If the RCEC *only* handles errors for CXL devices, i.e., not for a mix
of vanilla PCIe RCiEPs and CXL RCiEPs, I think I'm OK with (1).  I
think you said only the CXL driver knows how to collect and interpret
the error data.  Is it OK that when no such driver is loaded, we field
error interrupts silently, without even mentioning that an error
occurred?  I guess without the driver, the device is probably not in
use.

Bjorn
Jonathan Cameron June 1, 2023, 2:11 p.m. UTC | #4 
> > > > @@ -1432,6 +1495,7 @@ static int aer_probe(struct pcie_device *dev)
> > > >  		return status;
> > > >  	}
> > > >  
> > > > +	cxl_rch_enable_rcec(port);  
> > > 
> > > Could this be done by the driver that claims the CXL RCiEP?  There's
> > > no point in unmasking the errors before there's a driver with
> > > pci_error_handlers that can do something with them anyway.  
> > 
> > This sounds reasonable at the first glance. The problem is there could
> > be many devices associated with the RCEC. Not all of them will be
> > bound to a driver and handler at the same time. We would need to
> > refcount it or maintain a list of enabled devices. But there is
> > already something similar by checking dev->driver. But right, AER
> > errors could be seen and handled then at least on PCI level. I tent to
> > permanently enable RCEC AER, but that could cause side-effects. What
> > do you think?  
> 
> IIUC, this really just affects CXL devices, so I think the choice is
> (1) always unmask internal errors for RCECs where those CXL devices
> report errors (as this patch does), or (2) unmask when first CXL
> driver that can handle the errors is loaded and restore previous state
> when last one is unloaded.
> 
> If the RCEC *only* handles errors for CXL devices, i.e., not for a mix
> of vanilla PCIe RCiEPs and CXL RCiEPs, I think I'm OK with (1).  I
> think you said only the CXL driver knows how to collect and interpret
> the error data.  Is it OK that when no such driver is loaded, we field
> error interrupts silently, without even mentioning that an error
> occurred?  I guess without the driver, the device is probably not in
> use.

It might be in use.  Firmware may well have set up the CXL device and
even have put the kernel image in that memory for example. OS first RAS
handling won't be up until the driver loads though.  Would be a bit
odd to mix OS first handling with firmware setup. I'd expect firmware
first handling in that case, but I don't think anything stops the two
being mixed.

Jonathan
Robert Richter June 2, 2023, 4:41 p.m. UTC | #5 
On 01.06.23 15:11:34, Jonathan Cameron wrote:
> 
> > > > > @@ -1432,6 +1495,7 @@ static int aer_probe(struct pcie_device *dev)
> > > > >  		return status;
> > > > >  	}
> > > > >  
> > > > > +	cxl_rch_enable_rcec(port);  
> > > > 
> > > > Could this be done by the driver that claims the CXL RCiEP?  There's
> > > > no point in unmasking the errors before there's a driver with
> > > > pci_error_handlers that can do something with them anyway.  
> > > 
> > > This sounds reasonable at the first glance. The problem is there could
> > > be many devices associated with the RCEC. Not all of them will be
> > > bound to a driver and handler at the same time. We would need to
> > > refcount it or maintain a list of enabled devices. But there is
> > > already something similar by checking dev->driver. But right, AER
> > > errors could be seen and handled then at least on PCI level. I tent to
> > > permanently enable RCEC AER, but that could cause side-effects. What
> > > do you think?  
> > 
> > IIUC, this really just affects CXL devices, so I think the choice is
> > (1) always unmask internal errors for RCECs where those CXL devices
> > report errors (as this patch does), or (2) unmask when first CXL
> > driver that can handle the errors is loaded and restore previous state
> > when last one is unloaded.
> > 
> > If the RCEC *only* handles errors for CXL devices, i.e., not for a mix
> > of vanilla PCIe RCiEPs and CXL RCiEPs, I think I'm OK with (1).  I
> > think you said only the CXL driver knows how to collect and interpret
> > the error data.  Is it OK that when no such driver is loaded, we field
> > error interrupts silently, without even mentioning that an error
> > occurred?  I guess without the driver, the device is probably not in
> > use.
> 
> It might be in use.  Firmware may well have set up the CXL device and
> even have put the kernel image in that memory for example. OS first RAS
> handling won't be up until the driver loads though.  Would be a bit
> odd to mix OS first handling with firmware setup. I'd expect firmware
> first handling in that case, but I don't think anything stops the two
> being mixed.

Right, CXL memory may have been set up by firmware. We will only see
AER errors (for the unmasked error types) then without further CXL
handling, which is IMO OK.

This all assumes a non-CXL aware system can clear the error status by
only using PCIe AER. That is, a CXL RAS error may not trigger again
(or at all) by only clearing the AER status and not the CXL RAS status
in the capability. I don't know what the spec says here and how
devices actually operate.

Maybe option (2) is easy to implement with the refcount_t API. So with
the first device probed we just enable the RCEC's internal errors and
disable them when the last device is removed. I think CXL RAS errors
will not be triggered then as internal error must be enabled for this,
either in the RCEC or the endpoint. Since internal errors must be
unmasked first which can only be done by the CXL driver, CXL RAS error
wont trigger an AER error message.

Thanks,

-Robert
diff mbox series

Patch

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 2e3f00b6a5bd..c5076ae4eb58 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -948,6 +948,32 @@  static bool find_source_device(struct pci_dev *parent,
 
 #ifdef CONFIG_PCIEAER_CXL
 
+static int pci_aer_unmask_internal_errors(struct pci_dev *dev)
+{
+	int aer, rc;
+	u32 mask;
+
+	if (!pcie_aer_is_native(dev))
+		return -EIO;
+
+	aer = dev->aer_cap;
+	rc = pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, &mask);
+	if (rc)
+		return rc;
+	mask &= ~PCI_ERR_UNC_INTN;
+	rc = pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, mask);
+	if (rc)
+		return rc;
+
+	rc = pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK, &mask);
+	if (rc)
+		return rc;
+	mask &= ~PCI_ERR_COR_INTERNAL;
+	rc = pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, mask);
+
+	return rc;
+}
+
 static bool is_cxl_mem_dev(struct pci_dev *dev)
 {
 	/*
@@ -1031,7 +1057,44 @@  static void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info)
 		pcie_walk_rcec(dev, cxl_rch_handle_error_iter, info);
 }
 
+static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
+{
+	int *handles_cxl = data;
+
+	*handles_cxl = is_cxl_mem_dev(dev) && cxl_error_is_native(dev);
+
+	return *handles_cxl;
+}
+
+static bool handles_cxl_errors(struct pci_dev *rcec)
+{
+	int handles_cxl = 0;
+
+	if (pci_pcie_type(rcec) == PCI_EXP_TYPE_RC_EC &&
+	    pcie_aer_is_native(rcec))
+		pcie_walk_rcec(rcec, handles_cxl_error_iter, &handles_cxl);
+
+	return !!handles_cxl;
+}
+
+static void cxl_rch_enable_rcec(struct pci_dev *rcec)
+{
+	if (!handles_cxl_errors(rcec))
+		return;
+
+	/*
+	 * Internal errors are masked by default, unmask RCEC's here
+	 * PCI6.0 7.8.4.3 Uncorrectable Error Mask Register (Offset 08h)
+	 * PCI6.0 7.8.4.6 Correctable Error Mask Register (Offset 14h)
+	 */
+	if (pci_aer_unmask_internal_errors(rcec))
+		pci_err(rcec, "CXL: Failed to unmask internal errors");
+	else
+		pci_info(rcec, "CXL: Internal errors unmasked");
+}
+
 #else
+static inline void cxl_rch_enable_rcec(struct pci_dev *dev) { }
 static inline void cxl_rch_handle_error(struct pci_dev *dev,
 					struct aer_err_info *info) { }
 #endif
@@ -1432,6 +1495,7 @@  static int aer_probe(struct pcie_device *dev)
 		return status;
 	}
 
+	cxl_rch_enable_rcec(port);
 	aer_enable_rootport(rpc);
 	pci_info(port, "enabled with IRQ %d\n", dev->irq);
 	return 0;