Date   

[PATCH 1/2] EmulatorPkg/PeiEmuSerialPortLib: Update the INF file Guid

Zhang, Shenglei
 

From: shenglei <shenglei.zhang@...>

FILE GUID in PeiEmuSerialPortLib.inf is same to
MdePkg\Library\BaseSerialPortLibNull\BaseSerialPortLibNull.inf.
PeiEmuSerialPortLib.inf FILE_GUID should be updated.
https://bugzilla.tianocore.org/show_bug.cgi?id=2144

Cc: Jordan Justen <jordan.l.justen@...>
Cc: Andrew Fish <afish@...>
Cc: Ray Ni <ray.ni@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
EmulatorPkg/Library/PeiEmuSerialPortLib/PeiEmuSerialPortLib.inf | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmulatorPkg/Library/PeiEmuSerialPortLib/PeiEmuSerialPortLib.inf b/EmulatorPkg/Library/PeiEmuSerialPortLib/PeiEmuSerialPortLib.inf
index f61346174739..2f5e656fd753 100644
--- a/EmulatorPkg/Library/PeiEmuSerialPortLib/PeiEmuSerialPortLib.inf
+++ b/EmulatorPkg/Library/PeiEmuSerialPortLib/PeiEmuSerialPortLib.inf
@@ -12,7 +12,7 @@
[Defines]
INF_VERSION = 0x00010005
BASE_NAME = PeiEmuSerialPortLibNull
- FILE_GUID = E4541241-8897-411a-91F8-7D7E45837146
+ FILE_GUID = 76003416-0373-4C3C-BC4C-87E367FD4BD1
MODULE_TYPE = PEIM
VERSION_STRING = 1.0
LIBRARY_CLASS = SerialPortLib| PEI_CORE PEIM SEC
--
2.18.0.windows.1


[PATCH 0/2] Update file guids in INF files

Zhang, Shenglei
 

The file guids of PeiEmuSerialPortLib.inf and SerialDxe.inf
are same to other guids. So update them to new ones.
https://bugzilla.tianocore.org/show_bug.cgi?id=2144

Cc: Jordan Justen <jordan.l.justen@...>
Cc: Andrew Fish <afish@...>
Cc: Ray Ni <ray.ni@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Shenglei Zhang (1):
MdeModulePkg/SerialDxe: Update the file Guid in SerialDxe.inf

shenglei (1):
EmulatorPkg/PeiEmuSerialPortLib: Update the INF file Guid

EmulatorPkg/Library/PeiEmuSerialPortLib/PeiEmuSerialPortLib.inf | 2 +-
MdeModulePkg/Universal/SerialDxe/SerialDxe.inf | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

--
2.18.0.windows.1


Re: [PATCH] UefiCpuPkg/CpuExceptionHandlerLib: Fix #AC split lock #ac

Dong, Eric
 

Hi John,

I'm not sure whether I understand the code correctly. If not, please correct me.

1. You change to the code to only exchange 32 bits(eax) instead of 64 bits(rax). After your change, how to handle the above 32 bits value (from bit 32 to bit 63)?
2. In this file, also have another two xchg codes, do them need to be updated also?

Thanks,
Eric

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of John
E Lofgren
Sent: Wednesday, September 4, 2019 2:15 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH] UefiCpuPkg/CpuExceptionHandlerLib: Fix #AC
split lock

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2150

Fix #AC split lock's caused by seperating base and limit from sgdt and sidt by
changing xchg operands to 32-bit to stop from crossing cacheline.

Signed-off-by: John E Lofgren <john.e.lofgren@...>
---
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
| 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
m
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
m
index 4db1a09f28..6d83dca4b9 100644
---
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
m
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.
+++ nasm
@@ -184,17 +184,17 @@ HasErrorCode:
push rax
push rax
sidt [rsp]
- xchg rax, [rsp + 2]
- xchg rax, [rsp]
- xchg rax, [rsp + 8]
+ xchg eax, [rsp + 2]
+ xchg eax, [rsp]
+ xchg eax, [rsp + 8]

xor rax, rax
push rax
push rax
sgdt [rsp]
- xchg rax, [rsp + 2]
- xchg rax, [rsp]
- xchg rax, [rsp + 8]
+ xchg eax, [rsp + 2]
+ xchg eax, [rsp]
+ xchg eax, [rsp + 8]

;; UINT64 Ldtr, Tr;
xor rax, rax
--
2.16.2.windows.1



Re: [PATCH v2] MinPlatformPkg/SecFspWrapperPlatformSecLib: Change TopOfTemporaryRam type

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@...>

-----Original Message-----
From: Zhang, Shenglei
Sent: Thursday, September 5, 2019 2:20 PM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@...>; Chiu, Chasel
<chasel.chiu@...>; Desimone, Nathaniel L
<@natedesimone>; Gao, Liming <liming.gao@...>
Subject: [PATCH v2] MinPlatformPkg/SecFspWrapperPlatformSecLib: Change
TopOfTemporaryRam type

Update the type of TopOfTemporaryRam from UINT32 to UINTN.
This change is intended to support X64 build scenarios.
The original code(line 64) may cast the overfloewed result produced by
"(TopOfTemporaryRam - sizeof (UINT32))"from 32bit to 64bit.

Cc: Michael Kubacki <michael.a.kubacki@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <@natedesimone>
Cc: Liming Gao <liming.gao@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
v2: As TopOfTemporaryRam is defined as UINTN in v2, remove
all the related casting operations which exist in v1 patch.

.../SecFspWrapperPlatformSecLib/SecGetPerformance.c | 8 ++++----
.../SecFspWrapperPlatformSecLib/SecPlatformInformation.c | 6 +++---
2 files changed, 7 insertions(+), 7 deletions(-)

diff --git
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor
mSecLib/SecGetPerformance.c
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor
mSecLib/SecGetPerformance.c
index c4eeb2b1..8535ae04 100644
---
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor
mSecLib/SecGetPerformance.c
+++
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/SecGetPerformance.c
@@ -41,7 +41,7 @@ SecGetPerformance (
{
UINT32 Size;
UINT32 Count;
- UINT32 TopOfTemporaryRam;
+ UINTN TopOfTemporaryRam;
UINT64 Ticker;
VOID *TopOfTemporaryRamPpi;
EFI_STATUS Status;
@@ -77,12 +77,12 @@ SecGetPerformance (
// | TSC[31:00] |
// |--------------|
//
- TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi -
sizeof(UINT32);
+ TopOfTemporaryRam = (UINTN)TopOfTemporaryRamPpi - sizeof(UINT32);
TopOfTemporaryRam -= sizeof(UINT32) * 2;
- Count = *(UINT32 *) (UINTN) (TopOfTemporaryRam - sizeof
(UINT32));
+ Count = *(UINT32 *)(TopOfTemporaryRam - sizeof
(UINT32));
Size = Count * sizeof (UINT32);

- Ticker = *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32) - Size
- sizeof (UINT32) * 2);
+ Ticker = *(UINT64 *) (TopOfTemporaryRam - sizeof (UINT32) - Size -
+ sizeof (UINT32) * 2);
Performance->ResetEnd = GetTimeInNanoSecond (Ticker);

return EFI_SUCCESS;
diff --git
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor
mSecLib/SecPlatformInformation.c
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor
mSecLib/SecPlatformInformation.c
index 5b94ed2b..ade36ab5 100644
---
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfor
mSecLib/SecPlatformInformation.c
+++
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/SecPlatformInformation.c
@@ -36,7 +36,7 @@ SecPlatformInformation (
UINT32 *Bist;
UINT32 Size;
UINT32 Count;
- UINT32 TopOfTemporaryRam;
+ UINTN TopOfTemporaryRam;
VOID *TopOfTemporaryRamPpi;
EFI_STATUS Status;

@@ -59,9 +59,9 @@ SecPlatformInformation (
// This routine copies the BIST information to the buffer pointed by
// PlatformInformationRecord for output.
//
- TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof
(UINT32);
+ TopOfTemporaryRam = (UINTN)TopOfTemporaryRamPpi - sizeof (UINT32);
TopOfTemporaryRam -= sizeof(UINT32) * 2;
- Count = *((UINT32 *)(UINTN) (TopOfTemporaryRam - sizeof
(UINT32)));
+ Count = *((UINT32 *)(TopOfTemporaryRam - sizeof
(UINT32)));
Size = Count * sizeof (IA32_HANDOFF_STATUS);

if ((*StructureSize) < (UINT64) Size) {
--
2.18.0.windows.1


[PATCH 4/4] Platform: Intel: Remove hardcoded Stratix 10 UART clock

Loh, Tien Hock
 

From: "Tien Hock, Loh" <tien.hock.loh@...>

Added clock manager so that Stratix 10 UART clock doesn't need to be
hardcoded

Signed-off-by: "Tien Hock, Loh" <tien.hock.loh@...>
Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Leif Lindholm <leif.lindholm@...>
Cc: Michael D Kinney <michael.d.kinney@...>
---
Platform/Intel/Stratix10/Stratix10SoCPkg.dec | 3 +-
Platform/Intel/Stratix10/Stratix10SoCPkg.dsc | 11 +-
Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.inf | 41 ++++++
Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.inf | 34 +++++
Platform/Intel/Stratix10/Include/Library/S10ClockManager/S10ClockManager.h | 48 +++++++
Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.c | 43 +++++++
Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.c | 133 ++++++++++++++++++++
7 files changed, 310 insertions(+), 3 deletions(-)

diff --git a/Platform/Intel/Stratix10/Stratix10SoCPkg.dec b/Platform/Intel/Stratix10/Stratix10SoCPkg.dec
index 7c44670d591d..346f7f9a042b 100755
--- a/Platform/Intel/Stratix10/Stratix10SoCPkg.dec
+++ b/Platform/Intel/Stratix10/Stratix10SoCPkg.dec
@@ -10,7 +10,8 @@ [Defines]
PACKAGE_GUID = 45533DD0-C41F-4ab6-A5DF-65B52684AC60
PACKAGE_VERSION = 0.1

-[Includes.common]
+[Includes]
+ Include

[Guids.common]
gStratix10SocFpgaTokenSpaceGuid = { 0x0fe272eb, 0xb2cf, 0x4390, { 0xa5, 0xc4, 0x60, 0x13, 0x2c, 0x6b, 0xd0, 0x34 } }
diff --git a/Platform/Intel/Stratix10/Stratix10SoCPkg.dsc b/Platform/Intel/Stratix10/Stratix10SoCPkg.dsc
index 643ce625c563..d3ad0eba7e75 100755
--- a/Platform/Intel/Stratix10/Stratix10SoCPkg.dsc
+++ b/Platform/Intel/Stratix10/Stratix10SoCPkg.dsc
@@ -27,6 +27,8 @@ [Defines]
# Pcd Section - list of all EDK II PCD Entries defined by this Platform
#
################################################################################
+[PcdsPatchableInModule.common]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|184320

[PcdsFixedAtBuild.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE
@@ -226,7 +228,8 @@ [LibraryClasses.common]
SemihostLib|ArmPkg/Library/SemihostLib/SemihostLib.inf

SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
- PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
+ PlatformHookLib|Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.inf
+ S10ClockManager|Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.inf

SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -483,9 +486,13 @@ [Components.common]
}

#
- # Platform Specific Init for DXE phase
+ # Platform Specific Init
#
Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
+ Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.inf {
+ <LibraryClasses>
+ S10ClockManager|Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.inf
+ }

[BuildOptions]
#-------------------------------
diff --git a/Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.inf b/Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.inf
new file mode 100644
index 000000000000..dc18db7c5444
--- /dev/null
+++ b/Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.inf
@@ -0,0 +1,41 @@
+## @file
+# Platform Hook Library instance for UART device.
+#
+# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = PlatformHookLib
+ FILE_GUID = 90A73C58-A6E3-4EED-A1A3-6F9C7C3D998F
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = PlatformHookLib
+ CONSTRUCTOR = PlatformHookSerialPortInitialize
+
+[Sources]
+ PlatformHookLib.c
+
+[LibraryClasses]
+ PcdLib
+ PciLib
+ S10ClockManager
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ Platform/Intel/Stratix10/Stratix10SoCPkg.dec
+ UefiPayloadPkg/UefiPayloadPkg.dec
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio ## PRODUCES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase ## PRODUCES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialBaudRate ## PRODUCES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride ## PRODUCES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate ## PRODUCES
+ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## PRODUCES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPciSerialParameters ## PRODUCES
+
diff --git a/Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.inf b/Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.inf
new file mode 100644
index 000000000000..c0eccd304810
--- /dev/null
+++ b/Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.inf
@@ -0,0 +1,34 @@
+## @file
+# Clock Manager Library instance
+#
+# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = S10ClockManagerLib
+ FILE_GUID = 90A73C58-A6E3-4EED-A1A3-6F9C7C3E998F
+ MODULE_TYPE = BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = S10ClockManagerLib
+
+[Packages]
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ ArmPkg/ArmPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ Platform/Intel/Stratix10/Stratix10SoCPkg.dec
+
+[LibraryClasses]
+ ArmLib
+ ArmMmuLib
+ DebugLib
+ IoLib
+ PcdLib
+
+[Sources]
+ S10ClockManager.c
+
diff --git a/Platform/Intel/Stratix10/Include/Library/S10ClockManager/S10ClockManager.h b/Platform/Intel/Stratix10/Include/Library/S10ClockManager/S10ClockManager.h
new file mode 100644
index 000000000000..f081a70a7a11
--- /dev/null
+++ b/Platform/Intel/Stratix10/Include/Library/S10ClockManager/S10ClockManager.h
@@ -0,0 +1,48 @@
+/** @file
+Stratix 10 Clock Manager header
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _S10_CLOCK_MANAGER_
+#define _S10_CLOCK_MANAGER_
+#define CLOCK_MANAGER_MAINPLL 0xffd10030
+#define CLOCK_MANAGER_MAINPLL_NOCCLK 0x4c
+
+#define CLOCK_MANAGER_PERPLL 0xffd100a4
+#define CLOCK_MANAGER_MAINPLL_PLLGLOB 0x44
+#define CLOCK_MANAGER_PERPLL_PLLGLOB 0x40
+#define CLOCK_MANAGER_MAINPLL_PLLGLOB_PSRC(x) (((x) >> 16) & 3)
+#define CLOCK_MANAGER_CNTR6CLK 0x4c
+#define CLOCK_MANAGER_PLLGLOB_PSRC_EOSC1 0
+#define CLOCK_MANAGER_PLLGLOB_PSRC_INTOSC 1
+#define CLOCK_MANAGER_PLLGLOB_PSRC_F2S 2
+#define CLOCK_MANAGER_SRC 16
+#define CLOCK_MANAGER_SRC_MSK 0x7
+#define CLOCK_MANAGER_SRC_MAIN (0)
+#define CLOCK_MANAGER_SRC_PERI (1)
+#define CLOCK_MANAGER_SRC_OSC1 (2)
+#define CLOCK_MANAGER_SRC_INTOSC (3)
+#define CLOCK_MANAGER_SRC_FPGA (4)
+#define CLOCK_MANAGER_FDBCK 0x44
+#define CLOCK_MANAGER_FDBCK_MDIV(x) ((x) >> 24 & 0xff)
+#define CLOCK_MANAGER_PERPLL_FDBCK 0x48
+#define CLOCK_MANAGER_CNT_MSK 0x3ff
+#define CLOCK_MANAGER_PERPLL_CNTR6CLK 0x28
+#define CLOCK_MANAGER_PLLGLOB_REFCLKDIV(x) (((x) >> 8) & 0x3f)
+#define CLOCK_MANAGER_PLLC1_DIV(x) ((x) & 0x7f)
+#define CLOCK_MANAGER_PLLC1 0x54
+#define CLOCK_MANAGER_NOCDIV_L4SPCLK(x) ((x) >> 16 & 0x3)
+#define CLOCK_MANAGER_MAINPLL_NOCDIV 0x40
+
+#define S10_SYSTEM_MANAGER 0xffd12000
+#define S10_SYSTEM_MANAGER_BOOTSCRATCH_COLD1 0x204
+#define S10_SYSTEM_MANAGER_BOOTSCRATCH_COLD2 0x208
+#define S10_CLOCK_INTOSC 460000000
+
+UINT32 S10ClockManagerGetMmcClock();
+UINT32 S10ClockManagerGetUartClock();
+
+#endif
diff --git a/Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.c b/Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.c
new file mode 100644
index 000000000000..a204718909df
--- /dev/null
+++ b/Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.c
@@ -0,0 +1,43 @@
+/** @file
+ Platform Hook Library
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Uefi/UefiBaseType.h>
+#include <Library/PlatformHookLib.h>
+#include <Library/BaseLib.h>
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+#include <Library/S10ClockManager/S10ClockManager.h>
+
+/**
+ Performs platform specific initialization required for the CPU to access
+ the hardware associated with a SerialPortLib instance. This function does
+ not initialize the serial port hardware itself. Instead, it initializes
+ hardware devices that are required for the CPU to access the serial port
+ hardware. This function may be called more than once.
+
+ @retval RETURN_SUCCESS The platform specific initialization succeeded.
+ @retval RETURN_DEVICE_ERROR The platform specific initialization could not be completed.
+
+**/
+RETURN_STATUS
+EFIAPI
+PlatformHookSerialPortInitialize (
+ VOID
+ )
+{
+ RETURN_STATUS Status;
+
+ Status = PcdSet32S (PcdSerialClockRate, S10ClockManagerGetUartClock());
+
+ if (RETURN_ERROR (Status)) {
+ return Status;
+ }
+
+ return RETURN_SUCCESS;
+}
diff --git a/Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.c b/Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.c
new file mode 100644
index 000000000000..5e6fe283646d
--- /dev/null
+++ b/Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.c
@@ -0,0 +1,133 @@
+/** @file
+Stratix 10 Clock Manager
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/IoLib.h>
+#include <Library/S10ClockManager/S10ClockManager.h>
+
+UINT32
+S10ClockManagerGetPerClock() {
+ UINT32 PllGlob, MDiv, RefClkDiv, RefClk;
+
+ PllGlob = MmioRead32(CLOCK_MANAGER_PERPLL + CLOCK_MANAGER_PERPLL_PLLGLOB);
+
+ switch (CLOCK_MANAGER_MAINPLL_PLLGLOB_PSRC(PllGlob)) {
+ case CLOCK_MANAGER_PLLGLOB_PSRC_EOSC1:
+ RefClk = MmioRead32(S10_SYSTEM_MANAGER + S10_SYSTEM_MANAGER_BOOTSCRATCH_COLD1);
+ RefClk = 2500000;
+ break;
+ case CLOCK_MANAGER_PLLGLOB_PSRC_INTOSC:
+ RefClk = S10_CLOCK_INTOSC;
+ break;
+ case CLOCK_MANAGER_PLLGLOB_PSRC_F2S:
+ RefClk = MmioRead32(S10_SYSTEM_MANAGER + S10_SYSTEM_MANAGER_BOOTSCRATCH_COLD2);
+ RefClk = 5000000;
+ break;
+ }
+
+ RefClkDiv = CLOCK_MANAGER_PLLGLOB_REFCLKDIV(PllGlob);
+
+ MDiv = MmioRead32(CLOCK_MANAGER_MAINPLL + CLOCK_MANAGER_FDBCK);
+
+ return (RefClk / RefClkDiv) * (6 + MDiv);
+}
+
+UINT32
+S10ClockManagerGetMainClock() {
+ UINT32 PllGlob, MDiv, RefClkDiv, RefClk;
+
+ PllGlob = MmioRead32(CLOCK_MANAGER_MAINPLL + CLOCK_MANAGER_MAINPLL_PLLGLOB);
+
+ switch (CLOCK_MANAGER_MAINPLL_PLLGLOB_PSRC(PllGlob)) {
+ case CLOCK_MANAGER_PLLGLOB_PSRC_EOSC1:
+ RefClk = MmioRead32(S10_SYSTEM_MANAGER + S10_SYSTEM_MANAGER_BOOTSCRATCH_COLD1);
+ RefClk = 2500000;
+ break;
+ case CLOCK_MANAGER_PLLGLOB_PSRC_INTOSC:
+ RefClk = S10_CLOCK_INTOSC;
+ break;
+ case CLOCK_MANAGER_PLLGLOB_PSRC_F2S:
+ RefClk = MmioRead32(S10_SYSTEM_MANAGER + S10_SYSTEM_MANAGER_BOOTSCRATCH_COLD2);
+ RefClk = 5000000;
+ break;
+ }
+
+ RefClkDiv = CLOCK_MANAGER_PLLGLOB_REFCLKDIV(PllGlob);
+ MDiv = CLOCK_MANAGER_FDBCK_MDIV(MmioRead32(CLOCK_MANAGER_MAINPLL + CLOCK_MANAGER_PERPLL_FDBCK));
+
+ return (RefClk / RefClkDiv) * (6 + MDiv);
+}
+
+INTN
+S10ClockManagerGetL3MainClock() {
+ UINT32 Clock;
+ UINT32 ClockSrc = MmioRead32(CLOCK_MANAGER_MAINPLL + CLOCK_MANAGER_MAINPLL_NOCCLK);
+
+ ClockSrc = (ClockSrc >> CLOCK_MANAGER_SRC) & CLOCK_MANAGER_SRC_MSK;
+
+ switch (ClockSrc) {
+ case CLOCK_MANAGER_SRC_MAIN:
+ Clock = S10ClockManagerGetMainClock() /
+ CLOCK_MANAGER_PLLC1_DIV(MmioRead32(CLOCK_MANAGER_MAINPLL + CLOCK_MANAGER_PLLC1));
+ break;
+ case CLOCK_MANAGER_SRC_PERI:
+ Clock = S10ClockManagerGetPerClock() /
+ CLOCK_MANAGER_PLLC1_DIV(MmioRead32(CLOCK_MANAGER_PERPLL + CLOCK_MANAGER_PLLC1));
+ break;
+ case CLOCK_MANAGER_SRC_OSC1:
+ Clock = MmioRead32(S10_SYSTEM_MANAGER + S10_SYSTEM_MANAGER_BOOTSCRATCH_COLD1);
+ break;
+ case CLOCK_MANAGER_SRC_INTOSC:
+ Clock = S10_CLOCK_INTOSC;
+ break;
+ case CLOCK_MANAGER_SRC_FPGA:
+ Clock = MmioRead32(S10_SYSTEM_MANAGER + S10_SYSTEM_MANAGER_BOOTSCRATCH_COLD2);
+ break;
+ }
+
+ Clock /= 1 + (MmioRead32(CLOCK_MANAGER_MAINPLL + CLOCK_MANAGER_MAINPLL_NOCCLK) & CLOCK_MANAGER_CNT_MSK);
+
+ return Clock;
+}
+
+UINT32
+S10ClockManagerGetUartClock() {
+ return S10ClockManagerGetL3MainClock() /
+ (1 << (CLOCK_MANAGER_NOCDIV_L4SPCLK(MmioRead32(CLOCK_MANAGER_MAINPLL_NOCDIV))));
+}
+
+UINT32
+S10ClockManagerGetMmcClock() {
+ UINT32 Clock = MmioRead32(CLOCK_MANAGER_PERPLL + CLOCK_MANAGER_PERPLL_CNTR6CLK);
+
+ Clock = (Clock >> CLOCK_MANAGER_SRC) & CLOCK_MANAGER_SRC_MSK;
+
+ switch (Clock) {
+ case CLOCK_MANAGER_SRC_MAIN:
+ Clock = S10ClockManagerGetL3MainClock();
+ Clock /= 1 + (MmioRead32(CLOCK_MANAGER_PERPLL + CLOCK_MANAGER_PERPLL_CNTR6CLK) &
+ CLOCK_MANAGER_CNT_MSK);
+ break;
+ case CLOCK_MANAGER_SRC_PERI:
+ Clock = S10ClockManagerGetPerClock();
+ Clock /= 1 + (MmioRead32(CLOCK_MANAGER_PERPLL + CLOCK_MANAGER_PERPLL_CNTR6CLK) &
+ CLOCK_MANAGER_CNT_MSK);
+ break;
+ case CLOCK_MANAGER_SRC_OSC1:
+ Clock = MmioRead32(S10_SYSTEM_MANAGER + S10_SYSTEM_MANAGER_BOOTSCRATCH_COLD1);
+ break;
+ case CLOCK_MANAGER_SRC_INTOSC:
+ Clock = S10_CLOCK_INTOSC;
+ break;
+ case CLOCK_MANAGER_SRC_FPGA:
+ Clock = MmioRead32(S10_SYSTEM_MANAGER + S10_SYSTEM_MANAGER_BOOTSCRATCH_COLD2);
+ break;
+ }
+
+ return Clock / 4;
+}
+
--
2.19.0


[PATCH 3/4] Platform: Intel: Remove unused packages and clean up

Loh, Tien Hock
 

From: "Tien Hock, Loh" <tien.hock.loh@...>

Remove some unused packages in Stratix 10 packages, clean up some commented
out codes

Signed-off-by: "Tien Hock, Loh" <tien.hock.loh@...>
Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Leif Lindholm <leif.lindholm@...>
Cc: Michael D Kinney <michael.d.kinney@...>
---
Platform/Intel/Stratix10/Stratix10SoCPkg.dsc | 4 ----
Platform/Intel/Stratix10/Stratix10SoCPkg.fdf | 1 -
2 files changed, 5 deletions(-)

diff --git a/Platform/Intel/Stratix10/Stratix10SoCPkg.dsc b/Platform/Intel/Stratix10/Stratix10SoCPkg.dsc
index 17d0c5baadc6..643ce625c563 100755
--- a/Platform/Intel/Stratix10/Stratix10SoCPkg.dsc
+++ b/Platform/Intel/Stratix10/Stratix10SoCPkg.dsc
@@ -103,7 +103,6 @@ [PcdsFixedAtBuild.common]
# DEBUG_GCD 0x00100000 // Global Coherency Database changes
# DEBUG_CACHE 0x00200000 // Memory range cachability changes
# DEBUG_ERROR 0x80000000 // Error
-# gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x803010CF
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F

gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x00
@@ -251,7 +250,6 @@ [LibraryClasses.common]
PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf

CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
-# GenericBdsLib|IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

#
@@ -414,7 +412,6 @@ [Components.common]
#
MdeModulePkg/Core/Dxe/DxeMain.inf {
<LibraryClasses>
- #PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
}

@@ -475,7 +472,6 @@ [Components.common]
NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
-# NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
diff --git a/Platform/Intel/Stratix10/Stratix10SoCPkg.fdf b/Platform/Intel/Stratix10/Stratix10SoCPkg.fdf
index 2c4e5ee887ca..1ac2da28addf 100755
--- a/Platform/Intel/Stratix10/Stratix10SoCPkg.fdf
+++ b/Platform/Intel/Stratix10/Stratix10SoCPkg.fdf
@@ -140,7 +140,6 @@ [FV.FV_DXE]
INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
INF MdeModulePkg/Application/UiApp/UiApp.inf
- INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf

# FV Filesystem
--
2.19.0


[PATCH 2/4] Platform: Intel: Update license to SPDX

Loh, Tien Hock
 

From: "Tien Hock, Loh" <tien.hock.loh@...>

Update licenses to SPDX

Signed-off-by: "Tien Hock, Loh" <tien.hock.loh@...>
Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Leif Lindholm <leif.lindholm@...>
Cc: Michael D Kinney <michael.d.kinney@...>
---
Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf | 22 +++++++-------------
Platform/Intel/Stratix10/Library/IntelPlatformLib/IntelPlatformLib.inf | 22 +++++++-------------
Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c | 22 ++++++++------------
Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10Mmu.c | 20 +++++++-----------
Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10PlatformLib.c | 19 ++++++-----------
Platform/Intel/Stratix10/Library/IntelPlatformLib/AArch64/ArmPlatformHelper.S | 8 +------
6 files changed, 38 insertions(+), 75 deletions(-)

diff --git a/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf b/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
index 64b398969f1e..b16d93f22058 100644
--- a/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
+++ b/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
@@ -1,18 +1,12 @@
-#/** @file
-#
-# Copyright (c) 2019, Intel 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.
-#
-#
-#**/

+### @file
+#
+# Intel Stratix 10 Platform
+# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+###
+
[Defines]
INF_VERSION = 0x0001001B
BASE_NAME = IntelPlatformDxe
diff --git a/Platform/Intel/Stratix10/Library/IntelPlatformLib/IntelPlatformLib.inf b/Platform/Intel/Stratix10/Library/IntelPlatformLib/IntelPlatformLib.inf
index ef5c06aede7f..b6b6c743b800 100644
--- a/Platform/Intel/Stratix10/Library/IntelPlatformLib/IntelPlatformLib.inf
+++ b/Platform/Intel/Stratix10/Library/IntelPlatformLib/IntelPlatformLib.inf
@@ -1,18 +1,10 @@
-/** @file
-*
-* Stratix 10 Platform Library
-*
-* Copyright (c) 2019, Intel Corporations 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.
-*
-**/
+### @file
+#
+# Stratix 10 Platform Library
+# Copyright (c) 2019, Intel Corporation All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+###

[Defines]
INF_VERSION = 0x0001001B
diff --git a/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c b/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c
index 144b4c54ef55..b762fdc69ca4 100644
--- a/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c
+++ b/Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.c
@@ -1,16 +1,12 @@
-/** @file
-*
-* Copyright (c) 2019, Intel 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.
-*
-**/
+/** @file
+
+ Stratix 10 Platform Entry code
+
+ Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/


#include <Uefi.h>
diff --git a/Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10Mmu.c b/Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10Mmu.c
index 892387bf5d07..73ea1b423567 100644
--- a/Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10Mmu.c
+++ b/Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10Mmu.c
@@ -1,17 +1,11 @@
/** @file
-*
-* Stratix 10 Mmu configuration
-*
-* Copyright (c) 2019, Intel Corporations 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.
-*
+
+ Stratix 10 Mmu configuration
+
+ Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
**/

#include <Library/ArmLib.h>
diff --git a/Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10PlatformLib.c b/Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10PlatformLib.c
index 8ac30559362d..dc10716361b1 100644
--- a/Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10PlatformLib.c
+++ b/Platform/Intel/Stratix10/Library/IntelPlatformLib/Stratix10PlatformLib.c
@@ -1,17 +1,10 @@
/** @file
-*
-* Stratix 10 Platform Library
-*
-* Copyright (c) 2019, Intel Corporations 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.
-*
+ Stratix 10 Platform Library
+
+ Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
**/

#include <Library/ArmLib.h>
diff --git a/Platform/Intel/Stratix10/Library/IntelPlatformLib/AArch64/ArmPlatformHelper.S b/Platform/Intel/Stratix10/Library/IntelPlatformLib/AArch64/ArmPlatformHelper.S
index 2f4cf95cbf13..b7c6dbdc2e61 100644
--- a/Platform/Intel/Stratix10/Library/IntelPlatformLib/AArch64/ArmPlatformHelper.S
+++ b/Platform/Intel/Stratix10/Library/IntelPlatformLib/AArch64/ArmPlatformHelper.S
@@ -1,13 +1,7 @@
//
// Copyright (c) 2012-2013, ARM 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.
+// SPDX-License-Identifier: BSD-2-Clause-Patent
//
//

--
2.19.0


[PATCH 1/4] Platform: Intel: Update maintainers list for Stratix 10 device

Loh, Tien Hock
 

From: "Tien Hock, Loh" <tien.hock.loh@...>

Update maintainers list for Stratix 10 devices

Signed-off-by: "Tien Hock, Loh" <tien.hock.loh@...>
Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Leif Lindholm <leif.lindholm@...>
Cc: Michael D Kinney <michael.d.kinney@...>
---
Maintainers.txt | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 876ae5612ad8..47d58ffa0b2c 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -98,6 +98,11 @@ M: Shifei A Lu <shifei.a.lu@...>
M: Xiaohu Zhou <bowen.zhou@...>
M: Isaac W Oram <isaac.w.oram@...>

+Platform/Intel/Stratix10SocPkg
+M: Leif Lindholm <leif.lindholm@...>
+M: Michael D Kinney <michael.d.kinney@...>
+R: Tien Hock Loh <tien.hock.loh@...>
+
Platform/Intel/Tools
M: Bob Feng <bob.c.feng@...>
M: Liming Gao <liming.gao@...>
--
2.19.0


[PATCH 0/4] Update Stratix 10 platform support

Loh, Tien Hock
 

From: "Tien Hock, Loh" <tien.hock.loh@...>

Update maintainer list, SPDX license, remove unused packages
and remove hardcoded UART clock

Tien Hock, Loh (4):
Platform: Intel: Update maintainers list for Stratix 10 device
Platform: Intel: Update license to SPDX
Platform: Intel: Remove unused packages and clean up
Platform: Intel: Remove hardcoded Stratix 10 UART clock

Platform/Intel/Stratix10/Stratix10SoCPkg.dec | 3 +-
Platform/Intel/Stratix10/Stratix10SoCPkg.dsc | 15 +-
Platform/Intel/Stratix10/Stratix10SoCPkg.fdf | 1 -
.../IntelPlatformDxe/IntelPlatformDxe.inf | 22 ++-
.../IntelPlatformLib/IntelPlatformLib.inf | 22 +--
.../PlatformHookLib/PlatformHookLib.inf | 41 ++++++
.../S10ClockManager/S10ClockManager.inf | 34 +++++
.../Library/S10ClockManager/S10ClockManager.h | 48 +++++++
.../IntelPlatformDxe/IntelPlatformDxe.c | 22 ++-
.../Library/IntelPlatformLib/Stratix10Mmu.c | 20 +--
.../IntelPlatformLib/Stratix10PlatformLib.c | 19 +--
.../Library/PlatformHookLib/PlatformHookLib.c | 43 ++++++
.../Library/S10ClockManager/S10ClockManager.c | 133 ++++++++++++++++++
Maintainers.txt | 5 +
.../AArch64/ArmPlatformHelper.S | 8 +-
15 files changed, 353 insertions(+), 83 deletions(-)
create mode 100644 Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.inf
create mode 100644 Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.inf
create mode 100644 Platform/Intel/Stratix10/Include/Library/S10ClockManager/S10ClockManager.h
create mode 100644 Platform/Intel/Stratix10/Library/PlatformHookLib/PlatformHookLib.c
create mode 100644 Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.c

--
2.19.0


Re: [PATCH v2] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList

Ni, Ray
 

Reviewed-by: Ray Ni <ray.ni@...>

-----Original Message-----
From: Dong, Eric
Sent: Wednesday, September 4, 2019 11:15 PM
To: Chiu, Chasel <chasel.chiu@...>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>; Laszlo Ersek <lersek@...>
Subject: RE: [PATCH v2] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList

Reviewed-by: Eric Dong <eric.dong@...>

-----Original Message-----
From: Chiu, Chasel
Sent: Thursday, September 5, 2019 12:27 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Ni, Ray <ray.ni@...>; Laszlo
Ersek <lersek@...>
Subject: [PATCH v2] UefiCpuPkg: support single
EFI_PEI_CORE_FV_LOCATION_PPI in PpiList

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2153

Current logic will skip searching EFI_PEI_CORE_FV_LOCATION_PPI when the
PPI in PpiList having EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag, but
platform may pass single PPI in PpiList that should be supported.

Changed the logic to verify PpiList first before checking
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag.

Test: Verified both single EFI_PEI_CORE_FV_LOCATION_PPI and multiple
PPIs in PpiList cases and both can boot with the PeiCore
specified by EFI_PEI_CORE_FV_LOCATION_PPI.

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Chasel Chiu <chasel.chiu@...>
---
UefiCpuPkg/SecCore/SecMain.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
index 66c952b897..5d5e7f17dc 100644
--- a/UefiCpuPkg/SecCore/SecMain.c
+++ b/UefiCpuPkg/SecCore/SecMain.c
@@ -238,9 +238,8 @@ SecStartupPhase2(
// is enabled.
//
if (PpiList != NULL) {
- for (Index = 0;
- (PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) !=
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
- Index++) {
+ Index = 0;
+ do {
if (CompareGuid (PpiList[Index].Guid, &gEfiPeiCoreFvLocationPpiGuid) &&
(((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)-
PeiCoreFvLocation != 0)
) {
@@ -256,12 +255,12 @@ SecStartupPhase2(
break;
} else {
//
- // PeiCore not found
+ // Invalid PeiCore FV provided by platform
//
CpuDeadLoop ();
}
}
- }
+ } while ((PpiList[Index++].Flags &
+ EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) !=
+ EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST);
}
//
// If EFI_PEI_CORE_FV_LOCATION_PPI not found, try to locate PeiCore from
BFV.
--
2.13.3.windows.1


Re: [patch 2/3] MdeModulePkg: Unload image on EFI_SECURITY_VIOLATION

Dandan Bi
 

-----Original Message-----
From: Wu, Hao A
Sent: Thursday, September 5, 2019 1:38 PM
To: Bi, Dandan <dandan.bi@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Ni, Ray <ray.ni@...>; Gao,
Liming <liming.gao@...>; Laszlo Ersek <lersek@...>
Subject: RE: [patch 2/3] MdeModulePkg: Unload image on
EFI_SECURITY_VIOLATION

-----Original Message-----
From: Bi, Dandan
Sent: Wednesday, September 04, 2019 4:26 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Gao, Liming; Laszlo Ersek
Subject: [patch 2/3] MdeModulePkg: Unload image on
EFI_SECURITY_VIOLATION

For the LoadImage() boot service, with EFI_SECURITY_VIOLATION retval,
the Image was loaded and an ImageHandle was created with a valid
EFI_LOADED_IMAGE_PROTOCOL, but the image can not be started right
now.
This follows UEFI Spec.

But if the caller of LoadImage() doesn't have the option to defer the
execution of an image, we can not treat EFI_SECURITY_VIOLATION like
any other LoadImage() error, we should unload image for the
EFI_SECURITY_VIOLATION to avoid resource leak.

This patch is to do error handling for EFI_SECURITY_VIOLATION
explicitly for the callers in MdeModulePkg which don't have the policy
to defer the execution of the image.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Ray Ni <ray.ni@...>
Cc: Liming Gao <liming.gao@...>
Cc: Laszlo Ersek <lersek@...>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1992
Signed-off-by: Dandan Bi <dandan.bi@...>
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 9
+++++++++
MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c | 9
+++++++++
MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 9
+++++++++
.../Library/UefiBootManagerLib/BmLoadOption.c | 11 ++++++++++-
MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c | 11
++++++++++-
.../PlatformDriOverrideDxe/PlatDriOverrideLib.c | 11 ++++++++++-

Hello,

Could you help to provide the information on what tests have been
performed for this patch? Thanks.
Previously I only did the VS build since I think these are just the enhancement for error handling.
For these callers, they don't have the real use case to defer the execution of the image.
EFI_SECURITY_VIOLATION for them just like other errors, the only difference is that with EFI_SECURITY_VIOLATION retval, we need to call UnloadImage () to free resource.

Hao and other feature owners, do you have any suggestion for the tests?



Also, since the patch is touching multiple features (PCI, Capsule, BM and
driver override), I would suggest to break this patch into multiple ones so
that it will be more clear to evaluate the impact for each change.
I will separate the patch into module level and send the new patch series.


Thanks,
Dandan

Best Regards,
Hao Wu


6 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
index c994ed5fe3..1a8d9811b0 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
@@ -726,10 +726,19 @@ ProcessOpRomImage (
Buffer,
BufferSize,
&ImageHandle
);
if (EFI_ERROR (Status)) {
+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was loaded and
+ an
ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can
+ not
be started right now.
+ // If the caller doesn't have the option to defer the execution
+ of an
image, we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid resource
leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
//
// Record the Option ROM Image device path when LoadImage fails.
// PciOverride.GetDriver() will try to look for the Image
Handle using the device path later.
//
AddDriver (PciDevice, NULL, PciOptionRomImageDevicePath); diff
--git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
index 95aa9de087..74c00ecf9e 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
@@ -1028,10 +1028,19 @@ StartFmpImage (
ImageSize,
&ImageHandle
);
DEBUG((DEBUG_INFO, "FmpCapsule: LoadImage - %r\n", Status));
if (EFI_ERROR(Status)) {
+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was loaded and
+ an
ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can not
+ be
started right now.
+ // If the caller doesn't have the option to defer the execution
+ of an image,
we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid resource
leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
FreePool(DriverDevicePath);
return Status;
}

DEBUG((DEBUG_INFO, "FmpCapsule: StartImage ...\n")); diff --git
a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 952033fc82..c8de7eec03 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1859,10 +1859,19 @@ EfiBootManagerBoot (
if (FilePath != NULL) {
FreePool (FilePath);
}

if (EFI_ERROR (Status)) {
+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was loaded and
+ an
ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can
+ not
be started right now.
+ // If the caller doesn't have the option to defer the execution
+ of an
image, we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid resource
leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
//
// Report Status Code with the failure status to indicate that
the failure to load boot option
//
BmReportLoadFailure
(EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR, Status);
BootOption->Status = Status;
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 07592f8ebd..233fb43c27 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1,9 +1,9 @@
/** @file
Load option library functions which relate with creating and
processing load options.

-Copyright (c) 2011 - 2018, Intel Corporation. All rights
reserved.<BR>
+Copyright (c) 2011 - 2019, Intel Corporation. All rights
+reserved.<BR>
(C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/

@@ -1409,10 +1409,19 @@ EfiBootManagerProcessLoadOption (
FileSize,
&ImageHandle
);
FreePool (FileBuffer);

+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was loaded and
+ an
ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can not
+ be
started right now.
+ // If the caller doesn't have the option to defer the execution
+ of an image,
we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid resource
leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
if (!EFI_ERROR (Status)) {
Status = gBS->HandleProtocol (ImageHandle,
&gEfiLoadedImageProtocolGuid, (VOID **)&ImageInfo);
ASSERT_EFI_ERROR (Status);

ImageInfo->LoadOptionsSize = LoadOption->OptionalDataSize; diff
--git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
index 6b8fb4d924..cdfc57741b 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
@@ -1,9 +1,9 @@
/** @file
Misc library functions.

-Copyright (c) 2011 - 2018, Intel Corporation. All rights
reserved.<BR>
+Copyright (c) 2011 - 2019, Intel Corporation. All rights
+reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/

@@ -491,10 +491,19 @@ EfiBootManagerDispatchDeferredImages (
ImageDevicePath,
NULL,
0,
&ImageHandle
);
+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was loaded and
+ an
ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can
+ not
be started right now.
+ // If the caller doesn't have the option to defer the execution
+ of an
image, we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid resource
leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
if (!EFI_ERROR (Status)) {
LoadCount++;
//
// Before calling the image, enable the Watchdog Timer for
// a 5 Minute period
diff --git
a/MdeModulePkg/Universal/PlatformDriOverrideDxe/PlatDriOverrideLib.c
b/MdeModulePkg/Universal/PlatformDriOverrideDxe/PlatDriOverrideLib.c
index 2d3736b468..e4b6b26330 100644
---
a/MdeModulePkg/Universal/PlatformDriOverrideDxe/PlatDriOverrideLib.c
+++
b/MdeModulePkg/Universal/PlatformDriOverrideDxe/PlatDriOverrideLib.c
@@ -1,9 +1,9 @@
/** @file
Implementation of the shared functions to do the platform driver
vverride mapping.

- Copyright (c) 2007 - 2018, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2007 - 2019, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/

#include "InternalPlatDriOverrideDxe.h"
@@ -1484,10 +1484,19 @@ GetDriverFromMapping (
);
ASSERT (DriverBinding != NULL);
DriverImageInfo->ImageHandle = ImageHandle;
}
} else {
+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was
+ loaded and
an ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the
+ image can
not be started right now.
+ // If the caller doesn't have the option to defer the
+ execution of an
image, we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid
+ resource
leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
DriverImageInfo->UnLoadable = TRUE;
DriverImageInfo->ImageHandle = NULL;
}
}
}
--
2.18.0.windows.1


[PATCH v2] MinPlatformPkg/SecFspWrapperPlatformSecLib: Change TopOfTemporaryRam type

Zhang, Shenglei
 

Update the type of TopOfTemporaryRam from UINT32 to UINTN.
This change is intended to support X64 build scenarios.
The original code(line 64) may cast the overfloewed result
produced by "(TopOfTemporaryRam - sizeof (UINT32))"from
32bit to 64bit.

Cc: Michael Kubacki <michael.a.kubacki@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <@natedesimone>
Cc: Liming Gao <liming.gao@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
v2: As TopOfTemporaryRam is defined as UINTN in v2, remove
all the related casting operations which exist in v1 patch.

.../SecFspWrapperPlatformSecLib/SecGetPerformance.c | 8 ++++----
.../SecFspWrapperPlatformSecLib/SecPlatformInformation.c | 6 +++---
2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c
index c4eeb2b1..8535ae04 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c
@@ -41,7 +41,7 @@ SecGetPerformance (
{
UINT32 Size;
UINT32 Count;
- UINT32 TopOfTemporaryRam;
+ UINTN TopOfTemporaryRam;
UINT64 Ticker;
VOID *TopOfTemporaryRamPpi;
EFI_STATUS Status;
@@ -77,12 +77,12 @@ SecGetPerformance (
// | TSC[31:00] |
// |--------------|
//
- TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof(UINT32);
+ TopOfTemporaryRam = (UINTN)TopOfTemporaryRamPpi - sizeof(UINT32);
TopOfTemporaryRam -= sizeof(UINT32) * 2;
- Count = *(UINT32 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32));
+ Count = *(UINT32 *)(TopOfTemporaryRam - sizeof (UINT32));
Size = Count * sizeof (UINT32);

- Ticker = *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32) - Size - sizeof (UINT32) * 2);
+ Ticker = *(UINT64 *) (TopOfTemporaryRam - sizeof (UINT32) - Size - sizeof (UINT32) * 2);
Performance->ResetEnd = GetTimeInNanoSecond (Ticker);

return EFI_SUCCESS;
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c
index 5b94ed2b..ade36ab5 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c
@@ -36,7 +36,7 @@ SecPlatformInformation (
UINT32 *Bist;
UINT32 Size;
UINT32 Count;
- UINT32 TopOfTemporaryRam;
+ UINTN TopOfTemporaryRam;
VOID *TopOfTemporaryRamPpi;
EFI_STATUS Status;

@@ -59,9 +59,9 @@ SecPlatformInformation (
// This routine copies the BIST information to the buffer pointed by
// PlatformInformationRecord for output.
//
- TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof (UINT32);
+ TopOfTemporaryRam = (UINTN)TopOfTemporaryRamPpi - sizeof (UINT32);
TopOfTemporaryRam -= sizeof(UINT32) * 2;
- Count = *((UINT32 *)(UINTN) (TopOfTemporaryRam - sizeof (UINT32)));
+ Count = *((UINT32 *)(TopOfTemporaryRam - sizeof (UINT32)));
Size = Count * sizeof (IA32_HANDOFF_STATUS);

if ((*StructureSize) < (UINT64) Size) {
--
2.18.0.windows.1


Re: [patch] MdeModulePkg: Remove gEfiFormBrowserExProtocolGuid

Dong, Eric
 

Reviewed-by: Eric Dong <eric.dong@...>

-----Original Message-----
From: Bi, Dandan
Sent: Thursday, September 5, 2019 12:54 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Gao, Liming <liming.gao@...>
Subject: [patch] MdeModulePkg: Remove gEfiFormBrowserExProtocolGuid

gEfiFormBrowserExProtocolGuid is not used in edk2, and it can be replaced by
gEdkiiFormBrowserExProtocolGuid.
So remove gEfiFormBrowserExProtocolGuid to avoid confusion.

Cc: Eric Dong <eric.dong@...>
Cc: Liming Gao <liming.gao@...>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2145
Signed-off-by: Dandan Bi <dandan.bi@...>
---
MdeModulePkg/Include/Protocol/FormBrowserEx.h | 1 -
MdeModulePkg/MdeModulePkg.dec | 1 -
2 files changed, 2 deletions(-)

diff --git a/MdeModulePkg/Include/Protocol/FormBrowserEx.h
b/MdeModulePkg/Include/Protocol/FormBrowserEx.h
index c36aa6bdf1..e7e7cd18ab 100644
--- a/MdeModulePkg/Include/Protocol/FormBrowserEx.h
+++ b/MdeModulePkg/Include/Protocol/FormBrowserEx.h
@@ -141,10 +141,9 @@ struct
_EDKII_FORM_BROWSER_EXTENSION_PROTOCOL {
REGISTER_HOT_KEY RegisterHotKey;
REGISTER_EXIT_HANDLER RegiserExitHandler;
SAVE_REMINDER SaveReminder;
};

-extern EFI_GUID gEfiFormBrowserExProtocolGuid; extern EFI_GUID
gEdkiiFormBrowserExProtocolGuid;

#endif

diff --git a/MdeModulePkg/MdeModulePkg.dec
b/MdeModulePkg/MdeModulePkg.dec index 19935c88fa..ab29a279b1 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -529,11 +529,10 @@ [Protocols]

## Include/Protocol/LockBox.h
gEfiLockBoxProtocolGuid = { 0xbd445d79, 0xb7ad, 0x4f04, { 0x9a, 0xd8,
0x29, 0xbd, 0x20, 0x40, 0xeb, 0x3c }}

## Include/Protocol/FormBrowserEx.h
- gEfiFormBrowserExProtocolGuid = { 0x1f73b18d, 0x4630, 0x43c1, { 0xa1,
0xde, 0x6f, 0x80, 0x85, 0x5d, 0x7d, 0xa4 } }
gEdkiiFormBrowserExProtocolGuid = { 0x1f73b18d, 0x4630, 0x43c1, { 0xa1,
0xde, 0x6f, 0x80, 0x85, 0x5d, 0x7d, 0xa4 } }

## Include/Protocol/EbcVmTest.h
gEfiEbcVmTestProtocolGuid = { 0xAAEACCFD, 0xF27B, 0x4C17, { 0xB6, 0x10,
0x75, 0xCA, 0x1F, 0x2D, 0xFB, 0x52 } }

--
2.18.0.windows.1


Re: [PATCH v2] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList

Dong, Eric
 

Reviewed-by: Eric Dong <eric.dong@...>

-----Original Message-----
From: Chiu, Chasel
Sent: Thursday, September 5, 2019 12:27 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Ni, Ray <ray.ni@...>; Laszlo
Ersek <lersek@...>
Subject: [PATCH v2] UefiCpuPkg: support single
EFI_PEI_CORE_FV_LOCATION_PPI in PpiList

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2153

Current logic will skip searching EFI_PEI_CORE_FV_LOCATION_PPI when the
PPI in PpiList having EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag, but
platform may pass single PPI in PpiList that should be supported.

Changed the logic to verify PpiList first before checking
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag.

Test: Verified both single EFI_PEI_CORE_FV_LOCATION_PPI and multiple
PPIs in PpiList cases and both can boot with the PeiCore
specified by EFI_PEI_CORE_FV_LOCATION_PPI.

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Chasel Chiu <chasel.chiu@...>
---
UefiCpuPkg/SecCore/SecMain.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
index 66c952b897..5d5e7f17dc 100644
--- a/UefiCpuPkg/SecCore/SecMain.c
+++ b/UefiCpuPkg/SecCore/SecMain.c
@@ -238,9 +238,8 @@ SecStartupPhase2(
// is enabled.
//
if (PpiList != NULL) {
- for (Index = 0;
- (PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) !=
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
- Index++) {
+ Index = 0;
+ do {
if (CompareGuid (PpiList[Index].Guid, &gEfiPeiCoreFvLocationPpiGuid) &&
(((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)-
PeiCoreFvLocation != 0)
) {
@@ -256,12 +255,12 @@ SecStartupPhase2(
break;
} else {
//
- // PeiCore not found
+ // Invalid PeiCore FV provided by platform
//
CpuDeadLoop ();
}
}
- }
+ } while ((PpiList[Index++].Flags &
+ EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) !=
+ EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST);
}
//
// If EFI_PEI_CORE_FV_LOCATION_PPI not found, try to locate PeiCore from
BFV.
--
2.13.3.windows.1


Re: [PATCH] MinPlatformPkg/SecFspWrapperPlatformSecLib: Change TopOfTemporaryRam type

Zhang, Shenglei
 

It works after I apply the change according to your advice.
And that makes the code more clean. Thanks for your advice.
I'll sent out a v2 patch.

Best Regards,
Shenglei

-----Original Message-----
From: Chiu, Chasel
Sent: Thursday, September 5, 2019 10:54 AM
To: Zhang, Shenglei <shenglei.zhang@...>; devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@...>; Desimone, Nathaniel
L <@natedesimone>; Gao, Liming <liming.gao@...>
Subject: RE: [edk2-devel] [PATCH]
MinPlatformPkg/SecFspWrapperPlatformSecLib: Change
TopOfTemporaryRam type


Thanks for explanations.
So looks like the intention is to support X64 build scenarios, I think
TopOfTemporaryRam should be defined as UINTN from beginning so it can
hold 64bits address in X64 build.
Please evaluate if we could make these changes.

Thanks!
Chasel

-----Original Message-----
From: Zhang, Shenglei
Sent: Thursday, September 5, 2019 10:11 AM
To: Chiu, Chasel <chasel.chiu@...>; devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@...>; Desimone,
Nathaniel
L <@natedesimone>; Gao, Liming <liming.gao@...>
Subject: RE: [edk2-devel] [PATCH]
MinPlatformPkg/SecFspWrapperPlatformSecLib: Change
TopOfTemporaryRam type

Hi chasel,

-----Original Message-----
From: Chiu, Chasel
Sent: Wednesday, September 4, 2019 8:13 PM
To: devel@edk2.groups.io; Zhang, Shenglei <shenglei.zhang@...>
Cc: Kubacki, Michael A <michael.a.kubacki@...>; Desimone,
Nathaniel L <@natedesimone>; Gao, Liming
<liming.gao@...>
Subject: RE: [edk2-devel] [PATCH]
MinPlatformPkg/SecFspWrapperPlatformSecLib: Change
TopOfTemporaryRam
type


Hi Shenglei,

Would you please elaborate a little on how casting to UINTN can
resolve the overflow scenario and why 64bits OS will affect this code?
Actually casting to UINTN can't resolve the overflow.
What I mean is the result of (TopOfTemporaryRam - sizeof (UINT32)) may
overflow.
While it's meaningless to cast an already overflowed result to another type.
So I update the code to cast the variable to UINT before it is arithmetically
operated.

Under 64bits OS, the size of UINTN is 64-bit and the original
"(TopOfTemporaryRam - sizeof (UINT32)) "
is 32-bit. So the operation for casting will be performed. That's why 64bits
OS will affect this code.

Thanks,
Shenglei


Thanks!
Chasel

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
Zhang, Shenglei
Sent: Monday, September 2, 2019 8:35 PM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@...>; Chiu, Chasel
<chasel.chiu@...>; Desimone, Nathaniel L
<@natedesimone>; Gao, Liming <liming.gao@...>
Subject: [edk2-devel] [PATCH]
MinPlatformPkg/SecFspWrapperPlatformSecLib:
Change TopOfTemporaryRam type

Cast TopOfTemporaryRam's from UINT32 to UINTN in the expression.
The original code (TopOfTemporaryRam - sizeof (UINT32)) may cause
overflow. As a result the operation under 64-bit OS environment,
(UINT)(...),
may cast a overflowed 4-byte result to 8-byte one.

Cc: Michael Kubacki <michael.a.kubacki@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <@natedesimone>
Cc: Liming Gao <liming.gao@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
.../Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c | 2
+-
.../SecFspWrapperPlatformSecLib/SecPlatformInformation.c | 2
+-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
or
mSecLib/SecGetPerformance.c
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
or
mSecLib/SecGetPerformance.c
index c4eeb2b1..0cc42f96 100644
---
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
or
mSecLib/SecGetPerformance.c
+++
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/SecGetPerformance.c
@@ -79,7 +79,7 @@ SecGetPerformance (
//
TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi -
sizeof(UINT32);
TopOfTemporaryRam -= sizeof(UINT32) * 2;
- Count = *(UINT32 *) (UINTN) (TopOfTemporaryRam -
sizeof
(UINT32));
+ Count = *(UINT32 *)((UINTN)TopOfTemporaryRam -
sizeof
(UINT32));
Size = Count * sizeof (UINT32);

Ticker = *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32)
-
Size
- sizeof (UINT32) * 2); diff --git
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
or
mSecLib/SecPlatformInformation.c
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
or
mSecLib/SecPlatformInformation.c
index 5b94ed2b..1bcee5f4 100644
---
a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
or
mSecLib/SecPlatformInformation.c
+++
b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/SecPlatformInformation.c
@@ -61,7 +61,7 @@ SecPlatformInformation (
//
TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi -
sizeof
(UINT32);
TopOfTemporaryRam -= sizeof(UINT32) * 2;
- Count = *((UINT32 *)(UINTN) (TopOfTemporaryRam -
sizeof
(UINT32)));
+ Count = *((UINT32 *)((UINTN)TopOfTemporaryRam -
sizeof
(UINT32)));
Size = Count * sizeof (IA32_HANDOFF_STATUS);

if ((*StructureSize) < (UINT64) Size) {
--
2.18.0.windows.1



Re: [edk2] [Patch 32/33] BaseTools: ECC tool Python3 adaption

Bob Feng
 

Hi Leif,

Would you try to install antlr4-python3-runtime on debian.
pip install antlr4-python3-runtime

I think python3 + antlr3 would not be a good combination, since the antlr3 for python3 is still in beta and has not been update for 7 years. And I think there is no ECC test for such combination.

Thanks,
Bob

-----Original Message-----
From: Leif Lindholm [mailto:leif.lindholm@...]
Sent: Wednesday, September 4, 2019 5:52 PM
To: Feng, Bob C <bob.c.feng@...>
Cc: devel@edk2.groups.io; Gao, Liming <liming.gao@...>
Subject: Re: [edk2-devel] [edk2] [Patch 32/33] BaseTools: ECC tool Python3 adaption

Hi Bob,

On Wed, Sep 04, 2019 at 09:12:30AM +0000, Feng, Bob C wrote:
The CLexer.py and CParser.py under CParser3 were generated with
antlr3.0.1
(https://github.com/tianocore/tianocore.github.io/wiki/ECC-tool) . I
think API version error may be due to antlr-python-runtime on Debian
has different version. What's the antlr-python-runtime on Debian?
Running the antlr3 executable, it says
"ANTLR Parser Generator Version 3.5.2".

I guess it's worth clarifying here that I am seeing the same behaviour with python2 and antlr3.

So I am currently unable to run Ecc at all.

Best Regards,

Leif

-----Original Message-----
From: Leif Lindholm [mailto:leif.lindholm@...]
Sent: Wednesday, September 4, 2019 4:38 PM
To: Feng, Bob C <bob.c.feng@...>
Cc: devel@edk2.groups.io; Gao, Liming <liming.gao@...>
Subject: Re: [edk2-devel] [edk2] [Patch 32/33] BaseTools: ECC tool
Python3 adaption

Hi Bob,

On Wed, Sep 04, 2019 at 02:10:23AM +0000, Feng, Bob C wrote:
Hi Leif,

I have no Debian environment. On Debian, can python3 work with antlr3?
Yes. The below is equivalent to what I have already done.

Can you please respond to the question I asked about the API version error I see when I then try to run it, included in my original email?

Best Regards,

Leif

I checked the antlr3 python github repository, the source code is still in beta version and has not been updated for 7 years.

But If yes, I think the import statement in ECC can be changed as:
try:
import antlr4 as antlr
from Ecc.CParser4.CLexer import CLexer
from Ecc.CParser4.CParser import CParser
except:
import antlr3 as antlr
antlr.InputStream = antlr.StringStream
from Ecc.CParser3.CLexer import CLexer
from Ecc.CParser3.CParser import CParser


Thanks,
Bob


-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
Of Leif Lindholm
Sent: Tuesday, September 3, 2019 3:05 AM
To: Feng, Bob C <bob.c.feng@...>
Cc: devel@edk2.groups.io; Gao, Liming <liming.gao@...>
Subject: Re: [edk2-devel] [edk2] [Patch 32/33] BaseTools: ECC tool
Python3 adaption

Argh - forgot about the mailing list move, forwarding to current list.

/
Leif

On Mon, Sep 02, 2019 at 08:02:11PM +0100, Leif Lindholm wrote:
Hi Bob,

I was running Ecc today, apparently for the first time since I
switched to Python3 by default.

I have raised https://bugzilla.tianocore.org/show_bug.cgi?id=2148
over the way Python3 hard codes use of antlr4, whereas it seems to
me it should be possible to ue Python3 with antlr3 (but not
Python2 with antlr4).

However, whilst that issue could be looked at without extreme
urgency, I am curious as to what is causing the error I am seeing
upon working around the import failure on my Debian installation
(which lacks python3-antlr4).

The output I get when running
$ Ecc -t /work/git/edk2/EmbeddedPkg/Drivers/DtPlatformDxe/ -s

is

---
/work/git/edk2/BaseTools/Source/Python/Ecc/EccMain.py:409: DeprecationWarning: time.clock has been deprecated in Python 3.3 and will be removed from Python 3.8: use time.perf_counter or time.process_time instead
StartTime = time.clock()
11:44:43, Sep.02 2019 [00:00]

Loading ECC configuration ... done Building database for Meta Data
File Done!
Parsing
//work/git/edk2/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
Traceback (most recent call last):
File "/usr/lib/python3.7/runpy.py", line 193, in _run_module_as_main
"__main__", mod_spec)
File "/usr/lib/python3.7/runpy.py", line 85, in _run_code
exec(code, run_globals)
File "/work/git/edk2/BaseTools/Source/Python/Ecc/EccMain.py", line 410, in <module>
Ecc = Ecc()
File "/work/git/edk2/BaseTools/Source/Python/Ecc/EccMain.py", line 94, in __init__
self.DetectOnlyScanDirs()
File "/work/git/edk2/BaseTools/Source/Python/Ecc/EccMain.py", line 130, in DetectOnlyScanDirs
self.BuildDatabase()
File "/work/git/edk2/BaseTools/Source/Python/Ecc/EccMain.py", line 150, in BuildDatabase
c.CollectSourceCodeDataIntoDB(EccGlobalData.gTarget)
File "/work/git/edk2/BaseTools/Source/Python/Ecc/c.py", line 526, in CollectSourceCodeDataIntoDB
collector.ParseFile()
File "/work/git/edk2/BaseTools/Source/Python/Ecc/CodeFragmentCollector.py", line 517, in ParseFile
lexer = CLexer(cStream)
File "/work/git/edk2/BaseTools/Source/Python/Ecc/CParser3/CLexer.py", line 147, in __init__
Lexer.__init__(self, input)
File "/usr/lib/python3/dist-packages/antlr3/recognizers.py", line 1039, in __init__
BaseRecognizer.__init__(self, state)
File "/usr/lib/python3/dist-packages/antlr3/recognizers.py", line 169, in __init__
.format(self.api_version))
RuntimeError: ANTLR version mismatch: The recognizer has been generated with API V0, but this runtime does not support this.
---

Any idea?

Best Regards,

Leif

On Tue, Jan 29, 2019 at 10:06:09AM +0800, Feng, Bob C wrote:
v2:
The python files under CParser4 are generated by antlr4 and for
python3 usage. They have python3 specific syntax, for example
the data type declaration for the arguments of a function. That
is not compitable with python2. this patch is to remove these syntax.

ECC tool Python3 adaption.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <liming.gao@...>
---
BaseTools/Source/Python/Ecc/{ => CParser3}/CLexer.py | 0
BaseTools/Source/Python/Ecc/{ => CParser3}/CParser.py | 0
BaseTools/Source/Python/Ecc/CParser3/__init__.py | 0
BaseTools/Source/Python/Ecc/CParser4/C.g4 | 637 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
BaseTools/Source/Python/Ecc/CParser4/CLexer.py | 632 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
BaseTools/Source/Python/Ecc/CParser4/CListener.py | 815 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
BaseTools/Source/Python/Ecc/CParser4/CParser.py | 6279 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+
BaseTools/Source/Python/Ecc/CParser4/__init__.py | 0
BaseTools/Source/Python/Ecc/Check.py | 4 +-
BaseTools/Source/Python/Ecc/CodeFragmentCollector.py | 20 +--
BaseTools/Source/Python/Ecc/Configuration.py | 3 -
BaseTools/Source/Python/Ecc/EccMain.py | 2 +-
BaseTools/Source/Python/Ecc/EccToolError.py | 4 +-
BaseTools/Source/Python/Ecc/FileProfile.py | 2 +-
BaseTools/Source/Python/Ecc/MetaDataParser.py | 2 +-
BaseTools/Source/Python/Ecc/c.py | 6 +-
BaseTools/Source/Python/Ecc/config.ini | 2 -
17 files changed, 8385 insertions(+), 23 deletions(-)


Re: [PATCH v2] UefiCpuPkg: support single EFI_PEI_CORE_FV_LOCATION_PPI in PpiList

Zeng, Star
 

Reviewed-by: Star Zeng <star.zeng@...>

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
Chiu, Chasel
Sent: Thursday, September 5, 2019 12:27 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Ni, Ray <ray.ni@...>; Laszlo
Ersek <lersek@...>
Subject: [edk2-devel] [PATCH v2] UefiCpuPkg: support single
EFI_PEI_CORE_FV_LOCATION_PPI in PpiList

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2153

Current logic will skip searching EFI_PEI_CORE_FV_LOCATION_PPI when the
PPI in PpiList having EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag, but
platform may pass single PPI in PpiList that should be supported.

Changed the logic to verify PpiList first before checking
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST flag.

Test: Verified both single EFI_PEI_CORE_FV_LOCATION_PPI and multiple
PPIs in PpiList cases and both can boot with the PeiCore
specified by EFI_PEI_CORE_FV_LOCATION_PPI.

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Chasel Chiu <chasel.chiu@...>
---
UefiCpuPkg/SecCore/SecMain.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/SecCore/SecMain.c
b/UefiCpuPkg/SecCore/SecMain.c index 66c952b897..5d5e7f17dc 100644
--- a/UefiCpuPkg/SecCore/SecMain.c
+++ b/UefiCpuPkg/SecCore/SecMain.c
@@ -238,9 +238,8 @@ SecStartupPhase2(
// is enabled.
//
if (PpiList != NULL) {
- for (Index = 0;
- (PpiList[Index].Flags & EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) !=
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
- Index++) {
+ Index = 0;
+ do {
if (CompareGuid (PpiList[Index].Guid, &gEfiPeiCoreFvLocationPpiGuid)
&&
(((EFI_PEI_CORE_FV_LOCATION_PPI *) PpiList[Index].Ppi)-
PeiCoreFvLocation != 0)
) {
@@ -256,12 +255,12 @@ SecStartupPhase2(
break;
} else {
//
- // PeiCore not found
+ // Invalid PeiCore FV provided by platform
//
CpuDeadLoop ();
}
}
- }
+ } while ((PpiList[Index++].Flags &
+ EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST) !=
+ EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST);
}
//
// If EFI_PEI_CORE_FV_LOCATION_PPI not found, try to locate PeiCore
from BFV.
--
2.13.3.windows.1



Re: [patch 2/3] MdeModulePkg: Unload image on EFI_SECURITY_VIOLATION

Wu, Hao A
 

-----Original Message-----
From: Bi, Dandan
Sent: Wednesday, September 04, 2019 4:26 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Gao, Liming; Laszlo Ersek
Subject: [patch 2/3] MdeModulePkg: Unload image on
EFI_SECURITY_VIOLATION

For the LoadImage() boot service, with EFI_SECURITY_VIOLATION retval,
the Image was loaded and an ImageHandle was created with a valid
EFI_LOADED_IMAGE_PROTOCOL, but the image can not be started right now.
This follows UEFI Spec.

But if the caller of LoadImage() doesn't have the option to defer
the execution of an image, we can not treat EFI_SECURITY_VIOLATION
like any other LoadImage() error, we should unload image for the
EFI_SECURITY_VIOLATION to avoid resource leak.

This patch is to do error handling for EFI_SECURITY_VIOLATION explicitly
for the callers in MdeModulePkg which don't have the policy to defer the
execution of the image.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Ray Ni <ray.ni@...>
Cc: Liming Gao <liming.gao@...>
Cc: Laszlo Ersek <lersek@...>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1992
Signed-off-by: Dandan Bi <dandan.bi@...>
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 9
+++++++++
MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c | 9
+++++++++
MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 9 +++++++++
.../Library/UefiBootManagerLib/BmLoadOption.c | 11 ++++++++++-
MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c | 11
++++++++++-
.../PlatformDriOverrideDxe/PlatDriOverrideLib.c | 11 ++++++++++-

Hello,

Could you help to provide the information on what tests have been performed for
this patch? Thanks.

Also, since the patch is touching multiple features (PCI, Capsule, BM and
driver override), I would suggest to break this patch into multiple ones
so that it will be more clear to evaluate the impact for each change.

Best Regards,
Hao Wu


6 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
index c994ed5fe3..1a8d9811b0 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
@@ -726,10 +726,19 @@ ProcessOpRomImage (
Buffer,
BufferSize,
&ImageHandle
);
if (EFI_ERROR (Status)) {
+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was loaded and an
ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can not
be started right now.
+ // If the caller doesn't have the option to defer the execution of an
image, we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid resource leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
//
// Record the Option ROM Image device path when LoadImage fails.
// PciOverride.GetDriver() will try to look for the Image Handle using the
device path later.
//
AddDriver (PciDevice, NULL, PciOptionRomImageDevicePath);
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
index 95aa9de087..74c00ecf9e 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
@@ -1028,10 +1028,19 @@ StartFmpImage (
ImageSize,
&ImageHandle
);
DEBUG((DEBUG_INFO, "FmpCapsule: LoadImage - %r\n", Status));
if (EFI_ERROR(Status)) {
+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was loaded and an
ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can not be
started right now.
+ // If the caller doesn't have the option to defer the execution of an image,
we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid resource leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
FreePool(DriverDevicePath);
return Status;
}

DEBUG((DEBUG_INFO, "FmpCapsule: StartImage ...\n"));
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 952033fc82..c8de7eec03 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1859,10 +1859,19 @@ EfiBootManagerBoot (
if (FilePath != NULL) {
FreePool (FilePath);
}

if (EFI_ERROR (Status)) {
+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was loaded and an
ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can not
be started right now.
+ // If the caller doesn't have the option to defer the execution of an
image, we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid resource leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
//
// Report Status Code with the failure status to indicate that the failure to
load boot option
//
BmReportLoadFailure
(EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR, Status);
BootOption->Status = Status;
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 07592f8ebd..233fb43c27 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1,9 +1,9 @@
/** @file
Load option library functions which relate with creating and processing load
options.

-Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/

@@ -1409,10 +1409,19 @@ EfiBootManagerProcessLoadOption (
FileSize,
&ImageHandle
);
FreePool (FileBuffer);

+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was loaded and an
ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can not be
started right now.
+ // If the caller doesn't have the option to defer the execution of an image,
we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid resource leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
if (!EFI_ERROR (Status)) {
Status = gBS->HandleProtocol (ImageHandle,
&gEfiLoadedImageProtocolGuid, (VOID **)&ImageInfo);
ASSERT_EFI_ERROR (Status);

ImageInfo->LoadOptionsSize = LoadOption->OptionalDataSize;
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
index 6b8fb4d924..cdfc57741b 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
@@ -1,9 +1,9 @@
/** @file
Misc library functions.

-Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/

@@ -491,10 +491,19 @@ EfiBootManagerDispatchDeferredImages (
ImageDevicePath,
NULL,
0,
&ImageHandle
);
+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was loaded and an
ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can not
be started right now.
+ // If the caller doesn't have the option to defer the execution of an
image, we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid resource leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
if (!EFI_ERROR (Status)) {
LoadCount++;
//
// Before calling the image, enable the Watchdog Timer for
// a 5 Minute period
diff --git
a/MdeModulePkg/Universal/PlatformDriOverrideDxe/PlatDriOverrideLib.c
b/MdeModulePkg/Universal/PlatformDriOverrideDxe/PlatDriOverrideLib.c
index 2d3736b468..e4b6b26330 100644
---
a/MdeModulePkg/Universal/PlatformDriOverrideDxe/PlatDriOverrideLib.c
+++
b/MdeModulePkg/Universal/PlatformDriOverrideDxe/PlatDriOverrideLib.c
@@ -1,9 +1,9 @@
/** @file
Implementation of the shared functions to do the platform driver vverride
mapping.

- Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/

#include "InternalPlatDriOverrideDxe.h"
@@ -1484,10 +1484,19 @@ GetDriverFromMapping (
);
ASSERT (DriverBinding != NULL);
DriverImageInfo->ImageHandle = ImageHandle;
}
} else {
+ //
+ // With EFI_SECURITY_VIOLATION retval, the Image was loaded and
an ImageHandle was created
+ // with a valid EFI_LOADED_IMAGE_PROTOCOL, but the image can
not be started right now.
+ // If the caller doesn't have the option to defer the execution of an
image, we should
+ // unload image for the EFI_SECURITY_VIOLATION to avoid resource
leak.
+ //
+ if (Status == EFI_SECURITY_VIOLATION) {
+ gBS->UnloadImage (ImageHandle);
+ }
DriverImageInfo->UnLoadable = TRUE;
DriverImageInfo->ImageHandle = NULL;
}
}
}
--
2.18.0.windows.1


Re: [edk2-platforms] [PATCH v2] ClevoOpenBoardPkg: Fix GCC5 build issue

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@...>

-----Original Message-----
From: Agyeman, Prince
Sent: Wednesday, September 4, 2019 11:48 PM
To: devel@edk2.groups.io
Cc: Agyeman, Prince <prince.agyeman@...>; Sinha, Ankit
<ankit.sinha@...>; Desimone, Nathaniel L
<@natedesimone>; Kubacki, Michael A
<michael.a.kubacki@...>; Chiu, Chasel <chasel.chiu@...>; Bi,
Dandan <dandan.bi@...>
Subject: [edk2-platforms] [PATCH v2] ClevoOpenBoardPkg: Fix GCC5 build
issue

From: Agyeman <prince.agyeman@...>

Fixed GPIO table missing curly brackets

Cc: Ankit Sinha <ankit.sinha@...>
Cc: Nate DeSimone <@natedesimone>
Cc: Michael Kubacki <michael.a.kubacki@...>
Cc: Chasel Chiu <chasel.chiu@...>
CC: Dandan Bi <dandan.bi@...>

Signed-off-by: Agyeman <prince.agyeman@...>
---
.../N1xxWU/Library/BoardInitLib/N1xxWUGpioTable.c | 2
+-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git
a/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxW
UGpioTable.c
b/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxW
UGpioTable.c
index c99b83753f..27f70df001 100644
---
a/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxW
UGpioTable.c
+++
b/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxW
UGpioTable.c
@@ -155,7 +155,7 @@ GPIO_INIT_CONFIG mGpioTableN1xxWU[] =
{GPIO_SKL_LP_GPP_C16, {GpioPadModeGpio, GpioHostOwnGpio,
GpioDirOut, GpioOutLow, GpioIntDis, GpioHostDeepReset,
GpioTermNone}}, //I2C0_SDA
{GPIO_SKL_LP_GPP_C17, {GpioPadModeGpio, GpioHostOwnGpio,
GpioDirOut, GpioOutDefault, GpioIntDis, GpioHostDeepReset,
GpioTermNone}}, //I2C0_SCL
{GPIO_SKL_LP_GPP_C18, {GpioPadModeGpio, GpioHostOwnGpio,
GpioDirOut, GpioOutDefault, GpioIntDis, GpioHostDeepReset,
GpioTermNone}}, //I2C1_SDA
- {GPIO_SKL_LP_GPP_C19, GpioPadModeGpio, GpioHostOwnAcpi,
GpioDirInInv, GpioOutDefault, GpioIntLevel | GpioIntSci,
GpioHostDeepReset, GpioTermNone}, //I2C1_SCL
+ {GPIO_SKL_LP_GPP_C19, {GpioPadModeGpio, GpioHostOwnAcpi,
GpioDirInInv, GpioOutDefault, GpioIntLevel | GpioIntSci,
GpioHostDeepReset, GpioTermNone}}, //I2C1_SCL
{GPIO_SKL_LP_GPP_C20, {GpioPadModeNative1, GpioHostOwnDefault,
GpioDirDefault, GpioOutLow, GpioIntDis, GpioHostDeepReset,
GpioTermNone}}, //UART2_RXD
{GPIO_SKL_LP_GPP_C21, {GpioPadModeNative1, GpioHostOwnDefault,
GpioDirDefault, GpioOutDefault, GpioIntDis, GpioHostDeepReset,
GpioTermNone}}, //UART2_TXD
{GPIO_SKL_LP_GPP_C22, {GpioPadModeNative1, GpioHostOwnDefault,
GpioDirNone, GpioOutDefault, GpioIntDis, GpioHostDeepReset,
GpioTermNone}}, //UART2_RTSB
--
2.19.1.windows.1


Re: [edk2-platforms] [PATCH v3 1/1] MinPlatformPkg/Acpi: MADT NMI default flag set to Edge-Triggered

Chiu, Chasel
 

I do not see change on top of V2, why we need V3?

Reviewed-by: Chasel Chiu <chasel.chiu@...>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sinha,
Ankit
Sent: Thursday, September 5, 2019 6:18 AM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@...>; Chiu, Chasel
<chasel.chiu@...>; Desimone, Nathaniel L
<@natedesimone>; Gao, Liming <liming.gao@...>
Subject: [edk2-devel] [edk2-platforms] [PATCH v3 1/1] MinPlatformPkg/Acpi:
MADT NMI default flag set to Edge-Triggered

1. Default APIC NMI structure in MADT changed to expose
Level-Triggered interrupts.
2. x2APIC NMI structure won't be exposed to OS if x2APIC is not enabled.

Cc: Michael Kubacki <michael.a.kubacki@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <@natedesimone>
Cc: Liming Gao <liming.gao@...>

Signed-off-by: Ankit Sinha <ankit.sinha@...>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 54
++++++++++----------
1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 5eb727929bfb..1b251ca2962d 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1,7 +1,7 @@
/** @file
ACPI Platform Driver

-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -1049,7 +1049,7 @@ InstallMadtFromScratch (
LocalApciNmiStruct.Type = EFI_ACPI_4_0_LOCAL_APIC_NMI;
LocalApciNmiStruct.Length = sizeof
(EFI_ACPI_4_0_LOCAL_APIC_NMI_STRUCTURE);
LocalApciNmiStruct.AcpiProcessorId = 0xFF; // Applies to all
processors
- LocalApciNmiStruct.Flags = 0x000D; // Flags -
Level-tiggered, Active High
+ LocalApciNmiStruct.Flags = 0x0005; // Flags -
Edge-tiggered, Active High
LocalApciNmiStruct.LocalApicLint = 0x1;

ASSERT (MadtStructsIndex < MaxMadtStructCount); @@ -1066,24
+1066,26 @@ InstallMadtFromScratch (
//
// Build Local x2APIC NMI Structure
//
- LocalX2ApicNmiStruct.Type = EFI_ACPI_4_0_LOCAL_X2APIC_NMI;
- LocalX2ApicNmiStruct.Length = sizeof
(EFI_ACPI_4_0_LOCAL_X2APIC_NMI_STRUCTURE);
- LocalX2ApicNmiStruct.Flags = 0x000D; // Flags -
Level-tiggered, Active High
- LocalX2ApicNmiStruct.AcpiProcessorUid = 0xFFFFFFFF; // Applies to all
processors
- LocalX2ApicNmiStruct.LocalX2ApicLint = 0x01;
- LocalX2ApicNmiStruct.Reserved[0] = 0x00;
- LocalX2ApicNmiStruct.Reserved[1] = 0x00;
- LocalX2ApicNmiStruct.Reserved[2] = 0x00;
+ if (mX2ApicEnabled) {
+ LocalX2ApicNmiStruct.Type = EFI_ACPI_4_0_LOCAL_X2APIC_NMI;
+ LocalX2ApicNmiStruct.Length = sizeof
(EFI_ACPI_4_0_LOCAL_X2APIC_NMI_STRUCTURE);
+ LocalX2ApicNmiStruct.Flags = 0x000D; // Flags -
Level-tiggered, Active High
+ LocalX2ApicNmiStruct.AcpiProcessorUid = 0xFFFFFFFF; // Applies to
all processors
+ LocalX2ApicNmiStruct.LocalX2ApicLint = 0x01;
+ LocalX2ApicNmiStruct.Reserved[0] = 0x00;
+ LocalX2ApicNmiStruct.Reserved[1] = 0x00;
+ LocalX2ApicNmiStruct.Reserved[2] = 0x00;

- ASSERT (MadtStructsIndex < MaxMadtStructCount);
- Status = CopyStructure (
- &MadtTableHeader.Header,
- (STRUCTURE_HEADER *) &LocalX2ApicNmiStruct,
- &MadtStructs[MadtStructsIndex++]
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((EFI_D_ERROR, "CopyMadtStructure (x2APIC NMI) failed: %r\n",
Status));
- goto Done;
+ ASSERT (MadtStructsIndex < MaxMadtStructCount);
+ Status = CopyStructure (
+ &MadtTableHeader.Header,
+ (STRUCTURE_HEADER *) &LocalX2ApicNmiStruct,
+ &MadtStructs[MadtStructsIndex++]
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "CopyMadtStructure (x2APIC NMI) failed:
%r\n", Status));
+ goto Done;
+ }
}

//
@@ -1167,7 +1169,7 @@ InstallMcfgFromScratch (
//
// Set MCFG table "Length" field based on the number of PCIe segments
enumerated so far
//
- McfgTable->Header.Length = (UINT32)(sizeof
(EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEA
DER) +
+ McfgTable->Header.Length = (UINT32)(sizeof
+
(EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEA
DER) +
sizeof
(EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_A
DDRESS_ALLOCATION_STRUCTURE) * SegmentCount);

Segment = (VOID *)(McfgTable + 1);
@@ -1329,11 +1331,11 @@ PlatformUpdateTables (
HpetCapabilities.Uint64 = HpetCapabilitiesData;
HpetCapabilitiesData = MmioRead32 (HpetBaseAddress +
HPET_GENERAL_CAPABILITIES_ID_OFFSET + 4);
HpetCapabilities.Uint64 |= LShiftU64 (HpetCapabilitiesData, 32);
- HpetBlockId.Bits.Revision = HpetCapabilities.Bits.Revision;
- HpetBlockId.Bits.NumberOfTimers =
HpetCapabilities.Bits.NumberOfTimers;
- HpetBlockId.Bits.CounterSize = HpetCapabilities.Bits.CounterSize;
- HpetBlockId.Bits.Reserved = 0;
- HpetBlockId.Bits.LegacyRoute = HpetCapabilities.Bits.LegacyRoute;
+ HpetBlockId.Bits.Revision = HpetCapabilities.Bits.Revision;
+ HpetBlockId.Bits.NumberOfTimers =
HpetCapabilities.Bits.NumberOfTimers;
+ HpetBlockId.Bits.CounterSize = HpetCapabilities.Bits.CounterSize;
+ HpetBlockId.Bits.Reserved = 0;
+ HpetBlockId.Bits.LegacyRoute = HpetCapabilities.Bits.LegacyRoute;
HpetBlockId.Bits.VendorId = HpetCapabilities.Bits.VendorId;
HpetTable->EventTimerBlockId = HpetBlockId.Uint32;
HpetTable->MainCounterMinimumClockTickInPeriodicMode =
(UINT16)HpetCapabilities.Bits.CounterClockPeriod;
@@ -1459,7 +1461,7 @@ UpdateLocalTable (

for (Index = 0; Index < sizeof(mLocalTable)/sizeof(mLocalTable[0]);
Index++) {
CurrentTable = mLocalTable[Index];
-
+
PlatformUpdateTables (CurrentTable, &Version);

TableHandle = 0;
--
2.16.2.windows.1