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.

]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.

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? This would eliminate 
confusing macros, and make viewing the arguments in a debugger easier.
http://notabs.org/coding/theMacroMistake.htm

Thanks,
Scott

]-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

Reply via email to