MCE: support cpu notification chain

PATCH In review Christoph Egger 2 participants 6 messages Oct 25, 2012 – Oct 26, 2012

Share & tools

Christoph Egger

author

2012-10-25 13:54 UTC #0 lore raw

PATCH 1/1 MCE: support cpu notification chain

Add support for cpu notification chain.
This is useful with nested virtualization when Xen runs as l1 hypervisor.
This cleans up mce initialization cleanup as a side-effect.

Signed-off-by Christoph Egger <[email protected]>

Signature · 6 lines

---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

xen/arch/x86/cpu/mcheck/mce.c +1 ~~-25~~

diff -r 7abb25095de0 xen/arch/x86/cpu/mcheck/mce.c
@@ -560,30 +560,6 @@ void mcheck_mca_clearbanks(struct mca_ba
     }
 }
 
-static enum mcheck_type amd_mcheck_init(struct cpuinfo_x86 *ci)
-{
-    enum mcheck_type rc = mcheck_none;
-
-    switch (ci->x86) {
-    case 6:
-        rc = amd_k7_mcheck_init(ci);
-        break;
-
-    default:
-        /* Assume that machine check support is available.
-         * The minimum provided support is at least the K8. */
-    case 0xf:
-        rc = amd_k8_mcheck_init(ci);
-        break;
-
-    case 0x10 ... 0x17:
-        rc = amd_f10_mcheck_init(ci);
-        break;
-    }
-
-    return rc;
-}
-
 /*check the existence of Machine Check*/
 int mce_available(struct cpuinfo_x86 *c)
 {

@@ -774,7 +750,7 @@ void mcheck_init(struct cpuinfo_x86 *c, 
 
     switch (c->x86_vendor) {
     case X86_VENDOR_AMD:
-        inited = amd_mcheck_init(c);
+        inited = amd_mcheck_init(c, bsp);
         break;
 
     case X86_VENDOR_INTEL:

xen/arch/x86/cpu/mcheck/mce.h +1 -4

diff -r 7abb25095de0 xen/arch/x86/cpu/mcheck/mce.h
@@ -39,10 +39,7 @@ enum mcheck_type {
 };
 
 /* Init functions */
-enum mcheck_type amd_k7_mcheck_init(struct cpuinfo_x86 *c);
-enum mcheck_type amd_k8_mcheck_init(struct cpuinfo_x86 *c);
-enum mcheck_type amd_f10_mcheck_init(struct cpuinfo_x86 *c);
-
+enum mcheck_type amd_mcheck_init(struct cpuinfo_x86 *c, bool_t bsp);
 enum mcheck_type intel_mcheck_init(struct cpuinfo_x86 *c, bool_t bsp);
 
 void intel_mcheck_timer(struct cpuinfo_x86 *c);

xen/arch/x86/cpu/mcheck/mce_amd.c +83 -0

92 lines not on this page · raw message

xen/arch/x86/cpu/mcheck/mce_amd.h +4 -0

diff -r 7abb25095de0 xen/arch/x86/cpu/mcheck/mce_amd.h
@@ -1,6 +1,10 @@
 #ifndef _MCHECK_AMD_H
 #define _MCHECK_AMD_H
 
+enum mcheck_type amd_k7_mcheck_init(struct cpuinfo_x86 *c);
+enum mcheck_type amd_k8_mcheck_init(struct cpuinfo_x86 *c);
+enum mcheck_type amd_f10_mcheck_init(struct cpuinfo_x86 *c);
+
 int mc_amd_recoverable_scan(uint64_t status);
 int mc_amd_addrcheck(uint64_t status, uint64_t misc, int addrtype);

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Jan Beulich

2012-10-25 14:10 UTC #1 lore raw

Quoted 5 lines

Quoted 1 line
Quoted 1 line · Christoph Egger
Add support for cpu notification chain.
This is useful with nested virtualization when Xen runs as l1 hypervisor.
This cleans up mce initialization cleanup as a side-effect.

Did you notice the fix I had done to your earlier patch before committing c/s 26106:1883c1d29de9? With the change here, mce_clear_banks will get allocated twice (once in generic code and once in AMD specific code), leaking one instance.

Jan

Christoph Egger

author

2012-10-25 14:18 UTC #2 lore raw

Quoted 10 lines · Jan Beulich

Quoted 5 lines
Quoted 1 line
Quoted 1 line · Christoph Egger
Add support for cpu notification chain.
This is useful with nested virtualization when Xen runs as l1 hypervisor.
This cleans up mce initialization cleanup as a side-effect.
Did you notice the fix I had done to your earlier patch before committing c/s 26106:1883c1d29de9? With the change here, mce_clear_banks will get allocated twice (once in generic code and once in AMD specific code), leaking one instance.

Ugh... must be a merge botch.

Christoph

Signature · 6 lines

---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Christoph Egger

author

2012-10-25 14:45 UTC #3 lore raw

Quoted 7 lines · Christoph Egger

Quoted 6 lines · Jan Beulich
Quoted 5 lines
Quoted 1 line
Quoted 1 line · Christoph Egger
Add support for cpu notification chain.
This is useful with nested virtualization when Xen runs as l1 hypervisor.
This cleans up mce initialization cleanup as a side-effect.

Quoted 6 lines

Quoted 4 lines
Did you notice the fix I had done to your earlier patch before committing c/s 26106:1883c1d29de9? With the change here, mce_clear_banks will get allocated twice (once in generic code and once in AMD specific code), leaking one instance.
Ugh... must be a merge botch.

Updated patch attached.

Signed-off-by Christoph Egger <[email protected]>

Signature · 6 lines

---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

xen/arch/x86/cpu/mcheck/mce.c +1 ~~-25~~

diff -r 7abb25095de0 xen/arch/x86/cpu/mcheck/mce.c
@@ -560,30 +560,6 @@ void mcheck_mca_clearbanks(struct mca_ba
     }
 }
 
-static enum mcheck_type amd_mcheck_init(struct cpuinfo_x86 *ci)
-{
-    enum mcheck_type rc = mcheck_none;
-
-    switch (ci->x86) {
-    case 6:
-        rc = amd_k7_mcheck_init(ci);
-        break;
-
-    default:
-        /* Assume that machine check support is available.
-         * The minimum provided support is at least the K8. */
-    case 0xf:
-        rc = amd_k8_mcheck_init(ci);
-        break;
-
-    case 0x10 ... 0x17:
-        rc = amd_f10_mcheck_init(ci);
-        break;
-    }
-
-    return rc;
-}
-
 /*check the existence of Machine Check*/
 int mce_available(struct cpuinfo_x86 *c)
 {

@@ -774,7 +750,7 @@ void mcheck_init(struct cpuinfo_x86 *c, 
 
     switch (c->x86_vendor) {
     case X86_VENDOR_AMD:
-        inited = amd_mcheck_init(c);
+        inited = amd_mcheck_init(c, bsp);
         break;
 
     case X86_VENDOR_INTEL:

xen/arch/x86/cpu/mcheck/mce.h +1 -4

diff -r 7abb25095de0 xen/arch/x86/cpu/mcheck/mce.h
@@ -39,10 +39,7 @@ enum mcheck_type {
 };
 
 /* Init functions */
-enum mcheck_type amd_k7_mcheck_init(struct cpuinfo_x86 *c);
-enum mcheck_type amd_k8_mcheck_init(struct cpuinfo_x86 *c);
-enum mcheck_type amd_f10_mcheck_init(struct cpuinfo_x86 *c);
-
+enum mcheck_type amd_mcheck_init(struct cpuinfo_x86 *c, bool_t bsp);
 enum mcheck_type intel_mcheck_init(struct cpuinfo_x86 *c, bool_t bsp);
 
 void intel_mcheck_timer(struct cpuinfo_x86 *c);

xen/arch/x86/cpu/mcheck/mce_amd.c +51 -0

60 lines not on this page · raw message

xen/arch/x86/cpu/mcheck/mce_amd.h +4 -0

diff -r 7abb25095de0 xen/arch/x86/cpu/mcheck/mce_amd.h
@@ -1,6 +1,10 @@
 #ifndef _MCHECK_AMD_H
 #define _MCHECK_AMD_H
 
+enum mcheck_type amd_k7_mcheck_init(struct cpuinfo_x86 *c);
+enum mcheck_type amd_k8_mcheck_init(struct cpuinfo_x86 *c);
+enum mcheck_type amd_f10_mcheck_init(struct cpuinfo_x86 *c);
+
 int mc_amd_recoverable_scan(uint64_t status);
 int mc_amd_addrcheck(uint64_t status, uint64_t misc, int addrtype);

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Jan Beulich

2012-10-25 14:57 UTC #4 lore raw

Quoted 14 lines

Quoted 1 line
Quoted 1 line · Christoph Egger
Quoted 12 lines · Christoph Egger
Quoted 10 lines · Jan Beulich
Quoted 5 lines
Quoted 1 line
Quoted 1 line · Christoph Egger
Add support for cpu notification chain.
This is useful with nested virtualization when Xen runs as l1 hypervisor.
This cleans up mce initialization cleanup as a side-effect.
Did you notice the fix I had done to your earlier patch before committing c/s 26106:1883c1d29de9? With the change here, mce_clear_banks will get allocated twice (once in generic code and once in AMD specific code), leaking one instance.

Quoted 5 lines

Quoted 1 line
Ugh... must be a merge botch.
Updated patch attached.
Signed-off-by Christoph Egger <[email protected]>

Subject and patch don't really fit together anymore. This is mostly cleanup (code movement) now, and I wonder whether this

xen/arch/x86/cpu/mcheck/mce_amd.c mc_amd_addrcheck(uint64_t status, uint64 · line 111

Quoted 8 lines

xen/arch/x86/cpu/mcheck/mce_amd.c +8 -0

+amd_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
+{
+    int rc = 0;
+
+    switch (action) {
+    case CPU_DYING:
+        clear_in_cr4(X86_CR4_MCE);
+        break;

if needed at all, isn't rather to be put in generic code (you might need to settle on this with the Intel folks). In any case it would need an explanation in the commit message.

Jan

Christoph Egger

author

2012-10-26 08:24 UTC #5 lore raw

Quoted 16 lines · Jan Beulich

Quoted 16 lines
Quoted 1 line
Quoted 1 line · Christoph Egger
Quoted 13 lines · Christoph Egger
Quoted 10 lines · Jan Beulich
Quoted 5 lines
Quoted 1 line
Quoted 1 line · Christoph Egger
Add support for cpu notification chain.
This is useful with nested virtualization when Xen runs as l1 hypervisor.
This cleans up mce initialization cleanup as a side-effect.
Did you notice the fix I had done to your earlier patch before committing c/s 26106:1883c1d29de9? With the change here, mce_clear_banks will get allocated twice (once in generic code and once in AMD specific code), leaking one instance.
Ugh... must be a merge botch.

Quoted 7 lines

Quoted 3 lines
Updated patch attached.
Signed-off-by Christoph Egger <[email protected]>
Subject and patch don't really fit together anymore. This is mostly cleanup (code movement) now, and I wonder whether this

Will adjust and resend.

Christoph

Signature · 6 lines

---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632