lkml.org 
[lkml]   [2008]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86: fix wakeup_cpu with numaq/es7000 v2
Ingo Molnar wrote:
> * Yinghai Lu <yinghai@kernel.org> wrote:
>
>> Impact: fix wakeup path with numaq and es7000
>
> applied to tip/x86/quirks, thanks Yinghai!
>
> A couple of comments:
>
>> +static inline void inquire_remote_apic(int apicid)
>> +{
>> + if (apic_verbosity >= APIC_DEBUG)
>> + __inquire_remote_apic(apicid);
>> +}
>
> could you please make this a non-include-file function and only
> install the function vector if the platform necessiates it?

good idea, after genapic get assigned,
update the genapic .inquire_remote_apic according to apic_verbosity...

>
> I.e.:
>
>> + void (*wait_for_init_deassert)(atomic_t *deassert);
>> + void (*smp_callin_clear_local_apic)(void);
>> + void (*store_NMI_vector)(unsigned short *high, unsigned short *low);
>> + void (*restore_NMI_vector)(unsigned short *high, unsigned short *low);
>> + void (*inquire_remote_apic)(int apicid);
>
> should have NULL in the 'default' case - and only have some exception
> (quick) function installed in the non-default case.
>
> That way the 3 separate wait_for_init_deassert() and
> inquire_remote_apic() inline functions can be removed and the
> exception be moved to es7000_32.c and numaq_32.c.
>

will look at it...

> like you did it here properly to install the MIP-init sequence in the
> es7000 case:
>
>> Index: linux-2.6/arch/x86/kernel/es7000_32.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/kernel/es7000_32.c
>> +++ linux-2.6/arch/x86/kernel/es7000_32.c
>> @@ -40,6 +40,7 @@
>> #include <asm/smp.h>
>> #include <asm/apicdef.h>
>> #include <mach_mpparse.h>
>> +#include <asm/setup.h>
>>
>> /*
>> * ES7000 chipsets
>> @@ -161,6 +162,26 @@ es7000_rename_gsi(int ioapic, int gsi)
>> return gsi;
>> }
>>
>> +#ifdef CONFIG_ES7000_CLUSTERED_APIC
>> +static int wakeup_secondary_cpu_via_mip(int cpu, unsigned long eip)
>> +{
>> + unsigned long vect = 0, psaival = 0;
>> +
>> + if (psai == NULL)
>> + return -1;
>> +
>> + vect = ((unsigned long)__pa(eip)/0x1000) << 16;
>> + psaival = (0x1000000 | vect | cpu);
>> +
>> + while (*psai & 0x1000000)
>> + ;
>> +
>> + *psai = psaival;
>> +
>> + return 0;
>> +}
>> +#endif
>> +
>> void __init
>> setup_unisys(void)
>> {
>> @@ -176,6 +197,9 @@ setup_unisys(void)
>> else
>> es7000_plat = ES7000_CLASSIC;
>> ioapic_renumber_irq = es7000_rename_gsi;
>> +#ifdef CONFIG_ES7000_CLUSTERED_APIC
>> + x86_quirks->wakeup_secondary_cpu = wakeup_secondary_cpu_via_mip;
>> +#endif
>> }
>
> btw:
>
> shouldnt CONFIG_ES7000_CLUSTERED_APIC be converted to a proper genapic
> variant (like we have in apic.c), instead of this #ifdef driven
> mechanism?

you are right, need to autodetect that. and we can reuse one in mach_default/mach_wakecpu.h

YH


\
 
 \ /
  Last update: 2008-11-17 18:57    [W:0.072 / U:1.408 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site