Date   

回复: [edk2-devel] [PATCH] BaseTools: Add RISCV64 binding

gaoliming
 

Ack-by: Liming Gao <gaoliming@byosoft.com.cn>

-----邮件原件-----
发件人: bounce+27952+65443+4905953+8761045@groups.io
<bounce+27952+65443+4905953+8761045@groups.io> 代表 Nikita
发送时间: 2020年9月22日 18:38
收件人: devel@edk2.groups.io
抄送: Nikita Ermakov <sh1r4s3@mail.si-head.nl>
主题: [edk2-devel] [PATCH] BaseTools: Add RISCV64 binding

- Add RISCV64 ProcessorBind.h

- Add RISCV64 to Makefiles

Signed-off-by: Nikita Ermakov <sh1r4s3@mail.si-head.nl>
---
BaseTools/Source/C/GNUmakefile | 3 +
.../Source/C/Include/RiscV64/ProcessorBind.h | 85
+++++++++++++++++++
BaseTools/Source/C/Makefiles/header.makefile | 6 ++
3 files changed, 94 insertions(+)
create mode 100644 BaseTools/Source/C/Include/RiscV64/ProcessorBind.h

diff --git a/BaseTools/Source/C/GNUmakefile
b/BaseTools/Source/C/GNUmakefile
index df4eb64ea9..464f432774 100644
--- a/BaseTools/Source/C/GNUmakefile
+++ b/BaseTools/Source/C/GNUmakefile
@@ -26,6 +26,9 @@ ifndef HOST_ARCH
else ifneq (,$(findstring arm,$(uname_m)))

HOST_ARCH=ARM

endif

+ ifneq (,$(findstring riscv64,$(uname_m)))

+ HOST_ARCH=RISCV64

+ endif

ifndef HOST_ARCH

$(info Could not detected HOST_ARCH from uname results)

$(error HOST_ARCH is not defined!)

diff --git a/BaseTools/Source/C/Include/RiscV64/ProcessorBind.h
b/BaseTools/Source/C/Include/RiscV64/ProcessorBind.h
new file mode 100644
index 0000000000..1612d6ea7f
--- /dev/null
+++ b/BaseTools/Source/C/Include/RiscV64/ProcessorBind.h
@@ -0,0 +1,85 @@
+/** @file

+ Processor or Compiler specific defines and types for RISC-V.

+

+ SPDX-License-Identifier: BSD-2-Clause-Patent

+

+**/

+

+#ifndef __PROCESSOR_BIND_H__

+#define __PROCESSOR_BIND_H__

+

+//

+// Define the processor type so other code can make processor based
choices

+//

+#define MDE_CPU_RISCV64

+

+//

+// Make sure we are using the correct packing rules per EFI specification

+//

+#ifndef __GNUC__

+#pragma pack()

+#endif

+

+//

+// Use ANSI C 2000 stdint.h integer width declarations

+//

+#include <stdint.h>

+typedef uint8_t BOOLEAN;

+typedef int8_t INT8;

+typedef uint8_t UINT8;

+typedef int16_t INT16;

+typedef uint16_t UINT16;

+typedef int32_t INT32;

+typedef uint32_t UINT32;

+typedef int64_t INT64;

+typedef uint64_t UINT64;

+typedef char CHAR8;

+typedef uint16_t CHAR16;

+

+//

+// Unsigned value of native width. (4 bytes on supported 32-bit
processor
instructions,

+// 8 bytes on supported 64-bit processor instructions)

+//

+typedef UINT64 UINTN;

+

+//

+// Signed value of native width. (4 bytes on supported 32-bit processor
instructions,

+// 8 bytes on supported 64-bit processor instructions)

+//

+typedef INT64 INTN;

+

+//

+// Processor specific defines

+//

+

+//

+// A value of native width with the highest bit set.

+//

+#define MAX_BIT 0x8000000000000000

+

+//

+// A value of native width with the two highest bits set.

+//

+#define MAX_2_BITS 0xC000000000000000

+

+//

+// The stack alignment required for RISC-V

+//

+#define CPU_STACK_ALIGNMENT 16

+

+//

+// Modifier to ensure that all protocol member functions and EFI
intrinsics

+// use the correct C calling convention. All protocol member functions
and

+// EFI intrinsics are required to modify their member functions with
EFIAPI.

+//

+#define EFIAPI

+

+#if defined(__GNUC__)

+ //

+ // For GNU assembly code, .global or .globl can declare global symbols.

+ // Define this macro to unify the usage.

+ //

+ #define ASM_GLOBAL .globl

+#endif

+

+#endif

diff --git a/BaseTools/Source/C/Makefiles/header.makefile
b/BaseTools/Source/C/Makefiles/header.makefile
index 1c105ee7d4..0df728f327 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -28,6 +28,9 @@ ifndef HOST_ARCH
else ifneq (,$(findstring arm,$(uname_m)))

HOST_ARCH=ARM

endif

+ ifneq (,$(findstring riscv64,$(uname_m)))

+ HOST_ARCH=RISCV64

+ endif

ifndef HOST_ARCH

$(info Could not detected HOST_ARCH from uname results)

$(error HOST_ARCH is not defined!)

@@ -64,6 +67,9 @@ ARCH_INCLUDE = -I $(MAKEROOT)/Include/Arm/
else ifeq ($(HOST_ARCH), AARCH64)

ARCH_INCLUDE = -I $(MAKEROOT)/Include/AArch64/



+else ifeq ($(HOST_ARCH), RISCV64)

+ARCH_INCLUDE = -I $(MAKEROOT)/Include/RiscV64/

+

else

$(error Bad HOST_ARCH)

endif

--
2.28.0



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#65443): https://edk2.groups.io/g/devel/message/65443
Mute This Topic: https://groups.io/mt/77011297/4905953
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[gaoliming@byosoft.com.cn]
-=-=-=-=-=-=


回复: [edk2-devel] [Patch] BaseTools: Set section alignment as zero if its type is Auto

gaoliming
 

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

-----邮件原件-----
发件人: bounce+27952+65441+4905953+8761045@groups.io
<bounce+27952+65441+4905953+8761045@groups.io> 代表 Bob Feng
发送时间: 2020年9月22日 19:28
收件人: devel@edk2.groups.io
抄送: Liming Gao <gaoliming@byosoft.com.cn>; Yuwei Chen
<yuwei.chen@intel.com>
主题: [edk2-devel] [Patch] BaseTools: Set section alignment as zero if its
type
is Auto

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

Currently, the build tool try to read the section alignment
from efi file if the section alignment type is Auto.
If there is no efi generated, the section alignment will
be set to zero. This behavior causes the Makefile to be different
between the full build and the incremental build.

Since the Genffs can auto get the section alignment from
efi file during Genffs procedure, the build tool can just set section
alignment as zero. This change can make the autogen makefile
consistent for the full build and the incremental build.

Signed-off-by: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Yuwei Chen <yuwei.chen@intel.com>
---
BaseTools/Source/Python/GenFds/DataSection.py | 9 +--------
BaseTools/Source/Python/GenFds/EfiSection.py | 9 +--------
2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/DataSection.py
b/BaseTools/Source/Python/GenFds/DataSection.py
index f20fd70225..5af3ee7b7f 100644
--- a/BaseTools/Source/Python/GenFds/DataSection.py
+++ b/BaseTools/Source/Python/GenFds/DataSection.py
@@ -78,18 +78,11 @@ class DataSection (DataSectionClassObject):
if not os.path.exists(CopyMapFile) or
(os.path.getmtime(MapFile) > os.path.getmtime(CopyMapFile)):
CopyLongFilePath(MapFile, CopyMapFile)

#Get PE Section alignment when align is set to AUTO
if self.Alignment == 'Auto' and self.SecType in
(BINARY_FILE_TYPE_TE, BINARY_FILE_TYPE_PE32):
- ImageObj = PeImageClass (Filename)
- if ImageObj.SectionAlignment < 0x400:
- self.Alignment = str (ImageObj.SectionAlignment)
- elif ImageObj.SectionAlignment < 0x100000:
- self.Alignment = str (ImageObj.SectionAlignment //
0x400) + 'K'
- else:
- self.Alignment = str (ImageObj.SectionAlignment //
0x100000) + 'M'
-
+ self.Alignment = "0"
NoStrip = True
if self.SecType in (BINARY_FILE_TYPE_TE,
BINARY_FILE_TYPE_PE32):
if self.KeepReloc is not None:
NoStrip = self.KeepReloc

diff --git a/BaseTools/Source/Python/GenFds/EfiSection.py
b/BaseTools/Source/Python/GenFds/EfiSection.py
index e7d4639041..fd58391dac 100644
--- a/BaseTools/Source/Python/GenFds/EfiSection.py
+++ b/BaseTools/Source/Python/GenFds/EfiSection.py
@@ -258,18 +258,11 @@ class EfiSection (EfiSectionClassObject):
OutputFile = os.path.join( OutputPath,
ModuleName + SUP_MODULE_SEC + Num + SectionSuffix.get(SectionType))
File = GenFdsGlobalVariable.MacroExtend(File,
Dict)

#Get PE Section alignment when align is set to
AUTO
if self.Alignment == 'Auto' and (SectionType ==
BINARY_FILE_TYPE_PE32 or SectionType == BINARY_FILE_TYPE_TE):
- ImageObj = PeImageClass (File)
- if ImageObj.SectionAlignment < 0x400:
- Align = str (ImageObj.SectionAlignment)
- elif ImageObj.SectionAlignment < 0x100000:
- Align = str (ImageObj.SectionAlignment //
0x400) + 'K'
- else:
- Align = str (ImageObj.SectionAlignment //
0x100000) + 'M'
-
+ Align = "0"
if File[(len(File)-4):] == '.efi' and
FfsInf.InfModule.BaseName == os.path.basename(File)[:-4]:
MapFile = File.replace('.efi', '.map')
CopyMapFile = os.path.join(OutputPath,
ModuleName + '.map')
if IsMakefile:
if GenFdsGlobalVariable.CopyList == []:
--
2.20.1.windows.1





Re: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add PCD for shadowing all microcode.

Aaron Li
 

Hi Ray,

Accroding to https://edk2.groups.io/g/devel/files/Designs/2020/0214/Support%20the%202nd%20Microcode%20FV%20Flash%20Region.pdf
The ShadowMicrocodePei provide a FIT based shadow microcode ppi to MpInitLib. It's needed.


Best,
Aaron

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, September 23, 2020 2:25 PM
To: Li, Aaron <aaron.li@intel.com>; devel@edk2.groups.io
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Fu, Siyuan
<siyuan.fu@intel.com>
Subject: RE: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add PCD
for shadowing all microcode.

MpInitLib already contains logic to shadow microcode to memory.
Is this still needed?

-----Original Message-----
From: Li, Aaron <aaron.li@intel.com>
Sent: Wednesday, August 12, 2020 3:55 PM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
<rangasai.v.chaganty@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
Subject: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add PCD for
shadowing all microcode.

This patch is to add a PCD PcdShadowAllMicrocode to support shadowing
all microcode patch to memory.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2891

Signed-off-by: Aaron Li <aaron.li@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
---

Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodeP
ei.c
| 4 ++++

Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodeP
ei.i
nf | 3 +++
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 7 +++++++
3 files changed, 14 insertions(+)

diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
ePei
.c
b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
ePei
.c
index 8d6574f66794..5c7ee6910c8e 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
ePei
.c
+++
b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
ePei
.c
@@ -132,6 +132,10 @@ IsMicrocodePatchNeedLoad (
CPU_MICROCODE_EXTENDED_TABLE *ExtendedTable;

UINTN Index;



+ if (FeaturePcdGet (PcdShadowAllMicrocode)) {

+ return TRUE;

+ }

+

//

// Check the 'ProcessorSignature' and 'ProcessorFlags' in microcode patch
header.

//

diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
ePei
.inf
b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
ePei
.inf
index 019400ab31da..581780add891 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
ePei
.inf
+++
b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
ePei
.inf
@@ -39,5 +39,8 @@ [Guids]
gEdkiiMicrocodeShadowInfoHobGuid

gEdkiiMicrocodeStorageTypeFlashGuid



+[Pcd]

+ gIntelSiliconPkgTokenSpaceGuid.PcdShadowAllMicrocode

+

[Depex]

TRUE

diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
index e4a7fec3a3ea..3a12fe99fac6 100644
--- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
+++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
@@ -76,6 +76,13 @@ [Protocols]
# Include/Protocol/PlatformDeviceSecurityPolicy.h

gEdkiiDeviceSecurityPolicyProtocolGuid = {0x7ea41a99, 0x5e32, 0x4c97,
{0x88, 0xc4, 0xd6, 0xe7, 0x46, 0x84, 0x9, 0xd4}}



+[PcdsFeatureFlag]

+ ## Indicates if all microcode update patches shall be shadowed to
memory.

+ # TRUE - All microcode patches will be shadowed.<BR>

+ # FALSE - Only the microcode for current present processors will be
shadowed.<BR>

+ # @Prompt Shadow all microcode update patches.

+
gIntelSiliconPkgTokenSpaceGuid.PcdShadowAllMicrocode|FALSE|BOOLEAN|
0x
00000006

+

[PcdsFixedAtBuild, PcdsPatchableInModule]

## Error code for VTd error.<BR><BR>

# EDKII_ERROR_CODE_VTD_ERROR = (EFI_IO_BUS_UNSPECIFIED |
(EFI_OEM_SPECIFIC | 0x00000000)) = 0x02008000<BR>

--
2.23.0.windows.1


Re: [PATCH v3 1/1] MdeModulePkg/XhciDxe: Fix Broken Timeouts

Wu, Hao A
 

Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Henz,
Patrick
Sent: Thursday, September 24, 2020 3:36 AM
To: devel@edk2.groups.io
Cc: Patrick Henz <patrick.henz@hpe.com>; Wang, Jian J
<jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
<ray.ni@intel.com>
Subject: [edk2-devel] [PATCH v3 1/1] MdeModulePkg/XhciDxe: Fix Broken
Timeouts

From: Patrick Henz <patrick.henz@hpe.com>

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

Timeouts in the XhciDxe driver are taking longer than expected due to the
timeout loops not accounting for code execution time. As en example, 5
second timeouts have been observed to take around 36 seconds to
complete.
Use SetTimer and Create/CheckEvent from Boot Services to determine when
timeout occurred.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Patrick Henz <patrick.henz@hpe.com>
---
MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 59 +++++++++++++++++-----
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 63 ++++++++++++++++++--
----
2 files changed, 94 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
index 42b773ab31be..2bab09415b28 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
@@ -423,14 +423,15 @@ XhcClearOpRegBit (
Wait the operation register's bit as specified by Bit
to become set (or clear).

- @param Xhc The XHCI Instance.
- @param Offset The offset of the operation register.
- @param Bit The bit of the register to wait for.
- @param WaitToSet Wait the bit to set or clear.
- @param Timeout The time to wait before abort (in millisecond, ms).
+ @param Xhc The XHCI Instance.
+ @param Offset The offset of the operation register.
+ @param Bit The bit of the register to wait for.
+ @param WaitToSet Wait the bit to set or clear.
+ @param Timeout The time to wait before abort (in millisecond,
ms).

- @retval EFI_SUCCESS The bit successfully changed by host controller.
- @retval EFI_TIMEOUT The time out occurred.
+ @retval EFI_SUCCESS The bit successfully changed by host controller.
+ @retval EFI_TIMEOUT The time out occurred.
+ @retval EFI_OUT_OF_RESOURCES Memory for the timer event could not
be allocated.

**/
EFI_STATUS
@@ -442,20 +443,52 @@ XhcWaitOpRegBit (
IN UINT32 Timeout
)
{
- UINT32 Index;
- UINT64 Loop;
+ EFI_STATUS Status;
+ EFI_EVENT TimeoutEvent;

- Loop = Timeout * XHC_1_MILLISECOND;
+ TimeoutEvent = NULL;

- for (Index = 0; Index < Loop; Index++) {
+ if (Timeout == 0) {
+ return EFI_TIMEOUT;
+ }
+
+ Status = gBS->CreateEvent (
+ EVT_TIMER,
+ TPL_CALLBACK,
+ NULL,
+ NULL,
+ &TimeoutEvent
+ );
+
+ if (EFI_ERROR(Status)) {
+ goto DONE;
+ }
+
+ Status = gBS->SetTimer (TimeoutEvent,
+ TimerRelative,
+ EFI_TIMER_PERIOD_MILLISECONDS(Timeout));
+
+ if (EFI_ERROR(Status)) {
+ goto DONE;
+ }
+
+ do {
if (XHC_REG_BIT_IS_SET (Xhc, Offset, Bit) == WaitToSet) {
- return EFI_SUCCESS;
+ Status = EFI_SUCCESS;
+ goto DONE;
}

gBS->Stall (XHC_1_MICROSECOND);
+ } while (EFI_ERROR(gBS->CheckEvent (TimeoutEvent)));
+
+ Status = EFI_TIMEOUT;
+
+DONE:
+ if (TimeoutEvent != NULL) {
+ gBS->CloseEvent (TimeoutEvent);
}

- return EFI_TIMEOUT;
+ return Status;
}

/**
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index ab8957c546ee..9cb115363c8b 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -1254,14 +1254,15 @@ XhcCheckUrbResult (
/**
Execute the transfer by polling the URB. This is a synchronous operation.

- @param Xhc The XHCI Instance.
- @param CmdTransfer The executed URB is for cmd transfer or not.
- @param Urb The URB to execute.
- @param Timeout The time to wait before abort, in millisecond.
+ @param Xhc The XHCI Instance.
+ @param CmdTransfer The executed URB is for cmd transfer or not.
+ @param Urb The URB to execute.
+ @param Timeout The time to wait before abort, in millisecond.

- @return EFI_DEVICE_ERROR The transfer failed due to transfer error.
- @return EFI_TIMEOUT The transfer failed due to time out.
- @return EFI_SUCCESS The transfer finished OK.
+ @return EFI_DEVICE_ERROR The transfer failed due to transfer error.
+ @return EFI_TIMEOUT The transfer failed due to time out.
+ @return EFI_SUCCESS The transfer finished OK.
+ @retval EFI_OUT_OF_RESOURCES Memory for the timer event could not
be allocated.

**/
EFI_STATUS
@@ -1273,11 +1274,16 @@ XhcExecTransfer (
)
{
EFI_STATUS Status;
- UINTN Index;
- UINT64 Loop;
UINT8 SlotId;
UINT8 Dci;
BOOLEAN Finished;
+ EFI_EVENT TimeoutEvent;
+ BOOLEAN IndefiniteTimeout;
+
+ Status = EFI_SUCCESS;
+ Finished = FALSE;
+ TimeoutEvent = NULL;
+ IndefiniteTimeout = FALSE;

if (CmdTransfer) {
SlotId = 0;
@@ -1291,29 +1297,56 @@ XhcExecTransfer (
ASSERT (Dci < 32);
}

- Status = EFI_SUCCESS;
- Loop = Timeout * XHC_1_MILLISECOND;
if (Timeout == 0) {
- Loop = 0xFFFFFFFF;
+ IndefiniteTimeout = TRUE;
+ goto RINGDOORBELL;
}

+ Status = gBS->CreateEvent (
+ EVT_TIMER,
+ TPL_CALLBACK,
+ NULL,
+ NULL,
+ &TimeoutEvent
+ );
+
+ if (EFI_ERROR (Status)) {
+ goto DONE;
+ }
+
+ Status = gBS->SetTimer (TimeoutEvent,
+ TimerRelative,
+ EFI_TIMER_PERIOD_MILLISECONDS(Timeout));
+
+ if (EFI_ERROR (Status)) {
+ goto DONE;
+ }
+
+RINGDOORBELL:
XhcRingDoorBell (Xhc, SlotId, Dci);

- for (Index = 0; Index < Loop; Index++) {
+ do {
Finished = XhcCheckUrbResult (Xhc, Urb);
if (Finished) {
break;
}
gBS->Stall (XHC_1_MICROSECOND);
- }
+ } while (IndefiniteTimeout || EFI_ERROR(gBS->CheckEvent
+ (TimeoutEvent)));

- if (Index == Loop) {
+DONE:
+ if (EFI_ERROR(Status)) {
+ Urb->Result = EFI_USB_ERR_NOTEXECUTE;
+ } else if (!Finished) {
Urb->Result = EFI_USB_ERR_TIMEOUT;
Status = EFI_TIMEOUT;
} else if (Urb->Result != EFI_USB_NOERROR) {
Status = EFI_DEVICE_ERROR;
}

+ if (TimeoutEvent != NULL) {
+ gBS->CloseEvent (TimeoutEvent);
+ }
+
return Status;
}

--
2.28.0





回复: [edk2-devel] [PATCH 1/1] BaseTools: Copy PACKED definition from MdePkg Base.h

gaoliming
 

Mike:
Sure. Yes. This is the purpose for this change.

Thanks
Liming

-----邮件原件-----
发件人: Kinney, Michael D <michael.d.kinney@intel.com>
发送时间: 2020年9月24日 0:18
收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Kinney, Michael
D <michael.d.kinney@intel.com>
抄送: Feng, Bob C <bob.c.feng@intel.com>; Chen, Christine
<yuwei.chen@intel.com>
主题: RE: [edk2-devel] [PATCH 1/1] BaseTools: Copy PACKED definition from
MdePkg Base.h

Liming,

Thanks. This makes sense now. Can you update commit messages to help
explain
this.

I think what you are describing is the need to share include files between
BaseTools and FW packages to remove duplicate include content inside
BaseTools.
However, building BaseTools needs its own BaseTypes.h for the host build
environment.

Thanks,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
gaoliming
Sent: Tuesday, September 22, 2020 6:09 PM
To: devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@intel.com>
Cc: Feng, Bob C <bob.c.feng@intel.com>; Chen, Christine
<yuwei.chen@intel.com>
Subject: 回复: [edk2-devel] [PATCH 1/1] BaseTools: Copy PACKED definition
from MdePkg Base.h

Mike:
PACKED definition is still required in
BaseTools/Source/C/Include/Common/BaseTypes.h, because PACKED is used
in
MdePkg\Include\IndustryStandard\Acpi10.h.
After Include directory is changed, MdePkg Acpi10.h will be included.
Then, this definition is required.

C source tools include
BaseTools/Source/C/Include/Common/BaseTypes.h. They don't include
MdePkg Base.h. When C source tools
include MdePkg\Include\IndustryStandard\Acpi10.h, we need to add the
missing PACKED definition into
BaseTools/Source/C/Include/Common/BaseTypes.h. So, this change is still
related to BZ 2938.

Thanks
Liming
-----邮件原件-----
发件人: bounce+27952+65476+4905953+8761045@groups.io
<bounce+27952+65476+4905953+8761045@groups.io> 代表 Michael D
Kinney
发送时间: 2020年9月23日 7:53
收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Kinney,
Michael
D <michael.d.kinney@intel.com>
抄送: Feng, Bob C <bob.c.feng@intel.com>; Chen, Christine
<yuwei.chen@intel.com>
主题: Re: [edk2-devel] [PATCH 1/1] BaseTools: Copy PACKED definition
from
MdePkg Base.h

Liming,

Is this change still required if you change the orders of includes?

I agree that defining PACKED for BaseTools include usage makes sense,
but does not seem to be related to this BZ.

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
gaoliming
Sent: Tuesday, September 15, 2020 6:03 PM
To: devel@edk2.groups.io
Cc: Feng, Bob C <bob.c.feng@intel.com>; Chen, Christine
<yuwei.chen@intel.com>
Subject: [edk2-devel] [PATCH 1/1] BaseTools: Copy PACKED definition
from
MdePkg Base.h

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2938

MdePkg Acpi10.h definition depends on PACKED.
When structure PCD refers to Acpi10.h, build will fail,
because PACKED definition is missing in BaseTools BaseTypes.h.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
---
BaseTools/Source/C/Include/Common/BaseTypes.h | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/BaseTools/Source/C/Include/Common/BaseTypes.h
b/BaseTools/Source/C/Include/Common/BaseTypes.h
index 31d0662085a8..150980b4c0bf 100644
--- a/BaseTools/Source/C/Include/Common/BaseTypes.h
+++ b/BaseTools/Source/C/Include/Common/BaseTypes.h
@@ -57,6 +57,16 @@
#define NULL ((VOID *) 0)
#endif

+#ifdef __CC_ARM
+ //
+ // Older RVCT ARM compilers don't fully support #pragma pack and
require __packed
+ // as a prefix for the structure.
+ //
+ #define PACKED __packed
+#else
+ #define PACKED
+#endif
+
//
// Support for variable length argument lists using the ANSI
standard.
//
--
2.27.0.windows.1












Re: [EXTERNAL] [PATCH v8 00/14] Add the VariablePolicy feature

Bret Barkelew
 

Agreed. It's random that the previous config worked.


On Wed, Sep 23, 2020 at 2:32 PM Andrew Fish <afish@...> wrote:


On Sep 23, 2020, at 2:14 PM, Bret Barkelew <bret@...> wrote:

As far as I can tell, this is exposing a pre-existing race condition in EndOfDxe. It just so HAPPENS that the old "Disable RequestToLock" callback executed after this TcgMor callback, whereas the new VariablePolicy callback executes first.

I'm going to poke around for a solution, but this seems like an architectural problem with EndOfDxe.


The architecture does not guarantee order for the events. So if the events are dependent on the order of other events that is a bug in implementation. These kind of bugs are hard to notice as the DXE Core implementation event dispatch in a fixed order, I think in the order the event was registered. So it looks correct until you add more events. 

Thanks,

Andrew Fish

On Wed, Sep 23, 2020 at 6:02 AM Laszlo Ersek <lersek@...> wrote:
Hi Bret,

On 09/23/20 08:12, Bret Barkelew via groups.io wrote:
> To whom it may concern,
> This is as done as it’s going to get.
>
> Thank you all for your help!

I tried to merge this via <https://github.com/tianocore/edk2/pull/954>,
but it failed. Please review the build report, and submit v9 if necessary.

Thanks!
Laszlo



Re: [EXTERNAL] [PATCH v8 00/14] Add the VariablePolicy feature

Andrew Fish
 



On Sep 23, 2020, at 2:14 PM, Bret Barkelew <bret@...> wrote:

As far as I can tell, this is exposing a pre-existing race condition in EndOfDxe. It just so HAPPENS that the old "Disable RequestToLock" callback executed after this TcgMor callback, whereas the new VariablePolicy callback executes first.

I'm going to poke around for a solution, but this seems like an architectural problem with EndOfDxe.


The architecture does not guarantee order for the events. So if the events are dependent on the order of other events that is a bug in implementation. These kind of bugs are hard to notice as the DXE Core implementation event dispatch in a fixed order, I think in the order the event was registered. So it looks correct until you add more events. 

Thanks,

Andrew Fish

On Wed, Sep 23, 2020 at 6:02 AM Laszlo Ersek <lersek@...> wrote:
Hi Bret,

On 09/23/20 08:12, Bret Barkelew via groups.io wrote:
> To whom it may concern,
> This is as done as it’s going to get.
>
> Thank you all for your help!

I tried to merge this via <https://github.com/tianocore/edk2/pull/954>,
but it failed. Please review the build report, and submit v9 if necessary.

Thanks!
Laszlo



Re: [EXTERNAL] [PATCH v8 00/14] Add the VariablePolicy feature

Bret Barkelew
 

As far as I can tell, this is exposing a pre-existing race condition in EndOfDxe. It just so HAPPENS that the old "Disable RequestToLock" callback executed after this TcgMor callback, whereas the new VariablePolicy callback executes first.

I'm going to poke around for a solution, but this seems like an architectural problem with EndOfDxe.

On Wed, Sep 23, 2020 at 6:02 AM Laszlo Ersek <lersek@...> wrote:
Hi Bret,

On 09/23/20 08:12, Bret Barkelew via groups.io wrote:
> To whom it may concern,
> This is as done as it’s going to get.
>
> Thank you all for your help!

I tried to merge this via <https://github.com/tianocore/edk2/pull/954>,
but it failed. Please review the build report, and submit v9 if necessary.

Thanks!
Laszlo


Re: [PATCH] EmulatorPkg: Add RngLib to satisfy dependency of OpensslLib

Matthew Carlson
 

Hi Samer,

 

(I added you, zhiguang, because you had a similar question)

 

There’s some instructions in the patch series about how to enable RngLib to be used by OpenSSL (on the cover letter)

 

Since this changes the dependencies of OpenSSL, this has the potential of being
a breaking change for platforms in edk2-platforms. The easiest solution is just
to use the RngLib that uses the TimerLib as this closely mimics the behavior of
OpenSSL prior to this patch series. There is also a null version of RngLib for
CI environments that need this change
(https://edk2.groups.io/g/devel/message/50432). Though it should be pointed out
that in CI environments, the null version of BaseCryptLib or OpenSSL should be
used.

 

If you simply want the behavior that existed prior to this commit, you can just add the TimerLib based RngLib. It is not a good source of randomness but is arguably slightly better than what OpenSSL was using before.

You can see that’s what was done for OvmfPkg and ArmVirtualPkg (https://github.com/tianocore/edk2/commit/a09df5d2e1a7126e45198200628e388564f74668#diff-76767f2fe9e8f4acca7cbeb049bc8152).

I’d recommend adding a platform specific RngLib that leverages platform capabilities. If your platforms has a driver that published the RngProtocol,

you can leverage the new library at MdePkg/Library/DxeRngLib/DxeRngLib.inf (https://github.com/tianocore/edk2/commit/ed0dce7d5466b6b22ff9e0923f3a3e885540bbfc).

It will add whatever driver that produces the RngProtocol as a depex on any module that consumes crypto, so you might need to be careful not to introduce a circular depex chain, so this might not be an option for some platforms.

 

On the note of adding Azure Platform CI, OvmfPkg recently added PlatformCI and it could be a good jumping off point. https://github.com/tianocore/edk2/tree/master/OvmfPkg/PlatformCI

In a nutshell, you’ll create a new Python build file that stuart/pytools can leverage (https://github.com/tianocore/edk2-pytool-extensions) (https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/using.md)

Here’s actually an example I wrote where I ported RPi to use Pytools (https://github.com/tianocore/edk2-pytool-extensions/blob/master/docs/usability/porting_a_platform.md)

 

Once you have a platform that’s building, you can use the platform build pipeline (https://github.com/tianocore/edk2/tree/master/OvmfPkg/PlatformCI/.azurepipelines)

 

Of course, it is totally possible use a different tool like edkrepo or uefi-tools to not have to create a build file. You’d just call that from the build pipeline. I personally haven’t used them, but I’m sure there’s some folks on the mailing list that could point you in the right direction.

You’re also welcome to use something other azure pipelines, there are plenty of options out there. Azure pipelines is nice since it provides a good number of build agents for free to open source projects. But I’ve used TravisCI and Circle before (though not in EDK2) and liked the experience.

 

You’d likely need to setup a new project in the devops for tianocore (https://dev.azure.com/tianocore/) since the pipelines for edk2-ci should remain in one project. Perhaps edk2-platforms-ci?

 

  • Matthew Carlson

 

From: Samer El-Haj-Mahmoud
Sent: Wednesday, September 23, 2020 6:43 AM
To: devel@edk2.groups.io; divneil.r.wadhawan@...; matthewfcarlson@...
Cc: Ni, Ray; gaoliming; Andrew Fish; Justen, Jordan L; Kinney, Michael D; Laszlo Ersek; Yao, Jiewen; Ard Biesheuvel
Subject: RE: [PATCH] EmulatorPkg: Add RngLib to satisfy dependency of OpensslLib

 

Divneil,

 

Thanks for this patch.

 

However, it looks like multiple edk2-platforms are broken because of the OpensslLib change. I verified at least the following are broken:

- RaspberryPi/RPi3

- RaspberryPi/RPi4

- Qemu/SbsaQemu

- Socionext/DeveloperBox

- SolidRun/Armada80x0McBin

- Hisilicon/D0*

Etc.. Others are probably impacted. A quick search across edk2 and edk2-platform shows openssllib used in 26 DSC files, but RngLib is implicitly used in only 13 of them.

 

Mathew,

 

I think the offending commit (b5701a4c7a0fb185e0c5b9db9525939c78664bfd) needs to be reverted, and re-submitted with a series that fixes the build for all impacted platforms.

 

Also, what would it take to add the Azure pipeline CI that is currently used in edk2 to edk2-platform? I imagine some platform maintainers would appreciate that capability. Or should every platform look for their own CI/CD (possibly outside TianoCore)?

 

Thanks,

--Samer

 

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wadhawan, Divneil R via groups.io

Sent: Saturday, September 19, 2020 1:39 AM

To: devel@edk2.groups.io

Cc: Ni, Ray <ray.ni@...>; gaoliming <gaoliming@...>; 'Andrew Fish' <afish@...>; Justen, Jordan L <jordan.l.justen@...>; Kinney, Michael D <michael.d.kinney@...>; Wadhawan, Divneil R <divneil.r.wadhawan@...>

Subject: [edk2-devel] [PATCH] EmulatorPkg: Add RngLib to satisfy dependency of OpensslLib

 

 

o Recently, OpensslLib [LibraryClasses] has been changed

  to include RngLib which causes the SECURE_BOOT_ENABLE

  build to fail in want of RngLib

 

o This patch adds the RngLib for OpensslLib

 

Signed-off-by: Divneil Rai Wadhawan <mailto:divneil.r.wadhawan@...>

---

EmulatorPkg/EmulatorPkg.dsc | 1 +

1 file changed, 1 insertion(+)

 

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc

index c6e25c745e..a27cb1beb0 100644

--- a/EmulatorPkg/EmulatorPkg.dsc

+++ b/EmulatorPkg/EmulatorPkg.dsc

@@ -113,6 +113,7 @@

   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf

 

!if $(SECURE_BOOT_ENABLE) == TRUE

+  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf

   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf

   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf

   PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf

--

2.16.2.windows.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

 


Re: [EXTERNAL] [PATCH v8 00/14] Add the VariablePolicy feature

Sean
 

Ard/Laszlo/Samer (since you reported issue with RngLib),

Is now the time to start a discussion about using submodules or at least tracked dependencies in edk2-platforms. The way it stands the only thing you can use to correlate the two are the timestamp of the commit. This means that the history of edk2-platforms will not actually be any different if you merge into edk2 as a two part series or one.

This has been a major pain point as someone who wants to use edk2-platforms downstream.

Another benefit of tracked dependencies is that each platform or group of commonly managed platforms would move their dependencies as they incorporate any edk2 change. This also gives visibility into the maintenance and status of a platform.

Thanks
Sean

On 9/23/2020 3:17 AM, Ard Biesheuvel wrote:
On 9/23/20 12:04 PM, Laszlo Ersek wrote:
On 09/23/20 11:43, Laszlo Ersek wrote:
On 09/23/20 11:22, Ard Biesheuvel wrote:
On 9/23/20 10:45 AM, Laszlo Ersek wrote:
On 09/23/20 08:12, Bret Barkelew via groups.io wrote:
To whom it may concern,
This is as done as it’s going to get.

Thank you all for your help!
Seems like it's been fully reviewed. If that's the case -- do you want
me to merge it?

(Asking because the series modifies multiple packages, so there isn't a
maintainer that's uniquely responsible for merging the series.)
Yes, please. This has been going on long enough.

Only question I have is breakage in edk2-platforms - it seems that most
platforms there are broken atm anyway due to the RngLib change having
been merged, but it would be good to have an idea what the status is there.
Judged from patches 05 through 08, the platforms in edk2-platforms are
going to need some new lib class resolutions. Therefore I think we might
have to merge this in two parts:

- patches 01-08 in the first go,
- [update edk2-platforms to mimick patches 05-08],
- patches 09-14 in the second round.

If Bret is OK with that, I can start merging 01-08 soon.

(In theory, we could merge patches 05-08 as a part of the second round,
because technically, edk2-platforms only need 01-04. However, if some
commit messages in edk2-platforms would like to reference *example
platform code* from edk2, then having stable commit hashes for patches
05-08 in edk2 would be useful. Hence my suggestion to include 05-08 in
the first round of edk2 merging.)
... on a second thought, we could merge this series in a single PR as
well; only edk2-platforms would have to advance its edk2 submodule
reference in two stages:

- first advance the submodule to patch#8,
- then update its own platform DSC files with the new lib instances,
- then advance the edk2 submodule again, to patch#14.

If that works for you, I think we should merge this edk2 set in one go
-- less work for me, and much more intuitive when viewed from the edk2
side. (The series would not be stuck in some half-merged state for any
time at all.)
We don't actually use git submodules there, so this does not work.
But I am fine with just merging this, as edk2-platforms has been reported to be in broken state anyway.


[PATCH v3 1/1] MdeModulePkg/XhciDxe: Fix Broken Timeouts

Henz, Patrick
 

From: Patrick Henz <patrick.henz@hpe.com>

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

Timeouts in the XhciDxe driver are taking longer than
expected due to the timeout loops not accounting for
code execution time. As en example, 5 second timeouts
have been observed to take around 36 seconds to complete.
Use SetTimer and Create/CheckEvent from Boot Services to
determine when timeout occurred.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Patrick Henz <patrick.henz@hpe.com>
---
MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 59 +++++++++++++++++-----
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 63 ++++++++++++++++++------
2 files changed, 94 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
index 42b773ab31be..2bab09415b28 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
@@ -423,14 +423,15 @@ XhcClearOpRegBit (
Wait the operation register's bit as specified by Bit
to become set (or clear).

- @param Xhc The XHCI Instance.
- @param Offset The offset of the operation register.
- @param Bit The bit of the register to wait for.
- @param WaitToSet Wait the bit to set or clear.
- @param Timeout The time to wait before abort (in millisecond, ms).
+ @param Xhc The XHCI Instance.
+ @param Offset The offset of the operation register.
+ @param Bit The bit of the register to wait for.
+ @param WaitToSet Wait the bit to set or clear.
+ @param Timeout The time to wait before abort (in millisecond, ms).

- @retval EFI_SUCCESS The bit successfully changed by host controller.
- @retval EFI_TIMEOUT The time out occurred.
+ @retval EFI_SUCCESS The bit successfully changed by host controller.
+ @retval EFI_TIMEOUT The time out occurred.
+ @retval EFI_OUT_OF_RESOURCES Memory for the timer event could not be allocated.

**/
EFI_STATUS
@@ -442,20 +443,52 @@ XhcWaitOpRegBit (
IN UINT32 Timeout
)
{
- UINT32 Index;
- UINT64 Loop;
+ EFI_STATUS Status;
+ EFI_EVENT TimeoutEvent;

- Loop = Timeout * XHC_1_MILLISECOND;
+ TimeoutEvent = NULL;

- for (Index = 0; Index < Loop; Index++) {
+ if (Timeout == 0) {
+ return EFI_TIMEOUT;
+ }
+
+ Status = gBS->CreateEvent (
+ EVT_TIMER,
+ TPL_CALLBACK,
+ NULL,
+ NULL,
+ &TimeoutEvent
+ );
+
+ if (EFI_ERROR(Status)) {
+ goto DONE;
+ }
+
+ Status = gBS->SetTimer (TimeoutEvent,
+ TimerRelative,
+ EFI_TIMER_PERIOD_MILLISECONDS(Timeout));
+
+ if (EFI_ERROR(Status)) {
+ goto DONE;
+ }
+
+ do {
if (XHC_REG_BIT_IS_SET (Xhc, Offset, Bit) == WaitToSet) {
- return EFI_SUCCESS;
+ Status = EFI_SUCCESS;
+ goto DONE;
}

gBS->Stall (XHC_1_MICROSECOND);
+ } while (EFI_ERROR(gBS->CheckEvent (TimeoutEvent)));
+
+ Status = EFI_TIMEOUT;
+
+DONE:
+ if (TimeoutEvent != NULL) {
+ gBS->CloseEvent (TimeoutEvent);
}

- return EFI_TIMEOUT;
+ return Status;
}

/**
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index ab8957c546ee..9cb115363c8b 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -1254,14 +1254,15 @@ XhcCheckUrbResult (
/**
Execute the transfer by polling the URB. This is a synchronous operation.

- @param Xhc The XHCI Instance.
- @param CmdTransfer The executed URB is for cmd transfer or not.
- @param Urb The URB to execute.
- @param Timeout The time to wait before abort, in millisecond.
+ @param Xhc The XHCI Instance.
+ @param CmdTransfer The executed URB is for cmd transfer or not.
+ @param Urb The URB to execute.
+ @param Timeout The time to wait before abort, in millisecond.

- @return EFI_DEVICE_ERROR The transfer failed due to transfer error.
- @return EFI_TIMEOUT The transfer failed due to time out.
- @return EFI_SUCCESS The transfer finished OK.
+ @return EFI_DEVICE_ERROR The transfer failed due to transfer error.
+ @return EFI_TIMEOUT The transfer failed due to time out.
+ @return EFI_SUCCESS The transfer finished OK.
+ @retval EFI_OUT_OF_RESOURCES Memory for the timer event could not be allocated.

**/
EFI_STATUS
@@ -1273,11 +1274,16 @@ XhcExecTransfer (
)
{
EFI_STATUS Status;
- UINTN Index;
- UINT64 Loop;
UINT8 SlotId;
UINT8 Dci;
BOOLEAN Finished;
+ EFI_EVENT TimeoutEvent;
+ BOOLEAN IndefiniteTimeout;
+
+ Status = EFI_SUCCESS;
+ Finished = FALSE;
+ TimeoutEvent = NULL;
+ IndefiniteTimeout = FALSE;

if (CmdTransfer) {
SlotId = 0;
@@ -1291,29 +1297,56 @@ XhcExecTransfer (
ASSERT (Dci < 32);
}

- Status = EFI_SUCCESS;
- Loop = Timeout * XHC_1_MILLISECOND;
if (Timeout == 0) {
- Loop = 0xFFFFFFFF;
+ IndefiniteTimeout = TRUE;
+ goto RINGDOORBELL;
}

+ Status = gBS->CreateEvent (
+ EVT_TIMER,
+ TPL_CALLBACK,
+ NULL,
+ NULL,
+ &TimeoutEvent
+ );
+
+ if (EFI_ERROR (Status)) {
+ goto DONE;
+ }
+
+ Status = gBS->SetTimer (TimeoutEvent,
+ TimerRelative,
+ EFI_TIMER_PERIOD_MILLISECONDS(Timeout));
+
+ if (EFI_ERROR (Status)) {
+ goto DONE;
+ }
+
+RINGDOORBELL:
XhcRingDoorBell (Xhc, SlotId, Dci);

- for (Index = 0; Index < Loop; Index++) {
+ do {
Finished = XhcCheckUrbResult (Xhc, Urb);
if (Finished) {
break;
}
gBS->Stall (XHC_1_MICROSECOND);
- }
+ } while (IndefiniteTimeout || EFI_ERROR(gBS->CheckEvent (TimeoutEvent)));

- if (Index == Loop) {
+DONE:
+ if (EFI_ERROR(Status)) {
+ Urb->Result = EFI_USB_ERR_NOTEXECUTE;
+ } else if (!Finished) {
Urb->Result = EFI_USB_ERR_TIMEOUT;
Status = EFI_TIMEOUT;
} else if (Urb->Result != EFI_USB_NOERROR) {
Status = EFI_DEVICE_ERROR;
}

+ if (TimeoutEvent != NULL) {
+ gBS->CloseEvent (TimeoutEvent);
+ }
+
return Status;
}

--
2.28.0


[PATCH v3 0/1] MdeModulePkg/XhciDxe: Fix Broken Timeouts

Henz, Patrick
 

From: Patrick Henz <patrick.henz@hpe.com>

Timeouts in the XhciDxe driver are taking longer than
expected due to the timeout loops not accounting for
code execution time. As en example, 5 second timeouts
have been observed to take around 36 seconds to complete.
Use SetTimer and Create/CheckEvent from Boot Services to
determine when timeout occurred. This patch was tested
using forced timeouts and print statements with QEmu as
well as phycial hardware. The forced timeouts were
implemented in code via static variables that guaranteed
a timeout the first time the function with the broken
timeout was called.

Example:

XhcExecTransfer (
.
.
)
{
.
.
static int do_once = 1; // test line
.
.
do {
Finished = XhcCheckUrbResult (Xhc, Urb);
if (do_once) Finished = 0; // test line
if (Finished) {
break;
}
gBS->Stall (XHC_1_MICROSECOND);
} while (EFI_ERROR(gBS->CheckEvent (TimeoutEvent)));

do_once = 0; // test line

Using this forced timeout approach the correct timeouts
were observed on both hardware and in QEmu.

Similar broken timeout loops have been found in the Uhci
and Ehci drivers. This patch does not fix those issues.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Patrick Henz <patrick.henz@hpe.com>

Patrick Henz (1):
MdeModulePkg/XhciDxe: Fix Broken Timeouts

MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 59 +++++++++++++++++-----
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 63 ++++++++++++++++++------
2 files changed, 94 insertions(+), 28 deletions(-)

--
2.28.0


Re: [edk2-test][PATCH v1 2/4] uefi-sct/SctPkg: Add SCT User Guide Word document

G Edhaya Chandran
 

Reviewed-by: G Edhaya Chandran <edhaya.chandran@...>

Upstreamed by Commit ID:
https://github.com/tianocore/edk2-test/commit/9655dc86702804e11d5c233596a2aaf52bbe761e


[PATCH v2 1/1] UefiCpuPkg/MpInitLib: Reduce reset vector memory pressure

Lendacky, Thomas
 

From: Tom Lendacky <thomas.lendacky@amd.com>

The AP reset vector stack allocation is only required if running as an
SEV-ES guest. Since the reset vector allocation is below 1MB in memory,
eliminate the requirement for bare-metal systems and non SEV-ES guests
to allocate the extra stack area, which can be large if the
PcdCpuMaxLogicalProcessorNumber value is large, and also remove the
CPU_STACK_ALIGNMENT alignment.

Fixes: 7b7508ad784d ("UefiCpuPkg: Allow AP booting under SEV-ES")
Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
UefiCpuPkg/Library/MpInitLib/MpLib.c | 36 +++++++++-----------
1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 07426274f639..a9708c6479d2 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1141,20 +1141,6 @@ RestoreWakeupBuffer(
);
}

-/**
- Calculate the size of the reset stack.
-
- @return Total amount of memory required for stacks
-**/
-STATIC
-UINTN
-GetApResetStackSize (
- VOID
- )
-{
- return AP_RESET_STACK_SIZE * PcdGet32(PcdCpuMaxLogicalProcessorNumber);
-}
-
/**
Calculate the size of the reset vector.

@@ -1170,11 +1156,23 @@ GetApResetVectorSize (
{
UINTN Size;

- Size = ALIGN_VALUE (AddressMap->RendezvousFunnelSize +
- AddressMap->SwitchToRealSize +
- sizeof (MP_CPU_EXCHANGE_INFO),
- CPU_STACK_ALIGNMENT);
- Size += GetApResetStackSize ();
+ Size = AddressMap->RendezvousFunnelSize +
+ AddressMap->SwitchToRealSize +
+ sizeof (MP_CPU_EXCHANGE_INFO);
+
+ //
+ // The AP reset stack is only used by SEV-ES guests. Do not add to the
+ // allocation if SEV-ES is not enabled.
+ //
+ if (PcdGetBool (PcdSevEsIsEnabled)) {
+ //
+ // Stack location is based on APIC ID, so use the total number of
+ // processors for calculating the total stack area.
+ //
+ Size += AP_RESET_STACK_SIZE * PcdGet32(PcdCpuMaxLogicalProcessorNumber);
+
+ Size = ALIGN_VALUE (Size, CPU_STACK_ALIGNMENT);
+ }

return Size;
}
--
2.28.0


Re: [PATCH 1/1] BaseTools: Copy PACKED definition from MdePkg Base.h

Michael D Kinney
 

Liming,

Thanks. This makes sense now. Can you update commit messages to help explain
this.

I think what you are describing is the need to share include files between
BaseTools and FW packages to remove duplicate include content inside BaseTools.
However, building BaseTools needs its own BaseTypes.h for the host build
environment.

Thanks,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
Sent: Tuesday, September 22, 2020 6:09 PM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Feng, Bob C <bob.c.feng@intel.com>; Chen, Christine <yuwei.chen@intel.com>
Subject: 回复: [edk2-devel] [PATCH 1/1] BaseTools: Copy PACKED definition from MdePkg Base.h

Mike:
PACKED definition is still required in BaseTools/Source/C/Include/Common/BaseTypes.h, because PACKED is used in
MdePkg\Include\IndustryStandard\Acpi10.h.
After Include directory is changed, MdePkg Acpi10.h will be included. Then, this definition is required.

C source tools include BaseTools/Source/C/Include/Common/BaseTypes.h. They don't include MdePkg Base.h. When C source tools
include MdePkg\Include\IndustryStandard\Acpi10.h, we need to add the missing PACKED definition into
BaseTools/Source/C/Include/Common/BaseTypes.h. So, this change is still related to BZ 2938.

Thanks
Liming
-----邮件原件-----
发件人: bounce+27952+65476+4905953+8761045@groups.io
<bounce+27952+65476+4905953+8761045@groups.io> 代表 Michael D
Kinney
发送时间: 2020年9月23日 7:53
收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Kinney, Michael
D <michael.d.kinney@intel.com>
抄送: Feng, Bob C <bob.c.feng@intel.com>; Chen, Christine
<yuwei.chen@intel.com>
主题: Re: [edk2-devel] [PATCH 1/1] BaseTools: Copy PACKED definition from
MdePkg Base.h

Liming,

Is this change still required if you change the orders of includes?

I agree that defining PACKED for BaseTools include usage makes sense,
but does not seem to be related to this BZ.

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
gaoliming
Sent: Tuesday, September 15, 2020 6:03 PM
To: devel@edk2.groups.io
Cc: Feng, Bob C <bob.c.feng@intel.com>; Chen, Christine
<yuwei.chen@intel.com>
Subject: [edk2-devel] [PATCH 1/1] BaseTools: Copy PACKED definition from
MdePkg Base.h

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2938

MdePkg Acpi10.h definition depends on PACKED.
When structure PCD refers to Acpi10.h, build will fail,
because PACKED definition is missing in BaseTools BaseTypes.h.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
---
BaseTools/Source/C/Include/Common/BaseTypes.h | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/BaseTools/Source/C/Include/Common/BaseTypes.h
b/BaseTools/Source/C/Include/Common/BaseTypes.h
index 31d0662085a8..150980b4c0bf 100644
--- a/BaseTools/Source/C/Include/Common/BaseTypes.h
+++ b/BaseTools/Source/C/Include/Common/BaseTypes.h
@@ -57,6 +57,16 @@
#define NULL ((VOID *) 0)
#endif

+#ifdef __CC_ARM
+ //
+ // Older RVCT ARM compilers don't fully support #pragma pack and
require __packed
+ // as a prefix for the structure.
+ //
+ #define PACKED __packed
+#else
+ #define PACKED
+#endif
+
//
// Support for variable length argument lists using the ANSI standard.
//
--
2.27.0.windows.1












Re: [PATCH] uefi-sct/SctPkg: Correct issue with memory protection enabled.

Jeff Brasen
 

Didn't see it at first as it was not at the top of edk2-test but under uefi-sct. CC'd maintainers

Thanks,

Jeff


From: Laszlo Ersek <lersek@...>
Sent: Wednesday, September 23, 2020 2:59 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Jeff Brasen <jbrasen@...>
Subject: Re: [edk2-devel] [PATCH] uefi-sct/SctPkg: Correct issue with memory protection enabled.
 
External email: Use caution opening links or attachments


On 09/23/20 00:13, Jeff Brasen wrote:
> Any comments on this change?

I suggest CC'ing the maintainers responsible for reviewing this change.
(I don't know who they are, unfortunately -- is there a Maintainers.txt
file in the uefi-sct tree?)

Thanks
Laszlo

>
>
> Thanks,
>
> Jeff
>
> ________________________________
> From: Jeff Brasen <jbrasen@...>
> Sent: Friday, September 11, 2020 11:23 AM
> To: devel@edk2.groups.io <devel@edk2.groups.io>
> Cc: Jeff Brasen <jbrasen@...>
> Subject: [PATCH] uefi-sct/SctPkg: Correct issue with memory protection enabled.
>
> On systems with memory protection enabled the modification of local
> function initialization data results in permission issue. Make a copy of
> data prior to modification.
>
> Signed-off-by: Jeff Brasen <jbrasen@...>
> ---
>  .../UnicodeCollationBBTestFunction.c          | 38 ++++++++++---------
>  .../UnicodeCollation2BBTestFunction.c         | 38 ++++++++++---------
>  2 files changed, 42 insertions(+), 34 deletions(-)
>
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/UnicodeCollation/BlackBoxTest/UnicodeCollationBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/UnicodeCollation/BlackBoxTest/UnicodeCollationBBTestFunction.c
> index 6fa11e6c..e0b4c1d9 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/UnicodeCollation/BlackBoxTest/UnicodeCollationBBTestFunction.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/UnicodeCollation/BlackBoxTest/UnicodeCollationBBTestFunction.c
> @@ -25,7 +25,7 @@ Abstract:
>  --*/
>
>
>
>
>
> -#include "SctLib.h"
> +#include "SctLib.h"
>
>  #include "UnicodeCollationBBTestMain.h"
>
>
>
>
>
> @@ -337,6 +337,7 @@ BBTestStrLwrFunctionAutoTest (
>                                          };
>
>
>
>    CHAR16                               TestDataSav[MAX_SIZE_OF_STRING + 1];
>
> +  CHAR16                               TestDataRw[MAX_SIZE_OF_STRING + 1];
>
>
>
>
>
>
>
> @@ -368,14 +369,15 @@ BBTestStrLwrFunctionAutoTest (
>      //
>
>      // Backup current test data
>
>      //
>
> +    CopyUnicodeString (TestDataRw, TestData[Index]);
>
>      CopyUnicodeString (TestDataSav, TestData[Index]);
>
>
>
>      //
>
>      // For each test data, test the StrLwr functionality.
>
>      //
>
> -    UnicodeCollation->StrLwr (UnicodeCollation, TestData[Index]);
>
> +    UnicodeCollation->StrLwr (UnicodeCollation, TestDataRw);
>
>
>
> -    if (CheckStrLwr (TestDataSav, TestData[Index])) {
>
> +    if (CheckStrLwr (TestDataSav, TestDataRw)) {
>
>        AssertionType = EFI_TEST_ASSERTION_PASSED;
>
>      } else {
>
>        AssertionType = EFI_TEST_ASSERTION_FAILED;
>
> @@ -390,15 +392,15 @@ BBTestStrLwrFunctionAutoTest (
>                     __FILE__,
>
>                     (UINTN)__LINE__,
>
>                     TestDataSav,
>
> -                   TestData[Index]
>
> +                   TestDataRw
>
>                     );
>
>
>
>
>
> -    CopyUnicodeString (TestDataSav, TestData[Index]);
>
> -    UnicodeCollation->StrUpr (UnicodeCollation, TestData[Index]);
>
> -    UnicodeCollation->StrLwr (UnicodeCollation, TestData[Index]);
>
> +    CopyUnicodeString (TestDataSav, TestDataRw);
>
> +    UnicodeCollation->StrUpr (UnicodeCollation, TestDataRw);
>
> +    UnicodeCollation->StrLwr (UnicodeCollation, TestDataRw);
>
>
>
> -    if (CheckStrEql (TestDataSav, TestData[Index])) {
>
> +    if (CheckStrEql (TestDataSav, TestDataRw)) {
>
>        AssertionType = EFI_TEST_ASSERTION_PASSED;
>
>      } else {
>
>        AssertionType = EFI_TEST_ASSERTION_FAILED;
>
> @@ -413,7 +415,7 @@ BBTestStrLwrFunctionAutoTest (
>                     __FILE__,
>
>                     (UINTN)__LINE__,
>
>                     TestDataSav,
>
> -                   TestData[Index]
>
> +                   TestDataRw
>
>                     );
>
>    };
>
>
>
> @@ -458,6 +460,7 @@ BBTestStrUprFunctionAutoTest (
>                                          };
>
>
>
>    CHAR16                               TestDataSav[MAX_SIZE_OF_STRING + 1];
>
> +  CHAR16                               TestDataRw[MAX_SIZE_OF_STRING + 1];
>
>
>
>
>
>
>
> @@ -490,13 +493,14 @@ BBTestStrUprFunctionAutoTest (
>      // Backup current test data
>
>      //
>
>      CopyUnicodeString (TestDataSav, TestData[Index]);
>
> +    CopyUnicodeString (TestDataRw, TestData[Index]);
>
>
>
>      //
>
>      // For each test data, test the StrUpr functionality.
>
>      //
>
> -    UnicodeCollation->StrUpr (UnicodeCollation, TestData[Index]);
>
> +    UnicodeCollation->StrUpr (UnicodeCollation, TestDataRw);
>
>
>
> -    if (CheckStrUpr (TestDataSav, TestData[Index])) {
>
> +    if (CheckStrUpr (TestDataSav, TestDataRw)) {
>
>        AssertionType = EFI_TEST_ASSERTION_PASSED;
>
>      } else {
>
>        AssertionType = EFI_TEST_ASSERTION_FAILED;
>
> @@ -511,14 +515,14 @@ BBTestStrUprFunctionAutoTest (
>                     __FILE__,
>
>                     (UINTN)__LINE__,
>
>                     TestDataSav,
>
> -                   TestData[Index]
>
> +                   TestDataRw
>
>                     );
>
>
>
> -    CopyUnicodeString (TestDataSav, TestData[Index]);
>
> -    UnicodeCollation->StrLwr (UnicodeCollation, TestData[Index]);
>
> -    UnicodeCollation->StrUpr (UnicodeCollation, TestData[Index]);
>
> +    CopyUnicodeString (TestDataSav, TestDataRw);
>
> +    UnicodeCollation->StrLwr (UnicodeCollation, TestDataRw);
>
> +    UnicodeCollation->StrUpr (UnicodeCollation, TestDataRw);
>
>
>
> -    if (CheckStrEql (TestDataSav, TestData[Index])) {
>
> +    if (CheckStrEql (TestDataSav, TestDataRw)) {
>
>        AssertionType = EFI_TEST_ASSERTION_PASSED;
>
>      } else {
>
>        AssertionType = EFI_TEST_ASSERTION_FAILED;
>
> @@ -533,7 +537,7 @@ BBTestStrUprFunctionAutoTest (
>                     __FILE__,
>
>                     (UINTN)__LINE__,
>
>                     TestDataSav,
>
> -                   TestData[Index]
>
> +                   TestDataRw
>
>                     );
>
>    };
>
>
>
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/UnicodeCollation2/BlackBoxTest/UnicodeCollation2BBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/UnicodeCollation2/BlackBoxTest/UnicodeCollation2BBTestFunction.c
> index 653b263a..19ff6764 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/UnicodeCollation2/BlackBoxTest/UnicodeCollation2BBTestFunction.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/UnicodeCollation2/BlackBoxTest/UnicodeCollation2BBTestFunction.c
> @@ -25,7 +25,7 @@ Abstract:
>  --*/
>
>
>
>
>
> -#include "SctLib.h"
> +#include "SctLib.h"
>
>  #include "UnicodeCollation2BBTestMain.h"
>
>
>
>  STATIC CONST STRICOLL_TEST_DATA_FIELD             mStriCollTestData[] ={
>
> @@ -335,6 +335,7 @@ BBTestStrLwrFunctionAutoTest (
>                                          };
>
>
>
>    CHAR16                               TestDataSav[MAX_SIZE_OF_STRING + 1];
>
> +  CHAR16                               TestDataRw[MAX_SIZE_OF_STRING + 1];
>
>
>
>
>
>
>
> @@ -367,13 +368,14 @@ BBTestStrLwrFunctionAutoTest (
>      // Backup current test data
>
>      //
>
>      CopyUnicodeString (TestDataSav, TestData[Index]);
>
> +    CopyUnicodeString (TestDataRw, TestData[Index]);
>
>
>
>      //
>
>      // For each test data, test the StrLwr functionality.
>
>      //
>
> -    UnicodeCollation->StrLwr (UnicodeCollation, TestData[Index]);
>
> +    UnicodeCollation->StrLwr (UnicodeCollation, TestDataRw);
>
>
>
> -    if (CheckStrLwr (TestDataSav, TestData[Index])) {
>
> +    if (CheckStrLwr (TestDataSav, TestDataRw)) {
>
>        AssertionType = EFI_TEST_ASSERTION_PASSED;
>
>      } else {
>
>        AssertionType = EFI_TEST_ASSERTION_FAILED;
>
> @@ -388,15 +390,15 @@ BBTestStrLwrFunctionAutoTest (
>                     __FILE__,
>
>                     (UINTN)__LINE__,
>
>                     TestDataSav,
>
> -                   TestData[Index]
>
> +                   TestDataRw
>
>                     );
>
>
>
>
>
> -    CopyUnicodeString (TestDataSav, TestData[Index]);
>
> -    UnicodeCollation->StrUpr (UnicodeCollation, TestData[Index]);
>
> -    UnicodeCollation->StrLwr (UnicodeCollation, TestData[Index]);
>
> +    CopyUnicodeString (TestDataSav, TestDataRw);
>
> +    UnicodeCollation->StrUpr (UnicodeCollation, TestDataRw);
>
> +    UnicodeCollation->StrLwr (UnicodeCollation, TestDataRw);
>
>
>
> -    if (CheckStrEql (TestDataSav, TestData[Index])) {
>
> +    if (CheckStrEql (TestDataSav, TestDataRw)) {
>
>        AssertionType = EFI_TEST_ASSERTION_PASSED;
>
>      } else {
>
>        AssertionType = EFI_TEST_ASSERTION_FAILED;
>
> @@ -411,7 +413,7 @@ BBTestStrLwrFunctionAutoTest (
>                     __FILE__,
>
>                     (UINTN)__LINE__,
>
>                     TestDataSav,
>
> -                   TestData[Index]
>
> +                   TestDataRw
>
>                     );
>
>    };
>
>
>
> @@ -456,6 +458,7 @@ BBTestStrUprFunctionAutoTest (
>                                          };
>
>
>
>    CHAR16                               TestDataSav[MAX_SIZE_OF_STRING + 1];
>
> +  CHAR16                               TestDataRw[MAX_SIZE_OF_STRING + 1];
>
>
>
>
>
>
>
> @@ -488,13 +491,14 @@ BBTestStrUprFunctionAutoTest (
>      // Backup current test data
>
>      //
>
>      CopyUnicodeString (TestDataSav, TestData[Index]);
>
> +    CopyUnicodeString (TestDataRw, TestData[Index]);
>
>
>
>      //
>
>      // For each test data, test the StrUpr functionality.
>
>      //
>
> -    UnicodeCollation->StrUpr (UnicodeCollation, TestData[Index]);
>
> +    UnicodeCollation->StrUpr (UnicodeCollation, TestDataRw);
>
>
>
> -    if (CheckStrUpr (TestDataSav, TestData[Index])) {
>
> +    if (CheckStrUpr (TestDataSav, TestDataRw)) {
>
>        AssertionType = EFI_TEST_ASSERTION_PASSED;
>
>      } else {
>
>        AssertionType = EFI_TEST_ASSERTION_FAILED;
>
> @@ -509,14 +513,14 @@ BBTestStrUprFunctionAutoTest (
>                     __FILE__,
>
>                     (UINTN)__LINE__,
>
>                     TestDataSav,
>
> -                   TestData[Index]
>
> +                   TestDataRw
>
>                     );
>
>
>
> -    CopyUnicodeString (TestDataSav, TestData[Index]);
>
> -    UnicodeCollation->StrLwr (UnicodeCollation, TestData[Index]);
>
> -    UnicodeCollation->StrUpr (UnicodeCollation, TestData[Index]);
>
> +    CopyUnicodeString (TestDataSav, TestDataRw);
>
> +    UnicodeCollation->StrLwr (UnicodeCollation, TestDataRw);
>
> +    UnicodeCollation->StrUpr (UnicodeCollation, TestDataRw);
>
>
>
> -    if (CheckStrEql (TestDataSav, TestData[Index])) {
>
> +    if (CheckStrEql (TestDataSav, TestDataRw)) {
>
>        AssertionType = EFI_TEST_ASSERTION_PASSED;
>
>      } else {
>
>        AssertionType = EFI_TEST_ASSERTION_FAILED;
>
> @@ -531,7 +535,7 @@ BBTestStrUprFunctionAutoTest (
>                     __FILE__,
>
>                     (UINTN)__LINE__,
>
>                     TestDataSav,
>
> -                   TestData[Index]
>
> +                   TestDataRw
>
>                     );
>
>    };
>
>
>
> --
> 2.25.1
>
>
>
>
>
>
>


Re: [PATCH 1/1] UefiCpuPkg/MpInitLib: Reduce reset vector memory pressure

Lendacky, Thomas
 

On 9/23/20 8:58 AM, Tom Lendacky wrote:
On 9/23/20 3:31 AM, Laszlo Ersek wrote:
On 09/23/20 10:14, Laszlo Ersek wrote:

(3) Even better... can you modify GetApResetVectorSize() to take
&CpuMpData rather than &CpuMpData->AddressMap, and then check
CpuMpData->SevEsIsEnabled?

Hmmm, wait, that's not really simple, as we call GetApResetVectorSize()
from MpInitLibInitialize() too, way before we set
CpuMpData->SevEsIsEnabled from the PCD.

So I guess we should pass a dedicated BOOLEAN parameter to
GetApResetVectorSize(), called "SevEsIsEnabled". At the call site in
MpInitLibInitialize(), we should pass in the PCD's value. At the call
site in AllocateResetVector(), we should pass in
CpuMpData->SevEsIsEnabled.

The reason I'm suggesting (3) is that I don't feel comfortable with
checking dynamic PCDs outside of entry point functions / initialization
functions.
You know what, never mind (3) -- I've just realized that
PcdCpuMaxLogicalProcessorNumber may be a dynamic PCD too. It might
require a lot of work to restrict all dynamic PCD accesses to the init
function only, and I couldn't necessarily justify all that work at the
moment (for myself or for anyone else).

So please consider (1), (2) and (4).
Yup, will do.
Re. #4, the code uses the APIC ID to calculate the stack start, so the
calculation, now in GetApResetVectorSize(), uses the full value of
PcdGet32(PcdCpuMaxLogicalProcessorNumber) instead of subtracting one from
it. I can always update MpInitLibSevEsAPReet() to subtract one from the
APIC ID value returned if we want to eliminate the extra 64 bytes.

Thanks,
Tom


Thanks,
Tom


Thanks!
Laszlo


Re: [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.

Bob Feng
 

Yes. this bug only happens on case insensitive file systems.

 

Thanks,

Bob

 

From: Andrew Fish <afish@...>
Sent: Wednesday, September 23, 2020 11:31 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Feng, Bob C <bob.c.feng@...>
Cc: Liang, MingyueX <mingyuex.liang@...>; Liming Gao <gaoliming@...>; Chen, Christine <yuwei.chen@...>
Subject: Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.

 

Bob,

 

Sorry I was confused by the commit comment `inconsistent path case`. So this was only a bug on case insensitive file systems? 

 

I’m paranoid as macOS support case and case insensitive file systems and we have had a lot of strange bugs in the past. 

 

Thanks,

 

Andrew Fish



On Sep 23, 2020, at 7:59 AM, Bob Feng <bob.c.feng@...> wrote:

 

Yes. we did test on Windows and Linux.

From the 
https://docs.python.org/3/library/os.path.html
os.path.normcase(path)
   Normalize the case of a pathname. On Windows, convert all characters in the pathname to lowercase, and also convert forward slashes to  backward slashes. On other operating systems, return the path unchanged.

Thanks,
Bob
-----Original Message-----
From: Andrew Fish <
afish@...> 
Sent: Wednesday, September 23, 2020 10:24 PM
To: 
devel@edk2.groups.io; Feng, Bob C <bob.c.feng@...>
Cc: Liang, MingyueX <
mingyuex.liang@...>; Liming Gao <gaoliming@...>; Chen, Christine <yuwei.chen@...>
Subject: Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.

Does this work on case sensitive file systems?

On Sep 23, 2020, at 3:58 AM, Bob Feng <bob.c.feng@...> wrote:

From: Mingyue Liang <mingyuex.liang@...>

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

Currently, When doing the Incremental build, the directory macros 
extended to absolute path in output Makefile, which is inconsistent 
with the output of Clean build.

When we do macro replacement, we can't replace macro due to 
inconsistent path case, which results in inconsistent display of 
incremental build and clean build in makefile.Therefore, the path is 
converted to achieve the correct macro replacement.

Signed-off-by: Mingyue Liang <mingyuex.liang@...>
Cc: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <gaoliming@...>
Cc: Yuwei Chen <yuwei.chen@...>
---
BaseTools/Source/Python/AutoGen/GenMake.py | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 0314d0ea34..b04d3f5436 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -786,8 +786,10 @@ cleanlib:

   def ReplaceMacro(self, str):
       for Macro in self.MacroList:
-            if self._AutoGenObject.Macros[Macro] and self._AutoGenObject.Macros[Macro] in str:
-                str = str.replace(self._AutoGenObject.Macros[Macro], '$(' + Macro + ')')
+            if self._AutoGenObject.Macros[Macro] and os.path.normcase(self._AutoGenObject.Macros[Macro]) in os.path.normcase(str):
+                replace_dir = str[os.path.normcase(str).index(os.path.normcase(self._AutoGenObject.Macros[Macro])): os.path.normcase(str).index(
+                    os.path.normcase(self._AutoGenObject.Macros[Macro])) + len(self._AutoGenObject.Macros[Macro])]
+                str = str.replace(replace_dir, '$(' + Macro + ')')
       return str

   def CommandExceedLimit(self):
--
2.28.0.windows.1







 


Re: [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.

Andrew Fish
 

Bob,

Sorry I was confused by the commit comment `inconsistent path case`. So this was only a bug on case insensitive file systems? 

I’m paranoid as macOS support case and case insensitive file systems and we have had a lot of strange bugs in the past. 

Thanks,

Andrew Fish

On Sep 23, 2020, at 7:59 AM, Bob Feng <bob.c.feng@...> wrote:

Yes. we did test on Windows and Linux.

From the https://docs.python.org/3/library/os.path.html
os.path.normcase(path)
   Normalize the case of a pathname. On Windows, convert all characters in the pathname to lowercase, and also convert forward slashes to  backward slashes. On other operating systems, return the path unchanged.

Thanks,
Bob
-----Original Message-----
From: Andrew Fish <afish@...> 
Sent: Wednesday, September 23, 2020 10:24 PM
To: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@...>
Cc: Liang, MingyueX <mingyuex.liang@...>; Liming Gao <gaoliming@...>; Chen, Christine <yuwei.chen@...>
Subject: Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.

Does this work on case sensitive file systems?
On Sep 23, 2020, at 3:58 AM, Bob Feng <bob.c.feng@...> wrote:

From: Mingyue Liang <mingyuex.liang@...>

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

Currently, When doing the Incremental build, the directory macros 
extended to absolute path in output Makefile, which is inconsistent 
with the output of Clean build.

When we do macro replacement, we can't replace macro due to 
inconsistent path case, which results in inconsistent display of 
incremental build and clean build in makefile.Therefore, the path is 
converted to achieve the correct macro replacement.

Signed-off-by: Mingyue Liang <mingyuex.liang@...>
Cc: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <gaoliming@...>
Cc: Yuwei Chen <yuwei.chen@...>
---
BaseTools/Source/Python/AutoGen/GenMake.py | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 0314d0ea34..b04d3f5436 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -786,8 +786,10 @@ cleanlib:

   def ReplaceMacro(self, str):
       for Macro in self.MacroList:
-            if self._AutoGenObject.Macros[Macro] and self._AutoGenObject.Macros[Macro] in str:
-                str = str.replace(self._AutoGenObject.Macros[Macro], '$(' + Macro + ')')
+            if self._AutoGenObject.Macros[Macro] and os.path.normcase(self._AutoGenObject.Macros[Macro]) in os.path.normcase(str):
+                replace_dir = str[os.path.normcase(str).index(os.path.normcase(self._AutoGenObject.Macros[Macro])): os.path.normcase(str).index(
+                    os.path.normcase(self._AutoGenObject.Macros[Macro])) + len(self._AutoGenObject.Macros[Macro])]
+                str = str.replace(replace_dir, '$(' + Macro + ')')
       return str

   def CommandExceedLimit(self):
--
2.28.0.windows.1










Re: [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.

Bob Feng
 

Yes. we did test on Windows and Linux.

From the https://docs.python.org/3/library/os.path.html
os.path.normcase(path)
Normalize the case of a pathname. On Windows, convert all characters in the pathname to lowercase, and also convert forward slashes to backward slashes. On other operating systems, return the path unchanged.

Thanks,
Bob

-----Original Message-----
From: Andrew Fish <afish@apple.com>
Sent: Wednesday, September 23, 2020 10:24 PM
To: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>
Cc: Liang, MingyueX <mingyuex.liang@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>
Subject: Re: [edk2-devel] [PATCH] BaseTools: Normalize case of pathname when evaluating Macros.

Does this work on case sensitive file systems?
On Sep 23, 2020, at 3:58 AM, Bob Feng <bob.c.feng@intel.com> wrote:

From: Mingyue Liang <mingyuex.liang@intel.com>

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

Currently, When doing the Incremental build, the directory macros
extended to absolute path in output Makefile, which is inconsistent
with the output of Clean build.

When we do macro replacement, we can't replace macro due to
inconsistent path case, which results in inconsistent display of
incremental build and clean build in makefile.Therefore, the path is
converted to achieve the correct macro replacement.

Signed-off-by: Mingyue Liang <mingyuex.liang@intel.com>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Yuwei Chen <yuwei.chen@intel.com>
---
BaseTools/Source/Python/AutoGen/GenMake.py | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 0314d0ea34..b04d3f5436 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -786,8 +786,10 @@ cleanlib:

def ReplaceMacro(self, str):
for Macro in self.MacroList:
- if self._AutoGenObject.Macros[Macro] and self._AutoGenObject.Macros[Macro] in str:
- str = str.replace(self._AutoGenObject.Macros[Macro], '$(' + Macro + ')')
+ if self._AutoGenObject.Macros[Macro] and os.path.normcase(self._AutoGenObject.Macros[Macro]) in os.path.normcase(str):
+ replace_dir = str[os.path.normcase(str).index(os.path.normcase(self._AutoGenObject.Macros[Macro])): os.path.normcase(str).index(
+ os.path.normcase(self._AutoGenObject.Macros[Macro])) + len(self._AutoGenObject.Macros[Macro])]
+ str = str.replace(replace_dir, '$(' + Macro + ')')
return str

def CommandExceedLimit(self):
--
2.28.0.windows.1





13081 - 13100 of 78581