Re: git pull] drm for v4.1-rc1

Bjorn Helgaas
2015-04-23T21:30:22Z

[+cc Matthew]

On Tue, Apr 21, 2015 at 09:07:45AM -0700, Linus Torvalds wrote:
> Hmm. The odd Intel PCI resource mess is back.
>
> Or maybe it never went away.
>
> I get these when suspending. Things *work*, but it's really spamming
> my logs a fair bit:
>
> i915 0000:00:02.0: BAR 6: [??? 0x00000000 flags 0x2] has bogus alignment
> pci_bus 0000:01: Allocating resources
> pci_bus 0000:02: Allocating resources
> i915 0000:00:02.0: BAR 6: [??? 0x00000000 flags 0x2] has bogus alignment
> i915 0000:00:02.0: BAR 6: [??? 0x00000000 flags 0x2] has bogus alignment
> i915 0000:00:02.0: BAR 6: [??? 0x00000000 flags 0x2] has bogus alignment
> i915 0000:00:02.0: BAR 6: [??? 0x00000000 flags 0x2] has bogus alignment
> i915 0000:00:02.0: BAR 6: [??? 0x00000000 flags 0x2] has bogus alignment
> i915 0000:00:02.0: BAR 6: [??? 0x00000000 flags 0x2] has bogus alignment
>
> That resource is complete garbage. "flags 0x2" is not even a valid
> flag value. I'm *guessing* it might be IORESOURCE_ROM_SHADOW, but if
> that is valid, then it should also have have had the IORESOURCE_MEM
> bit, and it doesn't.

Your i915 does not have a ROM BAR in hardware. If the default video
device has no ROM BAR, pci_fixup_video() sets IORESOURCE_ROM_SHADOW
even though the resource flags are zero because the BAR itself doesn't
exist.

If IORESOURCE_ROM_SHADOW is set, pci_map_rom() assumes there's a
shadow ROM image at 0xC0000. Is there a shadow image even if the
device itself doesn't have a ROM BAR?

We could fabricate a resource even if the BAR doesn't exist, e.g.,
"flags = IORESOURCE_MEM | ... | IORESOURCE_ROM_SHADOW", but that
would be ugly and error-prone in other places that use the ROM.

Matthew added dev->rom for ROM images supplied by the platform
(84c1b80e3263 ("PCI: Add support for non-BAR ROMs")). A shadow
image seems like a similar thing. I think it would be cleaner to get
rid of IORESOURCE_ROM_SHADOW altogether and instead set "dev->rom =
0xC0000" if there's a shadow image, e.g.:

int pcibios_add_device(struct pci_dev *dev)
{
if (dev-is-default-vga-device) {
dev->rom = 0xC0000;
dev->romlen = 0x20000;
}

pa_data = boot_params.hdr.setup_data;
while (pa_data) {
...
if (data->type == SETUP_PCI) {
rom = (struct pci_setup_rom *)data;

if (dev-is-rom-dev) {
dev->rom = ...
dev->romlen = rom->pcilen;
}
}
}

But the rules for figuring out which image to use seem ...
complicated.

Bjorn