ACPI, APEI: Fix BERT resources conflict with ACPI NVS area

Started by Huang, Ying on Feb 14, 2017 · 4 messages from 4 authors · Last reply Feb 16, 2017

Subsystems (3)

Maintainers (2)

STALE3546d

open the first message on its own page

  1. [PATCH] ACPI, APEI: Fix BERT resources conflict with ACPI NVS area
    Related patches
From: Huang Ying <redacted>

It was reported that some firmware will use ACPI NVS area for BERT
address range.  This will cause resources conflict because the ACPI
NVS area is marked as busy already.  Fix this via excluding ACPI NVS
area when requesting IO resources for BERT.

Reported-and-tested-by: Hans Kristian Rosbach <redacted>
Signed-off-by: "Huang, Ying" <redacted>
---
 drivers/acpi/apei/bert.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/apei/bert.c b/drivers/acpi/apei/bert.c
index a05b5c0cf181..12771fcf0417 100644
--- a/drivers/acpi/apei/bert.c
+++ b/drivers/acpi/apei/bert.c
@@ -97,6 +97,7 @@ static int __init bert_check_table(struct acpi_table_bert *bert_tab)
 
 static int __init bert_init(void)
 {
+	struct apei_resources bert_resources;
 	struct acpi_bert_region *boot_error_region;
 	struct acpi_table_bert *bert_tab;
 	unsigned int region_len;
@@ -127,13 +128,14 @@ static int __init bert_init(void)
 	}
 
 	region_len = bert_tab->region_length;
-	if (!request_mem_region(bert_tab->address, region_len, "APEI BERT")) {
-		pr_err("Can't request iomem region <%016llx-%016llx>.\n",
-		       (unsigned long long)bert_tab->address,
-		       (unsigned long long)bert_tab->address + region_len - 1);
-		return -EIO;
-	}
-
+	apei_resources_init(&bert_resources);
+	rc = apei_resources_add(&bert_resources, bert_tab->address,
+				region_len, true);
+	if (rc)
+		return rc;
+	rc = apei_resources_request(&bert_resources, "APEI BERT");
+	if (rc)
+		goto out_fini;
 	boot_error_region = ioremap_cache(bert_tab->address, region_len);
 	if (boot_error_region) {
 		bert_print_all(boot_error_region, region_len);
@@ -142,7 +144,9 @@ static int __init bert_init(void)
 		rc = -ENOMEM;
 	}
 
-	release_mem_region(bert_tab->address, region_len);
+	apei_resources_release(&bert_resources);
+out_fini:
+	apei_resources_fini(&bert_resources);
 
 	return rc;
 }
-- 
2.11.0
On 2/13/2017 7:01 PM, Huang, Ying wrote:
From: Huang Ying <redacted>

It was reported that some firmware will use ACPI NVS area for BERT
address range.  This will cause resources conflict because the ACPI
NVS area is marked as busy already.  Fix this via excluding ACPI NVS
area when requesting IO resources for BERT.

Reported-and-tested-by: Hans Kristian Rosbach <redacted>
Signed-off-by: "Huang, Ying" <redacted>
Tested-by: Tyler Baicar <redacted>

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
On Tuesday, February 14, 2017 10:01:13 AM Huang, Ying wrote:
From: Huang Ying <redacted>

It was reported that some firmware will use ACPI NVS area for BERT
address range.  This will cause resources conflict because the ACPI
NVS area is marked as busy already.  Fix this via excluding ACPI NVS
area when requesting IO resources for BERT.

Reported-and-tested-by: Hans Kristian Rosbach <redacted>
Signed-off-by: "Huang, Ying" <redacted>
Boris, what do you think?
quoted hunk
---
 drivers/acpi/apei/bert.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/apei/bert.c b/drivers/acpi/apei/bert.c
index a05b5c0cf181..12771fcf0417 100644
--- a/drivers/acpi/apei/bert.c
+++ b/drivers/acpi/apei/bert.c
@@ -97,6 +97,7 @@ static int __init bert_check_table(struct acpi_table_bert *bert_tab)
 
 static int __init bert_init(void)
 {
+	struct apei_resources bert_resources;
 	struct acpi_bert_region *boot_error_region;
 	struct acpi_table_bert *bert_tab;
 	unsigned int region_len;
@@ -127,13 +128,14 @@ static int __init bert_init(void)
 	}
 
 	region_len = bert_tab->region_length;
-	if (!request_mem_region(bert_tab->address, region_len, "APEI BERT")) {
-		pr_err("Can't request iomem region <%016llx-%016llx>.\n",
-		       (unsigned long long)bert_tab->address,
-		       (unsigned long long)bert_tab->address + region_len - 1);
-		return -EIO;
-	}
-
+	apei_resources_init(&bert_resources);
+	rc = apei_resources_add(&bert_resources, bert_tab->address,
+				region_len, true);
+	if (rc)
+		return rc;
+	rc = apei_resources_request(&bert_resources, "APEI BERT");
+	if (rc)
+		goto out_fini;
 	boot_error_region = ioremap_cache(bert_tab->address, region_len);
 	if (boot_error_region) {
 		bert_print_all(boot_error_region, region_len);
@@ -142,7 +144,9 @@ static int __init bert_init(void)
 		rc = -ENOMEM;

10 more quoted lines

On Thu, Feb 16, 2017 at 12:42:00AM +0100, Rafael J. Wysocki wrote:
On Tuesday, February 14, 2017 10:01:13 AM Huang, Ying wrote:
quoted
From: Huang Ying <redacted>

It was reported that some firmware will use ACPI NVS area for BERT
address range.  This will cause resources conflict because the ACPI
NVS area is marked as busy already.  Fix this via excluding ACPI NVS
area when requesting IO resources for BERT.

Reported-and-tested-by: Hans Kristian Rosbach <redacted>
Signed-off-by: "Huang, Ying" <redacted>
Boris, what do you think?
Lemme see, so the BERT is for hw errors which have happened during the
previous boot and the machine couldn't handle them. So they do get saved
in some mem in the fw for inspection during the next boot.

And "some firmware" has decided to write them into non-volatile storage
- because you should never ever forget those errors! :-)

And I don't understand the "fix" here: we're excluding the NVS area when
mapping the BERT table but then how are we supposed to read those errors
from there?

Or am I missing something obvious?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help