Messages in this thread | | | Date | Mon, 17 Nov 2008 09:54:51 -0800 | From | Yinghai Lu <> | Subject | Re: [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
| |