From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] ix86: re-do permanent disabling of x2apic Date: Thu, 10 Feb 2011 14:17:44 +0000 Message-ID: References: <201102101505.01536.Christoph.Egger@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <201102101505.01536.Christoph.Egger@amd.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Christoph Egger , xen-devel@lists.xensource.com Cc: Gang Wei , Jan Beulich List-Id: xen-devel@lists.xenproject.org In this case I've just removed some of the ifdefs before checking in. I really don't care about dead code in the 32-bit target, and especially when it's mostly in __init-annotated functions. -- Keir On 10/02/2011 14:05, "Christoph Egger" wrote: > > Wouldn't it be better to factor out 32bit and 64bit functions into new files > in the x86_32 and x86_64 subdirectories and just call them from here? > > On Thursday 10 February 2011 14:50:15 Jan Beulich wrote: >> Move logic into check_x2apic_preenabled() (to make sure >> generic_apic_probe() doesn't see genapic already set) and disable dead >> code on ix86. >> >> Signed-off-by: Jan Beulich >> >> --- a/xen/arch/x86/apic.c >> +++ b/xen/arch/x86/apic.c >> @@ -71,10 +71,12 @@ static int enable_local_apic __initdata >> */ >> int apic_verbosity; >> >> +#ifndef __i386__ >> static bool_t __initdata opt_x2apic = 1; >> boolean_param("x2apic", opt_x2apic); >> >> bool_t __read_mostly x2apic_enabled = 0; >> +#endif >> bool_t __read_mostly directed_eoi_enabled = 0; >> >> /* >> @@ -962,20 +964,8 @@ void __init x2apic_bsp_setup(void) >> return; >> >> #ifdef __i386__ >> - clear_bit(X86_FEATURE_X2APIC, boot_cpu_data.x86_capability); >> - if ( x2apic_enabled ) >> - { >> - uint64_t msr_content; >> - rdmsrl(MSR_IA32_APICBASE, msr_content); >> - msr_content &= ~(MSR_IA32_APICBASE_ENABLE | >> MSR_IA32_APICBASE_EXTD); - wrmsrl(MSR_IA32_APICBASE, msr_content); >> - msr_content |= MSR_IA32_APICBASE_ENABLE; >> - wrmsrl(MSR_IA32_APICBASE, msr_content); >> - x2apic_enabled = 0; >> - } >> - printk("x2APIC disabled permanently on x86_32.\n"); >> - return; >> -#endif >> + BUG(); >> +#else >> >> if ( !opt_x2apic ) >> { >> @@ -1038,6 +1028,7 @@ restore_out: >> unmask_8259A(); >> >> out: >> +#endif /* !__i386__ */ >> if ( ioapic_entries ) >> free_ioapic_entries(ioapic_entries); >> } >> --- a/xen/arch/x86/genapic/x2apic.c >> +++ b/xen/arch/x86/genapic/x2apic.c >> @@ -29,6 +29,8 @@ >> #include >> #include >> >> +#ifndef __i386__ >> + >> static bool_t __initdata x2apic_phys; /* By default we use logical cluster >> mode. */ boolean_param("x2apic_phys", x2apic_phys); >> >> @@ -126,6 +128,8 @@ const struct genapic *__init apic_x2apic >> return x2apic_phys ? &apic_x2apic_phys : &apic_x2apic_cluster; >> } >> >> +#endif /* !__i386__ */ >> + >> void __init check_x2apic_preenabled(void) >> { >> u32 lo, hi; >> @@ -138,7 +142,19 @@ void __init check_x2apic_preenabled(void >> if ( lo & MSR_IA32_APICBASE_EXTD ) >> { >> printk("x2APIC mode is already enabled by BIOS.\n"); >> +#ifndef __i386__ >> x2apic_enabled = 1; >> genapic = apic_x2apic_probe(); >> +#else >> + lo &= ~(MSR_IA32_APICBASE_ENABLE | MSR_IA32_APICBASE_EXTD); >> + wrmsr(MSR_IA32_APICBASE, lo, hi); >> + lo |= MSR_IA32_APICBASE_ENABLE; >> + wrmsr(MSR_IA32_APICBASE, lo, hi); >> + printk("x2APIC disabled permanently on x86_32.\n"); >> +#endif >> } >> + >> +#ifdef __i386__ >> + clear_bit(X86_FEATURE_X2APIC, boot_cpu_data.x86_capability); >> +#endif >> } >> --- a/xen/include/asm-x86/apic.h >> +++ b/xen/include/asm-x86/apic.h >> @@ -22,7 +22,11 @@ >> #define IO_APIC_REDIR_DEST_PHYSICAL 0x00000 >> >> extern int apic_verbosity; >> +#ifdef __i386__ >> +#define x2apic_enabled 0 >> +#else >> extern bool_t x2apic_enabled; >> +#endif >> extern bool_t directed_eoi_enabled; >> >> void check_x2apic_preenabled(void); > >