linux-kernel - Re: [PATCH] 2.6.18-rt7: PowerPC: fix breakage in threaded fasteoi type IRQ handlers

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: <4561F43B.40000@ru.mvista.com>
Date:	Mon, 20 Nov 2006 21:30:19 +0300
From:	Sergei Shtylyov <sshtylyov@...mvista.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	linuxppc-dev@...abs.org, linux-kernel@...r.kernel.org,
	dwalker@...sta.com
Subject: Re: [PATCH] 2.6.18-rt7: PowerPC: fix breakage in threaded fasteoi
 type IRQ handlers

Hello.

Ingo Molnar wrote:

>>i'm hacking up something now to see whether it makes sense to 
>>introduce a central threaded flow type, or whether it's better the 
>>branch off the current flow types (as the code does it right now).

> ok, a central flow type caused more problems than good - the main 
> complication is that the handler needs to know the true 'flow' 
> (edge/level/fasteoi, etc.) anyway, even in the threaded case.

> So i rather went on making the existing flow handlers more 
> threading-friendly, and undoing the x86_64 and i386 arch changes to make 
> sure that the default handlers all work fine. Does the patch below 
> (against -rt4) do the trick for your on PPC too?

    Not without my patch.

> 	Ingo
> 
> Index: linux/arch/i386/kernel/io_apic.c
> ===================================================================
> --- linux.orig/arch/i386/kernel/io_apic.c
> +++ linux/arch/i386/kernel/io_apic.c
> @@ -1272,22 +1272,12 @@ static struct irq_chip ioapic_chip;
>  static void ioapic_register_intr(int irq, int vector, unsigned long trigger)
>  {
>  	if ((trigger == IOAPIC_AUTO && IO_APIC_irq_trigger(irq)) ||
> -			trigger == IOAPIC_LEVEL) {
> -#ifdef CONFIG_PREEMPT_HARDIRQS
> +			trigger == IOAPIC_LEVEL)
>  		set_irq_chip_and_handler_name(irq, &ioapic_chip,
> -					    handle_level_irq, "level-threaded");
> -#else
> -		set_irq_chip_and_handler_name(irq, &ioapic_chip,
> -					      handle_fasteoi_irq, "fasteoi");
> -#endif
> -	} else {
> -#ifdef CONFIG_PREEMPT_HARDIRQS
> +					 handle_fasteoi_irq, "fasteoi");
> +	else {
>  		set_irq_chip_and_handler_name(irq, &ioapic_chip,
> -					      handle_edge_irq, "edge-threaded");
> -#else
> -		set_irq_chip_and_handler_name(irq, &ioapic_chip,
> -					      handle_edge_irq, "edge");
> -#endif
> +					 handle_edge_irq, "edge");

    Hm, why force edge flow on edge-triggered IRQs?

>  	}
>  	set_intr_gate(vector, interrupt[irq]);
>  }
> Index: linux/arch/x86_64/kernel/io_apic.c
> ===================================================================
> --- linux.orig/arch/x86_64/kernel/io_apic.c
> +++ linux/arch/x86_64/kernel/io_apic.c
> @@ -787,22 +787,12 @@ static struct irq_chip ioapic_chip;
>  static void ioapic_register_intr(int irq, int vector, unsigned long trigger)
>  {
>  	if ((trigger == IOAPIC_AUTO && IO_APIC_irq_trigger(irq)) ||
> -			trigger == IOAPIC_LEVEL) {
> -#ifdef CONFIG_PREEMPT_HARDIRQS
> -		set_irq_chip_and_handler_name(irq, &ioapic_chip,
> -					    handle_level_irq, "level-threaded");
> -#else
> +			trigger == IOAPIC_LEVEL)
>  		set_irq_chip_and_handler_name(irq, &ioapic_chip,
>  					      handle_fasteoi_irq, "fasteoi");
> -#endif
> -	} else {
> -#ifdef CONFIG_PREEMPT_HARDIRQS
> -		set_irq_chip_and_handler_name(irq, &ioapic_chip,
> -					      handle_edge_irq, "edge-threaded");
> -#else
> +	else {
>  		set_irq_chip_and_handler_name(irq, &ioapic_chip,
>  					      handle_edge_irq, "edge");

     Same here...

> -#endif
>  	}
>  }
>  
> Index: linux/kernel/irq/chip.c
> ===================================================================
> --- linux.orig/kernel/irq/chip.c
> +++ linux/kernel/irq/chip.c
> @@ -238,8 +238,10 @@ static inline void mask_ack_irq(struct i
>  	if (desc->chip->mask_ack)
>  		desc->chip->mask_ack(irq);
>  	else {
> -		desc->chip->mask(irq);
> -		desc->chip->ack(irq);
> +		if (desc->chip->mask)
> +			desc->chip->mask(irq);
> +		if (desc->chip->mask)
> +			desc->chip->ack(irq);
>  	}
>  }

    Hmm, that just won't do for PPC threaded fasteoi flows! What you'll get is 
a threaded IRQ with EOI *never ever* issued, unless my PPC patch is also in...

WBR, Sergei
-
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/