> On Dec 4, 2014, at 4:30 PM, Jordan Justen <[email protected]> wrote:
>
> 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?)
>
I think you guys are missing a few subtle points.
The PciLib class, PciCf8Lib.h class, and the PciExpressLib.h share the same
abstraction for PCI access.
In general if you are just trying to read and write PCI config space you should
always use the PciLib. The platform will map it to the correct library instance
to do the operation.
The only reason the PciCf8Lib, and PciExpressLib classes exist is for early
chipset code. There are times when the chipset specifies you must use CF8, or
you must use PCIe, and that is the only reasons these library classes exist.
Thus likely Scott could just use the PciLib, and map it to the correct
instance. His code should not care about the underlying mechanism used to
accesses PCI Config space. The goal was also to make the algorithms in the code
more portable, and having the same abstraction for PCI config space does this.
Thanks,
Andrew Fish
>> ]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
------------------------------------------------------------------------------
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
