WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] fix c/s 18938

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel] [PATCH] fix c/s 18938
From: Christoph Egger <Christoph.Egger@xxxxxxx>
Date: Fri, 27 Mar 2009 17:49:15 +0100
Cc:
Delivery-date: Fri, 27 Mar 2009 09:51:11 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <49CD0E0F.76E4.0078.0@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <49CD0E0F.76E4.0078.0@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: KMail/1.9.7
This patch looks very good to me. Just one comment at the bottom.

Christoph

On Friday 27 March 2009 17:34:07 Jan Beulich wrote:
> Provide for up to 16/32 on (32/64-bit) extended MCE MSRs, and use
> actually existing extended MSRs on 64-bits that were inaccessible so
> far.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
>
> --- 2009-03-27.orig/xen/arch/x86/cpu/mcheck/mce_intel.c       2009-03-27
> 16:43:26.000000000 +0100 +++
> 2009-03-27/xen/arch/x86/cpu/mcheck/mce_intel.c        2009-03-27
> 12:06:52.000000000 +0100 @@ -117,6 +117,16 @@ static void
> intel_init_thermal(struct cp
>  }
>  #endif /* CONFIG_X86_MCE_THERMAL */
>
> +static inline void intel_get_extended_msr(struct mcinfo_extended *ext, u32
> msr) +{
> +    if ( ext->mc_msrs < ARRAY_SIZE(ext->mc_msr)
> +         && msr < MSR_IA32_MCG_EAX + nr_intel_ext_msrs ) {
> +        ext->mc_msr[ext->mc_msrs].reg = msr;
> +        rdmsrl(msr, ext->mc_msr[ext->mc_msrs].value);
> +        ++ext->mc_msrs;
> +    }
> +}
> +
>  static enum mca_extinfo
>  intel_get_extended_msrs(struct mc_info *mci, uint16_t bank, uint64_t
> status) {
> @@ -129,30 +139,29 @@ intel_get_extended_msrs(struct mc_info *
>      memset(&mc_ext, 0, sizeof(struct mcinfo_extended));
>      mc_ext.common.type = MC_TYPE_EXTENDED;
>      mc_ext.common.size = sizeof(mc_ext);
> -    mc_ext.mc_msrs = 10;
>
> -    mc_ext.mc_msr[0].reg = MSR_IA32_MCG_EAX;
> -    rdmsrl(MSR_IA32_MCG_EAX, mc_ext.mc_msr[0].value);
> -    mc_ext.mc_msr[1].reg = MSR_IA32_MCG_EBX;
> -    rdmsrl(MSR_IA32_MCG_EBX, mc_ext.mc_msr[1].value);
> -    mc_ext.mc_msr[2].reg = MSR_IA32_MCG_ECX;
> -    rdmsrl(MSR_IA32_MCG_ECX, mc_ext.mc_msr[2].value);
> -
> -    mc_ext.mc_msr[3].reg = MSR_IA32_MCG_EDX;
> -    rdmsrl(MSR_IA32_MCG_EDX, mc_ext.mc_msr[3].value);
> -    mc_ext.mc_msr[4].reg = MSR_IA32_MCG_ESI;
> -    rdmsrl(MSR_IA32_MCG_ESI, mc_ext.mc_msr[4].value);
> -    mc_ext.mc_msr[5].reg = MSR_IA32_MCG_EDI;
> -    rdmsrl(MSR_IA32_MCG_EDI, mc_ext.mc_msr[5].value);
> -
> -    mc_ext.mc_msr[6].reg = MSR_IA32_MCG_EBP;
> -    rdmsrl(MSR_IA32_MCG_EBP, mc_ext.mc_msr[6].value);
> -    mc_ext.mc_msr[7].reg = MSR_IA32_MCG_ESP;
> -    rdmsrl(MSR_IA32_MCG_ESP, mc_ext.mc_msr[7].value);
> -    mc_ext.mc_msr[8].reg = MSR_IA32_MCG_EFLAGS;
> -    rdmsrl(MSR_IA32_MCG_EFLAGS, mc_ext.mc_msr[8].value);
> -    mc_ext.mc_msr[9].reg = MSR_IA32_MCG_EIP;
> -    rdmsrl(MSR_IA32_MCG_EIP, mc_ext.mc_msr[9].value);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_EAX);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_EBX);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_ECX);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_EDX);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_ESI);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_EDI);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_EBP);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_ESP);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_EFLAGS);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_EIP);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_MISC);
> +
> +#ifdef __x86_64__
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_R8);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_R9);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_R10);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_R11);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_R12);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_R13);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_R14);
> +    intel_get_extended_msr(&mc_ext, MSR_IA32_MCG_R15);
> +#endif
>
>      x86_mcinfo_add(mci, &mc_ext);
>
> --- 2009-03-27.orig/xen/include/asm-x86/msr-index.h   2009-03-27
> 16:43:26.000000000 +0100 +++
> 2009-03-27/xen/include/asm-x86/msr-index.h    2009-03-27 13:18:04.000000000
> +0100 @@ -326,7 +326,15 @@
>  #define MSR_IA32_MCG_ESP             0x00000187
>  #define MSR_IA32_MCG_EFLAGS          0x00000188
>  #define MSR_IA32_MCG_EIP             0x00000189
> -#define MSR_IA32_MCG_RESERVED                0x0000018a
> +#define MSR_IA32_MCG_MISC            0x0000018a
> +#define MSR_IA32_MCG_R8                      0x00000190
> +#define MSR_IA32_MCG_R9                      0x00000191
> +#define MSR_IA32_MCG_R10             0x00000192
> +#define MSR_IA32_MCG_R11             0x00000193
> +#define MSR_IA32_MCG_R12             0x00000194
> +#define MSR_IA32_MCG_R13             0x00000195
> +#define MSR_IA32_MCG_R14             0x00000196
> +#define MSR_IA32_MCG_R15             0x00000197
>
>  /* Pentium IV performance counter MSRs */
>  #define MSR_P4_BPU_PERFCTR0          0x00000300
> --- 2009-03-27.orig/xen/include/public/arch-x86/xen-mca.h     2009-03-27
> 16:47:30.000000000 +0100 +++
> 2009-03-27/xen/include/public/arch-x86/xen-mca.h      2009-03-27
> 16:48:35.000000000 +0100 @@ -166,11 +166,11 @@ struct mcinfo_extended {
>
>      uint32_t mc_msrs; /* Number of msr with valid values. */
>      /*
> -     * Currently Intel extended MSR (32/64) including all gp registers
> -     * and E(R)DI, E(R)BP, E(R)SP, E(R)FLAGS, E(R)IP, E(R)MISC, only 10
> -     * of them might be useful. So expend this array to 10.
> -    */
> -    struct mcinfo_msr mc_msr[10];
> +     * Currently Intel extended MSR (32/64) include all gp registers
> +     * and E(R)FLAGS, E(R)IP, E(R)MISC, up to 11/19 of them might be
> +     * useful at present. So expand this array to 16/32 to leave room.
> +     */
> +    struct mcinfo_msr mc_msr[sizeof(void *) * 4];

Please make this a fixed sized array. There are users like Oracle who run
a 32bit PAE Dom0 on a 64bit Xen ...

>  };
>
>  /* Recovery Action flags. Giving recovery result information to DOM0 */



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>