linux-kernel - Re: [PATCH] x86: fix wakeup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4921AFEB.4060605@kernel.org>
Date:	Mon, 17 Nov 2008 09:54:51 -0800
From:	Yinghai Lu <yinghai@...nel.org>
To:	Ingo Molnar <mingo@...e.hu>
CC:	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86: fix wakeup_cpu with numaq/es7000 v2

Ingo Molnar wrote:
> * Yinghai Lu <yinghai@...nel.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/