linux-kernel - Re: [BUGFIX 3/4] PCI/PM: Fix config reg access for D3cold and bridge suspending

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: <201208042337.46394.rjw@sisk.pl>
Date:	Sat, 4 Aug 2012 23:37:46 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Huang Ying <ying.huang@...el.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	linux-pm@...r.kernel.org
Subject: Re: [BUGFIX 3/4] PCI/PM: Fix config reg access for D3cold and bridge suspending

On Friday, August 03, 2012, Alan Stern wrote:
> On Fri, 3 Aug 2012, Huang Ying wrote:
> 
> > This patch fixes the following bug:
> > 
> > http://marc.info/?l=linux-pci&m=134338059022620&w=2
> > 
> > Where lspci does not work properly if a device and the corresponding
> > parent bridge (such as PCIe port) is suspended.  This is because the
> > device configuration space registers will be not accessible if the
> > corresponding parent bridge is suspended or the device is put into
> > D3cold state.
> > 
> > To solve the issue, the bridge/PCIe port connected to the device is
> > put into active state before read/write configuration space registers.
> > If the device is in D3cold state, it will be put into active state
> > too.
> > 
> > To avoid resume/suspend PCIe port for each configuration register
> > read/write, a small delay is added before the PCIe port to go
> > suspended.
> 
> 
> > +static void
> > +pci_config_pm_runtime_put(struct pci_dev *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct device *parent = dev->parent;
> > +
> > +	pm_runtime_put(dev);
> > +	if (parent)
> > +		pm_runtime_put(parent);
> > +}
> 
> This is just the sort of thing Rafael and I have been talking about.  
> Why do an asynchronous put, going to all the trouble of using the 
> workqueue, if the idle routine is just going to call 
> pm_schedule_suspend()?

If that's PCI, it will call pm_runtime_suspend().  That probably _should_ be
pm_schedule_suspend(), but it isn't at the moment.

> Why not call pm_runtime_put_sync() instead?

I guess because the caller doesn't care whether or not the devices will be
suspended immediately and we seem to have agreed already that the added
workqueue overhead is minimal.

If the _idle() routine were to call pm_schedule_suspend(), though, I'd
agree that the overhead would be absolutely unnecessary.

Thanks,
Rafael
--
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/