On 2014-12-04 15:32:50, Scott Duplichan wrote: > Jordan Justen [mailto:[email protected]] wrote: > > ]On 2014-12-04 08:00:35, Scott Duplichan wrote: > ]> Rafael Machado [mailto:[email protected]] wrote: > ]> > ]> ]Hi everyone > ]> ] > ]> ]I'm facing a problem with a system and I'd like some help to check > ]> ]one strange behavior. At the > UDK2014.SP1\MdePkg\Include\Library\PciCf8Lib.h > ]> ] > ]> ]We have the following macro: > ]> ]#define PCI_CF8_LIB_ADDRESS(Bus,Device,Function,Offset) \ > ]> ] (((Offset) & 0xfff) | (((Function) & 0x07) << 12) | (((Device) & 0x1f) > << 15) | (((Bus) & 0xff) << ]20)) > ]> ] > ]> ]When using the following parameters Bus: 0xFF, Dev: 0x1F, Func: 0x7 > ]> ]The resulting value is: 0xFFFF000 > ]> > ]> That looks more like a PCIe offset, not a CF8 dword value. > ]> > ]> ]As far as I know, the expected value should be: 0xFF1F0700 > ]> > ]> Shouldn't the CF8 dword look like: > ]> 10000000 11111111 11111 111 000000 00 or 0x80FFFF00? > ] > ]I think the PCI_CF8_LIB_ADDRESS need not map directly to what is > ]programmed into the hardware, since the library code can adjust it as > ]needed. > > True, but the chosen name is quite confusing. PCI CF8 predates UEFI > (and even EFI) by years. When someone speaks of a PCI CF8 address, > there is little doubt they refer to the layout defined in the > conventional PCI specification. A name such as PCI_LIB_ADDRESS or > PCI_GENERIC_LIB_ADDRESS would have eliminated the confusion.
Yeah, it is a bit confusing. It seems likely that MdePkg/Library/BasePciLibCf8 could just directly access cf8/cfc, rather than going through PciCf8Lib. (And, perhaps then there is no need for the PcfCf8Lib interface?) > ]PCI_CF8_LIB_ADDRESS appears to be designed to match PCI_LIB_ADDRESS. > ](I guess PciCf8Lib did not want to reuse PCI_LIB_ADDRESS directly.) > ] > ]By the way, it is probably better to interface with PciLib rather than > ]PciCf8Lib. > > ] > ]> ]So I believe that the correct macro should be something like: > ]> ] > ]> ]#define PCI_CF8_LIB_ADDRESS(Bus,Device,Function,Offset) \ > ]> ] (((Offset) & 0xff) | (((Function) & 0x07) << 8) | (((Device) & 0x1f) << > 16) | (((Bus) & 0xff) << 24)) > ]> > ]> I think something more like this... > ]> #define PCI_CF8_LIB_ADDRESS(Bus,Device,Function,Offset) \ > ]> (((Offset) & 0xfc) | (((Function) & 0x07) << 8) | (((Device) & 0x1f) << > 11) | (((Bus) & 0xff) << 16)) > ] > ]This wouldn't let a PCI_CF8_LIB_ADDRESS access every possible byte. > ](Masking the 2 low bits.) Once again, I think the library code can > ]translate the address bits as needed for the cf8/cfc access mechanism. > > Masking the low two bits is a requirement of the PCI specification... > at least when making an actual CF8 address. So again, I think the choice > of name PCI_CF8_LIB_ADDRESS is confusing. In other words, PCI_CF8_LIB_ADDRESS > is in no way related to a PCI CF8 address. It is, however, related to the addresses used by the PciCf8Lib interface, and that is where the macro is defined. > Now, a complaint/rant not just about UEFI, but for many other coding projects > that include PCI access functions: Why not pass segment, bus, device, > function, > and offset directly to the PCI access functions? It is nice to have this all in a single integer to easily pass it around. For example, between layers of function calls, or to store within data structures. -Jordan ------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
