* [PATCH v4 0/3] CXL, ACPI, APEI, EINJ: Update EINJ for CXL 1.1 error types
@ 2023-09-07 19:19 Ben Cheatham
2023-09-07 19:19 ` [PATCH v4 1/3] CXL, PCIE: Add cxl_rcrb_addr file to dport_dev Ben Cheatham
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Ben Cheatham @ 2023-09-07 19:19 UTC (permalink / raw)
To: rafael, dan.j.williams, linux-cxl, linux-acpi
Cc: bhelgaas, benjamin.cheatham, yazen.ghannam
v4 Changes:
- Fix kernel test robot build errors
- Add imply rules for CONFIG_CXL_ACPI and CONFIG_CXL_BUS in
EINJ Kconfig to build the CXL support correctly by default
- Change description of building CXL error type support
in EINJ documentation for brevity
v3 Changes:
- Add sysfs files for finding valid CXL 1.1 downstream port
MMIO addresses, along with validation of said addresses
- Update EINJ documentation to include relevant information
for injecting CXL error types
- Dropped Yazen's from tag
This patch is a follow up to the discussion at [1], and builds on Tony's
CXL error patch at [2].
The new CXL error types will use the Memory Address field in the
SET_ERROR_TYPE_WITH_ADDRESS structure in order to target a CXL 1.1
compliant memory-mapped downstream port. The value of the memory address
will be in the port's MMIO range, and it will not represent physical
(normal or persistent) memory.
In v2 [3], the user supplied the MMIO address for the downstream port, but
per Dan Williams' suggestion [3], the addresses are predetermined and
the user only picks the error type to inject and the downstream port to
inject into. In order to inject an error, the user write the error type
to the error_type file under the einj debugfs directory, then writes any
integer into one of the files under the cxl directory.
[1]:
Link: https://lore.kernel.org/linux-acpi/20221206205234.606073-1-Benjamin.Cheatham@amd.com/
[2]:
Link: https://lore.kernel.org/linux-cxl/CAJZ5v0hNQUfWViqxbJ5B4JCGJUuHpWWSpqpCFWPNpGuagoFbsQ@mail.gmail.com/T/#t
[3]:
Link: https://lore.kernel.org/linux-cxl/20230403151849.43408-1-Benjamin.Cheatham@amd.com/
Ben Cheatham (3):
CXL, PCIE: Add cxl_rcrb_addr file to dport_dev
ACPI, APEI, EINJ: Add CXL 1.1 EINJ error type support
ACPI, APEI, EINJ: Update EINJ documentation
Documentation/ABI/testing/sysfs-bus-cxl | 8 +++
.../firmware-guide/acpi/apei/einj.rst | 25 +++++++--
drivers/acpi/apei/Kconfig | 2 +
drivers/acpi/apei/einj.c | 24 ++++++++-
drivers/cxl/acpi.c | 2 +
drivers/cxl/core/port.c | 52 +++++++++++++++++++
drivers/cxl/cxl.h | 3 ++
include/linux/cxl.h | 18 +++++++
8 files changed, 129 insertions(+), 5 deletions(-)
create mode 100644 include/linux/cxl.h
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/3] CXL, PCIE: Add cxl_rcrb_addr file to dport_dev
2023-09-07 19:19 [PATCH v4 0/3] CXL, ACPI, APEI, EINJ: Update EINJ for CXL 1.1 error types Ben Cheatham
@ 2023-09-07 19:19 ` Ben Cheatham
2023-09-12 14:11 ` Jonathan Cameron
2023-09-07 19:19 ` [PATCH v4 2/3] ACPI, APEI, EINJ: Add CXL 1.1 EINJ error type support Ben Cheatham
2023-09-07 19:19 ` [PATCH v4 3/3] ACPI, APEI, EINJ: Update EINJ documentation Ben Cheatham
2 siblings, 1 reply; 10+ messages in thread
From: Ben Cheatham @ 2023-09-07 19:19 UTC (permalink / raw)
To: rafael, dan.j.williams, linux-cxl, linux-acpi
Cc: bhelgaas, benjamin.cheatham, yazen.ghannam
Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie
device) for CXL RCH root ports. The file will print the RCRB base
MMIO address of the root port when read and will be used by
users looking to inject CXL EINJ error types for RCH hosts.
Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
---
Documentation/ABI/testing/sysfs-bus-cxl | 8 ++++++
drivers/cxl/acpi.c | 2 ++
drivers/cxl/core/port.c | 33 +++++++++++++++++++++++++
drivers/cxl/cxl.h | 2 ++
4 files changed, 45 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 087f762ebfd5..a7d169235543 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -177,6 +177,14 @@ Description:
integer reflects the hardware port unique-id used in the
hardware decoder target list.
+What: /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr
+Date: August, 2023
+KernelVersion: v6.6
+Contact: linux-cxl@vger.kernel.org
+Description:
+ (RO) The 'cxl_rcrb_addr' device file gives the MMIO base address
+ of the RCRB of the corresponding CXL 1.1 downstream port. Only
+ present for CXL 1.1 dports.
What: /sys/bus/cxl/devices/decoderX.Y
Date: June, 2021
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index d1c559879dcc..3e2ca946bf47 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -676,6 +676,8 @@ static int cxl_acpi_probe(struct platform_device *pdev)
if (IS_ERR(root_port))
return PTR_ERR(root_port);
+ set_cxl_root(root_port);
+
rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
add_host_bridge_dport);
if (rc < 0)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 724be8448eb4..001ab8742e21 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -875,6 +875,14 @@ struct cxl_port *find_cxl_root(struct cxl_port *port)
}
EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL);
+static struct cxl_port *cxl_root;
+
+void set_cxl_root(struct cxl_port *root_port)
+{
+ cxl_root = root_port;
+}
+EXPORT_SYMBOL_NS_GPL(set_cxl_root, CXL);
+
static struct cxl_dport *find_dport(struct cxl_port *port, int id)
{
struct cxl_dport *dport;
@@ -930,11 +938,30 @@ static void cond_cxl_root_unlock(struct cxl_port *port)
device_unlock(&port->dev);
}
+static ssize_t cxl_rcrb_addr_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct cxl_dport *dport;
+
+ if (!cxl_root)
+ return -ENODEV;
+
+ dport = cxl_find_dport_by_dev(cxl_root, dev);
+ if (!dport)
+ return -ENODEV;
+
+ return sysfs_emit(buf, "0x%llx\n", (u64) dport->rcrb.base);
+}
+DEVICE_ATTR_RO(cxl_rcrb_addr);
+
static void cxl_dport_remove(void *data)
{
struct cxl_dport *dport = data;
struct cxl_port *port = dport->port;
+ if (dport->rch)
+ device_remove_file(dport->dport_dev, &dev_attr_cxl_rcrb_addr);
+
xa_erase(&port->dports, (unsigned long) dport->dport_dev);
put_device(dport->dport_dev);
}
@@ -1021,6 +1048,12 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
if (rc)
return ERR_PTR(rc);
+ if (dport->rch && dport->rcrb.base != CXL_RESOURCE_NONE) {
+ rc = device_create_file(dport_dev, &dev_attr_cxl_rcrb_addr);
+ if (rc)
+ return ERR_PTR(rc);
+ }
+
return dport;
}
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 76d92561af29..4d5bce4bae7e 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -690,6 +690,8 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
resource_size_t component_reg_phys,
struct cxl_dport *parent_dport);
struct cxl_port *find_cxl_root(struct cxl_port *port);
+void set_cxl_root(struct cxl_port *root_port);
+
int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
void cxl_bus_rescan(void);
void cxl_bus_drain(void);
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/3] ACPI, APEI, EINJ: Add CXL 1.1 EINJ error type support
2023-09-07 19:19 [PATCH v4 0/3] CXL, ACPI, APEI, EINJ: Update EINJ for CXL 1.1 error types Ben Cheatham
2023-09-07 19:19 ` [PATCH v4 1/3] CXL, PCIE: Add cxl_rcrb_addr file to dport_dev Ben Cheatham
@ 2023-09-07 19:19 ` Ben Cheatham
2023-09-12 14:00 ` Jonathan Cameron
2023-09-07 19:19 ` [PATCH v4 3/3] ACPI, APEI, EINJ: Update EINJ documentation Ben Cheatham
2 siblings, 1 reply; 10+ messages in thread
From: Ben Cheatham @ 2023-09-07 19:19 UTC (permalink / raw)
To: rafael, dan.j.williams, linux-cxl, linux-acpi
Cc: bhelgaas, benjamin.cheatham, yazen.ghannam
Add support for CXL EINJ error types for CXL 1.1 hosts added in ACPI
v6.5. Because these error types target memory-mapped CXL 1.1 compliant
downstream ports and not physical (normal/persistent) memory, these
error types are not currently allowed through the memory range
validation done by the EINJ driver.
The MMIO address of a CXL 1.1 downstream port can be found in the
cxl_rcrb_addr file in the corresponding dport directory under
/sys/bus/cxl/devices/portX. CXL 1.1 error types follow the same
procedure as a memory error type, but with param1 set to the
downstream port MMIO address.
Example usage:
$ cd /sys/kernel/debug/apei/einj
$ cat available_error_type
0x00000008 Memory Correctable
0x00000010 Memory Uncorrectable non-fatal
0x00000020 Memory Uncorrectable fatal
0x00000040 PCI Express Correctable
0x00000080 PCI Express Uncorrectable non-fatal
0x00000100 PCI Express Uncorrectable fatal
0x00008000 CXL.mem Protocol Correctable
0x00020000 CXL.mem Protocol Uncorrectable fatal
$ echo 0x8000 > error_type
$ echo 0xfffffffffffff000 > param2
$ echo 0x3 > flags
$ cat /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr
0xb2f00000
$ echo 0xb2f00000 > param1
$ echo 1 > error_inject
Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
---
drivers/acpi/apei/Kconfig | 2 ++
drivers/acpi/apei/einj.c | 24 +++++++++++++++++++++++-
drivers/cxl/core/port.c | 19 +++++++++++++++++++
drivers/cxl/cxl.h | 1 +
include/linux/cxl.h | 18 ++++++++++++++++++
5 files changed, 63 insertions(+), 1 deletion(-)
create mode 100644 include/linux/cxl.h
diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
index 6b18f8bc7be3..eb9cc7157342 100644
--- a/drivers/acpi/apei/Kconfig
+++ b/drivers/acpi/apei/Kconfig
@@ -55,6 +55,8 @@ config ACPI_APEI_MEMORY_FAILURE
config ACPI_APEI_EINJ
tristate "APEI Error INJection (EINJ)"
depends on ACPI_APEI && DEBUG_FS
+ imply CXL_BUS
+ imply CXL_ACPI
help
EINJ provides a hardware error injection mechanism, it is
mainly used for debugging and testing the other parts of
diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index 013eb621dc92..8000417a5f26 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -21,6 +21,7 @@
#include <linux/nmi.h>
#include <linux/delay.h>
#include <linux/mm.h>
+#include <linux/cxl.h>
#include <asm/unaligned.h>
#include "apei-internal.h"
@@ -36,6 +37,7 @@
#define MEM_ERROR_MASK (ACPI_EINJ_MEMORY_CORRECTABLE | \
ACPI_EINJ_MEMORY_UNCORRECTABLE | \
ACPI_EINJ_MEMORY_FATAL)
+#define CXL_ERROR_MASK GENMASK(17, 12)
/*
* ACPI version 5 provides a SET_ERROR_TYPE_WITH_ADDRESS action.
@@ -512,6 +514,22 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
return rc;
}
+static int is_valid_cxl_addr(u64 addr)
+{
+ struct cxl_dport *dport;
+
+ if (IS_REACHABLE(CONFIG_CXL_ACPI))
+ dport = cxl_find_rch_dport_by_rcrb((resource_size_t) addr);
+ else
+ return 0;
+
+ if (!IS_ERR_OR_NULL(dport))
+ return 1;
+
+ pr_info("Could not find dport with rcrb 0x%llx\n", addr);
+ return 0;
+}
+
/* Inject the specified hardware error */
static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
u64 param3, u64 param4)
@@ -537,8 +555,11 @@ static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
if (type & ACPI5_VENDOR_BIT) {
if (vendor_flags != SETWA_FLAGS_MEM)
goto inject;
- } else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM))
+ } else if (!(type & MEM_ERROR_MASK) && !(flags & SETWA_FLAGS_MEM)) {
goto inject;
+ } else if (type & CXL_ERROR_MASK && is_valid_cxl_addr(param1)) {
+ goto inject;
+ }
/*
* Disallow crazy address masks that give BIOS leeway to pick
@@ -807,3 +828,4 @@ module_exit(einj_exit);
MODULE_AUTHOR("Huang Ying");
MODULE_DESCRIPTION("APEI Error INJection support");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(CXL);
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 001ab8742e21..f8f300496140 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1122,6 +1122,25 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
}
EXPORT_SYMBOL_NS_GPL(devm_cxl_add_rch_dport, CXL);
+#if IS_ENABLED(CONFIG_CXL_ACPI)
+struct cxl_dport *cxl_find_rch_dport_by_rcrb(resource_size_t rcrb_base)
+{
+ struct cxl_dport *dport;
+ unsigned long index;
+
+ if (!cxl_root)
+ return ERR_PTR(-ENODEV);
+
+ xa_for_each(&cxl_root->dports, index, dport)
+ if ((dport->rch && dport->rcrb.base != CXL_RESOURCE_NONE)
+ && dport->rcrb.base == rcrb_base)
+ return dport;
+
+ return NULL;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_find_rch_dport_by_rcrb, CXL);
+#endif
+
static int add_ep(struct cxl_ep *new)
{
struct cxl_port *port = new->dport->port;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 4d5bce4bae7e..3e6779dbcd23 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -8,6 +8,7 @@
#include <linux/bitfield.h>
#include <linux/bitops.h>
#include <linux/log2.h>
+#include <linux/cxl.h>
#include <linux/io.h>
/**
diff --git a/include/linux/cxl.h b/include/linux/cxl.h
new file mode 100644
index 000000000000..b57a4cc85005
--- /dev/null
+++ b/include/linux/cxl.h
@@ -0,0 +1,18 @@
+#ifndef _LINUX_CXL_H
+#define _LINUX_CXL_H
+
+#include <linux/xarray.h>
+#include <linux/errno.h>
+
+struct cxl_dport;
+
+#if IS_ENABLED(CONFIG_CXL_ACPI)
+struct cxl_dport *cxl_find_rch_dport_by_rcrb(resource_size_t rcrb_base);
+#else
+static inline struct cxl_dport *cxl_find_rch_dport_by_rcrb(resource_size_t rcrb_base)
+{
+ return NULL;
+}
+#endif
+
+#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 3/3] ACPI, APEI, EINJ: Update EINJ documentation
2023-09-07 19:19 [PATCH v4 0/3] CXL, ACPI, APEI, EINJ: Update EINJ for CXL 1.1 error types Ben Cheatham
2023-09-07 19:19 ` [PATCH v4 1/3] CXL, PCIE: Add cxl_rcrb_addr file to dport_dev Ben Cheatham
2023-09-07 19:19 ` [PATCH v4 2/3] ACPI, APEI, EINJ: Add CXL 1.1 EINJ error type support Ben Cheatham
@ 2023-09-07 19:19 ` Ben Cheatham
2023-09-12 13:51 ` Jonathan Cameron
2 siblings, 1 reply; 10+ messages in thread
From: Ben Cheatham @ 2023-09-07 19:19 UTC (permalink / raw)
To: rafael, dan.j.williams, linux-cxl, linux-acpi
Cc: bhelgaas, benjamin.cheatham, yazen.ghannam
Update EINJ documentation to include CXL errors in available_error_types
table and usage of the types.
Also fix a formatting error in the param4 file description that caused
the description to be on the same line as the bullet point.
Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
---
.../firmware-guide/acpi/apei/einj.rst | 25 ++++++++++++++++---
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/Documentation/firmware-guide/acpi/apei/einj.rst b/Documentation/firmware-guide/acpi/apei/einj.rst
index d6b61d22f525..c6f28118c48b 100644
--- a/Documentation/firmware-guide/acpi/apei/einj.rst
+++ b/Documentation/firmware-guide/acpi/apei/einj.rst
@@ -32,6 +32,9 @@ configuration::
CONFIG_ACPI_APEI
CONFIG_ACPI_APEI_EINJ
+To use CXL error types ``CONFIG_CXL_ACPI`` needs to be set to the same
+value as ``CONFIG_ACPI_APEI_EINJ`` (either "y" or "m").
+
The EINJ user interface is in <debugfs mount point>/apei/einj.
The following files belong to it:
@@ -40,9 +43,9 @@ The following files belong to it:
This file shows which error types are supported:
- ================ ===================================
+ ================ =========================================
Error Type Value Error Description
- ================ ===================================
+ ================ =========================================
0x00000001 Processor Correctable
0x00000002 Processor Uncorrectable non-fatal
0x00000004 Processor Uncorrectable fatal
@@ -55,7 +58,13 @@ The following files belong to it:
0x00000200 Platform Correctable
0x00000400 Platform Uncorrectable non-fatal
0x00000800 Platform Uncorrectable fatal
- ================ ===================================
+ 0x00001000 CXL.cache Protocol Correctable
+ 0x00002000 CXL.cache Protocol Uncorrectable non-fatal
+ 0x00004000 CXL.cache Protocol Uncorrectable fatal
+ 0x00008000 CXL.mem Protocol Correctable
+ 0x00010000 CXL.mem Protocol Uncorrectable non-fatal
+ 0x00020000 CXL.mem Protocol Uncorrectable fatal
+ ================ =========================================
The format of the file contents are as above, except present are only
the available error types.
@@ -106,6 +115,7 @@ The following files belong to it:
Used when the 0x1 bit is set in "flags" to specify the APIC id
- param4
+
Used when the 0x4 bit is set in "flags" to specify target PCIe device
- notrigger
@@ -159,6 +169,13 @@ and param2 (1 = PROCESSOR, 2 = MEMORY, 4 = PCI). See your BIOS vendor
documentation for details (and expect changes to this API if vendors
creativity in using this feature expands beyond our expectations).
+CXL error types are supported from ACPI 6.5 onwards. To use these error
+types you need the MMIO address of a CXL 1.1 downstream port. You can
+find the address of dportY in /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr
+(it's possible that the dport is under the CXL root, in that case the
+path would be /sys/us/cxl/devices/rootX/dportY/cxl_rcrb_addr).
+From there, write the address to param1 and continue as you would for a
+memory error type.
An error injection example::
@@ -201,4 +218,4 @@ The following sequence can be used:
7) Read from the virtual address. This will trigger the error
For more information about EINJ, please refer to ACPI specification
-version 4.0, section 17.5 and ACPI 5.0, section 18.6.
+version 4.0, section 17.5 and ACPI 6.5, section 18.6.
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] ACPI, APEI, EINJ: Update EINJ documentation
2023-09-07 19:19 ` [PATCH v4 3/3] ACPI, APEI, EINJ: Update EINJ documentation Ben Cheatham
@ 2023-09-12 13:51 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2023-09-12 13:51 UTC (permalink / raw)
To: Ben Cheatham
Cc: rafael, dan.j.williams, linux-cxl, linux-acpi, bhelgaas,
yazen.ghannam
On Thu, 7 Sep 2023 14:19:56 -0500
Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:
> Update EINJ documentation to include CXL errors in available_error_types
> table and usage of the types.
>
> Also fix a formatting error in the param4 file description that caused
> the description to be on the same line as the bullet point.
>
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
Matches what the spec says, so
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> .../firmware-guide/acpi/apei/einj.rst | 25 ++++++++++++++++---
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/firmware-guide/acpi/apei/einj.rst b/Documentation/firmware-guide/acpi/apei/einj.rst
> index d6b61d22f525..c6f28118c48b 100644
> --- a/Documentation/firmware-guide/acpi/apei/einj.rst
> +++ b/Documentation/firmware-guide/acpi/apei/einj.rst
> @@ -32,6 +32,9 @@ configuration::
> CONFIG_ACPI_APEI
> CONFIG_ACPI_APEI_EINJ
>
> +To use CXL error types ``CONFIG_CXL_ACPI`` needs to be set to the same
> +value as ``CONFIG_ACPI_APEI_EINJ`` (either "y" or "m").
> +
> The EINJ user interface is in <debugfs mount point>/apei/einj.
>
> The following files belong to it:
> @@ -40,9 +43,9 @@ The following files belong to it:
>
> This file shows which error types are supported:
>
> - ================ ===================================
> + ================ =========================================
> Error Type Value Error Description
> - ================ ===================================
> + ================ =========================================
> 0x00000001 Processor Correctable
> 0x00000002 Processor Uncorrectable non-fatal
> 0x00000004 Processor Uncorrectable fatal
> @@ -55,7 +58,13 @@ The following files belong to it:
> 0x00000200 Platform Correctable
> 0x00000400 Platform Uncorrectable non-fatal
> 0x00000800 Platform Uncorrectable fatal
> - ================ ===================================
> + 0x00001000 CXL.cache Protocol Correctable
> + 0x00002000 CXL.cache Protocol Uncorrectable non-fatal
> + 0x00004000 CXL.cache Protocol Uncorrectable fatal
> + 0x00008000 CXL.mem Protocol Correctable
> + 0x00010000 CXL.mem Protocol Uncorrectable non-fatal
> + 0x00020000 CXL.mem Protocol Uncorrectable fatal
> + ================ =========================================
>
> The format of the file contents are as above, except present are only
> the available error types.
> @@ -106,6 +115,7 @@ The following files belong to it:
> Used when the 0x1 bit is set in "flags" to specify the APIC id
>
> - param4
> +
> Used when the 0x4 bit is set in "flags" to specify target PCIe device
>
> - notrigger
> @@ -159,6 +169,13 @@ and param2 (1 = PROCESSOR, 2 = MEMORY, 4 = PCI). See your BIOS vendor
> documentation for details (and expect changes to this API if vendors
> creativity in using this feature expands beyond our expectations).
>
> +CXL error types are supported from ACPI 6.5 onwards. To use these error
> +types you need the MMIO address of a CXL 1.1 downstream port. You can
> +find the address of dportY in /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr
> +(it's possible that the dport is under the CXL root, in that case the
> +path would be /sys/us/cxl/devices/rootX/dportY/cxl_rcrb_addr).
> +From there, write the address to param1 and continue as you would for a
> +memory error type.
>
> An error injection example::
>
> @@ -201,4 +218,4 @@ The following sequence can be used:
> 7) Read from the virtual address. This will trigger the error
>
> For more information about EINJ, please refer to ACPI specification
> -version 4.0, section 17.5 and ACPI 5.0, section 18.6.
> +version 4.0, section 17.5 and ACPI 6.5, section 18.6.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/3] ACPI, APEI, EINJ: Add CXL 1.1 EINJ error type support
2023-09-07 19:19 ` [PATCH v4 2/3] ACPI, APEI, EINJ: Add CXL 1.1 EINJ error type support Ben Cheatham
@ 2023-09-12 14:00 ` Jonathan Cameron
2023-09-12 14:49 ` Ben Cheatham
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2023-09-12 14:00 UTC (permalink / raw)
To: Ben Cheatham
Cc: rafael, dan.j.williams, linux-cxl, linux-acpi, bhelgaas,
yazen.ghannam
On Thu, 7 Sep 2023 14:19:55 -0500
Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:
> Add support for CXL EINJ error types for CXL 1.1 hosts added in ACPI
> v6.5. Because these error types target memory-mapped CXL 1.1 compliant
> downstream ports and not physical (normal/persistent) memory, these
> error types are not currently allowed through the memory range
> validation done by the EINJ driver.
>
> The MMIO address of a CXL 1.1 downstream port can be found in the
> cxl_rcrb_addr file in the corresponding dport directory under
> /sys/bus/cxl/devices/portX. CXL 1.1 error types follow the same
> procedure as a memory error type, but with param1 set to the
> downstream port MMIO address.
>
> Example usage:
> $ cd /sys/kernel/debug/apei/einj
> $ cat available_error_type
> 0x00000008 Memory Correctable
> 0x00000010 Memory Uncorrectable non-fatal
> 0x00000020 Memory Uncorrectable fatal
> 0x00000040 PCI Express Correctable
> 0x00000080 PCI Express Uncorrectable non-fatal
> 0x00000100 PCI Express Uncorrectable fatal
> 0x00008000 CXL.mem Protocol Correctable
> 0x00020000 CXL.mem Protocol Uncorrectable fatal
> $ echo 0x8000 > error_type
> $ echo 0xfffffffffffff000 > param2
> $ echo 0x3 > flags
From einj.rst (and the ACPI spec) bit 0 here is
Processor APIC field valid. Why is that relevant here?
If it were you'd be writing param3 I think. So probably
harmless, but I think this should be 0x2 > flags
> $ cat /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr
> 0xb2f00000
> $ echo 0xb2f00000 > param1
> $ echo 1 > error_inject
>
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
Hi Ben,
Why bother with the config dependent build?
It doesn't save that much in code built and right now there
are no non ACPI CXL systems so in reality it's always built
anyway.
Otherwise this LGTM
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 001ab8742e21..f8f300496140 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1122,6 +1122,25 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
> }
> EXPORT_SYMBOL_NS_GPL(devm_cxl_add_rch_dport, CXL);
>
> +#if IS_ENABLED(CONFIG_CXL_ACPI)
I'm not a particular fan processor magic down in c files.
Do we really care about the saving of not having this in builds where
CONFIG_CXL_ACPI isn't set? I'm thinking those don't really exist.
> +struct cxl_dport *cxl_find_rch_dport_by_rcrb(resource_size_t rcrb_base)
> +{
> + struct cxl_dport *dport;
> + unsigned long index;
> +
> + if (!cxl_root)
> + return ERR_PTR(-ENODEV);
> +
> + xa_for_each(&cxl_root->dports, index, dport)
> + if ((dport->rch && dport->rcrb.base != CXL_RESOURCE_NONE)
> + && dport->rcrb.base == rcrb_base)
> + return dport;
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_find_rch_dport_by_rcrb, CXL);
> +#endif
> +
> static int add_ep(struct cxl_ep *new)
> {
> struct cxl_port *port = new->dport->port;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] CXL, PCIE: Add cxl_rcrb_addr file to dport_dev
2023-09-07 19:19 ` [PATCH v4 1/3] CXL, PCIE: Add cxl_rcrb_addr file to dport_dev Ben Cheatham
@ 2023-09-12 14:11 ` Jonathan Cameron
2023-09-12 14:49 ` Ben Cheatham
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2023-09-12 14:11 UTC (permalink / raw)
To: Ben Cheatham
Cc: rafael, dan.j.williams, linux-cxl, linux-acpi, bhelgaas,
yazen.ghannam
On Thu, 7 Sep 2023 14:19:54 -0500
Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:
> Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie
> device) for CXL RCH root ports. The file will print the RCRB base
> MMIO address of the root port when read and will be used by
> users looking to inject CXL EINJ error types for RCH hosts.
>
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
Can we use is_visble to control presence of the attribute rather than
race condition special that is dynamic addition of a sysfs file.
You are adding the file to the linked device which is a bit odd.
Why there rather than in the portX?
I'd also normally expect the docs to call out the non-link path for that
device which is somewhere in the PCI topology I think.
> ---
> Documentation/ABI/testing/sysfs-bus-cxl | 8 ++++++
> drivers/cxl/acpi.c | 2 ++
> drivers/cxl/core/port.c | 33 +++++++++++++++++++++++++
> drivers/cxl/cxl.h | 2 ++
> 4 files changed, 45 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 087f762ebfd5..a7d169235543 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -177,6 +177,14 @@ Description:
> integer reflects the hardware port unique-id used in the
> hardware decoder target list.
>
> +What: /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr
> +Date: August, 2023
> +KernelVersion: v6.6
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RO) The 'cxl_rcrb_addr' device file gives the MMIO base address
> + of the RCRB of the corresponding CXL 1.1 downstream port. Only
> + present for CXL 1.1 dports.
>
> What: /sys/bus/cxl/devices/decoderX.Y
> Date: June, 2021
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index d1c559879dcc..3e2ca946bf47 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -676,6 +676,8 @@ static int cxl_acpi_probe(struct platform_device *pdev)
> if (IS_ERR(root_port))
> return PTR_ERR(root_port);
>
> + set_cxl_root(root_port);
> +
> rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
> add_host_bridge_dport);
> if (rc < 0)
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 724be8448eb4..001ab8742e21 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -875,6 +875,14 @@ struct cxl_port *find_cxl_root(struct cxl_port *port)
> }
> EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL);
>
> +static struct cxl_port *cxl_root;
> +
> +void set_cxl_root(struct cxl_port *root_port)
> +{
> + cxl_root = root_port;
> +}
> +EXPORT_SYMBOL_NS_GPL(set_cxl_root, CXL);
> +
> static struct cxl_dport *find_dport(struct cxl_port *port, int id)
> {
> struct cxl_dport *dport;
> @@ -930,11 +938,30 @@ static void cond_cxl_root_unlock(struct cxl_port *port)
> device_unlock(&port->dev);
> }
>
> +static ssize_t cxl_rcrb_addr_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct cxl_dport *dport;
> +
> + if (!cxl_root)
> + return -ENODEV;
> +
> + dport = cxl_find_dport_by_dev(cxl_root, dev);
> + if (!dport)
> + return -ENODEV;
> +
> + return sysfs_emit(buf, "0x%llx\n", (u64) dport->rcrb.base);
> +}
> +DEVICE_ATTR_RO(cxl_rcrb_addr);
> +
> static void cxl_dport_remove(void *data)
> {
> struct cxl_dport *dport = data;
> struct cxl_port *port = dport->port;
>
> + if (dport->rch)
> + device_remove_file(dport->dport_dev, &dev_attr_cxl_rcrb_addr);
> +
> xa_erase(&port->dports, (unsigned long) dport->dport_dev);
> put_device(dport->dport_dev);
> }
> @@ -1021,6 +1048,12 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> if (rc)
> return ERR_PTR(rc);
>
> + if (dport->rch && dport->rcrb.base != CXL_RESOURCE_NONE) {
> + rc = device_create_file(dport_dev, &dev_attr_cxl_rcrb_addr);
> + if (rc)
> + return ERR_PTR(rc);
> + }
> +
> return dport;
> }
>
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 76d92561af29..4d5bce4bae7e 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -690,6 +690,8 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
> resource_size_t component_reg_phys,
> struct cxl_dport *parent_dport);
> struct cxl_port *find_cxl_root(struct cxl_port *port);
> +void set_cxl_root(struct cxl_port *root_port);
> +
> int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
> void cxl_bus_rescan(void);
> void cxl_bus_drain(void);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] CXL, PCIE: Add cxl_rcrb_addr file to dport_dev
2023-09-12 14:11 ` Jonathan Cameron
@ 2023-09-12 14:49 ` Ben Cheatham
2023-09-12 16:33 ` Jonathan Cameron
0 siblings, 1 reply; 10+ messages in thread
From: Ben Cheatham @ 2023-09-12 14:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: rafael, dan.j.williams, linux-cxl, linux-acpi, bhelgaas,
yazen.ghannam
Hi Jonathan, thanks for the review. Responses inline.
On 9/12/23 9:11 AM, Jonathan Cameron wrote:
> On Thu, 7 Sep 2023 14:19:54 -0500
> Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:
>
>> Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie
>> device) for CXL RCH root ports. The file will print the RCRB base
>> MMIO address of the root port when read and will be used by
>> users looking to inject CXL EINJ error types for RCH hosts.
>>
>> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
>
> Can we use is_visble to control presence of the attribute rather than
> race condition special that is dynamic addition of a sysfs file.
>
Yeah, I'll go ahead and change it. Not sure why I did it that way to be
honest.
> You are adding the file to the linked device which is a bit odd.
> Why there rather than in the portX?
>
I agree it's a bit odd. I went with adding the file to the linked device
because the ACPI spec specifies using downstream ports for EINJ. The
alternative was to have a file for each dport under portX (i.e. dportY_rcrb_addr)
which seemed messier to me. Now that I think about it though, I could just
have a single file under portX that you write the dport name/number to and
it returns the rcrb address. Let me know if you think that would be better.
> I'd also normally expect the docs to call out the non-link path for that
> device which is somewhere in the PCI topology I think.
>
Yeah that makes sense, I'll go ahead and change that.
>> ---
>> Documentation/ABI/testing/sysfs-bus-cxl | 8 ++++++
>> drivers/cxl/acpi.c | 2 ++
>> drivers/cxl/core/port.c | 33 +++++++++++++++++++++++++
>> drivers/cxl/cxl.h | 2 ++
>> 4 files changed, 45 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
>> index 087f762ebfd5..a7d169235543 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-cxl
>> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
>> @@ -177,6 +177,14 @@ Description:
>> integer reflects the hardware port unique-id used in the
>> hardware decoder target list.
>>
>> +What: /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr
>> +Date: August, 2023
>> +KernelVersion: v6.6
>> +Contact: linux-cxl@vger.kernel.org
>> +Description:
>> + (RO) The 'cxl_rcrb_addr' device file gives the MMIO base address
>> + of the RCRB of the corresponding CXL 1.1 downstream port. Only
>> + present for CXL 1.1 dports.
>>
>> What: /sys/bus/cxl/devices/decoderX.Y
>> Date: June, 2021
>> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
>> index d1c559879dcc..3e2ca946bf47 100644
>> --- a/drivers/cxl/acpi.c
>> +++ b/drivers/cxl/acpi.c
>> @@ -676,6 +676,8 @@ static int cxl_acpi_probe(struct platform_device *pdev)
>> if (IS_ERR(root_port))
>> return PTR_ERR(root_port);
>>
>> + set_cxl_root(root_port);
>> +
>> rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
>> add_host_bridge_dport);
>> if (rc < 0)
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 724be8448eb4..001ab8742e21 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -875,6 +875,14 @@ struct cxl_port *find_cxl_root(struct cxl_port *port)
>> }
>> EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL);
>>
>> +static struct cxl_port *cxl_root;
>> +
>> +void set_cxl_root(struct cxl_port *root_port)
>> +{
>> + cxl_root = root_port;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(set_cxl_root, CXL);
>> +
>> static struct cxl_dport *find_dport(struct cxl_port *port, int id)
>> {
>> struct cxl_dport *dport;
>> @@ -930,11 +938,30 @@ static void cond_cxl_root_unlock(struct cxl_port *port)
>> device_unlock(&port->dev);
>> }
>>
>> +static ssize_t cxl_rcrb_addr_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct cxl_dport *dport;
>> +
>> + if (!cxl_root)
>> + return -ENODEV;
>> +
>> + dport = cxl_find_dport_by_dev(cxl_root, dev);
>> + if (!dport)
>> + return -ENODEV;
>> +
>> + return sysfs_emit(buf, "0x%llx\n", (u64) dport->rcrb.base);
>> +}
>> +DEVICE_ATTR_RO(cxl_rcrb_addr);
>> +
>> static void cxl_dport_remove(void *data)
>> {
>> struct cxl_dport *dport = data;
>> struct cxl_port *port = dport->port;
>>
>> + if (dport->rch)
>> + device_remove_file(dport->dport_dev, &dev_attr_cxl_rcrb_addr);
>> +
>> xa_erase(&port->dports, (unsigned long) dport->dport_dev);
>> put_device(dport->dport_dev);
>> }
>> @@ -1021,6 +1048,12 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
>> if (rc)
>> return ERR_PTR(rc);
>>
>> + if (dport->rch && dport->rcrb.base != CXL_RESOURCE_NONE) {
>> + rc = device_create_file(dport_dev, &dev_attr_cxl_rcrb_addr);
>> + if (rc)
>> + return ERR_PTR(rc);
>> + }
>> +
>> return dport;
>> }
>>
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index 76d92561af29..4d5bce4bae7e 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -690,6 +690,8 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
>> resource_size_t component_reg_phys,
>> struct cxl_dport *parent_dport);
>> struct cxl_port *find_cxl_root(struct cxl_port *port);
>> +void set_cxl_root(struct cxl_port *root_port);
>> +
>> int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
>> void cxl_bus_rescan(void);
>> void cxl_bus_drain(void);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/3] ACPI, APEI, EINJ: Add CXL 1.1 EINJ error type support
2023-09-12 14:00 ` Jonathan Cameron
@ 2023-09-12 14:49 ` Ben Cheatham
0 siblings, 0 replies; 10+ messages in thread
From: Ben Cheatham @ 2023-09-12 14:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: rafael, dan.j.williams, linux-cxl, linux-acpi, bhelgaas,
yazen.ghannam
On 9/12/23 9:00 AM, Jonathan Cameron wrote:
> On Thu, 7 Sep 2023 14:19:55 -0500
> Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:
>
>> Add support for CXL EINJ error types for CXL 1.1 hosts added in ACPI
>> v6.5. Because these error types target memory-mapped CXL 1.1 compliant
>> downstream ports and not physical (normal/persistent) memory, these
>> error types are not currently allowed through the memory range
>> validation done by the EINJ driver.
>>
>> The MMIO address of a CXL 1.1 downstream port can be found in the
>> cxl_rcrb_addr file in the corresponding dport directory under
>> /sys/bus/cxl/devices/portX. CXL 1.1 error types follow the same
>> procedure as a memory error type, but with param1 set to the
>> downstream port MMIO address.
>>
>> Example usage:
>> $ cd /sys/kernel/debug/apei/einj
>> $ cat available_error_type
>> 0x00000008 Memory Correctable
>> 0x00000010 Memory Uncorrectable non-fatal
>> 0x00000020 Memory Uncorrectable fatal
>> 0x00000040 PCI Express Correctable
>> 0x00000080 PCI Express Uncorrectable non-fatal
>> 0x00000100 PCI Express Uncorrectable fatal
>> 0x00008000 CXL.mem Protocol Correctable
>> 0x00020000 CXL.mem Protocol Uncorrectable fatal
>> $ echo 0x8000 > error_type
>> $ echo 0xfffffffffffff000 > param2
>> $ echo 0x3 > flags
>
> From einj.rst (and the ACPI spec) bit 0 here is
> Processor APIC field valid. Why is that relevant here?
> If it were you'd be writing param3 I think. So probably
> harmless, but I think this should be 0x2 > flags
>
I don't think it matters in this case, but I agree.
I'll change it to use 0x2 instead.
>> $ cat /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr
>> 0xb2f00000
>> $ echo 0xb2f00000 > param1
>> $ echo 1 > error_inject
>>
>> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
> Hi Ben,
>
> Why bother with the config dependent build?
> It doesn't save that much in code built and right now there
> are no non ACPI CXL systems so in reality it's always built
> anyway.
>
Ok, I'll remove the config dependencies. It'll make my life a
lot easier :).
> Otherwise this LGTM
>
>
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 001ab8742e21..f8f300496140 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -1122,6 +1122,25 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
>> }
>> EXPORT_SYMBOL_NS_GPL(devm_cxl_add_rch_dport, CXL);
>>
>> +#if IS_ENABLED(CONFIG_CXL_ACPI)
> I'm not a particular fan processor magic down in c files.
>
> Do we really care about the saving of not having this in builds where
> CONFIG_CXL_ACPI isn't set? I'm thinking those don't really exist.
>
I don't like it much either, I did it as a quick fix for the build errors
from the kernel test bot.
Thanks,
Ben
>> +struct cxl_dport *cxl_find_rch_dport_by_rcrb(resource_size_t rcrb_base)
>> +{
>> + struct cxl_dport *dport;
>> + unsigned long index;
>> +
>> + if (!cxl_root)
>> + return ERR_PTR(-ENODEV);
>> +
>> + xa_for_each(&cxl_root->dports, index, dport)
>> + if ((dport->rch && dport->rcrb.base != CXL_RESOURCE_NONE)
>> + && dport->rcrb.base == rcrb_base)
>> + return dport;
>> +
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_find_rch_dport_by_rcrb, CXL);
>> +#endif
>> +
>> static int add_ep(struct cxl_ep *new)
>> {
>> struct cxl_port *port = new->dport->port;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] CXL, PCIE: Add cxl_rcrb_addr file to dport_dev
2023-09-12 14:49 ` Ben Cheatham
@ 2023-09-12 16:33 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2023-09-12 16:33 UTC (permalink / raw)
To: Ben Cheatham
Cc: rafael, dan.j.williams, linux-cxl, linux-acpi, bhelgaas,
yazen.ghannam
On Tue, 12 Sep 2023 09:49:00 -0500
Ben Cheatham <benjamin.cheatham@amd.com> wrote:
> Hi Jonathan, thanks for the review. Responses inline.
>
> On 9/12/23 9:11 AM, Jonathan Cameron wrote:
> > On Thu, 7 Sep 2023 14:19:54 -0500
> > Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:
> >
> >> Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie
> >> device) for CXL RCH root ports. The file will print the RCRB base
> >> MMIO address of the root port when read and will be used by
> >> users looking to inject CXL EINJ error types for RCH hosts.
> >>
> >> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
> >
> > Can we use is_visble to control presence of the attribute rather than
> > race condition special that is dynamic addition of a sysfs file.
> >
>
> Yeah, I'll go ahead and change it. Not sure why I did it that way to be
> honest.
>
> > You are adding the file to the linked device which is a bit odd.
> > Why there rather than in the portX?
> >
>
> I agree it's a bit odd. I went with adding the file to the linked device
> because the ACPI spec specifies using downstream ports for EINJ. The
> alternative was to have a file for each dport under portX (i.e. dportY_rcrb_addr)
> which seemed messier to me. Now that I think about it though, I could just
> have a single file under portX that you write the dport name/number to and
> it returns the rcrb address. Let me know if you think that would be better.
Ah. I was being half asleep. I'd missed the point that a portX can have multiple
dportY (which is obvious given the naming :)
What makes here probably one for Dan or Bjorn to comment on..
Jonathan
>
> > I'd also normally expect the docs to call out the non-link path for that
> > device which is somewhere in the PCI topology I think.
> >
>
> Yeah that makes sense, I'll go ahead and change that.
>
> >> ---
> >> Documentation/ABI/testing/sysfs-bus-cxl | 8 ++++++
> >> drivers/cxl/acpi.c | 2 ++
> >> drivers/cxl/core/port.c | 33 +++++++++++++++++++++++++
> >> drivers/cxl/cxl.h | 2 ++
> >> 4 files changed, 45 insertions(+)
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> >> index 087f762ebfd5..a7d169235543 100644
> >> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> >> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> >> @@ -177,6 +177,14 @@ Description:
> >> integer reflects the hardware port unique-id used in the
> >> hardware decoder target list.
> >>
> >> +What: /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr
> >> +Date: August, 2023
> >> +KernelVersion: v6.6
> >> +Contact: linux-cxl@vger.kernel.org
> >> +Description:
> >> + (RO) The 'cxl_rcrb_addr' device file gives the MMIO base address
> >> + of the RCRB of the corresponding CXL 1.1 downstream port. Only
> >> + present for CXL 1.1 dports.
> >>
> >> What: /sys/bus/cxl/devices/decoderX.Y
> >> Date: June, 2021
> >> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> >> index d1c559879dcc..3e2ca946bf47 100644
> >> --- a/drivers/cxl/acpi.c
> >> +++ b/drivers/cxl/acpi.c
> >> @@ -676,6 +676,8 @@ static int cxl_acpi_probe(struct platform_device *pdev)
> >> if (IS_ERR(root_port))
> >> return PTR_ERR(root_port);
> >>
> >> + set_cxl_root(root_port);
> >> +
> >> rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
> >> add_host_bridge_dport);
> >> if (rc < 0)
> >> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> >> index 724be8448eb4..001ab8742e21 100644
> >> --- a/drivers/cxl/core/port.c
> >> +++ b/drivers/cxl/core/port.c
> >> @@ -875,6 +875,14 @@ struct cxl_port *find_cxl_root(struct cxl_port *port)
> >> }
> >> EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL);
> >>
> >> +static struct cxl_port *cxl_root;
> >> +
> >> +void set_cxl_root(struct cxl_port *root_port)
> >> +{
> >> + cxl_root = root_port;
> >> +}
> >> +EXPORT_SYMBOL_NS_GPL(set_cxl_root, CXL);
> >> +
> >> static struct cxl_dport *find_dport(struct cxl_port *port, int id)
> >> {
> >> struct cxl_dport *dport;
> >> @@ -930,11 +938,30 @@ static void cond_cxl_root_unlock(struct cxl_port *port)
> >> device_unlock(&port->dev);
> >> }
> >>
> >> +static ssize_t cxl_rcrb_addr_show(struct device *dev,
> >> + struct device_attribute *attr, char *buf)
> >> +{
> >> + struct cxl_dport *dport;
> >> +
> >> + if (!cxl_root)
> >> + return -ENODEV;
> >> +
> >> + dport = cxl_find_dport_by_dev(cxl_root, dev);
> >> + if (!dport)
> >> + return -ENODEV;
> >> +
> >> + return sysfs_emit(buf, "0x%llx\n", (u64) dport->rcrb.base);
> >> +}
> >> +DEVICE_ATTR_RO(cxl_rcrb_addr);
> >> +
> >> static void cxl_dport_remove(void *data)
> >> {
> >> struct cxl_dport *dport = data;
> >> struct cxl_port *port = dport->port;
> >>
> >> + if (dport->rch)
> >> + device_remove_file(dport->dport_dev, &dev_attr_cxl_rcrb_addr);
> >> +
> >> xa_erase(&port->dports, (unsigned long) dport->dport_dev);
> >> put_device(dport->dport_dev);
> >> }
> >> @@ -1021,6 +1048,12 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
> >> if (rc)
> >> return ERR_PTR(rc);
> >>
> >> + if (dport->rch && dport->rcrb.base != CXL_RESOURCE_NONE) {
> >> + rc = device_create_file(dport_dev, &dev_attr_cxl_rcrb_addr);
> >> + if (rc)
> >> + return ERR_PTR(rc);
> >> + }
> >> +
> >> return dport;
> >> }
> >>
> >> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> >> index 76d92561af29..4d5bce4bae7e 100644
> >> --- a/drivers/cxl/cxl.h
> >> +++ b/drivers/cxl/cxl.h
> >> @@ -690,6 +690,8 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
> >> resource_size_t component_reg_phys,
> >> struct cxl_dport *parent_dport);
> >> struct cxl_port *find_cxl_root(struct cxl_port *port);
> >> +void set_cxl_root(struct cxl_port *root_port);
> >> +
> >> int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd);
> >> void cxl_bus_rescan(void);
> >> void cxl_bus_drain(void);
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-09-12 16:34 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-07 19:19 [PATCH v4 0/3] CXL, ACPI, APEI, EINJ: Update EINJ for CXL 1.1 error types Ben Cheatham
2023-09-07 19:19 ` [PATCH v4 1/3] CXL, PCIE: Add cxl_rcrb_addr file to dport_dev Ben Cheatham
2023-09-12 14:11 ` Jonathan Cameron
2023-09-12 14:49 ` Ben Cheatham
2023-09-12 16:33 ` Jonathan Cameron
2023-09-07 19:19 ` [PATCH v4 2/3] ACPI, APEI, EINJ: Add CXL 1.1 EINJ error type support Ben Cheatham
2023-09-12 14:00 ` Jonathan Cameron
2023-09-12 14:49 ` Ben Cheatham
2023-09-07 19:19 ` [PATCH v4 3/3] ACPI, APEI, EINJ: Update EINJ documentation Ben Cheatham
2023-09-12 13:51 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox