On Fri, Mar 11, 2016 at 3:29 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > On Fri, Mar 11, 2016 at 01:16:09PM -0800, Andy Lutomirski wrote: >> On Tue, Mar 8, 2016 at 9:45 AM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: >> > On Thu, Mar 03, 2016 at 10:53:50AM -0600, Bjorn Helgaas wrote: >> >> The purpose of this series is to: >> >> >> >> - Fix the "BAR 6: [??? 0x00000000 flags 0x2] has bogus alignment" >> >> messages reported by Linus [1], Andy [2], and others. >> >> >> >> - Move arch-specific shadow ROM location knowledge, e.g., >> >> 0xC0000-0xDFFFF, from PCI core to arch code. >> >> >> >> - Fix the ia64 and MIPS Loongson 3 oddity of keeping virtual >> >> addresses in shadow ROM struct resource (resources should always >> >> contain *physical* addresses). >> >> >> >> - Remove now-unused IORESOURCE_ROM_COPY and IORESOURCE_ROM_BIOS_COPY >> >> flags. >> >> >> >> This series is based on v4.5-rc1, and it's available on my >> >> pci/resource git branch (along with a couple tiny unrelated patches) >> >> at [3]. >> >> >> >> Bjorn >> >> >> >> >> >> [1] http://lkml.kernel.org/r/CA+55aFyVMfTBB0oz_yx8+eQOEJnzGtCsYSj9QuhEpdZ9BHdq5A@xxxxxxxxxxxxxx >> >> [2] http://lkml.kernel.org/r/CALCETrV+RwNPzxyL8UVNsrAGu-6cCzD_Cc9PFJT2NCTJPLZZiw@xxxxxxxxxxxxxx >> >> [3] https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/resource >> >> >> >> >> >> --- >> >> >> >> Bjorn Helgaas (12): >> >> PCI: Mark shadow copy of VGA ROM as IORESOURCE_PCI_FIXED >> >> PCI: Don't assign or reassign immutable resources >> >> PCI: Don't enable/disable ROM BAR if we're using a RAM shadow copy >> >> PCI: Set ROM shadow location in arch code, not in PCI core >> >> PCI: Clean up pci_map_rom() whitespace >> >> ia64/PCI: Use temporary struct resource * to avoid repetition >> >> ia64/PCI: Use ioremap() instead of open-coded equivalent >> >> ia64/PCI: Keep CPU physical (not virtual) addresses in shadow ROM resource >> >> MIPS: Loongson 3: Use temporary struct resource * to avoid repetition >> >> MIPS: Loongson 3: Keep CPU physical (not virtual) addresses in shadow ROM resource >> >> PCI: Remove unused IORESOURCE_ROM_COPY and IORESOURCE_ROM_BIOS_COPY >> >> PCI: Simplify sysfs ROM cleanup >> >> >> >> >> >> arch/ia64/pci/fixup.c | 21 +++++++-- >> >> arch/ia64/sn/kernel/io_acpi_init.c | 22 ++++++---- >> >> arch/ia64/sn/kernel/io_init.c | 51 ++++++++-------------- >> >> arch/mips/pci/fixup-loongson3.c | 19 +++++--- >> >> arch/x86/pci/fixup.c | 21 +++++++-- >> >> drivers/pci/pci-sysfs.c | 13 +----- >> >> drivers/pci/remove.c | 1 >> >> drivers/pci/rom.c | 83 +++++++++++------------------------- >> >> drivers/pci/setup-res.c | 6 +++ >> >> include/linux/ioport.h | 4 -- >> >> 10 files changed, 111 insertions(+), 130 deletions(-) >> > >> > I applied this series to pci/resource for v4.6. >> >> This gets rid of all the warnings for me until I try to read my i915 >> device's rom using sysfs. Then I get: >> >> i915 0000:00:02.0: Invalid PCI ROM header signature: expecting 0xaa55, >> got 0xffff >> >> So I suspect that something is still subtly wrong -- I'd imagine that >> this should either work or the intialization code should detect that >> there is no usable ROM and not expose it. >> >> (To be clear, there's no regression here.) > > Hmmm. Thanks for testing this. As you say, I think this is the way > it's always been, but it does seem non-intuitive. > > That "Invalid PCI ROM header signature" warning comes from > pci_get_rom_size(). We don't call that at enumeration-time; we only > call it later when somebody tries to read the ROM via sysfs: > > pci_bus_add_device > pci_fixup_device(pci_fixup_final) > pci_fixup_video # final fixup > res->flags = MEM | SHADOW | PCI_FIXED > pci_create_sysfs_dev_files > if (SHADOW) > rom_size = 0x20000 # oops, I should have fixed this too > if (rom_size) > attr->read = pci_read_rom > sysfs_create_bin_file # sysfs "rom" file > > pci_read_rom # sysfs "rom" read function > pci_map_rom > ioremap > pci_get_rom_size > dev_err("Invalid PCI ROM header signature") > memcpy_fromio > pci_unmap_rom > > I think this is the same for regular ROMs on the device as it is for > the magic VGA shadow ROM. In both cases, we create the sysfs "rom" > file regardless of what the ROM contains. > > I guess we could try to read the ROM at enumeration time and look for > a valid signature. I've considered doing that anyway so we could > cache the ROM contents and avoid permanently allocating MMIO space for > it, since many BIOSes don't allocate space, and Linux isn't really smart > enough to do a good job of it itself. > > I don't know why the PCI core cares about the ROM signature anyway, > since it doesn't use the content. Maybe we should get rid of > pci_get_rom_size() and allow you to read whatever is there, like > we do for other BARs. I suppose there's some history behind limiting > the size, but I don't know what it is. FWIW, if I disable all the checks in pci_get_rom_size, I learn that my video ROM consists entirely of 0xff bytes. Maybe there just isn't a ROM shadow on my laptop. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
