Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/29/17 11:49, Gao, Liming wrote:
> Laszlo:
> I agree GuidCName can't be used to initialize the global variable. If
> there is such requirement, GUID C MACRO will have to be defined.
> Then, its header file will be required.
> Now, we have no rule to forbid to add the header file if it has no
> other definitions except for GUID value, and we have no such
> discussion to define those rules. It is the developer choice to add
> it or not.
> Here, I want to mention that BaseTools has added "extern gGuidCName"
> into AutoGen header file. The consumer code can directly use
> gGuidCName without include its header file. If no GUID MACRO usage,
> its header file is not necessary.
>
> For this case gEdkiiPlatformHasAcpiGuid, I know it will be as
> protocol dependency. I don't see its GUID C MACRO usage. So, I
> suggest not to add its header file. This is just my opinion.
Thank you for explaining the situation.
For now we've moved all of the generic/core patches in this series to
EmbeddedPkg (and the v4 series has been committed). Once Leif returns,
we can perhaps think about moving those GUIDs / lib instances to
MdeModulePkg, and we could agree at that point to drop the Guid/ header.
> For BaseTools enhancement to generated GUID C MACRO in autogen.h, I
> think it is valuable. To avoid the generated MACRO conflict with the
> existing MACRO, the generated MACRO will still have "G" prefix, such
> as gEdkiiPlatformHasAcpiGuid ==> G_EDKII_PLATFORM_HAS_ACPI_GUID.
Thanks -- I filed <https://bugzilla.tianocore.org/show_bug.cgi?id=449>.
I marked it as a whishlist item.
Cheers
Laszlo
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Tuesday, March 28, 2017 3:50 PM
>> To: Gao, Liming ; Ard Biesheuvel
>>
>> Cc: Zeng, Star ; Kinney, Michael D
>> ; af...@apple.com; Tian, Feng
>> ; edk2-devel-01 ; Leif
>> Lindholm
>> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
>> ACPI Protocol, and plug-in library
>>
>> On 03/28/17 07:25, Gao, Liming wrote:
>>> Ersek:
>>> I don't want to block your patch. You can still check in Guid header file
>>> if you
>> think it is necessary.
>>
>> I don't want to check in without formal MdeModulePkg maintainer
>> approval. If I check in a patch without formal approval, that will only
>> lead to chaos. I don't want to circumvent the process; I want the
>> process to *work*.
>>
>> If MdeModulePkg maintainers agree with my
>>
>> [PATCH v3 04/12] MdeModulePkg: introduce EDKII Platform Has ACPI GUID
>>
>> then they should please give their Reviewed-by for it.
>>
>> If they disagree with it, they should please explain why.
>>
>> The specific argument you raised, namely that a BaseTools utility
>> generates the necessary C language artifacts for GUID use, is not
>> precise. BaseTools generate *some* artifacts, but they do not generate a
>> macro that is usable for initializing a GUID object of static storage
>> duration (= global variable GUID, or a GUID field in a global variable
>> structure).
>>
>> This is what the ISO C99 standard says:
>>
>> 6.7.8 Initialization
>>
>> [...]
>>
>> Constraints
>>
>> [...]
>>
>> 4 All the expressions in an initializer for an object that has static
>>storage duration shall be constant expressions or string literals.
>>
>> [...]
>>
>> 16 Otherwise, the initializer for an object that has aggregate or
>> union type shall be a brace-enclosed list of initializers for the
>> elements or named members.
>>
>> [...]
>>
>> 20 If the aggregate or union contains elements or members that are
>> aggregates or unions, these rules apply recursively to the
>> subaggregates or contained unions. [...]
>>
>> This is the GUID type definition:
>>
>> typedef struct {
>> UINT32 Data1;
>> UINT16 Data2;
>> UINT16 Data3;
>> UINT8 Data4[8];
>> } GUID;
>>
>> For this structure, the above standardese implies that we have to
>> provide integer constant expressions in the initializer.
>>
>> 6.6 Constant expressions
>>
>> [...]
>>
>> 6 An /integer constant expression/ shall have integer type and shall
>>only have operands that are integer constants, enumeration
>>constants, character constants, /sizeof/ expressions whose results
>>are integer constants, and floating constants that are the
>>immediate operands of casts. Cast operators in an intege
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
Laszlo:
I agree GuidCName can't be used to initialize the global variable. If there
is such requirement, GUID C MACRO will have to be defined. Then, its header
file will be required.
Now, we have no rule to forbid to add the header file if it has no other
definitions except for GUID value, and we have no such discussion to define
those rules. It is the developer choice to add it or not.
Here, I want to mention that BaseTools has added "extern gGuidCName" into
AutoGen header file. The consumer code can directly use gGuidCName without
include its header file. If no GUID MACRO usage, its header file is not
necessary.
For this case gEdkiiPlatformHasAcpiGuid, I know it will be as protocol
dependency. I don't see its GUID C MACRO usage. So, I suggest not to add its
header file. This is just my opinion.
For BaseTools enhancement to generated GUID C MACRO in autogen.h, I think it
is valuable. To avoid the generated MACRO conflict with the existing MACRO, the
generated MACRO will still have "G" prefix, such as gEdkiiPlatformHasAcpiGuid
==> G_EDKII_PLATFORM_HAS_ACPI_GUID.
Thanks
Liming
>-Original Message-
>From: Laszlo Ersek [mailto:ler...@redhat.com]
>Sent: Tuesday, March 28, 2017 3:50 PM
>To: Gao, Liming ; Ard Biesheuvel
>
>Cc: Zeng, Star ; Kinney, Michael D
>; af...@apple.com; Tian, Feng
>; edk2-devel-01 ; Leif
>Lindholm
>Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
>ACPI Protocol, and plug-in library
>
>On 03/28/17 07:25, Gao, Liming wrote:
>> Ersek:
>> I don't want to block your patch. You can still check in Guid header file
>> if you
>think it is necessary.
>
>I don't want to check in without formal MdeModulePkg maintainer
>approval. If I check in a patch without formal approval, that will only
>lead to chaos. I don't want to circumvent the process; I want the
>process to *work*.
>
>If MdeModulePkg maintainers agree with my
>
> [PATCH v3 04/12] MdeModulePkg: introduce EDKII Platform Has ACPI GUID
>
>then they should please give their Reviewed-by for it.
>
>If they disagree with it, they should please explain why.
>
>The specific argument you raised, namely that a BaseTools utility
>generates the necessary C language artifacts for GUID use, is not
>precise. BaseTools generate *some* artifacts, but they do not generate a
>macro that is usable for initializing a GUID object of static storage
>duration (= global variable GUID, or a GUID field in a global variable
>structure).
>
>This is what the ISO C99 standard says:
>
> 6.7.8 Initialization
>
> [...]
>
> Constraints
>
> [...]
>
> 4 All the expressions in an initializer for an object that has static
>storage duration shall be constant expressions or string literals.
>
> [...]
>
> 16 Otherwise, the initializer for an object that has aggregate or
> union type shall be a brace-enclosed list of initializers for the
> elements or named members.
>
> [...]
>
> 20 If the aggregate or union contains elements or members that are
> aggregates or unions, these rules apply recursively to the
> subaggregates or contained unions. [...]
>
>This is the GUID type definition:
>
>typedef struct {
> UINT32 Data1;
> UINT16 Data2;
> UINT16 Data3;
> UINT8 Data4[8];
>} GUID;
>
>For this structure, the above standardese implies that we have to
>provide integer constant expressions in the initializer.
>
> 6.6 Constant expressions
>
> [...]
>
> 6 An /integer constant expression/ shall have integer type and shall
>only have operands that are integer constants, enumeration
>constants, character constants, /sizeof/ expressions whose results
>are integer constants, and floating constants that are the
>immediate operands of casts. Cast operators in an integer constant
>expression shall only convert arithmetic types to integer types,
>except as part of an operand to the /sizeof/ operator.
>
>The end result is that "gEdkiiPlatformHasAcpiGuid" is not usable in such
>contexts, but the EDKII_PLATFORM_HAS_ACPI_GUID macro is.
>
>
>If you want, I can file a TianoCore feature request BZ for generating
>such macros as well in BaseTools, but for now, I don't think it should
>either block my patch noted above, or force me to drop the Guid/ header
>file from the patch. Right now, the Guid/ header adds something that is
>not available from BaseTools, and I wouldn't like to delay the v3 series
>any longer.
>
>Do you agree to the above method? I file a TianoCore Feature Request BZ
>for BaseTools, and MdeModulePkg maintainers formally approve the v3
>04/12 patch?
>
>
>(Side point: I think the
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/28/17 07:25, Gao, Liming wrote:
> Ersek:
> I don't want to block your patch. You can still check in Guid header file
> if you think it is necessary.
I don't want to check in without formal MdeModulePkg maintainer
approval. If I check in a patch without formal approval, that will only
lead to chaos. I don't want to circumvent the process; I want the
process to *work*.
If MdeModulePkg maintainers agree with my
[PATCH v3 04/12] MdeModulePkg: introduce EDKII Platform Has ACPI GUID
then they should please give their Reviewed-by for it.
If they disagree with it, they should please explain why.
The specific argument you raised, namely that a BaseTools utility
generates the necessary C language artifacts for GUID use, is not
precise. BaseTools generate *some* artifacts, but they do not generate a
macro that is usable for initializing a GUID object of static storage
duration (= global variable GUID, or a GUID field in a global variable
structure).
This is what the ISO C99 standard says:
6.7.8 Initialization
[...]
Constraints
[...]
4 All the expressions in an initializer for an object that has static
storage duration shall be constant expressions or string literals.
[...]
16 Otherwise, the initializer for an object that has aggregate or
union type shall be a brace-enclosed list of initializers for the
elements or named members.
[...]
20 If the aggregate or union contains elements or members that are
aggregates or unions, these rules apply recursively to the
subaggregates or contained unions. [...]
This is the GUID type definition:
typedef struct {
UINT32 Data1;
UINT16 Data2;
UINT16 Data3;
UINT8 Data4[8];
} GUID;
For this structure, the above standardese implies that we have to
provide integer constant expressions in the initializer.
6.6 Constant expressions
[...]
6 An /integer constant expression/ shall have integer type and shall
only have operands that are integer constants, enumeration
constants, character constants, /sizeof/ expressions whose results
are integer constants, and floating constants that are the
immediate operands of casts. Cast operators in an integer constant
expression shall only convert arithmetic types to integer types,
except as part of an operand to the /sizeof/ operator.
The end result is that "gEdkiiPlatformHasAcpiGuid" is not usable in such
contexts, but the EDKII_PLATFORM_HAS_ACPI_GUID macro is.
If you want, I can file a TianoCore feature request BZ for generating
such macros as well in BaseTools, but for now, I don't think it should
either block my patch noted above, or force me to drop the Guid/ header
file from the patch. Right now, the Guid/ header adds something that is
not available from BaseTools, and I wouldn't like to delay the v3 series
any longer.
Do you agree to the above method? I file a TianoCore Feature Request BZ
for BaseTools, and MdeModulePkg maintainers formally approve the v3
04/12 patch?
(Side point: I think the Guid/ header file has another benefit:
documentation. I don't think we can add the amount of documentation that
is acceptable in a standalone Guid/ header to an in-line comment in the
.DEC file. So, I think that the new rule for not adding Guid/ headers is
immature at this point -- and, worse, I don't recall an open
announcement or discussion about this rule, so that we could have raised
concerns before turning the proposal into an actual rule.)
Thanks
Laszlo
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Tuesday, March 28, 2017 1:32 AM
>> To: Ard Biesheuvel ; Gao, Liming
>>
>> Cc: Zeng, Star ; Kinney, Michael D
>> ; af...@apple.com; Tian, Feng
>> ; edk2-devel-01 ; Leif
>> Lindholm
>> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
>> ACPI Protocol, and plug-in library
>>
>> On 03/27/17 09:00, Ard Biesheuvel wrote:
>>> On 27 March 2017 at 03:42, Gao, Liming wrote:
>>>> Laszlo:
>>>> On GUID header file, some header file are here, because they are added
>>>> before BaseTools supports it. Now, we allow not to add
>> header file if the header file only has GUID value definition.
>>>
>>> I have to agree with Laszlo here. The BaseTools support is incomplete,
>>> since it does not add a #define for the GUID to AutoGen.h. This makes
>>> it impossible to initialize static structures containing GUIDs, such
>>> as templates containing vendor device paths.
>>>
>>> For instance, the following
>>>
>>> typedef struct {
>>> EFI_GUID foo;
>>> } TYPE;
>>>
>>> TYPE mFoo {
>>> SOME_GUID
>>> };
>>>
>>> is not possible without a Guid/xxx.h inclu
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
Ersek:
I don't want to block your patch. You can still check in Guid header file if
you think it is necessary.
Thanks
Liming
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, March 28, 2017 1:32 AM
> To: Ard Biesheuvel ; Gao, Liming
>
> Cc: Zeng, Star ; Kinney, Michael D
> ; af...@apple.com; Tian, Feng
> ; edk2-devel-01 ; Leif Lindholm
>
> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
> ACPI Protocol, and plug-in library
>
> On 03/27/17 09:00, Ard Biesheuvel wrote:
> > On 27 March 2017 at 03:42, Gao, Liming wrote:
> >> Laszlo:
> >> On GUID header file, some header file are here, because they are added
> >> before BaseTools supports it. Now, we allow not to add
> header file if the header file only has GUID value definition.
> >
> > I have to agree with Laszlo here. The BaseTools support is incomplete,
> > since it does not add a #define for the GUID to AutoGen.h. This makes
> > it impossible to initialize static structures containing GUIDs, such
> > as templates containing vendor device paths.
> >
> > For instance, the following
> >
> > typedef struct {
> > EFI_GUID foo;
> > } TYPE;
> >
> > TYPE mFoo {
> > SOME_GUID
> > };
> >
> > is not possible without a Guid/xxx.h include file containing a #define
> > for SOME_GUID.
>
> I wonder if we can commit this series before end of April.
>
> Or is that too soon? End of May perhaps?
>
> The mind boggles.
>
> Laszlo
>
> >
> >> On GUID C MACRO, we suggest to use GuidCName in every C source code. So,
> >> we don't suggest to add it. As you know, some
> existing header files have GuidCName and GUID Macro. Now, we have no plan to
> clean up the existing one. But, we expect new added
> file follows this rule.
> >>
> >> Thanks
> >> Liming
> >>> -Original Message-
> >>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >>> Laszlo Ersek
> >>> Sent: Saturday, March 25, 2017 1:09 AM
> >>> To: Zeng, Star ; Ard Biesheuvel
> >>> ; Kinney, Michael D
> >>> ; af...@apple.com
> >>> Cc: Tian, Feng ; Gao, Liming ;
> >>> edk2-devel-01 ; Leif Lindholm
> >>>
> >>> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
> >>> ACPI Protocol, and plug-in library
> >>>
> >>> On 03/24/17 04:44, Zeng, Star 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 = ----
> >>>> 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 {
> >>>>
> >>>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
> >>>>
> >>>> PcdDepexGuid|gEdkiiPlatformHasAcpiGuid
> >>>> }
> >>>> MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
> >>>>
> >>>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
> >>>>
> >>>> PcdDepexGuid|gEdkiiPlatformHasSmbiosGuid
> &g
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 27 March 2017 at 18:31, Laszlo Ersek wrote:
> On 03/27/17 09:00, Ard Biesheuvel wrote:
>> On 27 March 2017 at 03:42, Gao, Liming wrote:
>>> Laszlo:
>>> On GUID header file, some header file are here, because they are added
>>> before BaseTools supports it. Now, we allow not to add header file if the
>>> header file only has GUID value definition.
>>
>> I have to agree with Laszlo here. The BaseTools support is incomplete,
>> since it does not add a #define for the GUID to AutoGen.h. This makes
>> it impossible to initialize static structures containing GUIDs, such
>> as templates containing vendor device paths.
>>
>> For instance, the following
>>
>> typedef struct {
>> EFI_GUID foo;
>> } TYPE;
>>
>> TYPE mFoo {
>> SOME_GUID
>> };
>>
>> is not possible without a Guid/xxx.h include file containing a #define
>> for SOME_GUID.
>
> I wonder if we can commit this series before end of April.
>
> Or is that too soon? End of May perhaps?
>
> The mind boggles.
>
Reviewed-by: Ard Biesheuvel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/27/17 09:00, Ard Biesheuvel wrote:
> On 27 March 2017 at 03:42, Gao, Liming wrote:
>> Laszlo:
>> On GUID header file, some header file are here, because they are added
>> before BaseTools supports it. Now, we allow not to add header file if the
>> header file only has GUID value definition.
>
> I have to agree with Laszlo here. The BaseTools support is incomplete,
> since it does not add a #define for the GUID to AutoGen.h. This makes
> it impossible to initialize static structures containing GUIDs, such
> as templates containing vendor device paths.
>
> For instance, the following
>
> typedef struct {
> EFI_GUID foo;
> } TYPE;
>
> TYPE mFoo {
> SOME_GUID
> };
>
> is not possible without a Guid/xxx.h include file containing a #define
> for SOME_GUID.
I wonder if we can commit this series before end of April.
Or is that too soon? End of May perhaps?
The mind boggles.
Laszlo
>
>> On GUID C MACRO, we suggest to use GuidCName in every C source code. So,
>> we don't suggest to add it. As you know, some existing header files have
>> GuidCName and GUID Macro. Now, we have no plan to clean up the existing one.
>> But, we expect new added file follows this rule.
>>
>> Thanks
>> Liming
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>>> Laszlo Ersek
>>> Sent: Saturday, March 25, 2017 1:09 AM
>>> To: Zeng, Star ; Ard Biesheuvel
>>> ; Kinney, Michael D
>>> ; af...@apple.com
>>> Cc: Tian, Feng ; Gao, Liming ;
>>> edk2-devel-01 ; Leif Lindholm
>>>
>>> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
>>> ACPI Protocol, and plug-in library
>>>
>>> On 03/24/17 04:44, Zeng, Star 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 = ----
>>>> 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 {
>>>>
>>>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>>>
>>>> PcdDepexGuid|gEdkiiPlatformHasAcpiGuid
>>>> }
>>>> MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
>>>>
>>>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>>>
>>>> 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.
>>>
>>> Sounds reasonable, I'll do this.
>>>
>>>> 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.
>>>
>>> I disagree with this. I mean, I *personally* wouldn&#
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 27 March 2017 at 03:42, Gao, Liming wrote:
> Laszlo:
> On GUID header file, some header file are here, because they are added
> before BaseTools supports it. Now, we allow not to add header file if the
> header file only has GUID value definition.
I have to agree with Laszlo here. The BaseTools support is incomplete,
since it does not add a #define for the GUID to AutoGen.h. This makes
it impossible to initialize static structures containing GUIDs, such
as templates containing vendor device paths.
For instance, the following
typedef struct {
EFI_GUID foo;
} TYPE;
TYPE mFoo {
SOME_GUID
};
is not possible without a Guid/xxx.h include file containing a #define
for SOME_GUID.
> On GUID C MACRO, we suggest to use GuidCName in every C source code. So, we
> don't suggest to add it. As you know, some existing header files have
> GuidCName and GUID Macro. Now, we have no plan to clean up the existing one.
> But, we expect new added file follows this rule.
>
> Thanks
> Liming
>>-Original Message-
>>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>>Laszlo Ersek
>>Sent: Saturday, March 25, 2017 1:09 AM
>>To: Zeng, Star ; Ard Biesheuvel
>>; Kinney, Michael D
>>; af...@apple.com
>>Cc: Tian, Feng ; Gao, Liming ;
>>edk2-devel-01 ; Leif Lindholm
>>
>>Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
>>ACPI Protocol, and plug-in library
>>
>>On 03/24/17 04:44, Zeng, Star 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 = ----
>>> 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 {
>>>
>>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>>
>>> PcdDepexGuid|gEdkiiPlatformHasAcpiGuid
>>> }
>>> MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
>>>
>>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>>
>>> 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.
>>
>>Sounds reasonable, I'll do this.
>>
>>> 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.
>>
>>I disagree with this. I mean, I *personally* wouldn't mind, but this
>>would be inconsistent with existing MdeModulePkg practice. For example,
>>see
>>
>>MdeModulePkg/Include/Guid/TtyTerm.h
>>
>>Similarly to the current case, that GUID ("gEfiTtyTermGuid") has no data
>>structures associated with it; the header file only has an extern
>>declaration, and -- importantly -- an array initializer macro called
>>EFI_TTY_TERM_GUID, which can be used in the following syntax:
>>
>>STATIC CONST EFI_GUID mThisIsMyGuid = EFI_TTY_TERM_GUID;
>>
>>So, I think that the GUID header file is required. I will add it in v
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
Laszlo:
On GUID header file, some header file are here, because they are added before
BaseTools supports it. Now, we allow not to add header file if the header file
only has GUID value definition.
On GUID C MACRO, we suggest to use GuidCName in every C source code. So, we
don't suggest to add it. As you know, some existing header files have
GuidCName and GUID Macro. Now, we have no plan to clean up the existing one.
But, we expect new added file follows this rule.
Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Saturday, March 25, 2017 1:09 AM
>To: Zeng, Star ; Ard Biesheuvel
>; Kinney, Michael D
>; af...@apple.com
>Cc: Tian, Feng ; Gao, Liming ;
>edk2-devel-01 ; Leif Lindholm
>
>Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
>ACPI Protocol, and plug-in library
>
>On 03/24/17 04:44, Zeng, Star 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 = ----
>> 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 {
>>
>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>
>> PcdDepexGuid|gEdkiiPlatformHasAcpiGuid
>> }
>> MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
>>
>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>
>> 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.
>
>Sounds reasonable, I'll do this.
>
>> 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.
>
>I disagree with this. I mean, I *personally* wouldn't mind, but this
>would be inconsistent with existing MdeModulePkg practice. For example,
>see
>
>MdeModulePkg/Include/Guid/TtyTerm.h
>
>Similarly to the current case, that GUID ("gEfiTtyTermGuid") has no data
>structures associated with it; the header file only has an extern
>declaration, and -- importantly -- an array initializer macro called
>EFI_TTY_TERM_GUID, which can be used in the following syntax:
>
>STATIC CONST EFI_GUID mThisIsMyGuid = EFI_TTY_TERM_GUID;
>
>So, I think that the GUID header file is required. I will add it in v3.
>
>Once we generalize this idea to the design that you laid out above, we
>can perhaps eliminate the header file. But, for now, I think we should
>remain consistent with other MdeModulePkg GUID definitions.
>
>> 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).
>
>I filed:
>
>https://bugzilla.tianocore.org/show_bug.cgi?id=443 -- for BaseTools,
>https://bugzilla.tianocore.org/show_bug.cgi?id=444 -- for the INF spec
>
>> Before that, how about implementing the
>> PlatformHasAcpiLib in none core package?
>
>Works for me; I'll split that part off and keep it under ArmPkg.
>
>Thanks
>Laszlo
>
>>
>>
>> Thanks,
&g
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 24 March 2017 at 17:10, Laszlo Ersek wrote:
> On 03/24/17 08:15, Ard Biesheuvel wrote:
>> On 24 March 2017 at 03:44, Zeng, Star 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 = ----
>>> 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 {
>>>
>>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>>
>>> PcdDepexGuid|gEdkiiPlatformHasAcpiGuid
>>> }
>>> MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
>>>
>>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>>
>>> 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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/24/17 08:15, Ard Biesheuvel wrote:
> On 24 March 2017 at 03:44, Zeng, Star 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 = ----
>> 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 {
>>
>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>
>> PcdDepexGuid|gEdkiiPlatformHasAcpiGuid
>> }
>> MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
>>
>> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>>
>> 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.
Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/24/17 04:44, Zeng, Star 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 = ----
> 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 {
>
> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>
> PcdDepexGuid|gEdkiiPlatformHasAcpiGuid
> }
> MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
>
> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>
> 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.
Sounds reasonable, I'll do this.
> 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.
I disagree with this. I mean, I *personally* wouldn't mind, but this
would be inconsistent with existing MdeModulePkg practice. For example, see
MdeModulePkg/Include/Guid/TtyTerm.h
Similarly to the current case, that GUID ("gEfiTtyTermGuid") has no data
structures associated with it; the header file only has an extern
declaration, and -- importantly -- an array initializer macro called
EFI_TTY_TERM_GUID, which can be used in the following syntax:
STATIC CONST EFI_GUID mThisIsMyGuid = EFI_TTY_TERM_GUID;
So, I think that the GUID header file is required. I will add it in v3.
Once we generalize this idea to the design that you laid out above, we
can perhaps eliminate the header file. But, for now, I think we should
remain consistent with other MdeModulePkg GUID definitions.
> 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).
I filed:
https://bugzilla.tianocore.org/show_bug.cgi?id=443 -- for BaseTools,
https://bugzilla.tianocore.org/show_bug.cgi?id=444 -- for the INF spec
> Before that, how about implementing the
> PlatformHasAcpiLib in none core package?
Works for me; I'll split that part off and keep it under ArmPkg.
Thanks
Laszlo
>
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Thursday, March 23, 2017 5:09 PM
> To: Zeng, Star ; Kinney, Michael D
> ; af...@apple.com
> Cc: Tian, Feng ; Laszlo Ersek ;
> edk2-devel-01 ; Leif Lindholm
>
> 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 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
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 24 March 2017 at 03:44, Zeng, Star 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 = ----
> 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 {
>
> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>
> PcdDepexGuid|gEdkiiPlatformHasAcpiGuid
> }
> MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
>
> NULL|XXXPkg/Library/DepexLib/DepexLib.inf
>
> 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.
Thanks,
Ard.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
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 = ----
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 {
NULL|XXXPkg/Library/DepexLib/DepexLib.inf
PcdDepexGuid|gEdkiiPlatformHasAcpiGuid
}
MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf {
NULL|XXXPkg/Library/DepexLib/DepexLib.inf
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-boun...@lists.01.org] On Behalf Of Ard
Biesheuvel
Sent: Thursday, March 23, 2017 5:09 PM
To: Zeng, Star ; Kinney, Michael D
; af...@apple.com
Cc: Tian, Feng ; Laszlo Ersek ;
edk2-devel-01 ; Leif Lindholm
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 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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
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 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
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
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.
Thanks,
Star
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Wednesday, March 22, 2017 11:02 PM
To: Leif Lindholm ; Zeng, Star ;
Tian, Feng
Cc: edk2-devel-01 ; Ard Biesheuvel
Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI
Protocol, and plug-in library
On 03/18/17 16:00, Leif Lindholm wrote:
> On Fri, Mar 17, 2017 at 09:47:24PM +0100, Laszlo Ersek wrote:
>> The presence of this protocol in the DXE protocol database implies
>> that the platform provides the operating system with an ACPI-based
>> hardware description. This is not necessarily mutually exclusive with
>> a Device Tree-based hardware description. A platform driver is
>> supposed to produce a single instance of the protocol (with NULL contents),
>> if appropriate.
>>
>> The decision to produce the protocol is platform specific; for
>> example, it could depend on an HII checkbox / underlying non-volatile UEFI
>> variable.
>>
>> The protocol is meant to be consumed by
>> "MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the
>> PlatformHasAcpiLib plug-in. By linking this library into AcpiTableDxe
>> via NULL resolution in the platform DSC, the platform makes
>> EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL
>> dependent on the above dynamic decision.
>>
>> In turn, other (platform and universal) DXE drivers that produce ACPI
>> tables will wait for EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL,
>> via DEPEX, protocol notify, or a simple gBS->LocateProtocol() in a
>> "late enough" callback (such as Ready To Boot).
>>
>> Because this protocol is not standard, it is prefixed with EDKII /
>> Edkii, as seen elsewhere in MdeModulePkg and SecurityPkg, for
>> example. (ARM / Arm doesn't look future-proof enough; future UEFI
>> platforms could face the same issue.) In addition, an effort is made
>> to avoid the phrase "AcpiPlatform", as that belongs to drivers /
>> libraries that produce platform specific ACPI content (as opposed to
>> deciding whether the entire firmware will have access to
>> EFI_ACPI_TABLE_PROTOCOL).
>
> I greatly approve, but since you've already done this generically - is
> there any good reason to keep this in ArmPkg?
> I am strongly looking to get rid of "things that happen to have been
> implemented for ARM" from there and pruning it down to contain only
> things that are architecturelly ARM-specific.
>
> MdeModulePkg?
It looks like Ard is okay with this series after all, and you (Leif) also seem
to tolerate it. The question is now if the MdeModulePkg owners are okay to take
this patch for MdeModulePkg.
Star, Feng, can I add the protocol definition and the library instance in this
patch to MdeModulePkg? (We've ruled out the dynamic PCD discussed previously
for AcpiTableDxe, because platforms would have even greater trouble setting
such a PCD than using this protocol.)
Can you guys please give a decisive answer in one or two days?
Thank you,
Laszlo
>
> Regards,
>
> Leif
>
>> Cc: Ard Biesheuvel
>> Cc: Leif Lindholm
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek
>> ---
>> ArmPkg/ArmPkg.dec| 4 ++
>> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf | 40
>>
>> ArmPkg/Include/Protocol/PlatformHasAcpi.h| 34
>> +
>> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c | 36
>> ++
>> 4 files changed, 114 insertions(+)
>>
>> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index
>> c4b4da2f95bb..0e49360a386a 100644
>> --- a/ArmPkg/ArmPkg.dec
>> +++ b/ArmPkg/ArmPkg.dec
>> @@ -52,6 +52,10 @@ [Ppis]
>>## Include/Ppi/ArmMpCoreInfo.h
>>gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d,
>> 0xab, 0x44, 0xe7, 0x54, 0xa8, 0xfc} }
>>
>> +[Protocols]
>> + ## Include/Protocol/PlatformHasAcpi.h
>> + gEdkiiPlatformHasAcpiProtocolGuid = { 0xf0966b41, 0xc23f, 0x41b9, {
>> 0x96, 0x04, 0x0f, 0xf7, 0xe1, 0x11, 0x96, 0x5a } }
>> +
>> [PcdsFeatureFlag.common]
>>
>> gArmTokenSpaceGuid.PcdCpuDxeProduceDebugSupport|FALSE|BOOLEAN|0x0
>> 001
>>
>> diff --git a/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
>> b/ArmPkg/Library/PlatformHasAcpiLib/Plat
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
Mike,
considering time zones and to expedite progress on this series, can you
please offer an opinion whether you would be okay with this one patch
(see full context below) if I posted it for MdeModulePkg, rather than
for ArmPkg?
MdeModulePkg was suggested by Leif.
Thanks!
Laszlo
On 03/17/17 21:47, Laszlo Ersek wrote:
> The presence of this protocol in the DXE protocol database implies that
> the platform provides the operating system with an ACPI-based hardware
> description. This is not necessarily mutually exclusive with a Device
> Tree-based hardware description. A platform driver is supposed to produce
> a single instance of the protocol (with NULL contents), if appropriate.
>
> The decision to produce the protocol is platform specific; for example, it
> could depend on an HII checkbox / underlying non-volatile UEFI variable.
>
> The protocol is meant to be consumed by
> "MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the
> PlatformHasAcpiLib plug-in. By linking this library into AcpiTableDxe via
> NULL resolution in the platform DSC, the platform makes
> EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL dependent
> on the above dynamic decision.
>
> In turn, other (platform and universal) DXE drivers that produce ACPI
> tables will wait for EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL, via
> DEPEX, protocol notify, or a simple gBS->LocateProtocol() in a "late
> enough" callback (such as Ready To Boot).
>
> Because this protocol is not standard, it is prefixed with EDKII / Edkii,
> as seen elsewhere in MdeModulePkg and SecurityPkg, for example. (ARM / Arm
> doesn't look future-proof enough; future UEFI platforms could face the
> same issue.) In addition, an effort is made to avoid the phrase
> "AcpiPlatform", as that belongs to drivers / libraries that produce
> platform specific ACPI content (as opposed to deciding whether the entire
> firmware will have access to EFI_ACPI_TABLE_PROTOCOL).
>
> Cc: Ard Biesheuvel
> Cc: Leif Lindholm
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek
> ---
> ArmPkg/ArmPkg.dec| 4 ++
> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf | 40
>
> ArmPkg/Include/Protocol/PlatformHasAcpi.h| 34
> +
> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c | 36
> ++
> 4 files changed, 114 insertions(+)
>
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index c4b4da2f95bb..0e49360a386a 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -52,6 +52,10 @@ [Ppis]
>## Include/Ppi/ArmMpCoreInfo.h
>gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab,
> 0x44, 0xe7, 0x54, 0xa8, 0xfc} }
>
> +[Protocols]
> + ## Include/Protocol/PlatformHasAcpi.h
> + gEdkiiPlatformHasAcpiProtocolGuid = { 0xf0966b41, 0xc23f, 0x41b9, {
> 0x96, 0x04, 0x0f, 0xf7, 0xe1, 0x11, 0x96, 0x5a } }
> +
> [PcdsFeatureFlag.common]
>gArmTokenSpaceGuid.PcdCpuDxeProduceDebugSupport|FALSE|BOOLEAN|0x0001
>
> diff --git a/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
> b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
> new file mode 100644
> index ..c83da4d8e98a
> --- /dev/null
> +++ b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
> @@ -0,0 +1,40 @@
> +## @file
> +# A hook-in library for MdeModulePkg/Universal/Acpi/AcpiTableDxe.
> +#
> +# Plugging this library instance into AcpiTableDxe makes
> +# EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL depend on
> the
> +# platform's dynamic decision whether to expose an ACPI-based hardware
> +# description to the operating system.
> +#
> +# Universal and platform specific DXE drivers that produce ACPI tables depend
> +# on EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL in turn.
> +#
> +# Copyright (C) 2017, Red Hat, Inc.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License which accompanies this
> +# distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> WITHOUT
> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +##
> +
> +[Defines]
> + INF_VERSION= 1.25
> + BASE_NAME = PlatformHasAcpiLib
> + FILE_GUID = 29beb028-0958-447b-be0a-12229235d77d
> + MODULE_TYPE= BASE
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = PlatformHasAcpiLib|DXE_DRIVER
> + CONSTRUCTOR= PlatformHasAcpiInitialize
> +
> +[Sources]
> + PlatformHasAcpiLib.c
> +
> +[Packages]
> + ArmPkg/ArmPkg.dec
> + MdePkg/MdePkg.dec
> +
> +[Depex]
> + gEdkiiPlatformHasAcpiProtocolGuid
> diff --git a/ArmPkg/Include/Protocol/Pl
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/18/17 16:00, Leif Lindholm wrote:
> On Fri, Mar 17, 2017 at 09:47:24PM +0100, Laszlo Ersek wrote:
>> The presence of this protocol in the DXE protocol database implies that
>> the platform provides the operating system with an ACPI-based hardware
>> description. This is not necessarily mutually exclusive with a Device
>> Tree-based hardware description. A platform driver is supposed to produce
>> a single instance of the protocol (with NULL contents), if appropriate.
>>
>> The decision to produce the protocol is platform specific; for example, it
>> could depend on an HII checkbox / underlying non-volatile UEFI variable.
>>
>> The protocol is meant to be consumed by
>> "MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the
>> PlatformHasAcpiLib plug-in. By linking this library into AcpiTableDxe via
>> NULL resolution in the platform DSC, the platform makes
>> EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL dependent
>> on the above dynamic decision.
>>
>> In turn, other (platform and universal) DXE drivers that produce ACPI
>> tables will wait for EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL, via
>> DEPEX, protocol notify, or a simple gBS->LocateProtocol() in a "late
>> enough" callback (such as Ready To Boot).
>>
>> Because this protocol is not standard, it is prefixed with EDKII / Edkii,
>> as seen elsewhere in MdeModulePkg and SecurityPkg, for example. (ARM / Arm
>> doesn't look future-proof enough; future UEFI platforms could face the
>> same issue.) In addition, an effort is made to avoid the phrase
>> "AcpiPlatform", as that belongs to drivers / libraries that produce
>> platform specific ACPI content (as opposed to deciding whether the entire
>> firmware will have access to EFI_ACPI_TABLE_PROTOCOL).
>
> I greatly approve, but since you've already done this generically - is
> there any good reason to keep this in ArmPkg?
> I am strongly looking to get rid of "things that happen to have been
> implemented for ARM" from there and pruning it down to contain only
> things that are architecturelly ARM-specific.
>
> MdeModulePkg?
It looks like Ard is okay with this series after all, and you (Leif)
also seem to tolerate it. The question is now if the MdeModulePkg owners
are okay to take this patch for MdeModulePkg.
Star, Feng, can I add the protocol definition and the library instance
in this patch to MdeModulePkg? (We've ruled out the dynamic PCD
discussed previously for AcpiTableDxe, because platforms would have even
greater trouble setting such a PCD than using this protocol.)
Can you guys please give a decisive answer in one or two days?
Thank you,
Laszlo
>
> Regards,
>
> Leif
>
>> Cc: Ard Biesheuvel
>> Cc: Leif Lindholm
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek
>> ---
>> ArmPkg/ArmPkg.dec| 4 ++
>> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf | 40
>>
>> ArmPkg/Include/Protocol/PlatformHasAcpi.h| 34
>> +
>> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c | 36
>> ++
>> 4 files changed, 114 insertions(+)
>>
>> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
>> index c4b4da2f95bb..0e49360a386a 100644
>> --- a/ArmPkg/ArmPkg.dec
>> +++ b/ArmPkg/ArmPkg.dec
>> @@ -52,6 +52,10 @@ [Ppis]
>>## Include/Ppi/ArmMpCoreInfo.h
>>gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab,
>> 0x44, 0xe7, 0x54, 0xa8, 0xfc} }
>>
>> +[Protocols]
>> + ## Include/Protocol/PlatformHasAcpi.h
>> + gEdkiiPlatformHasAcpiProtocolGuid = { 0xf0966b41, 0xc23f, 0x41b9, {
>> 0x96, 0x04, 0x0f, 0xf7, 0xe1, 0x11, 0x96, 0x5a } }
>> +
>> [PcdsFeatureFlag.common]
>>gArmTokenSpaceGuid.PcdCpuDxeProduceDebugSupport|FALSE|BOOLEAN|0x0001
>>
>> diff --git a/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
>> b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
>> new file mode 100644
>> index ..c83da4d8e98a
>> --- /dev/null
>> +++ b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
>> @@ -0,0 +1,40 @@
>> +## @file
>> +# A hook-in library for MdeModulePkg/Universal/Acpi/AcpiTableDxe.
>> +#
>> +# Plugging this library instance into AcpiTableDxe makes
>> +# EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL depend on
>> the
>> +# platform's dynamic decision whether to expose an ACPI-based hardware
>> +# description to the operating system.
>> +#
>> +# Universal and platform specific DXE drivers that produce ACPI tables
>> depend
>> +# on EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL in turn.
>> +#
>> +# Copyright (C) 2017, Red Hat, Inc.
>> +#
>> +# This program and the accompanying materials are licensed and made
>> available
>> +# under the terms and conditions of the BSD License which accompanies this
>> +# distribution. The full text of the license may be found at
>> +# http://opensource.org/licenses/bsd-license.php
>> +#
>> +# THE
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 21 March 2017 at 18:00, Laszlo Ersek wrote: > On 03/21/17 10:02, Laszlo Ersek wrote: >> On 03/21/17 09:43, Ard Biesheuvel wrote: >>> On 21 March 2017 at 07:10, Ard Biesheuvel wrote: On 21 March 2017 at 02:27, Zeng, Star wrote: > Just an idea. > I like it! > There is a way to do not introduce a new PCD, but reuse > PcdAcpiExposedTableVersions. > When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI > table should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the > driver entrypoint. > So could we then upgrade the definition so it is possible to use a dynamic PCD for this? >>> >>> Oh, and how is the ordering ensured in this case? We need to set the >>> dynamic PCD at runtime, but obviously before AcpiTableDxe gets a >>> chance to load. If the only way to achieve this is another NULL class >>> library, then this is not much of an improvement. >> >> I've been silently waiting for this to get noticed :) >> >> In the ArmVirtQemu case, the PCD value would be set from fw_cfg. But in >> ArmVirtQemu, fw_cfg is not available in PEI, only in DXE, so yes, a NULL >> class library would be needed. >> >> In physical firmware (also in the planned "showcase QEMU" case), the >> toggle would be exposed by another DXE driver (a platform driver) using >> an HII form. Generally such a driver translates an nvvar to a PcdSet in >> its entry point, plus registers HII stuff that allows the nvvar to be >> toggled interactively in the setup browser, for the next boot. Either >> way, the PcdSet would again only happen in DXE. >> >>> Or could we detect >>> the table-loader node from PEI as well? >> >> Not at the moment; fw_cfg is currently not available in PEI, in ArmVirtQemu. >> >> ArmVirtQemu's PEI phase executes in-place from pflash. That means no >> writeable global variables (unless you make a PEIM, or the PEI library >> instance, dependent on the "memory discovered" PPI). Even using just the >> MMIO (no DMA) interface to fw_cfg, you'd have to parse the DTB on every >> invocation. (No writeable global variables imply you can't pre-parse the >> stuff in the library constructor.) >> >> A similar example is >> "ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c", >> where each serial port write has to re-parse the base address from the DT. >> >> (OVMF is different because OVMF's PEI runs from memory.) >> >> So, whatever you saved on the PcdSet ordering in the DXE phase, you >> would triply lose in fw_cfg complexity in PEI. A new PEI library >> instance would be necessary for fw_cfg, which would either have to parse >> the DT on each invocation, or else depend on permanent PEI RAM >> availability (which would be the same (or worse) kind of ordering >> restriction that you're trying to avoid in DXE in the first place). > > So... does that make this patch set (relatively) less "unwieldy" to you? > I suppose, yes :-) I guess we will never have a perfect solution for this, and using library classes and protocol dependencies is much preferred over having to reason about when a dynamic PCD assumes its correct value. ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/21/17 10:02, Laszlo Ersek wrote: > On 03/21/17 09:43, Ard Biesheuvel wrote: >> On 21 March 2017 at 07:10, Ard Biesheuvel wrote: >>> On 21 March 2017 at 02:27, Zeng, Star wrote: Just an idea. >>> >>> I like it! >>> There is a way to do not introduce a new PCD, but reuse PcdAcpiExposedTableVersions. When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI table should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the driver entrypoint. >>> >>> So could we then upgrade the definition so it is possible to use a >>> dynamic PCD for this? >>> >> >> Oh, and how is the ordering ensured in this case? We need to set the >> dynamic PCD at runtime, but obviously before AcpiTableDxe gets a >> chance to load. If the only way to achieve this is another NULL class >> library, then this is not much of an improvement. > > I've been silently waiting for this to get noticed :) > > In the ArmVirtQemu case, the PCD value would be set from fw_cfg. But in > ArmVirtQemu, fw_cfg is not available in PEI, only in DXE, so yes, a NULL > class library would be needed. > > In physical firmware (also in the planned "showcase QEMU" case), the > toggle would be exposed by another DXE driver (a platform driver) using > an HII form. Generally such a driver translates an nvvar to a PcdSet in > its entry point, plus registers HII stuff that allows the nvvar to be > toggled interactively in the setup browser, for the next boot. Either > way, the PcdSet would again only happen in DXE. > >> Or could we detect >> the table-loader node from PEI as well? > > Not at the moment; fw_cfg is currently not available in PEI, in ArmVirtQemu. > > ArmVirtQemu's PEI phase executes in-place from pflash. That means no > writeable global variables (unless you make a PEIM, or the PEI library > instance, dependent on the "memory discovered" PPI). Even using just the > MMIO (no DMA) interface to fw_cfg, you'd have to parse the DTB on every > invocation. (No writeable global variables imply you can't pre-parse the > stuff in the library constructor.) > > A similar example is > "ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c", > where each serial port write has to re-parse the base address from the DT. > > (OVMF is different because OVMF's PEI runs from memory.) > > So, whatever you saved on the PcdSet ordering in the DXE phase, you > would triply lose in fw_cfg complexity in PEI. A new PEI library > instance would be necessary for fw_cfg, which would either have to parse > the DT on each invocation, or else depend on permanent PEI RAM > availability (which would be the same (or worse) kind of ordering > restriction that you're trying to avoid in DXE in the first place). So... does that make this patch set (relatively) less "unwieldy" to you? Thanks, Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/21/17 09:41, Zeng, Star wrote:
> It is not been implemented yet. Just for our discussion. I should use future
> tense in the sentence, sorry.
Ah, I see. Yeah, it makes sense.
(Although it still wouldn't allow us to avoid a NULL class library
plug-in for AcpiTableDxe, for setting the PCD just in time -- the
information source for the PCD is generally available to us only within
the DXE phase, not in PEI.)
Thanks
Laszlo
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Tuesday, March 21, 2017 4:38 PM
> To: Zeng, Star ; edk2-devel-01
> ; Kinney, Michael D ;
> Yao, Jiewen
> Cc: Leif Lindholm ; Ard Biesheuvel
>
> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
> ACPI Protocol, and plug-in library
>
> On 03/21/17 03:27, Zeng, Star wrote:
>> Just an idea.
>>
>> There is a way to do not introduce a new PCD, but reuse
>> PcdAcpiExposedTableVersions.
>> When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI
>> table should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the driver
>> entrypoint.
>
> Hm, I don't see that:
>
> $ git grep EFI_UNSUPPORTED -- MdeModulePkg/Universal/Acpi/AcpiTableDxe/
> [no matches]
>
> Thanks
> Laszlo
>
>>
>>
>> Thanks,
>> Star
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Zeng, Star
>> Sent: Monday, March 20, 2017 5:57 PM
>> To: Laszlo Ersek ; edk2-devel-01
>> ; Kinney, Michael D
>> ; Yao, Jiewen
>> Cc: Leif Lindholm ; Ard Biesheuvel
>>
>> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform
>> Has ACPI Protocol, and plug-in library
>>
>> Oh got it, returning unsuccessfully in constructor of library does not
>> work. :(
>>
>> For (1), I definitely have no any comments. :) For (2), I got no strong
>> reason to support or reject it. We'd better could get comments from Mike and
>> Jiewen.
>>
>> Thanks,
>> Star
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Laszlo Ersek
>> Sent: Monday, March 20, 2017 5:18 PM
>> To: Zeng, Star ; edk2-devel-01
>>
>> Cc: Leif Lindholm ; Ard Biesheuvel
>>
>> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform
>> Has ACPI Protocol, and plug-in library
>>
>> On 03/20/17 03:23, Zeng, Star wrote:
>>> Laszlo,
>>>
>>> Could the implementation have multiple instances of
>>> PlatformHasAcpiLib to return unsuccessfully in the constructor when
>>> EFI_ACPI_TABLE_PROTOCOL is not needed? And then eliminate
>>> PlatformHasAcpiDtDxe, gEdkiiPlatformHasAcpiProtocolGuid and
>>> gEdkiiPlatformHasDeviceTreeProtocolGuid?
>>
>> If a library constructor fails, then the client module does not see an
>> explicit error (or automatic unloading); instead, an assertion failure is
>> triggered in auto-generated code.
>>
>> For example, in the
>> "Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe/DEBUG/AutoGen.c"
>> file, with this series applied, we find:
>>
>>> VOID
>>> EFIAPI
>>> ProcessLibraryConstructorList (
>>> IN EFI_HANDLEImageHandle,
>>> IN EFI_SYSTEM_TABLE *SystemTable
>>> )
>>> {
>>> EFI_STATUS Status;
>>>
>>> Status = HobLibConstructor (ImageHandle, SystemTable);
>>> ASSERT_EFI_ERROR (Status);
>>>
>>> Status = FdtPL011SerialPortLibInitialize ();
>>> ASSERT_EFI_ERROR (Status);
>>>
>>> Status = BaseDebugLibSerialPortConstructor ();
>>> ASSERT_EFI_ERROR (Status);
>>>
>>> Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
>>> ASSERT_EFI_ERROR (Status);
>>>
>>> Status = UefiRuntimeServicesTableLibConstructor (ImageHandle,
>>> SystemTable);
>>> ASSERT_EFI_ERROR (Status);
>>>
>>> Status = UefiLibConstructor (ImageHandle, SystemTable);
>>> ASSERT_EFI_ERROR (Status);
>>>
>>> Status = PlatformHasAcpiInitialize (); <--- constructor of
>>> PlatformHasAcpiLib plug-in lib
>>> ASSERT_EFI_ERROR (Status);
>>>
>>> }
>>
>> What we could do instead would be a new library class, with an explicit
>> initialization function. However
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/21/17 09:43, Ard Biesheuvel wrote: > On 21 March 2017 at 07:10, Ard Biesheuvel wrote: >> On 21 March 2017 at 02:27, Zeng, Star wrote: >>> Just an idea. >>> >> >> I like it! >> >>> There is a way to do not introduce a new PCD, but reuse >>> PcdAcpiExposedTableVersions. >>> When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI >>> table should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the >>> driver entrypoint. >>> >> >> So could we then upgrade the definition so it is possible to use a >> dynamic PCD for this? >> > > Oh, and how is the ordering ensured in this case? We need to set the > dynamic PCD at runtime, but obviously before AcpiTableDxe gets a > chance to load. If the only way to achieve this is another NULL class > library, then this is not much of an improvement. I've been silently waiting for this to get noticed :) In the ArmVirtQemu case, the PCD value would be set from fw_cfg. But in ArmVirtQemu, fw_cfg is not available in PEI, only in DXE, so yes, a NULL class library would be needed. In physical firmware (also in the planned "showcase QEMU" case), the toggle would be exposed by another DXE driver (a platform driver) using an HII form. Generally such a driver translates an nvvar to a PcdSet in its entry point, plus registers HII stuff that allows the nvvar to be toggled interactively in the setup browser, for the next boot. Either way, the PcdSet would again only happen in DXE. > Or could we detect > the table-loader node from PEI as well? Not at the moment; fw_cfg is currently not available in PEI, in ArmVirtQemu. ArmVirtQemu's PEI phase executes in-place from pflash. That means no writeable global variables (unless you make a PEIM, or the PEI library instance, dependent on the "memory discovered" PPI). Even using just the MMIO (no DMA) interface to fw_cfg, you'd have to parse the DTB on every invocation. (No writeable global variables imply you can't pre-parse the stuff in the library constructor.) A similar example is "ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c", where each serial port write has to re-parse the base address from the DT. (OVMF is different because OVMF's PEI runs from memory.) So, whatever you saved on the PcdSet ordering in the DXE phase, you would triply lose in fw_cfg complexity in PEI. A new PEI library instance would be necessary for fw_cfg, which would either have to parse the DT on each invocation, or else depend on permanent PEI RAM availability (which would be the same (or worse) kind of ordering restriction that you're trying to avoid in DXE in the first place). Thanks Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 21 March 2017 at 07:10, Ard Biesheuvel wrote: > On 21 March 2017 at 02:27, Zeng, Star wrote: >> Just an idea. >> > > I like it! > >> There is a way to do not introduce a new PCD, but reuse >> PcdAcpiExposedTableVersions. >> When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI >> table should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the driver >> entrypoint. >> > > So could we then upgrade the definition so it is possible to use a > dynamic PCD for this? > Oh, and how is the ordering ensured in this case? We need to set the dynamic PCD at runtime, but obviously before AcpiTableDxe gets a chance to load. If the only way to achieve this is another NULL class library, then this is not much of an improvement. Or could we detect the table-loader node from PEI as well? ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
It is not been implemented yet. Just for our discussion. I should use future
tense in the sentence, sorry.
Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
Ersek
Sent: Tuesday, March 21, 2017 4:38 PM
To: Zeng, Star ; edk2-devel-01 ;
Kinney, Michael D ; Yao, Jiewen
Cc: Leif Lindholm ; Ard Biesheuvel
Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI
Protocol, and plug-in library
On 03/21/17 03:27, Zeng, Star wrote:
> Just an idea.
>
> There is a way to do not introduce a new PCD, but reuse
> PcdAcpiExposedTableVersions.
> When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI
> table should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the driver
> entrypoint.
Hm, I don't see that:
$ git grep EFI_UNSUPPORTED -- MdeModulePkg/Universal/Acpi/AcpiTableDxe/
[no matches]
Thanks
Laszlo
>
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Zeng, Star
> Sent: Monday, March 20, 2017 5:57 PM
> To: Laszlo Ersek ; edk2-devel-01
> ; Kinney, Michael D
> ; Yao, Jiewen
> Cc: Leif Lindholm ; Ard Biesheuvel
>
> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform
> Has ACPI Protocol, and plug-in library
>
> Oh got it, returning unsuccessfully in constructor of library does not
> work. :(
>
> For (1), I definitely have no any comments. :) For (2), I got no strong
> reason to support or reject it. We'd better could get comments from Mike and
> Jiewen.
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Monday, March 20, 2017 5:18 PM
> To: Zeng, Star ; edk2-devel-01
>
> Cc: Leif Lindholm ; Ard Biesheuvel
>
> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform
> Has ACPI Protocol, and plug-in library
>
> On 03/20/17 03:23, Zeng, Star wrote:
>> Laszlo,
>>
>> Could the implementation have multiple instances of
>> PlatformHasAcpiLib to return unsuccessfully in the constructor when
>> EFI_ACPI_TABLE_PROTOCOL is not needed? And then eliminate
>> PlatformHasAcpiDtDxe, gEdkiiPlatformHasAcpiProtocolGuid and
>> gEdkiiPlatformHasDeviceTreeProtocolGuid?
>
> If a library constructor fails, then the client module does not see an
> explicit error (or automatic unloading); instead, an assertion failure is
> triggered in auto-generated code.
>
> For example, in the
> "Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe/DEBUG/AutoGen.c"
> file, with this series applied, we find:
>
>> VOID
>> EFIAPI
>> ProcessLibraryConstructorList (
>> IN EFI_HANDLEImageHandle,
>> IN EFI_SYSTEM_TABLE *SystemTable
>> )
>> {
>> EFI_STATUS Status;
>>
>> Status = HobLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = FdtPL011SerialPortLibInitialize ();
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = BaseDebugLibSerialPortConstructor ();
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = UefiRuntimeServicesTableLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = UefiLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = PlatformHasAcpiInitialize (); <--- constructor of
>> PlatformHasAcpiLib plug-in lib
>> ASSERT_EFI_ERROR (Status);
>>
>> }
>
> What we could do instead would be a new library class, with an explicit
> initialization function. However, for that to work, AcpiTableDxe would have
> to be extended with a new library class dependency and a call to the init
> function. Additionally, out-of-tree platform DSC files would have to resolve
> this library class to the default instance.
>
> Introducing a boolean dynamic PCD for the same would be much simpler then;
> the DEC default would cover out-of-tree platform DSCs without any change in
> behavior.
>
> In summary, there are two alternatives in my opinion:
>
> (1) If we prefer not to modify MdeModulePkg: stick with this series. The only
> update I'd see justified would be to move
> gEdkiiPlatformHasDeviceTreeProtocolGuid to EmbeddedPkg as Leif requested.
>
> (2) If we are willing to modify MdeModulePkg for this problem: introduce a
> new dynamic PCD such as "
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/21/17 03:27, Zeng, Star wrote:
> Just an idea.
>
> There is a way to do not introduce a new PCD, but reuse
> PcdAcpiExposedTableVersions.
> When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI
> table should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the driver
> entrypoint.
Hm, I don't see that:
$ git grep EFI_UNSUPPORTED -- MdeModulePkg/Universal/Acpi/AcpiTableDxe/
[no matches]
Thanks
Laszlo
>
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zeng,
> Star
> Sent: Monday, March 20, 2017 5:57 PM
> To: Laszlo Ersek ; edk2-devel-01
> ; Kinney, Michael D ;
> Yao, Jiewen
> Cc: Leif Lindholm ; Ard Biesheuvel
>
> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
> ACPI Protocol, and plug-in library
>
> Oh got it, returning unsuccessfully in constructor of library does not work.
> :(
>
> For (1), I definitely have no any comments. :) For (2), I got no strong
> reason to support or reject it. We'd better could get comments from Mike and
> Jiewen.
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Monday, March 20, 2017 5:18 PM
> To: Zeng, Star ; edk2-devel-01
> Cc: Leif Lindholm ; Ard Biesheuvel
>
> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
> ACPI Protocol, and plug-in library
>
> On 03/20/17 03:23, Zeng, Star wrote:
>> Laszlo,
>>
>> Could the implementation have multiple instances of PlatformHasAcpiLib
>> to return unsuccessfully in the constructor when
>> EFI_ACPI_TABLE_PROTOCOL is not needed? And then eliminate
>> PlatformHasAcpiDtDxe, gEdkiiPlatformHasAcpiProtocolGuid and
>> gEdkiiPlatformHasDeviceTreeProtocolGuid?
>
> If a library constructor fails, then the client module does not see an
> explicit error (or automatic unloading); instead, an assertion failure is
> triggered in auto-generated code.
>
> For example, in the
> "Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe/DEBUG/AutoGen.c"
> file, with this series applied, we find:
>
>> VOID
>> EFIAPI
>> ProcessLibraryConstructorList (
>> IN EFI_HANDLEImageHandle,
>> IN EFI_SYSTEM_TABLE *SystemTable
>> )
>> {
>> EFI_STATUS Status;
>>
>> Status = HobLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = FdtPL011SerialPortLibInitialize ();
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = BaseDebugLibSerialPortConstructor ();
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = UefiRuntimeServicesTableLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = UefiLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = PlatformHasAcpiInitialize (); <--- constructor of
>> PlatformHasAcpiLib plug-in lib
>> ASSERT_EFI_ERROR (Status);
>>
>> }
>
> What we could do instead would be a new library class, with an explicit
> initialization function. However, for that to work, AcpiTableDxe would have
> to be extended with a new library class dependency and a call to the init
> function. Additionally, out-of-tree platform DSC files would have to resolve
> this library class to the default instance.
>
> Introducing a boolean dynamic PCD for the same would be much simpler then;
> the DEC default would cover out-of-tree platform DSCs without any change in
> behavior.
>
> In summary, there are two alternatives in my opinion:
>
> (1) If we prefer not to modify MdeModulePkg: stick with this series. The only
> update I'd see justified would be to move
> gEdkiiPlatformHasDeviceTreeProtocolGuid to EmbeddedPkg as Leif requested.
>
> (2) If we are willing to modify MdeModulePkg for this problem: introduce a
> new dynamic PCD such as "PcdAcpiTableEnable", and make AcpiTableDxe exit
> early if the PCD is FALSE.
>
> Thanks,
> Laszlo
>
>>
>> Thanks,
>> Star
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Laszlo Ersek
>> Sent: Saturday, March 18, 2017 4:47 AM
>> To: edk2-devel-01
>> Cc: Leif Lindholm ; Ard Biesheuvel
>>
>> Subject: [edk2] [PATCH v2 05/12] ArmPkg:
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 21 March 2017 at 02:27, Zeng, Star wrote:
> Just an idea.
>
I llike it!
> There is a way to do not introduce a new PCD, but reuse
> PcdAcpiExposedTableVersions.
> When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI
> table should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the driver
> entrypoint.
>
So could we then upgrade the definition so it is possible to use a
dynamic PCD for this?
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zeng,
> Star
> Sent: Monday, March 20, 2017 5:57 PM
> To: Laszlo Ersek ; edk2-devel-01
> ; Kinney, Michael D ;
> Yao, Jiewen
> Cc: Leif Lindholm ; Ard Biesheuvel
>
> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
> ACPI Protocol, and plug-in library
>
> Oh got it, returning unsuccessfully in constructor of library does not work.
> :(
>
> For (1), I definitely have no any comments. :) For (2), I got no strong
> reason to support or reject it. We'd better could get comments from Mike and
> Jiewen.
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Monday, March 20, 2017 5:18 PM
> To: Zeng, Star ; edk2-devel-01
> Cc: Leif Lindholm ; Ard Biesheuvel
>
> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
> ACPI Protocol, and plug-in library
>
> On 03/20/17 03:23, Zeng, Star wrote:
>> Laszlo,
>>
>> Could the implementation have multiple instances of PlatformHasAcpiLib
>> to return unsuccessfully in the constructor when
>> EFI_ACPI_TABLE_PROTOCOL is not needed? And then eliminate
>> PlatformHasAcpiDtDxe, gEdkiiPlatformHasAcpiProtocolGuid and
>> gEdkiiPlatformHasDeviceTreeProtocolGuid?
>
> If a library constructor fails, then the client module does not see an
> explicit error (or automatic unloading); instead, an assertion failure is
> triggered in auto-generated code.
>
> For example, in the
> "Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe/DEBUG/AutoGen.c"
> file, with this series applied, we find:
>
>> VOID
>> EFIAPI
>> ProcessLibraryConstructorList (
>> IN EFI_HANDLEImageHandle,
>> IN EFI_SYSTEM_TABLE *SystemTable
>> )
>> {
>> EFI_STATUS Status;
>>
>> Status = HobLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = FdtPL011SerialPortLibInitialize ();
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = BaseDebugLibSerialPortConstructor ();
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = UefiRuntimeServicesTableLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = UefiLibConstructor (ImageHandle, SystemTable);
>> ASSERT_EFI_ERROR (Status);
>>
>> Status = PlatformHasAcpiInitialize (); <--- constructor of
>> PlatformHasAcpiLib plug-in lib
>> ASSERT_EFI_ERROR (Status);
>>
>> }
>
> What we could do instead would be a new library class, with an explicit
> initialization function. However, for that to work, AcpiTableDxe would have
> to be extended with a new library class dependency and a call to the init
> function. Additionally, out-of-tree platform DSC files would have to resolve
> this library class to the default instance.
>
> Introducing a boolean dynamic PCD for the same would be much simpler then;
> the DEC default would cover out-of-tree platform DSCs without any change in
> behavior.
>
> In summary, there are two alternatives in my opinion:
>
> (1) If we prefer not to modify MdeModulePkg: stick with this series. The only
> update I'd see justified would be to move
> gEdkiiPlatformHasDeviceTreeProtocolGuid to EmbeddedPkg as Leif requested.
>
> (2) If we are willing to modify MdeModulePkg for this problem: introduce a
> new dynamic PCD such as "PcdAcpiTableEnable", and make AcpiTableDxe exit
> early if the PCD is FALSE.
>
> Thanks,
> Laszlo
>
>>
>> Thanks,
>> Star
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Laszlo Ersek
>> Sent: Saturday, March 18, 2017 4:47 AM
>> To: edk2-devel-01
>> Cc: Leif Lindholm ; Ard Biesheuvel
>>
>> Subject: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
>> ACPI Protocol, and plug-in library
&g
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
Just an idea.
There is a way to do not introduce a new PCD, but reuse
PcdAcpiExposedTableVersions.
When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI table
should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the driver
entrypoint.
Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zeng,
Star
Sent: Monday, March 20, 2017 5:57 PM
To: Laszlo Ersek ; edk2-devel-01 ;
Kinney, Michael D ; Yao, Jiewen
Cc: Leif Lindholm ; Ard Biesheuvel
Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI
Protocol, and plug-in library
Oh got it, returning unsuccessfully in constructor of library does not work. :(
For (1), I definitely have no any comments. :) For (2), I got no strong reason
to support or reject it. We'd better could get comments from Mike and Jiewen.
Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
Ersek
Sent: Monday, March 20, 2017 5:18 PM
To: Zeng, Star ; edk2-devel-01
Cc: Leif Lindholm ; Ard Biesheuvel
Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI
Protocol, and plug-in library
On 03/20/17 03:23, Zeng, Star wrote:
> Laszlo,
>
> Could the implementation have multiple instances of PlatformHasAcpiLib
> to return unsuccessfully in the constructor when
> EFI_ACPI_TABLE_PROTOCOL is not needed? And then eliminate
> PlatformHasAcpiDtDxe, gEdkiiPlatformHasAcpiProtocolGuid and
> gEdkiiPlatformHasDeviceTreeProtocolGuid?
If a library constructor fails, then the client module does not see an explicit
error (or automatic unloading); instead, an assertion failure is triggered in
auto-generated code.
For example, in the
"Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe/DEBUG/AutoGen.c"
file, with this series applied, we find:
> VOID
> EFIAPI
> ProcessLibraryConstructorList (
> IN EFI_HANDLEImageHandle,
> IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
> EFI_STATUS Status;
>
> Status = HobLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = FdtPL011SerialPortLibInitialize ();
> ASSERT_EFI_ERROR (Status);
>
> Status = BaseDebugLibSerialPortConstructor ();
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiRuntimeServicesTableLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = PlatformHasAcpiInitialize (); <--- constructor of
> PlatformHasAcpiLib plug-in lib
> ASSERT_EFI_ERROR (Status);
>
> }
What we could do instead would be a new library class, with an explicit
initialization function. However, for that to work, AcpiTableDxe would have to
be extended with a new library class dependency and a call to the init
function. Additionally, out-of-tree platform DSC files would have to resolve
this library class to the default instance.
Introducing a boolean dynamic PCD for the same would be much simpler then; the
DEC default would cover out-of-tree platform DSCs without any change in
behavior.
In summary, there are two alternatives in my opinion:
(1) If we prefer not to modify MdeModulePkg: stick with this series. The only
update I'd see justified would be to move
gEdkiiPlatformHasDeviceTreeProtocolGuid to EmbeddedPkg as Leif requested.
(2) If we are willing to modify MdeModulePkg for this problem: introduce a new
dynamic PCD such as "PcdAcpiTableEnable", and make AcpiTableDxe exit early if
the PCD is FALSE.
Thanks,
Laszlo
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Saturday, March 18, 2017 4:47 AM
> To: edk2-devel-01
> Cc: Leif Lindholm ; Ard Biesheuvel
>
> Subject: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
> ACPI Protocol, and plug-in library
>
> The presence of this protocol in the DXE protocol database implies that the
> platform provides the operating system with an ACPI-based hardware
> description. This is not necessarily mutually exclusive with a Device
> Tree-based hardware description. A platform driver is supposed to produce a
> single instance of the protocol (with NULL contents), if appropriate.
>
> The decision to produce the protocol is platform specific; for example, it
> could depend on an HII checkbox / underlying non-volatile UEFI variable.
>
> The protocol is meant to be consumed by
> "MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the PlatformH
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On Mon, Mar 20, 2017 at 10:01:09AM +0100, Laszlo Ersek wrote:
> >> Because this protocol is not standard, it is prefixed with EDKII / Edkii,
> >> as seen elsewhere in MdeModulePkg and SecurityPkg, for example. (ARM / Arm
> >> doesn't look future-proof enough; future UEFI platforms could face the
> >> same issue.) In addition, an effort is made to avoid the phrase
> >> "AcpiPlatform", as that belongs to drivers / libraries that produce
> >> platform specific ACPI content (as opposed to deciding whether the entire
> >> firmware will have access to EFI_ACPI_TABLE_PROTOCOL).
> >
> > I greatly approve, but since you've already done this generically - is
> > there any good reason to keep this in ArmPkg?
> > I am strongly looking to get rid of "things that happen to have been
> > implemented for ARM" from there and pruning it down to contain only
> > things that are architecturelly ARM-specific.
>
> This patch is not specific to ARM architecturally, but it is specific to
> the ARM ecosystem / community. I'm unaware of another platform (that
> would affect edk2 ATM anyway) where such a conflict in beliefs has not
> been sorted out for years.
Indeed. However, I have developed a mild allergy towards things that
are not fundamentally ARM-specific, but are treated as such.
It tends to introduce a mental barrier towards code unification and
sharing, since it makes it look like an architecture-specific component.
> The somewhat speculative generality in the naming (i.e., Edkii prefix
> rather than Arm) is there only because I understand that DT / libfdt is
> used on other platforms as well, and I expect once those see UEFI (and
> edk2) enablement & porting, the same DT vs. ACPI conflict in Linux space
> will extend to those platforms as well.
>
> IOW, at the moment the patch is specific to ARM, it is not random, but
> it could change in the future. And, I wouldn't like to force client
> modules to rename the GUID at that time.
Oh, I perfectly agree.
I'm just saying I don't think that a feature only being applicable to
ARM at the current moment should be a barrier for it being introduced
in Mde*Pkg.
> > MdeModulePkg?
>
> Not a bad idea, but the point of this approach was to avoid touching
> core modules. If we modify MdeModulePkg *logic* (as opposed to the
> trivial typo fix elsewhere in this series), then the simplest solution
> is to just add a dynamic PCD to MdeModulePkg.dec, which forces
> AcpiTableDxe to exit immediately with EFI_ABORTED or EFI_UNSUPPORTED.
That actually sounds more palatable to me than this set, which I still
prefer over Ard's solution (I'll expand on my reservations about that
one on the correct thread).
> As I mentioned earlier, PcdAcpiS3Enable had been introduced very
> similarly to this. It controls partial or full functionality of several
> DXE drivers:
>
> 11a291a4d279 MdeModulePkg: Introduce new PCD PcdAcpiS3Enable
> a1726e308903 OvmfPkg: Set PcdAcpiS3Enable according to
>QemuFwCfgS3Enabled()
> 125e09387641 MdeModulePkg S3SaveStateDxe: Consume PcdAcpiS3Enable to
>control the code
> e96708de8837 IntelFrameworkModulePkg AcpiS3SaveDxe: Consume
>PcdAcpiS3Enable to control the code
> d2d38610603f MdeModulePkg SmmS3SaveStateDxe: Consume PcdAcpiS3Enable
>to control the code
> 800c02fbe2da MdeModulePkg BootScriptExecutorDxe: Consume
>PcdAcpiS3Enable to control the code
> ca98f6038294 UefiCpuPkg/CpuS3DataDxe: Consume PcdAcpiS3Enable to
>control the code
> b10d5ddc0385 UefiCpuPkg/PiSmmCpuDxeSmm: Consume PcdAcpiS3Enable to
>control the code
>
> If the AcpiTableDxe maintainers aren't opposed to another dynamic PCD
> like PcdAcpiS3Enable -- but in this case for controlling AcpiTableDxe --
> then I'm fine with it too.
>
> ... I knew that new PCDs in MdeModulePkg needed strong justification, so
> I figured I'd try another route first.
Yes, I understand. But I think it is well motivated here, and the
solution that feels the most UEFI to me.
Regards,
Leif
> Thanks
> Laszlo
>
> >
> > Regards,
> >
> > Leif
> >
> >> Cc: Ard Biesheuvel
> >> Cc: Leif Lindholm
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Laszlo Ersek
> >> ---
> >> ArmPkg/ArmPkg.dec| 4 ++
> >> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf | 40
> >>
> >> ArmPkg/Include/Protocol/PlatformHasAcpi.h| 34
> >> +
> >> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c | 36
> >> ++
> >> 4 files changed, 114 insertions(+)
> >>
> >> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> >> index c4b4da2f95bb..0e49360a386a 100644
> >> --- a/ArmPkg/ArmPkg.dec
> >> +++ b/ArmPkg/ArmPkg.dec
> >> @@ -52,6 +52,10 @@ [Ppis]
> >>## Include/Ppi/ArmMpCoreInfo.h
> >>gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d,
> >> 0xab, 0x44
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
Oh got it, returning unsuccessfully in constructor of library does not work. :(
For (1), I definitely have no any comments. :)
For (2), I got no strong reason to support or reject it. We'd better could get
comments from Mike and Jiewen.
Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
Ersek
Sent: Monday, March 20, 2017 5:18 PM
To: Zeng, Star ; edk2-devel-01
Cc: Leif Lindholm ; Ard Biesheuvel
Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI
Protocol, and plug-in library
On 03/20/17 03:23, Zeng, Star wrote:
> Laszlo,
>
> Could the implementation have multiple instances of PlatformHasAcpiLib
> to return unsuccessfully in the constructor when
> EFI_ACPI_TABLE_PROTOCOL is not needed? And then eliminate
> PlatformHasAcpiDtDxe, gEdkiiPlatformHasAcpiProtocolGuid and
> gEdkiiPlatformHasDeviceTreeProtocolGuid?
If a library constructor fails, then the client module does not see an explicit
error (or automatic unloading); instead, an assertion failure is triggered in
auto-generated code.
For example, in the
"Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe/DEBUG/AutoGen.c"
file, with this series applied, we find:
> VOID
> EFIAPI
> ProcessLibraryConstructorList (
> IN EFI_HANDLEImageHandle,
> IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
> EFI_STATUS Status;
>
> Status = HobLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = FdtPL011SerialPortLibInitialize ();
> ASSERT_EFI_ERROR (Status);
>
> Status = BaseDebugLibSerialPortConstructor ();
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiRuntimeServicesTableLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = PlatformHasAcpiInitialize (); <--- constructor of
> PlatformHasAcpiLib plug-in lib
> ASSERT_EFI_ERROR (Status);
>
> }
What we could do instead would be a new library class, with an explicit
initialization function. However, for that to work, AcpiTableDxe would have to
be extended with a new library class dependency and a call to the init
function. Additionally, out-of-tree platform DSC files would have to resolve
this library class to the default instance.
Introducing a boolean dynamic PCD for the same would be much simpler then; the
DEC default would cover out-of-tree platform DSCs without any change in
behavior.
In summary, there are two alternatives in my opinion:
(1) If we prefer not to modify MdeModulePkg: stick with this series. The only
update I'd see justified would be to move
gEdkiiPlatformHasDeviceTreeProtocolGuid to EmbeddedPkg as Leif requested.
(2) If we are willing to modify MdeModulePkg for this problem: introduce a new
dynamic PCD such as "PcdAcpiTableEnable", and make AcpiTableDxe exit early if
the PCD is FALSE.
Thanks,
Laszlo
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Saturday, March 18, 2017 4:47 AM
> To: edk2-devel-01
> Cc: Leif Lindholm ; Ard Biesheuvel
>
> Subject: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
> ACPI Protocol, and plug-in library
>
> The presence of this protocol in the DXE protocol database implies that the
> platform provides the operating system with an ACPI-based hardware
> description. This is not necessarily mutually exclusive with a Device
> Tree-based hardware description. A platform driver is supposed to produce a
> single instance of the protocol (with NULL contents), if appropriate.
>
> The decision to produce the protocol is platform specific; for example, it
> could depend on an HII checkbox / underlying non-volatile UEFI variable.
>
> The protocol is meant to be consumed by
> "MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the PlatformHasAcpiLib
> plug-in. By linking this library into AcpiTableDxe via NULL resolution in the
> platform DSC, the platform makes EFI_ACPI_TABLE_PROTOCOL and (if enabled)
> EFI_ACPI_SDT_PROTOCOL dependent on the above dynamic decision.
>
> In turn, other (platform and universal) DXE drivers that produce ACPI tables
> will wait for EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL, via DEPEX,
> protocol notify, or a simple gBS->LocateProtocol() in a "late enough"
> callback (such as Ready To Boot).
>
> Because this protocol is not standard, it is prefixed with EDKII / Edkii, as
> seen elsewhere
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/20/17 03:23, Zeng, Star wrote:
> Laszlo,
>
> Could the implementation have multiple instances of
> PlatformHasAcpiLib to return unsuccessfully in the constructor when
> EFI_ACPI_TABLE_PROTOCOL is not needed? And then eliminate
> PlatformHasAcpiDtDxe, gEdkiiPlatformHasAcpiProtocolGuid and
> gEdkiiPlatformHasDeviceTreeProtocolGuid?
If a library constructor fails, then the client module does not see an
explicit error (or automatic unloading); instead, an assertion failure
is triggered in auto-generated code.
For example, in the
"Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/AARCH64/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe/DEBUG/AutoGen.c"
file, with this series applied, we find:
> VOID
> EFIAPI
> ProcessLibraryConstructorList (
> IN EFI_HANDLEImageHandle,
> IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
> EFI_STATUS Status;
>
> Status = HobLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = FdtPL011SerialPortLibInitialize ();
> ASSERT_EFI_ERROR (Status);
>
> Status = BaseDebugLibSerialPortConstructor ();
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiRuntimeServicesTableLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = UefiLibConstructor (ImageHandle, SystemTable);
> ASSERT_EFI_ERROR (Status);
>
> Status = PlatformHasAcpiInitialize (); <--- constructor of
> PlatformHasAcpiLib plug-in lib
> ASSERT_EFI_ERROR (Status);
>
> }
What we could do instead would be a new library class, with an explicit
initialization function. However, for that to work, AcpiTableDxe would
have to be extended with a new library class dependency and a call to
the init function. Additionally, out-of-tree platform DSC files would
have to resolve this library class to the default instance.
Introducing a boolean dynamic PCD for the same would be much simpler
then; the DEC default would cover out-of-tree platform DSCs without any
change in behavior.
In summary, there are two alternatives in my opinion:
(1) If we prefer not to modify MdeModulePkg: stick with this series. The
only update I'd see justified would be to move
gEdkiiPlatformHasDeviceTreeProtocolGuid to EmbeddedPkg as Leif
requested.
(2) If we are willing to modify MdeModulePkg for this problem: introduce
a new dynamic PCD such as "PcdAcpiTableEnable", and make AcpiTableDxe
exit early if the PCD is FALSE.
Thanks,
Laszlo
>
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Saturday, March 18, 2017 4:47 AM
> To: edk2-devel-01
> Cc: Leif Lindholm ; Ard Biesheuvel
>
> Subject: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI
> Protocol, and plug-in library
>
> The presence of this protocol in the DXE protocol database implies that the
> platform provides the operating system with an ACPI-based hardware
> description. This is not necessarily mutually exclusive with a Device
> Tree-based hardware description. A platform driver is supposed to produce a
> single instance of the protocol (with NULL contents), if appropriate.
>
> The decision to produce the protocol is platform specific; for example, it
> could depend on an HII checkbox / underlying non-volatile UEFI variable.
>
> The protocol is meant to be consumed by
> "MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the PlatformHasAcpiLib
> plug-in. By linking this library into AcpiTableDxe via NULL resolution in the
> platform DSC, the platform makes EFI_ACPI_TABLE_PROTOCOL and (if enabled)
> EFI_ACPI_SDT_PROTOCOL dependent on the above dynamic decision.
>
> In turn, other (platform and universal) DXE drivers that produce ACPI tables
> will wait for EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL, via DEPEX,
> protocol notify, or a simple gBS->LocateProtocol() in a "late enough"
> callback (such as Ready To Boot).
>
> Because this protocol is not standard, it is prefixed with EDKII / Edkii, as
> seen elsewhere in MdeModulePkg and SecurityPkg, for example. (ARM / Arm
> doesn't look future-proof enough; future UEFI platforms could face the same
> issue.) In addition, an effort is made to avoid the phrase "AcpiPlatform", as
> that belongs to drivers / libraries that produce platform specific ACPI
> content (as opposed to deciding whether the entire firmware will have access
> to EFI_ACPI_TABLE_PROTOCOL).
>
> Cc: Ard Biesheuvel
> Cc: Leif Lindholm
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Er
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On 03/18/17 16:00, Leif Lindholm wrote: > On Fri, Mar 17, 2017 at 09:47:24PM +0100, Laszlo Ersek wrote: >> The presence of this protocol in the DXE protocol database implies that >> the platform provides the operating system with an ACPI-based hardware >> description. This is not necessarily mutually exclusive with a Device >> Tree-based hardware description. A platform driver is supposed to produce >> a single instance of the protocol (with NULL contents), if appropriate. >> >> The decision to produce the protocol is platform specific; for example, it >> could depend on an HII checkbox / underlying non-volatile UEFI variable. >> >> The protocol is meant to be consumed by >> "MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the >> PlatformHasAcpiLib plug-in. By linking this library into AcpiTableDxe via >> NULL resolution in the platform DSC, the platform makes >> EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL dependent >> on the above dynamic decision. >> >> In turn, other (platform and universal) DXE drivers that produce ACPI >> tables will wait for EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL, via >> DEPEX, protocol notify, or a simple gBS->LocateProtocol() in a "late >> enough" callback (such as Ready To Boot). >> >> Because this protocol is not standard, it is prefixed with EDKII / Edkii, >> as seen elsewhere in MdeModulePkg and SecurityPkg, for example. (ARM / Arm >> doesn't look future-proof enough; future UEFI platforms could face the >> same issue.) In addition, an effort is made to avoid the phrase >> "AcpiPlatform", as that belongs to drivers / libraries that produce >> platform specific ACPI content (as opposed to deciding whether the entire >> firmware will have access to EFI_ACPI_TABLE_PROTOCOL). > > I greatly approve, but since you've already done this generically - is > there any good reason to keep this in ArmPkg? > I am strongly looking to get rid of "things that happen to have been > implemented for ARM" from there and pruning it down to contain only > things that are architecturelly ARM-specific. This patch is not specific to ARM architecturally, but it is specific to the ARM ecosystem / community. I'm unaware of another platform (that would affect edk2 ATM anyway) where such a conflict in beliefs has not been sorted out for years. The somewhat speculative generality in the naming (i.e., Edkii prefix rather than Arm) is there only because I understand that DT / libfdt is used on other platforms as well, and I expect once those see UEFI (and edk2) enablement & porting, the same DT vs. ACPI conflict in Linux space will extend to those platforms as well. IOW, at the moment the patch is specific to ARM, it is not random, but it could change in the future. And, I wouldn't like to force client modules to rename the GUID at that time. > MdeModulePkg? Not a bad idea, but the point of this approach was to avoid touching core modules. If we modify MdeModulePkg *logic* (as opposed to the trivial typo fix elsewhere in this series), then the simplest solution is to just add a dynamic PCD to MdeModulePkg.dec, which forces AcpiTableDxe to exit immediately with EFI_ABORTED or EFI_UNSUPPORTED. As I mentioned earlier, PcdAcpiS3Enable had been introduced very similarly to this. It controls partial or full functionality of several DXE drivers: 11a291a4d279 MdeModulePkg: Introduce new PCD PcdAcpiS3Enable a1726e308903 OvmfPkg: Set PcdAcpiS3Enable according to QemuFwCfgS3Enabled() 125e09387641 MdeModulePkg S3SaveStateDxe: Consume PcdAcpiS3Enable to control the code e96708de8837 IntelFrameworkModulePkg AcpiS3SaveDxe: Consume PcdAcpiS3Enable to control the code d2d38610603f MdeModulePkg SmmS3SaveStateDxe: Consume PcdAcpiS3Enable to control the code 800c02fbe2da MdeModulePkg BootScriptExecutorDxe: Consume PcdAcpiS3Enable to control the code ca98f6038294 UefiCpuPkg/CpuS3DataDxe: Consume PcdAcpiS3Enable to control the code b10d5ddc0385 UefiCpuPkg/PiSmmCpuDxeSmm: Consume PcdAcpiS3Enable to control the code If the AcpiTableDxe maintainers aren't opposed to another dynamic PCD like PcdAcpiS3Enable -- but in this case for controlling AcpiTableDxe -- then I'm fine with it too. ... I knew that new PCDs in MdeModulePkg needed strong justification, so I figured I'd try another route first. Thanks Laszlo > > Regards, > > Leif > >> Cc: Ard Biesheuvel >> Cc: Leif Lindholm >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek >> --- >> ArmPkg/ArmPkg.dec| 4 ++ >> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf | 40 >> >> ArmPkg/Include/Protocol/PlatformHasAcpi.h| 34 >> + >> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c | 36 >> ++ >> 4 files changed, 114 insertions(+) >> >> diff --git a/ArmPkg/ArmPkg
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
Laszlo,
Could the implementation have multiple instances of PlatformHasAcpiLib to
return unsuccessfully in the constructor when EFI_ACPI_TABLE_PROTOCOL is not
needed? And then eliminate PlatformHasAcpiDtDxe,
gEdkiiPlatformHasAcpiProtocolGuid and gEdkiiPlatformHasDeviceTreeProtocolGuid?
Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
Ersek
Sent: Saturday, March 18, 2017 4:47 AM
To: edk2-devel-01
Cc: Leif Lindholm ; Ard Biesheuvel
Subject: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI
Protocol, and plug-in library
The presence of this protocol in the DXE protocol database implies that the
platform provides the operating system with an ACPI-based hardware description.
This is not necessarily mutually exclusive with a Device Tree-based hardware
description. A platform driver is supposed to produce a single instance of the
protocol (with NULL contents), if appropriate.
The decision to produce the protocol is platform specific; for example, it
could depend on an HII checkbox / underlying non-volatile UEFI variable.
The protocol is meant to be consumed by
"MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the PlatformHasAcpiLib
plug-in. By linking this library into AcpiTableDxe via NULL resolution in the
platform DSC, the platform makes EFI_ACPI_TABLE_PROTOCOL and (if enabled)
EFI_ACPI_SDT_PROTOCOL dependent on the above dynamic decision.
In turn, other (platform and universal) DXE drivers that produce ACPI tables
will wait for EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL, via DEPEX,
protocol notify, or a simple gBS->LocateProtocol() in a "late enough" callback
(such as Ready To Boot).
Because this protocol is not standard, it is prefixed with EDKII / Edkii, as
seen elsewhere in MdeModulePkg and SecurityPkg, for example. (ARM / Arm doesn't
look future-proof enough; future UEFI platforms could face the same issue.) In
addition, an effort is made to avoid the phrase "AcpiPlatform", as that belongs
to drivers / libraries that produce platform specific ACPI content (as opposed
to deciding whether the entire firmware will have access to
EFI_ACPI_TABLE_PROTOCOL).
Cc: Ard Biesheuvel
Cc: Leif Lindholm
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek
---
ArmPkg/ArmPkg.dec| 4 ++
ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf | 40
ArmPkg/Include/Protocol/PlatformHasAcpi.h| 34 +
ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c | 36
++
4 files changed, 114 insertions(+)
diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index
c4b4da2f95bb..0e49360a386a 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -52,6 +52,10 @@ [Ppis]
## Include/Ppi/ArmMpCoreInfo.h
gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab,
0x44, 0xe7, 0x54, 0xa8, 0xfc} }
+[Protocols]
+ ## Include/Protocol/PlatformHasAcpi.h
+ gEdkiiPlatformHasAcpiProtocolGuid = { 0xf0966b41, 0xc23f, 0x41b9, {
0x96, 0x04, 0x0f, 0xf7, 0xe1, 0x11, 0x96, 0x5a } }
+
[PcdsFeatureFlag.common]
gArmTokenSpaceGuid.PcdCpuDxeProduceDebugSupport|FALSE|BOOLEAN|0x0001
diff --git a/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
new file mode 100644
index ..c83da4d8e98a
--- /dev/null
+++ b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
@@ -0,0 +1,40 @@
+## @file
+# A hook-in library for MdeModulePkg/Universal/Acpi/AcpiTableDxe.
+#
+# Plugging this library instance into AcpiTableDxe makes #
+EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL depend
+on the # platform's dynamic decision whether to expose an ACPI-based
+hardware # description to the operating system.
+#
+# Universal and platform specific DXE drivers that produce ACPI tables
+depend # on EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL in turn.
+#
+# Copyright (C) 2017, Red Hat, Inc.
+#
+# This program and the accompanying materials are licensed and made
+available # under the terms and conditions of the BSD License which
+accompanies this # distribution. The full text of the license may be
+found at # http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT # WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+##
+
+[Defines]
+ INF_VERSION= 1.25
+ BASE_NAME = PlatformHasAcpiLib
+ FILE_GUID = 29beb028-0958-447b-be0a-12229235d77d
+ MODULE_TYPE= BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = PlatformHasAcpiLib|DXE_DRIVER
+ CONSTRUCTOR= PlatformHasAcpiInitialize
+
+[Sources]
+ PlatformHasA
Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
On Fri, Mar 17, 2017 at 09:47:24PM +0100, Laszlo Ersek wrote:
> The presence of this protocol in the DXE protocol database implies that
> the platform provides the operating system with an ACPI-based hardware
> description. This is not necessarily mutually exclusive with a Device
> Tree-based hardware description. A platform driver is supposed to produce
> a single instance of the protocol (with NULL contents), if appropriate.
>
> The decision to produce the protocol is platform specific; for example, it
> could depend on an HII checkbox / underlying non-volatile UEFI variable.
>
> The protocol is meant to be consumed by
> "MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the
> PlatformHasAcpiLib plug-in. By linking this library into AcpiTableDxe via
> NULL resolution in the platform DSC, the platform makes
> EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL dependent
> on the above dynamic decision.
>
> In turn, other (platform and universal) DXE drivers that produce ACPI
> tables will wait for EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL, via
> DEPEX, protocol notify, or a simple gBS->LocateProtocol() in a "late
> enough" callback (such as Ready To Boot).
>
> Because this protocol is not standard, it is prefixed with EDKII / Edkii,
> as seen elsewhere in MdeModulePkg and SecurityPkg, for example. (ARM / Arm
> doesn't look future-proof enough; future UEFI platforms could face the
> same issue.) In addition, an effort is made to avoid the phrase
> "AcpiPlatform", as that belongs to drivers / libraries that produce
> platform specific ACPI content (as opposed to deciding whether the entire
> firmware will have access to EFI_ACPI_TABLE_PROTOCOL).
I greatly approve, but since you've already done this generically - is
there any good reason to keep this in ArmPkg?
I am strongly looking to get rid of "things that happen to have been
implemented for ARM" from there and pruning it down to contain only
things that are architecturelly ARM-specific.
MdeModulePkg?
Regards,
Leif
> Cc: Ard Biesheuvel
> Cc: Leif Lindholm
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek
> ---
> ArmPkg/ArmPkg.dec| 4 ++
> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf | 40
>
> ArmPkg/Include/Protocol/PlatformHasAcpi.h| 34
> +
> ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c | 36
> ++
> 4 files changed, 114 insertions(+)
>
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index c4b4da2f95bb..0e49360a386a 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -52,6 +52,10 @@ [Ppis]
>## Include/Ppi/ArmMpCoreInfo.h
>gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab,
> 0x44, 0xe7, 0x54, 0xa8, 0xfc} }
>
> +[Protocols]
> + ## Include/Protocol/PlatformHasAcpi.h
> + gEdkiiPlatformHasAcpiProtocolGuid = { 0xf0966b41, 0xc23f, 0x41b9, {
> 0x96, 0x04, 0x0f, 0xf7, 0xe1, 0x11, 0x96, 0x5a } }
> +
> [PcdsFeatureFlag.common]
>gArmTokenSpaceGuid.PcdCpuDxeProduceDebugSupport|FALSE|BOOLEAN|0x0001
>
> diff --git a/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
> b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
> new file mode 100644
> index ..c83da4d8e98a
> --- /dev/null
> +++ b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
> @@ -0,0 +1,40 @@
> +## @file
> +# A hook-in library for MdeModulePkg/Universal/Acpi/AcpiTableDxe.
> +#
> +# Plugging this library instance into AcpiTableDxe makes
> +# EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL depend on
> the
> +# platform's dynamic decision whether to expose an ACPI-based hardware
> +# description to the operating system.
> +#
> +# Universal and platform specific DXE drivers that produce ACPI tables depend
> +# on EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL in turn.
> +#
> +# Copyright (C) 2017, Red Hat, Inc.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License which accompanies this
> +# distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> WITHOUT
> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +##
> +
> +[Defines]
> + INF_VERSION= 1.25
> + BASE_NAME = PlatformHasAcpiLib
> + FILE_GUID = 29beb028-0958-447b-be0a-12229235d77d
> + MODULE_TYPE= BASE
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = PlatformHasAcpiLib|DXE_DRIVER
> + CONSTRUCTOR= PlatformHasAcpiInitialize
> +
> +[Sources]
> + PlatformHasAcpiLib.c
> +
> +[Packages]
> + ArmPkg/ArmPkg.dec
> + MdePkg/MdePkg.dec
> +
> +[Depex]
> + gEdkiiP
[edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library
The presence of this protocol in the DXE protocol database implies that
the platform provides the operating system with an ACPI-based hardware
description. This is not necessarily mutually exclusive with a Device
Tree-based hardware description. A platform driver is supposed to produce
a single instance of the protocol (with NULL contents), if appropriate.
The decision to produce the protocol is platform specific; for example, it
could depend on an HII checkbox / underlying non-volatile UEFI variable.
The protocol is meant to be consumed by
"MdeModulePkg/Universal/Acpi/AcpiTableDxe", through the
PlatformHasAcpiLib plug-in. By linking this library into AcpiTableDxe via
NULL resolution in the platform DSC, the platform makes
EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL dependent
on the above dynamic decision.
In turn, other (platform and universal) DXE drivers that produce ACPI
tables will wait for EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL, via
DEPEX, protocol notify, or a simple gBS->LocateProtocol() in a "late
enough" callback (such as Ready To Boot).
Because this protocol is not standard, it is prefixed with EDKII / Edkii,
as seen elsewhere in MdeModulePkg and SecurityPkg, for example. (ARM / Arm
doesn't look future-proof enough; future UEFI platforms could face the
same issue.) In addition, an effort is made to avoid the phrase
"AcpiPlatform", as that belongs to drivers / libraries that produce
platform specific ACPI content (as opposed to deciding whether the entire
firmware will have access to EFI_ACPI_TABLE_PROTOCOL).
Cc: Ard Biesheuvel
Cc: Leif Lindholm
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek
---
ArmPkg/ArmPkg.dec| 4 ++
ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf | 40
ArmPkg/Include/Protocol/PlatformHasAcpi.h| 34 +
ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.c | 36
++
4 files changed, 114 insertions(+)
diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index c4b4da2f95bb..0e49360a386a 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -52,6 +52,10 @@ [Ppis]
## Include/Ppi/ArmMpCoreInfo.h
gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab,
0x44, 0xe7, 0x54, 0xa8, 0xfc} }
+[Protocols]
+ ## Include/Protocol/PlatformHasAcpi.h
+ gEdkiiPlatformHasAcpiProtocolGuid = { 0xf0966b41, 0xc23f, 0x41b9, {
0x96, 0x04, 0x0f, 0xf7, 0xe1, 0x11, 0x96, 0x5a } }
+
[PcdsFeatureFlag.common]
gArmTokenSpaceGuid.PcdCpuDxeProduceDebugSupport|FALSE|BOOLEAN|0x0001
diff --git a/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
new file mode 100644
index ..c83da4d8e98a
--- /dev/null
+++ b/ArmPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf
@@ -0,0 +1,40 @@
+## @file
+# A hook-in library for MdeModulePkg/Universal/Acpi/AcpiTableDxe.
+#
+# Plugging this library instance into AcpiTableDxe makes
+# EFI_ACPI_TABLE_PROTOCOL and (if enabled) EFI_ACPI_SDT_PROTOCOL depend on the
+# platform's dynamic decision whether to expose an ACPI-based hardware
+# description to the operating system.
+#
+# Universal and platform specific DXE drivers that produce ACPI tables depend
+# on EFI_ACPI_TABLE_PROTOCOL / EFI_ACPI_SDT_PROTOCOL in turn.
+#
+# Copyright (C) 2017, Red Hat, Inc.
+#
+# This program and the accompanying materials are licensed and made available
+# under the terms and conditions of the BSD License which accompanies this
+# distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+##
+
+[Defines]
+ INF_VERSION= 1.25
+ BASE_NAME = PlatformHasAcpiLib
+ FILE_GUID = 29beb028-0958-447b-be0a-12229235d77d
+ MODULE_TYPE= BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = PlatformHasAcpiLib|DXE_DRIVER
+ CONSTRUCTOR= PlatformHasAcpiInitialize
+
+[Sources]
+ PlatformHasAcpiLib.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ MdePkg/MdePkg.dec
+
+[Depex]
+ gEdkiiPlatformHasAcpiProtocolGuid
diff --git a/ArmPkg/Include/Protocol/PlatformHasAcpi.h
b/ArmPkg/Include/Protocol/PlatformHasAcpi.h
new file mode 100644
index ..3cd0cfe4515d
--- /dev/null
+++ b/ArmPkg/Include/Protocol/PlatformHasAcpi.h
@@ -0,0 +1,34 @@
+/** @file
+ EDKII Platform Has ACPI Protocol
+
+ The presence of this protocol in the DXE protocol database implies that the
+ platform provides the operating system with an ACPI-based hardware
+ description. Note that this is not necessarily mutually exclusive with a
+ Device Tree-based hardware description. A platform driver is supposed to
+
