On 09/30/14 18:59, Gabriel L. Somlo wrote:
> On Tue, Sep 30, 2014 at 11:52:28AM +0200, Laszlo Ersek wrote:
>>> So, my n00b question is why bother with PCDs in the first place,
>>> given that their use in AcpiTimerLib (where the bulk of the savings
>>> from ditching the IS_Q35 macro is realized) may be frowned upon ?
>>
>> I agree that PCDs are not optimal here; see my other email. In some
>> AcpiTimerLib clients they are downright unavailable.
>>
>>> Why set the global static variable based on retrieving a PCD and not
>>> based on just reading the host bridge DID during the constructor ?
>>
>> You can't set globals in AcpiTimerLib, no matter where you'd populate
>> them *from*. See my other email.
>
> And now I remember you telling me about commit d3a24ff5 and "no
> globals in AcpiTimerLib" a while back -- so we're condemned to check
> the host bridge DID each and every single time we try to access a
> power management register.
Almost. We're condemned to do that in the very early UEFI phases, but
we're not condemned to it in the later phases, where it really matters.
See below for more.
>>> Either way, we save reading the bridge DID during each of the
>>> bazillion calls to GetTimerTick, and what's reading the host bridge
>>> DID a few tens of times (15xTimerLibConstructor plus three or four
>>> more) if it gets us out of mixing PCDs with AcpiTimerLib ?
>>
>> The cost of the libraries I recommended in my other email is:
>> - SEC and PEI_CORE: same as now
>>
>> - PEIM, DXE_CORE, DXE_RUNTIME_DRIVER, DXE_DRIVER: GetFirstGuidHob().
>> A linear search that doesn't trap to the hypervisor. I expect the
>> HOB list to have a few tens of elements at most. Also, caching the
>> HOB once found would turn this into O(1).
>>
>> - UEFI_DRIVER, UEFI_APPLICATION, alternative 1: library construction
>> (ie. driver / app startup) is somewhat expensive, but access is
>> O(1) afterwards (cached values).
>>
>> - UEFI_DRIVER, UEFI_APPLICATION, alternative 2: same HOB-based
>> library instance as above.
>
> So before I decide whether or not to go down that rabbit hole,
> can we re-evaluate the decision to simply use some form of macro
> (of the IS_Q35_HOSTBRIDGE and PCI_PM_REG sort) ? With all the
> naming convention fixes you and others suggested earlier, of course.
>
> The library alternative would essentially just wrap this inside
> another (arguably more OO-ish, professional-looking) layer, or am
> I getting that wrong ?
Somewhat wrong; not completely. :)
A "library class" is basically a header file.
A "library instance" is code implementing the functions & extern
variables exported in the header file.
"Modules" (== libraries and executables) in edk2 express their library
dependencies on *library classes*, not library instances. This means
that *any* instance of the library class can satisfy the dependent
module's needs.
... Modulo a few restrictions, of course. :) There can be several
reasons for implementing the same library class in different ways (ie.
with different library instances):
- speed
- memory footprint
to name the trivial reasons.
There is a much more important customization category however: the UEFI
*module type* that a library is usable in.
UEFI module types are roughly parallel to UEFI phases.
- In the SEC phase you can run the SEC module.
- In the PEI phase you run the PEI_CORE (approx) and PEIMs (PEI
modules).
- In the DXE phase you run the DXE_CORE (approx), and DXE_DRIVER,
DXE_RUNTIME_DRIVER, DXE_SMM_DRIVER modules. (The DXE_RUNTIME_DRIVER
and the DXE_SMM_DRIVER survive into OS runtime.)
- In the BDS phase (approx) you run UEFI_DRIVER and UEFI_APPLICATION
modules.
The different UEFI phases put different restrictions on the
corresponding modules, and they provide them with different
possibilities and different responsibilities too. The important thing is
that a library *instance* implementing the WhateverLib *class* for (say)
PEIMs may (have to) be written very differently from a library
*instance* implementing the *same* WhateverLib *class* for (say)
DXE_DRIVERs. The library instance is statically linked into the
dependent module, hence it is subject to the exact same restrictions and
possibilities as the client code.
So, it is not unusual at all to have different library instances for the
same library class.
As an example, consider the HobLib class. OvmfPkgX64.dsc resolves this
class to:
[LibraryClasses.common.SEC]
HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
[LibraryClasses.common.PEI_CORE]
HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
[LibraryClasses.common.PEIM]
HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
[LibraryClasses.common.DXE_CORE]
HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
[LibraryClasses.common.DXE_RUNTIME_DRIVER]
HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
[LibraryClasses.common.UEFI_DRIVER]
HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
[LibraryClasses.common.DXE_DRIVER]
HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
[LibraryClasses.common.UEFI_APPLICATION]
HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
As you can see,
- for SEC, PEI_CORE, PEIM modules, we use the
MdePkg/Library/PeiHobLib/PeiHobLib.inf instance.
- for DXE_CORE, we use the the
MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
instance.
- for the rest, we use the
HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
instance.
All three instances implement the exact same library class (
- header: MdePkg/Include/Library/HobLib.h
- class decl: see in "MdePkg/MdePkg.dec"
), but their internals are very different, because the way to access the
HOB list differs between the PEI phase and the DXE phase (and the
DXE_CORE even sits in the middle, "between" those two phases).
Now, look into those three INF files:
$ git grep LIBRARY_CLASS -- 'MdePkg/Library/*HobLib/*HobLib.inf'
MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf: LIBRARY_CLASS
= HobLib|DXE_CORE
MdePkg/Library/DxeHobLib/DxeHobLib.inf: LIBRARY_CLASS =
HobLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER SMM_CORE DXE_SMM_DRIVER
UEFI_APPLICATION UEFI_DRIVER
MdePkg/Library/PeiHobLib/PeiHobLib.inf: LIBRARY_CLASS =
HobLib|PEIM PEI_CORE SEC
As you can see, each library instance *itself* lists (may list) the edk2
module types (ie. ~ the UEFI phases) that it can work in.
If you try to add a cross-restriction class->instance resolution; for
example, try to resolve the HobLib class for a "MODULE_TYPE = PEIM"
module with "MdePkg/Library/DxeHobLib/DxeHobLib.inf", then the build
system will catch it and report an error. Because "PEIM" is not on the
LIBRARY_CLASS restriction list in "DxeHobLib.inf".
Alright, so after this wall of text, why is your assumption "somewhat"
wrong? It is "somewhat" wrong because it doesn't account for the fact
that we can do different things in different UEFI phases and edk2 module
types.
If you re-read my other email, you'll see that I recommend a new library
class ("OvmfPlatformsLib"), to extract the Q35 / PIIX4 dependencies of
AcpiTimerLib into, and that I recommend three different library
*instances* for that class. (AcpiTimerLib itself is not restricted to
any module type, but it would inherit any module type restrictions from
the OvmfPlatformsLib *instance* that ultimately resolves its dependency
on the OvmfPlatformsLib *class*.)
So, in SEC and PEI_CORE, indeed we couldn't do better than the current
AcpiTimerLib code. Hence, in the library instance that would implement
the OvmfPlatformsLib class for SEC and PEI_CORE module types, there
would be no difference, and you are simply right -- in those instances
the new library would essentially be a wrapper for the current code.
However, for all the other module types, we could provide
OvmfPlatformsLib instances that rendered the dependent AcpiTimerLib much
more frugal. And, the bulk of UEFI "happens" in the DXE phase and later.
It's okay to be a bit wasteful when linked into in SEC and PEI_CORE, and
potentially a little bit wasteful when linked into PEIMs as well. The
"big win" is in DXE_DRIVER and UEFI_DRIVER modules.
I'm okay if you're willing to implement this. If you want me to do it
instead, I'm okay with that as well -- you decide. :)
Laszlo
------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
