* [PATCH v8 01/26] ACPI / APEI: Don't wait to serialise with oops messages when panic()ing
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 02/26] ACPI / APEI: Remove silent flag from ghes_read_estatus() James Morse
` (25 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
oops_begin() exists to group printk() messages with the oops message
printed by die(). To reach this caller we know that platform firmware
took this error first, then notified the OS via NMI with a 'panic'
severity.
Don't wait for another CPU to release the die-lock before panic()ing,
our only goal is to print this fatal error and panic().
This code is always called in_nmi(), and since commit 42a0bb3f7138
("printk/nmi: generic solution for safe printk in NMI"), it has been
safe to call printk() from this context. Messages are batched in a
per-cpu buffer and printed via irq-work, or a call back from panic().
Link: https://patchwork.kernel.org/patch/10313555/
Acked-by: Borislav Petkov <bp@suse.de>
Signed-off-by: James Morse <james.morse@arm.com>
---
Changes since v6:
* Capitals in patch subject
* Tinkered with the commit message.
---
drivers/acpi/apei/ghes.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index f008ba7c9ced..0c46b79e31b1 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -33,7 +33,6 @@
#include <linux/interrupt.h>
#include <linux/timer.h>
#include <linux/cper.h>
-#include <linux/kdebug.h>
#include <linux/platform_device.h>
#include <linux/mutex.h>
#include <linux/ratelimit.h>
@@ -949,7 +948,6 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
sev = ghes_severity(ghes->estatus->error_severity);
if (sev >= GHES_SEV_PANIC) {
- oops_begin();
ghes_print_queued_estatus();
__ghes_panic(ghes);
}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 02/26] ACPI / APEI: Remove silent flag from ghes_read_estatus()
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
2019-01-29 18:48 ` [PATCH v8 01/26] ACPI / APEI: Don't wait to serialise with oops messages when panic()ing James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 03/26] ACPI / APEI: Switch estatus pool to use vmalloc memory James Morse
` (24 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
Subsequent patches will split up ghes_read_estatus(), at which
point passing around the 'silent' flag gets annoying. This is to
suppress prink() messages, which prior to commit 42a0bb3f7138
("printk/nmi: generic solution for safe printk in NMI"), were
unsafe in NMI context.
This is no longer necessary, remove the flag. printk() messages
are batched in a per-cpu buffer and printed via irq-work, or a call
back from panic().
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
---
Changes since v6:
* Moved earlier in the series,
* Tinkered with the commit message.
* switched to pr_warn_ratelimited() to shut checkpatch up
---
drivers/acpi/apei/ghes.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 0c46b79e31b1..f0a704aed040 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -324,7 +324,7 @@ static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
}
}
-static int ghes_read_estatus(struct ghes *ghes, int silent)
+static int ghes_read_estatus(struct ghes *ghes)
{
struct acpi_hest_generic *g = ghes->generic;
u64 buf_paddr;
@@ -333,8 +333,7 @@ static int ghes_read_estatus(struct ghes *ghes, int silent)
rc = apei_read(&buf_paddr, &g->error_status_address);
if (rc) {
- if (!silent && printk_ratelimit())
- pr_warning(FW_WARN GHES_PFX
+ pr_warn_ratelimited(FW_WARN GHES_PFX
"Failed to read error status block address for hardware error source: %d.\n",
g->header.source_id);
return -EIO;
@@ -366,9 +365,9 @@ static int ghes_read_estatus(struct ghes *ghes, int silent)
rc = 0;
err_read_block:
- if (rc && !silent && printk_ratelimit())
- pr_warning(FW_WARN GHES_PFX
- "Failed to read error status block!\n");
+ if (rc)
+ pr_warn_ratelimited(FW_WARN GHES_PFX
+ "Failed to read error status block!\n");
return rc;
}
@@ -702,7 +701,7 @@ static int ghes_proc(struct ghes *ghes)
{
int rc;
- rc = ghes_read_estatus(ghes, 0);
+ rc = ghes_read_estatus(ghes);
if (rc)
goto out;
@@ -939,7 +938,7 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
return ret;
list_for_each_entry_rcu(ghes, &ghes_nmi, list) {
- if (ghes_read_estatus(ghes, 1)) {
+ if (ghes_read_estatus(ghes)) {
ghes_clear_estatus(ghes);
continue;
} else {
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 03/26] ACPI / APEI: Switch estatus pool to use vmalloc memory
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
2019-01-29 18:48 ` [PATCH v8 01/26] ACPI / APEI: Don't wait to serialise with oops messages when panic()ing James Morse
2019-01-29 18:48 ` [PATCH v8 02/26] ACPI / APEI: Remove silent flag from ghes_read_estatus() James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 04/26] ACPI / APEI: Make hest.c manage the estatus memory pool James Morse
` (23 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
The ghes code is careful to parse and round firmware's advertised
memory requirements for CPER records, up to a maximum of 64K.
However when ghes_estatus_pool_expand() does its work, it splits
the requested size into PAGE_SIZE granules.
This means if firmware generates 5K of CPER records, and correctly
describes this in the table, __process_error() will silently fail as it
is unable to allocate more than PAGE_SIZE.
Switch the estatus pool to vmalloc() memory. On x86 vmalloc() memory
may fault and be fixed up by vmalloc_fault(). To prevent this call
vmalloc_sync_all() before an NMI handler could discover the memory.
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
---
drivers/acpi/apei/ghes.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index f0a704aed040..ee9206d5e119 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -170,40 +170,40 @@ static int ghes_estatus_pool_init(void)
return 0;
}
-static void ghes_estatus_pool_free_chunk_page(struct gen_pool *pool,
+static void ghes_estatus_pool_free_chunk(struct gen_pool *pool,
struct gen_pool_chunk *chunk,
void *data)
{
- free_page(chunk->start_addr);
+ vfree((void *)chunk->start_addr);
}
static void ghes_estatus_pool_exit(void)
{
gen_pool_for_each_chunk(ghes_estatus_pool,
- ghes_estatus_pool_free_chunk_page, NULL);
+ ghes_estatus_pool_free_chunk, NULL);
gen_pool_destroy(ghes_estatus_pool);
}
static int ghes_estatus_pool_expand(unsigned long len)
{
- unsigned long i, pages, size, addr;
- int ret;
+ unsigned long size, addr;
ghes_estatus_pool_size_request += PAGE_ALIGN(len);
size = gen_pool_size(ghes_estatus_pool);
if (size >= ghes_estatus_pool_size_request)
return 0;
- pages = (ghes_estatus_pool_size_request - size) / PAGE_SIZE;
- for (i = 0; i < pages; i++) {
- addr = __get_free_page(GFP_KERNEL);
- if (!addr)
- return -ENOMEM;
- ret = gen_pool_add(ghes_estatus_pool, addr, PAGE_SIZE, -1);
- if (ret)
- return ret;
- }
- return 0;
+ addr = (unsigned long)vmalloc(PAGE_ALIGN(len));
+ if (!addr)
+ return -ENOMEM;
+
+ /*
+ * New allocation must be visible in all pgd before it can be found by
+ * an NMI allocating from the pool.
+ */
+ vmalloc_sync_all();
+
+ return gen_pool_add(ghes_estatus_pool, addr, PAGE_ALIGN(len), -1);
}
static int map_gen_v2(struct ghes *ghes)
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 04/26] ACPI / APEI: Make hest.c manage the estatus memory pool
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (2 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 03/26] ACPI / APEI: Switch estatus pool to use vmalloc memory James Morse
@ 2019-01-29 18:48 ` James Morse
2019-02-01 13:20 ` Borislav Petkov
2019-01-29 18:48 ` [PATCH v8 05/26] ACPI / APEI: Make estatus pool allocation a static size James Morse
` (22 subsequent siblings)
26 siblings, 1 reply; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
ghes.c has a memory pool it uses for the estatus cache and the estatus
queue. The cache is initialised when registering the platform driver.
For the queue, an NMI-like notification has to grow/shrink the pool
as it is registered and unregistered.
This is all pretty noisy when adding new NMI-like notifications, it
would be better to replace this with a static pool size based on the
number of users.
As a precursor, move the call that creates the pool from ghes_init(),
into hest.c. Later this will take the number of ghes entries and
consolidate the queue allocations.
Remove ghes_estatus_pool_exit() as hest.c doesn't have anywhere to put
this.
The pool is now initialised as part of ACPI's subsys_initcall():
(acpi_init(), acpi_scan_init(), acpi_pci_root_init(), acpi_hest_init())
Before this patch it happened later as a GHES specific device_initcall().
Signed-off-by: James Morse <james.morse@arm.com>
---
Changes since v7:
* Moved the pool init later, the driver isn't probed until device_init.
---
drivers/acpi/apei/ghes.c | 33 ++++++---------------------------
drivers/acpi/apei/hest.c | 10 +++++++++-
include/acpi/ghes.h | 2 ++
3 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index ee9206d5e119..4150c72c78cb 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -162,26 +162,16 @@ static void ghes_iounmap_irq(void)
clear_fixmap(FIX_APEI_GHES_IRQ);
}
-static int ghes_estatus_pool_init(void)
+static int ghes_estatus_pool_expand(unsigned long len); //temporary
+
+int ghes_estatus_pool_init(void)
{
ghes_estatus_pool = gen_pool_create(GHES_ESTATUS_POOL_MIN_ALLOC_ORDER, -1);
if (!ghes_estatus_pool)
return -ENOMEM;
- return 0;
-}
-static void ghes_estatus_pool_free_chunk(struct gen_pool *pool,
- struct gen_pool_chunk *chunk,
- void *data)
-{
- vfree((void *)chunk->start_addr);
-}
-
-static void ghes_estatus_pool_exit(void)
-{
- gen_pool_for_each_chunk(ghes_estatus_pool,
- ghes_estatus_pool_free_chunk, NULL);
- gen_pool_destroy(ghes_estatus_pool);
+ return ghes_estatus_pool_expand(GHES_ESTATUS_CACHE_AVG_SIZE *
+ GHES_ESTATUS_CACHE_ALLOCED_MAX);
}
static int ghes_estatus_pool_expand(unsigned long len)
@@ -1227,18 +1217,9 @@ static int __init ghes_init(void)
ghes_nmi_init_cxt();
- rc = ghes_estatus_pool_init();
- if (rc)
- goto err;
-
- rc = ghes_estatus_pool_expand(GHES_ESTATUS_CACHE_AVG_SIZE *
- GHES_ESTATUS_CACHE_ALLOCED_MAX);
- if (rc)
- goto err_pool_exit;
-
rc = platform_driver_register(&ghes_platform_driver);
if (rc)
- goto err_pool_exit;
+ goto err;
rc = apei_osc_setup();
if (rc == 0 && osc_sb_apei_support_acked)
@@ -1251,8 +1232,6 @@ static int __init ghes_init(void)
pr_info(GHES_PFX "Failed to enable APEI firmware first mode.\n");
return 0;
-err_pool_exit:
- ghes_estatus_pool_exit();
err:
return rc;
}
diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
index b1e9f81ebeea..097ba07a657b 100644
--- a/drivers/acpi/apei/hest.c
+++ b/drivers/acpi/apei/hest.c
@@ -32,6 +32,7 @@
#include <linux/io.h>
#include <linux/platform_device.h>
#include <acpi/apei.h>
+#include <acpi/ghes.h>
#include "apei-internal.h"
@@ -203,6 +204,11 @@ static int __init hest_ghes_dev_register(unsigned int ghes_count)
rc = apei_hest_parse(hest_parse_ghes, &ghes_arr);
if (rc)
goto err;
+
+ rc = ghes_estatus_pool_init();
+ if (rc)
+ goto err;
+
out:
kfree(ghes_arr.ghes_devs);
return rc;
@@ -251,7 +257,9 @@ void __init acpi_hest_init(void)
rc = apei_hest_parse(hest_parse_ghes_count, &ghes_count);
if (rc)
goto err;
- rc = hest_ghes_dev_register(ghes_count);
+
+ if (ghes_count)
+ rc = hest_ghes_dev_register(ghes_count);
if (rc)
goto err;
}
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 82cb4eb225a4..46ef5566e052 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -52,6 +52,8 @@ enum {
GHES_SEV_PANIC = 0x3,
};
+int ghes_estatus_pool_init(void);
+
/* From drivers/edac/ghes_edac.c */
#ifdef CONFIG_EDAC_GHES
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v8 04/26] ACPI / APEI: Make hest.c manage the estatus memory pool
2019-01-29 18:48 ` [PATCH v8 04/26] ACPI / APEI: Make hest.c manage the estatus memory pool James Morse
@ 2019-02-01 13:20 ` Borislav Petkov
0 siblings, 0 replies; 36+ messages in thread
From: Borislav Petkov @ 2019-02-01 13:20 UTC (permalink / raw)
To: James Morse
Cc: Rafael Wysocki, Tony Luck, linux-mm, Marc Zyngier,
Catalin Marinas, Xie XiuQi, Will Deacon, Christoffer Dall,
Dongjiu Geng, linux-acpi, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
On Tue, Jan 29, 2019 at 06:48:40PM +0000, James Morse wrote:
> ghes.c has a memory pool it uses for the estatus cache and the estatus
> queue. The cache is initialised when registering the platform driver.
> For the queue, an NMI-like notification has to grow/shrink the pool
> as it is registered and unregistered.
>
> This is all pretty noisy when adding new NMI-like notifications, it
> would be better to replace this with a static pool size based on the
> number of users.
>
> As a precursor, move the call that creates the pool from ghes_init(),
> into hest.c. Later this will take the number of ghes entries and
> consolidate the queue allocations.
> Remove ghes_estatus_pool_exit() as hest.c doesn't have anywhere to put
> this.
>
> The pool is now initialised as part of ACPI's subsys_initcall():
> (acpi_init(), acpi_scan_init(), acpi_pci_root_init(), acpi_hest_init())
> Before this patch it happened later as a GHES specific device_initcall().
>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> Changes since v7:
> * Moved the pool init later, the driver isn't probed until device_init.
> ---
> drivers/acpi/apei/ghes.c | 33 ++++++---------------------------
> drivers/acpi/apei/hest.c | 10 +++++++++-
> include/acpi/ghes.h | 2 ++
> 3 files changed, 17 insertions(+), 28 deletions(-)
Reviewed-by: Borislav Petkov <bp@suse.de>
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v8 05/26] ACPI / APEI: Make estatus pool allocation a static size
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (3 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 04/26] ACPI / APEI: Make hest.c manage the estatus memory pool James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 06/26] ACPI / APEI: Don't store CPER records physical address in struct ghes James Morse
` (21 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
Adding new NMI-like notifications duplicates the calls that grow
and shrink the estatus pool. This is all pretty pointless, as the
size is capped to 64K. Allocate this for each ghes and drop
the code that grows and shrinks the pool.
Suggested-by: Borislav Petkov <bp@suse.de>
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
---
drivers/acpi/apei/ghes.c | 49 +++++-----------------------------------
drivers/acpi/apei/hest.c | 2 +-
include/acpi/ghes.h | 2 +-
3 files changed, 8 insertions(+), 45 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 4150c72c78cb..33144ab0661a 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -162,27 +162,18 @@ static void ghes_iounmap_irq(void)
clear_fixmap(FIX_APEI_GHES_IRQ);
}
-static int ghes_estatus_pool_expand(unsigned long len); //temporary
-
-int ghes_estatus_pool_init(void)
+int ghes_estatus_pool_init(int num_ghes)
{
+ unsigned long addr, len;
+
ghes_estatus_pool = gen_pool_create(GHES_ESTATUS_POOL_MIN_ALLOC_ORDER, -1);
if (!ghes_estatus_pool)
return -ENOMEM;
- return ghes_estatus_pool_expand(GHES_ESTATUS_CACHE_AVG_SIZE *
- GHES_ESTATUS_CACHE_ALLOCED_MAX);
-}
-
-static int ghes_estatus_pool_expand(unsigned long len)
-{
- unsigned long size, addr;
-
- ghes_estatus_pool_size_request += PAGE_ALIGN(len);
- size = gen_pool_size(ghes_estatus_pool);
- if (size >= ghes_estatus_pool_size_request)
- return 0;
+ len = GHES_ESTATUS_CACHE_AVG_SIZE * GHES_ESTATUS_CACHE_ALLOCED_MAX;
+ len += (num_ghes * GHES_ESOURCE_PREALLOC_MAX_SIZE);
+ ghes_estatus_pool_size_request = PAGE_ALIGN(len);
addr = (unsigned long)vmalloc(PAGE_ALIGN(len));
if (!addr)
return -ENOMEM;
@@ -956,32 +947,8 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
return ret;
}
-static unsigned long ghes_esource_prealloc_size(
- const struct acpi_hest_generic *generic)
-{
- unsigned long block_length, prealloc_records, prealloc_size;
-
- block_length = min_t(unsigned long, generic->error_block_length,
- GHES_ESTATUS_MAX_SIZE);
- prealloc_records = max_t(unsigned long,
- generic->records_to_preallocate, 1);
- prealloc_size = min_t(unsigned long, block_length * prealloc_records,
- GHES_ESOURCE_PREALLOC_MAX_SIZE);
-
- return prealloc_size;
-}
-
-static void ghes_estatus_pool_shrink(unsigned long len)
-{
- ghes_estatus_pool_size_request -= PAGE_ALIGN(len);
-}
-
static void ghes_nmi_add(struct ghes *ghes)
{
- unsigned long len;
-
- len = ghes_esource_prealloc_size(ghes->generic);
- ghes_estatus_pool_expand(len);
mutex_lock(&ghes_list_mutex);
if (list_empty(&ghes_nmi))
register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0, "ghes");
@@ -991,8 +958,6 @@ static void ghes_nmi_add(struct ghes *ghes)
static void ghes_nmi_remove(struct ghes *ghes)
{
- unsigned long len;
-
mutex_lock(&ghes_list_mutex);
list_del_rcu(&ghes->list);
if (list_empty(&ghes_nmi))
@@ -1003,8 +968,6 @@ static void ghes_nmi_remove(struct ghes *ghes)
* freed after NMI handler finishes.
*/
synchronize_rcu();
- len = ghes_esource_prealloc_size(ghes->generic);
- ghes_estatus_pool_shrink(len);
}
static void ghes_nmi_init_cxt(void)
diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
index 097ba07a657b..fcc8cc1e4f3d 100644
--- a/drivers/acpi/apei/hest.c
+++ b/drivers/acpi/apei/hest.c
@@ -205,7 +205,7 @@ static int __init hest_ghes_dev_register(unsigned int ghes_count)
if (rc)
goto err;
- rc = ghes_estatus_pool_init();
+ rc = ghes_estatus_pool_init(ghes_count);
if (rc)
goto err;
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 46ef5566e052..cd9ee507d860 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -52,7 +52,7 @@ enum {
GHES_SEV_PANIC = 0x3,
};
-int ghes_estatus_pool_init(void);
+int ghes_estatus_pool_init(int num_ghes);
/* From drivers/edac/ghes_edac.c */
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 06/26] ACPI / APEI: Don't store CPER records physical address in struct ghes
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (4 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 05/26] ACPI / APEI: Make estatus pool allocation a static size James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 07/26] ACPI / APEI: Remove spurious GHES_TO_CLEAR check James Morse
` (20 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
When CPER records are found the address of the records is stashed
in the struct ghes. Once the records have been processed, this
address is overwritten with zero so that it won't be processed
again without being re-populated by firmware.
This goes wrong if a struct ghes can be processed concurrently,
as can happen at probe time when an NMI occurs. If the NMI arrives
on another CPU, the probing CPU may call ghes_clear_estatus() on the
records before the handler had finished with them.
Even on the same CPU, once the interrupted handler is resumed, it
will call ghes_clear_estatus() on the NMIs records, this memory may
have already been re-used by firmware.
Avoid this stashing by letting the caller hold the address. A
later patch will do away with the use of ghes->flags in the
read/clear code too.
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
---
Changes since v7:
* Added buf_paddr to ghes_panic, as it wants to print the estatus
Changes since v6:
* Moved earlier in the series
* Added buf_adder = 0 on all the error paths, and test for it in
ghes_estatus_clear() for extra sanity.
---
drivers/acpi/apei/ghes.c | 46 +++++++++++++++++++++++-----------------
include/acpi/ghes.h | 1 -
2 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 33144ab0661a..a34f79153b1a 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -305,29 +305,30 @@ static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
}
}
-static int ghes_read_estatus(struct ghes *ghes)
+static int ghes_read_estatus(struct ghes *ghes, u64 *buf_paddr)
{
struct acpi_hest_generic *g = ghes->generic;
- u64 buf_paddr;
u32 len;
int rc;
- rc = apei_read(&buf_paddr, &g->error_status_address);
+ rc = apei_read(buf_paddr, &g->error_status_address);
if (rc) {
+ *buf_paddr = 0;
pr_warn_ratelimited(FW_WARN GHES_PFX
"Failed to read error status block address for hardware error source: %d.\n",
g->header.source_id);
return -EIO;
}
- if (!buf_paddr)
+ if (!*buf_paddr)
return -ENOENT;
- ghes_copy_tofrom_phys(ghes->estatus, buf_paddr,
+ ghes_copy_tofrom_phys(ghes->estatus, *buf_paddr,
sizeof(*ghes->estatus), 1);
- if (!ghes->estatus->block_status)
+ if (!ghes->estatus->block_status) {
+ *buf_paddr = 0;
return -ENOENT;
+ }
- ghes->buffer_paddr = buf_paddr;
ghes->flags |= GHES_TO_CLEAR;
rc = -EIO;
@@ -339,7 +340,7 @@ static int ghes_read_estatus(struct ghes *ghes)
if (cper_estatus_check_header(ghes->estatus))
goto err_read_block;
ghes_copy_tofrom_phys(ghes->estatus + 1,
- buf_paddr + sizeof(*ghes->estatus),
+ *buf_paddr + sizeof(*ghes->estatus),
len - sizeof(*ghes->estatus), 1);
if (cper_estatus_check(ghes->estatus))
goto err_read_block;
@@ -349,15 +350,20 @@ static int ghes_read_estatus(struct ghes *ghes)
if (rc)
pr_warn_ratelimited(FW_WARN GHES_PFX
"Failed to read error status block!\n");
+
return rc;
}
-static void ghes_clear_estatus(struct ghes *ghes)
+static void ghes_clear_estatus(struct ghes *ghes, u64 buf_paddr)
{
ghes->estatus->block_status = 0;
if (!(ghes->flags & GHES_TO_CLEAR))
return;
- ghes_copy_tofrom_phys(ghes->estatus, ghes->buffer_paddr,
+
+ if (!buf_paddr)
+ return;
+
+ ghes_copy_tofrom_phys(ghes->estatus, buf_paddr,
sizeof(ghes->estatus->block_status), 0);
ghes->flags &= ~GHES_TO_CLEAR;
}
@@ -666,11 +672,11 @@ static int ghes_ack_error(struct acpi_hest_generic_v2 *gv2)
return apei_write(val, &gv2->read_ack_register);
}
-static void __ghes_panic(struct ghes *ghes)
+static void __ghes_panic(struct ghes *ghes, u64 buf_paddr)
{
__ghes_print_estatus(KERN_EMERG, ghes->generic, ghes->estatus);
- ghes_clear_estatus(ghes);
+ ghes_clear_estatus(ghes, buf_paddr);
/* reboot to log the error! */
if (!panic_timeout)
@@ -680,14 +686,15 @@ static void __ghes_panic(struct ghes *ghes)
static int ghes_proc(struct ghes *ghes)
{
+ u64 buf_paddr;
int rc;
- rc = ghes_read_estatus(ghes);
+ rc = ghes_read_estatus(ghes, &buf_paddr);
if (rc)
goto out;
if (ghes_severity(ghes->estatus->error_severity) >= GHES_SEV_PANIC) {
- __ghes_panic(ghes);
+ __ghes_panic(ghes, buf_paddr);
}
if (!ghes_estatus_cached(ghes->estatus)) {
@@ -697,7 +704,7 @@ static int ghes_proc(struct ghes *ghes)
ghes_do_proc(ghes, ghes->estatus);
out:
- ghes_clear_estatus(ghes);
+ ghes_clear_estatus(ghes, buf_paddr);
if (rc == -ENOENT)
return rc;
@@ -912,6 +919,7 @@ static void __process_error(struct ghes *ghes)
static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
{
+ u64 buf_paddr;
struct ghes *ghes;
int sev, ret = NMI_DONE;
@@ -919,8 +927,8 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
return ret;
list_for_each_entry_rcu(ghes, &ghes_nmi, list) {
- if (ghes_read_estatus(ghes)) {
- ghes_clear_estatus(ghes);
+ if (ghes_read_estatus(ghes, &buf_paddr)) {
+ ghes_clear_estatus(ghes, buf_paddr);
continue;
} else {
ret = NMI_HANDLED;
@@ -929,14 +937,14 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
sev = ghes_severity(ghes->estatus->error_severity);
if (sev >= GHES_SEV_PANIC) {
ghes_print_queued_estatus();
- __ghes_panic(ghes);
+ __ghes_panic(ghes, buf_paddr);
}
if (!(ghes->flags & GHES_TO_CLEAR))
continue;
__process_error(ghes);
- ghes_clear_estatus(ghes);
+ ghes_clear_estatus(ghes, buf_paddr);
}
#ifdef CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index cd9ee507d860..f82f4a7ddd90 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -22,7 +22,6 @@ struct ghes {
struct acpi_hest_generic_v2 *generic_v2;
};
struct acpi_hest_generic_status *estatus;
- u64 buffer_paddr;
unsigned long flags;
union {
struct list_head list;
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 07/26] ACPI / APEI: Remove spurious GHES_TO_CLEAR check
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (5 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 06/26] ACPI / APEI: Don't store CPER records physical address in struct ghes James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 08/26] ACPI / APEI: Don't update struct ghes' flags in read/clear estatus James Morse
` (19 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
ghes_notify_nmi() checks ghes->flags for GHES_TO_CLEAR before going
on to __process_error(). This is pointless as ghes_read_estatus()
will always set this flag if it returns success, which was checked
earlier in the loop. Remove it.
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
---
drivers/acpi/apei/ghes.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index a34f79153b1a..c20e1d0947b1 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -940,9 +940,6 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
__ghes_panic(ghes, buf_paddr);
}
- if (!(ghes->flags & GHES_TO_CLEAR))
- continue;
-
__process_error(ghes);
ghes_clear_estatus(ghes, buf_paddr);
}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 08/26] ACPI / APEI: Don't update struct ghes' flags in read/clear estatus
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (6 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 07/26] ACPI / APEI: Remove spurious GHES_TO_CLEAR check James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 09/26] ACPI / APEI: Generalise the estatus queue's notify code James Morse
` (18 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
ghes_read_estatus() sets a flag in struct ghes if the buffer of
CPER records needs to be cleared once the records have been
processed. This flag value is a problem if a struct ghes can be
processed concurrently, as happens at probe time if an NMI arrives
for the same error source. The NMI clears the flag, meaning the
interrupted handler may never do the ghes_estatus_clear() work.
The GHES_TO_CLEAR flags is only set at the same time as
buffer_paddr, which is now owned by the caller and passed to
ghes_clear_estatus(). Use this value as the flag.
A non-zero buf_paddr returned by ghes_read_estatus() means
ghes_clear_estatus() should clear this address. ghes_read_estatus()
already checks for a read of error_status_address being zero,
so CPER records cannot be written here.
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
--
Changes since v6:
* Added Boris' RB, then:
* Moved earlier in the series,
* Tinkered with the commit message,
* Always cleared buf_paddr on errors in the previous patch, which was
previously in here.
---
drivers/acpi/apei/ghes.c | 5 -----
include/acpi/ghes.h | 1 -
2 files changed, 6 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index c20e1d0947b1..af3c10f47f20 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -329,8 +329,6 @@ static int ghes_read_estatus(struct ghes *ghes, u64 *buf_paddr)
return -ENOENT;
}
- ghes->flags |= GHES_TO_CLEAR;
-
rc = -EIO;
len = cper_estatus_len(ghes->estatus);
if (len < sizeof(*ghes->estatus))
@@ -357,15 +355,12 @@ static int ghes_read_estatus(struct ghes *ghes, u64 *buf_paddr)
static void ghes_clear_estatus(struct ghes *ghes, u64 buf_paddr)
{
ghes->estatus->block_status = 0;
- if (!(ghes->flags & GHES_TO_CLEAR))
- return;
if (!buf_paddr)
return;
ghes_copy_tofrom_phys(ghes->estatus, buf_paddr,
sizeof(ghes->estatus->block_status), 0);
- ghes->flags &= ~GHES_TO_CLEAR;
}
static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int sev)
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index f82f4a7ddd90..e3f1cddb4ac8 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -13,7 +13,6 @@
* estatus: memory buffer for error status block, allocated during
* HEST parsing.
*/
-#define GHES_TO_CLEAR 0x0001
#define GHES_EXITING 0x0002
struct ghes {
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 09/26] ACPI / APEI: Generalise the estatus queue's notify code
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (7 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 08/26] ACPI / APEI: Don't update struct ghes' flags in read/clear estatus James Morse
@ 2019-01-29 18:48 ` James Morse
2019-02-01 13:46 ` Borislav Petkov
2019-01-29 18:48 ` [PATCH v8 10/26] ACPI / APEI: Don't allow ghes_ack_error() to mask earlier errors James Morse
` (17 subsequent siblings)
26 siblings, 1 reply; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
Refactor the estatus queue's pool notification routine from
NOTIFY_NMI's handlers. This will allow another notification
method to use the estatus queue without duplicating this code.
Add rcu_read_lock()/rcu_read_unlock() around the list
list_for_each_entry_rcu() walker. These aren't strictly necessary as
the whole nmi_enter/nmi_exit() window is a spooky RCU read-side
critical section.
in_nmi_queue_one_entry() is separate from the rcu-list walker for a
later caller that doesn't need to walk a list.
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>
Tested-by: Tyler Baicar <tbaicar@codeaurora.org>
---
Changes since v7:
* Moved err= onto a separate line to make this more readable
* Dropped ghes_ prefix on new static functions
* Renamed stuff, 'notify' has an overloaded meaning,
Changes since v6:
* Removed pool grow/remove code as this is no longer necessary.
Changes since v3:
* Removed duplicate or redundant paragraphs in commit message.
* Fixed the style of a zero check.
Changes since v1:
* Tidied up _in_nmi_notify_one().
---
drivers/acpi/apei/ghes.c | 65 ++++++++++++++++++++++++++--------------
1 file changed, 43 insertions(+), 22 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index af3c10f47f20..cb3d88de711f 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -912,37 +912,58 @@ static void __process_error(struct ghes *ghes)
#endif
}
-static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
+static int ghes_in_nmi_queue_one_entry(struct ghes *ghes)
{
u64 buf_paddr;
- struct ghes *ghes;
- int sev, ret = NMI_DONE;
+ int sev;
- if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
- return ret;
+ if (ghes_read_estatus(ghes, &buf_paddr)) {
+ ghes_clear_estatus(ghes, buf_paddr);
+ return -ENOENT;
+ }
- list_for_each_entry_rcu(ghes, &ghes_nmi, list) {
- if (ghes_read_estatus(ghes, &buf_paddr)) {
- ghes_clear_estatus(ghes, buf_paddr);
- continue;
- } else {
- ret = NMI_HANDLED;
- }
+ sev = ghes_severity(ghes->estatus->error_severity);
+ if (sev >= GHES_SEV_PANIC) {
+ ghes_print_queued_estatus();
+ __ghes_panic(ghes, buf_paddr);
+ }
- sev = ghes_severity(ghes->estatus->error_severity);
- if (sev >= GHES_SEV_PANIC) {
- ghes_print_queued_estatus();
- __ghes_panic(ghes, buf_paddr);
- }
+ __process_error(ghes);
+ ghes_clear_estatus(ghes, buf_paddr);
- __process_error(ghes);
- ghes_clear_estatus(ghes, buf_paddr);
+ return 0;
+}
+
+static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list)
+{
+ int err, ret = -ENOENT;
+ struct ghes *ghes;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(ghes, rcu_list, list) {
+ err = ghes_in_nmi_queue_one_entry(ghes);
+ if (!err)
+ ret = 0;
}
+ rcu_read_unlock();
-#ifdef CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG
- if (ret == NMI_HANDLED)
+ if (IS_ENABLED(CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG) && !ret)
irq_work_queue(&ghes_proc_irq_work);
-#endif
+
+ return ret;
+}
+
+static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
+{
+ int err, ret = NMI_DONE;
+
+ if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
+ return ret;
+
+ err = ghes_in_nmi_spool_from_list(&ghes_nmi);
+ if (!err)
+ ret = NMI_HANDLED;
+
atomic_dec(&ghes_in_nmi);
return ret;
}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v8 09/26] ACPI / APEI: Generalise the estatus queue's notify code
2019-01-29 18:48 ` [PATCH v8 09/26] ACPI / APEI: Generalise the estatus queue's notify code James Morse
@ 2019-02-01 13:46 ` Borislav Petkov
0 siblings, 0 replies; 36+ messages in thread
From: Borislav Petkov @ 2019-02-01 13:46 UTC (permalink / raw)
To: James Morse
Cc: Rafael Wysocki, Tony Luck, linux-mm, Marc Zyngier,
Catalin Marinas, Xie XiuQi, Will Deacon, Christoffer Dall,
Dongjiu Geng, linux-acpi, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
On Tue, Jan 29, 2019 at 06:48:45PM +0000, James Morse wrote:
> +static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list)
> +{
> + int err, ret = -ENOENT;
> + struct ghes *ghes;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(ghes, rcu_list, list) {
> + err = ghes_in_nmi_queue_one_entry(ghes);
> + if (!err)
> + ret = 0;
Do I understand this correctly that we want to do "ret = 0" for at least
one record which ghes_in_nmi_queue_one_entry() has succeeded queueing?
For those for which it has returned -ENOENT, estatus has been cleared,
nothing has been queued so we don't have to do anything for that
particular entry...
Btw, you don't really need the err variable:
if (!ghes_in_nmi_queue_one_entry(ghes))
ret = 0;
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v8 10/26] ACPI / APEI: Don't allow ghes_ack_error() to mask earlier errors
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (8 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 09/26] ACPI / APEI: Generalise the estatus queue's notify code James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 11/26] ACPI / APEI: Move NOTIFY_SEA between the estatus-queue and NOTIFY_NMI James Morse
` (16 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
During ghes_proc() we use ghes_ack_error() to tell an external agent
we are done with these records and it can re-use the memory.
rc may hold an error returned by ghes_read_estatus(), ENOENT causes
us to skip ghes_ack_error() (as there is nothing to ack), but rc may
also by EIO, which gets supressed.
ghes_clear_estatus() is where we mark the records as processed for
non GHESv2 error sources, and already spots the ENOENT case as
buf_paddr is set to 0 by ghes_read_estatus().
Move the ghes_ack_error() call in here to avoid extra logic with
the return code in ghes_proc().
This enables GHESv2 acking for NMI-like error sources. This is safe
as the buffer is pre-mapped by map_gen_v2() before the GHES is added
to any NMI handler lists.
This same pre-mapping step means we can't receive an error from
apei_read()/write() here as apei_check_gar() succeeded when it
was mapped, and the mapping was cached, so the address can't be
rejected at runtime. Remove the error-returns as this is now
called from a function with no return.
Signed-off-by: James Morse <james.morse@arm.com>
---
drivers/acpi/apei/ghes.c | 47 +++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 25 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index cb3d88de711f..bd58749d31bb 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -197,6 +197,21 @@ static void unmap_gen_v2(struct ghes *ghes)
apei_unmap_generic_address(&ghes->generic_v2->read_ack_register);
}
+static void ghes_ack_error(struct acpi_hest_generic_v2 *gv2)
+{
+ int rc;
+ u64 val = 0;
+
+ rc = apei_read(&val, &gv2->read_ack_register);
+ if (rc)
+ return;
+
+ val &= gv2->read_ack_preserve << gv2->read_ack_register.bit_offset;
+ val |= gv2->read_ack_write << gv2->read_ack_register.bit_offset;
+
+ apei_write(val, &gv2->read_ack_register);
+}
+
static struct ghes *ghes_new(struct acpi_hest_generic *generic)
{
struct ghes *ghes;
@@ -361,6 +376,13 @@ static void ghes_clear_estatus(struct ghes *ghes, u64 buf_paddr)
ghes_copy_tofrom_phys(ghes->estatus, buf_paddr,
sizeof(ghes->estatus->block_status), 0);
+
+ /*
+ * GHESv2 type HEST entries introduce support for error acknowledgment,
+ * so only acknowledge the error if this support is present.
+ */
+ if (is_hest_type_generic_v2(ghes))
+ ghes_ack_error(ghes->generic_v2);
}
static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int sev)
@@ -652,21 +674,6 @@ static void ghes_estatus_cache_add(
rcu_read_unlock();
}
-static int ghes_ack_error(struct acpi_hest_generic_v2 *gv2)
-{
- int rc;
- u64 val = 0;
-
- rc = apei_read(&val, &gv2->read_ack_register);
- if (rc)
- return rc;
-
- val &= gv2->read_ack_preserve << gv2->read_ack_register.bit_offset;
- val |= gv2->read_ack_write << gv2->read_ack_register.bit_offset;
-
- return apei_write(val, &gv2->read_ack_register);
-}
-
static void __ghes_panic(struct ghes *ghes, u64 buf_paddr)
{
__ghes_print_estatus(KERN_EMERG, ghes->generic, ghes->estatus);
@@ -701,16 +708,6 @@ static int ghes_proc(struct ghes *ghes)
out:
ghes_clear_estatus(ghes, buf_paddr);
- if (rc == -ENOENT)
- return rc;
-
- /*
- * GHESv2 type HEST entries introduce support for error acknowledgment,
- * so only acknowledge the error if this support is present.
- */
- if (is_hest_type_generic_v2(ghes))
- return ghes_ack_error(ghes->generic_v2);
-
return rc;
}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 11/26] ACPI / APEI: Move NOTIFY_SEA between the estatus-queue and NOTIFY_NMI
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (9 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 10/26] ACPI / APEI: Don't allow ghes_ack_error() to mask earlier errors James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 12/26] ACPI / APEI: Switch NOTIFY_SEA to use the estatus queue James Morse
` (15 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
The estatus-queue code is currently hidden by the NOTIFY_NMI #ifdefs.
Once NOTIFY_SEA starts using the estatus-queue we can stop hiding
it as each architecture has a user that can't be turned off.
Split the existing CONFIG_HAVE_ACPI_APEI_NMI block in two, and move
the SEA code into the gap.
Move the code around ... and changes the stale comment describing
why the status queue is necessary: printk() is no longer the issue,
its the helpers like memory_failure_queue() that aren't nmi safe.
Signed-off-by: James Morse <james.morse@arm.com>
---
drivers/acpi/apei/ghes.c | 113 ++++++++++++++++++++-------------------
1 file changed, 59 insertions(+), 54 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index bd58749d31bb..576dce29159d 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -767,66 +767,21 @@ static struct notifier_block ghes_notifier_hed = {
.notifier_call = ghes_notify_hed,
};
-#ifdef CONFIG_ACPI_APEI_SEA
-static LIST_HEAD(ghes_sea);
-
-/*
- * Return 0 only if one of the SEA error sources successfully reported an error
- * record sent from the firmware.
- */
-int ghes_notify_sea(void)
-{
- struct ghes *ghes;
- int ret = -ENOENT;
-
- rcu_read_lock();
- list_for_each_entry_rcu(ghes, &ghes_sea, list) {
- if (!ghes_proc(ghes))
- ret = 0;
- }
- rcu_read_unlock();
- return ret;
-}
-
-static void ghes_sea_add(struct ghes *ghes)
-{
- mutex_lock(&ghes_list_mutex);
- list_add_rcu(&ghes->list, &ghes_sea);
- mutex_unlock(&ghes_list_mutex);
-}
-
-static void ghes_sea_remove(struct ghes *ghes)
-{
- mutex_lock(&ghes_list_mutex);
- list_del_rcu(&ghes->list);
- mutex_unlock(&ghes_list_mutex);
- synchronize_rcu();
-}
-#else /* CONFIG_ACPI_APEI_SEA */
-static inline void ghes_sea_add(struct ghes *ghes) { }
-static inline void ghes_sea_remove(struct ghes *ghes) { }
-#endif /* CONFIG_ACPI_APEI_SEA */
-
#ifdef CONFIG_HAVE_ACPI_APEI_NMI
/*
- * printk is not safe in NMI context. So in NMI handler, we allocate
- * required memory from lock-less memory allocator
- * (ghes_estatus_pool), save estatus into it, put them into lock-less
- * list (ghes_estatus_llist), then delay printk into IRQ context via
- * irq_work (ghes_proc_irq_work). ghes_estatus_size_request record
- * required pool size by all NMI error source.
+ * Handlers for CPER records may not be NMI safe. For example,
+ * memory_failure_queue() takes spinlocks and calls schedule_work_on().
+ * In any NMI-like handler, memory from ghes_estatus_pool is used to save
+ * estatus, and added to the ghes_estatus_llist. irq_work_queue() causes
+ * ghes_proc_in_irq() to run in IRQ context where each estatus in
+ * ghes_estatus_llist is processed.
+ *
+ * Memory from the ghes_estatus_pool is also used with the ghes_estatus_cache
+ * to suppress frequent messages.
*/
static struct llist_head ghes_estatus_llist;
static struct irq_work ghes_proc_irq_work;
-/*
- * NMI may be triggered on any CPU, so ghes_in_nmi is used for
- * having only one concurrent reader.
- */
-static atomic_t ghes_in_nmi = ATOMIC_INIT(0);
-
-static LIST_HEAD(ghes_nmi);
-
static void ghes_proc_in_irq(struct irq_work *irq_work)
{
struct llist_node *llnode, *next;
@@ -949,6 +904,56 @@ static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list)
return ret;
}
+#endif /* CONFIG_HAVE_ACPI_APEI_NMI */
+
+#ifdef CONFIG_ACPI_APEI_SEA
+static LIST_HEAD(ghes_sea);
+
+/*
+ * Return 0 only if one of the SEA error sources successfully reported an error
+ * record sent from the firmware.
+ */
+int ghes_notify_sea(void)
+{
+ struct ghes *ghes;
+ int ret = -ENOENT;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(ghes, &ghes_sea, list) {
+ if (!ghes_proc(ghes))
+ ret = 0;
+ }
+ rcu_read_unlock();
+ return ret;
+}
+
+static void ghes_sea_add(struct ghes *ghes)
+{
+ mutex_lock(&ghes_list_mutex);
+ list_add_rcu(&ghes->list, &ghes_sea);
+ mutex_unlock(&ghes_list_mutex);
+}
+
+static void ghes_sea_remove(struct ghes *ghes)
+{
+ mutex_lock(&ghes_list_mutex);
+ list_del_rcu(&ghes->list);
+ mutex_unlock(&ghes_list_mutex);
+ synchronize_rcu();
+}
+#else /* CONFIG_ACPI_APEI_SEA */
+static inline void ghes_sea_add(struct ghes *ghes) { }
+static inline void ghes_sea_remove(struct ghes *ghes) { }
+#endif /* CONFIG_ACPI_APEI_SEA */
+
+#ifdef CONFIG_HAVE_ACPI_APEI_NMI
+/*
+ * NMI may be triggered on any CPU, so ghes_in_nmi is used for
+ * having only one concurrent reader.
+ */
+static atomic_t ghes_in_nmi = ATOMIC_INIT(0);
+
+static LIST_HEAD(ghes_nmi);
static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
{
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 12/26] ACPI / APEI: Switch NOTIFY_SEA to use the estatus queue
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (10 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 11/26] ACPI / APEI: Move NOTIFY_SEA between the estatus-queue and NOTIFY_NMI James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 13/26] KVM: arm/arm64: Add kvm_ras.h to collect kvm specific RAS plumbing James Morse
` (14 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
Now that the estatus queue can be used by more than one notification
method, we can move notifications that have NMI-like behaviour over.
Switch NOTIFY_SEA over to use the estatus queue. This makes it behave
in the same way as x86's NOTIFY_NMI.
Remove Kconfig's ability to turn ACPI_APEI_SEA off if ACPI_APEI_GHES
is selected. This roughly matches the x86 NOTIFY_NMI behaviour, and means
each architecture has at least one user of the estatus-queue, meaning it
doesn't need guarding with ifdef.
Signed-off-by: James Morse <james.morse@arm.com>
---
Changes since v6:
* Lost all the pool grow/shrink stuff,
* Changed Kconfig so this can't be turned off to avoid kconfig complexity:
* Dropped Tyler's tested-by.
* For now we need #ifdef around the SEA code as the arch code assumes its there.
* Removed Punit's reviewed-by due to the swirling #ifdeffery
---
drivers/acpi/apei/Kconfig | 12 +-----------
drivers/acpi/apei/ghes.c | 22 +++++-----------------
2 files changed, 6 insertions(+), 28 deletions(-)
diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
index 52ae5438edeb..6b18f8bc7be3 100644
--- a/drivers/acpi/apei/Kconfig
+++ b/drivers/acpi/apei/Kconfig
@@ -41,19 +41,9 @@ config ACPI_APEI_PCIEAER
Turn on this option to enable the corresponding support.
config ACPI_APEI_SEA
- bool "APEI Synchronous External Abort logging/recovering support"
+ bool
depends on ARM64 && ACPI_APEI_GHES
default y
- help
- This option should be enabled if the system supports
- firmware first handling of SEA (Synchronous External Abort).
- SEA happens with certain faults of data abort or instruction
- abort synchronous exceptions on ARMv8 systems. If a system
- supports firmware first handling of SEA, the platform analyzes
- and handles hardware error notifications from SEA, and it may then
- form a HW error record for the OS to parse and handle. This
- option allows the OS to look for such hardware error record, and
- take appropriate action.
config ACPI_APEI_MEMORY_FAILURE
bool "APEI memory error recovering support"
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 576dce29159d..ab794ab29554 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -767,7 +767,6 @@ static struct notifier_block ghes_notifier_hed = {
.notifier_call = ghes_notify_hed,
};
-#ifdef CONFIG_HAVE_ACPI_APEI_NMI
/*
* Handlers for CPER records may not be NMI safe. For example,
* memory_failure_queue() takes spinlocks and calls schedule_work_on().
@@ -904,7 +903,6 @@ static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list)
return ret;
}
-#endif /* CONFIG_HAVE_ACPI_APEI_NMI */
#ifdef CONFIG_ACPI_APEI_SEA
static LIST_HEAD(ghes_sea);
@@ -915,16 +913,7 @@ static LIST_HEAD(ghes_sea);
*/
int ghes_notify_sea(void)
{
- struct ghes *ghes;
- int ret = -ENOENT;
-
- rcu_read_lock();
- list_for_each_entry_rcu(ghes, &ghes_sea, list) {
- if (!ghes_proc(ghes))
- ret = 0;
- }
- rcu_read_unlock();
- return ret;
+ return ghes_in_nmi_spool_from_list(&ghes_sea);
}
static void ghes_sea_add(struct ghes *ghes)
@@ -992,16 +981,15 @@ static void ghes_nmi_remove(struct ghes *ghes)
*/
synchronize_rcu();
}
+#else /* CONFIG_HAVE_ACPI_APEI_NMI */
+static inline void ghes_nmi_add(struct ghes *ghes) { }
+static inline void ghes_nmi_remove(struct ghes *ghes) { }
+#endif /* CONFIG_HAVE_ACPI_APEI_NMI */
static void ghes_nmi_init_cxt(void)
{
init_irq_work(&ghes_proc_irq_work, ghes_proc_in_irq);
}
-#else /* CONFIG_HAVE_ACPI_APEI_NMI */
-static inline void ghes_nmi_add(struct ghes *ghes) { }
-static inline void ghes_nmi_remove(struct ghes *ghes) { }
-static inline void ghes_nmi_init_cxt(void) { }
-#endif /* CONFIG_HAVE_ACPI_APEI_NMI */
static int ghes_probe(struct platform_device *ghes_dev)
{
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 13/26] KVM: arm/arm64: Add kvm_ras.h to collect kvm specific RAS plumbing
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (11 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 12/26] ACPI / APEI: Switch NOTIFY_SEA to use the estatus queue James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 14/26] arm64: KVM/mm: Move SEA handling behind a single 'claim' interface James Morse
` (13 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
To split up APEIs in_nmi() path, the caller needs to always be
in_nmi(). KVM shouldn't have to know about this, pull the RAS plumbing
out into a header file.
Currently guest synchronous external aborts are claimed as RAS
notifications by handle_guest_sea(), which is hidden in the arch codes
mm/fault.c. 32bit gets a dummy declaration in system_misc.h.
There is going to be more of this in the future if/when the kernel
supports the SError-based firmware-first notification mechanism and/or
kernel-first notifications for both synchronous external abort and
SError. Each of these will come with some Kconfig symbols and a
handful of header files.
Create a header file for all this.
This patch gives handle_guest_sea() a 'kvm_' prefix, and moves the
declarations to kvm_ras.h as preparation for a future patch that moves
the ACPI-specific RAS code out of mm/fault.c.
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Tested-by: Tyler Baicar <tbaicar@codeaurora.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
Changes since v6:
* Tinkered with the commit message
---
arch/arm/include/asm/kvm_ras.h | 14 ++++++++++++++
arch/arm/include/asm/system_misc.h | 5 -----
arch/arm64/include/asm/kvm_ras.h | 11 +++++++++++
arch/arm64/include/asm/system_misc.h | 2 --
arch/arm64/mm/fault.c | 2 +-
virt/kvm/arm/mmu.c | 4 ++--
6 files changed, 28 insertions(+), 10 deletions(-)
create mode 100644 arch/arm/include/asm/kvm_ras.h
create mode 100644 arch/arm64/include/asm/kvm_ras.h
diff --git a/arch/arm/include/asm/kvm_ras.h b/arch/arm/include/asm/kvm_ras.h
new file mode 100644
index 000000000000..e9577292dfe4
--- /dev/null
+++ b/arch/arm/include/asm/kvm_ras.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 - Arm Ltd */
+
+#ifndef __ARM_KVM_RAS_H__
+#define __ARM_KVM_RAS_H__
+
+#include <linux/types.h>
+
+static inline int kvm_handle_guest_sea(phys_addr_t addr, unsigned int esr)
+{
+ return -1;
+}
+
+#endif /* __ARM_KVM_RAS_H__ */
diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h
index 8e76db83c498..66f6a3ae68d2 100644
--- a/arch/arm/include/asm/system_misc.h
+++ b/arch/arm/include/asm/system_misc.h
@@ -38,11 +38,6 @@ static inline void harden_branch_predictor(void)
extern unsigned int user_debug;
-static inline int handle_guest_sea(phys_addr_t addr, unsigned int esr)
-{
- return -1;
-}
-
#endif /* !__ASSEMBLY__ */
#endif /* __ASM_ARM_SYSTEM_MISC_H */
diff --git a/arch/arm64/include/asm/kvm_ras.h b/arch/arm64/include/asm/kvm_ras.h
new file mode 100644
index 000000000000..6096f0251812
--- /dev/null
+++ b/arch/arm64/include/asm/kvm_ras.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2018 - Arm Ltd */
+
+#ifndef __ARM64_KVM_RAS_H__
+#define __ARM64_KVM_RAS_H__
+
+#include <linux/types.h>
+
+int kvm_handle_guest_sea(phys_addr_t addr, unsigned int esr);
+
+#endif /* __ARM64_KVM_RAS_H__ */
diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
index 0e2a0ecaf484..32693f34f431 100644
--- a/arch/arm64/include/asm/system_misc.h
+++ b/arch/arm64/include/asm/system_misc.h
@@ -46,8 +46,6 @@ extern void __show_regs(struct pt_regs *);
extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
-int handle_guest_sea(phys_addr_t addr, unsigned int esr);
-
#endif /* __ASSEMBLY__ */
#endif /* __ASM_SYSTEM_MISC_H */
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index efb7b2cbead5..c76dc981e3fc 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -733,7 +733,7 @@ static const struct fault_info fault_info[] = {
{ do_bad, SIGKILL, SI_KERNEL, "unknown 63" },
};
-int handle_guest_sea(phys_addr_t addr, unsigned int esr)
+int kvm_handle_guest_sea(phys_addr_t addr, unsigned int esr)
{
return ghes_notify_sea();
}
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index fbdf3ac2f001..600e0cc74ea4 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -27,10 +27,10 @@
#include <asm/kvm_arm.h>
#include <asm/kvm_mmu.h>
#include <asm/kvm_mmio.h>
+#include <asm/kvm_ras.h>
#include <asm/kvm_asm.h>
#include <asm/kvm_emulate.h>
#include <asm/virt.h>
-#include <asm/system_misc.h>
#include "trace.h"
@@ -1903,7 +1903,7 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
* For RAS the host kernel may handle this abort.
* There is no need to pass the error into the guest.
*/
- if (!handle_guest_sea(fault_ipa, kvm_vcpu_get_hsr(vcpu)))
+ if (!kvm_handle_guest_sea(fault_ipa, kvm_vcpu_get_hsr(vcpu)))
return 1;
if (unlikely(!is_iabt)) {
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 14/26] arm64: KVM/mm: Move SEA handling behind a single 'claim' interface
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (12 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 13/26] KVM: arm/arm64: Add kvm_ras.h to collect kvm specific RAS plumbing James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 15/26] ACPI / APEI: Move locking to the notification helper James Morse
` (12 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
To split up APEIs in_nmi() path, the caller needs to always be
in_nmi(). Add a helper to do the work and claim the notification.
When KVM or the arch code takes an exception that might be a RAS
notification, it asks the APEI firmware-first code whether it wants
to claim the exception. A future kernel-first mechanism may be queried
afterwards, and claim the notification, otherwise we fall through
to the existing default behaviour.
The NOTIFY_SEA code was merged before considering multiple, possibly
interacting, NMI-like notifications and the need to consider kernel
first in the future. Make the 'claiming' behaviour explicit.
Restructuring the APEI code to allow multiple NMI-like notifications
means any notification that might interrupt interrupts-masked
code must always be wrapped in nmi_enter()/nmi_exit(). This will
allow APEI to use in_nmi() to use the right fixmap entries.
Mask SError over this window to prevent an asynchronous RAS error
arriving and tripping 'nmi_enter()'s BUG_ON(in_nmi()).
Signed-off-by: James Morse <james.morse@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Tested-by: Tyler Baicar <tbaicar@codeaurora.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
Why does apei_claim_sea() take a pt_regs? This gets used later to take
APEI by the hand through NMI->IRQ context, depending on what we
interrupted.
Changes since v6:
* Moved the voice of the commit message.
* moved arch_local_save_flags() below the !IS_ENABLED drop-out
* Moved the dummy declaration into the if-ACPI part of the header instead
of if-APEI.
Changes since v4:
* Made irqs-unmasked comment a lockdep assert.
Changes since v3:
* Removed spurious whitespace change
* Updated comment in acpi.c to cover SError masking
Changes since v2:
* Added dummy definition for !ACPI and culled IS_ENABLED() checks.
---
arch/arm64/include/asm/acpi.h | 4 +++-
arch/arm64/include/asm/daifflags.h | 1 +
arch/arm64/include/asm/kvm_ras.h | 16 ++++++++++++++-
arch/arm64/kernel/acpi.c | 31 ++++++++++++++++++++++++++++++
arch/arm64/mm/fault.c | 24 +++++------------------
5 files changed, 55 insertions(+), 21 deletions(-)
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 2def77ec14be..7628efbe6c12 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -18,6 +18,7 @@
#include <asm/cputype.h>
#include <asm/io.h>
+#include <asm/ptrace.h>
#include <asm/smp_plat.h>
#include <asm/tlbflush.h>
@@ -110,9 +111,10 @@ static inline u32 get_acpi_id_for_cpu(unsigned int cpu)
static inline void arch_fix_phys_package_id(int num, u32 slot) { }
void __init acpi_init_cpus(void);
-
+int apei_claim_sea(struct pt_regs *regs);
#else
static inline void acpi_init_cpus(void) { }
+static inline int apei_claim_sea(struct pt_regs *regs) { return -ENOENT; }
#endif /* CONFIG_ACPI */
#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
diff --git a/arch/arm64/include/asm/daifflags.h b/arch/arm64/include/asm/daifflags.h
index 8d91f2233135..fa90779fc752 100644
--- a/arch/arm64/include/asm/daifflags.h
+++ b/arch/arm64/include/asm/daifflags.h
@@ -20,6 +20,7 @@
#define DAIF_PROCCTX 0
#define DAIF_PROCCTX_NOIRQ PSR_I_BIT
+#define DAIF_ERRCTX (PSR_I_BIT | PSR_A_BIT)
/* mask/save/unmask/restore all exceptions, including interrupts. */
static inline void local_daif_mask(void)
diff --git a/arch/arm64/include/asm/kvm_ras.h b/arch/arm64/include/asm/kvm_ras.h
index 6096f0251812..8ac6ee77437c 100644
--- a/arch/arm64/include/asm/kvm_ras.h
+++ b/arch/arm64/include/asm/kvm_ras.h
@@ -4,8 +4,22 @@
#ifndef __ARM64_KVM_RAS_H__
#define __ARM64_KVM_RAS_H__
+#include <linux/acpi.h>
+#include <linux/errno.h>
#include <linux/types.h>
-int kvm_handle_guest_sea(phys_addr_t addr, unsigned int esr);
+#include <asm/acpi.h>
+
+/*
+ * Was this synchronous external abort a RAS notification?
+ * Returns '0' for errors handled by some RAS subsystem, or -ENOENT.
+ */
+static inline int kvm_handle_guest_sea(phys_addr_t addr, unsigned int esr)
+{
+ /* apei_claim_sea(NULL) expects to mask interrupts itself */
+ lockdep_assert_irqs_enabled();
+
+ return apei_claim_sea(NULL);
+}
#endif /* __ARM64_KVM_RAS_H__ */
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 44e3c351e1ea..803f0494dd3e 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -27,8 +27,10 @@
#include <linux/smp.h>
#include <linux/serial_core.h>
+#include <acpi/ghes.h>
#include <asm/cputype.h>
#include <asm/cpu_ops.h>
+#include <asm/daifflags.h>
#include <asm/pgtable.h>
#include <asm/smp_plat.h>
@@ -256,3 +258,32 @@ pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
return __pgprot(PROT_NORMAL_NC);
return __pgprot(PROT_DEVICE_nGnRnE);
}
+
+/*
+ * Claim Synchronous External Aborts as a firmware first notification.
+ *
+ * Used by KVM and the arch do_sea handler.
+ * @regs may be NULL when called from process context.
+ */
+int apei_claim_sea(struct pt_regs *regs)
+{
+ int err = -ENOENT;
+ unsigned long current_flags;
+
+ if (!IS_ENABLED(CONFIG_ACPI_APEI_GHES))
+ return err;
+
+ current_flags = arch_local_save_flags();
+
+ /*
+ * SEA can interrupt SError, mask it and describe this as an NMI so
+ * that APEI defers the handling.
+ */
+ local_daif_restore(DAIF_ERRCTX);
+ nmi_enter();
+ err = ghes_notify_sea();
+ nmi_exit();
+ local_daif_restore(current_flags);
+
+ return err;
+}
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c76dc981e3fc..e1c84c2e1cab 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -18,6 +18,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <linux/acpi.h>
#include <linux/extable.h>
#include <linux/signal.h>
#include <linux/mm.h>
@@ -33,6 +34,7 @@
#include <linux/preempt.h>
#include <linux/hugetlb.h>
+#include <asm/acpi.h>
#include <asm/bug.h>
#include <asm/cmpxchg.h>
#include <asm/cpufeature.h>
@@ -47,8 +49,6 @@
#include <asm/tlbflush.h>
#include <asm/traps.h>
-#include <acpi/ghes.h>
-
struct fault_info {
int (*fn)(unsigned long addr, unsigned int esr,
struct pt_regs *regs);
@@ -643,19 +643,10 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
inf = esr_to_fault_info(esr);
/*
- * Synchronous aborts may interrupt code which had interrupts masked.
- * Before calling out into the wider kernel tell the interested
- * subsystems.
+ * Return value ignored as we rely on signal merging.
+ * Future patches will make this more robust.
*/
- if (IS_ENABLED(CONFIG_ACPI_APEI_SEA)) {
- if (interrupts_enabled(regs))
- nmi_enter();
-
- ghes_notify_sea();
-
- if (interrupts_enabled(regs))
- nmi_exit();
- }
+ apei_claim_sea(regs);
if (esr & ESR_ELx_FnV)
siaddr = NULL;
@@ -733,11 +724,6 @@ static const struct fault_info fault_info[] = {
{ do_bad, SIGKILL, SI_KERNEL, "unknown 63" },
};
-int kvm_handle_guest_sea(phys_addr_t addr, unsigned int esr)
-{
- return ghes_notify_sea();
-}
-
asmlinkage void __exception do_mem_abort(unsigned long addr, unsigned int esr,
struct pt_regs *regs)
{
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 15/26] ACPI / APEI: Move locking to the notification helper
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (13 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 14/26] arm64: KVM/mm: Move SEA handling behind a single 'claim' interface James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 16/26] ACPI / APEI: Let the notification helper specify the fixmap slot James Morse
` (11 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
ghes_copy_tofrom_phys() takes different locks depending on in_nmi().
This doesn't work if there are multiple NMI-like notifications, that
can interrupt each other.
Now that NOTIFY_SEA is always called in the same context, move the
lock-taking to the notification helper. The helper will always know
which lock to take. This avoids ghes_copy_tofrom_phys() taking a guess
based on in_nmi().
This splits NOTIFY_NMI and NOTIFY_SEA to use different locks. All
the other notifications use ghes_proc(), and are called in process
or IRQ context. Move the spin_lock_irqsave() around their ghes_proc()
calls.
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
---
Changes since v7:
* Moved the locks into the function that uses them to make it clearer this
is only use in_nmi().
Changes since v6:
* Tinkered with the commit message
* Lock definitions have moved due to the #ifdefs
---
drivers/acpi/apei/ghes.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index ab794ab29554..c6bc73281d6a 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -114,11 +114,10 @@ static DEFINE_MUTEX(ghes_list_mutex);
* handler, but general ioremap can not be used in atomic context, so
* the fixmap is used instead.
*
- * These 2 spinlocks are used to prevent the fixmap entries from being used
+ * This spinlock is used to prevent the fixmap entry from being used
* simultaneously.
*/
-static DEFINE_RAW_SPINLOCK(ghes_ioremap_lock_nmi);
-static DEFINE_SPINLOCK(ghes_ioremap_lock_irq);
+static DEFINE_SPINLOCK(ghes_notify_lock_irq);
static struct gen_pool *ghes_estatus_pool;
static unsigned long ghes_estatus_pool_size_request;
@@ -287,7 +286,6 @@ static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
int from_phys)
{
void __iomem *vaddr;
- unsigned long flags = 0;
int in_nmi = in_nmi();
u64 offset;
u32 trunk;
@@ -295,10 +293,8 @@ static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
while (len > 0) {
offset = paddr - (paddr & PAGE_MASK);
if (in_nmi) {
- raw_spin_lock(&ghes_ioremap_lock_nmi);
vaddr = ghes_ioremap_pfn_nmi(paddr >> PAGE_SHIFT);
} else {
- spin_lock_irqsave(&ghes_ioremap_lock_irq, flags);
vaddr = ghes_ioremap_pfn_irq(paddr >> PAGE_SHIFT);
}
trunk = PAGE_SIZE - offset;
@@ -312,10 +308,8 @@ static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
buffer += trunk;
if (in_nmi) {
ghes_iounmap_nmi();
- raw_spin_unlock(&ghes_ioremap_lock_nmi);
} else {
ghes_iounmap_irq();
- spin_unlock_irqrestore(&ghes_ioremap_lock_irq, flags);
}
}
}
@@ -729,8 +723,11 @@ static void ghes_add_timer(struct ghes *ghes)
static void ghes_poll_func(struct timer_list *t)
{
struct ghes *ghes = from_timer(ghes, t, timer);
+ unsigned long flags;
+ spin_lock_irqsave(&ghes_notify_lock_irq, flags);
ghes_proc(ghes);
+ spin_unlock_irqrestore(&ghes_notify_lock_irq, flags);
if (!(ghes->flags & GHES_EXITING))
ghes_add_timer(ghes);
}
@@ -738,9 +735,12 @@ static void ghes_poll_func(struct timer_list *t)
static irqreturn_t ghes_irq_func(int irq, void *data)
{
struct ghes *ghes = data;
+ unsigned long flags;
int rc;
+ spin_lock_irqsave(&ghes_notify_lock_irq, flags);
rc = ghes_proc(ghes);
+ spin_unlock_irqrestore(&ghes_notify_lock_irq, flags);
if (rc)
return IRQ_NONE;
@@ -751,14 +751,17 @@ static int ghes_notify_hed(struct notifier_block *this, unsigned long event,
void *data)
{
struct ghes *ghes;
+ unsigned long flags;
int ret = NOTIFY_DONE;
+ spin_lock_irqsave(&ghes_notify_lock_irq, flags);
rcu_read_lock();
list_for_each_entry_rcu(ghes, &ghes_hed, list) {
if (!ghes_proc(ghes))
ret = NOTIFY_OK;
}
rcu_read_unlock();
+ spin_unlock_irqrestore(&ghes_notify_lock_irq, flags);
return ret;
}
@@ -913,7 +916,14 @@ static LIST_HEAD(ghes_sea);
*/
int ghes_notify_sea(void)
{
- return ghes_in_nmi_spool_from_list(&ghes_sea);
+ static DEFINE_RAW_SPINLOCK(ghes_notify_lock_sea);
+ int rv;
+
+ raw_spin_lock(&ghes_notify_lock_sea);
+ rv = ghes_in_nmi_spool_from_list(&ghes_sea);
+ raw_spin_unlock(&ghes_notify_lock_sea);
+
+ return rv;
}
static void ghes_sea_add(struct ghes *ghes)
@@ -946,14 +956,17 @@ static LIST_HEAD(ghes_nmi);
static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
{
+ static DEFINE_RAW_SPINLOCK(ghes_notify_lock_nmi);
int err, ret = NMI_DONE;
if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
return ret;
+ raw_spin_lock(&ghes_notify_lock_nmi);
err = ghes_in_nmi_spool_from_list(&ghes_nmi);
if (!err)
ret = NMI_HANDLED;
+ raw_spin_unlock(&ghes_notify_lock_nmi);
atomic_dec(&ghes_in_nmi);
return ret;
@@ -995,6 +1008,7 @@ static int ghes_probe(struct platform_device *ghes_dev)
{
struct acpi_hest_generic *generic;
struct ghes *ghes = NULL;
+ unsigned long flags;
int rc = -EINVAL;
@@ -1097,7 +1111,9 @@ static int ghes_probe(struct platform_device *ghes_dev)
ghes_edac_register(ghes, &ghes_dev->dev);
/* Handle any pending errors right away */
+ spin_lock_irqsave(&ghes_notify_lock_irq, flags);
ghes_proc(ghes);
+ spin_unlock_irqrestore(&ghes_notify_lock_irq, flags);
return 0;
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 16/26] ACPI / APEI: Let the notification helper specify the fixmap slot
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (14 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 15/26] ACPI / APEI: Move locking to the notification helper James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 17/26] ACPI / APEI: Pass ghes and estatus separately to avoid a later copy James Morse
` (10 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
ghes_copy_tofrom_phys() uses a different fixmap slot depending on in_nmi().
This doesn't work when there are multiple NMI-like notifications, that
could interrupt each other.
As with the locking, move the chosen fixmap_idx to the notification helper.
This only matters for NMI-like notifications, anything calling
ghes_proc() can use the IRQ fixmap slot as its already holding an irqsave
spinlock.
This lets us collapse the ghes_ioremap_pfn_*() helpers.
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
---
The fixmap-idx and vaddr are passed back to ghes_unmap()
to allow ioremap() to be used in process context in the
future. This will let us send tlbi-ipi for notifications
that don't mask irqs.
Changes since v7:
* Wwitched unmap arg order for concistency, p/v addr is always first
* use the enum name for the fixmap_idx, in the hope the compiler validates it.
---
drivers/acpi/apei/ghes.c | 92 +++++++++++++++++-----------------------
1 file changed, 39 insertions(+), 53 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index c6bc73281d6a..ccad57468ab7 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -41,6 +41,7 @@
#include <linux/llist.h>
#include <linux/genalloc.h>
#include <linux/pci.h>
+#include <linux/pfn.h>
#include <linux/aer.h>
#include <linux/nmi.h>
#include <linux/sched/clock.h>
@@ -127,38 +128,24 @@ static atomic_t ghes_estatus_cache_alloced;
static int ghes_panic_timeout __read_mostly = 30;
-static void __iomem *ghes_ioremap_pfn_nmi(u64 pfn)
+static void __iomem *ghes_map(u64 pfn, enum fixed_addresses fixmap_idx)
{
phys_addr_t paddr;
pgprot_t prot;
- paddr = pfn << PAGE_SHIFT;
+ paddr = PFN_PHYS(pfn);
prot = arch_apei_get_mem_attribute(paddr);
- __set_fixmap(FIX_APEI_GHES_NMI, paddr, prot);
+ __set_fixmap(fixmap_idx, paddr, prot);
- return (void __iomem *) fix_to_virt(FIX_APEI_GHES_NMI);
+ return (void __iomem *) __fix_to_virt(fixmap_idx);
}
-static void __iomem *ghes_ioremap_pfn_irq(u64 pfn)
+static void ghes_unmap(void __iomem *vaddr, enum fixed_addresses fixmap_idx)
{
- phys_addr_t paddr;
- pgprot_t prot;
-
- paddr = pfn << PAGE_SHIFT;
- prot = arch_apei_get_mem_attribute(paddr);
- __set_fixmap(FIX_APEI_GHES_IRQ, paddr, prot);
+ int _idx = virt_to_fix((unsigned long)vaddr);
- return (void __iomem *) fix_to_virt(FIX_APEI_GHES_IRQ);
-}
-
-static void ghes_iounmap_nmi(void)
-{
- clear_fixmap(FIX_APEI_GHES_NMI);
-}
-
-static void ghes_iounmap_irq(void)
-{
- clear_fixmap(FIX_APEI_GHES_IRQ);
+ WARN_ON_ONCE(fixmap_idx != _idx);
+ clear_fixmap(fixmap_idx);
}
int ghes_estatus_pool_init(int num_ghes)
@@ -283,20 +270,16 @@ static inline int ghes_severity(int severity)
}
static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
- int from_phys)
+ int from_phys,
+ enum fixed_addresses fixmap_idx)
{
void __iomem *vaddr;
- int in_nmi = in_nmi();
u64 offset;
u32 trunk;
while (len > 0) {
offset = paddr - (paddr & PAGE_MASK);
- if (in_nmi) {
- vaddr = ghes_ioremap_pfn_nmi(paddr >> PAGE_SHIFT);
- } else {
- vaddr = ghes_ioremap_pfn_irq(paddr >> PAGE_SHIFT);
- }
+ vaddr = ghes_map(PHYS_PFN(paddr), fixmap_idx);
trunk = PAGE_SIZE - offset;
trunk = min(trunk, len);
if (from_phys)
@@ -306,15 +289,13 @@ static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
len -= trunk;
paddr += trunk;
buffer += trunk;
- if (in_nmi) {
- ghes_iounmap_nmi();
- } else {
- ghes_iounmap_irq();
- }
+ ghes_unmap(vaddr, fixmap_idx);
}
}
-static int ghes_read_estatus(struct ghes *ghes, u64 *buf_paddr)
+static int ghes_read_estatus(struct ghes *ghes, u64 *buf_paddr,
+ enum fixed_addresses fixmap_idx)
+
{
struct acpi_hest_generic *g = ghes->generic;
u32 len;
@@ -332,7 +313,7 @@ static int ghes_read_estatus(struct ghes *ghes, u64 *buf_paddr)
return -ENOENT;
ghes_copy_tofrom_phys(ghes->estatus, *buf_paddr,
- sizeof(*ghes->estatus), 1);
+ sizeof(*ghes->estatus), 1, fixmap_idx);
if (!ghes->estatus->block_status) {
*buf_paddr = 0;
return -ENOENT;
@@ -348,7 +329,7 @@ static int ghes_read_estatus(struct ghes *ghes, u64 *buf_paddr)
goto err_read_block;
ghes_copy_tofrom_phys(ghes->estatus + 1,
*buf_paddr + sizeof(*ghes->estatus),
- len - sizeof(*ghes->estatus), 1);
+ len - sizeof(*ghes->estatus), 1, fixmap_idx);
if (cper_estatus_check(ghes->estatus))
goto err_read_block;
rc = 0;
@@ -361,7 +342,8 @@ static int ghes_read_estatus(struct ghes *ghes, u64 *buf_paddr)
return rc;
}
-static void ghes_clear_estatus(struct ghes *ghes, u64 buf_paddr)
+static void ghes_clear_estatus(struct ghes *ghes, u64 buf_paddr,
+ enum fixed_addresses fixmap_idx)
{
ghes->estatus->block_status = 0;
@@ -369,7 +351,8 @@ static void ghes_clear_estatus(struct ghes *ghes, u64 buf_paddr)
return;
ghes_copy_tofrom_phys(ghes->estatus, buf_paddr,
- sizeof(ghes->estatus->block_status), 0);
+ sizeof(ghes->estatus->block_status), 0,
+ fixmap_idx);
/*
* GHESv2 type HEST entries introduce support for error acknowledgment,
@@ -668,11 +651,12 @@ static void ghes_estatus_cache_add(
rcu_read_unlock();
}
-static void __ghes_panic(struct ghes *ghes, u64 buf_paddr)
+static void __ghes_panic(struct ghes *ghes, u64 buf_paddr,
+ enum fixed_addresses fixmap_idx)
{
__ghes_print_estatus(KERN_EMERG, ghes->generic, ghes->estatus);
- ghes_clear_estatus(ghes, buf_paddr);
+ ghes_clear_estatus(ghes, buf_paddr, fixmap_idx);
/* reboot to log the error! */
if (!panic_timeout)
@@ -685,12 +669,12 @@ static int ghes_proc(struct ghes *ghes)
u64 buf_paddr;
int rc;
- rc = ghes_read_estatus(ghes, &buf_paddr);
+ rc = ghes_read_estatus(ghes, &buf_paddr, FIX_APEI_GHES_IRQ);
if (rc)
goto out;
if (ghes_severity(ghes->estatus->error_severity) >= GHES_SEV_PANIC) {
- __ghes_panic(ghes, buf_paddr);
+ __ghes_panic(ghes, buf_paddr, FIX_APEI_GHES_IRQ);
}
if (!ghes_estatus_cached(ghes->estatus)) {
@@ -700,7 +684,7 @@ static int ghes_proc(struct ghes *ghes)
ghes_do_proc(ghes, ghes->estatus);
out:
- ghes_clear_estatus(ghes, buf_paddr);
+ ghes_clear_estatus(ghes, buf_paddr, FIX_APEI_GHES_IRQ);
return rc;
}
@@ -866,36 +850,38 @@ static void __process_error(struct ghes *ghes)
#endif
}
-static int ghes_in_nmi_queue_one_entry(struct ghes *ghes)
+static int ghes_in_nmi_queue_one_entry(struct ghes *ghes,
+ enum fixed_addresses fixmap_idx)
{
u64 buf_paddr;
int sev;
- if (ghes_read_estatus(ghes, &buf_paddr)) {
- ghes_clear_estatus(ghes, buf_paddr);
+ if (ghes_read_estatus(ghes, &buf_paddr, fixmap_idx)) {
+ ghes_clear_estatus(ghes, buf_paddr, fixmap_idx);
return -ENOENT;
}
sev = ghes_severity(ghes->estatus->error_severity);
if (sev >= GHES_SEV_PANIC) {
ghes_print_queued_estatus();
- __ghes_panic(ghes, buf_paddr);
+ __ghes_panic(ghes, buf_paddr, fixmap_idx);
}
__process_error(ghes);
- ghes_clear_estatus(ghes, buf_paddr);
+ ghes_clear_estatus(ghes, buf_paddr, fixmap_idx);
return 0;
}
-static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list)
+static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list,
+ enum fixed_addresses fixmap_idx)
{
int err, ret = -ENOENT;
struct ghes *ghes;
rcu_read_lock();
list_for_each_entry_rcu(ghes, rcu_list, list) {
- err = ghes_in_nmi_queue_one_entry(ghes);
+ err = ghes_in_nmi_queue_one_entry(ghes, fixmap_idx);
if (!err)
ret = 0;
}
@@ -920,7 +906,7 @@ int ghes_notify_sea(void)
int rv;
raw_spin_lock(&ghes_notify_lock_sea);
- rv = ghes_in_nmi_spool_from_list(&ghes_sea);
+ rv = ghes_in_nmi_spool_from_list(&ghes_sea, FIX_APEI_GHES_NMI);
raw_spin_unlock(&ghes_notify_lock_sea);
return rv;
@@ -963,7 +949,7 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
return ret;
raw_spin_lock(&ghes_notify_lock_nmi);
- err = ghes_in_nmi_spool_from_list(&ghes_nmi);
+ err = ghes_in_nmi_spool_from_list(&ghes_nmi, FIX_APEI_GHES_NMI);
if (!err)
ret = NMI_HANDLED;
raw_spin_unlock(&ghes_notify_lock_nmi);
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 17/26] ACPI / APEI: Pass ghes and estatus separately to avoid a later copy
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (15 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 16/26] ACPI / APEI: Let the notification helper specify the fixmap slot James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 18/26] ACPI / APEI: Make GHES estatus header validation more user friendly James Morse
` (9 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
The NMI-like notifications scribble over ghes->estatus, before
copying it somewhere else. If this interrupts the ghes_probe() code
calling ghes_proc() on each struct ghes, the data is corrupted.
All the NMI-like notifications should use a queued estatus entry
from the beginning, instead of the ghes version, then copying it.
To do this, break up any use of "ghes->estatus" so that all
functions take the estatus as an argument.
This patch just moves these ghes->estatus dereferences into separate
arguments, no change in behaviour. struct ghes becomes unused in
ghes_clear_estatus() as it only wanted ghes->estatus, which we now
pass directly. This is removed.
Signed-off-by: James Morse <james.morse@arm.com>
---
Changes since v6:
* Changed subject
* Renamed ghes_estatus to src_estatus, which is a little clearer
* Removed struct ghes from ghes_clear_estatus() now that this becomes
unused in this patch.
* Mangled the commit message to be different
---
drivers/acpi/apei/ghes.c | 92 +++++++++++++++++++++-------------------
1 file changed, 49 insertions(+), 43 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index ccad57468ab7..f95db2398dd5 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -293,9 +293,9 @@ static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
}
}
-static int ghes_read_estatus(struct ghes *ghes, u64 *buf_paddr,
- enum fixed_addresses fixmap_idx)
-
+static int ghes_read_estatus(struct ghes *ghes,
+ struct acpi_hest_generic_status *estatus,
+ u64 *buf_paddr, enum fixed_addresses fixmap_idx)
{
struct acpi_hest_generic *g = ghes->generic;
u32 len;
@@ -312,25 +312,25 @@ static int ghes_read_estatus(struct ghes *ghes, u64 *buf_paddr,
if (!*buf_paddr)
return -ENOENT;
- ghes_copy_tofrom_phys(ghes->estatus, *buf_paddr,
- sizeof(*ghes->estatus), 1, fixmap_idx);
- if (!ghes->estatus->block_status) {
+ ghes_copy_tofrom_phys(estatus, *buf_paddr, sizeof(*estatus), 1,
+ fixmap_idx);
+ if (!estatus->block_status) {
*buf_paddr = 0;
return -ENOENT;
}
rc = -EIO;
- len = cper_estatus_len(ghes->estatus);
- if (len < sizeof(*ghes->estatus))
+ len = cper_estatus_len(estatus);
+ if (len < sizeof(*estatus))
goto err_read_block;
if (len > ghes->generic->error_block_length)
goto err_read_block;
- if (cper_estatus_check_header(ghes->estatus))
+ if (cper_estatus_check_header(estatus))
goto err_read_block;
- ghes_copy_tofrom_phys(ghes->estatus + 1,
- *buf_paddr + sizeof(*ghes->estatus),
- len - sizeof(*ghes->estatus), 1, fixmap_idx);
- if (cper_estatus_check(ghes->estatus))
+ ghes_copy_tofrom_phys(estatus + 1,
+ *buf_paddr + sizeof(*estatus),
+ len - sizeof(*estatus), 1, fixmap_idx);
+ if (cper_estatus_check(estatus))
goto err_read_block;
rc = 0;
@@ -342,16 +342,17 @@ static int ghes_read_estatus(struct ghes *ghes, u64 *buf_paddr,
return rc;
}
-static void ghes_clear_estatus(struct ghes *ghes, u64 buf_paddr,
- enum fixed_addresses fixmap_idx)
+static void ghes_clear_estatus(struct ghes *ghes,
+ struct acpi_hest_generic_status *estatus,
+ u64 buf_paddr, enum fixed_addresses fixmap_idx)
{
- ghes->estatus->block_status = 0;
+ estatus->block_status = 0;
if (!buf_paddr)
return;
- ghes_copy_tofrom_phys(ghes->estatus, buf_paddr,
- sizeof(ghes->estatus->block_status), 0,
+ ghes_copy_tofrom_phys(estatus, buf_paddr,
+ sizeof(estatus->block_status), 0,
fixmap_idx);
/*
@@ -651,12 +652,13 @@ static void ghes_estatus_cache_add(
rcu_read_unlock();
}
-static void __ghes_panic(struct ghes *ghes, u64 buf_paddr,
- enum fixed_addresses fixmap_idx)
+static void __ghes_panic(struct ghes *ghes,
+ struct acpi_hest_generic_status *estatus,
+ u64 buf_paddr, enum fixed_addresses fixmap_idx)
{
- __ghes_print_estatus(KERN_EMERG, ghes->generic, ghes->estatus);
+ __ghes_print_estatus(KERN_EMERG, ghes->generic, estatus);
- ghes_clear_estatus(ghes, buf_paddr, fixmap_idx);
+ ghes_clear_estatus(ghes, estatus, buf_paddr, fixmap_idx);
/* reboot to log the error! */
if (!panic_timeout)
@@ -666,25 +668,25 @@ static void __ghes_panic(struct ghes *ghes, u64 buf_paddr,
static int ghes_proc(struct ghes *ghes)
{
+ struct acpi_hest_generic_status *estatus = ghes->estatus;
u64 buf_paddr;
int rc;
- rc = ghes_read_estatus(ghes, &buf_paddr, FIX_APEI_GHES_IRQ);
+ rc = ghes_read_estatus(ghes, estatus, &buf_paddr, FIX_APEI_GHES_IRQ);
if (rc)
goto out;
- if (ghes_severity(ghes->estatus->error_severity) >= GHES_SEV_PANIC) {
- __ghes_panic(ghes, buf_paddr, FIX_APEI_GHES_IRQ);
- }
+ if (ghes_severity(estatus->error_severity) >= GHES_SEV_PANIC)
+ __ghes_panic(ghes, estatus, buf_paddr, FIX_APEI_GHES_IRQ);
- if (!ghes_estatus_cached(ghes->estatus)) {
- if (ghes_print_estatus(NULL, ghes->generic, ghes->estatus))
- ghes_estatus_cache_add(ghes->generic, ghes->estatus);
+ if (!ghes_estatus_cached(estatus)) {
+ if (ghes_print_estatus(NULL, ghes->generic, estatus))
+ ghes_estatus_cache_add(ghes->generic, estatus);
}
- ghes_do_proc(ghes, ghes->estatus);
+ ghes_do_proc(ghes, estatus);
out:
- ghes_clear_estatus(ghes, buf_paddr, FIX_APEI_GHES_IRQ);
+ ghes_clear_estatus(ghes, estatus, buf_paddr, FIX_APEI_GHES_IRQ);
return rc;
}
@@ -825,17 +827,20 @@ static void ghes_print_queued_estatus(void)
}
/* Save estatus for further processing in IRQ context */
-static void __process_error(struct ghes *ghes)
+static void __process_error(struct ghes *ghes,
+ struct acpi_hest_generic_status *src_estatus)
{
-#ifdef CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG
u32 len, node_len;
struct ghes_estatus_node *estatus_node;
struct acpi_hest_generic_status *estatus;
- if (ghes_estatus_cached(ghes->estatus))
+ if (!IS_ENABLED(CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG))
return;
- len = cper_estatus_len(ghes->estatus);
+ if (ghes_estatus_cached(src_estatus))
+ return;
+
+ len = cper_estatus_len(src_estatus);
node_len = GHES_ESTATUS_NODE_LEN(len);
estatus_node = (void *)gen_pool_alloc(ghes_estatus_pool, node_len);
@@ -845,30 +850,31 @@ static void __process_error(struct ghes *ghes)
estatus_node->ghes = ghes;
estatus_node->generic = ghes->generic;
estatus = GHES_ESTATUS_FROM_NODE(estatus_node);
- memcpy(estatus, ghes->estatus, len);
+ memcpy(estatus, src_estatus, len);
llist_add(&estatus_node->llnode, &ghes_estatus_llist);
-#endif
}
static int ghes_in_nmi_queue_one_entry(struct ghes *ghes,
enum fixed_addresses fixmap_idx)
{
+ struct acpi_hest_generic_status *estatus = ghes->estatus;
u64 buf_paddr;
int sev;
- if (ghes_read_estatus(ghes, &buf_paddr, fixmap_idx)) {
- ghes_clear_estatus(ghes, buf_paddr, fixmap_idx);
+ if (ghes_read_estatus(ghes, estatus, &buf_paddr, fixmap_idx)) {
+ ghes_clear_estatus(ghes, estatus, buf_paddr, fixmap_idx);
return -ENOENT;
}
- sev = ghes_severity(ghes->estatus->error_severity);
+ sev = ghes_severity(estatus->error_severity);
if (sev >= GHES_SEV_PANIC) {
ghes_print_queued_estatus();
- __ghes_panic(ghes, buf_paddr, fixmap_idx);
+ __ghes_panic(ghes, estatus, buf_paddr, fixmap_idx);
+
}
- __process_error(ghes);
- ghes_clear_estatus(ghes, buf_paddr, fixmap_idx);
+ __process_error(ghes, estatus);
+ ghes_clear_estatus(ghes, estatus, buf_paddr, fixmap_idx);
return 0;
}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 18/26] ACPI / APEI: Make GHES estatus header validation more user friendly
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (16 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 17/26] ACPI / APEI: Pass ghes and estatus separately to avoid a later copy James Morse
@ 2019-01-29 18:48 ` James Morse
2019-02-01 14:30 ` Borislav Petkov
2019-01-29 18:48 ` [PATCH v8 19/26] ACPI / APEI: Split ghes_read_estatus() to allow a peek at the CPER length James Morse
` (8 subsequent siblings)
26 siblings, 1 reply; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
ghes_read_estatus() checks various lengths in the top-level header to
ensure the CPER records to be read aren't obviously corrupt.
Take the opportunity to make this more user-friendly, printing a
(ratelimited) message about the nature of the header format error.
Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: James Morse <james.morse@arm.com>
---
drivers/acpi/apei/ghes.c | 46 ++++++++++++++++++++++++++++------------
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index f95db2398dd5..9391fff71344 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -293,6 +293,30 @@ static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
}
}
+/* Check the top-level record header has an appropriate size. */
+int __ghes_check_estatus(struct ghes *ghes,
+ struct acpi_hest_generic_status *estatus)
+{
+ u32 len = cper_estatus_len(estatus);
+
+ if (len < sizeof(*estatus)) {
+ pr_warn_ratelimited(FW_WARN GHES_PFX "Truncated error status block!\n");
+ return -EIO;
+ }
+
+ if (len > ghes->generic->error_block_length) {
+ pr_warn_ratelimited(FW_WARN GHES_PFX "Invalid error status block length!\n");
+ return -EIO;
+ }
+
+ if (cper_estatus_check_header(estatus)) {
+ pr_warn_ratelimited(FW_WARN GHES_PFX "Invalid CPER header!\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
static int ghes_read_estatus(struct ghes *ghes,
struct acpi_hest_generic_status *estatus,
u64 *buf_paddr, enum fixed_addresses fixmap_idx)
@@ -319,27 +343,21 @@ static int ghes_read_estatus(struct ghes *ghes,
return -ENOENT;
}
- rc = -EIO;
+ rc = __ghes_check_estatus(ghes, estatus);
+ if (rc)
+ return rc;
+
len = cper_estatus_len(estatus);
- if (len < sizeof(*estatus))
- goto err_read_block;
- if (len > ghes->generic->error_block_length)
- goto err_read_block;
- if (cper_estatus_check_header(estatus))
- goto err_read_block;
ghes_copy_tofrom_phys(estatus + 1,
*buf_paddr + sizeof(*estatus),
len - sizeof(*estatus), 1, fixmap_idx);
- if (cper_estatus_check(estatus))
- goto err_read_block;
- rc = 0;
-
-err_read_block:
- if (rc)
+ if (cper_estatus_check(estatus)) {
pr_warn_ratelimited(FW_WARN GHES_PFX
"Failed to read error status block!\n");
+ return -EIO;
+ }
- return rc;
+ return 0;
}
static void ghes_clear_estatus(struct ghes *ghes,
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v8 18/26] ACPI / APEI: Make GHES estatus header validation more user friendly
2019-01-29 18:48 ` [PATCH v8 18/26] ACPI / APEI: Make GHES estatus header validation more user friendly James Morse
@ 2019-02-01 14:30 ` Borislav Petkov
0 siblings, 0 replies; 36+ messages in thread
From: Borislav Petkov @ 2019-02-01 14:30 UTC (permalink / raw)
To: James Morse
Cc: Rafael Wysocki, Tony Luck, linux-mm, Marc Zyngier,
Catalin Marinas, Xie XiuQi, Will Deacon, Christoffer Dall,
Dongjiu Geng, linux-acpi, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
On Tue, Jan 29, 2019 at 06:48:54PM +0000, James Morse wrote:
> ghes_read_estatus() checks various lengths in the top-level header to
> ensure the CPER records to be read aren't obviously corrupt.
>
> Take the opportunity to make this more user-friendly, printing a
> (ratelimited) message about the nature of the header format error.
>
> Suggested-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> drivers/acpi/apei/ghes.c | 46 ++++++++++++++++++++++++++++------------
> 1 file changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index f95db2398dd5..9391fff71344 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -293,6 +293,30 @@ static void ghes_copy_tofrom_phys(void *buffer, u64 paddr, u32 len,
> }
> }
>
> +/* Check the top-level record header has an appropriate size. */
> +int __ghes_check_estatus(struct ghes *ghes,
static.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v8 19/26] ACPI / APEI: Split ghes_read_estatus() to allow a peek at the CPER length
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (17 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 18/26] ACPI / APEI: Make GHES estatus header validation more user friendly James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 20/26] ACPI / APEI: Only use queued estatus entry during in_nmi_queue_one_entry() James Morse
` (7 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
ghes_read_estatus() reads the record address, then the record's
header, then performs some sanity checks before reading the
records into the provided estatus buffer.
To provide this estatus buffer the caller must know the size of the
records in advance, or always provide a worst-case sized buffer as
happens today for the non-NMI notifications.
Add a function to peek at the record's header to find the size. This
will let the NMI path allocate the right amount of memory before reading
the records, instead of using the worst-case size, and having to copy
the records.
Split ghes_read_estatus() to create __ghes_peek_estatus() which
returns the address and size of the CPER records.
Signed-off-by: James Morse <james.morse@arm.com>
Changes since v7:
* Grammar
* concistent argument ordering
Changes since v6:
* Additional buf_addr = 0 error handling
* Moved checking out of peek-estatus
* Reworded an error message so we can tell them apart
---
drivers/acpi/apei/ghes.c | 40 +++++++++++++++++++++++++++++-----------
1 file changed, 29 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 9391fff71344..12375a82fa03 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -317,12 +317,12 @@ int __ghes_check_estatus(struct ghes *ghes,
return 0;
}
-static int ghes_read_estatus(struct ghes *ghes,
- struct acpi_hest_generic_status *estatus,
- u64 *buf_paddr, enum fixed_addresses fixmap_idx)
+/* Read the CPER block, returning its address, and header in estatus. */
+static int __ghes_peek_estatus(struct ghes *ghes,
+ struct acpi_hest_generic_status *estatus,
+ u64 *buf_paddr, enum fixed_addresses fixmap_idx)
{
struct acpi_hest_generic *g = ghes->generic;
- u32 len;
int rc;
rc = apei_read(buf_paddr, &g->error_status_address);
@@ -343,14 +343,14 @@ static int ghes_read_estatus(struct ghes *ghes,
return -ENOENT;
}
- rc = __ghes_check_estatus(ghes, estatus);
- if (rc)
- return rc;
+ return __ghes_check_estatus(ghes, estatus);
+}
- len = cper_estatus_len(estatus);
- ghes_copy_tofrom_phys(estatus + 1,
- *buf_paddr + sizeof(*estatus),
- len - sizeof(*estatus), 1, fixmap_idx);
+static int __ghes_read_estatus(struct acpi_hest_generic_status *estatus,
+ u64 buf_paddr, enum fixed_addresses fixmap_idx,
+ size_t buf_len)
+{
+ ghes_copy_tofrom_phys(estatus, buf_paddr, buf_len, 1, fixmap_idx);
if (cper_estatus_check(estatus)) {
pr_warn_ratelimited(FW_WARN GHES_PFX
"Failed to read error status block!\n");
@@ -360,6 +360,24 @@ static int ghes_read_estatus(struct ghes *ghes,
return 0;
}
+static int ghes_read_estatus(struct ghes *ghes,
+ struct acpi_hest_generic_status *estatus,
+ u64 *buf_paddr, enum fixed_addresses fixmap_idx)
+{
+ int rc;
+
+ rc = __ghes_peek_estatus(ghes, estatus, buf_paddr, fixmap_idx);
+ if (rc)
+ return rc;
+
+ rc = __ghes_check_estatus(ghes, estatus);
+ if (rc)
+ return rc;
+
+ return __ghes_read_estatus(estatus, *buf_paddr, fixmap_idx,
+ cper_estatus_len(estatus));
+}
+
static void ghes_clear_estatus(struct ghes *ghes,
struct acpi_hest_generic_status *estatus,
u64 buf_paddr, enum fixed_addresses fixmap_idx)
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 20/26] ACPI / APEI: Only use queued estatus entry during in_nmi_queue_one_entry()
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (18 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 19/26] ACPI / APEI: Split ghes_read_estatus() to allow a peek at the CPER length James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 21/26] ACPI / APEI: Use separate fixmap pages for arm64 NMI-like notifications James Morse
` (6 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
Each struct ghes has an worst-case sized buffer for storing the
estatus. If an error is being processed by ghes_proc() in process
context this buffer will be in use. If the error source then triggers
an NMI-like notification, the same buffer will be used by
in_nmi_queue_one_entry() to stage the estatus data, before
__process_error() copys it into a queued estatus entry.
Merge __process_error()s work into in_nmi_queue_one_entry() so that
the queued estatus entry is used from the beginning. Use the new
ghes_peek_estatus() to know how much memory to allocate from
the ghes_estatus_pool before reading the records.
Reported-by: Borislav Petkov <bp@suse.de>
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Borislav Petkov <bp@suse.de>
Change since v6:
* Added a comment explaining the 'ack-error, then goto no_work'.
* Added missing esatus-clearing, which is necessary after reading the GAS,
---
drivers/acpi/apei/ghes.c | 64 +++++++++++++++++++++++-----------------
1 file changed, 37 insertions(+), 27 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 12375a82fa03..957c1559ebf5 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -862,57 +862,67 @@ static void ghes_print_queued_estatus(void)
}
}
-/* Save estatus for further processing in IRQ context */
-static void __process_error(struct ghes *ghes,
- struct acpi_hest_generic_status *src_estatus)
+static int ghes_in_nmi_queue_one_entry(struct ghes *ghes,
+ enum fixed_addresses fixmap_idx)
{
- u32 len, node_len;
+ struct acpi_hest_generic_status *estatus, tmp_header;
struct ghes_estatus_node *estatus_node;
- struct acpi_hest_generic_status *estatus;
+ u32 len, node_len;
+ u64 buf_paddr;
+ int sev, rc;
if (!IS_ENABLED(CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG))
- return;
+ return -EOPNOTSUPP;
- if (ghes_estatus_cached(src_estatus))
- return;
+ rc = __ghes_peek_estatus(ghes, &tmp_header, &buf_paddr, fixmap_idx);
+ if (rc) {
+ ghes_clear_estatus(ghes, &tmp_header, buf_paddr, fixmap_idx);
+ return rc;
+ }
- len = cper_estatus_len(src_estatus);
- node_len = GHES_ESTATUS_NODE_LEN(len);
+ rc = __ghes_check_estatus(ghes, &tmp_header);
+ if (rc) {
+ ghes_clear_estatus(ghes, &tmp_header, buf_paddr, fixmap_idx);
+ return rc;
+ }
+ len = cper_estatus_len(&tmp_header);
+ node_len = GHES_ESTATUS_NODE_LEN(len);
estatus_node = (void *)gen_pool_alloc(ghes_estatus_pool, node_len);
if (!estatus_node)
- return;
+ return -ENOMEM;
estatus_node->ghes = ghes;
estatus_node->generic = ghes->generic;
estatus = GHES_ESTATUS_FROM_NODE(estatus_node);
- memcpy(estatus, src_estatus, len);
- llist_add(&estatus_node->llnode, &ghes_estatus_llist);
-}
-static int ghes_in_nmi_queue_one_entry(struct ghes *ghes,
- enum fixed_addresses fixmap_idx)
-{
- struct acpi_hest_generic_status *estatus = ghes->estatus;
- u64 buf_paddr;
- int sev;
-
- if (ghes_read_estatus(ghes, estatus, &buf_paddr, fixmap_idx)) {
+ if (__ghes_read_estatus(estatus, buf_paddr, fixmap_idx, len)) {
ghes_clear_estatus(ghes, estatus, buf_paddr, fixmap_idx);
- return -ENOENT;
+ rc = -ENOENT;
+ goto no_work;
}
sev = ghes_severity(estatus->error_severity);
if (sev >= GHES_SEV_PANIC) {
ghes_print_queued_estatus();
__ghes_panic(ghes, estatus, buf_paddr, fixmap_idx);
-
}
- __process_error(ghes, estatus);
- ghes_clear_estatus(ghes, estatus, buf_paddr, fixmap_idx);
+ ghes_clear_estatus(ghes, &tmp_header, buf_paddr, fixmap_idx);
- return 0;
+ /* This error has been reported before, don't process it again. */
+ if (ghes_estatus_cached(estatus))
+ goto no_work;
+
+ llist_add(&estatus_node->llnode, &ghes_estatus_llist);
+
+ return rc;
+
+no_work:
+ gen_pool_free(ghes_estatus_pool, (unsigned long)estatus_node,
+ node_len);
+
+ return rc;
}
static int ghes_in_nmi_spool_from_list(struct list_head *rcu_list,
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 21/26] ACPI / APEI: Use separate fixmap pages for arm64 NMI-like notifications
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (19 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 20/26] ACPI / APEI: Only use queued estatus entry during in_nmi_queue_one_entry() James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 22/26] mm/memory-failure: Add memory_failure_queue_kick() James Morse
` (5 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
Now that ghes notification helpers provide the fixmap slots and
take the lock themselves, multiple NMI-like notifications can
be used on arm64.
These should be named after their notification method as they can't
all be called 'NMI'. x86's NOTIFY_NMI already is, change the SEA
fixmap entry to be called FIX_APEI_GHES_SEA.
Future patches can add support for FIX_APEI_GHES_SEI and
FIX_APEI_GHES_SDEI_{NORMAL,CRITICAL}.
Because all of ghes.c builds on both architectures, provide a
constant for each fixmap entry that the architecture will never
use.
Signed-off-by: James Morse <james.morse@arm.com>
---
Changes since v7:
* Removed v6's #ifdefs, these aren't needed now that SEA/NMI can't be
turned off on their respective architectures.
Changes since v6:
* Added #ifdef definitions of each missing fixmap entry.
Changes since v3:
* idx/lock are now in a separate struct.
* Add to the comment above ghes_fixmap_lock_irq so that it makes more
sense in isolation.
---
arch/arm64/include/asm/fixmap.h | 2 +-
drivers/acpi/apei/ghes.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
index ec1e6d6fa14c..966dd4bb23f2 100644
--- a/arch/arm64/include/asm/fixmap.h
+++ b/arch/arm64/include/asm/fixmap.h
@@ -55,7 +55,7 @@ enum fixed_addresses {
#ifdef CONFIG_ACPI_APEI_GHES
/* Used for GHES mapping from assorted contexts */
FIX_APEI_GHES_IRQ,
- FIX_APEI_GHES_NMI,
+ FIX_APEI_GHES_SEA,
#endif /* CONFIG_ACPI_APEI_GHES */
#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 957c1559ebf5..e6f0d176b245 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -958,7 +958,7 @@ int ghes_notify_sea(void)
int rv;
raw_spin_lock(&ghes_notify_lock_sea);
- rv = ghes_in_nmi_spool_from_list(&ghes_sea, FIX_APEI_GHES_NMI);
+ rv = ghes_in_nmi_spool_from_list(&ghes_sea, FIX_APEI_GHES_SEA);
raw_spin_unlock(&ghes_notify_lock_sea);
return rv;
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 22/26] mm/memory-failure: Add memory_failure_queue_kick()
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (20 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 21/26] ACPI / APEI: Use separate fixmap pages for arm64 NMI-like notifications James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:48 ` [PATCH v8 23/26] ACPI / APEI: Kick the memory_failure() queue for synchronous errors James Morse
` (4 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
The GHES code calls memory_failure_queue() from IRQ context to schedule
work on the current CPU so that memory_failure() can sleep.
For synchronous memory errors the arch code needs to know any signals
that memory_failure() will trigger are pending before it returns to
user-space, possibly when exiting from the IRQ.
Add a helper to kick the memory failure queue, to ensure the scheduled
work has happened. This has to be called from process context, so may
have been migrated from the original cpu. Pass the cpu the work was
queued on.
Change memory_failure_work_func() to permit being called on the 'wrong'
cpu.
Signed-off-by: James Morse <james.morse@arm.com>
---
include/linux/mm.h | 1 +
mm/memory-failure.c | 15 ++++++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 80bb6408fe73..b33bededc69d 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2743,6 +2743,7 @@ enum mf_flags {
};
extern int memory_failure(unsigned long pfn, int flags);
extern void memory_failure_queue(unsigned long pfn, int flags);
+extern void memory_failure_queue_kick(int cpu);
extern int unpoison_memory(unsigned long pfn);
extern int get_hwpoison_page(struct page *page);
#define put_hwpoison_page(page) put_page(page)
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 6379fff1a5ff..9b4705a53fed 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1494,7 +1494,7 @@ static void memory_failure_work_func(struct work_struct *work)
unsigned long proc_flags;
int gotten;
- mf_cpu = this_cpu_ptr(&memory_failure_cpu);
+ mf_cpu = container_of(work, struct memory_failure_cpu, work);
for (;;) {
spin_lock_irqsave(&mf_cpu->lock, proc_flags);
gotten = kfifo_get(&mf_cpu->fifo, &entry);
@@ -1508,6 +1508,19 @@ static void memory_failure_work_func(struct work_struct *work)
}
}
+/*
+ * Process memory_failure work queued on the specified CPU.
+ * Used to avoid return-to-userspace racing with the memory_failure workqueue.
+ */
+void memory_failure_queue_kick(int cpu)
+{
+ struct memory_failure_cpu *mf_cpu;
+
+ mf_cpu = &per_cpu(memory_failure_cpu, cpu);
+ cancel_work_sync(&mf_cpu->work);
+ memory_failure_work_func(&mf_cpu->work);
+}
+
static int __init memory_failure_init(void)
{
struct memory_failure_cpu *mf_cpu;
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 23/26] ACPI / APEI: Kick the memory_failure() queue for synchronous errors
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (21 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 22/26] mm/memory-failure: Add memory_failure_queue_kick() James Morse
@ 2019-01-29 18:48 ` James Morse
2019-01-29 18:49 ` [PATCH v8 24/26] arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work James Morse
` (3 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:48 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
memory_failure() offlines or repairs pages of memory that have been
discovered to be corrupt. These may be detected by an external
component, (e.g. the memory controller), and notified via an IRQ.
In this case the work is queued as not all of memory_failure()s work
can happen in IRQ context.
If the error was detected as a result of user-space accessing a
corrupt memory location the CPU may take an abort instead. On arm64
this is a 'synchronous external abort', and on a firmware first
system it is replayed using NOTIFY_SEA.
This notification has NMI like properties, (it can interrupt
IRQ-masked code), so the memory_failure() work is queued. If we
return to user-space before the queued memory_failure() work is
processed, we will take the fault again. This loop may cause platform
firmware to exceed some threshold and reboot when Linux could have
recovered from this error.
For NMIlike notifications keep track of whether memory_failure() work
was queued, and make task_work pending to flush out the queue.
To save memory allocations, the task_work is allocated as part of
the ghes_estatus_node, and free()ing it back to the pool is deferred.
Signed-off-by: James Morse <james.morse@arm.com>
---
current->mm == &init_mm ? I couldn't find a helper for this.
The intent is not to set TIF flags on kernel threads. What happens
if a kernel-thread takes on of these? Its just one of the many
not-handled-very-well cases we have already, as memory_failure()
puts it: "try to be lucky".
I assume that if NOTIFY_NMI is coming from SMM it must suffer from
this problem too.
Changes since v7:
* Don't allocate memory, stuff it in estatus_node. This means passing
back a 'queued' flag to ghes_proc_in_irq() which holds the estatus_node.
---
drivers/acpi/apei/ghes.c | 68 +++++++++++++++++++++++++++++++++-------
include/acpi/ghes.h | 3 ++
2 files changed, 60 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index e6f0d176b245..dfa8f155f964 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -47,6 +47,7 @@
#include <linux/sched/clock.h>
#include <linux/uuid.h>
#include <linux/ras.h>
+#include <linux/task_work.h>
#include <acpi/actbl1.h>
#include <acpi/ghes.h>
@@ -399,23 +400,46 @@ static void ghes_clear_estatus(struct ghes *ghes,
ghes_ack_error(ghes->generic_v2);
}
-static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int sev)
+/*
+ * Called as task_work before returning to user-space.
+ * Ensure any queued work has been done before we return to the context that
+ * triggered the notification.
+ */
+static void ghes_kick_task_work(struct callback_head *head)
+{
+ struct acpi_hest_generic_status *estatus;
+ struct ghes_estatus_node *estatus_node;
+ u32 node_len;
+
+ estatus_node = container_of(head, struct ghes_estatus_node, task_work);
+ memory_failure_queue_kick(estatus_node->task_work_cpu);
+
+ estatus = GHES_ESTATUS_FROM_NODE(estatus_node);
+ node_len = GHES_ESTATUS_NODE_LEN(cper_estatus_len(estatus));
+ gen_pool_free(ghes_estatus_pool, (unsigned long)estatus_node, node_len);
+}
+
+static bool ghes_handle_memory_failure(struct ghes *ghes,
+ struct acpi_hest_generic_data *gdata,
+ int sev)
{
-#ifdef CONFIG_ACPI_APEI_MEMORY_FAILURE
unsigned long pfn;
int flags = -1;
int sec_sev = ghes_severity(gdata->error_severity);
struct cper_sec_mem_err *mem_err = acpi_hest_get_payload(gdata);
+ if (!IS_ENABLED(CONFIG_ACPI_APEI_MEMORY_FAILURE))
+ return false;
+
if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
- return;
+ return false;
pfn = mem_err->physical_addr >> PAGE_SHIFT;
if (!pfn_valid(pfn)) {
pr_warn_ratelimited(FW_WARN GHES_PFX
"Invalid address in generic error data: %#llx\n",
mem_err->physical_addr);
- return;
+ return false;
}
/* iff following two events can be handled properly by now */
@@ -425,9 +449,12 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int
if (sev == GHES_SEV_RECOVERABLE && sec_sev == GHES_SEV_RECOVERABLE)
flags = 0;
- if (flags != -1)
+ if (flags != -1) {
memory_failure_queue(pfn, flags);
-#endif
+ return true;
+ }
+
+ return false;
}
/*
@@ -475,11 +502,12 @@ static void ghes_handle_aer(struct acpi_hest_generic_data *gdata)
#endif
}
-static void ghes_do_proc(struct ghes *ghes,
+static bool ghes_do_proc(struct ghes *ghes,
const struct acpi_hest_generic_status *estatus)
{
int sev, sec_sev;
struct acpi_hest_generic_data *gdata;
+ bool work_queued = false;
guid_t *sec_type;
guid_t *fru_id = &NULL_UUID_LE;
char *fru_text = "";
@@ -500,7 +528,8 @@ static void ghes_do_proc(struct ghes *ghes,
ghes_edac_report_mem_error(sev, mem_err);
arch_apei_report_mem_error(sev, mem_err);
- ghes_handle_memory_failure(gdata, sev);
+ if (ghes_handle_memory_failure(ghes, gdata, sev))
+ work_queued = true;
}
else if (guid_equal(sec_type, &CPER_SEC_PCIE)) {
ghes_handle_aer(gdata);
@@ -517,6 +546,8 @@ static void ghes_do_proc(struct ghes *ghes,
gdata->error_data_length);
}
}
+
+ return work_queued;
}
static void __ghes_print_estatus(const char *pfx,
@@ -812,7 +843,9 @@ static void ghes_proc_in_irq(struct irq_work *irq_work)
struct ghes_estatus_node *estatus_node;
struct acpi_hest_generic *generic;
struct acpi_hest_generic_status *estatus;
+ bool task_work_pending;
u32 len, node_len;
+ int ret;
llnode = llist_del_all(&ghes_estatus_llist);
/*
@@ -827,14 +860,26 @@ static void ghes_proc_in_irq(struct irq_work *irq_work)
estatus = GHES_ESTATUS_FROM_NODE(estatus_node);
len = cper_estatus_len(estatus);
node_len = GHES_ESTATUS_NODE_LEN(len);
- ghes_do_proc(estatus_node->ghes, estatus);
+ task_work_pending = ghes_do_proc(estatus_node->ghes, estatus);
if (!ghes_estatus_cached(estatus)) {
generic = estatus_node->generic;
if (ghes_print_estatus(NULL, generic, estatus))
ghes_estatus_cache_add(generic, estatus);
}
- gen_pool_free(ghes_estatus_pool, (unsigned long)estatus_node,
- node_len);
+
+ if (task_work_pending && current->mm != &init_mm) {
+ estatus_node->task_work.func = ghes_kick_task_work;
+ estatus_node->task_work_cpu = smp_processor_id();
+ ret = task_work_add(current, &estatus_node->task_work,
+ true);
+ if (ret)
+ estatus_node->task_work.func = NULL;
+ }
+
+ if (!estatus_node->task_work.func)
+ gen_pool_free(ghes_estatus_pool,
+ (unsigned long)estatus_node, node_len);
+
llnode = next;
}
}
@@ -894,6 +939,7 @@ static int ghes_in_nmi_queue_one_entry(struct ghes *ghes,
estatus_node->ghes = ghes;
estatus_node->generic = ghes->generic;
+ estatus_node->task_work.func = NULL;
estatus = GHES_ESTATUS_FROM_NODE(estatus_node);
if (__ghes_read_estatus(estatus, buf_paddr, fixmap_idx, len)) {
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index e3f1cddb4ac8..517a5231cc1b 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -33,6 +33,9 @@ struct ghes_estatus_node {
struct llist_node llnode;
struct acpi_hest_generic *generic;
struct ghes *ghes;
+
+ int task_work_cpu;
+ struct callback_head task_work;
};
struct ghes_estatus_cache {
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 24/26] arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (22 preceding siblings ...)
2019-01-29 18:48 ` [PATCH v8 23/26] ACPI / APEI: Kick the memory_failure() queue for synchronous errors James Morse
@ 2019-01-29 18:49 ` James Morse
2019-01-30 8:56 ` Julien Thierry
2019-01-29 18:49 ` [PATCH v8 25/26] firmware: arm_sdei: Add ACPI GHES registration helper James Morse
` (2 subsequent siblings)
26 siblings, 1 reply; 36+ messages in thread
From: James Morse @ 2019-01-29 18:49 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
APEI is unable to do all of its error handling work in nmi-context, so
it defers non-fatal work onto the irq_work queue. arch_irq_work_raise()
sends an IPI to the calling cpu, but this is not guaranteed to be taken
before returning to user-space.
Unless the exception interrupted a context with irqs-masked,
irq_work_run() can run immediately. Otherwise return -EINPROGRESS to
indicate ghes_notify_sea() found some work to do, but it hasn't
finished yet.
With this apei_claim_sea() returning '0' means this external-abort was
also notification of a firmware-first RAS error, and that APEI has
processed the CPER records.
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>
Tested-by: Tyler Baicar <tbaicar@codeaurora.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
CC: Xie XiuQi <xiexiuqi@huawei.com>
CC: gengdongjiu <gengdongjiu@huawei.com>
---
Changes since v7:
* Added Catalin's ack, then:
* Added __irq_enter()/exit() calls so if we interrupted preemptible code, the
preempt count matches what other irq-work expects.
* Changed the 'if (!arch_irqs_disabled_flags(interrupted_flags))' test to be
safe before/after Julien's PMR series.
Changes since v6:
* Added pr_warn() for the EINPROGRESS case so panic-tracebacks show
'APEI was here'.
* Tinkered with the commit message
Changes since v2:
* Removed IS_ENABLED() check, done by the caller unless we have a dummy
definition.
---
arch/arm64/kernel/acpi.c | 23 +++++++++++++++++++++++
arch/arm64/mm/fault.c | 9 ++++-----
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 803f0494dd3e..8288ae0c8f3b 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -22,6 +22,7 @@
#include <linux/init.h>
#include <linux/irq.h>
#include <linux/irqdomain.h>
+#include <linux/irq_work.h>
#include <linux/memblock.h>
#include <linux/of_fdt.h>
#include <linux/smp.h>
@@ -268,12 +269,17 @@ pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
int apei_claim_sea(struct pt_regs *regs)
{
int err = -ENOENT;
+ bool return_to_irqs_enabled;
unsigned long current_flags;
if (!IS_ENABLED(CONFIG_ACPI_APEI_GHES))
return err;
current_flags = arch_local_save_flags();
+ return_to_irqs_enabled = !irqs_disabled_flags(current_flags);
+
+ if (regs)
+ return_to_irqs_enabled = interrupts_enabled(regs);
/*
* SEA can interrupt SError, mask it and describe this as an NMI so
@@ -283,6 +289,23 @@ int apei_claim_sea(struct pt_regs *regs)
nmi_enter();
err = ghes_notify_sea();
nmi_exit();
+
+ /*
+ * APEI NMI-like notifications are deferred to irq_work. Unless
+ * we interrupted irqs-masked code, we can do that now.
+ */
+ if (!err) {
+ if (return_to_irqs_enabled) {
+ local_daif_restore(DAIF_PROCCTX_NOIRQ);
+ __irq_enter();
+ irq_work_run();
+ __irq_exit();
+ } else {
+ pr_warn("APEI work queued but not completed");
+ err = -EINPROGRESS;
+ }
+ }
+
local_daif_restore(current_flags);
return err;
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index e1c84c2e1cab..1611714f8333 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -642,11 +642,10 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
inf = esr_to_fault_info(esr);
- /*
- * Return value ignored as we rely on signal merging.
- * Future patches will make this more robust.
- */
- apei_claim_sea(regs);
+ if (apei_claim_sea(regs) == 0) {
+ /* APEI claimed this as a firmware-first notification */
+ return 0;
+ }
if (esr & ESR_ELx_FnV)
siaddr = NULL;
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v8 24/26] arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work
2019-01-29 18:49 ` [PATCH v8 24/26] arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work James Morse
@ 2019-01-30 8:56 ` Julien Thierry
0 siblings, 0 replies; 36+ messages in thread
From: Julien Thierry @ 2019-01-30 8:56 UTC (permalink / raw)
To: James Morse, linux-acpi
Cc: Tony Luck, Marc Zyngier, Catalin Marinas, Rafael Wysocki,
Will Deacon, Dongjiu Geng, linux-mm, Borislav Petkov,
Naoya Horiguchi, kvmarm, linux-arm-kernel, Len Brown
Hi James,
On 29/01/2019 18:49, James Morse wrote:
> APEI is unable to do all of its error handling work in nmi-context, so
> it defers non-fatal work onto the irq_work queue. arch_irq_work_raise()
> sends an IPI to the calling cpu, but this is not guaranteed to be taken
> before returning to user-space.
>
> Unless the exception interrupted a context with irqs-masked,
> irq_work_run() can run immediately. Otherwise return -EINPROGRESS to
> indicate ghes_notify_sea() found some work to do, but it hasn't
> finished yet.
>
> With this apei_claim_sea() returning '0' means this external-abort was
> also notification of a firmware-first RAS error, and that APEI has
> processed the CPER records.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> Reviewed-by: Punit Agrawal <punit.agrawal@arm.com>
> Tested-by: Tyler Baicar <tbaicar@codeaurora.org>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> CC: Xie XiuQi <xiexiuqi@huawei.com>
> CC: gengdongjiu <gengdongjiu@huawei.com>
>
> ---
> Changes since v7:
> * Added Catalin's ack, then:
> * Added __irq_enter()/exit() calls so if we interrupted preemptible code, the
> preempt count matches what other irq-work expects.
> * Changed the 'if (!arch_irqs_disabled_flags(interrupted_flags))' test to be
> safe before/after Julien's PMR series.
>
> Changes since v6:
> * Added pr_warn() for the EINPROGRESS case so panic-tracebacks show
> 'APEI was here'.
> * Tinkered with the commit message
>
> Changes since v2:
> * Removed IS_ENABLED() check, done by the caller unless we have a dummy
> definition.
> ---
> arch/arm64/kernel/acpi.c | 23 +++++++++++++++++++++++
> arch/arm64/mm/fault.c | 9 ++++-----
> 2 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index 803f0494dd3e..8288ae0c8f3b 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -22,6 +22,7 @@
> #include <linux/init.h>
> #include <linux/irq.h>
> #include <linux/irqdomain.h>
> +#include <linux/irq_work.h>
> #include <linux/memblock.h>
> #include <linux/of_fdt.h>
> #include <linux/smp.h>
> @@ -268,12 +269,17 @@ pgprot_t __acpi_get_mem_attribute(phys_addr_t addr)
> int apei_claim_sea(struct pt_regs *regs)
> {
> int err = -ENOENT;
> + bool return_to_irqs_enabled;
> unsigned long current_flags;
>
> if (!IS_ENABLED(CONFIG_ACPI_APEI_GHES))
> return err;
>
> current_flags = arch_local_save_flags();
> + return_to_irqs_enabled = !irqs_disabled_flags(current_flags);
> +
> + if (regs)
> + return_to_irqs_enabled = interrupts_enabled(regs);
>
> /*
> * SEA can interrupt SError, mask it and describe this as an NMI so
> @@ -283,6 +289,23 @@ int apei_claim_sea(struct pt_regs *regs)
> nmi_enter();
> err = ghes_notify_sea();
> nmi_exit();
> +
> + /*
> + * APEI NMI-like notifications are deferred to irq_work. Unless
> + * we interrupted irqs-masked code, we can do that now.
> + */
> + if (!err) {
> + if (return_to_irqs_enabled) {
> + local_daif_restore(DAIF_PROCCTX_NOIRQ);
> + __irq_enter();
> + irq_work_run();
> + __irq_exit();
> + } else {
> + pr_warn("APEI work queued but not completed");
> + err = -EINPROGRESS;
> + }
> + }
> +
Reviewed-by: Julien Thierry <julien.thierry@arm.com>
Cheers,
--
Julien Thierry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v8 25/26] firmware: arm_sdei: Add ACPI GHES registration helper
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (23 preceding siblings ...)
2019-01-29 18:49 ` [PATCH v8 24/26] arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work James Morse
@ 2019-01-29 18:49 ` James Morse
2019-01-29 18:49 ` [PATCH v8 26/26] ACPI / APEI: Add support for the SDEI GHES Notification type James Morse
2019-02-08 11:40 ` [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up Rafael J. Wysocki
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:49 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
APEI's Generic Hardware Error Source structures do not describe
whether the SDEI event is shared or private, as this information is
discoverable via the API.
GHES needs to know whether an event is normal or critical to avoid
sharing locks or fixmap entries, but GHES shouldn't have to know about
the SDEI API.
Add a helper to register the GHES using the appropriate normal or
critical callback.
Signed-off-by: James Morse <james.morse@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
Changes since v4:
* Moved normal/critical callbacks into the helper, as APEI needs to know.
* Tinkered with the commit message.
* Dropped Punit's Reviewed-by.
Changes since v3:
* Removed acpi_disabled() checks that aren't necessary after v2s #ifdef
change.
Changes since v2:
* Added header file, thanks kbuild-robot!
* changed ifdef to the GHES version to match the fixmap definition
Changes since v1:
* ghes->fixmap_idx variable rename
---
arch/arm64/include/asm/fixmap.h | 4 ++
drivers/firmware/arm_sdei.c | 68 +++++++++++++++++++++++++++++++++
include/linux/arm_sdei.h | 6 +++
3 files changed, 78 insertions(+)
diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
index 966dd4bb23f2..f987b8a8f325 100644
--- a/arch/arm64/include/asm/fixmap.h
+++ b/arch/arm64/include/asm/fixmap.h
@@ -56,6 +56,10 @@ enum fixed_addresses {
/* Used for GHES mapping from assorted contexts */
FIX_APEI_GHES_IRQ,
FIX_APEI_GHES_SEA,
+#ifdef CONFIG_ARM_SDE_INTERFACE
+ FIX_APEI_GHES_SDEI_NORMAL,
+ FIX_APEI_GHES_SDEI_CRITICAL,
+#endif
#endif /* CONFIG_ACPI_APEI_GHES */
#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
index c64c7da73829..e6376f985ef7 100644
--- a/drivers/firmware/arm_sdei.c
+++ b/drivers/firmware/arm_sdei.c
@@ -2,6 +2,7 @@
// Copyright (C) 2017 Arm Ltd.
#define pr_fmt(fmt) "sdei: " fmt
+#include <acpi/ghes.h>
#include <linux/acpi.h>
#include <linux/arm_sdei.h>
#include <linux/arm-smccc.h>
@@ -887,6 +888,73 @@ static void sdei_smccc_hvc(unsigned long function_id,
arm_smccc_hvc(function_id, arg0, arg1, arg2, arg3, arg4, 0, 0, res);
}
+int sdei_register_ghes(struct ghes *ghes, sdei_event_callback *normal_cb,
+ sdei_event_callback *critical_cb)
+{
+ int err;
+ u64 result;
+ u32 event_num;
+ sdei_event_callback *cb;
+
+ if (!IS_ENABLED(CONFIG_ACPI_APEI_GHES))
+ return -EOPNOTSUPP;
+
+ event_num = ghes->generic->notify.vector;
+ if (event_num == 0) {
+ /*
+ * Event 0 is reserved by the specification for
+ * SDEI_EVENT_SIGNAL.
+ */
+ return -EINVAL;
+ }
+
+ err = sdei_api_event_get_info(event_num, SDEI_EVENT_INFO_EV_PRIORITY,
+ &result);
+ if (err)
+ return err;
+
+ if (result == SDEI_EVENT_PRIORITY_CRITICAL)
+ cb = critical_cb;
+ else
+ cb = normal_cb;
+
+ err = sdei_event_register(event_num, cb, ghes);
+ if (!err)
+ err = sdei_event_enable(event_num);
+
+ return err;
+}
+
+int sdei_unregister_ghes(struct ghes *ghes)
+{
+ int i;
+ int err;
+ u32 event_num = ghes->generic->notify.vector;
+
+ might_sleep();
+
+ if (!IS_ENABLED(CONFIG_ACPI_APEI_GHES))
+ return -EOPNOTSUPP;
+
+ /*
+ * The event may be running on another CPU. Disable it
+ * to stop new events, then try to unregister a few times.
+ */
+ err = sdei_event_disable(event_num);
+ if (err)
+ return err;
+
+ for (i = 0; i < 3; i++) {
+ err = sdei_event_unregister(event_num);
+ if (err != -EINPROGRESS)
+ break;
+
+ schedule();
+ }
+
+ return err;
+}
+
static int sdei_get_conduit(struct platform_device *pdev)
{
const char *method;
diff --git a/include/linux/arm_sdei.h b/include/linux/arm_sdei.h
index 942afbd544b7..393899192906 100644
--- a/include/linux/arm_sdei.h
+++ b/include/linux/arm_sdei.h
@@ -11,6 +11,7 @@ enum sdei_conduit_types {
CONDUIT_HVC,
};
+#include <acpi/ghes.h>
#include <asm/sdei.h>
/* Arch code should override this to set the entry point from firmware... */
@@ -39,6 +40,11 @@ int sdei_event_unregister(u32 event_num);
int sdei_event_enable(u32 event_num);
int sdei_event_disable(u32 event_num);
+/* GHES register/unregister helpers */
+int sdei_register_ghes(struct ghes *ghes, sdei_event_callback *normal_cb,
+ sdei_event_callback *critical_cb);
+int sdei_unregister_ghes(struct ghes *ghes);
+
#ifdef CONFIG_ARM_SDE_INTERFACE
/* For use by arch code when CPU hotplug notifiers are not appropriate. */
int sdei_mask_local_cpu(void);
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v8 26/26] ACPI / APEI: Add support for the SDEI GHES Notification type
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (24 preceding siblings ...)
2019-01-29 18:49 ` [PATCH v8 25/26] firmware: arm_sdei: Add ACPI GHES registration helper James Morse
@ 2019-01-29 18:49 ` James Morse
2019-02-08 11:40 ` [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up Rafael J. Wysocki
26 siblings, 0 replies; 36+ messages in thread
From: James Morse @ 2019-01-29 18:49 UTC (permalink / raw)
To: linux-acpi
Cc: Rafael Wysocki, Tony Luck, Xie XiuQi, Marc Zyngier,
Catalin Marinas, Will Deacon, Christoffer Dall, Dongjiu Geng,
linux-mm, Borislav Petkov, james.morse, Naoya Horiguchi, kvmarm,
linux-arm-kernel, Len Brown
If the GHES notification type is SDEI, register the provided event
using the SDEI-GHES helper.
SDEI may be one of two types of event, normal and critical. Critical
events can interrupt normal events, so these must have separate
fixmap slots and locks in case both event types are in use.
Signed-off-by: James Morse <james.morse@arm.com>
--
Changes since v7:
* Use __end_of_fixed_addresses as an arch-agnostic invalid fixmap entry
* Move the locks definition into the function that uses them to make it
clear these are NMI only.
Changes since v6:
* Tinkering due to the absence of #ifdef
* Added SDEI to the new ghes_is_synchronous() helper.
---
drivers/acpi/apei/ghes.c | 85 ++++++++++++++++++++++++++++++++++++++++
include/linux/arm_sdei.h | 3 ++
2 files changed, 88 insertions(+)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index dfa8f155f964..d0f0a219bf8f 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -25,6 +25,7 @@
* GNU General Public License for more details.
*/
+#include <linux/arm_sdei.h>
#include <linux/kernel.h>
#include <linux/moduleparam.h>
#include <linux/init.h>
@@ -86,6 +87,15 @@
((struct acpi_hest_generic_status *) \
((struct ghes_estatus_node *)(estatus_node) + 1))
+/*
+ * NMI-like notifications vary by architecture, before the compiler can prune
+ * unused static functions it needs a value for these enums.
+ */
+#ifndef CONFIG_ARM_SDE_INTERFACE
+#define FIX_APEI_GHES_SDEI_NORMAL __end_of_fixed_addresses
+#define FIX_APEI_GHES_SDEI_CRITICAL __end_of_fixed_addresses
+#endif
+
static inline bool is_hest_type_generic_v2(struct ghes *ghes)
{
return ghes->generic->header.type == ACPI_HEST_TYPE_GENERIC_ERROR_V2;
@@ -1088,6 +1098,63 @@ static void ghes_nmi_init_cxt(void)
init_irq_work(&ghes_proc_irq_work, ghes_proc_in_irq);
}
+static int __ghes_sdei_callback(struct ghes *ghes,
+ enum fixed_addresses fixmap_idx)
+{
+ if (!ghes_in_nmi_queue_one_entry(ghes, fixmap_idx)) {
+ irq_work_queue(&ghes_proc_irq_work);
+
+ return 0;
+ }
+
+ return -ENOENT;
+}
+
+static int ghes_sdei_normal_callback(u32 event_num, struct pt_regs *regs,
+ void *arg)
+{
+ static DEFINE_RAW_SPINLOCK(ghes_notify_lock_sdei_normal);
+ struct ghes *ghes = arg;
+ int err;
+
+ raw_spin_lock(&ghes_notify_lock_sdei_normal);
+ err = __ghes_sdei_callback(ghes, FIX_APEI_GHES_SDEI_NORMAL);
+ raw_spin_unlock(&ghes_notify_lock_sdei_normal);
+
+ return err;
+}
+
+static int ghes_sdei_critical_callback(u32 event_num, struct pt_regs *regs,
+ void *arg)
+{
+ static DEFINE_RAW_SPINLOCK(ghes_notify_lock_sdei_critical);
+ struct ghes *ghes = arg;
+ int err;
+
+ raw_spin_lock(&ghes_notify_lock_sdei_critical);
+ err = __ghes_sdei_callback(ghes, FIX_APEI_GHES_SDEI_CRITICAL);
+ raw_spin_unlock(&ghes_notify_lock_sdei_critical);
+
+ return err;
+}
+
+static int apei_sdei_register_ghes(struct ghes *ghes)
+{
+ if (!IS_ENABLED(CONFIG_ARM_SDE_INTERFACE))
+ return -EOPNOTSUPP;
+
+ return sdei_register_ghes(ghes, ghes_sdei_normal_callback,
+ ghes_sdei_critical_callback);
+}
+
+static int apei_sdei_unregister_ghes(struct ghes *ghes)
+{
+ if (!IS_ENABLED(CONFIG_ARM_SDE_INTERFACE))
+ return -EOPNOTSUPP;
+
+ return sdei_unregister_ghes(ghes);
+}
+
static int ghes_probe(struct platform_device *ghes_dev)
{
struct acpi_hest_generic *generic;
@@ -1123,6 +1190,13 @@ static int ghes_probe(struct platform_device *ghes_dev)
goto err;
}
break;
+ case ACPI_HEST_NOTIFY_SOFTWARE_DELEGATED:
+ if (!IS_ENABLED(CONFIG_ARM_SDE_INTERFACE)) {
+ pr_warn(GHES_PFX "Generic hardware error source: %d notified via SDE Interface is not supported!\n",
+ generic->header.source_id);
+ goto err;
+ }
+ break;
case ACPI_HEST_NOTIFY_LOCAL:
pr_warning(GHES_PFX "Generic hardware error source: %d notified via local interrupt is not supported!\n",
generic->header.source_id);
@@ -1186,6 +1260,11 @@ static int ghes_probe(struct platform_device *ghes_dev)
case ACPI_HEST_NOTIFY_NMI:
ghes_nmi_add(ghes);
break;
+ case ACPI_HEST_NOTIFY_SOFTWARE_DELEGATED:
+ rc = apei_sdei_register_ghes(ghes);
+ if (rc)
+ goto err;
+ break;
default:
BUG();
}
@@ -1211,6 +1290,7 @@ static int ghes_probe(struct platform_device *ghes_dev)
static int ghes_remove(struct platform_device *ghes_dev)
{
+ int rc;
struct ghes *ghes;
struct acpi_hest_generic *generic;
@@ -1243,6 +1323,11 @@ static int ghes_remove(struct platform_device *ghes_dev)
case ACPI_HEST_NOTIFY_NMI:
ghes_nmi_remove(ghes);
break;
+ case ACPI_HEST_NOTIFY_SOFTWARE_DELEGATED:
+ rc = apei_sdei_unregister_ghes(ghes);
+ if (rc)
+ return rc;
+ break;
default:
BUG();
break;
diff --git a/include/linux/arm_sdei.h b/include/linux/arm_sdei.h
index 393899192906..3305ea7f9dc7 100644
--- a/include/linux/arm_sdei.h
+++ b/include/linux/arm_sdei.h
@@ -12,7 +12,10 @@ enum sdei_conduit_types {
};
#include <acpi/ghes.h>
+
+#ifdef CONFIG_ARM_SDE_INTERFACE
#include <asm/sdei.h>
+#endif
/* Arch code should override this to set the entry point from firmware... */
#ifndef sdei_arch_get_entry_point
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up
2019-01-29 18:48 [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up James Morse
` (25 preceding siblings ...)
2019-01-29 18:49 ` [PATCH v8 26/26] ACPI / APEI: Add support for the SDEI GHES Notification type James Morse
@ 2019-02-08 11:40 ` Rafael J. Wysocki
2019-02-08 14:13 ` James Morse
26 siblings, 1 reply; 36+ messages in thread
From: Rafael J. Wysocki @ 2019-02-08 11:40 UTC (permalink / raw)
To: James Morse
Cc: Tony Luck, linux-mm, Marc Zyngier, Catalin Marinas, Xie XiuQi,
Will Deacon, Christoffer Dall, Dongjiu Geng, linux-acpi,
Borislav Petkov, Naoya Horiguchi, kvmarm, linux-arm-kernel,
Len Brown
On Tuesday, January 29, 2019 7:48:36 PM CET James Morse wrote:
> Changes since v7?
> * Removed the memory allocation in the task_work stuff.
> * More user-friendly and easier on the eye,
> * Switched the irq-mask testing in the arch code to be safe before&after
> Julien's GIC PMR series.
> Specific changes are noted in each patch.
>
>
> This series aims to wire-up arm64's fancy new software-NMI notifications
> for firmware-first RAS. These need to use the estatus-queue, which is
> also needed for notifications via emulated-SError. All of these
> things take the 'in_nmi()' path through ghes_copy_tofrom_phys(), and
> so will deadlock if they can interact, which they might.
>
> To that end, this series removes the in_nmi() stuff from ghes.c.
> Locks are pushed out to the notification helpers, and fixmap entries
> are passed in to the code that needs them. This means the estatus-queue
> users can interrupt each other however they like.
>
> While doing this there is a fair amount of cleanup, which is (now) at the
> beginning of the series. NMIlike notifications interrupting
> ghes_probe() can go wrong for three different reasons. CPER record
> blocks greater than PAGE_SIZE dont' work.
> The estatus-pool allocation is simplified and the silent-flag/oops-begin
> is removed.
>
> Nothing in this series is intended as fixes, as its all cleanup or
> never-worked.
>
> ----------%<----------
> The earlier boiler-plate:
>
> What's SDEI? Its ARM's "Software Delegated Exception Interface" [0]. It's
> used by firmware to tell the OS about firmware-first RAS events.
>
> These Software exceptions can interrupt anything, so I describe them as
> NMI-like. They aren't the only NMI-like way to notify the OS about
> firmware-first RAS events, the ACPI spec also defines 'NOTFIY_SEA' and
> 'NOTIFY_SEI'.
>
> (Acronyms: SEA, Synchronous External Abort. The CPU requested some memory,
> but the owner of that memory said no. These are always synchronous with the
> instruction that caused them. SEI, System-Error Interrupt, commonly called
> SError. This is an asynchronous external abort, the memory-owner didn't say no
> at the right point. Collectively these things are called external-aborts
> How is firmware involved? It traps these and re-injects them into the kernel
> once its written the CPER records).
>
> APEI's GHES code only expects one source of NMI. If a platform implements
> more than one of these mechanisms, APEI needs to handle the interaction.
> 'SEA' and 'SEI' can interact as 'SEI' is asynchronous. SDEI can interact
> with itself: its exceptions can be 'normal' or 'critical', and firmware
> could use both types for RAS. (errors using normal, 'panic-now' using
> critical).
> ----------%<----------
>
> This series is base on v5.0-rc1, and can be retrieved from:
> git://linux-arm.org/linux-jm.git -b apei_ioremap_rework/v8
>
>
> Known issues:
> * ghes_copy_tofrom_phys() already takes a lock in NMI context, this
> series moves that around, and makes sure we never try to take the
> same lock from different NMIlike notifications. Since the switch to
> queued spinlocks it looks like the kernel can only be 4 context's
> deep in spinlock, which arm64 could exceed as it doesn't have a
> single architected NMI. This would be fixed by dropping back to
> test-and-set when the nesting gets too deep:
> lore.kernel.org/r/1548215351-18896-1-git-send-email-longman@redhat.com
>
> * Taking an NMI from a KVM guest on arm64 with VHE leaves HCR_EL2.TGE
> clear, meaning AT and TLBI point at the guest, and PAN/UAO are squiffy.
> Only TLBI matters for APEI, and this is fixed by Julien's patch:
> http://lore.kernel.org/r/1548084825-8803-2-git-send-email-julien.thierry@arm.com
>
> * Linux ignores the physical address mask, meaning it doesn't call
> memory_failure() on all the affected pages if firmware or hypervisor
> believe in a different page size. Easy to hit on arm64, (easy to fix too,
> it just conflicts with this series)
>
>
> [v7] https://lore.kernel.org/linux-arm-kernel/20181203180613.228133-1-james.morse@arm.com/
> [v6] https://www.spinics.net/lists/linux-acpi/msg84228.html
> [v5] https://www.spinics.net/lists/linux-acpi/msg82993.html
> [v4] https://www.spinics.net/lists/arm-kernel/msg653078.html
> [v3] https://www.spinics.net/lists/arm-kernel/msg649230.html
>
> [0] https://static.docs.arm.com/den0054/a/ARM_DEN0054A_Software_Delegated_Exception_Interface.pdf
>
>
> James Morse (26):
> ACPI / APEI: Don't wait to serialise with oops messages when
> panic()ing
> ACPI / APEI: Remove silent flag from ghes_read_estatus()
> ACPI / APEI: Switch estatus pool to use vmalloc memory
> ACPI / APEI: Make hest.c manage the estatus memory pool
> ACPI / APEI: Make estatus pool allocation a static size
> ACPI / APEI: Don't store CPER records physical address in struct ghes
> ACPI / APEI: Remove spurious GHES_TO_CLEAR check
> ACPI / APEI: Don't update struct ghes' flags in read/clear estatus
> ACPI / APEI: Generalise the estatus queue's notify code
> ACPI / APEI: Don't allow ghes_ack_error() to mask earlier errors
> ACPI / APEI: Move NOTIFY_SEA between the estatus-queue and NOTIFY_NMI
> ACPI / APEI: Switch NOTIFY_SEA to use the estatus queue
> KVM: arm/arm64: Add kvm_ras.h to collect kvm specific RAS plumbing
> arm64: KVM/mm: Move SEA handling behind a single 'claim' interface
> ACPI / APEI: Move locking to the notification helper
> ACPI / APEI: Let the notification helper specify the fixmap slot
> ACPI / APEI: Pass ghes and estatus separately to avoid a later copy
> ACPI / APEI: Make GHES estatus header validation more user friendly
> ACPI / APEI: Split ghes_read_estatus() to allow a peek at the CPER
> length
> ACPI / APEI: Only use queued estatus entry during
> in_nmi_queue_one_entry()
> ACPI / APEI: Use separate fixmap pages for arm64 NMI-like
> notifications
> mm/memory-failure: Add memory_failure_queue_kick()
> ACPI / APEI: Kick the memory_failure() queue for synchronous errors
> arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work
> firmware: arm_sdei: Add ACPI GHES registration helper
> ACPI / APEI: Add support for the SDEI GHES Notification type
>
> arch/arm/include/asm/kvm_ras.h | 14 +
> arch/arm/include/asm/system_misc.h | 5 -
> arch/arm64/include/asm/acpi.h | 4 +-
> arch/arm64/include/asm/daifflags.h | 1 +
> arch/arm64/include/asm/fixmap.h | 6 +-
> arch/arm64/include/asm/kvm_ras.h | 25 +
> arch/arm64/include/asm/system_misc.h | 2 -
> arch/arm64/kernel/acpi.c | 54 ++
> arch/arm64/mm/fault.c | 25 +-
> drivers/acpi/apei/Kconfig | 12 +-
> drivers/acpi/apei/ghes.c | 725 ++++++++++++++++-----------
> drivers/acpi/apei/hest.c | 10 +-
> drivers/firmware/arm_sdei.c | 68 +++
> include/acpi/ghes.h | 7 +-
> include/linux/arm_sdei.h | 9 +
> include/linux/mm.h | 1 +
> mm/memory-failure.c | 15 +-
> virt/kvm/arm/mmu.c | 4 +-
> 18 files changed, 646 insertions(+), 341 deletions(-)
> create mode 100644 arch/arm/include/asm/kvm_ras.h
> create mode 100644 arch/arm64/include/asm/kvm_ras.h
I can apply patches in this series up to and including patch [21/26].
Do you want me to do that?
Patch [22/26] requires an ACK from mm people.
Patch [23/26] has a problem that randconfig can generate a configuration
in which memory_failure_queue_kick() is not present, so it is necessary
to add a CONFIG_MEMORY_FAILURE dependency somewhere for things to
work (or define an empty stub for that function in case the symbol is
not set).
If patches [24-26/26] don't depend on the previous two, I can try to
apply them either, so please let me know.
Thanks,
Rafael
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up
2019-02-08 11:40 ` [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up Rafael J. Wysocki
@ 2019-02-08 14:13 ` James Morse
2019-02-11 11:05 ` Rafael J. Wysocki
0 siblings, 1 reply; 36+ messages in thread
From: James Morse @ 2019-02-08 14:13 UTC (permalink / raw)
To: Rafael J. Wysocki, Borislav Petkov
Cc: Tony Luck, linux-mm, Marc Zyngier, Catalin Marinas, Xie XiuQi,
Will Deacon, Christoffer Dall, Dongjiu Geng, linux-acpi,
Naoya Horiguchi, kvmarm, linux-arm-kernel, Len Brown
Hi Rafael,
On 08/02/2019 11:40, Rafael J. Wysocki wrote:
> On Tuesday, January 29, 2019 7:48:36 PM CET James Morse wrote:
>> This series aims to wire-up arm64's fancy new software-NMI notifications
>> for firmware-first RAS. These need to use the estatus-queue, which is
>> also needed for notifications via emulated-SError. All of these
>> things take the 'in_nmi()' path through ghes_copy_tofrom_phys(), and
>> so will deadlock if they can interact, which they might.
>> Known issues:
>> * ghes_copy_tofrom_phys() already takes a lock in NMI context, this
>> series moves that around, and makes sure we never try to take the
>> same lock from different NMIlike notifications. Since the switch to
>> queued spinlocks it looks like the kernel can only be 4 context's
>> deep in spinlock, which arm64 could exceed as it doesn't have a
>> single architected NMI. This would be fixed by dropping back to
>> test-and-set when the nesting gets too deep:
>> lore.kernel.org/r/1548215351-18896-1-git-send-email-longman@redhat.com
>>
>> * Taking an NMI from a KVM guest on arm64 with VHE leaves HCR_EL2.TGE
>> clear, meaning AT and TLBI point at the guest, and PAN/UAO are squiffy.
>> Only TLBI matters for APEI, and this is fixed by Julien's patch:
>> http://lore.kernel.org/r/1548084825-8803-2-git-send-email-julien.thierry@arm.com
>>
>> * Linux ignores the physical address mask, meaning it doesn't call
>> memory_failure() on all the affected pages if firmware or hypervisor
>> believe in a different page size. Easy to hit on arm64, (easy to fix too,
>> it just conflicts with this series)
>> James Morse (26):
>> ACPI / APEI: Don't wait to serialise with oops messages when
>> panic()ing
>> ACPI / APEI: Remove silent flag from ghes_read_estatus()
>> ACPI / APEI: Switch estatus pool to use vmalloc memory
>> ACPI / APEI: Make hest.c manage the estatus memory pool
>> ACPI / APEI: Make estatus pool allocation a static size
>> ACPI / APEI: Don't store CPER records physical address in struct ghes
>> ACPI / APEI: Remove spurious GHES_TO_CLEAR check
>> ACPI / APEI: Don't update struct ghes' flags in read/clear estatus
>> ACPI / APEI: Generalise the estatus queue's notify code
>> ACPI / APEI: Don't allow ghes_ack_error() to mask earlier errors
>> ACPI / APEI: Move NOTIFY_SEA between the estatus-queue and NOTIFY_NMI
>> ACPI / APEI: Switch NOTIFY_SEA to use the estatus queue
>> KVM: arm/arm64: Add kvm_ras.h to collect kvm specific RAS plumbing
>> arm64: KVM/mm: Move SEA handling behind a single 'claim' interface
>> ACPI / APEI: Move locking to the notification helper
>> ACPI / APEI: Let the notification helper specify the fixmap slot
>> ACPI / APEI: Pass ghes and estatus separately to avoid a later copy
>> ACPI / APEI: Make GHES estatus header validation more user friendly
>> ACPI / APEI: Split ghes_read_estatus() to allow a peek at the CPER
>> length
>> ACPI / APEI: Only use queued estatus entry during
>> in_nmi_queue_one_entry()
>> ACPI / APEI: Use separate fixmap pages for arm64 NMI-like
>> notifications
>> mm/memory-failure: Add memory_failure_queue_kick()
>> ACPI / APEI: Kick the memory_failure() queue for synchronous errors
>> arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work
>> firmware: arm_sdei: Add ACPI GHES registration helper
>> ACPI / APEI: Add support for the SDEI GHES Notification type
> I can apply patches in this series up to and including patch [21/26].
>
> Do you want me to do that?
9-12, 17-19, 21 are missing any review/ack tags, so I wouldn't ask, but as
you're offering, yes please!
> Patch [22/26] requires an ACK from mm people.
>
> Patch [23/26] has a problem that randconfig can generate a configuration
> in which memory_failure_queue_kick() is not present, so it is necessary
> to add a CONFIG_MEMORY_FAILURE dependency somewhere for things to
> work (or define an empty stub for that function in case the symbol is
> not set).
Damn-it! Thanks, I was just trying to work that report out...
> If patches [24-26/26] don't depend on the previous two, I can try to
> apply them either, so please let me know.
22-24 depend on each other. Merging 24 without the other two is no-improvement,
so I'd like them to be kept together.
25-26 don't depend on 22-24, but came later so that they weren't affected by the
same race.
(note to self: describe that in the cover letter next time.)
If I apply the tag's and Boris' changes and post a tested v9 as 1-21, 25-26, is
that easier, or does it cause extra work?
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up
2019-02-08 14:13 ` James Morse
@ 2019-02-11 11:05 ` Rafael J. Wysocki
2019-02-11 18:35 ` James Morse
0 siblings, 1 reply; 36+ messages in thread
From: Rafael J. Wysocki @ 2019-02-11 11:05 UTC (permalink / raw)
To: James Morse
Cc: Tony Luck, ACPI Devel Maling List, Marc Zyngier, Will Deacon,
Xie XiuQi, Rafael J. Wysocki, Christoffer Dall, Dongjiu Geng,
Linux Memory Management List, Borislav Petkov, Catalin Marinas,
Naoya Horiguchi, kvmarm, Linux ARM, Len Brown
On Fri, Feb 8, 2019 at 3:13 PM James Morse <james.morse@arm.com> wrote:
>
> Hi Rafael,
>
> On 08/02/2019 11:40, Rafael J. Wysocki wrote:
> > On Tuesday, January 29, 2019 7:48:36 PM CET James Morse wrote:
> >> This series aims to wire-up arm64's fancy new software-NMI notifications
> >> for firmware-first RAS. These need to use the estatus-queue, which is
> >> also needed for notifications via emulated-SError. All of these
> >> things take the 'in_nmi()' path through ghes_copy_tofrom_phys(), and
> >> so will deadlock if they can interact, which they might.
>
> >> Known issues:
> >> * ghes_copy_tofrom_phys() already takes a lock in NMI context, this
> >> series moves that around, and makes sure we never try to take the
> >> same lock from different NMIlike notifications. Since the switch to
> >> queued spinlocks it looks like the kernel can only be 4 context's
> >> deep in spinlock, which arm64 could exceed as it doesn't have a
> >> single architected NMI. This would be fixed by dropping back to
> >> test-and-set when the nesting gets too deep:
> >> lore.kernel.org/r/1548215351-18896-1-git-send-email-longman@redhat.com
> >>
> >> * Taking an NMI from a KVM guest on arm64 with VHE leaves HCR_EL2.TGE
> >> clear, meaning AT and TLBI point at the guest, and PAN/UAO are squiffy.
> >> Only TLBI matters for APEI, and this is fixed by Julien's patch:
> >> http://lore.kernel.org/r/1548084825-8803-2-git-send-email-julien.thierry@arm.com
> >>
> >> * Linux ignores the physical address mask, meaning it doesn't call
> >> memory_failure() on all the affected pages if firmware or hypervisor
> >> believe in a different page size. Easy to hit on arm64, (easy to fix too,
> >> it just conflicts with this series)
>
>
> >> James Morse (26):
> >> ACPI / APEI: Don't wait to serialise with oops messages when
> >> panic()ing
> >> ACPI / APEI: Remove silent flag from ghes_read_estatus()
> >> ACPI / APEI: Switch estatus pool to use vmalloc memory
> >> ACPI / APEI: Make hest.c manage the estatus memory pool
> >> ACPI / APEI: Make estatus pool allocation a static size
> >> ACPI / APEI: Don't store CPER records physical address in struct ghes
> >> ACPI / APEI: Remove spurious GHES_TO_CLEAR check
> >> ACPI / APEI: Don't update struct ghes' flags in read/clear estatus
> >> ACPI / APEI: Generalise the estatus queue's notify code
> >> ACPI / APEI: Don't allow ghes_ack_error() to mask earlier errors
> >> ACPI / APEI: Move NOTIFY_SEA between the estatus-queue and NOTIFY_NMI
> >> ACPI / APEI: Switch NOTIFY_SEA to use the estatus queue
> >> KVM: arm/arm64: Add kvm_ras.h to collect kvm specific RAS plumbing
> >> arm64: KVM/mm: Move SEA handling behind a single 'claim' interface
> >> ACPI / APEI: Move locking to the notification helper
> >> ACPI / APEI: Let the notification helper specify the fixmap slot
> >> ACPI / APEI: Pass ghes and estatus separately to avoid a later copy
> >> ACPI / APEI: Make GHES estatus header validation more user friendly
> >> ACPI / APEI: Split ghes_read_estatus() to allow a peek at the CPER
> >> length
> >> ACPI / APEI: Only use queued estatus entry during
> >> in_nmi_queue_one_entry()
> >> ACPI / APEI: Use separate fixmap pages for arm64 NMI-like
> >> notifications
> >> mm/memory-failure: Add memory_failure_queue_kick()
> >> ACPI / APEI: Kick the memory_failure() queue for synchronous errors
> >> arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work
> >> firmware: arm_sdei: Add ACPI GHES registration helper
> >> ACPI / APEI: Add support for the SDEI GHES Notification type
>
>
> > I can apply patches in this series up to and including patch [21/26].
> >
> > Do you want me to do that?
>
> 9-12, 17-19, 21 are missing any review/ack tags, so I wouldn't ask, but as
> you're offering, yes please!
>
>
> > Patch [22/26] requires an ACK from mm people.
> >
> > Patch [23/26] has a problem that randconfig can generate a configuration
> > in which memory_failure_queue_kick() is not present, so it is necessary
> > to add a CONFIG_MEMORY_FAILURE dependency somewhere for things to
> > work (or define an empty stub for that function in case the symbol is
> > not set).
>
> Damn-it! Thanks, I was just trying to work that report out...
>
>
> > If patches [24-26/26] don't depend on the previous two, I can try to
> > apply them either, so please let me know.
>
> 22-24 depend on each other. Merging 24 without the other two is no-improvement,
> so I'd like them to be kept together.
>
> 25-26 don't depend on 22-24, but came later so that they weren't affected by the
> same race.
> (note to self: describe that in the cover letter next time.)
>
>
> If I apply the tag's and Boris' changes and post a tested v9 as 1-21, 25-26, is
> that easier, or does it cause extra work?
Actually, I went ahead and applied them, since I had the 1-21 ready anyway.
I applied the Boris' fixups manually which led to a bit of rebasing,
so please check my linux-next branch.
Thanks!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up
2019-02-11 11:05 ` Rafael J. Wysocki
@ 2019-02-11 18:35 ` James Morse
2019-02-12 22:14 ` Rafael J. Wysocki
0 siblings, 1 reply; 36+ messages in thread
From: James Morse @ 2019-02-11 18:35 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Tony Luck, ACPI Devel Maling List, Marc Zyngier, Will Deacon,
Xie XiuQi, Rafael J. Wysocki, Christoffer Dall, Dongjiu Geng,
Linux Memory Management List, Borislav Petkov, Catalin Marinas,
Naoya Horiguchi, kvmarm, Linux ARM, Len Brown
Hi Rafael,
On 11/02/2019 11:05, Rafael J. Wysocki wrote:
> On Fri, Feb 8, 2019 at 3:13 PM James Morse <james.morse@arm.com> wrote:
>> On 08/02/2019 11:40, Rafael J. Wysocki wrote:
>>> On Tuesday, January 29, 2019 7:48:36 PM CET James Morse wrote:
>>>> This series aims to wire-up arm64's fancy new software-NMI notifications
>>>> for firmware-first RAS. These need to use the estatus-queue, which is
>>>> also needed for notifications via emulated-SError. All of these
>>>> things take the 'in_nmi()' path through ghes_copy_tofrom_phys(), and
>>>> so will deadlock if they can interact, which they might.
>>
>>>> Known issues:
>>>> * ghes_copy_tofrom_phys() already takes a lock in NMI context, this
>>>> series moves that around, and makes sure we never try to take the
>>>> same lock from different NMIlike notifications. Since the switch to
>>>> queued spinlocks it looks like the kernel can only be 4 context's
>>>> deep in spinlock, which arm64 could exceed as it doesn't have a
>>>> single architected NMI. This would be fixed by dropping back to
>>>> test-and-set when the nesting gets too deep:
>>>> lore.kernel.org/r/1548215351-18896-1-git-send-email-longman@redhat.com
>>>>
>>>> * Taking an NMI from a KVM guest on arm64 with VHE leaves HCR_EL2.TGE
>>>> clear, meaning AT and TLBI point at the guest, and PAN/UAO are squiffy.
>>>> Only TLBI matters for APEI, and this is fixed by Julien's patch:
>>>> http://lore.kernel.org/r/1548084825-8803-2-git-send-email-julien.thierry@arm.com
>>>>
>>>> * Linux ignores the physical address mask, meaning it doesn't call
>>>> memory_failure() on all the affected pages if firmware or hypervisor
>>>> believe in a different page size. Easy to hit on arm64, (easy to fix too,
>>>> it just conflicts with this series)
>>
>>
>>>> James Morse (26):
>>>> ACPI / APEI: Don't wait to serialise with oops messages when
>>>> panic()ing
>>>> ACPI / APEI: Remove silent flag from ghes_read_estatus()
>>>> ACPI / APEI: Switch estatus pool to use vmalloc memory
>>>> ACPI / APEI: Make hest.c manage the estatus memory pool
>>>> ACPI / APEI: Make estatus pool allocation a static size
>>>> ACPI / APEI: Don't store CPER records physical address in struct ghes
>>>> ACPI / APEI: Remove spurious GHES_TO_CLEAR check
>>>> ACPI / APEI: Don't update struct ghes' flags in read/clear estatus
>>>> ACPI / APEI: Generalise the estatus queue's notify code
>>>> ACPI / APEI: Don't allow ghes_ack_error() to mask earlier errors
>>>> ACPI / APEI: Move NOTIFY_SEA between the estatus-queue and NOTIFY_NMI
>>>> ACPI / APEI: Switch NOTIFY_SEA to use the estatus queue
>>>> KVM: arm/arm64: Add kvm_ras.h to collect kvm specific RAS plumbing
>>>> arm64: KVM/mm: Move SEA handling behind a single 'claim' interface
>>>> ACPI / APEI: Move locking to the notification helper
>>>> ACPI / APEI: Let the notification helper specify the fixmap slot
>>>> ACPI / APEI: Pass ghes and estatus separately to avoid a later copy
>>>> ACPI / APEI: Make GHES estatus header validation more user friendly
>>>> ACPI / APEI: Split ghes_read_estatus() to allow a peek at the CPER
>>>> length
>>>> ACPI / APEI: Only use queued estatus entry during
>>>> in_nmi_queue_one_entry()
>>>> ACPI / APEI: Use separate fixmap pages for arm64 NMI-like
>>>> notifications
>>>> mm/memory-failure: Add memory_failure_queue_kick()
>>>> ACPI / APEI: Kick the memory_failure() queue for synchronous errors
>>>> arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work
>>>> firmware: arm_sdei: Add ACPI GHES registration helper
>>>> ACPI / APEI: Add support for the SDEI GHES Notification type
>>
>>
>>> I can apply patches in this series up to and including patch [21/26].
>>>
>>> Do you want me to do that?
>>
>> 9-12, 17-19, 21 are missing any review/ack tags, so I wouldn't ask, but as
>> you're offering, yes please!
>>
>>
>>> Patch [22/26] requires an ACK from mm people.
>>>
>>> Patch [23/26] has a problem that randconfig can generate a configuration
>>> in which memory_failure_queue_kick() is not present, so it is necessary
>>> to add a CONFIG_MEMORY_FAILURE dependency somewhere for things to
>>> work (or define an empty stub for that function in case the symbol is
>>> not set).
>>
>> Damn-it! Thanks, I was just trying to work that report out...
>>
>>
>>> If patches [24-26/26] don't depend on the previous two, I can try to
>>> apply them either, so please let me know.
>>
>> 22-24 depend on each other. Merging 24 without the other two is no-improvement,
>> so I'd like them to be kept together.
>>
>> 25-26 don't depend on 22-24, but came later so that they weren't affected by the
>> same race.
>> (note to self: describe that in the cover letter next time.)
>>
>>
>> If I apply the tag's and Boris' changes and post a tested v9 as 1-21, 25-26, is
>> that easier, or does it cause extra work?
>
> Actually, I went ahead and applied them, since I had the 1-21 ready anyway.
> I applied the Boris' fixups manually which led to a bit of rebasing,
> so please check my linux-next branch.
Looks okay to me, and I ran your branch through the POLL/SEA/SDEI tests I've
been doing for each version so far.
Thanks!
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v8 00/26] APEI in_nmi() rework and SDEI wire-up
2019-02-11 18:35 ` James Morse
@ 2019-02-12 22:14 ` Rafael J. Wysocki
0 siblings, 0 replies; 36+ messages in thread
From: Rafael J. Wysocki @ 2019-02-12 22:14 UTC (permalink / raw)
To: James Morse
Cc: Tony Luck, Xie XiuQi, ACPI Devel Maling List, Marc Zyngier,
Catalin Marinas, Rafael J. Wysocki, Will Deacon,
Christoffer Dall, Dongjiu Geng, Linux Memory Management List,
Borislav Petkov, Naoya Horiguchi, kvmarm, Linux ARM, Len Brown
On Monday, February 11, 2019 7:35:03 PM CET James Morse wrote:
> Hi Rafael,
>
> On 11/02/2019 11:05, Rafael J. Wysocki wrote:
> > On Fri, Feb 8, 2019 at 3:13 PM James Morse <james.morse@arm.com> wrote:
> >> On 08/02/2019 11:40, Rafael J. Wysocki wrote:
> >>> On Tuesday, January 29, 2019 7:48:36 PM CET James Morse wrote:
> >>>> This series aims to wire-up arm64's fancy new software-NMI notifications
> >>>> for firmware-first RAS. These need to use the estatus-queue, which is
> >>>> also needed for notifications via emulated-SError. All of these
> >>>> things take the 'in_nmi()' path through ghes_copy_tofrom_phys(), and
> >>>> so will deadlock if they can interact, which they might.
> >>
> >>>> Known issues:
> >>>> * ghes_copy_tofrom_phys() already takes a lock in NMI context, this
> >>>> series moves that around, and makes sure we never try to take the
> >>>> same lock from different NMIlike notifications. Since the switch to
> >>>> queued spinlocks it looks like the kernel can only be 4 context's
> >>>> deep in spinlock, which arm64 could exceed as it doesn't have a
> >>>> single architected NMI. This would be fixed by dropping back to
> >>>> test-and-set when the nesting gets too deep:
> >>>> lore.kernel.org/r/1548215351-18896-1-git-send-email-longman@redhat.com
> >>>>
> >>>> * Taking an NMI from a KVM guest on arm64 with VHE leaves HCR_EL2.TGE
> >>>> clear, meaning AT and TLBI point at the guest, and PAN/UAO are squiffy.
> >>>> Only TLBI matters for APEI, and this is fixed by Julien's patch:
> >>>> http://lore.kernel.org/r/1548084825-8803-2-git-send-email-julien.thierry@arm.com
> >>>>
> >>>> * Linux ignores the physical address mask, meaning it doesn't call
> >>>> memory_failure() on all the affected pages if firmware or hypervisor
> >>>> believe in a different page size. Easy to hit on arm64, (easy to fix too,
> >>>> it just conflicts with this series)
> >>
> >>
> >>>> James Morse (26):
> >>>> ACPI / APEI: Don't wait to serialise with oops messages when
> >>>> panic()ing
> >>>> ACPI / APEI: Remove silent flag from ghes_read_estatus()
> >>>> ACPI / APEI: Switch estatus pool to use vmalloc memory
> >>>> ACPI / APEI: Make hest.c manage the estatus memory pool
> >>>> ACPI / APEI: Make estatus pool allocation a static size
> >>>> ACPI / APEI: Don't store CPER records physical address in struct ghes
> >>>> ACPI / APEI: Remove spurious GHES_TO_CLEAR check
> >>>> ACPI / APEI: Don't update struct ghes' flags in read/clear estatus
> >>>> ACPI / APEI: Generalise the estatus queue's notify code
> >>>> ACPI / APEI: Don't allow ghes_ack_error() to mask earlier errors
> >>>> ACPI / APEI: Move NOTIFY_SEA between the estatus-queue and NOTIFY_NMI
> >>>> ACPI / APEI: Switch NOTIFY_SEA to use the estatus queue
> >>>> KVM: arm/arm64: Add kvm_ras.h to collect kvm specific RAS plumbing
> >>>> arm64: KVM/mm: Move SEA handling behind a single 'claim' interface
> >>>> ACPI / APEI: Move locking to the notification helper
> >>>> ACPI / APEI: Let the notification helper specify the fixmap slot
> >>>> ACPI / APEI: Pass ghes and estatus separately to avoid a later copy
> >>>> ACPI / APEI: Make GHES estatus header validation more user friendly
> >>>> ACPI / APEI: Split ghes_read_estatus() to allow a peek at the CPER
> >>>> length
> >>>> ACPI / APEI: Only use queued estatus entry during
> >>>> in_nmi_queue_one_entry()
> >>>> ACPI / APEI: Use separate fixmap pages for arm64 NMI-like
> >>>> notifications
> >>>> mm/memory-failure: Add memory_failure_queue_kick()
> >>>> ACPI / APEI: Kick the memory_failure() queue for synchronous errors
> >>>> arm64: acpi: Make apei_claim_sea() synchronise with APEI's irq work
> >>>> firmware: arm_sdei: Add ACPI GHES registration helper
> >>>> ACPI / APEI: Add support for the SDEI GHES Notification type
> >>
> >>
> >>> I can apply patches in this series up to and including patch [21/26].
> >>>
> >>> Do you want me to do that?
> >>
> >> 9-12, 17-19, 21 are missing any review/ack tags, so I wouldn't ask, but as
> >> you're offering, yes please!
> >>
> >>
> >>> Patch [22/26] requires an ACK from mm people.
> >>>
> >>> Patch [23/26] has a problem that randconfig can generate a configuration
> >>> in which memory_failure_queue_kick() is not present, so it is necessary
> >>> to add a CONFIG_MEMORY_FAILURE dependency somewhere for things to
> >>> work (or define an empty stub for that function in case the symbol is
> >>> not set).
> >>
> >> Damn-it! Thanks, I was just trying to work that report out...
> >>
> >>
> >>> If patches [24-26/26] don't depend on the previous two, I can try to
> >>> apply them either, so please let me know.
> >>
> >> 22-24 depend on each other. Merging 24 without the other two is no-improvement,
> >> so I'd like them to be kept together.
> >>
> >> 25-26 don't depend on 22-24, but came later so that they weren't affected by the
> >> same race.
> >> (note to self: describe that in the cover letter next time.)
> >>
> >>
> >> If I apply the tag's and Boris' changes and post a tested v9 as 1-21, 25-26, is
> >> that easier, or does it cause extra work?
> >
> > Actually, I went ahead and applied them, since I had the 1-21 ready anyway.
>
> > I applied the Boris' fixups manually which led to a bit of rebasing,
> > so please check my linux-next branch.
>
> Looks okay to me, and I ran your branch through the POLL/SEA/SDEI tests I've
> been doing for each version so far.
Thanks for the confirmation!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread