Date   

[Patch v5 02/48] UefiCpuPkg/MpInitLib: Add microcode definitions defined in IA32 SDM

Jeff Fan <jeff.fan@...>
 

Add microcode definitions defined in Intel(R) 64 and IA-32 Architectures
Software Developer's Manual Volume 3A, Section 9.11.

v4:
1. ProcessorSignature type changed to CPU_MICROCODE_PROCESSOR_SIGNATURE
2. Add pack(1) for structure CPU_MICROCODE_HEADER and
CPU_MICROCODE_EXTENDED_TABLE.
v3:
1. Update SDM date to June, 2016
2. Mention BCD format in CPU_MICROCODE_DATE
3. Rename ProcessorChecksum to Checksum to match SDM.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Giri P Mudusuru <giri.p.mudusuru@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/Include/Register/Microcode.h | 200 ++++++++++++++++++++++++++++++++
1 file changed, 200 insertions(+)
create mode 100644 UefiCpuPkg/Include/Register/Microcode.h

diff --git a/UefiCpuPkg/Include/Register/Microcode.h b/UefiCpuPkg/Include/Register/Microcode.h
new file mode 100644
index 0000000..94529a1
--- /dev/null
+++ b/UefiCpuPkg/Include/Register/Microcode.h
@@ -0,0 +1,200 @@
+/** @file
+ Microcode Definitions.
+
+ Microcode Definitions based on contents of the
+ Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ Volume 3A, Section 9.11 Microcode Definitions
+
+ Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+ @par Specification Reference:
+ Intel(R) 64 and IA-32 Architectures Software Developer's Manual, Volume 3A,
+ June 2016, Chapter 9 Processor Management and Initialization, Section 9-11.
+
+**/
+
+#ifndef __MICROCODE_H__
+#define __MICROCODE_H__
+
+///
+/// CPU Microcode Date in BCD format
+///
+typedef union {
+ struct {
+ UINT32 Year:16;
+ UINT32 Day:8;
+ UINT32 Month:8;
+ } Bits;
+ UINT32 Uint32;
+} CPU_MICROCODE_DATE;
+
+///
+/// CPU Microcode Processor Signature format
+///
+typedef union {
+ struct {
+ UINT32 Stepping:4;
+ UINT32 Model:4;
+ UINT32 Family:4;
+ UINT32 Type:2;
+ UINT32 Reserved1:2;
+ UINT32 ExtendedModel:4;
+ UINT32 ExtendedFamily:8;
+ UINT32 Reserved2:4;
+ } Bits;
+ UINT32 Uint32;
+} CPU_MICROCODE_PROCESSOR_SIGNATURE;
+
+#pragma pack (1)
+
+///
+/// Microcode Update Format definition
+///
+typedef struct {
+ ///
+ /// Version number of the update header
+ ///
+ UINT32 HeaderVersion;
+ ///
+ /// Unique version number for the update, the basis for the update
+ /// signature provided by the processor to indicate the current update
+ /// functioning within the processor. Used by the BIOS to authenticate
+ /// the update and verify that the processor loads successfully. The
+ /// value in this field cannot be used for processor stepping identification
+ /// alone. This is a signed 32-bit number.
+ ///
+ UINT32 UpdateRevision;
+ ///
+ /// Date of the update creation in binary format: mmddyyyy (e.g.
+ /// 07/18/98 is 07181998H).
+ ///
+ CPU_MICROCODE_DATE Date;
+ ///
+ /// Extended family, extended model, type, family, model, and stepping
+ /// of processor that requires this particular update revision (e.g.,
+ /// 00000650H). Each microcode update is designed specifically for a
+ /// given extended family, extended model, type, family, model, and
+ /// stepping of the processor.
+ /// The BIOS uses the processor signature field in conjunction with the
+ /// CPUID instruction to determine whether or not an update is
+ /// appropriate to load on a processor. The information encoded within
+ /// this field exactly corresponds to the bit representations returned by
+ /// the CPUID instruction.
+ ///
+ CPU_MICROCODE_PROCESSOR_SIGNATURE ProcessorSignature;
+ ///
+ /// Checksum of Update Data and Header. Used to verify the integrity of
+ /// the update header and data. Checksum is correct when the
+ /// summation of all the DWORDs (including the extended Processor
+ /// Signature Table) that comprise the microcode update result in
+ /// 00000000H.
+ ///
+ UINT32 Checksum;
+ ///
+ /// Version number of the loader program needed to correctly load this
+ /// update. The initial version is 00000001H
+ ///
+ UINT32 LoaderRevision;
+ ///
+ /// Platform type information is encoded in the lower 8 bits of this 4-
+ /// byte field. Each bit represents a particular platform type for a given
+ /// CPUID. The BIOS uses the processor flags field in conjunction with
+ /// the platform Id bits in MSR (17H) to determine whether or not an
+ /// update is appropriate to load on a processor. Multiple bits may be set
+ /// representing support for multiple platform IDs.
+ ///
+ UINT32 ProcessorFlags;
+ ///
+ /// Specifies the size of the encrypted data in bytes, and must be a
+ /// multiple of DWORDs. If this value is 00000000H, then the microcode
+ /// update encrypted data is 2000 bytes (or 500 DWORDs).
+ ///
+ UINT32 DataSize;
+ ///
+ /// Specifies the total size of the microcode update in bytes. It is the
+ /// summation of the header size, the encrypted data size and the size of
+ /// the optional extended signature table. This value is always a multiple
+ /// of 1024.
+ ///
+ UINT32 TotalSize;
+ ///
+ /// Reserved fields for future expansion.
+ ///
+ UINT8 Reserved[12];
+} CPU_MICROCODE_HEADER;
+
+///
+/// Extended Signature Table Header Field Definitions
+///
+typedef struct {
+ ///
+ /// Specifies the number of extended signature structures (Processor
+ /// Signature[n], processor flags[n] and checksum[n]) that exist in this
+ /// microcode update
+ ///
+ UINT32 ExtendedSignatureCount;
+ ///
+ /// Checksum of update extended processor signature table. Used to
+ /// verify the integrity of the extended processor signature table.
+ /// Checksum is correct when the summation of the DWORDs that
+ /// comprise the extended processor signature table results in
+ /// 00000000H.
+ ///
+ UINT32 ExtendedChecksum;
+ ///
+ /// Reserved fields.
+ ///
+ UINT8 Reserved[12];
+} CPU_MICROCODE_EXTENDED_TABLE_HEADER;
+
+///
+/// Extended Signature Table Field Definitions
+///
+typedef struct {
+ ///
+ /// Extended family, extended model, type, family, model, and stepping
+ /// of processor that requires this particular update revision (e.g.,
+ /// 00000650H). Each microcode update is designed specifically for a
+ /// given extended family, extended model, type, family, model, and
+ /// stepping of the processor.
+ /// The BIOS uses the processor signature field in conjunction with the
+ /// CPUID instruction to determine whether or not an update is
+ /// appropriate to load on a processor. The information encoded within
+ /// this field exactly corresponds to the bit representations returned by
+ /// the CPUID instruction.
+ ///
+ CPU_MICROCODE_PROCESSOR_SIGNATURE ProcessorSignature;
+ ///
+ /// Platform type information is encoded in the lower 8 bits of this 4-
+ /// byte field. Each bit represents a particular platform type for a given
+ /// CPUID. The BIOS uses the processor flags field in conjunction with
+ /// the platform Id bits in MSR (17H) to determine whether or not an
+ /// update is appropriate to load on a processor. Multiple bits may be set
+ /// representing support for multiple platform IDs.
+ ///
+ UINT32 ProcessorFlag;
+ ///
+ /// Used by utility software to decompose a microcode update into
+ /// multiple microcode updates where each of the new updates is
+ /// constructed without the optional Extended Processor Signature
+ /// Table.
+ /// To calculate the Checksum, substitute the Primary Processor
+ /// Signature entry and the Processor Flags entry with the
+ /// corresponding Extended Patch entry. Delete the Extended Processor
+ /// Signature Table entries. The Checksum is correct when the
+ /// summation of all DWORDs that comprise the created Extended
+ /// Processor Patch results in 00000000H.
+ ///
+ UINT32 Checksum;
+} CPU_MICROCODE_EXTENDED_TABLE;
+
+#pragma pack ()
+
+#endif
--
2.7.4.windows.1


[Patch v5 01/48] UefiCpuPkg/LocalApic.h: Remove duplicated/conflicted definitions

Jeff Fan <jeff.fan@...>
 

#define MSR_IA32_APIC_BASE_ADDRESS is duplicated with #define MSR_IA32_APIC_BASE
defined in UefiCpuPkg/Include/Register/ArchitecturalMsr.h, so we could remove it
and update the modules to use MSR_IA32_APIC_BASE from ArchitecturalMsr.h.

Structure MSR_IA32_APIC_BASE conflicts with #define MSR_IA32_APIC_BASE defined
in UefiCpuPkg/Include/Register/ArchitecturalMsr.h, so we could remove it and
update the modules to use structure MSR_IA32_APIC_BASE_REGISTER from
ArchitecturalMsr.h.

v5:
1. Update SourceLevelDebugPkg to use APIC Base MSR from ArchitecturalMsr.h.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Giri P Mudusuru <giri.p.mudusuru@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
Reviewed-by: Giri P Mudusuru <giri.p.mudusuru@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
.../DebugAgent/DebugAgentCommon/DebugAgent.h | 1 +
.../Library/DebugAgent/DebugAgentCommon/DebugMp.c | 5 ++-
UefiCpuPkg/CpuMpPei/CpuMpPei.h | 1 +
UefiCpuPkg/CpuMpPei/PeiMpServices.c | 20 ++++-----
UefiCpuPkg/Include/Register/LocalApic.h | 20 +--------
UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 29 ++++++------
.../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 51 +++++++++++-----------
7 files changed, 58 insertions(+), 69 deletions(-)

diff --git a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h
index 64e4c3e..18b93a3 100644
--- a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h
+++ b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h
@@ -34,6 +34,7 @@
#include <Library/PrintLib.h>
#include <Library/PeCoffGetEntryPointLib.h>
#include <Library/PeCoffExtraActionLib.h>
+#include <Register/ArchitecturalMsr.h>

#include <TransferProtocol.h>
#include <ImageDebugSupport.h>
diff --git a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugMp.c b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugMp.c
index bdb6742..db9eb6a 100644
--- a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugMp.c
+++ b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugMp.c
@@ -141,6 +141,8 @@ IsBsp (
IN UINT32 ProcessorIndex
)
{
+ MSR_IA32_APIC_BASE_REGISTER MsrApicBase;
+
//
// If there are less than 2 CPUs detected, then the currently executing CPU
// must be the BSP. This avoids an access to an MSR that may not be supported
@@ -150,7 +152,8 @@ IsBsp (
return TRUE;
}

- if (AsmMsrBitFieldRead64 (MSR_IA32_APIC_BASE_ADDRESS, 8, 8) == 1) {
+ MsrApicBase.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
+ if (MsrApicBase.Bits.BSP == 1) {
if (mDebugMpContext.BspIndex != ProcessorIndex) {
AcquireMpSpinLock (&mDebugMpContext.MpContextSpinLock);
mDebugMpContext.BspIndex = ProcessorIndex;
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
index b2e578b..0d1a14a 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
@@ -25,6 +25,7 @@

#include <Register/Cpuid.h>
#include <Register/LocalApic.h>
+#include <Register/Msr.h>

#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
diff --git a/UefiCpuPkg/CpuMpPei/PeiMpServices.c b/UefiCpuPkg/CpuMpPei/PeiMpServices.c
index e784377..e06fdf1 100644
--- a/UefiCpuPkg/CpuMpPei/PeiMpServices.c
+++ b/UefiCpuPkg/CpuMpPei/PeiMpServices.c
@@ -1,7 +1,7 @@
/** @file
Implementation of Multiple Processor PPI services.

- Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -729,9 +729,9 @@ PeiSwitchBSP (
IN BOOLEAN EnableOldBSP
)
{
- PEI_CPU_MP_DATA *PeiCpuMpData;
- UINTN CallerNumber;
- MSR_IA32_APIC_BASE ApicBaseMsr;
+ PEI_CPU_MP_DATA *PeiCpuMpData;
+ UINTN CallerNumber;
+ MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;

PeiCpuMpData = GetMpHobData ();
if (PeiCpuMpData == NULL) {
@@ -774,9 +774,9 @@ PeiSwitchBSP (
//
// Clear the BSP bit of MSR_IA32_APIC_BASE
//
- ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS);
- ApicBaseMsr.Bits.Bsp = 0;
- AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64);
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
+ ApicBaseMsr.Bits.BSP = 0;
+ AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64);

PeiCpuMpData->BSPInfo.State = CPU_SWITCH_STATE_IDLE;
PeiCpuMpData->APInfo.State = CPU_SWITCH_STATE_IDLE;
@@ -805,9 +805,9 @@ PeiSwitchBSP (
//
// Set the BSP bit of MSR_IA32_APIC_BASE on new BSP
//
- ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS);
- ApicBaseMsr.Bits.Bsp = 1;
- AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64);
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
+ ApicBaseMsr.Bits.BSP = 1;
+ AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64);
//
// Set old BSP enable state
//
diff --git a/UefiCpuPkg/Include/Register/LocalApic.h b/UefiCpuPkg/Include/Register/LocalApic.h
index 346cce6..cfb6d76 100644
--- a/UefiCpuPkg/Include/Register/LocalApic.h
+++ b/UefiCpuPkg/Include/Register/LocalApic.h
@@ -1,7 +1,7 @@
/** @file
IA32 Local APIC Definitions.

- Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -16,11 +16,6 @@
#define __LOCAL_APIC_H__

//
-// Definitions for IA32 architectural MSRs
-//
-#define MSR_IA32_APIC_BASE_ADDRESS 0x1B
-
-//
// Definition for Local APIC registers and related values
//
#define XAPIC_ID_OFFSET 0x20
@@ -53,19 +48,6 @@
#define LOCAL_APIC_DESTINATION_SHORTHAND_ALL_INCLUDING_SELF 2
#define LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF 3

-typedef union {
- struct {
- UINT32 Reserved0:8; ///< Reserved.
- UINT32 Bsp:1; ///< Processor is BSP.
- UINT32 Reserved1:1; ///< Reserved.
- UINT32 Extd:1; ///< Enable x2APIC mode.
- UINT32 En:1; ///< xAPIC global enable/disable.
- UINT32 ApicBaseLow:20; ///< APIC Base physical address. The actual field width depends on physical address width.
- UINT32 ApicBaseHigh:32;
- } Bits;
- UINT64 Uint64;
-} MSR_IA32_APIC_BASE;
-
//
// Local APIC Version Register.
//
diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
index 1fca66e..8d0fb02 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
@@ -3,7 +3,7 @@

This local APIC library instance supports xAPIC mode only.

- Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -15,6 +15,7 @@
**/

#include <Register/Cpuid.h>
+#include <Register/Msr.h>
#include <Register/LocalApic.h>

#include <Library/BaseLib.h>
@@ -67,7 +68,7 @@ GetLocalApicBaseAddress (
VOID
)
{
- MSR_IA32_APIC_BASE ApicBaseMsr;
+ MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;

if (!LocalApicBaseAddressMsrSupported ()) {
//
@@ -77,10 +78,10 @@ GetLocalApicBaseAddress (
return PcdGet32 (PcdCpuLocalApicBaseAddress);
}

- ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS);
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);

- return (UINTN)(LShiftU64 ((UINT64) ApicBaseMsr.Bits.ApicBaseHigh, 32)) +
- (((UINTN)ApicBaseMsr.Bits.ApicBaseLow) << 12);
+ return (UINTN)(LShiftU64 ((UINT64) ApicBaseMsr.Bits.ApicBaseHi, 32)) +
+ (((UINTN)ApicBaseMsr.Bits.ApicBase) << 12);
}

/**
@@ -97,7 +98,7 @@ SetLocalApicBaseAddress (
IN UINTN BaseAddress
)
{
- MSR_IA32_APIC_BASE ApicBaseMsr;
+ MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;

ASSERT ((BaseAddress & (SIZE_4KB - 1)) == 0);

@@ -108,12 +109,12 @@ SetLocalApicBaseAddress (
return;
}

- ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS);
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);

- ApicBaseMsr.Bits.ApicBaseLow = (UINT32) (BaseAddress >> 12);
- ApicBaseMsr.Bits.ApicBaseHigh = (UINT32) (RShiftU64((UINT64) BaseAddress, 32));
+ ApicBaseMsr.Bits.ApicBase = (UINT32) (BaseAddress >> 12);
+ ApicBaseMsr.Bits.ApicBaseHi = (UINT32) (RShiftU64((UINT64) BaseAddress, 32));

- AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64);
+ AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64);
}

/**
@@ -246,18 +247,18 @@ GetApicMode (
{
DEBUG_CODE (
{
- MSR_IA32_APIC_BASE ApicBaseMsr;
+ MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;

//
// Check to see if the CPU supports the APIC Base Address MSR
//
if (LocalApicBaseAddressMsrSupported ()) {
- ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS);
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
//
// Local APIC should have been enabled
//
- ASSERT (ApicBaseMsr.Bits.En != 0);
- ASSERT (ApicBaseMsr.Bits.Extd == 0);
+ ASSERT (ApicBaseMsr.Bits.EN != 0);
+ ASSERT (ApicBaseMsr.Bits.EXTD == 0);
}
}
);
diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
index 38f5370..4c42696 100644
--- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
@@ -4,7 +4,7 @@
This local APIC library instance supports x2APIC capable processors
which have xAPIC and x2APIC modes.

- Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -16,6 +16,7 @@
**/

#include <Register/Cpuid.h>
+#include <Register/Msr.h>
#include <Register/LocalApic.h>

#include <Library/BaseLib.h>
@@ -68,7 +69,7 @@ GetLocalApicBaseAddress (
VOID
)
{
- MSR_IA32_APIC_BASE ApicBaseMsr;
+ MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;

if (!LocalApicBaseAddressMsrSupported ()) {
//
@@ -78,10 +79,10 @@ GetLocalApicBaseAddress (
return PcdGet32 (PcdCpuLocalApicBaseAddress);
}

- ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS);
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);

- return (UINTN)(LShiftU64 ((UINT64) ApicBaseMsr.Bits.ApicBaseHigh, 32)) +
- (((UINTN)ApicBaseMsr.Bits.ApicBaseLow) << 12);
+ return (UINTN)(LShiftU64 ((UINT64) ApicBaseMsr.Bits.ApicBaseHi, 32)) +
+ (((UINTN)ApicBaseMsr.Bits.ApicBase) << 12);
}

/**
@@ -98,7 +99,7 @@ SetLocalApicBaseAddress (
IN UINTN BaseAddress
)
{
- MSR_IA32_APIC_BASE ApicBaseMsr;
+ MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;

ASSERT ((BaseAddress & (SIZE_4KB - 1)) == 0);

@@ -109,12 +110,12 @@ SetLocalApicBaseAddress (
return;
}

- ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS);
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);

- ApicBaseMsr.Bits.ApicBaseLow = (UINT32) (BaseAddress >> 12);
- ApicBaseMsr.Bits.ApicBaseHigh = (UINT32) (RShiftU64((UINT64) BaseAddress, 32));
+ ApicBaseMsr.Bits.ApicBase = (UINT32) (BaseAddress >> 12);
+ ApicBaseMsr.Bits.ApicBaseHi = (UINT32) (RShiftU64((UINT64) BaseAddress, 32));

- AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64);
+ AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64);
}

/**
@@ -301,7 +302,7 @@ GetApicMode (
VOID
)
{
- MSR_IA32_APIC_BASE ApicBaseMsr;
+ MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;

if (!LocalApicBaseAddressMsrSupported ()) {
//
@@ -310,12 +311,12 @@ GetApicMode (
return LOCAL_APIC_MODE_XAPIC;
}

- ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS);
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
//
// Local APIC should have been enabled
//
- ASSERT (ApicBaseMsr.Bits.En != 0);
- if (ApicBaseMsr.Bits.Extd != 0) {
+ ASSERT (ApicBaseMsr.Bits.EN != 0);
+ if (ApicBaseMsr.Bits.EXTD != 0) {
return LOCAL_APIC_MODE_X2APIC;
} else {
return LOCAL_APIC_MODE_XAPIC;
@@ -339,8 +340,8 @@ SetApicMode (
IN UINTN ApicMode
)
{
- UINTN CurrentMode;
- MSR_IA32_APIC_BASE ApicBaseMsr;
+ UINTN CurrentMode;
+ MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;

if (!LocalApicBaseAddressMsrSupported ()) {
//
@@ -355,9 +356,9 @@ SetApicMode (
case LOCAL_APIC_MODE_XAPIC:
break;
case LOCAL_APIC_MODE_X2APIC:
- ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS);
- ApicBaseMsr.Bits.Extd = 1;
- AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64);
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
+ ApicBaseMsr.Bits.EXTD = 1;
+ AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64);
break;
default:
ASSERT (FALSE);
@@ -369,12 +370,12 @@ SetApicMode (
// Transition from x2APIC mode to xAPIC mode is a two-step process:
// x2APIC -> Local APIC disabled -> xAPIC
//
- ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS);
- ApicBaseMsr.Bits.Extd = 0;
- ApicBaseMsr.Bits.En = 0;
- AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64);
- ApicBaseMsr.Bits.En = 1;
- AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64);
+ ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
+ ApicBaseMsr.Bits.EXTD = 0;
+ ApicBaseMsr.Bits.EN = 0;
+ AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64);
+ ApicBaseMsr.Bits.EN = 1;
+ AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64);
break;
case LOCAL_APIC_MODE_X2APIC:
break;
--
2.7.4.windows.1


[Patch v5 00/48] MP Initialize Library

Jeff Fan <jeff.fan@...>
 

We add MP Initialize Library defined in UefiCpuPkg/Include/Library/MpInitLib.h.
It will provide basic functionalities of MP services and could be consumed by
CPU MP PEI and CPU MP DXE to produce CPU MP PPI and CPU MP Protocol. Then most
of code could be shared between PEI and DXE modules.

PeiMpInitLib and DxeMpInitLib are added to make the CpuMpPei and CpuDxe more
simply.

I also updated the Ovmf Platform and Quark platform to consume MP Initialize
library.

Thanks Laszlo to verify on OVMF and Mike to verify on Quark.

v5:
1. Update Patches #1, #5, #10 - #12, #14, #16 - #18, #20, #21, #28, #29, #37,
#43.
2. Add Patches #44, #48
(Please see the patches commit log for more details)

v4:
1. Update Patches #2 - #6, #10, #15, #28, #30, #31, #33, #34, #38, #41, #43.
2. Add Patches #7, #8, #42, #44 - #46.
3. Add Reviewed-by: Laszlo Ersek <lersek@redhat.com> on Patches #1, #2.
(Please see the patches commit log for more details)

v3:
1. Update Patch #2, #4 - #8, #28, #33, #36, #38 per Giri's comments to
a. Update SDM date to June, 2016
b. Mention BCD format in CPU_MICROCODE_DATE
c. Rename ProcessorChecksum to Checksum to match SDM.
d. Add whitespace after MpInitLibInitialize
e. Rename MpInitLibSwitchBsp to MpInitLibSwitchBSP to match PI spec.
f. Rename NumApsExecutingLoction to NumApsExecutingLocation
g. Add whitespace after ; in .nasm file
h. Rename *RellocateAp* to *RelocateAp*
2. Update Patch #16, #17, #29-#32 to
a. Use CamelCase for mStopCheckAllApsStatus and CheckAndUpdateApsStatus().
3. Update Patch #36 and #39 to
a. Add PeiMpInitLib instance in UefiCpuPkg.dsc
b. Add DxeMpInitLib instance in UefiCpuPkg.dsc
4. Update Patch #39 and #40 to
a. move the code of consuming MP Initialize library from patch #40 to
patch #39.
5. Update Patch #1, #3 - #8, #16 to
a. Add Reviewed-by: Giri P Mudusuru <giri.p.mudusuru@intel.com>

I fork the whole tree with the updated v3 patches
at https://github.com/vanjeff/edk2/tree/MpInitLibV5 for review.


Jeff Fan (48):
UefiCpuPkg/LocalApic.h: Remove duplicated/conflicted definitions
UefiCpuPkg/MpInitLib: Add microcode definitions defined in IA32 SDM
UefiCpuPkg/CpuS3DataDxe: Move StartupVector allocation to EndOfDxe()
UefiCpuPkg/MpInitLib: Add MP Initialize library class definition
UefiCpuPkg/MpInitLib: Add two instances PeiMpInitLib and DxeMpInitLib
UefiCpuPkg/MpInitLib: Add AP assembly code and MP_CPU_EXCHANGE_INFO
UefiCpuPkg/MpInitLib: Fix typo and clean up the code
UefiCpuPkg/MpInitLib: Add EnableExecuteDisable in MP_CPU_EXCHANGE_INFO
UefiCpuPkg/MpInitLib: Add AsmRelocateApLoop() assembly code
UefiCpuPkg/MpInitLib: Add MP_ASSEMBLY_ADDRESS_MAP
UefiCpuPkg/MpInitLib: Get ApLoopMode and MointorFilter size
UefiCpuPkg/MpInitLib: Allocate and initialize memory of MP Data buffer
UefiCpuPkg/MpInitLib: Initialize CPU_AP_DATA for CPU APs
UefiCpuPkg/MpInitLib: Add CPU_VOLATILE_REGISTERS & worker functions
UefiCpuPkg/MpInitLib: Add MicrocodeDetect() and load microcode on BSP
UefiCpuPkg/MpInitLib: Save CPU MP Data pointer
UefiCpuPkg/MpInitLib: Register one End of PEI callback function
UefiCpuPkg/MpInitLib: Register one period event to check APs status
UefiCpuPkg/MpInitLib: Allocate AP reset vector buffer under 1MB
UefiCpuPkg/MpInitLib: Add ApWakeupFunction() executed by assembly code
UefiCpuPkg/MpInitLib: Fill MP_CPU_EXCHANGE_INFO fields
UefiCpuPkg/MpInitLib: Add WakeUpAP()
UefiCpuPkg/MpInitLib: Send INIT-SIPI-SIPI to get processor count
UefiCpuPkg/MpInitLib: Enable x2APIC mode on BSP/APs
UefiCpuPkg/MpInitLib: Sort processor by ascending order of APIC ID
UefiCpuPkg/MpInitLib: Skip collect processor count if GUIDed HOB exist
UefiCpuPkg/MpInitLib: Implementation of
MpInitLibGetNumberOfProcessors()
UefiCpuPkg/MpInitLib: Implementation of MpInitLibGetProcessorInfo()
UefiCpuPkg/MpInitLib: Implementation of MpInitLibWhoAmI()
UefiCpuPkg/MpInitLib: Implementation of MpInitLibSwitchBSP()
UefiCpuPkg/MpInitLib: Implementation of MpInitLibEnableDisableAP()
UefiCpuPkg/MpInitLib: Check APs Status and update APs status
UefiCpuPkg/MpInitLib: Implementation of MpInitLibStartupThisAP()
UefiCpuPkg/MpInitLib: Implementation of MpInitLibStartupAllAPs()
UefiCpuPkg/MpInitLib: Place APs in safe loop before hand-off to OS
OvmfPkg: Add MpInitLib reference in DSC files.
QuarkPlatformPkg: Add MpInitLib reference in DSC files.
UefiCpuPkg/CpuMpPei: Consume MpInitLib to produce CPU MP PPI services
UefiCpuPkg/CpuMpPei: Remove unused files and codes
UefiCpuPkg/CpuMpPei: Delete PeiMpServices.c and PeiMpServices.h
UefiCpuPkg/CpuDxe: Consume MpInitLib to produce CPU MP Protocol
services
UefiCpuPkg/CpuDxe: Move SetMtrrsFromBuffer() location.
UefiCpuPkg/CpuDxe: Remove unused codes and files
UefiCpuPkg/CpuDxe: Remove PcdCpuMaxLogicalProcessorNumber consuming
MdePkg/MpService.h: Fixed typo in function header to match PI spec
MdePkg/MpService.h: Trim whitespace at end of line
UefiCpuPkg/CpuDxe: Fixed typo in function header to match PI spec
UefiCpuPkg/PiSmmCpuDxeSmm: Add gEfiVariableArchProtocolGuid dependency

MdePkg/Include/Protocol/MpService.h | 490 ++---
OvmfPkg/OvmfPkgIa32.dsc | 2 +
OvmfPkg/OvmfPkgIa32X64.dsc | 2 +
OvmfPkg/OvmfPkgX64.dsc | 2 +
QuarkPlatformPkg/Quark.dsc | 2 +
QuarkPlatformPkg/QuarkMin.dsc | 4 +-
.../DebugAgent/DebugAgentCommon/DebugAgent.h | 1 +
.../Library/DebugAgent/DebugAgentCommon/DebugMp.c | 5 +-
UefiCpuPkg/CpuDxe/ApStartup.c | 478 -----
UefiCpuPkg/CpuDxe/CpuDxe.c | 17 +-
UefiCpuPkg/CpuDxe/CpuDxe.h | 13 +-
UefiCpuPkg/CpuDxe/CpuDxe.inf | 17 +-
UefiCpuPkg/CpuDxe/CpuDxe.uni | 10 +-
UefiCpuPkg/CpuDxe/CpuDxeExtra.uni | 4 +-
UefiCpuPkg/CpuDxe/CpuMp.c | 1306 +------------
UefiCpuPkg/CpuDxe/CpuMp.h | 186 +-
UefiCpuPkg/CpuDxe/Ia32/MpAsm.asm | 76 -
UefiCpuPkg/CpuDxe/Ia32/MpAsm.nasm | 68 -
UefiCpuPkg/CpuDxe/X64/MpAsm.asm | 76 -
UefiCpuPkg/CpuDxe/X64/MpAsm.nasm | 70 -
UefiCpuPkg/CpuMpPei/CpuBist.c | 53 +-
UefiCpuPkg/CpuMpPei/CpuMpPei.c | 1118 ++++-------
UefiCpuPkg/CpuMpPei/CpuMpPei.h | 515 ++---
UefiCpuPkg/CpuMpPei/CpuMpPei.inf | 32 +-
UefiCpuPkg/CpuMpPei/Ia32/MpFuncs.asm | 250 ---
UefiCpuPkg/CpuMpPei/Microcode.h | 58 -
UefiCpuPkg/CpuMpPei/PeiMpServices.c | 956 ----------
UefiCpuPkg/CpuMpPei/PeiMpServices.h | 377 ----
UefiCpuPkg/CpuMpPei/X64/MpFuncs.asm | 290 ---
UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c | 42 +-
UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf | 2 +-
UefiCpuPkg/Include/Library/MpInitLib.h | 353 ++++
UefiCpuPkg/Include/Register/LocalApic.h | 20 +-
UefiCpuPkg/Include/Register/Microcode.h | 200 ++
UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 29 +-
.../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 51 +-
.../MpInitLib/DxeMpInitLib.inf} | 68 +-
.../MpInitLib/DxeMpInitLib.uni} | 12 +-
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 649 +++++++
.../{CpuMpPei => Library/MpInitLib}/Ia32/MpEqu.inc | 4 +-
.../MpInitLib}/Ia32/MpFuncs.nasm | 66 +-
.../{CpuMpPei => Library/MpInitLib}/Microcode.c | 77 +-
UefiCpuPkg/Library/MpInitLib/MpLib.c | 2013 ++++++++++++++++++++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 554 ++++++
.../MpInitLib/PeiMpInitLib.inf} | 60 +-
.../MpInitLib/PeiMpInitLib.uni} | 12 +-
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 643 +++++++
.../{CpuMpPei => Library/MpInitLib}/X64/MpEqu.inc | 6 +-
.../MpInitLib}/X64/MpFuncs.nasm | 84 +-
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 +-
UefiCpuPkg/UefiCpuPkg.dec | 4 +
UefiCpuPkg/UefiCpuPkg.dsc | 4 +
52 files changed, 5777 insertions(+), 5658 deletions(-)
delete mode 100644 UefiCpuPkg/CpuDxe/ApStartup.c
delete mode 100644 UefiCpuPkg/CpuDxe/Ia32/MpAsm.asm
delete mode 100644 UefiCpuPkg/CpuDxe/Ia32/MpAsm.nasm
delete mode 100644 UefiCpuPkg/CpuDxe/X64/MpAsm.asm
delete mode 100644 UefiCpuPkg/CpuDxe/X64/MpAsm.nasm
delete mode 100644 UefiCpuPkg/CpuMpPei/Ia32/MpFuncs.asm
delete mode 100644 UefiCpuPkg/CpuMpPei/Microcode.h
delete mode 100644 UefiCpuPkg/CpuMpPei/PeiMpServices.c
delete mode 100644 UefiCpuPkg/CpuMpPei/PeiMpServices.h
delete mode 100644 UefiCpuPkg/CpuMpPei/X64/MpFuncs.asm
create mode 100644 UefiCpuPkg/Include/Library/MpInitLib.h
create mode 100644 UefiCpuPkg/Include/Register/Microcode.h
copy UefiCpuPkg/{CpuMpPei/CpuMpPei.inf => Library/MpInitLib/DxeMpInitLib.inf} (52%)
copy UefiCpuPkg/{CpuDxe/CpuDxeExtra.uni => Library/MpInitLib/DxeMpInitLib.uni} (53%)
create mode 100644 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
rename UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/Ia32/MpEqu.inc (88%)
rename UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/Ia32/MpFuncs.nasm (77%)
rename UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/Microcode.c (68%)
create mode 100644 UefiCpuPkg/Library/MpInitLib/MpLib.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/MpLib.h
copy UefiCpuPkg/{CpuMpPei/CpuMpPei.inf => Library/MpInitLib/PeiMpInitLib.inf} (58%)
copy UefiCpuPkg/{CpuDxe/CpuDxeExtra.uni => Library/MpInitLib/PeiMpInitLib.uni} (53%)
create mode 100644 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
rename UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/X64/MpEqu.inc (88%)
rename UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/X64/MpFuncs.nasm (73%)

--
2.7.4.windows.1


Re: [PATCH v5 5/8] ArmPkg: add prebuilt glue binaries for GCC5 LTO support

Leif Lindholm <leif.lindholm@...>
 

On Mon, Aug 01, 2016 at 10:01:34AM +0200, Ard Biesheuvel wrote:
GCC in LTO mode interoperates poorly with non-standard libraries that
provide implementations of compiler intrinsics such as memcpy/memset
or the stack protector entry points. Such libraries need to be built
in non-LTO mode, and then referenced explicitly on the linker command
line using a -plugin-opt=-pass-through=-lxxx linker option.

However, if these intrinsics are also referenced directly, the LTO
version of the code will be pulled in, and will happily satisfy all
other references to the same symbol.

So add a pair of glue libraries, for ARM and AARCH64, that reference
the known intrinsics. Since the binaries live under ArmPkg directly,
we can reference them in tools_def.txt. Under LD garbage collection,
the object itself will be pruned, and so will the intrinsics that end
up unused by the module.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Jordan Justen <jordan.l.justen@intel.com>
If everyone else is happy about this binary file inclusion, I
certainly don't object (given that the source is included).

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

---
ArmPkg/Library/GccLto/liblto-aarch64.a | Bin 0 -> 1016 bytes
ArmPkg/Library/GccLto/liblto-aarch64.s | 27 +++++++++
ArmPkg/Library/GccLto/liblto-arm.a | Bin 0 -> 2096 bytes
ArmPkg/Library/GccLto/liblto-arm.s | 61 ++++++++++++++++++++
4 files changed, 88 insertions(+)

diff --git a/ArmPkg/Library/GccLto/liblto-aarch64.a b/ArmPkg/Library/GccLto/liblto-aarch64.a
new file mode 100644
index 0000000000000000000000000000000000000000..2ab00238f0dad882abf08a1fb9623c9cdea9f17b
GIT binary patch
literal 1016
zcmbu7&rZTX5XPqz6oLo!M8btcV>pmaJb3R#Pab@Ovb0qUG$GwJk&}<*)yMErJh~su
zhGA(FBh#ci^V@G{X8(NLKR&dgh`dGgNxR5Xq8|a14Nj;_ot@whUR;}*D0W|+#ne8)
z+crcqtbp?TKuy$d;Fk^js)5sWPGwPMt2G8wSV~i4b+$;e`67MRugg8~@}{d?w$pJf
z%hU2Z13wYMF8ko8f}aWQH5;VNy0m&m%Ghc<&b?O^ORa42Zb{|ZYEm;}M9QPwkz0*h
zki8>ef}gYSE})e*bOFvFk<j^57EYNXKak(^fcXvc@Z~)5d^m*lCr*Hz|6PAkvlcbK
oxX>*EVPSp5Eivz1-~TrQyaBwMaQ{8W!rrlD%!Td{2n*}~0;u;a`v3p{

literal 0
HcmV?d00001

diff --git a/ArmPkg/Library/GccLto/liblto-aarch64.s b/ArmPkg/Library/GccLto/liblto-aarch64.s
new file mode 100644
index 000000000000..45000d327758
--- /dev/null
+++ b/ArmPkg/Library/GccLto/liblto-aarch64.s
@@ -0,0 +1,27 @@
+//
+// Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
+//
+// This program and the accompanying materials are licensed and made available under
+// the terms and conditions of the BSD License that accompanies this distribution.
+// The full text of the license may be found at
+// http://opensource.org/licenses/bsd-license.php.
+//
+// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+//
+
+//
+// GCC in LTO mode interoperates poorly with non-standard libraries that
+// provide implementations of compiler intrinsics such as memcpy/memset
+// or the stack protector entry points.
+//
+// By referencing these functions from a non-LTO object that can be passed
+// to the linker via the -plugin-opt=-pass-through=-lxxx options, the
+// intrinsics are included in the link in a way that allows them to be
+// pruned again if no other references to them exist.
+//
+
+ .long memcpy - .
+ .long memset - .
+ .long __stack_chk_fail - .
+ .long __stack_chk_guard - .
diff --git a/ArmPkg/Library/GccLto/liblto-arm.a b/ArmPkg/Library/GccLto/liblto-arm.a
new file mode 100644
index 0000000000000000000000000000000000000000..d811c09573a35ea87a8002ecf01be18e1c6e7fd3
GIT binary patch
literal 2096
zcmd6o%WKq76vn?XQ*C|Js#WW|YOD1@u(oJH7cHm=wjd(Xg%C3{sS}#eGD-SEunQLz
zT?o1mx)E36&V>ti;!4n^e}Mi6u3h<^n@J{fT<Fp}Ouq9w_nvbfNlqSKoxD~mm5{X(
zhR`D5^G4ItF=}K8T}U0-`2R^Kc5yYX=T>}_x@dNmx{6!*W2si#P63O*VzW?IBihqh
z=)ig*pojKb#bw326`xc*toV}R>x!op&nRA0ysG%I;^&HAD}JZ=gW^w$zbO8u_=n=3
zihn6C7jA)^cemm`#e<4%#TOM%D88Zij$&7Fpm;^`6UFO_-zdgF4UQAVZgtkF)@Pj=
z*ALnp_Y=1vL)@s|sQDwQ6*Mh*7aYIlFNiybaLxo6PTG16rQHlllhBAv-k>#u2@Sol
zI=`G}CPrQiN;tRR(ak(*AdNItn6xb{AamTrttlr6972!~lYGJ?&mg`uh4`8leFjDu
zR1H=l|GXG+(@3h}e9gF`ML(|A$Jm)#Ny{9*kb6fYIz6K#U~GZXiE;<`AQQJZh#Ex*
zvPafpsg(EM+QeEU%F9+!7AJXjt<6BM=oX+)l${4fw*md4-N1n8cCac_8FW^32XIbw
zCm?m%V%-}PWwOhnEHdMwdw?sVdjY8%7AKh$-3Qzh-4EOrJpf1@u{il%(L=yJ(ZfJZ
z^axNF?FRzUqrklAF(4K_4lIdsu@6KCfmP8Hz#~x>xiwL4;;HB<;F;)Y;DzWJUhHT&
zjNJ+~ZlqeztcDlZv9}b%uDP)byAnmP`PA5M95?(*5_=I7{9EHzOij<eVx#1jh0yHv
z<B{-Nm!6|^Pj~Rl*~wdJ;>*-d{<&4d*_Y!hx!AINvPBvHw{hmarpIg2NWNZUrI#!p
wAAvlV^sI41<6<;hHcoUy=A?e-|05l;7C8giM-Tt9*KBPx@rv+1OG3`f-+dwCzyJUM

literal 0
HcmV?d00001

diff --git a/ArmPkg/Library/GccLto/liblto-arm.s b/ArmPkg/Library/GccLto/liblto-arm.s
new file mode 100644
index 000000000000..bc16320a46c0
--- /dev/null
+++ b/ArmPkg/Library/GccLto/liblto-arm.s
@@ -0,0 +1,61 @@
+//
+// Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
+//
+// This program and the accompanying materials are licensed and made available under
+// the terms and conditions of the BSD License that accompanies this distribution.
+// The full text of the license may be found at
+// http://opensource.org/licenses/bsd-license.php.
+//
+// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+//
+
+//
+// GCC in LTO mode interoperates poorly with non-standard libraries that
+// provide implementations of compiler intrinsics such as memcpy/memset
+// or the stack protector entry points.
+//
+// By referencing these functions from a non-LTO object that can be passed
+// to the linker via the -plugin-opt=-pass-through=-lxxx options, the
+// intrinsics are included in the link in a way that allows them to be
+// pruned again if no other references to them exist.
+//
+
+ .long memcpy - .
+ .long memset - .
+ .long __stack_chk_fail - .
+ .long __stack_chk_guard - .
+ .long __ashrdi3 - .
+ .long __ashldi3 - .
+ .long __aeabi_idiv - .
+ .long __aeabi_idivmod - .
+ .long __aeabi_uidiv - .
+ .long __aeabi_uidivmod - .
+ .long __divdi3 - .
+ .long __divsi3 - .
+ .long __lshrdi3 - .
+ .long __aeabi_memcpy - .
+ .long __aeabi_memset - .
+ .long memmove - .
+ .long __modsi3 - .
+ .long __moddi3 - .
+ .long __muldi3 - .
+ .long __aeabi_lmul - .
+ .long __ARM_ll_mullu - .
+ .long __udivsi3 - .
+ .long __umodsi3 - .
+ .long __udivdi3 - .
+ .long __umoddi3 - .
+ .long __udivmoddi4 - .
+ .long __clzsi2 - .
+ .long __ctzsi2 - .
+ .long __ucmpdi2 - .
+ .long __switch8 - .
+ .long __switchu8 - .
+ .long __switch16 - .
+ .long __switch32 - .
+ .long __aeabi_ulcmp - .
+ .long __aeabi_uldivmod - .
+ .long __aeabi_ldivmod - .
+ .long __aeabi_llsr - .
+ .long __aeabi_llsl - .
--
2.7.4


Re: [PATCH] OvmfPkg: use StatusCode Router and Handler from MdeModulePkg

Laszlo Ersek
 

On 08/02/16 03:13, Laszlo Ersek wrote:

(1) So, here's what I would like to see in the commit message:

----------

In the Platform Init v1.4a spec,
- Volume 1 "4.7 Status Code Service" defines the
EFI_PEI_REPORT_STATUS_CODE.ReportStatusCode() service,
sorry, the above is a typo, I meant EFI_PEI_SERVICES.ReportStatusCode().


- Volume 1 "6.3.5 Status Code PPI (Optional)" defines the
EFI_PEI_PROGRESS_CODE_PPI (equivalent to the above),
- Volume 2 "14.2 Status Code Runtime Protocol" defines the
EFI_STATUS_CODE_PROTOCOL.

These allow PEIMs and DXE (and later) modules to report status codes.

Currently OvmfPkg uses modules from under
"IntelFrameworkModulePkg/Universal/StatusCode/", which produce the above
abstractions (PPI and PROTOCOL) directly, and write the status codes, as
they are reported, to the serial port or to a memory buffer. This is
called "handling" the status codes.

In the Platform Init v1.4a spec,
- Volume 3 "7.2.2 Report Status Code Handler PPI" defines
EFI_PEI_RSC_HANDLER_PPI,
- Volume 3 "7.2.1 Report Status Code Handler Protocol" defines
EFI_RSC_HANDLER_PROTOCOL.

These allow PEIMs and runtime DXE drivers to register several callbacks
for status code handling.
The woring here would also be better if we said "These allow several
PEIMs and runtime DXE drivers to register callbacks for status code
handling". In other words, "several" should refer to the drivers, not
the callbacks.


MdeModulePkg offers a PEIM under
"MdeModulePkg/Universal/ReportStatusCodeRouter/Pei" that procudes both
EFI_PEI_PROGRESS_CODE_PPI and EFI_PEI_RSC_HANDLER_PPI, and a runtime DXE
driver under "MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe"
that produces both EFI_STATUS_CODE_PROTOCOL and EFI_RSC_HANDLER_PROTOCOL.

MdeModulePkg also offers status code handler modules under
MdeModulePkg/Universal/StatusCodeHandler/ that depend on
EFI_PEI_RSC_HANDLER_PPI and EFI_RSC_HANDLER_PROTOCOL, respectively.

The StatusCodeHandler modules register themselves with
ReportStatusCodeRouter through EFI_PEI_RSC_HANDLER_PPI /
EFI_RSC_HANDLER_PROTOCOL. When another module reports a status code
through EFI_PEI_PROGRESS_CODE_PPI / EFI_STATUS_CODE_PROTOCOL, it reaches
phase-matching ReportStatusCodeRouter module first, which in turn passes
"it reaches [the] phase-matching" -- the definite article missing.

Sorry about these typos, it was very late last night when I wrote the email.

Thanks!
Laszlo

the status code to the pre-registered, phase-matching StatusCodeHandler
module.

The status code handling in the StatusCodeHandler modules is identical
to the one currently provided by the IntelFrameworkModulePkg modules.
Replace the IntelFareworkModulePkg modules with the MdeModulePkg ones,
so we can decrease our dependency on IntelFareworkModulePkg.

----------


[patch] MdeModulePkg/FvSimpleFileSystem: fix assertions when FV is empty

Feng Tian <feng.tian@...>
 

The original code will assert when dealing with those empty FVs.
The fix is used to solve this bug.

Cc: Chao Zhang <chao.b.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian <feng.tian@intel.com>
---
.../Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c | 9 +++++++--
.../FvSimpleFileSystemDxe/FvSimpleFileSystemEntryPoint.c | 6 +++++-
2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
index c6137ac..b81110f 100644
--- a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
+++ b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystem.c
@@ -526,7 +526,10 @@ FvSimpleFileSystemOpen (
InitializeListHead (&NewFile->Link);
InsertHeadList (&Instance->FileHead, &NewFile->Link);

- NewFile->DirReadNext = FVFS_GET_FIRST_FILE_INFO (Instance);
+ NewFile->DirReadNext = NULL;
+ if (!IsListEmpty (&Instance->FileInfoHead)) {
+ NewFile->DirReadNext = FVFS_GET_FIRST_FILE_INFO (Instance);
+ }

*NewHandle = &NewFile->FileProtocol;
return EFI_SUCCESS;
@@ -821,7 +824,9 @@ FvSimpleFileSystemSetPosition (
//
// Reset directory position to first entry
//
- File->DirReadNext = FVFS_GET_FIRST_FILE_INFO (Instance);
+ if (File->DirReadNext) {
+ File->DirReadNext = FVFS_GET_FIRST_FILE_INFO (Instance);
+ }
} else if (Position == 0xFFFFFFFFFFFFFFFFull) {
File->Position = File->FvFileInfo->FileInfo.FileSize;
} else {
diff --git a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystemEntryPoint.c b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystemEntryPoint.c
index 7167fb9..4e6089b 100644
--- a/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystemEntryPoint.c
+++ b/MdeModulePkg/Universal/FvSimpleFileSystemDxe/FvSimpleFileSystemEntryPoint.c
@@ -223,7 +223,11 @@ FvSimpleFileSystemOpenVolume (
}
}

- Instance->Root->DirReadNext = FVFS_GET_FIRST_FILE_INFO (Instance);
+ Instance->Root->DirReadNext = NULL;
+ if (!IsListEmpty (&Instance->FileInfoHead)) {
+ Instance->Root->DirReadNext = FVFS_GET_FIRST_FILE_INFO (Instance);
+ }
+
*RootFile = &Instance->Root->FileProtocol;
return Status;
}
--
2.7.1.windows.2


Re: [Patch] MdeModulePkg LoadFileOnFv2: Fix the potential NULL pointer access

Tian, Feng <feng.tian@...>
 

Reviewed-by: Feng Tian <feng.tian@intel.com>

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Liming Gao
Sent: Tuesday, August 2, 2016 1:39 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch] MdeModulePkg LoadFileOnFv2: Fix the potential NULL pointer access

Check NULL pointer before access it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
index 9eea50d..18a07d8 100644
--- a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
+++ b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
@@ -345,6 +345,9 @@ FvNotificationEvent (
Index = 0;
BufferSize = sizeof (EFI_HANDLE);
Handle = AllocateZeroPool (BufferSize);
+ if (Handle == NULL) {
+ return;
+ }
Status = gBS->LocateHandle (
ByProtocol,
&gEfiFirmwareVolume2ProtocolGuid, @@ -355,6 +358,9 @@ FvNotificationEvent (
if (EFI_BUFFER_TOO_SMALL == Status) {
FreePool (Handle);
Handle = AllocateZeroPool (BufferSize);
+ if (Handle == NULL) {
+ return;
+ }
Status = gBS->LocateHandle (
ByProtocol,
&gEfiFirmwareVolume2ProtocolGuid,
--
2.8.0.windows.1

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


Re: [PATCH] MdePkg: move to 'hidden' visibility for all symbols under GCC/X64

Ard Biesheuvel
 

On 2 August 2016 at 04:49, Gao, Liming <liming.gao@intel.com> wrote:
Reviewed-by: Liming Gao <liming.gao@intel.com>
Thanks
Pushed as 28ade7b802e0

--
Ard.

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Monday, August 01, 2016 2:57 PM
To: Shi, Steven <steven.shi@intel.com>; Zhu, Yonghong
<yonghong.zhu@intel.com>; Gao, Liming <liming.gao@intel.com>; Justen,
Jordan L <jordan.l.justen@intel.com>; edk2-devel@lists.01.org
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH] MdePkg: move to 'hidden' visibility for all symbols under
GCC/X64

When using GCC to build for X64, we switched to the position independent
small code model, which is much more efficient in terms of code generation
and runtime relocation footprint, and produces binaries that can execute
correctly from any offset.

However, the PIC routines are by default geared towards hosted binaries
containing symbol references that may resolve to definitions in other
dynamic objects, and for this reason, external symbol references are
indirected via a GOT entry by default (which also results in a .reloc fixup
entry) unless we annotate them.

For this reason, we introduced the 'protected' visibility annotation for
all symbol definitions and references, by setting the GCC visibility
pragma. However, as it turns out, this is not sufficient for all versions
of GCC, and in some cases (GCC 5.x using the GCC49 toolchain tag), may
still result in GOT based relocations.

So switch to 'hidden' visibility instead, which is slightly stronger, and
fixes this issue for the versions of GCC that exhibit the problem.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
MdePkg/Include/X64/ProcessorBind.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/X64/ProcessorBind.h
b/MdePkg/Include/X64/ProcessorBind.h
index a4aad3e524e8..666cc8e8bd16 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -29,12 +29,12 @@

#if defined(__GNUC__) && defined(__pic__)
//
-// Mark all symbol declarations and references as protected, meaning they
will
+// Mark all symbol declarations and references as hidden, meaning they will
// not be subject to symbol preemption. This allows the compiler to refer to
// symbols directly using relative references rather than via the GOT, which
// contains absolute symbol addresses that are subject to runtime relocation.
//
-#pragma GCC visibility push (protected)
+#pragma GCC visibility push (hidden)
#endif

#if defined(__INTEL_COMPILER)
--
2.7.4


Re: [patch] BaseTool/UPT: Not expand macro for UserExtension

Zhu, Yonghong <yonghong.zhu@...>
 

Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>

Best Regards,
Zhu Yonghong

-----Original Message-----
From: Chen, Hesheng
Sent: Monday, August 01, 2016 11:26 PM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: [patch] BaseTool/UPT: Not expand macro for UserExtension

All MACRO values defined by the DEFINE statements n any section (except [Userextensions] sections other than TianoCore."ExtraFiles) of the INF or DEC file must be expanded before processing of the file.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: hesschen <hesheng.chen@intel.com>
---
BaseTools/Source/Python/UPT/Parser/DecParser.py | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/UPT/Parser/DecParser.py b/BaseTools/Source/Python/UPT/Parser/DecParser.py
index e6658a9..937047f 100644
--- a/BaseTools/Source/Python/UPT/Parser/DecParser.py
+++ b/BaseTools/Source/Python/UPT/Parser/DecParser.py
@@ -270,7 +270,21 @@ class _DecBase:
self._LoggerError(ST.ERR_DECPARSE_BACKSLASH_EMPTY)
CatLine += Line

- self._RawData.CurrentLine = self._ReplaceMacro(CatLine)
+ #
+ # All MACRO values defined by the DEFINE statements in any section
+ # (except [Userextensions] sections for Intel) of the INF or DEC file
+ # must be expanded before processing of the file.
+ #
+ __IsReplaceMacro = True
+ Header = self._RawData.CurrentScope[0] if self._RawData.CurrentScope else None
+ if Header and len(Header) > 2:
+ if Header[0].upper() == 'USEREXTENSIONS' and not (Header[1] == 'TianoCore' and Header[2] == '"ExtraFiles"'):
+ __IsReplaceMacro = False
+ if __IsReplaceMacro:
+ self._RawData.CurrentLine = self._ReplaceMacro(CatLine)
+ else:
+ self._RawData.CurrentLine = CatLine
+
return CatLine, CommentList

## Parse
--
2.7.2.windows.1


[Patch] MdeModulePkg LoadFileOnFv2: Fix the potential NULL pointer access

Liming Gao
 

Check NULL pointer before access it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
index 9eea50d..18a07d8 100644
--- a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
+++ b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
@@ -345,6 +345,9 @@ FvNotificationEvent (
Index = 0;
BufferSize = sizeof (EFI_HANDLE);
Handle = AllocateZeroPool (BufferSize);
+ if (Handle == NULL) {
+ return;
+ }
Status = gBS->LocateHandle (
ByProtocol,
&gEfiFirmwareVolume2ProtocolGuid,
@@ -355,6 +358,9 @@ FvNotificationEvent (
if (EFI_BUFFER_TOO_SMALL == Status) {
FreePool (Handle);
Handle = AllocateZeroPool (BufferSize);
+ if (Handle == NULL) {
+ return;
+ }
Status = gBS->LocateHandle (
ByProtocol,
&gEfiFirmwareVolume2ProtocolGuid,
--
2.8.0.windows.1


Re: [Patch] MdeModulePkg UefiBootManagerLib: Fix VS2012 build failure

Tian, Feng <feng.tian@...>
 

Reviewed-by: Feng Tian <feng.tian@intel.com>

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Liming Gao
Sent: Tuesday, August 2, 2016 10:36 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch] MdeModulePkg UefiBootManagerLib: Fix VS2012 build failure

Initialize local variable Description as NULL first.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index d5818ca..ecd0ae3 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -2213,6 +2213,7 @@ BmRegisterBootManagerMenu (
UINTN DataSize;

DevicePath = NULL;
+ Description = NULL;
//
// Try to find BootMenuApp from LoadFile protocol
//
--
2.8.0.windows.1

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


Re: [Patch] MdeModulePkg LoadFileOnFv2: Correct copy right format

Tian, Feng <feng.tian@...>
 

Reviewed-by: Feng Tian <feng.tian@intel.com>

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Liming Gao
Sent: Tuesday, August 2, 2016 10:36 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch] MdeModulePkg LoadFileOnFv2: Correct copy right format

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
index 6ef9fac..9eea50d 100644
--- a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
+++ b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
@@ -1,8 +1,8 @@
/** @file
Produce Load File Protocol for UEFI Applications in Firmware Volumes

- Copyright (c) 2011 - 2013, Intel Corporation
- All rights reserved. This program and the accompanying materials
+ Copyright (c) 2011 - 2016, Intel Corporation. All rights
+ reserved.<BR> This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
http://opensource.org/licenses/bsd-license.php
--
2.8.0.windows.1

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


Re: [PATCH v5 7/8] MdePkg GCC/X64: avoid 'hidden' visibility for module entry points

Liming Gao
 

Ard:
This update works. It is better. For my question, I get the answer from your previous patch. Please ignore it.

Patches 3&4&6&8 are good to me. Reviewed-by: Liming Gao <liming.gao@intel.com>

Thanks
Liming

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
Gao, Liming
Sent: Tuesday, August 02, 2016 10:39 AM
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Justen, Jordan L <jordan.l.justen@intel.com>; edk2-devel@lists.01.org;
leif.lindholm@linaro.org; lersek@redhat.com
Subject: Re: [edk2] [PATCH v5 7/8] MdePkg GCC/X64: avoid 'hidden' visibility
for module entry points

Ard:
I will verify it. And, I would ask why only X64 requires it? IA32, ARM and
AARCH64 doesn't specially handle it?

Thanks
Liming
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Tuesday, August 02, 2016 12:12 AM
To: Gao, Liming <liming.gao@intel.com>
Cc: Shi, Steven <steven.shi@intel.com>; Zhu, Yonghong
<yonghong.zhu@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
edk2-devel@lists.01.org; lersek@redhat.com; leif.lindholm@linaro.org
Subject: Re: [edk2] [PATCH v5 7/8] MdePkg GCC/X64: avoid 'hidden'
visibility
for module entry points

On 1 August 2016 at 17:51, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:56, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:49, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:18, Gao, Liming <liming.gao@intel.com> wrote:
Ard:
I don't think it is good way to define GCC_VISIBILITY_PROTECTED
and
apply it in EntryPointLib. We only need to expose _ModuleEntryPoint. It
has
been specified in LINK_FLAGS in tools_def.txt. Could we also specify its
attribute in CC_FLAGS or LINK_FLAGS in tools_def.txt?
It seems this does the trick as well

diff --git a/BaseTools/Scripts/GccBase.lds
b/BaseTools/Scripts/GccBase.lds
index 281af8a9bd33..02387d4f8d6f 100644
--- a/BaseTools/Scripts/GccBase.lds
+++ b/BaseTools/Scripts/GccBase.lds
@@ -80,3 +80,7 @@ SECTIONS {
*(COMMON)
}
}
+
+VERSION {
+ { global: _ModuleEntryPoint*; };
+};


Note that * at the end: this is necessary since _ModuleEntryPoint will
be called _ModuleEntryPoint.lto_priv.xxx in the LTO objects.
Hmm, looks like I spoke too soon. I don't know what I did wrong, but
this does not actually work.
The only alternative I can think of is to add a static non-lto object
to the tree that refers to _ModuleEntryPoint, similar to the way I
handle the ARM intrinsics in patch #5
As it turns out, the LTO linker does not need to visibility pragma to
prevent it from emitting GOT based relocations. This makes sense,
considering that the LTO linker can see that no symbol references are
ever satisfied across dynamic object boundaries. That means I could
work around this in the following way:

diff --git a/BaseTools/Conf/tools_def.template
b/BaseTools/Conf/tools_def.template
index 314adaf6bfa8..983e2fea7390 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4462,7 +4462,7 @@ DEFINE GCC49_ARM_ASLDLINK_FLAGS =
DEF(GCC48_ARM_ASLDLINK_FLAGS)
DEFINE GCC49_AARCH64_ASLDLINK_FLAGS =
DEF(GCC48_AARCH64_ASLDLINK_FLAGS)

DEFINE GCC5_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -flto
-fno-builtin
-DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -flto
-fno-builtin
+DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -flto
-fno-builtin -DUSING_LTO
DEFINE GCC5_IA32_X64_DLINK_COMMON =
DEF(GCC49_IA32_X64_DLINK_COMMON)
DEFINE GCC5_IA32_X64_ASLDLINK_FLAGS =
DEF(GCC49_IA32_X64_ASLDLINK_FLAGS)
DEFINE GCC5_IA32_X64_DLINK_FLAGS =
DEF(GCC49_IA32_X64_DLINK_FLAGS) -flto
diff --git a/MdePkg/Include/X64/ProcessorBind.h
b/MdePkg/Include/X64/ProcessorBind.h
index 666cc8e8bd16..77fab7055afc 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -27,12 +27,15 @@
#pragma pack()
#endif

-#if defined(__GNUC__) && defined(__pic__)
+#if defined(__GNUC__) && defined(__pic__) && !defined(USING_LTO)
//
// Mark all symbol declarations and references as hidden, meaning they
will
// not be subject to symbol preemption. This allows the compiler to refer
to
// symbols directly using relative references rather than via the GOT, which
// contains absolute symbol addresses that are subject to runtime
relocation.
+// The LTO linker will not emit GOT based relocations anyway, so there is
no
+// need to set the pragma in that case (and doing so will cause issues of its
+// own)
//
#pragma GCC visibility push (hidden)
#endif

and I can drop this patch.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Foreign keyboard support in UEFI

Senthilarasu_ARUNACH@...
 

Hi,
Any one shed some light on supporting multi language key board support on UEFI application? Scan code received from EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL not returns value for
certain keys in German/Arabic USB keyboard. We are also not sure in mapping UEFI code from EFI_HII_DATABASE_PROTOCOL.GetKeyBoardLayOut()).


Thanks
Senthil


Re: [patch] BaseTool/UPT: Add Test Install

Zhu, Yonghong <yonghong.zhu@...>
 

Please remove the trailing whitespace when commit.

Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>

Best Regards,
Zhu Yonghong

-----Original Message-----
From: Chen, Hesheng
Sent: Friday, July 29, 2016 3:58 PM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: [patch] BaseTool/UPT: Add Test Install

Add a new function to test if a DIST file list one by one to see if they can meet the requirement of Dependency.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: hesschen <hesheng.chen@intel.com>
---
.../Source/Python/UPT/Core/DependencyRules.py | 21 ++++-
BaseTools/Source/Python/UPT/Logger/StringTable.py | 5 ++
BaseTools/Source/Python/UPT/TestInstall.py | 100 +++++++++++++++++++++
BaseTools/Source/Python/UPT/UPT.py | 16 ++++
4 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 BaseTools/Source/Python/UPT/TestInstall.py

diff --git a/BaseTools/Source/Python/UPT/Core/DependencyRules.py b/BaseTools/Source/Python/UPT/Core/DependencyRules.py
index 4608ed6..ee06c53 100644
--- a/BaseTools/Source/Python/UPT/Core/DependencyRules.py
+++ b/BaseTools/Source/Python/UPT/Core/DependencyRules.py
@@ -1,7 +1,7 @@
## @file
# This file is for installed package information database operations # -# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2016, Intel Corporation. All rights
+reserved.<BR>
#
# This program and the accompanying materials are licensed and made available # under the terms and conditions of the BSD License which accompanies this @@ -183,6 +183,25 @@ class DependencyRules(object):
def CheckInstallDpDepexSatisfied(self, DpObj):
self.PkgsToBeDepend = [(PkgInfo[1], PkgInfo[2]) for PkgInfo in self.WsPkgList]
return self.CheckDpDepexSatisfied(DpObj)
+
+ # # Check whether multiple DP depex satisfied by current workspace for Install
+ #
+ # @param DpObjList: A distribution object list
+ # @return: True if distribution depex satisfied
+ # False else
+ #
+ def CheckTestInstallPdDepexSatisfied(self, DpObjList):
+ self.PkgsToBeDepend = [(PkgInfo[1], PkgInfo[2]) for PkgInfo in self.WsPkgList]
+ for DpObj in DpObjList:
+ if self.CheckDpDepexSatisfied(DpObj):
+ for PkgKey in DpObj.PackageSurfaceArea.keys():
+ PkgObj = DpObj.PackageSurfaceArea[PkgKey]
+ self.PkgsToBeDepend.append((PkgObj.Guid, PkgObj.Version))
+ else:
+ return False, DpObj
+
+ return True, DpObj
+

## Check whether a DP depex satisfied by current workspace
# (excluding the original distribution's packages to be replaced) for Replace diff --git a/BaseTools/Source/Python/UPT/Logger/StringTable.py b/BaseTools/Source/Python/UPT/Logger/StringTable.py
index 96f0e1c..4c42661 100644
--- a/BaseTools/Source/Python/UPT/Logger/StringTable.py
+++ b/BaseTools/Source/Python/UPT/Logger/StringTable.py
@@ -858,3 +858,8 @@ HLP_SPECIFY_PACKAGE_NAME_TO_BE_REPLACED = _(
"Specify the UEFI Distribution Package file name to be replaced") HLP_USE_GUIDED_PATHS = _(
"Install packages to the following directory path by default: <PackageName>_<PACKAGE_GUID>_<PACKAGE_VERSION>")
+HLP_TEST_INSTALL = _(
+ "Specify the UEFI Distribution Package filenames to install")
+
+MSG_TEST_INSTALL_PASS = _("All distribution package file are satisfied
+for dependence check.") MSG_TEST_INSTALL_FAIL = _("NOT all distribution
+package file are satisfied for dependence check.")
diff --git a/BaseTools/Source/Python/UPT/TestInstall.py b/BaseTools/Source/Python/UPT/TestInstall.py
new file mode 100644
index 0000000..71fe928
--- /dev/null
+++ b/BaseTools/Source/Python/UPT/TestInstall.py
@@ -0,0 +1,100 @@
+# # @file
+# Test Install distribution package
+#
+# Copyright (c) 2016, Intel Corporation. All rights reserved.<BR> # #
+This program and the accompanying materials are licensed and made
+available # under the terms and conditions of the BSD License which
+accompanies this # distribution. The full text of the license may be
+found at # http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+"""
+Test Install multiple distribution package """
+# #
+# Import Modules
+#
+from Library import GlobalData
+import Logger.Log as Logger
+from Logger import StringTable as ST
+import Logger.ToolError as TE
+from Core.DependencyRules import DependencyRules from InstallPkg import
+UnZipDp
+
+import shutil
+from traceback import format_exc
+from platform import python_version
+from sys import platform
+
+# # Tool entrance method
+#
+# This method mainly dispatch specific methods per the command line options.
+# If no error found, return zero value so the caller of this tool can
+know # if it's executed successfully or not.
+#
+# @param Options: command Options
+#
+def Main(Options=None):
+ ContentZipFile, DistFile = None, None
+ ReturnCode = 0
+
+ try:
+ DataBase = GlobalData.gDB
+ WorkspaceDir = GlobalData.gWORKSPACE
+ if not Options.DistFiles:
+ Logger.Error("TestInstallPkg", TE.OPTION_MISSING,
+ ExtraData=ST.ERR_SPECIFY_PACKAGE)
+
+ DistPkgList = []
+ for DistFile in Options.DistFiles:
+ DistPkg, ContentZipFile, __, DistFile = UnZipDp(WorkspaceDir, DistFile)
+ DistPkgList.append(DistPkg)
+
+ #
+ # check dependency
+ #
+ Dep = DependencyRules(DataBase)
+ Result = True
+ DpObj = None
+ try:
+ Result, DpObj = Dep.CheckTestInstallPdDepexSatisfied(DistPkgList)
+ except:
+ Result = False
+
+ if Result:
+ Logger.Quiet(ST.MSG_TEST_INSTALL_PASS)
+ else:
+ Logger.Quiet(ST.MSG_TEST_INSTALL_FAIL)
+
+ except TE.FatalError, XExcept:
+ ReturnCode = XExcept.args[0]
+ if Logger.GetLevel() <= Logger.DEBUG_9:
+ Logger.Quiet(ST.MSG_PYTHON_ON % (python_version(),
+ platform) + format_exc())
+
+ except Exception, x:
+ ReturnCode = TE.CODE_ERROR
+ Logger.Error(
+ "\nTestInstallPkg",
+ TE.CODE_ERROR,
+ ST.ERR_UNKNOWN_FATAL_INSTALL_ERR % Options.DistFiles,
+ ExtraData=ST.MSG_SEARCH_FOR_HELP,
+ RaiseError=False
+ )
+ Logger.Quiet(ST.MSG_PYTHON_ON % (python_version(), platform) +
+ format_exc())
+
+ finally:
+ Logger.Quiet(ST.MSG_REMOVE_TEMP_FILE_STARTED)
+ if DistFile:
+ DistFile.Close()
+ if ContentZipFile:
+ ContentZipFile.Close()
+ if GlobalData.gUNPACK_DIR:
+ shutil.rmtree(GlobalData.gUNPACK_DIR)
+ GlobalData.gUNPACK_DIR = None
+ Logger.Quiet(ST.MSG_REMOVE_TEMP_FILE_DONE)
+ if ReturnCode == 0:
+ Logger.Quiet(ST.MSG_FINISH)
+ return ReturnCode
+
diff --git a/BaseTools/Source/Python/UPT/UPT.py b/BaseTools/Source/Python/UPT/UPT.py
index 59c4a88..8dd949a 100644
--- a/BaseTools/Source/Python/UPT/UPT.py
+++ b/BaseTools/Source/Python/UPT/UPT.py
@@ -46,6 +46,7 @@ import InstallPkg
import RmPkg
import InventoryWs
import ReplacePkg
+import TestInstall
from Library.Misc import GetWorkspace
from Library import GlobalData
from Core.IpiDb import IpiDatabase
@@ -69,6 +70,9 @@ def CheckConflictOption(Opt):
Logger.Error("UPT", OPTION_CONFLICT, ExtraData=ST.ERR_I_R_EXCLUSIVE)
elif Opt.PackFileToCreate and Opt.PackFileToRemove:
Logger.Error("UPT", OPTION_CONFLICT, ExtraData=ST.ERR_C_R_EXCLUSIVE)
+ elif Opt.TestDistFiles and (Opt.PackFileToCreate or Opt.PackFileToInstall \
+ or Opt.PackFileToRemove or Opt.PackFileToReplace):
+ Logger.Error("UPT", OPTION_CONFLICT,
+ ExtraData=ST.ERR_C_R_EXCLUSIVE)

if Opt.CustomPath and Opt.UseGuidedPkgPath:
Logger.Warn("UPT", ST.WARN_CUSTOMPATH_OVERRIDE_USEGUIDEDPATH)
@@ -146,6 +150,9 @@ def Main():

Parser.add_option("--use-guided-paths", action="store_true", dest="Use_Guided_Paths", help=ST.HLP_USE_GUIDED_PATHS)

+ Parser.add_option("-j", "--test-install", action="append", type="string",
+ dest="Test_Install_Distribution_Package_Files",
+ help=ST.HLP_TEST_INSTALL)
+
Opt = Parser.parse_args()[0]

Var2Var = [
@@ -159,6 +166,7 @@ def Main():
("PackFileToReplace", Opt.Replace_Distribution_Package_File),
("PackFileToBeReplaced", Opt.Original_Distribution_Package_File),
("UseGuidedPkgPath", Opt.Use_Guided_Paths),
+ ("TestDistFiles", Opt.Test_Install_Distribution_Package_Files)
]

for Var in Var2Var:
@@ -265,6 +273,14 @@ def Main():
Opt.PackFileToReplace = AbsPath
RunModule = ReplacePkg.Main

+ elif Opt.Test_Install_Distribution_Package_Files:
+ for Dist in Opt.Test_Install_Distribution_Package_Files:
+ if not Dist.endswith('.dist'):
+ Logger.Error("TestInstall", FILE_TYPE_MISMATCH,
+ ExtraData=ST.ERR_DIST_EXT_ERROR % Dist)
+
+ setattr(Opt, 'DistFiles', Opt.Test_Install_Distribution_Package_Files)
+ RunModule = TestInstall.Main
+
else:
Parser.print_usage()
return OPTION_MISSING
--
2.7.2.windows.1


Re: [patch] BaseTool/Upt: Avoid UNI file name conflict

Zhu, Yonghong <yonghong.zhu@...>
 

Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>

Best Regards,
Zhu Yonghong

-----Original Message-----
From: Chen, Hesheng
Sent: Friday, July 29, 2016 2:09 PM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: [patch] BaseTool/Upt: Avoid UNI file name conflict

When creating a UNI file if there is a name conflict, add an index from 0 to the file name

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: hesschen <hesheng.chen@intel.com>
---
.../Source/Python/UPT/GenMetaFile/GenDecFile.py | 5 +++--
.../Source/Python/UPT/GenMetaFile/GenInfFile.py | 7 +++---
BaseTools/Source/Python/UPT/Library/String.py | 26 +++++++++++++++++++++-
3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py b/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py
index 31abd23..d39c182 100644
--- a/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py
+++ b/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py
@@ -65,6 +65,7 @@ from Library.DataType import TAB_SECTION_END from Library.DataType import TAB_SPLIT import Library.DataType as DT from Library.UniClassObject import FormatUniEntry
+from Library.String import GetUniFileName

def GenPcd(Package, Content):
#
@@ -586,9 +587,9 @@ def GenPackageUNIEncodeFile(PackageObject, UniFileHeader = '', Encoding=TAB_ENCO

if not os.path.exists(os.path.dirname(PackageObject.GetFullPath())):
os.makedirs(os.path.dirname(PackageObject.GetFullPath()))
- ContainerFile = os.path.normpath(os.path.join(os.path.dirname(PackageObject.GetFullPath()),
- (PackageObject.GetBaseName() + '.uni')))

+ ContainerFile =
+ GetUniFileName(os.path.dirname(PackageObject.GetFullPath()),
+ PackageObject.GetBaseName())
+
Content = UniFileHeader + '\r\n'
Content += '\r\n'

diff --git a/BaseTools/Source/Python/UPT/GenMetaFile/GenInfFile.py b/BaseTools/Source/Python/UPT/GenMetaFile/GenInfFile.py
index a131f98..c1362e6 100644
--- a/BaseTools/Source/Python/UPT/GenMetaFile/GenInfFile.py
+++ b/BaseTools/Source/Python/UPT/GenMetaFile/GenInfFile.py
@@ -2,7 +2,7 @@
#
# This file contained the logical of transfer package object to INF files.
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2016, Intel Corporation. All rights
+reserved.<BR>
#
# This program and the accompanying materials are licensed and made available # under the terms and conditions of the BSD License which accompanies this @@ -41,6 +41,7 @@ import Logger.Log as Logger from Library import DataType as DT from GenMetaFile import GenMetaFileMisc from Library.UniClassObject import FormatUniEntry
+from Library.String import GetUniFileName


## Transfer Module Object to Inf files
@@ -225,8 +226,8 @@ def GenModuleUNIEncodeFile(ModuleObject, UniFileHeader='', Encoding=DT.TAB_ENCOD
return
else:
ModuleObject.UNIFlag = True
- ContainerFile = os.path.normpath(os.path.join(os.path.dirname(ModuleObject.GetFullPath()),
- (ModuleObject.GetBaseName() + '.uni')))
+ ContainerFile = GetUniFileName(os.path.dirname(ModuleObject.GetFullPath()), ModuleObject.GetBaseName())
+
if not os.path.exists(os.path.dirname(ModuleObject.GetFullPath())):
os.makedirs(os.path.dirname(ModuleObject.GetFullPath()))

diff --git a/BaseTools/Source/Python/UPT/Library/String.py b/BaseTools/Source/Python/UPT/Library/String.py
index 37ce141..05b5fb1 100644
--- a/BaseTools/Source/Python/UPT/Library/String.py
+++ b/BaseTools/Source/Python/UPT/Library/String.py
@@ -2,7 +2,7 @@
# This file is used to define common string related functions used in parsing
# process
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
#
# This program and the accompanying materials are licensed and made available
# under the terms and conditions of the BSD License which accompanies this
@@ -957,3 +957,27 @@ def IsMatchArch(Arch1, Arch2):
return True

return False
+
+# Search all files in FilePath to find the FileName with the largest index
+# Return the FileName with index +1 under the FilePath
+#
+def GetUniFileName(FilePath, FileName):
+ Files = os.listdir(FilePath)
+ LargestIndex = -1
+ for File in Files:
+ if File.upper().startswith(FileName.upper()) and File.upper().endswith('.UNI'):
+ Index = File.upper().replace(FileName.upper(), '').replace('.UNI', '')
+ if Index:
+ try:
+ Index = int(Index)
+ except Exception:
+ Index = -1
+ else:
+ Index = 0
+ if Index > LargestIndex:
+ LargestIndex = Index + 1
+
+ if LargestIndex > -1:
+ return os.path.normpath(os.path.join(FilePath, FileName + str(LargestIndex) + '.uni'))
+ else:
+ return os.path.normpath(os.path.join(FilePath, FileName + '.uni'))
--
2.7.2.windows.1


Re: [patch 2/2] BaseTool/Upt: Add support for Private

Zhu, Yonghong <yonghong.zhu@...>
 

Please remove the trailing whitespace when commit.

Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>


Best Regards,
Zhu Yonghong

-----Original Message-----
From: Chen, Hesheng
Sent: Friday, July 29, 2016 10:31 AM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: [patch 2/2] BaseTool/Upt: Add support for Private

Support new syntax in package DEC file as below:
[Includes.Common.Private]
[Ppis.Common.Private]
[Guids.Common.Private]
[Protocols.Common.Private]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: hesschen <hesheng.chen@intel.com>
---
.../Source/Python/UPT/GenMetaFile/GenDecFile.py | 9 +++++++-
BaseTools/Source/Python/UPT/Library/DataType.py | 4 +++-
.../Source/Python/UPT/Library/UniClassObject.py | 16 +++++++-------
BaseTools/Source/Python/UPT/Parser/DecParser.py | 25 ++++++++++++++++++++--
.../Python/UPT/PomAdapter/DecPomAlignment.py | 11 +++++++++-
5 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py b/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py
index f22363b..31abd23 100644
--- a/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py
+++ b/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py
@@ -2,7 +2,7 @@
#
# This file contained the logical of transfer package object to DEC files.
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2016, Intel Corporation. All rights
+reserved.<BR>
#
# This program and the accompanying materials are licensed and made available # under the terms and conditions of the BSD License which accompanies this @@ -63,6 +63,7 @@ from Library.DataType import TAB_PCD_ERROR from Library.DataType import TAB_SECTION_START from Library.DataType import TAB_SECTION_END from Library.DataType import TAB_SPLIT
+import Library.DataType as DT
from Library.UniClassObject import FormatUniEntry

def GenPcd(Package, Content):
@@ -487,6 +488,12 @@ def PackageToDec(Package, DistHeader = None):
if UserExtension.GetUserID() == TAB_BINARY_HEADER_USERID and \
UserExtension.GetIdentifier() == TAB_BINARY_HEADER_IDENTIFIER:
continue
+
+ # Generate Private Section first
+ if UserExtension.GetUserID() == DT.TAB_INTEL and UserExtension.GetIdentifier() == DT.TAB_PRIVATE:
+ Content += '\n' + UserExtension.GetStatement()
+ continue
+
Statement = UserExtension.GetStatement()
if not Statement:
continue
diff --git a/BaseTools/Source/Python/UPT/Library/DataType.py b/BaseTools/Source/Python/UPT/Library/DataType.py
index 8449dc8..c151be3 100644
--- a/BaseTools/Source/Python/UPT/Library/DataType.py
+++ b/BaseTools/Source/Python/UPT/Library/DataType.py
@@ -1,7 +1,7 @@
## @file
# This file is used to define class for data type structure # -# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2016, Intel Corporation. All rights
+reserved.<BR>
#
# This program and the accompanying materials are licensed and made available # under the terms and conditions of the BSD License which accompanies this @@ -680,6 +680,8 @@ TAB_DEFINE = 'DEFINE'
TAB_NMAKE = 'Nmake'
TAB_USER_EXTENSIONS = 'UserExtensions'
TAB_INCLUDE = '!include'
+TAB_PRIVATE = 'Private'
+TAB_INTEL = 'Intel'

#
# Common Define
diff --git a/BaseTools/Source/Python/UPT/Library/UniClassObject.py b/BaseTools/Source/Python/UPT/Library/UniClassObject.py
index 1e73d3e..27804cc 100644
--- a/BaseTools/Source/Python/UPT/Library/UniClassObject.py
+++ b/BaseTools/Source/Python/UPT/Library/UniClassObject.py
@@ -328,11 +328,11 @@ class UniFileClassObject(object):
Lang = distutils.util.split_quoted((Line.split(u"//")[0]))
if len(Lang) != 3:
try:
- FileIn = codecs.open(File.Path, mode='rb', encoding='utf_8').read()
+ FileIn = codecs.open(File.Path, mode='rb',
+ encoding='utf_8').readlines()
except UnicodeError, Xstr:
- FileIn = codecs.open(File.Path, mode='rb', encoding='utf_16').read()
+ FileIn = codecs.open(File.Path, mode='rb',
+ encoding='utf_16').readlines()
except UnicodeError, Xstr:
- FileIn = codecs.open(File.Path, mode='rb', encoding='utf_16_le').read()
+ FileIn = codecs.open(File.Path, mode='rb',
+ encoding='utf_16_le').readlines()
except:
EdkLogger.Error("Unicode File Parser",
ToolError.FILE_OPEN_FAILURE, @@ -437,7 +437,7 @@ class UniFileClassObject(object):
# ExtraData='The file %s is either invalid UTF-16LE or it is missing the BOM.' % File.Path)

try:
- FileIn = codecs.open(File.Path, mode='rb', encoding='utf_8').read()
+ FileIn = codecs.open(File.Path, mode='rb',
+ encoding='utf_8').readlines()
except UnicodeError, Xstr:
FileIn = codecs.open(File.Path, mode='rb', encoding='utf_16').readlines()
except UnicodeError:
@@ -579,9 +579,9 @@ class UniFileClassObject(object):
#
if Line.startswith(u'"'):
if StringEntryExistsFlag == 2:
- EdkLogger.Error("Unicode File Parser", ToolError.FORMAT_INVALID,
+ EdkLogger.Error("Unicode File Parser",
+ ToolError.FORMAT_INVALID,
Message=ST.ERR_UNIPARSE_LINEFEED_UP_EXIST % Line, ExtraData=File.Path)
-
+
StringEntryExistsFlag = 1
if not Line.endswith('"'):
EdkLogger.Error("Unicode File Parser", ToolError.FORMAT_INVALID, @@ -589,7 +589,7 @@ class UniFileClassObject(object):
% (LineCount, File.Path))
elif Line.startswith(u'#language'):
if StringEntryExistsFlag == 2:
- EdkLogger.Error("Unicode File Parser", ToolError.FORMAT_INVALID,
+ EdkLogger.Error("Unicode File Parser",
+ ToolError.FORMAT_INVALID,
Message=ST.ERR_UNI_MISS_STRING_ENTRY % Line, ExtraData=File.Path)
StringEntryExistsFlag = 0
else:
@@ -1050,7 +1050,7 @@ class UniFileClassObject(object):
ToolError.FILE_NOT_FOUND,
ExtraData=FilaPath)
try:
- FileIn = codecs.open(FilaPath, mode='rb', encoding='utf_8').read()
+ FileIn = codecs.open(FilaPath, mode='rb',
+ encoding='utf_8').readlines()
except UnicodeError, Xstr:
FileIn = codecs.open(FilaPath, mode='rb', encoding='utf_16').readlines()
except UnicodeError:
diff --git a/BaseTools/Source/Python/UPT/Parser/DecParser.py b/BaseTools/Source/Python/UPT/Parser/DecParser.py
index 23d1ed4..e6658a9 100644
--- a/BaseTools/Source/Python/UPT/Parser/DecParser.py
+++ b/BaseTools/Source/Python/UPT/Parser/DecParser.py
@@ -1,7 +1,7 @@
## @file
# This file is used to parse DEC file. It will consumed by DecParser # -# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2016, Intel Corporation. All rights
+reserved.<BR>
#
# This program and the accompanying materials are licensed and made available # under the terms and conditions of the BSD License which accompanies this @@ -742,7 +742,26 @@ class Dec(_DecBase, _DecComments):
except BaseException:
Logger.Error(TOOL_NAME, FILE_OPEN_FAILURE, File=DecFile,
ExtraData=ST.ERR_DECPARSE_FILEOPEN % DecFile)
- RawData = FileContent(DecFile, Content)
+
+ #
+ # Pre-parser for Private section
+ #
+ self._Private = ''
+ __IsFoundPrivate = False
+ NewContent = []
+ for Line in Content:
+ Line = Line.strip()
+ if Line.startswith(DT.TAB_SECTION_START) and Line.endswith(DT.TAB_PRIVATE + DT.TAB_SECTION_END):
+ __IsFoundPrivate = True
+ if Line.startswith(DT.TAB_SECTION_START) and Line.endswith(DT.TAB_SECTION_END)\
+ and not Line.endswith(DT.TAB_PRIVATE + DT.TAB_SECTION_END):
+ __IsFoundPrivate = False
+ if __IsFoundPrivate:
+ self._Private += Line + '\r'
+ if not __IsFoundPrivate:
+ NewContent.append(Line + '\r')
+
+ RawData = FileContent(DecFile, NewContent)

_DecComments.__init__(self)
_DecBase.__init__(self, RawData) @@ -1060,3 +1079,5 @@ class Dec(_DecBase, _DecComments):
return self._Define.GetDataObject().GetPackageVersion()
def GetPackageUniFile(self):
return self._Define.GetDataObject().GetPackageUniFile()
+ def GetPrivateSections(self):
+ return self._Private
diff --git a/BaseTools/Source/Python/UPT/PomAdapter/DecPomAlignment.py b/BaseTools/Source/Python/UPT/PomAdapter/DecPomAlignment.py
index 11b0359..88edf35 100644
--- a/BaseTools/Source/Python/UPT/PomAdapter/DecPomAlignment.py
+++ b/BaseTools/Source/Python/UPT/PomAdapter/DecPomAlignment.py
@@ -1,7 +1,7 @@
## @file DecPomAlignment.py
# This file contained the adapter for convert INF parser object to POM Object # -# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2016, Intel Corporation. All rights
+reserved.<BR>
#
# This program and the accompanying materials are licensed and made available # under the terms and conditions of the BSD License which accompanies this @@ -63,6 +63,7 @@ from Library.DataType import TAB_STR_TOKENHELP from Library.DataType import TAB_STR_TOKENERR from Library.DataType import TAB_HEX_START from Library.DataType import TAB_SPLIT
+import Library.DataType as DT
from Library.CommentParsing import ParseHeaderCommentSection from Library.CommentParsing import ParseGenericComment from Library.CommentParsing import ParseDecPcdGenericComment @@ -221,6 +222,14 @@ class DecPomAlignment(PackageObject):
self.SetUserExtensionList(
self.GetUserExtensionList() + [UserExtension]
)
+
+ # Add Private sections to UserExtension
+ if self.DecParser.GetPrivateSections():
+ PrivateUserExtension = UserExtensionObject()
+ PrivateUserExtension.SetStatement(self.DecParser.GetPrivateSections())
+ PrivateUserExtension.SetIdentifier(DT.TAB_PRIVATE)
+ PrivateUserExtension.SetUserID(DT.TAB_INTEL)
+ self.SetUserExtensionList(self.GetUserExtensionList() +
+ [PrivateUserExtension])

## Generate miscellaneous files on DEC file
#
--
2.7.2.windows.1


Re: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS definitions with the standardized one

Long, Qin <qin.long@...>
 

The supported cipher suite should be one self-defined scope (I am not sure if any minimal sets for TLS from RFC/Spec. Need to double-check). It's one typical cipher negotiation process in TLS protocol. Tell the peer what I support, what you support, then agree one set supported by both.
We can add other cipher supports according to the real requirements. It's OK to remove those unsupported cipher suite for this phase.


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: Wu, Jiaxin
Sent: Tuesday, August 02, 2016 10:03 AM
To: Palmer, Thomas; Long, Qin; edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan; Gao, Liming
Subject: RE: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS definitions with
the standardized one

Thomas,

Thanks your effort to test the new ciphers, can you provide the info which
one is unsupported currently?

As Qin's comments, "we'd better to keep the current supported cipher suite
for our UEFI- TLS enabling". If so, I agree to remove the unsupported one in
TlsCipherMappingTable instead of changing the current openssl configuration.
If dsa /idea is required in future, then we can consider how to enable the
configuration.

So, can you provide the patch to remove the unsupported one in
TlsCipherMappingTable?


Thanks,
Jiaxin



-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
Palmer, Thomas
Sent: Tuesday, August 2, 2016 9:51 AM
To: Wu, Jiaxin <jiaxin.wu@intel.com>; Long, Qin <qin.long@intel.com>;
edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2] [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
definitions with the standardized one


Hi Jiaxin,

It sounds like we both agree that TlsCipherMappingTable is the list
of what UEFI officially supports. If it is advertised in
TlsCipherMappingTable then OpenSSL needs to support it.

My proposal of removing no-dsa / no-idea and adding weak-ciphers
is
specifically aimed to syncing how OpenSSL is configured/built to what
is in TlsCipherMappingTable. I was busy last week testing out the new
ciphers and realized a few were not even getting configured in OpenSSL.

Thomas

-----Original Message-----
From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
Sent: Monday, August 1, 2016 8:34 PM
To: Palmer, Thomas <thomas.palmer@hpe.com>; Long, Qin
<qin.long@intel.com>; edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
Gao, Liming <liming.gao@intel.com>
Subject: RE: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
definitions with the standardized one

Hi Thomas,

Since the Tls1.h is used to hold the standardized definitions, openssl
part is not taken into consideration. The Cipher Suites added in
Tls1.h only refers to
A.5 of rfc-2246, rfc-4346 and rfc-5246. The criteria is removing all
the limited/insecurity/deprecated ones that specified in RFC -- "Note
that this mode is vulnerable to man-in-the middle attacks and is
therefore deprecated." I know the IDEA and DES cipher suites are also
deprecated in TLS1.2, but it does means to TLS1.1. So, some of them are
still kept in Tls1.h.

As for the TlsCipherMappingTable, it takes on the link between Tls1.h
defined cipher suites and openssl supported cipher suites. If we
eliminate the factors of configuration, I believe the cipher suites in
TlsCipherMappingTable should be implemented in OpenSSL lib. I haven't
check them one by one but openssl official document is referred @
https://www.openssl.org/docs/manmaster/apps/ciphers.html, which
gives
the lists of TLS cipher suites names from the relevant specification
and their OpenSSL equivalents.

If the cipher suites in Tls1.h is not found in TlsCipherMappingTable,
EFI_UNSUPPORTED will be returned. I think it's reasonable.

Thanks,
Jiaxin

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
Of Palmer, Thomas
Sent: Tuesday, August 2, 2016 5:46 AM
To: Long, Qin <qin.long@intel.com>; Wu, Jiaxin
<jiaxin.wu@intel.com>; edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2] [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
definitions with the standardized one

Jiaxin / Qin,


I'm unaware of what criteria is required for a cipher to be in this
TlsCipherMappingTable. I had presumed that it would be b/c UEFI
supported the cipher for TLS operation. If unsupported ciphers are
allowed ... then logically wouldn't we need to add all ciphers?
What advantage do we gain by having an entry in this table but not
actually use
the cipher in communication?

Currently TlsGetCipherString is the only means we have to validate
the cipher string. If a cipher is in the table but not in OpenSSL
lib, then we will provide imperfect feedback if the unsupported
cipher is buried in a list of supported ciphers. OpenSSL will
simply use only the ciphers it supports and quietly drop the unsupported
cipher.
A user that inspects the list of set ciphers would be curious why a
certain
cipher was being "dropped" /
"filtered". However, if TlsGetCipherString sees that the cipher is not in
our
mapping table the TlsSetCipherList function will return immediate
feedback.

I'm not enthralled with supporting weak/idea ciphers either. I
would vouch for us removing those ciphers from
TlsCipherMappingTable. It is not our responsibility to document the
IANA/Description string description in code.

This document
(http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-52
r1
.pdf) would be a good list for initial cipher support. We have some
of the ciphers on the list already. Here is the sorted list:

TLS_DH_DSS_WITH_3DES_EDE_CBC_SHA
TLS_DH_DSS_WITH_AES_128_CBC_SHA
TLS_DH_DSS_WITH_AES_128_CBC_SHA256
TLS_DH_DSS_WITH_AES_128_GCM_SHA256
TLS_DH_DSS_WITH_AES_256_CBC_SHA
TLS_DH_DSS_WITH_AES_256_CBC_SHA256
TLS_DH_DSS_WITH_AES_256_GCM_SHA384
TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA
TLS_DHE_DSS_WITH_AES_128_CBC_SHA
TLS_DHE_DSS_WITH_AES_128_CBC_SHA256
TLS_DHE_DSS_WITH_AES_128_GCM_SHA256
TLS_DHE_DSS_WITH_AES_256_CBC_SHA
TLS_DHE_DSS_WITH_AES_256_CBC_SHA256
TLS_DHE_DSS_WITH_AES_256_GCM_SHA384
TLS_ECDH_ECDSA_WITH_3DES_EDE_CBC_SHA
TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA
TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA256
TLS_ECDH_ECDSA_WITH_AES_128_GCM_SHA256
TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA
TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA384
TLS_ECDH_ECDSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
TLS_RSA_WITH_3DES_EDE_CBC_SHA
TLS_RSA_WITH_AES_128_CBC_SHA
TLS_RSA_WITH_AES_128_CBC_SHA256
TLS_RSA_WITH_AES_128_CCM17
TLS_RSA_WITH_AES_128_GCM_SHA256
TLS_RSA_WITH_AES_256_CBC_SHA
TLS_RSA_WITH_AES_256_CBC_SHA256
TLS_RSA_WITH_AES_256_CCM
TLS_RSA_WITH_AES_256_GCM_SHA384

Thomas

-----Original Message-----
From: Long, Qin [mailto:qin.long@intel.com]
Sent: Sunday, July 31, 2016 8:48 PM
To: Wu, Jiaxin <jiaxin.wu@intel.com>; Palmer, Thomas
<thomas.palmer@hpe.com>; edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
Gao, Liming <liming.gao@intel.com>
Subject: RE: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
definitions with the standardized one

I personally prefer to keep the current supported cipher suite for
our
UEFI- TLS enabling. We can have the full RFC definitions, and
platform specific cipher sets for validation now. It's better to
maintain one minimal scope in this phase.

"enable-weak-ssl-ciphers" looks odd. Disabling weak ciphers is the
recommendation for hardening SSL communications.
For other ciphers (idea, dsa, etc), we can enable them step-by-step
depending on the real requirements.


Best Regards & Thanks,
LONG, Qin

-----Original Message-----
From: Wu, Jiaxin
Sent: Monday, August 01, 2016 9:23 AM
To: Palmer, Thomas; Long, Qin; edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan; Gao, Liming
Subject: RE: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
definitions with the standardized one

Thomas,
I agree some of them are not supported due to the UEFI OpenSSL
configuration, but it doesn't affect those mapping relationship
added in the patch. So, I have no strong opinion whether to
support it by modifying the current OpenSSL configuration. Since
Qin is the OpenSSL expert, I'd like to hear his views.

Qin,
What's your opinion?

Thanks.
Jiaxin

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
Behalf Of Palmer, Thomas
Sent: Saturday, July 30, 2016 6:03 AM
To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan
<siyuan.fu@intel.com>; Gao, Liming <liming.gao@intel.com>; Long,
Qin <qin.long@intel.com>
Subject: Re: [edk2] [staging/HTTPS-TLS][PATCH 0/4] Replace the
TLS definitions with the standardized one

Jiaxin,

UEFI's OpenSSL library does not support all the ciphers that
were added in your patch due to the UEFI configuration. We need
to remove
"no- idea" and "no-dsa" from the process_files.sh and add
"enable-weak-ssl- ciphers"

While we are modifying process_files.sh, we can remove "no-
pqueue"
from process_files.sh so that OpensslLib.inf is in sync.

I can send out a patch to do so if you wish.

Thomas

-----Original Message-----
From: Jiaxin Wu [mailto:jiaxin.wu@intel.com]
Sent: Thursday, July 14, 2016 12:51 AM
To: edk2-devel@lists.01.org
Cc: Liming Gao <liming.gao@intel.com>; Palmer, Thomas
<thomas.palmer@hpe.com>; Long Qin <qin.long@intel.com>; Ye Ting
<ting.ye@intel.com>; Fu Siyuan <siyuan.fu@intel.com>; Wu Jiaxin
<jiaxin.wu@intel.com>
Subject: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
definitions with the standardized one

The series patches are used to replace the TLS definitions with
the standardized one. In addition, more TLS cipher suite mapping
between Cipher Suite definitions and OpenSSL-used Cipher Suite
name
are added.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Palmer Thomas <thomas.palmer@hpe.com>
Cc: Long Qin <qin.long@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

Jiaxin Wu (4):
MdePkg: Add a header to standardize TLS definitions
CryptoPkg: Add more TLS cipher suite mapping
NetworkPkg/TlsDxe: Replace the definitions with the
standardized
one
NetworkPkg/HttpDxe: Replace the definitions with the
standardized one

CryptoPkg/Library/TlsLib/TlsLib.c | 3585 ++++++++++++++++-------
--
---
--
--
MdePkg/Include/IndustryStandard/Tls1.h | 93 +
NetworkPkg/HttpDxe/HttpDriver.h | 2 +
NetworkPkg/HttpDxe/HttpProto.c | 12 +-
NetworkPkg/HttpDxe/HttpsSupport.c | 22 +-
NetworkPkg/HttpDxe/HttpsSupport.h | 44 -
NetworkPkg/TlsDxe/TlsImpl.c | 56 +-
NetworkPkg/TlsDxe/TlsImpl.h | 30 +-
NetworkPkg/TlsDxe/TlsProtocol.c | 2 +-
9 files changed, 1945 insertions(+), 1901 deletions(-) create
mode
100644 MdePkg/Include/IndustryStandard/Tls1.h

--
1.9.5.msysgit.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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [PATCH] MdePkg: move to 'hidden' visibility for all symbols under GCC/X64

Liming Gao
 

Reviewed-by: Liming Gao <liming.gao@intel.com>

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Monday, August 01, 2016 2:57 PM
To: Shi, Steven <steven.shi@intel.com>; Zhu, Yonghong
<yonghong.zhu@intel.com>; Gao, Liming <liming.gao@intel.com>; Justen,
Jordan L <jordan.l.justen@intel.com>; edk2-devel@lists.01.org
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH] MdePkg: move to 'hidden' visibility for all symbols under
GCC/X64

When using GCC to build for X64, we switched to the position independent
small code model, which is much more efficient in terms of code generation
and runtime relocation footprint, and produces binaries that can execute
correctly from any offset.

However, the PIC routines are by default geared towards hosted binaries
containing symbol references that may resolve to definitions in other
dynamic objects, and for this reason, external symbol references are
indirected via a GOT entry by default (which also results in a .reloc fixup
entry) unless we annotate them.

For this reason, we introduced the 'protected' visibility annotation for
all symbol definitions and references, by setting the GCC visibility
pragma. However, as it turns out, this is not sufficient for all versions
of GCC, and in some cases (GCC 5.x using the GCC49 toolchain tag), may
still result in GOT based relocations.

So switch to 'hidden' visibility instead, which is slightly stronger, and
fixes this issue for the versions of GCC that exhibit the problem.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
MdePkg/Include/X64/ProcessorBind.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/X64/ProcessorBind.h
b/MdePkg/Include/X64/ProcessorBind.h
index a4aad3e524e8..666cc8e8bd16 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -29,12 +29,12 @@

#if defined(__GNUC__) && defined(__pic__)
//
-// Mark all symbol declarations and references as protected, meaning they
will
+// Mark all symbol declarations and references as hidden, meaning they will
// not be subject to symbol preemption. This allows the compiler to refer to
// symbols directly using relative references rather than via the GOT, which
// contains absolute symbol addresses that are subject to runtime relocation.
//
-#pragma GCC visibility push (protected)
+#pragma GCC visibility push (hidden)
#endif

#if defined(__INTEL_COMPILER)
--
2.7.4


Re: [PATCH v5 7/8] MdePkg GCC/X64: avoid 'hidden' visibility for module entry points

Liming Gao
 

Ard:
I will verify it. And, I would ask why only X64 requires it? IA32, ARM and AARCH64 doesn't specially handle it?

Thanks
Liming

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Tuesday, August 02, 2016 12:12 AM
To: Gao, Liming <liming.gao@intel.com>
Cc: Shi, Steven <steven.shi@intel.com>; Zhu, Yonghong
<yonghong.zhu@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
edk2-devel@lists.01.org; lersek@redhat.com; leif.lindholm@linaro.org
Subject: Re: [edk2] [PATCH v5 7/8] MdePkg GCC/X64: avoid 'hidden' visibility
for module entry points

On 1 August 2016 at 17:51, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:56, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:49, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:
On 1 August 2016 at 16:18, Gao, Liming <liming.gao@intel.com> wrote:
Ard:
I don't think it is good way to define GCC_VISIBILITY_PROTECTED and
apply it in EntryPointLib. We only need to expose _ModuleEntryPoint. It has
been specified in LINK_FLAGS in tools_def.txt. Could we also specify its
attribute in CC_FLAGS or LINK_FLAGS in tools_def.txt?
It seems this does the trick as well

diff --git a/BaseTools/Scripts/GccBase.lds
b/BaseTools/Scripts/GccBase.lds
index 281af8a9bd33..02387d4f8d6f 100644
--- a/BaseTools/Scripts/GccBase.lds
+++ b/BaseTools/Scripts/GccBase.lds
@@ -80,3 +80,7 @@ SECTIONS {
*(COMMON)
}
}
+
+VERSION {
+ { global: _ModuleEntryPoint*; };
+};


Note that * at the end: this is necessary since _ModuleEntryPoint will
be called _ModuleEntryPoint.lto_priv.xxx in the LTO objects.
Hmm, looks like I spoke too soon. I don't know what I did wrong, but
this does not actually work.
The only alternative I can think of is to add a static non-lto object
to the tree that refers to _ModuleEntryPoint, similar to the way I
handle the ARM intrinsics in patch #5
As it turns out, the LTO linker does not need to visibility pragma to
prevent it from emitting GOT based relocations. This makes sense,
considering that the LTO linker can see that no symbol references are
ever satisfied across dynamic object boundaries. That means I could
work around this in the following way:

diff --git a/BaseTools/Conf/tools_def.template
b/BaseTools/Conf/tools_def.template
index 314adaf6bfa8..983e2fea7390 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4462,7 +4462,7 @@ DEFINE GCC49_ARM_ASLDLINK_FLAGS =
DEF(GCC48_ARM_ASLDLINK_FLAGS)
DEFINE GCC49_AARCH64_ASLDLINK_FLAGS =
DEF(GCC48_AARCH64_ASLDLINK_FLAGS)

DEFINE GCC5_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -flto
-fno-builtin
-DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -flto
-fno-builtin
+DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -flto
-fno-builtin -DUSING_LTO
DEFINE GCC5_IA32_X64_DLINK_COMMON =
DEF(GCC49_IA32_X64_DLINK_COMMON)
DEFINE GCC5_IA32_X64_ASLDLINK_FLAGS =
DEF(GCC49_IA32_X64_ASLDLINK_FLAGS)
DEFINE GCC5_IA32_X64_DLINK_FLAGS =
DEF(GCC49_IA32_X64_DLINK_FLAGS) -flto
diff --git a/MdePkg/Include/X64/ProcessorBind.h
b/MdePkg/Include/X64/ProcessorBind.h
index 666cc8e8bd16..77fab7055afc 100644
--- a/MdePkg/Include/X64/ProcessorBind.h
+++ b/MdePkg/Include/X64/ProcessorBind.h
@@ -27,12 +27,15 @@
#pragma pack()
#endif

-#if defined(__GNUC__) && defined(__pic__)
+#if defined(__GNUC__) && defined(__pic__) && !defined(USING_LTO)
//
// Mark all symbol declarations and references as hidden, meaning they will
// not be subject to symbol preemption. This allows the compiler to refer to
// symbols directly using relative references rather than via the GOT, which
// contains absolute symbol addresses that are subject to runtime relocation.
+// The LTO linker will not emit GOT based relocations anyway, so there is no
+// need to set the pragma in that case (and doing so will cause issues of its
+// own)
//
#pragma GCC visibility push (hidden)
#endif

and I can drop this patch.