[PATCH] MCE: consolidate AMD initialization

[PATCH] MCE: consolidate AMD initialization

[Christoph Egger]

Move AMD specific initialization to AMD files.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

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

Re: [PATCH] MCE: consolidate AMD initialization

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 411 bytes --]

On 10/26/12 10:25, Christoph Egger wrote:
> 
> Move AMD specific initialization to AMD files.
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
> 


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

[-- Attachment #2: xen_mce_init.diff --]
[-- Type: text/plain, Size: 2855 bytes --]

diff -r 7abb25095de0 xen/arch/x86/cpu/mcheck/mce.c
--- a/xen/arch/x86/cpu/mcheck/mce.c	Thu Oct 25 13:15:19 2012 +0200
+++ b/xen/arch/x86/cpu/mcheck/mce.c	Fri Oct 26 08:50:17 2012 +0200
@@ -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)
 {
diff -r 7abb25095de0 xen/arch/x86/cpu/mcheck/mce.h
--- a/xen/arch/x86/cpu/mcheck/mce.h	Thu Oct 25 13:15:19 2012 +0200
+++ b/xen/arch/x86/cpu/mcheck/mce.h	Fri Oct 26 08:50:17 2012 +0200
@@ -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);
 enum mcheck_type intel_mcheck_init(struct cpuinfo_x86 *c, bool_t bsp);
 
 void intel_mcheck_timer(struct cpuinfo_x86 *c);
diff -r 7abb25095de0 xen/arch/x86/cpu/mcheck/mce_amd.c
--- a/xen/arch/x86/cpu/mcheck/mce_amd.c	Thu Oct 25 13:15:19 2012 +0200
+++ b/xen/arch/x86/cpu/mcheck/mce_amd.c	Fri Oct 26 08:50:17 2012 +0200
@@ -98,3 +98,28 @@ mc_amd_addrcheck(uint64_t status, uint64
     BUG();
     return 0;
 }
+
+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;
+}
diff -r 7abb25095de0 xen/arch/x86/cpu/mcheck/mce_amd.h
--- a/xen/arch/x86/cpu/mcheck/mce_amd.h	Thu Oct 25 13:15:19 2012 +0200
+++ b/xen/arch/x86/cpu/mcheck/mce_amd.h	Fri Oct 26 08:50:17 2012 +0200
@@ -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);
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] MCE: consolidate AMD initialization

[Jan Beulich]

>>> On 26.10.12 at 10:26, Christoph Egger <Christoph.Egger@amd.com> wrote:
> On 10/26/12 10:25, Christoph Egger wrote:
>> 
>> Move AMD specific initialization to AMD files.
>> 
>> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

Let's do this properly: There's no K7 supporting 64-bit mode afaict,
so rather than moving around the call to amd_k7_mcheck_init()
can't we just drop it and the whole (inconsistently named) k7.c file?

Also (not in this patch of course), I'd prefer mce_amd_quirks.c
to get merged into mce_amd.c now that we have the latter.

Jan

Re: [PATCH] MCE: consolidate AMD initialization

[Christoph Egger]

On 10/29/12 10:48, Jan Beulich wrote:
>>>> On 26.10.12 at 10:26, Christoph Egger <Christoph.Egger@amd.com> wrote:
>> On 10/26/12 10:25, Christoph Egger wrote:
>>>
>>> Move AMD specific initialization to AMD files.
>>>
>>> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
> 
> Let's do this properly: There's no K7 supporting 64-bit mode afaict,
> so rather than moving around the call to amd_k7_mcheck_init()
> can't we just drop it and the whole (inconsistently named) k7.c file?

Yes.

> Also (not in this patch of course), I'd prefer mce_amd_quirks.c
> to get merged into mce_amd.c now that we have the latter.

Ok.

Christoph


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

Re: [PATCH] MCE: consolidate AMD initialization

[Christoph Egger]

On 10/29/12 10:48, Jan Beulich wrote:
>>>> On 26.10.12 at 10:26, Christoph Egger <Christoph.Egger@amd.com> wrote:
>> On 10/26/12 10:25, Christoph Egger wrote:
>>>
>>> Move AMD specific initialization to AMD files.
>>>
>>> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
> 
> Let's do this properly: There's no K7 supporting 64-bit mode afaict,
> so rather than moving around the call to amd_k7_mcheck_init()
> can't we just drop it and the whole (inconsistently named) k7.c file?

I think it is better to apply this first and then remove k7 to
simplify backporting if needed/wanted.

> Also (not in this patch of course), I'd prefer mce_amd_quirks.c
> to get merged into mce_amd.c now that we have the latter.

After some thinking is there some good reason to do this?

Christoph


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

Re: [PATCH] MCE: consolidate AMD initialization

[Jan Beulich]

>>> On 29.10.12 at 11:03, Christoph Egger <Christoph.Egger@amd.com> wrote:
> On 10/29/12 10:48, Jan Beulich wrote:
>>>>> On 26.10.12 at 10:26, Christoph Egger <Christoph.Egger@amd.com> wrote:
>>> On 10/26/12 10:25, Christoph Egger wrote:
>>>>
>>>> Move AMD specific initialization to AMD files.
>>>>
>>>> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
>> 
>> Let's do this properly: There's no K7 supporting 64-bit mode afaict,
>> so rather than moving around the call to amd_k7_mcheck_init()
>> can't we just drop it and the whole (inconsistently named) k7.c file?
> 
> I think it is better to apply this first and then remove k7 to
> simplify backporting if needed/wanted.

I'm not seeing these changes as backporting candidates.

>> Also (not in this patch of course), I'd prefer mce_amd_quirks.c
>> to get merged into mce_amd.c now that we have the latter.
> 
> After some thinking is there some good reason to do this?

Imo it had been there simply because there was no mce_amd.c at
the time it got introduced (and afaics it should nevertheless have
been named mce_amd.c from the beginning).

Jan

Re: [PATCH] MCE: consolidate AMD initialization

[Christoph Egger]

On 10/29/12 11:22, Jan Beulich wrote:
>>>> On 29.10.12 at 11:03, Christoph Egger <Christoph.Egger@amd.com> wrote:
>> On 10/29/12 10:48, Jan Beulich wrote:
>>>>>> On 26.10.12 at 10:26, Christoph Egger <Christoph.Egger@amd.com> wrote:
>>>> On 10/26/12 10:25, Christoph Egger wrote:
>>>>>
>>>>> Move AMD specific initialization to AMD files.
>>>>>
>>>>> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
>>>
>>> Let's do this properly: There's no K7 supporting 64-bit mode afaict,
>>> so rather than moving around the call to amd_k7_mcheck_init()
>>> can't we just drop it and the whole (inconsistently named) k7.c file?
>>
>> I think it is better to apply this first and then remove k7 to
>> simplify backporting if needed/wanted.
> 
> I'm not seeing these changes as backporting candidates.

I had SLES in mind.

>>> Also (not in this patch of course), I'd prefer mce_amd_quirks.c
>>> to get merged into mce_amd.c now that we have the latter.
>>
>> After some thinking is there some good reason to do this?
> 
> Imo it had been there simply because there was no mce_amd.c at
> the time it got introduced (and afaics it should nevertheless have
> been named mce_amd.c from the beginning).

I have a patch ready that removes k7 support which is on top of this
init cleanup patch.
I also have a patch which merges mce_amd_quirks into mce_amd.c on
top of the k7 removal.
In which order do you want them?

Christoph


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

Re: [PATCH] MCE: consolidate AMD initialization

[Jan Beulich]

>>> On 29.10.12 at 11:32, Christoph Egger <Christoph.Egger@amd.com> wrote:
> On 10/29/12 11:22, Jan Beulich wrote:
>>>>> On 29.10.12 at 11:03, Christoph Egger <Christoph.Egger@amd.com> wrote:
>>> On 10/29/12 10:48, Jan Beulich wrote:
>>>>>>> On 26.10.12 at 10:26, Christoph Egger <Christoph.Egger@amd.com> wrote:
>>>>> On 10/26/12 10:25, Christoph Egger wrote:
>>>>>>
>>>>>> Move AMD specific initialization to AMD files.
>>>>>>
>>>>>> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
>>>>
>>>> Let's do this properly: There's no K7 supporting 64-bit mode afaict,
>>>> so rather than moving around the call to amd_k7_mcheck_init()
>>>> can't we just drop it and the whole (inconsistently named) k7.c file?
>>>
>>> I think it is better to apply this first and then remove k7 to
>>> simplify backporting if needed/wanted.
>> 
>> I'm not seeing these changes as backporting candidates.
> 
> I had SLES in mind.
> 
>>>> Also (not in this patch of course), I'd prefer mce_amd_quirks.c
>>>> to get merged into mce_amd.c now that we have the latter.
>>>
>>> After some thinking is there some good reason to do this?
>> 
>> Imo it had been there simply because there was no mce_amd.c at
>> the time it got introduced (and afaics it should nevertheless have
>> been named mce_amd.c from the beginning).
> 
> I have a patch ready that removes k7 support which is on top of this
> init cleanup patch.
> I also have a patch which merges mce_amd_quirks into mce_amd.c on
> top of the k7 removal.
> In which order do you want them?

Okay, if you got them done already, let's go with the order you
have.

Jan

Re: [PATCH] MCE: consolidate AMD initialization

[Christoph Egger]

On 10/29/12 11:49, Jan Beulich wrote:
>>>> On 29.10.12 at 11:32, Christoph Egger <Christoph.Egger@amd.com> wrote:
>> On 10/29/12 11:22, Jan Beulich wrote:
>>>>>> On 29.10.12 at 11:03, Christoph Egger <Christoph.Egger@amd.com> wrote:
>>>> On 10/29/12 10:48, Jan Beulich wrote:
>>>>>>>> On 26.10.12 at 10:26, Christoph Egger <Christoph.Egger@amd.com> wrote:
>>>>>> On 10/26/12 10:25, Christoph Egger wrote:
>>>>>>>
>>>>>>> Move AMD specific initialization to AMD files.
>>>>>>>
>>>>>>> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
>>>>>
>>>>> Let's do this properly: There's no K7 supporting 64-bit mode afaict,
>>>>> so rather than moving around the call to amd_k7_mcheck_init()
>>>>> can't we just drop it and the whole (inconsistently named) k7.c file?
>>>>
>>>> I think it is better to apply this first and then remove k7 to
>>>> simplify backporting if needed/wanted.
>>>
>>> I'm not seeing these changes as backporting candidates.
>>
>> I had SLES in mind.
>>
>>>>> Also (not in this patch of course), I'd prefer mce_amd_quirks.c
>>>>> to get merged into mce_amd.c now that we have the latter.
>>>>
>>>> After some thinking is there some good reason to do this?
>>>
>>> Imo it had been there simply because there was no mce_amd.c at
>>> the time it got introduced (and afaics it should nevertheless have
>>> been named mce_amd.c from the beginning).
>>
>> I have a patch ready that removes k7 support which is on top of this
>> init cleanup patch.
>> I also have a patch which merges mce_amd_quirks into mce_amd.c on
>> top of the k7 removal.
>> In which order do you want them?
> 
> Okay, if you got them done already, let's go with the order you
> have.

Patches sent.

Christoph


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