Ben Cheatham wrote:
> Add functions to the EINJ module to be used in the CXL module for CXL
> protocol error injection. The callbacks implement the einj_types and
> einj_inject files under /sys/kernel/debug/cxl/portX/dportY. These two
> files work in the same way as the available_error_types and error_inject
> files under /sys/kernel/debug/apei/einj, but only for CXL error types.
> If the dport is enumerated in RCH (CXL 1.1) mode, a CXL 1.1 error is
> injected into the dport; a CXL 2.0 error is injected otherwise.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@xxxxxxx>
> ---
> drivers/acpi/apei/Kconfig | 3 ++
> drivers/acpi/apei/einj.c | 105 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 108 insertions(+)
>
> diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
> index 6b18f8bc7be3..100c27beb581 100644
> --- a/drivers/acpi/apei/Kconfig
> +++ b/drivers/acpi/apei/Kconfig
> @@ -11,6 +11,9 @@ config ACPI_APEI
> select PSTORE
> select UEFI_CPER
> depends on HAVE_ACPI_APEI
> + imply CXL_BUS
> + imply CXL_ACPI
> + imply CXL_PORT
This goes away with the CONFIG_CXL_EINJ scheme proposed on patch1.
> help
> APEI allows to report errors (for example from the chipset)
> to the operating system. This improves NMI handling
> diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
> index 330329ac2f1f..98d5e6d60a02 100644
> --- a/drivers/acpi/apei/einj.c
> +++ b/drivers/acpi/apei/einj.c
> @@ -21,9 +21,11 @@
> #include <linux/nmi.h>
> #include <linux/delay.h>
> #include <linux/mm.h>
> +#include <linux/pci.h>
> #include <asm/unaligned.h>
>
> #include "apei-internal.h"
> +#include "../../cxl/cxl.h"
EINJ has no business knowing all of the details in cxl.h. In fact all
EINJ cares about is dport->dport_dev and dport->rch. I think just add
those to the calling convention, see below:
> #undef pr_fmt
> #define pr_fmt(fmt) "EINJ: " fmt
> @@ -627,6 +629,25 @@ static int available_error_type_show(struct seq_file *m, void *v)
>
> DEFINE_SHOW_ATTRIBUTE(available_error_type);
>
> +static int cxl_einj_available_error_type(struct seq_file *m, void *v)
> +{
> + int cxl_err, rc;
> + u32 available_error_type = 0;
> +
> + rc = einj_get_available_error_type(&available_error_type);
> + if (rc)
> + return rc;
> +
> + for (int pos = 0; pos < ARRAY_SIZE(einj_cxl_error_type_string); pos++) {
> + cxl_err = ACPI_EINJ_CXL_CACHE_CORRECTABLE << pos;
> +
> + if (available_error_type & cxl_err)
> + seq_puts(m, einj_cxl_error_type_string[pos]);
> + }
> +
> + return 0;
> +}
> +
> static int validate_error_type(u64 type)
> {
> u32 tval, vendor, available_error_type = 0;
> @@ -657,6 +678,68 @@ static int validate_error_type(u64 type)
> return 0;
> }
>
> +static int cxl_dport_get_sbdf(struct cxl_dport *dport, u64 *sbdf)
> +{
> + struct pci_dev *pdev;
> + struct pci_bus *pbus;
> + struct pci_host_bridge *bridge;
> + u64 seg = 0, bus;
> +
> + if (!dev_is_pci(dport->dport_dev))
> + return -EINVAL;
> +
> + pdev = to_pci_dev(dport->dport_dev);
> + pbus = pdev->bus;
> + bridge = pci_find_host_bridge(pbus);
> +
> + if (!bridge)
> + return -ENODEV;
> +
> + if (bridge->domain_nr != PCI_DOMAIN_NR_NOT_SET)
> + seg = bridge->domain_nr << 24;
> +
> + bus = pbus->number << 16;
> + *sbdf = seg | bus | pdev->devfn;
> +
> + return 0;
> +}
> +
> +static int cxl_einj_inject_error(struct cxl_dport *dport, u64 type)
If you split this into cxl_einj_inject_error() and
cxl_einj_inject_rch_error() with following prototypes:
static int cxl_einj_inject_error(struct pci_dev *dport_dev, u64 type)
static int cxl_einj_inject_rch_error(u64 rcrb, u64 type)
...then you don't need any of the cxl.h definitions. The dev_is_pci()
and rch determination details can remain private to CXL.
> +{
> + u64 param1 = 0, param2 = 0, param4 = 0;
> + u32 flags;
> + int rc;
> +
> + /* Only CXL error types can be specified */
> + if (type & ~CXL_ERROR_MASK || (type & ACPI5_VENDOR_BIT))
> + return -EINVAL;
> +
> + rc = validate_error_type(type);
> + if (rc)
> + return rc;
> +
> + /*
> + * If dport is in restricted mode, inject a CXL 1.1 error,
> + * otherwise inject a CXL 2.0 error
> + */
> + if (dport->rch) {
> + if (dport->rcrb.base == CXL_RESOURCE_NONE)
> + return -EINVAL;
> +
> + param1 = (u64) dport->rcrb.base;
> + param2 = 0xfffffffffffff000;
> + flags = 0x2;
> + } else {
> + rc = cxl_dport_get_sbdf(dport, ¶m4);
> + if (rc)
> + return rc;
> +
> + flags = 0x4;
> + }
> +
> + return einj_error_inject(type, flags, param1, param2, 0, param4);
> +}
> +
> static int error_type_get(void *data, u64 *val)
> {
> *val = error_type;
> @@ -668,6 +751,7 @@ static int error_type_set(void *data, u64 val)
> {
> int rc;
>
> + /* CXL error types have to be injected from cxl debugfs */
> if (val & CXL_ERROR_MASK && !(val & ACPI5_VENDOR_BIT))
> return -EINVAL;
>
> @@ -714,6 +798,7 @@ static int __init einj_init(void)
> {
> int rc;
> acpi_status status;
> + struct cxl_einj_ops cxl_ops;
> struct apei_exec_context ctx;
>
> if (acpi_disabled) {
> @@ -793,6 +878,15 @@ static int __init einj_init(void)
> einj_debug_dir, &vendor_flags);
> }
>
> + if (IS_REACHABLE(CONFIG_CXL_ACPI) || IS_REACHABLE(CONFIG_CXL_PORT)) {
> + cxl_ops = (struct cxl_einj_ops) {
> + .einj_type = cxl_einj_available_error_type,
> + .einj_inject = cxl_einj_inject_error,
> + };
> +
> + cxl_einj_set_ops_cbs(&cxl_ops);
> + }
This goes away with the new Kconfig dependency scheme.
> +
> pr_info("Error INJection is initialized.\n");
>
> return 0;
> @@ -810,8 +904,18 @@ static int __init einj_init(void)
>
> static void __exit einj_exit(void)
> {
> + struct cxl_einj_ops cxl_ops;
> struct apei_exec_context ctx;
>
> + if (IS_REACHABLE(CONFIG_CXL_ACPI) || IS_REACHABLE(CONFIG_CXL_PORT)) {
> + cxl_ops = (struct cxl_einj_ops) {
> + .einj_type = NULL,
> + .einj_inject = NULL,
> + };
> +
> + cxl_einj_set_ops_cbs(&cxl_ops);
> + }
> +
> if (einj_param) {
> acpi_size size = (acpi5) ?
> sizeof(struct set_error_type_with_address) :
> @@ -832,4 +936,5 @@ module_exit(einj_exit);
>
> MODULE_AUTHOR("Huang Ying");
> MODULE_DESCRIPTION("APEI Error INJection support");
> +MODULE_IMPORT_NS(CXL);
EINJ never references a CXL symbol in the new proposed scheme.
