| Message ID | CAE9FiQWd69=6-UvCWsPOHvnZ96grJyzEt_+WR0-Q3M9wHz69hw@mail.gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, Sep 24, 2015 at 10:35 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Thu, Sep 24, 2015 at 12:01 PM, Yinghai Lu <yinghai@kernel.org> wrote: > >> Or do we want to keep a white list to say which device should have >> ROM bar as mush have, and other is optional to have ? I suppose this idea is one possible outcome that could occur but I think we need to have a discussion first before we start making a lot of changes. We need to try and come to some consensus with BIOS engineers. I know that both sets have been alerted to this conversation so *if* we come up with some good arguments to support the kernel's current view/actions perhaps things will start to progress. In the prior thread you replied: "They are wrong. It still depends on how addon card firmware and drivers from OS to use it." So continuing my suggested thought experiment where you are sitting across the table from your platform's BIOS engineers having this discussion ... Do you *really* think your reply was helpful in any way? Do you *really* think you did anything what so ever to get the BIOS engineers to consider something they hadn't before. Do you *really* think your reply was technically based in any way? Do you have any specification references or such to back up such a strong claim? Come on here Yinghai - you are an intelligent person. Take 1/10th the time that you spent developing this patch and think, gather your thoughts, and then sit down calmly, have a beer or coffee or tea (which ever you prefer), relax, and take some time to reply in a logical, thoughtful manner here with enough expression that others can understand what you are getting at and hopefully even with some passion or logic to try to convince the BIOS engineers that the kernel's current behavior is correct. This is your area of expertise - so stand up and rise to the occasion here! Hacking out a patch before this thread has played out serves no purpose what so ever so I'm not even going to waste my time and look at it. It serves no purpose and will only make matters worse as there is already strong disagreement with the kernel's actions in these regards. > > Subject: [RFC PATCH] PCI: Add pci_dev_need_rom_bar() > > Only set that for > 1. if BIOS/firmware already set ROM bar. > 2. via quirks for some devices. > > We assign those needed ROM bar as required > and other ROM bars as optional resources. > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > > --- > arch/x86/pci/i386.c | 9 +++++- > drivers/dma/pch_dma.c | 1 > drivers/gpio/gpio-ml-ioh.c | 2 - > drivers/misc/pch_phub.c | 12 -------- > drivers/pci/probe.c | 7 +++++ > drivers/pci/quirks.c | 63 +++++++++++++++++++++++++++++++++++++++++++++ > drivers/pci/setup-bus.c | 18 ++++++++++-- > include/linux/pci.h | 13 +++++++++ > include/linux/pci_ids.h | 7 +++++ > 9 files changed, 112 insertions(+), 20 deletions(-) > > Index: linux-2.6/arch/x86/pci/i386.c > =================================================================== > --- linux-2.6.orig/arch/x86/pci/i386.c > +++ linux-2.6/arch/x86/pci/i386.c > @@ -377,11 +377,16 @@ static void pcibios_allocate_rom_resourc > } > } > > +bool pci_assign_roms(void) > +{ > + return !!(pci_probe & PCI_ASSIGN_ROMS); > +} > + > static int __init pcibios_assign_resources(void) > { > struct pci_host_bridge *host_bridge = NULL; > > - if (!(pci_probe & PCI_ASSIGN_ROMS)) > + if (!pci_assign_roms()) > for_each_pci_host_bridge(host_bridge) > pcibios_allocate_rom_resources(host_bridge->bus); > > @@ -406,7 +411,7 @@ void pcibios_resource_survey_bus(struct > pcibios_allocate_resources(bus, 0); > pcibios_allocate_resources(bus, 1); > > - if (!(pci_probe & PCI_ASSIGN_ROMS)) > + if (!pci_assign_roms()) > pcibios_allocate_rom_resources(bus); > } > > Index: linux-2.6/drivers/pci/probe.c > =================================================================== > --- linux-2.6.orig/drivers/pci/probe.c > +++ linux-2.6/drivers/pci/probe.c > @@ -224,6 +224,13 @@ int __pci_read_base(struct pci_dev *dev, > l64 = l & PCI_ROM_ADDRESS_MASK; > sz64 = sz & PCI_ROM_ADDRESS_MASK; > mask64 = (u32)PCI_ROM_ADDRESS_MASK; > + /* simple validation */ > + if (l64 && sz64 && > + (l64 & 0xff000000) != 0xff000000 && > + system_state == SYSTEM_BOOTING) { > + dev_printk(KERN_DEBUG, &dev->dev, "set dev_flags NEED_ROM_BAR\n"); > + pci_dev_set_need_rom_bar(dev); > + } > } > > if (res->flags & IORESOURCE_MEM_64) { > Index: linux-2.6/drivers/pci/quirks.c > =================================================================== > --- linux-2.6.orig/drivers/pci/quirks.c > +++ linux-2.6/drivers/pci/quirks.c > @@ -4197,3 +4197,66 @@ static void quirk_intel_qat_vf_cap(struc > } > } > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443, quirk_intel_qat_vf_cap); > + > +/* from drivers/mtd/maps/pci.c */ > +static void quirk_set_need_rom_bar(struct pci_dev *pdev) > +{ > + if (!pci_dev_need_rom_bar(pdev)) { > + dev_printk(KERN_DEBUG, &pdev->dev, "set dev_flags NEED_ROM_BAR\n"); > + pci_dev_set_need_rom_bar(pdev); > + } > +} > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_DEC, PCI_DEVICE_ID_DEC_21285, > + quirk_set_need_rom_bar); > + > +#ifdef CONFIG_PARISC > +/* from drivers/video/console/sticore.c */ > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_EG, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FX6, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FX4, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FX2, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FXE, > + quirk_set_need_rom_bar); > +#endif > + > +/* from drivers/misc/pch_phub.c */ > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_PCH1_PHUB, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7213_PHUB, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7223_mPHUB, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7223_nPHUB, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7831_PHUB, > + quirk_set_need_rom_bar); > + > +/* from drivers/net/ethernet/sun/sungem.c */ > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_GEM, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_RIO_GEM, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_UNI_N_GMAC, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_UNI_N_GMACP, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_UNI_N_GMAC2, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_K2_GMAC, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_SH_SUNGEM, > + quirk_set_need_rom_bar); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_IPID2_GMAC, > + quirk_set_need_rom_bar); > + > +/* from drivers/net/ethernet/sun/sunhme.c */ > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_HAPPYMEAL, > + quirk_set_need_rom_bar); > + > +/* from drm and fbdev */ > +DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, > + 8, quirk_set_need_rom_bar); > Index: linux-2.6/drivers/pci/setup-bus.c > =================================================================== > --- linux-2.6.orig/drivers/pci/setup-bus.c > +++ linux-2.6/drivers/pci/setup-bus.c > @@ -226,6 +226,10 @@ static void pdev_assign_resources_prepar > if (resource_disabled(r) || r->parent) > continue; > > + if (i == PCI_ROM_RESOURCE && > + !pci_assign_roms() && !pci_dev_need_rom_bar(dev)) > + continue; > + > if ((r->flags & IORESOURCE_IO) && > !pci_find_host_bridge(dev->bus)->has_ioport) > continue; > @@ -1461,11 +1465,16 @@ out: > return good_size; > } > > -static inline bool is_optional(int i) > +bool __weak pci_assign_roms(void) > +{ > + return false; > +} > + > +static inline bool is_optional(struct pci_dev *dev, int i) > { > > if (i == PCI_ROM_RESOURCE) > - return true; > + return !pci_assign_roms() && !pci_dev_need_rom_bar(dev); > > #ifdef CONFIG_PCI_IOV > if (i >= PCI_IOV_RESOURCES && i <= PCI_IOV_RESOURCE_END) > @@ -1538,7 +1547,7 @@ static int pbus_size_mem(struct pci_bus > r_size = resource_size(r); > align = pci_resource_alignment(dev, r); > /* put SRIOV/ROM res to realloc list */ > - if (realloc_head && is_optional(i)) { > + if (realloc_head && is_optional(dev, i)) { > add_to_align_test_list(&align_test_add_list, > align, r_size, &dev->dev, i); > r->end = r->start - 1; > @@ -1549,6 +1558,9 @@ static int pbus_size_mem(struct pci_bus > max_add_align = align; > continue; > } > + if (!realloc_head && i == PCI_ROM_RESOURCE && > + !pci_assign_roms() && !pci_dev_need_rom_bar(dev)) > + continue; > > if (align > (1ULL<<37)) { /*128 Gb*/ > dev_warn(&dev->dev, "disabling BAR %d: %pR (bad > alignment %#llx)\n", > Index: linux-2.6/include/linux/pci.h > =================================================================== > --- linux-2.6.orig/include/linux/pci.h > +++ linux-2.6/include/linux/pci.h > @@ -182,6 +182,8 @@ enum pci_dev_flags { > PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 7), > /* Get VPD from function 0 VPD */ > PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 8), > + /* Need to have ROM BAR */ > + PCI_DEV_FLAGS_NEED_ROM_BAR = (__force pci_dev_flags_t) (1 << 9), > }; > > enum pci_irq_reroute_variant { > @@ -1965,6 +1967,14 @@ static inline bool pci_is_dev_assigned(s > { > return (pdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED) == > PCI_DEV_FLAGS_ASSIGNED; > } > +static inline void pci_dev_set_need_rom_bar(struct pci_dev *pdev) > +{ > + pdev->dev_flags |= PCI_DEV_FLAGS_NEED_ROM_BAR; > +} > +static inline bool pci_dev_need_rom_bar(struct pci_dev *pdev) > +{ > + return !!(pdev->dev_flags & PCI_DEV_FLAGS_NEED_ROM_BAR); > +} > > /** > * pci_ari_enabled - query ARI forwarding status > @@ -1976,4 +1986,7 @@ static inline bool pci_ari_enabled(struc > { > return bus->self && bus->self->ari_enabled; > } > + > +bool pci_assign_roms(void); > + > #endif /* LINUX_PCI_H */ > Index: linux-2.6/drivers/misc/pch_phub.c > =================================================================== > --- linux-2.6.orig/drivers/misc/pch_phub.c > +++ linux-2.6/drivers/misc/pch_phub.c > @@ -49,7 +49,6 @@ > > /* MAX number of INT_REDUCE_CONTROL registers */ > #define MAX_NUM_INT_REDUCE_CONTROL_REG 128 > -#define PCI_DEVICE_ID_PCH1_PHUB 0x8801 > #define PCH_MINOR_NOS 1 > #define CLKCFG_CAN_50MHZ 0x12000000 > #define CLKCFG_CANCLK_MASK 0xFF000000 > @@ -61,17 +60,6 @@ > #define CLKCFG_PLL2VCO (8 << 9) > #define CLKCFG_UARTCLKSEL (1 << 18) > > -/* Macros for ML7213 */ > -#define PCI_VENDOR_ID_ROHM 0x10db > -#define PCI_DEVICE_ID_ROHM_ML7213_PHUB 0x801A > - > -/* Macros for ML7223 */ > -#define PCI_DEVICE_ID_ROHM_ML7223_mPHUB 0x8012 /* for Bus-m */ > -#define PCI_DEVICE_ID_ROHM_ML7223_nPHUB 0x8002 /* for Bus-n */ > - > -/* Macros for ML7831 */ > -#define PCI_DEVICE_ID_ROHM_ML7831_PHUB 0x8801 > - > /* SROM ACCESS Macro */ > #define PCH_WORD_ADDR_MASK (~((1 << 2) - 1)) > > Index: linux-2.6/include/linux/pci_ids.h > =================================================================== > --- linux-2.6.orig/include/linux/pci_ids.h > +++ linux-2.6/include/linux/pci_ids.h > @@ -1122,6 +1122,12 @@ > #define PCI_VENDOR_ID_TCONRAD 0x10da > #define PCI_DEVICE_ID_TCONRAD_TOKENRING 0x0508 > > +#define PCI_VENDOR_ID_ROHM 0x10DB > +#define PCI_DEVICE_ID_ROHM_ML7213_PHUB 0x801A > +#define PCI_DEVICE_ID_ROHM_ML7223_mPHUB 0x8012 /* for Bus-m */ > +#define PCI_DEVICE_ID_ROHM_ML7223_nPHUB 0x8002 /* for Bus-n */ > +#define PCI_DEVICE_ID_ROHM_ML7831_PHUB 0x8801 > + > #define PCI_VENDOR_ID_NVIDIA 0x10de > #define PCI_DEVICE_ID_NVIDIA_TNT 0x0020 > #define PCI_DEVICE_ID_NVIDIA_TNT2 0x0028 > @@ -2900,6 +2906,7 @@ > #define PCI_DEVICE_ID_INTEL_82454NX 0x84cb > #define PCI_DEVICE_ID_INTEL_84460GX 0x84ea > #define PCI_DEVICE_ID_INTEL_IXP4XX 0x8500 > +#define PCI_DEVICE_ID_PCH1_PHUB 0x8801 > #define PCI_DEVICE_ID_INTEL_IXP2800 0x9004 > #define PCI_DEVICE_ID_INTEL_S21152BB 0xb152 > > Index: linux-2.6/drivers/dma/pch_dma.c > =================================================================== > --- linux-2.6.orig/drivers/dma/pch_dma.c > +++ linux-2.6/drivers/dma/pch_dma.c > @@ -976,7 +976,6 @@ static void pch_dma_remove(struct pci_de > } > > /* PCI Device ID of DMA device */ > -#define PCI_VENDOR_ID_ROHM 0x10DB > #define PCI_DEVICE_ID_EG20T_PCH_DMA_8CH 0x8810 > #define PCI_DEVICE_ID_EG20T_PCH_DMA_4CH 0x8815 > #define PCI_DEVICE_ID_ML7213_DMA1_8CH 0x8026 > Index: linux-2.6/drivers/gpio/gpio-ml-ioh.c > =================================================================== > --- linux-2.6.orig/drivers/gpio/gpio-ml-ioh.c > +++ linux-2.6/drivers/gpio/gpio-ml-ioh.c > @@ -31,8 +31,6 @@ > > #define IOH_IRQ_BASE 0 > > -#define PCI_VENDOR_ID_ROHM 0x10DB > - > struct ioh_reg_comn { > u32 ien; > u32 istatus; -- 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 Thu, Sep 24, 2015 at 09:35:20PM -0700, Yinghai Lu wrote: > On Thu, Sep 24, 2015 at 12:01 PM, Yinghai Lu <yinghai@kernel.org> wrote: > > > Or do we want to keep a white list to say which device should have > > ROM bar as mush have, and other is optional to have ? > > Subject: [RFC PATCH] PCI: Add pci_dev_need_rom_bar() > > Only set that for > 1. if BIOS/firmware already set ROM bar. > 2. via quirks for some devices. > > We assign those needed ROM bar as required > and other ROM bars as optional resources. I'd rather not have a whitelist if we can avoid it. We'd be continually adding new devices to it, and it makes the system harder to understand because there's no consistent rule about how ROMs are handled. Alex mentioned the idea of ripping the ROM, and I'd like to explore that idea a little more. What if we could temporarily assign space for the ROM during enumeration, read the contents, cache the contents somewhere, then deallocate the actual BAR space? We could hang onto the cache and give it to anybody who later needs the ROM. I know there are probably issues here, but I don't know what they are, so I'd like to at least have a conversation about it. Bjorn -- 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 Fri, 2015-09-25 at 09:35 -0500, Bjorn Helgaas wrote: > On Thu, Sep 24, 2015 at 09:35:20PM -0700, Yinghai Lu wrote: > > On Thu, Sep 24, 2015 at 12:01 PM, Yinghai Lu <yinghai@kernel.org> wrote: > > > > > Or do we want to keep a white list to say which device should have > > > ROM bar as mush have, and other is optional to have ? > > > > Subject: [RFC PATCH] PCI: Add pci_dev_need_rom_bar() > > > > Only set that for > > 1. if BIOS/firmware already set ROM bar. > > 2. via quirks for some devices. > > > > We assign those needed ROM bar as required > > and other ROM bars as optional resources. > > I'd rather not have a whitelist if we can avoid it. We'd be > continually adding new devices to it, and it makes the system harder > to understand because there's no consistent rule about how ROMs are > handled. > > Alex mentioned the idea of ripping the ROM, and I'd like to explore > that idea a little more. What if we could temporarily assign space > for the ROM during enumeration, read the contents, cache the contents > somewhere, then deallocate the actual BAR space? We could hang onto > the cache and give it to anybody who later needs the ROM. > > I know there are probably issues here, but I don't know what they are, > so I'd like to at least have a conversation about it. Ok, so for background in the case I mentioned, we can often use the pci-sysfs rom interface to get a copy of the device ROM, which we then pass to QEMU and we avoid any access to the physical ROM from the VM. We can obviously get the ROM from other sources too, but that's not really relevant here. If we want to extend this idea into the kernel, creating a buffer that holds the ROM contents that we access rather than mapping and enabling the ROM to provide access, we first need to get access to the ROM at least once. The simplification here is that we can do this on boot and we can re-use the space allocated to the standard BARs since we don't need space for both the ROM and the standard BARs at the same time. I think that in the vast majority of cases, we're going to find that the ROM BAR is smaller than the largest standard MMIO BAR of the device or at least smaller than the minimum bridge aperture. This suggests that we could simply record the BAR values, reprogram them to zero, then overlap the ROM BAR to the orignal addresses, enable, rip the ROM, then restore the configuration without needing to adjust any bridge apertures. That sounds pretty good, so long as we can consider the ROM to be perfectly static. I don't know if anything relies on an in-place update or if there are ROMs that are less read-only than others. Maybe it's safe to assume that or at least safe to assume that if the BIOS didn't leave room for them, then we can consider them static. It might be interesting to strace some of the userspace firmware update programs for add-in cards to see if they re-read the ROM to determine if it has changed. We already sort of do this for VGA ROMs. When a driver tries to map the boot VGA ROM they actually map the shadow copy at 0xC0000 (iirc) rather than the one on the device. This actually sort of sucks because this particular shadow copy certainly is not read-only and in all the glory that is VGA, the shadow sometimes gets modified by the execution of the VGA ROM itself and we no longer have access to the pristine device version of it (bad for device assignment of primary graphics). Now, if we want to make our own shadow copies for all ROMs, it seems pretty clear that we first need to get access to the ROM, which means we need to figure out if the BIOS mapped it. If the ROM BAR is outside of the bridge aperture (catching both 0 and 0xFFF..000) or overlaps a standard ROM BAR, we can consider it unprogrammed. In that case we need to try to do the trick above with unmapping standard BARs to get the shadow copy. Otherwise we should be able to get the shadow copy in place (maybe a question of which we prefer to use in this case). I would be willing to risk that if the BIOS didn't leave room for the ROM and we can't map it into the space used by other BARs or it doesn't fit the bridge aperture, we can spit out a printk warning and skip it. I expect a very low failure rate. What dependencies would we have on the BIOS programming of the ROM BAR if we took such an approach? It seems like we should always be able to detect invalid contents in the ROM BAR and it doesn't matter if they think we should have access or not, we own the device and can give ourselves access. Thanks, Alex -- 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 Fri, Sep 25, 2015 at 7:31 AM, Myron Stowe <myron.stowe@gmail.com> wrote: > On Thu, Sep 24, 2015 at 10:35 PM, Yinghai Lu <yinghai@kernel.org> wrote: >> On Thu, Sep 24, 2015 at 12:01 PM, Yinghai Lu <yinghai@kernel.org> wrote: >> >>> Or do we want to keep a white list to say which device should have >>> ROM bar as mush have, and other is optional to have ? > > I suppose this idea is one possible outcome that could occur but I > think we need to have a discussion first before we start making a lot > of changes. We need to try and come to some consensus with BIOS > engineers. I know that both sets have been alerted to this > conversation so *if* we come up with some good arguments to support > the kernel's current view/actions perhaps things will start to > progress. > > In the prior thread you replied: > "They are wrong. > It still depends on how addon card firmware and drivers > from OS to use it." > > So continuing my suggested thought experiment where you are sitting > across the table from your platform's BIOS engineers having this > discussion ... Do you *really* think your reply was helpful in any > way? Do you *really* think you did anything what so ever to get the > BIOS engineers to consider something they hadn't before. Do you > *really* think your reply was technically based in any way? Do you > have any specification references or such to back up such a strong > claim? > > Come on here Yinghai - you are an intelligent person. Take 1/10th the > time that you spent developing this patch and think, gather your > thoughts, and then sit down calmly, have a beer or coffee or tea > (which ever you prefer), relax, and take some time to reply in a > logical, thoughtful manner here with enough expression that others can > understand what you are getting at and hopefully even with some > passion or logic to try to convince the BIOS engineers that the > kernel's current behavior is correct. This is your area of expertise > - so stand up and rise to the occasion here! > > Hacking out a patch before this thread has played out serves no > purpose what so ever so I'm not even going to waste my time and look > at it. It serves no purpose and will only make matters worse as there > is already strong disagreement with the kernel's actions in these > regards. Sigh, that was a terrible response on my part - I'm trying to encourage engagement in this discussion and yet my response likely has exactly the opposite result; shutting you down. Yinghai, I apologize, truly! Others have privately said that you may be uncomfortable with expressing your views due to language skills. If that's the case then please don't be intimidated and limit your contributions. I expect you know at least two languages which is 50% more than me so don't worry about grammar or such - that's not important and we could benefit from your experience and knowledge. English is my only language and I still too often find it difficult to express my opinions. Again, I'm sorry for my rash, harsh, response, Myron > >> >> Subject: [RFC PATCH] PCI: Add pci_dev_need_rom_bar() >> >> Only set that for >> 1. if BIOS/firmware already set ROM bar. >> 2. via quirks for some devices. >> >> We assign those needed ROM bar as required >> and other ROM bars as optional resources. >> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> >> --- >> arch/x86/pci/i386.c | 9 +++++- >> drivers/dma/pch_dma.c | 1 >> drivers/gpio/gpio-ml-ioh.c | 2 - >> drivers/misc/pch_phub.c | 12 -------- >> drivers/pci/probe.c | 7 +++++ >> drivers/pci/quirks.c | 63 +++++++++++++++++++++++++++++++++++++++++++++ >> drivers/pci/setup-bus.c | 18 ++++++++++-- >> include/linux/pci.h | 13 +++++++++ >> include/linux/pci_ids.h | 7 +++++ >> 9 files changed, 112 insertions(+), 20 deletions(-) >> >> Index: linux-2.6/arch/x86/pci/i386.c >> =================================================================== >> --- linux-2.6.orig/arch/x86/pci/i386.c >> +++ linux-2.6/arch/x86/pci/i386.c >> @@ -377,11 +377,16 @@ static void pcibios_allocate_rom_resourc >> } >> } >> >> +bool pci_assign_roms(void) >> +{ >> + return !!(pci_probe & PCI_ASSIGN_ROMS); >> +} >> + >> static int __init pcibios_assign_resources(void) >> { >> struct pci_host_bridge *host_bridge = NULL; >> >> - if (!(pci_probe & PCI_ASSIGN_ROMS)) >> + if (!pci_assign_roms()) >> for_each_pci_host_bridge(host_bridge) >> pcibios_allocate_rom_resources(host_bridge->bus); >> >> @@ -406,7 +411,7 @@ void pcibios_resource_survey_bus(struct >> pcibios_allocate_resources(bus, 0); >> pcibios_allocate_resources(bus, 1); >> >> - if (!(pci_probe & PCI_ASSIGN_ROMS)) >> + if (!pci_assign_roms()) >> pcibios_allocate_rom_resources(bus); >> } >> >> Index: linux-2.6/drivers/pci/probe.c >> =================================================================== >> --- linux-2.6.orig/drivers/pci/probe.c >> +++ linux-2.6/drivers/pci/probe.c >> @@ -224,6 +224,13 @@ int __pci_read_base(struct pci_dev *dev, >> l64 = l & PCI_ROM_ADDRESS_MASK; >> sz64 = sz & PCI_ROM_ADDRESS_MASK; >> mask64 = (u32)PCI_ROM_ADDRESS_MASK; >> + /* simple validation */ >> + if (l64 && sz64 && >> + (l64 & 0xff000000) != 0xff000000 && >> + system_state == SYSTEM_BOOTING) { >> + dev_printk(KERN_DEBUG, &dev->dev, "set dev_flags NEED_ROM_BAR\n"); >> + pci_dev_set_need_rom_bar(dev); >> + } >> } >> >> if (res->flags & IORESOURCE_MEM_64) { >> Index: linux-2.6/drivers/pci/quirks.c >> =================================================================== >> --- linux-2.6.orig/drivers/pci/quirks.c >> +++ linux-2.6/drivers/pci/quirks.c >> @@ -4197,3 +4197,66 @@ static void quirk_intel_qat_vf_cap(struc >> } >> } >> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443, quirk_intel_qat_vf_cap); >> + >> +/* from drivers/mtd/maps/pci.c */ >> +static void quirk_set_need_rom_bar(struct pci_dev *pdev) >> +{ >> + if (!pci_dev_need_rom_bar(pdev)) { >> + dev_printk(KERN_DEBUG, &pdev->dev, "set dev_flags NEED_ROM_BAR\n"); >> + pci_dev_set_need_rom_bar(pdev); >> + } >> +} >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_DEC, PCI_DEVICE_ID_DEC_21285, >> + quirk_set_need_rom_bar); >> + >> +#ifdef CONFIG_PARISC >> +/* from drivers/video/console/sticore.c */ >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_EG, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FX6, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FX4, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FX2, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FXE, >> + quirk_set_need_rom_bar); >> +#endif >> + >> +/* from drivers/misc/pch_phub.c */ >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_PCH1_PHUB, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7213_PHUB, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7223_mPHUB, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7223_nPHUB, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7831_PHUB, >> + quirk_set_need_rom_bar); >> + >> +/* from drivers/net/ethernet/sun/sungem.c */ >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_GEM, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_RIO_GEM, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_UNI_N_GMAC, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_UNI_N_GMACP, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_UNI_N_GMAC2, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_K2_GMAC, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_SH_SUNGEM, >> + quirk_set_need_rom_bar); >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_IPID2_GMAC, >> + quirk_set_need_rom_bar); >> + >> +/* from drivers/net/ethernet/sun/sunhme.c */ >> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_HAPPYMEAL, >> + quirk_set_need_rom_bar); >> + >> +/* from drm and fbdev */ >> +DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, >> + 8, quirk_set_need_rom_bar); >> Index: linux-2.6/drivers/pci/setup-bus.c >> =================================================================== >> --- linux-2.6.orig/drivers/pci/setup-bus.c >> +++ linux-2.6/drivers/pci/setup-bus.c >> @@ -226,6 +226,10 @@ static void pdev_assign_resources_prepar >> if (resource_disabled(r) || r->parent) >> continue; >> >> + if (i == PCI_ROM_RESOURCE && >> + !pci_assign_roms() && !pci_dev_need_rom_bar(dev)) >> + continue; >> + >> if ((r->flags & IORESOURCE_IO) && >> !pci_find_host_bridge(dev->bus)->has_ioport) >> continue; >> @@ -1461,11 +1465,16 @@ out: >> return good_size; >> } >> >> -static inline bool is_optional(int i) >> +bool __weak pci_assign_roms(void) >> +{ >> + return false; >> +} >> + >> +static inline bool is_optional(struct pci_dev *dev, int i) >> { >> >> if (i == PCI_ROM_RESOURCE) >> - return true; >> + return !pci_assign_roms() && !pci_dev_need_rom_bar(dev); >> >> #ifdef CONFIG_PCI_IOV >> if (i >= PCI_IOV_RESOURCES && i <= PCI_IOV_RESOURCE_END) >> @@ -1538,7 +1547,7 @@ static int pbus_size_mem(struct pci_bus >> r_size = resource_size(r); >> align = pci_resource_alignment(dev, r); >> /* put SRIOV/ROM res to realloc list */ >> - if (realloc_head && is_optional(i)) { >> + if (realloc_head && is_optional(dev, i)) { >> add_to_align_test_list(&align_test_add_list, >> align, r_size, &dev->dev, i); >> r->end = r->start - 1; >> @@ -1549,6 +1558,9 @@ static int pbus_size_mem(struct pci_bus >> max_add_align = align; >> continue; >> } >> + if (!realloc_head && i == PCI_ROM_RESOURCE && >> + !pci_assign_roms() && !pci_dev_need_rom_bar(dev)) >> + continue; >> >> if (align > (1ULL<<37)) { /*128 Gb*/ >> dev_warn(&dev->dev, "disabling BAR %d: %pR (bad >> alignment %#llx)\n", >> Index: linux-2.6/include/linux/pci.h >> =================================================================== >> --- linux-2.6.orig/include/linux/pci.h >> +++ linux-2.6/include/linux/pci.h >> @@ -182,6 +182,8 @@ enum pci_dev_flags { >> PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 7), >> /* Get VPD from function 0 VPD */ >> PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 8), >> + /* Need to have ROM BAR */ >> + PCI_DEV_FLAGS_NEED_ROM_BAR = (__force pci_dev_flags_t) (1 << 9), >> }; >> >> enum pci_irq_reroute_variant { >> @@ -1965,6 +1967,14 @@ static inline bool pci_is_dev_assigned(s >> { >> return (pdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED) == >> PCI_DEV_FLAGS_ASSIGNED; >> } >> +static inline void pci_dev_set_need_rom_bar(struct pci_dev *pdev) >> +{ >> + pdev->dev_flags |= PCI_DEV_FLAGS_NEED_ROM_BAR; >> +} >> +static inline bool pci_dev_need_rom_bar(struct pci_dev *pdev) >> +{ >> + return !!(pdev->dev_flags & PCI_DEV_FLAGS_NEED_ROM_BAR); >> +} >> >> /** >> * pci_ari_enabled - query ARI forwarding status >> @@ -1976,4 +1986,7 @@ static inline bool pci_ari_enabled(struc >> { >> return bus->self && bus->self->ari_enabled; >> } >> + >> +bool pci_assign_roms(void); >> + >> #endif /* LINUX_PCI_H */ >> Index: linux-2.6/drivers/misc/pch_phub.c >> =================================================================== >> --- linux-2.6.orig/drivers/misc/pch_phub.c >> +++ linux-2.6/drivers/misc/pch_phub.c >> @@ -49,7 +49,6 @@ >> >> /* MAX number of INT_REDUCE_CONTROL registers */ >> #define MAX_NUM_INT_REDUCE_CONTROL_REG 128 >> -#define PCI_DEVICE_ID_PCH1_PHUB 0x8801 >> #define PCH_MINOR_NOS 1 >> #define CLKCFG_CAN_50MHZ 0x12000000 >> #define CLKCFG_CANCLK_MASK 0xFF000000 >> @@ -61,17 +60,6 @@ >> #define CLKCFG_PLL2VCO (8 << 9) >> #define CLKCFG_UARTCLKSEL (1 << 18) >> >> -/* Macros for ML7213 */ >> -#define PCI_VENDOR_ID_ROHM 0x10db >> -#define PCI_DEVICE_ID_ROHM_ML7213_PHUB 0x801A >> - >> -/* Macros for ML7223 */ >> -#define PCI_DEVICE_ID_ROHM_ML7223_mPHUB 0x8012 /* for Bus-m */ >> -#define PCI_DEVICE_ID_ROHM_ML7223_nPHUB 0x8002 /* for Bus-n */ >> - >> -/* Macros for ML7831 */ >> -#define PCI_DEVICE_ID_ROHM_ML7831_PHUB 0x8801 >> - >> /* SROM ACCESS Macro */ >> #define PCH_WORD_ADDR_MASK (~((1 << 2) - 1)) >> >> Index: linux-2.6/include/linux/pci_ids.h >> =================================================================== >> --- linux-2.6.orig/include/linux/pci_ids.h >> +++ linux-2.6/include/linux/pci_ids.h >> @@ -1122,6 +1122,12 @@ >> #define PCI_VENDOR_ID_TCONRAD 0x10da >> #define PCI_DEVICE_ID_TCONRAD_TOKENRING 0x0508 >> >> +#define PCI_VENDOR_ID_ROHM 0x10DB >> +#define PCI_DEVICE_ID_ROHM_ML7213_PHUB 0x801A >> +#define PCI_DEVICE_ID_ROHM_ML7223_mPHUB 0x8012 /* for Bus-m */ >> +#define PCI_DEVICE_ID_ROHM_ML7223_nPHUB 0x8002 /* for Bus-n */ >> +#define PCI_DEVICE_ID_ROHM_ML7831_PHUB 0x8801 >> + >> #define PCI_VENDOR_ID_NVIDIA 0x10de >> #define PCI_DEVICE_ID_NVIDIA_TNT 0x0020 >> #define PCI_DEVICE_ID_NVIDIA_TNT2 0x0028 >> @@ -2900,6 +2906,7 @@ >> #define PCI_DEVICE_ID_INTEL_82454NX 0x84cb >> #define PCI_DEVICE_ID_INTEL_84460GX 0x84ea >> #define PCI_DEVICE_ID_INTEL_IXP4XX 0x8500 >> +#define PCI_DEVICE_ID_PCH1_PHUB 0x8801 >> #define PCI_DEVICE_ID_INTEL_IXP2800 0x9004 >> #define PCI_DEVICE_ID_INTEL_S21152BB 0xb152 >> >> Index: linux-2.6/drivers/dma/pch_dma.c >> =================================================================== >> --- linux-2.6.orig/drivers/dma/pch_dma.c >> +++ linux-2.6/drivers/dma/pch_dma.c >> @@ -976,7 +976,6 @@ static void pch_dma_remove(struct pci_de >> } >> >> /* PCI Device ID of DMA device */ >> -#define PCI_VENDOR_ID_ROHM 0x10DB >> #define PCI_DEVICE_ID_EG20T_PCH_DMA_8CH 0x8810 >> #define PCI_DEVICE_ID_EG20T_PCH_DMA_4CH 0x8815 >> #define PCI_DEVICE_ID_ML7213_DMA1_8CH 0x8026 >> Index: linux-2.6/drivers/gpio/gpio-ml-ioh.c >> =================================================================== >> --- linux-2.6.orig/drivers/gpio/gpio-ml-ioh.c >> +++ linux-2.6/drivers/gpio/gpio-ml-ioh.c >> @@ -31,8 +31,6 @@ >> >> #define IOH_IRQ_BASE 0 >> >> -#define PCI_VENDOR_ID_ROHM 0x10DB >> - >> struct ioh_reg_comn { >> u32 ien; >> u32 istatus; -- 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 Fri, Sep 25, 2015 at 9:18 AM, Alex Williamson <alex.williamson@redhat.com> wrote: >> > > Or do we want to keep a white list to say which device should have >> > > ROM bar as mush have, and other is optional to have ? >> > >> > Subject: [RFC PATCH] PCI: Add pci_dev_need_rom_bar() >> > >> > Only set that for >> > 1. if BIOS/firmware already set ROM bar. >> > 2. via quirks for some devices. >> > >> > We assign those needed ROM bar as required >> > and other ROM bars as optional resources. >> >> I'd rather not have a whitelist if we can avoid it. We'd be >> continually adding new devices to it, and it makes the system harder >> to understand because there's no consistent rule about how ROMs are >> handled. I don't like that whitelist way, and hope we can find better way to handle all the cases elegantly. >> >> Alex mentioned the idea of ripping the ROM, and I'd like to explore >> that idea a little more. What if we could temporarily assign space >> for the ROM during enumeration, read the contents, cache the contents >> somewhere, then deallocate the actual BAR space? We could hang onto >> the cache and give it to anybody who later needs the ROM. > > That sounds pretty good, so long as we can consider the ROM to be > perfectly static. I don't know if anything relies on an in-place update > or if there are ROMs that are less read-only than others. Maybe it's > safe to assume that or at least safe to assume that if the BIOS didn't > leave room for them, then we can consider them static. It might be > interesting to strace some of the userspace firmware update programs for > add-in cards to see if they re-read the ROM to determine if it has > changed. Should cover most cases. But there is some driver like drivers/mtd/maps/pci.c::drivers/mtd/maps/pci.c do have write operation to the mmio range. > > We already sort of do this for VGA ROMs. When a driver tries to map the > boot VGA ROM they actually map the shadow copy at 0xC0000 (iirc) rather > than the one on the device. This actually sort of sucks because this > particular shadow copy certainly is not read-only and in all the glory > that is VGA, the shadow sometimes gets modified by the execution of the > VGA ROM itself and we no longer have access to the pristine device > version of it (bad for device assignment of primary graphics). > > Now, if we want to make our own shadow copies for all ROMs, it seems > pretty clear that we first need to get access to the ROM, which means we > need to figure out if the BIOS mapped it. If the ROM BAR is outside of > the bridge aperture (catching both 0 and 0xFFF..000) or overlaps a > standard ROM BAR, we can consider it unprogrammed. In that case we need > to try to do the trick above with unmapping standard BARs to get the > shadow copy. Otherwise we should be able to get the shadow copy in > place (maybe a question of which we prefer to use in this case). I > would be willing to risk that if the BIOS didn't leave room for the ROM > and we can't map it into the space used by other BARs or it doesn't fit > the bridge aperture, we can spit out a printk warning and skip it. I > expect a very low failure rate. Maybe we can combine two methods together: 1. have NEED_ROM_BAR, and it is set a) if BIOS allocate resource to yet (maybe not, so we leave space for MMIO bars) b) via limited whitelist that will not support static copy. 2. kernel will try to allocate resource to ROM bar with NEED_ROM_BAR as required, and others as optional 3. for ROM bar that can not get assigned, kernel try to borrow mmio range from other MMIO bar on the device, and save a copy and expose that via /sys/.../rom That should happen FINAL_FIXUP stage before driver get loaded. --- There is risk on it, some add-on card firmware would stop working if kernel ever try to change MMIO bar. then we will need blacklist to skip BAR on those devices. -- 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
Index: linux-2.6/arch/x86/pci/i386.c =================================================================== --- linux-2.6.orig/arch/x86/pci/i386.c +++ linux-2.6/arch/x86/pci/i386.c @@ -377,11 +377,16 @@ static void pcibios_allocate_rom_resourc } } +bool pci_assign_roms(void) +{ + return !!(pci_probe & PCI_ASSIGN_ROMS); +} + static int __init pcibios_assign_resources(void) { struct pci_host_bridge *host_bridge = NULL; - if (!(pci_probe & PCI_ASSIGN_ROMS)) + if (!pci_assign_roms()) for_each_pci_host_bridge(host_bridge) pcibios_allocate_rom_resources(host_bridge->bus); @@ -406,7 +411,7 @@ void pcibios_resource_survey_bus(struct pcibios_allocate_resources(bus, 0); pcibios_allocate_resources(bus, 1); - if (!(pci_probe & PCI_ASSIGN_ROMS)) + if (!pci_assign_roms()) pcibios_allocate_rom_resources(bus); } Index: linux-2.6/drivers/pci/probe.c =================================================================== --- linux-2.6.orig/drivers/pci/probe.c +++ linux-2.6/drivers/pci/probe.c @@ -224,6 +224,13 @@ int __pci_read_base(struct pci_dev *dev, l64 = l & PCI_ROM_ADDRESS_MASK; sz64 = sz & PCI_ROM_ADDRESS_MASK; mask64 = (u32)PCI_ROM_ADDRESS_MASK; + /* simple validation */ + if (l64 && sz64 && + (l64 & 0xff000000) != 0xff000000 && + system_state == SYSTEM_BOOTING) { + dev_printk(KERN_DEBUG, &dev->dev, "set dev_flags NEED_ROM_BAR\n"); + pci_dev_set_need_rom_bar(dev); + } } if (res->flags & IORESOURCE_MEM_64) { Index: linux-2.6/drivers/pci/quirks.c =================================================================== --- linux-2.6.orig/drivers/pci/quirks.c +++ linux-2.6/drivers/pci/quirks.c @@ -4197,3 +4197,66 @@ static void quirk_intel_qat_vf_cap(struc } } DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443, quirk_intel_qat_vf_cap); + +/* from drivers/mtd/maps/pci.c */ +static void quirk_set_need_rom_bar(struct pci_dev *pdev) +{ + if (!pci_dev_need_rom_bar(pdev)) { + dev_printk(KERN_DEBUG, &pdev->dev, "set dev_flags NEED_ROM_BAR\n"); + pci_dev_set_need_rom_bar(pdev); + } +} +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_DEC, PCI_DEVICE_ID_DEC_21285, + quirk_set_need_rom_bar); + +#ifdef CONFIG_PARISC +/* from drivers/video/console/sticore.c */ +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_EG, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FX6, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FX4, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FX2, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_VISUALIZE_FXE, + quirk_set_need_rom_bar); +#endif + +/* from drivers/misc/pch_phub.c */ +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_PCH1_PHUB, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7213_PHUB, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7223_mPHUB, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7223_nPHUB, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ROHM, PCI_DEVICE_ID_ROHM_ML7831_PHUB, + quirk_set_need_rom_bar); + +/* from drivers/net/ethernet/sun/sungem.c */ +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_GEM, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_RIO_GEM, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_UNI_N_GMAC, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_UNI_N_GMACP, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_UNI_N_GMAC2, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_K2_GMAC, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_SH_SUNGEM, + quirk_set_need_rom_bar); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_IPID2_GMAC, + quirk_set_need_rom_bar); + +/* from drivers/net/ethernet/sun/sunhme.c */ +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_HAPPYMEAL, + quirk_set_need_rom_bar); + +/* from drm and fbdev */ +DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_DISPLAY_VGA, + 8, quirk_set_need_rom_bar); Index: linux-2.6/drivers/pci/setup-bus.c =================================================================== --- linux-2.6.orig/drivers/pci/setup-bus.c +++ linux-2.6/drivers/pci/setup-bus.c @@ -226,6 +226,10 @@ static void pdev_assign_resources_prepar if (resource_disabled(r) || r->parent) continue; + if (i == PCI_ROM_RESOURCE && + !pci_assign_roms() && !pci_dev_need_rom_bar(dev)) + continue; + if ((r->flags & IORESOURCE_IO) && !pci_find_host_bridge(dev->bus)->has_ioport) continue; @@ -1461,11 +1465,16 @@ out: return good_size; } -static inline bool is_optional(int i) +bool __weak pci_assign_roms(void) +{ + return false; +} + +static inline bool is_optional(struct pci_dev *dev, int i) { if (i == PCI_ROM_RESOURCE) - return true; + return !pci_assign_roms() && !pci_dev_need_rom_bar(dev); #ifdef CONFIG_PCI_IOV if (i >= PCI_IOV_RESOURCES && i <= PCI_IOV_RESOURCE_END) @@ -1538,7 +1547,7 @@ static int pbus_size_mem(struct pci_bus r_size = resource_size(r); align = pci_resource_alignment(dev, r); /* put SRIOV/ROM res to realloc list */ - if (realloc_head && is_optional(i)) { + if (realloc_head && is_optional(dev, i)) { add_to_align_test_list(&align_test_add_list, align, r_size, &dev->dev, i); r->end = r->start - 1; @@ -1549,6 +1558,9 @@ static int pbus_size_mem(struct pci_bus max_add_align = align; continue; } + if (!realloc_head && i == PCI_ROM_RESOURCE && + !pci_assign_roms() && !pci_dev_need_rom_bar(dev)) + continue; if (align > (1ULL<<37)) { /*128 Gb*/ dev_warn(&dev->dev, "disabling BAR %d: %pR (bad alignment %#llx)\n", Index: linux-2.6/include/linux/pci.h =================================================================== --- linux-2.6.orig/include/linux/pci.h +++ linux-2.6/include/linux/pci.h @@ -182,6 +182,8 @@ enum pci_dev_flags { PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 7), /* Get VPD from function 0 VPD */ PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 8), + /* Need to have ROM BAR */ + PCI_DEV_FLAGS_NEED_ROM_BAR = (__force pci_dev_flags_t) (1 << 9), }; enum pci_irq_reroute_variant { @@ -1965,6 +1967,14 @@ static inline bool pci_is_dev_assigned(s { return (pdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED) == PCI_DEV_FLAGS_ASSIGNED; } +static inline void pci_dev_set_need_rom_bar(struct pci_dev *pdev) +{ + pdev->dev_flags |= PCI_DEV_FLAGS_NEED_ROM_BAR; +} +static inline bool pci_dev_need_rom_bar(struct pci_dev *pdev) +{ + return !!(pdev->dev_flags & PCI_DEV_FLAGS_NEED_ROM_BAR); +} /** * pci_ari_enabled - query ARI forwarding status @@ -1976,4 +1986,7 @@ static inline bool pci_ari_enabled(struc { return bus->self && bus->self->ari_enabled; } + +bool pci_assign_roms(void); + #endif /* LINUX_PCI_H */ Index: linux-2.6/drivers/misc/pch_phub.c =================================================================== --- linux-2.6.orig/drivers/misc/pch_phub.c +++ linux-2.6/drivers/misc/pch_phub.c @@ -49,7 +49,6 @@ /* MAX number of INT_REDUCE_CONTROL registers */ #define MAX_NUM_INT_REDUCE_CONTROL_REG 128 -#define PCI_DEVICE_ID_PCH1_PHUB 0x8801 #define PCH_MINOR_NOS 1 #define CLKCFG_CAN_50MHZ 0x12000000 #define CLKCFG_CANCLK_MASK 0xFF000000 @@ -61,17 +60,6 @@ #define CLKCFG_PLL2VCO (8 << 9) #define CLKCFG_UARTCLKSEL (1 << 18) -/* Macros for ML7213 */ -#define PCI_VENDOR_ID_ROHM 0x10db -#define PCI_DEVICE_ID_ROHM_ML7213_PHUB 0x801A - -/* Macros for ML7223 */ -#define PCI_DEVICE_ID_ROHM_ML7223_mPHUB 0x8012 /* for Bus-m */ -#define PCI_DEVICE_ID_ROHM_ML7223_nPHUB 0x8002 /* for Bus-n */ - -/* Macros for ML7831 */ -#define PCI_DEVICE_ID_ROHM_ML7831_PHUB 0x8801 - /* SROM ACCESS Macro */ #define PCH_WORD_ADDR_MASK (~((1 << 2) - 1)) Index: linux-2.6/include/linux/pci_ids.h =================================================================== --- linux-2.6.orig/include/linux/pci_ids.h +++ linux-2.6/include/linux/pci_ids.h @@ -1122,6 +1122,12 @@ #define PCI_VENDOR_ID_TCONRAD 0x10da #define PCI_DEVICE_ID_TCONRAD_TOKENRING 0x0508 +#define PCI_VENDOR_ID_ROHM 0x10DB +#define PCI_DEVICE_ID_ROHM_ML7213_PHUB 0x801A +#define PCI_DEVICE_ID_ROHM_ML7223_mPHUB 0x8012 /* for Bus-m */ +#define PCI_DEVICE_ID_ROHM_ML7223_nPHUB 0x8002 /* for Bus-n */ +#define PCI_DEVICE_ID_ROHM_ML7831_PHUB 0x8801 + #define PCI_VENDOR_ID_NVIDIA 0x10de #define PCI_DEVICE_ID_NVIDIA_TNT 0x0020 #define PCI_DEVICE_ID_NVIDIA_TNT2 0x0028 @@ -2900,6 +2906,7 @@ #define PCI_DEVICE_ID_INTEL_82454NX 0x84cb #define PCI_DEVICE_ID_INTEL_84460GX 0x84ea #define PCI_DEVICE_ID_INTEL_IXP4XX 0x8500 +#define PCI_DEVICE_ID_PCH1_PHUB 0x8801 #define PCI_DEVICE_ID_INTEL_IXP2800 0x9004 #define PCI_DEVICE_ID_INTEL_S21152BB 0xb152 Index: linux-2.6/drivers/dma/pch_dma.c =================================================================== --- linux-2.6.orig/drivers/dma/pch_dma.c +++ linux-2.6/drivers/dma/pch_dma.c @@ -976,7 +976,6 @@ static void pch_dma_remove(struct pci_de } /* PCI Device ID of DMA device */ -#define PCI_VENDOR_ID_ROHM 0x10DB #define PCI_DEVICE_ID_EG20T_PCH_DMA_8CH 0x8810 #define PCI_DEVICE_ID_EG20T_PCH_DMA_4CH 0x8815 #define PCI_DEVICE_ID_ML7213_DMA1_8CH 0x8026 Index: linux-2.6/drivers/gpio/gpio-ml-ioh.c =================================================================== --- linux-2.6.orig/drivers/gpio/gpio-ml-ioh.c +++ linux-2.6/drivers/gpio/gpio-ml-ioh.c @@ -31,8 +31,6 @@ #define IOH_IRQ_BASE 0 -#define PCI_VENDOR_ID_ROHM 0x10DB - struct ioh_reg_comn { u32 ien; u32 istatus;