Re: [PATCH] [PCI] Prevent user config space access during power state transitions

[Greg KH <gregkh@suse.de>]

On Fri, Oct 13, 2006 at 10:26:08PM -0700, Andrew Morton wrote:
> On Tue, 10 Oct 2006 07:38:17 -0600
> Matthew Wilcox <matthew@wil.cx> wrote:
> 
> > Section 5.3 of PCI Bus Power Management 1.2 states:
> > 
> >   There is a minimum recovery time requirement of 200 ?s between when
> >   a function is programmed from D2 to D0 and when the function can be
> >   next accessed as a target (including PCI configuration accesses). If
> >   an access is attempted in violation of the specified minimum recovery
> >   time, undefined system behavior may result.
> > 
> > We have to prevent the user running lspci during this time, and
> > fortunately we already have the pci_block_user_cfg_access() API to
> > do this.
> > 
> > Signed-off-by: Matthew Wilcox <matthew@wil.cx>
> > ---
> >  drivers/pci/pci.c |    8 ++++++++
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index a544997..1bb059a 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -366,6 +366,11 @@ pci_set_power_state(struct pci_dev *dev,
> >  		break;
> >  	}
> >  
> > +	/* We have to prevent accesses to config space while transitioning
> > +	 * between power states
> > +	 */
> > +	pci_block_user_cfg_access(dev);
> > +
> >  	/* enter specified state */
> >  	pci_write_config_word(dev, pm + PCI_PM_CTRL, pmcsr);
> >  
> > @@ -383,6 +388,9 @@ pci_set_power_state(struct pci_dev *dev,
> >  	if (platform_pci_set_power_state)
> >  		platform_pci_set_power_state(dev, state);
> >  
> > +	/* Should be safe to allow userspace access to the device again now */
> > +	pci_unblock_user_cfg_access(dev);
> > +
> >  	dev->current_state = state;
> >  
> >  	/* According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT
> 
> This patch independently causes the same failure: the Vaio doesn't power
> off after suspend-to-disk and after a manual power cycle and resume,
> networking is dead.
> 
> The message `acpi_power_off called' never comes out, so something probably
> got stuck.
> 
> Or maybe something failed somewhere and the error code which would have
> helped us solve this bug was simply ignored.
> 
> 
> 
> Note that pci_block_user_cfg_access() calls pci_save_state(), which can
> fail.  But pci_block_user_cfg_access() discards that information and
> returns void.  If this happens, userspace config space reads will return...
> what?  

There is a thread right now on the linux-pm mailing list about how this
patch is wrong.  I'm going to drop it right now...

thanks,

greg k-h