[PATCH 0/8] ACPI, APEI patches for 3.2

[PATCH 0/8] ACPI, APEI patches for 3.2

[Huang Ying]

[PATCH 1/8] ACPI, APEI, Print resource errors in conventional
[PATCH 2/8] ACPI, APEI, Remove table not found message
[PATCH 3/8] ACPI, Record ACPI NVS regions
[PATCH 4/8] ACPI, APEI, Resolve false conflict between ACPI NVS
[PATCH 5/8] ACPI, APEI, EINJ, Fix resource conflict on some
[PATCH 6/8] ACPI, Add RAM mapping support to ACPI atomic IO
[PATCH 7/8] ACPI, APEI, GHES: Add PCIe AER recovery support
[PATCH 8/8] ACPI, APEI, GHES, Distinguish interleaved error report

[PATCH 1/8] ACPI, APEI, Print resource errors in conventional format

[Huang Ying]

From: Bjorn Helgaas <bhelgaas@google.com>

Use the normal %pR-like format for MMIO and I/O port ranges.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/acpi/apei/apei-base.c |    8 ++++----
 drivers/acpi/apei/einj.c      |   11 ++++++-----
 2 files changed, 10 insertions(+), 9 deletions(-)

--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -460,9 +460,9 @@ int apei_resources_request(struct apei_r
 				       desc);
 		if (!r) {
 			pr_err(APEI_PFX
-		"Can not request iomem region <%016llx-%016llx> for GARs.\n",
+		"Can not request [mem %#010llx-%#010llx] for %s registers\n",
 			       (unsigned long long)res->start,
-			       (unsigned long long)res->end);
+			       (unsigned long long)res->end - 1, desc);
 			res_bak = res;
 			goto err_unmap_iomem;
 		}
@@ -472,9 +472,9 @@ int apei_resources_request(struct apei_r
 		r = request_region(res->start, res->end - res->start, desc);
 		if (!r) {
 			pr_err(APEI_PFX
-		"Can not request ioport region <%016llx-%016llx> for GARs.\n",
+		"Can not request [io  %#06llx-%#06llx] for %s registers\n",
 			       (unsigned long long)res->start,
-			       (unsigned long long)res->end);
+			       (unsigned long long)res->end - 1, desc);
 			res_bak = res;
 			goto err_unmap_ioport;
 		}
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -209,9 +209,10 @@ static int __einj_error_trigger(u64 trig
 			       "APEI EINJ Trigger Table");
 	if (!r) {
 		pr_err(EINJ_PFX
-	"Can not request iomem region <%016llx-%016llx> for Trigger table.\n",
+	"Can not request [mem %#010llx-%#010llx] for Trigger table\n",
 		       (unsigned long long)trigger_paddr,
-		       (unsigned long long)trigger_paddr+sizeof(*trigger_tab));
+		       (unsigned long long)trigger_paddr +
+			    sizeof(*trigger_tab) - 1);
 		goto out;
 	}
 	trigger_tab = ioremap_cache(trigger_paddr, sizeof(*trigger_tab));
@@ -232,9 +233,9 @@ static int __einj_error_trigger(u64 trig
 			       "APEI EINJ Trigger Table");
 	if (!r) {
 		pr_err(EINJ_PFX
-"Can not request iomem region <%016llx-%016llx> for Trigger Table Entry.\n",
-		       (unsigned long long)trigger_paddr+sizeof(*trigger_tab),
-		       (unsigned long long)trigger_paddr + table_size);
+"Can not request [mem %#010llx-%#010llx] for Trigger Table Entry\n",
+		       (unsigned long long)trigger_paddr + sizeof(*trigger_tab),
+		       (unsigned long long)trigger_paddr + table_size - 1);
 		goto out_rel_header;
 	}
 	iounmap(trigger_tab);

[PATCH 2/8] ACPI, APEI, Remove table not found message

[Huang Ying]

Because APEI tables are optional, these message may confuse users, for
example,

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/599715

Reported-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/acpi/apei/einj.c |    5 ++---
 drivers/acpi/apei/erst.c |    5 ++---
 drivers/acpi/apei/hest.c |    5 ++---
 3 files changed, 6 insertions(+), 9 deletions(-)

--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -466,10 +466,9 @@ static int __init einj_init(void)
 
 	status = acpi_get_table(ACPI_SIG_EINJ, 0,
 				(struct acpi_table_header **)&einj_tab);
-	if (status == AE_NOT_FOUND) {
-		pr_info(EINJ_PFX "Table is not found!\n");
+	if (status == AE_NOT_FOUND)
 		return -ENODEV;
-	} else if (ACPI_FAILURE(status)) {
+	else if (ACPI_FAILURE(status)) {
 		const char *msg = acpi_format_exception(status);
 		pr_err(EINJ_PFX "Failed to get table, %s\n", msg);
 		return -EINVAL;
--- a/drivers/acpi/apei/erst.c
+++ b/drivers/acpi/apei/erst.c
@@ -1110,10 +1110,9 @@ static int __init erst_init(void)
 
 	status = acpi_get_table(ACPI_SIG_ERST, 0,
 				(struct acpi_table_header **)&erst_tab);
-	if (status == AE_NOT_FOUND) {
-		pr_info(ERST_PFX "Table is not found!\n");
+	if (status == AE_NOT_FOUND)
 		goto err;
-	} else if (ACPI_FAILURE(status)) {
+	else if (ACPI_FAILURE(status)) {
 		const char *msg = acpi_format_exception(status);
 		pr_err(ERST_PFX "Failed to get table, %s\n", msg);
 		rc = -EINVAL;
--- a/drivers/acpi/apei/hest.c
+++ b/drivers/acpi/apei/hest.c
@@ -221,10 +221,9 @@ void __init acpi_hest_init(void)
 
 	status = acpi_get_table(ACPI_SIG_HEST, 0,
 				(struct acpi_table_header **)&hest_tab);
-	if (status == AE_NOT_FOUND) {
-		pr_info(HEST_PFX "Table not found.\n");
+	if (status == AE_NOT_FOUND)
 		goto err;
-	} else if (ACPI_FAILURE(status)) {
+	else if (ACPI_FAILURE(status)) {
 		const char *msg = acpi_format_exception(status);
 		pr_err(HEST_PFX "Failed to get table, %s\n", msg);
 		rc = -EINVAL;

[PATCH 3/8] ACPI, Record ACPI NVS regions

[Huang Ying]

Some firmware will access memory in ACPI NVS region via APEI.  That
is, instructions in APEI ERST/EINJ table will read/write ACPI NVS
region.  The original resource conflict checking in APEI code will
check memory/ioport accessed by APEI via general resource management
mechanism.  But ACPI NVS region is marked as busy already, so that the
false resource conflict will prevent APEI ERST/EINJ to work.

To fix this, this patch record ACPI NVS regions, so that we can avoid
request resources for memory region inside it.

Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 arch/x86/kernel/e820.c |    4 +--
 drivers/acpi/Makefile  |    3 +-
 drivers/acpi/nvs.c     |   53 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/acpi.h   |   20 ++++++++++++------
 4 files changed, 70 insertions(+), 10 deletions(-)

--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -713,7 +713,7 @@ void __init e820_mark_nosave_regions(uns
 }
 #endif
 
-#ifdef CONFIG_HIBERNATION
+#ifdef CONFIG_ACPI
 /**
  * Mark ACPI NVS memory region, so that we can save/restore it during
  * hibernation and the subsequent resume.
@@ -726,7 +726,7 @@ static int __init e820_mark_nvs_memory(v
 		struct e820entry *ei = &e820.map[i];
 
 		if (ei->type == E820_NVS)
-			suspend_nvs_register(ei->addr, ei->size);
+			acpi_nvs_register(ei->addr, ei->size);
 	}
 
 	return 0;
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -20,11 +20,12 @@ obj-y				+= acpi.o \
 # All the builtin files are in the "acpi." module_param namespace.
 acpi-y				+= osl.o utils.o reboot.o
 acpi-y				+= atomicio.o
+acpi-y				+= nvs.o
 
 # sleep related files
 acpi-y				+= wakeup.o
 acpi-y				+= sleep.o
-acpi-$(CONFIG_ACPI_SLEEP)	+= proc.o nvs.o
+acpi-$(CONFIG_ACPI_SLEEP)	+= proc.o
 
 
 #
--- a/drivers/acpi/nvs.c
+++ b/drivers/acpi/nvs.c
@@ -15,6 +15,56 @@
 #include <linux/acpi_io.h>
 #include <acpi/acpiosxf.h>
 
+/* ACPI NVS regions, APEI may use it */
+
+struct nvs_region {
+	__u64 phys_start;
+	__u64 size;
+	struct list_head node;
+};
+
+static LIST_HEAD(nvs_region_list);
+
+#ifdef CONFIG_ACPI_SLEEP
+static int suspend_nvs_register(unsigned long start, unsigned long size);
+#else
+static inline int suspend_nvs_register(unsigned long a, unsigned long b)
+{
+	return 0;
+}
+#endif
+
+int acpi_nvs_register(__u64 start, __u64 size)
+{
+	struct nvs_region *region;
+
+	region = kmalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return -ENOMEM;
+	region->phys_start = start;
+	region->size = size;
+	list_add_tail(&region->node, &nvs_region_list);
+
+	return suspend_nvs_register(start, size);
+}
+
+int acpi_nvs_for_each_region(int (*func)(__u64 start, __u64 size, void *data),
+			     void *data)
+{
+	int rc;
+	struct nvs_region *region;
+
+	list_for_each_entry(region, &nvs_region_list, node) {
+		rc = func(region->phys_start, region->size, data);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
+
+#ifdef CONFIG_ACPI_SLEEP
 /*
  * Platforms, like ACPI, may want us to save some memory used by them during
  * suspend and to restore the contents of this memory during the subsequent
@@ -41,7 +91,7 @@ static LIST_HEAD(nvs_list);
  *	things so that the data from page-aligned addresses in this region will
  *	be copied into separate RAM pages.
  */
-int suspend_nvs_register(unsigned long start, unsigned long size)
+static int suspend_nvs_register(unsigned long start, unsigned long size)
 {
 	struct nvs_page *entry, *next;
 
@@ -159,3 +209,4 @@ void suspend_nvs_restore(void)
 		if (entry->data)
 			memcpy(entry->kaddr, entry->data, entry->size);
 }
+#endif
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -306,6 +306,11 @@ extern acpi_status acpi_pci_osc_control_
 					     u32 *mask, u32 req);
 extern void acpi_early_init(void);
 
+extern int acpi_nvs_register(__u64 start, __u64 size);
+
+extern int acpi_nvs_for_each_region(int (*func)(__u64, __u64, void *),
+				    void *data);
+
 #else	/* !CONFIG_ACPI */
 
 #define acpi_disabled 1
@@ -348,15 +353,18 @@ static inline int acpi_table_parse(char
 {
 	return -1;
 }
-#endif	/* !CONFIG_ACPI */
 
-#ifdef CONFIG_ACPI_SLEEP
-int suspend_nvs_register(unsigned long start, unsigned long size);
-#else
-static inline int suspend_nvs_register(unsigned long a, unsigned long b)
+static inline int acpi_nvs_register(__u64 start, __u64 size)
 {
 	return 0;
 }
-#endif
+
+static inline int acpi_nvs_for_each_region(int (*func)(__u64, __u64, void *),
+					   void *data)
+{
+	return 0;
+}
+
+#endif	/* !CONFIG_ACPI */
 
 #endif	/*_LINUX_ACPI_H*/

[PATCH 4/8] ACPI, APEI, Resolve false conflict between ACPI NVS and APEI

[Huang Ying]

Some firmware will access memory in ACPI NVS region via APEI.  That
is, instructions in APEI ERST/EINJ table will read/write ACPI NVS
region.  The original resource conflict checking in APEI code will
check memory/ioport accessed by APEI via general resource management
mech.  But ACPI NVS region is marked as busy already, so that the
false resource conflict will prevent APEI ERST/EINJ to work.

To fix this, this patch excludes ACPI NVS regions when APEI components
request resources.  So that they will not conflict with ACPI NVS
regions.

Reported-and-tested-by: Pavel Ivanov <paivanof@gmail.com>
Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/acpi/apei/apei-base.c |   29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -438,8 +438,19 @@ int apei_resources_sub(struct apei_resou
 }
 EXPORT_SYMBOL_GPL(apei_resources_sub);
 
+static int apei_get_nvs_callback(__u64 start, __u64 size, void *data)
+{
+	struct apei_resources *resources = data;
+	return apei_res_add(&resources->iomem, start, size);
+}
+
+static int apei_get_nvs_resources(struct apei_resources *resources)
+{
+	return acpi_nvs_for_each_region(apei_get_nvs_callback, resources);
+}
+
 /*
- * IO memory/port rersource management mechanism is used to check
+ * IO memory/port resource management mechanism is used to check
  * whether memory/port area used by GARs conflicts with normal memory
  * or IO memory/port of devices.
  */
@@ -448,12 +459,26 @@ int apei_resources_request(struct apei_r
 {
 	struct apei_res *res, *res_bak = NULL;
 	struct resource *r;
+	struct apei_resources nvs_resources;
 	int rc;
 
 	rc = apei_resources_sub(resources, &apei_resources_all);
 	if (rc)
 		return rc;
 
+	/*
+	 * Some firmware uses ACPI NVS region, that has been marked as
+	 * busy, so exclude it from APEI resources to avoid false
+	 * conflict.
+	 */
+	apei_resources_init(&nvs_resources);
+	rc = apei_get_nvs_resources(&nvs_resources);
+	if (rc)
+		goto res_fini;
+	rc = apei_resources_sub(resources, &nvs_resources);
+	if (rc)
+		goto res_fini;
+
 	rc = -EINVAL;
 	list_for_each_entry(res, &resources->iomem, list) {
 		r = request_mem_region(res->start, res->end - res->start,
@@ -500,6 +525,8 @@ err_unmap_iomem:
 			break;
 		release_mem_region(res->start, res->end - res->start);
 	}
+res_fini:
+	apei_resources_fini(&nvs_resources);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(apei_resources_request);

[PATCH 5/8] ACPI, APEI, EINJ, Fix resource conflict on some machine

[Huang Ying]

Some APEI firmware implementation will access injected address
specified in param1 to trigger the error when injecting memory error.
This will cause resource conflict with RAM.

On one of our testing machine, if injecting at memory address
0x10000000, the following error will be reported in dmesg:

  APEI: Can not request iomem region <0000000010000000-0000000010000008> for GARs.

This patch removes the injecting memory address range from trigger
table resources to avoid conflict.

Signed-off-by: Huang Ying <ying.huang@intel.com>
Tested-by: Tony Luck <tony.luck@intel.com>
---
 drivers/acpi/apei/apei-base.c     |   11 +++++++++++
 drivers/acpi/apei/apei-internal.h |    3 +++
 drivers/acpi/apei/einj.c          |   24 ++++++++++++++++++++++--
 3 files changed, 36 insertions(+), 2 deletions(-)

--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -421,6 +421,17 @@ static int apei_resources_merge(struct a
 	return 0;
 }
 
+int apei_resources_add(struct apei_resources *resources,
+		       unsigned long start, unsigned long size,
+		       bool iomem)
+{
+	if (iomem)
+		return apei_res_add(&resources->iomem, start, size);
+	else
+		return apei_res_add(&resources->ioport, start, size);
+}
+EXPORT_SYMBOL_GPL(apei_resources_add);
+
 /*
  * EINJ has two groups of GARs (EINJ table entry and trigger table
  * entry), so common resources are subtracted from the trigger table
--- a/drivers/acpi/apei/apei-internal.h
+++ b/drivers/acpi/apei/apei-internal.h
@@ -95,6 +95,9 @@ static inline void apei_resources_init(s
 }
 
 void apei_resources_fini(struct apei_resources *resources);
+int apei_resources_add(struct apei_resources *resources,
+		       unsigned long start, unsigned long size,
+		       bool iomem);
 int apei_resources_sub(struct apei_resources *resources1,
 		       struct apei_resources *resources2);
 int apei_resources_request(struct apei_resources *resources,
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -195,7 +195,8 @@ static int einj_check_trigger_header(str
 }
 
 /* Execute instructions in trigger error action table */
-static int __einj_error_trigger(u64 trigger_paddr)
+static int __einj_error_trigger(u64 trigger_paddr, u32 type,
+				u64 param1, u64 param2)
 {
 	struct acpi_einj_trigger *trigger_tab = NULL;
 	struct apei_exec_context trigger_ctx;
@@ -256,6 +257,25 @@ static int __einj_error_trigger(u64 trig
 	rc = apei_resources_sub(&trigger_resources, &einj_resources);
 	if (rc)
 		goto out_fini;
+	/*
+	 * Some firmware will access target address specified in
+	 * param1 to trigger the error when injecting memory error.
+	 * This will cause resource conflict with regular memory.  So
+	 * remove it from trigger table resources.
+	 */
+	if (param_extension && (type & 0x0038) && param2) {
+		struct apei_resources addr_resources;
+		apei_resources_init(&addr_resources);
+		rc = apei_resources_add(&addr_resources,
+					param1 & param2,
+					~param2 + 1, true);
+		if (rc)
+			goto out_fini;
+		rc = apei_resources_sub(&trigger_resources, &addr_resources);
+		apei_resources_fini(&addr_resources);
+		if (rc)
+			goto out_fini;
+	}
 	rc = apei_resources_request(&trigger_resources, "APEI EINJ Trigger");
 	if (rc)
 		goto out_fini;
@@ -325,7 +345,7 @@ static int __einj_error_inject(u32 type,
 	if (rc)
 		return rc;
 	trigger_paddr = apei_exec_ctx_get_output(&ctx);
-	rc = __einj_error_trigger(trigger_paddr);
+	rc = __einj_error_trigger(trigger_paddr, type, param1, param2);
 	if (rc)
 		return rc;
 	rc = apei_exec_run_optional(&ctx, ACPI_EINJ_END_OPERATION);

[PATCH 6/8] ACPI, Add RAM mapping support to ACPI atomic IO support

[Huang Ying]

On one of our testing machine, the following EINJ command lines:

  # echo 0x10000000 > param1
  # echo 0xfffffffffffff000 > param2
  # echo 0x8 > error_type
  # echo 1 > error_inject

Will get:

  echo: write error: Input/output error

The EIO comes from:

    rc = apei_exec_pre_map_gars(&trigger_ctx);

The root cause is as follow.  Normally, ACPI atomic IO support is used
to access IO memory.  But in EINJ of that machine, it is used to
access RAM to trigger the injected error.  And the ioremap() called by
apei_exec_pre_map_gars() can not map the RAM.

This patch add RAM mapping support to ACPI atomic IO support to
satisfy EINJ requirement.

Signed-off-by: Huang Ying <ying.huang@intel.com>
Tested-by: Tony Luck <tony.luck@intel.com>
---
 drivers/acpi/atomicio.c |   34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

--- a/drivers/acpi/atomicio.c
+++ b/drivers/acpi/atomicio.c
@@ -32,6 +32,8 @@
 #include <linux/rculist.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
+#include <linux/mm.h>
+#include <linux/highmem.h>
 #include <acpi/atomicio.h>
 
 #define ACPI_PFX "ACPI: "
@@ -97,6 +99,30 @@ static void __iomem *__acpi_try_ioremap(
 		return NULL;
 }
 
+static void __iomem *acpi_map(phys_addr_t pg_off, unsigned long pg_sz)
+{
+	unsigned long pfn;
+
+	pfn = pg_off >> PAGE_SHIFT;
+	if (page_is_ram(pfn)) {
+		if (pg_sz > PAGE_SIZE)
+			return NULL;
+		return (void __iomem __force *)kmap(pfn_to_page(pfn));
+	} else
+		return ioremap(pg_off, pg_sz);
+}
+
+static void acpi_unmap(phys_addr_t pg_off, void __iomem *vaddr)
+{
+	unsigned long pfn;
+
+	pfn = pg_off >> PAGE_SHIFT;
+	if (page_is_ram(pfn))
+		kunmap(pfn_to_page(pfn));
+	else
+		iounmap(vaddr);
+}
+
 /*
  * Used to pre-map the specified IO memory area. First try to find
  * whether the area is already pre-mapped, if it is, increase the
@@ -119,7 +145,7 @@ static void __iomem *acpi_pre_map(phys_a
 
 	pg_off = paddr & PAGE_MASK;
 	pg_sz = ((paddr + size + PAGE_SIZE - 1) & PAGE_MASK) - pg_off;
-	vaddr = ioremap(pg_off, pg_sz);
+	vaddr = acpi_map(pg_off, pg_sz);
 	if (!vaddr)
 		return NULL;
 	map = kmalloc(sizeof(*map), GFP_KERNEL);
@@ -135,7 +161,7 @@ static void __iomem *acpi_pre_map(phys_a
 	vaddr = __acpi_try_ioremap(paddr, size);
 	if (vaddr) {
 		spin_unlock_irqrestore(&acpi_iomaps_lock, flags);
-		iounmap(map->vaddr);
+		acpi_unmap(pg_off, map->vaddr);
 		kfree(map);
 		return vaddr;
 	}
@@ -144,7 +170,7 @@ static void __iomem *acpi_pre_map(phys_a
 
 	return map->vaddr + (paddr - map->paddr);
 err_unmap:
-	iounmap(vaddr);
+	acpi_unmap(pg_off, vaddr);
 	return NULL;
 }
 
@@ -177,7 +203,7 @@ static void acpi_post_unmap(phys_addr_t
 		return;
 
 	synchronize_rcu();
-	iounmap(map->vaddr);
+	acpi_unmap(map->paddr, map->vaddr);
 	kfree(map);
 }
 

[PATCH 7/8] ACPI, APEI, GHES: Add PCIe AER recovery support

[Huang Ying]

aer_recover_queue() is called when recoverable PCIe AER errors are
notified by firmware to do the recovery work.

Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/acpi/apei/ghes.c |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -45,6 +45,8 @@
 #include <linux/irq_work.h>
 #include <linux/llist.h>
 #include <linux/genalloc.h>
+#include <linux/pci.h>
+#include <linux/aer.h>
 #include <acpi/apei.h>
 #include <acpi/atomicio.h>
 #include <acpi/hed.h>
@@ -475,6 +477,27 @@ static void ghes_do_proc(const struct ac
 			}
 #endif
 		}
+#ifdef CONFIG_ACPI_APEI_PCIEAER
+		else if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
+				      CPER_SEC_PCIE)) {
+			struct cper_sec_pcie *pcie_err;
+			pcie_err = (struct cper_sec_pcie *)(gdata+1);
+			if (sev == GHES_SEV_RECOVERABLE &&
+			    sec_sev == GHES_SEV_RECOVERABLE &&
+			    pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
+			    pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) {
+				unsigned int devfn;
+				int aer_severity;
+				devfn = PCI_DEVFN(pcie_err->device_id.device,
+						  pcie_err->device_id.function);
+				aer_severity = cper_severity_to_aer(sev);
+				aer_recover_queue(pcie_err->device_id.segment,
+						  pcie_err->device_id.bus,
+						  devfn, aer_severity);
+			}
+
+		}
+#endif
 	}
 }
 

[PATCH 8/8] ACPI, APEI, GHES, Distinguish interleaved error report in kernel log

[Huang Ying]

In most cases, printk only guarantees messages from different printk
calling will not be interleaved between each other.  But, one APEI
GHES hardware error report will involve multiple printk calling,
normally each for one line.  So it is possible that the hardware error
report comes from different generic hardware error source will be
interleaved.

In this patch, a sequence number is prefixed to each line of error
report.  So that, even if they are interleaved, they still can be
distinguished by the prefixed sequence number.

Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/acpi/apei/ghes.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -505,16 +505,22 @@ static void __ghes_print_estatus(const c
 				 const struct acpi_hest_generic *generic,
 				 const struct acpi_hest_generic_status *estatus)
 {
+	static atomic_t seqno;
+	int curr_seqno;
+	char pfx_seq[64];
+
 	if (pfx == NULL) {
 		if (ghes_severity(estatus->error_severity) <=
 		    GHES_SEV_CORRECTED)
-			pfx = KERN_WARNING HW_ERR;
+			pfx = KERN_WARNING;
 		else
-			pfx = KERN_ERR HW_ERR;
+			pfx = KERN_ERR;
 	}
+	curr_seqno = atomic_inc_return(&seqno);
+	snprintf(pfx_seq, sizeof(pfx_seq), "%s{%d}" HW_ERR, pfx, curr_seqno);
 	printk("%s""Hardware error from APEI Generic Hardware Error Source: %d\n",
-	       pfx, generic->header.source_id);
-	apei_estatus_print(pfx, estatus);
+	       pfx_seq, generic->header.source_id);
+	apei_estatus_print(pfx_seq, estatus);
 }
 
 static int ghes_print_estatus(const char *pfx,
@@ -801,7 +807,7 @@ static int ghes_notify_nmi(struct notifi
 
 	if (sev_global >= GHES_SEV_PANIC) {
 		oops_begin();
-		__ghes_print_estatus(KERN_EMERG HW_ERR, ghes_global->generic,
+		__ghes_print_estatus(KERN_EMERG, ghes_global->generic,
 				     ghes_global->estatus);
 		/* reboot to log the error! */
 		if (panic_timeout == 0)

Re: [PATCH 4/8] ACPI, APEI, Resolve false conflict between ACPI NVS and APEI

[Bjorn Helgaas]

On Mon, Sep 19, 2011 at 7:38 PM, Huang Ying <ying.huang@intel.com> wrote:
> Some firmware will access memory in ACPI NVS region via APEI.  That
> is, instructions in APEI ERST/EINJ table will read/write ACPI NVS
> region.  The original resource conflict checking in APEI code will
> check memory/ioport accessed by APEI via general resource management
> mech.  But ACPI NVS region is marked as busy already, so that the
> false resource conflict will prevent APEI ERST/EINJ to work.
>
> To fix this, this patch excludes ACPI NVS regions when APEI components
> request resources.  So that they will not conflict with ACPI NVS
> regions.

I think this is much, much too complicated.

Yinghai's three-line e820.c patch to leave ACPI NVS regions in the
iomem_resource tree, but as not busy, is far better.

Bjorn

> Reported-and-tested-by: Pavel Ivanov <paivanof@gmail.com>
> Signed-off-by: Huang Ying <ying.huang@intel.com>
> ---
>  drivers/acpi/apei/apei-base.c |   29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
>
> --- a/drivers/acpi/apei/apei-base.c
> +++ b/drivers/acpi/apei/apei-base.c
> @@ -438,8 +438,19 @@ int apei_resources_sub(struct apei_resou
>  }
>  EXPORT_SYMBOL_GPL(apei_resources_sub);
>
> +static int apei_get_nvs_callback(__u64 start, __u64 size, void *data)
> +{
> +       struct apei_resources *resources = data;
> +       return apei_res_add(&resources->iomem, start, size);
> +}
> +
> +static int apei_get_nvs_resources(struct apei_resources *resources)
> +{
> +       return acpi_nvs_for_each_region(apei_get_nvs_callback, resources);
> +}
> +
>  /*
> - * IO memory/port rersource management mechanism is used to check
> + * IO memory/port resource management mechanism is used to check
>  * whether memory/port area used by GARs conflicts with normal memory
>  * or IO memory/port of devices.
>  */
> @@ -448,12 +459,26 @@ int apei_resources_request(struct apei_r
>  {
>        struct apei_res *res, *res_bak = NULL;
>        struct resource *r;
> +       struct apei_resources nvs_resources;
>        int rc;
>
>        rc = apei_resources_sub(resources, &apei_resources_all);
>        if (rc)
>                return rc;
>
> +       /*
> +        * Some firmware uses ACPI NVS region, that has been marked as
> +        * busy, so exclude it from APEI resources to avoid false
> +        * conflict.
> +        */
> +       apei_resources_init(&nvs_resources);
> +       rc = apei_get_nvs_resources(&nvs_resources);
> +       if (rc)
> +               goto res_fini;
> +       rc = apei_resources_sub(resources, &nvs_resources);
> +       if (rc)
> +               goto res_fini;
> +
>        rc = -EINVAL;
>        list_for_each_entry(res, &resources->iomem, list) {
>                r = request_mem_region(res->start, res->end - res->start,
> @@ -500,6 +525,8 @@ err_unmap_iomem:
>                        break;
>                release_mem_region(res->start, res->end - res->start);
>        }
> +res_fini:
> +       apei_resources_fini(&nvs_resources);
>        return rc;
>  }
>  EXPORT_SYMBOL_GPL(apei_resources_request);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH 4/8] ACPI, APEI, Resolve false conflict between ACPI NVS and APEI

[Huang Ying]

On 09/20/2011 10:09 AM, Bjorn Helgaas wrote:
> On Mon, Sep 19, 2011 at 7:38 PM, Huang Ying <ying.huang@intel.com> wrote:
>> Some firmware will access memory in ACPI NVS region via APEI.  That
>> is, instructions in APEI ERST/EINJ table will read/write ACPI NVS
>> region.  The original resource conflict checking in APEI code will
>> check memory/ioport accessed by APEI via general resource management
>> mech.  But ACPI NVS region is marked as busy already, so that the
>> false resource conflict will prevent APEI ERST/EINJ to work.
>>
>> To fix this, this patch excludes ACPI NVS regions when APEI components
>> request resources.  So that they will not conflict with ACPI NVS
>> regions.
> 
> I think this is much, much too complicated.
> 
> Yinghai's three-line e820.c patch to leave ACPI NVS regions in the
> iomem_resource tree, but as not busy, is far better.

ACPI NVS should only be used by firmware or firmware interpreter instead
of the ordinary drivers.  So I think that is reasonable to make it busy
in iomem resource tree.

Best Regards,
Huang Ying

Re: [PATCH 5/8] ACPI, APEI, EINJ, Fix resource conflict on some machine

[Chen Gong]

于 2011/9/20 9:38, Huang Ying 写道:
> Some APEI firmware implementation will access injected address
> specified in param1 to trigger the error when injecting memory error.
> This will cause resource conflict with RAM.
>
> On one of our testing machine, if injecting at memory address
> 0x10000000, the following error will be reported in dmesg:
>
>    APEI: Can not request iomem region<0000000010000000-0000000010000008>  for GARs.
>
> This patch removes the injecting memory address range from trigger
> table resources to avoid conflict.
>
> Signed-off-by: Huang Ying<ying.huang@intel.com>
> Tested-by: Tony Luck<tony.luck@intel.com>
> ---
>   drivers/acpi/apei/apei-base.c     |   11 +++++++++++
>   drivers/acpi/apei/apei-internal.h |    3 +++
>   drivers/acpi/apei/einj.c          |   24 ++++++++++++++++++++++--
>   3 files changed, 36 insertions(+), 2 deletions(-)
>
> --- a/drivers/acpi/apei/apei-base.c
> +++ b/drivers/acpi/apei/apei-base.c
> @@ -421,6 +421,17 @@ static int apei_resources_merge(struct a
>   	return 0;
>   }
>
> +int apei_resources_add(struct apei_resources *resources,
> +		       unsigned long start, unsigned long size,
> +		       bool iomem)
> +{
> +	if (iomem)
> +		return apei_res_add(&resources->iomem, start, size);
> +	else
> +		return apei_res_add(&resources->ioport, start, size);
> +}
> +EXPORT_SYMBOL_GPL(apei_resources_add);
> +
>   /*
>    * EINJ has two groups of GARs (EINJ table entry and trigger table
>    * entry), so common resources are subtracted from the trigger table
> --- a/drivers/acpi/apei/apei-internal.h
> +++ b/drivers/acpi/apei/apei-internal.h
> @@ -95,6 +95,9 @@ static inline void apei_resources_init(s
>   }
>
>   void apei_resources_fini(struct apei_resources *resources);
> +int apei_resources_add(struct apei_resources *resources,
> +		       unsigned long start, unsigned long size,
> +		       bool iomem);
>   int apei_resources_sub(struct apei_resources *resources1,
>   		       struct apei_resources *resources2);
>   int apei_resources_request(struct apei_resources *resources,
> --- a/drivers/acpi/apei/einj.c
> +++ b/drivers/acpi/apei/einj.c
> @@ -195,7 +195,8 @@ static int einj_check_trigger_header(str
>   }
>
>   /* Execute instructions in trigger error action table */
> -static int __einj_error_trigger(u64 trigger_paddr)
> +static int __einj_error_trigger(u64 trigger_paddr, u32 type,
> +				u64 param1, u64 param2)
>   {
>   	struct acpi_einj_trigger *trigger_tab = NULL;
>   	struct apei_exec_context trigger_ctx;
> @@ -256,6 +257,25 @@ static int __einj_error_trigger(u64 trig
>   	rc = apei_resources_sub(&trigger_resources,&einj_resources);
>   	if (rc)
>   		goto out_fini;
> +	/*
> +	 * Some firmware will access target address specified in
> +	 * param1 to trigger the error when injecting memory error.
> +	 * This will cause resource conflict with regular memory.  So
> +	 * remove it from trigger table resources.
> +	 */
> +	if (param_extension&&  (type&  0x0038)&&  param2) {
> +		struct apei_resources addr_resources;
> +		apei_resources_init(&addr_resources);
> +		rc = apei_resources_add(&addr_resources,
> +					param1&  param2,
> +					~param2 + 1, true);
> +		if (rc)
> +			goto out_fini;
> +		rc = apei_resources_sub(&trigger_resources,&addr_resources);
> +		apei_resources_fini(&addr_resources);
> +		if (rc)
> +			goto out_fini;
> +	}
>   	rc = apei_resources_request(&trigger_resources, "APEI EINJ Trigger");
>   	if (rc)
>   		goto out_fini;
> @@ -325,7 +345,7 @@ static int __einj_error_inject(u32 type,
>   	if (rc)
>   		return rc;
>   	trigger_paddr = apei_exec_ctx_get_output(&ctx);
> -	rc = __einj_error_trigger(trigger_paddr);
> +	rc = __einj_error_trigger(trigger_paddr, type, param1, param2);
>   	if (rc)
>   		return rc;
>   	rc = apei_exec_run_optional(&ctx, ACPI_EINJ_END_OPERATION);
> --
Hi, Ying

Would you please add Hui's patch
(http://marc.info/?l=linux-acpi&m=131616383227196&w=2) between patch 5 
and patch 6?

Thx a lot!

Re: [PATCH 4/8] ACPI, APEI, Resolve false conflict between ACPI NVS and APEI

[Bjorn Helgaas]

On Mon, Sep 19, 2011 at 8:34 PM, Huang Ying <ying.huang@intel.com> wrote:
> On 09/20/2011 10:09 AM, Bjorn Helgaas wrote:
>> On Mon, Sep 19, 2011 at 7:38 PM, Huang Ying <ying.huang@intel.com> wrote:
>>> Some firmware will access memory in ACPI NVS region via APEI.  That
>>> is, instructions in APEI ERST/EINJ table will read/write ACPI NVS
>>> region.  The original resource conflict checking in APEI code will
>>> check memory/ioport accessed by APEI via general resource management
>>> mech.  But ACPI NVS region is marked as busy already, so that the
>>> false resource conflict will prevent APEI ERST/EINJ to work.
>>>
>>> To fix this, this patch excludes ACPI NVS regions when APEI components
>>> request resources.  So that they will not conflict with ACPI NVS
>>> regions.
>>
>> I think this is much, much too complicated.
>>
>> Yinghai's three-line e820.c patch to leave ACPI NVS regions in the
>> iomem_resource tree, but as not busy, is far better.
>
> ACPI NVS should only be used by firmware or firmware interpreter instead
> of the ordinary drivers.  So I think that is reasonable to make it busy
> in iomem resource tree.

"My driver is not like ordinary drivers" is a common excuse for adding
special cases. I don't buy it.

These patches (3 and 4) add a lot of complexity but I don't believe
they add any real protection.

Regions are marked busy by their owners, i.e., by drivers that claim
devices and know how to operate them.  The e820 code is not an owner
of ACPI NVS regions, so it should not mark them busy.

I don't really think we have a problem here that needs to be solved.
Ordinary drivers have no way of learning an address in ACPI NVS, so
they aren't even going to try to use it.

Bjorn

Re: [PATCH 4/8] ACPI, APEI, Resolve false conflict between ACPI NVS and APEI

[Huang Ying]

On 09/21/2011 12:26 AM, Bjorn Helgaas wrote:
> On Mon, Sep 19, 2011 at 8:34 PM, Huang Ying <ying.huang@intel.com> wrote:
>> On 09/20/2011 10:09 AM, Bjorn Helgaas wrote:
>>> On Mon, Sep 19, 2011 at 7:38 PM, Huang Ying <ying.huang@intel.com> wrote:
>>>> Some firmware will access memory in ACPI NVS region via APEI.  That
>>>> is, instructions in APEI ERST/EINJ table will read/write ACPI NVS
>>>> region.  The original resource conflict checking in APEI code will
>>>> check memory/ioport accessed by APEI via general resource management
>>>> mech.  But ACPI NVS region is marked as busy already, so that the
>>>> false resource conflict will prevent APEI ERST/EINJ to work.
>>>>
>>>> To fix this, this patch excludes ACPI NVS regions when APEI components
>>>> request resources.  So that they will not conflict with ACPI NVS
>>>> regions.
>>>
>>> I think this is much, much too complicated.
>>>
>>> Yinghai's three-line e820.c patch to leave ACPI NVS regions in the
>>> iomem_resource tree, but as not busy, is far better.
>>
>> ACPI NVS should only be used by firmware or firmware interpreter instead
>> of the ordinary drivers.  So I think that is reasonable to make it busy
>> in iomem resource tree.
> 
> "My driver is not like ordinary drivers" is a common excuse for adding
> special cases. I don't buy it.
> 
> These patches (3 and 4) add a lot of complexity but I don't believe
> they add any real protection.
> 
> Regions are marked busy by their owners, i.e., by drivers that claim
> devices and know how to operate them.  The e820 code is not an owner
> of ACPI NVS regions, so it should not mark them busy.
> 
> I don't really think we have a problem here that needs to be solved.
> Ordinary drivers have no way of learning an address in ACPI NVS, so
> they aren't even going to try to use it.

So what resource conflict checking is for?  If something wrong with
driver configuration, resource description in ACPI table etc, the driver
may request iomem inside ACPI NVS regions.

ACPI NVS regions already have a user, that is the ACPI AML interpreter,
so it is always busy.

Best Regards,
Huang Ying

Re: [PATCH 4/8] ACPI, APEI, Resolve false conflict between ACPI NVS and APEI

[Bjorn Helgaas]

On Tue, Sep 20, 2011 at 7:43 PM, Huang Ying <ying.huang@intel.com> wrote:
> On 09/21/2011 12:26 AM, Bjorn Helgaas wrote:
>> On Mon, Sep 19, 2011 at 8:34 PM, Huang Ying <ying.huang@intel.com> wrote:
>>> On 09/20/2011 10:09 AM, Bjorn Helgaas wrote:
>>>> On Mon, Sep 19, 2011 at 7:38 PM, Huang Ying <ying.huang@intel.com> wrote:
>>>>> Some firmware will access memory in ACPI NVS region via APEI.  That
>>>>> is, instructions in APEI ERST/EINJ table will read/write ACPI NVS
>>>>> region.  The original resource conflict checking in APEI code will
>>>>> check memory/ioport accessed by APEI via general resource management
>>>>> mech.  But ACPI NVS region is marked as busy already, so that the
>>>>> false resource conflict will prevent APEI ERST/EINJ to work.
>>>>>
>>>>> To fix this, this patch excludes ACPI NVS regions when APEI components
>>>>> request resources.  So that they will not conflict with ACPI NVS
>>>>> regions.
>>>>
>>>> I think this is much, much too complicated.
>>>>
>>>> Yinghai's three-line e820.c patch to leave ACPI NVS regions in the
>>>> iomem_resource tree, but as not busy, is far better.
>>>
>>> ACPI NVS should only be used by firmware or firmware interpreter instead
>>> of the ordinary drivers.  So I think that is reasonable to make it busy
>>> in iomem resource tree.
>>
>> "My driver is not like ordinary drivers" is a common excuse for adding
>> special cases. I don't buy it.
>>
>> These patches (3 and 4) add a lot of complexity but I don't believe
>> they add any real protection.
>>
>> Regions are marked busy by their owners, i.e., by drivers that claim
>> devices and know how to operate them.  The e820 code is not an owner
>> of ACPI NVS regions, so it should not mark them busy.
>>
>> I don't really think we have a problem here that needs to be solved.
>> Ordinary drivers have no way of learning an address in ACPI NVS, so
>> they aren't even going to try to use it.
>
> So what resource conflict checking is for?  If something wrong with
> driver configuration, resource description in ACPI table etc, the driver
> may request iomem inside ACPI NVS regions.
>
> ACPI NVS regions already have a user, that is the ACPI AML interpreter,
> so it is always busy.

If the AML interpreter is the user, *it* should mark the regions busy, not e820.

Bjorn