Weirdness in pci_read_bases()

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

Hi !

There is something dodgy going on in pci_read_bases().

In addition to the fact (or related to it) that we tend to treat
res->start == 0 as meaning "unassigned" (which at this stage is mostly
incorrect, but let's say I can cope), however, there is some bit of code
that I think is just plain wrong:

		reg = PCI_BASE_ADDRESS_0 + (pos << 2);
		pci_read_config_dword(dev, reg, &l);
		pci_write_config_dword(dev, reg, ~0);
		pci_read_config_dword(dev, reg, &sz);
		pci_write_config_dword(dev, reg, l);
		if (!sz || sz == 0xffffffff)
			continue;
		if (l == 0xffffffff)
			l = 0;

So here, we read the BAR value, which at this stage could either be some
assigned address or some ff's from a freshly unassigned BAR. _however_
that test against 0xffffffff looks totally bogus to me.

If we read that value, we write 0 in l. Which means that if we read some
unassigned resource that had the address space bits set to IO, we
overwrite this with a bit encoding that means Memory, and further along,
start sizing & treating this as a memory resource.

In fact, a BAR value should always contain a 0 bit. A Memory BAR must
contain a 0 at the bottom, and an IO BAR should contain a 0 in bit 1.
And we do check that case fine when reading back sz.

Thus a l value of 0xffffffff should not happen in practice, and if it
does, we should -at-least- try to get the address space bits from sz
(since in this case sz looks allright), not from l, no ? Or maybe just
skip the whole resource ?

Do we have practical cases where we see that 0xffffffff value ?

This isn't a problem I'm encountering in practice. I just noticed that
while trying to audit what it would take to fix the code to stop using
res->start = 0 as a way to mark unassigned resources (unrelated), so it
doesn't -need- to be fixed per-se, but I'm curious where that came from
in the first place.

Cheers,
Ben.