From: "Chen, Gong" <[email protected]>
Some BIOSes utilize PCI MMCFG space read/write opertion to trigger
specific errors. EINJ will report errors as below when hitting such
cases:
APEI: Can not request [mem 0x83f990a0-0x83f990a3] for APEI EINJ Trigger registers
It is because on x86 platform ACPI based PCI MMCFG logic has
reserved all MMCFG spaces so that EINJ can't reserve it again.
We already trust the ACPI/APEI code when using the EINJ interface
so it is not a big leap to also trust it to access the right
MMCFG addresses. Skip address checking to allow the access.
Signed-off-by: Chen, Gong <[email protected]>
Signed-off-by: Tony Luck <[email protected]>
---
drivers/acpi/apei/apei-base.c | 58 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 52 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 2cd7bdd6c8b3..e809d3c4904d 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -41,6 +41,9 @@
#include <linux/interrupt.h>
#include <linux/debugfs.h>
#include <asm/unaligned.h>
+#ifdef CONFIG_PCI_MMCONFIG
+#include <asm/pci_x86.h>
+#endif
#include "apei-internal.h"
@@ -449,7 +452,7 @@ int apei_resources_sub(struct apei_resources *resources1,
}
EXPORT_SYMBOL_GPL(apei_resources_sub);
-static int apei_get_nvs_callback(__u64 start, __u64 size, void *data)
+static int apei_get_res_callback(__u64 start, __u64 size, void *data)
{
struct apei_resources *resources = data;
return apei_res_add(&resources->iomem, start, size);
@@ -457,9 +460,42 @@ static int apei_get_nvs_callback(__u64 start, __u64 size, void *data)
static int apei_get_nvs_resources(struct apei_resources *resources)
{
- return acpi_nvs_for_each_region(apei_get_nvs_callback, resources);
+ return acpi_nvs_for_each_region(apei_get_res_callback, resources);
}
+#ifdef CONFIG_PCI_MMCONFIG
+static int pci_mmcfg_for_each_region(int (*func)(__u64 start, __u64 size,
+ void *data), void *data)
+{
+ struct pci_mmcfg_region *cfg;
+ int rc;
+
+ if ((pci_probe & PCI_PROBE_MMCONF) == 0)
+ return 0;
+
+ if (list_empty(&pci_mmcfg_list))
+ return 0;
+
+ list_for_each_entry(cfg, &pci_mmcfg_list, list) {
+ rc = func(cfg->res.start, resource_size(&cfg->res), data);
+ if (rc)
+ return rc;
+ }
+
+ return 0;
+}
+
+static int apei_get_mmcfg_resources(struct apei_resources *resources)
+{
+ return pci_mmcfg_for_each_region(apei_get_res_callback, resources);
+}
+#else
+static int apei_get_mmcfg_resources(struct apei_resources *resources)
+{
+ return 0;
+}
+#endif
+
/*
* IO memory/port resource management mechanism is used to check
* whether memory/port area used by GARs conflicts with normal memory
@@ -470,7 +506,7 @@ int apei_resources_request(struct apei_resources *resources,
{
struct apei_res *res, *res_bak = NULL;
struct resource *r;
- struct apei_resources nvs_resources;
+ struct apei_resources nvs_resources, mmcfg_res;
int rc;
rc = apei_resources_sub(resources, &apei_resources_all);
@@ -485,10 +521,18 @@ int apei_resources_request(struct apei_resources *resources,
apei_resources_init(&nvs_resources);
rc = apei_get_nvs_resources(&nvs_resources);
if (rc)
- goto res_fini;
+ goto nvs_res_fini;
rc = apei_resources_sub(resources, &nvs_resources);
if (rc)
- goto res_fini;
+ goto nvs_res_fini;
+
+ apei_resources_init(&mmcfg_res);
+ rc = apei_get_mmcfg_resources(&mmcfg_res);
+ if (rc)
+ goto mmcfg_res_fini;
+ rc = apei_resources_sub(resources, &mmcfg_res);
+ if (rc)
+ goto mmcfg_res_fini;
rc = -EINVAL;
list_for_each_entry(res, &resources->iomem, list) {
@@ -536,7 +580,9 @@ err_unmap_iomem:
break;
release_mem_region(res->start, res->end - res->start);
}
-res_fini:
+mmcfg_res_fini:
+ apei_resources_fini(&mmcfg_res);
+nvs_res_fini:
apei_resources_fini(&nvs_resources);
return rc;
}
--
2.1.0
W dniu 03.12.2014 o 22:22, Luck, Tony pisze:
> From: "Chen, Gong" <[email protected]>
>
> Some BIOSes utilize PCI MMCFG space read/write opertion to trigger
> specific errors. EINJ will report errors as below when hitting such
> cases:
>
> APEI: Can not request [mem 0x83f990a0-0x83f990a3] for APEI EINJ Trigger registers
>
> It is because on x86 platform ACPI based PCI MMCFG logic has
> reserved all MMCFG spaces so that EINJ can't reserve it again.
> We already trust the ACPI/APEI code when using the EINJ interface
> so it is not a big leap to also trust it to access the right
> MMCFG addresses. Skip address checking to allow the access.
>
> Signed-off-by: Chen, Gong <[email protected]>
> Signed-off-by: Tony Luck <[email protected]>
> ---
> drivers/acpi/apei/apei-base.c | 58 ++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 52 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
> index 2cd7bdd6c8b3..e809d3c4904d 100644
> --- a/drivers/acpi/apei/apei-base.c
> +++ b/drivers/acpi/apei/apei-base.c
> @@ -41,6 +41,9 @@
> #include <linux/interrupt.h>
> #include <linux/debugfs.h>
> #include <asm/unaligned.h>
> +#ifdef CONFIG_PCI_MMCONFIG
> +#include <asm/pci_x86.h>
> +#endif
>
> #include "apei-internal.h"
>
> @@ -449,7 +452,7 @@ int apei_resources_sub(struct apei_resources *resources1,
> }
> EXPORT_SYMBOL_GPL(apei_resources_sub);
>
> -static int apei_get_nvs_callback(__u64 start, __u64 size, void *data)
> +static int apei_get_res_callback(__u64 start, __u64 size, void *data)
> {
> struct apei_resources *resources = data;
> return apei_res_add(&resources->iomem, start, size);
> @@ -457,9 +460,42 @@ static int apei_get_nvs_callback(__u64 start, __u64 size, void *data)
>
> static int apei_get_nvs_resources(struct apei_resources *resources)
> {
> - return acpi_nvs_for_each_region(apei_get_nvs_callback, resources);
> + return acpi_nvs_for_each_region(apei_get_res_callback, resources);
> }
>
> +#ifdef CONFIG_PCI_MMCONFIG
> +static int pci_mmcfg_for_each_region(int (*func)(__u64 start, __u64 size,
> + void *data), void *data)
> +{
> + struct pci_mmcfg_region *cfg;
> + int rc;
> +
> + if ((pci_probe & PCI_PROBE_MMCONF) == 0)
> + return 0;
> +
> + if (list_empty(&pci_mmcfg_list))
> + return 0;
> +
> + list_for_each_entry(cfg, &pci_mmcfg_list, list) {
> + rc = func(cfg->res.start, resource_size(&cfg->res), data);
> + if (rc)
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +static int apei_get_mmcfg_resources(struct apei_resources *resources)
> +{
> + return pci_mmcfg_for_each_region(apei_get_res_callback, resources);
> +}
> +#else
> +static int apei_get_mmcfg_resources(struct apei_resources *resources)
> +{
> + return 0;
> +}
> +#endif
> +
If this is the case for x86 only, IMHO it should be wrapped into:
#if CONFIG_X86 && CONFIG_PCI_MMCONFIG or something similar.
If it solves problem related to ACPI, you should not use in this file:
> + if ((pci_probe & PCI_PROBE_MMCONF) == 0)
> + return 0;
This is very x86 specific. We are making a lot of effort to make
MMCONFIG platform independent now.
Regards,
Tomasz
> If it solves problem related to ACPI, you should not use in this file:
> > + if ((pci_probe & PCI_PROBE_MMCONF) == 0)
> > + return 0;
> This is very x86 specific. We are making a lot of effort to make
> MMCONFIG platform independent now.
So you'd like to see the changes in this patch spread out across more
appropriate files? pci_mmcfg_for_each_region() moving off into
arch/x86/pci/mmconfig-shared.c, for example.
I'm not sure how to make that pretty in apei_resources_request()
where essentially we need to add an extra scan for some arch
specific addresses to ignore from the list of addresses.
-Tony
From: "Chen, Gong" <[email protected]>
Some BIOSes utilize PCI MMCFG space read/write opertion to trigger
specific errors. EINJ will report errors as below when hitting such
cases:
APEI: Can not request [mem 0x83f990a0-0x83f990a3] for APEI EINJ Trigger registers
It is because on x86 platform ACPI based PCI MMCFG logic has
reserved all MMCFG spaces so that EINJ can't reserve it again.
We already trust the ACPI/APEI code when using the EINJ interface
so it is not a big leap to also trust it to access the right
MMCFG addresses. Skip address checking to allow the access.
Signed-off-by: Chen, Gong <[email protected]>
Signed-off-by: Tony Luck <[email protected]>
---
Chen Gong still gets author credit for this. All I did was rename
some "mmcfg" things to be "arch", and move the mmcfg scan routine
out of apei.c and into the x86 specific mmconfig-shared.c to
address Tomasz's concerns.
arch/x86/pci/mmconfig-shared.c | 28 ++++++++++++++++++++++++++++
drivers/acpi/apei/apei-base.c | 35 +++++++++++++++++++++++++++++------
2 files changed, 57 insertions(+), 6 deletions(-)
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 326198a4434e..c08b985f9441 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -610,6 +610,32 @@ static int __init pci_parse_mcfg(struct acpi_table_header *header)
return 0;
}
+#ifdef CONFIG_ACPI_APEI
+extern int (*arch_apei_filter_addr)(int (*func)(__u64 start, __u64 size,
+ void *data), void *data);
+
+static int pci_mmcfg_for_each_region(int (*func)(__u64 start, __u64 size,
+ void *data), void *data)
+{
+ struct pci_mmcfg_region *cfg;
+ int rc;
+
+ if (list_empty(&pci_mmcfg_list))
+ return 0;
+
+ list_for_each_entry(cfg, &pci_mmcfg_list, list) {
+ rc = func(cfg->res.start, resource_size(&cfg->res), data);
+ if (rc)
+ return rc;
+ }
+
+ return 0;
+}
+#define set_apei_filter() (arch_apei_filter_addr = pci_mmcfg_for_each_region)
+#else
+#define set_apei_filter()
+#endif
+
static void __init __pci_mmcfg_init(int early)
{
pci_mmcfg_reject_broken(early);
@@ -644,6 +670,8 @@ void __init pci_mmcfg_early_init(void)
else
acpi_sfi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg);
__pci_mmcfg_init(1);
+
+ set_apei_filter();
}
}
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 2cd7bdd6c8b3..f74779b1e035 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -41,6 +41,9 @@
#include <linux/interrupt.h>
#include <linux/debugfs.h>
#include <asm/unaligned.h>
+#ifdef CONFIG_PCI_MMCONFIG
+#include <asm/pci_x86.h>
+#endif
#include "apei-internal.h"
@@ -449,7 +452,7 @@ int apei_resources_sub(struct apei_resources *resources1,
}
EXPORT_SYMBOL_GPL(apei_resources_sub);
-static int apei_get_nvs_callback(__u64 start, __u64 size, void *data)
+static int apei_get_res_callback(__u64 start, __u64 size, void *data)
{
struct apei_resources *resources = data;
return apei_res_add(&resources->iomem, start, size);
@@ -457,7 +460,15 @@ static int apei_get_nvs_callback(__u64 start, __u64 size, void *data)
static int apei_get_nvs_resources(struct apei_resources *resources)
{
- return acpi_nvs_for_each_region(apei_get_nvs_callback, resources);
+ return acpi_nvs_for_each_region(apei_get_res_callback, resources);
+}
+
+int (*arch_apei_filter_addr)(int (*func)(__u64 start, __u64 size,
+ void *data), void *data);
+static int apei_get_arch_resources(struct apei_resources *resources)
+
+{
+ return arch_apei_filter_addr(apei_get_res_callback, resources);
}
/*
@@ -470,7 +481,7 @@ int apei_resources_request(struct apei_resources *resources,
{
struct apei_res *res, *res_bak = NULL;
struct resource *r;
- struct apei_resources nvs_resources;
+ struct apei_resources nvs_resources, arch_res;
int rc;
rc = apei_resources_sub(resources, &apei_resources_all);
@@ -485,10 +496,20 @@ int apei_resources_request(struct apei_resources *resources,
apei_resources_init(&nvs_resources);
rc = apei_get_nvs_resources(&nvs_resources);
if (rc)
- goto res_fini;
+ goto nvs_res_fini;
rc = apei_resources_sub(resources, &nvs_resources);
if (rc)
- goto res_fini;
+ goto nvs_res_fini;
+
+ if (arch_apei_filter_addr) {
+ apei_resources_init(&arch_res);
+ rc = apei_get_arch_resources(&arch_res);
+ if (rc)
+ goto arch_res_fini;
+ rc = apei_resources_sub(resources, &arch_res);
+ if (rc)
+ goto arch_res_fini;
+ }
rc = -EINVAL;
list_for_each_entry(res, &resources->iomem, list) {
@@ -536,7 +557,9 @@ err_unmap_iomem:
break;
release_mem_region(res->start, res->end - res->start);
}
-res_fini:
+arch_res_fini:
+ apei_resources_fini(&arch_res);
+nvs_res_fini:
apei_resources_fini(&nvs_resources);
return rc;
}
--
2.1.0
From: "Chen, Gong" <[email protected]>
Some BIOSes utilize PCI MMCFG space read/write opertion to trigger
specific errors. EINJ will report errors as below when hitting such
cases:
APEI: Can not request [mem 0x83f990a0-0x83f990a3] for APEI EINJ Trigger registers
It is because on x86 platform ACPI based PCI MMCFG logic has
reserved all MMCFG spaces so that EINJ can't reserve it again.
We already trust the ACPI/APEI code when using the EINJ interface
so it is not a big leap to also trust it to access the right
MMCFG addresses. Skip address checking to allow the access.
Signed-off-by: Chen, Gong <[email protected]>
Signed-off-by: Tony Luck <[email protected]>
---
v3: Same as v2, except dropped the
+#ifdef CONFIG_PCI_MMCONFIG
+#include <asm/pci_x86.h>
+#endif
hunk from apei-base.c as it is not needed
arch/x86/pci/mmconfig-shared.c | 28 ++++++++++++++++++++++++++++
drivers/acpi/apei/apei-base.c | 32 ++++++++++++++++++++++++++------
2 files changed, 54 insertions(+), 6 deletions(-)
diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 326198a4434e..676e5e04e4d4 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -610,6 +610,32 @@ static int __init pci_parse_mcfg(struct acpi_table_header *header)
return 0;
}
+#ifdef CONFIG_ACPI_APEI
+extern int (*arch_apei_filter_addr)(int (*func)(__u64 start, __u64 size,
+ void *data), void *data);
+
+static int pci_mmcfg_for_each_region(int (*func)(__u64 start, __u64 size,
+ void *data), void *data)
+{
+ struct pci_mmcfg_region *cfg;
+ int rc;
+
+ if (list_empty(&pci_mmcfg_list))
+ return 0;
+
+ list_for_each_entry(cfg, &pci_mmcfg_list, list) {
+ rc = func(cfg->res.start, resource_size(&cfg->res), data);
+ if (rc)
+ return rc;
+ }
+
+ return 0;
+}
+#define set_apei_filter() (arch_apei_filter_addr = pci_mmcfg_for_each_region)
+#else
+#define set_apei_filter()
+#endif
+
static void __init __pci_mmcfg_init(int early)
{
pci_mmcfg_reject_broken(early);
@@ -644,6 +670,8 @@ void __init pci_mmcfg_early_init(void)
else
acpi_sfi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg);
__pci_mmcfg_init(1);
+
+ set_apei_filter();
}
}
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 2cd7bdd6c8b3..a85ac07f3da3 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -449,7 +449,7 @@ int apei_resources_sub(struct apei_resources *resources1,
}
EXPORT_SYMBOL_GPL(apei_resources_sub);
-static int apei_get_nvs_callback(__u64 start, __u64 size, void *data)
+static int apei_get_res_callback(__u64 start, __u64 size, void *data)
{
struct apei_resources *resources = data;
return apei_res_add(&resources->iomem, start, size);
@@ -457,7 +457,15 @@ static int apei_get_nvs_callback(__u64 start, __u64 size, void *data)
static int apei_get_nvs_resources(struct apei_resources *resources)
{
- return acpi_nvs_for_each_region(apei_get_nvs_callback, resources);
+ return acpi_nvs_for_each_region(apei_get_res_callback, resources);
+}
+
+int (*arch_apei_filter_addr)(int (*func)(__u64 start, __u64 size,
+ void *data), void *data);
+static int apei_get_arch_resources(struct apei_resources *resources)
+
+{
+ return arch_apei_filter_addr(apei_get_res_callback, resources);
}
/*
@@ -470,7 +478,7 @@ int apei_resources_request(struct apei_resources *resources,
{
struct apei_res *res, *res_bak = NULL;
struct resource *r;
- struct apei_resources nvs_resources;
+ struct apei_resources nvs_resources, arch_res;
int rc;
rc = apei_resources_sub(resources, &apei_resources_all);
@@ -485,10 +493,20 @@ int apei_resources_request(struct apei_resources *resources,
apei_resources_init(&nvs_resources);
rc = apei_get_nvs_resources(&nvs_resources);
if (rc)
- goto res_fini;
+ goto nvs_res_fini;
rc = apei_resources_sub(resources, &nvs_resources);
if (rc)
- goto res_fini;
+ goto nvs_res_fini;
+
+ if (arch_apei_filter_addr) {
+ apei_resources_init(&arch_res);
+ rc = apei_get_arch_resources(&arch_res);
+ if (rc)
+ goto arch_res_fini;
+ rc = apei_resources_sub(resources, &arch_res);
+ if (rc)
+ goto arch_res_fini;
+ }
rc = -EINVAL;
list_for_each_entry(res, &resources->iomem, list) {
@@ -536,7 +554,9 @@ err_unmap_iomem:
break;
release_mem_region(res->start, res->end - res->start);
}
-res_fini:
+arch_res_fini:
+ apei_resources_fini(&arch_res);
+nvs_res_fini:
apei_resources_fini(&nvs_resources);
return rc;
}
--
2.1.0