Turns out all .vfr files in the tree interacting with DynamicPcds
manually copy the same set of EFI_VARIABLE_* definitions, since the rest
of UefiMultiPhase.h is incompatible with VfrCompile.
Split these out into a separate header file UefiMultiPhaseDefinitions.h
in order to make it possible to include just that portion into .vfr
files. Then include that from UefiMultiPhase.h.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
MdePkg/Include/Uefi/UefiMultiPhase.h | 23 +-----------
MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h | 39 ++++++++++++++++++++
2 files changed, 41 insertions(+), 21 deletions(-)
diff --git a/MdePkg/Include/Uefi/UefiMultiPhase.h b/MdePkg/Include/Uefi/UefiMultiPhase.h
index 0dcbb1b9ee..b360c9513b 100644
--- a/MdePkg/Include/Uefi/UefiMultiPhase.h
+++ b/MdePkg/Include/Uefi/UefiMultiPhase.h
@@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#ifndef __UEFI_MULTIPHASE_H__
#define __UEFI_MULTIPHASE_H__
+#include "UefiMultiPhaseDefinitions.h"
+
#include <Guid/WinCertificate.h>
///
/// Enumeration of memory types introduced in UEFI.
@@ -156,27 +158,6 @@ typedef struct {
} EFI_TABLE_HEADER;
///
-/// Attributes of variable.
-///
-#define EFI_VARIABLE_NON_VOLATILE 0x00000001
-#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002
-#define EFI_VARIABLE_RUNTIME_ACCESS 0x00000004
-///
-/// This attribute is identified by the mnemonic 'HR'
-/// elsewhere in this specification.
-///
-#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x00000008
-///
-/// Attributes of Authenticated Variable
-///
-#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x00000020
-#define EFI_VARIABLE_APPEND_WRITE 0x00000040
-///
-/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be considered reserved.
-///
-#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x00000010
-
-///
/// AuthInfo is a WIN_CERTIFICATE using the wCertificateType
/// WIN_CERTIFICATE_UEFI_GUID and the CertType
/// EFI_CERT_TYPE_RSA2048_SHA256_GUID. If the attribute specifies
diff --git a/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h
new file mode 100644
index 0000000000..df55a92dfa
--- /dev/null
+++ b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h
@@ -0,0 +1,39 @@
+/** @file
+ This includes some definitions introduced in UEFI that will be used in both PEI and DXE phases.
+
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+This program and the accompanying materials are licensed and made available under
+the terms and conditions of the BSD License that 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.
+
+**/
+
+#ifndef __UEFI_MULTIPHASE_DEFS_H__
+#define __UEFI_MULTIPHASE_DEFS_H__
+
+///
+/// Attributes of variable.
+///
+#define EFI_VARIABLE_NON_VOLATILE 0x00000001
+#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002
+#define EFI_VARIABLE_RUNTIME_ACCESS 0x00000004
+///
+/// This attribute is identified by the mnemonic 'HR'
+/// elsewhere in this specification.
+///
+#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x00000008
+///
+/// Attributes of Authenticated Variable
+///
+#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x00000020
+#define EFI_VARIABLE_APPEND_WRITE 0x00000040
+///
+/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be considered reserved.
+///
+#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x00000010
+
+#endif
--
2.11.0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 12/13/2017 8:26 PM, Leif Lindholm wrote: > Turns out all .vfr files in the tree interacting with DynamicPcds > manually copy the same set of EFI_VARIABLE_* definitions, since the rest > of UefiMultiPhase.h is incompatible with VfrCompile. > > Split these out into a separate header file UefiMultiPhaseDefinitions.h > in order to make it possible to include just that portion into .vfr > files. Then include that from UefiMultiPhase.h. > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > Cc: Liming Gao <liming.gao@intel.com> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Star Zeng <star.zeng@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> > --- > MdePkg/Include/Uefi/UefiMultiPhase.h | 23 +----------- > MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h | 39 ++++++++++++++++++++ > 2 files changed, 41 insertions(+), 21 deletions(-) > > diff --git a/MdePkg/Include/Uefi/UefiMultiPhase.h b/MdePkg/Include/Uefi/UefiMultiPhase.h > index 0dcbb1b9ee..b360c9513b 100644 > --- a/MdePkg/Include/Uefi/UefiMultiPhase.h > +++ b/MdePkg/Include/Uefi/UefiMultiPhase.h > @@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > #ifndef __UEFI_MULTIPHASE_H__ > #define __UEFI_MULTIPHASE_H__ > > +#include "UefiMultiPhaseDefinitions.h" > + > #include <Guid/WinCertificate.h> > /// > /// Enumeration of memory types introduced in UEFI. > @@ -156,27 +158,6 @@ typedef struct { > } EFI_TABLE_HEADER; > > /// > -/// Attributes of variable. > -/// > -#define EFI_VARIABLE_NON_VOLATILE 0x00000001 > -#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002 > -#define EFI_VARIABLE_RUNTIME_ACCESS 0x00000004 > -/// > -/// This attribute is identified by the mnemonic 'HR' > -/// elsewhere in this specification. > -/// > -#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x00000008 > -/// > -/// Attributes of Authenticated Variable > -/// > -#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x00000020 > -#define EFI_VARIABLE_APPEND_WRITE 0x00000040 > -/// > -/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be considered reserved. > -/// > -#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x00000010 > - > -/// > /// AuthInfo is a WIN_CERTIFICATE using the wCertificateType > /// WIN_CERTIFICATE_UEFI_GUID and the CertType > /// EFI_CERT_TYPE_RSA2048_SHA256_GUID. If the attribute specifies > diff --git a/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h > new file mode 100644 > index 0000000000..df55a92dfa > --- /dev/null > +++ b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h > @@ -0,0 +1,39 @@ > +/** @file > + This includes some definitions introduced in UEFI that will be used in both PEI and DXE phases. > + > +Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR> > +This program and the accompanying materials are licensed and made available under > +the terms and conditions of the BSD License that 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. > + > +**/ > + > +#ifndef __UEFI_MULTIPHASE_DEFS_H__ > +#define __UEFI_MULTIPHASE_DEFS_H__ > + > +/// > +/// Attributes of variable. > +/// > +#define EFI_VARIABLE_NON_VOLATILE 0x00000001 > +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002 > +#define EFI_VARIABLE_RUNTIME_ACCESS 0x00000004 > +/// > +/// This attribute is identified by the mnemonic 'HR' > +/// elsewhere in this specification. > +/// > +#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x00000008 > +/// > +/// Attributes of Authenticated Variable > +/// > +#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x00000020 > +#define EFI_VARIABLE_APPEND_WRITE 0x00000040 > +/// > +/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be considered reserved. > +/// > +#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x00000010 > + > +#endif > Can we just move the definitions to UefiBaseTypes.h? Even vfrcompiler still complains, the syntax enhancement to vfrcompiler should be simple. I personally do not like creating more and more files. -- Thanks, Ray _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Thu, Dec 14, 2017 at 10:44:04AM +0800, Ni, Ruiyu wrote: > On 12/13/2017 8:26 PM, Leif Lindholm wrote: > > Turns out all .vfr files in the tree interacting with DynamicPcds > > manually copy the same set of EFI_VARIABLE_* definitions, since the rest > > of UefiMultiPhase.h is incompatible with VfrCompile. > > > > Split these out into a separate header file UefiMultiPhaseDefinitions.h > > in order to make it possible to include just that portion into .vfr > > files. Then include that from UefiMultiPhase.h. > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com> > > Cc: Liming Gao <liming.gao@intel.com> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Cc: Star Zeng <star.zeng@intel.com> > > Cc: Eric Dong <eric.dong@intel.com> > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> > > --- > > MdePkg/Include/Uefi/UefiMultiPhase.h | 23 +----------- > > MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h | 39 ++++++++++++++++++++ > > 2 files changed, 41 insertions(+), 21 deletions(-) > > > > diff --git a/MdePkg/Include/Uefi/UefiMultiPhase.h b/MdePkg/Include/Uefi/UefiMultiPhase.h > > index 0dcbb1b9ee..b360c9513b 100644 > > --- a/MdePkg/Include/Uefi/UefiMultiPhase.h > > +++ b/MdePkg/Include/Uefi/UefiMultiPhase.h > > @@ -15,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > #ifndef __UEFI_MULTIPHASE_H__ > > #define __UEFI_MULTIPHASE_H__ > > +#include "UefiMultiPhaseDefinitions.h" > > + > > #include <Guid/WinCertificate.h> > > /// > > /// Enumeration of memory types introduced in UEFI. > > @@ -156,27 +158,6 @@ typedef struct { > > } EFI_TABLE_HEADER; > > /// > > -/// Attributes of variable. > > -/// > > -#define EFI_VARIABLE_NON_VOLATILE 0x00000001 > > -#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002 > > -#define EFI_VARIABLE_RUNTIME_ACCESS 0x00000004 > > -/// > > -/// This attribute is identified by the mnemonic 'HR' > > -/// elsewhere in this specification. > > -/// > > -#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x00000008 > > -/// > > -/// Attributes of Authenticated Variable > > -/// > > -#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x00000020 > > -#define EFI_VARIABLE_APPEND_WRITE 0x00000040 > > -/// > > -/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be considered reserved. > > -/// > > -#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x00000010 > > - > > -/// > > /// AuthInfo is a WIN_CERTIFICATE using the wCertificateType > > /// WIN_CERTIFICATE_UEFI_GUID and the CertType > > /// EFI_CERT_TYPE_RSA2048_SHA256_GUID. If the attribute specifies > > diff --git a/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h > > new file mode 100644 > > index 0000000000..df55a92dfa > > --- /dev/null > > +++ b/MdePkg/Include/Uefi/UefiMultiPhaseDefinitions.h > > @@ -0,0 +1,39 @@ > > +/** @file > > + This includes some definitions introduced in UEFI that will be used in both PEI and DXE phases. > > + > > +Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR> > > +This program and the accompanying materials are licensed and made available under > > +the terms and conditions of the BSD License that 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. > > + > > +**/ > > + > > +#ifndef __UEFI_MULTIPHASE_DEFS_H__ > > +#define __UEFI_MULTIPHASE_DEFS_H__ > > + > > +/// > > +/// Attributes of variable. > > +/// > > +#define EFI_VARIABLE_NON_VOLATILE 0x00000001 > > +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002 > > +#define EFI_VARIABLE_RUNTIME_ACCESS 0x00000004 > > +/// > > +/// This attribute is identified by the mnemonic 'HR' > > +/// elsewhere in this specification. > > +/// > > +#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x00000008 > > +/// > > +/// Attributes of Authenticated Variable > > +/// > > +#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x00000020 > > +#define EFI_VARIABLE_APPEND_WRITE 0x00000040 > > +/// > > +/// NOTE: EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated and should be considered reserved. > > +/// > > +#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x00000010 > > + > > +#endif > > > Can we just move the definitions to UefiBaseTypes.h? There is nothing in UefiBaseTypes.h that is fundamentally easier to deal with than what is in UefiMultiPhase.h to begin with. > Even vfrcompiler still complains, the syntax enhancement to > vfrcompiler should be simple. It's (probably) not VfrCompile that would need to change. The problem is that the build runs the C preprocessor on the .vfr source files before passing them to VfrCompile. #defines that just replace text work fine, but typedefs and struct definitions make no sense in vfr source. It runs the preprocessor in C mode (for gpp, -x c) which means we cannot use __ASSEMBLER__ to filter out typedefs and struct definitions. We could perhaps switch to -x assembler-with-cpp (and filter UefiMultiPhase.h on __ASSEMBLER__), like we do for DTC. It _seems_ to work in my test, but I do not know what a corresponding change for VS would be. > I personally do not like creating more and more files. I am sort of with you on that one. But I consider it a much lesser evil than duplicated definitions. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Thu, Dec 14, 2017 at 12:03:56PM +0000, Leif Lindholm wrote: > On Thu, Dec 14, 2017 at 10:44:04AM +0800, Ni, Ruiyu wrote: > > Can we just move the definitions to UefiBaseTypes.h? > > There is nothing in UefiBaseTypes.h that is fundamentally easier to > deal with than what is in UefiMultiPhase.h to begin with. > > > Even vfrcompiler still complains, the syntax enhancement to > > vfrcompiler should be simple. > > It's (probably) not VfrCompile that would need to change. > The problem is that the build runs the C preprocessor on the .vfr > source files before passing them to VfrCompile. #defines that just > replace text work fine, but typedefs and struct definitions make no > sense in vfr source. > > It runs the preprocessor in C mode (for gpp, -x c) which means we > cannot use __ASSEMBLER__ to filter out typedefs and struct > definitions. > > We could perhaps switch to -x assembler-with-cpp (and filter > UefiMultiPhase.h on __ASSEMBLER__), like we do for DTC. > It _seems_ to work in my test, but I do not know what a corresponding > change for VS would be. Well, this turns out to be unworkable anyway, since the affected .vfr source files depend on certain typedefs and struct definitions. I was under the misconception that only #defines were used in .vfr. So, yes, the better solution would be to teach VfrCompile to understand enums and other things, but I wouldn't really know where to start (and I'm about to disappear for a month). Symptoms when building MdeModulePkg DriverSampleDxe with UefiMultiPhase.h include ERROR 12288: EFI_GUID : undefined ERROR 12288: enum : unexpected token If I then add UefiBaseTypes.h, we insted have issues from the inclusion of ProcessorBind.h: ERROR 12288: unsigned : unexpected token ERROR 12288: long : unexpected token and so on. An alternative less invasive solution, which would avoid adding new files, would be to add #ifndef VFRCOMPILE around everything other than the EFI_VARIABLE_* #defines in UefiMultiPhase.h. Would you be happier with that as a temporary measure until someone gets around to looking at VfrCompile? / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Leif: Right. Current VfrCompiler supports the limited C syntax. Many C syntax are not recognized. So, you will meet with the below build error. For this change, I agree to share the definition between C and VFR source files. The temp solution is to use #ifndef VFRCOMPILE around the definition. The long term solution is to enhance VfrCompiler tool to recognize more C syntax so that C and VFR can share more definitions. For long term solution, you can submit feature request in bugzillia to catch it first. Thanks Liming > -----Original Message----- > From: Leif Lindholm [mailto:leif.lindholm@linaro.org] > Sent: Friday, December 15, 2017 1:00 AM > To: Ni, Ruiyu <ruiyu.ni@intel.com> > Cc: edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star > <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org> > Subject: Re: [edk2] [PATCH 1/3] MdePkg: break #defines out of Uefi/UefiMultiPhase.h > > On Thu, Dec 14, 2017 at 12:03:56PM +0000, Leif Lindholm wrote: > > On Thu, Dec 14, 2017 at 10:44:04AM +0800, Ni, Ruiyu wrote: > > > Can we just move the definitions to UefiBaseTypes.h? > > > > There is nothing in UefiBaseTypes.h that is fundamentally easier to > > deal with than what is in UefiMultiPhase.h to begin with. > > > > > Even vfrcompiler still complains, the syntax enhancement to > > > vfrcompiler should be simple. > > > > It's (probably) not VfrCompile that would need to change. > > The problem is that the build runs the C preprocessor on the .vfr > > source files before passing them to VfrCompile. #defines that just > > replace text work fine, but typedefs and struct definitions make no > > sense in vfr source. > > > > It runs the preprocessor in C mode (for gpp, -x c) which means we > > cannot use __ASSEMBLER__ to filter out typedefs and struct > > definitions. > > > > We could perhaps switch to -x assembler-with-cpp (and filter > > UefiMultiPhase.h on __ASSEMBLER__), like we do for DTC. > > It _seems_ to work in my test, but I do not know what a corresponding > > change for VS would be. > > Well, this turns out to be unworkable anyway, since the affected > .vfr source files depend on certain typedefs and struct definitions. > I was under the misconception that only #defines were used in .vfr. > > So, yes, the better solution would be to teach VfrCompile to > understand enums and other things, but I wouldn't really know where to > start (and I'm about to disappear for a month). > > Symptoms when building MdeModulePkg DriverSampleDxe with > UefiMultiPhase.h include > > ERROR 12288: EFI_GUID > : undefined > ERROR 12288: enum > : unexpected token > > If I then add UefiBaseTypes.h, we insted have issues from the > inclusion of ProcessorBind.h: > > ERROR 12288: unsigned > : unexpected token > ERROR 12288: long > : unexpected token > > and so on. > > An alternative less invasive solution, which would avoid adding new > files, would be to add #ifndef VFRCOMPILE around everything other > than the EFI_VARIABLE_* #defines in UefiMultiPhase.h. > > Would you be happier with that as a temporary measure until someone > gets around to looking at VfrCompile? > > / > Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2025 Red Hat, Inc.