Re: PATCH 2.5.x disable BAR when sizing

[ebiederm@xmission.com (Eric W. Biederman)]

Ivan Kokshaysky <ink@jurassic.park.msu.ru> writes:

> On Sat, Dec 21, 2002 at 02:44:23PM -0800, Linus Torvalds wrote:
> > And hotplug devices should be disabled at plug-in, so later BAR probing
> > should be pretty harmless too (and, as you point out about bridging, they
> > should be shielded by the hotplug bridge itself).
> 
> Agreed.
> 
> > This is why I repeated my "turn the power off at the whole house" analogy,
> > even if David didn't like it. It's _fine_ to turn the power off if we know
> > things are quiet, it's just that as things stand now, we don't actually
> > know that.
> 
> The analogy is wonderful. I'd like to extend it to mentioned ia64 system:
> any attempt to replace a light bulb that consumes more than 32 Mb^H^HWatts
> without turning the power off will blow up fuses in the whole house.
> 
> >  - (new) phase 1 - scan for and turn off all devices
> >  - phase 2 - go back and check the resources (BAR probing etc)
> >  - phase 3 - allocate unassigned resources.
> 
> I don't think we want to turn off *all* devices in phase 1.
> Probably we need another level of fixups - PCI_FIXUP_EARLY, which
> will be called after phase 1, i.e. before fiddling with BARs.
> 
> However, I'm still not convinced that we should ever change anything.
> 
> On Sun, Dec 22, 2002 at 03:38:04AM -0700, Eric W. Biederman wrote:
> > So the problem we are facing is that we have some activity while BARs
> > are being sized.  Activity can be in the form of IRQs, DMA, IO, and
> > MEM accesses.
> 
> I would suggest another classification: a) bus activity generated by CPU
> and b) bus activity generated by other devices.
> 
> Case 'a' is mostly under our control. The only problem is an interrupt
> arriving during BAR probe, as Paul pointed out. This can be fixed:
> 		pci_read_config_dword(dev, reg, &l);
> +		local_irq_save(flags);
> 		pci_write_config_dword(dev, reg, ~0);
> 		pci_read_config_dword(dev, reg, &sz);
> 		pci_write_config_dword(dev, reg, l);
> +		local_irq_restore(flags);

Not if it is an NMI or an SCI interrupt.  The latter on x86 places
the cpu in System Management Mode, and what the cpu does from that
point forward is out of our control.

Though disabling cpu controlled IRQs help if you are dealing
with any normal IRQs.

> Case 'b', two variants.
> - The target of bus activity is a region located in the low bus
>   addresses (USB case), so there are no conflicts with probed BARs
>   of other devices. But we cannot safely disable devices while BARs
>   are being sized as we don't know for sure what device controls
>   that region.
> - Target region is in the high bus addresses (ia64 case), and conflicts
>   are possible when we do the sizing. Yes, disabling the devices does help
>   to avoid conflicts and it happens to work with *some* configurations.
>   But what if the said region in its turn is defined by BAR? or just
>   controlled by some PCI device and therefore can be disconnected from
>   the bus? Too many if's.
> 
> I think it's obvious now that "disabling the BARs" is not a fix at all,
> it's just a hack for that particular setup.
> BTW, I think it is *not* required by PCI spec - v2.1 says nothing about it,
> in v2.2 it seems to be just an implementation example.
> 
> The real fix is to shut up the device that makes a noise on the bus,
> at least during bus probe.
> I refuse to believe that it cannot be done with that ia64 SAPIC (must be
> a hidden switch somewhere ;-)
> So I'd wait for more tech details regarding that.
> 
> > The goal being to keep the window when uncontrolled pci accesses
> > can blow the system up as small as possible.
> 
> The window will be large enough anyway - the PCI config space accesses are
> very slow on most systems.

The window needs to be small from the PCI bus perspective, not in cpu
clocks.  Write, Read, Write is only something like 9 PCI bus clocks.
If IRQs also traveled over the PCI bus we could be fairly confident of
saturating the PCI bus and not having problems.  Except that the PCI
bus arbiter may not choose the CPU for back to back transactions.

Not that I would like to trust to the small window of opportunity
idea, but closing all of the other holes looks quite difficult.

Eric