linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64
@ 2017-02-21 21:21 Tyler Baicar
  2017-02-21 21:21 ` [PATCH V11 01/10] acpi: apei: read ack upon ghes record consumption Tyler Baicar
                   ` (9 more replies)
  0 siblings, 10 replies; 29+ messages in thread
From: Tyler Baicar @ 2017-02-21 21:21 UTC (permalink / raw)
  To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, james.morse, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe
  Cc: Tyler Baicar

When a memory error, CPU error, PCIe error, or other type of hardware error
that's covered by RAS occurs, firmware should populate the shared GHES memory
location with the proper GHES structures to notify the OS of the error.
For example, platforms that implement firmware first handling may implement
separate GHES sources for corrected errors and uncorrected errors. If the
error is an uncorrectable error, then the firmware will notify the OS
immediately since the error needs to be handled ASAP. The OS will then be able
to take the appropriate action needed such as offlining a page. If the error
is a corrected error, then the firmware will not interrupt the OS immediately.
Instead, the OS will see and report the error the next time it's GHES timer
expires. The kernel will first parse the GHES structures and report the errors
through the kernel logs and then notify the user space through RAS trace
events. This allows user space applications such as RAS Daemon to see the
errors and report them however the user desires. This patchset extends the
kernel functionality for RAS errors based on updates in the UEFI 2.6 and
ACPI 6.1 specifications.

An example flow from firmware to user space could be:

                 +---------------+
       +-------->|               |
       |         |  GHES polling |--+
+-------------+  |    source     |  |   +---------------+   +------------+
|             |  +---------------+  |   |  Kernel GHES  |   |            |
|  Firmware   |                     +-->|  CPER AER and |-->|  RAS trace |
|             |  +---------------+  |   |  EDAC drivers |   |   event    |
+-------------+  |               |  |   +---------------+   +------------+
       |         |  GHES sci     |--+
       +-------->|   source      |
                 +---------------+

Add support for Generic Hardware Error Source (GHES) v2, which introduces the
capability for the OS to acknowledge the consumption of the error record
generated by the Reliability, Availability and Serviceability (RAS) controller.
This eliminates potential race conditions between the OS and the RAS controller.

Add support for the timestamp field added to the Generic Error Data Entry v3,
allowing the OS to log the time that the error is generated by the firmware,
rather than the time the error is consumed. This improves the correctness of
event sequences when analyzing error logs. The timestamp is added in
ACPI 6.1, reference Table 18-343 Generic Error Data Entry.

Add support for ARMv8 Common Platform Error Record (CPER) per UEFI 2.6
specification. ARMv8 specific processor error information is reported as part of
the CPER records.  This provides more detail on for processor error logs. This
can help describe ARMv8 cache, tlb, and bus errors.

Synchronous External Abort (SEA) represents a specific processor error condition
in ARM systems. A handler is added to recognize SEA errors, and a notifier is
added to parse and report the errors before the process is killed. Refer to
section N.2.1.1 in the Common Platform Error Record appendix of the UEFI 2.6
specification.

Currently the kernel ignores CPER records that are unrecognized.
On the other hand, UEFI spec allows for non-standard (eg. vendor
proprietary) error section type in CPER (Common Platform Error Record),
as defined in section N2.3 of UEFI version 2.5. Therefore, user
is not able to see hardware error data of non-standard section.

If section Type field of Generic Error Data Entry is unrecognized,
prints out the raw data in dmesg buffer, and also adds a tracepoint
for reporting such hardware errors.

Currently even if an error status block's severity is fatal, the kernel
does not honor the severity level and panic. With the firmware first
model, the platform could inform the OS about a fatal hardware error
through the non-NMI GHES notification type. The OS should panic when a
hardware error record is received with this severity.

Add support to handle SEAs that occur while a KVM guest kernel is
running. Currently these are unsupported by the guest abort handling.

V11:Change print_hex_dump calls to include ASCII output
    Change HAVE_ACPI_APEI_SEA to ACPI_APEI_SEA and make it 'default y'
    Add unknown print back when printing unknown CPER section
    Make sure to use "%s"" in CPER code
    Spacing fix when checking if SEA is enabled

V10:Fix spacing of trace event enabled if statement

V9: Move SEA_FnV_MASK to ESR_ELx_FnV
    Move HAVE_NMI into alphabetical order
    Remove duplicate hardirq.h include
    Only call ghes_notify_sea if HAVE_ACPI_APEI_SEA
    Make ACPI_APEI_SEA depend on ACPI_APEI_GHES
    Use phys_addr_t for physical address variable
    Make ghes_sea_add() return void
    Add include guard to ghes.h
    Verify HAVE_RAS before calling ras trace events
    Call __ghes_print_estatus() before __ghes_call_panic()
    Add trace_*_event_enabled() checks for both new trace events

V8: Remove SEA notifier
    Add FAR not valid bit check when populating the SEA error address
    Move nmi_enter/exit() to architecture specific code
    Add synchronize_rcu() usage to SEA handling
    Make GHES_IOREMAP_PAGES always 2
    Update ghes_ioremap_pfn_nmi() to work like ghes_ioremap_pfn_irq()
    Remove the SEA print from handle_guest_sea()

V7: Update a couple prints for ARM processor errors
    Add Print notifying if overflow occurred for ARM processor errors
    Check for ARM configuration to allow the compiler to ignore ARM code
     on non-ARM systems
    Use SEA acronym instead of spelling it out
    Update fault_info prints to be more clear
    Add NMI locking to SEA notification
    Remove error info structure from ARM trace event since there can be
     a variable amount of these structures

V6: Change HEST_TYPE_GENERIC_V2 to IS_HEST_TYPE_GENERIC_V2 for readability
    Move APEI helper defines from cper.h to ghes.h
    Add data_len decrement back into print loop
    Change references to ARMv8 to just ARM
    Rewrite ARM processor context info parsing
    Check valid bit of ARM error info field before printing it
    Add include of linux/uuid.h in ghes.c

V5: Fix GHES goto logic for error conditions
    Change ghes_do_read_ack to ghes_ack_error
    Make sure data version check is >= 3
    Use CPER helper functions in print functions
    Make handle_guest_sea() dummy function static for arm
    Add arm to subject line for KVM patch

V4: Add bit offset left shift to read_ack_write value
    Make HEST generic and generic_v2 structures a union in the ghes structure
    Move gdata v3 helper functions into ghes.h to avoid duplication
    Reorder the timestamp print and avoid memcpy
    Add helper functions for gdata size checking
    Rename the SEA functions
    Add helper function for GHES panics
    Set fru_id to NULL UUID at variable declaration
    Limit ARM trace event parameters to the needed structures
    Reorder the ARM trace event variables to save space
    Add comment for why we don't pass SEAs to the guest when it aborts
    Move ARM trace event call into GHES driver instead of CPER

V3: Fix unmapped address to the read_ack_register in ghes.c
    Add helper function to get the proper payload based on generic data entry
     version
    Move timestamp print to avoid changing function calls in cper.c
    Remove patch "arm64: exception: handle instruction abort at current EL"
     since the el1_ia handler is already added in 4.8
    Add EFI and ARM64 dependencies for HAVE_ACPI_APEI_SEA
    Add a new trace event for ARM type errors
    Add support to handle KVM guest SEAs

V2: Add PSCI state print for the ARMv8 error type.
    Separate timestamp year into year and century using BCD format.
    Rebase on top of ACPICA 20160318 release and remove header file changes
     in include/acpi/actbl1.h.
    Add panic OS with fatal error status block patch.
    Add processing of unrecognized CPER error section patches with updates
     from previous comments. Original patches: https://lkml.org/lkml/2015/9/8/646

V1: https://lkml.org/lkml/2016/2/5/544

Jonathan (Zhixiong) Zhang (1):
  acpi: apei: panic OS with fatal error status block

Tyler Baicar (9):
  acpi: apei: read ack upon ghes record consumption
  ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
  efi: parse ARM processor error
  arm64: exception: handle Synchronous External Abort
  acpi: apei: handle SEA notification type for ARMv8
  efi: print unrecognized CPER section
  ras: acpi / apei: generate trace event for unrecognized CPER section
  trace, ras: add ARM processor error trace event
  arm/arm64: KVM: add guest SEA support

 arch/arm/include/asm/kvm_arm.h       |   1 +
 arch/arm/include/asm/system_misc.h   |   5 +
 arch/arm/kvm/mmu.c                   |  18 ++-
 arch/arm64/Kconfig                   |   1 +
 arch/arm64/include/asm/esr.h         |   1 +
 arch/arm64/include/asm/kvm_arm.h     |   1 +
 arch/arm64/include/asm/system_misc.h |   2 +
 arch/arm64/mm/fault.c                |  74 +++++++++++--
 drivers/acpi/apei/Kconfig            |  15 +++
 drivers/acpi/apei/ghes.c             | 184 +++++++++++++++++++++++++++----
 drivers/acpi/apei/hest.c             |   7 +-
 drivers/firmware/efi/cper.c          | 207 ++++++++++++++++++++++++++++++++---
 drivers/ras/ras.c                    |   2 +
 include/acpi/ghes.h                  |  34 +++++-
 include/linux/cper.h                 |  54 +++++++++
 include/ras/ras_event.h              |  79 +++++++++++++
 16 files changed, 636 insertions(+), 49 deletions(-)

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH V11 01/10] acpi: apei: read ack upon ghes record consumption
  2017-02-21 21:21 [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar
@ 2017-02-21 21:21 ` Tyler Baicar
  2017-02-21 21:21 ` [PATCH V11 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1 Tyler Baicar
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Tyler Baicar @ 2017-02-21 21:21 UTC (permalink / raw)
  To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, james.morse, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe
  Cc: Tyler Baicar

A RAS (Reliability, Availability, Serviceability) controller
may be a separate processor running in parallel with OS
execution, and may generate error records for consumption by
the OS. If the RAS controller produces multiple error records,
then they may be overwritten before the OS has consumed them.

The Generic Hardware Error Source (GHES) v2 structure
introduces the capability for the OS to acknowledge the
consumption of the error record generated by the RAS
controller. A RAS controller supporting GHESv2 shall wait for
the acknowledgment before writing a new error record, thus
eliminating the race condition.

Add support for parsing of GHESv2 sub-tables as well.

Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Reviewed-by: James Morse <james.morse@arm.com>
---
 drivers/acpi/apei/ghes.c | 49 +++++++++++++++++++++++++++++++++++++++++++++---
 drivers/acpi/apei/hest.c |  7 +++++--
 include/acpi/ghes.h      |  5 ++++-
 3 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index e53bef6..5e1ec41 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -45,6 +45,7 @@
 #include <linux/aer.h>
 #include <linux/nmi.h>
 
+#include <acpi/actbl1.h>
 #include <acpi/ghes.h>
 #include <acpi/apei.h>
 #include <asm/tlbflush.h>
@@ -79,6 +80,10 @@
 	((struct acpi_hest_generic_status *)				\
 	 ((struct ghes_estatus_node *)(estatus_node) + 1))
 
+#define IS_HEST_TYPE_GENERIC_V2(ghes)				\
+	((struct acpi_hest_header *)ghes->generic)->type ==	\
+	 ACPI_HEST_TYPE_GENERIC_ERROR_V2
+
 /*
  * This driver isn't really modular, however for the time being,
  * continuing to use module_param is the easiest way to remain
@@ -248,10 +253,18 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
 	ghes = kzalloc(sizeof(*ghes), GFP_KERNEL);
 	if (!ghes)
 		return ERR_PTR(-ENOMEM);
+
 	ghes->generic = generic;
+	if (IS_HEST_TYPE_GENERIC_V2(ghes)) {
+		rc = apei_map_generic_address(
+			&ghes->generic_v2->read_ack_register);
+		if (rc)
+			goto err_free;
+	}
+
 	rc = apei_map_generic_address(&generic->error_status_address);
 	if (rc)
-		goto err_free;
+		goto err_unmap_read_ack_addr;
 	error_block_length = generic->error_block_length;
 	if (error_block_length > GHES_ESTATUS_MAX_SIZE) {
 		pr_warning(FW_WARN GHES_PFX
@@ -263,13 +276,17 @@ static struct ghes *ghes_new(struct acpi_hest_generic *generic)
 	ghes->estatus = kmalloc(error_block_length, GFP_KERNEL);
 	if (!ghes->estatus) {
 		rc = -ENOMEM;
-		goto err_unmap;
+		goto err_unmap_status_addr;
 	}
 
 	return ghes;
 
-err_unmap:
+err_unmap_status_addr:
 	apei_unmap_generic_address(&generic->error_status_address);
+err_unmap_read_ack_addr:
+	if (IS_HEST_TYPE_GENERIC_V2(ghes))
+		apei_unmap_generic_address(
+			&ghes->generic_v2->read_ack_register);
 err_free:
 	kfree(ghes);
 	return ERR_PTR(rc);
@@ -279,6 +296,9 @@ static void ghes_fini(struct ghes *ghes)
 {
 	kfree(ghes->estatus);
 	apei_unmap_generic_address(&ghes->generic->error_status_address);
+	if (IS_HEST_TYPE_GENERIC_V2(ghes))
+		apei_unmap_generic_address(
+			&ghes->generic_v2->read_ack_register);
 }
 
 static inline int ghes_severity(int severity)
@@ -648,6 +668,23 @@ static void ghes_estatus_cache_add(
 	rcu_read_unlock();
 }
 
+static int ghes_ack_error(struct acpi_hest_generic_v2 *generic_v2)
+{
+	int rc;
+	u64 val = 0;
+
+	rc = apei_read(&val, &generic_v2->read_ack_register);
+	if (rc)
+		return rc;
+	val &= generic_v2->read_ack_preserve <<
+		generic_v2->read_ack_register.bit_offset;
+	val |= generic_v2->read_ack_write <<
+		generic_v2->read_ack_register.bit_offset;
+	rc = apei_write(val, &generic_v2->read_ack_register);
+
+	return rc;
+}
+
 static int ghes_proc(struct ghes *ghes)
 {
 	int rc;
@@ -660,6 +697,12 @@ static int ghes_proc(struct ghes *ghes)
 			ghes_estatus_cache_add(ghes->generic, ghes->estatus);
 	}
 	ghes_do_proc(ghes, ghes->estatus);
+
+	if (IS_HEST_TYPE_GENERIC_V2(ghes)) {
+		rc = ghes_ack_error(ghes->generic_v2);
+		if (rc)
+			return rc;
+	}
 out:
 	ghes_clear_estatus(ghes);
 	return rc;
diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
index 8f2a98e..456b488 100644
--- a/drivers/acpi/apei/hest.c
+++ b/drivers/acpi/apei/hest.c
@@ -52,6 +52,7 @@
 	[ACPI_HEST_TYPE_AER_ENDPOINT] = sizeof(struct acpi_hest_aer),
 	[ACPI_HEST_TYPE_AER_BRIDGE] = sizeof(struct acpi_hest_aer_bridge),
 	[ACPI_HEST_TYPE_GENERIC_ERROR] = sizeof(struct acpi_hest_generic),
+	[ACPI_HEST_TYPE_GENERIC_ERROR_V2] = sizeof(struct acpi_hest_generic_v2),
 };
 
 static int hest_esrc_len(struct acpi_hest_header *hest_hdr)
@@ -141,7 +142,8 @@ static int __init hest_parse_ghes_count(struct acpi_hest_header *hest_hdr, void
 {
 	int *count = data;
 
-	if (hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR)
+	if (hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR ||
+	    hest_hdr->type == ACPI_HEST_TYPE_GENERIC_ERROR_V2)
 		(*count)++;
 	return 0;
 }
@@ -152,7 +154,8 @@ static int __init hest_parse_ghes(struct acpi_hest_header *hest_hdr, void *data)
 	struct ghes_arr *ghes_arr = data;
 	int rc, i;
 
-	if (hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR)
+	if (hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR &&
+	    hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR_V2)
 		return 0;
 
 	if (!((struct acpi_hest_generic *)hest_hdr)->enabled)
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 720446c..68f088a 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -13,7 +13,10 @@
 #define GHES_EXITING		0x0002
 
 struct ghes {
-	struct acpi_hest_generic *generic;
+	union {
+		struct acpi_hest_generic *generic;
+		struct acpi_hest_generic_v2 *generic_v2;
+	};
 	struct acpi_hest_generic_status *estatus;
 	u64 buffer_paddr;
 	unsigned long flags;
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH V11 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1
  2017-02-21 21:21 [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar
  2017-02-21 21:21 ` [PATCH V11 01/10] acpi: apei: read ack upon ghes record consumption Tyler Baicar
@ 2017-02-21 21:21 ` Tyler Baicar
  2017-02-21 21:21 ` [PATCH V11 03/10] efi: parse ARM processor error Tyler Baicar
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Tyler Baicar @ 2017-02-21 21:21 UTC (permalink / raw)
  To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, james.morse, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe
  Cc: Tyler Baicar

Currently when a RAS error is reported it is not timestamped.
The ACPI 6.1 spec adds the timestamp field to the generic error
data entry v3 structure. The timestamp of when the firmware
generated the error is now being reported.

Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Reviewed-by: James Morse <james.morse@arm.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/acpi/apei/ghes.c    |  9 ++++---
 drivers/firmware/efi/cper.c | 63 +++++++++++++++++++++++++++++++++++----------
 include/acpi/ghes.h         | 22 ++++++++++++++++
 3 files changed, 77 insertions(+), 17 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 5e1ec41..b25e7cf 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -420,7 +420,8 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int
 	int flags = -1;
 	int sec_sev = ghes_severity(gdata->error_severity);
 	struct cper_sec_mem_err *mem_err;
-	mem_err = (struct cper_sec_mem_err *)(gdata + 1);
+
+	mem_err = acpi_hest_generic_data_payload(gdata);
 
 	if (!(mem_err->validation_bits & CPER_MEM_VALID_PA))
 		return;
@@ -457,7 +458,8 @@ static void ghes_do_proc(struct ghes *ghes,
 		if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
 				 CPER_SEC_PLATFORM_MEM)) {
 			struct cper_sec_mem_err *mem_err;
-			mem_err = (struct cper_sec_mem_err *)(gdata+1);
+
+			mem_err = acpi_hest_generic_data_payload(gdata);
 			ghes_edac_report_mem_error(ghes, sev, mem_err);
 
 			arch_apei_report_mem_error(sev, mem_err);
@@ -467,7 +469,8 @@ static void ghes_do_proc(struct ghes *ghes,
 		else if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
 				      CPER_SEC_PCIE)) {
 			struct cper_sec_pcie *pcie_err;
-			pcie_err = (struct cper_sec_pcie *)(gdata+1);
+
+			pcie_err = acpi_hest_generic_data_payload(gdata);
 			if (sev == GHES_SEV_RECOVERABLE &&
 			    sec_sev == GHES_SEV_RECOVERABLE &&
 			    pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index d425374..8fa4e23 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -32,6 +32,9 @@
 #include <linux/acpi.h>
 #include <linux/pci.h>
 #include <linux/aer.h>
+#include <linux/printk.h>
+#include <linux/bcd.h>
+#include <acpi/ghes.h>
 
 #define INDENT_SP	" "
 
@@ -386,13 +389,37 @@ static void cper_print_pcie(const char *pfx, const struct cper_sec_pcie *pcie,
 	pfx, pcie->bridge.secondary_status, pcie->bridge.control);
 }
 
+static void cper_estatus_print_section_v300(const char *pfx,
+	const struct acpi_hest_generic_data_v300 *gdata)
+{
+	__u8 hour, min, sec, day, mon, year, century, *timestamp;
+
+	if (gdata->validation_bits & ACPI_HEST_GEN_VALID_TIMESTAMP) {
+		timestamp = (__u8 *)&(gdata->time_stamp);
+		sec = bcd2bin(timestamp[0]);
+		min = bcd2bin(timestamp[1]);
+		hour = bcd2bin(timestamp[2]);
+		day = bcd2bin(timestamp[4]);
+		mon = bcd2bin(timestamp[5]);
+		year = bcd2bin(timestamp[6]);
+		century = bcd2bin(timestamp[7]);
+		printk("%stime: %7s %02d%02d-%02d-%02d %02d:%02d:%02d\n", pfx,
+			0x01 & *(timestamp + 3) ? "precise" : "", century,
+			year, mon, day, hour, min, sec);
+	}
+}
+
 static void cper_estatus_print_section(
-	const char *pfx, const struct acpi_hest_generic_data *gdata, int sec_no)
+	const char *pfx, struct acpi_hest_generic_data *gdata, int sec_no)
 {
 	uuid_le *sec_type = (uuid_le *)gdata->section_type;
 	__u16 severity;
 	char newpfx[64];
 
+	if (acpi_hest_generic_data_version(gdata) >= 3)
+		cper_estatus_print_section_v300(pfx,
+			(const struct acpi_hest_generic_data_v300 *)gdata);
+
 	severity = gdata->error_severity;
 	printk("%s""Error %d, type: %s\n", pfx, sec_no,
 	       cper_severity_str(severity));
@@ -403,14 +430,18 @@ static void cper_estatus_print_section(
 
 	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
 	if (!uuid_le_cmp(*sec_type, CPER_SEC_PROC_GENERIC)) {
-		struct cper_sec_proc_generic *proc_err = (void *)(gdata + 1);
+		struct cper_sec_proc_generic *proc_err;
+
+		proc_err = acpi_hest_generic_data_payload(gdata);
 		printk("%s""section_type: general processor error\n", newpfx);
 		if (gdata->error_data_length >= sizeof(*proc_err))
 			cper_print_proc_generic(newpfx, proc_err);
 		else
 			goto err_section_too_small;
 	} else if (!uuid_le_cmp(*sec_type, CPER_SEC_PLATFORM_MEM)) {
-		struct cper_sec_mem_err *mem_err = (void *)(gdata + 1);
+		struct cper_sec_mem_err *mem_err;
+
+		mem_err = acpi_hest_generic_data_payload(gdata);
 		printk("%s""section_type: memory error\n", newpfx);
 		if (gdata->error_data_length >=
 		    sizeof(struct cper_sec_mem_err_old))
@@ -419,7 +450,9 @@ static void cper_estatus_print_section(
 		else
 			goto err_section_too_small;
 	} else if (!uuid_le_cmp(*sec_type, CPER_SEC_PCIE)) {
-		struct cper_sec_pcie *pcie = (void *)(gdata + 1);
+		struct cper_sec_pcie *pcie;
+
+		pcie = acpi_hest_generic_data_payload(gdata);
 		printk("%s""section_type: PCIe error\n", newpfx);
 		if (gdata->error_data_length >= sizeof(*pcie))
 			cper_print_pcie(newpfx, pcie, gdata);
@@ -438,7 +471,7 @@ void cper_estatus_print(const char *pfx,
 			const struct acpi_hest_generic_status *estatus)
 {
 	struct acpi_hest_generic_data *gdata;
-	unsigned int data_len, gedata_len;
+	unsigned int data_len;
 	int sec_no = 0;
 	char newpfx[64];
 	__u16 severity;
@@ -451,12 +484,13 @@ void cper_estatus_print(const char *pfx,
 	printk("%s""event severity: %s\n", pfx, cper_severity_str(severity));
 	data_len = estatus->data_length;
 	gdata = (struct acpi_hest_generic_data *)(estatus + 1);
+
 	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
-	while (data_len >= sizeof(*gdata)) {
-		gedata_len = gdata->error_data_length;
+
+	while (data_len >= acpi_hest_generic_data_size(gdata)) {
 		cper_estatus_print_section(newpfx, gdata, sec_no);
-		data_len -= gedata_len + sizeof(*gdata);
-		gdata = (void *)(gdata + 1) + gedata_len;
+		data_len -= acpi_hest_generic_data_record_size(gdata);
+		gdata = acpi_hest_generic_data_next(gdata);
 		sec_no++;
 	}
 }
@@ -486,12 +520,13 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
 		return rc;
 	data_len = estatus->data_length;
 	gdata = (struct acpi_hest_generic_data *)(estatus + 1);
-	while (data_len >= sizeof(*gdata)) {
-		gedata_len = gdata->error_data_length;
-		if (gedata_len > data_len - sizeof(*gdata))
+
+	while (data_len >= acpi_hest_generic_data_size(gdata)) {
+		gedata_len = acpi_hest_generic_data_error_length(gdata);
+		if (gedata_len > data_len - acpi_hest_generic_data_size(gdata))
 			return -EINVAL;
-		data_len -= gedata_len + sizeof(*gdata);
-		gdata = (void *)(gdata + 1) + gedata_len;
+		data_len -= gedata_len + acpi_hest_generic_data_size(gdata);
+		gdata = acpi_hest_generic_data_next(gdata);
 	}
 	if (data_len)
 		return -EINVAL;
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 68f088a..6ae318b 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -12,6 +12,18 @@
 #define GHES_TO_CLEAR		0x0001
 #define GHES_EXITING		0x0002
 
+#define acpi_hest_generic_data_error_length(gdata)	\
+	(((struct acpi_hest_generic_data *)(gdata))->error_data_length)
+#define acpi_hest_generic_data_size(gdata)		\
+	((acpi_hest_generic_data_version(gdata) >= 3) ?	\
+	sizeof(struct acpi_hest_generic_data_v300) :	\
+	sizeof(struct acpi_hest_generic_data))
+#define acpi_hest_generic_data_record_size(gdata)	\
+	(acpi_hest_generic_data_size(gdata) +		\
+	acpi_hest_generic_data_error_length(gdata))
+#define acpi_hest_generic_data_next(gdata)		\
+	((void *)(gdata) + acpi_hest_generic_data_record_size(gdata))
+
 struct ghes {
 	union {
 		struct acpi_hest_generic *generic;
@@ -73,3 +85,13 @@ static inline void ghes_edac_unregister(struct ghes *ghes)
 {
 }
 #endif
+
+#define acpi_hest_generic_data_version(gdata)			\
+	(gdata->revision >> 8)
+
+static inline void *acpi_hest_generic_data_payload(struct acpi_hest_generic_data *gdata)
+{
+	return acpi_hest_generic_data_version(gdata) >= 3 ?
+		(void *)(((struct acpi_hest_generic_data_v300 *)(gdata)) + 1) :
+		gdata + 1;
+}
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH V11 03/10] efi: parse ARM processor error
  2017-02-21 21:21 [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar
  2017-02-21 21:21 ` [PATCH V11 01/10] acpi: apei: read ack upon ghes record consumption Tyler Baicar
  2017-02-21 21:21 ` [PATCH V11 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1 Tyler Baicar
@ 2017-02-21 21:21 ` Tyler Baicar
  2017-02-21 21:21 ` [PATCH V11 04/10] arm64: exception: handle Synchronous External Abort Tyler Baicar
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Tyler Baicar @ 2017-02-21 21:21 UTC (permalink / raw)
  To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, james.morse, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe
  Cc: Tyler Baicar

Add support for ARM Common Platform Error Record (CPER).
UEFI 2.6 specification adds support for ARM specific
processor error information to be reported as part of the
CPER records. This provides more detail on for processor error logs.

Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Reviewed-by: James Morse <james.morse@arm.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/cper.c | 133 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/cper.h        |  54 ++++++++++++++++++
 2 files changed, 187 insertions(+)

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index 8fa4e23..0238877 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -110,12 +110,15 @@ void cper_print_bits(const char *pfx, unsigned int bits,
 static const char * const proc_type_strs[] = {
 	"IA32/X64",
 	"IA64",
+	"ARM",
 };
 
 static const char * const proc_isa_strs[] = {
 	"IA32",
 	"IA64",
 	"X64",
+	"ARM A32/T32",
+	"ARM A64",
 };
 
 static const char * const proc_error_type_strs[] = {
@@ -139,6 +142,18 @@ void cper_print_bits(const char *pfx, unsigned int bits,
 	"corrected",
 };
 
+static const char * const arm_reg_ctx_strs[] = {
+	"AArch32 general purpose registers",
+	"AArch32 EL1 context registers",
+	"AArch32 EL2 context registers",
+	"AArch32 secure context registers",
+	"AArch64 general purpose registers",
+	"AArch64 EL1 context registers",
+	"AArch64 EL2 context registers",
+	"AArch64 EL3 context registers",
+	"Misc. system register structure",
+};
+
 static void cper_print_proc_generic(const char *pfx,
 				    const struct cper_sec_proc_generic *proc)
 {
@@ -184,6 +199,114 @@ static void cper_print_proc_generic(const char *pfx,
 		printk("%s""IP: 0x%016llx\n", pfx, proc->ip);
 }
 
+static void cper_print_proc_arm(const char *pfx,
+				const struct cper_sec_proc_arm *proc)
+{
+	int i, len, max_ctx_type;
+	struct cper_arm_err_info *err_info;
+	struct cper_arm_ctx_info *ctx_info;
+	char newpfx[64];
+
+	printk("%s""section length: %d\n", pfx, proc->section_length);
+	printk("%s""MIDR: 0x%016llx\n", pfx, proc->midr);
+
+	len = proc->section_length - (sizeof(*proc) +
+		proc->err_info_num * (sizeof(*err_info)));
+	if (len < 0) {
+		printk("%s""section length is too small\n", pfx);
+		printk("%s""firmware-generated error record is incorrect\n", pfx);
+		printk("%s""ERR_INFO_NUM is %d\n", pfx, proc->err_info_num);
+		return;
+	}
+
+	if (proc->validation_bits & CPER_ARM_VALID_MPIDR)
+		printk("%s""MPIDR: 0x%016llx\n", pfx, proc->mpidr);
+	if (proc->validation_bits & CPER_ARM_VALID_AFFINITY_LEVEL)
+		printk("%s""error affinity level: %d\n", pfx,
+			proc->affinity_level);
+	if (proc->validation_bits & CPER_ARM_VALID_RUNNING_STATE) {
+		printk("%s""running state: 0x%x\n", pfx, proc->running_state);
+		printk("%s""PSCI state: %d\n", pfx, proc->psci_state);
+	}
+
+	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
+
+	err_info = (struct cper_arm_err_info *)(proc + 1);
+	for (i = 0; i < proc->err_info_num; i++) {
+		printk("%s""Error info structure %d:\n", pfx, i);
+		printk("%s""version:%d\n", newpfx, err_info->version);
+		printk("%s""length:%d\n", newpfx, err_info->length);
+		if (err_info->validation_bits &
+		    CPER_ARM_INFO_VALID_MULTI_ERR) {
+			if (err_info->multiple_error == 0)
+				printk("%s""single error\n", newpfx);
+			else if (err_info->multiple_error == 1)
+				printk("%s""multiple errors\n", newpfx);
+			else
+				printk("%s""multiple errors count:%u\n",
+				newpfx, err_info->multiple_error);
+		}
+		if (err_info->validation_bits & CPER_ARM_INFO_VALID_FLAGS) {
+			if (err_info->flags & CPER_ARM_INFO_FLAGS_FIRST)
+				printk("%s""first error captured\n", newpfx);
+			if (err_info->flags & CPER_ARM_INFO_FLAGS_LAST)
+				printk("%s""last error captured\n", newpfx);
+			if (err_info->flags & CPER_ARM_INFO_FLAGS_PROPAGATED)
+				printk("%s""propagated error captured\n",
+				       newpfx);
+			if (err_info->flags & CPER_ARM_INFO_FLAGS_OVERFLOW)
+				printk("%s""overflow occurred, error info is incomplete\n",
+				       newpfx);
+		}
+		printk("%s""error_type: %d, %s\n", newpfx, err_info->type,
+			err_info->type < ARRAY_SIZE(proc_error_type_strs) ?
+			proc_error_type_strs[err_info->type] : "unknown");
+		if (err_info->validation_bits & CPER_ARM_INFO_VALID_ERR_INFO)
+			printk("%s""error_info: 0x%016llx\n", newpfx,
+			       err_info->error_info);
+		if (err_info->validation_bits & CPER_ARM_INFO_VALID_VIRT_ADDR)
+			printk("%s""virtual fault address: 0x%016llx\n",
+				newpfx, err_info->virt_fault_addr);
+		if (err_info->validation_bits &
+		    CPER_ARM_INFO_VALID_PHYSICAL_ADDR)
+			printk("%s""physical fault address: 0x%016llx\n",
+				newpfx, err_info->physical_fault_addr);
+		err_info += 1;
+	}
+	ctx_info = (struct cper_arm_ctx_info *)err_info;
+	max_ctx_type = ARRAY_SIZE(arm_reg_ctx_strs) - 1;
+	for (i = 0; i < proc->context_info_num; i++) {
+		int size = sizeof(*ctx_info) + ctx_info->size;
+
+		printk("%s""Context info structure %d:\n", pfx, i);
+		if (len < size) {
+			printk("%s""section length is too small\n", newpfx);
+			printk("%s""firmware-generated error record is incorrect\n", pfx);
+			return;
+		}
+		if (ctx_info->type > max_ctx_type) {
+			printk("%s""Invalid context type: %d\n", newpfx,
+							ctx_info->type);
+			printk("%s""Max context type: %d\n", newpfx,
+							max_ctx_type);
+			return;
+		}
+		printk("%s""register context type %d: %s\n", newpfx,
+			ctx_info->type, arm_reg_ctx_strs[ctx_info->type]);
+		print_hex_dump(newpfx, "", DUMP_PREFIX_OFFSET, 16, 4,
+				(ctx_info + 1), ctx_info->size, 0);
+		len -= size;
+		ctx_info = (struct cper_arm_ctx_info *)((long)ctx_info + size);
+	}
+
+	if (len > 0) {
+		printk("%s""Vendor specific error info has %u bytes:\n", pfx,
+		       len);
+		print_hex_dump(newpfx, "", DUMP_PREFIX_OFFSET, 16, 4, ctx_info,
+				len, true);
+	}
+}
+
 static const char * const mem_err_type_strs[] = {
 	"unknown",
 	"no error",
@@ -458,6 +581,16 @@ static void cper_estatus_print_section(
 			cper_print_pcie(newpfx, pcie, gdata);
 		else
 			goto err_section_too_small;
+	} else if ((IS_ENABLED(CONFIG_ARM64) || IS_ENABLED(CONFIG_ARM)) &&
+		   !uuid_le_cmp(*sec_type, CPER_SEC_PROC_ARM)) {
+		struct cper_sec_proc_arm *arm_err;
+
+		arm_err = acpi_hest_generic_data_payload(gdata);
+		printk("%s""section_type: ARM processor error\n", newpfx);
+		if (gdata->error_data_length >= sizeof(*arm_err))
+			cper_print_proc_arm(newpfx, arm_err);
+		else
+			goto err_section_too_small;
 	} else
 		printk("%s""section type: unknown, %pUl\n", newpfx, sec_type);
 
diff --git a/include/linux/cper.h b/include/linux/cper.h
index dcacb1a..85450f3 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -180,6 +180,10 @@ enum {
 #define CPER_SEC_PROC_IPF						\
 	UUID_LE(0xE429FAF1, 0x3CB7, 0x11D4, 0x0B, 0xCA, 0x07, 0x00,	\
 		0x80, 0xC7, 0x3C, 0x88, 0x81)
+/* Processor Specific: ARM */
+#define CPER_SEC_PROC_ARM						\
+	UUID_LE(0xE19E3D16, 0xBC11, 0x11E4, 0x9C, 0xAA, 0xC2, 0x05,	\
+		0x1D, 0x5D, 0x46, 0xB0)
 /* Platform Memory */
 #define CPER_SEC_PLATFORM_MEM						\
 	UUID_LE(0xA5BC1114, 0x6F64, 0x4EDE, 0xB8, 0x63, 0x3E, 0x83,	\
@@ -255,6 +259,22 @@ enum {
 
 #define CPER_PCIE_SLOT_SHIFT			3
 
+#define CPER_ARM_VALID_MPIDR			0x00000001
+#define CPER_ARM_VALID_AFFINITY_LEVEL		0x00000002
+#define CPER_ARM_VALID_RUNNING_STATE		0x00000004
+#define CPER_ARM_VALID_VENDOR_INFO		0x00000008
+
+#define CPER_ARM_INFO_VALID_MULTI_ERR		0x0001
+#define CPER_ARM_INFO_VALID_FLAGS		0x0002
+#define CPER_ARM_INFO_VALID_ERR_INFO		0x0004
+#define CPER_ARM_INFO_VALID_VIRT_ADDR		0x0008
+#define CPER_ARM_INFO_VALID_PHYSICAL_ADDR	0x0010
+
+#define CPER_ARM_INFO_FLAGS_FIRST		0x0001
+#define CPER_ARM_INFO_FLAGS_LAST		0x0002
+#define CPER_ARM_INFO_FLAGS_PROPAGATED		0x0004
+#define CPER_ARM_INFO_FLAGS_OVERFLOW		0x0008
+
 /*
  * All tables and structs must be byte-packed to match CPER
  * specification, since the tables are provided by the system BIOS
@@ -340,6 +360,40 @@ struct cper_ia_proc_ctx {
 	__u64	mm_reg_addr;
 };
 
+/* ARM Processor Error Section */
+struct cper_sec_proc_arm {
+	__u32	validation_bits;
+	__u16	err_info_num; /* Number of Processor Error Info */
+	__u16	context_info_num; /* Number of Processor Context Info Records*/
+	__u32	section_length;
+	__u8	affinity_level;
+	__u8	reserved[3];	/* must be zero */
+	__u64	mpidr;
+	__u64	midr;
+	__u32	running_state; /* Bit 0 set - Processor running. PSCI = 0 */
+	__u32	psci_state;
+};
+
+/* ARM Processor Error Information Structure */
+struct cper_arm_err_info {
+	__u8	version;
+	__u8	length;
+	__u16	validation_bits;
+	__u8	type;
+	__u16	multiple_error;
+	__u8	flags;
+	__u64	error_info;
+	__u64	virt_fault_addr;
+	__u64	physical_fault_addr;
+};
+
+/* ARM Processor Context Information Structure */
+struct cper_arm_ctx_info {
+	__u16	version;
+	__u16	type;
+	__u32	size;
+};
+
 /* Old Memory Error Section UEFI 2.1, 2.2 */
 struct cper_sec_mem_err_old {
 	__u64	validation_bits;
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH V11 04/10] arm64: exception: handle Synchronous External Abort
  2017-02-21 21:21 [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar
                   ` (2 preceding siblings ...)
  2017-02-21 21:21 ` [PATCH V11 03/10] efi: parse ARM processor error Tyler Baicar
@ 2017-02-21 21:21 ` Tyler Baicar
  2017-02-21 21:21 ` [PATCH V11 05/10] acpi: apei: handle SEA notification type for ARMv8 Tyler Baicar
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Tyler Baicar @ 2017-02-21 21:21 UTC (permalink / raw)
  To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, james.morse, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe
  Cc: Tyler Baicar

SEA exceptions are often caused by an uncorrected hardware
error, and are handled when data abort and instruction abort
exception classes have specific values for their Fault Status
Code.
When SEA occurs, before killing the process, report the error
in the kernel logs.
Update fault_info[] with specific SEA faults so that the
new SEA handler is used.

Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Reviewed-by: James Morse <james.morse@arm.com>
---
 arch/arm64/include/asm/esr.h |  1 +
 arch/arm64/mm/fault.c        | 43 +++++++++++++++++++++++++++++++++----------
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index d14c478..f20c64a 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -83,6 +83,7 @@
 #define ESR_ELx_WNR		(UL(1) << 6)
 
 /* Shared ISS field definitions for Data/Instruction aborts */
+#define ESR_ELx_FnV		(UL(1) << 10)
 #define ESR_ELx_EA		(UL(1) << 9)
 #define ESR_ELx_S1PTW		(UL(1) << 7)
 
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 156169c..d178dc0 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -487,6 +487,29 @@ static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	return 1;
 }
 
+/*
+ * This abort handler deals with Synchronous External Abort.
+ * It calls notifiers, and then returns "fault".
+ */
+static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
+{
+	struct siginfo info;
+
+	pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
+		 fault_name(esr), esr, addr);
+
+	info.si_signo = SIGBUS;
+	info.si_errno = 0;
+	info.si_code  = 0;
+	if (esr & ESR_ELx_FnV)
+		info.si_addr = 0;
+	else
+		info.si_addr  = (void __user *)addr;
+	arm64_notify_die("", regs, &info, esr);
+
+	return 0;
+}
+
 static const struct fault_info {
 	int	(*fn)(unsigned long addr, unsigned int esr, struct pt_regs *regs);
 	int	sig;
@@ -509,22 +532,22 @@ static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	{ do_page_fault,	SIGSEGV, SEGV_ACCERR,	"level 1 permission fault"	},
 	{ do_page_fault,	SIGSEGV, SEGV_ACCERR,	"level 2 permission fault"	},
 	{ do_page_fault,	SIGSEGV, SEGV_ACCERR,	"level 3 permission fault"	},
-	{ do_bad,		SIGBUS,  0,		"synchronous external abort"	},
+	{ do_sea,		SIGBUS,  0,		"synchronous external abort"	},
 	{ do_bad,		SIGBUS,  0,		"unknown 17"			},
 	{ do_bad,		SIGBUS,  0,		"unknown 18"			},
 	{ do_bad,		SIGBUS,  0,		"unknown 19"			},
-	{ do_bad,		SIGBUS,  0,		"synchronous external abort (translation table walk)" },
-	{ do_bad,		SIGBUS,  0,		"synchronous external abort (translation table walk)" },
-	{ do_bad,		SIGBUS,  0,		"synchronous external abort (translation table walk)" },
-	{ do_bad,		SIGBUS,  0,		"synchronous external abort (translation table walk)" },
-	{ do_bad,		SIGBUS,  0,		"synchronous parity error"	},
+	{ do_sea,		SIGBUS,  0,		"level 0 (translation table walk)"	},
+	{ do_sea,		SIGBUS,  0,		"level 1 (translation table walk)"	},
+	{ do_sea,		SIGBUS,  0,		"level 2 (translation table walk)"	},
+	{ do_sea,		SIGBUS,  0,		"level 3 (translation table walk)"	},
+	{ do_sea,		SIGBUS,  0,		"synchronous parity or ECC error" },
 	{ do_bad,		SIGBUS,  0,		"unknown 25"			},
 	{ do_bad,		SIGBUS,  0,		"unknown 26"			},
 	{ do_bad,		SIGBUS,  0,		"unknown 27"			},
-	{ do_bad,		SIGBUS,  0,		"synchronous parity error (translation table walk)" },
-	{ do_bad,		SIGBUS,  0,		"synchronous parity error (translation table walk)" },
-	{ do_bad,		SIGBUS,  0,		"synchronous parity error (translation table walk)" },
-	{ do_bad,		SIGBUS,  0,		"synchronous parity error (translation table walk)" },
+	{ do_sea,		SIGBUS,  0,		"level 0 synchronous parity error (translation table walk)"	},
+	{ do_sea,		SIGBUS,  0,		"level 1 synchronous parity error (translation table walk)"	},
+	{ do_sea,		SIGBUS,  0,		"level 2 synchronous parity error (translation table walk)"	},
+	{ do_sea,		SIGBUS,  0,		"level 3 synchronous parity error (translation table walk)"	},
 	{ do_bad,		SIGBUS,  0,		"unknown 32"			},
 	{ do_alignment_fault,	SIGBUS,  BUS_ADRALN,	"alignment fault"		},
 	{ do_bad,		SIGBUS,  0,		"unknown 34"			},
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH V11 05/10] acpi: apei: handle SEA notification type for ARMv8
  2017-02-21 21:21 [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar
                   ` (3 preceding siblings ...)
  2017-02-21 21:21 ` [PATCH V11 04/10] arm64: exception: handle Synchronous External Abort Tyler Baicar
@ 2017-02-21 21:21 ` Tyler Baicar
  2017-03-01  7:42   ` Xie XiuQi
  2017-02-21 21:21 ` [PATCH V11 06/10] acpi: apei: panic OS with fatal error status block Tyler Baicar
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 29+ messages in thread
From: Tyler Baicar @ 2017-02-21 21:21 UTC (permalink / raw)
  To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, james.morse, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe
  Cc: Tyler Baicar

ARM APEI extension proposal added SEA (Synchronous External Abort)
notification type for ARMv8.
Add a new GHES error source handling function for SEA. If an error
source's notification type is SEA, then this function can be registered
into the SEA exception handler. That way GHES will parse and report
SEA exceptions when they occur.
An SEA can interrupt code that had interrupts masked and is treated as
an NMI. To aid this the page of address space for mapping APEI buffers
while in_nmi() is always reserved, and ghes_ioremap_pfn_nmi() is
changed to use the helper methods to find the prot_t to map with in
the same way as ghes_ioremap_pfn_irq().

Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
---
 arch/arm64/Kconfig        |  1 +
 arch/arm64/mm/fault.c     | 13 ++++++++
 drivers/acpi/apei/Kconfig | 15 +++++++++
 drivers/acpi/apei/ghes.c  | 77 +++++++++++++++++++++++++++++++++++++++++++----
 include/acpi/ghes.h       |  7 +++++
 5 files changed, 107 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1117421..fca4dc1 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -88,6 +88,7 @@ config ARM64
 	select HAVE_IRQ_TIME_ACCOUNTING
 	select HAVE_MEMBLOCK
 	select HAVE_MEMBLOCK_NODE_MAP if NUMA
+	select HAVE_NMI if ACPI_APEI_SEA
 	select HAVE_PATA_PLATFORM
 	select HAVE_PERF_EVENTS
 	select HAVE_PERF_REGS
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index d178dc0..b2d57fc 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -41,6 +41,8 @@
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
 
+#include <acpi/ghes.h>
+
 static const char *fault_name(unsigned int esr);
 
 #ifdef CONFIG_KPROBES
@@ -498,6 +500,17 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
 		 fault_name(esr), esr, addr);
 
+	/*
+	 * Synchronous aborts may interrupt code which had interrupts masked.
+	 * Before calling out into the wider kernel tell the interested
+	 * subsystems.
+	 */
+	if (IS_ENABLED(ACPI_APEI_SEA)) {
+		nmi_enter();
+		ghes_notify_sea();
+		nmi_exit();
+	}
+
 	info.si_signo = SIGBUS;
 	info.si_errno = 0;
 	info.si_code  = 0;
diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
index b0140c8..c545dd1 100644
--- a/drivers/acpi/apei/Kconfig
+++ b/drivers/acpi/apei/Kconfig
@@ -39,6 +39,21 @@ config ACPI_APEI_PCIEAER
 	  PCIe AER errors may be reported via APEI firmware first mode.
 	  Turn on this option to enable the corresponding support.
 
+config ACPI_APEI_SEA
+	bool "APEI Synchronous External Abort logging/recovering support"
+	depends on ARM64 && ACPI_APEI && 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"
 	depends on ACPI_APEI && MEMORY_FAILURE
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index b25e7cf..b0596ba 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -114,11 +114,7 @@
  * Two virtual pages are used, one for IRQ/PROCESS context, the other for
  * NMI context (optionally).
  */
-#ifdef CONFIG_HAVE_ACPI_APEI_NMI
 #define GHES_IOREMAP_PAGES           2
-#else
-#define GHES_IOREMAP_PAGES           1
-#endif
 #define GHES_IOREMAP_IRQ_PAGE(base)	(base)
 #define GHES_IOREMAP_NMI_PAGE(base)	((base) + PAGE_SIZE)
 
@@ -157,10 +153,14 @@ static void ghes_ioremap_exit(void)
 static void __iomem *ghes_ioremap_pfn_nmi(u64 pfn)
 {
 	unsigned long vaddr;
+	phys_addr_t paddr;
+	pgprot_t prot;
 
 	vaddr = (unsigned long)GHES_IOREMAP_NMI_PAGE(ghes_ioremap_area->addr);
-	ioremap_page_range(vaddr, vaddr + PAGE_SIZE,
-			   pfn << PAGE_SHIFT, PAGE_KERNEL);
+
+	paddr = pfn << PAGE_SHIFT;
+	prot = arch_apei_get_mem_attribute(paddr);
+	ioremap_page_range(vaddr, vaddr + PAGE_SIZE, paddr, prot);
 
 	return (void __iomem *)vaddr;
 }
@@ -767,6 +767,50 @@ static int ghes_notify_sci(struct notifier_block *this,
 	.notifier_call = ghes_notify_sci,
 };
 
+#ifdef CONFIG_ACPI_APEI_SEA
+static LIST_HEAD(ghes_sea);
+
+void ghes_notify_sea(void)
+{
+	struct ghes *ghes;
+
+	/*
+	 * synchronize_rcu() will wait for nmi_exit(), so no need to
+	 * rcu_read_lock().
+	 */
+	list_for_each_entry_rcu(ghes, &ghes_sea, list) {
+		ghes_proc(ghes);
+	}
+}
+
+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)
+{
+	pr_err(GHES_PFX "ID: %d, trying to add SEA notification which is not supported\n",
+	       ghes->generic->header.source_id);
+}
+
+static inline void ghes_sea_remove(struct ghes *ghes)
+{
+	pr_err(GHES_PFX "ID: %d, trying to remove SEA notification which is not supported\n",
+	       ghes->generic->header.source_id);
+}
+#endif /* CONFIG_ACPI_APEI_SEA */
+
 #ifdef CONFIG_HAVE_ACPI_APEI_NMI
 /*
  * printk is not safe in NMI context.  So in NMI handler, we allocate
@@ -1012,6 +1056,14 @@ static int ghes_probe(struct platform_device *ghes_dev)
 	case ACPI_HEST_NOTIFY_EXTERNAL:
 	case ACPI_HEST_NOTIFY_SCI:
 		break;
+	case ACPI_HEST_NOTIFY_SEA:
+		if (!IS_ENABLED(CONFIG_ACPI_APEI_SEA)) {
+			pr_warn(GHES_PFX "Generic hardware error source: %d notified via SEA is not supported\n",
+				generic->header.source_id);
+			rc = -ENOTSUPP;
+			goto err;
+		}
+		break;
 	case ACPI_HEST_NOTIFY_NMI:
 		if (!IS_ENABLED(CONFIG_HAVE_ACPI_APEI_NMI)) {
 			pr_warn(GHES_PFX "Generic hardware error source: %d notified via NMI interrupt is not supported!\n",
@@ -1023,6 +1075,13 @@ static int ghes_probe(struct platform_device *ghes_dev)
 		pr_warning(GHES_PFX "Generic hardware error source: %d notified via local interrupt is not supported!\n",
 			   generic->header.source_id);
 		goto err;
+	case ACPI_HEST_NOTIFY_GPIO:
+	case ACPI_HEST_NOTIFY_SEI:
+	case ACPI_HEST_NOTIFY_GSIV:
+		pr_warn(GHES_PFX "Generic hardware error source: %d notified via notification type %u is not supported\n",
+			generic->header.source_id, generic->header.source_id);
+		rc = -ENOTSUPP;
+		goto err;
 	default:
 		pr_warning(FW_WARN GHES_PFX "Unknown notification type: %u for generic hardware error source: %d\n",
 			   generic->notify.type, generic->header.source_id);
@@ -1077,6 +1136,9 @@ static int ghes_probe(struct platform_device *ghes_dev)
 		list_add_rcu(&ghes->list, &ghes_sci);
 		mutex_unlock(&ghes_list_mutex);
 		break;
+	case ACPI_HEST_NOTIFY_SEA:
+		ghes_sea_add(ghes);
+		break;
 	case ACPI_HEST_NOTIFY_NMI:
 		ghes_nmi_add(ghes);
 		break;
@@ -1119,6 +1181,9 @@ static int ghes_remove(struct platform_device *ghes_dev)
 			unregister_acpi_hed_notifier(&ghes_notifier_sci);
 		mutex_unlock(&ghes_list_mutex);
 		break;
+	case ACPI_HEST_NOTIFY_SEA:
+		ghes_sea_remove(ghes);
+		break;
 	case ACPI_HEST_NOTIFY_NMI:
 		ghes_nmi_remove(ghes);
 		break;
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 6ae318b..18bc935 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -1,3 +1,6 @@
+#ifndef GHES_H
+#define GHES_H
+
 #include <acpi/apei.h>
 #include <acpi/hed.h>
 
@@ -95,3 +98,7 @@ static inline void *acpi_hest_generic_data_payload(struct acpi_hest_generic_data
 		(void *)(((struct acpi_hest_generic_data_v300 *)(gdata)) + 1) :
 		gdata + 1;
 }
+
+void ghes_notify_sea(void);
+
+#endif /* GHES_H */
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH V11 06/10] acpi: apei: panic OS with fatal error status block
  2017-02-21 21:21 [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar
                   ` (4 preceding siblings ...)
  2017-02-21 21:21 ` [PATCH V11 05/10] acpi: apei: handle SEA notification type for ARMv8 Tyler Baicar
@ 2017-02-21 21:21 ` Tyler Baicar
  2017-02-21 21:21 ` [PATCH V11 07/10] efi: print unrecognized CPER section Tyler Baicar
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Tyler Baicar @ 2017-02-21 21:21 UTC (permalink / raw)
  To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, james.morse, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe
  Cc: Tyler Baicar

From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>

Even if an error status block's severity is fatal, the kernel does not
honor the severity level and panic.

With the firmware first model, the platform could inform the OS about a
fatal hardware error through the non-NMI GHES notification type. The OS
should panic when a hardware error record is received with this
severity.

Call panic() after CPER data in error status block is printed if
severity is fatal, before each error section is handled.

Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
Reviewed-by: James Morse <james.morse@arm.com>
---
 drivers/acpi/apei/ghes.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index b0596ba..d6a3b9f 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -133,6 +133,8 @@
 static struct ghes_estatus_cache *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE];
 static atomic_t ghes_estatus_cache_alloced;
 
+static int ghes_panic_timeout __read_mostly = 30;
+
 static int ghes_ioremap_init(void)
 {
 	ghes_ioremap_area = __get_vm_area(PAGE_SIZE * GHES_IOREMAP_PAGES,
@@ -688,6 +690,13 @@ static int ghes_ack_error(struct acpi_hest_generic_v2 *generic_v2)
 	return rc;
 }
 
+static void __ghes_call_panic(void)
+{
+	if (panic_timeout == 0)
+		panic_timeout = ghes_panic_timeout;
+	panic("Fatal hardware error!");
+}
+
 static int ghes_proc(struct ghes *ghes)
 {
 	int rc;
@@ -695,6 +704,10 @@ static int ghes_proc(struct ghes *ghes)
 	rc = ghes_read_estatus(ghes, 0);
 	if (rc)
 		goto out;
+	if (ghes_severity(ghes->estatus->error_severity) >= GHES_SEV_PANIC) {
+		__ghes_print_estatus(KERN_EMERG, ghes->generic, ghes->estatus);
+		__ghes_call_panic();
+	}
 	if (!ghes_estatus_cached(ghes->estatus)) {
 		if (ghes_print_estatus(NULL, ghes->generic, ghes->estatus))
 			ghes_estatus_cache_add(ghes->generic, ghes->estatus);
@@ -831,8 +844,6 @@ static inline void ghes_sea_remove(struct ghes *ghes)
 
 static LIST_HEAD(ghes_nmi);
 
-static int ghes_panic_timeout	__read_mostly = 30;
-
 static void ghes_proc_in_irq(struct irq_work *irq_work)
 {
 	struct llist_node *llnode, *next;
@@ -925,9 +936,7 @@ static void __ghes_panic(struct ghes *ghes)
 	__ghes_print_estatus(KERN_EMERG, ghes->generic, ghes->estatus);
 
 	/* reboot to log the error! */
-	if (panic_timeout == 0)
-		panic_timeout = ghes_panic_timeout;
-	panic("Fatal hardware error!");
+	__ghes_call_panic();
 }
 
 static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH V11 07/10] efi: print unrecognized CPER section
  2017-02-21 21:21 [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar
                   ` (5 preceding siblings ...)
  2017-02-21 21:21 ` [PATCH V11 06/10] acpi: apei: panic OS with fatal error status block Tyler Baicar
@ 2017-02-21 21:21 ` Tyler Baicar
  2017-02-21 21:21 ` [PATCH V11 08/10] ras: acpi / apei: generate trace event for " Tyler Baicar
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 29+ messages in thread
From: Tyler Baicar @ 2017-02-21 21:21 UTC (permalink / raw)
  To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, james.morse, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe
  Cc: Tyler Baicar

UEFI spec allows for non-standard section in Common Platform Error
Record. This is defined in section N.2.3 of UEFI version 2.5.

Currently if the CPER section's type (UUID) does not match with
one of the section types that the kernel knows how to parse, the
section is skipped. Therefore, user is not able to see
such CPER data, for instance, error record of non-standard section.

For above mentioned case, this change prints out the raw data in
hex in dmesg buffer. Data length is taken from Error Data length
field of Generic Error Data Entry.

Following is a sample output from dmesg:
[  115.771702] {1}[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 2
[  115.779042] {1}[Hardware Error]: It has been corrected by h/w and requires no further action
[  115.787456] {1}[Hardware Error]: event severity: corrected
[  115.792927] {1}[Hardware Error]:  Error 0, type: corrected
[  115.798415] {1}[Hardware Error]:  fru_id: 00000000-0000-0000-0000-000000000000
[  115.805596] {1}[Hardware Error]:  fru_text:
[  115.816105] {1}[Hardware Error]:  section type: d2e2621c-f936-468d-0d84-15a4ed015c8b
[  115.823880] {1}[Hardware Error]:  section length: 88
[  115.828779] {1}[Hardware Error]:   00000000: 01000001 00000002 5f434345 525f4543
[  115.836153] {1}[Hardware Error]:   00000010: 0000574d 00000000 00000000 00000000
[  115.843531] {1}[Hardware Error]:   00000020: 00000000 00000000 00000000 00000000
[  115.850908] {1}[Hardware Error]:   00000030: 00000000 00000000 00000000 00000000
[  115.858288] {1}[Hardware Error]:   00000040: fe800000 00000000 00000004 5f434345
[  115.865665] {1}[Hardware Error]:   00000050: 525f4543 0000574d

The raw data from the error can then be decoded using vendor
specific tools.

Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
Reviewed-by: James Morse <james.morse@arm.com>
---
 drivers/firmware/efi/cper.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index 0238877..d1b6edc 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -591,8 +591,16 @@ static void cper_estatus_print_section(
 			cper_print_proc_arm(newpfx, arm_err);
 		else
 			goto err_section_too_small;
-	} else
+	} else {
+		const void *unknown_err;
+
+		unknown_err = acpi_hest_generic_data_payload(gdata);
 		printk("%s""section type: unknown, %pUl\n", newpfx, sec_type);
+		printk("%s""section length: %d\n", newpfx,
+		       gdata->error_data_length);
+		print_hex_dump(newpfx, "", DUMP_PREFIX_OFFSET, 16, 4,
+			       unknown_err, gdata->error_data_length, true);
+	}
 
 	return;
 
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH V11 08/10] ras: acpi / apei: generate trace event for unrecognized CPER section
  2017-02-21 21:21 [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar
                   ` (6 preceding siblings ...)
  2017-02-21 21:21 ` [PATCH V11 07/10] efi: print unrecognized CPER section Tyler Baicar
@ 2017-02-21 21:21 ` Tyler Baicar
  2017-02-21 21:22 ` [PATCH V11 09/10] trace, ras: add ARM processor error trace event Tyler Baicar
  2017-02-21 21:22 ` [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support Tyler Baicar
  9 siblings, 0 replies; 29+ messages in thread
From: Tyler Baicar @ 2017-02-21 21:21 UTC (permalink / raw)
  To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, james.morse, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe
  Cc: Tyler Baicar

UEFI spec allows for non-standard section in Common Platform Error
Record. This is defined in section N.2.3 of UEFI version 2.5.

Currently if the CPER section's type (UUID) does not match with
any section type that the kernel knows how to parse, trace event
is not generated for such section. And thus user is not able to know
happening of such hardware error, including error record of
non-standard section.

This commit generates a trace event which contains raw error data
for unrecognized CPER section.

Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
---
 drivers/acpi/apei/ghes.c | 24 ++++++++++++++++++++++--
 drivers/ras/ras.c        |  1 +
 include/ras/ras_event.h  | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index d6a3b9f..842c0cc 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -44,11 +44,13 @@
 #include <linux/pci.h>
 #include <linux/aer.h>
 #include <linux/nmi.h>
+#include <linux/uuid.h>
 
 #include <acpi/actbl1.h>
 #include <acpi/ghes.h>
 #include <acpi/apei.h>
 #include <asm/tlbflush.h>
+#include <ras/ras_event.h>
 
 #include "apei-internal.h"
 
@@ -453,11 +455,21 @@ static void ghes_do_proc(struct ghes *ghes,
 {
 	int sev, sec_sev;
 	struct acpi_hest_generic_data *gdata;
+	uuid_le sec_type;
+	uuid_le *fru_id = &NULL_UUID_LE;
+	char *fru_text = "";
 
 	sev = ghes_severity(estatus->error_severity);
 	apei_estatus_for_each_section(estatus, gdata) {
 		sec_sev = ghes_severity(gdata->error_severity);
-		if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
+		sec_type = *(uuid_le *)gdata->section_type;
+
+		if (gdata->validation_bits & CPER_SEC_VALID_FRU_ID)
+			fru_id = (uuid_le *)gdata->fru_id;
+		if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
+			fru_text = gdata->fru_text;
+
+		if (!uuid_le_cmp(sec_type,
 				 CPER_SEC_PLATFORM_MEM)) {
 			struct cper_sec_mem_err *mem_err;
 
@@ -468,7 +480,7 @@ static void ghes_do_proc(struct ghes *ghes,
 			ghes_handle_memory_failure(gdata, sev);
 		}
 #ifdef CONFIG_ACPI_APEI_PCIEAER
-		else if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
+		else if (!uuid_le_cmp(sec_type,
 				      CPER_SEC_PCIE)) {
 			struct cper_sec_pcie *pcie_err;
 
@@ -501,6 +513,14 @@ static void ghes_do_proc(struct ghes *ghes,
 
 		}
 #endif
+#ifdef CONFIG_RAS
+		else if (trace_unknown_sec_event_enabled()) {
+			void *unknown_err = acpi_hest_generic_data_payload(gdata);
+			trace_unknown_sec_event(&sec_type,
+					fru_id, fru_text, sec_sev,
+					unknown_err, gdata->error_data_length);
+		}
+#endif
 	}
 }
 
diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c
index b67dd36..fb2500b 100644
--- a/drivers/ras/ras.c
+++ b/drivers/ras/ras.c
@@ -27,3 +27,4 @@ static int __init ras_init(void)
 EXPORT_TRACEPOINT_SYMBOL_GPL(extlog_mem_event);
 #endif
 EXPORT_TRACEPOINT_SYMBOL_GPL(mc_event);
+EXPORT_TRACEPOINT_SYMBOL_GPL(unknown_sec_event);
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index 1791a12..5861b6f 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -162,6 +162,51 @@
 );
 
 /*
+ * Unknown Section Report
+ *
+ * This event is generated when hardware detected a hardware
+ * error event, which may be of non-standard section as defined
+ * in UEFI spec appendix "Common Platform Error Record", or may
+ * be of sections for which TRACE_EVENT is not defined.
+ *
+ */
+TRACE_EVENT(unknown_sec_event,
+
+	TP_PROTO(const uuid_le *sec_type,
+		 const uuid_le *fru_id,
+		 const char *fru_text,
+		 const u8 sev,
+		 const u8 *err,
+		 const u32 len),
+
+	TP_ARGS(sec_type, fru_id, fru_text, sev, err, len),
+
+	TP_STRUCT__entry(
+		__array(char, sec_type, 16)
+		__array(char, fru_id, 16)
+		__string(fru_text, fru_text)
+		__field(u8, sev)
+		__field(u32, len)
+		__dynamic_array(u8, buf, len)
+	),
+
+	TP_fast_assign(
+		memcpy(__entry->sec_type, sec_type, sizeof(uuid_le));
+		memcpy(__entry->fru_id, fru_id, sizeof(uuid_le));
+		__assign_str(fru_text, fru_text);
+		__entry->sev = sev;
+		__entry->len = len;
+		memcpy(__get_dynamic_array(buf), err, len);
+	),
+
+	TP_printk("severity: %d; sec type:%pU; FRU: %pU %s; data len:%d; raw data:%s",
+		  __entry->sev, __entry->sec_type,
+		  __entry->fru_id, __get_str(fru_text),
+		  __entry->len,
+		  __print_hex(__get_dynamic_array(buf), __entry->len))
+);
+
+/*
  * PCIe AER Trace event
  *
  * These events are generated when hardware detects a corrected or
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH V11 09/10] trace, ras: add ARM processor error trace event
  2017-02-21 21:21 [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar
                   ` (7 preceding siblings ...)
  2017-02-21 21:21 ` [PATCH V11 08/10] ras: acpi / apei: generate trace event for " Tyler Baicar
@ 2017-02-21 21:22 ` Tyler Baicar
  2017-02-21 21:22 ` [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support Tyler Baicar
  9 siblings, 0 replies; 29+ messages in thread
From: Tyler Baicar @ 2017-02-21 21:22 UTC (permalink / raw)
  To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, james.morse, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe
  Cc: Tyler Baicar

Currently there are trace events for the various RAS
errors with the exception of ARM processor type errors.
Add a new trace event for such errors so that the user
will know when they occur. These trace events are
consistent with the ARM processor error section type
defined in UEFI 2.6 spec section N.2.4.4.

Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
---
 drivers/acpi/apei/ghes.c    |  8 +++++++-
 drivers/firmware/efi/cper.c |  1 +
 drivers/ras/ras.c           |  1 +
 include/ras/ras_event.h     | 34 ++++++++++++++++++++++++++++++++++
 4 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 842c0cc..81d7b79 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -514,7 +514,13 @@ static void ghes_do_proc(struct ghes *ghes,
 		}
 #endif
 #ifdef CONFIG_RAS
-		else if (trace_unknown_sec_event_enabled()) {
+		else if (!uuid_le_cmp(sec_type, CPER_SEC_PROC_ARM) &&
+			 trace_arm_event_enabled()) {
+			struct cper_sec_proc_arm *arm_err;
+
+			arm_err = acpi_hest_generic_data_payload(gdata);
+			trace_arm_event(arm_err);
+		} else if (trace_unknown_sec_event_enabled()) {
 			void *unknown_err = acpi_hest_generic_data_payload(gdata);
 			trace_unknown_sec_event(&sec_type,
 					fru_id, fru_text, sec_sev,
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index d1b6edc..fe0136f 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -35,6 +35,7 @@
 #include <linux/printk.h>
 #include <linux/bcd.h>
 #include <acpi/ghes.h>
+#include <ras/ras_event.h>
 
 #define INDENT_SP	" "
 
diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c
index fb2500b..8ba5a94 100644
--- a/drivers/ras/ras.c
+++ b/drivers/ras/ras.c
@@ -28,3 +28,4 @@ static int __init ras_init(void)
 #endif
 EXPORT_TRACEPOINT_SYMBOL_GPL(mc_event);
 EXPORT_TRACEPOINT_SYMBOL_GPL(unknown_sec_event);
+EXPORT_TRACEPOINT_SYMBOL_GPL(arm_event);
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index 5861b6f..b36db48 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -162,6 +162,40 @@
 );
 
 /*
+ * ARM Processor Events Report
+ *
+ * This event is generated when hardware detects an ARM processor error
+ * has occurred. UEFI 2.6 spec section N.2.4.4.
+ */
+TRACE_EVENT(arm_event,
+
+	TP_PROTO(const struct cper_sec_proc_arm *proc),
+
+	TP_ARGS(proc),
+
+	TP_STRUCT__entry(
+		__field(u64, mpidr)
+		__field(u64, midr)
+		__field(u32, running_state)
+		__field(u32, psci_state)
+		__field(u8, affinity)
+	),
+
+	TP_fast_assign(
+		__entry->affinity = proc->affinity_level;
+		__entry->mpidr = proc->mpidr;
+		__entry->midr = proc->midr;
+		__entry->running_state = proc->running_state;
+		__entry->psci_state = proc->psci_state;
+	),
+
+	TP_printk("affinity level: %d; MPIDR: %016llx; MIDR: %016llx; "
+		  "running state: %d; PSCI state: %d",
+		  __entry->affinity, __entry->mpidr, __entry->midr,
+		  __entry->running_state, __entry->psci_state)
+);
+
+/*
  * Unknown Section Report
  *
  * This event is generated when hardware detected a hardware
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-21 21:21 [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar
                   ` (8 preceding siblings ...)
  2017-02-21 21:22 ` [PATCH V11 09/10] trace, ras: add ARM processor error trace event Tyler Baicar
@ 2017-02-21 21:22 ` Tyler Baicar
  2017-02-24 10:42   ` James Morse
  2017-02-25  7:15   ` Xiongfeng Wang
  9 siblings, 2 replies; 29+ messages in thread
From: Tyler Baicar @ 2017-02-21 21:22 UTC (permalink / raw)
  To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, james.morse, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe
  Cc: Tyler Baicar

Currently external aborts are unsupported by the guest abort
handling. Add handling for SEAs so that the host kernel reports
SEAs which occur in the guest kernel.

Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
---
 arch/arm/include/asm/kvm_arm.h       |  1 +
 arch/arm/include/asm/system_misc.h   |  5 +++++
 arch/arm/kvm/mmu.c                   | 18 ++++++++++++++++--
 arch/arm64/include/asm/kvm_arm.h     |  1 +
 arch/arm64/include/asm/system_misc.h |  2 ++
 arch/arm64/mm/fault.c                | 18 ++++++++++++++++++
 6 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
index e22089f..33a77509 100644
--- a/arch/arm/include/asm/kvm_arm.h
+++ b/arch/arm/include/asm/kvm_arm.h
@@ -187,6 +187,7 @@
 #define FSC_FAULT	(0x04)
 #define FSC_ACCESS	(0x08)
 #define FSC_PERM	(0x0c)
+#define FSC_EXTABT	(0x10)
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK	(~0xf)
diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h
index a3d61ad..ea45d94 100644
--- a/arch/arm/include/asm/system_misc.h
+++ b/arch/arm/include/asm/system_misc.h
@@ -24,4 +24,9 @@
 
 #endif /* !__ASSEMBLY__ */
 
+static inline int handle_guest_sea(unsigned long addr, unsigned int esr)
+{
+	return -1;
+}
+
 #endif /* __ASM_ARM_SYSTEM_MISC_H */
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
index a5265ed..04f1dd50 100644
--- a/arch/arm/kvm/mmu.c
+++ b/arch/arm/kvm/mmu.c
@@ -29,6 +29,7 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_emulate.h>
 #include <asm/virt.h>
+#include <asm/system_misc.h>
 
 #include "trace.h"
 
@@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
 
 	/* Check the stage-2 fault is trans. fault or write fault */
 	fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
-	if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
-	    fault_status != FSC_ACCESS) {
+
+	/* The host kernel will handle the synchronous external abort. There
+	 * is no need to pass the error into the guest.
+	 */
+	if (fault_status == FSC_EXTABT) {
+		if(handle_guest_sea((unsigned long)fault_ipa,
+				    kvm_vcpu_get_hsr(vcpu))) {
+			kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
+				kvm_vcpu_trap_get_class(vcpu),
+				(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
+				(unsigned long)kvm_vcpu_get_hsr(vcpu));
+			return -EFAULT;
+		}
+	} else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
+		   fault_status != FSC_ACCESS) {
 		kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
 			kvm_vcpu_trap_get_class(vcpu),
 			(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 2a2752b..2b11d59 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -201,6 +201,7 @@
 #define FSC_FAULT	ESR_ELx_FSC_FAULT
 #define FSC_ACCESS	ESR_ELx_FSC_ACCESS
 #define FSC_PERM	ESR_ELx_FSC_PERM
+#define FSC_EXTABT	ESR_ELx_FSC_EXTABT
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK	(~UL(0xf))
diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
index bc81243..5b2cecd1 100644
--- a/arch/arm64/include/asm/system_misc.h
+++ b/arch/arm64/include/asm/system_misc.h
@@ -58,4 +58,6 @@ void hook_debug_fault_code(int nr, int (*fn)(unsigned long, unsigned int,
 
 #endif	/* __ASSEMBLY__ */
 
+int handle_guest_sea(unsigned long addr, unsigned int esr);
+
 #endif	/* __ASM_SYSTEM_MISC_H */
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index b2d57fc..403277b 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
 }
 
 /*
+ * Handle Synchronous External Aborts that occur in a guest kernel.
+ */
+int handle_guest_sea(unsigned long addr, unsigned int esr)
+{
+	/*
+	 * synchronize_rcu() will wait for nmi_exit(), so no need to
+	 * rcu_read_lock().
+	 */
+	if(IS_ENABLED(HAVE_ACPI_APEI_SEA)) {
+		nmi_enter();
+		ghes_notify_sea();
+		nmi_exit();
+	}
+
+	return 0;
+}
+
+/*
  * Dispatch a data abort to the relevant handler.
  */
 asmlinkage void __exception do_mem_abort(unsigned long addr, unsigned int esr,
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-21 21:22 ` [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support Tyler Baicar
@ 2017-02-24 10:42   ` James Morse
  2017-02-27 11:31     ` gengdongjiu
  2017-02-28 19:43     ` Baicar, Tyler
  2017-02-25  7:15   ` Xiongfeng Wang
  1 sibling, 2 replies; 29+ messages in thread
From: James Morse @ 2017-02-24 10:42 UTC (permalink / raw)
  To: Tyler Baicar
  Cc: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, akpm, eun.taik.lee,
	sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe

Hi Tyler,

On 21/02/17 21:22, Tyler Baicar wrote:
> Currently external aborts are unsupported by the guest abort
> handling. Add handling for SEAs so that the host kernel reports
> SEAs which occur in the guest kernel.

> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index e22089f..33a77509 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -187,6 +187,7 @@
>  #define FSC_FAULT	(0x04)
>  #define FSC_ACCESS	(0x08)
>  #define FSC_PERM	(0x0c)
> +#define FSC_EXTABT	(0x10)

arm64 has ESR_ELx_FSC_EXTABT which is used in inject_abt64(), but for matching
an external abort coming from hardware the range is wider.

Looking at the ARM-ARMs 'ISS encoding for an exception from an Instruction
Abort' in 'D7.2.27 ESR_ELx, Exception Syndrome Register (ELx)' (page D7-1954 of
version 'k'...iss10775), the ten flavours of you Synchronous abort you hooked
with do_sea() in patch 4 occupy 0x10 to 0x1f...


> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index a5265ed..04f1dd50 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -29,6 +29,7 @@
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/virt.h>
> +#include <asm/system_misc.h>
>  
>  #include "trace.h"
>  
> @@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  
>  	/* Check the stage-2 fault is trans. fault or write fault */
>  	fault_status = kvm_vcpu_trap_get_fault_type(vcpu);

... kvm_vcpu_trap_get_fault_type() on both arm and arm64 masks the HSR/ESR_EL2
with 0x3c ...


> -	if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> -	    fault_status != FSC_ACCESS) {
> +
> +	/* The host kernel will handle the synchronous external abort. There
> +	 * is no need to pass the error into the guest.
> +	 */
> +	if (fault_status == FSC_EXTABT) {

... but here we only check for 'Synchronous external abort, not on a translation
table walk'. Are the other types relevant?

If so we need some helper as this range is sparse and 'all other values are
reserved'. The aarch32 HSR format is slightly different. (G6-4411 ISS encoding
from an exception from a Data Abort).

If not, can we change patch 4 to check this type too so we don't call out to
APEI for a fault type we know isn't relevant.


> +		if(handle_guest_sea((unsigned long)fault_ipa,
> +				    kvm_vcpu_get_hsr(vcpu))) {
> +			kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
> +				kvm_vcpu_trap_get_class(vcpu),
> +				(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> +				(unsigned long)kvm_vcpu_get_hsr(vcpu));
> +			return -EFAULT;
> +		}
> +	} else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> +		   fault_status != FSC_ACCESS) {
>  		kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>  			kvm_vcpu_trap_get_class(vcpu),
>  			(unsigned long)kvm_vcpu_trap_get_fault(vcpu),

> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index b2d57fc..403277b 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
>  }
>
>  /*
> + * Handle Synchronous External Aborts that occur in a guest kernel.
> + */
> +int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{

> +	if(IS_ENABLED(HAVE_ACPI_APEI_SEA)) {
> +		nmi_enter();
> +		ghes_notify_sea();
> +		nmi_exit();

This nmi stuff was needed for synchronous aborts that may have interrupted
APEI's interrupts-masked code. We want to avoid trying to take the same set of
locks, hence taking the in_nmi() path through APEI. Here we know we interrupted
a guest, so there is no risk that we have interrupted APEI on the host.
ghes_notify_sea() can safely take the normal path.


Thanks,

James

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-21 21:22 ` [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support Tyler Baicar
  2017-02-24 10:42   ` James Morse
@ 2017-02-25  7:15   ` Xiongfeng Wang
  2017-02-27 13:58     ` James Morse
  1 sibling, 1 reply; 29+ messages in thread
From: Xiongfeng Wang @ 2017-02-25  7:15 UTC (permalink / raw)
  To: Tyler Baicar, christoffer.dall, marc.zyngier, pbonzini, rkrcmar,
	linux, catalin.marinas, will.deacon, rjw, lenb, matt,
	robert.moore, lv.zheng, nkaje, zjzhang, mark.rutland,
	james.morse, akpm, eun.taik.lee, sandeepa.s.prabhu, labbott,
	shijie.huang, rruigrok, paul.gortmaker, tn, fu.wei, rostedt,
	bristot, linux-arm-kernel, kvmarm, kvm, linux-kernel, linux-acpi,
	linux-efi, devel, Suzuki.Poulose, punit.agrawal, astone, harba,
	hanjun.guo, john.garry, shiju.jose, joe

Hi Tyler,


On 2017/2/22 5:22, Tyler Baicar wrote:
> Currently external aborts are unsupported by the guest abort
> handling. Add handling for SEAs so that the host kernel reports
> SEAs which occur in the guest kernel.
> 
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> ---
>  arch/arm/include/asm/kvm_arm.h       |  1 +
>  arch/arm/include/asm/system_misc.h   |  5 +++++
>  arch/arm/kvm/mmu.c                   | 18 ++++++++++++++++--
>  arch/arm64/include/asm/kvm_arm.h     |  1 +
>  arch/arm64/include/asm/system_misc.h |  2 ++
>  arch/arm64/mm/fault.c                | 18 ++++++++++++++++++
>  6 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index e22089f..33a77509 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -187,6 +187,7 @@
>  #define FSC_FAULT	(0x04)
>  #define FSC_ACCESS	(0x08)
>  #define FSC_PERM	(0x0c)
> +#define FSC_EXTABT	(0x10)
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK	(~0xf)
> diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h
> index a3d61ad..ea45d94 100644
> --- a/arch/arm/include/asm/system_misc.h
> +++ b/arch/arm/include/asm/system_misc.h
> @@ -24,4 +24,9 @@
>  
>  #endif /* !__ASSEMBLY__ */
>  
> +static inline int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{
> +	return -1;
> +}
> +
>  #endif /* __ASM_ARM_SYSTEM_MISC_H */
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index a5265ed..04f1dd50 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -29,6 +29,7 @@
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/virt.h>
> +#include <asm/system_misc.h>
>  
>  #include "trace.h"
>  
> @@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  
>  	/* Check the stage-2 fault is trans. fault or write fault */
>  	fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
> -	if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> -	    fault_status != FSC_ACCESS) {
> +
> +	/* The host kernel will handle the synchronous external abort. There
> +	 * is no need to pass the error into the guest.
> +	 */

Can we inject an sea into the guest, so that the guest can kill the
application which causes the error if the guest won't be terminated
later. I'm not sure whether ghes_handle_memory_failure() called in
ghes_do_proc() will kill the qemu process. I think it only kill user
processes marked with PF_MCE_PROCESS & PF_MCE_EARLY.

> +	if (fault_status == FSC_EXTABT) {
> +		if(handle_guest_sea((unsigned long)fault_ipa,
> +				    kvm_vcpu_get_hsr(vcpu))) {
> +			kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
> +				kvm_vcpu_trap_get_class(vcpu),
> +				(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> +				(unsigned long)kvm_vcpu_get_hsr(vcpu));
> +			return -EFAULT;
> +		}
> +	} else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> +		   fault_status != FSC_ACCESS) {
>  		kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>  			kvm_vcpu_trap_get_class(vcpu),
>  			(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 2a2752b..2b11d59 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -201,6 +201,7 @@
>  #define FSC_FAULT	ESR_ELx_FSC_FAULT
>  #define FSC_ACCESS	ESR_ELx_FSC_ACCESS
>  #define FSC_PERM	ESR_ELx_FSC_PERM
> +#define FSC_EXTABT	ESR_ELx_FSC_EXTABT
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK	(~UL(0xf))
> diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
> index bc81243..5b2cecd1 100644
> --- a/arch/arm64/include/asm/system_misc.h
> +++ b/arch/arm64/include/asm/system_misc.h
> @@ -58,4 +58,6 @@ void hook_debug_fault_code(int nr, int (*fn)(unsigned long, unsigned int,
>  
>  #endif	/* __ASSEMBLY__ */
>  
> +int handle_guest_sea(unsigned long addr, unsigned int esr);
> +
>  #endif	/* __ASM_SYSTEM_MISC_H */
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index b2d57fc..403277b 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
>  }
>  
>  /*
> + * Handle Synchronous External Aborts that occur in a guest kernel.
> + */
> +int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{
> +	/*
> +	 * synchronize_rcu() will wait for nmi_exit(), so no need to
> +	 * rcu_read_lock().
> +	 */
> +	if(IS_ENABLED(HAVE_ACPI_APEI_SEA)) {
> +		nmi_enter();
> +		ghes_notify_sea();
> +		nmi_exit();
> +	}
> +
> +	return 0;
> +}
> +
> +/*
>   * Dispatch a data abort to the relevant handler.
>   */
>  asmlinkage void __exception do_mem_abort(unsigned long addr, unsigned int esr,
> 

Thanks,
Wang Xiongfeng

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-24 10:42   ` James Morse
@ 2017-02-27 11:31     ` gengdongjiu
  2017-02-28 19:43     ` Baicar, Tyler
  1 sibling, 0 replies; 29+ messages in thread
From: gengdongjiu @ 2017-02-27 11:31 UTC (permalink / raw)
  To: James Morse
  Cc: Tyler Baicar, christoffer.dall, Marc Zyngier, pbonzini, rkrcmar,
	linux, catalin.marinas, will.deacon, rjw, lenb, matt,
	robert.moore, lv.zheng, nkaje, zjzhang, mark.rutland, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe, gengdongjiu

@@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu
*vcpu, struct kvm_run *run)

        /* Check the stage-2 fault is trans. fault or write fault */
        fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
-       if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
-           fault_status != FSC_ACCESS) {
+
+       /* The host kernel will handle the synchronous external abort. There
+        * is no need to pass the error into the guest.
+        */
+       if (fault_status == FSC_EXTABT) {
+               if(handle_guest_sea((unsigned long)fault_ipa,
+                                   kvm_vcpu_get_hsr(vcpu))) {
+                       kvm_err("Failed to handle guest SEA, FSC:
EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
+                               kvm_vcpu_trap_get_class(vcpu),
+                               (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
+                               (unsigned long)kvm_vcpu_get_hsr(vcpu));
+                       return -EFAULT;
+               }
+       } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
+                  fault_status != FSC_ACCESS) {
                kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
                        kvm_vcpu_trap_get_class(vcpu),
                        (unsigned long)kvm_vcpu_trap_get_fault(vcpu),



if the error is SEA and we want to inject the sea to guest OK, after
finish the handle, whether we need to directly return? instead of
continuation? as shown below:

       if (fault_status == FSC_EXTABT) {
               if(handle_guest_sea((unsigned long)fault_ipa,
                                   kvm_vcpu_get_hsr(vcpu))) {
                       kvm_err("Failed to handle guest SEA, FSC:
EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
                               kvm_vcpu_trap_get_class(vcpu),
                               (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
                               (unsigned long)kvm_vcpu_get_hsr(vcpu));
                       return -EFAULT;
          } else
                       return 1;






2017-02-24 18:42 GMT+08:00 James Morse <james.morse@arm.com>:
> Hi Tyler,
>
> On 21/02/17 21:22, Tyler Baicar wrote:
>> Currently external aborts are unsupported by the guest abort
>> handling. Add handling for SEAs so that the host kernel reports
>> SEAs which occur in the guest kernel.
>
>> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
>> index e22089f..33a77509 100644
>> --- a/arch/arm/include/asm/kvm_arm.h
>> +++ b/arch/arm/include/asm/kvm_arm.h
>> @@ -187,6 +187,7 @@
>>  #define FSC_FAULT    (0x04)
>>  #define FSC_ACCESS   (0x08)
>>  #define FSC_PERM     (0x0c)
>> +#define FSC_EXTABT   (0x10)
>
> arm64 has ESR_ELx_FSC_EXTABT which is used in inject_abt64(), but for matching
> an external abort coming from hardware the range is wider.
>
> Looking at the ARM-ARMs 'ISS encoding for an exception from an Instruction
> Abort' in 'D7.2.27 ESR_ELx, Exception Syndrome Register (ELx)' (page D7-1954 of
> version 'k'...iss10775), the ten flavours of you Synchronous abort you hooked
> with do_sea() in patch 4 occupy 0x10 to 0x1f...
>
>
>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
>> index a5265ed..04f1dd50 100644
>> --- a/arch/arm/kvm/mmu.c
>> +++ b/arch/arm/kvm/mmu.c
>> @@ -29,6 +29,7 @@
>>  #include <asm/kvm_asm.h>
>>  #include <asm/kvm_emulate.h>
>>  #include <asm/virt.h>
>> +#include <asm/system_misc.h>
>>
>>  #include "trace.h"
>>
>> @@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>
>>       /* Check the stage-2 fault is trans. fault or write fault */
>>       fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
>
> ... kvm_vcpu_trap_get_fault_type() on both arm and arm64 masks the HSR/ESR_EL2
> with 0x3c ...
>
>
>> -     if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
>> -         fault_status != FSC_ACCESS) {
>> +
>> +     /* The host kernel will handle the synchronous external abort. There
>> +      * is no need to pass the error into the guest.
>> +      */
>> +     if (fault_status == FSC_EXTABT) {
>
> ... but here we only check for 'Synchronous external abort, not on a translation
> table walk'. Are the other types relevant?
>
> If so we need some helper as this range is sparse and 'all other values are
> reserved'. The aarch32 HSR format is slightly different. (G6-4411 ISS encoding
> from an exception from a Data Abort).
>
> If not, can we change patch 4 to check this type too so we don't call out to
> APEI for a fault type we know isn't relevant.
>
>
>> +             if(handle_guest_sea((unsigned long)fault_ipa,
>> +                                 kvm_vcpu_get_hsr(vcpu))) {
>> +                     kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>> +                             kvm_vcpu_trap_get_class(vcpu),
>> +                             (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
>> +                             (unsigned long)kvm_vcpu_get_hsr(vcpu));
>> +                     return -EFAULT;
>> +             }
>> +     } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
>> +                fault_status != FSC_ACCESS) {
>>               kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>>                       kvm_vcpu_trap_get_class(vcpu),
>>                       (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
>
>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>> index b2d57fc..403277b 100644
>> --- a/arch/arm64/mm/fault.c
>> +++ b/arch/arm64/mm/fault.c
>> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
>>  }
>>
>>  /*
>> + * Handle Synchronous External Aborts that occur in a guest kernel.
>> + */
>> +int handle_guest_sea(unsigned long addr, unsigned int esr)
>> +{
>
>> +     if(IS_ENABLED(HAVE_ACPI_APEI_SEA)) {
>> +             nmi_enter();
>> +             ghes_notify_sea();
>> +             nmi_exit();
>
> This nmi stuff was needed for synchronous aborts that may have interrupted
> APEI's interrupts-masked code. We want to avoid trying to take the same set of
> locks, hence taking the in_nmi() path through APEI. Here we know we interrupted
> a guest, so there is no risk that we have interrupted APEI on the host.
> ghes_notify_sea() can safely take the normal path.
>
>
> Thanks,
>
> James

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-25  7:15   ` Xiongfeng Wang
@ 2017-02-27 13:58     ` James Morse
  2017-02-28  6:25       ` Xiongfeng Wang
                         ` (2 more replies)
  0 siblings, 3 replies; 29+ messages in thread
From: James Morse @ 2017-02-27 13:58 UTC (permalink / raw)
  To: Xiongfeng Wang, punit.agrawal
  Cc: Tyler Baicar, christoffer.dall, marc.zyngier, pbonzini, rkrcmar,
	linux, catalin.marinas, will.deacon, rjw, lenb, matt,
	robert.moore, lv.zheng, nkaje, zjzhang, mark.rutland, akpm,
	eun.taik.lee, sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, astone, harba, hanjun.guo, john.garry,
	shiju.jose, joe

Hi Wang Xiongfeng,

On 25/02/17 07:15, Xiongfeng Wang wrote:
> On 2017/2/22 5:22, Tyler Baicar wrote:
>> Currently external aborts are unsupported by the guest abort
>> handling. Add handling for SEAs so that the host kernel reports
>> SEAs which occur in the guest kernel.

>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
>> index a5265ed..04f1dd50 100644
>> --- a/arch/arm/kvm/mmu.c
>> +++ b/arch/arm/kvm/mmu.c
>> @@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>  
>>  	/* Check the stage-2 fault is trans. fault or write fault */
>>  	fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
>> -	if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
>> -	    fault_status != FSC_ACCESS) {
>> +
>> +	/* The host kernel will handle the synchronous external abort. There
>> +	 * is no need to pass the error into the guest.
>> +	 */

> Can we inject an sea into the guest, so that the guest can kill the
> application which causes the error if the guest won't be terminated
> later. I'm not sure whether ghes_handle_memory_failure() called in
> ghes_do_proc() will kill the qemu process. I think it only kill user
> processes marked with PF_MCE_PROCESS & PF_MCE_EARLY.

My understanding is the pages will get unmapped and recovered where possible
(e.g. re-read from disk), the user space process will get SIGBUS/SIGSEV when it
next tries to access that page, which could be some time later.
These flags in find_early_kill_thread() are a way to make the memory-failure
code signal the process early, before it does any recovery. The 'MCE' makes me
think its x86 specific.
(early and late are described more in [0])


Guests are a special case as QEMU may never access the faulty memory itself, so
it won't receive the 'late' signal. It looks like ARM/arm64 KVM lacks support
for KVM_PFN_ERR_HWPOISON which sends SIGBUS from KVM's fault-handling code. I
have patches to add support for this which I intend to send at rc1.

[0] suggests 'KVM qemu' sets these MCE flags to take the 'early' path, but given
x86s KVM_PFN_ERR_HWPOISON, this may be out of date.


Either way, once QEMU gets a signal indicating the virtual address, it can
generate its own APEI CPER records and use the KVM APIs to mock up an
Synchronous External Abort, (or inject an IRQ or run the vcpu waiting for the
guest's polling thread to come round, whichever was described to the guest via
the HEST/GHES tables).

We can't hand the APEI CPER records we have in the kernel to the guest, as they
hold a host physical address, and maybe a host virtual address. We don't know
where in guest memory we could write new APEI CPER records as these locations
have to be reserved in the guests-UEFI memory map, and only QEMU knows where
they are.

To deliver RAS events to a guest we have to get QEMU involved.


Thanks,

James


[0] https://www.kernel.org/doc/Documentation/vm/hwpoison.txt

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-27 13:58     ` James Morse
@ 2017-02-28  6:25       ` Xiongfeng Wang
  2017-02-28 13:21         ` James Morse
  2017-03-06  1:28       ` gengdongjiu
  2017-03-22  2:46       ` Xiongfeng Wang
  2 siblings, 1 reply; 29+ messages in thread
From: Xiongfeng Wang @ 2017-02-28  6:25 UTC (permalink / raw)
  To: James Morse, punit.agrawal
  Cc: mark.rutland, linux-efi, kvm, rkrcmar, matt, catalin.marinas,
	Tyler Baicar, will.deacon, robert.moore, paul.gortmaker,
	lv.zheng, kvmarm, fu.wei, tn, zjzhang, linux, linux-acpi,
	eun.taik.lee, shijie.huang, labbott, lenb, harba, Suzuki.Poulose,
	marc.zyngier, john.garry, rostedt, nkaje, sandeepa.s.prabhu,
	linux-arm-kernel, devel, rjw, rruigrok, linux-kernel, astone,
	hanjun.guo, joe, pbonzini, akpm, bristot, christoffer.dall,
	shiju.jose

Hi James,

On 2017/2/27 21:58, James Morse wrote:
> Hi Wang Xiongfeng,
> 
> On 25/02/17 07:15, Xiongfeng Wang wrote:
>> On 2017/2/22 5:22, Tyler Baicar wrote:
>>> Currently external aborts are unsupported by the guest abort
>>> handling. Add handling for SEAs so that the host kernel reports
>>> SEAs which occur in the guest kernel.
> 
>>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
>>> index a5265ed..04f1dd50 100644
>>> --- a/arch/arm/kvm/mmu.c
>>> +++ b/arch/arm/kvm/mmu.c
>>> @@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>>  
>>>  	/* Check the stage-2 fault is trans. fault or write fault */
>>>  	fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
>>> -	if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
>>> -	    fault_status != FSC_ACCESS) {
>>> +
>>> +	/* The host kernel will handle the synchronous external abort. There
>>> +	 * is no need to pass the error into the guest.
>>> +	 */
> 
>> Can we inject an sea into the guest, so that the guest can kill the
>> application which causes the error if the guest won't be terminated
>> later. I'm not sure whether ghes_handle_memory_failure() called in
>> ghes_do_proc() will kill the qemu process. I think it only kill user
>> processes marked with PF_MCE_PROCESS & PF_MCE_EARLY.
> 
> My understanding is the pages will get unmapped and recovered where possible
> (e.g. re-read from disk), the user space process will get SIGBUS/SIGSEV when it
> next tries to access that page, which could be some time later.
> These flags in find_early_kill_thread() are a way to make the memory-failure
> code signal the process early, before it does any recovery. The 'MCE' makes me
> think its x86 specific.
> (early and late are described more in [0])
> 
> 
> Guests are a special case as QEMU may never access the faulty memory itself, so
> it won't receive the 'late' signal. It looks like ARM/arm64 KVM lacks support
> for KVM_PFN_ERR_HWPOISON which sends SIGBUS from KVM's fault-handling code. I
> have patches to add support for this which I intend to send at rc1.
> 
> [0] suggests 'KVM qemu' sets these MCE flags to take the 'early' path, but given
> x86s KVM_PFN_ERR_HWPOISON, this may be out of date.
> 
> 
> Either way, once QEMU gets a signal indicating the virtual address, it can
> generate its own APEI CPER records and use the KVM APIs to mock up an
> Synchronous External Abort, (or inject an IRQ or run the vcpu waiting for the
> guest's polling thread to come round, whichever was described to the guest via
> the HEST/GHES tables).
> 
> We can't hand the APEI CPER records we have in the kernel to the guest, as they
> hold a host physical address, and maybe a host virtual address. We don't know
> where in guest memory we could write new APEI CPER records as these locations
> have to be reserved in the guests-UEFI memory map, and only QEMU knows where
> they are.
> 
> To deliver RAS events to a guest we have to get QEMU involved.

Thanks for your reply!

I have another idea about the handling procedure of SEA. Can we divide
the SEA handing procedure into two procedures? The first procedure does
the more urgent work, including sending SIGBUS to user process or panic,
just as PATCH 04/10 does. The second procedure does the APEI analysis
work, including calling memory_failure. The second procedure is executed
when actual errors detected in memory, such as a 2-bit ECC error is
detected  on memory read or write, in which case, a fault handling
interrupt is generated by the memory controller, as RAS Extension
specification says.

We can route this fault handling interrupt into EL3. After BIOS has
filled the HEST table, it can notify OS with an IRQ. And the second
procedure is executed in the IRQ handler. The notification type of
HEST/GHES tables is GSIV.

When uncorrectable data error is detected on write data, a fault
handling interrupt is generated, and no SEA is generated, as RAS
extension specification 6.4.4 says. In this situation, the second
procedure should be executed since error occurs in memory.

In ARM/arm64 KVM situation, when an SEA takes place, an SEA is injected
into guest os directly in kvm_handle_guest_abort(). And the guest os can
execute the first procedure.

When the host OS executes the second procedure and analyses the HEST
table, it sends SIGBUS to qemu process in memory_failure(). And the qemu
process can mock up a HEST table with IPA of the error data. Then the
qemu process can notify the guest OS with an IRQ, and the second
procedure is executed in guest OS. Is this idea reasonable?


Thanks!
Wang Xiongfeng

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-28  6:25       ` Xiongfeng Wang
@ 2017-02-28 13:21         ` James Morse
  2017-03-01  2:31           ` Xiongfeng Wang
  0 siblings, 1 reply; 29+ messages in thread
From: James Morse @ 2017-02-28 13:21 UTC (permalink / raw)
  To: Xiongfeng Wang
  Cc: punit.agrawal, mark.rutland, linux-efi, kvm, rkrcmar, matt,
	catalin.marinas, Tyler Baicar, will.deacon, linux,
	paul.gortmaker, lv.zheng, kvmarm, fu.wei, tn, zjzhang,
	robert.moore, linux-acpi, eun.taik.lee, shijie.huang, labbott,
	lenb, harba, Suzuki.Poulose, marc.zyngier, john.garry, rostedt,
	nkaje, sandeepa.s.prabhu, linux-arm-kernel, devel, rjw, rruigrok,
	linux-kernel, astone, hanjun.guo, joe, pbonzini, akpm, bristot,
	christoffer.dall, shiju.jose

Hi,

On 28/02/17 06:25, Xiongfeng Wang wrote:
> On 2017/2/27 21:58, James Morse wrote:
>> On 25/02/17 07:15, Xiongfeng Wang wrote:
>>> Can we inject an sea into the guest, so that the guest can kill the
>>> application which causes the error if the guest won't be terminated
>>> later. I'm not sure whether ghes_handle_memory_failure() called in
>>> ghes_do_proc() will kill the qemu process. I think it only kill user
>>> processes marked with PF_MCE_PROCESS & PF_MCE_EARLY.
>>
>> My understanding is the pages will get unmapped and recovered where possible
>> (e.g. re-read from disk), the user space process will get SIGBUS/SIGSEV when it
>> next tries to access that page, which could be some time later.
>> These flags in find_early_kill_thread() are a way to make the memory-failure
>> code signal the process early, before it does any recovery. The 'MCE' makes me
>> think its x86 specific.
>> (early and late are described more in [0])
>>
>>
>> Guests are a special case as QEMU may never access the faulty memory itself, so
>> it won't receive the 'late' signal. It looks like ARM/arm64 KVM lacks support
>> for KVM_PFN_ERR_HWPOISON which sends SIGBUS from KVM's fault-handling code. I
>> have patches to add support for this which I intend to send at rc1.
>>
>> [0] suggests 'KVM qemu' sets these MCE flags to take the 'early' path, but given
>> x86s KVM_PFN_ERR_HWPOISON, this may be out of date.
>>
>>
>> Either way, once QEMU gets a signal indicating the virtual address, it can
>> generate its own APEI CPER records and use the KVM APIs to mock up an
>> Synchronous External Abort, (or inject an IRQ or run the vcpu waiting for the
>> guest's polling thread to come round, whichever was described to the guest via
>> the HEST/GHES tables).
>>
>> We can't hand the APEI CPER records we have in the kernel to the guest, as they
>> hold a host physical address, and maybe a host virtual address. We don't know
>> where in guest memory we could write new APEI CPER records as these locations
>> have to be reserved in the guests-UEFI memory map, and only QEMU knows where
>> they are.
>>
>> To deliver RAS events to a guest we have to get QEMU involved.

> I have another idea about the handling procedure of SEA. Can we divide
> the SEA handing procedure into two procedures? The first procedure does
> the more urgent work, including sending SIGBUS to user process or panic,
> just as PATCH 04/10 does.

How do we know which user processes to signal? (There may be more than one - we
need a memory address to find them).
How do we know if this error is serious and we should panic?
This information is in the APEI CPER records.


> The second procedure does the APEI analysis
> work, including calling memory_failure. The second procedure is executed
> when actual errors detected in memory, such as a 2-bit ECC error is
> detected  on memory read or write, in which case, a fault handling
> interrupt is generated by the memory controller, as RAS Extension
> specification says.

You are splitting the APEI notification and the processing of records. One has
to happen immediately after the other because we want to contain the error.


> We can route this fault handling interrupt into EL3. After BIOS has
> filled the HEST table, it can notify OS with an IRQ. And the second
> procedure is executed in the IRQ handler. The notification type of
> HEST/GHES tables is GSIV.
> 
> When uncorrectable data error is detected on write data, a fault
> handling interrupt is generated, and no SEA is generated,

This sounds more like ACPI's firmware first error handling. Yes errors should be
routed to EL3 where firmware can do some platform-specific work, then describe
them to the host OS via CPER records.
By doing this, you could prevent a hardware-generated External Abort reaching
the host OS, but you still need to notify the OS via one of the mechanisms in
'18.3.2.9 Hardware Error Notification'.

If the error is synchronous (we read a bad page of memory instead of it being
detected on background DRAM scrub) we need to notify the OS synchronously. Using
SEA would be a firmware-generated External Abort delivered to EL2/EL1.

However the notification is done it needs to match one of the GHES records in
the HEST, so firmware has to decide which notification methods it will use
before we boot the OS.


> In ARM/arm64 KVM situation, when an SEA takes place, an SEA is injected
> into guest os directly in kvm_handle_guest_abort(). And the guest os can
> execute the first procedure.

What can the guest do with this? Without the APEI CPER records it doesn't know
what happened. Was it unrecoverable memory corruption? In which case killing the
running task is a start... Which memory ranges should we mark as unusable? Maybe
it was something more catastrophic for the running CPU, in which case we should
panic().


> When the host OS executes the second procedure and analyses the HEST
> table, it sends SIGBUS to qemu process in memory_failure(). And the qemu
> process can mock up a HEST table with IPA of the error data. Then the
> qemu process can notify the guest OS with an IRQ, and the second
> procedure is executed in guest OS. Is this idea reasonable?

So we tell the guest something happened, and it should wait a while to find out
what... I don't think this will work. It is best to not run the guest until Qemu
has done its work and called VCPU_RUN again. This way we only notify the guest
once the records are available for processing. This is how APEI's firmware-first
works between the host OS and EL3, it should be the same between a guest and
QEMU (which plays the part of firmware for the guest).


Can you share more details of the problem you are trying to solve? I don't think
we can get RAS support in a guest 'for free', somewhere along the line we need
support from Qemu.


Thanks,

James

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-24 10:42   ` James Morse
  2017-02-27 11:31     ` gengdongjiu
@ 2017-02-28 19:43     ` Baicar, Tyler
  2017-03-06 10:28       ` James Morse
  1 sibling, 1 reply; 29+ messages in thread
From: Baicar, Tyler @ 2017-02-28 19:43 UTC (permalink / raw)
  To: James Morse
  Cc: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, akpm, eun.taik.lee,
	sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe

Hello James,


On 2/24/2017 3:42 AM, James Morse wrote:
> On 21/02/17 21:22, Tyler Baicar wrote:
>> Currently external aborts are unsupported by the guest abort
>> handling. Add handling for SEAs so that the host kernel reports
>> SEAs which occur in the guest kernel.
>> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
>> index e22089f..33a77509 100644
>> --- a/arch/arm/include/asm/kvm_arm.h
>> +++ b/arch/arm/include/asm/kvm_arm.h
>> @@ -187,6 +187,7 @@
>>   #define FSC_FAULT	(0x04)
>>   #define FSC_ACCESS	(0x08)
>>   #define FSC_PERM	(0x0c)
>> +#define FSC_EXTABT	(0x10)
> arm64 has ESR_ELx_FSC_EXTABT which is used in inject_abt64(), but for matching
> an external abort coming from hardware the range is wider.
>
> Looking at the ARM-ARMs 'ISS encoding for an exception from an Instruction
> Abort' in 'D7.2.27 ESR_ELx, Exception Syndrome Register (ELx)' (page D7-1954 of
> version 'k'...iss10775), the ten flavours of you Synchronous abort you hooked
> with do_sea() in patch 4 occupy 0x10 to 0x1f...
>
>
>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
>> index a5265ed..04f1dd50 100644
>> --- a/arch/arm/kvm/mmu.c
>> +++ b/arch/arm/kvm/mmu.c
>> @@ -29,6 +29,7 @@
>>   #include <asm/kvm_asm.h>
>>   #include <asm/kvm_emulate.h>
>>   #include <asm/virt.h>
>> +#include <asm/system_misc.h>
>>   
>>   #include "trace.h"
>>   
>> @@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>   
>>   	/* Check the stage-2 fault is trans. fault or write fault */
>>   	fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
> ... kvm_vcpu_trap_get_fault_type() on both arm and arm64 masks the HSR/ESR_EL2
> with 0x3c ...
>
>
>> -	if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
>> -	    fault_status != FSC_ACCESS) {
>> +
>> +	/* The host kernel will handle the synchronous external abort. There
>> +	 * is no need to pass the error into the guest.
>> +	 */
>> +	if (fault_status == FSC_EXTABT) {
> ... but here we only check for 'Synchronous external abort, not on a translation
> table walk'. Are the other types relevant?
I believe the others should be relevant here as well. I haven't been 
able to test the other types within a guest though.
> If so we need some helper as this range is sparse and 'all other values are
> reserved'. The aarch32 HSR format is slightly different. (G6-4411 ISS encoding
> from an exception from a Data Abort).
I can add a helper so that this if statement matches any of the 10 FSC 
values which are mapped to the do_sea in the host code.
> If not, can we change patch 4 to check this type too so we don't call out to
> APEI for a fault type we know isn't relevant.
>
>
>> +		if(handle_guest_sea((unsigned long)fault_ipa,
>> +				    kvm_vcpu_get_hsr(vcpu))) {
>> +			kvm_err("Failed to handle guest SEA, FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>> +				kvm_vcpu_trap_get_class(vcpu),
>> +				(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
>> +				(unsigned long)kvm_vcpu_get_hsr(vcpu));
>> +			return -EFAULT;
>> +		}
>> +	} else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
>> +		   fault_status != FSC_ACCESS) {
>>   		kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>>   			kvm_vcpu_trap_get_class(vcpu),
>>   			(unsigned long)kvm_vcpu_trap_get_fault(vcpu),
>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>> index b2d57fc..403277b 100644
>> --- a/arch/arm64/mm/fault.c
>> +++ b/arch/arm64/mm/fault.c
>> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
>>   }
>>
>>   /*
>> + * Handle Synchronous External Aborts that occur in a guest kernel.
>> + */
>> +int handle_guest_sea(unsigned long addr, unsigned int esr)
>> +{
>> +	if(IS_ENABLED(HAVE_ACPI_APEI_SEA)) {
>> +		nmi_enter();
>> +		ghes_notify_sea();
>> +		nmi_exit();
> This nmi stuff was needed for synchronous aborts that may have interrupted
> APEI's interrupts-masked code. We want to avoid trying to take the same set of
> locks, hence taking the in_nmi() path through APEI. Here we know we interrupted
> a guest, so there is no risk that we have interrupted APEI on the host.
> ghes_notify_sea() can safely take the normal path.
Makes sense, I can remove the nmi_* calls here.

Thanks,
Tyler

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-28 13:21         ` James Morse
@ 2017-03-01  2:31           ` Xiongfeng Wang
  2017-03-02  9:39             ` Marc Zyngier
  0 siblings, 1 reply; 29+ messages in thread
From: Xiongfeng Wang @ 2017-03-01  2:31 UTC (permalink / raw)
  To: James Morse
  Cc: punit.agrawal, mark.rutland, linux-efi, kvm, rkrcmar, matt,
	catalin.marinas, Tyler Baicar, will.deacon, linux,
	paul.gortmaker, lv.zheng, kvmarm, fu.wei, tn, zjzhang,
	robert.moore, linux-acpi, eun.taik.lee, shijie.huang, labbott,
	lenb, harba, Suzuki.Poulose, marc.zyngier, john.garry, rostedt,
	nkaje, sandeepa.s.prabhu, linux-arm-kernel, devel, rjw, rruigrok,
	linux-kernel, astone, hanjun.guo, joe, pbonzini, akpm, bristot,
	christoffer.dall, shiju.jose

Hi James,

On 2017/2/28 21:21, James Morse wrote:
> Hi,
> 
> On 28/02/17 06:25, Xiongfeng Wang wrote:
>> On 2017/2/27 21:58, James Morse wrote:
>>> On 25/02/17 07:15, Xiongfeng Wang wrote:
>>>> Can we inject an sea into the guest, so that the guest can kill the
>>>> application which causes the error if the guest won't be terminated
>>>> later. I'm not sure whether ghes_handle_memory_failure() called in
>>>> ghes_do_proc() will kill the qemu process. I think it only kill user
>>>> processes marked with PF_MCE_PROCESS & PF_MCE_EARLY.
>>>
>>> My understanding is the pages will get unmapped and recovered where possible
>>> (e.g. re-read from disk), the user space process will get SIGBUS/SIGSEV when it
>>> next tries to access that page, which could be some time later.
>>> These flags in find_early_kill_thread() are a way to make the memory-failure
>>> code signal the process early, before it does any recovery. The 'MCE' makes me
>>> think its x86 specific.
>>> (early and late are described more in [0])
>>>
>>>
>>> Guests are a special case as QEMU may never access the faulty memory itself, so
>>> it won't receive the 'late' signal. It looks like ARM/arm64 KVM lacks support
>>> for KVM_PFN_ERR_HWPOISON which sends SIGBUS from KVM's fault-handling code. I
>>> have patches to add support for this which I intend to send at rc1.
>>>
>>> [0] suggests 'KVM qemu' sets these MCE flags to take the 'early' path, but given
>>> x86s KVM_PFN_ERR_HWPOISON, this may be out of date.
>>>
>>>
>>> Either way, once QEMU gets a signal indicating the virtual address, it can
>>> generate its own APEI CPER records and use the KVM APIs to mock up an
>>> Synchronous External Abort, (or inject an IRQ or run the vcpu waiting for the
>>> guest's polling thread to come round, whichever was described to the guest via
>>> the HEST/GHES tables).
>>>
>>> We can't hand the APEI CPER records we have in the kernel to the guest, as they
>>> hold a host physical address, and maybe a host virtual address. We don't know
>>> where in guest memory we could write new APEI CPER records as these locations
>>> have to be reserved in the guests-UEFI memory map, and only QEMU knows where
>>> they are.
>>>
>>> To deliver RAS events to a guest we have to get QEMU involved.
> 
>> I have another idea about the handling procedure of SEA. Can we divide
>> the SEA handing procedure into two procedures? The first procedure does
>> the more urgent work, including sending SIGBUS to user process or panic,
>> just as PATCH 04/10 does.
> 
> How do we know which user processes to signal? (There may be more than one - we
> need a memory address to find them).
> How do we know if this error is serious and we should panic?
> This information is in the APEI CPER records.
> 
Since the SEA exception is synchronous, the current user process is the
one to be signaled if the exception is taken from EL0. Certainly, the
error memory may be mapped to several processes. When another process
read that area again, another SEA will be generated, and that process
will be signaled. Also we can get the virtual address of the error data
from FAR_EL1. When the user process is signaled, virtual address is
attached, and the process can register its own signal handler if it can
handle the error according to the virtual address of the error data.

We can determine where the exception is taken from according to CPSR
stored in the stack. And if the exception is taken from EL1, the error
is in the kernel space now, and we are going to consume it, so we need
to panic now.
> 
>> The second procedure does the APEI analysis
>> work, including calling memory_failure. The second procedure is executed
>> when actual errors detected in memory, such as a 2-bit ECC error is
>> detected  on memory read or write, in which case, a fault handling
>> interrupt is generated by the memory controller, as RAS Extension
>> specification says.
> 
> You are splitting the APEI notification and the processing of records. One has
> to happen immediately after the other because we want to contain the error.
> 
My understanding is that processing of records is not so urgent since
the process access the error data has been killed (The first procedure
is executed in SEA exception handler). Other codes won't access the
error data, so the error won't be consumed and propagated.
> 
>> We can route this fault handling interrupt into EL3. After BIOS has
>> filled the HEST table, it can notify OS with an IRQ. And the second
>> procedure is executed in the IRQ handler. The notification type of
>> HEST/GHES tables is GSIV.
>>
>> When uncorrectable data error is detected on write data, a fault
>> handling interrupt is generated, and no SEA is generated,
> 
> This sounds more like ACPI's firmware first error handling. Yes errors should be
> routed to EL3 where firmware can do some platform-specific work, then describe
> them to the host OS via CPER records.

Yes , I'm saying the ACPI's firmware first error handling.

> By doing this, you could prevent a hardware-generated External Abort reaching
> the host OS, but you still need to notify the OS via one of the mechanisms in
> '18.3.2.9 Hardware Error Notification'.

Yes, the BIOS will notify OS with GSIV notify type, which will rely on
Shiju's patch 'acpi: apei: handle GSIV notification type'

> 
> If the error is synchronous (we read a bad page of memory instead of it being
> detected on background DRAM scrub) we need to notify the OS synchronously. Using
> SEA would be a firmware-generated External Abort delivered to EL2/EL1.

Yes, the first procedure is executed in SEA exception handler and is
synchronous. The second procedure won't access the error data and is not
so urgent, so it may not need to be synchronous.
> 
> However the notification is done it needs to match one of the GHES records in
> the HEST, so firmware has to decide which notification methods it will use
> before we boot the OS.
> 
> 
>> In ARM/arm64 KVM situation, when an SEA takes place, an SEA is injected
>> into guest os directly in kvm_handle_guest_abort(). And the guest os can
>> execute the first procedure.
> 
> What can the guest do with this? Without the APEI CPER records it doesn't know
> what happened. Was it unrecoverable memory corruption? In which case killing the
> running task is a start... Which memory ranges should we mark as unusable? Maybe
> it was something more catastrophic for the running CPU, in which case we should
> panic().
> 
If an SEA is injected into guest OS, the guest OS will jump to the SEA
exception entry when the context switched to guest OS. And the CPSR and
FAR_EL1 are recovered according to the content of vcpu. Then the guest
OS can signal a process or panic. If another guest process read the
error data, another SEA will be generated and it will be single too.

Without QEMU involved, the drawback is that no APEI table can be mocked
up in guest OS, and no memory_failure() will be called. So the memory of
error data will be released into buddy system and assigned to another
process. If the error was caused by instantaneous radiation or
electromagnetic, the memory is usable again if it is written with a
correct data. If the memory has wore out and a correct data is written,
the ECC error may occurs again with high possibility. Before a 2-bit ECC
error is reported, much more 1-bit errors will be reported. This is
report to host os, the host os can determine the memory node has worn
out and hot-plug out the memory node, and guest os may be terminated if
its memory data can't be migrated.

Of course, it is better to get QEMU involved, so the memory_failure can
be executed in guest OS. But before that implemented, can we add SEA
injection in kvm_handle_guest_abort()?
> 
>> When the host OS executes the second procedure and analyses the HEST
>> table, it sends SIGBUS to qemu process in memory_failure(). And the qemu
>> process can mock up a HEST table with IPA of the error data. Then the
>> qemu process can notify the guest OS with an IRQ, and the second
>> procedure is executed in guest OS. Is this idea reasonable?
> 
> So we tell the guest something happened, and it should wait a while to find out
> what... I don't think this will work. It is best to not run the guest until Qemu
> has done its work and called VCPU_RUN again. This way we only notify the guest
> once the records are available for processing. This is how APEI's firmware-first
> works between the host OS and EL3, it should be the same between a guest and
> QEMU (which plays the part of firmware for the guest).
> 
> 
> Can you share more details of the problem you are trying to solve? I don't think
> we can get RAS support in a guest 'for free', somewhere along the line we need
> support from Qemu.
> 


Thanks,

Wang Xiongfeng

.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 05/10] acpi: apei: handle SEA notification type for ARMv8
  2017-02-21 21:21 ` [PATCH V11 05/10] acpi: apei: handle SEA notification type for ARMv8 Tyler Baicar
@ 2017-03-01  7:42   ` Xie XiuQi
  2017-03-01 19:22     ` Baicar, Tyler
  0 siblings, 1 reply; 29+ messages in thread
From: Xie XiuQi @ 2017-03-01  7:42 UTC (permalink / raw)
  To: Tyler Baicar, christoffer.dall, marc.zyngier, pbonzini, rkrcmar,
	linux, catalin.marinas, will.deacon, rjw, lenb, matt,
	robert.moore, lv.zheng, nkaje, zjzhang, mark.rutland,
	james.morse, akpm, eun.taik.lee, sandeepa.s.prabhu, labbott,
	shijie.huang, rruigrok, paul.gortmaker, tn, fu.wei, rostedt,
	bristot, linux-arm-kernel, kvmarm, kvm, linux-kernel, linux-acpi,
	linux-efi, devel, Suzuki.Poulose, punit.agrawal, astone, harba,
	hanjun.guo, john.garry, shiju.jose, joe

Hi Tyler,

On 2017/2/22 5:21, Tyler Baicar wrote:
> ARM APEI extension proposal added SEA (Synchronous External Abort)
> notification type for ARMv8.
> Add a new GHES error source handling function for SEA. If an error
> source's notification type is SEA, then this function can be registered
> into the SEA exception handler. That way GHES will parse and report
> SEA exceptions when they occur.

I have a question about ghes_proc. In ghes_proc, we just parse and report
the error information, but no one use it for error recovery now.

Take the SEA case for example, we get the physical address from parsing
the GHES table. But the memory management system or other drivers/modules
know what the really meaning of the error address/page. There is no way to
notify them to do the recovery now.

So, could we add a notify at appropriate position. All drivers or modules
which are interested in this error could receive and take the corresponding
action.

---
Thanks,
Xie XiuQi


> An SEA can interrupt code that had interrupts masked and is treated as
> an NMI. To aid this the page of address space for mapping APEI buffers
> while in_nmi() is always reserved, and ghes_ioremap_pfn_nmi() is
> changed to use the helper methods to find the prot_t to map with in
> the same way as ghes_ioremap_pfn_irq().
> 
> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
> ---
>  arch/arm64/Kconfig        |  1 +
>  arch/arm64/mm/fault.c     | 13 ++++++++
>  drivers/acpi/apei/Kconfig | 15 +++++++++
>  drivers/acpi/apei/ghes.c  | 77 +++++++++++++++++++++++++++++++++++++++++++----
>  include/acpi/ghes.h       |  7 +++++
>  5 files changed, 107 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1117421..fca4dc1 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -88,6 +88,7 @@ config ARM64
>  	select HAVE_IRQ_TIME_ACCOUNTING
>  	select HAVE_MEMBLOCK
>  	select HAVE_MEMBLOCK_NODE_MAP if NUMA
> +	select HAVE_NMI if ACPI_APEI_SEA
>  	select HAVE_PATA_PLATFORM
>  	select HAVE_PERF_EVENTS
>  	select HAVE_PERF_REGS
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index d178dc0..b2d57fc 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -41,6 +41,8 @@
>  #include <asm/pgtable.h>
>  #include <asm/tlbflush.h>
>  
> +#include <acpi/ghes.h>
> +
>  static const char *fault_name(unsigned int esr);
>  
>  #ifdef CONFIG_KPROBES
> @@ -498,6 +500,17 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
>  	pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
>  		 fault_name(esr), esr, addr);
>  
> +	/*
> +	 * Synchronous aborts may interrupt code which had interrupts masked.
> +	 * Before calling out into the wider kernel tell the interested
> +	 * subsystems.
> +	 */
> +	if (IS_ENABLED(ACPI_APEI_SEA)) {
> +		nmi_enter();
> +		ghes_notify_sea();
> +		nmi_exit();
> +	}
> +
>  	info.si_signo = SIGBUS;
>  	info.si_errno = 0;
>  	info.si_code  = 0;
> diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
> index b0140c8..c545dd1 100644
> --- a/drivers/acpi/apei/Kconfig
> +++ b/drivers/acpi/apei/Kconfig
> @@ -39,6 +39,21 @@ config ACPI_APEI_PCIEAER
>  	  PCIe AER errors may be reported via APEI firmware first mode.
>  	  Turn on this option to enable the corresponding support.
>  
> +config ACPI_APEI_SEA
> +	bool "APEI Synchronous External Abort logging/recovering support"
> +	depends on ARM64 && ACPI_APEI && 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"
>  	depends on ACPI_APEI && MEMORY_FAILURE
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index b25e7cf..b0596ba 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -114,11 +114,7 @@
>   * Two virtual pages are used, one for IRQ/PROCESS context, the other for
>   * NMI context (optionally).
>   */
> -#ifdef CONFIG_HAVE_ACPI_APEI_NMI
>  #define GHES_IOREMAP_PAGES           2
> -#else
> -#define GHES_IOREMAP_PAGES           1
> -#endif
>  #define GHES_IOREMAP_IRQ_PAGE(base)	(base)
>  #define GHES_IOREMAP_NMI_PAGE(base)	((base) + PAGE_SIZE)
>  
> @@ -157,10 +153,14 @@ static void ghes_ioremap_exit(void)
>  static void __iomem *ghes_ioremap_pfn_nmi(u64 pfn)
>  {
>  	unsigned long vaddr;
> +	phys_addr_t paddr;
> +	pgprot_t prot;
>  
>  	vaddr = (unsigned long)GHES_IOREMAP_NMI_PAGE(ghes_ioremap_area->addr);
> -	ioremap_page_range(vaddr, vaddr + PAGE_SIZE,
> -			   pfn << PAGE_SHIFT, PAGE_KERNEL);
> +
> +	paddr = pfn << PAGE_SHIFT;
> +	prot = arch_apei_get_mem_attribute(paddr);
> +	ioremap_page_range(vaddr, vaddr + PAGE_SIZE, paddr, prot);
>  
>  	return (void __iomem *)vaddr;
>  }
> @@ -767,6 +767,50 @@ static int ghes_notify_sci(struct notifier_block *this,
>  	.notifier_call = ghes_notify_sci,
>  };
>  
> +#ifdef CONFIG_ACPI_APEI_SEA
> +static LIST_HEAD(ghes_sea);
> +
> +void ghes_notify_sea(void)
> +{
> +	struct ghes *ghes;
> +
> +	/*
> +	 * synchronize_rcu() will wait for nmi_exit(), so no need to
> +	 * rcu_read_lock().
> +	 */
> +	list_for_each_entry_rcu(ghes, &ghes_sea, list) {
> +		ghes_proc(ghes);
> +	}
> +}
> +
> +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)
> +{
> +	pr_err(GHES_PFX "ID: %d, trying to add SEA notification which is not supported\n",
> +	       ghes->generic->header.source_id);
> +}
> +
> +static inline void ghes_sea_remove(struct ghes *ghes)
> +{
> +	pr_err(GHES_PFX "ID: %d, trying to remove SEA notification which is not supported\n",
> +	       ghes->generic->header.source_id);
> +}
> +#endif /* CONFIG_ACPI_APEI_SEA */
> +
>  #ifdef CONFIG_HAVE_ACPI_APEI_NMI
>  /*
>   * printk is not safe in NMI context.  So in NMI handler, we allocate
> @@ -1012,6 +1056,14 @@ static int ghes_probe(struct platform_device *ghes_dev)
>  	case ACPI_HEST_NOTIFY_EXTERNAL:
>  	case ACPI_HEST_NOTIFY_SCI:
>  		break;
> +	case ACPI_HEST_NOTIFY_SEA:
> +		if (!IS_ENABLED(CONFIG_ACPI_APEI_SEA)) {
> +			pr_warn(GHES_PFX "Generic hardware error source: %d notified via SEA is not supported\n",
> +				generic->header.source_id);
> +			rc = -ENOTSUPP;
> +			goto err;
> +		}
> +		break;
>  	case ACPI_HEST_NOTIFY_NMI:
>  		if (!IS_ENABLED(CONFIG_HAVE_ACPI_APEI_NMI)) {
>  			pr_warn(GHES_PFX "Generic hardware error source: %d notified via NMI interrupt is not supported!\n",
> @@ -1023,6 +1075,13 @@ static int ghes_probe(struct platform_device *ghes_dev)
>  		pr_warning(GHES_PFX "Generic hardware error source: %d notified via local interrupt is not supported!\n",
>  			   generic->header.source_id);
>  		goto err;
> +	case ACPI_HEST_NOTIFY_GPIO:
> +	case ACPI_HEST_NOTIFY_SEI:
> +	case ACPI_HEST_NOTIFY_GSIV:
> +		pr_warn(GHES_PFX "Generic hardware error source: %d notified via notification type %u is not supported\n",
> +			generic->header.source_id, generic->header.source_id);
> +		rc = -ENOTSUPP;
> +		goto err;
>  	default:
>  		pr_warning(FW_WARN GHES_PFX "Unknown notification type: %u for generic hardware error source: %d\n",
>  			   generic->notify.type, generic->header.source_id);
> @@ -1077,6 +1136,9 @@ static int ghes_probe(struct platform_device *ghes_dev)
>  		list_add_rcu(&ghes->list, &ghes_sci);
>  		mutex_unlock(&ghes_list_mutex);
>  		break;
> +	case ACPI_HEST_NOTIFY_SEA:
> +		ghes_sea_add(ghes);
> +		break;
>  	case ACPI_HEST_NOTIFY_NMI:
>  		ghes_nmi_add(ghes);
>  		break;
> @@ -1119,6 +1181,9 @@ static int ghes_remove(struct platform_device *ghes_dev)
>  			unregister_acpi_hed_notifier(&ghes_notifier_sci);
>  		mutex_unlock(&ghes_list_mutex);
>  		break;
> +	case ACPI_HEST_NOTIFY_SEA:
> +		ghes_sea_remove(ghes);
> +		break;
>  	case ACPI_HEST_NOTIFY_NMI:
>  		ghes_nmi_remove(ghes);
>  		break;
> diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
> index 6ae318b..18bc935 100644
> --- a/include/acpi/ghes.h
> +++ b/include/acpi/ghes.h
> @@ -1,3 +1,6 @@
> +#ifndef GHES_H
> +#define GHES_H
> +
>  #include <acpi/apei.h>
>  #include <acpi/hed.h>
>  
> @@ -95,3 +98,7 @@ static inline void *acpi_hest_generic_data_payload(struct acpi_hest_generic_data
>  		(void *)(((struct acpi_hest_generic_data_v300 *)(gdata)) + 1) :
>  		gdata + 1;
>  }
> +
> +void ghes_notify_sea(void);
> +
> +#endif /* GHES_H */
> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 05/10] acpi: apei: handle SEA notification type for ARMv8
  2017-03-01  7:42   ` Xie XiuQi
@ 2017-03-01 19:22     ` Baicar, Tyler
  0 siblings, 0 replies; 29+ messages in thread
From: Baicar, Tyler @ 2017-03-01 19:22 UTC (permalink / raw)
  To: Xie XiuQi, christoffer.dall, marc.zyngier, pbonzini, rkrcmar,
	linux, catalin.marinas, will.deacon, rjw, lenb, matt,
	robert.moore, lv.zheng, nkaje, zjzhang, mark.rutland,
	james.morse, akpm, eun.taik.lee, sandeepa.s.prabhu, labbott,
	shijie.huang, rruigrok, paul.gortmaker, tn, fu.wei, rostedt,
	bristot, linux-arm-kernel, kvmarm, kvm, linux-kernel, linux-acpi,
	linux-efi, devel, Suzuki.Poulose, punit.agrawal, astone, harba,
	hanjun.guo, john.garry, shiju.jose, joe

Hello Xie XiuQi,


On 3/1/2017 12:42 AM, Xie XiuQi wrote:
> Hi Tyler,
>
> On 2017/2/22 5:21, Tyler Baicar wrote:
>> ARM APEI extension proposal added SEA (Synchronous External Abort)
>> notification type for ARMv8.
>> Add a new GHES error source handling function for SEA. If an error
>> source's notification type is SEA, then this function can be registered
>> into the SEA exception handler. That way GHES will parse and report
>> SEA exceptions when they occur.
> I have a question about ghes_proc. In ghes_proc, we just parse and report
> the error information, but no one use it for error recovery now.
>
> Take the SEA case for example, we get the physical address from parsing
> the GHES table. But the memory management system or other drivers/modules
> know what the really meaning of the error address/page. There is no way to
> notify them to do the recovery now.
>
> So, could we add a notify at appropriate position. All drivers or modules
> which are interested in this error could receive and take the corresponding
> action.
Error recovery is outside the scope of these patches. These patches are 
supposed to setup the infrastructure to parse/report the SEAs. Error 
recovery can be added after the fact which is what has been done for 
platform memory errors; the page off-lining support was added after the 
error parsing/reporting code was in.

Thanks,
Tyler
>
>> An SEA can interrupt code that had interrupts masked and is treated as
>> an NMI. To aid this the page of address space for mapping APEI buffers
>> while in_nmi() is always reserved, and ghes_ioremap_pfn_nmi() is
>> changed to use the helper methods to find the prot_t to map with in
>> the same way as ghes_ioremap_pfn_irq().
>>
>> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
>> CC: Jonathan (Zhixiong) Zhang <zjzhang@codeaurora.org>
>> ---
>>   arch/arm64/Kconfig        |  1 +
>>   arch/arm64/mm/fault.c     | 13 ++++++++
>>   drivers/acpi/apei/Kconfig | 15 +++++++++
>>   drivers/acpi/apei/ghes.c  | 77 +++++++++++++++++++++++++++++++++++++++++++----
>>   include/acpi/ghes.h       |  7 +++++
>>   5 files changed, 107 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 1117421..fca4dc1 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -88,6 +88,7 @@ config ARM64
>>   	select HAVE_IRQ_TIME_ACCOUNTING
>>   	select HAVE_MEMBLOCK
>>   	select HAVE_MEMBLOCK_NODE_MAP if NUMA
>> +	select HAVE_NMI if ACPI_APEI_SEA
>>   	select HAVE_PATA_PLATFORM
>>   	select HAVE_PERF_EVENTS
>>   	select HAVE_PERF_REGS
>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>> index d178dc0..b2d57fc 100644
>> --- a/arch/arm64/mm/fault.c
>> +++ b/arch/arm64/mm/fault.c
>> @@ -41,6 +41,8 @@
>>   #include <asm/pgtable.h>
>>   #include <asm/tlbflush.h>
>>   
>> +#include <acpi/ghes.h>
>> +
>>   static const char *fault_name(unsigned int esr);
>>   
>>   #ifdef CONFIG_KPROBES
>> @@ -498,6 +500,17 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
>>   	pr_err("Synchronous External Abort: %s (0x%08x) at 0x%016lx\n",
>>   		 fault_name(esr), esr, addr);
>>   
>> +	/*
>> +	 * Synchronous aborts may interrupt code which had interrupts masked.
>> +	 * Before calling out into the wider kernel tell the interested
>> +	 * subsystems.
>> +	 */
>> +	if (IS_ENABLED(ACPI_APEI_SEA)) {
>> +		nmi_enter();
>> +		ghes_notify_sea();
>> +		nmi_exit();
>> +	}
>> +
>>   	info.si_signo = SIGBUS;
>>   	info.si_errno = 0;
>>   	info.si_code  = 0;
>> diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
>> index b0140c8..c545dd1 100644
>> --- a/drivers/acpi/apei/Kconfig
>> +++ b/drivers/acpi/apei/Kconfig
>> @@ -39,6 +39,21 @@ config ACPI_APEI_PCIEAER
>>   	  PCIe AER errors may be reported via APEI firmware first mode.
>>   	  Turn on this option to enable the corresponding support.
>>   
>> +config ACPI_APEI_SEA
>> +	bool "APEI Synchronous External Abort logging/recovering support"
>> +	depends on ARM64 && ACPI_APEI && 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"
>>   	depends on ACPI_APEI && MEMORY_FAILURE
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index b25e7cf..b0596ba 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -114,11 +114,7 @@
>>    * Two virtual pages are used, one for IRQ/PROCESS context, the other for
>>    * NMI context (optionally).
>>    */
>> -#ifdef CONFIG_HAVE_ACPI_APEI_NMI
>>   #define GHES_IOREMAP_PAGES           2
>> -#else
>> -#define GHES_IOREMAP_PAGES           1
>> -#endif
>>   #define GHES_IOREMAP_IRQ_PAGE(base)	(base)
>>   #define GHES_IOREMAP_NMI_PAGE(base)	((base) + PAGE_SIZE)
>>   
>> @@ -157,10 +153,14 @@ static void ghes_ioremap_exit(void)
>>   static void __iomem *ghes_ioremap_pfn_nmi(u64 pfn)
>>   {
>>   	unsigned long vaddr;
>> +	phys_addr_t paddr;
>> +	pgprot_t prot;
>>   
>>   	vaddr = (unsigned long)GHES_IOREMAP_NMI_PAGE(ghes_ioremap_area->addr);
>> -	ioremap_page_range(vaddr, vaddr + PAGE_SIZE,
>> -			   pfn << PAGE_SHIFT, PAGE_KERNEL);
>> +
>> +	paddr = pfn << PAGE_SHIFT;
>> +	prot = arch_apei_get_mem_attribute(paddr);
>> +	ioremap_page_range(vaddr, vaddr + PAGE_SIZE, paddr, prot);
>>   
>>   	return (void __iomem *)vaddr;
>>   }
>> @@ -767,6 +767,50 @@ static int ghes_notify_sci(struct notifier_block *this,
>>   	.notifier_call = ghes_notify_sci,
>>   };
>>   
>> +#ifdef CONFIG_ACPI_APEI_SEA
>> +static LIST_HEAD(ghes_sea);
>> +
>> +void ghes_notify_sea(void)
>> +{
>> +	struct ghes *ghes;
>> +
>> +	/*
>> +	 * synchronize_rcu() will wait for nmi_exit(), so no need to
>> +	 * rcu_read_lock().
>> +	 */
>> +	list_for_each_entry_rcu(ghes, &ghes_sea, list) {
>> +		ghes_proc(ghes);
>> +	}
>> +}
>> +
>> +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)
>> +{
>> +	pr_err(GHES_PFX "ID: %d, trying to add SEA notification which is not supported\n",
>> +	       ghes->generic->header.source_id);
>> +}
>> +
>> +static inline void ghes_sea_remove(struct ghes *ghes)
>> +{
>> +	pr_err(GHES_PFX "ID: %d, trying to remove SEA notification which is not supported\n",
>> +	       ghes->generic->header.source_id);
>> +}
>> +#endif /* CONFIG_ACPI_APEI_SEA */
>> +
>>   #ifdef CONFIG_HAVE_ACPI_APEI_NMI
>>   /*
>>    * printk is not safe in NMI context.  So in NMI handler, we allocate
>> @@ -1012,6 +1056,14 @@ static int ghes_probe(struct platform_device *ghes_dev)
>>   	case ACPI_HEST_NOTIFY_EXTERNAL:
>>   	case ACPI_HEST_NOTIFY_SCI:
>>   		break;
>> +	case ACPI_HEST_NOTIFY_SEA:
>> +		if (!IS_ENABLED(CONFIG_ACPI_APEI_SEA)) {
>> +			pr_warn(GHES_PFX "Generic hardware error source: %d notified via SEA is not supported\n",
>> +				generic->header.source_id);
>> +			rc = -ENOTSUPP;
>> +			goto err;
>> +		}
>> +		break;
>>   	case ACPI_HEST_NOTIFY_NMI:
>>   		if (!IS_ENABLED(CONFIG_HAVE_ACPI_APEI_NMI)) {
>>   			pr_warn(GHES_PFX "Generic hardware error source: %d notified via NMI interrupt is not supported!\n",
>> @@ -1023,6 +1075,13 @@ static int ghes_probe(struct platform_device *ghes_dev)
>>   		pr_warning(GHES_PFX "Generic hardware error source: %d notified via local interrupt is not supported!\n",
>>   			   generic->header.source_id);
>>   		goto err;
>> +	case ACPI_HEST_NOTIFY_GPIO:
>> +	case ACPI_HEST_NOTIFY_SEI:
>> +	case ACPI_HEST_NOTIFY_GSIV:
>> +		pr_warn(GHES_PFX "Generic hardware error source: %d notified via notification type %u is not supported\n",
>> +			generic->header.source_id, generic->header.source_id);
>> +		rc = -ENOTSUPP;
>> +		goto err;
>>   	default:
>>   		pr_warning(FW_WARN GHES_PFX "Unknown notification type: %u for generic hardware error source: %d\n",
>>   			   generic->notify.type, generic->header.source_id);
>> @@ -1077,6 +1136,9 @@ static int ghes_probe(struct platform_device *ghes_dev)
>>   		list_add_rcu(&ghes->list, &ghes_sci);
>>   		mutex_unlock(&ghes_list_mutex);
>>   		break;
>> +	case ACPI_HEST_NOTIFY_SEA:
>> +		ghes_sea_add(ghes);
>> +		break;
>>   	case ACPI_HEST_NOTIFY_NMI:
>>   		ghes_nmi_add(ghes);
>>   		break;
>> @@ -1119,6 +1181,9 @@ static int ghes_remove(struct platform_device *ghes_dev)
>>   			unregister_acpi_hed_notifier(&ghes_notifier_sci);
>>   		mutex_unlock(&ghes_list_mutex);
>>   		break;
>> +	case ACPI_HEST_NOTIFY_SEA:
>> +		ghes_sea_remove(ghes);
>> +		break;
>>   	case ACPI_HEST_NOTIFY_NMI:
>>   		ghes_nmi_remove(ghes);
>>   		break;
>> diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
>> index 6ae318b..18bc935 100644
>> --- a/include/acpi/ghes.h
>> +++ b/include/acpi/ghes.h
>> @@ -1,3 +1,6 @@
>> +#ifndef GHES_H
>> +#define GHES_H
>> +
>>   #include <acpi/apei.h>
>>   #include <acpi/hed.h>
>>   
>> @@ -95,3 +98,7 @@ static inline void *acpi_hest_generic_data_payload(struct acpi_hest_generic_data
>>   		(void *)(((struct acpi_hest_generic_data_v300 *)(gdata)) + 1) :
>>   		gdata + 1;
>>   }
>> +
>> +void ghes_notify_sea(void);
>> +
>> +#endif /* GHES_H */
>>

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-03-01  2:31           ` Xiongfeng Wang
@ 2017-03-02  9:39             ` Marc Zyngier
  2017-03-06  3:38               ` Xiongfeng Wang
  0 siblings, 1 reply; 29+ messages in thread
From: Marc Zyngier @ 2017-03-02  9:39 UTC (permalink / raw)
  To: Xiongfeng Wang, James Morse
  Cc: punit.agrawal, mark.rutland, linux-efi, kvm, rkrcmar, matt,
	catalin.marinas, Tyler Baicar, will.deacon, linux,
	paul.gortmaker, lv.zheng, kvmarm, fu.wei, tn, zjzhang,
	robert.moore, linux-acpi, eun.taik.lee, shijie.huang, labbott,
	lenb, harba, Suzuki.Poulose, john.garry, rostedt, nkaje,
	sandeepa.s.prabhu, linux-arm-kernel, devel, rjw, rruigrok,
	linux-kernel, astone, hanjun.guo, joe, pbonzini, akpm, bristot,
	christoffer.dall, shiju.jose

On 01/03/17 02:31, Xiongfeng Wang wrote:

[lot of things]

> If an SEA is injected into guest OS, the guest OS will jump to the SEA
> exception entry when the context switched to guest OS. And the CPSR and
> FAR_EL1 are recovered according to the content of vcpu. Then the guest
> OS can signal a process or panic. If another guest process read the
> error data, another SEA will be generated and it will be single too.
> 
> Without QEMU involved, the drawback is that no APEI table can be mocked
> up in guest OS, and no memory_failure() will be called. So the memory of
> error data will be released into buddy system and assigned to another
> process. If the error was caused by instantaneous radiation or
> electromagnetic, the memory is usable again if it is written with a
> correct data. If the memory has wore out and a correct data is written,
> the ECC error may occurs again with high possibility. Before a 2-bit ECC
> error is reported, much more 1-bit errors will be reported. This is
> report to host os, the host os can determine the memory node has worn
> out and hot-plug out the memory node, and guest os may be terminated if
> its memory data can't be migrated.
> 
> Of course, it is better to get QEMU involved, so the memory_failure can
> be executed in guest OS. But before that implemented, can we add SEA
> injection in kvm_handle_guest_abort()?

No. I will strongly object to that. This is a platform decision to
forward SEAs, not an architectural one. The core KVM code is only
concerned about implementing the ARM architecture, and not something
that is firmware dependent.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-27 13:58     ` James Morse
  2017-02-28  6:25       ` Xiongfeng Wang
@ 2017-03-06  1:28       ` gengdongjiu
  2017-03-22  2:46       ` Xiongfeng Wang
  2 siblings, 0 replies; 29+ messages in thread
From: gengdongjiu @ 2017-03-06  1:28 UTC (permalink / raw)
  To: James Morse
  Cc: Xiongfeng Wang, Punit Agrawal, Tyler Baicar, Christoffer Dall,
	Marc Zyngier, pbonzini, rkrcmar, linux, catalin.marinas,
	will.deacon, rjw, Len Brown, matt, robert.moore, lv.zheng, nkaje,
	zjzhang, mark.rutland, akpm, eun.taik.lee, Sandeepa Prabhu,
	labbott, shijie.huang, rruigrok, paul.gortmaker, tn, Fu Wei,
	rostedt, bristot, linux-arm-kernel, kvmarm, kvm, linux-kernel,
	linux-acpi, linux-efi, devel, Suzuki.Poulose, astone, harba,
	Hanjun Guo, john.garry, shiju.jose, joe

Hi James,


> Hi Wang Xiongfeng,
>
> On 25/02/17 07:15, Xiongfeng Wang wrote:
>> On 2017/2/22 5:22, Tyler Baicar wrote:
>>> Currently external aborts are unsupported by the guest abort
>>> handling. Add handling for SEAs so that the host kernel reports
>>> SEAs which occur in the guest kernel.
>
>>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
>>> index a5265ed..04f1dd50 100644
>>> --- a/arch/arm/kvm/mmu.c
>>> +++ b/arch/arm/kvm/mmu.c
>>> @@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
>>>
>>>      /* Check the stage-2 fault is trans. fault or write fault */
>>>      fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
>>> -    if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
>>> -        fault_status != FSC_ACCESS) {
>>> +
>>> +    /* The host kernel will handle the synchronous external abort. There
>>> +     * is no need to pass the error into the guest.
>>> +     */
>
>> Can we inject an sea into the guest, so that the guest can kill the
>> application which causes the error if the guest won't be terminated
>> later. I'm not sure whether ghes_handle_memory_failure() called in
>> ghes_do_proc() will kill the qemu process. I think it only kill user
>> processes marked with PF_MCE_PROCESS & PF_MCE_EARLY.
>
> My understanding is the pages will get unmapped and recovered where possible
> (e.g. re-read from disk), the user space process will get SIGBUS/SIGSEV when it
> next tries to access that page, which could be some time later.
> These flags in find_early_kill_thread() are a way to make the memory-failure
> code signal the process early, before it does any recovery. The 'MCE' makes me
> think its x86 specific.
> (early and late are described more in [0])
>
>
> Guests are a special case as QEMU may never access the faulty memory itself, so
> it won't receive the 'late' signal. It looks like ARM/arm64 KVM lacks support
> for KVM_PFN_ERR_HWPOISON which sends SIGBUS from KVM's fault-handling code. I
> have patches to add support for this which I intend to send at rc1.

could you push this patch to opensource?


>
> [0] suggests 'KVM qemu' sets these MCE flags to take the 'early' path, but given
> x86s KVM_PFN_ERR_HWPOISON, this may be out of date.
>
>
> Either way, once QEMU gets a signal indicating the virtual address, it can
> generate its own APEI CPER records and use the KVM APIs to mock up an
> Synchronous External Abort, (or inject an IRQ or run the vcpu waiting for the
> guest's polling thread to come round, whichever was described to the guest via
> the HEST/GHES tables).
>
> We can't hand the APEI CPER records we have in the kernel to the guest, as they
> hold a host physical address, and maybe a host virtual address. We don't know
> where in guest memory we could write new APEI CPER records as these locations
> have to be reserved in the guests-UEFI memory map, and only QEMU knows where
> they are.
>
> To deliver RAS events to a guest we have to get QEMU involved.
>
>
> Thanks,
>
> James
>
>
> [0] https://www.kernel.org/doc/Documentation/vm/hwpoison.txt
>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-03-02  9:39             ` Marc Zyngier
@ 2017-03-06  3:38               ` Xiongfeng Wang
  0 siblings, 0 replies; 29+ messages in thread
From: Xiongfeng Wang @ 2017-03-06  3:38 UTC (permalink / raw)
  To: Marc Zyngier, James Morse
  Cc: mark.rutland, linux-efi, kvm, rkrcmar, matt, catalin.marinas,
	Tyler Baicar, will.deacon, linux, paul.gortmaker, lv.zheng,
	kvmarm, fu.wei, tn, zjzhang, robert.moore, linux-acpi,
	eun.taik.lee, shijie.huang, labbott, lenb, harba, john.garry,
	Suzuki.Poulose, punit.agrawal, rostedt, nkaje, sandeepa.s.prabhu,
	linux-arm-kernel, devel, rjw, rruigrok, linux-kernel, astone,
	hanjun.guo, joe, pbonzini, akpm, bristot, christoffer.dall,
	shiju.jose

Hi Marc,

On 2017/3/2 17:39, Marc Zyngier wrote:
> On 01/03/17 02:31, Xiongfeng Wang wrote:
> 
> [lot of things]
> 
>> If an SEA is injected into guest OS, the guest OS will jump to the SEA
>> exception entry when the context switched to guest OS. And the CPSR and
>> FAR_EL1 are recovered according to the content of vcpu. Then the guest
>> OS can signal a process or panic. If another guest process read the
>> error data, another SEA will be generated and it will be single too.
>>
>> Without QEMU involved, the drawback is that no APEI table can be mocked
>> up in guest OS, and no memory_failure() will be called. So the memory of
>> error data will be released into buddy system and assigned to another
>> process. If the error was caused by instantaneous radiation or
>> electromagnetic, the memory is usable again if it is written with a
>> correct data. If the memory has wore out and a correct data is written,
>> the ECC error may occurs again with high possibility. Before a 2-bit ECC
>> error is reported, much more 1-bit errors will be reported. This is
>> report to host os, the host os can determine the memory node has worn
>> out and hot-plug out the memory node, and guest os may be terminated if
>> its memory data can't be migrated.
>>
>> Of course, it is better to get QEMU involved, so the memory_failure can
>> be executed in guest OS. But before that implemented, can we add SEA
>> injection in kvm_handle_guest_abort()?
> 
> No. I will strongly object to that. This is a platform decision to
> forward SEAs, not an architectural one. The core KVM code is only
> concerned about implementing the ARM architecture, and not something
> that is firmware dependent.
> 
Thanks for the reply!
I'm not sure if there exists some misunderstanding here. I was saying
that APEI stuff is not included in the core KVM code, but SEA injection
can be included, just like the SEI injection in the core KVM code. Yes,
APEI is firmware dependent, but I think SEA is not. And the processing
for SEA doesn't depend on whether APEI is implemented.


Thanks,

Wang Xiongfeng
.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-28 19:43     ` Baicar, Tyler
@ 2017-03-06 10:28       ` James Morse
  2017-03-06 14:00         ` Baicar, Tyler
  0 siblings, 1 reply; 29+ messages in thread
From: James Morse @ 2017-03-06 10:28 UTC (permalink / raw)
  To: Baicar, Tyler
  Cc: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, akpm, eun.taik.lee,
	sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe

Hi Tyler,

On 28/02/17 19:43, Baicar, Tyler wrote:
> On 2/24/2017 3:42 AM, James Morse wrote:
>> On 21/02/17 21:22, Tyler Baicar wrote:
>>> Currently external aborts are unsupported by the guest abort
>>> handling. Add handling for SEAs so that the host kernel reports
>>> SEAs which occur in the guest kernel.

>>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>>> index b2d57fc..403277b 100644
>>> --- a/arch/arm64/mm/fault.c
>>> +++ b/arch/arm64/mm/fault.c
>>> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
>>>   }
>>>
>>>   /*
>>> + * Handle Synchronous External Aborts that occur in a guest kernel.
>>> + */
>>> +int handle_guest_sea(unsigned long addr, unsigned int esr)
>>> +{
>>> +    if(IS_ENABLED(HAVE_ACPI_APEI_SEA)) {
>>> +        nmi_enter();
>>> +        ghes_notify_sea();
>>> +        nmi_exit();

>> This nmi stuff was needed for synchronous aborts that may have interrupted
>> APEI's interrupts-masked code. We want to avoid trying to take the same set of
>> locks, hence taking the in_nmi() path through APEI. Here we know we interrupted
>> a guest, so there is no risk that we have interrupted APEI on the host.
>> ghes_notify_sea() can safely take the normal path.

> Makes sense, I can remove the nmi_* calls here.

Just occurs to me: if we do this we need to add the rcu_read_lock() in
ghes_notify_sea() as its not protected by the rcu/nmi weirdness.


Thanks,

James

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-03-06 10:28       ` James Morse
@ 2017-03-06 14:00         ` Baicar, Tyler
  0 siblings, 0 replies; 29+ messages in thread
From: Baicar, Tyler @ 2017-03-06 14:00 UTC (permalink / raw)
  To: James Morse
  Cc: christoffer.dall, marc.zyngier, pbonzini, rkrcmar, linux,
	catalin.marinas, will.deacon, rjw, lenb, matt, robert.moore,
	lv.zheng, nkaje, zjzhang, mark.rutland, akpm, eun.taik.lee,
	sandeepa.s.prabhu, labbott, shijie.huang, rruigrok,
	paul.gortmaker, tn, fu.wei, rostedt, bristot, linux-arm-kernel,
	kvmarm, kvm, linux-kernel, linux-acpi, linux-efi, devel,
	Suzuki.Poulose, punit.agrawal, astone, harba, hanjun.guo,
	john.garry, shiju.jose, joe

Hello James,


On 3/6/2017 3:28 AM, James Morse wrote:
> On 28/02/17 19:43, Baicar, Tyler wrote:
>> On 2/24/2017 3:42 AM, James Morse wrote:
>>> On 21/02/17 21:22, Tyler Baicar wrote:
>>>> Currently external aborts are unsupported by the guest abort
>>>> handling. Add handling for SEAs so that the host kernel reports
>>>> SEAs which occur in the guest kernel.
>>>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>>>> index b2d57fc..403277b 100644
>>>> --- a/arch/arm64/mm/fault.c
>>>> +++ b/arch/arm64/mm/fault.c
>>>> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
>>>>    }
>>>>
>>>>    /*
>>>> + * Handle Synchronous External Aborts that occur in a guest kernel.
>>>> + */
>>>> +int handle_guest_sea(unsigned long addr, unsigned int esr)
>>>> +{
>>>> +    if(IS_ENABLED(HAVE_ACPI_APEI_SEA)) {
>>>> +        nmi_enter();
>>>> +        ghes_notify_sea();
>>>> +        nmi_exit();
>>> This nmi stuff was needed for synchronous aborts that may have interrupted
>>> APEI's interrupts-masked code. We want to avoid trying to take the same set of
>>> locks, hence taking the in_nmi() path through APEI. Here we know we interrupted
>>> a guest, so there is no risk that we have interrupted APEI on the host.
>>> ghes_notify_sea() can safely take the normal path.
>> Makes sense, I can remove the nmi_* calls here.
> Just occurs to me: if we do this we need to add the rcu_read_lock() in
> ghes_notify_sea() as its not protected by the rcu/nmi weirdness.
>
True, would you suggest leaving these nmi_* calls or adding the rcu_* 
calls? And since that's only needed for this KVM case, shouldn't the 
rcu_* calls just replace the nmi_* calls here (outside of ghes_notify_sea)?

Thanks,
Tyler

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-02-27 13:58     ` James Morse
  2017-02-28  6:25       ` Xiongfeng Wang
  2017-03-06  1:28       ` gengdongjiu
@ 2017-03-22  2:46       ` Xiongfeng Wang
  2017-03-22 11:14         ` James Morse
  2 siblings, 1 reply; 29+ messages in thread
From: Xiongfeng Wang @ 2017-03-22  2:46 UTC (permalink / raw)
  To: James Morse, xiexiuqi, gengdongjiu, punit.agrawal
  Cc: mark.rutland, linux-efi, kvm, rkrcmar, matt, catalin.marinas,
	Tyler Baicar, will.deacon, robert.moore, paul.gortmaker,
	lv.zheng, kvmarm, fu.wei, tn, zjzhang, linux, linux-acpi,
	eun.taik.lee, shijie.huang, labbott, lenb, harba, Suzuki.Poulose,
	marc.zyngier, john.garry, rostedt, nkaje, sandeepa.s.prabhu,
	linux-arm-kernel, devel, rjw, rruigrok, linux-kernel, astone,
	hanjun.guo, joe, pbonzini, akpm, bristot, christoffer.dall,
	shiju.jose

Hi James,


> Guests are a special case as QEMU may never access the faulty memory itself, so
> it won't receive the 'late' signal. It looks like ARM/arm64 KVM lacks support
> for KVM_PFN_ERR_HWPOISON which sends SIGBUS from KVM's fault-handling code. I
> have patches to add support for this which I intend to send at rc1.
> 
> [0] suggests 'KVM qemu' sets these MCE flags to take the 'early' path, but given
> x86s KVM_PFN_ERR_HWPOISON, this may be out of date.
> 
> 
> Either way, once QEMU gets a signal indicating the virtual address, it can
> generate its own APEI CPER records and use the KVM APIs to mock up an
> Synchronous External Abort, (or inject an IRQ or run the vcpu waiting for the
> guest's polling thread to come round, whichever was described to the guest via
> the HEST/GHES tables).
> 

I have another confusion about the SIGBUS signal. Can QEMU always get a SIGBUS when needed.
I know one circumstance which will send SIGBUS. The ghes_handle_memory_failure() in
ghes_do_proc() will send SIGBUS to QEMU, but this only happens when there exists memory section
in ghes, that is the section type is CPER_SEC_PLATFORM_MEM.
Suppose this case, an load  error in guest application causes an SEA, and the firmware take it.
The firmware begin to scan the error record and fill the ghes. But the error record in memory node
has been read by other handler. The firmware won't add memory section in ghes, so
ghes_handle_memory_failure() won't be called.
I mean that we may not rely on ghes_handle_memory_failure() to send SIGBUS to QEMU. Whether we should
add some other code to send SIGBUS in handle_guest_abort(). I don't know whether the ARM/arm64
 KVM_PFN_ERR_HWPOISON you mentioned above will cover all the cases.

Thanks,

Wang Xiongfeng
.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-03-22  2:46       ` Xiongfeng Wang
@ 2017-03-22 11:14         ` James Morse
  2017-03-22 12:08           ` Xie XiuQi
  0 siblings, 1 reply; 29+ messages in thread
From: James Morse @ 2017-03-22 11:14 UTC (permalink / raw)
  To: Xiongfeng Wang
  Cc: xiexiuqi, gengdongjiu, punit.agrawal, mark.rutland, linux-efi,
	kvm, rkrcmar, matt, catalin.marinas, Tyler Baicar, will.deacon,
	robert.moore, paul.gortmaker, lv.zheng, kvmarm, fu.wei, tn,
	zjzhang, linux, linux-acpi, eun.taik.lee, shijie.huang, labbott,
	lenb, harba, Suzuki.Poulose, marc.zyngier, john.garry, rostedt,
	nkaje, sandeepa.s.prabhu, linux-arm-kernel, devel, rjw, rruigrok,
	linux-kernel, astone, hanjun.guo, joe, pbonzini, akpm, bristot,
	christoffer.dall, shiju.jose

Hi Wang Xiongfeng,

On 22/03/17 02:46, Xiongfeng Wang wrote:
>> Guests are a special case as QEMU may never access the faulty memory itself, so
>> it won't receive the 'late' signal. It looks like ARM/arm64 KVM lacks support
>> for KVM_PFN_ERR_HWPOISON which sends SIGBUS from KVM's fault-handling code. I
>> have patches to add support for this which I intend to send at rc1.
>>
>> [0] suggests 'KVM qemu' sets these MCE flags to take the 'early' path, but given
>> x86s KVM_PFN_ERR_HWPOISON, this may be out of date.
>>
>>
>> Either way, once QEMU gets a signal indicating the virtual address, it can
>> generate its own APEI CPER records and use the KVM APIs to mock up an
>> Synchronous External Abort, (or inject an IRQ or run the vcpu waiting for the
>> guest's polling thread to come round, whichever was described to the guest via
>> the HEST/GHES tables).
>>
> 
> I have another confusion about the SIGBUS signal. Can QEMU always get a SIGBUS when needed.
> I know one circumstance which will send SIGBUS. The ghes_handle_memory_failure() in
> ghes_do_proc() will send SIGBUS to QEMU, but this only happens when there exists memory section
> in ghes, that is the section type is CPER_SEC_PLATFORM_MEM.
> Suppose this case, an load  error in guest application causes an SEA, and the firmware take it.
> The firmware begin to scan the error record and fill the ghes. But the error record in memory node
> has been read by other handler.

(this looks like a race)

> The firmware won't add memory section in ghes, so ghes_handle_memory_failure() won't be called.

I think this would be a firmware bug. Firmware can reserve as much memory as it
needs for writing CPER records, there should not be a case where 'the memory' is
currently being processed by another handler.

The memory firmware uses to write CPER records too shouldn't be published to the
OS until it has finished. Once firmware has finished writing the CPER records it
can update the memory pointed to by GHES->ErrorStatusAddress with the location
of the CPER records and invoke the Notification method for this GHES. (SEI, SEA,
IRQ etc). We should always get a complete set of CPER records to describe the error.

It firmware uses GHESv2 it can get an 'ack' write from APEI once it has finished
processing the records. Once it gets this firmware knows it can re-use the memory.

(Obviously each GHES entry can only process one error at a time. Firmware should
either handle this, or have one entry for each Error Source that can happen
independently)


> I mean that we may not rely on ghes_handle_memory_failure() to send SIGBUS to QEMU. Whether we should
> add some other code to send SIGBUS in handle_guest_abort(). I don't know whether the ARM/arm64
>  KVM_PFN_ERR_HWPOISON you mentioned above will cover all the cases.

The SIGBUS routine is part of the kernel's recovery method for memory errors. It
should cover all the errors reported with this CPER_SEC_PLATFORM_MEM.

Back to the race you describe. It shouldn't matter if one CPU processes an error
for guest memory while a vcpu is running on another. This may happen if the
error was detected by DRAM's background scrub.
If we don't treat KVM/Qemu as anything special the memory_failure()->SIGBUS path
will happen regardless of whether the fault interrupted the guest or not.


There are other types of error such as PCIe, CPU, BUS error etc. If it's
possible to recover from these we may need additional code in the kernel. This
shouldn't necessarily treat KVM as a special case.


Thanks,

James

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support
  2017-03-22 11:14         ` James Morse
@ 2017-03-22 12:08           ` Xie XiuQi
  0 siblings, 0 replies; 29+ messages in thread
From: Xie XiuQi @ 2017-03-22 12:08 UTC (permalink / raw)
  To: James Morse, Xiongfeng Wang
  Cc: gengdongjiu, punit.agrawal, mark.rutland, linux-efi, kvm,
	rkrcmar, matt, catalin.marinas, Tyler Baicar, will.deacon,
	robert.moore, paul.gortmaker, lv.zheng, kvmarm, fu.wei, tn,
	zjzhang, linux, linux-acpi, eun.taik.lee, shijie.huang, labbott,
	lenb, harba, Suzuki.Poulose, marc.zyngier, john.garry, rostedt,
	nkaje, sandeepa.s.prabhu, linux-arm-kernel, devel, rjw, rruigrok,
	linux-kernel, astone, hanjun.guo, joe, pbonzini, akpm, bristot,
	christoffer.dall, shiju.jose

Hi James,

On 2017/3/22 19:14, James Morse wrote:
> Hi Wang Xiongfeng,
> 
> On 22/03/17 02:46, Xiongfeng Wang wrote:
>>> Guests are a special case as QEMU may never access the faulty memory itself, so
>>> it won't receive the 'late' signal. It looks like ARM/arm64 KVM lacks support
>>> for KVM_PFN_ERR_HWPOISON which sends SIGBUS from KVM's fault-handling code. I
>>> have patches to add support for this which I intend to send at rc1.
>>>
>>> [0] suggests 'KVM qemu' sets these MCE flags to take the 'early' path, but given
>>> x86s KVM_PFN_ERR_HWPOISON, this may be out of date.
>>>
>>>
>>> Either way, once QEMU gets a signal indicating the virtual address, it can
>>> generate its own APEI CPER records and use the KVM APIs to mock up an
>>> Synchronous External Abort, (or inject an IRQ or run the vcpu waiting for the
>>> guest's polling thread to come round, whichever was described to the guest via
>>> the HEST/GHES tables).
>>>
>>
>> I have another confusion about the SIGBUS signal. Can QEMU always get a SIGBUS when needed.
>> I know one circumstance which will send SIGBUS. The ghes_handle_memory_failure() in
>> ghes_do_proc() will send SIGBUS to QEMU, but this only happens when there exists memory section
>> in ghes, that is the section type is CPER_SEC_PLATFORM_MEM.
>> Suppose this case, an load  error in guest application causes an SEA, and the firmware take it.
>> The firmware begin to scan the error record and fill the ghes. But the error record in memory node
>> has been read by other handler.
> 
> (this looks like a race)
> 
>> The firmware won't add memory section in ghes, so ghes_handle_memory_failure() won't be called.
> 
> I think this would be a firmware bug. Firmware can reserve as much memory as it
> needs for writing CPER records, there should not be a case where 'the memory' is
> currently being processed by another handler.

I have a question here:
Consider this case, the memory controller first detected a memory error,
but it has not been consumed. So it will not generate the SEA. Memory error
may be reported to the OS by IRQ with MEM section in CPER record; and
after for a while, the error data was loaded into the cache and consumed,
when the SEA is generated. Is it possible only processor section, and no
MEM section in CPER record?

Obviously there are two different GHES above, one for SEA and another for IRQ/GSIV.
Could we assume that there is mem section in the SEA ghes table?

> 
> The memory firmware uses to write CPER records too shouldn't be published to the
> OS until it has finished. Once firmware has finished writing the CPER records it
> can update the memory pointed to by GHES->ErrorStatusAddress with the location
> of the CPER records and invoke the Notification method for this GHES. (SEI, SEA,
> IRQ etc). We should always get a complete set of CPER records to describe the error.
> 

Does it mean that the BIOS has the responsibility to ensure that the GHES table has a
complete error info, including memory, bus, tlb, cache and other related error info?

-- 
Thanks,
Xie XiuQi

> It firmware uses GHESv2 it can get an 'ack' write from APEI once it has finished
> processing the records. Once it gets this firmware knows it can re-use the memory.
> 
> (Obviously each GHES entry can only process one error at a time. Firmware should
> either handle this, or have one entry for each Error Source that can happen
> independently)
> 
> 
>> I mean that we may not rely on ghes_handle_memory_failure() to send SIGBUS to QEMU. Whether we should
>> add some other code to send SIGBUS in handle_guest_abort(). I don't know whether the ARM/arm64
>>  KVM_PFN_ERR_HWPOISON you mentioned above will cover all the cases.
> 
> The SIGBUS routine is part of the kernel's recovery method for memory errors. It
> should cover all the errors reported with this CPER_SEC_PLATFORM_MEM.
> 
> Back to the race you describe. It shouldn't matter if one CPU processes an error
> for guest memory while a vcpu is running on another. This may happen if the
> error was detected by DRAM's background scrub.
> If we don't treat KVM/Qemu as anything special the memory_failure()->SIGBUS path
> will happen regardless of whether the fault interrupted the guest or not.
> 
> 
> There are other types of error such as PCIe, CPU, BUS error etc. If it's
> possible to recover from these we may need additional code in the kernel. This
> shouldn't necessarily treat KVM as a special case.
> 
> 
> Thanks,
> 
> James
> 
> 
> .
> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2017-03-22 12:20 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-21 21:21 [PATCH V11 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar
2017-02-21 21:21 ` [PATCH V11 01/10] acpi: apei: read ack upon ghes record consumption Tyler Baicar
2017-02-21 21:21 ` [PATCH V11 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1 Tyler Baicar
2017-02-21 21:21 ` [PATCH V11 03/10] efi: parse ARM processor error Tyler Baicar
2017-02-21 21:21 ` [PATCH V11 04/10] arm64: exception: handle Synchronous External Abort Tyler Baicar
2017-02-21 21:21 ` [PATCH V11 05/10] acpi: apei: handle SEA notification type for ARMv8 Tyler Baicar
2017-03-01  7:42   ` Xie XiuQi
2017-03-01 19:22     ` Baicar, Tyler
2017-02-21 21:21 ` [PATCH V11 06/10] acpi: apei: panic OS with fatal error status block Tyler Baicar
2017-02-21 21:21 ` [PATCH V11 07/10] efi: print unrecognized CPER section Tyler Baicar
2017-02-21 21:21 ` [PATCH V11 08/10] ras: acpi / apei: generate trace event for " Tyler Baicar
2017-02-21 21:22 ` [PATCH V11 09/10] trace, ras: add ARM processor error trace event Tyler Baicar
2017-02-21 21:22 ` [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support Tyler Baicar
2017-02-24 10:42   ` James Morse
2017-02-27 11:31     ` gengdongjiu
2017-02-28 19:43     ` Baicar, Tyler
2017-03-06 10:28       ` James Morse
2017-03-06 14:00         ` Baicar, Tyler
2017-02-25  7:15   ` Xiongfeng Wang
2017-02-27 13:58     ` James Morse
2017-02-28  6:25       ` Xiongfeng Wang
2017-02-28 13:21         ` James Morse
2017-03-01  2:31           ` Xiongfeng Wang
2017-03-02  9:39             ` Marc Zyngier
2017-03-06  3:38               ` Xiongfeng Wang
2017-03-06  1:28       ` gengdongjiu
2017-03-22  2:46       ` Xiongfeng Wang
2017-03-22 11:14         ` James Morse
2017-03-22 12:08           ` Xie XiuQi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).