[edk2] Question about memory reservation from PrePi to DXE

[Benjamin Herrenschmidt]

Hi !

I'm still trying to get my head around the internals of EDK here so bear
with me if there's an obvious answer that I missed ... :-)

I'm trying to understand how I can properly reserve bits of memory from
my PrePei or Pei so that DXE won't stomp on them.

To simplify the discussion, let's assume that I have a resource
descriptor of type EFI_RESOURCE_SYSTEM_MEMORY that cover, well .. all of
my system memory.

Then on top of that, I have a number of EFI_HOB_TYPE_MEMORY_ALLOCATION
HOBs that represent bits of that memory that I want to reserve.

>From what I can see of the code in Gcd.c, this won't work, as
CoreInitializeMemoryServices() will not consider the allocation HOBs
before picking a piece of initial memory, and that initial memory will
be directed straight to the free list.

Or am I missing something ?

So I need to "split" my EFI_RESOURCE_SYSTEM_MEMORY resource descriptor
HOBs to "avoid" reserved regions of memory before starting DXE,
correct ?

Now if I'm correct, then I don't understand how the code in
OvmfPkg/PlatformPei/MemDetect.c works, since as far as I can tell,
it declares system memory, then create allocation HOBs for areas that
are reserved, but doesn't take those area out of the system memory hob
list...

What am I missing ? :-)

Thanks !

Cheers,
Ben.


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 10/10] Vlv2TbltDevicePkg: Support format string argument checking

[Scott Duplichan]

Support compile time argument consistency checking for functions that
accept a PrintLib format string followed by a variable argument list.
Macro 'EFIFORMAT' added to the function prototype accepts a single
argument indicating which function argument holds the format string.
The EFIFORMAT macro assumes the variable argument list immediately
follows the format string. Format string argument checking requires
a compiler that understands EDK2 format strings, such as GCC with
the gcc_format from BaseTools/gcc applied.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
---


 Vlv2TbltDevicePkg/Include/FileHandleLib.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Vlv2TbltDevicePkg/Include/FileHandleLib.h 
b/Vlv2TbltDevicePkg/Include/FileHandleLib.h
index 59d1b92..85622a0 100644
--- a/Vlv2TbltDevicePkg/Include/FileHandleLib.h
+++ b/Vlv2TbltDevicePkg/Include/FileHandleLib.h
@@ -476,6 +476,7 @@ FileHandleWriteLine(
 **/
 EFI_STATUS
 EFIAPI
+EFIFORMAT (2)
 FileHandlePrintLine(
   IN EFI_FILE_HANDLE  Handle,
   IN CONST CHAR16 *Format,


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 1/10] BaseTools: Add GCC patch for EDK2 format string argument checking

[Scott Duplichan]

Add patch to extend GCC format string argument consistency checks to
include the UEFI EDK2 PrintLib and ShellLib format strings. The patch
adds GCC predefined compiler macro __GCC_EDK2_FORMATS__ for
identification of the feature. EDK2 format string of type CHAR8[] and
CHAR16[] can be checked.


Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
---


 BaseTools/gcc/gcc-4.9.3_format.patch | 240 +++
 BaseTools/gcc/gcc-5.2.0_format.patch | 240 +++
 2 files changed, 480 insertions(+)

diff --git a/BaseTools/gcc/gcc-4.9.3_format.patch 
b/BaseTools/gcc/gcc-4.9.3_format.patch
new file mode 100644
index 000..65f56ba
--- /dev/null
+++ b/BaseTools/gcc/gcc-4.9.3_format.patch
@@ -0,0 +1,240 @@
+[PATCH] Extend gcc format string argument consistency checks to
+include the UEFI EDK2 PrintLib and ShellLib format strings.
+For PrintLib format checking, use:
+__attribute__((format (edk2_printf, FormatPosition, FormatPosition+1)))  
+For ShellLib format checking, use:
+__attribute__((format (edk2_shell_printf, FormatPosition, FormatPosition+1)))  
+
+
+diff -rupN gcc-4.9.3/gcc/c-family/c-cppbuiltin.c 
gcc-4.9.3-patched/gcc/c-family/c-cppbuiltin.c
+--- gcc-4.9.3/gcc/c-family/c-cppbuiltin.c  2014-10-08 06:05:43.0 
-0500
 gcc-4.9.3-patched/gcc/c-family/c-cppbuiltin.c  2015-08-02 
11:44:01.445024600 -0500
+@@ -782,6 +782,9 @@ c_cpp_builtins (cpp_reader *pfile)
+   if (flag_undef)
+ return;
+ 
++  /* identify presence of edk2_printf and edk2_shell_printf format checking */
++  cpp_define (pfile, "__GCC_EDK2_FORMATS__");
++
+   define_language_independent_builtin_macros (pfile);
+ 
+   if (c_dialect_cxx ())
+diff -rupN gcc-4.9.3/gcc/c-family/c-format.c 
gcc-4.9.3-patched/gcc/c-family/c-format.c
+--- gcc-4.9.3/gcc/c-family/c-format.c  2014-01-02 16:23:26.0 -0600
 gcc-4.9.3-patched/gcc/c-family/c-format.c  2015-08-02 11:44:01.445024600 
-0500
+@@ -33,6 +33,140 @@ along with GCC; see the file COPYING3.
+ #include "alloc-pool.h"
+ #include "c-target.h"
+ 
++//
++
++/* format attributes for edk2_printf */
++#undef TARGET_FORMAT_TYPES
++#define TARGET_FORMAT_TYPES targetFormatTypes()
++
++//
++/* format attributes edk2_printf */
++
++#define DIMENSION(array) (sizeof (array) / sizeof (array [0]))
++
++static format_length_info edk2_printf_length_specs[] =
++{
++  { "L", FMT_LEN_ll, STD_EXT, NULL, FMT_LEN_none, STD_C89, 1 },
++  { "l", FMT_LEN_ll, STD_EXT, NULL, FMT_LEN_none, STD_C89, 1 },
++  { NULL, FMT_LEN_none, STD_C89, NULL, FMT_LEN_none, STD_C89, 0 }
++};
++
++static const format_flag_spec edk2_printf_flag_specs[] =
++{
++  { ' ',  0, 0, N_("' ' flag"),N_("the ' ' printf flag"), 
 STD_C89 },
++  { '+',  0, 0, N_("'+' flag"),N_("the '+' printf flag"), 
 STD_C89 },
++  { '0',  0, 0, N_("'0' flag"),N_("the '0' printf flag"), 
 STD_C89 },
++  { '-',  0, 0, N_("'-' flag"),N_("the '-' printf flag"), 
 STD_C89 },
++  { ',',  0, 0, N_("',' flag"),N_("the ',' printf flag"), 
 STD_C89 },
++  { 'w',  0, 0, N_("field width"), N_("field width in printf format"),
 STD_C89 },
++  { 'p',  0, 0, N_("precision"),   N_("precision in printf format"),  
 STD_C89 },
++  { 'L',  0, 0, N_("length modifier"), N_("length modifier in printf 
format"), STD_C89 },
++  { 'l',  0, 0, N_("length modifier"), N_("length modifier in printf 
format"), STD_C89 },
++  { 0, 0, 0, NULL, NULL, STD_C89 }
++};
++
++static const format_flag_pair edk2_printf_flag_pairs[] =
++{
++  { ' ', '+', 1, 0   },
++  { '0', '-', 1, 0   }, { '0', 'p', 1, 'i' },
++  { 0, 0, 0, 0 }
++};
++
++static const format_char_info edk2_print_char_table_template[] =
++{
++  /* C89 conversion specifiers.  */
++//   none hh   hlll   L   
 z   tjHDDD
++  { "r",   0, STD_C89, { T89_I,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN, 
 BADLEN, BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN }, "","i",  NULL },
++  { "d",   0, STD_C89, { T89_I,   BADLEN,  T89_S,   T89_L,   T9L_LL,  
T99_SST, BADLEN, BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN }, "-wp0 +,", "i",  
NULL },
++  { "oxX", 0, STD_C89, { T89_UI,  BADLEN,  T89_US,  T89_UL,  T9L_ULL, T99_ST, 
 BADLEN, BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN }, "-wp0 +",  "i",  NULL },
++  { "u",   0, STD_C89, { T89_UI,  BADLEN,  T89_US,  T89_UL,  T9L_ULL, T99_ST, 
 BADLEN, BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN }, "-wp", "i",  NULL },
++  { "c",   0, STD_C89, { T89_I,   BADLEN,  T89_S,   T94_WI,  BADLEN,  BADLEN, 
 BADLEN, BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN }, "-w",  "",   NULL },
++  { "a",   1, STD_C89, { T89_C,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLE

[edk2] [PATCH 9/10] UefiCpuPkg: Support format string argument checking

[Scott Duplichan]

Support compile time argument consistency checking for functions that
accept a PrintLib format string followed by a variable argument list.
Macro 'EFIFORMAT' added to the function prototype accepts a single
argument indicating which function argument holds the format string.
The EFIFORMAT macro assumes the variable argument list immediately
follows the format string. Format string argument checking requires
a compiler that understands EDK2 format strings, such as GCC with
the gcc_format from BaseTools/gcc applied.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
---


 UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h 
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
index b28e9c5..4fe9192 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
@@ -92,6 +92,7 @@ ArchGetIdtHandler (
 **/
 VOID
 EFIAPI
+EFIFORMAT (1)
 InternalPrintMessage (
   IN  CONST CHAR8  *Format,
   ...


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 8/10] SourceLevelDebugPkg: Support format string argument checking

[Scott Duplichan]

Support compile time argument consistency checking for functions that
accept a PrintLib format string followed by a variable argument list.
Macro 'EFIFORMAT' added to the function prototype accepts a single
argument indicating which function argument holds the format string.
The EFIFORMAT macro assumes the variable argument list immediately
follows the format string. Format string argument checking requires
a compiler that understands EDK2 format strings, such as GCC with
the gcc_format from BaseTools/gcc applied.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
---


 SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h 
b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h
index 64e4c3e..c831314 100644
--- a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h
+++ b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h
@@ -290,6 +290,7 @@ DebugReadBreakSymbol (
 **/
 VOID
 EFIAPI
+EFIFORMAT (2)
 DebugAgentMsgPrint (
   IN UINT8 ErrorLevel,
   IN CHAR8 *Format,


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 7/10] ShellPkg: Support format string argument checking

[Scott Duplichan]

Support compile time argument consistency checking for functions that
accept a PrintLib format string followed by a variable argument list.
Macro 'EFIFORMAT' added to the function prototype accepts a single
argument indicating which function argument holds the format string.
The EFIFORMAT macro assumes the variable argument list immediately
follows the format string. Format string argument checking requires
a compiler that understands EDK2 format strings, such as GCC with
the gcc_format from BaseTools/gcc applied.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
---


 ShellPkg/Include/Library/ShellLib.h   | 1 +
 ShellPkg/Library/UefiShellCommandLib/ConsistMapping.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/ShellPkg/Include/Library/ShellLib.h 
b/ShellPkg/Include/Library/ShellLib.h
index 23da4ee..0be5613 100644
--- a/ShellPkg/Include/Library/ShellLib.h
+++ b/ShellPkg/Include/Library/ShellLib.h
@@ -885,6 +885,7 @@ ShellInitialize (
 **/
 EFI_STATUS
 EFIAPI
+EFIFORMAT_SHELL (3)
 ShellPrintEx(
   IN INT32Col OPTIONAL,
   IN INT32Row OPTIONAL,
diff --git a/ShellPkg/Library/UefiShellCommandLib/ConsistMapping.c 
b/ShellPkg/Library/UefiShellCommandLib/ConsistMapping.c
index 9bd7b2c..6dd81d4 100644
--- a/ShellPkg/Library/UefiShellCommandLib/ConsistMapping.c
+++ b/ShellPkg/Library/UefiShellCommandLib/ConsistMapping.c
@@ -81,6 +81,7 @@ typedef struct {
 **/
 CHAR16 *
 EFIAPI
+EFIFORMAT (2)
 CatPrint (
   IN OUT POOL_PRINT   *Str,
   IN CHAR16   *Fmt,


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 6/10] SecurityPkg: Support format string argument checking

[Scott Duplichan]

Support compile time argument consistency checking for functions that
accept a PrintLib format string followed by a variable argument list.
Macro 'EFIFORMAT' added to the function prototype accepts a single
argument indicating which function argument holds the format string.
The EFIFORMAT macro assumes the variable argument list immediately
follows the format string. Format string argument checking requires
a compiler that understands EDK2 format strings, such as GCC with
the gcc_format from BaseTools/gcc applied.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
---


 SecurityPkg/Tcg/TcgDxe/TpmComm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/SecurityPkg/Tcg/TcgDxe/TpmComm.h b/SecurityPkg/Tcg/TcgDxe/TpmComm.h
index 763ad76..66ab0dd 100644
--- a/SecurityPkg/Tcg/TcgDxe/TpmComm.h
+++ b/SecurityPkg/Tcg/TcgDxe/TpmComm.h
@@ -90,6 +90,7 @@ TpmCommGetFlags (
 **/
 EFI_STATUS
 EFIAPI
+EFIFORMAT (2)
 TisPcExecute (
   IN  TIS_TPM_HANDLETisReg,
   IN  CONST CHAR8   *Fmt,


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 5/10] OvmfPkg: Support format string argument checking

[Scott Duplichan]

Support compile time argument consistency checking for functions that
accept a PrintLib format string followed by a variable argument list.
Macro 'EFIFORMAT' added to the function prototype accepts a single
argument indicating which function argument holds the format string.
The EFIFORMAT macro assumes the variable argument list immediately
follows the format string. Format string argument checking requires
a compiler that understands EDK2 format strings, such as GCC with
the gcc_format from BaseTools/gcc applied.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
Reviewed-by: Laszlo Ersek
---


 OvmfPkg/Include/Protocol/XenBus.h | 2 +-
 OvmfPkg/XenBusDxe/XenStore.h  | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Include/Protocol/XenBus.h 
b/OvmfPkg/Include/Protocol/XenBus.h
index 3509691..46ad93b 100644
--- a/OvmfPkg/Include/Protocol/XenBus.h
+++ b/OvmfPkg/Include/Protocol/XenBus.h
@@ -132,7 +132,7 @@ XENSTORE_STATUS
 **/
 typedef
 XENSTORE_STATUS
-(EFIAPI *XENBUS_XS_PRINTF) (
+(EFIAPI EFIFORMAT (5) *XENBUS_XS_PRINTF) (
   IN XENBUS_PROTOCOL*This,
   IN CONST XENSTORE_TRANSACTION *Transaction,
   IN CONST CHAR8*Directory,
diff --git a/OvmfPkg/XenBusDxe/XenStore.h b/OvmfPkg/XenBusDxe/XenStore.h
index de56901..5a26cbb 100644
--- a/OvmfPkg/XenBusDxe/XenStore.h
+++ b/OvmfPkg/XenBusDxe/XenStore.h
@@ -188,6 +188,7 @@ XenStoreTransactionEnd (
 **/
 XENSTORE_STATUS
 EFIAPI
+EFIFORMAT (4)
 XenStoreSPrint (
   IN CONST XENSTORE_TRANSACTION *Transaction,
   IN CONST CHAR8*DirectoryPath,
@@ -344,6 +345,7 @@ XenBusXenStoreTransactionEnd (
 
 XENSTORE_STATUS
 EFIAPI
+EFIFORMAT (5)
 XenBusXenStoreSPrint (
   IN XENBUS_PROTOCOL*This,
   IN CONST XENSTORE_TRANSACTION *Transaction,


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 4/10] MdeModulePkg: Support format string argument checking

[Scott Duplichan]

Support compile time argument consistency checking for functions that
accept a PrintLib format string followed by a variable argument list.
Macro 'EFIFORMAT' added to the function prototype accepts a single
argument indicating which function argument holds the format string.
The EFIFORMAT macro assumes the variable argument list immediately
follows the format string. Format string argument checking requires
a compiler that understands EDK2 format strings, such as GCC with
the gcc_format from BaseTools/gcc applied.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
---


 MdeModulePkg/Include/Library/NetLib.h  |  1 +
 MdeModulePkg/Include/Protocol/Print2.h |  8 +++---
 .../CustomizedDisplayLibInternal.h |  1 +
 MdePkg/Include/AArch64/ProcessorBind.h | 33 ++
 MdePkg/Include/Arm/ProcessorBind.h | 31 
 MdePkg/Include/Ia32/ProcessorBind.h| 32 +
 MdePkg/Include/Ipf/ProcessorBind.h | 32 +
 MdePkg/Include/Library/DebugLib.h  |  1 +
 MdePkg/Include/Library/FileHandleLib.h |  1 +
 MdePkg/Include/Library/PrintLib.h  |  4 +++
 MdePkg/Include/Library/UefiLib.h   |  7 +
 MdePkg/Include/X64/ProcessorBind.h | 32 +
 MdePkg/Library/BasePrintLib/PrintLibInternal.h |  1 +
 .../Library/UefiDevicePathLib/DevicePathToText.c   |  1 +
 14 files changed, 181 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Include/Library/NetLib.h 
b/MdeModulePkg/Include/Library/NetLib.h
index 280c51a..9608335 100644
--- a/MdeModulePkg/Include/Library/NetLib.h
+++ b/MdeModulePkg/Include/Library/NetLib.h
@@ -292,6 +292,7 @@ typedef struct {
 **/
 CHAR8 *
 EFIAPI
+EFIFORMAT (1)
 NetDebugASPrint (
   IN CHAR8  *Format,
   ...
diff --git a/MdeModulePkg/Include/Protocol/Print2.h 
b/MdeModulePkg/Include/Protocol/Print2.h
index 1c127d5..795faba 100644
--- a/MdeModulePkg/Include/Protocol/Print2.h
+++ b/MdeModulePkg/Include/Protocol/Print2.h
@@ -102,7 +102,7 @@ UINTN
 **/
 typedef
 UINTN
-(EFIAPI *UNICODE_S_PRINT)(
+(EFIAPI EFIFORMAT (3) *UNICODE_S_PRINT)(
   OUT CHAR16*StartOfBuffer,
   IN  UINTN BufferSize,
   IN  CONST CHAR16  *FormatString,
@@ -184,7 +184,7 @@ UINTN
 **/
 typedef
 UINTN
-(EFIAPI *UNICODE_S_PRINT_ASCII_FORMAT)(
+(EFIAPI EFIFORMAT (3) *UNICODE_S_PRINT_ASCII_FORMAT)(
   OUT CHAR16   *StartOfBuffer,
   IN  UINTNBufferSize,
   IN  CONST CHAR8  *FormatString,
@@ -314,7 +314,7 @@ UINTN
 **/
 typedef
 UINTN
-(EFIAPI *ASCII_S_PRINT)(
+(EFIAPI EFIFORMAT (3) *ASCII_S_PRINT)(
   OUT CHAR8*StartOfBuffer,
   IN  UINTNBufferSize,
   IN  CONST CHAR8  *FormatString,
@@ -396,7 +396,7 @@ UINTN
 **/
 typedef
 UINTN
-(EFIAPI *ASCII_S_PRINT_UNICODE_FORMAT)(
+(EFIAPI EFIFORMAT (3) *ASCII_S_PRINT_UNICODE_FORMAT)(
   OUT CHAR8 *StartOfBuffer,
   IN  UINTN BufferSize,
   IN  CONST CHAR16  *FormatString,
diff --git 
a/MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLibInternal.h 
b/MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLibInternal.h
index 7342b50..6e37cc1 100644
--- a/MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLibInternal.h
+++ b/MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLibInternal.h
@@ -275,6 +275,7 @@ LibSetUnicodeMem (
 **/
 UINTN
 EFIAPI
+EFIFORMAT (4)
 PrintAt (
   IN UINTN Width,
   IN UINTN Column,
diff --git a/MdePkg/Include/AArch64/ProcessorBind.h 
b/MdePkg/Include/AArch64/ProcessorBind.h
index c5fe874..2a6faa5 100644
--- a/MdePkg/Include/AArch64/ProcessorBind.h
+++ b/MdePkg/Include/AArch64/ProcessorBind.h
@@ -145,4 +145,37 @@ typedef INT64   INTN;
 #define __USER_LABEL_PREFIX__
 #endif
 
+/**
+  Define modifiers to enable function argument checking for a format
+  string followed by a variable argument list.
+
+  Modifier EFIFORMAT enables argument checking for a format string of the
+  type described in PrintLib.h. Modifier EFIFORMAT_SHELL enables argument
+  checking for a format string of the type described in ShellLib.h. The
+  FormatPosition macro argument identifies which function argument contains
+  the format string (relative 1). The variable argument list must start
+  immediately after the format string. To use this argument checking with
+  gcc, the gcc tool chain must be rebuilt using the patch in BaseTools/gcc.
+
+  @param   FormatPositionPosition (relative 1) of the function argument
+ containing the format string.
+
+  @return  None
+
+**/
+#if defined(EDK2_FORMAT_CHECKING)
+  #if defined(__GCC_EDK2_FORMATS__)
+#define EFIFORMAT(FormatPosition) \
+__attribute__((format (edk2_printf, FormatPosition, FormatPosition+1)))
+#define EFIFORMAT_SHELL(FormatPosition) \
+__attribute__((format (edk2_shell_printf, FormatPosition, 

[edk2] [PATCH 3/10] IntelFrameworkModulePkg: Support format string argument checking

[Scott Duplichan]

Support compile time argument consistency checking for functions that
accept a PrintLib format string followed by a variable argument list.
Macro 'EFIFORMAT' added to the function prototype accepts a single
argument indicating which function argument holds the format string.
The EFIFORMAT macro assumes the variable argument list immediately
follows the format string. Format string argument checking requires
a compiler that understands EDK2 format strings, such as GCC with
the gcc_format from BaseTools/gcc applied.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
---


 IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h 
b/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h
index ecd68a0..899eb71 100644
--- a/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h
+++ b/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h
@@ -1074,6 +1074,7 @@ DevPathVendor (
 **/
 CHAR16 *
 EFIAPI
+EFIFORMAT (2)
 CatPrint (
   IN OUT POOL_PRINT   *Str,
   IN CHAR16   *Fmt,


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 2/10] DuetPkg: Support format string argument checking

[Scott Duplichan]

Support compile time argument consistency checking for functions that
accept a PrintLib format string followed by a variable argument list.
Macro 'EFIFORMAT' added to the function prototype accepts a single
argument indicating which function argument holds the format string.
The EFIFORMAT macro assumes the variable argument list immediately
follows the format string. Format string argument checking requires
a compiler that understands EDK2 format strings, such as GCC with
the gcc_format from BaseTools/gcc applied.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
---


 DuetPkg/DxeIpl/Debug.h | 1 +
 DuetPkg/EfiLdr/Debug.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/DuetPkg/DxeIpl/Debug.h b/DuetPkg/DxeIpl/Debug.h
index 7fcad26..0f9e65f 100644
--- a/DuetPkg/DxeIpl/Debug.h
+++ b/DuetPkg/DxeIpl/Debug.h
@@ -27,6 +27,7 @@ PrintHeader (
   );
 
 VOID
+EFIFORMAT (1)
 PrintString (
   IN CONST CHAR8  *FormatString,
   ...
diff --git a/DuetPkg/EfiLdr/Debug.h b/DuetPkg/EfiLdr/Debug.h
index f6aa7a2..f93d56a 100644
--- a/DuetPkg/EfiLdr/Debug.h
+++ b/DuetPkg/EfiLdr/Debug.h
@@ -27,6 +27,7 @@ PrintHeader (
   );
 
 VOID
+EFIFORMAT (1)
 PrintString (
   IN CONST CHAR8  *FormatString,
   ...


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 0/10] Check arguments for PrintLib format string

[Scott Duplichan]

This is essentially the same patch as before, broken up by Pkg. The
only change is the macro modifier added to function prototypes now
needs only one argument instead of two, as suggested by Andrew.
The macro name itself selects between PrintLib style format string
and ShellLib style format strings, rather than a dedicated macro
argument. The set also adds the GCC patch to BaseLib/gcc as suggested
by Liming.

 BaseTools/gcc/gcc-4.9.3_format.patch   | 240 +
 BaseTools/gcc/gcc-5.2.0_format.patch   | 240 +
 DuetPkg/DxeIpl/Debug.h |   1 +
 DuetPkg/EfiLdr/Debug.h |   1 +
 MdeModulePkg/Include/Library/NetLib.h  |   1 +
 MdeModulePkg/Include/Protocol/Print2.h |   8 +-
 .../CustomizedDisplayLibInternal.h |   1 +
 MdePkg/Include/AArch64/ProcessorBind.h |  33 +++
 MdePkg/Include/Arm/ProcessorBind.h |  31 +++
 MdePkg/Include/Ia32/ProcessorBind.h|  32 +++
 MdePkg/Include/Ipf/ProcessorBind.h |  32 +++
 MdePkg/Include/Library/DebugLib.h  |   1 +
 MdePkg/Include/Library/FileHandleLib.h |   1 +
 MdePkg/Include/Library/PrintLib.h  |   4 +
 MdePkg/Include/Library/UefiLib.h   |   7 +
 MdePkg/Include/X64/ProcessorBind.h |  32 +++
 MdePkg/Library/BasePrintLib/PrintLibInternal.h |   1 +
 .../Library/UefiDevicePathLib/DevicePathToText.c   |   1 +
 OvmfPkg/Include/Protocol/XenBus.h  |   2 +-
 OvmfPkg/XenBusDxe/XenStore.h   |   2 +
 SecurityPkg/Tcg/TcgDxe/TpmComm.h   |   1 +
 ShellPkg/Include/Library/ShellLib.h|   1 +
 .../Library/UefiShellCommandLib/ConsistMapping.c   |   1 +
 .../DebugAgent/DebugAgentCommon/DebugAgent.h   |   1 +
 .../CpuExceptionHandlerLib/CpuExceptionCommon.h|   1 +
 Vlv2TbltDevicePkg/Include/FileHandleLib.h  |   1 +
 26 files changed, 672 insertions(+), 5 deletions(-)

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack

[Zeng, Star]

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Saturday, August 8, 2015 12:00 AM
> To: edk2-devel-01
> Cc: Paolo Bonzini; Zeng, Star; Justen, Jordan L
> Subject: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack
> 
> SVN rev 18166 ("MdeModulePkg DxeIpl: Add stack NX support") enables
> platforms to request non-executable stack for the DXE phase, by setting
> PcdSetNxForStack to TRUE.
> 
> The PCD defaults to FALSE, because:
> 
> (a) A non-executable DXE stack is a new feature and causes changes in
> behavior. Some platform could rely on executing code from the stack.
> 
> (b) The code enabling NX in the DXE IPL PEIM enforces the
> 
>   PcdSetNxForStack ==> PcdDxeIplBuildPageTables
> 
> implication for "64-bit PEI + 64-bit DXE" platforms, with a new
> ASSERT(). Some platform might not comply with this requirement
> immediately.
> 
> Regarding (a), in none of the OVMF builds do we try to execute code from
> the stack.
> 
> Regarding (b):
> 
> - In the OvmfPkgX64.dsc build (which is where (b) applies) we simply
>   inherit the PcdDxeIplBuildPageTables|TRUE default from
>   "MdeModulePkg/MdeModulePkg.dec". Therefore we can set
> PcdSetNxForStack
>   to TRUE.
> 
> - In OvmfPkgIa32X64.dsc, page tables are built by default for DXE. Hence
>   we can set PcdSetNxForStack to TRUE.
> 
> - In OvmfPkgIa32.dsc, page tables used not to be necessary until now.
>   After we set PcdSetNxForStack to TRUE in this patch, the DXE IPL will
>   construct page tables even when it is built as part of OvmfPkgIa32.dsc,
>   provided the (virtual) hardware supports both PAE mode and the XD bit.
> 
> Should this setting cause problems in a GPU (or other device) passthru
> scenario, with a UEFI_DRIVER in the PCI option rom attempting to execute
> code from the stack, the feature can be dynamically disabled on the QEMU
> command line, with "-cpu ,-nx".
> 
> Cc: Paolo Bonzini 
> Cc: Jordan Justen 
> Cc: "Zeng, Star" 
> Suggested-by: Paolo Bonzini 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 

Reviewed by: Star Zeng 

> ---
> 
> Notes:
> - This patch depends on Star's
> 
>   [edk2] [PATCH] UefiCpuPkg CpuDxe: Sync up the settings of Execute
>  Disable to APs
>   http://thread.gmane.org/gmane.comp.bios.edk2.devel/960
> 
>   and should be applied only after that patch.
> 
>  OvmfPkg/OvmfPkgIa32.dsc| 1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>  OvmfPkg/OvmfPkgX64.dsc | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index
> 48118cc..38954109 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -303,6 +303,7 @@ [PcdsFeatureFlag]
>  !endif
> 
>  [PcdsFixedAtBuild]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC
> hange|FALSE
>gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 6860ad7..6f6517c 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -308,6 +308,7 @@ [PcdsFeatureFlag]
>  !endif
> 
>  [PcdsFixedAtBuild]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC
> hange|FALSE
>gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index
> f877fda..6b7f955 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -308,6 +308,7 @@ [PcdsFeatureFlag]
>  !endif
> 
>  [PcdsFixedAtBuild]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC
> hange|FALSE
>gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
> --
> 1.8.3.1
> 
> ___
> 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] BaseTools: Fix GCC49 build failure

[Scott Duplichan]

Scott Duplichan [mailto:sc...@notabs.org] wrote:

]Sent: Friday, August 07, 2015 03:16 PM
]To: 'edk2-devel@lists.01.org' 
]Cc: 'Yingke D Liu' ; 'Liming Gao' 

]Subject: [edk2] [PATCH] BaseTools: Fix GCC49 build failure
^

Bad subject line. Should read:

[PATCH] BaseTools: Fix GCC44 build failure

On the other hand, if GCC-4.4 is on its way out then this
patch is unneeded.

Thanks,
Scott


Some gnu linkers used with GCC44, such as GNU ld 2.19.1, require a
--defsym= command line option to precede the --script= option in
order for the definition to be available for use by the script.
Move the --defsym= command line option to satisfy this requirement
and avoid a GCC44 build failure.  

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
---

I found this using a Windows hosted GCC 4.4.7 and Shell build:
build.exe -p d:\edk2build\edk2\ShellPkg\ShellPkg.dsc -b DEBUG -t GCC44 -n 16 -a 
X64
"ld" -o 
d:\edk2build\edk2\Build\Shell\DEBUG_GCC44\X64\ShellPkg\Application\Shell\Shell\DEBUG\Shell.dll
 -nostdlib -n -q --gc-sections -z common-page-size=0x20 --entry 
_ModuleEntryPoint -u _ModuleEntryPoint -Map 
d:\edk2build\edk2\Build\Shell\DEBUG_GCC44\X64\ShellPkg\Application\Shell\Shell\DEBUG/Shell.map
 -melf_x86_64 --oformat=elf64-x86-64 --start-group  
@d:\edk2build\edk2\Build\Shell\DEBUG_GCC44\X64\ShellPkg\Application\Shell\Shell\OUTPUT\static_library_files.lst
 --end-group --script=d:\edk2build\edk2\basetools/Scripts/GccBase.lds 
--defsym=PECOFF_HEADER_SIZE=0x228
d:\edk2build\edk2\basetools/Scripts/GccBase.lds:1: undefined symbol 
`PECOFF_HEADER_SIZE' referenced in expression
---

 BaseTools/Conf/tools_def.template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index eeb488f..806e6e6 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3850,9 +3850,9 @@ DEFINE GCC44_X64_CC_FLAGS= 
DEF(GCC44_ALL_CC_FLAGS) -m64 -fno-stack-p
 DEFINE GCC44_IA32_X64_DLINK_COMMON   = -nostdlib -n -q --gc-sections -z 
common-page-size=0x20
 DEFINE GCC44_IA32_X64_ASLDLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_COMMON) 
--entry ReferenceAcpiTable -u ReferenceAcpiTable
 DEFINE GCC44_IA32_X64_DLINK_FLAGS= DEF(GCC44_IA32_X64_DLINK_COMMON) 
--entry $(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Map 
$(DEST_DIR_DEBUG)/$(BASE_NAME).map
-DEFINE GCC44_IA32_DLINK2_FLAGS   = DEF(GCC_DLINK2_FLAGS_COMMON) 
--defsym=PECOFF_HEADER_SIZE=0x220
+DEFINE GCC44_IA32_DLINK2_FLAGS   = --defsym=PECOFF_HEADER_SIZE=0x220 
DEF(GCC_DLINK2_FLAGS_COMMON)
 DEFINE GCC44_X64_DLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_FLAGS)  
-melf_x86_64 --oformat=elf64-x86-64
-DEFINE GCC44_X64_DLINK2_FLAGS= DEF(GCC_DLINK2_FLAGS_COMMON) 
--defsym=PECOFF_HEADER_SIZE=0x228
+DEFINE GCC44_X64_DLINK2_FLAGS= --defsym=PECOFF_HEADER_SIZE=0x228 
DEF(GCC_DLINK2_FLAGS_COMMON)
 DEFINE GCC44_ASM_FLAGS   = DEF(GCC_ASM_FLAGS)
 
 DEFINE GCC45_IA32_CC_FLAGS   = DEF(GCC44_IA32_CC_FLAGS)

___
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] [edk2} [MinnowBoard] Firmware 0.82 Build error on Windows (VS2008)..?

[Gerard Bucas]

Hi Andrew

 

I started from scratch (deleted all my old versions & exported only the
required version from svn) so I am sure I don't have any mixed versions on
my computer.

 

I would have thought that the scripts would have everything in the right
places.. :(

 

I am using windows to build it (as recommended due to linux compilers
generating 20% larger code which won't fit with full efi shell!), I am not
sure how I can do that grep you show in a windows command line (stupid
question I know!):

 

git grep PcdCpuLockBoxDataAddress -- *.dec

 

?

 

Regards

 

Gerard

 

From: Andrew Fish [mailto:af...@apple.com] 
Sent: Friday, August 7, 2015 4:50 PM
To: gerar...@tekmagic.net
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [edk2} [MinnowBoard] Firmware 0.82 Build error on
Windows (VS2008)..?

 

 

On Aug 7, 2015, at 1:40 PM, Gerard Bucas mailto:gerar...@tekmagic.net> > wrote:

 



Hi guys,



Need some help!



Trying to build latest firmware 0.82 on Windows. Using VS2008 - so normally
pretty simple!



I think I have followed all the steps correctly but I get this error when
building:






Build_IFWI.bat MNW2 Release




build...

c:\uefi\myworkspace\Vlv2BinaryPkg\X64RELEASE\X64\MpCpu.inf(61): error 3000:
PCD

[gEfiVLVTokenSpaceGuid.PcdCpuLockBoxDataAddress] in
[c:\uefi\myworkspace\Vlv2Bin

aryPkg\X64RELEASE\X64\MpCpu.inf] is not found in dependent packages:

   c:\uefi\myworkspace\MdePkg\MdePkg.dec

   c:\uefi\myworkspace\IntelFrameworkPkg\IntelFrameworkPkg.dec

   c:\uefi\myworkspace\IA32FamilyCpuPkg\IA32FamilyCpuPkg.dec

   c:\uefi\myworkspace\Vlv2TbltDevicePkg\PlatformPkg.dec

   c:\uefi\myworkspace\MdeModulePkg\MdeModulePkg.dec


c:\uefi\myworkspace\IntelFrameworkModulePkg\IntelFrameworkModulePkg.dec

   c:\uefi\myworkspace\Vlv2DeviceRefCodePkg\Vlv2DeviceRefCodePkg.dec




 

 

PCD values are declared to exist by a DEC file. As long as the INF list the
DEC file in the Packages section of the INF it should just work?

 

~/work/src/edk2(acpica)>git grep PcdCpuLockBoxDataAddress -- *.dec

Vlv2DeviceRefCodePkg/Vlv2DeviceRefCodePkg.dec:119:
gEfiVLVTokenSpaceGuid.PcdCpuLockBoxDataAddress|0x0|UINT64|0x1211

 

Maybe you have a mixture of packages at different versions? I see it in
master?

 

Thanks,

 

Andrew Fish








- Failed -



Any obvious things I can do!?



Thanks!



Gerard

___
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] [edk2} [MinnowBoard] Firmware 0.82 Build error on Windows (VS2008)..?

[Andrew Fish]

> On Aug 7, 2015, at 1:40 PM, Gerard Bucas  wrote:
> 
> 
> 
> Hi guys,
> 
> 
> 
> Need some help!
> 
> 
> 
> Trying to build latest firmware 0.82 on Windows. Using VS2008 - so normally
> pretty simple!
> 
> 
> 
> I think I have followed all the steps correctly but I get this error when
> building:
> 
> 
> 
>> Build_IFWI.bat MNW2 Release
> 
> 
> 
> build...
> 
> c:\uefi\myworkspace\Vlv2BinaryPkg\X64RELEASE\X64\MpCpu.inf(61): error 3000:
> PCD
> 
> [gEfiVLVTokenSpaceGuid.PcdCpuLockBoxDataAddress] in
> [c:\uefi\myworkspace\Vlv2Bin
> 
> aryPkg\X64RELEASE\X64\MpCpu.inf] is not found in dependent packages:
> 
>c:\uefi\myworkspace\MdePkg\MdePkg.dec
> 
>c:\uefi\myworkspace\IntelFrameworkPkg\IntelFrameworkPkg.dec
> 
>c:\uefi\myworkspace\IA32FamilyCpuPkg\IA32FamilyCpuPkg.dec
> 
>c:\uefi\myworkspace\Vlv2TbltDevicePkg\PlatformPkg.dec
> 
>c:\uefi\myworkspace\MdeModulePkg\MdeModulePkg.dec
> 
> 
> c:\uefi\myworkspace\IntelFrameworkModulePkg\IntelFrameworkModulePkg.dec
> 
>c:\uefi\myworkspace\Vlv2DeviceRefCodePkg\Vlv2DeviceRefCodePkg.dec
> 
> 


PCD values are declared to exist by a DEC file. As long as the INF list the DEC 
file in the Packages section of the INF it should just work?

~/work/src/edk2(acpica)>git grep PcdCpuLockBoxDataAddress -- *.dec
Vlv2DeviceRefCodePkg/Vlv2DeviceRefCodePkg.dec:119:  
gEfiVLVTokenSpaceGuid.PcdCpuLockBoxDataAddress|0x0|UINT64|0x1211

Maybe you have a mixture of packages at different versions? I see it in master?

Thanks,

Andrew Fish

> 
> 
> 
> - Failed -
> 
> 
> 
> Any obvious things I can do!?
> 
> 
> 
> Thanks!
> 
> 
> 
> Gerard
> 
> ___
> 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

[edk2] [edk2} [MinnowBoard] Firmware 0.82 Build error on Windows (VS2008)..?

[Gerard Bucas]

 

Hi guys,

 

Need some help!

 

Trying to build latest firmware 0.82 on Windows. Using VS2008 - so normally
pretty simple!

 

I think I have followed all the steps correctly but I get this error when
building:

 

> Build_IFWI.bat MNW2 Release

 

build...

c:\uefi\myworkspace\Vlv2BinaryPkg\X64RELEASE\X64\MpCpu.inf(61): error 3000:
PCD

[gEfiVLVTokenSpaceGuid.PcdCpuLockBoxDataAddress] in
[c:\uefi\myworkspace\Vlv2Bin

aryPkg\X64RELEASE\X64\MpCpu.inf] is not found in dependent packages:

c:\uefi\myworkspace\MdePkg\MdePkg.dec

c:\uefi\myworkspace\IntelFrameworkPkg\IntelFrameworkPkg.dec

c:\uefi\myworkspace\IA32FamilyCpuPkg\IA32FamilyCpuPkg.dec

c:\uefi\myworkspace\Vlv2TbltDevicePkg\PlatformPkg.dec

c:\uefi\myworkspace\MdeModulePkg\MdeModulePkg.dec

 
c:\uefi\myworkspace\IntelFrameworkModulePkg\IntelFrameworkModulePkg.dec

c:\uefi\myworkspace\Vlv2DeviceRefCodePkg\Vlv2DeviceRefCodePkg.dec

 

 

- Failed -

 

Any obvious things I can do!?

 

Thanks!

 

Gerard

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] BaseTools: Fix GCC49 build failure

[Scott Duplichan]

Some gnu linkers used with GCC44, such as GNU ld 2.19.1, require a
--defsym= command line option to precede the --script= option in
order for the definition to be available for use by the script.
Move the --defsym= command line option to satisfy this requirement
and avoid a GCC44 build failure.  

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Scott Duplichan 
---

I found this using a Windows hosted GCC 4.4.7 and Shell build:
build.exe -p d:\edk2build\edk2\ShellPkg\ShellPkg.dsc -b DEBUG -t GCC44 -n 16 -a 
X64
"ld" -o 
d:\edk2build\edk2\Build\Shell\DEBUG_GCC44\X64\ShellPkg\Application\Shell\Shell\DEBUG\Shell.dll
 -nostdlib -n -q --gc-sections -z common-page-size=0x20 --entry 
_ModuleEntryPoint -u _ModuleEntryPoint -Map 
d:\edk2build\edk2\Build\Shell\DEBUG_GCC44\X64\ShellPkg\Application\Shell\Shell\DEBUG/Shell.map
 -melf_x86_64 --oformat=elf64-x86-64 --start-group  
@d:\edk2build\edk2\Build\Shell\DEBUG_GCC44\X64\ShellPkg\Application\Shell\Shell\OUTPUT\static_library_files.lst
 --end-group --script=d:\edk2build\edk2\basetools/Scripts/GccBase.lds 
--defsym=PECOFF_HEADER_SIZE=0x228
d:\edk2build\edk2\basetools/Scripts/GccBase.lds:1: undefined symbol 
`PECOFF_HEADER_SIZE' referenced in expression
---

 BaseTools/Conf/tools_def.template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index eeb488f..806e6e6 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3850,9 +3850,9 @@ DEFINE GCC44_X64_CC_FLAGS= 
DEF(GCC44_ALL_CC_FLAGS) -m64 -fno-stack-p
 DEFINE GCC44_IA32_X64_DLINK_COMMON   = -nostdlib -n -q --gc-sections -z 
common-page-size=0x20
 DEFINE GCC44_IA32_X64_ASLDLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_COMMON) 
--entry ReferenceAcpiTable -u ReferenceAcpiTable
 DEFINE GCC44_IA32_X64_DLINK_FLAGS= DEF(GCC44_IA32_X64_DLINK_COMMON) 
--entry $(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Map 
$(DEST_DIR_DEBUG)/$(BASE_NAME).map
-DEFINE GCC44_IA32_DLINK2_FLAGS   = DEF(GCC_DLINK2_FLAGS_COMMON) 
--defsym=PECOFF_HEADER_SIZE=0x220
+DEFINE GCC44_IA32_DLINK2_FLAGS   = --defsym=PECOFF_HEADER_SIZE=0x220 
DEF(GCC_DLINK2_FLAGS_COMMON)
 DEFINE GCC44_X64_DLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_FLAGS)  
-melf_x86_64 --oformat=elf64-x86-64
-DEFINE GCC44_X64_DLINK2_FLAGS= DEF(GCC_DLINK2_FLAGS_COMMON) 
--defsym=PECOFF_HEADER_SIZE=0x228
+DEFINE GCC44_X64_DLINK2_FLAGS= --defsym=PECOFF_HEADER_SIZE=0x228 
DEF(GCC_DLINK2_FLAGS_COMMON)
 DEFINE GCC44_ASM_FLAGS   = DEF(GCC_ASM_FLAGS)
 
 DEFINE GCC45_IA32_CC_FLAGS   = DEF(GCC44_IA32_CC_FLAGS)

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg DxeIpl: Add stack NX support

[Laszlo Ersek]

On 08/07/15 19:00, Andrew Fish wrote:
> 
>> On Aug 7, 2015, at 5:21 AM, Laszlo Ersek > > wrote:
>>
>>
>> On 08/07/15 03:27, Zeng, Star wrote:
 -Original Message-
 From: Laszlo Ersek [mailto:ler...@redhat.com]
 Sent: Thursday, August 6, 2015 9:43 PM
 To: Zeng, Star; Paolo Bonzini
 Cc: Andrew Fish; Justen, Jordan L; edk2-de...@ml01.01.org
 ; Yao, Jiewen;
 Chen Fan; Fan, Jeff
 Subject: Re: [edk2] [PATCH] MdeModulePkg DxeIpl: Add stack NX support
>>
> +  if (IsBspExecuteDisableEnabled ()) {
> +CopyMem (
> +  (VOID*) &StartupCode->EnableExecuteDisble,

 (7) No need for the (VOID*) cast.
>>>
>>> With (VOID *) cast, I met build failure below.
>>>
>>> f:\svngit\edk2gitsvn\UefiCpuPkg\CpuDxe\ApStartup.c(412) : error
>>> C2220: warning treated as error - no 'object' file generated
>>> f:\svngit\edk2gitsvn\UefiCpuPkg\CpuDxe\ApStartup.c(412) : warning
>>> C4090: 'function' : different 'volatile' qualifiers
>>> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual
>>> Studio 12.0\Vc\bin\x86_amd64\cl.exe"' : return code '0x2'
>>> Stop.
>>>
>>> I also saw the cast at  CopyMem ((VOID*) StartupCode,
>>> &mStartupCodeTemplate, sizeof (*StartupCode));.
>>
>> Ah, yes. I missed that the cast was casting away volatile.
>>
>> Strictly speaking, that's undefined behavior in ISO C (and it's also why
>> the compiler warns about it). But, since this pattern exists already, I
>> guess we can stick with it.
>>
> 
> FYI clang is very aggressive about optimizing away undefined behavior. 
> http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html
> 
> I’ve updated versions of the compiler and all of a sudden undefined
> behavior gets optimized away. So code that works before breaks. 

I agree completely, but I'm powerless against the edk2 coding style.

If one has a structure declared volatile, it can be zeroed in only four
ways (in practice), according to ISO C:

- Use a zero initializer (= { 0 }). Not allowed in edk2.
- Use a (volatile UINT8 *) pointer, and fill the struct byte-wise.
  Would be slow and would draw stares.
- Assign 0 to each field (down to scalars, recursively), individually.
  Would draw stares.
- ZeroMem() a non-volatile struct of the same type, and use structure
  assignment. Not allowed in edk2.

I can only name SVN rev 13878 again. Before that commit, I tried #1 (it
caused a build failure), deemed #2 and #3 ugly, therefore I wrote #4.

In SVN rev 13878 my correct code (#4) was replaced with undefined
behavior. What can I do.

In the case at hand, as long as volatile is indeed necessary, CopyMem()
should not be used. Instead,

  *StartupCode = mStartupCodeTemplate;

should be written.

We can't satisfy "clang is super aggressive about optimizing away
undefined behavior" and "MSVC / gcc-4.4 / blah are too stupid to assign
structures" at the same time. (Unless we're willing to busy-work on
option #2 or option #3.)

Thanks
Laszlo

> 
> Thanks,
> 
> Andrew Fish
> 
>>
>>>

> +  &mEnableExecuteDisbleCodeTemplate,
> +  sizeof (ENABLE_EXECUTE_DISABLE_CODE)
> +  );
> +  }
> #if defined (MDE_CPU_X64)
>   StartupCode->Cr3Value = (UINT32) AsmReadCr3 ();
>   StartupCode->LongJmpOffset += (UINT32) StartAddress;
> +#else
> +  StartupCode->EnableExecuteDisble.Cr3Value = (UINT32) AsmReadCr3 ();

 (8) I think the (UINT32) cast can be dropped in this branch, but
 it's not
 important.
>>>
>>> Prefer to keep it as Cr3Value is defined as UINT32 although UINTN ==
>>> UINT32 at IA32 build.
>>
>> Okay.
>>
>> Thanks!
>> Laszlo
>> ___
>> 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 v3 7/7] BaseTools: add support for CLANG compiler to GCC family

[Ard Biesheuvel]

On 7 August 2015 at 18:54, Jordan Justen  wrote:
> On 2015-08-07 07:59:24, Ard Biesheuvel wrote:
>> On 7 August 2015 at 16:51, Jordan Justen  wrote:
>> > I think the subject should say 'Add CLANG toolchain with AARCH64
>> > support' to highlight that a new toolchain name is being defined.
>> >
>>
>> OK. I can fix that up before applying.
>>
>> > Should we consider adding the version to the toolchain name, like
>> > CLANG37? Then we can change parameters between major releases. At
>> > least for GCC it has proved useful.
>> >
>>
>> Actually, I was going to suggest folding everything except GCC-4.4
>> into GCC4X since the differences are so minor.
>
> Minor, but there are differences. Would you propose forcing people to
> have to cusomize tools_def if they don't use the GCC-4.X that
> tools_def currently supports? How would that work?
>

No, not at all. As far as I can tell (and I sent you the RFC series to
illustrate it), we can support GCC-4.5 - 4.9 with the exact same
options.
The only difference that remains after my series is applied is the
presence of --32/--64 in the CC flags, which can be replaced with
-Wa,--32 / -Wa,--64 respectively, which works on all GCC versions.

>> (and GCC-4.4 may be on its way out due to its poor support of
>> ms_abi).
>
> Arg. Not that tired old argument about how difficult it is to use
> EFIAPI properly? :)
>

OK, Sorry I brought that up. I never had any issues with it myself, I
just thought based on the recent discussions that it was indeed on its
way out.

> By the way, we have never deprecated a toolchain in EDK II. Personally
> I think we should move some older toolchains to
> BaseTools/Conf/tools_def.deprecated.
>

Well, I wouldn't necessarily suggest deprecating the GCC45 GCC46 etc
names, but having them resolve to the same values under the hood at
least removes some of the maintenance overhead.

>> With GCC 5.0 and 5.1 around the corner, it is becoming a bit
>> unwieldy by my taste.
>
> There would be a GCC5 and GCC6. Not GCC50 and GCC51. (GCC moved
> releases to bumping the first version number.)
>
> https://gcc.gnu.org/develop.html#num_scheme
>

Ugh. So we will now have GCC5 GCC6 etc regardless of whether anything
actually requires it?

> This method does add 1~2 toolchains per year, but it doesn't seem too
> bad.
>
> CLANG seems to have releases about 1~2 per year as well, and it would
> be good to plan for some minor parameter tweaks between releases.
> That's why I'd suggest CLANG37, to match up with CLANG/LLVM release
> numbering.
>

It would be CLANG35, in fact, since that is what I have been using,
and I deliberately used a mature release.

I don't mind using CLANG35 though, as long as we have a way for this
not to get out of hand. If we end up adding CLANG36 CLANG37 ad
infinitum with no changes in what they resolve to, and no other
motivation than tracking the upstream numbering scheme, I seriously
think we are doing something wrong.

-- 
Ard.


>> For clang, there is no reason right now afaict. So perhaps we could
>> use CLANG3X instead? That way, we can still keep the current support
>> as newer CLANG versions become available, but are not forced to
>> declare 2 or 3 discrete but identical versions right from the start.
>>
>> --
>> Ard.
>>
>> > On 2015-08-07 07:23:23, Ard Biesheuvel wrote:
>> >> This adds support for building the AARCH64 platforms using the
>> >> Clang compiler and assembler combined with the GNU (cross-)linker.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.0
>> >> Signed-off-by: Ard Biesheuvel 
>> >> Reviewed-by: Leif Lindholm 
>> >> Tested-by: Leif Lindholm 
>> >> ---
>> >>  BaseTools/Conf/tools_def.template | 51 
>> >>  1 file changed, 51 insertions(+)
>> >>
>> >> diff --git a/BaseTools/Conf/tools_def.template 
>> >> b/BaseTools/Conf/tools_def.template
>> >> index eeb488fb3597..974c9805435b 100644
>> >> --- a/BaseTools/Conf/tools_def.template
>> >> +++ b/BaseTools/Conf/tools_def.template
>> >> @@ -4644,6 +4644,57 @@ RELEASE_GCC49_AARCH64_CC_FLAGS   = 
>> >> DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-s
>> >>
>> >>  
>> >> 
>> >>  #
>> >> +# CLANG   - This configuration is used to compile under Linux to produce
>> >> +#   PE/COFF binaries using the clang compiler and assembler and 
>> >> GNU linker
>> >> +#
>> >> +
>> >> +*_CLANG_*_*_FAMILY   = GCC
>> >> +
>> >> +*_CLANG_*_MAKE_PATH  = make
>> >> +*_CLANG_*_*_DLL  = ENV(CLANG_DLL)
>> >> +*_CLANG_*_ASL_PATH   = DEF(UNIX_IASL_BIN)
>> >> +
>> >> +*_CLANG_*_PP_FLAGS   = DEF(GCC_PP_FLAGS)
>> >> +*_CLANG_*_ASLPP_FLAGS= DEF(GCC_ASLPP_FLAGS)
>> >> +*_CLANG_*_ASLCC_FLAGS= DEF(GCC_ASLCC_FLAGS)
>> >> +*_CLANG_*_VFRPP_FLAGS= DEF(GCC_VFRPP_FLAGS)
>> >> +*_CLANG_*_APP_FLAGS  =
>

Re: [edk2] [PATCH] MdeModulePkg DxeIpl: Add stack NX support

[Andrew Fish]

> On Aug 7, 2015, at 5:21 AM, Laszlo Ersek  wrote:
> 
> 
> On 08/07/15 03:27, Zeng, Star wrote:
>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Thursday, August 6, 2015 9:43 PM
>>> To: Zeng, Star; Paolo Bonzini
>>> Cc: Andrew Fish; Justen, Jordan L; edk2-de...@ml01.01.org; Yao, Jiewen;
>>> Chen Fan; Fan, Jeff
>>> Subject: Re: [edk2] [PATCH] MdeModulePkg DxeIpl: Add stack NX support
> 
 +  if (IsBspExecuteDisableEnabled ()) {
 +CopyMem (
 +  (VOID*) &StartupCode->EnableExecuteDisble,
>>> 
>>> (7) No need for the (VOID*) cast.
>> 
>> With (VOID *) cast, I met build failure below.
>> 
>> f:\svngit\edk2gitsvn\UefiCpuPkg\CpuDxe\ApStartup.c(412) : error C2220: 
>> warning treated as error - no 'object' file generated
>> f:\svngit\edk2gitsvn\UefiCpuPkg\CpuDxe\ApStartup.c(412) : warning C4090: 
>> 'function' : different 'volatile' qualifiers
>> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 
>> 12.0\Vc\bin\x86_amd64\cl.exe"' : return code '0x2'
>> Stop.
>> 
>> I also saw the cast at  CopyMem ((VOID*) StartupCode, &mStartupCodeTemplate, 
>> sizeof (*StartupCode));.
> 
> Ah, yes. I missed that the cast was casting away volatile.
> 
> Strictly speaking, that's undefined behavior in ISO C (and it's also why
> the compiler warns about it). But, since this pattern exists already, I
> guess we can stick with it.
> 

FYI clang is very aggressive about optimizing away undefined behavior. 
http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html 


I’ve updated versions of the compiler and all of a sudden undefined behavior 
gets optimized away. So code that works before breaks. 

Thanks,

Andrew Fish

> 
>> 
>>> 
 +  &mEnableExecuteDisbleCodeTemplate,
 +  sizeof (ENABLE_EXECUTE_DISABLE_CODE)
 +  );
 +  }
 #if defined (MDE_CPU_X64)
   StartupCode->Cr3Value = (UINT32) AsmReadCr3 ();
   StartupCode->LongJmpOffset += (UINT32) StartAddress;
 +#else
 +  StartupCode->EnableExecuteDisble.Cr3Value = (UINT32) AsmReadCr3 ();
>>> 
>>> (8) I think the (UINT32) cast can be dropped in this branch, but it's not
>>> important.
>> 
>> Prefer to keep it as Cr3Value is defined as UINT32 although UINTN == UINT32 
>> at IA32 build.
> 
> Okay.
> 
> Thanks!
> Laszlo
> ___
> 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 0/2] ArmPlatformPkg: reduce .data contents of XIP modules

[Leif Lindholm]

On Fri, Jul 24, 2015 at 02:38:05PM +0200, Ard Biesheuvel wrote:
> SEC and PEI modules may be executed in place, in which case their .data
> sections are read-only. So declare some globals as CONST, so that they
> are emitted into .text instead. This may result in no .data section being
> emitted at all, saving substantial FV space if section alignment is high.
> 
> It should also help catch inadvertent modifications of these variables,
> since that may not be possible if the module is executed in place.
> 
> Note that this series applies on top of 'BaseTools/GCC: move AutoGen.obj
> contents to .text section' that is part of the series 'BaseTools: unify all
> GCC linker scripts', of which I sent out a v2 today. I.e., without moving
> all AutoGen.obj contents to .text, these changes won't make a lot of
> difference.
> 
> Ard Biesheuvel (2):
>   ArmPlatformPkg/PrePeiCore: constify PPI globals
>   ArmPlatformPkg/PlatformPeim: constify EFI_PEI_PPI_DESCRIPTOR globals
> 
>  ArmPlatformPkg/PlatformPei/PlatformPeim.c |  4 ++--
>  ArmPlatformPkg/PrePeiCore/PrePeiCore.c| 10 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)

Looks good to me.

Reviewed-by: Leif Lindholm 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 7/7] BaseTools: add support for CLANG compiler to GCC family

[Jordan Justen]

On 2015-08-07 07:59:24, Ard Biesheuvel wrote:
> On 7 August 2015 at 16:51, Jordan Justen  wrote:
> > I think the subject should say 'Add CLANG toolchain with AARCH64
> > support' to highlight that a new toolchain name is being defined.
> >
> 
> OK. I can fix that up before applying.
> 
> > Should we consider adding the version to the toolchain name, like
> > CLANG37? Then we can change parameters between major releases. At
> > least for GCC it has proved useful.
> >
> 
> Actually, I was going to suggest folding everything except GCC-4.4
> into GCC4X since the differences are so minor.

Minor, but there are differences. Would you propose forcing people to
have to cusomize tools_def if they don't use the GCC-4.X that
tools_def currently supports? How would that work?

> (and GCC-4.4 may be on its way out due to its poor support of
> ms_abi).

Arg. Not that tired old argument about how difficult it is to use
EFIAPI properly? :)

By the way, we have never deprecated a toolchain in EDK II. Personally
I think we should move some older toolchains to
BaseTools/Conf/tools_def.deprecated.

> With GCC 5.0 and 5.1 around the corner, it is becoming a bit
> unwieldy by my taste.

There would be a GCC5 and GCC6. Not GCC50 and GCC51. (GCC moved
releases to bumping the first version number.)

https://gcc.gnu.org/develop.html#num_scheme

This method does add 1~2 toolchains per year, but it doesn't seem too
bad.

CLANG seems to have releases about 1~2 per year as well, and it would
be good to plan for some minor parameter tweaks between releases.
That's why I'd suggest CLANG37, to match up with CLANG/LLVM release
numbering.

-Jordan

> For clang, there is no reason right now afaict. So perhaps we could
> use CLANG3X instead? That way, we can still keep the current support
> as newer CLANG versions become available, but are not forced to
> declare 2 or 3 discrete but identical versions right from the start.
> 
> -- 
> Ard.
> 
> > On 2015-08-07 07:23:23, Ard Biesheuvel wrote:
> >> This adds support for building the AARCH64 platforms using the
> >> Clang compiler and assembler combined with the GNU (cross-)linker.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel 
> >> Reviewed-by: Leif Lindholm 
> >> Tested-by: Leif Lindholm 
> >> ---
> >>  BaseTools/Conf/tools_def.template | 51 
> >>  1 file changed, 51 insertions(+)
> >>
> >> diff --git a/BaseTools/Conf/tools_def.template 
> >> b/BaseTools/Conf/tools_def.template
> >> index eeb488fb3597..974c9805435b 100644
> >> --- a/BaseTools/Conf/tools_def.template
> >> +++ b/BaseTools/Conf/tools_def.template
> >> @@ -4644,6 +4644,57 @@ RELEASE_GCC49_AARCH64_CC_FLAGS   = 
> >> DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-s
> >>
> >>  
> >> 
> >>  #
> >> +# CLANG   - This configuration is used to compile under Linux to produce
> >> +#   PE/COFF binaries using the clang compiler and assembler and 
> >> GNU linker
> >> +#
> >> +
> >> +*_CLANG_*_*_FAMILY   = GCC
> >> +
> >> +*_CLANG_*_MAKE_PATH  = make
> >> +*_CLANG_*_*_DLL  = ENV(CLANG_DLL)
> >> +*_CLANG_*_ASL_PATH   = DEF(UNIX_IASL_BIN)
> >> +
> >> +*_CLANG_*_PP_FLAGS   = DEF(GCC_PP_FLAGS)
> >> +*_CLANG_*_ASLPP_FLAGS= DEF(GCC_ASLPP_FLAGS)
> >> +*_CLANG_*_ASLCC_FLAGS= DEF(GCC_ASLCC_FLAGS)
> >> +*_CLANG_*_VFRPP_FLAGS= DEF(GCC_VFRPP_FLAGS)
> >> +*_CLANG_*_APP_FLAGS  =
> >> +*_CLANG_*_ASL_FLAGS  = DEF(IASL_FLAGS)
> >> +*_CLANG_*_ASL_OUTFLAGS   = DEF(IASL_OUTFLAGS)
> >> +
> >> +*_CLANG_*_CC_PATH= ENV(CLANG_BIN)clang
> >> +*_CLANG_*_ASM_PATH   = ENV(CLANG_BIN)clang
> >> +*_CLANG_*_PP_PATH= ENV(CLANG_BIN)clang
> >> +*_CLANG_*_VFRPP_PATH = ENV(CLANG_BIN)clang
> >> +*_CLANG_*_ASLCC_PATH = ENV(CLANG_BIN)clang
> >> +*_CLANG_*_ASLPP_PATH = ENV(CLANG_BIN)clang
> >> +
> >> +DEFINE CLANG_WARNING_OVERRIDES   = -Wno-parentheses-equality 
> >> -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare 
> >> -Wno-empty-body
> >> +DEFINE CLANG_AARCH64_CC_FLAGS= DEF(GCC_AARCH64_CC_FLAGS) -target 
> >> aarch64 -mcmodel=small -mstrict-align DEF(CLANG_WARNING_OVERRIDES)
> >> +
> >> +##
> >> +# CLANG AARCH64 definitions
> >> +##
> >> +*_CLANG_AARCH64_SLINK_PATH   = ENV(CLANG_AARCH64_PREFIX)ar
> >> +*_CLANG_AARCH64_DLINK_PATH   = ENV(CLANG_AARCH64_PREFIX)ld
> >> +*_CLANG_AARCH64_ASLDLINK_PATH= ENV(CLANG_AARCH64_PREFIX)ld
> >> +*_CLANG_AARCH64_RC_PATH  = ENV(CLANG_AARCH64_PREFIX)objcopy
> >> +
> >> +*_CLANG_AARCH64_ASLCC_FLAGS  = DEF(GCC_ASLCC_FLAGS)
> >> +*_CLANG_AARCH64_ASLDLINK_FLAGS   = DEF(GCC_AARCH64_ASLDLINK_FLAGS)
> >> +*_CLANG_AARCH64_

[edk2] [PATCH 2/3] ArmPlatformPkg/ArmJunoDxe: drop sky2.mac_address kernel argument

[Ard Biesheuvel]

When detecting Juno rev1 hardware, we add a sky2.mac_address=xx:xx:...
kernel command line parameter, in an apparent attempt to override the
MAC address that the Linux driver probes from the hardware. However,
the Linux driver does not in fact take a 'mac_address' parameter, so
there is no point in doing this. So drop it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 28 
++--
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index 756c2751e160..c82657aef718 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -31,13 +31,6 @@
 #include 
 
 //
-// Size in number of characters of the Linux boot argument
-// passing the MAC address to be used by the PCI GigaByte
-// Ethernet device : " sky2.mac_address=0x11,0x22,0x33,0x44,0x55,0x66"
-//
-#define SKY2_MAC_ADDRESS_BOOTARG_LEN  47
-
-//
 // Hardware platform identifiers
 //
 typedef enum {
@@ -416,9 +409,6 @@ SetJunoR1DefaultBootEntries (
   UINTN   Size;
   EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
   EFI_DEVICE_PATH*BootDevicePath;
-  UINT32  SysPciGbeL;
-  UINT32  SysPciGbeH;
-  CHAR16* DefaultBootArgument;
   CHAR16* DefaultBootArgument1;
   UINTN   DefaultBootArgument1Size;
   CHAR16* DefaultBootArgument2;
@@ -463,9 +453,8 @@ SetJunoR1DefaultBootEntries (
 return EFI_UNSUPPORTED;
   }
 
-  DefaultBootArgument = (CHAR16*)PcdGetPtr (PcdDefaultBootArgument);
-  DefaultBootArgument1Size = StrSize (DefaultBootArgument) +
- (SKY2_MAC_ADDRESS_BOOTARG_LEN * sizeof (CHAR16));
+  DefaultBootArgument1 = (CHAR16*)PcdGetPtr (PcdDefaultBootArgument);
+  DefaultBootArgument1Size = StrSize (DefaultBootArgument1);
   DefaultBootArgument2Size = DefaultBootArgument1Size + StrSize 
(ExtraBootArgument);
 
   Status = EFI_OUT_OF_RESOURCES;
@@ -478,19 +467,6 @@ SetJunoR1DefaultBootEntries (
 goto Error;
   }
 
-  SysPciGbeL = MmioRead32 (ARM_JUNO_SYS_PCIGBE_L);
-  SysPciGbeH = MmioRead32 (ARM_JUNO_SYS_PCIGBE_H);
-
-  UnicodeSPrint (
-DefaultBootArgument1,
-DefaultBootArgument1Size,
-L"%s sky2.mac_address=0x%02x,0x%02x,0x%02x,0x%02x,0x%02x,0x%02x",
-DefaultBootArgument,
-(SysPciGbeH >> 8 ) & 0xFF, (SysPciGbeH  ) & 0xFF,
-(SysPciGbeL >> 24) & 0xFF, (SysPciGbeL >> 16) & 0xFF,
-(SysPciGbeL >> 8 ) & 0xFF, (SysPciGbeL  ) & 0xFF
-);
-
   CopyMem (DefaultBootArgument2, DefaultBootArgument1, 
DefaultBootArgument1Size);
   CopyMem (
 (UINT8*)DefaultBootArgument2 + DefaultBootArgument1Size - sizeof (CHAR16),
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 3/3] ArmPlatformPkg/ArmJunoDxe: use single DTB for Juno rev1 boot options

[Ard Biesheuvel]

The ArmJunoDxe driver installs two default boot options:
- Boot0001, that exposes only the 2xA57 cluster
- Boot0002, that exposes both the 2xA57 and 4xA53 clusters.

This is implemented by overriding the DTB passed via the config table
with a dtb= kernel command line, which requires that a DTB file by the exact
name 'r1a57a53.dtb' is present in the same volume that hosts the kernel Image.

So instead, define only a single Juno rev1 FDT, and implement the Boot0001
option by passing maxcpus=2 on the kernel command line. This is also more
flexible, since the A53 cluster may still be onlined manually through sysfs.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec   |  3 +--
 ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c   | 16 

 ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf |  3 +--
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec 
b/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec
index 040a906ac1d2..834305c06dd7 100644
--- a/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec
+++ b/ArmPlatformPkg/ArmJunoPkg/ArmJuno.dec
@@ -45,5 +45,4 @@ [PcdsFixedAtBuild.common]
 
   # Juno Device Trees are loaded from NOR Flash
   
gArmJunoTokenSpaceGuid.PcdJunoR0FdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/juno.dtb"|VOID*|0x0006
-  
gArmJunoTokenSpaceGuid.PcdJunoR1A57x2FdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/r1a57.dtb"|VOID*|0x0007
-  
gArmJunoTokenSpaceGuid.PcdJunoR1A57x2A53x4FdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/r1a57a53.dtb"|VOID*|0x0008
+  
gArmJunoTokenSpaceGuid.PcdJunoR1FdtDevicePath|L"VenHw(E7223039-5836-41E1-B542-D7EC736C5E59)/r1a57a53.dtb"|VOID*|0x0007
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index c82657aef718..e63b88f2eb6b 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -361,7 +361,7 @@ ArmJunoEntryPoint (
 break;
 
   case JUNO_R1:
-TextDevicePath = (CHAR16*)FixedPcdGetPtr (PcdJunoR1A57x2FdtDevicePath);
+TextDevicePath = (CHAR16*)FixedPcdGetPtr (PcdJunoR1FdtDevicePath);
 break;
 
   default:
@@ -405,7 +405,7 @@ SetJunoR1DefaultBootEntries (
   )
 {
   EFI_STATUS  Status;
-  CONST CHAR16*   ExtraBootArgument = L" dtb=r1a57a53.dtb";
+  CONST CHAR16ExtraBootArgument[] = L" maxcpus=2";
   UINTN   Size;
   EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
   EFI_DEVICE_PATH*BootDevicePath;
@@ -453,9 +453,9 @@ SetJunoR1DefaultBootEntries (
 return EFI_UNSUPPORTED;
   }
 
-  DefaultBootArgument1 = (CHAR16*)PcdGetPtr (PcdDefaultBootArgument);
-  DefaultBootArgument1Size = StrSize (DefaultBootArgument1);
-  DefaultBootArgument2Size = DefaultBootArgument1Size + StrSize 
(ExtraBootArgument);
+  DefaultBootArgument2 = (CHAR16*)PcdGetPtr (PcdDefaultBootArgument);
+  DefaultBootArgument2Size = StrSize (DefaultBootArgument2);
+  DefaultBootArgument1Size = DefaultBootArgument2Size + sizeof 
(ExtraBootArgument);
 
   Status = EFI_OUT_OF_RESOURCES;
   DefaultBootArgument1 = AllocatePool (DefaultBootArgument1Size);
@@ -467,11 +467,11 @@ SetJunoR1DefaultBootEntries (
 goto Error;
   }
 
-  CopyMem (DefaultBootArgument2, DefaultBootArgument1, 
DefaultBootArgument1Size);
+  CopyMem (DefaultBootArgument1, DefaultBootArgument2, 
DefaultBootArgument2Size);
   CopyMem (
-(UINT8*)DefaultBootArgument2 + DefaultBootArgument1Size - sizeof (CHAR16),
+(UINT8*)DefaultBootArgument1 + DefaultBootArgument2Size - sizeof (CHAR16),
 ExtraBootArgument,
-StrSize (ExtraBootArgument)
+sizeof (ExtraBootArgument)
   );
 
   //
diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
index e0e62fae108d..754f950ee4e9 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
@@ -74,8 +74,7 @@ [FixedPcd]
   gArmJunoTokenSpaceGuid.PcdSynopsysUsbOhciBaseAddress
 
   gArmJunoTokenSpaceGuid.PcdJunoR0FdtDevicePath
-  gArmJunoTokenSpaceGuid.PcdJunoR1A57x2FdtDevicePath
-  gArmJunoTokenSpaceGuid.PcdJunoR1A57x2A53x4FdtDevicePath
+  gArmJunoTokenSpaceGuid.PcdJunoR1FdtDevicePath
 
   gArmPlatformTokenSpaceGuid.PcdDefaultBootDevicePath
   gArmPlatformTokenSpaceGuid.PcdDefaultBootArgument
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 1/3] ArmPlatformPkg/ArmJunoDxe: drop unused BdsLib dependency

[Ard Biesheuvel]

The ArmJunoDxe driver declares a dependency on BdsLib, but never
actually uses anything it provides. So drop it from the .inf

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
index 6157f9807faf..e0e62fae108d 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
@@ -38,7 +38,6 @@ [LibraryClasses]
   ArmLib
   ArmShellCmdRunAxfLib
   BaseMemoryLib
-  BdsLib
   DebugLib
   DmaLib
   DxeServicesTableLib
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack

[Laszlo Ersek]

SVN rev 18166 ("MdeModulePkg DxeIpl: Add stack NX support") enables
platforms to request non-executable stack for the DXE phase, by setting
PcdSetNxForStack to TRUE.

The PCD defaults to FALSE, because:

(a) A non-executable DXE stack is a new feature and causes changes in
behavior. Some platform could rely on executing code from the stack.

(b) The code enabling NX in the DXE IPL PEIM enforces the

  PcdSetNxForStack ==> PcdDxeIplBuildPageTables

implication for "64-bit PEI + 64-bit DXE" platforms, with a new
ASSERT(). Some platform might not comply with this requirement
immediately.

Regarding (a), in none of the OVMF builds do we try to execute code from
the stack.

Regarding (b):

- In the OvmfPkgX64.dsc build (which is where (b) applies) we simply
  inherit the PcdDxeIplBuildPageTables|TRUE default from
  "MdeModulePkg/MdeModulePkg.dec". Therefore we can set PcdSetNxForStack
  to TRUE.

- In OvmfPkgIa32X64.dsc, page tables are built by default for DXE. Hence
  we can set PcdSetNxForStack to TRUE.

- In OvmfPkgIa32.dsc, page tables used not to be necessary until now.
  After we set PcdSetNxForStack to TRUE in this patch, the DXE IPL will
  construct page tables even when it is built as part of OvmfPkgIa32.dsc,
  provided the (virtual) hardware supports both PAE mode and the XD bit.

Should this setting cause problems in a GPU (or other device) passthru
scenario, with a UEFI_DRIVER in the PCI option rom attempting to execute
code from the stack, the feature can be dynamically disabled on the QEMU
command line, with "-cpu ,-nx".

Cc: Paolo Bonzini 
Cc: Jordan Justen 
Cc: "Zeng, Star" 
Suggested-by: Paolo Bonzini 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---

Notes:
- This patch depends on Star's

  [edk2] [PATCH] UefiCpuPkg CpuDxe: Sync up the settings of Execute
 Disable to APs
  http://thread.gmane.org/gmane.comp.bios.edk2.devel/960

  and should be applied only after that patch.

 OvmfPkg/OvmfPkgIa32.dsc| 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
 OvmfPkg/OvmfPkgX64.dsc | 1 +
 3 files changed, 3 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 48118cc..38954109 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -303,6 +303,7 @@ [PcdsFeatureFlag]
 !endif
 
 [PcdsFixedAtBuild]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
   gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 6860ad7..6f6517c 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -308,6 +308,7 @@ [PcdsFeatureFlag]
 !endif
 
 [PcdsFixedAtBuild]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
   gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index f877fda..6b7f955 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -308,6 +308,7 @@ [PcdsFeatureFlag]
 !endif
 
 [PcdsFixedAtBuild]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
   gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
-- 
1.8.3.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC PATCH] ArmPlatformPkg/PlatformIntelBdsLib: remove ARM BDS dependency

[Laszlo Ersek]

one remark below

On 08/07/15 14:11, Ard Biesheuvel wrote:
> The Intel BDS platform library still depends on the ARM BDS specific
> BdsLib. So replace its invocations with GenericBdsLib counterparts,
> and fix up where neeeded.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  .../Library/PlatformIntelBdsLib/IntelBdsPlatform.c   | 20 
> ++--
>  .../Library/PlatformIntelBdsLib/IntelBdsPlatform.h   |  1 -
>  .../PlatformIntelBdsLib/PlatformIntelBdsLib.inf  |  1 -
>  3 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c 
> b/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
> index c82f27fb4edd..1949365f7d09 100644
> --- a/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
> +++ b/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
> @@ -63,8 +63,10 @@ GetConsoleDevicePathFromVariable (
>CHAR16*   NextDevicePathStr;
>EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
>  
> -  Status = GetGlobalEnvironmentVariable (ConsoleVarName, NULL, NULL, 
> (VOID**)&DevicePathInstances);
> -  if (EFI_ERROR(Status)) {
> +  Size = 0;
> +
> +  DevicePathInstances = BdsLibGetVariableAndSize (ConsoleVarName, 
> &gEfiGlobalVariableGuid, &Size);
> +  if (DevicePathInstances == NULL) {
>  // In case no default console device path has been defined we assume a 
> driver handles the console (eg: SimpleTextInOutSerial)
>  if ((DefaultConsolePaths == NULL) || (DefaultConsolePaths[0] == L'\0')) {
>*DevicePaths = NULL;
> @@ -74,8 +76,6 @@ GetConsoleDevicePathFromVariable (
>  Status = gBS->LocateProtocol (&gEfiDevicePathFromTextProtocolGuid, NULL, 
> (VOID **)&EfiDevicePathFromTextProtocol);
>  ASSERT_EFI_ERROR(Status);
>  
> -DevicePathInstances = NULL;
> -
>  // Extract the Device Path instances from the multi-device path string
>  while ((DefaultConsolePaths != NULL) && (DefaultConsolePaths[0] != 
> L'\0')) {
>NextDevicePathStr = StrStr (DefaultConsolePaths, L";");
> @@ -141,7 +141,15 @@ InitializeConsolePipe (
>while (ConsoleDevicePaths != NULL) {
>  DevicePath = GetNextDevicePathInstance (&ConsoleDevicePaths, &Size);
>  
> -Status = BdsConnectDevicePath (DevicePath, Handle, NULL);
> +Status = BdsLibConnectDevicePath (DevicePath);
> +if (!EFI_ERROR (Status)) {
> +  //
> +  // If BdsLibConnectDevicePath () succeeded, *Handle must have a 
> non-NULL
> +  // value. So ASSERT that this is the case.
> +  //
> +  gBS->LocateDevicePath (&gEfiDevicePathProtocolGuid, &DevicePath, 
> Handle);
> +  ASSERT (*Handle != NULL);
> +}

According to the UEFI spec,

If the handle for DevicePath supports the protocol (a direct
match), the resulting device path is advanced to the device path
terminator node.

I believe this could also be asserted here, but it's certainly not
necessary (even if I'm correct).

>  DEBUG_CODE_BEGIN();
>if (EFI_ERROR(Status)) {
>  // We convert back to the text representation of the device Path
> @@ -171,7 +179,7 @@ InitializeConsolePipe (
>if (*Interface == NULL) {
>  Status = gBS->LocateHandleBuffer (ByProtocol, Protocol, NULL, 
> &NoHandles, &Buffer);
>  if (EFI_ERROR (Status)) {
> -  BdsConnectAllDrivers ();
> +  BdsLibConnectAll ();
>Status = gBS->LocateHandleBuffer (ByProtocol, Protocol, NULL, 
> &NoHandles, &Buffer);
>  }
>  
> diff --git a/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.h 
> b/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.h
> index a244ac913255..7122d58be7d7 100644
> --- a/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.h
> +++ b/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.h
> @@ -19,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #include 
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git 
> a/ArmPlatformPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf 
> b/ArmPlatformPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
> index 07de4cae4824..d47298d01a81 100644
> --- a/ArmPlatformPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
> +++ b/ArmPlatformPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
> @@ -44,7 +44,6 @@ [Packages]
>  [LibraryClasses]
>BaseLib
>BaseMemoryLib
> -  BdsLib
>DebugLib
>DevicePathLib
>MemoryAllocationLib
> 

Reviewed-by: Laszlo Ersek 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 7/7] BaseTools: add support for CLANG compiler to GCC family

[Ard Biesheuvel]

On 7 August 2015 at 16:59, Ard Biesheuvel  wrote:
> On 7 August 2015 at 16:51, Jordan Justen  wrote:
>> I think the subject should say 'Add CLANG toolchain with AARCH64
>> support' to highlight that a new toolchain name is being defined.
>>
>
> OK. I can fix that up before applying.
>
>> Should we consider adding the version to the toolchain name, like
>> CLANG37? Then we can change parameters between major releases. At
>> least for GCC it has proved useful.
>>
>
> Actually, I was going to suggest folding everything except GCC-4.4
> into GCC4X since the differences are so minor. (and GCC-4.4 may be on
> its way out due to its poor support of ms_abi). With GCC 5.0 and 5.1
> around the corner, it is becoming a bit unwieldy by my taste.
>
> For clang, there is no reason right now afaict. So perhaps we could
> use CLANG3X instead? That way, we can still keep the current support
> as newer CLANG versions become available, but are not forced to
> declare 2 or 3 discrete but identical versions right from the start.
>

... or simply CLANG3

-- 
Ard.

>> On 2015-08-07 07:23:23, Ard Biesheuvel wrote:
>>> This adds support for building the AARCH64 platforms using the
>>> Clang compiler and assembler combined with the GNU (cross-)linker.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel 
>>> Reviewed-by: Leif Lindholm 
>>> Tested-by: Leif Lindholm 
>>> ---
>>>  BaseTools/Conf/tools_def.template | 51 
>>>  1 file changed, 51 insertions(+)
>>>
>>> diff --git a/BaseTools/Conf/tools_def.template 
>>> b/BaseTools/Conf/tools_def.template
>>> index eeb488fb3597..974c9805435b 100644
>>> --- a/BaseTools/Conf/tools_def.template
>>> +++ b/BaseTools/Conf/tools_def.template
>>> @@ -4644,6 +4644,57 @@ RELEASE_GCC49_AARCH64_CC_FLAGS   = 
>>> DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-s
>>>
>>>  
>>> 
>>>  #
>>> +# CLANG   - This configuration is used to compile under Linux to produce
>>> +#   PE/COFF binaries using the clang compiler and assembler and 
>>> GNU linker
>>> +#
>>> +
>>> +*_CLANG_*_*_FAMILY   = GCC
>>> +
>>> +*_CLANG_*_MAKE_PATH  = make
>>> +*_CLANG_*_*_DLL  = ENV(CLANG_DLL)
>>> +*_CLANG_*_ASL_PATH   = DEF(UNIX_IASL_BIN)
>>> +
>>> +*_CLANG_*_PP_FLAGS   = DEF(GCC_PP_FLAGS)
>>> +*_CLANG_*_ASLPP_FLAGS= DEF(GCC_ASLPP_FLAGS)
>>> +*_CLANG_*_ASLCC_FLAGS= DEF(GCC_ASLCC_FLAGS)
>>> +*_CLANG_*_VFRPP_FLAGS= DEF(GCC_VFRPP_FLAGS)
>>> +*_CLANG_*_APP_FLAGS  =
>>> +*_CLANG_*_ASL_FLAGS  = DEF(IASL_FLAGS)
>>> +*_CLANG_*_ASL_OUTFLAGS   = DEF(IASL_OUTFLAGS)
>>> +
>>> +*_CLANG_*_CC_PATH= ENV(CLANG_BIN)clang
>>> +*_CLANG_*_ASM_PATH   = ENV(CLANG_BIN)clang
>>> +*_CLANG_*_PP_PATH= ENV(CLANG_BIN)clang
>>> +*_CLANG_*_VFRPP_PATH = ENV(CLANG_BIN)clang
>>> +*_CLANG_*_ASLCC_PATH = ENV(CLANG_BIN)clang
>>> +*_CLANG_*_ASLPP_PATH = ENV(CLANG_BIN)clang
>>> +
>>> +DEFINE CLANG_WARNING_OVERRIDES   = -Wno-parentheses-equality 
>>> -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare 
>>> -Wno-empty-body
>>> +DEFINE CLANG_AARCH64_CC_FLAGS= DEF(GCC_AARCH64_CC_FLAGS) -target 
>>> aarch64 -mcmodel=small -mstrict-align DEF(CLANG_WARNING_OVERRIDES)
>>> +
>>> +##
>>> +# CLANG AARCH64 definitions
>>> +##
>>> +*_CLANG_AARCH64_SLINK_PATH   = ENV(CLANG_AARCH64_PREFIX)ar
>>> +*_CLANG_AARCH64_DLINK_PATH   = ENV(CLANG_AARCH64_PREFIX)ld
>>> +*_CLANG_AARCH64_ASLDLINK_PATH= ENV(CLANG_AARCH64_PREFIX)ld
>>> +*_CLANG_AARCH64_RC_PATH  = ENV(CLANG_AARCH64_PREFIX)objcopy
>>> +
>>> +*_CLANG_AARCH64_ASLCC_FLAGS  = DEF(GCC_ASLCC_FLAGS)
>>> +*_CLANG_AARCH64_ASLDLINK_FLAGS   = DEF(GCC_AARCH64_ASLDLINK_FLAGS)
>>> +*_CLANG_AARCH64_ASM_FLAGS= DEF(GCC_ASM_FLAGS) $(ARCHASM_FLAGS) 
>>> $(PLATFORM_FLAGS) -target aarch64 -Qunused-arguments
>>> +*_CLANG_AARCH64_DLINK_FLAGS  = DEF(GCC_AARCH64_DLINK_FLAGS) -z 
>>> common-page-size=0x1000
>>> +*_CLANG_AARCH64_DLINK2_FLAGS = DEF(GCC_DLINK2_FLAGS_COMMON) 
>>> --defsym=PECOFF_HEADER_SIZE=0x228
>>> +*_CLANG_AARCH64_PLATFORM_FLAGS   =
>>> +*_CLANG_AARCH64_PP_FLAGS = DEF(GCC_PP_FLAGS) $(ARCHCC_FLAGS) 
>>> $(PLATFORM_FLAGS)
>>> +*_CLANG_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS)
>>> +*_CLANG_AARCH64_VFRPP_FLAGS  = DEF(GCC_VFRPP_FLAGS) $(ARCHCC_FLAGS) 
>>> $(PLATFORM_FLAGS)
>>> +
>>> +  DEBUG_CLANG_AARCH64_CC_FLAGS   = DEF(CLANG_AARCH64_CC_FLAGS) 
>>> $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -O0
>>> +RELEASE_CLANG_AARCH64_CC_FLAGS   = DEF(CLANG_AARCH64_CC_FLAGS) 
>>> $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -Oz
>>> +
>>> +###

[edk2] [RFC PATCH 2/4] BaseTools GCC: unify warning flags for all GCC versions

[Ard Biesheuvel]

The warning flags -Wno-address -Wno-unused-but-set-variable are added
for version 4.6 and up, but since they are happily accepted by version
4.4 and 4.5 as well, add them there as well.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Conf/tools_def.template | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 2f9533b56ec8..15d0ea3985ff 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3844,7 +3844,7 @@ DEFINE GCC_IPF_RC_FLAGS= -I binary -O 
elf64-ia64-little   -B ia64
 DEFINE GCC_ARM_RC_FLAGS= -I binary -O elf32-littlearm -B arm   
  --rename-section .data=.hii
 DEFINE GCC_AARCH64_RC_FLAGS= -I binary -O elf64-littleaarch64 -B 
aarch64 --rename-section .data=.hii
 
-DEFINE GCC44_ALL_CC_FLAGS= -g -fshort-wchar -fno-strict-aliasing 
-Wall -Werror -Wno-array-bounds -ffunction-sections -fdata-sections -c -include 
AutoGen.h -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
+DEFINE GCC44_ALL_CC_FLAGS= -g -fshort-wchar -fno-strict-aliasing 
-Wall -Werror -Wno-array-bounds -Wno-address -Wno-unused-but-set-variable 
-ffunction-sections -fdata-sections -c -include AutoGen.h 
-DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
 DEFINE GCC44_IA32_CC_FLAGS   = DEF(GCC44_ALL_CC_FLAGS) -m32 
-malign-double -fno-stack-protector -D EFI32
 DEFINE GCC44_X64_CC_FLAGS= DEF(GCC44_ALL_CC_FLAGS) -m64 
-fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))" -DNO_BUILTIN_VA_FUNCS 
-mno-red-zone -Wno-address -mcmodel=large
 DEFINE GCC44_IA32_X64_DLINK_COMMON   = -nostdlib -n -q --gc-sections -z 
common-page-size=0x20
@@ -3865,8 +3865,8 @@ DEFINE GCC45_X64_DLINK_FLAGS = 
DEF(GCC44_X64_DLINK_FLAGS)
 DEFINE GCC45_X64_DLINK2_FLAGS= DEF(GCC44_X64_DLINK2_FLAGS)
 DEFINE GCC45_ASM_FLAGS   = DEF(GCC44_ASM_FLAGS)
 
-DEFINE GCC46_IA32_CC_FLAGS   = DEF(GCC45_IA32_CC_FLAGS) -Wno-address 
-Wno-unused-but-set-variable
-DEFINE GCC46_X64_CC_FLAGS= DEF(GCC45_X64_CC_FLAGS) -Wno-address 
-Wno-unused-but-set-variable
+DEFINE GCC46_IA32_CC_FLAGS   = DEF(GCC45_IA32_CC_FLAGS)
+DEFINE GCC46_X64_CC_FLAGS= DEF(GCC45_X64_CC_FLAGS)
 DEFINE GCC46_IA32_X64_DLINK_COMMON   = DEF(GCC45_IA32_X64_DLINK_COMMON)
 DEFINE GCC46_IA32_X64_ASLDLINK_FLAGS = DEF(GCC45_IA32_X64_ASLDLINK_FLAGS)
 DEFINE GCC46_IA32_X64_DLINK_FLAGS= DEF(GCC45_IA32_X64_DLINK_FLAGS)
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [RFC PATCH 4/4] BaseTools GCC: unify GCC toolchain command line options

[Ard Biesheuvel]

This unifies the command line options passed to the various versions
of the GCC toolchain.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Conf/tools_def.template | 306 
 1 file changed, 117 insertions(+), 189 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index c65a0ebbc989..e78a8cb2816e 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3844,97 +3844,25 @@ DEFINE GCC_IPF_RC_FLAGS= -I binary -O 
elf64-ia64-little   -B ia64
 DEFINE GCC_ARM_RC_FLAGS= -I binary -O elf32-littlearm -B arm   
  --rename-section .data=.hii
 DEFINE GCC_AARCH64_RC_FLAGS= -I binary -O elf64-littleaarch64 -B 
aarch64 --rename-section .data=.hii
 
-DEFINE GCC44_ALL_CC_FLAGS= -g -fshort-wchar -fno-strict-aliasing 
-Wall -Werror -Wno-array-bounds -Wno-address -Wno-unused-but-set-variable 
-ffunction-sections -fdata-sections -c -include AutoGen.h 
-DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
-DEFINE GCC44_IA32_CC_FLAGS   = DEF(GCC44_ALL_CC_FLAGS) -m32 
-malign-double -fno-stack-protector -D EFI32
-DEFINE GCC44_X64_CC_FLAGS= DEF(GCC44_ALL_CC_FLAGS) -m64 
-fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))" -DNO_BUILTIN_VA_FUNCS 
-mno-red-zone -Wno-address -mcmodel=large
-DEFINE GCC44_IA32_X64_DLINK_COMMON   = -nostdlib -n -q --gc-sections -z 
common-page-size=0x20
-DEFINE GCC44_IA32_X64_ASLDLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_COMMON) 
--entry ReferenceAcpiTable -u ReferenceAcpiTable
-DEFINE GCC44_IA32_X64_DLINK_FLAGS= DEF(GCC44_IA32_X64_DLINK_COMMON) 
--entry $(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Map 
$(DEST_DIR_DEBUG)/$(BASE_NAME).map
-DEFINE GCC44_IA32_DLINK2_FLAGS   = DEF(GCC_DLINK2_FLAGS_COMMON) 
--defsym=PECOFF_HEADER_SIZE=0x220
-DEFINE GCC44_X64_DLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_FLAGS)  
-melf_x86_64 --oformat=elf64-x86-64
-DEFINE GCC44_X64_DLINK2_FLAGS= DEF(GCC_DLINK2_FLAGS_COMMON) 
--defsym=PECOFF_HEADER_SIZE=0x228
-DEFINE GCC44_ASM_FLAGS   = DEF(GCC_ASM_FLAGS)
-
-DEFINE GCC45_IA32_CC_FLAGS   = DEF(GCC44_IA32_CC_FLAGS)
-DEFINE GCC45_X64_CC_FLAGS= DEF(GCC44_X64_CC_FLAGS)
-DEFINE GCC45_IA32_X64_DLINK_COMMON   = DEF(GCC44_IA32_X64_DLINK_COMMON)
-DEFINE GCC45_IA32_X64_ASLDLINK_FLAGS = DEF(GCC44_IA32_X64_ASLDLINK_FLAGS)
-DEFINE GCC45_IA32_X64_DLINK_FLAGS= DEF(GCC44_IA32_X64_DLINK_FLAGS)
-DEFINE GCC45_IA32_DLINK2_FLAGS   = DEF(GCC44_IA32_DLINK2_FLAGS)
-DEFINE GCC45_X64_DLINK_FLAGS = DEF(GCC44_X64_DLINK_FLAGS)
-DEFINE GCC45_X64_DLINK2_FLAGS= DEF(GCC44_X64_DLINK2_FLAGS)
-DEFINE GCC45_ASM_FLAGS   = DEF(GCC44_ASM_FLAGS)
-
-DEFINE GCC46_IA32_CC_FLAGS   = DEF(GCC45_IA32_CC_FLAGS)
-DEFINE GCC46_X64_CC_FLAGS= DEF(GCC45_X64_CC_FLAGS)
-DEFINE GCC46_IA32_X64_DLINK_COMMON   = DEF(GCC45_IA32_X64_DLINK_COMMON)
-DEFINE GCC46_IA32_X64_ASLDLINK_FLAGS = DEF(GCC45_IA32_X64_ASLDLINK_FLAGS)
-DEFINE GCC46_IA32_X64_DLINK_FLAGS= DEF(GCC45_IA32_X64_DLINK_FLAGS)
-DEFINE GCC46_IA32_DLINK2_FLAGS   = DEF(GCC45_IA32_DLINK2_FLAGS)
-DEFINE GCC46_X64_DLINK_FLAGS = DEF(GCC45_X64_DLINK_FLAGS)
-DEFINE GCC46_X64_DLINK2_FLAGS= DEF(GCC45_X64_DLINK2_FLAGS)
-DEFINE GCC46_ASM_FLAGS   = DEF(GCC45_ASM_FLAGS)
-DEFINE GCC46_ARM_ASM_FLAGS   = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_ASM_FLAGS) -mlittle-endian
-DEFINE GCC46_ARM_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector 
-mno-unaligned-access
-DEFINE GCC46_ARM_DLINK_FLAGS = DEF(GCC_ARM_DLINK_FLAGS) 
--oformat=elf32-littlearm
-DEFINE GCC46_ARM_ASLDLINK_FLAGS  = DEF(GCC_ARM_ASLDLINK_FLAGS) 
--oformat=elf32-littlearm
-
-DEFINE GCC47_IA32_CC_FLAGS   = DEF(GCC46_IA32_CC_FLAGS)
-DEFINE GCC47_X64_CC_FLAGS= DEF(GCC46_X64_CC_FLAGS)
-DEFINE GCC47_IA32_X64_DLINK_COMMON   = DEF(GCC46_IA32_X64_DLINK_COMMON)
-DEFINE GCC47_IA32_X64_ASLDLINK_FLAGS = DEF(GCC46_IA32_X64_ASLDLINK_FLAGS)
-DEFINE GCC47_IA32_X64_DLINK_FLAGS= DEF(GCC46_IA32_X64_DLINK_FLAGS)
-DEFINE GCC47_IA32_DLINK2_FLAGS   = DEF(GCC46_IA32_DLINK2_FLAGS)
-DEFINE GCC47_X64_DLINK_FLAGS = DEF(GCC46_X64_DLINK_FLAGS)
-DEFINE GCC47_X64_DLINK2_FLAGS= DEF(GCC46_X64_DLINK2_FLAGS)
-DEFINE GCC47_ASM_FLAGS   = DEF(GCC46_ASM_FLAGS)
-DEFINE GCC47_ARM_ASM_FLAGS   = DEF(GCC46_ARM_ASM_FLAGS)
-DEFINE GCC47_AARCH64_ASM_FLAGS   = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_ASM_FLAGS) -mlittle-endian
-DEFINE GCC47_ARM_CC_FLAGS= DEF(GCC46_ARM_CC_FLAGS)
-DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS)
-DEFINE GCC47_ARM_DLINK_FLAGS = DEF(GCC46_ARM_DLINK_FLAGS)
-DEFINE GCC47_AARCH64_DLINK_FLAGS = DEF(GCC_AARCH64_DLINK_FLAGS)
-DEFINE GCC47_AARCH64_DLINK2_FLAGS   

[edk2] [RFC PATCH 3/4] BaseTools GCC: unify ARM CC flags for all GCC versions

[Ard Biesheuvel]

There is no reason to pass -mno-unaligned-access only to ARM GCC
versions 4.7 and up, since version 4.6 understands it just fine.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Conf/tools_def.template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 15d0ea3985ff..c65a0ebbc989 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3875,7 +3875,7 @@ DEFINE GCC46_X64_DLINK_FLAGS = 
DEF(GCC45_X64_DLINK_FLAGS)
 DEFINE GCC46_X64_DLINK2_FLAGS= DEF(GCC45_X64_DLINK2_FLAGS)
 DEFINE GCC46_ASM_FLAGS   = DEF(GCC45_ASM_FLAGS)
 DEFINE GCC46_ARM_ASM_FLAGS   = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_ASM_FLAGS) -mlittle-endian
-DEFINE GCC46_ARM_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector
+DEFINE GCC46_ARM_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector 
-mno-unaligned-access
 DEFINE GCC46_ARM_DLINK_FLAGS = DEF(GCC_ARM_DLINK_FLAGS) 
--oformat=elf32-littlearm
 DEFINE GCC46_ARM_ASLDLINK_FLAGS  = DEF(GCC_ARM_ASLDLINK_FLAGS) 
--oformat=elf32-littlearm
 
@@ -3890,7 +3890,7 @@ DEFINE GCC47_X64_DLINK2_FLAGS= 
DEF(GCC46_X64_DLINK2_FLAGS)
 DEFINE GCC47_ASM_FLAGS   = DEF(GCC46_ASM_FLAGS)
 DEFINE GCC47_ARM_ASM_FLAGS   = DEF(GCC46_ARM_ASM_FLAGS)
 DEFINE GCC47_AARCH64_ASM_FLAGS   = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_ASM_FLAGS) -mlittle-endian
-DEFINE GCC47_ARM_CC_FLAGS= DEF(GCC46_ARM_CC_FLAGS) 
-mno-unaligned-access
+DEFINE GCC47_ARM_CC_FLAGS= DEF(GCC46_ARM_CC_FLAGS)
 DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_AARCH64_CC_FLAGS)
 DEFINE GCC47_ARM_DLINK_FLAGS = DEF(GCC46_ARM_DLINK_FLAGS)
 DEFINE GCC47_AARCH64_DLINK_FLAGS = DEF(GCC_AARCH64_DLINK_FLAGS)
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [RFC PATCH 1/4] BaseTools GCC: remove 4.9 specific linker alignment override

[Ard Biesheuvel]

If any version of GCC emits any object whose actual alignment
requirement exceeds 32 bytes, this actual alignment value will
automatically become the PE/COFF section alignment value after
PE/COFF conversion now that GenFw propagates the alignment of
the ELF input sections. So there is no longer a need for special
treatment of GCC 4.9, and the linker command line override can
be removed.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Conf/tools_def.template | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index eeb488fb3597..2f9533b56ec8 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3919,11 +3919,11 @@ DEFINE GCC48_AARCH64_ASLDLINK_FLAGS  = 
DEF(GCC47_AARCH64_ASLDLINK_FLAGS)
 
 DEFINE GCC49_IA32_CC_FLAGS   = DEF(GCC48_IA32_CC_FLAGS)
 DEFINE GCC49_X64_CC_FLAGS= DEF(GCC48_X64_CC_FLAGS)
-DEFINE GCC49_IA32_X64_DLINK_COMMON   = -nostdlib -n -q --gc-sections -z 
common-page-size=0x40
-DEFINE GCC49_IA32_X64_ASLDLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_COMMON) 
--entry ReferenceAcpiTable -u ReferenceAcpiTable
-DEFINE GCC49_IA32_X64_DLINK_FLAGS= DEF(GCC49_IA32_X64_DLINK_COMMON) 
--entry $(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Map 
$(DEST_DIR_DEBUG)/$(BASE_NAME).map
+DEFINE GCC49_IA32_X64_DLINK_COMMON   = DEF(GCC48_IA32_X64_DLINK_COMMON)
+DEFINE GCC49_IA32_X64_ASLDLINK_FLAGS = DEF(GCC48_IA32_X64_ASLDLINK_FLAGS)
+DEFINE GCC49_IA32_X64_DLINK_FLAGS= DEF(GCC48_IA32_X64_DLINK_FLAGS)
 DEFINE GCC49_IA32_DLINK2_FLAGS   = DEF(GCC48_IA32_DLINK2_FLAGS)
-DEFINE GCC49_X64_DLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_FLAGS)  
-melf_x86_64 --oformat=elf64-x86-64
+DEFINE GCC49_X64_DLINK_FLAGS = DEF(GCC48_X64_DLINK_FLAGS)
 DEFINE GCC49_X64_DLINK2_FLAGS= DEF(GCC48_X64_DLINK2_FLAGS)
 DEFINE GCC49_ASM_FLAGS   = DEF(GCC48_ASM_FLAGS)
 DEFINE GCC49_ARM_ASM_FLAGS   = DEF(GCC48_ARM_ASM_FLAGS)
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [RFC PATCH 0/4] unify GCC command line options

[Ard Biesheuvel]

This unifies all command line option defines in tools_def.txt, in order
to reduce the maintenance burden.

Note that this does not add or remove any GCC4x toolchains, it just folds
the common DEFINEs into a single series of GCC4X defines.

Ard Biesheuvel (4):
  BaseTools GCC: remove 4.9 specific linker alignment override
  BaseTools GCC: unify warning flags for all GCC versions
  BaseTools GCC: unify ARM CC flags for all GCC versions
  BaseTools GCC: unify GCC toolchain command line options

 BaseTools/Conf/tools_def.template | 306 
 1 file changed, 117 insertions(+), 189 deletions(-)

-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 7/7] BaseTools: add support for CLANG compiler to GCC family

[Ard Biesheuvel]

On 7 August 2015 at 16:51, Jordan Justen  wrote:
> I think the subject should say 'Add CLANG toolchain with AARCH64
> support' to highlight that a new toolchain name is being defined.
>

OK. I can fix that up before applying.

> Should we consider adding the version to the toolchain name, like
> CLANG37? Then we can change parameters between major releases. At
> least for GCC it has proved useful.
>

Actually, I was going to suggest folding everything except GCC-4.4
into GCC4X since the differences are so minor. (and GCC-4.4 may be on
its way out due to its poor support of ms_abi). With GCC 5.0 and 5.1
around the corner, it is becoming a bit unwieldy by my taste.

For clang, there is no reason right now afaict. So perhaps we could
use CLANG3X instead? That way, we can still keep the current support
as newer CLANG versions become available, but are not forced to
declare 2 or 3 discrete but identical versions right from the start.

-- 
Ard.

> On 2015-08-07 07:23:23, Ard Biesheuvel wrote:
>> This adds support for building the AARCH64 platforms using the
>> Clang compiler and assembler combined with the GNU (cross-)linker.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> Reviewed-by: Leif Lindholm 
>> Tested-by: Leif Lindholm 
>> ---
>>  BaseTools/Conf/tools_def.template | 51 
>>  1 file changed, 51 insertions(+)
>>
>> diff --git a/BaseTools/Conf/tools_def.template 
>> b/BaseTools/Conf/tools_def.template
>> index eeb488fb3597..974c9805435b 100644
>> --- a/BaseTools/Conf/tools_def.template
>> +++ b/BaseTools/Conf/tools_def.template
>> @@ -4644,6 +4644,57 @@ RELEASE_GCC49_AARCH64_CC_FLAGS   = 
>> DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-s
>>
>>  
>> 
>>  #
>> +# CLANG   - This configuration is used to compile under Linux to produce
>> +#   PE/COFF binaries using the clang compiler and assembler and GNU 
>> linker
>> +#
>> +
>> +*_CLANG_*_*_FAMILY   = GCC
>> +
>> +*_CLANG_*_MAKE_PATH  = make
>> +*_CLANG_*_*_DLL  = ENV(CLANG_DLL)
>> +*_CLANG_*_ASL_PATH   = DEF(UNIX_IASL_BIN)
>> +
>> +*_CLANG_*_PP_FLAGS   = DEF(GCC_PP_FLAGS)
>> +*_CLANG_*_ASLPP_FLAGS= DEF(GCC_ASLPP_FLAGS)
>> +*_CLANG_*_ASLCC_FLAGS= DEF(GCC_ASLCC_FLAGS)
>> +*_CLANG_*_VFRPP_FLAGS= DEF(GCC_VFRPP_FLAGS)
>> +*_CLANG_*_APP_FLAGS  =
>> +*_CLANG_*_ASL_FLAGS  = DEF(IASL_FLAGS)
>> +*_CLANG_*_ASL_OUTFLAGS   = DEF(IASL_OUTFLAGS)
>> +
>> +*_CLANG_*_CC_PATH= ENV(CLANG_BIN)clang
>> +*_CLANG_*_ASM_PATH   = ENV(CLANG_BIN)clang
>> +*_CLANG_*_PP_PATH= ENV(CLANG_BIN)clang
>> +*_CLANG_*_VFRPP_PATH = ENV(CLANG_BIN)clang
>> +*_CLANG_*_ASLCC_PATH = ENV(CLANG_BIN)clang
>> +*_CLANG_*_ASLPP_PATH = ENV(CLANG_BIN)clang
>> +
>> +DEFINE CLANG_WARNING_OVERRIDES   = -Wno-parentheses-equality 
>> -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare 
>> -Wno-empty-body
>> +DEFINE CLANG_AARCH64_CC_FLAGS= DEF(GCC_AARCH64_CC_FLAGS) -target 
>> aarch64 -mcmodel=small -mstrict-align DEF(CLANG_WARNING_OVERRIDES)
>> +
>> +##
>> +# CLANG AARCH64 definitions
>> +##
>> +*_CLANG_AARCH64_SLINK_PATH   = ENV(CLANG_AARCH64_PREFIX)ar
>> +*_CLANG_AARCH64_DLINK_PATH   = ENV(CLANG_AARCH64_PREFIX)ld
>> +*_CLANG_AARCH64_ASLDLINK_PATH= ENV(CLANG_AARCH64_PREFIX)ld
>> +*_CLANG_AARCH64_RC_PATH  = ENV(CLANG_AARCH64_PREFIX)objcopy
>> +
>> +*_CLANG_AARCH64_ASLCC_FLAGS  = DEF(GCC_ASLCC_FLAGS)
>> +*_CLANG_AARCH64_ASLDLINK_FLAGS   = DEF(GCC_AARCH64_ASLDLINK_FLAGS)
>> +*_CLANG_AARCH64_ASM_FLAGS= DEF(GCC_ASM_FLAGS) $(ARCHASM_FLAGS) 
>> $(PLATFORM_FLAGS) -target aarch64 -Qunused-arguments
>> +*_CLANG_AARCH64_DLINK_FLAGS  = DEF(GCC_AARCH64_DLINK_FLAGS) -z 
>> common-page-size=0x1000
>> +*_CLANG_AARCH64_DLINK2_FLAGS = DEF(GCC_DLINK2_FLAGS_COMMON) 
>> --defsym=PECOFF_HEADER_SIZE=0x228
>> +*_CLANG_AARCH64_PLATFORM_FLAGS   =
>> +*_CLANG_AARCH64_PP_FLAGS = DEF(GCC_PP_FLAGS) $(ARCHCC_FLAGS) 
>> $(PLATFORM_FLAGS)
>> +*_CLANG_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS)
>> +*_CLANG_AARCH64_VFRPP_FLAGS  = DEF(GCC_VFRPP_FLAGS) $(ARCHCC_FLAGS) 
>> $(PLATFORM_FLAGS)
>> +
>> +  DEBUG_CLANG_AARCH64_CC_FLAGS   = DEF(CLANG_AARCH64_CC_FLAGS) 
>> $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -O0
>> +RELEASE_CLANG_AARCH64_CC_FLAGS   = DEF(CLANG_AARCH64_CC_FLAGS) 
>> $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -Oz
>> +
>> +
>> +#
>>  # Cygwin GCC And Intel ACPI Compiler
>>  #
>>  
>> 
>> --
>> 1.9.1
>>
>> 

Re: [edk2] [PATCH v3 7/7] BaseTools: add support for CLANG compiler to GCC family

[Jordan Justen]

I think the subject should say 'Add CLANG toolchain with AARCH64
support' to highlight that a new toolchain name is being defined.

Should we consider adding the version to the toolchain name, like
CLANG37? Then we can change parameters between major releases. At
least for GCC it has proved useful.

-Jordan

On 2015-08-07 07:23:23, Ard Biesheuvel wrote:
> This adds support for building the AARCH64 platforms using the
> Clang compiler and assembler combined with the GNU (cross-)linker.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> Reviewed-by: Leif Lindholm 
> Tested-by: Leif Lindholm 
> ---
>  BaseTools/Conf/tools_def.template | 51 
>  1 file changed, 51 insertions(+)
> 
> diff --git a/BaseTools/Conf/tools_def.template 
> b/BaseTools/Conf/tools_def.template
> index eeb488fb3597..974c9805435b 100644
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -4644,6 +4644,57 @@ RELEASE_GCC49_AARCH64_CC_FLAGS   = 
> DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-s
>  
>  
> 
>  #
> +# CLANG   - This configuration is used to compile under Linux to produce
> +#   PE/COFF binaries using the clang compiler and assembler and GNU 
> linker
> +#
> +
> +*_CLANG_*_*_FAMILY   = GCC
> +
> +*_CLANG_*_MAKE_PATH  = make
> +*_CLANG_*_*_DLL  = ENV(CLANG_DLL)
> +*_CLANG_*_ASL_PATH   = DEF(UNIX_IASL_BIN)
> +
> +*_CLANG_*_PP_FLAGS   = DEF(GCC_PP_FLAGS)
> +*_CLANG_*_ASLPP_FLAGS= DEF(GCC_ASLPP_FLAGS)
> +*_CLANG_*_ASLCC_FLAGS= DEF(GCC_ASLCC_FLAGS)
> +*_CLANG_*_VFRPP_FLAGS= DEF(GCC_VFRPP_FLAGS)
> +*_CLANG_*_APP_FLAGS  =
> +*_CLANG_*_ASL_FLAGS  = DEF(IASL_FLAGS)
> +*_CLANG_*_ASL_OUTFLAGS   = DEF(IASL_OUTFLAGS)
> +
> +*_CLANG_*_CC_PATH= ENV(CLANG_BIN)clang
> +*_CLANG_*_ASM_PATH   = ENV(CLANG_BIN)clang
> +*_CLANG_*_PP_PATH= ENV(CLANG_BIN)clang
> +*_CLANG_*_VFRPP_PATH = ENV(CLANG_BIN)clang
> +*_CLANG_*_ASLCC_PATH = ENV(CLANG_BIN)clang
> +*_CLANG_*_ASLPP_PATH = ENV(CLANG_BIN)clang
> +
> +DEFINE CLANG_WARNING_OVERRIDES   = -Wno-parentheses-equality 
> -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare 
> -Wno-empty-body
> +DEFINE CLANG_AARCH64_CC_FLAGS= DEF(GCC_AARCH64_CC_FLAGS) -target aarch64 
> -mcmodel=small -mstrict-align DEF(CLANG_WARNING_OVERRIDES)
> +
> +##
> +# CLANG AARCH64 definitions
> +##
> +*_CLANG_AARCH64_SLINK_PATH   = ENV(CLANG_AARCH64_PREFIX)ar
> +*_CLANG_AARCH64_DLINK_PATH   = ENV(CLANG_AARCH64_PREFIX)ld
> +*_CLANG_AARCH64_ASLDLINK_PATH= ENV(CLANG_AARCH64_PREFIX)ld
> +*_CLANG_AARCH64_RC_PATH  = ENV(CLANG_AARCH64_PREFIX)objcopy
> +
> +*_CLANG_AARCH64_ASLCC_FLAGS  = DEF(GCC_ASLCC_FLAGS)
> +*_CLANG_AARCH64_ASLDLINK_FLAGS   = DEF(GCC_AARCH64_ASLDLINK_FLAGS)
> +*_CLANG_AARCH64_ASM_FLAGS= DEF(GCC_ASM_FLAGS) $(ARCHASM_FLAGS) 
> $(PLATFORM_FLAGS) -target aarch64 -Qunused-arguments
> +*_CLANG_AARCH64_DLINK_FLAGS  = DEF(GCC_AARCH64_DLINK_FLAGS) -z 
> common-page-size=0x1000
> +*_CLANG_AARCH64_DLINK2_FLAGS = DEF(GCC_DLINK2_FLAGS_COMMON) 
> --defsym=PECOFF_HEADER_SIZE=0x228
> +*_CLANG_AARCH64_PLATFORM_FLAGS   =
> +*_CLANG_AARCH64_PP_FLAGS = DEF(GCC_PP_FLAGS) $(ARCHCC_FLAGS) 
> $(PLATFORM_FLAGS)
> +*_CLANG_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS)
> +*_CLANG_AARCH64_VFRPP_FLAGS  = DEF(GCC_VFRPP_FLAGS) $(ARCHCC_FLAGS) 
> $(PLATFORM_FLAGS)
> +
> +  DEBUG_CLANG_AARCH64_CC_FLAGS   = DEF(CLANG_AARCH64_CC_FLAGS) 
> $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -O0
> +RELEASE_CLANG_AARCH64_CC_FLAGS   = DEF(CLANG_AARCH64_CC_FLAGS) 
> $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) -Oz
> +
> +
> +#
>  # Cygwin GCC And Intel ACPI Compiler
>  #
>  
> 
> -- 
> 1.9.1
> 
> ___
> 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

[edk2] [PATCH v3 7/7] BaseTools: add support for CLANG compiler to GCC family

[Ard Biesheuvel]

This adds support for building the AARCH64 platforms using the
Clang compiler and assembler combined with the GNU (cross-)linker.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
Tested-by: Leif Lindholm 
---
 BaseTools/Conf/tools_def.template | 51 
 1 file changed, 51 insertions(+)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index eeb488fb3597..974c9805435b 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4644,6 +4644,57 @@ RELEASE_GCC49_AARCH64_CC_FLAGS   = 
DEF(GCC49_AARCH64_CC_FLAGS) -Wno-unused-but-s
 
 

 #
+# CLANG   - This configuration is used to compile under Linux to produce
+#   PE/COFF binaries using the clang compiler and assembler and GNU 
linker
+#
+
+*_CLANG_*_*_FAMILY   = GCC
+
+*_CLANG_*_MAKE_PATH  = make
+*_CLANG_*_*_DLL  = ENV(CLANG_DLL)
+*_CLANG_*_ASL_PATH   = DEF(UNIX_IASL_BIN)
+
+*_CLANG_*_PP_FLAGS   = DEF(GCC_PP_FLAGS)
+*_CLANG_*_ASLPP_FLAGS= DEF(GCC_ASLPP_FLAGS)
+*_CLANG_*_ASLCC_FLAGS= DEF(GCC_ASLCC_FLAGS)
+*_CLANG_*_VFRPP_FLAGS= DEF(GCC_VFRPP_FLAGS)
+*_CLANG_*_APP_FLAGS  =
+*_CLANG_*_ASL_FLAGS  = DEF(IASL_FLAGS)
+*_CLANG_*_ASL_OUTFLAGS   = DEF(IASL_OUTFLAGS)
+
+*_CLANG_*_CC_PATH= ENV(CLANG_BIN)clang
+*_CLANG_*_ASM_PATH   = ENV(CLANG_BIN)clang
+*_CLANG_*_PP_PATH= ENV(CLANG_BIN)clang
+*_CLANG_*_VFRPP_PATH = ENV(CLANG_BIN)clang
+*_CLANG_*_ASLCC_PATH = ENV(CLANG_BIN)clang
+*_CLANG_*_ASLPP_PATH = ENV(CLANG_BIN)clang
+
+DEFINE CLANG_WARNING_OVERRIDES   = -Wno-parentheses-equality 
-Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare 
-Wno-empty-body
+DEFINE CLANG_AARCH64_CC_FLAGS= DEF(GCC_AARCH64_CC_FLAGS) -target aarch64 
-mcmodel=small -mstrict-align DEF(CLANG_WARNING_OVERRIDES)
+
+##
+# CLANG AARCH64 definitions
+##
+*_CLANG_AARCH64_SLINK_PATH   = ENV(CLANG_AARCH64_PREFIX)ar
+*_CLANG_AARCH64_DLINK_PATH   = ENV(CLANG_AARCH64_PREFIX)ld
+*_CLANG_AARCH64_ASLDLINK_PATH= ENV(CLANG_AARCH64_PREFIX)ld
+*_CLANG_AARCH64_RC_PATH  = ENV(CLANG_AARCH64_PREFIX)objcopy
+
+*_CLANG_AARCH64_ASLCC_FLAGS  = DEF(GCC_ASLCC_FLAGS)
+*_CLANG_AARCH64_ASLDLINK_FLAGS   = DEF(GCC_AARCH64_ASLDLINK_FLAGS)
+*_CLANG_AARCH64_ASM_FLAGS= DEF(GCC_ASM_FLAGS) $(ARCHASM_FLAGS) 
$(PLATFORM_FLAGS) -target aarch64 -Qunused-arguments
+*_CLANG_AARCH64_DLINK_FLAGS  = DEF(GCC_AARCH64_DLINK_FLAGS) -z 
common-page-size=0x1000
+*_CLANG_AARCH64_DLINK2_FLAGS = DEF(GCC_DLINK2_FLAGS_COMMON) 
--defsym=PECOFF_HEADER_SIZE=0x228
+*_CLANG_AARCH64_PLATFORM_FLAGS   =
+*_CLANG_AARCH64_PP_FLAGS = DEF(GCC_PP_FLAGS) $(ARCHCC_FLAGS) 
$(PLATFORM_FLAGS)
+*_CLANG_AARCH64_RC_FLAGS = DEF(GCC_AARCH64_RC_FLAGS)
+*_CLANG_AARCH64_VFRPP_FLAGS  = DEF(GCC_VFRPP_FLAGS) $(ARCHCC_FLAGS) 
$(PLATFORM_FLAGS)
+
+  DEBUG_CLANG_AARCH64_CC_FLAGS   = DEF(CLANG_AARCH64_CC_FLAGS) $(ARCHCC_FLAGS) 
$(PLATFORM_FLAGS) -O0
+RELEASE_CLANG_AARCH64_CC_FLAGS   = DEF(CLANG_AARCH64_CC_FLAGS) $(ARCHCC_FLAGS) 
$(PLATFORM_FLAGS) -Oz
+
+
+#
 # Cygwin GCC And Intel ACPI Compiler
 #
 

-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v3 2/7] ArmPkg/GenericWatchdogDxe: add missing VOID* cast

[Ard Biesheuvel]

Use an explicit VOID* cast when passing a static char array into
a function taking a void pointer.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
Tested-by: Leif Lindholm 
---
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index b1d9c027d80e..54a1625a3213 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -111,7 +111,7 @@ WatchdogInterruptHandler (
  EfiResetCold,
  EFI_TIMEOUT,
  StrSize (ResetString),
- &ResetString
+ (VOID *) &ResetString
  );
 
   // If we got here then the reset didn't work
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v3 5/7] ArmPlatformPkg/FVP: use 'auto' alignment and FIXED placement for XIP modules

[Ard Biesheuvel]

Now that GenFw correctly propagates the minimum alignment of the ELF
input sections to the PE/COFF binary, we can simply select 'auto'
alignment in the FDF Rule section instead of tweaking it by hand.

Also add the FIXED FFS attribute to the module types that may execute
in place. This enables a newly added optimization in GenFfs that strips
redundant padding, preventing excessive waste of FV space if the section
alignment is considerable (i.e., 2 KB or 4 KB)

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
Tested-by: Leif Lindholm 
---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-FVP-AArch64.fdf | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-FVP-AArch64.fdf 
b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-FVP-AArch64.fdf
index 53e2ca8b0203..9c447084e316 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-FVP-AArch64.fdf
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-FVP-AArch64.fdf
@@ -309,20 +309,20 @@ [FV.FVMAIN_COMPACT]
 
 
 [Rule.Common.SEC]
-  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
-TE  TE Align = 4K  $(INF_OUTPUT)/$(MODULE_NAME).efi
+  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED FIXED {
+TE  TE Align = Auto $(INF_OUTPUT)/$(MODULE_NAME).efi
   }
 
 [Rule.Common.PEI_CORE]
-  FILE PEI_CORE = $(NAMED_GUID) {
-TE TE Align = 8 $(INF_OUTPUT)/$(MODULE_NAME).efi
+  FILE PEI_CORE = $(NAMED_GUID) FIXED {
+TE TE Align = Auto  $(INF_OUTPUT)/$(MODULE_NAME).efi
 UI STRING ="$(MODULE_NAME)" Optional
   }
 
 [Rule.Common.PEIM]
-  FILE PEIM = $(NAMED_GUID) {
+  FILE PEIM = $(NAMED_GUID) FIXED {
  PEI_DEPEX PEI_DEPEX Optional   $(INF_OUTPUT)/$(MODULE_NAME).depex
- TE   TE Align = 8  $(INF_OUTPUT)/$(MODULE_NAME).efi
+ TE   TE Align = Auto   $(INF_OUTPUT)/$(MODULE_NAME).efi
  UI   STRING="$(MODULE_NAME)" Optional
   }
 
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v3 3/7] ArmPlatformPkg/ArmJunoPkg: use a rodata symbol for ReferenceAcpiTable

[Ard Biesheuvel]

The ACPI .aslc files contain a ReferenceAcpiTable() function whose
sole purpose is to ensure that the table itself does not get optimized
away. However, when using clang, these dummy functions result in a 4 KB
section alignment requirement, which is silly since everything except
the .data section is discarded later anyway.

So instead, make ReferenceAcpiTable a CONST pointer to VOID*. This way,
we still have a .text section, which is mandatory for the PE/COFF
conversion, but no executable code with small model relocations that
impose additional alignment requirements.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
Tested-by: Leif Lindholm 
---
 ArmPlatformPkg/ArmJunoPkg/AcpiTables/Facs.aslc | 16 +---
 ArmPlatformPkg/ArmJunoPkg/AcpiTables/Fadt.aslc | 16 +---
 ArmPlatformPkg/ArmJunoPkg/AcpiTables/Gtdt.aslc | 16 +---
 ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc | 16 +---
 4 files changed, 20 insertions(+), 44 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Facs.aslc 
b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Facs.aslc
index 9743ddb5ee85..137ead77c199 100644
--- a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Facs.aslc
+++ b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Facs.aslc
@@ -55,14 +55,8 @@ EFI_ACPI_5_0_FIRMWARE_ACPI_CONTROL_STRUCTURE Facs = {
   EFI_ACPI_RESERVED_BYTE },   // UINT8   
Reserved1[23]
 };
 
-VOID*
-ReferenceAcpiTable (
-  VOID
-  )
-{
-  //
-  // Reference the table being generated to prevent the optimizer from 
removing the
-  // data structure from the executable
-  //
-  return (VOID*)&Facs;
-}
+//
+// Reference the table being generated to prevent the optimizer from removing 
the
+// data structure from the executable
+//
+VOID* CONST ReferenceAcpiTable = &Facs;
diff --git a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Fadt.aslc 
b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Fadt.aslc
index ef6d786b7c4d..eafdecb8aff7 100644
--- a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Fadt.aslc
+++ b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Fadt.aslc
@@ -92,14 +92,8 @@ EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE Fadt = {
   NULL_GAS  // 
EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE  SleepStatusReg
 };
 
-VOID*
-ReferenceAcpiTable (
-  VOID
-  )
-{
-  //
-  // Reference the table being generated to prevent the optimizer from 
removing the
-  // data structure from the executable
-  //
-  return (VOID*)&Fadt;
-}
+//
+// Reference the table being generated to prevent the optimizer from removing 
the
+// data structure from the executable
+//
+VOID* CONST ReferenceAcpiTable = &Fadt;
diff --git a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Gtdt.aslc 
b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Gtdt.aslc
index 49d6e8e2136c..50057c2641d7 100644
--- a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Gtdt.aslc
+++ b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Gtdt.aslc
@@ -96,14 +96,8 @@
   };
 #endif
 
-VOID*
-ReferenceAcpiTable (
-  VOID
-  )
-{
-  //
-  // Reference the table being generated to prevent the optimizer from 
removing the
-  // data structure from the exeutable
-  //
-  return (VOID*)&Gtdt;
-}
+//
+// Reference the table being generated to prevent the optimizer from removing 
the
+// data structure from the executable
+//
+VOID* CONST ReferenceAcpiTable = &Gtdt;
diff --git a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc 
b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc
index f8f50800c0bc..406bd94f5636 100644
--- a/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc
+++ b/ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc
@@ -123,14 +123,8 @@
   };
 #endif
 
-VOID*
-ReferenceAcpiTable (
-  VOID
-  )
-{
-  //
-  // Reference the table being generated to prevent the optimizer from 
removing the
-  // data structure from the executable
-  //
-  return (VOID*)&Madt;
-}
+//
+// Reference the table being generated to prevent the optimizer from removing 
the
+// data structure from the executable
+//
+VOID* CONST ReferenceAcpiTable = &Madt;
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v3 1/7] ArmPkg/GicV3: use GICv3 generic sysreg names only for GNU as

[Ard Biesheuvel]

The GNU assembler extends the generic notation for IMPLEMENTATION
DEFINED system registers to support any system register, so that
system registers defined by newer versions of the architecture can
still be used by older versions of the toolchain.

Clang before v3.6 supports the generic notation, but does not
support this extension, nor does it need to in the particular case
of the GICv3 support code, since it knows the GICv3 registers by
their architectural names. So only redefine their real names to
their generic aliases if we are not using clang.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
Tested-by: Leif Lindholm 
---
 ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S 
b/ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S
index c30ae76d9455..f1c227f2c421 100644
--- a/ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S
+++ b/ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S
@@ -13,6 +13,14 @@
 
 #include 
 
+#if !defined(__clang__)
+
+//
+// Clang versions before v3.6 do not support the GNU extension that allows
+// system registers outside of the IMPLEMENTATION DEFINED range to be specified
+// using the generic notation below. However, clang knows these registers by
+// their architectural names, so it has no need for these aliases anyway.
+//
 #define ICC_SRE_EL1 S3_0_C12_C12_5
 #define ICC_SRE_EL2 S3_4_C12_C9_5
 #define ICC_SRE_EL3 S3_6_C12_C12_5
@@ -22,6 +30,8 @@
 #define ICC_PMR_EL1 S3_0_C4_C6_0
 #define ICC_BPR1_EL1S3_0_C12_C12_3
 
+#endif
+
 .text
 .align 2
 
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v3 6/7] BaseTools/GenFw: allow AArch64 tiny and small code model relocations

[Ard Biesheuvel]

The AArch64 small C model makes extensive use of ADRP/ADD and
ADRP/{LDR,STR} pairs to emit PC-relative symbol references with
a +/- 4 GB range. Since the relocation pair splits the relative
offset into a relative page offset and an absolute offset into
a 4 KB page, we need to take extra care to ensure that the target
of the relocation preserves its alignment relative to a 4 KB
alignment boundary.

Also, due to a problem with the --emit-relocs GNU ld option, where
it does not recalculate the addends for section relative relocations,
the only way to guarantee correct code is by requiring the relative
section offset to be equal in the ELF and PE/COFF versions of the
binary. This affects both the 'tiny' and 'small' GCC code models.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
Tested-by: Leif Lindholm 
---
 BaseTools/Source/C/GenFw/Elf64Convert.c | 92 +++-
 1 file changed, 49 insertions(+), 43 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 1c0f4a4dc87c..c758ed9d64a6 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -740,53 +740,60 @@ WriteSections64 (
   }
 } else if (mEhdr->e_machine == EM_AARCH64) {
 
-  // AARCH64 GCC uses RELA relocation, so all relocations have to be 
fixed up.
-  // As opposed to ARM32 using REL.
-
   switch (ELF_R_TYPE(Rel->r_info)) {
 
-  case R_AARCH64_ADR_PREL_LO21:
-if  (Rel->r_addend != 0 ) { /* TODO */
-  Error (NULL, 0, 3000, "Invalid", "AArch64: 
R_AARCH64_ADR_PREL_LO21 Need to fixup with addend!.");
+  case R_AARCH64_ADR_PREL_PG_HI21:
+  case R_AARCH64_ADD_ABS_LO12_NC:
+  case R_AARCH64_LDST8_ABS_LO12_NC:
+  case R_AARCH64_LDST16_ABS_LO12_NC:
+  case R_AARCH64_LDST32_ABS_LO12_NC:
+  case R_AARCH64_LDST64_ABS_LO12_NC:
+  case R_AARCH64_LDST128_ABS_LO12_NC:
+//
+// AArch64 PG_H21 relocations are typically paired with ABS_LO12
+// relocations, where a PC-relative reference with +/- 4 GB range 
is
+// split into a relative high part and an absolute low part. Since
+// the absolute low part represents the offset into a 4 KB page, we
+// have to make sure that the 4 KB relative offsets of both the
+// section containing the reference as well as the section to which
+// it refers have not been changed during PE/COFF conversion (i.e.,
+// in ScanSections64() above).
+//
+if (((SecShdr->sh_addr ^ SecOffset) & 0xfff) != 0 ||
+((SymShdr->sh_addr ^ mCoffSectionsOffset[Sym->st_shndx]) & 
0xfff) != 0 ||
+mCoffAlignment < 0x1000) {
+  Error (NULL, 0, 3000, "Invalid", "WriteSections64(): %s AARCH64 
small code model requires 4 KB section alignment.",
+mInImageName);
+  break;
 }
-break;
+/* fall through */
 
+  case R_AARCH64_ADR_PREL_LO21:
   case R_AARCH64_CONDBR19:
-if  (Rel->r_addend != 0 ) { /* TODO */
-  Error (NULL, 0, 3000, "Invalid", "AArch64: R_AARCH64_CONDBR19 
Need to fixup with addend!.");
-}
-break;
-
   case R_AARCH64_LD_PREL_LO19:
-if  (Rel->r_addend != 0 ) { /* TODO */
-  Error (NULL, 0, 3000, "Invalid", "AArch64: 
R_AARCH64_LD_PREL_LO19 Need to fixup with addend!.");
-}
-break;
-
   case R_AARCH64_CALL26:
   case R_AARCH64_JUMP26:
-if  (Rel->r_addend != 0 ) {
-  // Some references to static functions sometime start at the 
base of .text + addend.
-  // It is safe to ignore these relocations because they patch a 
`BL` instructions that
-  // contains an offset from the instruction itself and there is 
only a single .text section.
-  // So we check if the symbol is a "section symbol"
-  if (ELF64_ST_TYPE (Sym->st_info) == STT_SECTION) {
-break;
-  }
-  Error (NULL, 0, 3000, "Invalid", "AArch64: R_AARCH64_JUMP26 Need 
to fixup with addend!.");
+//
+// The GCC toolchains (i.e., binutils) may corrupt section relative
+// relocations when emitting relocation sections into fully linked
+// binaries. More specifically, they tend to fail to take into
+// account the fact that a '.rodata + XXX' relocation needs to have
+// its addend recalculated once .rodata is merged into the .text
+// section, and the relocation emitted into the .rela.text section.
+//
+// We cannot really recover from this loss of information, so the
+// only workaround is to prevent having

[edk2] [PATCH v3 4/7] ArmPlatformPkg/ArmJunoPkg: use TE 'auto' alignment for SEC modules

[Ard Biesheuvel]

No need to hardcode the TE alignment anymore, now that GenFw sets
the PE/COFF alignment according to the alignment requirements of
the ELF input sections.

Also enable FIXED FFS placement so that we can reclaim some of the
space wasted to padding when using clang with 4 KB section alignment.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Leif Lindholm 
Tested-by: Leif Lindholm 
---
 ArmPlatformPkg/ArmJunoPkg/ArmJuno.fdf | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ArmPlatformPkg/ArmJunoPkg/ArmJuno.fdf 
b/ArmPlatformPkg/ArmJunoPkg/ArmJuno.fdf
index 0012efcef5be..c8f5831093a7 100644
--- a/ArmPlatformPkg/ArmJunoPkg/ArmJuno.fdf
+++ b/ArmPlatformPkg/ArmJunoPkg/ArmJuno.fdf
@@ -276,8 +276,8 @@ [Rule.ARM.SEC]
   }
 
 [Rule.AARCH64.SEC]
-  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
-TE  TEAlign = 4K$(INF_OUTPUT)/$(MODULE_NAME).efi
+  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED FIXED {
+TE  TEAlign = Auto  $(INF_OUTPUT)/$(MODULE_NAME).efi
   }
 
 # A shim specific rule is required to ensure the alignment is 4K.
@@ -288,13 +288,13 @@ [Rule.ARM.SEC.SHIM]
   }
 
 [Rule.Common.PEI_CORE]
-  FILE PEI_CORE = $(NAMED_GUID) {
+  FILE PEI_CORE = $(NAMED_GUID) FIXED {
 TE TE   $(INF_OUTPUT)/$(MODULE_NAME).efi
 UI STRING ="$(MODULE_NAME)" Optional
   }
 
 [Rule.Common.PEIM]
-  FILE PEIM = $(NAMED_GUID) {
+  FILE PEIM = $(NAMED_GUID) FIXED {
  PEI_DEPEX PEI_DEPEX Optional   $(INF_OUTPUT)/$(MODULE_NAME).depex
  TE   TE$(INF_OUTPUT)/$(MODULE_NAME).efi
  UI   STRING="$(MODULE_NAME)" Optional
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v3 0/7] small model and clang support for AARCH64

[Ard Biesheuvel]

This is v3 of the followup to the series 'small C model and LLVM/clang support
for AARCH64' that I sent out on July 17th. Now that the FFS/FV optimization
patches have been merged, this is what remains to allow the AARCH64 platforms to
be built using clang, combined with the GNU binutils (cross-)toolchain.


@Dennis, Liming: may I please have your comments on patches 6/7 and 7/7?
They only affect AARCH64, but they touch BaseTools/ so I need your
reviewed-by before committing them. Thanks.


Patch #1 is a simple fixup to allow clang to assemble the GICv3 support code.

Patch #2 adds a VOID* cast to a static char array used as an argument for a
void* formal parameter.

Patch #3 replaces the ReferenceAcpiTable() entry point functions with VOID*
pointers. Since this is a dummy entry point that is only intended to prevent the
optimizer from pruning the table it refers to, we can replace it with no loss of
functionality. This way, we don't need to impose 4 KB section alignment, since
the binary contains no executable code anymore.

Patch #4 sets the correct TE alignment for ArmJuno.fdf, which is required for
the 4 KB section aligned binaries that we build when using clang and its small
code model. It also sets the 'FIXED' FFS attribute which is required to enable
the FFS/FV padding optimization logic.

Patch #5 does the same as patch #4 for ArmVExpress-FVP-AArch64.fdf

Patch #6 relaxes the relocation checks in GenFw to allow relative relocations
for AARCH64, but only if they don't require recalculation. This is needed to
work around an issue with GNU binutils 2.24 and older, which emits incorrect
relocation information into fully linked ELF binaries (i.e., when using the
--emit-relocs option). This patch supersedes 'BaseTools/GenFw: allow AArch64
small model relocations' sent out July 17th.

Patch #7 adds the tool definition for CLANG as a member of the GCC family.

Changes since v1:
- dropped patch that sets the TE alignment for ArmVirtQemu, it has been merged
  in the mean time since it also fixed a latent build error (SVN r18122)
- added patches #2 .. #5 that address minor issues that break the build for
  CLANG
- added -mstrict-align to CLANG CC flags, this is required to prevent CLANG from
  emitting unaligned memory references that may cause an abort when executed
  before the MMU is up

Changes since v2:
- add -Qunused-arguments to the clang assembler flags to suppress a warning that
  it emits when encountering -I command line arguments, which it ignores
- add -Oz to the RELEASE CC flags for a bit more aggressive size optimization
- added Leif's R-b and T-b tags

Build and boot tested with ArmVirtQemu and ArmVExpressPkg-FVP built in both
DEBUG and RELEASE modes using:

Ubuntu clang version 3.5.0-4ubuntu2 (tags/RELEASE_350/final) (based on LLVM 
3.5.0)
Target: aarch64
Ard Biesheuvel (7):
  ArmPkg/GicV3: use GICv3 generic sysreg names only for GNU as
  ArmPkg/GenericWatchdogDxe: add missing VOID* cast
  ArmPlatformPkg/ArmJunoPkg: use a rodata symbol for ReferenceAcpiTable
  ArmPlatformPkg/ArmJunoPkg: use TE 'auto' alignment for SEC modules
  ArmPlatformPkg/FVP: use 'auto' alignment and FIXED placement for XIP
modules
  BaseTools/GenFw: allow AArch64 tiny and small code model relocations
  BaseTools: add support for CLANG compiler to GCC family

 ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S| 10 +++
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c|  2 +-
 ArmPlatformPkg/ArmJunoPkg/AcpiTables/Facs.aslc| 16 ++--
 ArmPlatformPkg/ArmJunoPkg/AcpiTables/Fadt.aslc| 16 ++--
 ArmPlatformPkg/ArmJunoPkg/AcpiTables/Gtdt.aslc| 16 ++--
 ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc| 16 ++--
 ArmPlatformPkg/ArmJunoPkg/ArmJuno.fdf |  8 +-
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-FVP-AArch64.fdf | 12 +--
 BaseTools/Conf/tools_def.template | 51 +++
 BaseTools/Source/C/GenFw/Elf64Convert.c   | 92 
+++-
 10 files changed, 141 insertions(+), 98 deletions(-)

-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/7] small model and clang support for AARCH64

[Ard Biesheuvel]

On 7 August 2015 at 16:05, Leif Lindholm  wrote:
> Thanks Ard,
>
> Tested on Juno/FVP.
>
> I can see two issues:
> - While building, clang gives a fair amount of
>   clang: warning: argument unused during compilation: '-I '.

Yes, this turns out to be the assembler driver that does not take
-I. I will add -Qunused-arguments to just the asm-to-object build
step

> - The increase in code size compared to GCC means that Juno with
>   IntelBds no longer fits in the FV.
>

This is unfortunately a downside to using CLANG. The code is
substantially bigger than what GCC produces for AARCH64, even when not
taking the 4 KB alignment into account that CLANG requires.

As it turns out, there is a -Oz optimization flag that optimizes for
size more aggressively than -Os. It shaves off a couple of percent but
the bottom line is still 5% increase compared to a GCC small model
build (i.e., also with 4 KB alignment requirement)

> But the latter is not supported by the .dsc in current edk2 (and
> Juno's FV is not the full size of the available flash, so could be
> increased), and the former is cosmetic only.
>

OK

> If you want to, you could add a -Qunused-arguments to the CFLAGS to
> get rid of the warnings. But regardless, for the series:
>
> Reviewed-by: Leif Lindholm 
> Tested-by: Leif Lindholm 
>

Thanks

> I guess you need further nods for at least 7/7?
>

Indeed. I will repost with these minor changes, and your tags added,
and poke Dennis for his comments.

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

Re: [edk2] [PATCH v2 0/7] small model and clang support for AARCH64

[Leif Lindholm]

Thanks Ard,

Tested on Juno/FVP.

I can see two issues:
- While building, clang gives a fair amount of 
  clang: warning: argument unused during compilation: '-I '.
- The increase in code size compared to GCC means that Juno with
  IntelBds no longer fits in the FV.

But the latter is not supported by the .dsc in current edk2 (and
Juno's FV is not the full size of the available flash, so could be
increased), and the former is cosmetic only.

If you want to, you could add a -Qunused-arguments to the CFLAGS to
get rid of the warnings. But regardless, for the series:

Reviewed-by: Leif Lindholm 
Tested-by: Leif Lindholm 

I guess you need further nods for at least 7/7?

/
Leif

On Mon, Aug 03, 2015 at 12:14:38PM +0200, Ard Biesheuvel wrote:
> This is another followup to the series 'small C model and LLVM/clang support 
> for
> AARCH64' that I sent out on July 17th. Now that the FFS/FV optimization 
> patches
> have been merged, this is what remains to allow the AARCH64 platforms to be
> built using clang, combined with the GNU binutils (cross-)toolchain.
> 
> Patch #1 is a simple fixup to allow clang to assemble the GICv3 support code.
> 
> Patch #2 adds a VOID* cast to a static char array used as an argument for a
> void* formal parameter.
> 
> Patch #3 replaces the ReferenceAcpiTable() entry point functions with VOID*
> pointers. Since this is a dummy entry point that is only intended to prevent 
> the
> optimizer from pruning the table it refers to, we can replace it with no loss 
> of
> functionality. This way, we don't need to impose 4 KB section alignment, since
> the binary contains no executable code anymore.
> 
> Patch #4 sets the correct TE alignment for ArmJuno.fdf, which is required for
> the 4 KB section aligned binaries that we build when using clang and its small
> code model. It also sets the 'FIXED' FFS attribute which is required to enable
> the FFS/FV padding optimization logic.
> 
> Patch #5 does the same as patch #4 for ArmVExpress-FVP-AArch64.fdf
> 
> Patch #6 relaxes the relocation checks in GenFw to allow relative relocations
> for AARCH64, but only if they don't require recalculation. This is needed to
> work around an issue with GNU binutils 2.24 and older, which emits incorrect
> relocation information into fully linked ELF binaries (i.e., when using the
> --emit-relocs option). This patch supersedes 'BaseTools/GenFw: allow AArch64
> small model relocations' sent out July 17th.
> 
> Patch #7 adds the tool definition for CLANG as a member of the GCC family.
> 
> Changes since v1:
> - dropped patch that sets the TE alignment for ArmVirtQemu, it has been merged
>   in the mean time since it also fixed a latent build error (SVN r18122)
> - added patches #2 .. #5 that address minor issues that break the build for
>   CLANG
> - added -mstrict-align to CLANG CC flags, this is required to prevent CLANG 
> from
>   emitting unaligned memory references that may cause an abort when executed
>   before the MMU is up
> 
> Build and boot tested with ArmVirtQemu and ArmVExpressPkg-FVP built in both
> DEBUG and RELEASE modes using:
> 
> Ubuntu clang version 3.5.0-4ubuntu2 (tags/RELEASE_350/final) (based on LLVM 
> 3.5.0)
> Target: aarch64
> 
> Ard Biesheuvel (7):
>   ArmPkg/GicV3: use GICv3 generic sysreg names only for GNU as
>   ArmPkg/GenericWatchdogDxe: add missing VOID* cast
>   ArmPlatformPkg/ArmJunoPkg: use a rodata symbol for ReferenceAcpiTable
>   ArmPlatformPkg/ArmJunoPkg: use TE 'auto' alignment for SEC modules
>   ArmPlatformPkg/FVP: use 'auto' alignment and FIXED placement for XIP
> modules
>   BaseTools/GenFw: allow AArch64 tiny and small code model relocations
>   BaseTools: add support for CLANG compiler to GCC family
> 
>  ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S| 10 +++
>  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c|  2 +-
>  ArmPlatformPkg/ArmJunoPkg/AcpiTables/Facs.aslc| 16 ++--
>  ArmPlatformPkg/ArmJunoPkg/AcpiTables/Fadt.aslc| 16 ++--
>  ArmPlatformPkg/ArmJunoPkg/AcpiTables/Gtdt.aslc| 16 ++--
>  ArmPlatformPkg/ArmJunoPkg/AcpiTables/Madt.aslc| 16 ++--
>  ArmPlatformPkg/ArmJunoPkg/ArmJuno.fdf |  8 +-
>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpress-FVP-AArch64.fdf | 12 +--
>  BaseTools/Conf/tools_def.template | 51 +++
>  BaseTools/Source/C/GenFw/Elf64Convert.c   | 92 
> +++-
>  10 files changed, 141 insertions(+), 98 deletions(-)
> 
> -- 
> 1.9.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] apparent SMBASE relocation issue with noexec enabled [was: MdeModulePkg DxeIpl: Add stack NX support]

[Laszlo Ersek]

On 08/07/15 12:38, Paolo Bonzini wrote:
> 
> 
> On 07/08/2015 01:02, Laszlo Ersek wrote:
 The trace covers the full lifetime of the guest (I started tracing
 before launching the guest, and I passed -no-reboot to qemu, so when the
 guest crashed, QEMU exited.)

 This was on 3.10.0-299.el7.x86_64.
>> I repeated the test with EPT off. The guest doesn't crash this way, it spins 
>> in a busy loop.
>>
>>  qemu-system-i38-32767 [002] 55142.911133: kvm_emulate_insn: 0:7ffd790b: 
>> 0f aa
>>  qemu-system-i38-32767 [002] 55142.911139: kvm_cpuid:func 
>> 8001 rax 6e8 rbx 0 rcx 0 rdx 10
>>  qemu-system-i38-32767 [002] 55142.911148: kvm_enter_smm:vcpu 0: 
>> leaving SMM, smbase 0x7ffc
>>  qemu-system-i38-32767 [002] 55142.911150: kvm_mmu_get_page: existing sp 
>> gfn 7fe65 1/2 q3 --- !pge !nxe root 0 sync
>>  qemu-system-i38-32767 [002] 55142.911151: kvm_mmu_get_page: existing sp 
>> gfn 7fe66 1/2 q3 --- !pge !nxe root 0 sync
>>  qemu-system-i38-32767 [002] 55142.911151: kvm_mmu_get_page: existing sp 
>> gfn 7fe67 1/2 q3 --- !pge !nxe root 0 sync
>>  qemu-system-i38-32767 [002] 55142.911151: kvm_mmu_get_page: existing sp 
>> gfn 7fe68 1/2 q3 --- !pge !nxe root 0 sync
>>  qemu-system-i38-32767 [002] 55142.911152: kvm_entry:vcpu 0
>>  qemu-system-i38-32767 [002] 55142.911152: kvm_exit: reason 
>> EXCEPTION_NMI rip 0x7ffdb6b2 info 7fe88760 8b0e
>>  qemu-system-i38-32767 [002] 55142.911153: kvm_page_fault:   address 
>> 7fe88760 error_code b
>>
>> And then the last triplet is repeated infinitely.
>>
>> 0x7ffdb6b2 is the address of the same first instruction executed after the 
>> RSM.
>>
>> The address 0x7fe88760 seems to fall into an EfiBootServicesData allocation, 
>> made in PEI (via a suitable HOB):
>>
>> Memory Allocation 0x0004 0x7FE69000 - 0x7FE88FFF
> 
> You probably should use "-cpu model,-lm,-nx".  EFER is not part of the
> 32-bit state save map, so the EFER.NXE bit is not restored correctly on
> exit from SMM if you emulate a 32-bit CPU.

That did the trick, thank you! I added



under /domain/cpu in the libvirt XML, and the SMM stuff works fine again.

I'm going to update

  [edk2] [PATCH 58/58] OvmfPkg: README: document SMM status
  http://thread.gmane.org/gmane.comp.bios.edk2.devel/329/focus=382

with the -nx feature flag.

Thanks!
Laszlo

> I have not debugged yet why it works without KVM, nor why the symptoms
> are different between EPT and non-EPT.
> 
> Paolo
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] UefiCpuPkg CpuDxe: Sync up the settings of Execute Disable to APs

[Laszlo Ersek]

On 08/07/15 04:14, Star Zeng wrote:
> when stack NX has been enabled for BSP.
> 
> DxeIpl may have enabled Execute Disable for BSP,
> APs need to get the status and sync up the settings,
> otherwise EFI_MP_SERVICES_PROTOCOL->StartupAllAPs
> may not work.
> 
> Got positive comments and test result from Laszlo
> for the early draft patch, thanks.
> 
> Cc: Laszlo Ersek 
> Cc: Jeff Fan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  UefiCpuPkg/CpuDxe/ApStartup.c | 133 
> +-
>  1 file changed, 132 insertions(+), 1 deletion(-)

Tested-by: Laszlo Ersek 
Reviewed-by: Laszlo Ersek 

Thanks
Laszlo

> 
> diff --git a/UefiCpuPkg/CpuDxe/ApStartup.c b/UefiCpuPkg/CpuDxe/ApStartup.c
> index 7613b47..6f617a0 100644
> --- a/UefiCpuPkg/CpuDxe/ApStartup.c
> +++ b/UefiCpuPkg/CpuDxe/ApStartup.c
> @@ -19,6 +19,47 @@
>  #pragma pack(1)
>  
>  typedef struct {
> +  UINT8  MoveIa32EferMsrToEcx[5];
> +  UINT8  ReadIa32EferMsr[2];
> +  UINT8  SetExecuteDisableBitEnableBit[4];
> +  UINT8  WriteIa32EferMsr[2];
> +
> +#if defined (MDE_CPU_IA32)
> +  UINT8  MovEaxCr3;
> +  UINT32 Cr3Value;
> +  UINT8  MovCr3Eax[3];
> +
> +  UINT8  MoveCr4ToEax[3];
> +  UINT8  SetCr4Bit5[4];
> +  UINT8  MoveEaxToCr4[3];
> +
> +  UINT8  MoveCr0ToEax[3];
> +  UINT8  SetCr0PagingBit[4];
> +  UINT8  MoveEaxToCr0[3];
> +#endif
> +} ENABLE_EXECUTE_DISABLE_CODE;
> +
> +ENABLE_EXECUTE_DISABLE_CODE mEnableExecuteDisableCodeTemplate = {
> +  { 0xB9, 0x80, 0x00, 0x00, 0xC0 },   // mov ecx, 0xc080
> +  { 0x0F, 0x32 }, // rdmsr
> +  { 0x0F, 0xBA, 0xE8, 0x0B }, // bts eax, 11
> +  { 0x0F, 0x30 }, // wrmsr
> +
> +#if defined (MDE_CPU_IA32)
> +  0xB8, 0x,   // mov eax, cr3 value
> +  { 0x0F, 0x22, 0xd8 },   // mov cr3, eax
> +
> +  { 0x0F, 0x20, 0xE0 },   // mov eax, cr4
> +  { 0x0F, 0xBA, 0xE8, 0x05 }, // bts eax, 5
> +  { 0x0F, 0x22, 0xE0 },   // mov cr4, eax
> +
> +  { 0x0F, 0x20, 0xC0 },   // mov eax, cr0
> +  { 0x0F, 0xBA, 0xE8, 0x1F }, // bts eax, 31
> +  { 0x0F, 0x22, 0xC0 },   // mov cr0, eax
> +#endif
> +};
> +
> +typedef struct {
>UINT8  JmpToCli[2];
>  
>UINT16 GdtLimit;
> @@ -52,6 +93,12 @@ typedef struct {
>UINT8  MoveFlatDataSelectorFromAxToGs[2];
>UINT8  MoveFlatDataSelectorFromAxToSs[2];
>  
> +  //
> +  // Code placeholder to enable PAE Execute Disable for IA32
> +  // and enable Execute Disable Bit for X64
> +  //
> +  ENABLE_EXECUTE_DISABLE_CODE EnableExecuteDisable;
> +
>  #if defined (MDE_CPU_X64)
>//
>// Transition to X64
> @@ -207,6 +254,17 @@ STARTUP_CODE mStartupCodeTemplate = {
>{ 0x8e, 0xd0 }, // mov ss, ax
>  
>  #if defined (MDE_CPU_X64)
> +  //
> +  // Code placeholder to enable Execute Disable Bit for X64
> +  // Default is all NOP - No Operation
> +  //
> +  {
> +{ 0x90, 0x90, 0x90, 0x90, 0x90 },
> +{ 0x90, 0x90 },
> +{ 0x90, 0x90, 0x90, 0x90 },
> +{ 0x90, 0x90 },
> +  },
> +
>0xB8, 0x,   // mov eax, cr3 value
>{ 0x0F, 0x22, 0xd8 },   // mov cr3, eax
>  
> @@ -226,7 +284,29 @@ STARTUP_CODE mStartupCodeTemplate = {
>0xEA,   // FarJmp32LongMode
>  OFFSET_OF(STARTUP_CODE, MovEaxOrRaxCpuDxeEntry),
>  LINEAR_CODE64_SEL,
> -#endif // defined (MDE_CPU_X64)
> +#else
> +  //
> +  // Code placeholder to enable PAE Execute Disable for IA32
> +  // Default is all NOP - No Operation
> +  //
> +  {
> +{ 0x90, 0x90, 0x90, 0x90, 0x90 },
> +{ 0x90, 0x90 },
> +{ 0x90, 0x90, 0x90, 0x90 },
> +{ 0x90, 0x90 },
> +
> +0x90, 0x90909090,
> +{ 0x90, 0x90, 0x90 },
> +
> +{ 0x90, 0x90, 0x90 },
> +{ 0x90, 0x90, 0x90, 0x90 },
> +{ 0x90, 0x90, 0x90 },
> +
> +{ 0x90, 0x90, 0x90 },
> +{ 0x90, 0x90, 0x90, 0x90 },
> +{ 0x90, 0x90, 0x90 },
> +  },
> +#endif
>  
>//0xeb, 0xfe,   // jmp $
>  #if defined (MDE_CPU_X64)
> @@ -241,6 +321,48 @@ STARTUP_CODE mStartupCodeTemplate = {
>  volatile STARTUP_CODE *StartupCode = NULL;
>  
>  /**
> +  The function will check if BSP Execute Disable is enabled.
> +  DxeIpl may have enabled Execute Disable for BSP,
> +  APs need to get the status and sync up the settings.
> +
> +  @retval TRUE  BSP Execute Disable is enabled.
> +  @retval FALSE BSP Execute Disable is not enabled.
> +
> +**/
> +BOOLEAN
> +IsBspExecuteDisableEnabled (
> +  VOID
> +  )
> +{
> +  UINT32RegEax;
> +  UINT32RegEdx;
> +  UINT64MsrRegisters;
> +  BOOLEAN   Enabled;
> +
> +  Enabled = FALSE;
> +  AsmCpuid (0x8000, &RegEax, NULL, NULL, NULL);
> +  if (RegEax >= 0x8001) {
> +AsmCpuid (0x8001, NULL, NULL, NULL, &RegEdx);
> +//
> +// Cpuid 0x8001
> +// Bit 20: Execute Disable Bit available.
> +//
> +if ((RegEdx & BIT20) != 0) {
> +  Msr

Re: [edk2] [PATCH] MdeModulePkg DxeIpl: Add stack NX support

[Laszlo Ersek]

On 08/07/15 03:27, Zeng, Star wrote:
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Thursday, August 6, 2015 9:43 PM
>> To: Zeng, Star; Paolo Bonzini
>> Cc: Andrew Fish; Justen, Jordan L; edk2-de...@ml01.01.org; Yao, Jiewen;
>> Chen Fan; Fan, Jeff
>> Subject: Re: [edk2] [PATCH] MdeModulePkg DxeIpl: Add stack NX support

>>> +  if (IsBspExecuteDisableEnabled ()) {
>>> +CopyMem (
>>> +  (VOID*) &StartupCode->EnableExecuteDisble,
>>
>> (7) No need for the (VOID*) cast.
> 
> With (VOID *) cast, I met build failure below.
> 
> f:\svngit\edk2gitsvn\UefiCpuPkg\CpuDxe\ApStartup.c(412) : error C2220: 
> warning treated as error - no 'object' file generated
> f:\svngit\edk2gitsvn\UefiCpuPkg\CpuDxe\ApStartup.c(412) : warning C4090: 
> 'function' : different 'volatile' qualifiers
> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 
> 12.0\Vc\bin\x86_amd64\cl.exe"' : return code '0x2'
> Stop.
> 
> I also saw the cast at  CopyMem ((VOID*) StartupCode, &mStartupCodeTemplate, 
> sizeof (*StartupCode));.

Ah, yes. I missed that the cast was casting away volatile.

Strictly speaking, that's undefined behavior in ISO C (and it's also why
the compiler warns about it). But, since this pattern exists already, I
guess we can stick with it.


> 
>>
>>> +  &mEnableExecuteDisbleCodeTemplate,
>>> +  sizeof (ENABLE_EXECUTE_DISABLE_CODE)
>>> +  );
>>> +  }
>>>  #if defined (MDE_CPU_X64)
>>>StartupCode->Cr3Value = (UINT32) AsmReadCr3 ();
>>>StartupCode->LongJmpOffset += (UINT32) StartAddress;
>>> +#else
>>> +  StartupCode->EnableExecuteDisble.Cr3Value = (UINT32) AsmReadCr3 ();
>>
>> (8) I think the (UINT32) cast can be dropped in this branch, but it's not
>> important.
> 
> Prefer to keep it as Cr3Value is defined as UINT32 although UINTN == UINT32 
> at IA32 build.

Okay.

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [RFC PATCH] ArmPlatformPkg/PlatformIntelBdsLib: remove ARM BDS dependency

[Ard Biesheuvel]

The Intel BDS platform library still depends on the ARM BDS specific
BdsLib. So replace its invocations with GenericBdsLib counterparts,
and fix up where neeeded.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 .../Library/PlatformIntelBdsLib/IntelBdsPlatform.c   | 20 ++--
 .../Library/PlatformIntelBdsLib/IntelBdsPlatform.h   |  1 -
 .../PlatformIntelBdsLib/PlatformIntelBdsLib.inf  |  1 -
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c 
b/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
index c82f27fb4edd..1949365f7d09 100644
--- a/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
+++ b/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c
@@ -63,8 +63,10 @@ GetConsoleDevicePathFromVariable (
   CHAR16*   NextDevicePathStr;
   EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
 
-  Status = GetGlobalEnvironmentVariable (ConsoleVarName, NULL, NULL, 
(VOID**)&DevicePathInstances);
-  if (EFI_ERROR(Status)) {
+  Size = 0;
+
+  DevicePathInstances = BdsLibGetVariableAndSize (ConsoleVarName, 
&gEfiGlobalVariableGuid, &Size);
+  if (DevicePathInstances == NULL) {
 // In case no default console device path has been defined we assume a 
driver handles the console (eg: SimpleTextInOutSerial)
 if ((DefaultConsolePaths == NULL) || (DefaultConsolePaths[0] == L'\0')) {
   *DevicePaths = NULL;
@@ -74,8 +76,6 @@ GetConsoleDevicePathFromVariable (
 Status = gBS->LocateProtocol (&gEfiDevicePathFromTextProtocolGuid, NULL, 
(VOID **)&EfiDevicePathFromTextProtocol);
 ASSERT_EFI_ERROR(Status);
 
-DevicePathInstances = NULL;
-
 // Extract the Device Path instances from the multi-device path string
 while ((DefaultConsolePaths != NULL) && (DefaultConsolePaths[0] != L'\0')) 
{
   NextDevicePathStr = StrStr (DefaultConsolePaths, L";");
@@ -141,7 +141,15 @@ InitializeConsolePipe (
   while (ConsoleDevicePaths != NULL) {
 DevicePath = GetNextDevicePathInstance (&ConsoleDevicePaths, &Size);
 
-Status = BdsConnectDevicePath (DevicePath, Handle, NULL);
+Status = BdsLibConnectDevicePath (DevicePath);
+if (!EFI_ERROR (Status)) {
+  //
+  // If BdsLibConnectDevicePath () succeeded, *Handle must have a non-NULL
+  // value. So ASSERT that this is the case.
+  //
+  gBS->LocateDevicePath (&gEfiDevicePathProtocolGuid, &DevicePath, Handle);
+  ASSERT (*Handle != NULL);
+}
 DEBUG_CODE_BEGIN();
   if (EFI_ERROR(Status)) {
 // We convert back to the text representation of the device Path
@@ -171,7 +179,7 @@ InitializeConsolePipe (
   if (*Interface == NULL) {
 Status = gBS->LocateHandleBuffer (ByProtocol, Protocol, NULL, &NoHandles, 
&Buffer);
 if (EFI_ERROR (Status)) {
-  BdsConnectAllDrivers ();
+  BdsLibConnectAll ();
   Status = gBS->LocateHandleBuffer (ByProtocol, Protocol, NULL, 
&NoHandles, &Buffer);
 }
 
diff --git a/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.h 
b/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.h
index a244ac913255..7122d58be7d7 100644
--- a/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.h
+++ b/ArmPlatformPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.h
@@ -19,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/ArmPlatformPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf 
b/ArmPlatformPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
index 07de4cae4824..d47298d01a81 100644
--- a/ArmPlatformPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
+++ b/ArmPlatformPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf
@@ -44,7 +44,6 @@ [Packages]
 [LibraryClasses]
   BaseLib
   BaseMemoryLib
-  BdsLib
   DebugLib
   DevicePathLib
   MemoryAllocationLib
-- 
1.9.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] apparent SMBASE relocation issue with noexec enabled [was: MdeModulePkg DxeIpl: Add stack NX support]

[Paolo Bonzini]

On 07/08/2015 01:02, Laszlo Ersek wrote:
>> > The trace covers the full lifetime of the guest (I started tracing
>> > before launching the guest, and I passed -no-reboot to qemu, so when the
>> > guest crashed, QEMU exited.)
>> > 
>> > This was on 3.10.0-299.el7.x86_64.
> I repeated the test with EPT off. The guest doesn't crash this way, it spins 
> in a busy loop.
> 
>  qemu-system-i38-32767 [002] 55142.911133: kvm_emulate_insn: 0:7ffd790b: 
> 0f aa
>  qemu-system-i38-32767 [002] 55142.911139: kvm_cpuid:func 
> 8001 rax 6e8 rbx 0 rcx 0 rdx 10
>  qemu-system-i38-32767 [002] 55142.911148: kvm_enter_smm:vcpu 0: 
> leaving SMM, smbase 0x7ffc
>  qemu-system-i38-32767 [002] 55142.911150: kvm_mmu_get_page: existing sp 
> gfn 7fe65 1/2 q3 --- !pge !nxe root 0 sync
>  qemu-system-i38-32767 [002] 55142.911151: kvm_mmu_get_page: existing sp 
> gfn 7fe66 1/2 q3 --- !pge !nxe root 0 sync
>  qemu-system-i38-32767 [002] 55142.911151: kvm_mmu_get_page: existing sp 
> gfn 7fe67 1/2 q3 --- !pge !nxe root 0 sync
>  qemu-system-i38-32767 [002] 55142.911151: kvm_mmu_get_page: existing sp 
> gfn 7fe68 1/2 q3 --- !pge !nxe root 0 sync
>  qemu-system-i38-32767 [002] 55142.911152: kvm_entry:vcpu 0
>  qemu-system-i38-32767 [002] 55142.911152: kvm_exit: reason 
> EXCEPTION_NMI rip 0x7ffdb6b2 info 7fe88760 8b0e
>  qemu-system-i38-32767 [002] 55142.911153: kvm_page_fault:   address 
> 7fe88760 error_code b
> 
> And then the last triplet is repeated infinitely.
> 
> 0x7ffdb6b2 is the address of the same first instruction executed after the 
> RSM.
> 
> The address 0x7fe88760 seems to fall into an EfiBootServicesData allocation, 
> made in PEI (via a suitable HOB):
> 
> Memory Allocation 0x0004 0x7FE69000 - 0x7FE88FFF

You probably should use "-cpu model,-lm,-nx".  EFER is not part of the
32-bit state save map, so the EFER.NXE bit is not restored correctly on
exit from SMM if you emulate a 32-bit CPU.

I have not debugged yet why it works without KVM, nor why the symptoms
are different between EPT and non-EPT.

Paolo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] some new BFD warnings

[Ard Biesheuvel]

On 5 August 2015 at 18:00, Laszlo Ersek  wrote:
> On 08/05/15 17:17, Ard Biesheuvel wrote:
>>
>>
>>> On 5 aug. 2015, at 15:53, Laszlo Ersek  wrote:
>>>
>>> Hi,
>>>
>>> I assume that it is in response to the recent BaseTools changes that now 
>>> I'm getting the following warnings during the OvmfIa32.dsc build:
>>>
>>> BFD: 
>>> Build/OvmfIa32/DEBUG_GCC48/IA32/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe/DEBUG/CapsuleRuntimeDxe.dll:
>>>  warning: Empty loadable segment detected, is this intentional ?
>>
>> Which build step throws this warning?
>> I haven't seen it, and i did test x64. May have missed it, though: our build 
>> log output isn't exactly terse ...
>
> I think you have missed it. It doesn't abort the build, and it is buried
> between the other build messages. I only have these messages in such a
> compact form because I build with a wrapper script that, as one of the
> last steps, greps the build log for warnings.
>
> The warnings seem to be emitted by the "objcopy" utility:
>
> objcopy --strip-unneeded -R .eh_frame
> Build/OvmfX64/DEBUG_GCC48/X64/PcAtChipsetPkg/KbcResetDxe/Reset/DEBUG/KbcReset.dll
>
> BFD:
> Build/OvmfX64/DEBUG_GCC48/X64/PcAtChipsetPkg/KbcResetDxe/Reset/DEBUG/KbcReset.dll:
> warning: Empty loadable segment detected, is this intentional ?
>

OK, first of all, the warning is indeed harmless, as you suspected.

The issue is caused by the fact that, due to some heuristic used by
the linker, the .eh_frame section now gets its own PT_LOAD program
header, which means that we end up with an empty loadable segment
after dropping this section.

Old situation (from KbcReset.dll before doing the objcopy):

Program Headers:
  Type   Offset VirtAddr   PhysAddr
 FileSizMemSiz  Flags  Align
  LOAD   0x00c0 0x0240 0x0240
 0x5a38 0x5a38  RWE20
  GNU_STACK  0x 0x 0x
 0x 0x  RWE10

 Section to Segment mapping:
  Segment Sections...
   00 .text .data .eh_frame
   01

New situation:

Program Headers:
  Type   Offset VirtAddr   PhysAddr
 FileSizMemSiz  Flags  Align
  LOAD   0x0100 0x0240 0x0240
 0x5110 0x5150  RWE20
  LOAD   0x5210 0x53a0 0x53a0
 0x08f8 0x08f8  R  8
  GNU_STACK  0x 0x 0x
 0x 0x  RWE10

 Section to Segment mapping:
  Segment Sections...
   00 .text .data
   01 .eh_frame
   02

If I revert SVN r18137 ("BaseTools GCC: move AutoGen.obj contents to
.text section"), things go back to the old situation, so it is not
entirely clear how the decision is made to allocate a separate
loadable segment for .eh_frame

I can work around it by using (NOLOAD) for the .eh_frame section
address rather than ALIGN(CONSTANT(COMMONPAGESIZE)), but since we are
discarding it anyway (even from the .dlls under DEBUG/), we're
probably better off discarding it altogether (i.e., add .eh_frame to
/DISCARD/)

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

Re: [edk2] [Patch 0/3] Use new BDS and UiApp for OvmfPkg

[Ni, Ruiyu]

Laszlo,
Lots of questions were raised. :)
Q1. can you please tell me more about the new BDS and UiApp? What the 
motivation for it was, what it does? Do the GenericBdsLib functions continue to 
work?
Q2. given that ArmVirtPkg is not being modified, it appears that GenericBdsLib 
is not planned (in the short/mid term at least) to be phased out
The BDS in IntelFrameworkModulePkg mixed the UI part and the real BDS part 
together that makes platform impossible to integrate a simpler UI but still use 
the BDS part. That's the main reason we create the new BDS and also the core 
sample UiApp.
Other reasons are more related to just the BDS part. The interaction between 
old BDS and platform is not very clear. The boot option management in old BDS 
is a mess. It checks whether a boot option is valid by connecting to it, which 
is unexpected from caller's perspective. I cannot remember all the reasons now. 
Overall the new BDS is much more cleaner than the old BDS.
GenericBdsLib still works but I will not integrate new UEFI features to it. The 
goal is every platform uses new BDS and old BDS will be removed when no one 
cares about it.
I didn't change ArmVirtPkg is because I don't know how to even bulid the 
package. So I just throw the OvmfPkg changes out and if someone can help we can 
co-work on that package to also use the new BDS.
The reason I want to enable the new BDS is to prove that new BDS is feasible to 
not only Intel close sourced platforms but also open sourced platforms.

Q3. push the series to a public repo
Sure. the URL is: https://github.com/niruiyu/edk2/commits/OVMF_NEWBDS
There are three commits. Check it and tell me whether it fits the GIT working 
style:)
I clone edk2 master in my local disk. Then I created a OVMF_NEWBDS branch in 
local and checked in the three patches locally.
Then I forked a edk2 in my personal GIT repo in github. Then Then I configure 
my GIT to add a new "remote" name "niruiyu" pointing to my personal GIT repo in 
github. Then I push the OVMF_NEWBDS local branch to "niruiyu".
Did I do the whole thing correctly? Any unnecessary steps?

Q4: A build flag to use new/old BDS
Standing on my point (my reason is just to prove the new BDS's design), I am ok 
to use a build flag and default to use old BDS. But I am afraid that the build 
flag will always point to old BDS and new features enabling will only be 
developed and tested with old BDS. In a very near future when someone change 
the build flag to point to new BDS, the OVMF even cannot build.
My understanding is the change only impacts boot timeout and boot order 
override from QEMU, and the legacy boot. I've already tested the boot timeout 
override. If we can prove that boot order override from QEMU and legacy boot 
still works, we are safe to just use the new BDS.
What do you think?


-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Tuesday, August 4, 2015 7:10 PM
To: Ni, Ruiyu 
Cc: edk2-de...@ml01.01.org; David Woodhouse 
Subject: Re: [edk2] [Patch 0/3] Use new BDS and UiApp for OvmfPkg

On 08/04/15 12:53, Laszlo Ersek wrote:

> No matter how carefully
> we review and test the new code, something will inevitably break,

This wasn't meant as lack of trust in your code; it's just that there
are many cases and corner cases in the related OVMF code, and it's quite
hard to test them all.

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel