Re: PATCH 2.5.x disable BAR when sizing

[Ivan Kokshaysky]

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);

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.

Ivan.