Re: [PATCH] ix86: re-do permanent disabling of x2apic

[Keir Fraser <keir@xen.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" <Christoph.Egger@amd.com> 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 <jbeulich@novell.com>
>> 
>> --- 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 <xen/smp.h>
>>  #include <asm/mach-default/mach_mpparse.h>
>> 
>> +#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);
> 
>