linux-kernel - RE: [PATCH v5 21/26] cxl/pci: Update CXL error logging to use RAS register address

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <64878a219f3d6_142af8294c1@dwillia2-xfh.jf.intel.com.notmuch>
Date:   Mon, 12 Jun 2023 14:12:01 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Terry Bowman <terry.bowman@....com>, <alison.schofield@...el.com>,
        <vishal.l.verma@...el.com>, <ira.weiny@...el.com>,
        <bwidawsk@...nel.org>, <dan.j.williams@...el.com>,
        <dave.jiang@...el.com>, <Jonathan.Cameron@...wei.com>,
        <linux-cxl@...r.kernel.org>
CC:     <terry.bowman@....com>, <rrichter@....com>,
        <linux-kernel@...r.kernel.org>, <bhelgaas@...gle.com>
Subject: RE: [PATCH v5 21/26] cxl/pci: Update CXL error logging to use RAS
 register address

Terry Bowman wrote:
> The CXL error handler currently only logs endpoint RAS status. The CXL
> topology includes several components providing RAS details to be logged
> during error handling.[1] Update the current handler's RAS logging to use a
> RAS register address. This will allow for adding support to log other CXL
> component's RAS details in the future.
> 
> [1] CXL3.0 Table 8-22 CXL_Capability_ID Assignment
> 
> Co-developed-by: Robert Richter <rrichter@....com>
> Signed-off-by: Robert Richter <rrichter@....com>
> Signed-off-by: Terry Bowman <terry.bowman@....com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> ---
>  drivers/cxl/core/pci.c | 42 ++++++++++++++++++++++++++++++------------
>  1 file changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 67f4ab6daa34..def6ee5ab4f5 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -665,32 +665,36 @@ void read_cdat_data(struct cxl_port *port)
>  }
>  EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
>  
> -void cxl_cor_error_detected(struct pci_dev *pdev)
> +static void __cxl_log_correctable_ras(struct cxl_dev_state *cxlds,
> +				      void __iomem *ras_base)
>  {
> -	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
>  	void __iomem *addr;
>  	u32 status;
>  
> -	if (!cxlds->regs.ras)
> +	if (!ras_base)
>  		return;
>  
> -	addr = cxlds->regs.ras + CXL_RAS_CORRECTABLE_STATUS_OFFSET;
> +	addr = ras_base + CXL_RAS_CORRECTABLE_STATUS_OFFSET;
>  	status = readl(addr);
>  	if (status & CXL_RAS_CORRECTABLE_STATUS_MASK) {
>  		writel(status & CXL_RAS_CORRECTABLE_STATUS_MASK, addr);
>  		trace_cxl_aer_correctable_error(cxlds->cxlmd, status);
>  	}
>  }
> -EXPORT_SYMBOL_NS_GPL(cxl_cor_error_detected, CXL);
> +
> +static void cxl_log_correctable_ras_endpoint(struct cxl_dev_state *cxlds)
> +{
> +	return __cxl_log_correctable_ras(cxlds, cxlds->regs.ras);
> +}

As I review patch 24 it leads me back here to grumble about some naming
choices. We now have:

cxl_cor_error_detected()
__cxl_log_correctable_ras()
cxl_report_and_clear()

Which of those clear the status? What is the difference between "report"
and a "log"? I don't much care what name gets chosen but these three
functions at least need to give the impression they were written with a
common vision. The term "handle" would not surprise me in the names of
functions that emit messages and clear register status bits.