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 ... > 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 :-( 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. 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. > > http://tianocore.sourceforge.net/wiki/UEFI_Driver_Writer's_Guide > 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
