| Message ID | 1345739803-21017-4-git-send-email-mjg@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 08/23/2012 12:36 PM, Matthew Garrett wrote: > Platforms may provide their own mechanisms for obtaining ROMs. Add support > for using data provided by the platform in that case. > > Signed-off-by: Matthew Garrett<mjg@redhat.com> > --- > drivers/pci/rom.c | 11 +++++++++-- > include/linux/pci.h | 2 ++ > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/rom.c b/drivers/pci/rom.c > index 48ebdb2..46026e4 100644 > --- a/drivers/pci/rom.c > +++ b/drivers/pci/rom.c > @@ -118,11 +118,17 @@ void __iomem *pci_map_rom(struct pci_dev *pdev, size_t *size) > void __iomem *rom; > > /* > + * Some devices may provide ROMs via a source other than the BAR > + */ > + if (pdev->rom&& pdev->romlen) { > + *size = pdev->romlen; > + return phys_to_virt((phys_addr_t)pdev->rom); ^^^^^ ioremap_nocache() ? ... or is caching rom ok? > + /* > * IORESOURCE_ROM_SHADOW set on x86, x86_64 and IA64 supports legacy > * memory map if the VGA enable bit of the Bridge Control register is > * set for embedded VGA. > */ > - if (res->flags& IORESOURCE_ROM_SHADOW) { > + } else if (res->flags& IORESOURCE_ROM_SHADOW) { > /* primary video rom always starts here */ > start = (loff_t)0xC0000; > *size = 0x20000; /* cover C000:0 through E000:0 */ > @@ -219,7 +225,8 @@ void pci_unmap_rom(struct pci_dev *pdev, void __iomem *rom) > if (res->flags& (IORESOURCE_ROM_COPY | IORESOURCE_ROM_BIOS_COPY)) > return; > > - iounmap(rom); > + if (!pdev->rom || !pdev->romlen) > + iounmap(rom); > > /* Disable again before continuing, leave enabled if pci=rom */ > if (!(res->flags& (IORESOURCE_ROM_ENABLE | IORESOURCE_ROM_SHADOW))) > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 6a2625c..2668bb9 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -355,6 +355,8 @@ struct pci_dev { > }; > struct pci_ats *ats; /* Address Translation Service */ > #endif > + void *rom; /* Physical pointer to ROM if it's not from the BAR */ > + size_t romlen; /* Length of ROM if it's not from the BAR */ > }; > > static inline struct pci_dev *pci_physfn(struct pci_dev *dev) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 04, 2012 at 10:18:48PM -0400, Don Dutile wrote: > > /* > >+ * Some devices may provide ROMs via a source other than the BAR > >+ */ > >+ if (pdev->rom&& pdev->romlen) { > >+ *size = pdev->romlen; > >+ return phys_to_virt((phys_addr_t)pdev->rom); > ^^^^^ > ioremap_nocache() ? ... or is caching rom ok? If it's appearing through this pathway then it's not actually in ROM, the platform has pulled it out of somewhere else.
On Wed, 5 Sep 2012 03:29:32 +0100 Matthew Garrett <mjg@redhat.com> wrote: > On Tue, Sep 04, 2012 at 10:18:48PM -0400, Don Dutile wrote: > > > /* > > >+ * Some devices may provide ROMs via a source other than the BAR > > >+ */ > > >+ if (pdev->rom&& pdev->romlen) { > > >+ *size = pdev->romlen; > > >+ return phys_to_virt((phys_addr_t)pdev->rom); > > ^^^^^ > > ioremap_nocache() ? ... or is caching rom ok? > > If it's appearing through this pathway then it's not actually in ROM, > the platform has pulled it out of somewhere else. If that somewhere else is on the PCI bus then it should be a bus not a virt translation surely ? -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 05, 2012 at 01:46:21PM +0100, Alan Cox wrote: > On Wed, 5 Sep 2012 03:29:32 +0100 > Matthew Garrett <mjg@redhat.com> wrote: > > > On Tue, Sep 04, 2012 at 10:18:48PM -0400, Don Dutile wrote: > > > > /* > > > >+ * Some devices may provide ROMs via a source other than the BAR > > > >+ */ > > > >+ if (pdev->rom&& pdev->romlen) { > > > >+ *size = pdev->romlen; > > > >+ return phys_to_virt((phys_addr_t)pdev->rom); > > > ^^^^^ > > > ioremap_nocache() ? ... or is caching rom ok? > > > > If it's appearing through this pathway then it's not actually in ROM, > > the platform has pulled it out of somewhere else. > > If that somewhere else is on the PCI bus then it should be a bus not a > virt translation surely ? We've no good way of knowing what the firmware's giving us - we copy it to RAM in the EFI init process, so by the time we're here it certainly shouldn't be on the PCI bus.
On Wed, 5 Sep 2012 14:20:07 +0100 Matthew Garrett <mjg@redhat.com> wrote: > On Wed, Sep 05, 2012 at 01:46:21PM +0100, Alan Cox wrote: > > On Wed, 5 Sep 2012 03:29:32 +0100 > > Matthew Garrett <mjg@redhat.com> wrote: > > > > > On Tue, Sep 04, 2012 at 10:18:48PM -0400, Don Dutile wrote: > > > > > /* > > > > >+ * Some devices may provide ROMs via a source other than the BAR > > > > >+ */ > > > > >+ if (pdev->rom&& pdev->romlen) { > > > > >+ *size = pdev->romlen; > > > > >+ return phys_to_virt((phys_addr_t)pdev->rom); > > > > ^^^^^ > > > > ioremap_nocache() ? ... or is caching rom ok? > > > > > > If it's appearing through this pathway then it's not actually in ROM, > > > the platform has pulled it out of somewhere else. > > > > If that somewhere else is on the PCI bus then it should be a bus not a > > virt translation surely ? > > We've no good way of knowing what the firmware's giving us - we copy it > to RAM in the EFI init process, so by the time we're here it certainly > shouldn't be on the PCI bus. In which case how do you know that physical address you have been given actually has a mapping in kernel virtual space. It seems like it ought to be getting an ioremap in that case ? phys_to_virt is not valid for arbitary addresses. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 05, 2012 at 02:30:10PM +0100, Alan Cox wrote: > On Wed, 5 Sep 2012 14:20:07 +0100 > Matthew Garrett <mjg@redhat.com> wrote: > > We've no good way of knowing what the firmware's giving us - we copy it > > to RAM in the EFI init process, so by the time we're here it certainly > > shouldn't be on the PCI bus. > > In which case how do you know that physical address you have been given > actually has a mapping in kernel virtual space. It seems like it ought to > be getting an ioremap in that case ? > > phys_to_virt is not valid for arbitary addresses. The pages are allocated as EFI_LOADER_DATA, and that gets mapped as E820_RAM.
diff --git a/drivers/pci/rom.c b/drivers/pci/rom.c index 48ebdb2..46026e4 100644 --- a/drivers/pci/rom.c +++ b/drivers/pci/rom.c @@ -118,11 +118,17 @@ void __iomem *pci_map_rom(struct pci_dev *pdev, size_t *size) void __iomem *rom; /* + * Some devices may provide ROMs via a source other than the BAR + */ + if (pdev->rom && pdev->romlen) { + *size = pdev->romlen; + return phys_to_virt((phys_addr_t)pdev->rom); + /* * IORESOURCE_ROM_SHADOW set on x86, x86_64 and IA64 supports legacy * memory map if the VGA enable bit of the Bridge Control register is * set for embedded VGA. */ - if (res->flags & IORESOURCE_ROM_SHADOW) { + } else if (res->flags & IORESOURCE_ROM_SHADOW) { /* primary video rom always starts here */ start = (loff_t)0xC0000; *size = 0x20000; /* cover C000:0 through E000:0 */ @@ -219,7 +225,8 @@ void pci_unmap_rom(struct pci_dev *pdev, void __iomem *rom) if (res->flags & (IORESOURCE_ROM_COPY | IORESOURCE_ROM_BIOS_COPY)) return; - iounmap(rom); + if (!pdev->rom || !pdev->romlen) + iounmap(rom); /* Disable again before continuing, leave enabled if pci=rom */ if (!(res->flags & (IORESOURCE_ROM_ENABLE | IORESOURCE_ROM_SHADOW))) diff --git a/include/linux/pci.h b/include/linux/pci.h index 6a2625c..2668bb9 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -355,6 +355,8 @@ struct pci_dev { }; struct pci_ats *ats; /* Address Translation Service */ #endif + void *rom; /* Physical pointer to ROM if it's not from the BAR */ + size_t romlen; /* Length of ROM if it's not from the BAR */ }; static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
Platforms may provide their own mechanisms for obtaining ROMs. Add support for using data provided by the platform in that case. Signed-off-by: Matthew Garrett <mjg@redhat.com> --- drivers/pci/rom.c | 11 +++++++++-- include/linux/pci.h | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-)