Re: [PATCH v3 2/3] i386: Explicitly ignore unsupported BUS_MCEERR_AO MCE on AMD guest

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

On 9/7/23 13:12, Gupta, Pankaj wrote:
>
>>
>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
>> index 5fce74aac5..4d42d3ed4c 100644
>> --- a/target/i386/kvm/kvm.c
>> +++ b/target/i386/kvm/kvm.c
>> @@ -604,6 +604,10 @@ static void kvm_mce_inject(X86CPU *cpu, hwaddr 
>> paddr, int code)
>>               mcg_status |= MCG_STATUS_RIPV;
>>           }
>>       } else {
>> +        if (code == BUS_MCEERR_AO) {
>> +            /* XXX we don't support BUS_MCEERR_AO injection on AMD 
>> yet */
>> +            return;
>> +        }
>>           mcg_status |= MCG_STATUS_EIPV | MCG_STATUS_RIPV;
>>       }
>> @@ -655,7 +659,9 @@ void kvm_arch_on_sigbus_vcpu(CPUState *c, int 
>> code, void *addr)
>>           if (ram_addr != RAM_ADDR_INVALID &&
>>               kvm_physical_memory_addr_from_host(c->kvm_state, addr, 
>> &paddr)) {
>>               kvm_hwpoison_page_add(ram_addr);
>> -            kvm_mce_inject(cpu, paddr, code);
>> +            if (!IS_AMD_CPU(env) || code != BUS_MCEERR_AO) {
> 
> Isn't the 'optional' case we already handle inside kvm_mce_inject()?
> So this check seems repetitive to me.

You are right, it is repetitive, but can be considered as a reminder of 
the situation and an explanation of the "ignored on AMD guest" message 
later in this function.

Of course it can be removed if you think that the code is easier to read 
without it. When the AMD BUS_MCEERR_AO support is integrated, both 
locations would need to be cleared, but this sounds reasonable to me.

John, it's up to you.

William.