Thanks, Andrew Fish
> On Aug 25, 2015, at 2:18 PM, Benjamin Herrenschmidt > <[email protected]> wrote: > > On Tue, 2015-08-25 at 06:47 -0700, Andrew Fish wrote: > >>> Sadly it doesn't seem like the UEFI spec caters well to that non >>> -1:1 >>> mapping (which is quite common outside of the x86 world), and >>> doesn't >>> really sat what GetBarAttributes() is supposed to return. However, >>> practically, for GOP drivers (and possibly others) to work, we need >>> to >>> return a CPU physical address, so we need to *translate* the BAR >>> value. >>> >> I think you have it backwards. The UEFI spec abstracts PCI properly, >> and drivers are implemented incorrectly, but they work due to x86 PC >> assumptions. >> >> GetBarAttributes() returns the BAR. > > Well, the spec doesn't spell it out precisely but … > The BAR is a PCI address by definition. >> That is the correct thing to do. If that Bar was accessed via> >> EFI_PCI_IO_PROTOCOL.Mem.Read()/EFI_PCI_IO_PROTOCOL.Mem.Write()//EFI_P >> CI_IO_PROTOCOL.CopyMem() then it is being accessed relative to the >> BAR and everything should work. Casting a pointer to the value >> returned in GetBarAttributes() is not guaranteed to work. > > Except that a lot drivers do it :-( > Then we should get these drivers fixed. > For example anything using OptionRomPkg/Library/BitBltLib does it. > > So the GOP drivers in-tree in EDK2 do it, I can ask nVidia what their > EFI driver does but I woudn't be surprised if it's everywhere. > Well the GOP returns a FrameBuffer, so that address needs to be adjusted. So that seems to be the bug in the spec. I’ll bring this up to the UEFI Forum. But I think for the non FrameBuffer case it would be better to fix the drivers. Thanks, Andrew Fish > UfsPciHcDxe is another one in the tree. > > Arguably it's the only way to have usable performance out of a GOP... > So in any case, it doesn't change the situation. A spec not follow by > drivers is worthless, only what the drivers do actually matter and thus > I need to provide something that will make them work (famous Linus > quote more/less :-) > > One could argue that the BAR can be read just fine by a driver using a > PCI configuration space read, and thus GetBarAttribute() is the perfect > place to return the CPU address. This would be a nop on platforms using > a 1:1 mapping and make things "just work" on others. That would require > a simple clarification in the next spec version. > >> There have also been lots of bugs in regards to DMA. Drivers don’t >> use Map()/Unmap()/AllocateBuffer() correctly, but since x86 makes DMA >> cache coherent it all seems to work. When we did the 1st port to ARM >> even the edk2 USB stack had issues not doing DMA correctly (On ARM >> you end up doing DMA into non-cached buffers, so you need to follow >> the rules). > > Well, on some ARMs at least but yes. I see. > >> Adding hacks to work around broken drivers is just going to cause >> chaos. > > While I generally agree, I think in this specific case, it does make > sense to redefine the semantics of GetBarAttributes(). That or provided > a new optional protocol to allow direct mapping of the BAR into the CPU > space. > > Something tells me that otherwise a bunch of drivers are essentially > going to be unfixable (/me eyes GPUs ...) > >> You are going to need to work with the driver writers to get their >> driver fixed. >> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__tianocore.sourceforge.net_wiki_UEFI-5FDriver-5FWriter-27s-5FGuide&d=BQIDaQ&c=eEvniauFctOgLOKGJOplqw&r=1HnUuXD1wDvw67rut5_idw&m=hlb6m5Pc6p6DlF5msA_xOLq7RbOHDol2huBe2-9uvT8&s=GZPYMjLcMvwAh5svYi8l4whKOKof3r3JVWIZUXsBB4g&e= >> >> The PCI I/O Protocol provides services that allow a PCI driver to >> easily access the resources of the PCI controllers it is currently >> managing. These services hide platform- specific implementation >> details and prevent a PCI driver from inadvertently accessing >> resources of the motherboard or other PCI controllers. The PCI I/O >> Protocol has also been designed to simplify the implementation of PCI >> drivers. For example, a PCI driver should never read the BARs in the >> PCI configuration header. Instead, the PCI driver passes in a >> BarIndex and Offset into the PCI I/O Protocol services. The PCI bus >> driver is responsible for managing the PCI controller’s BARs. > > Yes, I understand that, but in practice I very much doubt those drivers > will be fixed and the end user is going to blame the platform instead. > > Ben. > >> Thanks, >> >> Andrew Fish >> >>> Cheers, >>> Ben. >>> _______________________________________________ >>> edk2-devel mailing list >>> [email protected] >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.01.org_m >>> ailman_listinfo_edk2 >>> -2Ddevel&d=BQICAg&c=eEvniauFctOgLOKGJOplqw&r=1HnUuXD1wDvw67rut5_idw >>> &m=QE9d1rL3ijH-gexHgoNe54QWnW >>> -gCrncwCMR3WJMIDs&s=MyGORn2OZERedpoAMdvuxIcbai_x3TEDR9WZqoxJmJg&e= _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
