Re: [PATCH v19 08/14] cxl/ras: Handle RCH correctable and uncorrectable errors in one pass

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect inline severity calculation for uncorrectable errors fails to apply the uncorrectable error mask, potentially escalating unmasked non-fatal errors to AER_FATAL.
--

--- Patch [8]: [PATCH v19 08/14] cxl/ras: Handle RCH correctable and uncorrectable errors in one pass ---
commit 57f34ecdf069076e73ab0e60927403af518bdfc9
Author: Terry Bowman <terry.bowman@amd.com>

cxl/ras: Handle RCH correctable and uncorrectable errors in one pass

This commit modifies the handling of RCH correctable and uncorrectable errors
to process both in a single pass so co-logged events are not lost. It removes
the cxl_rch_get_aer_severity helper and decodes the uncorrectable severity
inline.

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

[ ... ]

> @@ -127,12 +106,21 @@ void cxl_handle_rdport_errors(struct pci_dev *pdev)
>  	if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
>  		return;
>  
> -	if (!cxl_rch_get_aer_severity(&aer_regs, &severity))
> -		return;
> +	/*
> +	 * Handle correctable and uncorrectable errors independently; both
> +	 * may be set in the same pass and cxl_rch_get_aer_info() has already
> +	 * cleared both status registers.
> +	 */
> +	if (aer_regs.cor_status & ~aer_regs.cor_mask) {
> +		pci_print_aer(pdev, AER_CORRECTABLE, &aer_regs);
> +		cxl_handle_cor_ras(dport->dport_dev, to_ras_base(port, dport));
> +	}
> +
> +	if (aer_regs.uncor_status & ~aer_regs.uncor_mask) {
> +		int severity = (aer_regs.uncor_status & aer_regs.uncor_severity) ?
> +			       AER_FATAL : AER_NONFATAL;

[Severity: High]
Does this inline calculation miss the uncorrectable error mask? 

In the previous implementation, the severity check masked uncor_status:

    u32 uncor_status = aer_regs->uncor_status & ~aer_regs->uncor_mask;
    if (uncor_status) {
        *severity = (uncor_status & aer_regs->uncor_severity) ?
                     AER_FATAL : AER_NONFATAL;

Without applying ~aer_regs.uncor_mask here in the new inline code, if hardware
simultaneously logs a masked fatal error and an unmasked non-fatal error, the
masked fatal bit will be evaluated. Could this wrongly escalate the severity
to AER_FATAL?

>  
> -	pci_print_aer(pdev, severity, &aer_regs);
> -	if (severity == AER_CORRECTABLE)
> -		cxl_handle_cor_ras(&pdev->dev, to_ras_base(port, dport));
> -	else
> +		pci_print_aer(pdev, severity, &aer_regs);
>  		cxl_do_recovery(pdev, dport->port, dport);
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803221810.3685703-1-terry.bowman@amd.com?part=8