Re: [PATCH 1/2] accel/kvm: Define KVM_ARCH_HAVE_MCE_INJECTION in each target

[john strange <johnstra10@gmail.com>]

It might be good to replace "MCE" in kvm-all.c with a generic 
abstraction for error injection to avoid carrying the MCE constructs 
into non-x86 sigbus handlers.

thanks,
-john

On 1/24/24 07:54, Philippe Mathieu-Daudé wrote:
> Instead of having KVM_HAVE_MCE_INJECTION optionally defined,
> always define KVM_ARCH_HAVE_MCE_INJECTION for each target,
> and set KVM_HAVE_MCE_INJECTION if it is not zero.
>
> It is now clearer for KVM targets to detect if they lack the
> MCE injection implementation. Also, moving headers around
> is no more painful, because if a target neglects to define
> KVM_ARCH_HAVE_MCE_INJECTION, the build will fail.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> I'd rather keep "cpu-param.h" simple/short.
>
> Is a per-target "kvm-param.h" any better?
> Or per-target "accel-param.h"? For example TCG_GUEST_DEFAULT_MO
> is TCG specific and could also go there. Otherwise it will go
> in "cpu-param.h".
> ---
>   include/sysemu/kvm.h         | 5 +++++
>   target/arm/cpu-param.h       | 5 +++++
>   target/arm/cpu.h             | 4 ----
>   target/i386/cpu-param.h      | 2 ++
>   target/i386/cpu.h            | 2 --
>   target/loongarch/cpu-param.h | 2 ++
>   target/mips/cpu-param.h      | 2 ++
>   target/ppc/cpu-param.h       | 2 ++
>   target/riscv/cpu-param.h     | 2 ++
>   target/s390x/cpu-param.h     | 2 ++
>   10 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index d614878164..2e9aa2fc53 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -212,6 +212,7 @@ int kvm_on_sigbus(int code, void *addr);
>   
>   #ifdef NEED_CPU_H
>   #include "cpu.h"
> +#include "cpu-param.h"
>   
>   void kvm_flush_coalesced_mmio_buffer(void);
>   
> @@ -349,6 +350,10 @@ 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);
>   
> +#if KVM_ARCH_HAVE_MCE_INJECTION
> +#define KVM_HAVE_MCE_INJECTION
> +#endif
> +
>   #ifdef KVM_HAVE_MCE_INJECTION
>   void kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
>   #endif
> diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
> index f9b462a98f..d71cc29864 100644
> --- a/target/arm/cpu-param.h
> +++ b/target/arm/cpu-param.h
> @@ -30,7 +30,12 @@
>    */
>   # define TARGET_PAGE_BITS_VARY
>   # define TARGET_PAGE_BITS_MIN  10
> +#endif
>   
> +#ifdef TARGET_AARCH64
> +#define KVM_ARCH_HAVE_MCE_INJECTION 1
> +#else
> +#define KVM_ARCH_HAVE_MCE_INJECTION 0
>   #endif
>   
>   #endif
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index ec276fcd57..f92c8d3b88 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -30,10 +30,6 @@
>   /* ARM processors have a weak memory model */
>   #define TCG_GUEST_DEFAULT_MO      (0)
>   
> -#ifdef TARGET_AARCH64
> -#define KVM_HAVE_MCE_INJECTION 1
> -#endif
> -
>   #define EXCP_UDEF            1   /* undefined instruction */
>   #define EXCP_SWI             2   /* software interrupt */
>   #define EXCP_PREFETCH_ABORT  3
> diff --git a/target/i386/cpu-param.h b/target/i386/cpu-param.h
> index 911b4cd51b..5933b0b756 100644
> --- a/target/i386/cpu-param.h
> +++ b/target/i386/cpu-param.h
> @@ -24,4 +24,6 @@
>   #endif
>   #define TARGET_PAGE_BITS 12
>   
> +#define KVM_ARCH_HAVE_MCE_INJECTION 1
> +
>   #endif
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 7f0786e8b9..230ab1cded 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -33,8 +33,6 @@
>   /* The x86 has a strong memory model with some store-after-load re-ordering */
>   #define TCG_GUEST_DEFAULT_MO      (TCG_MO_ALL & ~TCG_MO_ST_LD)
>   
> -#define KVM_HAVE_MCE_INJECTION 1
> -
>   /* support for self modifying code even if the modified instruction is
>      close to the modifying instruction */
>   #define TARGET_HAS_PRECISE_SMC
> diff --git a/target/loongarch/cpu-param.h b/target/loongarch/cpu-param.h
> index cfe195db4e..f69a94e6b5 100644
> --- a/target/loongarch/cpu-param.h
> +++ b/target/loongarch/cpu-param.h
> @@ -14,4 +14,6 @@
>   
>   #define TARGET_PAGE_BITS 12
>   
> +#define KVM_ARCH_HAVE_MCE_INJECTION 0
> +
>   #endif
> diff --git a/target/mips/cpu-param.h b/target/mips/cpu-param.h
> index 594c91a156..45c885e00e 100644
> --- a/target/mips/cpu-param.h
> +++ b/target/mips/cpu-param.h
> @@ -30,4 +30,6 @@
>   #define TARGET_PAGE_BITS_MIN 12
>   #endif
>   
> +#define KVM_ARCH_HAVE_MCE_INJECTION 0
> +
>   #endif
> diff --git a/target/ppc/cpu-param.h b/target/ppc/cpu-param.h
> index 0a0416e0a8..9975ae73ab 100644
> --- a/target/ppc/cpu-param.h
> +++ b/target/ppc/cpu-param.h
> @@ -33,4 +33,6 @@
>   #endif
>   #define TARGET_PAGE_BITS 12
>   
> +#define KVM_ARCH_HAVE_MCE_INJECTION 0
> +
>   #endif
> diff --git a/target/riscv/cpu-param.h b/target/riscv/cpu-param.h
> index b2a9396dec..e6199f4f6d 100644
> --- a/target/riscv/cpu-param.h
> +++ b/target/riscv/cpu-param.h
> @@ -28,4 +28,6 @@
>    *  - M mode HLV/HLVX/HSV 0b111
>    */
>   
> +#define KVM_ARCH_HAVE_MCE_INJECTION 0
> +
>   #endif
> diff --git a/target/s390x/cpu-param.h b/target/s390x/cpu-param.h
> index 84ca08626b..4728b3957e 100644
> --- a/target/s390x/cpu-param.h
> +++ b/target/s390x/cpu-param.h
> @@ -13,4 +13,6 @@
>   #define TARGET_PHYS_ADDR_SPACE_BITS 64
>   #define TARGET_VIRT_ADDR_SPACE_BITS 64
>   
> +#define KVM_ARCH_HAVE_MCE_INJECTION 0
> +
>   #endif