On 10/23/14 19:49, Andrew Fish wrote:
>
>> On Oct 23, 2014, at 10:31 AM, Laszlo Ersek <ler...@redhat.com
>> <mailto:ler...@redhat.com>> wrote:
>>
>> On 10/23/14 19:02, Gabriel Somlo wrote:
>>> Jordan, Laszlo:
>>>
>>> On Mon, Oct 06, 2014 at 11:42:53AM -0400, Gabriel L. Somlo wrote:
>>>> So, to decide how I feel about further splitting out DxeAcpiTimerLib
>>>> into a version which does use PCDs and another version which does not,
>>>> I added some DEBUG statements to track every time the host bridge
>>>> gets queried right now, and here's what I got:
>>>>
>>>>
>>>> PlatformPei:MiscInitialization()
>>>> Base:AcpiTimerLibConstructor() (PEIM)
>>>> Dxe:AcpiTimerLibConstructor() (DXE_CORE)
>>>> Dxe:AcpiTimerLibConstructor() (DXE_RUNTIME_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (DXE_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (DXE_RUNTIME_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (DXE_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (DXE_RUNTIME_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (DXE_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (DXE_RUNTIME_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (DXE_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (UEFI_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (UEFI_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (UEFI_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (UEFI_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (DXE_DRIVER)
>>>> Dxe:AcpiTimerLibConstructor() (UEFI_DRIVER)
>>>> BdsPlatform:PciInitialization()
>>>> BdsPlatform:AcpiInitialization()
>>>> Dxe:AcpiTimerLibConstructor() (DXE_DRIVER)
>>>
>>> I think I have the mechanics of PCDs figured out.
>>>
>>> Between the two of you, you've suggested I use the DxeAcpiTimerLib
>>> instance from DXE_DRIVER, DXE_RUNTIME_DRIVER, UEFI_DRIVER, and
>>> UEFI_APPLICATION.
>>>
>>> After some testing, it appears DXE_DRIVER and DXE_RUNTIME_DRIVER do
>>> indeed see the dynamic PCD I set from PEI.
>>>
>>> UEFI_DRIVER *clearly* does not; it gets linked against
>>> MdePkg/Library/BasePcdLibNull/PcdLib.c, which implements
>>> LibPcdGet* with ASSERT(FALSE).
>>
>> Yes, because by default we resolve the PcdLib class to the Null instance
>> (in the DSC files) for UEFI_DRIVER. The background is that UEFI drivers
>> are meant to be "portable" across UEFI implementations, and the PCD
>> protocol is not a standard protocol -- it's an "edk2-only" feature.
>>
>> But, referring to
>>
>> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/10156/focus=10197
>>
>> On 09/30/14 00:17, Jordan Justen wrote:
>>> In OvmfPkg*.dsc:
>>> * [...]
>>> * Replace BasePcdLibNull.inf with DxePcdLib.inf as needed in DXE/UEFI
>>> LibraryClasses.
>>>
>>> (This paragraph is a bit tangential. Feel free to ignore it. :) I
>>> think this is not quite valid for pure UEFI drivers, since they should
>>> not rely on the PCD protocol. But, I think that OVMF as a platform can
>>> bend this rule. [...]
>>
>> So, we might want to decide that going forward we will indeed bend this
>> rule, and *explicitly* declare all UEFI_DRIVERs under OvmfPkg/
>> non-portable outside of edk2.
>>
>> In which case you'd simply change the default PcdLib resolution for
>> UEFI_DRIVER and UEFI_APPLICATION modules in the DSC files, as Jordan
>> describes in the quote.
>>
>
> In general it is a bad idea for a UEFI Driver/Application to depend on a
> platform specific lib. A UEF Driver/Application should be using
> gBS->Stall() for a portable delay.
>
> Historically I think the TimerLib was used for for performance profiling
> too. But the MdeModulePkg can produce the gPerformanceExProtocolGuid,
> and has a PerformanceLib that depends on the protocol.
>
> So why don’t we just drop support for UEFI_DRIVER and UEFI_APPLICATION,
> and fix the consumers.
The only UEFI_DRIVER under OvmfPkg/ that directly depends on TimerLib is
QemuVideoDxe (based on the [LibraryClasses] sections in the respective
INF files). I grepped the QemuVideoDxe subdir for the five functions
that the TimerLib class exports -- no hits. So, even that one *direct*
dependency should be bogus.
Then, I cross-referenced the TimerLib usages listed in "build.report"
with the UEFI_DRIVER module inf files under OvmfPkg, in order to find
any indirect dependencies (ie. where a UEFI_DRIVER module inherits a
TimerLib dependency via another library instance). The only match was
QemuVideoDxe.
So this looked like a good idea. I actually eliminated the TimerLib
dependency from QemuVideoDxe, restricted the client module types in
AcpiTimerLib.inf (so that UEFI_DRIVER would be excluded), and fired off
another build.
And then:
.../OvmfPkg/OvmfPkgX64.dsc(...): error 1001: Module type [UEFI_DRIVER]
is not supported by library instance
[.../OvmfPkg/Library/AcpiTimerLib/AcpiTimerLib.inf]
consumed by
[.../IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf]
I don't think we'll set out to detach UEFI_DRIVERs under
IntelFrameworkModulePkg/ from the TimerLib class...
Thanks,
Laszlo
------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
