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] DOM0: Adding MCA Loging support in DOM0

To: 'Jan Beulich' <JBeulich@xxxxxxxxxx>, "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH] DOM0: Adding MCA Loging support in DOM0
From: "Ke, Liping" <liping.ke@xxxxxxxxx>
Date: Wed, 10 Jun 2009 17:14:22 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 10 Jun 2009 02:15:39 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <3D611D48027E5D42999A29059180DD7101D6675824@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
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: <3D611D48027E5D42999A29059180DD7101D6675824@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acnpo8cFaOPuw/GnTg6Hh8EXkLZe1AAAQCcgAAD++iAAAMCysA==
Thread-topic: [Xen-devel] [PATCH] DOM0: Adding MCA Loging support in DOM0
Hi, Jan

When enabing mce in DOM0,  we only want to fully reuse machine check handler in 
mce.c as
guest virtual mca handler. 
mce_intel.c and mce_amd.c contain two AMD and Intel hardware features which is 
no use to (both mce virq log and virtual mca enabling) actually.

Problem is that mce_XXX_feature_init in (mce_intel.c and mce_amd.c) is called 
in mce.c 
when do mce init. So how about:
1) Make those two files only compile under native linux
2) in mce.c, don't call mce_XXX_feature_init under XEN envrionment? 

Then many of problems will not exist, please see below answers.

Thanks!
Criping

Jan Beulich wrote:
>>>> "Ke, Liping" <liping.ke@xxxxxxxxx> 10.06.09 08:52 >>>
>> According to the discussion result, I modified the patch
>> 1)  use Kconfig to identify when mce_dom.c and mce_XXX.c will be
>>     built. mce_dom.c will only be build when it is under XEN MCE
>> environment. 2) virq bind function will only be called when
>> mce_dom.c is used. 
>> 
>> I test the compiling both under native/dom0 with all possible
>> combinations. 
> 
> I think that's not covering all the issue I pointed out:
> 
>> --- a/arch/x86_64/Kconfig    Tue Jun 09 09:58:11 2009 +0800
>> +++ b/arch/x86_64/Kconfig    Wed Jun 10 14:12:55 2009 +0800 @@ -472,7
>> +472,6 @@ 
>> 
>> config X86_MCE
>>      bool "Machine check support" if EMBEDDED
>> -    depends on !X86_64_XEN
>>      default y
>>      help
>>         Include a machine check error handler to report hardware errors.
> 
> Shouldn't this rather change the dependency to XEN_PRIVILEGED_GUEST?
> 
Native depends on this config too, so have about change it like this
depends on   !X86_64_XEN || ( X86_64_XEN && XEN_PRIVILEGED_GUEST)

>> @@ -483,7 +482,7 @@
>> config X86_MCE_INTEL
>>      bool "Intel MCE features"
>>      depends on X86_MCE && X86_LOCAL_APIC
>> -    default y
>> +    default n
>>      help
>>         Additional support for intel specific MCE features such as
>>         the thermal monitor.
> 
> This hunk should go.

This option will not appear under XEN, only for native?
depends on X86_MCE && X86_LOCAL_APIC && !X86_64_XEN

> 
>> @@ -491,11 +490,15 @@
>> config X86_MCE_AMD
>>      bool "AMD MCE features"
>>      depends on X86_MCE && X86_LOCAL_APIC
>> -    default y
>> +    default n
>>      help
>>         Additional support for AMD specific MCE features such as
>>         the DRAM Error Threshold.
> 
> And likewise this part.
> 
This option will not appear under XEN, only for native.
depends on X86_MCE && X86_LOCAL_APIC && !X86_64_XEN

So above two files will not be compiled for DOM0. Compiling hunks for them will 
not be needed any more.

>> 
>> +config X86_64_XEN_MCE
>> +    def_bool y
>> +    depends on X86_64_XEN && (X86_MCE_INTEL || X86_MCE_AMD)
>> +
> 
> I wonder if this wouldn't better be named X86_XEN_MCE (for consistency
> with a potential 32-bit implementation).
> 

This option will depends on X86_64_XEN && X86_MCE.
Yes, we could rename it. Yet this config file is under X86_64 sub dir, also, 
seems 32bit system will not 
have mca support?

>> config KEXEC
>>      bool "kexec system call (EXPERIMENTAL)"
>>      depends on EXPERIMENTAL && !XEN_UNPRIVILEGED_GUEST ...
>> --- a/arch/x86_64/kernel/apic-xen.c  Tue Jun 09 09:58:11 2009 +0800
>> +++ b/arch/x86_64/kernel/apic-xen.c  Wed Jun 10 14:12:55 2009 +0800
>> @@ -195,3 +195,13 @@ 
>> 
>>      return 1;
>> }
>> +
>> +/* Pull back for compiling arch/x86_64/kernel/mce_amd.c */
>> +void setup_APIC_extened_lvt(unsigned char lvt_off, unsigned char
>> vector, +                        unsigned char msg_type, unsigned char mask) 
>> +{
>> +    unsigned long reg = (lvt_off << 4) + K8_APIC_EXT_LVT_BASE;
>> +    unsigned int  v   = (mask << 16) | (msg_type << 8) | vector;
>> +    apic_write(reg, v); +}
> 
> This continues to be wrong (yes, I realize
> arch/x86_64/kernel/apic-xen.c is full of such broken code, but that
> doesn't mean more should be added). 
> 

We no long need this change.

>> ...
>> --- a/arch/x86_64/kernel/mce.c       Tue Jun 09 09:58:11 2009 +0800
>> +++ b/arch/x86_64/kernel/mce.c       Wed Jun 10 14:12:55 2009 +0800 @@
>> -165,7 +165,7 @@ 
>>  * The actual machine check handler
>>  */
>> 
>> -void do_machine_check(struct pt_regs * regs, long error_code)
>> +asmlinkage void do_machine_check(struct pt_regs * regs, long
>>      error_code) { struct mce m, panicm;
>>      int nowayout = (tolerant < 1);
> 
> Why?

Yes, this change is not necessary, I will remove it.
> 
>> @@ -276,9 +276,16 @@
>> 
>> /*
>>  * Periodic polling timer for "silent" machine check errors. - */
>> + * We will disable polling in DOM0 since all CMCI/Polling
>> + * mechanism will be done in XEN for Intel CPUs
>> +*/
>> 
>> +#if defined (CONFIG_XEN) && defined(CONFIG_X86_MCE_INTEL)
>> +static int check_interval = 0; /* disable polling */ +#else
>> static int check_interval = 5 * 60; /* 5 minutes */ +#endif
>> +
>> static void mcheck_timer(void *data);
>> static DECLARE_WORK(mcheck_work, mcheck_timer, NULL);
> 
> Shouldn't that now simply be #ifdef CONFIG_X86_64_XEN_MCE?
> 

yes, you're right. Missed this chunk.

>> ...
>> --- a/include/asm-x86_64/mach-xen/asm/hw_irq.h       Tue Jun 09 09:58:11
>> 2009 +0800 +++ b/include/asm-x86_64/mach-xen/asm/hw_irq.h    Wed Jun 10
>> 14:12:55 2009 +0800 @@ -60,6 +60,9 @@ #define
>> NUM_INVALIDATE_TLB_VECTORS   8 #endif
>> 
>> +/* Pull back for compiling arch/x86_64/kernel/mce_amd.c */
>> +#define THRESHOLD_APIC_VECTOR   0xf9
>> +#define THERMAL_APIC_VECTOR 0xfa
>> /*
>>  * Local APIC timer IRQ vector is on a different priority level,
>>  * to work around the 'lost local interrupt if more than 2 IRQ
> 
> These vectors mean nothing under Xen.

no longer need this change.

> 
> Jan

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

<Prev in Thread] Current Thread [Next in Thread>
  • RE: [Xen-devel] [PATCH] DOM0: Adding MCA Loging support in DOM0, Ke, Liping <=