Skip to contentSkip to conversation

kvm, x86: broadcast mce depending on the cpu version

PATCH In review Jin Dongming 4 participants 8 messages Oct 14, 2010 – Oct 19, 2010 KVM

lore
Share & tools
lore

Jin Dongming

author
#0 lore raw

PATCH 11/11 kvm, x86: broadcast mce depending on the cpu version

There is no reason why SRAO event received by the main thread is the only one that being broadcasted.

According to the x86 ASDM vol.3A 15.10.4.1,
MCE signal is broadcast on processor version 06H_EH or later.

This change is required to handle SRAR in the guest.

Signed-off-by Hidetoshi Seto <[email protected]>
Tested-by Jin Dongming <[email protected]>
---
 qemu-kvm.c |   63 +++++++++++++++++++++++++++++------------------------------
 1 files changed, 31 insertions(+), 32 deletions(-)
qemu-kvm.c +31 -32
index d2b2459..846f0b6 100644
@@ -1149,6 +1149,34 @@ static int kvm_mce_in_progress(CPUState *env)
     return !!(msr_mcg_status.data & MCG_STATUS_MCIP);
 }
 
+static void kvm_mce_inj_broadcast(CPUState *env, struct kvm_x86_mce *mce)
+{
+    struct kvm_x86_mce mce_sub = {
+        .bank = 1,
+        .status = MCI_STATUS_VAL | MCI_STATUS_UC,
+        .mcg_status = MCG_STATUS_MCIP | MCG_STATUS_RIPV,
+        .addr = 0,
+        .misc = 0,
+    };
+    CPUState *cenv;
+    int family, model, cpuver = env->cpuid_version;
+
+    family = (cpuver >> 8) & 0xf;
+    model = ((cpuver >> 12) & 0xf0) + ((cpuver >> 4) & 0xf);
+
+    kvm_inject_x86_mce_on(env, mce, 1);
+
+    /* Broadcast MCA signal for processor version 06H_EH and above */
+    if ((family == 6 && model >= 14) || family > 6) {
+        for (cenv = first_cpu; cenv != NULL; cenv = cenv->next_cpu) {
+            if (cenv == env) {
+                continue;
+            }
+            kvm_inject_x86_mce_on(cenv, &mce_sub, 1);
+        }
+    }
+}
+
 static void kvm_do_set_mce(CPUState *env, struct kvm_x86_mce *mce,
                            int abort_on_error)
 {

@@ -1175,7 +1203,7 @@ static void kvm_mce_inj_srar_dataload(CPUState *env, target_phys_addr_t paddr)
         .misc = (MCM_ADDR_PHYS << 6) | 0xc,
     };
 
-    kvm_do_set_mce(env, &mce, 1);
+    kvm_mce_inj_broadcast(env, &mce);
 }
 
 static void kvm_mce_inj_srao_memscrub(CPUState *env, target_phys_addr_t paddr)

@@ -1190,32 +1218,7 @@ static void kvm_mce_inj_srao_memscrub(CPUState *env, target_phys_addr_t paddr)
         .misc = (MCM_ADDR_PHYS << 6) | 0xc,
     };
 
-    kvm_do_set_mce(env, &mce, 1);
-}
-
-static void kvm_mce_inj_srao_broadcast(target_phys_addr_t paddr)
-{
-    struct kvm_x86_mce mce_srao_memscrub = {
-        .bank = 9,
-        .status = MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN
-                  | MCI_STATUS_MISCV | MCI_STATUS_ADDRV | MCI_STATUS_S
-                  | 0xc0,
-        .mcg_status = MCG_STATUS_MCIP | MCG_STATUS_RIPV,
-        .addr = paddr,
-        .misc = (MCM_ADDR_PHYS << 6) | 0xc,
-    };
-    struct kvm_x86_mce mce_dummy = {
-        .bank = 1,
-        .status = MCI_STATUS_VAL | MCI_STATUS_UC,
-        .mcg_status = MCG_STATUS_MCIP | MCG_STATUS_RIPV,
-        .addr = 0,
-        .misc = 0,
-    };
-    CPUState *cenv;
-
-    kvm_inject_x86_mce_on(first_cpu, &mce_srao_memscrub, 1);
-    for (cenv = first_cpu->next_cpu; cenv != NULL; cenv = cenv->next_cpu)
-        kvm_inject_x86_mce_on(cenv, &mce_dummy, 1);
+    kvm_mce_inj_broadcast(env, &mce);
 }
 #endif
 

@@ -1255,11 +1258,7 @@ static void kvm_handle_sigbus(CPUState *env, int code, void *vaddr)
             kvm_mce_inj_srar_dataload(target_env, paddr);
         } else {
             /* Fake an Intel architectural Memory scrubbing UCR */
-            if (env) {
-                kvm_mce_inj_srao_memscrub(target_env, paddr);
-            } else {
-                kvm_mce_inj_srao_broadcast(paddr);
-            }
+            kvm_mce_inj_srao_memscrub(target_env, paddr);
         }
         return;
     }
Signature · 1 line
1.7.1.1
#1 lore raw
Quoted 6 lines · Jin Dongming

There is no reason why SRAO event received by the main thread is the only one that being broadcasted.

According to the x86 ASDM vol.3A 15.10.4.1,
MCE signal is broadcast on processor version 06H_EH or later.

Quoted 7 lines

This change is required to handle SRAR in the guest.

Signed-off-by Hidetoshi Seto <[email protected]>
Tested-by Jin Dongming <[email protected]>
---
 qemu-kvm.c |   63 +++++++++++++++++++++++++++++------------------------------
 1 files changed, 31 insertions(+), 32 deletions(-)

Why is this necessary? _AO SIGBUS should be sent to all vcpu threads and main thread.

Please separate bug fixes from cleanups. Very nice, thanks.

#2 lore raw
Quoted 9 lines · Marcelo Tosatti
Quoted 8 lines · Jin Dongming

There is no reason why SRAO event received by the main thread is the only one that being broadcasted.

According to the x86 ASDM vol.3A 15.10.4.1,
MCE signal is broadcast on processor version 06H_EH or later.

This change is required to handle SRAR in the guest.

Quoted 8 lines
Quoted 5 lines
Signed-off-by Hidetoshi Seto <[email protected]>
Tested-by Jin Dongming <[email protected]>
---
 qemu-kvm.c |   63 +++++++++++++++++++++++++++++------------------------------
 1 files changed, 31 insertions(+), 32 deletions(-)

Why is this necessary? _AO SIGBUS should be sent to all vcpu threads and main thread.

Humm? If you are right, vcpu threads will receive same SRAO event twice, one is that received by itself and another is that received by main thread and forwarded by the broadcast.

My understanding is (Jin, please correct me if something wrong):
- _AO SIGBUS is sent to main thread only, and then SRAO event is broadcasted to all vcpu threads.
- _AR SIGBUS is sent to a vcpu thread that tried to touch the unmapped poisoned page, and SRAR event is posted to the vcpu.

One problem here is that SRAR is not broadcasted. The guest might observe the event differently, like "some cpus don't enter machine check."

Quoted 1 line

Please separate bug fixes from cleanups. Very nice, thanks.

Maybe this set is considered as 10 cleanups + 1 fix.
I think this fix will be complicated one without preceding cleanups.

Thanks,
H.Seto

#3 lore raw
Quoted 19 lines · Hidetoshi Seto
Quoted 9 lines · Marcelo Tosatti
Quoted 8 lines · Jin Dongming

There is no reason why SRAO event received by the main thread is the only one that being broadcasted.

According to the x86 ASDM vol.3A 15.10.4.1,
MCE signal is broadcast on processor version 06H_EH or later.

This change is required to handle SRAR in the guest.

Quoted 8 lines
Quoted 5 lines
Signed-off-by Hidetoshi Seto <[email protected]>
Tested-by Jin Dongming <[email protected]>
---
 qemu-kvm.c |   63 +++++++++++++++++++++++++++++------------------------------
 1 files changed, 31 insertions(+), 32 deletions(-)

Why is this necessary? _AO SIGBUS should be sent to all vcpu threads and main thread.

Quoted 7 lines

Humm? If you are right, vcpu threads will receive same SRAO event twice, one is that received by itself and another is that received by main thread and forwarded by the broadcast.

My understanding is (Jin, please correct me if something wrong):
- _AO SIGBUS is sent to main thread only, and then SRAO event is broadcasted to all vcpu threads.

Yes. It is.

Quoted 6 lines

- _AR SIGBUS is sent to a vcpu thread that tried to touch the unmapped poisoned page, and SRAR event is posted to the vcpu.

One problem here is that SRAR is not broadcasted. The guest might observe the event differently, like "some cpus don't enter machine check."

Yes. SRAR "Broadcast" follows spec better.

Best Regards,
Huang Ying

#4 lore raw
In reply to Hidetoshi Seto
Quoted 29 lines · Hidetoshi Seto
Quoted 9 lines · Marcelo Tosatti
Quoted 8 lines · Jin Dongming

There is no reason why SRAO event received by the main thread is the only one that being broadcasted.

According to the x86 ASDM vol.3A 15.10.4.1,
MCE signal is broadcast on processor version 06H_EH or later.

This change is required to handle SRAR in the guest.

Quoted 8 lines
Quoted 5 lines
Signed-off-by Hidetoshi Seto <[email protected]>
Tested-by Jin Dongming <[email protected]>
---
 qemu-kvm.c |   63 +++++++++++++++++++++++++++++------------------------------
 1 files changed, 31 insertions(+), 32 deletions(-)

Why is this necessary? _AO SIGBUS should be sent to all vcpu threads and main thread.

Humm? If you are right, vcpu threads will receive same SRAO event twice, one is that received by itself and another is that received by main thread and forwarded by the broadcast.

My understanding is (Jin, please correct me if something wrong):
- _AO SIGBUS is sent to main thread only, and then SRAO event is broadcasted to all vcpu threads.
- _AR SIGBUS is sent to a vcpu thread that tried to touch the unmapped poisoned page, and SRAR event is posted to the vcpu.

Quoted 3 lines

One problem here is that SRAR is not broadcasted. The guest might observe the event differently, like "some cpus don't enter machine check."

Right.

Quoted 4 lines
Quoted 1 line

Please separate bug fixes from cleanups. Very nice, thanks.

Maybe this set is considered as 10 cleanups + 1 fix.
I think this fix will be complicated one without preceding cleanups.

Why? All you need is to broadcast from vcpu context.

Please do a minimal fix separately so it can be backported, and the cleanups can be done later once its merged upstream.

Thanks.

#5 lore raw
Quoted 34 lines · Marcelo Tosatti
Quoted 29 lines · Hidetoshi Seto
Quoted 9 lines · Marcelo Tosatti
Quoted 8 lines · Jin Dongming

There is no reason why SRAO event received by the main thread is the only one that being broadcasted.

According to the x86 ASDM vol.3A 15.10.4.1,
MCE signal is broadcast on processor version 06H_EH or later.

This change is required to handle SRAR in the guest.

Quoted 8 lines
Quoted 5 lines
Signed-off-by Hidetoshi Seto <[email protected]>
Tested-by Jin Dongming <[email protected]>
---
 qemu-kvm.c |   63 +++++++++++++++++++++++++++++------------------------------
 1 files changed, 31 insertions(+), 32 deletions(-)

Why is this necessary? _AO SIGBUS should be sent to all vcpu threads and main thread.

Humm? If you are right, vcpu threads will receive same SRAO event twice, one is that received by itself and another is that received by main thread and forwarded by the broadcast.

My understanding is (Jin, please correct me if something wrong):
- _AO SIGBUS is sent to main thread only, and then SRAO event is broadcasted to all vcpu threads.
- _AR SIGBUS is sent to a vcpu thread that tried to touch the unmapped poisoned page, and SRAR event is posted to the vcpu.

Quoted 3 lines

One problem here is that SRAR is not broadcasted. The guest might observe the event differently, like "some cpus don't enter machine check."

Quoted 8 lines

Right.

Quoted 4 lines
Quoted 1 line

Please separate bug fixes from cleanups. Very nice, thanks.

Maybe this set is considered as 10 cleanups + 1 fix.
I think this fix will be complicated one without preceding cleanups.

Why? All you need is to broadcast from vcpu context.

No, it is not correct. What I really need is reliable QEMU and maintainable source codes with open community.

Anyway, since I found it could be simpler than what I expected, I rebased 2 "functional change" pieces in this set to today's uq/master.

But these are not tested on the tree yet since I could not build the uq/master due to many warnings on it (even without my fixes).

Quoted 2 lines

Please do a minimal fix separately so it can be backported, and the cleanups can be done later once its merged upstream.

When it will be merged?

Thanks,
H.Seto

#6 lore raw

PATCH 1/2 kvm, x86: ignore SRAO only when MCG_SER_P is available

And restruct this block to call kvm_mce_in_exception() only when it is required.

Signed-off-by Hidetoshi Seto <[email protected]>
---
 target-i386/kvm.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)
target-i386/kvm.c +10 -6
index d940175..98a0505 100644
@@ -239,12 +239,16 @@ static void kvm_do_inject_x86_mce(void *_data)
     struct kvm_x86_mce_data *data = _data;
     int r;
 
-    /* If there is an MCE excpetion being processed, ignore this SRAO MCE */
-    r = kvm_mce_in_exception(data->env);
-    if (r == -1)
-        fprintf(stderr, "Failed to get MCE status\n");
-    else if (r && !(data->mce->status & MCI_STATUS_AR))
-        return;
+    /* If there is an MCE exception being processed, ignore this SRAO MCE */
+    if ((data->env->mcg_cap & MCG_SER_P) &&
+        !(data->mce->status & MCI_STATUS_AR)) {
+        r = kvm_mce_in_exception(data->env);
+        if (r == -1) {
+            fprintf(stderr, "Failed to get MCE status\n");
+        } else if (r) {
+            return;
+        }
+    }
 
     r = kvm_set_mce(data->env, data->mce);
     if (r < 0) {
Signature · 1 line
1.7.3.1
#7 lore raw

PATCH 2/2 kvm, x86: broadcast mce depending on the cpu version

In reply to Hidetoshi Seto

There is no reason why SRAO event received by the main thread is the only one that being broadcasted.

According to the x86 ASDM vol.3A 15.10.4.1,
MCE signal is broadcast on processor version 06H_EH or later.

This change is required to handle SRAR in smp guests.

Signed-off-by Hidetoshi Seto <[email protected]>
---
 target-i386/kvm.c |   28 ++++++++++++++++++++++++----
 1 files changed, 24 insertions(+), 4 deletions(-)
target-i386/kvm.c +24 -4
index 98a0505..e97fbd3 100644
@@ -1631,6 +1631,28 @@ static void hardware_memory_error(void)
     exit(1);
 }
 
+#ifdef KVM_CAP_MCE
+static void kvm_mce_broadcast_rest(CPUState *env)
+{
+    CPUState *cenv;
+    int family, model, cpuver = env->cpuid_version;
+
+    family = (cpuver >> 8) & 0xf;
+    model = ((cpuver >> 12) & 0xf0) + ((cpuver >> 4) & 0xf);
+
+    /* Broadcast MCA signal for processor version 06H_EH and above */
+    if ((family == 6 && model >= 14) || family > 6) {
+        if (cenv == env) {
+            continue;
+        }
+        for (cenv = first_cpu; cenv != NULL; cenv = cenv->next_cpu) {
+            kvm_inject_x86_mce(cenv, 1, MCI_STATUS_VAL | MCI_STATUS_UC,
+                               MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0, 1);
+        }
+    }
+}
+#endif
+
 int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
 {
 #if defined(KVM_CAP_MCE)

@@ -1688,6 +1710,7 @@ int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
             fprintf(stderr, "kvm_set_mce: %s\n", strerror(errno));
             abort();
         }
+        kvm_mce_broadcast_rest(env);
     } else
 #endif
     {

@@ -1726,10 +1749,7 @@ int kvm_on_sigbus(int code, void *addr)
         kvm_inject_x86_mce(first_cpu, 9, status,
                            MCG_STATUS_MCIP | MCG_STATUS_RIPV, paddr,
                            (MCM_ADDR_PHYS << 6) | 0xc, 1);
-        for (cenv = first_cpu->next_cpu; cenv != NULL; cenv = cenv->next_cpu) {
-            kvm_inject_x86_mce(cenv, 1, MCI_STATUS_VAL | MCI_STATUS_UC,
-                               MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0, 1);
-        }
+        kvm_mce_broadcast_rest(first_cpu);
     } else
 #endif
     {
Signature · 1 line
1.7.3.1