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

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

[Huang Ying]

Resend it because of kernel.org failure before.

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

[PATCH 1/9] 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/9] 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
@@ -1112,10 +1112,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/9] 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
@@ -714,7 +714,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.
@@ -727,7 +727,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*/

Re: [PATCH 3/9] ACPI, Record ACPI NVS regions

[Bjorn Helgaas]

On Mon, Nov 7, 2011 at 7:39 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
> 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.

I don't really like these patches (3 & 4), as I mentioned before:
  https://lkml.org/lkml/2011/8/22/270
  https://lkml.org/lkml/2011/9/19/397

To recap, e820 currently marks everything except E820_RESERVED areas
as "BUSY" in the iomem_resource tree.  That basically means that RAM,
ACPI, and NVS are marked as BUSY.  Later APEI attempts a
request_mem_region() on a a resource, and it fails because e820 has
already marked it BUSY.

It makes sense for e820 to mark RAM as BUSY, because we we don't use
iomem_resource to manage it; we pass it all off to memblock and
eventually the regular memory allocator.  But from e820's point of
view, everything else (RESERVED, ACPI, NVS, etc) is just a clue  that
something uses that region and we shouldn't place anything else on top
of it.

We normally mark something BUSY when a driver calls request_region()
to indicate that it owns the region and any other requests for the
same region should fail.  I think it's a mistake for e820 to mark
things BUSY because it really is not the the owner of any of those
regions (except for RAM, because in that case e820 *is* essentially
taking ownership of RAM and passing it off to memblock).

I think a better solution for the APEI problem would be to have e820
leave non-RAM regions in iomem_resource, but not mark them BUSY, as in
this patch from Yinghai:

--- linux-2.6.orig/arch/x86/kernel/e820.c
+++ linux-2.6/arch/x86/kernel/e820.c
@@ -995,7 +995,8 @@ void __init e820_reserve_resources(void)
 		 * pcibios_resource_survey()
 		 */
 		if (e820.map[i].type != E820_RESERVED || res->start < (1ULL<<20)) {
-			res->flags |= IORESOURCE_BUSY;
+			if (e820.map[i].type != E820_NVS)
+				res->flags |= IORESOURCE_BUSY;
 			insert_resource(&iomem_resource, res);
 		}
 		res++;

That's much simpler than having e820 pretend to be the owner of NVS
regions, building an extra list of the NVS regions, and subtracting
them from the regions APEI needs to request ownership of.

Bjorn

> 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
> @@ -714,7 +714,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.
> @@ -727,7 +727,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*/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH 4/9] 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);

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

[Thomas Renninger]

Hi,

On Tuesday 08 November 2011 03:39:29 Huang Ying 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.

find below an approach which still allows requesting the resources,
similar to Yinghai's patch.
But instead of allowing everybody to access NVS memory, it needs to
be explicitly requested.

Compile tested only.

Also this is a bit hacky, as it reserves the last free bits in the
the generic resource flags for X86 only.

I fear doing it properly needs quite some more efforts.

   Thomas

---
 arch/x86/kernel/e820.c        |   30 ++++++++++++++++++++++--------
 drivers/acpi/apei/apei-base.c |    4 ++--
 include/linux/ioport.h        |    5 +++++
 kernel/resource.c             |    5 ++++-
 4 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 303a0e4..4b1b15f 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -958,15 +958,29 @@ void __init e820_reserve_resources(void)
 
 		res->flags = IORESOURCE_MEM;
 
-		/*
-		 * don't register the region that could be conflicted with
-		 * pci device BAR resource and insert them later in
-		 * pcibios_resource_survey()
-		 */
-		if (e820.map[i].type != E820_RESERVED || res->start < (1ULL<<20)) {
+		switch (e820.map[i].type) {
+		case E820_RESERVED:
+			/*
+			 * don't register the region that could be conflicted
+			 * with pci device BAR resource and insert them later in
+			 * pcibios_resource_survey()
+			 */
+			if (res->start < (1ULL<<20))
+				res->flags |= IORESOURCE_BUSY;
+			res->flags |= IORESOURCE_RESERVED_RAM;
+			break;
+		case E820_ACPI:
+			res->flags |= IORESOURCE_ACPI_RAM;
 			res->flags |= IORESOURCE_BUSY;
-			insert_resource(&iomem_resource, res);
-		}
+			break;
+		case E820_NVS:
+			res->flags |= IORESOURCE_NVS_RAM;
+			res->flags |= IORESOURCE_BUSY;
+			break;
+		default:
+			res->flags |= IORESOURCE_BUSY;
+		}			
+		insert_resource(&iomem_resource, res);
 		res++;
 	}
 
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 6154036..a86aa24 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -456,8 +456,8 @@ int apei_resources_request(struct apei_resources *resources,
 
 	rc = -EINVAL;
 	list_for_each_entry(res, &resources->iomem, list) {
-		r = request_mem_region(res->start, res->end - res->start,
-				       desc);
+		r = __request_mem_region(res->start, res->end - res->start,
+					 desc, IORESOURCE_NVS_RAM);
 		if (!r) {
 			pr_err(APEI_PFX
 		"Can not request iomem region <%016llx-%016llx> for GARs.\n",
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 9d57a71..2c1b3db 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -54,6 +54,11 @@ struct resource_list {
 #define IORESOURCE_WINDOW	0x00200000	/* forwarded by bridge */
 #define IORESOURCE_MUXED	0x00400000	/* Resource is software muxed */
 
+/* X86 specific e820 marked memory resources */
+#define IORESOURCE_NVS_RAM      0x01000000
+#define IORESOURCE_ACPI_RAM     0x02000000
+#define IORESOURCE_RESERVED_RAM 0x04000000
+
 #define IORESOURCE_EXCLUSIVE	0x08000000	/* Userland may not map this resource */
 #define IORESOURCE_DISABLED	0x10000000
 #define IORESOURCE_UNSET	0x20000000
diff --git a/kernel/resource.c b/kernel/resource.c
index 7640b3a..4397543 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -861,7 +861,10 @@ struct resource * __request_region(struct resource *parent,
 			break;
 		if (conflict != parent) {
 			parent = conflict;
-			if (!(conflict->flags & IORESOURCE_BUSY))
+			if (!(conflict->flags & IORESOURCE_BUSY) ||
+			    /* Even the region is busy, it was explicitly
+			       requested -> allow access */
+			    flags & conflict->flags)
 				continue;
 		}
 		if (conflict->flags & flags & IORESOURCE_MUXED) {

[PATCH 5/9] 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/9] 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);
 }
 

Re: [PATCH 6/9] ACPI, Add RAM mapping support to ACPI atomic IO support

[Myron Stowe]

On Mon, Nov 7, 2011 at 7:39 PM, Huang Ying <ying.huang@intel.com> wrote:
> 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(-)
>

Hi Huang:

This patch collides with my series to remove atomicio.[ch]:
  https://mail.google.com/mail/?shva=1#label/linux-acpi+list/133805812264a542
and I don't want such functionality to get lost if/when the removal series
is accepted.  I'm interested in working with you, not against you, so would
you like me to do anything with respect to this patch within the osl.c based
mapping routines so this capability would not become lost?

The obvious choices would be for me to post a new patch, copying this
functionality into osl.c's mapping routines, that would apply on top of the
removal series.  Another tact could be for me to do basically the same
thing but within the removal series (by adding this into it, and reposting).
The latter tactic seems like it could be a constant problem if more changes
to atomicio occur during this interim period.  Of course you may have other
ideas here as how to progress.

This type of occurrence, among many others, is a good example of why we
need to get down to just one set of mapping routines as soon as possible.
During this interim period I'll try and monitor the linux-acpi-list
for future such
occurrences but if you could, please try and cc me on any future atomicio
modifications.

Thanks,
 Myron

> --- 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);
>  }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] ACPI, Add RAM mapping support to ACPI atomic IO support

[Myron Stowe]

On Mon, Nov 7, 2011 at 7:39 PM, Huang Ying <ying.huang@intel.com> wrote:
> 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(-)
>

Hi Huang:

This patch collides with my series to remove atomicio.[ch]:
  https://mail.google.com/mail/?shva=1#label/linux-acpi+list/133805812264a542
and I don't want such functionality to get lost if/when the removal series
is accepted.  I'm interested in working with you, not against you, so would
you like me to do anything with respect to this patch within the osl.c based
mapping routines so this capability would not become lost?

The obvious choices would be for me to post a new patch, copying this
functionality into osl.c's mapping routines, that would apply on top of the
removal series.  Another tact could be for me to do basically the same
thing but within the removal series (by adding this into it, and reposting).
The latter tactic seems like it could be a constant problem if more changes
to atomicio occur during this interim period.  Of course you may have other
ideas here as how to progress.

This type of occurrence, among many others, is a good example of why we
need to get down to just one set of mapping routines as soon as possible.
During this interim period I'll try and monitor the linux-acpi-list
for future such
occurrences but if you could, please try and cc me on any future atomicio
modifications.

Thanks,
 Myron

> --- 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);
>  }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 6/9] ACPI, Add RAM mapping support to ACPI atomic IO support

[Len Brown]

On 11/08/2011 04:38 PM, Myron Stowe wrote:

> On Mon, Nov 7, 2011 at 7:39 PM, Huang Ying <ying.huang@intel.com> wrote:
>> 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(-)
>>
> 
> Hi Huang:
> 
> This patch collides with my series to remove atomicio.[ch]:
>   https://mail.google.com/mail/?shva=1#label/linux-acpi+list/133805812264a542
> and I don't want such functionality to get lost if/when the removal series
> is accepted.  I'm interested in working with you, not against you, so would
> you like me to do anything with respect to this patch within the osl.c based
> mapping routines so this capability would not become lost?
> 
> The obvious choices would be for me to post a new patch, copying this
> functionality into osl.c's mapping routines, that would apply on top of the
> removal series.  Another tact could be for me to do basically the same
> thing but within the removal series (by adding this into it, and reposting).
> The latter tactic seems like it could be a constant problem if more changes
> to atomicio occur during this interim period.  Of course you may have other
> ideas here as how to progress.
> 
> This type of occurrence, among many others, is a good example of why we
> need to get down to just one set of mapping routines as soon as possible.
> During this interim period I'll try and monitor the linux-acpi-list
> for future such
> occurrences but if you could, please try and cc me on any future atomicio
> modifications.
> 
> Thanks,
>  Myron


Hi Myron,
I've included your 1-3 of "ACPI: Re-factor and remove ./drivers/acpi/atomicio.[ch]"
on top of Ying's series "[PATCH 00/11] ACPI, APEI, Patches for 3.3"

Can you re-fresh your #4 so it applies on top?
Would be delightful to get rid of atomicio.

You can pull my branch from
git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git next

thanks!
-Len



thanks,
-Len

[PATCH 7/9] ACPI, APEI, EINJ, Refine the fix of resource conflict

[Huang Ying]

From: "Xiao, Hui" <hui.xiao@linux.intel.com>

Current fix for resource conflict is to remove the address region <param1 &
param2, ~param2+1> from trigger resource, which is highly relies on valid user
input. This patch is trying to avoid such potential issues by fetching the
exact address region from trigger action table entry.

Signed-off-by: Xiao, Hui <hui.xiao@linux.intel.com>
Signed-off-by: Huang Ying <ying.huang@intel.com>
---
 drivers/acpi/apei/einj.c |   38 ++++++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -194,6 +194,26 @@ static int einj_check_trigger_header(str
 	return 0;
 }
 
+static struct acpi_generic_address *einj_get_trigger_parameter_region(
+	struct acpi_einj_trigger *trigger_tab, u64 param1, u64 param2)
+{
+	int i;
+	struct acpi_whea_header *entry;
+
+	entry = (struct acpi_whea_header *)
+		((char *)trigger_tab + sizeof(struct acpi_einj_trigger));
+	for (i = 0; i < trigger_tab->entry_count; i++) {
+		if (entry->action == ACPI_EINJ_TRIGGER_ERROR &&
+		entry->instruction == ACPI_EINJ_WRITE_REGISTER_VALUE &&
+		entry->register_region.space_id ==
+			ACPI_ADR_SPACE_SYSTEM_MEMORY &&
+		(entry->register_region.address & param2) == (param1 & param2))
+			return &entry->register_region;
+		entry++;
+	}
+
+	return NULL;
+}
 /* Execute instructions in trigger error action table */
 static int __einj_error_trigger(u64 trigger_paddr, u32 type,
 				u64 param1, u64 param2)
@@ -205,6 +225,7 @@ static int __einj_error_trigger(u64 trig
 	struct resource *r;
 	u32 table_size;
 	int rc = -EIO;
+	struct acpi_generic_address *trigger_param_region = NULL;
 
 	r = request_mem_region(trigger_paddr, sizeof(*trigger_tab),
 			       "APEI EINJ Trigger Table");
@@ -266,12 +287,17 @@ static int __einj_error_trigger(u64 trig
 	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);
+		trigger_param_region = einj_get_trigger_parameter_region(
+			trigger_tab, param1, param2);
+		if (trigger_param_region) {
+			rc = apei_resources_add(&addr_resources,
+				trigger_param_region->address,
+				trigger_param_region->bit_width/8, true);
+			if (rc)
+				goto out_fini;
+			rc = apei_resources_sub(&trigger_resources,
+					&addr_resources);
+		}
 		apei_resources_fini(&addr_resources);
 		if (rc)
 			goto out_fini;

[PATCH 8/9] 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>
@@ -476,6 +478,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 9/9] 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
@@ -506,16 +506,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,
@@ -798,7 +804,7 @@ static int ghes_notify_nmi(unsigned int
 
 	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)