| 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
| ||
|
Date: Tue, 09 Mar 2010 10:14:18 +0100
From: "kerstin.jonsson" <kerstin.jonsson@...csson.com>
To: Thomas Renninger <trenn@...e.de>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jbohac@...ell.com" <jbohac@...ell.com>,
Yinghai Lu <yinghai@...nel.org>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"mingo@...e.hu" <mingo@...e.hu>, Avi Kivity <avi@...hat.com>
Subject: Re: [PATCH] x86 apic: Ack all pending irqs when crashed/on kexec
- V4
On 03/08/2010 12:43 PM, Thomas Renninger wrote:
> From: Kerstin Jonsson<kerstin.jonsson@...csson.com>
>
> When the SMP kernel decides to crash_kexec() the local APICs may have
> pending interrupts in their vector tables.
> The setup routine for the local APIC has a deficient mechanism for
> clearing these interrupts, it only handles interrupts that has already
> been dispatched to the local core for servicing (the ISR register)
> safely, it doesn't consider lower prioritized queued interrupts stored
> in the IRR register.
>
> If you have more than one pending interrupt within the same 32 bit word
> in the LAPIC vector table registers you may find yourself entering the
> IO APIC setup with pending interrupts left in the LAPIC. This is a
> situation for wich the IO APIC setup is not prepared. Depending of
> what/which interrupt vector/vectors are stuck in the APIC tables your
> system may show various degrees of malfunctioning.
> That was the reason why the check_timer() failed in our system, the
> timer interrupts was blocked by pending interrupts from the old kernel
> when routed trough the IO APIC.
>
> Additional comment from Jiri Bohac:
> ==============
> If this should go into stable release,
> I'd add some kind of limit on the number of iterations, just to be safe from
> hard to debug lock-ups:
>
> +if (loops++> MAX_LOOPS) {
> + printk("LAPIC pending clean-up")
> + break;
> +}
> while (queued);
>
> with MAX_LOOPS something like 1E9 this would leave plenty of time for the
> pending IRQs to be cleared and would and still cause at most a second of delay
> if the loop were to lock-up for whatever reason.
> ==============
>
> From trenn@...e.de:
> V2: Use tsc if avail to bail out after 1 sec due to possible virtual apic_read
> calls which may take rather long (suggested by: Avi Kivity<avi@...hat.com>)
> If no tsc is available bail out quickly after cpu_khz, if we broke out too
> early and still have irqs pending (which should never happen?) we still
> get a WARN_ON...
>
> V3: - Fixed indentation -> checkpatch clean
> - max_loops must be signed
>
> V4: - Fix typo, mixed up tsc and ntsc in first rdtscll() call
>
> CC: jbohac@...ell.com
> CC: "Yinghai Lu"<yinghai@...nel.org>
> CC: akpm@...ux-foundation.org
> CC: mingo@...e.hu
> CC: "Kerstin Jonsson"<kerstin.jonsson@...csson.com>
> CC: "Avi Kivity"<avi@...hat.com>
> Signed-off-by: Thomas Renninger<trenn@...e.de>
> ---
> arch/x86/kernel/apic/apic.c | 41 +++++++++++++++++++++++++++++++++--------
> 1 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 3987e44..414a5df 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -51,6 +51,7 @@
> #include<asm/smp.h>
> #include<asm/mce.h>
> #include<asm/kvm_para.h>
> +#include<asm/tsc.h>
>
> unsigned int num_processors;
>
> @@ -1151,8 +1152,13 @@ static void __cpuinit lapic_setup_esr(void)
> */
> void __cpuinit setup_local_APIC(void)
> {
> - unsigned int value;
> - int i, j;
> + unsigned int value, queued;
> + int i, j, acked = 0;
> + unsigned long long tsc = 0, ntsc;
> + long long max_loops = cpu_khz;
> +
> + if (cpu_has_tsc)
> + rdtscll(tsc);
>
> if (disable_apic) {
> arch_disable_smp_support();
> @@ -1204,13 +1210,32 @@ void __cpuinit setup_local_APIC(void)
> * the interrupt. Hence a vector might get locked. It was noticed
> * for timer irq (vector 0x31). Issue an extra EOI to clear ISR.
> */
> - for (i = APIC_ISR_NR - 1; i>= 0; i--) {
> - value = apic_read(APIC_ISR + i*0x10);
> - for (j = 31; j>= 0; j--) {
> - if (value& (1<<j))
> - ack_APIC_irq();
> + do {
> + queued = 0;
> + for (i = APIC_ISR_NR - 1; i>= 0; i--)
> + queued |= apic_read(APIC_IRR + i*0x10);
> +
> + for (i = APIC_ISR_NR - 1; i>= 0; i--) {
> + value = apic_read(APIC_ISR + i*0x10);
> + for (j = 31; j>= 0; j--) {
> + if (value& (1<<j)) {
> + ack_APIC_irq();
> + acked++;
> + }
> + }
> }
> - }
> + if (acked> 256) {
> + printk(KERN_ERR "LAPIC pending interrupts after %d EOI\n",
> + acked);
> + break;
> + }
> + if (cpu_has_tsc) {
> + rdtscll(ntsc);
> + max_loops = (cpu_khz<< 10) - (ntsc - tsc);
> + } else
> + max_loops--;
> + } while (queued&& max_loops> 0);
> + WARN_ON(!max_loops);
>
> /*
> * Now that we are all set up, enable the APIC
>
On the verge of being overzealous:
WARN_ON(!max_loops); max_loops< 0 will probably be the most common error exit in a system that has tsc...
--
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/
Powered by blists - more mailing lists