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

["Zeng, Star" <star.zeng@intel.com>]

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?


Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, March 23, 2017 5:09 PM
To: Zeng, Star <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; afish@apple.com
Cc: Tian, Feng <feng.tian@intel.com>; Laszlo Ersek <lersek@redhat.com>; edk2-devel-01 <edk2-devel@lists.01.org>; Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library

On 23 March 2017 at 01:41, Zeng, Star <star.zeng@intel.com> wrote:
> I prefer to do not include the protocol definition and the library instance into MdeModulePkg at this moment until they need to be used by multiple platforms/archs.
>

I disagree. Nowhere in the PI or UEFI spec is there any mention (for any architecture) that ACPI is mandatory. Given that EDK2 is the reference platform UEFI/PI, and not for ACPI or the PC/x86 platform, I think it is unreasonable to have lots and lots of PC quirks (i.e., allocations below 4 GB) in MdeModulePkg, but then disallow a clean approach to make ACPI selectable, in a way that is true to the spirit of PI (i.e., using protocol dependencies)

So I think at least the protocol definitions belong in MdeModulePkg.

Thanks,
Ard.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel