Date   

Re: [PATCH 1/5] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

Ard Biesheuvel
 

On 8 June 2018 at 04:53, Zeng, Star <star.zeng@...> wrote:
I suggest to use goto/adjust code to have one place for both paths to perform cache maintenance (with comments).
Something like this?

@@ -253,6 +254,7 @@ ValidateCapsuleByMemoryResource (
)
{
UINTN Index;
+ BOOLEAN Found;

//
// Sanity Check
@@ -274,19 +276,32 @@ ValidateCapsuleByMemoryResource (
//
// No memory resource descriptor reported in HOB list before
capsule Coalesce.
//
- return TRUE;
+ Found = TRUE;
+ } else {
+ Found = FALSE;
}

- for (Index = 0; MemoryResource[Index].ResourceLength != 0; Index++) {
+ for (Index = 0; !Found && MemoryResource[Index].ResourceLength !=
0; Index++) {
if ((Address >= MemoryResource[Index].PhysicalStart) &&
((Address + Size) <= (MemoryResource[Index].PhysicalStart +
MemoryResource[Index].ResourceLength))) {
DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in
MemoryResource[0x%x] - Start(0x%lx) Length(0x%lx)\n",
Address, Size,
Index, MemoryResource[Index].PhysicalStart,
MemoryResource[Index].ResourceLength));
- return TRUE;
+ Found = TRUE;
}
}

+ if (Found) {
+ //
+ // At this point, we may still be running with the MMU and caches disabled,
+ // and on architectures such as ARM or AARCH64, capsule [meta]data loaded
+ // into memory with the caches on is only guaranteed to be visible to the
+ // CPU running with the caches off after performing an explicit writeback.
+ //
+ WriteBackDataCacheRange ((VOID *)(UINTN)Address, (UINTN)Size);
+ return TRUE;
+ }
+
DEBUG ((EFI_D_ERROR, "ERROR: Address(0x%lx) Size(0x%lx) not in any
MemoryResource\n", Address, Size));
return FALSE;
}



Thanks,
Star
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@...]
Sent: Thursday, June 7, 2018 7:08 PM
To: edk2-devel@...
Cc: leif.lindholm@...; Kinney, Michael D <michael.d.kinney@...>; Yao, Jiewen <jiewen.yao@...>; Zeng, Star <star.zeng@...>; Ard Biesheuvel <ard.biesheuvel@...>
Subject: [PATCH 1/5] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

When capsule updates are staged for processing after a warm reboot, they are copied into memory with the MMU and caches enabled. When the capsule PEI gets around to coalescing the capsule, the MMU and caches may still be disabled, and so on architectures where uncached accesses are incoherent with the caches (such as ARM and AARCH64), we may read stale data if we don't clean the caches to memory first.

Note that this cache maintenance cannot be done during the invocation of UpdateCapsule(), since the ScatterGatherList structures are only identified by physical address, and at runtime, the firmware doesn't know whether and where this memory is mapped, and cache maintenance requires a virtual address.

Reviewed-by: Jiewen Yao <jiewen.yao@...>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>
---
MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 1 +
MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c | 10 ++++++++++
2 files changed, 11 insertions(+)

diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
index c54bc21a95a8..594e110d1f8a 100644
--- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
+++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
@@ -48,6 +48,7 @@ [Packages]

[LibraryClasses]
BaseLib
+ CacheMaintenanceLib
HobLib
BaseMemoryLib
PeiServicesLib
diff --git a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
index 3e7054cd38a9..fb59f338f100 100644
--- a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
+++ b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
@@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Guid/CapsuleVendor.h>

#include <Library/BaseMemoryLib.h>
+#include <Library/CacheMaintenanceLib.h>
#include <Library/DebugLib.h>
#include <Library/PrintLib.h>
#include <Library/BaseLib.h>
@@ -274,6 +275,7 @@ ValidateCapsuleByMemoryResource (
//
// No memory resource descriptor reported in HOB list before capsule Coalesce.
//
+ WriteBackDataCacheRange ((VOID *)(UINTN)Address, (UINTN)Size);
return TRUE;
}

@@ -283,6 +285,14 @@ ValidateCapsuleByMemoryResource (
DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in MemoryResource[0x%x] - Start(0x%lx) Length(0x%lx)\n",
Address, Size,
Index, MemoryResource[Index].PhysicalStart, MemoryResource[Index].ResourceLength));
+
+ //
+ // At this point, we may still be running with the MMU and caches disabled,
+ // and on architectures such as ARM or AARCH64, capsule [meta]data loaded
+ // into memory with the caches on is only guaranteed to be visible to the
+ // CPU running with the caches off after performing an explicit writeback.
+ //
+ WriteBackDataCacheRange ((VOID *)(UINTN)Address, (UINTN)Size);
return TRUE;
}
}
--
2.17.0


Re: [PATCH] CorebootModulePkg/CbSupportDxe: Remove SCI_EN setting

Benjamin You
 

Hi Matt,

Thanks. I've submitted patch v2 based on your feedbacks.

- ben

From: Matt Delco [mailto:delco@...]
Sent: Friday, June 8, 2018 1:07 AM
To: Ma, Maurice <maurice.ma@...>
Cc: You, Benjamin <benjamin.you@...>; Agyeman, Prince <prince.agyeman@...>; edk2-devel@...
Subject: Re: [PATCH] CorebootModulePkg/CbSupportDxe: Remove SCI_EN setting

Thanks for working on this. I did a quick test to confirm that things still work okay. The code change looks fine though there's some optional optimizations to consider:

The new code added to CbParseFadtInfo() is mostly to add debug checks but I think it'll still result in a IO port access in a release build so it might be worthwhile to make the entire code block specific to a debug build (possibly by sticking the code within a "#if !defined(MDEPKG_NDEBUG)" block, though I don't see any other such cases in edk2 so I suspect this isn't an ideal suggestion). Regarding "Pm1CntLen" It looks like the "if (==2) {} else if (==4) {} else {}" could be simplified to "if (==4) {} else {}".

In CbDxeEntryPoint() the "Find the acpi board information guid hob" code block can probably be deleted. Its only remaining use is the debug log regarding the value of PmCtrlReg, which I suspect isn't that valuable given the other deletions. This would also allow the removal of the "mPmCtrlReg" variable at file scope.

On Thu, Jun 7, 2018 at 8:40 AM, Ma, Maurice <maurice.ma@...> wrote:
It looks good to me.
Reviewed-by: Maurice Ma <maurice.ma@...>


-----Original Message-----
From: You, Benjamin
Sent: Thursday, June 7, 2018 1:19
To: edk2-devel@...
Cc: Ma, Maurice <maurice.ma@...>; Agyeman, Prince <prince.agyeman@...>; delco@...
Subject: [PATCH] CorebootModulePkg/CbSupportDxe: Remove SCI_EN setting

Current implemenation sets PM1_CNT.SCI_EN bit at ReadyToBoot event.
However, this should not be done because this causes OS to skip triggering FADT.SMI_CMD, which leads to the functions implemented in the SMI handler being omitted.

This issue was identified by Matt Delco <delco@...>.

The fix does the following:
- The SCI_EN bit setting is removed from CbSupportDxe driver.
- Some additional checks are added in CbParseFadtInfo() in CbParseLib.c to
output some error message and ASSERT (FALSE) if ALL of the following
conditions are met:
1) HARDWARE_REDUCED_ACPI is not set;
2) SMI_CMD field is zero;
3) SCI_EN bit is zero;
which indicates the ACPI enabling status is inconsistent: SCI is not
enabled but the ACPI table does not provide a means to enable it through
FADT->SMI_CMD. This may cause issues in OS.

Cc: Maurice Ma <maurice.ma@...>
Cc: Prince Agyeman <prince.agyeman@...>
Cc: Matt Delco <delco@...>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Benjamin You <benjamin.you@...>
---
CorebootModulePkg/CbSupportDxe/CbSupportDxe.c | 39 ----------------------
CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf | 1 -
CorebootModulePkg/Library/CbParseLib/CbParseLib.c | 28 ++++++++++++++++
.../Library/CbParseLib/CbParseLib.inf | 3 +-
4 files changed, 30 insertions(+), 41 deletions(-)

diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
index c526c9e871..b41c0885f7 100755
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
@@ -86,31 +86,6 @@ CbReserveResourceInGcd (
return Status;
}

-/**
- Notification function of EVT_GROUP_READY_TO_BOOT event group.
-
- This is a notification function registered on EVT_GROUP_READY_TO_BOOT event group.
- When the Boot Manager is about to load and execute a boot option, it reclaims variable
- storage if free size is below the threshold.
-
- @param Event Event whose notification function is being invoked.
- @param Context Pointer to the notification function's context.
-
-**/
-VOID
-EFIAPI
-OnReadyToBoot (
- IN EFI_EVENT Event,
- IN VOID *Context
- )
-{
- //
- // Enable SCI
- //
- IoOr16 (mPmCtrlReg, BIT0);
-
- DEBUG ((EFI_D_ERROR, "Enable SCI bit at 0x%lx before boot\n", (UINT64)mPmCtrlReg)); -}

/**
Main entry for the Coreboot Support DXE module.
@@ -130,7 +105,6 @@ CbDxeEntryPoint (
)
{
EFI_STATUS Status;
- EFI_EVENT ReadyToBootEvent;
EFI_HOB_GUID_TYPE *GuidHob;
SYSTEM_TABLE_INFO *pSystemTableInfo;
ACPI_BOARD_INFO *pAcpiBoardInfo;
@@ -197,19 +171,6 @@ CbDxeEntryPoint (
ASSERT_EFI_ERROR (Status);
}

- //
- // Register callback on the ready to boot event
- // in order to enable SCI
- //
- ReadyToBootEvent = NULL;
- Status = EfiCreateEventReadyToBootEx (
- TPL_CALLBACK,
- OnReadyToBoot,
- NULL,
- &ReadyToBootEvent
- );
- ASSERT_EFI_ERROR (Status);
-
return EFI_SUCCESS;
}

diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
index 99245183ea..15b0dac774 100644
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
@@ -46,7 +46,6 @@
DebugLib
BaseMemoryLib
UefiLib
- IoLib
HobLib

[Guids]
diff --git a/CorebootModulePkg/Library/CbParseLib/CbParseLib.c b/CorebootModulePkg/Library/CbParseLib/CbParseLib.c
index 0909b0f492..cd98a72f01 100644
--- a/CorebootModulePkg/Library/CbParseLib/CbParseLib.c
+++ b/CorebootModulePkg/Library/CbParseLib/CbParseLib.c
@@ -18,6 +18,7 @@
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/PcdLib.h>
+#include <Library/IoLib.h>
#include <Library/CbParseLib.h>

#include <IndustryStandard/Acpi.h>
@@ -412,6 +413,7 @@ CbParseFadtInfo (
UINTN Entry64Num;
UINTN Idx;
RETURN_STATUS Status;
+ BOOLEAN SciEnabled;

Rsdp = NULL;
Status = RETURN_SUCCESS;
@@ -477,6 +479,32 @@ CbParseFadtInfo (
ASSERT(Fadt->Pm1aEvtBlk != 0);
ASSERT(Fadt->Gpe0Blk != 0);

+ //
+ // Get SCI_EN value
+ //
+ if (Fadt->Pm1CntLen == 2) {
+ SciEnabled = (IoRead16 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
+ } else if (Fadt->Pm1CntLen == 4) {
+ SciEnabled = (IoRead32 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
+ } else {
+ //
+ // Unsupported PM1 CNT Length, revert to 16 bit
+ //
+ SciEnabled = (IoRead16 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
+ }
+
+ if (!(Fadt->Flags & EFI_ACPI_5_0_HW_REDUCED_ACPI) &&
+ (Fadt->SmiCmd == 0) &&
+ !SciEnabled) {
+ //
+ // The ACPI enabling status is inconsistent: SCI is not enabled but ACPI
+ // table does not provide a means to enable it through FADT->SmiCmd
+ //
+ DEBUG ((DEBUG_ERROR, "ERROR: The ACPI enabling status is inconsistent: SCI is not"
+ " enabled but the ACPI table does not provide a means to enable it through FADT->SmiCmd."
+ " This may cause issues in OS.\n"));
+ ASSERT (FALSE);
+ }
return RETURN_SUCCESS;
}
}
diff --git a/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf b/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf
index d7146a415b..25b847946c 100644
--- a/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf
+++ b/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf
@@ -37,7 +37,8 @@
[LibraryClasses]
BaseLib
BaseMemoryLib
- DebugLib

+ IoLib
+ DebugLib
PcdLib

[Pcd]
--
2.14.3.windows.1


[PATCH v2] CorebootModulePkg/CbSupportDxe: Remove SCI_EN setting

Benjamin You
 

Current implemenation sets PM1_CNT.SCI_EN bit at ReadyToBoot event.
However, this should not be done because this causes OS to skip triggering
FADT.SMI_CMD, which leads to the functions implemented in the SMI
handler being omitted.

This issue was identified by Matt Delco <delco@...>.

The fix does the following:
- The SCI_EN bit setting is removed from CbSupportDxe driver.
- Some additional checks are added in CbParseFadtInfo() in CbParseLib.c to
output some error message and ASSERT (FALSE) if ALL of the following
conditions are met:
1) HARDWARE_REDUCED_ACPI is not set;
2) SMI_CMD field is zero;
3) SCI_EN bit is zero;
which indicates the ACPI enabling status is inconsistent: SCI is not
enabled but the ACPI table does not provide a means to enable it through
FADT->SMI_CMD. This may cause issues in OS.

Cc: Maurice Ma <maurice.ma@...>
Cc: Prince Agyeman <prince.agyeman@...>
Cc: Matt Delco <delco@...>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Benjamin You <benjamin.you@...>
---
CorebootModulePkg/CbSupportDxe/CbSupportDxe.c | 51 ----------------------
CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf | 1 -
CorebootModulePkg/Library/CbParseLib/CbParseLib.c | 34 +++++++++++++++
.../Library/CbParseLib/CbParseLib.inf | 3 +-
4 files changed, 36 insertions(+), 53 deletions(-)

diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
index c526c9e871..ec42f7ff35 100755
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
@@ -14,7 +14,6 @@
**/
#include "CbSupportDxe.h"

-UINTN mPmCtrlReg = 0;
/**
Reserve MMIO/IO resource in GCD

@@ -86,31 +85,6 @@ CbReserveResourceInGcd (
return Status;
}

-/**
- Notification function of EVT_GROUP_READY_TO_BOOT event group.
-
- This is a notification function registered on EVT_GROUP_READY_TO_BOOT event group.
- When the Boot Manager is about to load and execute a boot option, it reclaims variable
- storage if free size is below the threshold.
-
- @param Event Event whose notification function is being invoked.
- @param Context Pointer to the notification function's context.
-
-**/
-VOID
-EFIAPI
-OnReadyToBoot (
- IN EFI_EVENT Event,
- IN VOID *Context
- )
-{
- //
- // Enable SCI
- //
- IoOr16 (mPmCtrlReg, BIT0);
-
- DEBUG ((EFI_D_ERROR, "Enable SCI bit at 0x%lx before boot\n", (UINT64)mPmCtrlReg));
-}

/**
Main entry for the Coreboot Support DXE module.
@@ -130,10 +104,8 @@ CbDxeEntryPoint (
)
{
EFI_STATUS Status;
- EFI_EVENT ReadyToBootEvent;
EFI_HOB_GUID_TYPE *GuidHob;
SYSTEM_TABLE_INFO *pSystemTableInfo;
- ACPI_BOARD_INFO *pAcpiBoardInfo;
FRAME_BUFFER_INFO *FbInfo;

Status = EFI_SUCCESS;
@@ -171,16 +143,6 @@ CbDxeEntryPoint (
ASSERT_EFI_ERROR (Status);
}

- //
- // Find the acpi board information guid hob
- //
- GuidHob = GetFirstGuidHob (&gUefiAcpiBoardInfoGuid);
- ASSERT (GuidHob != NULL);
- pAcpiBoardInfo = (ACPI_BOARD_INFO *)GET_GUID_HOB_DATA (GuidHob);
-
- mPmCtrlReg = (UINTN)pAcpiBoardInfo->PmCtrlRegBase;
- DEBUG ((EFI_D_ERROR, "PmCtrlReg at 0x%lx\n", (UINT64)mPmCtrlReg));
-
//
// Find the frame buffer information and update PCDs
//
@@ -197,19 +159,6 @@ CbDxeEntryPoint (
ASSERT_EFI_ERROR (Status);
}

- //
- // Register callback on the ready to boot event
- // in order to enable SCI
- //
- ReadyToBootEvent = NULL;
- Status = EfiCreateEventReadyToBootEx (
- TPL_CALLBACK,
- OnReadyToBoot,
- NULL,
- &ReadyToBootEvent
- );
- ASSERT_EFI_ERROR (Status);
-
return EFI_SUCCESS;
}

diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
index 99245183ea..15b0dac774 100644
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
@@ -46,7 +46,6 @@
DebugLib
BaseMemoryLib
UefiLib
- IoLib
HobLib

[Guids]
diff --git a/CorebootModulePkg/Library/CbParseLib/CbParseLib.c b/CorebootModulePkg/Library/CbParseLib/CbParseLib.c
index 0909b0f492..da227dea5e 100644
--- a/CorebootModulePkg/Library/CbParseLib/CbParseLib.c
+++ b/CorebootModulePkg/Library/CbParseLib/CbParseLib.c
@@ -18,6 +18,7 @@
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/PcdLib.h>
+#include <Library/IoLib.h>
#include <Library/CbParseLib.h>

#include <IndustryStandard/Acpi.h>
@@ -477,6 +478,39 @@ CbParseFadtInfo (
ASSERT(Fadt->Pm1aEvtBlk != 0);
ASSERT(Fadt->Gpe0Blk != 0);

+ DEBUG_CODE_BEGIN ();
+ BOOLEAN SciEnabled;
+
+ //
+ // Check the consistency of SCI enabling
+ //
+
+ //
+ // Get SCI_EN value
+ //
+ if (Fadt->Pm1CntLen == 4) {
+ SciEnabled = (IoRead32 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
+ } else {
+ //
+ // if (Pm1CntLen == 2), use 16 bit IO read;
+ // if (Pm1CntLen != 2 && Pm1CntLen != 4), use 16 bit IO read as a fallback
+ //
+ SciEnabled = (IoRead16 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
+ }
+
+ if (!(Fadt->Flags & EFI_ACPI_5_0_HW_REDUCED_ACPI) &&
+ (Fadt->SmiCmd == 0) &&
+ !SciEnabled) {
+ //
+ // The ACPI enabling status is inconsistent: SCI is not enabled but ACPI
+ // table does not provide a means to enable it through FADT->SmiCmd
+ //
+ DEBUG ((DEBUG_ERROR, "ERROR: The ACPI enabling status is inconsistent: SCI is not"
+ " enabled but the ACPI table does not provide a means to enable it through FADT->SmiCmd."
+ " This may cause issues in OS.\n"));
+ ASSERT (FALSE);
+ }
+ DEBUG_CODE_END ();
return RETURN_SUCCESS;
}
}
diff --git a/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf b/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf
index d7146a415b..25b847946c 100644
--- a/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf
+++ b/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf
@@ -37,7 +37,8 @@
[LibraryClasses]
BaseLib
BaseMemoryLib
- DebugLib

+ IoLib
+ DebugLib
PcdLib

[Pcd]
--
2.14.3.windows.1


Re: [PATCH v2] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw

Andrew Fish
 

Zenith,

Stupid question, I'm not trying to derail the code review.....

In a static lib world like EFI, I can't figure out why we need support for GOT PC REL addressing modes? I say that as I would think they would be relocation entries in object files, and then the final link would happen and these relocation types would not be required in the final executable? Seems like the linker, at link time, would know the relative offset for all the static libraries that got linked together (edk2 build) and there is no need for a relocation.

So I want to make sure this is not an issue caused by the wrong linker flags (emitting relocation for dynamic libs that is not supported in EF)? I'll freely admit there could be some circumstances I don't understand that require this support, but I just want to make sure we understand why we need to do it. Or maybe I'm missing something fundamental?

Thanks,

Andrew Fish

On Jun 7, 2018, at 5:49 AM, Zenith432 <zenith432@...> wrote:

Adds support for the following X64 ELF relocations to GenFw
R_X86_64_GOTPCREL
R_X86_64_GOTPCRELX
R_X86_64_REX_GOTPCRELX

CC: Shi Steven <steven.shi@...>
CC: Yonghong Zhu <yonghong.zhu@...>
CC: Liming Gao <liming.gao@...>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zenith432 <zenith432@...>
---
BaseTools/Source/C/GenFw/Elf64Convert.c | 166 +++++++++++++++++++++++-
BaseTools/Source/C/GenFw/elf_common.h | 23 ++++
2 files changed, 188 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index c39bdff0..1518c395 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -94,6 +94,15 @@ STATIC Elf_Ehdr *mEhdr;
STATIC Elf_Shdr *mShdrBase;
STATIC Elf_Phdr *mPhdrBase;

+//
+// GOT information
+//
+STATIC Elf_Shdr *mGOTShdr = NULL;
+STATIC UINT32 mGOTShindex = 0;
+STATIC UINT32 *mGOTCoffEntries = NULL;
+STATIC UINT32 mGOTMaxCoffEntries = 0;
+STATIC UINT32 mGOTNumCoffEntries = 0;
+
//
// Coff information
//
@@ -322,6 +331,117 @@ GetSymName (
return StrtabContents + Sym->st_name;
}

+//
+// Find the ELF section hosting the GOT from an ELF Rva
+// of a single GOT entry. Normally, GOT is placed in
+// ELF .text section, so assume once we find in which
+// section the GOT is, all GOT entries are there, and
+// just verify this.
+//
+STATIC
+VOID
+FindElfGOTSectionFromGOTEntryElfRva (
+ Elf64_Addr GOTEntryElfRva
+ )
+{
+ UINT32 i;
+ if (mGOTShdr != NULL) {
+ if (GOTEntryElfRva >= mGOTShdr->sh_addr &&
+ GOTEntryElfRva < mGOTShdr->sh_addr + mGOTShdr->sh_size)
+ return;
+ Error (NULL, 0, 3000, "Unsupported", "FindElfGOTSectionFromGOTEntryElfRva: GOT entries found in multiple sections.");
+ exit(EXIT_FAILURE);
+ }
+ for (i = 0; i < mEhdr->e_shnum; i++) {
+ Elf_Shdr *shdr = GetShdrByIndex(i);
+ if (GOTEntryElfRva >= shdr->sh_addr &&
+ GOTEntryElfRva < shdr->sh_addr + shdr->sh_size) {
+ mGOTShdr = shdr;
+ mGOTShindex = i;
+ return;
+ }
+ }
+ Error (NULL, 0, 3000, "Invalid", "FindElfGOTSectionFromGOTEntryElfRva: ElfRva 0x%016LX for GOT entry not found in any section.", GOTEntryElfRva);
+ exit(EXIT_FAILURE);
+}
+
+//
+// Stores locations of GOT entries in COFF image.
+// Returns TRUE if GOT entry is new.
+// Simple implementation as number of GOT
+// entries is expected to be low.
+//
+
+STATIC
+BOOLEAN
+AccumulateCoffGOTEntries (
+ UINT32 GOTCoffEntry
+ )
+{
+ UINT32 i;
+ if (mGOTCoffEntries != NULL) {
+ for (i = 0; i < mGOTNumCoffEntries; i++)
+ if (mGOTCoffEntries[i] == GOTCoffEntry)
+ return FALSE;
+ }
+ if (mGOTCoffEntries == NULL) {
+ mGOTCoffEntries = (UINT32*)malloc(5 * sizeof *mGOTCoffEntries);
+ if (mGOTCoffEntries == NULL) {
+ Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+ }
+ assert (mGOTCoffEntries != NULL);
+ mGOTMaxCoffEntries = 5;
+ mGOTNumCoffEntries = 0;
+ } else if (mGOTNumCoffEntries == mGOTMaxCoffEntries) {
+ mGOTCoffEntries = (UINT32*)realloc(mGOTCoffEntries, 2 * mGOTMaxCoffEntries * sizeof *mGOTCoffEntries);
+ if (mGOTCoffEntries == NULL) {
+ Error (NULL, 0, 4001, "Resource", "memory cannot be allocated!");
+ }
+ assert (mGOTCoffEntries != NULL);
+ mGOTMaxCoffEntries += mGOTMaxCoffEntries;
+ }
+ mGOTCoffEntries[mGOTNumCoffEntries++] = GOTCoffEntry;
+ return TRUE;
+}
+
+STATIC
+int
+__comparator (
+ const void* lhs,
+ const void* rhs
+ )
+{
+ if (*(const UINT32*)lhs < *(const UINT32*)rhs)
+ return -1;
+ return *(const UINT32*)lhs > *(const UINT32*)rhs;
+}
+
+STATIC
+VOID
+EmitGOTRelocations (
+ VOID
+ )
+{
+ UINT32 i;
+ if (mGOTCoffEntries == NULL)
+ return;
+ qsort(
+ mGOTCoffEntries,
+ mGOTNumCoffEntries,
+ sizeof *mGOTCoffEntries,
+ __comparator);
+ for (i = 0; i < mGOTNumCoffEntries; i++) {
+ VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X", mGOTCoffEntries[i]);
+ CoffAddFixup(
+ mGOTCoffEntries[i],
+ EFI_IMAGE_REL_BASED_DIR64);
+ }
+ free(mGOTCoffEntries);
+ mGOTCoffEntries = NULL;
+ mGOTMaxCoffEntries = 0;
+ mGOTNumCoffEntries = 0;
+}
+
//
// Elf functions interface implementation
//
@@ -698,7 +818,7 @@ WriteSections64 (
// section that applies to the entire binary, and which will have its section
// index set to #0 (which is a NULL section with the SHF_ALLOC bit cleared).
//
- // In the absence of GOT based relocations (which we currently don't support),
+ // In the absence of GOT based relocations,
// this RELA section will contain redundant R_xxx_RELATIVE relocations, one
// for every R_xxx_xx64 relocation appearing in the per-section RELA sections.
// (i.e., .rela.text and .rela.data)
@@ -780,6 +900,7 @@ WriteSections64 (
// Determine how to handle each relocation type based on the machine type.
//
if (mEhdr->e_machine == EM_X86_64) {
+ Elf64_Addr GOTEntryRva;
switch (ELF_R_TYPE(Rel->r_info)) {
case R_X86_64_NONE:
break;
@@ -834,6 +955,32 @@ WriteSections64 (
- (SecOffset - SecShdr->sh_addr));
VerboseMsg ("Relocation: 0x%08X", *(UINT32 *)Targ);
break;
+ case R_X86_64_GOTPCREL:
+ case R_X86_64_GOTPCRELX:
+ case R_X86_64_REX_GOTPCRELX:
+ VerboseMsg ("R_X86_64_GOTPCREL family");
+ VerboseMsg ("Offset: 0x%08X, Addend: 0x%08X",
+ (UINT32)(SecOffset + (Rel->r_offset - SecShdr->sh_addr)),
+ *(UINT32 *)Targ);
+ GOTEntryRva = Rel->r_offset - Rel->r_addend + *(INT32 *)Targ;
+ FindElfGOTSectionFromGOTEntryElfRva(GOTEntryRva);
+ *(UINT32 *)Targ = (UINT32) (*(UINT32 *)Targ
+ + (mCoffSectionsOffset[mGOTShindex] - mGOTShdr->sh_addr)
+ - (SecOffset - SecShdr->sh_addr));
+ VerboseMsg ("Relocation: 0x%08X", *(UINT32 *)Targ);
+ GOTEntryRva += (mCoffSectionsOffset[mGOTShindex] - mGOTShdr->sh_addr); // ELF Rva -> COFF Rva
+ if (AccumulateCoffGOTEntries((UINT32)GOTEntryRva)) {
+ //
+ // Relocate GOT entry if it's the first time we run into it
+ //
+ Targ = mCoffFile + GOTEntryRva;
+ VerboseMsg ("Offset: 0x%08X, Addend: 0x%016LX",
+ (UINT32)GOTEntryRva,
+ *(UINT64 *)Targ);
+ *(UINT64 *)Targ = *(UINT64 *)Targ - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx];
+ VerboseMsg ("Relocation: 0x%016LX", *(UINT64*)Targ);
+ }
+ break;
default:
Error (NULL, 0, 3000, "Invalid", "%s unsupported ELF EM_X86_64 relocation 0x%x.", mInImageName, (unsigned) ELF_R_TYPE(Rel->r_info));
}
@@ -972,6 +1119,9 @@ WriteRelocations64 (
case R_X86_64_NONE:
case R_X86_64_PC32:
case R_X86_64_PLT32:
+ case R_X86_64_GOTPCREL:
+ case R_X86_64_GOTPCRELX:
+ case R_X86_64_REX_GOTPCRELX:
break;
case R_X86_64_64:
VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X",
@@ -1040,10 +1190,24 @@ WriteRelocations64 (
Error (NULL, 0, 3000, "Not Supported", "This tool does not support relocations for ELF with e_machine %u (processor type).", (unsigned) mEhdr->e_machine);
}
}
+ if (mEhdr->e_machine == EM_X86_64 && RelShdr->sh_info == mGOTShindex) {
+ //
+ // Tack relocations for GOT entries after other relocations for
+ // the section the GOT is in, as it's usually found at the end
+ // of the section.
+ //
+ EmitGOTRelocations();
+ }
}
}
}

+ if (mEhdr->e_machine == EM_X86_64) {
+ //
+ // Just in case GOT is in a section with no other relocations
+ //
+ EmitGOTRelocations();
+ }
//
// Pad by adding empty entries.
//
diff --git a/BaseTools/Source/C/GenFw/elf_common.h b/BaseTools/Source/C/GenFw/elf_common.h
index 766d0e42..50b4e1f2 100644
--- a/BaseTools/Source/C/GenFw/elf_common.h
+++ b/BaseTools/Source/C/GenFw/elf_common.h
@@ -544,6 +544,12 @@ typedef struct {
#define R_386_TLS_DTPMOD32 35 /* GOT entry containing TLS index */
#define R_386_TLS_DTPOFF32 36 /* GOT entry containing TLS offset */
#define R_386_TLS_TPOFF32 37 /* GOT entry of -ve static TLS offset */
+#define R_386_SIZE32 38 /* 32-bit symbol size */
+#define R_386_TLS_GOTDESC 39 /* GOT offset for TLS descriptor. */
+#define R_386_TLS_DESC_CALL 40 /* Marker of call through TLS descriptor for relaxation. */
+#define R_386_TLS_DESC 41 /* TLS descriptor containing pointer to code and to argument, returning the TLS offset for the symbol. */
+#define R_386_IRELATIVE 42 /* Adjust indirectly by program base */
+#define R_386_GOT32X 43 /* Load from 32 bit GOT entry, relaxable. */

/* Null relocation */
#define R_AARCH64_NONE 256 /* No relocation */
@@ -1052,6 +1058,23 @@ typedef struct {
#define R_X86_64_DTPOFF32 21 /* Offset in TLS block */
#define R_X86_64_GOTTPOFF 22 /* PC relative offset to IE GOT entry */
#define R_X86_64_TPOFF32 23 /* Offset in static TLS block */
+#define R_X86_64_PC64 24 /* PC relative 64 bit */
+#define R_X86_64_GOTOFF64 25 /* 64 bit offset to GOT */
+#define R_X86_64_GOTPC32 26 /* 32 bit signed pc relative offset to GOT */
+#define R_X86_64_GOT64 27 /* 64-bit GOT entry offset */
+#define R_X86_64_GOTPCREL64 28 /* 64-bit PC relative offset to GOT entry */
+#define R_X86_64_GOTPC64 29 /* 64-bit PC relative offset to GOT */
+#define R_X86_64_GOTPLT64 30 /* like GOT64, says PLT entry needed */
+#define R_X86_64_PLTOFF64 31 /* 64-bit GOT relative offset to PLT entry */
+#define R_X86_64_SIZE32 32 /* Size of symbol plus 32-bit addend */
+#define R_X86_64_SIZE64 33 /* Size of symbol plus 64-bit addend */
+#define R_X86_64_GOTPC32_TLSDESC 34 /* GOT offset for TLS descriptor. */
+#define R_X86_64_TLSDESC_CALL 35 /* Marker for call through TLS descriptor. */
+#define R_X86_64_TLSDESC 36 /* TLS descriptor. */
+#define R_X86_64_IRELATIVE 37 /* Adjust indirectly by program base */
+#define R_X86_64_RELATIVE64 38 /* 64-bit adjust by program base */
+#define R_X86_64_GOTPCRELX 41 /* Load from 32 bit signed pc relative offset to GOT entry without REX prefix, relaxable. */
+#define R_X86_64_REX_GOTPCRELX 42 /* Load from 32 bit signed pc relative offset to GOT entry with REX prefix, relaxable. */


#endif /* !_SYS_ELF_COMMON_H_ */
--
2.17.1

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


Re: [Patch] BaseTools: Fix Section header size larger than elf file size bug

Liming Gao
 

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

-----Original Message-----
From: Zhu, Yonghong
Sent: Thursday, June 7, 2018 10:09 AM
To: edk2-devel@...
Cc: Feng, YunhuaX <yunhuax.feng@...>; Gao, Liming <liming.gao@...>
Subject: [Patch] BaseTools: Fix Section header size larger than elf file size bug

From: Yunhua Feng <yunhuax.feng@...>

Add the logic to handle the case that Section header size larger than
elf file size.

Cc: Liming Gao <liming.gao@...>
Cc: Yonghong Zhu <yonghong.zhu@...>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng <yunhuax.feng@...>
---
BaseTools/Source/C/GenFw/Elf32Convert.c | 3 +++
BaseTools/Source/C/GenFw/Elf64Convert.c | 3 +++
BaseTools/Source/C/GenFw/ElfConvert.c | 20 ++++++++++++++++----
BaseTools/Source/C/GenFw/ElfConvert.h | 3 ++-
4 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c b/BaseTools/Source/C/GenFw/Elf32Convert.c
index e0f6491..e26b10b 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -672,10 +672,13 @@ WriteSections32 (
Elf_Shdr *Shdr = GetShdrByIndex(Idx);
if ((*Filter)(Shdr)) {
switch (Shdr->sh_type) {
case SHT_PROGBITS:
/* Copy. */
+ if (Shdr->sh_offset + Shdr->sh_size > mFileBufferSize) {
+ return FALSE;
+ }
memcpy(mCoffFile + mCoffSectionsOffset[Idx],
(UINT8*)mEhdr + Shdr->sh_offset,
Shdr->sh_size);
break;

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 9e68d22..cc0c2cf 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -668,10 +668,13 @@ WriteSections64 (
Elf_Shdr *Shdr = GetShdrByIndex(Idx);
if ((*Filter)(Shdr)) {
switch (Shdr->sh_type) {
case SHT_PROGBITS:
/* Copy. */
+ if (Shdr->sh_offset + Shdr->sh_size > mFileBufferSize) {
+ return FALSE;
+ }
memcpy(mCoffFile + mCoffSectionsOffset[Idx],
(UINT8*)mEhdr + Shdr->sh_offset,
(size_t) Shdr->sh_size);
break;

diff --git a/BaseTools/Source/C/GenFw/ElfConvert.c b/BaseTools/Source/C/GenFw/ElfConvert.c
index 17913ff..6844c69 100644
--- a/BaseTools/Source/C/GenFw/ElfConvert.c
+++ b/BaseTools/Source/C/GenFw/ElfConvert.c
@@ -1,9 +1,9 @@
/** @file
Elf convert solution

-Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2018, 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
@@ -56,10 +56,15 @@ UINT32 mCoffOffset;
// Offset in Coff file of headers and sections.
//
UINT32 mTableOffset;

//
+//mFileBufferSize
+//
+UINT32 mFileBufferSize;
+
+//
//*****************************************************************************
// Common ELF Functions
//*****************************************************************************
//

@@ -171,10 +176,11 @@ ConvertElf (
)
{
ELF_FUNCTION_TABLE ElfFunctions;
UINT8 EiClass;

+ mFileBufferSize = *FileLength;
//
// Determine ELF type and set function table pointer correctly.
//
VerboseMsg ("Check Elf Image Header");
EiClass = (*FileBuffer)[EI_CLASS];
@@ -199,13 +205,19 @@ ConvertElf (

//
// Write and relocate sections.
//
VerboseMsg ("Write and relocate sections.");
- ElfFunctions.WriteSections (SECTION_TEXT);
- ElfFunctions.WriteSections (SECTION_DATA);
- ElfFunctions.WriteSections (SECTION_HII);
+ if (!ElfFunctions.WriteSections (SECTION_TEXT)) {
+ return FALSE;
+ }
+ if (!ElfFunctions.WriteSections (SECTION_DATA)) {
+ return FALSE;
+ }
+ if (!ElfFunctions.WriteSections (SECTION_HII)) {
+ return FALSE;
+ }

//
// Translate and write relocations.
//
VerboseMsg ("Translate and write relocations.");
diff --git a/BaseTools/Source/C/GenFw/ElfConvert.h b/BaseTools/Source/C/GenFw/ElfConvert.h
index abf434d..fc8c63f 100644
--- a/BaseTools/Source/C/GenFw/ElfConvert.h
+++ b/BaseTools/Source/C/GenFw/ElfConvert.h
@@ -1,9 +1,9 @@
/** @file
Header file for Elf convert solution

-Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2018, 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
@@ -27,10 +27,11 @@ extern UINT32 mCoffOffset;
extern CHAR8 *mInImageName;
extern UINT32 mImageTimeStamp;
extern UINT8 *mCoffFile;
extern UINT32 mTableOffset;
extern UINT32 mOutImageType;
+extern UINT32 mFileBufferSize;

//
// Common EFI specific data.
//
#define ELF_HII_SECTION_NAME ".hii"
--
2.6.1.windows.1


Re: [Patch] BaseTools: Check elf sections alignment with MAX_COFF_ALIGNMENT

Liming Gao
 

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

-----Original Message-----
From: Zhu, Yonghong
Sent: Thursday, June 7, 2018 10:03 AM
To: edk2-devel@...
Cc: Feng, YunhuaX <yunhuax.feng@...>; Gao, Liming <liming.gao@...>
Subject: [Patch] BaseTools: Check elf sections alignment with MAX_COFF_ALIGNMENT

From: Yunhua Feng <yunhuax.feng@...>

Add the logic to check whether mCoffAlignment is larger than
MAX_COFF_ALIGNMENT, and report error for it.

Cc: Liming Gao <liming.gao@...>
Cc: Yonghong Zhu <yonghong.zhu@...>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng <yunhuax.feng@...>
---
BaseTools/Source/C/GenFw/Elf32Convert.c | 10 +++++++++-
BaseTools/Source/C/GenFw/Elf64Convert.c | 11 ++++++++++-
2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c b/BaseTools/Source/C/GenFw/Elf32Convert.c
index 14fe4a2..e0f6491 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -1,9 +1,9 @@
/** @file
Elf32 Convert solution

-Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
Portions copyright (c) 2013, ARM 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 which accompanies this
distribution. The full text of the license may be found at
@@ -382,10 +382,18 @@ ScanSections32 (
mCoffAlignment = (UINT32)shdr->sh_addralign;
}
}

//
+ // Check if mCoffAlignment larger than MAX_COFF_ALIGNMENT
+ //
+ if (mCoffAlignment > MAX_COFF_ALIGNMENT) {
+ Error (NULL, 0, 3000, "Invalid", "Section alignment larger than MAX_COFF_ALIGNMENT.");
+ assert (FALSE);
+ }
+
+ //
// Move the PE/COFF header right before the first section. This will help us
// save space when converting to TE.
//
if (mCoffAlignment > mCoffOffset) {
mNtHdrOffset += mCoffAlignment - mCoffOffset;
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index c39bdff..9e68d22 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -1,9 +1,9 @@
/** @file
Elf64 convert solution

-Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
Portions copyright (c) 2013-2014, ARM 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 which accompanies this
distribution. The full text of the license may be found at
@@ -375,10 +375,19 @@ ScanSections64 (
mCoffAlignment = (UINT32)shdr->sh_addralign;
}
}

//
+ // Check if mCoffAlignment larger than MAX_COFF_ALIGNMENT
+ //
+ if (mCoffAlignment > MAX_COFF_ALIGNMENT) {
+ Error (NULL, 0, 3000, "Invalid", "Section alignment larger than MAX_COFF_ALIGNMENT.");
+ assert (FALSE);
+ }
+
+
+ //
// Move the PE/COFF header right before the first section. This will help us
// save space when converting to TE.
//
if (mCoffAlignment > mCoffOffset) {
mNtHdrOffset += mCoffAlignment - mCoffOffset;
--
2.6.1.windows.1


Re: [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

Zeng, Star <star.zeng@...>
 

Good patch.

Another choice is to use DEBUG_VERBOSE.
We see other driver uses DEBUG_VERBOSE for BlockIo service (Hao can comment on that).
We'd better to align them for consistency.


Thanks,
Star

-----Original Message-----
From: Laszlo Ersek [mailto:lersek@...]
Sent: Friday, June 8, 2018 1:23 AM
To: Ard Biesheuvel <ard.biesheuvel@...>; edk2-devel@...
Cc: Zeng, Star <star.zeng@...>
Subject: Re: [edk2] [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

On 06/07/18 11:10, Ard Biesheuvel wrote:
Lower the priority of the DEBUG print in EmmcReadWrite(), which is
emitted for each read or write operation to the eMMC device, which
clutters up the log output of builds created with DEBUG_INFO enabled.

Suggested-by: Pipat Methavanitpong
<methavanitpong.pipat@...>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>
---
MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
index e1d0f394a954..f6b230514b71 100644
--- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
+++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
@@ -901,7 +901,10 @@ EmmcReadWrite (
if (EFI_ERROR (Status)) {
return Status;
}
- DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n", IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum, (Token != NULL) ? Token->Event : NULL, Status));
+ DEBUG ((DEBUG_BLKIO,
+ "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n",
+ IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum,
+ (Token != NULL) ? Token->Event : NULL, Status));

Lba += BlockNum;
Buffer = (UINT8*)Buffer + BufferSize;
Reviewed-by: Laszlo Ersek <lersek@...>


Re: [PATCH 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules () to be called once

Zeng, Star <star.zeng@...>
 

Without the patch, PopulateCapsuleInConfigurationTable is only run at first round.
With the patch, PopulateCapsuleInConfigurationTable is only run at last round.

Is that expected?

Jiewen, could you help check whether the patch meets the original design purpose or any security concern?


Thanks,
Star

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@...]
Sent: Thursday, June 7, 2018 7:08 PM
To: edk2-devel@...
Cc: leif.lindholm@...; Kinney, Michael D <michael.d.kinney@...>; Yao, Jiewen <jiewen.yao@...>; Zeng, Star <star.zeng@...>; Ard Biesheuvel <ard.biesheuvel@...>
Subject: [PATCH 2/5] MdeModulePkg/DxeCapsuleLibFmp: permit ProcessCapsules () to be called once

Permit ProcessCapsules () to be called only a single time, after EndOfDxe. This allows platforms that are able to update system firmware after EndOfDxe (e.g., because the flash ROM is not locked
down) to do so at a time when a non-trusted console is up and running, and progress can be reported to the user.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>
---
MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
index 26ca4e295f20..52691fa68be4 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
@@ -100,6 +100,7 @@ IsValidCapsuleHeader (

extern BOOLEAN mDxeCapsuleLibEndOfDxe;
BOOLEAN mNeedReset;
+BOOLEAN mFirstRound = TRUE;

VOID **mCapsulePtr;
EFI_STATUS *mCapsuleStatusArray;
@@ -364,8 +365,10 @@ PopulateCapsuleInConfigurationTable (

Each individual capsule result is recorded in capsule record variable.

- @param[in] FirstRound TRUE: First round. Need skip the FMP capsules with non zero EmbeddedDriverCount.
- FALSE: Process rest FMP capsules.
+ @param[in] LastRound FALSE: First of multiple rounds. Need skip the
+ FMP capsules with non zero
+ EmbeddedDriverCount.
+ TRUE: Process rest FMP capsules.

@retval EFI_SUCCESS There is no error when processing capsules.
@retval EFI_OUT_OF_RESOURCES No enough resource to process capsules.
@@ -373,7 +376,7 @@ PopulateCapsuleInConfigurationTable ( **/ EFI_STATUS ProcessTheseCapsules (
- IN BOOLEAN FirstRound
+ IN BOOLEAN LastRound
)
{
EFI_STATUS Status;
@@ -384,8 +387,9 @@ ProcessTheseCapsules (

REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE | PcdGet32(PcdStatusCodeSubClassCapsule) | PcdGet32(PcdCapsuleStatusCodeProcessCapsulesBegin)));

- if (FirstRound) {
+ if (mFirstRound) {
InitCapsulePtr ();
+ mFirstRound = FALSE;
}

if (mCapsuleTotalNumber == 0) {
@@ -404,7 +408,7 @@ ProcessTheseCapsules (
// Check the capsule flags,if contains CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE, install
// capsuleTable to configure table with EFI_CAPSULE_GUID
//
- if (FirstRound) {
+ if (LastRound) {
PopulateCapsuleInConfigurationTable ();
}

@@ -453,7 +457,7 @@ ProcessTheseCapsules (
continue;
}

- if ((!FirstRound) || (EmbeddedDriverCount == 0)) {
+ if (LastRound || (EmbeddedDriverCount == 0)) {
DEBUG((DEBUG_INFO, "ProcessCapsuleImage - 0x%x\n", CapsuleHeader));
Status = ProcessCapsuleImage (CapsuleHeader);
mCapsuleStatusArray [Index] = Status; @@ -546,7 +550,7 @@ ProcessCapsules (
EFI_STATUS Status;

if (!mDxeCapsuleLibEndOfDxe) {
- Status = ProcessTheseCapsules(TRUE);
+ Status = ProcessTheseCapsules(FALSE);

//
// Reboot System if and only if all capsule processed.
@@ -556,7 +560,7 @@ ProcessCapsules (
DoResetSystem();
}
} else {
- Status = ProcessTheseCapsules(FALSE);
+ Status = ProcessTheseCapsules(TRUE);
//
// Reboot System if required after all capsule processed
//
--
2.17.0


Re: [PATCH 1/5] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

Zeng, Star <star.zeng@...>
 

I suggest to use goto/adjust code to have one place for both paths to perform cache maintenance (with comments).


Thanks,
Star

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@...]
Sent: Thursday, June 7, 2018 7:08 PM
To: edk2-devel@...
Cc: leif.lindholm@...; Kinney, Michael D <michael.d.kinney@...>; Yao, Jiewen <jiewen.yao@...>; Zeng, Star <star.zeng@...>; Ard Biesheuvel <ard.biesheuvel@...>
Subject: [PATCH 1/5] MdeModulePkg/CapsulePei: clean Dcache before consuming capsule data

When capsule updates are staged for processing after a warm reboot, they are copied into memory with the MMU and caches enabled. When the capsule PEI gets around to coalescing the capsule, the MMU and caches may still be disabled, and so on architectures where uncached accesses are incoherent with the caches (such as ARM and AARCH64), we may read stale data if we don't clean the caches to memory first.

Note that this cache maintenance cannot be done during the invocation of UpdateCapsule(), since the ScatterGatherList structures are only identified by physical address, and at runtime, the firmware doesn't know whether and where this memory is mapped, and cache maintenance requires a virtual address.

Reviewed-by: Jiewen Yao <jiewen.yao@...>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>
---
MdeModulePkg/Universal/CapsulePei/CapsulePei.inf | 1 +
MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c | 10 ++++++++++
2 files changed, 11 insertions(+)

diff --git a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
index c54bc21a95a8..594e110d1f8a 100644
--- a/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
+++ b/MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
@@ -48,6 +48,7 @@ [Packages]

[LibraryClasses]
BaseLib
+ CacheMaintenanceLib
HobLib
BaseMemoryLib
PeiServicesLib
diff --git a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
index 3e7054cd38a9..fb59f338f100 100644
--- a/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
+++ b/MdeModulePkg/Universal/CapsulePei/Common/CapsuleCoalesce.c
@@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Guid/CapsuleVendor.h>

#include <Library/BaseMemoryLib.h>
+#include <Library/CacheMaintenanceLib.h>
#include <Library/DebugLib.h>
#include <Library/PrintLib.h>
#include <Library/BaseLib.h>
@@ -274,6 +275,7 @@ ValidateCapsuleByMemoryResource (
//
// No memory resource descriptor reported in HOB list before capsule Coalesce.
//
+ WriteBackDataCacheRange ((VOID *)(UINTN)Address, (UINTN)Size);
return TRUE;
}

@@ -283,6 +285,14 @@ ValidateCapsuleByMemoryResource (
DEBUG ((EFI_D_INFO, "Address(0x%lx) Size(0x%lx) in MemoryResource[0x%x] - Start(0x%lx) Length(0x%lx)\n",
Address, Size,
Index, MemoryResource[Index].PhysicalStart, MemoryResource[Index].ResourceLength));
+
+ //
+ // At this point, we may still be running with the MMU and caches disabled,
+ // and on architectures such as ARM or AARCH64, capsule [meta]data loaded
+ // into memory with the caches on is only guaranteed to be visible to the
+ // CPU running with the caches off after performing an explicit writeback.
+ //
+ WriteBackDataCacheRange ((VOID *)(UINTN)Address, (UINTN)Size);
return TRUE;
}
}
--
2.17.0


[Patch] BaseTools/UPT: Update the import statement to use StringUtils

Yonghong Zhu <yonghong.zhu@...>
 

The patch 5a57246eab80 Rename String to StringUtils, but it didn't
update the UPT Tool for the import statement which cause UPT tool
break.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu <yonghong.zhu@...>
---
BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py | 4 ++--
BaseTools/Source/Python/UPT/GenMetaFile/GenInfFile.py | 6 +++---
BaseTools/Source/Python/UPT/Library/CommentGenerating.py | 4 ++--
BaseTools/Source/Python/UPT/Library/CommentParsing.py | 6 +++---
BaseTools/Source/Python/UPT/Library/Misc.py | 4 ++--
BaseTools/Source/Python/UPT/Library/ParserValidate.py | 4 ++--
BaseTools/Source/Python/UPT/Library/Parsing.py | 14 +++++++-------
BaseTools/Source/Python/UPT/Library/StringUtils.py | 4 ++--
BaseTools/Source/Python/UPT/Library/UniClassObject.py | 2 +-
.../Source/Python/UPT/Object/Parser/InfDefineObject.py | 4 ++--
BaseTools/Source/Python/UPT/Object/Parser/InfPcdObject.py | 6 +++---
BaseTools/Source/Python/UPT/Parser/DecParser.py | 10 +++++-----
BaseTools/Source/Python/UPT/Parser/InfAsBuiltProcess.py | 4 ++--
BaseTools/Source/Python/UPT/Parser/InfParser.py | 6 +++---
BaseTools/Source/Python/UPT/Parser/InfParserMisc.py | 6 +++---
BaseTools/Source/Python/UPT/Parser/InfPcdSectionParser.py | 4 ++--
BaseTools/Source/Python/UPT/Parser/InfSectionParser.py | 4 ++--
BaseTools/Source/Python/UPT/PomAdapter/InfPomAlignment.py | 10 +++++-----
.../Source/Python/UPT/PomAdapter/InfPomAlignmentMisc.py | 4 ++--
.../Python/UPT/UnitTest/CommentGeneratingUnitTest.py | 4 ++--
.../Source/Python/UPT/UnitTest/CommentParsingUnitTest.py | 4 ++--
BaseTools/Source/Python/UPT/Xml/CommonXml.py | 10 +++++-----
BaseTools/Source/Python/UPT/Xml/GuidProtocolPpiXml.py | 8 ++++----
BaseTools/Source/Python/UPT/Xml/IniToXml.py | 4 ++--
BaseTools/Source/Python/UPT/Xml/ModuleSurfaceAreaXml.py | 10 +++++-----
BaseTools/Source/Python/UPT/Xml/PackageSurfaceAreaXml.py | 4 ++--
BaseTools/Source/Python/UPT/Xml/PcdXml.py | 8 ++++----
27 files changed, 79 insertions(+), 79 deletions(-)

diff --git a/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py b/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py
index d39c182..9397359 100644
--- a/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py
+++ b/BaseTools/Source/Python/UPT/GenMetaFile/GenDecFile.py
@@ -1,10 +1,10 @@
## @file GenDecFile.py
#
# This file contained the logical of transfer package object to DEC files.
#
-# Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -63,11 +63,11 @@ 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
-from Library.String import GetUniFileName
+from Library.StringUtils import GetUniFileName

def GenPcd(Package, Content):
#
# generate [Pcd] section
# <TokenSpcCName>.<TokenCName>|<Value>|<DatumType>|<Token>
diff --git a/BaseTools/Source/Python/UPT/GenMetaFile/GenInfFile.py b/BaseTools/Source/Python/UPT/GenMetaFile/GenInfFile.py
index 9373a14..bfd422b 100644
--- a/BaseTools/Source/Python/UPT/GenMetaFile/GenInfFile.py
+++ b/BaseTools/Source/Python/UPT/GenMetaFile/GenInfFile.py
@@ -1,10 +1,10 @@
## @file GenInfFile.py
#
# This file contained the logical of transfer package object to INF files.
#
-# Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -18,11 +18,11 @@ GenInf
import os
import stat
import codecs
import md5
from Core.FileHook import __FileHookOpen__
-from Library.String import GetSplitValueList
+from Library.StringUtils import GetSplitValueList
from Library.Parsing import GenSection
from Library.Parsing import GetWorkspacePackage
from Library.Parsing import ConvertArchForInstall
from Library.Misc import SaveFileOnChange
from Library.Misc import IsAllModuleList
@@ -39,11 +39,11 @@ from Logger import StringTable as ST
from Logger import ToolError
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
+from Library.StringUtils import GetUniFileName


## Transfer Module Object to Inf files
#
# Transfer all contents of a standard Module Object to an Inf file
diff --git a/BaseTools/Source/Python/UPT/Library/CommentGenerating.py b/BaseTools/Source/Python/UPT/Library/CommentGenerating.py
index 9c6e3aa..ab1725f 100644
--- a/BaseTools/Source/Python/UPT/Library/CommentGenerating.py
+++ b/BaseTools/Source/Python/UPT/Library/CommentGenerating.py
@@ -1,9 +1,9 @@
## @file
# This file is used to define comment generating interface
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -17,11 +17,11 @@ CommentGenerating
'''

##
# Import Modules
#
-from Library.String import GetSplitValueList
+from Library.StringUtils import GetSplitValueList
from Library.DataType import TAB_SPACE_SPLIT
from Library.DataType import TAB_INF_GUIDTYPE_VAR
from Library.DataType import USAGE_ITEM_NOTIFY
from Library.DataType import ITEM_UNDEFINED
from Library.DataType import TAB_HEADER_COMMENT
diff --git a/BaseTools/Source/Python/UPT/Library/CommentParsing.py b/BaseTools/Source/Python/UPT/Library/CommentParsing.py
index 38f7012..4713614 100644
--- a/BaseTools/Source/Python/UPT/Library/CommentParsing.py
+++ b/BaseTools/Source/Python/UPT/Library/CommentParsing.py
@@ -1,9 +1,9 @@
## @file
# This file is used to define comment parsing interface
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -19,12 +19,12 @@ CommentParsing
##
# Import Modules
#
import re

-from Library.String import GetSplitValueList
-from Library.String import CleanString2
+from Library.StringUtils import GetSplitValueList
+from Library.StringUtils import CleanString2
from Library.DataType import HEADER_COMMENT_NOT_STARTED
from Library.DataType import TAB_COMMENT_SPLIT
from Library.DataType import HEADER_COMMENT_LICENSE
from Library.DataType import HEADER_COMMENT_ABSTRACT
from Library.DataType import HEADER_COMMENT_COPYRIGHT
diff --git a/BaseTools/Source/Python/UPT/Library/Misc.py b/BaseTools/Source/Python/UPT/Library/Misc.py
index 719445b..e16d309 100644
--- a/BaseTools/Source/Python/UPT/Library/Misc.py
+++ b/BaseTools/Source/Python/UPT/Library/Misc.py
@@ -1,9 +1,9 @@
## @file
# Common routines used by all tools
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -43,11 +43,11 @@ from Library.DataType import END_OF_LINE
from Library.DataType import TAB_SPLIT
from Library.DataType import TAB_LANGUAGE_EN_US
from Library.DataType import TAB_LANGUAGE_EN
from Library.DataType import TAB_LANGUAGE_EN_X
from Library.DataType import TAB_UNI_FILE_SUFFIXS
-from Library.String import GetSplitValueList
+from Library.StringUtils import GetSplitValueList
from Library.ParserValidate import IsValidHexVersion
from Library.ParserValidate import IsValidPath
from Object.POM.CommonObject import TextObject
from Core.FileHook import __FileHookOpen__
from Common.MultipleWorkspace import MultipleWorkspace as mws
diff --git a/BaseTools/Source/Python/UPT/Library/ParserValidate.py b/BaseTools/Source/Python/UPT/Library/ParserValidate.py
index 2def90a..3f8ca9d 100644
--- a/BaseTools/Source/Python/UPT/Library/ParserValidate.py
+++ b/BaseTools/Source/Python/UPT/Library/ParserValidate.py
@@ -1,9 +1,9 @@
## @file ParserValidate.py
# Functions for parser validation
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -22,11 +22,11 @@ import platform

from Library.DataType import MODULE_LIST
from Library.DataType import COMPONENT_TYPE_LIST
from Library.DataType import PCD_USAGE_TYPE_LIST_OF_MODULE
from Library.DataType import TAB_SPACE_SPLIT
-from Library.String import GetSplitValueList
+from Library.StringUtils import GetSplitValueList
from Library.ExpressionValidate import IsValidBareCString
from Library.ExpressionValidate import IsValidFeatureFlagExp
from Common.MultipleWorkspace import MultipleWorkspace as mws

## __HexDigit() method
diff --git a/BaseTools/Source/Python/UPT/Library/Parsing.py b/BaseTools/Source/Python/UPT/Library/Parsing.py
index 791e064..22030e7 100644
--- a/BaseTools/Source/Python/UPT/Library/Parsing.py
+++ b/BaseTools/Source/Python/UPT/Library/Parsing.py
@@ -1,10 +1,10 @@
## @file
# This file is used to define common parsing related functions used in parsing
# INF/DEC/DSC process
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -21,16 +21,16 @@ Parsing
# Import Modules
#
import os.path
import re

-from Library.String import RaiseParserError
-from Library.String import GetSplitValueList
-from Library.String import CheckFileType
-from Library.String import CheckFileExist
-from Library.String import CleanString
-from Library.String import NormPath
+from Library.StringUtils import RaiseParserError
+from Library.StringUtils import GetSplitValueList
+from Library.StringUtils import CheckFileType
+from Library.StringUtils import CheckFileExist
+from Library.StringUtils import CleanString
+from Library.StringUtils import NormPath

from Logger.ToolError import FILE_NOT_FOUND
from Logger.ToolError import FatalError
from Logger.ToolError import FORMAT_INVALID

diff --git a/BaseTools/Source/Python/UPT/Library/StringUtils.py b/BaseTools/Source/Python/UPT/Library/StringUtils.py
index b79891e..a7a7b86 100644
--- a/BaseTools/Source/Python/UPT/Library/StringUtils.py
+++ b/BaseTools/Source/Python/UPT/Library/StringUtils.py
@@ -1,21 +1,21 @@
## @file
# This file is used to define common string related functions used in parsing
# process
#
-# Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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.
#
'''
-String
+StringUtils
'''
##
# Import Modules
#
import re
diff --git a/BaseTools/Source/Python/UPT/Library/UniClassObject.py b/BaseTools/Source/Python/UPT/Library/UniClassObject.py
index 66eefee..7dcf0cf 100644
--- a/BaseTools/Source/Python/UPT/Library/UniClassObject.py
+++ b/BaseTools/Source/Python/UPT/Library/UniClassObject.py
@@ -21,11 +21,11 @@ Collect all defined strings in multiple uni files
import os, codecs, re
import distutils.util
from Logger import ToolError
from Logger import Log as EdkLogger
from Logger import StringTable as ST
-from Library.String import GetLineNo
+from Library.StringUtils import GetLineNo
from Library.Misc import PathClass
from Library.Misc import GetCharIndexOutStr
from Library import DataType as DT
from Library.ParserValidate import CheckUTF16FileHeader

diff --git a/BaseTools/Source/Python/UPT/Object/Parser/InfDefineObject.py b/BaseTools/Source/Python/UPT/Object/Parser/InfDefineObject.py
index bbc797f..3995546 100644
--- a/BaseTools/Source/Python/UPT/Object/Parser/InfDefineObject.py
+++ b/BaseTools/Source/Python/UPT/Object/Parser/InfDefineObject.py
@@ -1,10 +1,10 @@
## @file
# This file is used to define class objects of [Defines] section for INF file.
# It will consumed by InfParser
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -21,11 +21,11 @@ import re

from Logger import StringTable as ST
from Logger import ToolError
from Library import GlobalData
from Library import DataType as DT
-from Library.String import GetSplitValueList
+from Library.StringUtils import GetSplitValueList
from Library.Misc import CheckGuidRegFormat
from Library.Misc import Sdict
from Library.Misc import ConvPathFromAbsToRel
from Library.Misc import ValidateUNIFilePath
from Library.ExpressionValidate import IsValidFeatureFlagExp
diff --git a/BaseTools/Source/Python/UPT/Object/Parser/InfPcdObject.py b/BaseTools/Source/Python/UPT/Object/Parser/InfPcdObject.py
index d2712a9..62c1e84 100644
--- a/BaseTools/Source/Python/UPT/Object/Parser/InfPcdObject.py
+++ b/BaseTools/Source/Python/UPT/Object/Parser/InfPcdObject.py
@@ -1,10 +1,10 @@
## @file
# This file is used to define class objects of INF file [Pcds] section.
# It will consumed by InfParser.
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -29,12 +29,12 @@ from Library.Misc import GetHelpStringByRemoveHashKey
from Library.ParserValidate import IsValidPcdType
from Library.ParserValidate import IsValidCVariableName
from Library.ParserValidate import IsValidPcdValue
from Library.ParserValidate import IsValidArch
from Library.CommentParsing import ParseComment
-from Library.String import GetSplitValueList
-from Library.String import IsHexDigitUINT32
+from Library.StringUtils import GetSplitValueList
+from Library.StringUtils import IsHexDigitUINT32
from Library.ExpressionValidate import IsValidFeatureFlagExp
from Parser.InfAsBuiltProcess import GetPackageListInfo
from Parser.DecParser import Dec

from Object.Parser.InfPackagesObject import InfPackageItem
diff --git a/BaseTools/Source/Python/UPT/Parser/DecParser.py b/BaseTools/Source/Python/UPT/Parser/DecParser.py
index 85b8a17..7ac0dfa 100644
--- a/BaseTools/Source/Python/UPT/Parser/DecParser.py
+++ b/BaseTools/Source/Python/UPT/Parser/DecParser.py
@@ -1,9 +1,9 @@
## @file
# This file is used to parse DEC file. It will consumed by DecParser
#
-# Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -55,14 +55,14 @@ from Object.Parser.DecObject import DecUserExtensionObject
from Object.Parser.DecObject import DecUserExtensionItemObject
from Object.Parser.DecObject import DecPcdObject
from Object.Parser.DecObject import DecPcdItemObject
from Library.Misc import GuidStructureStringToGuidString
from Library.Misc import CheckGuidRegFormat
-from Library.String import ReplaceMacro
-from Library.String import GetSplitValueList
-from Library.String import gMACRO_PATTERN
-from Library.String import ConvertSpecialChar
+from Library.StringUtils import ReplaceMacro
+from Library.StringUtils import GetSplitValueList
+from Library.StringUtils import gMACRO_PATTERN
+from Library.StringUtils import ConvertSpecialChar
from Library.CommentParsing import ParsePcdErrorCode

##
# _DecBase class for parsing
#
diff --git a/BaseTools/Source/Python/UPT/Parser/InfAsBuiltProcess.py b/BaseTools/Source/Python/UPT/Parser/InfAsBuiltProcess.py
index 4eed04c..760f28a 100644
--- a/BaseTools/Source/Python/UPT/Parser/InfAsBuiltProcess.py
+++ b/BaseTools/Source/Python/UPT/Parser/InfAsBuiltProcess.py
@@ -1,9 +1,9 @@
## @file
# This file is used to provide method for process AsBuilt INF file. It will consumed by InfParser
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -21,11 +21,11 @@ import re
from Library import GlobalData
import Logger.Log as Logger
from Logger import StringTable as ST
from Logger import ToolError

-from Library.String import GetSplitValueList
+from Library.StringUtils import GetSplitValueList
from Library.Misc import GetHelpStringByRemoveHashKey
from Library.Misc import ValidFile
from Library.Misc import ProcessLineExtender
from Library.ParserValidate import IsValidPath
from Library.Parsing import MacroParser
diff --git a/BaseTools/Source/Python/UPT/Parser/InfParser.py b/BaseTools/Source/Python/UPT/Parser/InfParser.py
index 7bea49e..e6048a1 100644
--- a/BaseTools/Source/Python/UPT/Parser/InfParser.py
+++ b/BaseTools/Source/Python/UPT/Parser/InfParser.py
@@ -1,9 +1,9 @@
## @file
# This file contained the parser for INF file
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -21,12 +21,12 @@ InfParser
#
import re
import os
from copy import deepcopy

-from Library.String import GetSplitValueList
-from Library.String import ConvertSpecialChar
+from Library.StringUtils import GetSplitValueList
+from Library.StringUtils import ConvertSpecialChar
from Library.Misc import ProcessLineExtender
from Library.Misc import ProcessEdkComment
from Library.Parsing import NormPath
from Library.ParserValidate import IsValidInfMoudleTypeList
from Library.ParserValidate import IsValidArch
diff --git a/BaseTools/Source/Python/UPT/Parser/InfParserMisc.py b/BaseTools/Source/Python/UPT/Parser/InfParserMisc.py
index 6a335e8..df32225 100644
--- a/BaseTools/Source/Python/UPT/Parser/InfParserMisc.py
+++ b/BaseTools/Source/Python/UPT/Parser/InfParserMisc.py
@@ -1,9 +1,9 @@
## @file
# This file contained the miscellaneous functions for INF parser
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -23,12 +23,12 @@ import re


from Library import DataType as DT


-from Library.String import gMACRO_PATTERN
-from Library.String import ReplaceMacro
+from Library.StringUtils import gMACRO_PATTERN
+from Library.StringUtils import ReplaceMacro
from Object.Parser.InfMisc import ErrorInInf
from Logger.StringTable import ERR_MARCO_DEFINITION_MISS_ERROR

#
# Global variable
diff --git a/BaseTools/Source/Python/UPT/Parser/InfPcdSectionParser.py b/BaseTools/Source/Python/UPT/Parser/InfPcdSectionParser.py
index a9b87fd..13535a37 100644
--- a/BaseTools/Source/Python/UPT/Parser/InfPcdSectionParser.py
+++ b/BaseTools/Source/Python/UPT/Parser/InfPcdSectionParser.py
@@ -1,9 +1,9 @@
## @file
# This file contained the parser for [Pcds] sections in INF file
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -24,11 +24,11 @@ from Logger.ToolError import FORMAT_INVALID
from Parser.InfParserMisc import InfExpandMacro
from Library import DataType as DT
from Library.Parsing import MacroParser
from Library.Misc import GetSplitValueList
from Library import GlobalData
-from Library.String import SplitPcdEntry
+from Library.StringUtils import SplitPcdEntry
from Parser.InfParserMisc import InfParserSectionRoot

class InfPcdSectionParser(InfParserSectionRoot):
## Section PCD related parser
#
diff --git a/BaseTools/Source/Python/UPT/Parser/InfSectionParser.py b/BaseTools/Source/Python/UPT/Parser/InfSectionParser.py
index 727164c..8ba4c3f 100644
--- a/BaseTools/Source/Python/UPT/Parser/InfSectionParser.py
+++ b/BaseTools/Source/Python/UPT/Parser/InfSectionParser.py
@@ -1,9 +1,9 @@
## @file
# This file contained the parser for sections in INF file
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -19,11 +19,11 @@ InfSectionParser
# Import Modules
#
from copy import deepcopy
import re

-from Library.String import GetSplitValueList
+from Library.StringUtils import GetSplitValueList
from Library.CommentParsing import ParseHeaderCommentSection
from Library.CommentParsing import ParseComment

from Library import DataType as DT

diff --git a/BaseTools/Source/Python/UPT/PomAdapter/InfPomAlignment.py b/BaseTools/Source/Python/UPT/PomAdapter/InfPomAlignment.py
index 165ac11..a5929e1 100644
--- a/BaseTools/Source/Python/UPT/PomAdapter/InfPomAlignment.py
+++ b/BaseTools/Source/Python/UPT/PomAdapter/InfPomAlignment.py
@@ -1,9 +1,9 @@
## @file InfPomAlignment.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 - 2018, 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
@@ -18,14 +18,14 @@ InfPomAlignment
# Import modules
#
import os.path
from Logger import StringTable as ST
import Logger.Log as Logger
-from Library.String import FORMAT_INVALID
-from Library.String import PARSER_ERROR
-from Library.String import NormPath
-from Library.String import GetSplitValueList
+from Library.StringUtils import FORMAT_INVALID
+from Library.StringUtils import PARSER_ERROR
+from Library.StringUtils import NormPath
+from Library.StringUtils import GetSplitValueList
from Library.Misc import ConvertVersionToDecimal
from Library.Misc import GetHelpStringByRemoveHashKey
from Library.Misc import ConvertArchList
from Library.Misc import GetRelativePath
from Library.Misc import PathClass
diff --git a/BaseTools/Source/Python/UPT/PomAdapter/InfPomAlignmentMisc.py b/BaseTools/Source/Python/UPT/PomAdapter/InfPomAlignmentMisc.py
index cca70e5..cee4251 100644
--- a/BaseTools/Source/Python/UPT/PomAdapter/InfPomAlignmentMisc.py
+++ b/BaseTools/Source/Python/UPT/PomAdapter/InfPomAlignmentMisc.py
@@ -1,9 +1,9 @@
## @file InfPomAlignmentMisc.py
# This file contained the routines for InfPomAlignment
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -22,11 +22,11 @@ InfPomAlignmentMisc
import Logger.Log as Logger
from Library import DataType as DT
from Library.Misc import ConvertArchList
from Object.POM.ModuleObject import BinaryFileObject
from Object.POM import CommonObject
-from Library.String import FORMAT_INVALID
+from Library.StringUtils import FORMAT_INVALID
from Library.Misc import CheckGuidRegFormat
from Logger import StringTable as ST


## GenModuleHeaderUserExt
diff --git a/BaseTools/Source/Python/UPT/UnitTest/CommentGeneratingUnitTest.py b/BaseTools/Source/Python/UPT/UnitTest/CommentGeneratingUnitTest.py
index 42a2ba3..9c50d2d 100644
--- a/BaseTools/Source/Python/UPT/UnitTest/CommentGeneratingUnitTest.py
+++ b/BaseTools/Source/Python/UPT/UnitTest/CommentGeneratingUnitTest.py
@@ -1,9 +1,9 @@
## @file
# This file contain unit test for CommentParsing
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -26,11 +26,11 @@ from Object.POM.CommonObject import GuidObject
from Object.POM.CommonObject import ProtocolObject
from Object.POM.CommonObject import PpiObject
from Object.POM.CommonObject import PcdObject
from Object.POM.ModuleObject import HobObject

-from Library.String import GetSplitValueList
+from Library.StringUtils import GetSplitValueList
from Library.DataType import TAB_SPACE_SPLIT
from Library.DataType import TAB_LANGUAGE_EN_US
from Library.DataType import TAB_LANGUAGE_ENG
from Library.DataType import ITEM_UNDEFINED
from Library.DataType import TAB_INF_FEATURE_PCD
diff --git a/BaseTools/Source/Python/UPT/UnitTest/CommentParsingUnitTest.py b/BaseTools/Source/Python/UPT/UnitTest/CommentParsingUnitTest.py
index a114ff2..4593506 100644
--- a/BaseTools/Source/Python/UPT/UnitTest/CommentParsingUnitTest.py
+++ b/BaseTools/Source/Python/UPT/UnitTest/CommentParsingUnitTest.py
@@ -1,9 +1,9 @@
## @file
# This file contain unit test for CommentParsing
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -17,11 +17,11 @@ import Logger.Log as Logger
from Library.CommentParsing import ParseHeaderCommentSection, \
ParseGenericComment, \
ParseDecPcdGenericComment, \
ParseDecPcdTailComment
from Library.CommentParsing import _IsCopyrightLine
-from Library.String import GetSplitValueList
+from Library.StringUtils import GetSplitValueList
from Library.DataType import TAB_SPACE_SPLIT
from Library.DataType import TAB_LANGUAGE_EN_US

#
# Test ParseHeaderCommentSection
diff --git a/BaseTools/Source/Python/UPT/Xml/CommonXml.py b/BaseTools/Source/Python/UPT/Xml/CommonXml.py
index e28aec5..805310d 100644
--- a/BaseTools/Source/Python/UPT/Xml/CommonXml.py
+++ b/BaseTools/Source/Python/UPT/Xml/CommonXml.py
@@ -1,9 +1,9 @@
## @file
# This file is used to parse a PCD file of .PKG file
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -19,14 +19,14 @@ CommonXml
##
# Import Modules
#

from Core.DistributionPackageClass import DistributionPackageHeaderObject
-from Library.String import ConvertNEToNOTEQ
-from Library.String import ConvertNOTEQToNE
-from Library.String import GetSplitValueList
-from Library.String import GetStringOfList
+from Library.StringUtils import ConvertNEToNOTEQ
+from Library.StringUtils import ConvertNOTEQToNE
+from Library.StringUtils import GetSplitValueList
+from Library.StringUtils import GetStringOfList
from Library.Xml.XmlRoutines import XmlElement
from Library.Xml.XmlRoutines import XmlElement2
from Library.Xml.XmlRoutines import XmlAttribute
from Library.Xml.XmlRoutines import XmlNode
from Library.Xml.XmlRoutines import XmlList
diff --git a/BaseTools/Source/Python/UPT/Xml/GuidProtocolPpiXml.py b/BaseTools/Source/Python/UPT/Xml/GuidProtocolPpiXml.py
index bfd8d4f..a747b02 100644
--- a/BaseTools/Source/Python/UPT/Xml/GuidProtocolPpiXml.py
+++ b/BaseTools/Source/Python/UPT/Xml/GuidProtocolPpiXml.py
@@ -1,9 +1,9 @@
## @file
# This file is used to parse a xml file of .PKG file
#
-# Copyright (c) 2011, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -13,13 +13,13 @@
#

'''
GuidProtocolPpiXml
'''
-from Library.String import ConvertNEToNOTEQ
-from Library.String import ConvertNOTEQToNE
-from Library.String import GetStringOfList
+from Library.StringUtils import ConvertNEToNOTEQ
+from Library.StringUtils import ConvertNOTEQToNE
+from Library.StringUtils import GetStringOfList
from Library.Xml.XmlRoutines import XmlElement
from Library.Xml.XmlRoutines import XmlAttribute
from Library.Xml.XmlRoutines import XmlNode
from Library.Xml.XmlRoutines import XmlList
from Library.Xml.XmlRoutines import CreateXmlElement
diff --git a/BaseTools/Source/Python/UPT/Xml/IniToXml.py b/BaseTools/Source/Python/UPT/Xml/IniToXml.py
index 0374710..aa6f230 100644
--- a/BaseTools/Source/Python/UPT/Xml/IniToXml.py
+++ b/BaseTools/Source/Python/UPT/Xml/IniToXml.py
@@ -1,9 +1,9 @@
## @file
# This file is for converting package information data file to xml file.
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -28,11 +28,11 @@ from Library.Xml.XmlRoutines import CreateXmlElement
from Library.DataType import TAB_VALUE_SPLIT
from Library.DataType import TAB_EQUAL_SPLIT
from Library.DataType import TAB_SECTION_START
from Library.DataType import TAB_SECTION_END
from Logger import StringTable as ST
-from Library.String import ConvertSpecialChar
+from Library.StringUtils import ConvertSpecialChar
from Library.ParserValidate import IsValidPath
from Library import GlobalData

## log error:
#
diff --git a/BaseTools/Source/Python/UPT/Xml/ModuleSurfaceAreaXml.py b/BaseTools/Source/Python/UPT/Xml/ModuleSurfaceAreaXml.py
index 51ac48a..7480cb5 100644
--- a/BaseTools/Source/Python/UPT/Xml/ModuleSurfaceAreaXml.py
+++ b/BaseTools/Source/Python/UPT/Xml/ModuleSurfaceAreaXml.py
@@ -1,9 +1,9 @@
## @file
# This file is used to parse a Module file of .PKG file
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -15,14 +15,14 @@
'''
ModuleSurfaceAreaXml
'''
from xml.dom import minidom

-from Library.String import ConvertNEToNOTEQ
-from Library.String import ConvertNOTEQToNE
-from Library.String import GetStringOfList
-from Library.String import IsMatchArch
+from Library.StringUtils import ConvertNEToNOTEQ
+from Library.StringUtils import ConvertNOTEQToNE
+from Library.StringUtils import GetStringOfList
+from Library.StringUtils import IsMatchArch
from Library.Xml.XmlRoutines import XmlElement
from Library.Xml.XmlRoutines import XmlAttribute
from Library.Xml.XmlRoutines import XmlNode
from Library.Xml.XmlRoutines import XmlList
from Library.Xml.XmlRoutines import CreateXmlElement
diff --git a/BaseTools/Source/Python/UPT/Xml/PackageSurfaceAreaXml.py b/BaseTools/Source/Python/UPT/Xml/PackageSurfaceAreaXml.py
index d6ed8c5..30091c6 100644
--- a/BaseTools/Source/Python/UPT/Xml/PackageSurfaceAreaXml.py
+++ b/BaseTools/Source/Python/UPT/Xml/PackageSurfaceAreaXml.py
@@ -1,9 +1,9 @@
## @file
# This file is used to parse a Package file of .PKG file
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -15,11 +15,11 @@
'''
PackageSurfaceAreaXml
'''
from xml.dom import minidom

-from Library.String import GetStringOfList
+from Library.StringUtils import GetStringOfList
from Library.Xml.XmlRoutines import XmlElement
from Library.Xml.XmlRoutines import XmlNode
from Library.Xml.XmlRoutines import XmlList
from Library.Xml.XmlRoutines import CreateXmlElement
from Object.POM.CommonObject import IncludeObject
diff --git a/BaseTools/Source/Python/UPT/Xml/PcdXml.py b/BaseTools/Source/Python/UPT/Xml/PcdXml.py
index 4603918..c0dc654 100644
--- a/BaseTools/Source/Python/UPT/Xml/PcdXml.py
+++ b/BaseTools/Source/Python/UPT/Xml/PcdXml.py
@@ -1,9 +1,9 @@
## @file
# This file is used to parse a PCD file of .PKG file
#
-# Copyright (c) 2011 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2011 - 2018, 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
@@ -23,13 +23,13 @@ PcdXml
from Library.Xml.XmlRoutines import XmlElement
from Library.Xml.XmlRoutines import XmlAttribute
from Library.Xml.XmlRoutines import XmlNode
from Library.Xml.XmlRoutines import CreateXmlElement
from Library.Xml.XmlRoutines import XmlList
-from Library.String import GetStringOfList
-from Library.String import ConvertNEToNOTEQ
-from Library.String import ConvertNOTEQToNE
+from Library.StringUtils import GetStringOfList
+from Library.StringUtils import ConvertNEToNOTEQ
+from Library.StringUtils import ConvertNOTEQToNE
from Library import GlobalData
from Object.POM.CommonObject import PcdObject
from Object.POM.CommonObject import PcdErrorObject
from Xml.CommonXml import HelpTextXml
from Xml.CommonXml import PromptXml
--
2.6.1.windows.1


Re: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to GenFw

Shi, Steven <steven.shi@...>
 

Next week is OK, take your time. I appreciate the test cases with build steps, thanks!

BTW, I'm not sure where is the right place in edk2 to submit a patch's test cases, but I do welcome the test case contribution.


Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522

-----Original Message-----
From: Zenith432 [mailto:zenith432@...]
Sent: Thursday, June 7, 2018 2:09 PM
To: Shi, Steven <steven.shi@...>
Cc: Gao, Liming <liming.gao@...>; edk2-devel@...
Subject: RE: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to
GenFw

As I said in other mailing - I only tested on cases in builld. If you want
standalone test cases I only have time next week to make some.

--------------------------------------------
On Thu, 6/7/18, Shi, Steven <steven.shi@...> wrote:

Subject: RE: [PATCH] BaseTools/GenFw: Add X64 GOTPCREL Support to
GenFw
To: "Gao, Liming" <liming.gao@...>, "Zenith432"
<zenith432@...>, "edk2-devel@..." <edk2-
devel@...>
Date: Thursday, June 7, 2018, 5:24 AM

Hi Zenith,
BTW,
besides the build pass, did you try to run a Uefi binary,
e.g. a simple shell application, which contain the GOTPCREL
relocations? If yes. Please send out your test case code as
well. I appreciate if you could contribute your test cases
as the patch together.


Re: OSFC 2018

Philipp Deppenwiese <zaolin.daisuki@...>
 

Yes.

On 07.06.2018 17:56, Rafael Machado wrote:
Nice initiative!

Will the presentations be recorded and posted at youtube?

Thanks
Rafael R. Machado

Em ter, 5 de jun de 2018 às 09:10, Philipp Deppenwiese
<zaolin.daisuki@... <mailto:zaolin.daisuki@...>> escreveu:

Dear Ladies and Gentlemen,

We invite you to the Open Source Firmware Conference 2018
( www.osfc.io <http://www.osfc.io> ) which takes place at the 12th
- 15th September
in Erlangen, Germany.

The OSFC 2018 is the first conference focusing exclusively on Open
Source firmware.
Therefore, our mission is to provide an appropriate platform to bring
together as many Open Source projects,
hardware manufacturers and developers as possible.
In order to collaborate, share knowledge and push the Open Source
firmware development.

Get in touch with community of coreboot, LinuxBoot, Tianocore, U-Boot
and be part of our amazing firmware conference!

Tickets are available: https://osfc.io/tickets
CFP is available: https://easychair.org/cfp/osfc2018
Follow us on Twitter: osfc_io


Best Regards, Philipp
_______________________________________________
edk2-devel mailing list
edk2-devel@... <mailto:edk2-devel@...>
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [PATCH] MdeModulePkg/EmmcDxe: demote DEBUG print to DEBUG_BLKIO

Laszlo Ersek
 

On 06/07/18 11:10, Ard Biesheuvel wrote:
Lower the priority of the DEBUG print in EmmcReadWrite(), which
is emitted for each read or write operation to the eMMC device,
which clutters up the log output of builds created with DEBUG_INFO
enabled.

Suggested-by: Pipat Methavanitpong <methavanitpong.pipat@...>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>
---
MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
index e1d0f394a954..f6b230514b71 100644
--- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
+++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
@@ -901,7 +901,10 @@ EmmcReadWrite (
if (EFI_ERROR (Status)) {
return Status;
}
- DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n", IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum, (Token != NULL) ? Token->Event : NULL, Status));
+ DEBUG ((DEBUG_BLKIO,
+ "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p with %r\n",
+ IsRead ? "Read " : "Write", Partition->PartitionType, Lba, BlockNum,
+ (Token != NULL) ? Token->Event : NULL, Status));

Lba += BlockNum;
Buffer = (UINT8*)Buffer + BufferSize;
Reviewed-by: Laszlo Ersek <lersek@...>


Re: [PATCH] MdePkg/BaseIoLibIntrinsic: make BaseIoLibIntrinsic safe for ArmVirt/KVM

Laszlo Ersek
 

On 06/07/18 12:46, Ard Biesheuvel wrote:
KVM on ARM refuses to decode load/store instructions used to perform
I/O to emulated devices, and instead relies on the exception syndrome
information to describe the operand register, access size, etc.
This is only possible for instructions that have a single input/output
register (as opposed to ones that increment the offset register, or
load/store pair instructions, etc). Otherwise, QEMU crashes with the
following error

error: kvm run failed Function not implemented
R00=01010101 R01=00000008 R02=00000048 R03=08000820
R04=00000120 R05=7faaa0e0 R06=7faaa0dc R07=7faaa0e8
R08=7faaa0ec R09=7faaa088 R10=000000ff R11=00000080
R12=ff000000 R13=7fccfe08 R14=7faa835f R15=7faa887c
PSR=800001f3 N--- T svc32
QEMU: Terminated

and KVM produces a warning such as the following in the kernel log

kvm [17646]: load/store instruction decoding not implemented

The IoLib implementation provided by MdePkg/Library/BaseIoLibIntrinsic
is based on C code, and when LTO is in effect, the MMIO accesses could
be merged with, e.g., manipulations of the loop counter, producing
opcodes that KVM does not support for emulated MMIO.

So let's add a special ArmVirt flavor of this library that implements
that actual load/store operations in assembler, ensuring that the
instructions involved can be emulated by KVM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>
---
ArmVirtPkg/ArmVirt.dsc.inc | 2 +-
MdePkg/Library/BaseIoLibIntrinsic/AArch64/ArmVirtMmio.S | 164 +++++
MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.S | 161 +++++
MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 165 +++++
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 50 ++
MdePkg/Library/BaseIoLibIntrinsic/IoLibArmVirt.c | 641 ++++++++++++++++++++
6 files changed, 1182 insertions(+), 1 deletion(-)
Assuming Liming is OK with this patch for MdePkg, I have some comments
(specific to adding this package to MdePkg):

(1) ArmVirtPkg and MdePkg should likely not be modified in the same patch.

(2) The library instance should be added as a standalone component to
MdePkg.dsc (so it can be built without any client modules):

[Components.ARM, Components.AARCH64]
MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf <-HERE

(3) Library instances under MdePkg are supposed to have
MODULE_UNI_FILEs. I think customizing "BaseIoLibIntrinsic.uni" shouldn't
be hard; it's basically a different format for re-stating the blurb of
the new INF file.


(Obviously, don't do anything about these until Liming follows up.)

The rest looks OK to me!

Thank you for this!
Laszlo


Re: [PATCH] CorebootModulePkg/CbSupportDxe: Remove SCI_EN setting

Matt Delco <delco@...>
 

Thanks for working on this. I did a quick test to confirm that things
still work okay. The code change looks fine though there's some optional
optimizations to consider:

The new code added to CbParseFadtInfo() is mostly to add debug checks but I
think it'll still result in a IO port access in a release build so it might
be worthwhile to make the entire code block specific to a debug build
(possibly by sticking the code within a "#if !defined(MDEPKG_NDEBUG)"
block, though I don't see any other such cases in edk2 so I suspect this
isn't an ideal suggestion). Regarding "Pm1CntLen" It looks like the "if
(==2) {} else if (==4) {} else {}" could be simplified to "if (==4) {} else
{}".

In CbDxeEntryPoint() the "Find the acpi board information guid hob" code
block can probably be deleted. Its only remaining use is the debug log
regarding the value of PmCtrlReg, which I suspect isn't that valuable given
the other deletions. This would also allow the removal of the "mPmCtrlReg"
variable at file scope.

On Thu, Jun 7, 2018 at 8:40 AM, Ma, Maurice <maurice.ma@...> wrote:

It looks good to me.
Reviewed-by: Maurice Ma <maurice.ma@...>


-----Original Message-----
From: You, Benjamin
Sent: Thursday, June 7, 2018 1:19
To: edk2-devel@...
Cc: Ma, Maurice <maurice.ma@...>; Agyeman, Prince <
prince.agyeman@...>; delco@...
Subject: [PATCH] CorebootModulePkg/CbSupportDxe: Remove SCI_EN setting

Current implemenation sets PM1_CNT.SCI_EN bit at ReadyToBoot event.
However, this should not be done because this causes OS to skip triggering
FADT.SMI_CMD, which leads to the functions implemented in the SMI handler
being omitted.

This issue was identified by Matt Delco <delco@...>.

The fix does the following:
- The SCI_EN bit setting is removed from CbSupportDxe driver.
- Some additional checks are added in CbParseFadtInfo() in CbParseLib.c to
output some error message and ASSERT (FALSE) if ALL of the following
conditions are met:
1) HARDWARE_REDUCED_ACPI is not set;
2) SMI_CMD field is zero;
3) SCI_EN bit is zero;
which indicates the ACPI enabling status is inconsistent: SCI is not
enabled but the ACPI table does not provide a means to enable it through
FADT->SMI_CMD. This may cause issues in OS.

Cc: Maurice Ma <maurice.ma@...>
Cc: Prince Agyeman <prince.agyeman@...>
Cc: Matt Delco <delco@...>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Benjamin You <benjamin.you@...>
---
CorebootModulePkg/CbSupportDxe/CbSupportDxe.c | 39
----------------------
CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf | 1 -
CorebootModulePkg/Library/CbParseLib/CbParseLib.c | 28 ++++++++++++++++
.../Library/CbParseLib/CbParseLib.inf | 3 +-
4 files changed, 30 insertions(+), 41 deletions(-)

diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
index c526c9e871..b41c0885f7 100755
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
@@ -86,31 +86,6 @@ CbReserveResourceInGcd (
return Status;
}

-/**
- Notification function of EVT_GROUP_READY_TO_BOOT event group.
-
- This is a notification function registered on EVT_GROUP_READY_TO_BOOT
event group.
- When the Boot Manager is about to load and execute a boot option, it
reclaims variable
- storage if free size is below the threshold.
-
- @param Event Event whose notification function is being invoked.
- @param Context Pointer to the notification function's context.
-
-**/
-VOID
-EFIAPI
-OnReadyToBoot (
- IN EFI_EVENT Event,
- IN VOID *Context
- )
-{
- //
- // Enable SCI
- //
- IoOr16 (mPmCtrlReg, BIT0);
-
- DEBUG ((EFI_D_ERROR, "Enable SCI bit at 0x%lx before boot\n",
(UINT64)mPmCtrlReg)); -}

/**
Main entry for the Coreboot Support DXE module.
@@ -130,7 +105,6 @@ CbDxeEntryPoint (
)
{
EFI_STATUS Status;
- EFI_EVENT ReadyToBootEvent;
EFI_HOB_GUID_TYPE *GuidHob;
SYSTEM_TABLE_INFO *pSystemTableInfo;
ACPI_BOARD_INFO *pAcpiBoardInfo;
@@ -197,19 +171,6 @@ CbDxeEntryPoint (
ASSERT_EFI_ERROR (Status);
}

- //
- // Register callback on the ready to boot event
- // in order to enable SCI
- //
- ReadyToBootEvent = NULL;
- Status = EfiCreateEventReadyToBootEx (
- TPL_CALLBACK,
- OnReadyToBoot,
- NULL,
- &ReadyToBootEvent
- );
- ASSERT_EFI_ERROR (Status);
-
return EFI_SUCCESS;
}

diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
index 99245183ea..15b0dac774 100644
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
@@ -46,7 +46,6 @@
DebugLib
BaseMemoryLib
UefiLib
- IoLib
HobLib

[Guids]
diff --git a/CorebootModulePkg/Library/CbParseLib/CbParseLib.c
b/CorebootModulePkg/Library/CbParseLib/CbParseLib.c
index 0909b0f492..cd98a72f01 100644
--- a/CorebootModulePkg/Library/CbParseLib/CbParseLib.c
+++ b/CorebootModulePkg/Library/CbParseLib/CbParseLib.c
@@ -18,6 +18,7 @@
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/PcdLib.h>
+#include <Library/IoLib.h>
#include <Library/CbParseLib.h>

#include <IndustryStandard/Acpi.h>
@@ -412,6 +413,7 @@ CbParseFadtInfo (
UINTN Entry64Num;
UINTN Idx;
RETURN_STATUS Status;
+ BOOLEAN SciEnabled;

Rsdp = NULL;
Status = RETURN_SUCCESS;
@@ -477,6 +479,32 @@ CbParseFadtInfo (
ASSERT(Fadt->Pm1aEvtBlk != 0);
ASSERT(Fadt->Gpe0Blk != 0);

+ //
+ // Get SCI_EN value
+ //
+ if (Fadt->Pm1CntLen == 2) {
+ SciEnabled = (IoRead16 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
+ } else if (Fadt->Pm1CntLen == 4) {
+ SciEnabled = (IoRead32 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
+ } else {
+ //
+ // Unsupported PM1 CNT Length, revert to 16 bit
+ //
+ SciEnabled = (IoRead16 (Fadt->Pm1aCntBlk) & BIT0)? TRUE : FALSE;
+ }
+
+ if (!(Fadt->Flags & EFI_ACPI_5_0_HW_REDUCED_ACPI) &&
+ (Fadt->SmiCmd == 0) &&
+ !SciEnabled) {
+ //
+ // The ACPI enabling status is inconsistent: SCI is not enabled
but ACPI
+ // table does not provide a means to enable it through
FADT->SmiCmd
+ //
+ DEBUG ((DEBUG_ERROR, "ERROR: The ACPI enabling status is
inconsistent: SCI is not"
+ " enabled but the ACPI table does not provide a means to
enable it through FADT->SmiCmd."
+ " This may cause issues in OS.\n"));
+ ASSERT (FALSE);
+ }
return RETURN_SUCCESS;
}
}
diff --git a/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf
b/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf
index d7146a415b..25b847946c 100644
--- a/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf
+++ b/CorebootModulePkg/Library/CbParseLib/CbParseLib.inf
@@ -37,7 +37,8 @@
[LibraryClasses]
BaseLib
BaseMemoryLib
- DebugLib

+ IoLib
+ DebugLib
PcdLib

[Pcd]
--
2.14.3.windows.1


Re: [PATCH] ArmPkg/ArmDisassemblerLib: fix check for MSR instruction

Michael Zimmermann
 

yes I'll do that next time. Thanks for the hint.

Thanks
Michael

On Thu, Jun 7, 2018 at 9:10 AM Ard Biesheuvel <ard.biesheuvel@...> wrote:

On 7 June 2018 at 07:08, Michael Zimmermann <sigmaepsilon92@...> wrote:
CC the arm maintainers
On Thu, Jun 7, 2018 at 7:07 AM Michael Zimmermann
<sigmaepsilon92@...> wrote:

From: M1cha <sigmaepsilon92@...>
Could you please use the same 'from' name+address as in the signoff?
That saves me the hassle of fixing it up manually.

GCC8 reported it with the following warning:
ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c: In function 'DisassembleArmInstruction':
ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c:397:30: error: bitwise
comparison always evaluates to false [-Werror=tautological-compare]
if ((OpCode & 0x0db00000) == 0x03200000) {

This condition trys to be true for both the immediate and the register
version of the MSR instruction. They get identified inside the if-block
using the variable I, which contains the value of bit 25.

The problem with the comparison reported by GCC is that the
bitmask excludes bit 25, while the value requires it to be set to one:
0x0db00000: 0000 11011 0 11 00 00 0000 000000000000
0x03200000: 0000 00110 0 10 00 00 0000 000000000000
^
So the solution is to just don't require that bit to be set, because
it gets checked later using 'I', which results in the following value:
0x01200000: 0000 00010 0 10 00 00 0000 000000000000

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael Zimmermann <sigmaepsilon92@...>
---
ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c b/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c
index 29d9414a78b3..b449a5d3cd83 100644
--- a/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c
+++ b/ArmPkg/Library/ArmDisassemblerLib/ArmDisassembler.c
@@ -394,7 +394,7 @@ DisassembleArmInstruction (
}


- if ((OpCode & 0x0db00000) == 0x03200000) {
+ if ((OpCode & 0x0db00000) == 0x01200000) {
// A4.1.38 MSR{<cond>} CPSR_<fields>, #<immediate> MSR{<cond>} CPSR_<fields>, <Rm>
if (I) {
// MSR{<cond>} CPSR_<fields>, #<immediate>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@...>

Pushed as b20085454e91bb1ded87009722c9994b4684472c

Thanks,


Re: [PATCH] ArmPkg/CompilerIntrinsicsLib: fix GCC8 warning for __aeabi_memcpy aliases

Michael Zimmermann
 

Hi Ard,

yes that fixes the problem too and looks much better, thx!

On Thu, Jun 7, 2018 at 9:05 AM Ard Biesheuvel <ard.biesheuvel@...> wrote:

On 7 June 2018 at 07:47, Michael Zimmermann <sigmaepsilon92@...> wrote:
This was the warning(shown for __aeabi_memcpy, __aeabi_memcpy4 and __aeabi_memcpy8):
ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c:42:6: error: '__aeabi_memcpy8' alias between functions of incompatible types 'void(void
*, const void *, size_t)' {aka 'void(void *, const void *, unsigned int)'} and 'void *(void *, const void *, size_t)' {aka 'void
*(void *, const void *, unsigned int)'} [-Werror=attribute-alias]
void __aeabi_memcpy8(void *dest, const void *src, size_t n);
^~~~~~~~~~~~~~~
ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c:19:7: note: aliased declaration here
void *__memcpy(void *dest, const void *src, size_t n)

The problem is the different return type(void vs void*).
This commit adds a wrapper '__aeabi___memcpy' with a void return value.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael Zimmermann <sigmaepsilon92@...>
---
ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c b/ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c
index a944e00b89e1..507234186fa9 100644
--- a/ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c
+++ b/ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c
@@ -31,14 +31,19 @@ __attribute__((__alias__("__memcpy")))
void *memcpy(void *dest, const void *src, size_t n);

#ifdef __arm__
+static __attribute__((__used__))
+void __aeabi___memcpy(void *dest, const void *src, size_t n)
+{
+ __memcpy(dest, src, n);
+}

-__attribute__((__alias__("__memcpy")))
+__attribute__((__alias__("__aeabi___memcpy")))
void __aeabi_memcpy(void *dest, const void *src, size_t n);

-__attribute__((__alias__("__memcpy")))
+__attribute__((__alias__("__aeabi___memcpy")))
void __aeabi_memcpy4(void *dest, const void *src, size_t n);

-__attribute__((__alias__("__memcpy")))
+__attribute__((__alias__("__aeabi___memcpy")))
void __aeabi_memcpy8(void *dest, const void *src, size_t n);

#endif
Thanks Michael.

Would this fix the problem as well?

--- a/ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c
+++ b/ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c
@@ -16,20 +16,21 @@
typedef __SIZE_TYPE__ size_t;

static __attribute__((__used__))
-void *__memcpy(void *dest, const void *src, size_t n)
+void __memcpy(void *dest, const void *src, size_t n)
{
unsigned char *d = dest;
unsigned char const *s = src;

while (n--)
*d++ = *s++;
+}

+void *memcpy(void *dest, const void *src, size_t n)
+{
+ __memcpy(dest, src, n);
return dest;
}

-__attribute__((__alias__("__memcpy")))
-void *memcpy(void *dest, const void *src, size_t n);
-
#ifdef __arm__

__attribute__((__alias__("__memcpy")))


Re: [PATCH] MdePkg/BaseIoLibIntrinsic: make BaseIoLibIntrinsic safe for ArmVirt/KVM

Laszlo Ersek
 

On 06/07/18 17:09, Ard Biesheuvel wrote:
On 7 June 2018 at 17:08, Gao, Liming <liming.gao@...> wrote:
Ard:
If this library instance is specific to ARM emulator with KVM, how about add it into ArmVirtPkg?
Laszlo, do you want to take this question?
Certainly.

Liming, in commit b6d11d7c4678, we added "BaseIoLibIntrinsicSev.inf" to
"MdePkg/Library/BaseIoLibIntrinsic", with some custom Ia32 and X64 NASM
files, but mostly reusing the large existent files ("IoLibMmioBuffer.c",
"IoHighLevel.c" etc).

That library instance (and the NASM customizations) target AMD SEV; that
is, when the code runs in a virtual machine for which AMD Secure
Encrypted Virtualization is enabled. The NASM code uses CPUID to
dynamically determine whether it runs in a SEV guest, or in a normal
guest, and in the former case, it avoids using the REP prefix in the
IoFifo routines (it uses open-coded LOOPs).

The "BaseIoLibIntrinsicSev.inf" instance is only ever used in OvmfPkg
(it only makes sense in virtual machines), but the consensus back then
was to add the lib instance to MdePkg.


This time, Ard has already implemented (and posted) the IoLib instance
that we need for ARM KVM such that it lands under ArmVirtPkg -- however,
that library instance duplicates a huge amount of code, un-changed, from
"BaseIoLibIntrinsic" (such as "IoLibMmioBuffer.c", "IoHighLevel.c"). I
suggested that Ard instead post the new library instance as a new INF
file (plus some assembly files) for BaseIoLibIntrinsic itself, because:
(a) this way we avoid code duplication, and (b) that's exactly what we
did -- for another virtualization-only use case -- in commit
b6d11d7c4678 earlier.

If you'd like us to add the code to ArmVirtPkg, I don't think that's the
right or consequent action, but technically, we can simply fall back to
the ArmVirtPkg patches that Ard already wrote and posted.

Thanks,
Laszlo

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@...]
Sent: Thursday, June 7, 2018 6:47 PM
To: edk2-devel@...
Cc: lersek@...; leif.lindholm@...; Gao, Liming <liming.gao@...>; Kinney, Michael D
<michael.d.kinney@...>; Ard Biesheuvel <ard.biesheuvel@...>
Subject: [PATCH] MdePkg/BaseIoLibIntrinsic: make BaseIoLibIntrinsic safe for ArmVirt/KVM

KVM on ARM refuses to decode load/store instructions used to perform
I/O to emulated devices, and instead relies on the exception syndrome
information to describe the operand register, access size, etc.
This is only possible for instructions that have a single input/output
register (as opposed to ones that increment the offset register, or
load/store pair instructions, etc). Otherwise, QEMU crashes with the
following error

error: kvm run failed Function not implemented
R00=01010101 R01=00000008 R02=00000048 R03=08000820
R04=00000120 R05=7faaa0e0 R06=7faaa0dc R07=7faaa0e8
R08=7faaa0ec R09=7faaa088 R10=000000ff R11=00000080
R12=ff000000 R13=7fccfe08 R14=7faa835f R15=7faa887c
PSR=800001f3 N--- T svc32
QEMU: Terminated

and KVM produces a warning such as the following in the kernel log

kvm [17646]: load/store instruction decoding not implemented

The IoLib implementation provided by MdePkg/Library/BaseIoLibIntrinsic
is based on C code, and when LTO is in effect, the MMIO accesses could
be merged with, e.g., manipulations of the loop counter, producing
opcodes that KVM does not support for emulated MMIO.

So let's add a special ArmVirt flavor of this library that implements
that actual load/store operations in assembler, ensuring that the
instructions involved can be emulated by KVM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>
---
ArmVirtPkg/ArmVirt.dsc.inc | 2 +-
MdePkg/Library/BaseIoLibIntrinsic/AArch64/ArmVirtMmio.S | 164 +++++
MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.S | 161 +++++
MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm | 165 +++++
MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf | 50 ++
MdePkg/Library/BaseIoLibIntrinsic/IoLibArmVirt.c | 641 ++++++++++++++++++++
6 files changed, 1182 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 766e4f598a07..7464ac70ed1b 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -41,7 +41,7 @@ [LibraryClasses.common]
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
- IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+ IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf

diff --git a/MdePkg/Library/BaseIoLibIntrinsic/AArch64/ArmVirtMmio.S b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/ArmVirtMmio.S
new file mode 100644
index 000000000000..47be68a3e783
--- /dev/null
+++ b/MdePkg/Library/BaseIoLibIntrinsic/AArch64/ArmVirtMmio.S
@@ -0,0 +1,164 @@
+#
+# Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
+#
+# 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.
+#
+#
+
+.text
+.align 3
+
+GCC_ASM_EXPORT(MmioRead8Internal)
+GCC_ASM_EXPORT(MmioWrite8Internal)
+GCC_ASM_EXPORT(MmioRead16Internal)
+GCC_ASM_EXPORT(MmioWrite16Internal)
+GCC_ASM_EXPORT(MmioRead32Internal)
+GCC_ASM_EXPORT(MmioWrite32Internal)
+GCC_ASM_EXPORT(MmioRead64Internal)
+GCC_ASM_EXPORT(MmioWrite64Internal)
+
+//
+// Reads an 8-bit MMIO register.
+//
+// Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
+// returned. This function must guarantee that all MMIO read and write
+// operations are serialized.
+//
+// If 8-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to read.
+//
+// @return The value read.
+//
+ASM_PFX(MmioRead8Internal):
+ ldrb w0, [x0]
+ dmb ld
+ ret
+
+//
+// Writes an 8-bit MMIO register.
+//
+// Writes the 8-bit MMIO register specified by Address with the value specified
+// by Value and returns Value. This function must guarantee that all MMIO read
+// and write operations are serialized.
+//
+// If 8-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to write.
+// @param Value The value to write to the MMIO register.
+//
+ASM_PFX(MmioWrite8Internal):
+ dmb st
+ strb w1, [x0]
+ ret
+
+//
+// Reads a 16-bit MMIO register.
+//
+// Reads the 16-bit MMIO register specified by Address. The 16-bit read value is
+// returned. This function must guarantee that all MMIO read and write
+// operations are serialized.
+//
+// If 16-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to read.
+//
+// @return The value read.
+//
+ASM_PFX(MmioRead16Internal):
+ ldrh w0, [x0]
+ dmb ld
+ ret
+
+//
+// Writes a 16-bit MMIO register.
+//
+// Writes the 16-bit MMIO register specified by Address with the value specified
+// by Value and returns Value. This function must guarantee that all MMIO read
+// and write operations are serialized.
+//
+// If 16-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to write.
+// @param Value The value to write to the MMIO register.
+//
+ASM_PFX(MmioWrite16Internal):
+ dmb st
+ strh w1, [x0]
+ ret
+
+//
+// Reads a 32-bit MMIO register.
+//
+// Reads the 32-bit MMIO register specified by Address. The 32-bit read value is
+// returned. This function must guarantee that all MMIO read and write
+// operations are serialized.
+//
+// If 32-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to read.
+//
+// @return The value read.
+//
+ASM_PFX(MmioRead32Internal):
+ ldr w0, [x0]
+ dmb ld
+ ret
+
+//
+// Writes a 32-bit MMIO register.
+//
+// Writes the 32-bit MMIO register specified by Address with the value specified
+// by Value and returns Value. This function must guarantee that all MMIO read
+// and write operations are serialized.
+//
+// If 32-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to write.
+// @param Value The value to write to the MMIO register.
+//
+ASM_PFX(MmioWrite32Internal):
+ dmb st
+ str w1, [x0]
+ ret
+
+//
+// Reads a 64-bit MMIO register.
+//
+// Reads the 64-bit MMIO register specified by Address. The 64-bit read value is
+// returned. This function must guarantee that all MMIO read and write
+// operations are serialized.
+//
+// If 64-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to read.
+//
+// @return The value read.
+//
+ASM_PFX(MmioRead64Internal):
+ ldr x0, [x0]
+ dmb ld
+ ret
+
+//
+// Writes a 64-bit MMIO register.
+//
+// Writes the 64-bit MMIO register specified by Address with the value specified
+// by Value and returns Value. This function must guarantee that all MMIO read
+// and write operations are serialized.
+//
+// If 64-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to write.
+// @param Value The value to write to the MMIO register.
+//
+ASM_PFX(MmioWrite64Internal):
+ dmb st
+ str x1, [x0]
+ ret
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.S b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.S
new file mode 100644
index 000000000000..438805ca7c24
--- /dev/null
+++ b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.S
@@ -0,0 +1,161 @@
+#
+# Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
+#
+# 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.
+#
+#
+
+GCC_ASM_EXPORT(MmioRead8Internal)
+GCC_ASM_EXPORT(MmioWrite8Internal)
+GCC_ASM_EXPORT(MmioRead16Internal)
+GCC_ASM_EXPORT(MmioWrite16Internal)
+GCC_ASM_EXPORT(MmioRead32Internal)
+GCC_ASM_EXPORT(MmioWrite32Internal)
+GCC_ASM_EXPORT(MmioRead64Internal)
+GCC_ASM_EXPORT(MmioWrite64Internal)
+
+//
+// Reads an 8-bit MMIO register.
+//
+// Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
+// returned. This function must guarantee that all MMIO read and write
+// operations are serialized.
+//
+// If 8-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to read.
+//
+// @return The value read.
+//
+ASM_PFX(MmioRead8Internal):
+ ldrb r0, [r0]
+ dmb
+ bx lr
+
+//
+// Writes an 8-bit MMIO register.
+//
+// Writes the 8-bit MMIO register specified by Address with the value specified
+// by Value and returns Value. This function must guarantee that all MMIO read
+// and write operations are serialized.
+//
+// If 8-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to write.
+// @param Value The value to write to the MMIO register.
+//
+ASM_PFX(MmioWrite8Internal):
+ dmb st
+ strb r1, [r0]
+ bx lr
+
+//
+// Reads a 16-bit MMIO register.
+//
+// Reads the 16-bit MMIO register specified by Address. The 16-bit read value is
+// returned. This function must guarantee that all MMIO read and write
+// operations are serialized.
+//
+// If 16-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to read.
+//
+// @return The value read.
+//
+ASM_PFX(MmioRead16Internal):
+ ldrh r0, [r0]
+ dmb
+ bx lr
+
+//
+// Writes a 16-bit MMIO register.
+//
+// Writes the 16-bit MMIO register specified by Address with the value specified
+// by Value and returns Value. This function must guarantee that all MMIO read
+// and write operations are serialized.
+//
+// If 16-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to write.
+// @param Value The value to write to the MMIO register.
+//
+ASM_PFX(MmioWrite16Internal):
+ dmb st
+ strh r1, [r0]
+ bx lr
+
+//
+// Reads a 32-bit MMIO register.
+//
+// Reads the 32-bit MMIO register specified by Address. The 32-bit read value is
+// returned. This function must guarantee that all MMIO read and write
+// operations are serialized.
+//
+// If 32-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to read.
+//
+// @return The value read.
+//
+ASM_PFX(MmioRead32Internal):
+ ldr r0, [r0]
+ dmb
+ bx lr
+
+//
+// Writes a 32-bit MMIO register.
+//
+// Writes the 32-bit MMIO register specified by Address with the value specified
+// by Value and returns Value. This function must guarantee that all MMIO read
+// and write operations are serialized.
+//
+// If 32-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to write.
+// @param Value The value to write to the MMIO register.
+//
+ASM_PFX(MmioWrite32Internal):
+ dmb st
+ str r1, [r0]
+ bx lr
+
+//
+// Reads a 64-bit MMIO register.
+//
+// Reads the 64-bit MMIO register specified by Address. The 64-bit read value is
+// returned. This function must guarantee that all MMIO read and write
+// operations are serialized.
+//
+// If 64-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to read.
+//
+// @return The value read.
+//
+ASM_PFX(MmioRead64Internal):
+ ldrd r0, r1, [r0]
+ dmb
+ bx lr
+
+//
+// Writes a 64-bit MMIO register.
+//
+// Writes the 64-bit MMIO register specified by Address with the value specified
+// by Value and returns Value. This function must guarantee that all MMIO read
+// and write operations are serialized.
+//
+// If 64-bit MMIO register operations are not supported, then ASSERT().
+//
+// @param Address The MMIO register to write.
+// @param Value The value to write to the MMIO register.
+//
+ASM_PFX(MmioWrite64Internal):
+ dmb st
+ strd r2, r3, [r0]
+ bx lr
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
new file mode 100644
index 000000000000..82add82f800a
--- /dev/null
+++ b/MdePkg/Library/BaseIoLibIntrinsic/Arm/ArmVirtMmio.asm
@@ -0,0 +1,165 @@
+;
+; Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
+;
+; 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.
+;
+
+
+AREA IoLibMmio, CODE, READONLY
+
+EXPORT MmioRead8Internal
+EXPORT MmioWrite8Internal
+EXPORT MmioRead16Internal
+EXPORT MmioWrite16Internal
+EXPORT MmioRead32Internal
+EXPORT MmioWrite32Internal
+EXPORT MmioRead64Internal
+EXPORT MmioWrite64Internal
+
+;
+; Reads an 8-bit MMIO register.
+;
+; Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
+; returned. This function must guarantee that all MMIO read and write
+; operations are serialized.
+;
+; If 8-bit MMIO register operations are not supported, then ASSERT().
+;
+; @param Address The MMIO register to read.
+;
+; @return The value read.
+;
+MmioRead8Internal
+ ldrb r0, [r0]
+ dmb
+ bx lr
+
+;
+; Writes an 8-bit MMIO register.
+;
+; Writes the 8-bit MMIO register specified by Address with the value specified
+; by Value and returns Value. This function must guarantee that all MMIO read
+; and write operations are serialized.
+;
+; If 8-bit MMIO register operations are not supported, then ASSERT().
+;
+; @param Address The MMIO register to write.
+; @param Value The value to write to the MMIO register.
+;
+MmioWrite8Internal
+ dmb st
+ strb r1, [r0]
+ bx lr
+
+;
+; Reads a 16-bit MMIO register.
+;
+; Reads the 16-bit MMIO register specified by Address. The 16-bit read value is
+; returned. This function must guarantee that all MMIO read and write
+; operations are serialized.
+;
+; If 16-bit MMIO register operations are not supported, then ASSERT().
+;
+; @param Address The MMIO register to read.
+;
+; @return The value read.
+;
+MmioRead16Internal
+ ldrh r0, [r0]
+ dmb
+ bx lr
+
+;
+; Writes a 16-bit MMIO register.
+;
+; Writes the 16-bit MMIO register specified by Address with the value specified
+; by Value and returns Value. This function must guarantee that all MMIO read
+; and write operations are serialized.
+;
+; If 16-bit MMIO register operations are not supported, then ASSERT().
+;
+; @param Address The MMIO register to write.
+; @param Value The value to write to the MMIO register.
+;
+MmioWrite16Internal
+ dmb st
+ strh r1, [r0]
+ bx lr
+
+;
+; Reads a 32-bit MMIO register.
+;
+; Reads the 32-bit MMIO register specified by Address. The 32-bit read value is
+; returned. This function must guarantee that all MMIO read and write
+; operations are serialized.
+;
+; If 32-bit MMIO register operations are not supported, then ASSERT().
+;
+; @param Address The MMIO register to read.
+;
+; @return The value read.
+;
+MmioRead32Internal
+ ldr r0, [r0]
+ dmb
+ bx lr
+
+;
+; Writes a 32-bit MMIO register.
+;
+; Writes the 32-bit MMIO register specified by Address with the value specified
+; by Value and returns Value. This function must guarantee that all MMIO read
+; and write operations are serialized.
+;
+; If 32-bit MMIO register operations are not supported, then ASSERT().
+;
+; @param Address The MMIO register to write.
+; @param Value The value to write to the MMIO register.
+;
+MmioWrite32Internal
+ dmb st
+ str r1, [r0]
+ bx lr
+
+;
+; Reads a 64-bit MMIO register.
+;
+; Reads the 64-bit MMIO register specified by Address. The 64-bit read value is
+; returned. This function must guarantee that all MMIO read and write
+; operations are serialized.
+;
+; If 64-bit MMIO register operations are not supported, then ASSERT().
+;
+; @param Address The MMIO register to read.
+;
+; @return The value read.
+;
+MmioRead64Internal
+ ldrd r0, r1, [r0]
+ dmb
+ bx lr
+
+;
+; Writes a 64-bit MMIO register.
+;
+; Writes the 64-bit MMIO register specified by Address with the value specified
+; by Value and returns Value. This function must guarantee that all MMIO read
+; and write operations are serialized.
+;
+; If 64-bit MMIO register operations are not supported, then ASSERT().
+;
+; @param Address The MMIO register to write.
+; @param Value The value to write to the MMIO register.
+;
+MmioWrite64Internal
+ dmb st
+ strd r2, r3, [r0]
+ bx lr
+
+ END
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
new file mode 100644
index 000000000000..5a1dc87ed2bd
--- /dev/null
+++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicArmVirt.inf
@@ -0,0 +1,50 @@
+## @file
+# Instance of I/O Library using KVM/ARM safe assembler routines
+#
+# Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR>
+# Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+# Copyright (c) 2018, 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 which accompanies this
+# distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php.
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = BaseIoLibIntrinsicArmVirt
+ FILE_GUID = 217102b4-b465-4a1d-a2de-93dd385ec480
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = IoLib
+
+#
+# VALID_ARCHITECTURES = ARM AARCH64
+#
+
+[Sources]
+ IoLibMmioBuffer.c
+ BaseIoLibIntrinsicInternal.h
+ IoHighLevel.c
+
+[Sources.ARM]
+ IoLibArmVirt.c
+ Arm/ArmVirtMmio.S | GCC
+ Arm/ArmVirtMmio.asm | RVCT
+
+[Sources.AARCH64]
+ IoLibArmVirt.c
+ AArch64/ArmVirtMmio.S
+
+[Packages]
+ MdePkg/MdePkg.dec
+
+[LibraryClasses]
+ DebugLib
+ BaseLib
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/IoLibArmVirt.c b/MdePkg/Library/BaseIoLibIntrinsic/IoLibArmVirt.c
new file mode 100644
index 000000000000..154d583246ac
--- /dev/null
+++ b/MdePkg/Library/BaseIoLibIntrinsic/IoLibArmVirt.c
@@ -0,0 +1,641 @@
+/** @file
+ I/O Library for ARM.
+
+ Copyright (c) 2006 - 2009, Intel Corporation. All rights reserved.<BR>
+ Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
+ Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+ Copyright (c) 2018, 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 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.
+
+**/
+
+#include "BaseIoLibIntrinsicInternal.h"
+
+UINT8
+EFIAPI
+MmioRead8Internal (
+ IN UINTN Address
+ );
+
+VOID
+EFIAPI
+MmioWrite8Internal (
+ IN UINTN Address,
+ IN UINT8 Value
+ );
+
+UINT16
+EFIAPI
+MmioRead16Internal (
+ IN UINTN Address
+ );
+
+VOID
+EFIAPI
+MmioWrite16Internal (
+ IN UINTN Address,
+ IN UINT16 Value
+ );
+
+UINT32
+EFIAPI
+MmioRead32Internal (
+ IN UINTN Address
+ );
+
+VOID
+EFIAPI
+MmioWrite32Internal (
+ IN UINTN Address,
+ IN UINT32 Value
+ );
+
+UINT64
+EFIAPI
+MmioRead64Internal (
+ IN UINTN Address
+ );
+
+VOID
+EFIAPI
+MmioWrite64Internal (
+ IN UINTN Address,
+ IN UINT64 Value
+ );
+
+/**
+ Reads an 8-bit I/O port.
+
+ Reads the 8-bit I/O port specified by Port. The 8-bit read value is returned.
+ This function must guarantee that all I/O read and write operations are
+ serialized.
+
+ If 8-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to read.
+
+ @return The value read.
+
+**/
+UINT8
+EFIAPI
+IoRead8 (
+ IN UINTN Port
+ )
+{
+ ASSERT (FALSE);
+ return 0;
+}
+
+/**
+ Writes an 8-bit I/O port.
+
+ Writes the 8-bit I/O port specified by Port with the value specified by Value
+ and returns Value. This function must guarantee that all I/O read and write
+ operations are serialized.
+
+ If 8-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to write.
+ @param Value The value to write to the I/O port.
+
+ @return The value written the I/O port.
+
+**/
+UINT8
+EFIAPI
+IoWrite8 (
+ IN UINTN Port,
+ IN UINT8 Value
+ )
+{
+ ASSERT (FALSE);
+ return Value;
+}
+
+/**
+ Reads a 16-bit I/O port.
+
+ Reads the 16-bit I/O port specified by Port. The 16-bit read value is returned.
+ This function must guarantee that all I/O read and write operations are
+ serialized.
+
+ If 16-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to read.
+
+ @return The value read.
+
+**/
+UINT16
+EFIAPI
+IoRead16 (
+ IN UINTN Port
+ )
+{
+ ASSERT (FALSE);
+ return 0;
+}
+
+/**
+ Writes a 16-bit I/O port.
+
+ Writes the 16-bit I/O port specified by Port with the value specified by Value
+ and returns Value. This function must guarantee that all I/O read and write
+ operations are serialized.
+
+ If 16-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to write.
+ @param Value The value to write to the I/O port.
+
+ @return The value written the I/O port.
+
+**/
+UINT16
+EFIAPI
+IoWrite16 (
+ IN UINTN Port,
+ IN UINT16 Value
+ )
+{
+ ASSERT (FALSE);
+ return Value;
+}
+
+/**
+ Reads a 32-bit I/O port.
+
+ Reads the 32-bit I/O port specified by Port. The 32-bit read value is returned.
+ This function must guarantee that all I/O read and write operations are
+ serialized.
+
+ If 32-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to read.
+
+ @return The value read.
+
+**/
+UINT32
+EFIAPI
+IoRead32 (
+ IN UINTN Port
+ )
+{
+ ASSERT (FALSE);
+ return 0;
+}
+
+/**
+ Writes a 32-bit I/O port.
+
+ Writes the 32-bit I/O port specified by Port with the value specified by Value
+ and returns Value. This function must guarantee that all I/O read and write
+ operations are serialized.
+
+ If 32-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to write.
+ @param Value The value to write to the I/O port.
+
+ @return The value written the I/O port.
+
+**/
+UINT32
+EFIAPI
+IoWrite32 (
+ IN UINTN Port,
+ IN UINT32 Value
+ )
+{
+ ASSERT (FALSE);
+ return Value;
+}
+
+/**
+ Reads a 64-bit I/O port.
+
+ Reads the 64-bit I/O port specified by Port. The 64-bit read value is returned.
+ This function must guarantee that all I/O read and write operations are
+ serialized.
+
+ If 64-bit I/O port operations are not supported, then ASSERT().
+ If Port is not aligned on a 64-bit boundary, then ASSERT().
+
+ @param Port The I/O port to read.
+
+ @return The value read.
+
+**/
+UINT64
+EFIAPI
+IoRead64 (
+ IN UINTN Port
+ )
+{
+ ASSERT (FALSE);
+ return 0;
+}
+
+/**
+ Writes a 64-bit I/O port.
+
+ Writes the 64-bit I/O port specified by Port with the value specified by Value
+ and returns Value. This function must guarantee that all I/O read and write
+ operations are serialized.
+
+ If 64-bit I/O port operations are not supported, then ASSERT().
+ If Port is not aligned on a 64-bit boundary, then ASSERT().
+
+ @param Port The I/O port to write.
+ @param Value The value to write to the I/O port.
+
+ @return The value written to the I/O port.
+
+**/
+UINT64
+EFIAPI
+IoWrite64 (
+ IN UINTN Port,
+ IN UINT64 Value
+ )
+{
+ ASSERT (FALSE);
+ return 0;
+}
+
+/**
+ Reads an 8-bit I/O port fifo into a block of memory.
+
+ Reads the 8-bit I/O fifo port specified by Port.
+ The port is read Count times, and the read data is
+ stored in the provided Buffer.
+
+ This function must guarantee that all I/O read and write operations are
+ serialized.
+
+ If 8-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to read.
+ @param Count The number of times to read I/O port.
+ @param Buffer The buffer to store the read data into.
+
+**/
+VOID
+EFIAPI
+IoReadFifo8 (
+ IN UINTN Port,
+ IN UINTN Count,
+ OUT VOID *Buffer
+ )
+{
+ ASSERT (FALSE);
+}
+
+/**
+ Writes a block of memory into an 8-bit I/O port fifo.
+
+ Writes the 8-bit I/O fifo port specified by Port.
+ The port is written Count times, and the write data is
+ retrieved from the provided Buffer.
+
+ This function must guarantee that all I/O write and write operations are
+ serialized.
+
+ If 8-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to write.
+ @param Count The number of times to write I/O port.
+ @param Buffer The buffer to retrieve the write data from.
+
+**/
+VOID
+EFIAPI
+IoWriteFifo8 (
+ IN UINTN Port,
+ IN UINTN Count,
+ IN VOID *Buffer
+ )
+{
+ ASSERT (FALSE);
+}
+
+/**
+ Reads a 16-bit I/O port fifo into a block of memory.
+
+ Reads the 16-bit I/O fifo port specified by Port.
+ The port is read Count times, and the read data is
+ stored in the provided Buffer.
+
+ This function must guarantee that all I/O read and write operations are
+ serialized.
+
+ If 16-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to read.
+ @param Count The number of times to read I/O port.
+ @param Buffer The buffer to store the read data into.
+
+**/
+VOID
+EFIAPI
+IoReadFifo16 (
+ IN UINTN Port,
+ IN UINTN Count,
+ OUT VOID *Buffer
+ )
+{
+ ASSERT (FALSE);
+}
+
+/**
+ Writes a block of memory into a 16-bit I/O port fifo.
+
+ Writes the 16-bit I/O fifo port specified by Port.
+ The port is written Count times, and the write data is
+ retrieved from the provided Buffer.
+
+ This function must guarantee that all I/O write and write operations are
+ serialized.
+
+ If 16-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to write.
+ @param Count The number of times to write I/O port.
+ @param Buffer The buffer to retrieve the write data from.
+
+**/
+VOID
+EFIAPI
+IoWriteFifo16 (
+ IN UINTN Port,
+ IN UINTN Count,
+ IN VOID *Buffer
+ )
+{
+ ASSERT (FALSE);
+}
+
+/**
+ Reads a 32-bit I/O port fifo into a block of memory.
+
+ Reads the 32-bit I/O fifo port specified by Port.
+ The port is read Count times, and the read data is
+ stored in the provided Buffer.
+
+ This function must guarantee that all I/O read and write operations are
+ serialized.
+
+ If 32-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to read.
+ @param Count The number of times to read I/O port.
+ @param Buffer The buffer to store the read data into.
+
+**/
+VOID
+EFIAPI
+IoReadFifo32 (
+ IN UINTN Port,
+ IN UINTN Count,
+ OUT VOID *Buffer
+ )
+{
+ ASSERT (FALSE);
+}
+
+/**
+ Writes a block of memory into a 32-bit I/O port fifo.
+
+ Writes the 32-bit I/O fifo port specified by Port.
+ The port is written Count times, and the write data is
+ retrieved from the provided Buffer.
+
+ This function must guarantee that all I/O write and write operations are
+ serialized.
+
+ If 32-bit I/O port operations are not supported, then ASSERT().
+
+ @param Port The I/O port to write.
+ @param Count The number of times to write I/O port.
+ @param Buffer The buffer to retrieve the write data from.
+
+**/
+VOID
+EFIAPI
+IoWriteFifo32 (
+ IN UINTN Port,
+ IN UINTN Count,
+ IN VOID *Buffer
+ )
+{
+ ASSERT (FALSE);
+}
+
+/**
+ Reads an 8-bit MMIO register.
+
+ Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
+ returned. This function must guarantee that all MMIO read and write
+ operations are serialized.
+
+ If 8-bit MMIO register operations are not supported, then ASSERT().
+
+ @param Address The MMIO register to read.
+
+ @return The value read.
+
+**/
+UINT8
+EFIAPI
+MmioRead8 (
+ IN UINTN Address
+ )
+{
+ return MmioRead8Internal (Address);
+}
+
+/**
+ Writes an 8-bit MMIO register.
+
+ Writes the 8-bit MMIO register specified by Address with the value specified
+ by Value and returns Value. This function must guarantee that all MMIO read
+ and write operations are serialized.
+
+ If 8-bit MMIO register operations are not supported, then ASSERT().
+
+ @param Address The MMIO register to write.
+ @param Value The value to write to the MMIO register.
+
+**/
+UINT8
+EFIAPI
+MmioWrite8 (
+ IN UINTN Address,
+ IN UINT8 Value
+ )
+{
+ MmioWrite8Internal (Address, Value);
+ return Value;
+}
+
+/**
+ Reads a 16-bit MMIO register.
+
+ Reads the 16-bit MMIO register specified by Address. The 16-bit read value is
+ returned. This function must guarantee that all MMIO read and write
+ operations are serialized.
+
+ If 16-bit MMIO register operations are not supported, then ASSERT().
+
+ @param Address The MMIO register to read.
+
+ @return The value read.
+
+**/
+UINT16
+EFIAPI
+MmioRead16 (
+ IN UINTN Address
+ )
+{
+ ASSERT ((Address & 1) == 0);
+
+ return MmioRead16Internal (Address);
+}
+
+/**
+ Writes a 16-bit MMIO register.
+
+ Writes the 16-bit MMIO register specified by Address with the value specified
+ by Value and returns Value. This function must guarantee that all MMIO read
+ and write operations are serialized.
+
+ If 16-bit MMIO register operations are not supported, then ASSERT().
+
+ @param Address The MMIO register to write.
+ @param Value The value to write to the MMIO register.
+
+**/
+UINT16
+EFIAPI
+MmioWrite16 (
+ IN UINTN Address,
+ IN UINT16 Value
+ )
+{
+ ASSERT ((Address & 1) == 0);
+
+ MmioWrite16Internal (Address, Value);
+ return Value;
+}
+
+/**
+ Reads a 32-bit MMIO register.
+
+ Reads the 32-bit MMIO register specified by Address. The 32-bit read value is
+ returned. This function must guarantee that all MMIO read and write
+ operations are serialized.
+
+ If 32-bit MMIO register operations are not supported, then ASSERT().
+
+ @param Address The MMIO register to read.
+
+ @return The value read.
+
+**/
+UINT32
+EFIAPI
+MmioRead32 (
+ IN UINTN Address
+ )
+{
+ ASSERT ((Address & 3) == 0);
+
+ return MmioRead32Internal (Address);
+}
+
+/**
+ Writes a 32-bit MMIO register.
+
+ Writes the 32-bit MMIO register specified by Address with the value specified
+ by Value and returns Value. This function must guarantee that all MMIO read
+ and write operations are serialized.
+
+ If 32-bit MMIO register operations are not supported, then ASSERT().
+
+ @param Address The MMIO register to write.
+ @param Value The value to write to the MMIO register.
+
+**/
+UINT32
+EFIAPI
+MmioWrite32 (
+ IN UINTN Address,
+ IN UINT32 Value
+ )
+{
+ ASSERT ((Address & 3) == 0);
+
+ MmioWrite32Internal (Address, Value);
+ return Value;
+}
+
+/**
+ Reads a 64-bit MMIO register.
+
+ Reads the 64-bit MMIO register specified by Address. The 64-bit read value is
+ returned. This function must guarantee that all MMIO read and write
+ operations are serialized.
+
+ If 64-bit MMIO register operations are not supported, then ASSERT().
+
+ @param Address The MMIO register to read.
+
+ @return The value read.
+
+**/
+UINT64
+EFIAPI
+MmioRead64 (
+ IN UINTN Address
+ )
+{
+ ASSERT ((Address & 7) == 0);
+
+ return MmioRead64Internal (Address);
+}
+
+/**
+ Writes a 64-bit MMIO register.
+
+ Writes the 64-bit MMIO register specified by Address with the value specified
+ by Value and returns Value. This function must guarantee that all MMIO read
+ and write operations are serialized.
+
+ If 64-bit MMIO register operations are not supported, then ASSERT().
+
+ @param Address The MMIO register to write.
+ @param Value The value to write to the MMIO register.
+
+**/
+UINT64
+EFIAPI
+MmioWrite64 (
+ IN UINTN Address,
+ IN UINT64 Value
+ )
+{
+ ASSERT ((Address & 7) == 0);
+
+ MmioWrite64Internal (Address, Value);
+ return Value;
+}
--
2.17.0


Re: [PATCH] MdePkg/BaseIoLibIntrinsic: make BaseIoLibIntrinsic safe for ArmVirt/KVM

Leif Lindholm <leif.lindholm@...>
 

This addresses all of the items I mentioned in my review of the
previous solution to this problem, so for me:

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

On Thu, Jun 07, 2018 at 12:46:45PM +0200, Ard Biesheuvel wrote:
KVM on ARM refuses to decode load/store instructions used to perform
I/O to emulated devices, and instead relies on the exception syndrome
information to describe the operand register, access size, etc.
This is only possible for instructions that have a single input/output
register (as opposed to ones that increment the offset register, or
load/store pair instructions, etc). Otherwise, QEMU crashes with the
following error

error: kvm run failed Function not implemented
R00=01010101 R01=00000008 R02=00000048 R03=08000820
R04=00000120 R05=7faaa0e0 R06=7faaa0dc R07=7faaa0e8
R08=7faaa0ec R09=7faaa088 R10=000000ff R11=00000080
R12=ff000000 R13=7fccfe08 R14=7faa835f R15=7faa887c
PSR=800001f3 N--- T svc32
QEMU: Terminated

and KVM produces a warning such as the following in the kernel log

kvm [17646]: load/store instruction decoding not implemented

The IoLib implementation provided by MdePkg/Library/BaseIoLibIntrinsic
is based on C code, and when LTO is in effect, the MMIO accesses could
be merged with, e.g., manipulations of the loop counter, producing
opcodes that KVM does not support for emulated MMIO.

So let's add a special ArmVirt flavor of this library that implements
that actual load/store operations in assembler, ensuring that the
instructions involved can be emulated by KVM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@...>


Re: [PATCH edk2-platforms 0/2] DeveloperBox: prepare for expanding the capsule payload

Ard Biesheuvel
 

On 7 June 2018 at 17:08, Ard Biesheuvel <ard.biesheuvel@...> wrote:
We intend to start distributing firmware update capsules that include the SCP
firmware partition. In order to allow for more flexibility regarding whether
a capsule contains this piece or not, update the flash access routines and
the flash partition descriptions so we can update any part of the first
4 MB, consisting of the CM3 bootstrap code, emergency flasher, pseudo
EEPROM, SCP firmware, ARM-TF and UEFI.

Note that this means we can drop the 'TEMPORARY' patch I proposed a while
ago to hack around this limitation.
Additional note: this approach allows you to downgrade to older
capsules as well, although
a) you will keep the newer versions of the pieces that the older
capsule does not cover
b) upgrading to the latest version will require you to go via an
intermediate version that includes these changes.

Ard Biesheuvel (2):
Silicon/SynQuacerPlatformFlashAccessLib: relax FV address check
Silicon/NorFlashSynQuacerLib: describe entire firmware region as FV

.../NorFlashSynQuacerLib/NorFlashSynQuacer.c | 5 +-
.../SynQuacerPlatformFlashAccessLib.c | 53 +++++++++----------
2 files changed, 28 insertions(+), 30 deletions(-)

--
2.17.0