Re: [PATCH v14 2/4] EINJ: Add CXL error type support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ben Cheatham wrote:
> This patch had an outdated commit message, so here's the patch with an updated description.
> 
> I also realized that I was wrong about letting CXL 2.0+ error types (discussed a revision
> or two ago) and I wasn't actually letting them through. I've went ahead and added
> the ability to inject CXL 2.0+ error through the legacy interface. This pretty
> much amounts to returning an error for CXL 1.0/1.1 injection types in einj_error_inject()
> and instead routing them through a new einj_cxl_rch_error_inject() function called
> in einj-cxl.c
> 
> If this change is too big I can send in another revision, I just wanted to avoid
> spamming the list(s).
> 
> From eea1cf991dc2a551f6db2e3bb9510ed43c86762d Mon Sep 17 00:00:00 2001
> From: Ben Cheatham <Benjamin.Cheatham@xxxxxxx>
> Date: Fri, 16 Feb 2024 11:12:51 -0600
> Subject: [PATCH v14 2/4] EINJ: Add CXL error type support
> 
> Move CXL protocol error types from einj.c (now einj-core.c) to einj-cxl.c.
> einj-cxl.c implements the necessary handling for CXL protocol error
> injection and exposes an API for the CXL core to use said functionality,
> while also allowing the EINJ module to be built without CXL support.
> Because CXL error types targeting CXL 1.0/1.1 ports require special
> handling, only allow them to be injected through the new cxl debugfs
> interface (next commit) and return an error when attempting to inject
> through the legacy interface.
> 
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@xxxxxxx>
> ---
>  MAINTAINERS                               |   1 +
>  drivers/acpi/apei/Kconfig                 |  12 +++
>  drivers/acpi/apei/Makefile                |   2 +
>  drivers/acpi/apei/apei-internal.h         |  18 ++++
>  drivers/acpi/apei/{einj.c => einj-core.c} |  85 +++++++++++----
>  drivers/acpi/apei/einj-cxl.c              | 121 ++++++++++++++++++++++
>  include/linux/einj-cxl.h                  |  40 +++++++
>  7 files changed, 257 insertions(+), 22 deletions(-)
>  rename drivers/acpi/apei/{einj.c => einj-core.c} (94%)
>  create mode 100644 drivers/acpi/apei/einj-cxl.c
>  create mode 100644 include/linux/einj-cxl.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2ecaaec6a6bf..90cf8403dd17 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5289,6 +5289,7 @@ M:	Dan Williams <dan.j.williams@xxxxxxxxx>
>  L:	linux-cxl@xxxxxxxxxxxxxxx
>  S:	Maintained
>  F:	drivers/cxl/
> +F:	include/linux/cxl-einj.h
>  F:	include/linux/cxl-event.h
>  F:	include/uapi/linux/cxl_mem.h
>  F:	tools/testing/cxl/
> diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
> index 6b18f8bc7be3..f01afa2805be 100644
> --- a/drivers/acpi/apei/Kconfig
> +++ b/drivers/acpi/apei/Kconfig
> @@ -60,6 +60,18 @@ config ACPI_APEI_EINJ
>  	  mainly used for debugging and testing the other parts of
>  	  APEI and some other RAS features.
>  
> +config ACPI_APEI_EINJ_CXL
> +	bool "CXL Error INJection Support"
> +	default ACPI_APEI_EINJ
> +	depends on ACPI_APEI_EINJ && CXL_BUS <= ACPI_APEI_EINJ
> +	help
> +	  Support for CXL protocol Error INJection through debugfs/cxl.
> +	  Availability and which errors are supported is dependent on
> +	  the host platform. Look to ACPI v6.5 section 18.6.4 and kernel
> +	  EINJ documentation for more information.
> +
> +	  If unsure say 'n'
> +
>  config ACPI_APEI_ERST_DEBUG
>  	tristate "APEI Error Record Serialization Table (ERST) Debug Support"
>  	depends on ACPI_APEI
> diff --git a/drivers/acpi/apei/Makefile b/drivers/acpi/apei/Makefile
> index 4dfac2128737..2c474e6477e1 100644
> --- a/drivers/acpi/apei/Makefile
> +++ b/drivers/acpi/apei/Makefile
> @@ -2,6 +2,8 @@
>  obj-$(CONFIG_ACPI_APEI)		+= apei.o
>  obj-$(CONFIG_ACPI_APEI_GHES)	+= ghes.o
>  obj-$(CONFIG_ACPI_APEI_EINJ)	+= einj.o
> +einj-y				:= einj-core.o
> +einj-$(CONFIG_ACPI_APEI_EINJ_CXL) += einj-cxl.o
>  obj-$(CONFIG_ACPI_APEI_ERST_DEBUG) += erst-dbg.o
>  
>  apei-y := apei-base.o hest.o erst.o bert.o
> diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
> index 67c2c3b959e1..cd2766c69d78 100644
> --- a/drivers/acpi/apei/apei-internal.h
> +++ b/drivers/acpi/apei/apei-internal.h
> @@ -130,4 +130,22 @@ static inline u32 cper_estatus_len(struct acpi_hest_generic_status *estatus)
>  }
>  
>  int apei_osc_setup(void);
> +
> +int einj_get_available_error_type(u32 *type);
> +int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
> +		      u64 param4);
> +int einj_cxl_rch_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> +			      u64 param3, u64 param4);
> +bool einj_is_cxl_error_type(u64 type);
> +int einj_validate_error_type(u64 type);
> +
> +#ifndef ACPI_EINJ_CXL_CACHE_CORRECTABLE
> +#define ACPI_EINJ_CXL_CACHE_CORRECTABLE     BIT(12)
> +#define ACPI_EINJ_CXL_CACHE_UNCORRECTABLE   BIT(13)
> +#define ACPI_EINJ_CXL_CACHE_FATAL           BIT(14)
> +#define ACPI_EINJ_CXL_MEM_CORRECTABLE       BIT(15)
> +#define ACPI_EINJ_CXL_MEM_UNCORRECTABLE     BIT(16)
> +#define ACPI_EINJ_CXL_MEM_FATAL             BIT(17)
> +#endif
> +
>  #endif
> diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj-core.c
> similarity index 94%
> rename from drivers/acpi/apei/einj.c
> rename to drivers/acpi/apei/einj-core.c
> index 937c69844dac..437c13949be7 100644
> --- a/drivers/acpi/apei/einj.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -37,6 +37,12 @@
>  #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
>  				ACPI_EINJ_MEMORY_UNCORRECTABLE | \
>  				ACPI_EINJ_MEMORY_FATAL)
> +#define CXL_ERROR_MASK		(ACPI_EINJ_CXL_CACHE_CORRECTABLE | \
> +				ACPI_EINJ_CXL_CACHE_UNCORRECTABLE | \
> +				ACPI_EINJ_CXL_CACHE_FATAL | \
> +				ACPI_EINJ_CXL_MEM_CORRECTABLE | \
> +				ACPI_EINJ_CXL_MEM_UNCORRECTABLE | \
> +				ACPI_EINJ_CXL_MEM_FATAL)
>  
>  /*
>   * ACPI version 5 provides a SET_ERROR_TYPE_WITH_ADDRESS action.
> @@ -141,7 +147,7 @@ static DEFINE_MUTEX(einj_mutex);
>  /*
>   * Exported APIs use this flag to exit early if einj_probe() failed.
>   */
> -static bool einj_initialized __ro_after_init;
> +bool einj_initialized __ro_after_init;
>  
>  static void *einj_param;
>  
> @@ -166,7 +172,7 @@ static int __einj_get_available_error_type(u32 *type)
>  }
>  
>  /* Get error injection capabilities of the platform */
> -static int einj_get_available_error_type(u32 *type)
> +int einj_get_available_error_type(u32 *type)
>  {
>  	int rc;
>  
> @@ -536,8 +542,8 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>  }
>  
>  /* Inject the specified hardware error */
> -static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
> -			     u64 param3, u64 param4)
> +int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
> +		      u64 param4)
>  {
>  	int rc;
>  	u64 base_addr, size;
> @@ -560,8 +566,18 @@ 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;
> +	}
> +
> +	/*
> +	 * Injections targeting a CXL 1.0/1.1 port have to be injected
> +	 * from the CXL debugfs interface so that we can guarantee a
> +	 * correct MMIO address.
> +	 */

Given that the CXL debugfs is not present in this file I would update
this comment to give a hints using local references. Something like:

	/*
	 * Injections targeting a CXL 1.0/1.1 port have to be injected
	 * via the einj_cxl_rch_error_inject() path as that does proper
	 * input validation that passed address is an RCRB base address.
	 */

...that said, how does this work for the CXL 2.0 path? Does not
einj_cxl_inject_error() need to use __einj_error_inject() rather than
einj_error_inject() to get by this check?





[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux