On Thu, Aug 31, 2023 at 11:54:56PM +0200, William Roche wrote:
> 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;
> [...]
Yes, that looks correct. I will fix this in the next version of the
series.
Thanks,
John
>
>
> 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.ro...@oracle.com>
> > Signed-off-by: John Allen <john.al...@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;
