| Message ID | 20230726204157.3604531-3-john.allen@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix MCE handling on AMD hosts | expand |
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;
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.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;
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;
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(-)