Re: [PATCH v19 5/5] target-arm: kvm64: handle SIGBUS signal from kernel or KVM

2019-10-17 Thread Xiang Zheng



On 2019/10/15 22:48, Peter Maydell wrote:
> On Tue, 15 Oct 2019 at 15:02, Xiang Zheng  wrote:
>>
>> From: Dongjiu Geng 
>>
>> Add a SIGBUS signal handler. In this handler, it checks the SIGBUS type,
>> translates the host VA delivered by host to guest PA, then fills this PA
>> to guest APEI GHES memory, then notifies guest according to the SIGBUS
>> type.
>>
>> When guest accesses the poisoned memory, it will generate a Synchronous
>> External Abort(SEA). Then host kernel gets an APEI notification and calls
>> memory_failure() to unmapped the affected page in stage 2, finally
>> returns to guest.
>>
>> Guest continues to access the PG_hwpoison page, it will trap to KVM as
>> stage2 fault, then a SIGBUS_MCEERR_AR synchronous signal is delivered to
>> Qemu, Qemu records this error address into guest APEI GHES memory and
>> notifes guest using Synchronous-External-Abort(SEA).
>>
>> In order to inject a vSEA, we introduce the kvm_inject_arm_sea() function
>> in which we can setup the type of exception and the syndrome information.
>> When switching to guest, the target vcpu will jump to the synchronous
>> external abort vector table entry.
>>
>> The ESR_ELx.DFSC is set to synchronous external abort(0x10), and the
>> ESR_ELx.FnV is set to not valid(0x1), which will tell guest that FAR is
>> not valid and hold an UNKNOWN value. These values will be set to KVM
>> register structures through KVM_SET_ONE_REG IOCTL.
>>
>> Signed-off-by: Dongjiu Geng 
>> Signed-off-by: Xiang Zheng 
> 
>> +static int acpi_ghes_record_mem_error(uint64_t error_block_address,
>> +  uint64_t error_physical_addr,
>> +  uint32_t data_length)
>> +{
>> +GArray *block;
>> +uint64_t current_block_length;
>> +/* Memory Error Section Type */
>> +QemuUUID mem_section_id_le = UEFI_CPER_SEC_PLATFORM_MEM;
>> +QemuUUID fru_id = {0};
> 
> Hi; this makes at least some versions of clang complain
> (this is a clang bug, but it's present in shipped versions):
> 
> /home/petmay01/linaro/qemu-from-laptop/qemu/hw/acpi/acpi_ghes.c:135:24:
> error: suggest braces around
>   initialization of subobject [-Werror,-Wmissing-braces]
> QemuUUID fru_id = {0};
>^
>{}
> 
> We generally use "{}" as the generic zero-initializer for
> this reason (it's gcc/clang specific whereas "{0}" is
> in the standard, but all of the compilers we care about
> support it and don't warn about its use).
> 
>> +uint8_t fru_text[20] = {0};
> 
> Clang doesn't mind this one because it's not initializing
> a struct type, but you could use "{}" here too for consistency.
> 

OK, I will replace all the "{0}" with "{}".

> thanks
> -- PMM
> 
> .
> 

-- 

Thanks,
Xiang




Re: [PATCH v19 5/5] target-arm: kvm64: handle SIGBUS signal from kernel or KVM

2019-10-15 Thread Peter Maydell
On Tue, 15 Oct 2019 at 15:02, Xiang Zheng  wrote:
>
> From: Dongjiu Geng 
>
> Add a SIGBUS signal handler. In this handler, it checks the SIGBUS type,
> translates the host VA delivered by host to guest PA, then fills this PA
> to guest APEI GHES memory, then notifies guest according to the SIGBUS
> type.
>
> When guest accesses the poisoned memory, it will generate a Synchronous
> External Abort(SEA). Then host kernel gets an APEI notification and calls
> memory_failure() to unmapped the affected page in stage 2, finally
> returns to guest.
>
> Guest continues to access the PG_hwpoison page, it will trap to KVM as
> stage2 fault, then a SIGBUS_MCEERR_AR synchronous signal is delivered to
> Qemu, Qemu records this error address into guest APEI GHES memory and
> notifes guest using Synchronous-External-Abort(SEA).
>
> In order to inject a vSEA, we introduce the kvm_inject_arm_sea() function
> in which we can setup the type of exception and the syndrome information.
> When switching to guest, the target vcpu will jump to the synchronous
> external abort vector table entry.
>
> The ESR_ELx.DFSC is set to synchronous external abort(0x10), and the
> ESR_ELx.FnV is set to not valid(0x1), which will tell guest that FAR is
> not valid and hold an UNKNOWN value. These values will be set to KVM
> register structures through KVM_SET_ONE_REG IOCTL.
>
> Signed-off-by: Dongjiu Geng 
> Signed-off-by: Xiang Zheng 

> +static int acpi_ghes_record_mem_error(uint64_t error_block_address,
> +  uint64_t error_physical_addr,
> +  uint32_t data_length)
> +{
> +GArray *block;
> +uint64_t current_block_length;
> +/* Memory Error Section Type */
> +QemuUUID mem_section_id_le = UEFI_CPER_SEC_PLATFORM_MEM;
> +QemuUUID fru_id = {0};

Hi; this makes at least some versions of clang complain
(this is a clang bug, but it's present in shipped versions):

/home/petmay01/linaro/qemu-from-laptop/qemu/hw/acpi/acpi_ghes.c:135:24:
error: suggest braces around
  initialization of subobject [-Werror,-Wmissing-braces]
QemuUUID fru_id = {0};
   ^
   {}

We generally use "{}" as the generic zero-initializer for
this reason (it's gcc/clang specific whereas "{0}" is
in the standard, but all of the compilers we care about
support it and don't warn about its use).

> +uint8_t fru_text[20] = {0};

Clang doesn't mind this one because it's not initializing
a struct type, but you could use "{}" here too for consistency.

thanks
-- PMM



[PATCH v19 5/5] target-arm: kvm64: handle SIGBUS signal from kernel or KVM

2019-10-15 Thread Xiang Zheng
From: Dongjiu Geng 

Add a SIGBUS signal handler. In this handler, it checks the SIGBUS type,
translates the host VA delivered by host to guest PA, then fills this PA
to guest APEI GHES memory, then notifies guest according to the SIGBUS
type.

When guest accesses the poisoned memory, it will generate a Synchronous
External Abort(SEA). Then host kernel gets an APEI notification and calls
memory_failure() to unmapped the affected page in stage 2, finally
returns to guest.

Guest continues to access the PG_hwpoison page, it will trap to KVM as
stage2 fault, then a SIGBUS_MCEERR_AR synchronous signal is delivered to
Qemu, Qemu records this error address into guest APEI GHES memory and
notifes guest using Synchronous-External-Abort(SEA).

In order to inject a vSEA, we introduce the kvm_inject_arm_sea() function
in which we can setup the type of exception and the syndrome information.
When switching to guest, the target vcpu will jump to the synchronous
external abort vector table entry.

The ESR_ELx.DFSC is set to synchronous external abort(0x10), and the
ESR_ELx.FnV is set to not valid(0x1), which will tell guest that FAR is
not valid and hold an UNKNOWN value. These values will be set to KVM
register structures through KVM_SET_ONE_REG IOCTL.

Signed-off-by: Dongjiu Geng 
Signed-off-by: Xiang Zheng 
---
 hw/acpi/acpi_ghes.c | 259 
 include/hw/acpi/acpi_ghes.h |  42 +++
 include/sysemu/kvm.h|   3 +-
 target/arm/cpu.h|   4 +
 target/arm/helper.c |   2 +-
 target/arm/internals.h  |   5 +-
 target/arm/kvm64.c  |  64 +++
 target/arm/tlb_helper.c |   2 +-
 target/i386/cpu.h   |   2 +
 9 files changed, 377 insertions(+), 6 deletions(-)

diff --git a/hw/acpi/acpi_ghes.c b/hw/acpi/acpi_ghes.c
index 36f102d..1bd4850 100644
--- a/hw/acpi/acpi_ghes.c
+++ b/hw/acpi/acpi_ghes.c
@@ -28,6 +28,174 @@
 #include "qemu/error-report.h"
 
 /*
+ * Total size for Generic Error Status Block
+ * ACPI 6.2: 18.3.2.7.1 Generic Error Data,
+ * Table 18-380 Generic Error Status Block
+ */
+#define ACPI_GHES_GESB_SIZE 20
+/* The offset of Data Length in Generic Error Status Block */
+#define ACPI_GHES_GESB_DATA_LENGTH_OFFSET   12
+
+/*
+ * Record the value of data length for each error status block to avoid getting
+ * this value from guest.
+ */
+static uint32_t acpi_ghes_data_length[ACPI_GHES_ERROR_SOURCE_COUNT];
+
+/*
+ * Generic Error Data Entry
+ * ACPI 6.1: 18.3.2.7.1 Generic Error Data
+ */
+static void acpi_ghes_generic_error_data(GArray *table, QemuUUID section_type,
+uint32_t error_severity, uint16_t revision,
+uint8_t validation_bits, uint8_t flags,
+uint32_t error_data_length, QemuUUID fru_id,
+uint8_t *fru_text, uint64_t time_stamp)
+{
+QemuUUID uuid_le;
+
+/* Section Type */
+uuid_le = qemu_uuid_bswap(section_type);
+g_array_append_vals(table, uuid_le.data, ARRAY_SIZE(uuid_le.data));
+
+/* Error Severity */
+build_append_int_noprefix(table, error_severity, 4);
+/* Revision */
+build_append_int_noprefix(table, revision, 2);
+/* Validation Bits */
+build_append_int_noprefix(table, validation_bits, 1);
+/* Flags */
+build_append_int_noprefix(table, flags, 1);
+/* Error Data Length */
+build_append_int_noprefix(table, error_data_length, 4);
+
+/* FRU Id */
+uuid_le = qemu_uuid_bswap(fru_id);
+g_array_append_vals(table, uuid_le.data, ARRAY_SIZE(uuid_le.data));
+
+/* FRU Text */
+g_array_append_vals(table, fru_text, 20);
+/* Timestamp */
+build_append_int_noprefix(table, time_stamp, 8);
+}
+
+/*
+ * Generic Error Status Block
+ * ACPI 6.1: 18.3.2.7.1 Generic Error Data
+ */
+static void acpi_ghes_generic_error_status(GArray *table, uint32_t 
block_status,
+uint32_t raw_data_offset, uint32_t raw_data_length,
+uint32_t data_length, uint32_t error_severity)
+{
+/* Block Status */
+build_append_int_noprefix(table, block_status, 4);
+/* Raw Data Offset */
+build_append_int_noprefix(table, raw_data_offset, 4);
+/* Raw Data Length */
+build_append_int_noprefix(table, raw_data_length, 4);
+/* Data Length */
+build_append_int_noprefix(table, data_length, 4);
+/* Error Severity */
+build_append_int_noprefix(table, error_severity, 4);
+}
+
+/* UEFI 2.6: N.2.5 Memory Error Section */
+static void acpi_ghes_build_append_mem_cper(GArray *table,
+uint64_t error_physical_addr)
+{
+/*
+ * Memory Error Record
+ */
+
+/* Validation Bits */
+build_append_int_noprefix(table,
+  (1UL << 14) | /* Type Valid */
+  (1UL << 1) /* Physical Address Valid */,
+  8);
+/* Error Status */
+build_append_int_noprefix(table, 0, 8);
+/* Physical Address */
+