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

[Gupta, Pankaj]

> > > 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.

Yes, I think it helps to read the code better and less conditional.

Thanks,
Pankaj

> John, it's up to you.
>
> William.