On Mon, Mar 13, 2017 at 10:57:48PM +0100, Mason wrote: > On 13/03/2017 22:40, Bjorn Helgaas wrote: > > > On Sat, Mar 11, 2017 at 11:57:56AM +0100, Mason wrote: > > > >> On 10/03/2017 18:49, Mason wrote: > >> > >>> static void tango_pcie_bar_quirk(struct pci_dev *dev) > >>> { > >>> struct pci_bus *bus = dev->bus; > >>> > >>> printk("%s: bus=%d devfn=%d\n", __func__, bus->number, dev->devfn); > >>> > >>> pci_write_config_dword(dev, PCI_BASE_ADDRESS_0, 0x80000004); > >>> } > >>> DECLARE_PCI_FIXUP_FINAL(0x1105, PCI_ANY_ID, tango_pcie_bar_quirk); > >> > >> And this is where the elusive "black magic" happens. > >> > >> Is it "safe" to configure a BAR behind Linux's back? > > > > No. Linux maintains a struct resource for every BAR. This quirk > > makes the BAR out of sync with the resource, so Linux no longer has an > > accurate idea of what bus address space is consumed and what is > > available. > > Even when Linux is not able to map the BAR, since it's too > large to fit in the mem window? I don't think there's much point in advertising a BAR that isn't really a BAR and making assumptions about how Linux will handle it. So my answer remains "No, I don't think it's a good idea to change a BAR behind the back of the PCI core. It might work now, but there's no guarantee it will keep working." > > Normally a BAR is for mapping device registers into PCI bus address > > space. If this BAR controls how the RC forwards PCI DMA transactions > > to RAM, then it's not really a BAR and you should prevent Linux from > > seeing it as a BAR. You could do this by special-casing it in the > > config accessor so reads return 0 and writes are dropped. Then you > > could write the register in your host bridge driver safely because the > > PCI core would think the BAR is not implemented. > > In fact, that's what I used to do in a previous version :-) > > I'd like to push support for this PCIe controller upstream. > > Is the code I posted on the right track? > Maybe I can post a RFC patch tomorrow? No need to ask before posting a patch :)