Messages in this thread | | | Date | Mon, 17 Nov 2008 17:52:24 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH] x86: fix wakeup_cpu with numaq/es7000 v2 |
| |
* 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?
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.
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?
Ingo
| |