Re: [PATCH 1/2] pci: Block config access during BIST (resend)

[Grant Grundler <grundler@parisc-linux.org>]

On Sat, Sep 03, 2005 at 09:11:30AM +1000, Paul Mackerras wrote:
> Think about it.  Taking the lock ensures that we don't do the
> assignment (dev->block_ucfg_access = 1) while any other cpu has the
> pci_lock.  In other words, the reason for taking the lock is so that
> we wait until nobody else is doing an access, not to make others wait.

The block_ucfg_access field is only used when making the choice to
use saved state or call the PCI bus cfg accessor.
I don't what problem waiting solves here since any CPU already
accessing real cfg space will finish what they are doing anyway.
ie they already made the choice to access real cfg space.
We just need to make sure successive accesses to cfg space
for this device only access the saved state. For that, a memory barrier
around all uses of block_ucfg_access should be sufficient.
Or do you think I'm still drinking the wrong color cool-aid?

> > If you had:
> > 	spin_lock_irqsave(&pci_lock, flags);
> > 	pci_save_state(dev);
> > 	dev->block_ucfg_access = 1;
> > 	spin_unlock_irqrestore(&pci_lock, flags);
> > 
> > Then I could buy your arguement since the flag now implies
> > we need to atomically save state and set the flag.
> 
> That's probably a good thing to do to.

One needs to verify pci_lock isn't acquired in pci_save_state()
(or some other obvious dead lock).

It would make sense to block pci cfg *writes* to that device
while we save the state.

grant