Re: [Qemu-arm] [PATCH v14 9/9] target-arm: kvm64: handle SIGBUS signal from kernel or KVM

[Igor Mammedov]

On Thu, 28 Dec 2017 13:54:18 +0800
Dongjiu Geng <address@hidden> wrote:

> Add SIGBUS signal handler. In this handler, it checks the SIGBUS type,
> translates the host VA which is delivered by host to guest PA, then fill
> this PA to CPER and fill the CPER to guest APEI GHES memory, finally
> notify guest according to the SIGBUS type. There are two kinds of SIGBUS
> that QEMU needs to handle, which are BUS_MCEERR_AO and BUS_MCEERR_AR.
> 
> If guest accesses the poisoned memory, it generates Synchronous External
> Abort(SEA). Then host kernel gets an APEI notification and call 
> memory_failure()
> to unmapped the affected page from the guest's stage2, and SIGBUS_MCEERR_AO
s/unmapped/unmap/

> is delivered to Qemu's main thread. If Qemu receives this SIGBUS, it will
> create a new CPER and add it to guest APEI GHES memory, then notify the
> guest with a GPIO-Signal notification.
too long sentence, it's hard get what goes on here, pls split it in simple
sentences/rephrase so it would be easy to understand behavior.

> 
> When guest hits a PG_hwpoison page, it will trap to KVM as stage2 fault, then 
> a
> SIGBUS_MCEERR_AR synchronous signal is delivered to Qemu, Qemu record this 
> error
> into guest APEI GHES memory and notify guest using 
> Synchronous-External-Abort(SEA).
> 
> Suggested-by: James Morse <address@hidden>
> Signed-off-by: Dongjiu Geng <address@hidden>
> ---
> Address James's comments to record CPER and notify guest for SIGBUS signal 
> handling.
> Shown some discussion in [1].
> 
> [1]:
> https://lkml.org/lkml/2017/2/27/246
> https://lkml.org/lkml/2017/9/14/241
> https://lkml.org/lkml/2017/9/22/499
> ---
>  include/sysemu/kvm.h |  2 +-
>  target/arm/kvm.c     |  2 ++
>  target/arm/kvm64.c   | 34 ++++++++++++++++++++++++++++++++++
>  3 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 3a458f5..90c1605 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -361,7 +361,7 @@ bool kvm_vcpu_id_is_valid(int vcpu_id);
>  /* Returns VCPU ID to be used on KVM_CREATE_VCPU ioctl() */
>  unsigned long kvm_arch_vcpu_id(CPUState *cpu);
>  
> -#ifdef TARGET_I386
> +#if defined(TARGET_I386) || defined(TARGET_AARCH64)
>  #define KVM_HAVE_MCE_INJECTION 1
>  void kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
>  #endif
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 7c17f0d..9d25f51 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -26,6 +26,7 @@
>  #include "exec/address-spaces.h"
>  #include "hw/boards.h"
>  #include "qemu/log.h"
> +#include "exec/ram_addr.h"
>  
>  const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>      KVM_CAP_LAST_INFO
> @@ -182,6 +183,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>  
>      cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
>  
> +    qemu_register_reset(kvm_unpoison_all, NULL);
>      type_register_static(&host_arm_cpu_type_info);
>  
>      return 0;
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index c00450d..6955d85 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -27,6 +27,9 @@
>  #include "kvm_arm.h"
>  #include "internals.h"
>  #include "hw/arm/arm.h"
> +#include "exec/ram_addr.h"
> +#include "hw/acpi/acpi-defs.h"
> +#include "hw/acpi/hest_ghes.h"
>  
>  static bool have_guest_debug;
>  
> @@ -944,6 +947,37 @@ int kvm_arch_get_registers(CPUState *cs)
>      return ret;
>  }
>  
> +void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
> +{
> +    ram_addr_t ram_addr;
> +    hwaddr paddr;
> +
> +    assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO);
> +    if (addr) {
> +        ram_addr = qemu_ram_addr_from_host(addr);
> +        if (ram_addr != RAM_ADDR_INVALID &&
> +            kvm_physical_memory_addr_from_host(c->kvm_state, addr, &paddr)) {
> +            kvm_hwpoison_page_add(ram_addr);
> +            if (code == BUS_MCEERR_AR) {
> +                kvm_cpu_synchronize_state(c);
> +                ghes_record_errors(ACPI_HEST_NOTIFY_SEA, paddr);
> +                kvm_inject_arm_sea(c);
> +            } else if (code == BUS_MCEERR_AO) {
> +                ghes_record_errors(ACPI_HEST_NOTIFY_GPIO, paddr);
> +                qemu_hardware_error_notify();
> +            }
> +            return;
> +        }
> +        fprintf(stderr, "Hardware memory error for memory used by "
> +                "QEMU itself instead of guest system!\n");
not quite sure what above message means,

also fprintf() probably shouldn't be used by new code.

> +    }
> +
> +    if (code == BUS_MCEERR_AR) {
> +        fprintf(stderr, "Hardware memory error!\n");
> +        exit(1);
> +    }
> +}
> +
>  /* C6.6.29 BRK instruction */
>  static const uint32_t brk_insn = 0xd4200000;
>