| 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
| ||
|
Message-ID: <20251104180318.00005d9a@huawei.com>
Date: Tue, 4 Nov 2025 18:03:18 +0000
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Terry Bowman <terry.bowman@....com>
CC: <dave@...olabs.net>, <dave.jiang@...el.com>, <alison.schofield@...el.com>,
<dan.j.williams@...el.com>, <bhelgaas@...gle.com>, <shiju.jose@...wei.com>,
<ming.li@...omail.com>, <Smita.KoralahalliChannabasappa@....com>,
<rrichter@....com>, <dan.carpenter@...aro.org>,
<PradeepVineshReddy.Kodamati@....com>, <lukas@...ner.de>,
<Benjamin.Cheatham@....com>, <sathyanarayanan.kuppuswamy@...ux.intel.com>,
<linux-cxl@...r.kernel.org>, <alucerop@....com>, <ira.weiny@...el.com>,
<linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>
Subject: Re: [RESEND v13 06/25] cxl: Move CXL driver's RCH error handling
into core/ras_rch.c
On Tue, 4 Nov 2025 11:02:46 -0600
Terry Bowman <terry.bowman@....com> wrote:
> Restricted CXL Host (RCH) protocol error handling uses a procedure distinct
> from the CXL Virtual Hierarchy (VH) handling. This is because of the
> differences in the RCH and VH topologies. Improve the maintainability and
> add ability to enable/disable RCH handling.
>
> Move and combine the RCH handling code into a single block conditionally
> compiled with the CONFIG_CXL_RCH_RAS kernel config.
>
> Signed-off-by: Terry Bowman <terry.bowman@....com>
>
> ---
>
Fairly sure this patch had changes seeing as code now in a different file.
A few minor comments inline. With those tidied up
Reviewed-by: Jonathan Cameron <jonathan.cameron@...wei.com>
J
> Changes in v12->v13:
> - None
>
> Changes v11->v12:
> - Moved CXL_RCH_RAS Kconfig definition here from following commit.
>
> Changes v10->v11:
> - New patch
> ---
> drivers/cxl/Kconfig | 7 +++
> drivers/cxl/core/Makefile | 1 +
> drivers/cxl/core/core.h | 5 +-
> drivers/cxl/core/pci.c | 115 -----------------------------------
> drivers/cxl/core/ras_rch.c | 120 +++++++++++++++++++++++++++++++++++++
> tools/testing/cxl/Kbuild | 1 +
> 6 files changed, 132 insertions(+), 117 deletions(-)
> create mode 100644 drivers/cxl/core/ras_rch.c
>
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 217888992c88..ffe6ad981434 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -237,4 +237,11 @@ config CXL_RAS
> def_bool y
> depends on ACPI_APEI_GHES && PCIEAER && CXL_PCI
>
> +config CXL_RCH_RAS
> + bool "CXL: Restricted CXL Host (RCH) protocol error handling"
> + def_bool n
Don't need to specify a default of no as that is always the default if
not overridden.
> + depends on CXL_RAS
> + help
> + RAS support for Restricted CXL Host (RCH) defined in CXL1.1.
> +
> endif
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index b2930cc54f8b..fa1d4aed28b9 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -20,3 +20,4 @@ cxl_core-$(CONFIG_CXL_MCE) += mce.o
> cxl_core-$(CONFIG_CXL_FEATURES) += features.o
> cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += edac.o
> cxl_core-$(CONFIG_CXL_RAS) += ras.o
> +cxl_core-$(CONFIG_CXL_RCH_RAS) += ras_rch.o
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index bc818de87ccc..c30ab7c25a92 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -4,6 +4,7 @@
> #ifndef __CXL_CORE_H__
> #define __CXL_CORE_H__
>
> +#include <linux/pci.h>
Why this include. I'm not spotting anything pci specific being added to this header.
If it should already have been here, belongs in a separate patch.
> #include <cxl/mailbox.h>
> #include <linux/rwsem.h>
>
> @@ -167,7 +168,7 @@ static inline void cxl_handle_cor_ras(struct cxl_dev_state *cxlds, void __iomem
> #endif /* CONFIG_CXL_RAS */
>
> /* Restricted CXL Host specific RAS functions */
> -#ifdef CONFIG_CXL_RAS
> +#ifdef CONFIG_CXL_RCH_RAS
> void cxl_dport_map_rch_aer(struct cxl_dport *dport);
> void cxl_disable_rch_root_ints(struct cxl_dport *dport);
> void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds);
> @@ -175,7 +176,7 @@ void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds);
> static inline void cxl_dport_map_rch_aer(struct cxl_dport *dport) { }
> static inline void cxl_disable_rch_root_ints(struct cxl_dport *dport) { }
> static inline void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds) { }
> -#endif /* CONFIG_CXL_RAS */
> +#endif /* CONFIG_CXL_RCH_RAS */
>
> int cxl_gpf_port_setup(struct cxl_dport *dport);
> diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
> new file mode 100644
> index 000000000000..f6de5492a8b7
> --- /dev/null
> +++ b/drivers/cxl/core/ras_rch.c
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2025 AMD Corporation. All rights reserved. */
> +
> +#include <linux/pci.h>
> +#include <linux/aer.h>
> +#include <cxl/event.h>
> +#include <cxlmem.h>
> +#include "trace.h"
We should be trying to follow include what you use principles.
So linux/types.h for resource_size_t
Probably a forwards def for struct device as well.
> +
> +void cxl_dport_map_rch_aer(struct cxl_dport *dport)
> +{
> + resource_size_t aer_phys;
> + struct device *host;
> + u16 aer_cap;
> +
> + aer_cap = cxl_rcrb_to_aer(dport->dport_dev, dport->rcrb.base);
> + if (aer_cap) {
> + host = dport->reg_map.host;
> + aer_phys = aer_cap + dport->rcrb.base;
> + dport->regs.dport_aer = devm_cxl_iomap_block(host, aer_phys,
> + sizeof(struct aer_capability_regs));
Maybe keep original alignment or even something like
dport->regs.dport_aer =
devm_cxl_iomap_block(host, aer_phys,
sizeof(struct aer_capability_regs));
> + }
> +}
> +
Powered by blists - more mailing lists