[PATCH v2 0/9] Enable EINJv2 support

[PATCH v2 0/9] Enable EINJv2 support

[Zaid Alali]

The goal of this update is to allow the driver to simultaneously
support EINJ and EINJv2. The implementation follows the approved
ACPI specs(1)(2) that enables the driver to discover system
capabilities through GET_ERROR_TYPE.

Note: The first two ACPICA patches are to be dropped once merged in
ACPICA project, see pull request(3).

(1) https://bugzilla.tianocore.org/show_bug.cgi?id=4615
(2) https://bugzilla.tianocore.org/attachment.cgi?id=1521
(3) https://github.com/acpica/acpica/pull/977

Note: This revision includes an update on patch 5/9 to avoid
ambiguity between EINJv1 and EINJv2 error types. Also, it includes
an additional patch to fix sparse warnings triggered by test robot.

Zaid Alali (9):
  ACPICA: Update values to hex to follow ACPI specs
  ACPICA: Add EINJv2 get error type action
  ACPI: APEI: EINJ: Fix kernel test robot sparse warning
  ACPI: APEI: EINJ: Remove redundant calls to
    einj_get_available_error_type
  ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities
  ACPI: APEI: EINJ: Add einjv2 extension struct
  ACPI: APEI: EINJ: Add debugfs files for EINJv2 support
  ACPI: APEI: EINJ: Enable EINJv2 error injections
  ACPI: APEI: EINJ: Update the documentation for EINJv2 support

 .../firmware-guide/acpi/apei/einj.rst         |  41 ++-
 drivers/acpi/apei/apei-internal.h             |   2 +-
 drivers/acpi/apei/einj-core.c                 | 254 ++++++++++++++----
 drivers/acpi/apei/einj-cxl.c                  |   2 +-
 include/acpi/actbl1.h                         |  25 +-
 5 files changed, 260 insertions(+), 64 deletions(-)

-- 
2.34.1

[PATCH v2 1/9] ACPICA: Update values to hex to follow ACPI specs

[Zaid Alali]

ACPI specs(1) define Error Injection Actions in hex values.
This commit intends to update values from decimal to hex to be
consistent with ACPI specs. This commit and the following one are
not to be merged and will come form ACPICA project see pull request(2).

(1) https://uefi.org/specs/ACPI/6.5/18_Platform_Error_Interfaces.html
(2) https://github.com/acpica/acpica/pull/977

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 include/acpi/actbl1.h | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 387fc821703a..c701c434976c 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -1024,18 +1024,18 @@ struct acpi_einj_entry {
 /* Values for Action field above */
 
 enum acpi_einj_actions {
-	ACPI_EINJ_BEGIN_OPERATION = 0,
-	ACPI_EINJ_GET_TRIGGER_TABLE = 1,
-	ACPI_EINJ_SET_ERROR_TYPE = 2,
-	ACPI_EINJ_GET_ERROR_TYPE = 3,
-	ACPI_EINJ_END_OPERATION = 4,
-	ACPI_EINJ_EXECUTE_OPERATION = 5,
-	ACPI_EINJ_CHECK_BUSY_STATUS = 6,
-	ACPI_EINJ_GET_COMMAND_STATUS = 7,
-	ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS = 8,
-	ACPI_EINJ_GET_EXECUTE_TIMINGS = 9,
-	ACPI_EINJ_ACTION_RESERVED = 10,	/* 10 and greater are reserved */
-	ACPI_EINJ_TRIGGER_ERROR = 0xFF	/* Except for this value */
+	ACPI_EINJ_BEGIN_OPERATION =		0x0,
+	ACPI_EINJ_GET_TRIGGER_TABLE =		0x1,
+	ACPI_EINJ_SET_ERROR_TYPE =		0x2,
+	ACPI_EINJ_GET_ERROR_TYPE =		0x3,
+	ACPI_EINJ_END_OPERATION =		0x4,
+	ACPI_EINJ_EXECUTE_OPERATION =		0x5,
+	ACPI_EINJ_CHECK_BUSY_STATUS =		0x6,
+	ACPI_EINJ_GET_COMMAND_STATUS =		0x7,
+	ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS =	0x8,
+	ACPI_EINJ_GET_EXECUTE_TIMINGS =		0x9,
+	ACPI_EINJ_ACTION_RESERVED =		0xA,	/* 0xA and greater are reserved */
+	ACPI_EINJ_TRIGGER_ERROR =		0xFF	/* Except for this value */
 };
 
 /* Values for Instruction field above */
-- 
2.34.1

[PATCH v2 2/9] ACPICA: Add EINJv2 get error type action

[Zaid Alali]

Add EINJV2_GET_ERROR_TYPE as defined in the approved new ACPI
specs(1)(2).

Proposed ACPI spces for EINJv2:
(1) https://bugzilla.tianocore.org/show_bug.cgi?id=4615
(2) https://bugzilla.tianocore.org/attachment.cgi?id=1521

This commit is not a direct merge, it will come from ACPICA
project, see pull request(3).

(3) https://github.com/acpica/acpica/pull/977

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 include/acpi/actbl1.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index c701c434976c..f52d5cafaf76 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -1034,7 +1034,8 @@ enum acpi_einj_actions {
 	ACPI_EINJ_GET_COMMAND_STATUS =		0x7,
 	ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS =	0x8,
 	ACPI_EINJ_GET_EXECUTE_TIMINGS =		0x9,
-	ACPI_EINJ_ACTION_RESERVED =		0xA,	/* 0xA and greater are reserved */
+	ACPI_EINJV2_GET_ERROR_TYPE =		0x11,
+	ACPI_EINJ_ACTION_RESERVED =		0x12,	/* 0x12 and greater are reserved */
 	ACPI_EINJ_TRIGGER_ERROR =		0xFF	/* Except for this value */
 };
 
-- 
2.34.1

[PATCH v2 3/9] ACPI: APEI: EINJ: Fix kernel test robot sparse warning

[Zaid Alali]

This patch fixes the kernel test robot warning reported here:
https://lore.kernel.org/all/202410241620.oApALow5-lkp@intel.com/

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/einj-core.c | 41 +++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index 04731a5b01fa..74dfb3daba50 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -215,20 +215,22 @@ static void check_vendor_extension(u64 paddr,
 {
 	int	offset = v5param->vendor_extension;
 	struct	vendor_error_type_extension *v;
+	void __iomem *p;
 	u32	sbdf;
 
 	if (!offset)
 		return;
-	v = acpi_os_map_iomem(paddr + offset, sizeof(*v));
-	if (!v)
+	p = acpi_os_map_iomem(paddr + offset, sizeof(*v));
+	if (!p)
 		return;
+	v = __io_virt(p);
 	get_oem_vendor_struct(paddr, offset, v);
 	sbdf = v->pcie_sbdf;
 	sprintf(vendor_dev, "%x:%x:%x.%x vendor_id=%x device_id=%x rev_id=%x\n",
 		sbdf >> 24, (sbdf >> 16) & 0xff,
 		(sbdf >> 11) & 0x1f, (sbdf >> 8) & 0x7,
 		 v->vendor_id, v->device_id, v->rev_id);
-	acpi_os_unmap_iomem(v, sizeof(*v));
+	acpi_os_unmap_iomem(p, sizeof(*v));
 }
 
 static void *einj_get_parameter_address(void)
@@ -253,9 +255,11 @@ static void *einj_get_parameter_address(void)
 	}
 	if (pa_v5) {
 		struct set_error_type_with_address *v5param;
+		void __iomem *p;
 
-		v5param = acpi_os_map_iomem(pa_v5, sizeof(*v5param));
-		if (v5param) {
+		p = acpi_os_map_iomem(pa_v5, sizeof(*v5param));
+		if (p) {
+			v5param = __io_virt(p);
 			acpi5 = 1;
 			check_vendor_extension(pa_v5, v5param);
 			return v5param;
@@ -263,12 +267,14 @@ static void *einj_get_parameter_address(void)
 	}
 	if (param_extension && pa_v4) {
 		struct einj_parameter *v4param;
+		void __iomem *p;
 
-		v4param = acpi_os_map_iomem(pa_v4, sizeof(*v4param));
-		if (!v4param)
+		p = acpi_os_map_iomem(pa_v4, sizeof(*v4param));
+		if (!p)
 			return NULL;
+		v4param = __io_virt(p);
 		if (v4param->reserved1 || v4param->reserved2) {
-			acpi_os_unmap_iomem(v4param, sizeof(*v4param));
+			acpi_os_unmap_iomem(p, sizeof(*v4param));
 			return NULL;
 		}
 		return v4param;
@@ -325,6 +331,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 	u32 table_size;
 	int rc = -EIO;
 	struct acpi_generic_address *trigger_param_region = NULL;
+	void __iomem *p;
 
 	r = request_mem_region(trigger_paddr, sizeof(*trigger_tab),
 			       "APEI EINJ Trigger Table");
@@ -335,11 +342,12 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 			    sizeof(*trigger_tab) - 1);
 		goto out;
 	}
-	trigger_tab = ioremap_cache(trigger_paddr, sizeof(*trigger_tab));
-	if (!trigger_tab) {
+	p = ioremap_cache(trigger_paddr, sizeof(*trigger_tab));
+	if (!p) {
 		pr_err("Failed to map trigger table!\n");
 		goto out_rel_header;
 	}
+	trigger_tab = __io_virt(p);
 	rc = einj_check_trigger_header(trigger_tab);
 	if (rc) {
 		pr_warn(FW_BUG "Invalid trigger error action table.\n");
@@ -361,12 +369,13 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 		       (unsigned long long)trigger_paddr + table_size - 1);
 		goto out_rel_header;
 	}
-	iounmap(trigger_tab);
-	trigger_tab = ioremap_cache(trigger_paddr, table_size);
-	if (!trigger_tab) {
+	iounmap(p);
+	p = ioremap_cache(trigger_paddr, table_size);
+	if (!p) {
 		pr_err("Failed to map trigger table!\n");
 		goto out_rel_entry;
 	}
+	trigger_tab = __io_virt(p);
 	trigger_entry = (struct acpi_whea_header *)
 		((char *)trigger_tab + sizeof(struct acpi_einj_trigger));
 	apei_resources_init(&trigger_resources);
@@ -424,8 +433,8 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 out_rel_header:
 	release_mem_region(trigger_paddr, sizeof(*trigger_tab));
 out:
-	if (trigger_tab)
-		iounmap(trigger_tab);
+	if (p)
+		iounmap(p);
 
 	return rc;
 }
@@ -860,7 +869,7 @@ static void __exit einj_remove(struct platform_device *pdev)
 			sizeof(struct set_error_type_with_address) :
 			sizeof(struct einj_parameter);
 
-		acpi_os_unmap_iomem(einj_param, size);
+		acpi_os_unmap_iomem((void __iomem *)einj_param, size);
 		if (vendor_errors.size)
 			acpi_os_unmap_memory(vendor_errors.data, vendor_errors.size);
 	}
-- 
2.34.1

Re: [PATCH v2 3/9] ACPI: APEI: EINJ: Fix kernel test robot sparse warning

[kernel test robot]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 19090 bytes --]

Hi Zaid,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge linus/master v6.13-rc1 next-20241205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zaid-Alali/ACPICA-Update-values-to-hex-to-follow-ACPI-specs/20241206-052420
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20241205211854.43215-4-zaidal%40os.amperecomputing.com
patch subject: [PATCH v2 3/9] ACPI: APEI: EINJ: Fix kernel test robot sparse warning
config: i386-buildonly-randconfig-005-20241206 (https://download.01.org/0day-ci/archive/20241206/202412061152.Zar8cGxV-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241206/202412061152.Zar8cGxV-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412061152.Zar8cGxV-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/acpi/apei/einj-core.c:23:
   In file included from include/linux/mm.h:2223:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/acpi/apei/einj-core.c:338:6: warning: variable 'p' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
     338 |         if (!r) {
         |             ^~
   drivers/acpi/apei/einj-core.c:436:6: note: uninitialized use occurs here
     436 |         if (p)
         |             ^
   drivers/acpi/apei/einj-core.c:338:2: note: remove the 'if' if its condition is always false
     338 |         if (!r) {
         |         ^~~~~~~~~
     339 |                 pr_err("Can not request [mem %#010llx-%#010llx] for Trigger table\n",
         |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     340 |                        (unsigned long long)trigger_paddr,
         |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     341 |                        (unsigned long long)trigger_paddr +
         |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     342 |                             sizeof(*trigger_tab) - 1);
         |                             ~~~~~~~~~~~~~~~~~~~~~~~~~~
     343 |                 goto out;
         |                 ~~~~~~~~~
     344 |         }
         |         ~
   drivers/acpi/apei/einj-core.c:334:17: note: initialize the variable 'p' to silence this warning
     334 |         void __iomem *p;
         |                        ^
         |                         = NULL
   2 warnings generated.


vim +338 drivers/acpi/apei/einj-core.c

e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  301  
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  302  static struct acpi_generic_address *einj_get_trigger_parameter_region(
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  303  	struct acpi_einj_trigger *trigger_tab, u64 param1, u64 param2)
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  304  {
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  305  	int i;
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  306  	struct acpi_whea_header *entry;
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  307  
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  308  	entry = (struct acpi_whea_header *)
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  309  		((char *)trigger_tab + sizeof(struct acpi_einj_trigger));
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  310  	for (i = 0; i < trigger_tab->entry_count; i++) {
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  311  		if (entry->action == ACPI_EINJ_TRIGGER_ERROR &&
1d5d820b8fe83b5 drivers/acpi/apei/einj.c      Yazen Ghannam    2017-08-28  312  		entry->instruction <= ACPI_EINJ_WRITE_REGISTER_VALUE &&
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  313  		entry->register_region.space_id ==
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  314  			ACPI_ADR_SPACE_SYSTEM_MEMORY &&
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  315  		(entry->register_region.address & param2) == (param1 & param2))
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  316  			return &entry->register_region;
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  317  		entry++;
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  318  	}
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  319  
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  320  	return NULL;
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  321  }
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  322  /* Execute instructions in trigger error action table */
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  323  static int __einj_error_trigger(u64 trigger_paddr, u32 type,
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  324  				u64 param1, u64 param2)
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  325  {
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  326  	struct acpi_einj_trigger *trigger_tab = NULL;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  327  	struct apei_exec_context trigger_ctx;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  328  	struct apei_resources trigger_resources;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  329  	struct acpi_whea_header *trigger_entry;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  330  	struct resource *r;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  331  	u32 table_size;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  332  	int rc = -EIO;
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  333  	struct acpi_generic_address *trigger_param_region = NULL;
37dbc95939d19c3 drivers/acpi/apei/einj-core.c Zaid Alali       2024-12-05  334  	void __iomem *p;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  335  
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  336  	r = request_mem_region(trigger_paddr, sizeof(*trigger_tab),
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  337  			       "APEI EINJ Trigger Table");
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18 @338  	if (!r) {
b2f740baa421525 drivers/acpi/apei/einj.c      Borislav Petkov  2016-05-23  339  		pr_err("Can not request [mem %#010llx-%#010llx] for Trigger table\n",
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  340  		       (unsigned long long)trigger_paddr,
46b91e379f7180b drivers/acpi/apei/einj.c      Bjorn Helgaas    2011-12-08  341  		       (unsigned long long)trigger_paddr +
46b91e379f7180b drivers/acpi/apei/einj.c      Bjorn Helgaas    2011-12-08  342  			    sizeof(*trigger_tab) - 1);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  343  		goto out;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  344  	}
37dbc95939d19c3 drivers/acpi/apei/einj-core.c Zaid Alali       2024-12-05  345  	p = ioremap_cache(trigger_paddr, sizeof(*trigger_tab));
37dbc95939d19c3 drivers/acpi/apei/einj-core.c Zaid Alali       2024-12-05  346  	if (!p) {
b2f740baa421525 drivers/acpi/apei/einj.c      Borislav Petkov  2016-05-23  347  		pr_err("Failed to map trigger table!\n");
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  348  		goto out_rel_header;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  349  	}
37dbc95939d19c3 drivers/acpi/apei/einj-core.c Zaid Alali       2024-12-05  350  	trigger_tab = __io_virt(p);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  351  	rc = einj_check_trigger_header(trigger_tab);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  352  	if (rc) {
933ca4e323de777 drivers/acpi/apei/einj.c      Kefeng Wang      2019-10-18  353  		pr_warn(FW_BUG "Invalid trigger error action table.\n");
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  354  		goto out_rel_header;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  355  	}
4c40aed869a200a drivers/acpi/apei/einj.c      Niklas Söderlund 2012-01-09  356  
4c40aed869a200a drivers/acpi/apei/einj.c      Niklas Söderlund 2012-01-09  357  	/* No action structures in the TRIGGER_ERROR table, nothing to do */
4c40aed869a200a drivers/acpi/apei/einj.c      Niklas Söderlund 2012-01-09  358  	if (!trigger_tab->entry_count)
4c40aed869a200a drivers/acpi/apei/einj.c      Niklas Söderlund 2012-01-09  359  		goto out_rel_header;
4c40aed869a200a drivers/acpi/apei/einj.c      Niklas Söderlund 2012-01-09  360  
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  361  	rc = -EIO;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  362  	table_size = trigger_tab->table_size;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  363  	r = request_mem_region(trigger_paddr + sizeof(*trigger_tab),
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  364  			       table_size - sizeof(*trigger_tab),
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  365  			       "APEI EINJ Trigger Table");
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  366  	if (!r) {
b2f740baa421525 drivers/acpi/apei/einj.c      Borislav Petkov  2016-05-23  367  		pr_err("Can not request [mem %#010llx-%#010llx] for Trigger Table Entry\n",
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  368  		       (unsigned long long)trigger_paddr + sizeof(*trigger_tab),
46b91e379f7180b drivers/acpi/apei/einj.c      Bjorn Helgaas    2011-12-08  369  		       (unsigned long long)trigger_paddr + table_size - 1);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  370  		goto out_rel_header;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  371  	}
37dbc95939d19c3 drivers/acpi/apei/einj-core.c Zaid Alali       2024-12-05  372  	iounmap(p);
37dbc95939d19c3 drivers/acpi/apei/einj-core.c Zaid Alali       2024-12-05  373  	p = ioremap_cache(trigger_paddr, table_size);
37dbc95939d19c3 drivers/acpi/apei/einj-core.c Zaid Alali       2024-12-05  374  	if (!p) {
b2f740baa421525 drivers/acpi/apei/einj.c      Borislav Petkov  2016-05-23  375  		pr_err("Failed to map trigger table!\n");
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  376  		goto out_rel_entry;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  377  	}
37dbc95939d19c3 drivers/acpi/apei/einj-core.c Zaid Alali       2024-12-05  378  	trigger_tab = __io_virt(p);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  379  	trigger_entry = (struct acpi_whea_header *)
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  380  		((char *)trigger_tab + sizeof(struct acpi_einj_trigger));
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  381  	apei_resources_init(&trigger_resources);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  382  	apei_exec_ctx_init(&trigger_ctx, einj_ins_type,
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  383  			   ARRAY_SIZE(einj_ins_type),
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  384  			   trigger_entry, trigger_tab->entry_count);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  385  	rc = apei_exec_collect_resources(&trigger_ctx, &trigger_resources);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  386  	if (rc)
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  387  		goto out_fini;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  388  	rc = apei_resources_sub(&trigger_resources, &einj_resources);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  389  	if (rc)
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  390  		goto out_fini;
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  391  	/*
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  392  	 * Some firmware will access target address specified in
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  393  	 * param1 to trigger the error when injecting memory error.
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  394  	 * This will cause resource conflict with regular memory.  So
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  395  	 * remove it from trigger table resources.
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  396  	 */
c5a130325f13b21 drivers/acpi/apei/einj.c      Chen Gong        2013-06-06  397  	if ((param_extension || acpi5) && (type & MEM_ERROR_MASK) && param2) {
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  398  		struct apei_resources addr_resources;
37ea9693869627d drivers/acpi/apei/einj.c      Jay Lu           2022-12-06  399  
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  400  		apei_resources_init(&addr_resources);
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  401  		trigger_param_region = einj_get_trigger_parameter_region(
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  402  			trigger_tab, param1, param2);
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  403  		if (trigger_param_region) {
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  404  			rc = apei_resources_add(&addr_resources,
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  405  				trigger_param_region->address,
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  406  				trigger_param_region->bit_width/8, true);
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  407  			if (rc)
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  408  				goto out_fini;
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  409  			rc = apei_resources_sub(&trigger_resources,
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  410  					&addr_resources);
b4e008dc53a31cb drivers/acpi/apei/einj.c      Xiao, Hui        2011-12-08  411  		}
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  412  		apei_resources_fini(&addr_resources);
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  413  		if (rc)
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  414  			goto out_fini;
fdea163d8c17ba0 drivers/acpi/apei/einj.c      Huang Ying       2011-12-08  415  	}
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  416  	rc = apei_resources_request(&trigger_resources, "APEI EINJ Trigger");
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  417  	if (rc)
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  418  		goto out_fini;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  419  	rc = apei_exec_pre_map_gars(&trigger_ctx);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  420  	if (rc)
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  421  		goto out_release;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  422  
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  423  	rc = apei_exec_run(&trigger_ctx, ACPI_EINJ_TRIGGER_ERROR);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  424  
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  425  	apei_exec_post_unmap_gars(&trigger_ctx);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  426  out_release:
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  427  	apei_resources_release(&trigger_resources);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  428  out_fini:
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  429  	apei_resources_fini(&trigger_resources);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  430  out_rel_entry:
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  431  	release_mem_region(trigger_paddr + sizeof(*trigger_tab),
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  432  			   table_size - sizeof(*trigger_tab));
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  433  out_rel_header:
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  434  	release_mem_region(trigger_paddr, sizeof(*trigger_tab));
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  435  out:
37dbc95939d19c3 drivers/acpi/apei/einj-core.c Zaid Alali       2024-12-05  436  	if (p)
37dbc95939d19c3 drivers/acpi/apei/einj-core.c Zaid Alali       2024-12-05  437  		iounmap(p);
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  438  
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  439  	return rc;
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  440  }
e40213450b53157 drivers/acpi/apei/einj.c      Huang Ying       2010-05-18  441  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/9] ACPI: APEI: EINJ: Fix kernel test robot sparse warning

[Jonathan Cameron]

On Thu,  5 Dec 2024 13:18:48 -0800
Zaid Alali <zaidal@os.amperecomputing.com> wrote:

> This patch fixes the kernel test robot warning reported here:
> https://lore.kernel.org/all/202410241620.oApALow5-lkp@intel.com/
> 
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> ---
>  drivers/acpi/apei/einj-core.c | 41 +++++++++++++++++++++--------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index 04731a5b01fa..74dfb3daba50 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -215,20 +215,22 @@ static void check_vendor_extension(u64 paddr,
>  {
>  	int	offset = v5param->vendor_extension;
>  	struct	vendor_error_type_extension *v;
> +	void __iomem *p;
>  	u32	sbdf;
>  
>  	if (!offset)
>  		return;
> -	v = acpi_os_map_iomem(paddr + offset, sizeof(*v));
> -	if (!v)
> +	p = acpi_os_map_iomem(paddr + offset, sizeof(*v));
> +	if (!p)
>  		return;
> +	v = __io_virt(p);

That's a nasty forced cast. Far as I can tell all this code
should not be assuming it can just cast away the __iomem

I think it should be fixed by using readl() etc or
memcpy_fromio()

Maybe it is safe to just ignore the marking for all current ACPI
platforms, I'm not sure.  This isn't a high performance path so
personally I'd just do it the generic way even if it is not
strictly necessary.

Jonathan



>  	get_oem_vendor_struct(paddr, offset, v);
>  	sbdf = v->pcie_sbdf;
>  	sprintf(vendor_dev, "%x:%x:%x.%x vendor_id=%x device_id=%x rev_id=%x\n",
>  		sbdf >> 24, (sbdf >> 16) & 0xff,
>  		(sbdf >> 11) & 0x1f, (sbdf >> 8) & 0x7,
>  		 v->vendor_id, v->device_id, v->rev_id);
> -	acpi_os_unmap_iomem(v, sizeof(*v));
> +	acpi_os_unmap_iomem(p, sizeof(*v));
>  }
>  
>  static void *einj_get_parameter_address(void)
> @@ -253,9 +255,11 @@ static void *einj_get_parameter_address(void)
>  	}
>  	if (pa_v5) {
>  		struct set_error_type_with_address *v5param;
> +		void __iomem *p;
>  
> -		v5param = acpi_os_map_iomem(pa_v5, sizeof(*v5param));
> -		if (v5param) {
> +		p = acpi_os_map_iomem(pa_v5, sizeof(*v5param));
> +		if (p) {
> +			v5param = __io_virt(p);
>  			acpi5 = 1;
>  			check_vendor_extension(pa_v5, v5param);
>  			return v5param;
> @@ -263,12 +267,14 @@ static void *einj_get_parameter_address(void)
>  	}
>  	if (param_extension && pa_v4) {
>  		struct einj_parameter *v4param;
> +		void __iomem *p;
>  
> -		v4param = acpi_os_map_iomem(pa_v4, sizeof(*v4param));
> -		if (!v4param)
> +		p = acpi_os_map_iomem(pa_v4, sizeof(*v4param));
> +		if (!p)
>  			return NULL;
> +		v4param = __io_virt(p);
>  		if (v4param->reserved1 || v4param->reserved2) {
> -			acpi_os_unmap_iomem(v4param, sizeof(*v4param));
> +			acpi_os_unmap_iomem(p, sizeof(*v4param));
>  			return NULL;
>  		}
>  		return v4param;
> @@ -325,6 +331,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
>  	u32 table_size;
>  	int rc = -EIO;
>  	struct acpi_generic_address *trigger_param_region = NULL;
> +	void __iomem *p;
>  
>  	r = request_mem_region(trigger_paddr, sizeof(*trigger_tab),
>  			       "APEI EINJ Trigger Table");
> @@ -335,11 +342,12 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
>  			    sizeof(*trigger_tab) - 1);
>  		goto out;
>  	}
> -	trigger_tab = ioremap_cache(trigger_paddr, sizeof(*trigger_tab));
> -	if (!trigger_tab) {
> +	p = ioremap_cache(trigger_paddr, sizeof(*trigger_tab));
> +	if (!p) {
>  		pr_err("Failed to map trigger table!\n");
>  		goto out_rel_header;
>  	}
> +	trigger_tab = __io_virt(p);
>  	rc = einj_check_trigger_header(trigger_tab);
>  	if (rc) {
>  		pr_warn(FW_BUG "Invalid trigger error action table.\n");
> @@ -361,12 +369,13 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
>  		       (unsigned long long)trigger_paddr + table_size - 1);
>  		goto out_rel_header;
>  	}
> -	iounmap(trigger_tab);
> -	trigger_tab = ioremap_cache(trigger_paddr, table_size);
> -	if (!trigger_tab) {
> +	iounmap(p);
> +	p = ioremap_cache(trigger_paddr, table_size);
> +	if (!p) {
>  		pr_err("Failed to map trigger table!\n");
>  		goto out_rel_entry;
>  	}
> +	trigger_tab = __io_virt(p);
>  	trigger_entry = (struct acpi_whea_header *)
>  		((char *)trigger_tab + sizeof(struct acpi_einj_trigger));
>  	apei_resources_init(&trigger_resources);
> @@ -424,8 +433,8 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
>  out_rel_header:
>  	release_mem_region(trigger_paddr, sizeof(*trigger_tab));
>  out:
> -	if (trigger_tab)
> -		iounmap(trigger_tab);
> +	if (p)
> +		iounmap(p);
>  
>  	return rc;
>  }
> @@ -860,7 +869,7 @@ static void __exit einj_remove(struct platform_device *pdev)
>  			sizeof(struct set_error_type_with_address) :
>  			sizeof(struct einj_parameter);
>  
> -		acpi_os_unmap_iomem(einj_param, size);
> +		acpi_os_unmap_iomem((void __iomem *)einj_param, size);
>  		if (vendor_errors.size)
>  			acpi_os_unmap_memory(vendor_errors.data, vendor_errors.size);
>  	}

Re: [PATCH v2 3/9] ACPI: APEI: EINJ: Fix kernel test robot sparse warning

[Zaid Alali]

On Tue, Dec 24, 2024 at 03:28:55PM +0000, Jonathan Cameron wrote:
> On Thu,  5 Dec 2024 13:18:48 -0800
> Zaid Alali <zaidal@os.amperecomputing.com> wrote:
> 
> > This patch fixes the kernel test robot warning reported here:
> > https://lore.kernel.org/all/202410241620.oApALow5-lkp@intel.com/
> > 
> > Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> > ---
> >  drivers/acpi/apei/einj-core.c | 41 +++++++++++++++++++++--------------
> >  1 file changed, 25 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> > index 04731a5b01fa..74dfb3daba50 100644
> > --- a/drivers/acpi/apei/einj-core.c
> > +++ b/drivers/acpi/apei/einj-core.c
> > @@ -215,20 +215,22 @@ static void check_vendor_extension(u64 paddr,
> >  {
> >  	int	offset = v5param->vendor_extension;
> >  	struct	vendor_error_type_extension *v;
> > +	void __iomem *p;
> >  	u32	sbdf;
> >  
> >  	if (!offset)
> >  		return;
> > -	v = acpi_os_map_iomem(paddr + offset, sizeof(*v));
> > -	if (!v)
> > +	p = acpi_os_map_iomem(paddr + offset, sizeof(*v));
> > +	if (!p)
> >  		return;
> > +	v = __io_virt(p);
> 
> That's a nasty forced cast. Far as I can tell all this code
> should not be assuming it can just cast away the __iomem
> 
> I think it should be fixed by using readl() etc or
> memcpy_fromio()
> 
> Maybe it is safe to just ignore the marking for all current ACPI
> platforms, I'm not sure.  This isn't a high performance path so
> personally I'd just do it the generic way even if it is not
> strictly necessary.
> 
> Jonathan
> 
Hi Jonathan,

Thank you for taking the time to review the patches!
These iomem warnings were there before the new patches, since the code has to
access iomem location for read/write to communicate with firmware.

I agree with you on ignoring the marking for ACPI platforms. I can also change
the code to use memcpy_fromio() to create local copies of iomem locations, and
write them back later instead. However, it will add a little more
complexity to the code, because iomem mapping and subsequent reads/writes
happen in different functions.

please let me know if you have any more thoughts on this.

Thanks,
Zaid Alali



> 
> 
> >  	get_oem_vendor_struct(paddr, offset, v);
> >  	sbdf = v->pcie_sbdf;
> >  	sprintf(vendor_dev, "%x:%x:%x.%x vendor_id=%x device_id=%x rev_id=%x\n",
> >  		sbdf >> 24, (sbdf >> 16) & 0xff,
> >  		(sbdf >> 11) & 0x1f, (sbdf >> 8) & 0x7,
> >  		 v->vendor_id, v->device_id, v->rev_id);
> > -	acpi_os_unmap_iomem(v, sizeof(*v));
> > +	acpi_os_unmap_iomem(p, sizeof(*v));
> >  }
> >  
> >  static void *einj_get_parameter_address(void)
> > @@ -253,9 +255,11 @@ static void *einj_get_parameter_address(void)
> >  	}
> >  	if (pa_v5) {
> >  		struct set_error_type_with_address *v5param;
> > +		void __iomem *p;
> >  
> > -		v5param = acpi_os_map_iomem(pa_v5, sizeof(*v5param));
> > -		if (v5param) {
> > +		p = acpi_os_map_iomem(pa_v5, sizeof(*v5param));
> > +		if (p) {
> > +			v5param = __io_virt(p);
> >  			acpi5 = 1;
> >  			check_vendor_extension(pa_v5, v5param);
> >  			return v5param;
> > @@ -263,12 +267,14 @@ static void *einj_get_parameter_address(void)
> >  	}
> >  	if (param_extension && pa_v4) {
> >  		struct einj_parameter *v4param;
> > +		void __iomem *p;
> >  
> > -		v4param = acpi_os_map_iomem(pa_v4, sizeof(*v4param));
> > -		if (!v4param)
> > +		p = acpi_os_map_iomem(pa_v4, sizeof(*v4param));
> > +		if (!p)
> >  			return NULL;
> > +		v4param = __io_virt(p);
> >  		if (v4param->reserved1 || v4param->reserved2) {
> > -			acpi_os_unmap_iomem(v4param, sizeof(*v4param));
> > +			acpi_os_unmap_iomem(p, sizeof(*v4param));
> >  			return NULL;
> >  		}
> >  		return v4param;
> > @@ -325,6 +331,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
> >  	u32 table_size;
> >  	int rc = -EIO;
> >  	struct acpi_generic_address *trigger_param_region = NULL;
> > +	void __iomem *p;
> >  
> >  	r = request_mem_region(trigger_paddr, sizeof(*trigger_tab),
> >  			       "APEI EINJ Trigger Table");
> > @@ -335,11 +342,12 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
> >  			    sizeof(*trigger_tab) - 1);
> >  		goto out;
> >  	}
> > -	trigger_tab = ioremap_cache(trigger_paddr, sizeof(*trigger_tab));
> > -	if (!trigger_tab) {
> > +	p = ioremap_cache(trigger_paddr, sizeof(*trigger_tab));
> > +	if (!p) {
> >  		pr_err("Failed to map trigger table!\n");
> >  		goto out_rel_header;
> >  	}
> > +	trigger_tab = __io_virt(p);
> >  	rc = einj_check_trigger_header(trigger_tab);
> >  	if (rc) {
> >  		pr_warn(FW_BUG "Invalid trigger error action table.\n");
> > @@ -361,12 +369,13 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
> >  		       (unsigned long long)trigger_paddr + table_size - 1);
> >  		goto out_rel_header;
> >  	}
> > -	iounmap(trigger_tab);
> > -	trigger_tab = ioremap_cache(trigger_paddr, table_size);
> > -	if (!trigger_tab) {
> > +	iounmap(p);
> > +	p = ioremap_cache(trigger_paddr, table_size);
> > +	if (!p) {
> >  		pr_err("Failed to map trigger table!\n");
> >  		goto out_rel_entry;
> >  	}
> > +	trigger_tab = __io_virt(p);
> >  	trigger_entry = (struct acpi_whea_header *)
> >  		((char *)trigger_tab + sizeof(struct acpi_einj_trigger));
> >  	apei_resources_init(&trigger_resources);
> > @@ -424,8 +433,8 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
> >  out_rel_header:
> >  	release_mem_region(trigger_paddr, sizeof(*trigger_tab));
> >  out:
> > -	if (trigger_tab)
> > -		iounmap(trigger_tab);
> > +	if (p)
> > +		iounmap(p);
> >  
> >  	return rc;
> >  }
> > @@ -860,7 +869,7 @@ static void __exit einj_remove(struct platform_device *pdev)
> >  			sizeof(struct set_error_type_with_address) :
> >  			sizeof(struct einj_parameter);
> >  
> > -		acpi_os_unmap_iomem(einj_param, size);
> > +		acpi_os_unmap_iomem((void __iomem *)einj_param, size);
> >  		if (vendor_errors.size)
> >  			acpi_os_unmap_memory(vendor_errors.data, vendor_errors.size);
> >  	}
> 

[PATCH v2 4/9] ACPI: APEI: EINJ: Remove redundant calls to einj_get_available_error_type

[Zaid Alali]

A single call to einj_get_available_error_type in init function is
sufficient to save the return value in a global variable to be used
later in various places in the code. This commit does not introduce
any functional changes, but only removing unnecessary redundant
function calls.

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/einj-core.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index 74dfb3daba50..a6b648361d96 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -83,6 +83,8 @@ static struct debugfs_blob_wrapper vendor_blob;
 static struct debugfs_blob_wrapper vendor_errors;
 static char vendor_dev[64];
 
+static u32 available_error_type;
+
 /*
  * Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
  * EINJ table through an unpublished extension. Use with caution as
@@ -657,14 +659,9 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
 
 static int available_error_type_show(struct seq_file *m, void *v)
 {
-	int rc;
-	u32 error_type = 0;
 
-	rc = einj_get_available_error_type(&error_type);
-	if (rc)
-		return rc;
 	for (int pos = 0; pos < ARRAY_SIZE(einj_error_type_string); pos++)
-		if (error_type & einj_error_type_string[pos].mask)
+		if (available_error_type & einj_error_type_string[pos].mask)
 			seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
 				   einj_error_type_string[pos].str);
 
@@ -687,8 +684,7 @@ bool einj_is_cxl_error_type(u64 type)
 
 int einj_validate_error_type(u64 type)
 {
-	u32 tval, vendor, available_error_type = 0;
-	int rc;
+	u32 tval, vendor;
 
 	/* Only low 32 bits for error type are valid */
 	if (type & GENMASK_ULL(63, 32))
@@ -704,13 +700,9 @@ int einj_validate_error_type(u64 type)
 	/* Only one error type can be specified */
 	if (tval & (tval - 1))
 		return -EINVAL;
-	if (!vendor) {
-		rc = einj_get_available_error_type(&available_error_type);
-		if (rc)
-			return rc;
+	if (!vendor)
 		if (!(type & available_error_type))
 			return -EINVAL;
-	}
 
 	return 0;
 }
@@ -786,6 +778,10 @@ static int __init einj_probe(struct platform_device *pdev)
 		goto err_put_table;
 	}
 
+	rc = einj_get_available_error_type(&available_error_type);
+	if (rc)
+		return rc;
+
 	rc = -ENOMEM;
 	einj_debug_dir = debugfs_create_dir("einj", apei_get_debugfs_dir());
 
-- 
2.34.1

Re: [PATCH v2 4/9] ACPI: APEI: EINJ: Remove redundant calls to einj_get_available_error_type

[Jonathan Cameron]

On Thu,  5 Dec 2024 13:18:49 -0800
Zaid Alali <zaidal@os.amperecomputing.com> wrote:

> A single call to einj_get_available_error_type in init function is
> sufficient to save the return value in a global variable to be used
> later in various places in the code. This commit does not introduce
> any functional changes, but only removing unnecessary redundant
> function calls.
> 
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
ok. I guess this is ok given how many globals are in use in here
already.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

[PATCH v2 5/9] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities

[Zaid Alali]

Enable the driver to show all supported error injections for EINJ
and EINJv2 at the same time. EINJv2 capabilities can be discovered
by checking the return value of get_error_type, where bit 30 set
indicates EINJv2 support.

This update makes the driver parse the error_type as a string to
avoid any ambiguity with EINJv1 and EINJv2 error types that has
the same value, where EINJv2 error types has the prefix "V2_".

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/apei-internal.h |  2 +-
 drivers/acpi/apei/einj-core.c     | 70 ++++++++++++++++++++++++-------
 drivers/acpi/apei/einj-cxl.c      |  2 +-
 3 files changed, 56 insertions(+), 18 deletions(-)

diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
index cd2766c69d78..9a3dbaeed39a 100644
--- a/drivers/acpi/apei/apei-internal.h
+++ b/drivers/acpi/apei/apei-internal.h
@@ -131,7 +131,7 @@ 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_get_available_error_type(u32 *type, int version);
 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,
diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index a6b648361d96..2c57e25252ac 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -33,6 +33,7 @@
 #define SLEEP_UNIT_MAX		5000			/* 5ms */
 /* Firmware should respond within 1 seconds */
 #define FIRMWARE_TIMEOUT	(1 * USEC_PER_SEC)
+#define ACPI65_EINJV2_SUPP	BIT(30)
 #define ACPI5_VENDOR_BIT	BIT(31)
 #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
 				ACPI_EINJ_MEMORY_UNCORRECTABLE | \
@@ -84,6 +85,7 @@ static struct debugfs_blob_wrapper vendor_errors;
 static char vendor_dev[64];
 
 static u32 available_error_type;
+static u32 available_error_type_v2;
 
 /*
  * Some BIOSes allow parameters to the SET_ERROR_TYPE entries in the
@@ -159,13 +161,13 @@ static void einj_exec_ctx_init(struct apei_exec_context *ctx)
 			   EINJ_TAB_ENTRY(einj_tab), einj_tab->entries);
 }
 
-static int __einj_get_available_error_type(u32 *type)
+static int __einj_get_available_error_type(u32 *type, int version)
 {
 	struct apei_exec_context ctx;
 	int rc;
 
 	einj_exec_ctx_init(&ctx);
-	rc = apei_exec_run(&ctx, ACPI_EINJ_GET_ERROR_TYPE);
+	rc = apei_exec_run(&ctx, version);
 	if (rc)
 		return rc;
 	*type = apei_exec_ctx_get_output(&ctx);
@@ -174,12 +176,12 @@ static int __einj_get_available_error_type(u32 *type)
 }
 
 /* Get error injection capabilities of the platform */
-int einj_get_available_error_type(u32 *type)
+int einj_get_available_error_type(u32 *type, int version)
 {
 	int rc;
 
 	mutex_lock(&einj_mutex);
-	rc = __einj_get_available_error_type(type);
+	rc = __einj_get_available_error_type(type, version);
 	mutex_unlock(&einj_mutex);
 
 	return rc;
@@ -641,6 +643,7 @@ static u64 error_param2;
 static u64 error_param3;
 static u64 error_param4;
 static struct dentry *einj_debug_dir;
+static char *einj_buf;
 static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
 	{ BIT(0), "Processor Correctable" },
 	{ BIT(1), "Processor Uncorrectable non-fatal" },
@@ -656,6 +659,11 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
 	{ BIT(11), "Platform Uncorrectable fatal"},
 	{ BIT(31), "Vendor Defined Error Types" },
 };
+static struct { u32 mask; const char *str; } const einjv2_error_type_string[] = {
+	{ BIT(0), "EINJV2 Processor Error" },
+	{ BIT(1), "EINJV2 Memory Error" },
+	{ BIT(2), "EINJV2 PCI Express Error" },
+};
 
 static int available_error_type_show(struct seq_file *m, void *v)
 {
@@ -663,18 +671,22 @@ static int available_error_type_show(struct seq_file *m, void *v)
 	for (int pos = 0; pos < ARRAY_SIZE(einj_error_type_string); pos++)
 		if (available_error_type & einj_error_type_string[pos].mask)
 			seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
-				   einj_error_type_string[pos].str);
-
+				einj_error_type_string[pos].str);
+	if (available_error_type & ACPI65_EINJV2_SUPP) {
+		for (int pos = 0; pos < ARRAY_SIZE(einjv2_error_type_string); pos++)
+			if (available_error_type_v2 & einjv2_error_type_string[pos].mask)
+				seq_printf(m, "V2_0x%08x\t%s\n", einjv2_error_type_string[pos].mask,
+					einjv2_error_type_string[pos].str);
+	}
 	return 0;
 }
 
 DEFINE_SHOW_ATTRIBUTE(available_error_type);
 
-static int error_type_get(void *data, u64 *val)
+static ssize_t error_type_get(struct file *file, char __user *buf,
+				size_t count, loff_t *ppos)
 {
-	*val = error_type;
-
-	return 0;
+	return simple_read_from_buffer(buf, count, ppos, einj_buf, strlen(einj_buf));
 }
 
 bool einj_is_cxl_error_type(u64 type)
@@ -701,15 +713,28 @@ int einj_validate_error_type(u64 type)
 	if (tval & (tval - 1))
 		return -EINVAL;
 	if (!vendor)
-		if (!(type & available_error_type))
+		if (!(type & (available_error_type | available_error_type_v2)))
 			return -EINVAL;
 
 	return 0;
 }
 
-static int error_type_set(void *data, u64 val)
+static ssize_t error_type_set(struct file *file, const char __user *buf,
+				size_t count, loff_t *ppos)
 {
 	int rc;
+	u64 val;
+
+	memset(einj_buf, 0, sizeof(einj_buf));
+	if (copy_from_user(einj_buf, buf, count))
+		return -EFAULT;
+
+	if (strncmp(einj_buf, "V2_", 3) == 0) {
+		if (!sscanf(einj_buf, "V2_%llx", &val))
+			return -EINVAL;
+	} else
+		if (!sscanf(einj_buf, "%llx", &val))
+			return -EINVAL;
 
 	rc = einj_validate_error_type(val);
 	if (rc)
@@ -717,11 +742,13 @@ static int error_type_set(void *data, u64 val)
 
 	error_type = val;
 
-	return 0;
+	return count;
 }
 
-DEFINE_DEBUGFS_ATTRIBUTE(error_type_fops, error_type_get, error_type_set,
-			 "0x%llx\n");
+static const struct file_operations error_type_fops = {
+	.read		= error_type_get,
+	.write		= error_type_set,
+};
 
 static int error_inject_set(void *data, u64 val)
 {
@@ -778,9 +805,14 @@ static int __init einj_probe(struct platform_device *pdev)
 		goto err_put_table;
 	}
 
-	rc = einj_get_available_error_type(&available_error_type);
+	rc = einj_get_available_error_type(&available_error_type, ACPI_EINJ_GET_ERROR_TYPE);
 	if (rc)
 		return rc;
+	if (available_error_type & ACPI65_EINJV2_SUPP) {
+		rc = einj_get_available_error_type(&available_error_type_v2, ACPI_EINJV2_GET_ERROR_TYPE);
+		if (rc)
+			return rc;
+	}
 
 	rc = -ENOMEM;
 	einj_debug_dir = debugfs_create_dir("einj", apei_get_debugfs_dir());
@@ -828,6 +860,11 @@ static int __init einj_probe(struct platform_device *pdev)
 				   einj_debug_dir, &notrigger);
 	}
 
+	einj_buf = kzalloc(32, GFP_KERNEL);
+	if (!einj_buf) {
+		goto err_release;
+	}
+
 	if (vendor_dev[0]) {
 		vendor_blob.data = vendor_dev;
 		vendor_blob.size = strlen(vendor_dev);
@@ -875,6 +912,7 @@ static void __exit einj_remove(struct platform_device *pdev)
 	apei_resources_fini(&einj_resources);
 	debugfs_remove_recursive(einj_debug_dir);
 	acpi_put_table((struct acpi_table_header *)einj_tab);
+	kfree(einj_buf);
 }
 
 static struct platform_device *einj_dev;
diff --git a/drivers/acpi/apei/einj-cxl.c b/drivers/acpi/apei/einj-cxl.c
index 78da9ae543a2..e70a416ec925 100644
--- a/drivers/acpi/apei/einj-cxl.c
+++ b/drivers/acpi/apei/einj-cxl.c
@@ -30,7 +30,7 @@ int einj_cxl_available_error_type_show(struct seq_file *m, void *v)
 	int cxl_err, rc;
 	u32 available_error_type = 0;
 
-	rc = einj_get_available_error_type(&available_error_type);
+	rc = einj_get_available_error_type(&available_error_type, ACPI_EINJ_GET_ERROR_TYPE);
 	if (rc)
 		return rc;
 
-- 
2.34.1

Re: [PATCH v2 5/9] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities

[kernel test robot]

Hi Zaid,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge linus/master v6.13-rc1 next-20241205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zaid-Alali/ACPICA-Update-values-to-hex-to-follow-ACPI-specs/20241206-052420
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20241205211854.43215-6-zaidal%40os.amperecomputing.com
patch subject: [PATCH v2 5/9] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities
config: arm64-randconfig-002-20241206 (https://download.01.org/0day-ci/archive/20241206/202412061056.fk2xNw7W-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241206/202412061056.fk2xNw7W-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412061056.fk2xNw7W-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/acpi/apei/einj-core.c: In function 'error_type_set':
>> drivers/acpi/apei/einj-core.c:728:35: warning: argument to 'sizeof' in 'memset' call is the same expression as the destination; did you mean to provide an explicit length? [-Wsizeof-pointer-memaccess]
     728 |         memset(einj_buf, 0, sizeof(einj_buf));
         |                                   ^


vim +728 drivers/acpi/apei/einj-core.c

   721	
   722	static ssize_t error_type_set(struct file *file, const char __user *buf,
   723					size_t count, loff_t *ppos)
   724	{
   725		int rc;
   726		u64 val;
   727	
 > 728		memset(einj_buf, 0, sizeof(einj_buf));
   729		if (copy_from_user(einj_buf, buf, count))
   730			return -EFAULT;
   731	
   732		if (strncmp(einj_buf, "V2_", 3) == 0) {
   733			if (!sscanf(einj_buf, "V2_%llx", &val))
   734				return -EINVAL;
   735		} else
   736			if (!sscanf(einj_buf, "%llx", &val))
   737				return -EINVAL;
   738	
   739		rc = einj_validate_error_type(val);
   740		if (rc)
   741			return rc;
   742	
   743		error_type = val;
   744	
   745		return count;
   746	}
   747	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 5/9] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities

[kernel test robot]

Hi Zaid,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge linus/master v6.13-rc1 next-20241205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zaid-Alali/ACPICA-Update-values-to-hex-to-follow-ACPI-specs/20241206-052420
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20241205211854.43215-6-zaidal%40os.amperecomputing.com
patch subject: [PATCH v2 5/9] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities
config: i386-buildonly-randconfig-005-20241206 (https://download.01.org/0day-ci/archive/20241206/202412061210.H6R0JvCL-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241206/202412061210.H6R0JvCL-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412061210.H6R0JvCL-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/acpi/apei/einj-core.c:23:
   In file included from include/linux/mm.h:2223:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   drivers/acpi/apei/einj-core.c:342:6: warning: variable 'p' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
     342 |         if (!r) {
         |             ^~
   drivers/acpi/apei/einj-core.c:440:6: note: uninitialized use occurs here
     440 |         if (p)
         |             ^
   drivers/acpi/apei/einj-core.c:342:2: note: remove the 'if' if its condition is always false
     342 |         if (!r) {
         |         ^~~~~~~~~
     343 |                 pr_err("Can not request [mem %#010llx-%#010llx] for Trigger table\n",
         |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     344 |                        (unsigned long long)trigger_paddr,
         |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     345 |                        (unsigned long long)trigger_paddr +
         |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     346 |                             sizeof(*trigger_tab) - 1);
         |                             ~~~~~~~~~~~~~~~~~~~~~~~~~~
     347 |                 goto out;
         |                 ~~~~~~~~~
     348 |         }
         |         ~
   drivers/acpi/apei/einj-core.c:338:17: note: initialize the variable 'p' to silence this warning
     338 |         void __iomem *p;
         |                        ^
         |                         = NULL
>> drivers/acpi/apei/einj-core.c:728:29: warning: 'memset' call operates on objects of type 'char' while the size is based on a different type 'char *' [-Wsizeof-pointer-memaccess]
     728 |         memset(einj_buf, 0, sizeof(einj_buf));
         |                ~~~~~~~~            ^~~~~~~~
   drivers/acpi/apei/einj-core.c:728:29: note: did you mean to provide an explicit length?
     728 |         memset(einj_buf, 0, sizeof(einj_buf));
         |                                    ^~~~~~~~
   3 warnings generated.


vim +728 drivers/acpi/apei/einj-core.c

   721	
   722	static ssize_t error_type_set(struct file *file, const char __user *buf,
   723					size_t count, loff_t *ppos)
   724	{
   725		int rc;
   726		u64 val;
   727	
 > 728		memset(einj_buf, 0, sizeof(einj_buf));
   729		if (copy_from_user(einj_buf, buf, count))
   730			return -EFAULT;
   731	
   732		if (strncmp(einj_buf, "V2_", 3) == 0) {
   733			if (!sscanf(einj_buf, "V2_%llx", &val))
   734				return -EINVAL;
   735		} else
   736			if (!sscanf(einj_buf, "%llx", &val))
   737				return -EINVAL;
   738	
   739		rc = einj_validate_error_type(val);
   740		if (rc)
   741			return rc;
   742	
   743		error_type = val;
   744	
   745		return count;
   746	}
   747	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 5/9] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities

[kernel test robot]

Hi Zaid,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge linus/master v6.13-rc1 next-20241206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zaid-Alali/ACPICA-Update-values-to-hex-to-follow-ACPI-specs/20241206-052420
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20241205211854.43215-6-zaidal%40os.amperecomputing.com
patch subject: [PATCH v2 5/9] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities
config: x86_64-randconfig-101-20241206 (https://download.01.org/0day-ci/archive/20241207/202412070418.9pHXTR91-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412070418.9pHXTR91-lkp@intel.com/

cocci warnings: (new ones prefixed by >>)
>> drivers/acpi/apei/einj-core.c:728:21-27: ERROR: application of sizeof to pointer

vim +728 drivers/acpi/apei/einj-core.c

   721	
   722	static ssize_t error_type_set(struct file *file, const char __user *buf,
   723					size_t count, loff_t *ppos)
   724	{
   725		int rc;
   726		u64 val;
   727	
 > 728		memset(einj_buf, 0, sizeof(einj_buf));
   729		if (copy_from_user(einj_buf, buf, count))
   730			return -EFAULT;
   731	
   732		if (strncmp(einj_buf, "V2_", 3) == 0) {
   733			if (!sscanf(einj_buf, "V2_%llx", &val))
   734				return -EINVAL;
   735		} else
   736			if (!sscanf(einj_buf, "%llx", &val))
   737				return -EINVAL;
   738	
   739		rc = einj_validate_error_type(val);
   740		if (rc)
   741			return rc;
   742	
   743		error_type = val;
   744	
   745		return count;
   746	}
   747	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 5/9] ACPI: APEI: EINJ: Enable the discovery of EINJv2 capabilities

[Jonathan Cameron]

On Thu,  5 Dec 2024 13:18:50 -0800
Zaid Alali <zaidal@os.amperecomputing.com> wrote:

> Enable the driver to show all supported error injections for EINJ
> and EINJv2 at the same time. EINJv2 capabilities can be discovered
> by checking the return value of get_error_type, where bit 30 set
> indicates EINJv2 support.
> 
> This update makes the driver parse the error_type as a string to
> avoid any ambiguity with EINJv1 and EINJv2 error types that has
> the same value, where EINJv2 error types has the prefix "V2_".
> 
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
Hi Zaid,

Some comments inline.

Thanks,

Jonathan

> ---
>  drivers/acpi/apei/apei-internal.h |  2 +-
>  drivers/acpi/apei/einj-core.c     | 70 ++++++++++++++++++++++++-------
>  drivers/acpi/apei/einj-cxl.c      |  2 +-
>  3 files changed, 56 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
> index cd2766c69d78..9a3dbaeed39a 100644
> --- a/drivers/acpi/apei/apei-internal.h
> +++ b/drivers/acpi/apei/apei-internal.h
> @@ -131,7 +131,7 @@ 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_get_available_error_type(u32 *type, int version);

As below. I'm not sure version is a good name for this as it
is not the version number at all.

>  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,
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index a6b648361d96..2c57e25252ac 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c

> @@ -641,6 +643,7 @@ static u64 error_param2;
>  static u64 error_param3;
>  static u64 error_param4;
>  static struct dentry *einj_debug_dir;
> +static char *einj_buf;
>  static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
>  	{ BIT(0), "Processor Correctable" },
>  	{ BIT(1), "Processor Uncorrectable non-fatal" },
> @@ -656,6 +659,11 @@ static struct { u32 mask; const char *str; } const einj_error_type_string[] = {
>  	{ BIT(11), "Platform Uncorrectable fatal"},
>  	{ BIT(31), "Vendor Defined Error Types" },
>  };

blank line here.

> +static struct { u32 mask; const char *str; } const einjv2_error_type_string[] = {
> +	{ BIT(0), "EINJV2 Processor Error" },
> +	{ BIT(1), "EINJV2 Memory Error" },
> +	{ BIT(2), "EINJV2 PCI Express Error" },
> +};
>  
>  static int available_error_type_show(struct seq_file *m, void *v)
>  {
> @@ -663,18 +671,22 @@ static int available_error_type_show(struct seq_file *m, void *v)
>  	for (int pos = 0; pos < ARRAY_SIZE(einj_error_type_string); pos++)
>  		if (available_error_type & einj_error_type_string[pos].mask)
>  			seq_printf(m, "0x%08x\t%s\n", einj_error_type_string[pos].mask,
> -				   einj_error_type_string[pos].str);
> -
> +				einj_error_type_string[pos].str);

Fix this up and check for any other accidental changes like this. They just
make the patches harder to review.


> +	if (available_error_type & ACPI65_EINJV2_SUPP) {
> +		for (int pos = 0; pos < ARRAY_SIZE(einjv2_error_type_string); pos++)
> +			if (available_error_type_v2 & einjv2_error_type_string[pos].mask)
> +				seq_printf(m, "V2_0x%08x\t%s\n", einjv2_error_type_string[pos].mask,

Long line. I'd wrap before last parameter for readability.

> +					einjv2_error_type_string[pos].str);

Align after closing bracket.

> +	}
>  	return 0;
>  }
>  
>  DEFINE_SHOW_ATTRIBUTE(available_error_type);
>  
> -static int error_type_get(void *data, u64 *val)
> +static ssize_t error_type_get(struct file *file, char __user *buf,
> +				size_t count, loff_t *ppos)
>  {
> -	*val = error_type;
> -
> -	return 0;
> +	return simple_read_from_buffer(buf, count, ppos, einj_buf, strlen(einj_buf));
>  }
>  
>  bool einj_is_cxl_error_type(u64 type)
> @@ -701,15 +713,28 @@ int einj_validate_error_type(u64 type)
>  	if (tval & (tval - 1))
>  		return -EINVAL;
>  	if (!vendor)
> -		if (!(type & available_error_type))
> +		if (!(type & (available_error_type | available_error_type_v2)))

Maybe a comment on this. Not obvious to me which the | makes sense.

>  			return -EINVAL;
>  
>  	return 0;
>  }
>  
> -static int error_type_set(void *data, u64 val)
> +static ssize_t error_type_set(struct file *file, const char __user *buf,
> +				size_t count, loff_t *ppos)
>  {
>  	int rc;
> +	u64 val;
> +
> +	memset(einj_buf, 0, sizeof(einj_buf));

sizeof the pointer?

> +	if (copy_from_user(einj_buf, buf, count))

What stops this being bigger than einj_buf?  Perhaps
best to check that.

> +		return -EFAULT;
> +
> +	if (strncmp(einj_buf, "V2_", 3) == 0) {
> +		if (!sscanf(einj_buf, "V2_%llx", &val))
> +			return -EINVAL;
> +	} else
	} else {

Both because you kernel style is same bracketing for all legs
of if / else and because what follows is multi line.

> +		if (!sscanf(einj_buf, "%llx", &val))
> +			return -EINVAL;
>  
>  	rc = einj_validate_error_type(val);
>  	if (rc)
> @@ -717,11 +742,13 @@ static int error_type_set(void *data, u64 val)
>  
>  	error_type = val;
>  
> -	return 0;
> +	return count;
>  }

>  static int error_inject_set(void *data, u64 val)
>  {
> @@ -778,9 +805,14 @@ static int __init einj_probe(struct platform_device *pdev)
>  		goto err_put_table;
>  	}
>  
> -	rc = einj_get_available_error_type(&available_error_type);
> +	rc = einj_get_available_error_type(&available_error_type, ACPI_EINJ_GET_ERROR_TYPE);
>  	if (rc)
>  		return rc;
> +	if (available_error_type & ACPI65_EINJV2_SUPP) {
> +		rc = einj_get_available_error_type(&available_error_type_v2, ACPI_EINJV2_GET_ERROR_TYPE);
The parameter is called version. I'd expect that to just be 1 or 2 giving naming.
Maybe a different parameter name would be less confusing?

> +		if (rc)
> +			return rc;
> +	}
>  
>  	rc = -ENOMEM;
>  	einj_debug_dir = debugfs_create_dir("einj", apei_get_debugfs_dir());
> @@ -828,6 +860,11 @@ static int __init einj_probe(struct platform_device *pdev)
>  				   einj_debug_dir, &notrigger);
>  	}
>  
> +	einj_buf = kzalloc(32, GFP_KERNEL);

Why 32? Can we base that on a define or similar?
Given it is global anyway and fairly small, why not just declare
a static array and skip the allocation and free?


> +	if (!einj_buf) {
> +		goto err_release;

Not sure on local style, but general kernel style is no brackets for single line if block.

> +	}
> +
>  	if (vendor_dev[0]) {
>  		vendor_blob.data = vendor_dev;
>  		vendor_blob.size = strlen(vendor_dev);
> @@ -875,6 +912,7 @@ static void __exit einj_remove(struct platform_device *pdev)
>  	apei_resources_fini(&einj_resources);
>  	debugfs_remove_recursive(einj_debug_dir);
>  	acpi_put_table((struct acpi_table_header *)einj_tab);
> +	kfree(einj_buf);
>  }
>  

>  	if (rc)
>  		return rc;
>  

[PATCH v2 6/9] ACPI: APEI: EINJ: Add einjv2 extension struct

[Zaid Alali]

Add einjv2 extension struct and EINJv2 error types to prepare
the driver for EINJv2 support. ACPI specifications(1) enables
EINJv2 by extending set_error_type_with_address struct.

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/einj-core.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index 2c57e25252ac..039d36472342 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -50,6 +50,28 @@
  */
 static int acpi5;
 
+struct syndrome_array {
+	union {
+		u32	acpi_id;
+		u32	device_id;
+		u32	pcie_sbdf;
+		u8	vendor_id[16];
+	} comp_id;
+	union {
+		u32	proc_synd;
+		u32	mem_synd;
+		u32	pcie_synd;
+		u8	vendor_synd[16];
+	} comp_synd;
+};
+
+struct einjv2_extension_struct {
+	u32 length;
+	u16 revision;
+	u16 component_arr_count;
+	struct syndrome_array component_arr[];
+};
+
 struct set_error_type_with_address {
 	u32	type;
 	u32	vendor_extension;
@@ -58,6 +80,7 @@ struct set_error_type_with_address {
 	u64	memory_address;
 	u64	memory_address_range;
 	u32	pcie_sbdf;
+	struct	einjv2_extension_struct einjv2_struct;
 };
 enum {
 	SETWA_FLAGS_APICID = 1,
-- 
2.34.1

Re: [PATCH v2 6/9] ACPI: APEI: EINJ: Add einjv2 extension struct

[Jonathan Cameron]

On Thu,  5 Dec 2024 13:18:51 -0800
Zaid Alali <zaidal@os.amperecomputing.com> wrote:

> Add einjv2 extension struct and EINJv2 error types to prepare
> the driver for EINJv2 support. ACPI specifications(1) enables

Reference got lost? 

> EINJv2 by extending set_error_type_with_address struct.
> 
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> ---
>  drivers/acpi/apei/einj-core.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index 2c57e25252ac..039d36472342 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c

> +struct einjv2_extension_struct {
> +	u32 length;
> +	u16 revision;
> +	u16 component_arr_count;
> +	struct syndrome_array component_arr[];

__counted_by marking seems appropriate here.

> +};
> +
>  struct set_error_type_with_address {
>  	u32	type;
>  	u32	vendor_extension;
> @@ -58,6 +80,7 @@ struct set_error_type_with_address {
>  	u64	memory_address;
>  	u64	memory_address_range;
>  	u32	pcie_sbdf;
> +	struct	einjv2_extension_struct einjv2_struct;
>  };
>  enum {
>  	SETWA_FLAGS_APICID = 1,

Re: [PATCH v2 6/9] ACPI: APEI: EINJ: Add einjv2 extension struct

[Zaid Alali]

On Tue, Dec 24, 2024 at 03:51:13PM +0000, Jonathan Cameron wrote:
> On Thu,  5 Dec 2024 13:18:51 -0800
> Zaid Alali <zaidal@os.amperecomputing.com> wrote:
> 
> > Add einjv2 extension struct and EINJv2 error types to prepare
> > the driver for EINJv2 support. ACPI specifications(1) enables
> 
> Reference got lost? 
>
I will fix this.
 
> > EINJv2 by extending set_error_type_with_address struct.
> > 
> > Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> > ---
> >  drivers/acpi/apei/einj-core.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> > index 2c57e25252ac..039d36472342 100644
> > --- a/drivers/acpi/apei/einj-core.c
> > +++ b/drivers/acpi/apei/einj-core.c
> 
> > +struct einjv2_extension_struct {
> > +	u32 length;
> > +	u16 revision;
> > +	u16 component_arr_count;
> > +	struct syndrome_array component_arr[];
> 
> __counted_by marking seems appropriate here.

I understand __counted_by may seem appropriate here, however, component_arr_count
is a value set be the user and does not represent syndrome_array size.
The size of syndrome_array will be calculated in Patch 8/9.

> 
> > +};
> > +
> >  struct set_error_type_with_address {
> >  	u32	type;
> >  	u32	vendor_extension;
> > @@ -58,6 +80,7 @@ struct set_error_type_with_address {
> >  	u64	memory_address;
> >  	u64	memory_address_range;
> >  	u32	pcie_sbdf;
> > +	struct	einjv2_extension_struct einjv2_struct;
> >  };
> >  enum {
> >  	SETWA_FLAGS_APICID = 1,
> 

[PATCH v2 7/9] ACPI: APEI: EINJ: Add debugfs files for EINJv2 support

[Zaid Alali]

Create a debugfs blob file to be used for reading the user
input for the component array. EINJv2 enables users to inject
errors to multiple components/devices at the same time using
component array.

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/einj-core.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index 039d36472342..1961f140ada8 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -33,6 +33,7 @@
 #define SLEEP_UNIT_MAX		5000			/* 5ms */
 /* Firmware should respond within 1 seconds */
 #define FIRMWARE_TIMEOUT	(1 * USEC_PER_SEC)
+#define COMP_ARR_SIZE		1024
 #define ACPI65_EINJV2_SUPP	BIT(30)
 #define ACPI5_VENDOR_BIT	BIT(31)
 #define MEM_ERROR_MASK		(ACPI_EINJ_MEMORY_CORRECTABLE | \
@@ -107,6 +108,9 @@ static struct debugfs_blob_wrapper vendor_blob;
 static struct debugfs_blob_wrapper vendor_errors;
 static char vendor_dev[64];
 
+static struct debugfs_blob_wrapper einjv2_component_arr;
+static u64 component_count;
+static void *user_input;
 static u32 available_error_type;
 static u32 available_error_type_v2;
 
@@ -881,6 +885,19 @@ static int __init einj_probe(struct platform_device *pdev)
 				   &error_param4);
 		debugfs_create_x32("notrigger", S_IRUSR | S_IWUSR,
 				   einj_debug_dir, &notrigger);
+		if (available_error_type & ACPI65_EINJV2_SUPP) {
+			debugfs_create_x64("einjv2_component_count", S_IRUSR | S_IWUSR,
+					einj_debug_dir,	&component_count);
+			user_input = kzalloc(COMP_ARR_SIZE, GFP_KERNEL);
+			if (!user_input) {
+				rc = -ENOMEM;
+				goto err_release;
+			}
+			einjv2_component_arr.data = user_input;
+			einjv2_component_arr.size = COMP_ARR_SIZE;
+			debugfs_create_blob("einjv2_component_array", S_IRUSR | S_IWUSR,
+					einj_debug_dir, &einjv2_component_arr);
+		}
 	}
 
 	einj_buf = kzalloc(32, GFP_KERNEL);
@@ -936,6 +953,7 @@ static void __exit einj_remove(struct platform_device *pdev)
 	debugfs_remove_recursive(einj_debug_dir);
 	acpi_put_table((struct acpi_table_header *)einj_tab);
 	kfree(einj_buf);
+	kfree(user_input);
 }
 
 static struct platform_device *einj_dev;
-- 
2.34.1

[PATCH v2 8/9] ACPI: APEI: EINJ: Enable EINJv2 error injections

[Zaid Alali]

Enable the driver to inject EINJv2 type errors. The component
array values are parsed from user_input and expected to contain
hex values for component id and syndrome separated by space,
and multiple components are separated by new line as follows:

component_id1 component_syndrome1
component_id2 component_syndrome2
 :
component_id(n) component_syndrome(n)

for example:

$comp_arr="0x1 0x2
>0x1 0x4
>0x2 0x4"
$cd /sys/kernel/debug/apei/einj/
$echo "$comp_arr" > einjv2_component_array

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 drivers/acpi/apei/einj-core.c | 82 +++++++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
index 1961f140ada8..d8ce859e6b5c 100644
--- a/drivers/acpi/apei/einj-core.c
+++ b/drivers/acpi/apei/einj-core.c
@@ -87,6 +87,13 @@ enum {
 	SETWA_FLAGS_APICID = 1,
 	SETWA_FLAGS_MEM = 2,
 	SETWA_FLAGS_PCIE_SBDF = 4,
+	SETWA_FLAGS_EINJV2 = 8,
+};
+
+enum {
+	EINJV2_PROCESSOR_ERROR = 0x1,
+	EINJV2_MEMORY_ERROR = 0x2,
+	EINJV2_PCIE_ERROR = 0x4,
 };
 
 /*
@@ -111,6 +118,7 @@ static char vendor_dev[64];
 static struct debugfs_blob_wrapper einjv2_component_arr;
 static u64 component_count;
 static void *user_input;
+static int nr_components;
 static u32 available_error_type;
 static u32 available_error_type_v2;
 
@@ -181,6 +189,7 @@ static DEFINE_MUTEX(einj_mutex);
 bool einj_initialized __ro_after_init;
 
 static void *einj_param;
+static bool is_V2;
 
 static void einj_exec_ctx_init(struct apei_exec_context *ctx)
 {
@@ -290,9 +299,19 @@ static void *einj_get_parameter_address(void)
 
 		p = acpi_os_map_iomem(pa_v5, sizeof(*v5param));
 		if (p) {
+			int offset, len;
 			v5param = __io_virt(p);
 			acpi5 = 1;
 			check_vendor_extension(pa_v5, v5param);
+			if (available_error_type & ACPI65_EINJV2_SUPP) {
+				len = v5param->einjv2_struct.length;
+				offset = offsetof(struct einjv2_extension_struct, component_arr);
+				nr_components = (len - offset) / 32;
+				acpi_os_unmap_iomem(p, sizeof(*v5param));
+				p = acpi_os_map_iomem(pa_v5, sizeof(*v5param) +
+					((nr_components) * sizeof(struct syndrome_array)));
+				v5param = __io_virt(p);
+			}
 			return v5param;
 		}
 	}
@@ -505,8 +524,49 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 			v5param->flags = flags;
 			v5param->memory_address = param1;
 			v5param->memory_address_range = param2;
-			v5param->apicid = param3;
-			v5param->pcie_sbdf = param4;
+
+			if (is_V2) {
+				int count = 0, bytes_read, pos = 0;
+				unsigned int comp, synd;
+				struct syndrome_array *component_arr;
+
+				if (component_count > nr_components)
+					goto err_out;
+
+				v5param->einjv2_struct.component_arr_count = component_count;
+				component_arr = v5param->einjv2_struct.component_arr;
+
+				while (sscanf(user_input + pos, "%x %x\n%n", &comp, &synd,
+					&bytes_read) == 2) {
+					pos += bytes_read;
+					if (count > component_count)
+						goto err_out;
+
+					switch (type) {
+					case EINJV2_PROCESSOR_ERROR:
+						component_arr[count].comp_id.acpi_id = comp;
+						component_arr[count].comp_synd.proc_synd = synd;
+						break;
+					case EINJV2_MEMORY_ERROR:
+						component_arr[count].comp_id.device_id = comp;
+						component_arr[count].comp_synd.mem_synd = synd;
+						break;
+					case EINJV2_PCIE_ERROR:
+						component_arr[count].comp_id.pcie_sbdf = comp;
+						component_arr[count].comp_synd.pcie_synd = synd;
+						break;
+					}
+					count++;
+				}
+				if (count != component_count)
+					goto err_out;
+
+				/* clear buffer after user input for next injection */
+				memset(user_input, 0, COMP_ARR_SIZE);
+			} else {
+				v5param->apicid = param3;
+				v5param->pcie_sbdf = param4;
+			}
 		} else {
 			switch (type) {
 			case ACPI_EINJ_PROCESSOR_CORRECTABLE:
@@ -579,6 +639,9 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
 	rc = apei_exec_run_optional(&ctx, ACPI_EINJ_END_OPERATION);
 
 	return rc;
+err_out:
+	memset(user_input, 0, COMP_ARR_SIZE);
+	return -EINVAL;
 }
 
 /* Inject the specified hardware error */
@@ -590,9 +653,14 @@ int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
 
 	/* If user manually set "flags", make sure it is legal */
 	if (flags && (flags &
-		~(SETWA_FLAGS_APICID|SETWA_FLAGS_MEM|SETWA_FLAGS_PCIE_SBDF)))
+		~(SETWA_FLAGS_APICID|SETWA_FLAGS_MEM|SETWA_FLAGS_PCIE_SBDF|SETWA_FLAGS_EINJV2)))
 		return -EINVAL;
 
+	/*check if type is a valid EINJv2 error type*/
+	if (is_V2) {
+		if (!(type & available_error_type_v2))
+			return -EINVAL;
+	}
 	/*
 	 * We need extra sanity checks for memory errors.
 	 * Other types leap directly to injection.
@@ -753,12 +821,14 @@ static ssize_t error_type_set(struct file *file, const char __user *buf,
 	u64 val;
 
 	memset(einj_buf, 0, sizeof(einj_buf));
+	is_V2 = false;
 	if (copy_from_user(einj_buf, buf, count))
 		return -EFAULT;
 
 	if (strncmp(einj_buf, "V2_", 3) == 0) {
 		if (!sscanf(einj_buf, "V2_%llx", &val))
 			return -EINVAL;
+		is_V2 = true;
 	} else
 		if (!sscanf(einj_buf, "%llx", &val))
 			return -EINVAL;
@@ -782,6 +852,9 @@ static int error_inject_set(void *data, u64 val)
 	if (!error_type)
 		return -EINVAL;
 
+	if (is_V2)
+		error_flags |= SETWA_FLAGS_EINJV2;
+
 	return einj_error_inject(error_type, error_flags, error_param1, error_param2,
 		error_param3, error_param4);
 }
@@ -942,7 +1015,8 @@ static void __exit einj_remove(struct platform_device *pdev)
 			sizeof(struct set_error_type_with_address) :
 			sizeof(struct einj_parameter);
 
-		acpi_os_unmap_iomem((void __iomem *)einj_param, size);
+		acpi_os_unmap_iomem((void __iomem *)einj_param,
+				size + (nr_components * sizeof(struct syndrome_array)));
 		if (vendor_errors.size)
 			acpi_os_unmap_memory(vendor_errors.data, vendor_errors.size);
 	}
-- 
2.34.1

Re: [PATCH v2 8/9] ACPI: APEI: EINJ: Enable EINJv2 error injections

[Jonathan Cameron]

On Thu,  5 Dec 2024 13:18:53 -0800
Zaid Alali <zaidal@os.amperecomputing.com> wrote:

> Enable the driver to inject EINJv2 type errors. The component
> array values are parsed from user_input and expected to contain
> hex values for component id and syndrome separated by space,
> and multiple components are separated by new line as follows:
> 
> component_id1 component_syndrome1
> component_id2 component_syndrome2
>  :
> component_id(n) component_syndrome(n)
> 
> for example:
> 
> $comp_arr="0x1 0x2
> >0x1 0x4
> >0x2 0x4"  
> $cd /sys/kernel/debug/apei/einj/
> $echo "$comp_arr" > einjv2_component_array
> 
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> ---
>  drivers/acpi/apei/einj-core.c | 82 +++++++++++++++++++++++++++++++++--
>  1 file changed, 78 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index 1961f140ada8..d8ce859e6b5c 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -87,6 +87,13 @@ enum {
>  	SETWA_FLAGS_APICID = 1,
>  	SETWA_FLAGS_MEM = 2,
>  	SETWA_FLAGS_PCIE_SBDF = 4,
> +	SETWA_FLAGS_EINJV2 = 8,
> +};
> +
> +enum {
> +	EINJV2_PROCESSOR_ERROR = 0x1,
> +	EINJV2_MEMORY_ERROR = 0x2,
> +	EINJV2_PCIE_ERROR = 0x4,
>  };
>  
>  /*
> @@ -111,6 +118,7 @@ static char vendor_dev[64];
>  static struct debugfs_blob_wrapper einjv2_component_arr;
>  static u64 component_count;
>  static void *user_input;
> +static int nr_components;
>  static u32 available_error_type;
>  static u32 available_error_type_v2;
>  
> @@ -181,6 +189,7 @@ static DEFINE_MUTEX(einj_mutex);
>  bool einj_initialized __ro_after_init;
>  
>  static void *einj_param;
> +static bool is_V2;
>  
>  static void einj_exec_ctx_init(struct apei_exec_context *ctx)
>  {
> @@ -290,9 +299,19 @@ static void *einj_get_parameter_address(void)
>  
>  		p = acpi_os_map_iomem(pa_v5, sizeof(*v5param));
>  		if (p) {
> +			int offset, len;
>  			v5param = __io_virt(p);
>  			acpi5 = 1;
>  			check_vendor_extension(pa_v5, v5param);
> +			if (available_error_type & ACPI65_EINJV2_SUPP) {
> +				len = v5param->einjv2_struct.length;
> +				offset = offsetof(struct einjv2_extension_struct, component_arr);
> +				nr_components = (len - offset) / 32;
> +				acpi_os_unmap_iomem(p, sizeof(*v5param));
> +				p = acpi_os_map_iomem(pa_v5, sizeof(*v5param) +
> +					((nr_components) * sizeof(struct syndrome_array)));

Use struct_size() which is there to help with variable elements on the end of structures.


> +				v5param = __io_virt(p);

As before. This to me looks like a hack where we should be using standard accessors
for the __iomem addresses.

> +			}
>  			return v5param;
>  		}
>  	}
> @@ -505,8 +524,49 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>  			v5param->flags = flags;
>  			v5param->memory_address = param1;
>  			v5param->memory_address_range = param2;
> -			v5param->apicid = param3;
> -			v5param->pcie_sbdf = param4;
> +
> +			if (is_V2) {
> +				int count = 0, bytes_read, pos = 0;
> +				unsigned int comp, synd;
> +				struct syndrome_array *component_arr;
> +
> +				if (component_count > nr_components)
> +					goto err_out;
> +
> +				v5param->einjv2_struct.component_arr_count = component_count;
> +				component_arr = v5param->einjv2_struct.component_arr;
> +
> +				while (sscanf(user_input + pos, "%x %x\n%n", &comp, &synd,
> +					&bytes_read) == 2) {
> +					pos += bytes_read;
> +					if (count > component_count)
> +						goto err_out;
> +
> +					switch (type) {
> +					case EINJV2_PROCESSOR_ERROR:
> +						component_arr[count].comp_id.acpi_id = comp;
> +						component_arr[count].comp_synd.proc_synd = synd;
> +						break;
> +					case EINJV2_MEMORY_ERROR:
> +						component_arr[count].comp_id.device_id = comp;
> +						component_arr[count].comp_synd.mem_synd = synd;
> +						break;
> +					case EINJV2_PCIE_ERROR:
> +						component_arr[count].comp_id.pcie_sbdf = comp;
> +						component_arr[count].comp_synd.pcie_synd = synd;
> +						break;
> +					}
> +					count++;
> +				}
> +				if (count != component_count)
> +					goto err_out;
> +
> +				/* clear buffer after user input for next injection */
> +				memset(user_input, 0, COMP_ARR_SIZE);
> +			} else {
> +				v5param->apicid = param3;
> +				v5param->pcie_sbdf = param4;
> +			}
>  		} else {
>  			switch (type) {
>  			case ACPI_EINJ_PROCESSOR_CORRECTABLE:
> @@ -579,6 +639,9 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
>  	rc = apei_exec_run_optional(&ctx, ACPI_EINJ_END_OPERATION);
>  
>  	return rc;
> +err_out:
> +	memset(user_input, 0, COMP_ARR_SIZE);
> +	return -EINVAL;
>  }
>  
>  /* Inject the specified hardware error */
> @@ -590,9 +653,14 @@ int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, u64 param3,
>  
>  	/* If user manually set "flags", make sure it is legal */
>  	if (flags && (flags &
> -		~(SETWA_FLAGS_APICID|SETWA_FLAGS_MEM|SETWA_FLAGS_PCIE_SBDF)))
> +		~(SETWA_FLAGS_APICID|SETWA_FLAGS_MEM|SETWA_FLAGS_PCIE_SBDF|SETWA_FLAGS_EINJV2)))
Whilst here, make it compliant with kernel style and add space around | 

>  		return -EINVAL;
>  
> +	/*check if type is a valid EINJv2 error type*/
Space after /* 
Make sure to run checkpatch (I'm not sure it catches this though!)

> +	if (is_V2) {
> +		if (!(type & available_error_type_v2))
> +			return -EINVAL;
> +	}
>  	/*
>  	 * We need extra sanity checks for memory errors.
>  	 * Other types leap directly to injection.
> @@ -753,12 +821,14 @@ static ssize_t error_type_set(struct file *file, const char __user *buf,
>  	u64 val;
>  
>  	memset(einj_buf, 0, sizeof(einj_buf));
> +	is_V2 = false;
>  	if (copy_from_user(einj_buf, buf, count))
>  		return -EFAULT;
>  
>  	if (strncmp(einj_buf, "V2_", 3) == 0) {
>  		if (!sscanf(einj_buf, "V2_%llx", &val))
>  			return -EINVAL;
> +		is_V2 = true;
>  	} else
>  		if (!sscanf(einj_buf, "%llx", &val))
>  			return -EINVAL;
> @@ -782,6 +852,9 @@ static int error_inject_set(void *data, u64 val)
>  	if (!error_type)
>  		return -EINVAL;
>  
> +	if (is_V2)
> +		error_flags |= SETWA_FLAGS_EINJV2;
> +
>  	return einj_error_inject(error_type, error_flags, error_param1, error_param2,
>  		error_param3, error_param4);
>  }
> @@ -942,7 +1015,8 @@ static void __exit einj_remove(struct platform_device *pdev)
>  			sizeof(struct set_error_type_with_address) :
>  			sizeof(struct einj_parameter);
>  
> -		acpi_os_unmap_iomem((void __iomem *)einj_param, size);
> +		acpi_os_unmap_iomem((void __iomem *)einj_param,
> +				size + (nr_components * sizeof(struct syndrome_array)));
struct_size()

>  		if (vendor_errors.size)
>  			acpi_os_unmap_memory(vendor_errors.data, vendor_errors.size);
>  	}

[PATCH v2 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Zaid Alali]

Add documentation for the updated ACPI specs for EINJv2(1)(2)

(1)https://bugzilla.tianocore.org/show_bug.cgi?id=4615
(2)https://bugzilla.tianocore.org/attachment.cgi?id=1446

Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
---
 .../firmware-guide/acpi/apei/einj.rst         | 41 ++++++++++++++++++-
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/Documentation/firmware-guide/acpi/apei/einj.rst b/Documentation/firmware-guide/acpi/apei/einj.rst
index c52b9da08fa9..b1c0464f6002 100644
--- a/Documentation/firmware-guide/acpi/apei/einj.rst
+++ b/Documentation/firmware-guide/acpi/apei/einj.rst
@@ -59,6 +59,9 @@ The following files belong to it:
   0x00000200        Platform Correctable
   0x00000400        Platform Uncorrectable non-fatal
   0x00000800        Platform Uncorrectable fatal
+  V2_0x00000001     EINJV2 Processor Error
+  V2_0x00000002     EINJV2 Memory Error
+  V2_0x00000004     EINJV2 PCI Express Error
   ================  ===================================
 
   The format of the file contents are as above, except present are only
@@ -85,9 +88,11 @@ The following files belong to it:
     Bit 0
       Processor APIC field valid (see param3 below).
     Bit 1
-      Memory address and mask valid (param1 and param2).
+      Memory address and range valid (param1 and param2).
     Bit 2
       PCIe (seg,bus,dev,fn) valid (see param4 below).
+    Bit 3
+      EINJv2 extension structure is valid
 
   If set to zero, legacy behavior is mimicked where the type of
   injection specifies just one bit set, and param1 is multiplexed.
@@ -110,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
@@ -122,6 +128,18 @@ The following files belong to it:
   this actually works depends on what operations the BIOS actually
   includes in the trigger phase.
 
+- einjv2_component_count
+
+  The value from this file is used to set the "Component Array Count"
+  field of EINJv2 Extension Structure.
+
+- einjv2_component_array
+
+  The contents of this file are used to set the "Component Array" field
+  of the EINJv2 Extension Structure. The expected format is hex values
+  for component id and syndrome separated by space, and multiple
+  components are separated by new line.
+
 CXL error types are supported from ACPI 6.5 onwards (given a CXL port
 is present). The EINJ user interface for CXL error types is at
 <debugfs mount point>/cxl. The following files belong to it:
@@ -139,7 +157,6 @@ is present). The EINJ user interface for CXL error types is at
   under <debugfs mount point>/apei/einj, while CXL 1.1/1.0 port injections
   must use this file.
 
-
 BIOS versions based on the ACPI 4.0 specification have limited options
 in controlling where the errors are injected. Your BIOS may support an
 extension (enabled with the param_extension=1 module parameter, or boot
@@ -194,6 +211,26 @@ An error injection example::
   # echo 0x8 > error_type			# Choose correctable memory error
   # echo 1 > error_inject			# Inject now
 
+An EINJv2 error injection example::
+
+  # cd /sys/kernel/debug/apei/einj
+  # cat available_error_type			# See which errors can be injected
+  0x00000002	Processor Uncorrectable non-fatal
+  0x00000008	Memory Correctable
+  0x00000010	Memory Uncorrectable non-fatal
+  0x00000001	EINJV2 Processor Error
+  0x00000002	EINJV2 Memory Error
+
+  # echo 0x12345000 > param1			# Set memory address for injection
+  # echo 0xfffffffffffff000 > param2		# Range - anywhere in this page
+  # comp_arr="0x1 0x2				# Fill in the component array
+    >0x1 0x4
+    >0x2 0x4"
+  # echo "$comp_arr" > einjv2_component_array
+  # echo 0x2 > error_type			# Choose EINJv2 memory error
+  # echo 0xa > flags				# set flags to indicate EINJv2
+  # echo 1 > error_inject			# Inject now
+
 You should see something like this in dmesg::
 
   [22715.830801] EDAC sbridge MC3: HANDLING MCE MEMORY ERROR
-- 
2.34.1

Re: [PATCH v2 9/9] ACPI: APEI: EINJ: Update the documentation for EINJv2 support

[Lai, Yi]

On Thu, Dec 05, 2024 at 01:18:54PM -0800, Zaid Alali wrote:
> Add documentation for the updated ACPI specs for EINJv2(1)(2)
> 
> (1)https://bugzilla.tianocore.org/show_bug.cgi?id=4615
> (2)https://bugzilla.tianocore.org/attachment.cgi?id=1446
> 
> Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> ---
>  .../firmware-guide/acpi/apei/einj.rst         | 41 ++++++++++++++++++-
>  1 file changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/firmware-guide/acpi/apei/einj.rst b/Documentation/firmware-guide/acpi/apei/einj.rst
> index c52b9da08fa9..b1c0464f6002 100644
> --- a/Documentation/firmware-guide/acpi/apei/einj.rst
> +++ b/Documentation/firmware-guide/acpi/apei/einj.rst
> @@ -59,6 +59,9 @@ The following files belong to it:
>    0x00000200        Platform Correctable
>    0x00000400        Platform Uncorrectable non-fatal
>    0x00000800        Platform Uncorrectable fatal
> +  V2_0x00000001     EINJV2 Processor Error
> +  V2_0x00000002     EINJV2 Memory Error
> +  V2_0x00000004     EINJV2 PCI Express Error
>    ================  ===================================
>  
>    The format of the file contents are as above, except present are only
> @@ -85,9 +88,11 @@ The following files belong to it:
>      Bit 0
>        Processor APIC field valid (see param3 below).
>      Bit 1
> -      Memory address and mask valid (param1 and param2).
> +      Memory address and range valid (param1 and param2).
>      Bit 2
>        PCIe (seg,bus,dev,fn) valid (see param4 below).
> +    Bit 3
> +      EINJv2 extension structure is valid
>  
>    If set to zero, legacy behavior is mimicked where the type of
>    injection specifies just one bit set, and param1 is multiplexed.
> @@ -110,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
> @@ -122,6 +128,18 @@ The following files belong to it:
>    this actually works depends on what operations the BIOS actually
>    includes in the trigger phase.
>  
> +- einjv2_component_count
> +
> +  The value from this file is used to set the "Component Array Count"
> +  field of EINJv2 Extension Structure.
> +
> +- einjv2_component_array
> +
> +  The contents of this file are used to set the "Component Array" field
> +  of the EINJv2 Extension Structure. The expected format is hex values
> +  for component id and syndrome separated by space, and multiple
> +  components are separated by new line.
> +
>  CXL error types are supported from ACPI 6.5 onwards (given a CXL port
>  is present). The EINJ user interface for CXL error types is at
>  <debugfs mount point>/cxl. The following files belong to it:
> @@ -139,7 +157,6 @@ is present). The EINJ user interface for CXL error types is at
>    under <debugfs mount point>/apei/einj, while CXL 1.1/1.0 port injections
>    must use this file.
>  
> -
>  BIOS versions based on the ACPI 4.0 specification have limited options
>  in controlling where the errors are injected. Your BIOS may support an
>  extension (enabled with the param_extension=1 module parameter, or boot
> @@ -194,6 +211,26 @@ An error injection example::
>    # echo 0x8 > error_type			# Choose correctable memory error
>    # echo 1 > error_inject			# Inject now
>  
> +An EINJv2 error injection example::
> +
> +  # cd /sys/kernel/debug/apei/einj
> +  # cat available_error_type			# See which errors can be injected
> +  0x00000002	Processor Uncorrectable non-fatal
> +  0x00000008	Memory Correctable
> +  0x00000010	Memory Uncorrectable non-fatal
> +  0x00000001	EINJV2 Processor Error
> +  0x00000002	EINJV2 Memory Error
> +
> +  # echo 0x12345000 > param1			# Set memory address for injection
> +  # echo 0xfffffffffffff000 > param2		# Range - anywhere in this page
> +  # comp_arr="0x1 0x2				# Fill in the component array
> +    >0x1 0x4
> +    >0x2 0x4"
> +  # echo "$comp_arr" > einjv2_component_array
> +  # echo 0x2 > error_type			# Choose EINJv2 memory error

Missing the V2_ prefix here.

Regards,
Yi Lai

> +  # echo 0xa > flags				# set flags to indicate EINJv2
> +  # echo 1 > error_inject			# Inject now
> +
>  You should see something like this in dmesg::
>  
>    [22715.830801] EDAC sbridge MC3: HANDLING MCE MEMORY ERROR
> -- 
> 2.34.1
>