Re: [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 24 March 2017 at 17:10, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/24/17 08:15, Ard Biesheuvel wrote:
>> On 24 March 2017 at 03:44, Zeng, Star <star.zeng@intel.com> wrote:
>>> I just think it seems a generic problem.
>>> Some platforms may dynamically determine whether ACPI should be supported or not.
>>> Some platforms may dynamically determine whether SMBIOS should be supported or not.
>>> Some platforms may dynamically determine whether HII should be supported or not.
>>> ...
>>>
>>> We are thinking whether we can have a generic NULL instance to support all this kind of cases, for example:
>>> 1. Define a FixedAtBuild PCD PcdDepexGuid that will be a VOID* type PCD point to a depex GUID.
>>> 2. Implement a NULL instance DepexLib.
>>> [Defines]
>>>   INF_VERSION                    = 0x00010005
>>>   BASE_NAME                      = DepexLib
>>>   FILE_GUID                      = XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX
>>>   MODULE_TYPE                    = BASE
>>>   VERSION_STRING                 = 1.0
>>>   LIBRARY_CLASS                  = NULL
>>>
>>> [Sources]
>>>   DepexLib.c
>>>
>>> [Packages]
>>>   XXXPkg/XXXPkg.dec
>>>
>>> [Depex]
>>>   PcdDepexGuid
>>>
>>> 3. Link NULL instance and configure PCD in dsc.
>>> MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf {
>>>     <LibraryClasses>
>>>       NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>>     <PcdsFixedAtBuild>
>>>       PcdDepexGuid|gEdkiiPlatformHasAcpiGuid
>>> }
>>> MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
>>>     <LibraryClasses>
>>>       NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>>     <PcdsFixedAtBuild>
>>>       PcdDepexGuid|gEdkiiPlatformHasSmbiosGuid
>>> }
>>>
>>> But current BaseTools does not support the syntax to declare PCD in [Depex] section of inf yet.
>>>
>>> Based on the statements above, I have below comments.
>>> 1. I agree to add the GUID into MdeModulePkg, but how about using gEdkiiPlatformHasAcpiGuid instead of gEdkiiPlatformHasAcpiProtocolGuid and add the GUID into [Guids] section of MdeModulePkg.dec? As Platform can install gEdkiiPlatformHasAcpiGuid PPI to control PEIM and install gEdkiiPlatformHasAcpiGuid PROTOCOL to control DRIVER. And another, no need to add a include file PlatformHasAcpi.h as BaseTools supports to add the GUID definitions to AutoGen files from package dec.
>>> 2. You can file bugzilla bug to request BaseTools syntax support to declare PCD in [Depex] section of inf. Then PcdDepexGuid and DepexLib could be finally added into core package MdeModulePkg or even MdePkg (I prefer). Before that, how about implementing the PlatformHasAcpiLib in none core package?
>>>
>>
>> Hello Star,
>>
>> Thanks for taking the time to think about this. It is much appreciated.
>>
>> I like the generic approach for dynamic dependencies. The more
>> architectures are supported in UEFI, the more often we are likely to
>> see the need for such things.
>>
>> I will let Laszlo speak for himself, as the author of the patches, but
>> I am perfectly fine with
>> a) the outline above of how we address this in the long term,
>> b) taking anything we need for the short term into ArmPkg, and move it
>> to MdeModulePkg and/or MdePkg later.
>
> To confirm what Leif requested and what we've since discussed on IRC:
> since the Has-Acpi GUID is now going under MdeModulePkg, I'll move the
> Has-DeviceTree GUID under EmbeddedPkg.
>

Ack