Re: [PATCH v2 2/2] i386: Fix MCE support for AMD hosts

[William Roche <william.roche@oracle.com>]

John,

I also noticed something important about this specific code:

Qemu commit cb48748af7dfd7654323eb839d1f853ffa873652 introduced the use 
of the MCG_STATUS_RIPV in the case of a BUS_MCEERR_AR error, but it 
looks like your reference code is not having this flag.

According to me, we should keep this flag in the case of a non-AMD 
machine with a BUS_MCEERR_AR.

The patch should look something like that:

[...]
-    if (code == BUS_MCEERR_AR) {
-        status |= MCI_STATUS_AR | 0x134;
-        mcg_status |= MCG_STATUS_RIPV | MCG_STATUS_EIPV;
+    if (!IS_AMD_CPU(env)) {
+        status |= MCI_STATUS_S;
+        if (code == BUS_MCEERR_AR) {
+            status |= MCI_STATUS_AR | 0x134;
+            mcg_status |= MCG_STATUS_RIPV | MCG_STATUS_EIPV;
[...]


Cheers,
William.


On 7/26/23 22:41, John Allen wrote:
> For the most part, AMD hosts can use the same MCE injection code as Intel but,
> there are instances where the qemu implementation is Intel specific. First, MCE
> deliviery works differently on AMD and does not support broadcast. Second,
> kvm_mce_inject generates MCEs that include a number of Intel specific status
> bits. Modify kvm_mce_inject to properly generate MCEs on AMD platforms.
>
> Reported-by: William Roche <william.roche@oracle.com>
> Signed-off-by: John Allen <john.allen@amd.com>
> ---
>   target/i386/helper.c  |  4 ++++
>   target/i386/kvm/kvm.c | 17 +++++++++++------
>   2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/target/i386/helper.c b/target/i386/helper.c
> index 533b29cb91..a6523858e0 100644
> --- a/target/i386/helper.c
> +++ b/target/i386/helper.c
> @@ -76,6 +76,10 @@ int cpu_x86_support_mca_broadcast(CPUX86State *env)
>       int family = 0;
>       int model = 0;
>   
> +    if (IS_AMD_CPU(env)) {
> +        return 0;
> +    }
> +
>       cpu_x86_version(env, &family, &model);
>       if ((family == 6 && model >= 14) || family > 6) {
>           return 1;
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 4b62138459..87a50c8aaf 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -532,16 +532,21 @@ static void kvm_mce_inject(X86CPU *cpu, hwaddr paddr, int code)
>       CPUState *cs = CPU(cpu);
>       CPUX86State *env = &cpu->env;
>       uint64_t status = MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN |
> -                      MCI_STATUS_MISCV | MCI_STATUS_ADDRV | MCI_STATUS_S;
> +                      MCI_STATUS_MISCV | MCI_STATUS_ADDRV;
>       uint64_t mcg_status = MCG_STATUS_MCIP;
>       int flags = 0;
>   
> -    if (code == BUS_MCEERR_AR) {
> -        status |= MCI_STATUS_AR | 0x134;
> -        mcg_status |= MCG_STATUS_EIPV;
> +    if (!IS_AMD_CPU(env)) {
> +        status |= MCI_STATUS_S;
> +        if (code == BUS_MCEERR_AR) {
> +            status |= MCI_STATUS_AR | 0x134;
> +            mcg_status |= MCG_STATUS_EIPV;
> +        } else {
> +            status |= 0xc0;
> +            mcg_status |= MCG_STATUS_RIPV;
> +        }
>       } else {
> -        status |= 0xc0;
> -        mcg_status |= MCG_STATUS_RIPV;
> +        mcg_status |= MCG_STATUS_EIPV | MCG_STATUS_RIPV;
>       }
>   
>       flags = cpu_x86_support_mca_broadcast(env) ? MCE_INJECT_BROADCAST : 0;