linux-kernel - 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]

Message-Id: <200902030833.08222.jesse.barnes@intel.com>
Date:	Tue, 3 Feb 2009 08:33:07 -0800
From:	Jesse Barnes <jesse.barnes@...el.com>
To:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andreas Schwab <schwab@...e.de>, Len Brown <lenb@...nel.org>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: PCI PM: Restore standard config registers of all devices early

On Monday, February 2, 2009 10:07 pm Benjamin Herrenschmidt wrote:
> On Mon, 2009-02-02 at 16:28 -0800, Linus Torvalds wrote:
> > On Tue, 3 Feb 2009, Rafael J. Wysocki wrote:
> > > BTW, on the PMAC the problematic driver appears to be the radeon
> > > driver, according to Ben, and the breakage is not related to USB.
> >
> > Hmm. atyfb_base.c has the same kind of things with magic PMAC code, but
> > it doesn't follow the USB pattern - it just replaces
> > "pci_set_power_state()" _entirely_.
> >
> > It seems a very interesting suspend routine, btw. It doesn't seem to do
> > any of the pci_save_state() at all, just re-initializes from scratch.
> > Maybe it is unhappy with the PM layer deciding to try to restore state
> > since it clearly didn't..
>
> In fact, atyfb is also busted in -rc3 for different reasons though
> probably by the same patch. I just dug an old powerbook with a mach64
> and it crashes on resume with a machine check in there. (Among 2 or 3
> other problems introduced by recent kernels, such as pci_get_* now
> kmallocs with GFP_KERNEL internally which makes it WARN when I use it in
> my low level suspend/resume code to whack the memory controller, etc...
> this one is going to bite others I reckon, or IDE having some interrupt
> problems on resume).
>
> Adding a pci_save_state() to atyfb_pci_suspend() and a
> pci_set_power_state() + pci_restore_state() at the beginning of
> atyfb_pci_resume() fixes the machine check here.
>
> Now where it gets funny is that I've added code to read the BAR and
> command register content before, between, and after those calls and
> print it and .. they are sane... Until i discovered that what happens is
> that the new generic code seems to actually blast 0 all over my config
> space if I don't call pci_save_state() in suspend(). I suppose I was
> missing a "mandatory" call here... but the core should be more robust,
> ie it shouldn't erase the config space of something because a driver
> "forgot" to call pci_save_state() !

Whoa, I don't think we actually zero the contents in the suspend/resume core, 
but if the device goes into D3 the config space contents may be lost, maybe 
that's what happening here?

-- 
Jesse Barnes, Intel Open Source Technology Center
--
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/