linux-kernel - Re: Reworking suspend-resume sequence (was: Re: PCI PM: Restore standard config registers of all devices early)

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]

Date:	Tue, 3 Feb 2009 20:53:04 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>
Cc:	Jesse Barnes <jesse.barnes@...el.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andreas Schwab <schwab@...e.de>, Len Brown <lenb@...nel.org>
Subject: Re: Reworking suspend-resume sequence (was: Re: PCI PM: Restore
	standard config registers of all devices early)


* Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Tue, 3 Feb 2009, Ingo Molnar wrote:
> > > 
> > > I'm really not sure why that handle_edge_irq thing uses "ack_and_mask()" 
> > > instead of just "desc->chip->ack()"? I'm also totally flummoxed as to why 
> > > it feels it needs to go all the way out to the device to mask things, 
> > > instead of just masking at an apic level, which is much simpler and faster 
> > > (especially since masking should never happen in practice anyway).
> > 
> > Hm, do you mean mask_ack_irq()?
> 
> Yes.
> 
> >		 The ->mask_ack() irqchip method is just a 
> > small tweak normally: if we get an irq while the irq was disabled, we can 
> > mark it pending and masks it for real.
> 
> No, I know why mask_ack_irq() does what it does and I agree with it. What 
> I was really reacting to was that handle_edge_irq() calls it at _all_. 
> IOW, I'm talking about this code:
> 
> 	handle_edge_irq(unsigned int irq, struct irq_desc *desc)
> 	...
> 	        if (unlikely((desc->status & (IRQ_INPROGRESS | IRQ_DISABLED)) ||
> 	                    !desc->action)) {
> 	                desc->status |= (IRQ_PENDING | IRQ_MASKED);
> 	                mask_ack_irq(desc, irq);
> 	..
> 
> where the masking part seems a bit pointless. And in the case of MSI, it 
> causes us to go all the way out to the device, which sounds pretty 
> expensive too.

hm, i agree that your patched version looks much simpler.

There's two reasons we have this variant:

 - huge bikeshed painting thread in the early days of the genirq patchset, 
   with folks expressing concern about our original plans to keep 
   edge-triggered unmasked _always_. (which your patch does too)
   So we just went with the path of least resistence and used this hybride.

 - the screaming-irq observation i had - do you consider that valid?:

   >> [ In theory this also solves screaming level-triggered irqs that 
   >>   advertise themselves as edge-triggered [due to firmware/BIOS bug - 
   >>   these do happen] and then keep spamming the system. ]

   I wanted to have a pretty much interchangeable flow method between edge 
   and level triggered - so that the BIOS cannot screw us by enumerating an 
   irq as edge-triggered while it's level-triggered.

   Especially for legacy x86 irqs in the low <16 range the trigger mode can 
   be influenced by chipset settings and might not always be what we think 
   it is.

That's my rough recollection - Thomas, is that correct and do you have 
anything to add here?

	Ingo
--
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/