Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Race condition between APHandler's release of Busy spinlock and user-triggered SmmStartupThisAP's
Agree with Ray, no need to call AcquireSpinLockOrFail anymore. I think final
code like change like below:
- if (Token == NULL) {
-AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
- } else {
-if (!AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy)) {
- DEBUG((DEBUG_ERROR, "Can't acquire mSmmMpSyncData->CpuData[%d].Busy\n",
CpuIndex));
- return EFI_NOT_READY;
-}
+ AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
+ if (Token != NULL) {
Thanks,
Eric
> -Original Message-
> From: Ni, Ray
> Sent: Wednesday, September 4, 2019 12:56 AM
> To: Nikodem, Damian ; devel@edk2.groups.io
> Cc: Dong, Eric ; You, Benjamin
> ; Laszlo Ersek ; Rusocki,
> Krzysztof
> Subject: RE: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Race condition between
> APHandler's release of Busy spinlock and user-triggered SmmStartupThisAP's
>
> 1. can we directly call AcquireSpinLock()? *OrFail() can be removed IMO.
> 2. It's a patch to change the behavior of SmmStartupThisAP(). So that to
> reduce the potential bugs in caller's code. Patch title is a bit mis-leading.
>
> > -Original Message-
> > From: Nikodem, Damian
> > Sent: Tuesday, September 3, 2019 7:58 AM
> > To: devel@edk2.groups.io
> > Cc: Nikodem, Damian ; Dong, Eric
> > ; Ni, Ray ; You, Benjamin
> > ; Laszlo Ersek ; Rusocki,
> > Krzysztof
> > Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Race condition between
> > APHandler's release of Busy spinlock and user- triggered
> > SmmStartupThisAP's
> >
> > Race condition between APHandler's release of Busy spinlock and
> > user-triggered SmmStartupThisAP's acquisition attempt of the Busy spinlock
> (non-blocking mode).
> >
> > UserProc is the user's procedure to execute on an AP.
> > UserProcCompletion is the user procedure's completion spinlock.
> > All other variables are from EDK2.
> >
> > BSP AP
> >
> ===
> ==
> >
>APHandler ()
> >
> WaitForSemaphore (Run)
> >
> ><< initial
> state >>
> >
> > AcquireSpinLock (UserProcCompletion)
> > SmmStartupThisAp (Procedure)
> > AcquireSpinLockOrFail (Busy)
> > ReleaseSemaphore (Run)
> >
> UserProc ()
> > DoStuff()DoSomeOtherStuff ()
> >
> > AcquireSpinLockOrFail (UserProcCompletion) AcquireSpinLockOrFail
> > (UserProcCompletion)
> >
> > ^^ waiting in a loop for user procedure's
> >completion == these fail
> > ReleaseSpinLock (UserProcCompletion) AcquireSpinLockOrFail
> (UserProcCompletion)
> >
> > ^^ this succeeds
> >
> > ReleaseSpinLock (UserProcCompletion)
> >
> > << return control to the caller and
> >reenter the flow >>>
> >
> > AcquireSpinLock (UserProcCompletion)
> > SmmStartupThisAp (Procedure)
> > AcquireSpinLockOrFail (Busy)
> > ^^ this wins the race with AP's
> > ReleaseSpinLock and fails;
> >
> ReleaseSpinLock (Busy)
> > return EFI_INVALID_PARAMETER;
> >
> > To remedy, if AcquireSpinLockOrFail (of the Busy spinlock) fails,
> > perform regular AcquireSpinLock -- this eliminates the race condition.
> >
> > Signed-off-by: Damian Nikodem
> > Cc: Eric Dong
> > Cc: Ray Ni
> > Cc: Benjamin You
> > Cc: Laszlo Ersek
> > Cc: Krzysztof Rusocki
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > index d8d2b6f444..206e196a76 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> > @@ -1239,8 +1239,16 @@ InternalSmmStartupThisAp (
> > AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
> >} else {
> > if (!AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy))
> {
> > - DEBUG((DEBUG_ERROR, "Can't acquire mSmmMpSyncData-
> >CpuData[%d].Busy\n", CpuIndex));
> > - return EFI_NOT_READY;
> > + DEBUG ((DEBUG_INFO, "BSP[%d] finds AP[%d] busy at proc 0x%llX
> (param 0x%llX), ",
> > +mSmmMpSyncData->BspIndex,
> > +CpuIndex,
> > +*mSmmMpSyncData->CpuData[CpuIndex].Procedure,
> > +(VOID*)mSmmMpSyncData->CpuData[CpuIndex].Parameter));
> > + DEBUG ((DEBUG_INFO, "new proc 0x%llX (param 0x%llX). Waiting for
> the previous AP procedure to complete...\n",
> > +Procedure,
> > +ProcArguments));
> > +
> > + AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
> > }
> >
> > *Token = (MM_COMPLETION) CreateToken ();
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46744): https://edk2.groups.io/g/devel/message/46744
Mute This Topic: https://groups.io/mt/33128825/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH 1/1] Platform: Intel: Update licenses to SPDX and update clock settings
From: "Tien Hock, Loh"
Update all license to SPDX. Also update UART clock to be calculated instead
of hardcoded, removed some unused packages, and updated maintainers.
Signed-off-by: "Tien Hock, Loh"
Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Ard Biesheuvel
Cc: Leif Lindholm
Cc: Michael D Kinney
---
Platform/Intel/Stratix10/Stratix10SoCPkg.dec
| 3 +-
Platform/Intel/Stratix10/Stratix10SoCPkg.dsc
| 15 ++-
Platform/Intel/Stratix10/Stratix10SoCPkg.fdf
| 1 -
Platform/Intel/Stratix10/Drivers/IntelPlatformDxe/IntelPlatformDxe.inf
| 22 ++--
Platform/Intel/Stratix10/Library/IntelPlatformLib/IntelPlatformLib.inf
| 94 +++---
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/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/PlatformHookLib/PlatformHookLib.c
| 43 +++
Platform/Intel/Stratix10/Library/S10ClockManager/S10ClockManager.c
| 133
Maintainers.txt
| 5 +
Platform/Intel/Stratix10/Library/IntelPlatformLib/AArch64/ArmPlatformHelper.S
| 8 +-
15 files changed, 391 insertions(+), 117 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 17d0c5baadc6..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
@@ -103,7 +105,6 @@ [PcdsFixedAtBuild.common]
# DEBUG_GCD 0x0010 // Global Coherency Database changes
# DEBUG_CACHE 0x0020 // Memory range cachability changes
# DEBUG_ERROR 0x8000 // Error
-# gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x803010CF
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x800F
gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x00
@@ -227,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
@@ -251,7 +253,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 +415,6 @@ [Components.common]
#
MdeModulePkg/Core/Dxe/DxeMain.inf {
- #PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
}
@@ -475,7 +475,6 @@ [Components.common]
NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
-#
Re: [edk2-devel] [edk2-platforms] [PATCH] ClevoOpenBoardPkg: Fix GCC5 build issue
Reviewed-by: Michael Kubacki
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Agyeman,
> Prince
> Sent: Tuesday, September 3, 2019 11:18 AM
> To: devel@edk2.groups.io
> Cc: Agyeman, Prince ; Sinha, Ankit
> ; Desimone, Nathaniel L
> ; Kubacki, Michael A
> ; Chiu, Chasel ; Bi,
> Dandan
> Subject: [edk2-devel] [edk2-platforms] [PATCH] ClevoOpenBoardPkg: Fix GCC5
> build issue
>
> From: Agyeman
>
> Fixed GPIO table missing curly brackets
>
> Cc: Ankit Sinha
> Cc: Nate DeSimone
> Cc: Michael Kubacki
> Cc: Chasel Chiu
> CC: Dandan Bi
>
> Signed-off-by: Prince Agyeman
> ---
> .../N1xxWU/Library/BoardInitLib/N1xxWUGpioTable.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxWUG
> pioTable.c
> b/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxWUG
> pioTable.c
> index c99b83753f..aa2c770729 100644
> ---
> a/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxWUG
> pioTable.c
> +++
> b/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxWUG
> pioTable.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
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46742): https://edk2.groups.io/g/devel/message/46742
Mute This Topic: https://groups.io/mt/33129653/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] BaseTools/GenFw AARCH64: fix up GOT based relative relocations
We take great care to avoid GOT based relocations in EDK2 executables,
primarily because they are pointless - we don't care about things like
the CoW footprint or relocations that target read-only sections, and so
GOT entries only bloat the binary.
However, in some cases (e.g., when building the relocatable PrePi SEC
module in ArmVirtPkg with the CLANG38 toolchain), we may end up with
some GOT based relocations nonetheless, which break the build since
GenFw does not know how to deal with them.
The relocations emitted in this case are ADRP/LDR instruction pairs
that are annotated as GOT based, which means that it is the linker's
job to emit the GOT entry and tag it with an appropriate dynamic
relocation that ensures that the correct absolute value is stored into
the GOT entry when the executable is loaded. This dynamic relocation
not visible to GenFw, and so populating the PE/COFF relocation section
for these entries is non-trivial.
Since each ADRP/LDR pair refers to a single symbol that is local to the
binary (given that shared libraries are not supported), we can actually
convert the ADRP/LDR pair into an ADRP/ADD pair that produces the symbol
address directly rather than loading it from memory. This leaves the
GOT entry in the binary, but since it is now unused, it is no longer
necessary to emit a PE/COFF relocation entry for it.
Signed-off-by: Ard Biesheuvel
---
BaseTools/Source/C/GenFw/Elf64Convert.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 3d6319c821e9..d574300ac4fe 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -1017,6 +1017,31 @@ WriteSections64 (
} else if (mEhdr->e_machine == EM_AARCH64) {
switch (ELF_R_TYPE(Rel->r_info)) {
+INT64 Offset;
+
+ case R_AARCH64_LD64_GOT_LO12_NC:
+//
+// Convert into an ADD instruction - see R_AARCH64_ADR_GOT_PAGE
below.
+//
+*(UINT32 *)Targ &= 0x3ff;
+*(UINT32 *)Targ |= 0x9100 | ((Sym->st_value & 0xfff) << 10);
+break;
+
+ case R_AARCH64_ADR_GOT_PAGE:
+//
+// This relocation points to the GOT entry that contains the
absolute
+// address of the symbol we are referring to. Since EDK2 only uses
+// fully linked binaries, we can avoid the indirection, and simply
+// refer to the symbol directly. This implies having to patch the
+// subsequent LDR instruction (covered by a
R_AARCH64_LD64_GOT_LO12_NC
+// relocation) into an ADD instruction - this is handled above.
+//
+Offset = (Sym->st_value - (Rel->r_offset & ~0xfff)) >> 12;
+
+*(UINT32 *)Targ &= 0x901f;
+*(UINT32 *)Targ |= ((Offset & 0x1c) << (5 - 2)) | ((Offset &
0x3) << 29);
+
+/* fall through */
case R_AARCH64_ADR_PREL_PG_HI21:
//
@@ -1037,7 +1062,6 @@ WriteSections64 (
// Attempt to convert the ADRP into an ADR instruction.
// This is only possible if the symbol is within +/- 1 MB.
//
- INT64 Offset;
// Decode the ADRP instruction
Offset = (INT32)((*(UINT32 *)Targ & 0xe0) << 8);
@@ -1212,6 +1236,8 @@ WriteRelocations64 (
case R_AARCH64_LDST32_ABS_LO12_NC:
case R_AARCH64_LDST64_ABS_LO12_NC:
case R_AARCH64_LDST128_ABS_LO12_NC:
+case R_AARCH64_ADR_GOT_PAGE:
+case R_AARCH64_LD64_GOT_LO12_NC:
//
// No fixups are required for relative relocations, provided that
// the relative offsets between sections have been preserved in
--
2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46741): https://edk2.groups.io/g/devel/message/46741
Mute This Topic: https://groups.io/mt/33134949/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 1/1] ShellPkg/Pci.c: Update supported link speed to PCI5.0
Reviewed-by: Ray Ni > -Original Message- > From: Gao, Zhichao > Sent: Sunday, September 1, 2019 8:15 AM > To: devel@edk2.groups.io > Cc: Gao, Zhichao ; Ni, Ray ; Oleksiy > ; Gao, Liming > > Subject: [PATCH v3 1/1] ShellPkg/Pci.c: Update supported link speed to PCI5.0 > > From: "Gao, Zhichao" > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1955 > > Refer to PCI express base specification Reversion 5.0, Version > 1.0, Table 7-33, Supported Link Speeds Vector bit 3 indicate > the speed 16 GT/s and bit 4 indicate the speed 32 GT/s. > Add the support to shell command 'pci ...'. > > Change the MaxLinkSpeed other values' result from 'Unknown' > to 'Reserved' to make the result align. > > Cc: Ray Ni > Cc: Oleksiy > Cc: Liming Gao > Signed-off-by: Zhichao Gao > --- > V2: > Update the copyright. > > V3: > Update the link speed to support PCI 5.0. > > Update the support > .../Library/UefiShellDebug1CommandsLib/Pci.c | 16 ++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > index ba9caa774398..3e138188cec3 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c > @@ -1,7 +1,7 @@ > /** @file >Main file for Pci shell Debug1 function. > > - Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved. > + Copyright (c) 2005 - 2019, Intel Corporation. All rights reserved. >(C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P. >(C) Copyright 2016 Hewlett Packard Enterprise Development LP >SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -4515,8 +4515,14 @@ ExplainPcieLinkCap ( > case 3: >MaxLinkSpeed = L"8.0 GT/s"; >break; > +case 4: > + MaxLinkSpeed = L"16.0 GT/s"; > + break; > +case 5: > + MaxLinkSpeed = L"32.0 GT/s"; > + break; > default: > - MaxLinkSpeed = L"Unknown"; > + MaxLinkSpeed = L"Reserved"; >break; >} >ShellPrintEx (-1, -1, > @@ -4672,6 +4678,12 @@ ExplainPcieLinkStatus ( > case 3: >CurLinkSpeed = L"8.0 GT/s"; >break; > +case 4: > + CurLinkSpeed = L"16.0 GT/s"; > + break; > +case 5: > + CurLinkSpeed = L"32.0 GT/s"; > + break; > default: >CurLinkSpeed = L"Reserved"; >break; > -- > 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46740): https://edk2.groups.io/g/devel/message/46740 Mute This Topic: https://groups.io/mt/33101378/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs for Admin commands
> -Original Message-
> From: Wu, Hao A
> Sent: Wednesday, September 04, 2019 11:39 AM
> To: devel@edk2.groups.io; Tyler Erickson
> Cc: Wu, Hao A; Ni, Ray
> Subject: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs
> for Admin commands
>
> Repost the mail to the list.
>
> Best Regards,
> Hao Wu
>
> -Original Message-
> From: Tyler Erickson [mailto:tyler.j.erick...@seagate.com]
> Sent: Tuesday, September 03, 2019 9:55 PM
> To: edk2-de...@lists.01.org
> Cc: Wu, Hao A; Ni, Ray
> Subject: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs
> for Admin commands
>
> Cc: Hao A Wu
> Cc: Ray Ni
> Signed-off-by: Tyler Erickson
> ---
> MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> index 8e721379466a..78a3c663ded4 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> @@ -561,7 +561,7 @@ NvmExpressPassThru (
>Sq = Private->SqBuffer[QueueId] + Private->SqTdbl[QueueId].Sqt;
>Cq = Private->CqBuffer[QueueId] + Private->CqHdbl[QueueId].Cqh;
>
> - if (Packet->NvmeCmd->Nsid != NamespaceId) {
> + if (Packet->QueueType != NVME_ADMIN_QUEUE && Packet->NvmeCmd-
> >Nsid != NamespaceId) {
Hello,
Per my understanding to the codes, I think the:
* Input parameter 'NamespaceId' for the PassThru() service
* The 'Nsid' field of the EFI_NVM_EXPRESS_COMMAND
are of the same meaning.
Do you think setting these 2 values identical when calling the PassThru()
service can resolve the issue you met?
Best Regards,
Hao Wu
> return EFI_INVALID_PARAMETER;
>}
>
> --
> 2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46739): https://edk2.groups.io/g/devel/message/46739
Mute This Topic: https://groups.io/mt/33134758/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs for Admin commands
Repost the mail to the list.
Best Regards,
Hao Wu
-Original Message-
From: Tyler Erickson [mailto:tyler.j.erick...@seagate.com]
Sent: Tuesday, September 03, 2019 9:55 PM
To: edk2-de...@lists.01.org
Cc: Wu, Hao A; Ni, Ray
Subject: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs for Admin
commands
Cc: Hao A Wu
Cc: Ray Ni
Signed-off-by: Tyler Erickson
---
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index 8e721379466a..78a3c663ded4 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -561,7 +561,7 @@ NvmExpressPassThru (
Sq = Private->SqBuffer[QueueId] + Private->SqTdbl[QueueId].Sqt;
Cq = Private->CqBuffer[QueueId] + Private->CqHdbl[QueueId].Cqh;
- if (Packet->NvmeCmd->Nsid != NamespaceId) {
+ if (Packet->QueueType != NVME_ADMIN_QUEUE && Packet->NvmeCmd->Nsid !=
NamespaceId) {
return EFI_INVALID_PARAMETER;
}
--
2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46738): https://edk2.groups.io/g/devel/message/46738
Mute This Topic: https://groups.io/mt/33134758/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] Cancelled Event: TianoCore Design / Bug Triage - EMEA - Wednesday, 4 September 2019 #cal-cancelled
Canceling, as there were no submissions. Also, OSFC is this week. --S On Tue, Sep 3, 2019 at 8:36 PM devel@edk2.groups.io Calendar wrote: > > Cancelled: TianoCore Design / Bug Triage - EMEA > > This event has been cancelled. > > When: > Wednesday, 4 September 2019 > 8:00am to 9:00am > (UTC-07:00) America/Los Angeles > > Where: > https://zoom.us/j/695893389 > > Organizer: Stephano Cetola stephano.cet...@linux.intel.com > > Description: > > Join Zoom Meeting > > https://zoom.us/j/695893389 > > > > One tap mobile > > +17207072699,,695893389# US > > +16465588656,,695893389# US (New York) > > > > Dial by your location > > +1 720 707 2699 US > > +1 646 558 8656 US (New York) > > Meeting ID: 695 893 389 > > Find your local number: https://zoom.us/u/abOtdJckxL > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46736): https://edk2.groups.io/g/devel/message/46736 Mute This Topic: https://groups.io/mt/33134746/21656 Mute #cal-cancelled: https://groups.io/mk?hashtag=cal-cancelled=3846945 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v1 0/1] Allow any NSID value for NVMe admin commands
Repost the mail to the list. Best Regards, Hao Wu -Original Message- From: Tyler Erickson [mailto:tyler.j.erick...@seagate.com] Sent: Tuesday, September 03, 2019 9:55 PM To: edk2-de...@lists.01.org Cc: Wu, Hao A; Ni, Ray Subject: [PATCH v1 0/1] Allow any NSID value for NVMe admin commands This patch is to allow issuing admin commands to NVMe devices with any value for the NSID. Use cases for this are for DST commands, format commands, identify commands, and get log page commands. Changing the NSID in these admin commands changes behavior on what data is returned (controller wide versus individual namespace for logs), or changes what is formatted (a particular namespace versus all namespaces on the controller), or changes what DST is testing (controller only, single namespace, or controller and all namespaces). Changes can be found here: https://github.com/vonericsen/edk2/tree/nvm_passthru_admin_any_NSID Cc: Hao A Wu Cc: Ray Ni Tyler Erickson (1): MdeModulePkg/NvmExpressDxe: Allow other NSIDs for Admin commands MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46737): https://edk2.groups.io/g/devel/message/46737 Mute This Topic: https://groups.io/mt/33134753/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] Cancelled Event: TianoCore Design / Bug Triage - EMEA - Wednesday, 4 September 2019 #cal-cancelled
BEGIN:VCALENDAR VERSION:2.0 PRODID:-//Groups.io Inc//Groups.io Calendar//EN METHOD:CANCEL CALSCALE:GREGORIAN BEGIN:VEVENT STATUS:CANCELLED UID:calendar.15...@groups.io DTSTAMP:20190904T033646Z ORGANIZER;CN=Stephano Cetola:mailto:stephano.cet...@linux.intel.com DTSTART;TZID=America/Los_Angeles:20190904T08 DTEND;TZID=America/Los_Angeles:20190904T09 SUMMARY:TianoCore Design / Bug Triage - EMEA DESCRIPTION:Join Zoom Meeting\n\nhttps://zoom.us/j/695893389\n\nOne tap mobile\n\n+17207072699,,695893389# US\n\n+16465588656,,695893389# US (New York)\n\nDial by your location\n\n+1 720 707 2699 US\n\n+1 646 558 8656 US (New York)\n\nMeeting ID: 695 893 389\n\nFind your local number: https://zoom.us/u/abOtdJckxL LOCATION:https://zoom.us/j/695893389 RECURRENCE-ID;TZID=America/Los_Angeles:20190904T08 SEQUENCE:0 END:VEVENT END:VCALENDAR invite.ics Description: application/ics
Re: [edk2-devel] [edk2-test] [PATCH 1/1] uefi-sct/SctPkg: fix typo 'Remained test cases'
Hi Heinrich Schuchardt,
According to
https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format
The statement "Contributed-under:..." need be removed.
With that: Reviewed-by: Eric Jin
If no object, I will help to push with this change by end of this week.
-Original Message-
From: Heinrich Schuchardt
Sent: Tuesday, September 3, 2019 5:53 PM
To: devel@edk2.groups.io
Cc: Jin, Eric ; Supreeth Venkatesh
; Stephano Cetola
; Heinrich Schuchardt
Subject: [edk2-test] [PATCH 1/1] uefi-sct/SctPkg: fix typo 'Remained test cases'
%s/Remained/Remaining/g
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heinrich Schuchardt
---
.../SctPkg/TestInfrastructure/SCT/Framework/Execute/Execute.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/Execute/Execute.c
b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/Execute/Execute.c
index 9e7f672e..6fd36914 100644
--- a/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/Execute/Execute.c
+++ b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/Execute/Execute.c
@@ -608,10 +608,10 @@ Routine Description:
//
while (TRUE) {
//
-// Calculate the number of remained cases
+// Calculate the number of remaining test cases
//
GetTestCaseRemainNum();
-SctPrint(L" Remained test cases: %d\n", Remain);
+SctPrint(L" Remaining test cases: %d\n", Remain);
//
// Send assertion to remotion computer in passive mode to inform case
begin.
--
2.23.0.rc1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46734): https://edk2.groups.io/g/devel/message/46734
Mute This Topic: https://groups.io/mt/33124786/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2] [Patch 32/33] BaseTools: ECC tool Python3 adaption
Hi Leif,
I have no Debian environment. On Debian, can python3 work with antlr3? 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
Cc: devel@edk2.groups.io; Gao, Liming
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
>
> 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
> > Cc: 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
> > ++
> >
Re: [edk2-devel] [PATCH] ArmVirtPkg/PlatformBootManagerLib: unload image on EFI_SECURITY_VIOLATION
Reviewed-by: Dandan Bi
Thanks,
Dandan
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, September 4, 2019 12:38 AM
> To: edk2-devel-groups-io
> Cc: Ard Biesheuvel ; Bi, Dandan
> ; Leif Lindholm
> Subject: [PATCH] ArmVirtPkg/PlatformBootManagerLib: unload image on
> EFI_SECURITY_VIOLATION
>
> The LoadImage() boot service is a bit unusual in that it allocates resources
> in a
> particular failure case; namely, it produces a valid "ImageHandle" when it
> returns EFI_SECURITY_VIOLATION. This is supposed to happen e.g. when
> Secure Boot verification fails for the image, but the platform policy for the
> particular image origin (such as "fixed media" or "removable media") is
> DEFER_EXECUTE_ON_SECURITY_VIOLATION. The return code allows
> platform logic to selectively override the verification failure, and launch
> the
> image nonetheless.
>
> ArmVirtPkg/PlatformBootManagerLib does not override
> EFI_SECURITY_VIOLATION for the kernel image loaded from fw_cfg -- any
> LoadImage() error is considered fatal. When we simply treat
> EFI_SECURITY_VIOLATION like any other LoadImage() error, we leak the
> resources associated with "KernelImageHandle". From a resource usage
> perspective, EFI_SECURITY_VIOLATION must be considered "success", and
> rolled back.
>
> Implement this rollback, without breaking the proper "nesting" of error
> handling jumps and labels.
>
> Cc: Ard Biesheuvel
> Cc: Dandan Bi
> Cc: Leif Lindholm
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1992
> Fixes: 23d04b58e27b382bbd3f9b16ba9adb1cb203dad5
> Signed-off-by: Laszlo Ersek
> ---
>
> Notes:
> Repo: https://github.com/lersek/edk2.git
> Branch: ldimg_armvirt_bz1992
>
> ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
> b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
> index 3cc83e3b7b95..d3851fd75fa5 100644
> --- a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
> +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
> @@ -968,53 +968,60 @@ TryRunningQemuKernel (
>
>//
>// Create a device path for the kernel image to be loaded from that will
> call
>// back into our file system.
>//
>KernelDevicePath = FileDevicePath (FileSystemHandle, KernelBlob->Name);
>if (KernelDevicePath == NULL) {
> DEBUG ((EFI_D_ERROR, "%a: failed to allocate kernel device path\n",
>__FUNCTION__));
> Status = EFI_OUT_OF_RESOURCES;
> goto UninstallProtocols;
>}
>
>//
>// Load the image. This should call back into our file system.
>//
>Status = gBS->LoadImage (
>FALSE, // BootPolicy: exact match required
>gImageHandle, // ParentImageHandle
>KernelDevicePath,
>NULL, // SourceBuffer
>0, // SourceSize
>
>);
>if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "%a: LoadImage(): %r\n", __FUNCTION__,
> Status));
> -goto FreeKernelDevicePath;
> +if (Status != EFI_SECURITY_VIOLATION) {
> + goto FreeKernelDevicePath;
> +}
> +//
> +// From the resource allocation perspective, EFI_SECURITY_VIOLATION
> means
> +// "success", so we must roll back the image loading.
> +//
> +goto UnloadKernelImage;
>}
>
>//
>// Construct the kernel command line.
>//
>Status = gBS->OpenProtocol (
>KernelImageHandle,
>,
>(VOID **),
>gImageHandle, // AgentHandle
>NULL, // ControllerHandle
>EFI_OPEN_PROTOCOL_GET_PROTOCOL
>);
>ASSERT_EFI_ERROR (Status);
>
>if (CommandLineBlob->Data == NULL) {
> KernelLoadedImage->LoadOptionsSize = 0;
>} else {
> //
> // Verify NUL-termination of the command line.
> //
> if (CommandLineBlob->Data[CommandLineBlob->Size - 1] != '\0') {
>DEBUG ((EFI_D_ERROR, "%a: kernel command line is not NUL-
> terminated\n",
> __FUNCTION__));
>Status = EFI_PROTOCOL_ERROR;
>goto UnloadKernelImage;
> --
> 2.19.1.3.g30247aa5d201
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46732): https://edk2.groups.io/g/devel/message/46732
Mute This Topic: https://groups.io/mt/33128626/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms] [PATCH] ClevoOpenBoardPkg: Fix GCC5 build issue
> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Agyeman, Prince
> Sent: Wednesday, September 4, 2019 2:18 AM
> To: devel@edk2.groups.io
> Cc: Agyeman, Prince ; Sinha, Ankit
> ; Desimone, Nathaniel L
> ; Kubacki, Michael A
> ; Chiu, Chasel ; Bi,
> Dandan
> Subject: [edk2-devel] [edk2-platforms] [PATCH] ClevoOpenBoardPkg: Fix
> GCC5 build issue
>
> From: Agyeman
>
> Fixed GPIO table missing curly brackets
>
> Cc: Ankit Sinha
> Cc: Nate DeSimone
> Cc: Michael Kubacki
> Cc: Chasel Chiu
> CC: Dandan Bi
>
> Signed-off-by: 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..aa2c770729 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,
One minor comment, please remove one space before " GpioHostOwnAcpi " to make
the column aligned before commit the patch.
With this update, Reviewed-by: Dandan Bi
Thanks,
Dandan
> 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
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46731): https://edk2.groups.io/g/devel/message/46731
Mute This Topic: https://groups.io/mt/33129653/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch] BaseTools: Update GenFw tool to support -e -z option together
Thanks, with the change,
Reviewed-by: Bob Feng
-Original Message-
From: Gao, Liming
Sent: Wednesday, September 4, 2019 9:21 AM
To: Feng, Bob C ; devel@edk2.groups.io
Subject: RE: [edk2-devel] [Patch] BaseTools: Update GenFw tool to support -e -z
option together
Yes. Thanks for your comments. I will update it and push the change.
Thanks
Liming
>-Original Message-
>From: Feng, Bob C
>Sent: Wednesday, September 04, 2019 9:03 AM
>To: devel@edk2.groups.io; Gao, Liming
>Subject: RE: [edk2-devel] [Patch] BaseTools: Update GenFw tool to
>support -e -z option together
>
>Liming,
>
>The patch looks good to me.
>
>A minor comment that the code looks fix the bug of the combinations of
>-e -z and -t -z, so would you please update the commit message to add "-t -z"
>
>Thanks,
>Bob
>
>-Original Message-
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Liming Gao
>Sent: Monday, September 2, 2019 9:31 AM
>To: devel@edk2.groups.io
>Cc: Feng, Bob C
>Subject: [edk2-devel] [Patch] BaseTools: Update GenFw tool to support
>-e -z option together
>
>BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1727
>-e -z option is to generate EFI image with zero debug entry.
>It can be used to check the EFI image in DEBUG build.
>
>Signed-off-by: Liming Gao
>Cc: Bob Feng
>---
> BaseTools/Source/C/GenFw/GenFw.c | 17 +++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
>diff --git a/BaseTools/Source/C/GenFw/GenFw.c
>b/BaseTools/Source/C/GenFw/GenFw.c
>index 973bae5fe4..c99782b78e 100644
>--- a/BaseTools/Source/C/GenFw/GenFw.c
>+++ b/BaseTools/Source/C/GenFw/GenFw.c
>@@ -,6 +,7 @@ Returns:
> time_t InputFileTime;
> time_t OutputFileTime;
> struct stat Stat_Buf;
>+ BOOLEAN ZeroDebugFlag;
>
> SetUtilityName (UTILITY_NAME);
>
>@@ -1158,6 +1159,7 @@ Returns:
> NegativeAddr = FALSE;
> InputFileTime = 0;
> OutputFileTime = 0;
>+ ZeroDebugFlag = FALSE;
>
> if (argc == 1) {
> Error (NULL, 0, 1001, "Missing options", "No input options."); @@
> -1197,6
>+1199,9 @@ Returns:
> goto Finish;
> }
> ModuleType = argv[1];
>+ if (mOutImageType == FW_ZERO_DEBUG_IMAGE) {
>+ZeroDebugFlag = TRUE;
>+ }
> if (mOutImageType != FW_TE_IMAGE) {
> mOutImageType = FW_EFI_IMAGE;
> }
>@@ -1220,6 +1225,9 @@ Returns:
> }
>
> if ((stricmp (argv[0], "-t") == 0) || (stricmp (argv[0],
> "--terse") == 0)) {
>+ if (mOutImageType == FW_ZERO_DEBUG_IMAGE) {
>+ZeroDebugFlag = TRUE;
>+ }
> mOutImageType = FW_TE_IMAGE;
> argc --;
> argv ++;
>@@ -1241,7 +1249,12 @@ Returns:
> }
>
> if ((stricmp (argv[0], "-z") == 0) || (stricmp (argv[0], "--zero") == 0))
> {
>- mOutImageType = FW_ZERO_DEBUG_IMAGE;
>+ if (mOutImageType == FW_DUMMY_IMAGE) {
>+mOutImageType = FW_ZERO_DEBUG_IMAGE;
>+ }
>+ if (mOutImageType == FW_TE_IMAGE || mOutImageType ==
>FW_EFI_IMAGE) {
>+ZeroDebugFlag = TRUE;
>+ }
> argc --;
> argv ++;
> continue;
>@@ -2588,7 +2601,7 @@ Returns:
> //
> // Zero Time/Data field
> //
>- ZeroDebugData (FileBuffer, FALSE);
>+ ZeroDebugData (FileBuffer, ZeroDebugFlag);
>
> if (mOutImageType == FW_TE_IMAGE) {
> if ((PeHdr->Pe32.FileHeader.NumberOfSections &~0xFF) || (Type
>&~0xFF)) {
>--
>2.13.0.windows.1
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46730): https://edk2.groups.io/g/devel/message/46730
Mute This Topic: https://groups.io/mt/33107042/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch] BaseTools: Update GenFw tool to support -e -z option together
Yes. Thanks for your comments. I will update it and push the change.
Thanks
Liming
>-Original Message-
>From: Feng, Bob C
>Sent: Wednesday, September 04, 2019 9:03 AM
>To: devel@edk2.groups.io; Gao, Liming
>Subject: RE: [edk2-devel] [Patch] BaseTools: Update GenFw tool to support -e
>-z option together
>
>Liming,
>
>The patch looks good to me.
>
>A minor comment that the code looks fix the bug of the combinations of -e -z
>and -t -z, so would you please update the commit message to add "-t -z"
>
>Thanks,
>Bob
>
>-Original Message-
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Liming Gao
>Sent: Monday, September 2, 2019 9:31 AM
>To: devel@edk2.groups.io
>Cc: Feng, Bob C
>Subject: [edk2-devel] [Patch] BaseTools: Update GenFw tool to support -e -z
>option together
>
>BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1727
>-e -z option is to generate EFI image with zero debug entry.
>It can be used to check the EFI image in DEBUG build.
>
>Signed-off-by: Liming Gao
>Cc: Bob Feng
>---
> BaseTools/Source/C/GenFw/GenFw.c | 17 +++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
>diff --git a/BaseTools/Source/C/GenFw/GenFw.c
>b/BaseTools/Source/C/GenFw/GenFw.c
>index 973bae5fe4..c99782b78e 100644
>--- a/BaseTools/Source/C/GenFw/GenFw.c
>+++ b/BaseTools/Source/C/GenFw/GenFw.c
>@@ -,6 +,7 @@ Returns:
> time_t InputFileTime;
> time_t OutputFileTime;
> struct stat Stat_Buf;
>+ BOOLEAN ZeroDebugFlag;
>
> SetUtilityName (UTILITY_NAME);
>
>@@ -1158,6 +1159,7 @@ Returns:
> NegativeAddr = FALSE;
> InputFileTime = 0;
> OutputFileTime = 0;
>+ ZeroDebugFlag = FALSE;
>
> if (argc == 1) {
> Error (NULL, 0, 1001, "Missing options", "No input options."); @@ -1197,6
>+1199,9 @@ Returns:
> goto Finish;
> }
> ModuleType = argv[1];
>+ if (mOutImageType == FW_ZERO_DEBUG_IMAGE) {
>+ZeroDebugFlag = TRUE;
>+ }
> if (mOutImageType != FW_TE_IMAGE) {
> mOutImageType = FW_EFI_IMAGE;
> }
>@@ -1220,6 +1225,9 @@ Returns:
> }
>
> if ((stricmp (argv[0], "-t") == 0) || (stricmp (argv[0], "--terse") ==
> 0)) {
>+ if (mOutImageType == FW_ZERO_DEBUG_IMAGE) {
>+ZeroDebugFlag = TRUE;
>+ }
> mOutImageType = FW_TE_IMAGE;
> argc --;
> argv ++;
>@@ -1241,7 +1249,12 @@ Returns:
> }
>
> if ((stricmp (argv[0], "-z") == 0) || (stricmp (argv[0], "--zero") == 0))
> {
>- mOutImageType = FW_ZERO_DEBUG_IMAGE;
>+ if (mOutImageType == FW_DUMMY_IMAGE) {
>+mOutImageType = FW_ZERO_DEBUG_IMAGE;
>+ }
>+ if (mOutImageType == FW_TE_IMAGE || mOutImageType ==
>FW_EFI_IMAGE) {
>+ZeroDebugFlag = TRUE;
>+ }
> argc --;
> argv ++;
> continue;
>@@ -2588,7 +2601,7 @@ Returns:
> //
> // Zero Time/Data field
> //
>- ZeroDebugData (FileBuffer, FALSE);
>+ ZeroDebugData (FileBuffer, ZeroDebugFlag);
>
> if (mOutImageType == FW_TE_IMAGE) {
> if ((PeHdr->Pe32.FileHeader.NumberOfSections &~0xFF) || (Type &~0xFF))
>{
>--
>2.13.0.windows.1
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46729): https://edk2.groups.io/g/devel/message/46729
Mute This Topic: https://groups.io/mt/33107042/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] IntelFsp2WrapperPkg: Remove unneeded MdeModulePkg dependency
Reviewed-by: Star Zeng > -Original Message- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni, > Ray > Sent: Wednesday, September 4, 2019 2:18 AM > To: devel@edk2.groups.io > Cc: Chiu, Chasel ; Desimone, Nathaniel L > ; Zeng, Star > Subject: [edk2-devel] [PATCH] IntelFsp2WrapperPkg: Remove unneeded > MdeModulePkg dependency > > Signed-off-by: Ray Ni > Cc: Chasel Chiu > Cc: Nate DeSimone > Cc: Star Zeng > --- > IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf | 4 +--- > IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf | 4 +--- > 2 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf > b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf > index 82113dd148..dce7ef3d0b 100644 > --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf > +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf > @@ -6,7 +6,7 @@ > # register TemporaryRamDonePpi to call TempRamExit API, and register > MemoryDiscoveredPpi > # notify to call FspSiliconInit API. > # > -# Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved. > +# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -37,7 +37,6 @@ [LibraryClasses] >HobLib >FspWrapperPlatformLib >FspWrapperHobProcessLib > - DebugAgentLib >UefiCpuLib >PeCoffGetEntryPointLib >PeCoffExtraActionLib > @@ -48,7 +47,6 @@ [LibraryClasses] > > [Packages] >MdePkg/MdePkg.dec > - MdeModulePkg/MdeModulePkg.dec >UefiCpuPkg/UefiCpuPkg.dec >IntelFsp2Pkg/IntelFsp2Pkg.dec >IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec > diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf > b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf > index 1be4bf56a6..7da92991c8 100644 > --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf > +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf > @@ -6,7 +6,7 @@ > # register TemporaryRamDonePpi to call TempRamExit API, and register > MemoryDiscoveredPpi > # notify to call FspSiliconInit API. > # > -# Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved. > +# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -38,7 +38,6 @@ [LibraryClasses] >MemoryAllocationLib >FspWrapperPlatformLib >FspWrapperHobProcessLib > - DebugAgentLib >UefiCpuLib >PeCoffGetEntryPointLib >PeCoffExtraActionLib > @@ -48,7 +47,6 @@ [LibraryClasses] > > [Packages] >MdePkg/MdePkg.dec > - MdeModulePkg/MdeModulePkg.dec >UefiCpuPkg/UefiCpuPkg.dec >IntelFsp2Pkg/IntelFsp2Pkg.dec >IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec > -- > 2.21.0.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46728): https://edk2.groups.io/g/devel/message/46728 Mute This Topic: https://groups.io/mt/33129661/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch] BaseTools: Update GenFw tool to support -e -z option together
Liming,
The patch looks good to me.
A minor comment that the code looks fix the bug of the combinations of -e -z
and -t -z, so would you please update the commit message to add "-t -z"
Thanks,
Bob
-Original Message-
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Liming Gao
Sent: Monday, September 2, 2019 9:31 AM
To: devel@edk2.groups.io
Cc: Feng, Bob C
Subject: [edk2-devel] [Patch] BaseTools: Update GenFw tool to support -e -z
option together
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1727
-e -z option is to generate EFI image with zero debug entry.
It can be used to check the EFI image in DEBUG build.
Signed-off-by: Liming Gao
Cc: Bob Feng
---
BaseTools/Source/C/GenFw/GenFw.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 973bae5fe4..c99782b78e 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -,6 +,7 @@ Returns:
time_t InputFileTime;
time_t OutputFileTime;
struct stat Stat_Buf;
+ BOOLEAN ZeroDebugFlag;
SetUtilityName (UTILITY_NAME);
@@ -1158,6 +1159,7 @@ Returns:
NegativeAddr = FALSE;
InputFileTime = 0;
OutputFileTime = 0;
+ ZeroDebugFlag = FALSE;
if (argc == 1) {
Error (NULL, 0, 1001, "Missing options", "No input options."); @@ -1197,6
+1199,9 @@ Returns:
goto Finish;
}
ModuleType = argv[1];
+ if (mOutImageType == FW_ZERO_DEBUG_IMAGE) {
+ZeroDebugFlag = TRUE;
+ }
if (mOutImageType != FW_TE_IMAGE) {
mOutImageType = FW_EFI_IMAGE;
}
@@ -1220,6 +1225,9 @@ Returns:
}
if ((stricmp (argv[0], "-t") == 0) || (stricmp (argv[0], "--terse") == 0))
{
+ if (mOutImageType == FW_ZERO_DEBUG_IMAGE) {
+ZeroDebugFlag = TRUE;
+ }
mOutImageType = FW_TE_IMAGE;
argc --;
argv ++;
@@ -1241,7 +1249,12 @@ Returns:
}
if ((stricmp (argv[0], "-z") == 0) || (stricmp (argv[0], "--zero") == 0)) {
- mOutImageType = FW_ZERO_DEBUG_IMAGE;
+ if (mOutImageType == FW_DUMMY_IMAGE) {
+mOutImageType = FW_ZERO_DEBUG_IMAGE;
+ }
+ if (mOutImageType == FW_TE_IMAGE || mOutImageType == FW_EFI_IMAGE) {
+ZeroDebugFlag = TRUE;
+ }
argc --;
argv ++;
continue;
@@ -2588,7 +2601,7 @@ Returns:
//
// Zero Time/Data field
//
- ZeroDebugData (FileBuffer, FALSE);
+ ZeroDebugData (FileBuffer, ZeroDebugFlag);
if (mOutImageType == FW_TE_IMAGE) {
if ((PeHdr->Pe32.FileHeader.NumberOfSections &~0xFF) || (Type &~0xFF)) {
--
2.13.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46727): https://edk2.groups.io/g/devel/message/46727
Mute This Topic: https://groups.io/mt/33107042/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] [edk2-stable201908] MdePkg/BluetoothLeConfig.h: Add type EfiBluetoothSmpPeerAddressList
Reviewed-by: Liming Gao
>-Original Message-
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni,
>Ray
>Sent: Tuesday, August 27, 2019 2:16 AM
>To: devel@edk2.groups.io
>Cc: Kinney, Michael D ; Gao, Liming
>
>Subject: [edk2-devel] [PATCH] [edk2-stable201908]
>MdePkg/BluetoothLeConfig.h: Add type EfiBluetoothSmpPeerAddressList
>
>To support auto-connection, EFI_BLUETOOTH_LE_SMP_DATA_TYPE needs to
>add a new data type EfiBluetoothSmpPeerAddressList which associates
>with a list of Bluetooth per address connected before.
>
>This new data type was added in UEFI spec 2.7b.
>
>Signed-off-by: Ray Ni
>Cc: Michael D Kinney
>Cc: Liming Gao
>---
> MdePkg/Include/Protocol/BluetoothLeConfig.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/MdePkg/Include/Protocol/BluetoothLeConfig.h
>b/MdePkg/Include/Protocol/BluetoothLeConfig.h
>index 8c0f881f85..8726a58b15 100644
>--- a/MdePkg/Include/Protocol/BluetoothLeConfig.h
>+++ b/MdePkg/Include/Protocol/BluetoothLeConfig.h
>@@ -2,7 +2,7 @@
> EFI Bluetooth LE Config Protocol as defined in UEFI 2.7.
> This protocol abstracts user interface configuration for BluetoothLe device.
>
>- Copyright (c) 2017, Intel Corporation. All rights reserved.
>+ Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @par Revision Reference:
>@@ -451,6 +451,8 @@ typedef enum {
> EfiBluetoothSmpLocalCSRK, /* If Key hierarchy not supported */
> EfiBluetoothSmpLocalSignCounter,
> EfiBluetoothSmpLocalDIV,
>+ EfiBluetoothSmpPeerAddressList,
>+ EfiBluetoothSmpMax,
> } EFI_BLUETOOTH_LE_SMP_DATA_TYPE;
>
> /**
>--
>2.21.0.windows.1
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46726): https://edk2.groups.io/g/devel/message/46726
Mute This Topic: https://groups.io/mt/33037003/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] IntelFsp2WrapperPkg: Remove unneeded MdeModulePkg dependency
Signed-off-by: Ray Ni Cc: Chasel Chiu Cc: Nate DeSimone Cc: Star Zeng --- IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf | 4 +--- IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf index 82113dd148..dce7ef3d0b 100644 --- a/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf +++ b/IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf @@ -6,7 +6,7 @@ # register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi # notify to call FspSiliconInit API. # -# Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved. +# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -37,7 +37,6 @@ [LibraryClasses] HobLib FspWrapperPlatformLib FspWrapperHobProcessLib - DebugAgentLib UefiCpuLib PeCoffGetEntryPointLib PeCoffExtraActionLib @@ -48,7 +47,6 @@ [LibraryClasses] [Packages] MdePkg/MdePkg.dec - MdeModulePkg/MdeModulePkg.dec UefiCpuPkg/UefiCpuPkg.dec IntelFsp2Pkg/IntelFsp2Pkg.dec IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec diff --git a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf index 1be4bf56a6..7da92991c8 100644 --- a/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf +++ b/IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf @@ -6,7 +6,7 @@ # register TemporaryRamDonePpi to call TempRamExit API, and register MemoryDiscoveredPpi # notify to call FspSiliconInit API. # -# Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved. +# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -38,7 +38,6 @@ [LibraryClasses] MemoryAllocationLib FspWrapperPlatformLib FspWrapperHobProcessLib - DebugAgentLib UefiCpuLib PeCoffGetEntryPointLib PeCoffExtraActionLib @@ -48,7 +47,6 @@ [LibraryClasses] [Packages] MdePkg/MdePkg.dec - MdeModulePkg/MdeModulePkg.dec UefiCpuPkg/UefiCpuPkg.dec IntelFsp2Pkg/IntelFsp2Pkg.dec IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dec -- 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46725): https://edk2.groups.io/g/devel/message/46725 Mute This Topic: https://groups.io/mt/33129661/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [edk2-platforms] [PATCH] ClevoOpenBoardPkg: Fix GCC5 build issue
From: Agyeman
Fixed GPIO table missing curly brackets
Cc: Ankit Sinha
Cc: Nate DeSimone
Cc: Michael Kubacki
Cc: Chasel Chiu
CC: Dandan Bi
Signed-off-by: Prince Agyeman
---
.../N1xxWU/Library/BoardInitLib/N1xxWUGpioTable.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxWUGpioTable.c
b/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxWUGpioTable.c
index c99b83753f..aa2c770729 100644
---
a/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxWUGpioTable.c
+++
b/Platform/Intel/ClevoOpenBoardPkg/N1xxWU/Library/BoardInitLib/N1xxWUGpioTable.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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46724): https://edk2.groups.io/g/devel/message/46724
Mute This Topic: https://groups.io/mt/33129653/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[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 --- UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm index 4db1a09f28..6d83dca4b9 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm @@ -184,17 +184,17 @@ HasErrorCode: pushrax pushrax sidt[rsp] -xchgrax, [rsp + 2] -xchgrax, [rsp] -xchgrax, [rsp + 8] +xchgeax, [rsp + 2] +xchgeax, [rsp] +xchgeax, [rsp + 8] xor rax, rax pushrax pushrax sgdt[rsp] -xchgrax, [rsp + 2] -xchgrax, [rsp] -xchgrax, [rsp + 8] +xchgeax, [rsp + 2] +xchgeax, [rsp] +xchgeax, [rsp + 8] ;; UINT64 Ldtr, Tr; xor rax, rax -- 2.16.2.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46723): https://edk2.groups.io/g/devel/message/46723 Mute This Topic: https://groups.io/mt/33129650/21656 Mute #ac: https://groups.io/mk?hashtag=ac=3846945 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous Integration Phase 1
On 09/03/19 19:09, Sean Brogan wrote: > Laszlo/Mike, > > The idea that the maintainer must create the PR is fighting the > optimized github PR flow. Github and PRs process is optimized for > letting everyone contribute from "their" fork while still protecting > and putting process in place for the "upstream". > > Why not use github to assign maintainers to each package or filepath > and then let contributors submit their own PR to edk2 after the > mailing list has approved. Then the PR has a policy that someone that > is the maintainer of all files changed must approve before the PR can > be completed (or you could even set it so that maintainer must > complete PR). This would have the benefit of less monotonous work > for the maintainers and on rejected PRs the contributor could easily > update their branch and resubmit their PR. I'll let Mike respond. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46722): https://edk2.groups.io/g/devel/message/46722 Mute This Topic: https://groups.io/mt/33122986/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Race condition between APHandler's release of Busy spinlock and user-triggered SmmStartupThisAP's
On 09/03/19 16:57, Damian Nikodem wrote:
> Race condition between APHandler's release of Busy spinlock and
> user-triggered SmmStartupThisAP's acquisition attempt of the Busy spinlock
> (non-blocking mode).
>
> UserProc is the user's procedure to execute on an AP.
> UserProcCompletion is the user procedure's completion spinlock.
> All other variables are from EDK2.
>
> BSP AP
> =
>
> APHandler ()
>
>WaitForSemaphore (Run)
>
> << initial
> state >>
>
> AcquireSpinLock (UserProcCompletion)
> SmmStartupThisAp (Procedure)
> AcquireSpinLockOrFail (Busy)
> ReleaseSemaphore (Run)
>
>UserProc ()
> DoStuff()DoSomeOtherStuff ()
>
> AcquireSpinLockOrFail (UserProcCompletion) AcquireSpinLockOrFail
> (UserProcCompletion)
>
> ^^ waiting in a loop for user procedure's
>completion == these fail
> ReleaseSpinLock (UserProcCompletion) AcquireSpinLockOrFail
> (UserProcCompletion)
>
> ^^ this succeeds
>
> ReleaseSpinLock (UserProcCompletion)
>
> << return control to the caller and
>reenter the flow >>>
>
> AcquireSpinLock (UserProcCompletion)
> SmmStartupThisAp (Procedure)
> AcquireSpinLockOrFail (Busy)
> ^^ this wins the race with AP's
>ReleaseSpinLock and fails;
>
>ReleaseSpinLock (Busy)
> return EFI_INVALID_PARAMETER;
Sorry, I can't make any sense of this sequence diagram. It seems to have
fallen apart due to formatting or other email issues. For example, if we
have "AP" and "BSP" columns, I would expect every function name to show
up in either. But APHandler() is to the right of *both* columns.
Please clean up the commit message:
- subject line should be no longer than 74 chars
- continuous text paragraphs should be properly filled, and wrapped at
74 chars
- the diagram can extend more widely, but it should be a diagram please.
(I'm not as familiar with this code as other UefiCpuPkg reviewers, so I
absolutely depend on the commit message to guide me.)
Thanks
Laszlo
>
> To remedy, if AcquireSpinLockOrFail (of the Busy spinlock) fails, perform
> regular AcquireSpinLock -- this eliminates the race condition.
>
> Signed-off-by: Damian Nikodem
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Benjamin You
> Cc: Laszlo Ersek
> Cc: Krzysztof Rusocki
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index d8d2b6f444..206e196a76 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -1239,8 +1239,16 @@ InternalSmmStartupThisAp (
> AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
>} else {
> if (!AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy)) {
> - DEBUG((DEBUG_ERROR, "Can't acquire
> mSmmMpSyncData->CpuData[%d].Busy\n", CpuIndex));
> - return EFI_NOT_READY;
> + DEBUG ((DEBUG_INFO, "BSP[%d] finds AP[%d] busy at proc 0x%llX (param
> 0x%llX), ",
> +mSmmMpSyncData->BspIndex,
> +CpuIndex,
> +*mSmmMpSyncData->CpuData[CpuIndex].Procedure,
> +(VOID*)mSmmMpSyncData->CpuData[CpuIndex].Parameter));
> + DEBUG ((DEBUG_INFO, "new proc 0x%llX (param 0x%llX). Waiting for the
> previous AP procedure to complete...\n",
> +Procedure,
> +ProcArguments));
> +
> + AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
> }
>
> *Token = (MM_COMPLETION) CreateToken ();
>
>
> Intel Technology Poland sp. z o.o.
> ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII
> Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP
> 957-07-52-316 | Kapital zakladowy 200.000 PLN.
>
> Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i
> moze zawierac informacje poufne. W razie przypadkowego otrzymania tej
> wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie;
> jakiekolwiek
> przegladanie lub rozpowszechnianie jest zabronione.
> This e-mail and any attachments may contain confidential material for the
> sole use of the intended recipient(s). If you are not the intended recipient,
> please contact the sender and delete all copies; any review or distribution by
> others is strictly prohibited.
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46721): https://edk2.groups.io/g/devel/message/46721
Mute
Re: [Qemu-devel] [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF
On 09/03/19 16:53, Igor Mammedov wrote: > On Mon, 2 Sep 2019 21:09:58 +0200 > Laszlo Ersek wrote: > >> On 09/02/19 10:45, Igor Mammedov wrote: >>> On Fri, 30 Aug 2019 20:46:14 +0200 >>> Laszlo Ersek wrote: >>> On 08/30/19 16:48, Igor Mammedov wrote: > (01) On boot firmware maps and initializes SMI handler at default SMBASE > (3) > (using dedicated SMRAM at 3 would allow us to avoid save/restore > steps and make SMM handler pointer not vulnerable to DMA attacks) > > (02) QEMU hotplugs a new CPU in reset-ed state and sends SCI > > (03) on receiving SCI, host CPU calls GPE cpu hotplug handler > which writes to IO port 0xB2 (broadcast SMI) > > (04) firmware waits for all existing CPUs rendezvous in SMM mode, > new CPU(s) have SMI pending but does nothing yet > > (05) host CPU wakes up one new CPU (INIT-INIT-SIPI) > SIPI vector points to RO flash HLT loop. > (how host CPU will know which new CPUs to relocate? > possibly reuse QEMU CPU hotplug MMIO interface???) > > (06) new CPU does relocation. > (in case of attacker sends SIPI to several new CPUs, > open question how to detect collision of several CPUs at the same > default SMBASE) > > (07) once new CPU relocated host CPU completes initialization, returns > from IO port write and executes the rest of GPE handler, telling OS > to online new CPU. In step (03), it is the OS that handles the SCI; it transfers control to ACPI. The AML can write to IO port 0xB2 only because the OS allows it. If the OS decides to omit that step, and sends an INIT-SIPI-SIPI directly to the new CPU, can it steal the CPU? >>> It sure can but this way it won't get access to privileged SMRAM >>> so OS can't subvert firmware. >>> The next time SMI broadcast is sent the CPU will use SMI handler at >>> default 3 SMBASE. It's up to us to define behavior here (for example >>> relocation handler can put such CPU in shutdown state). >>> >>> It's in the best interest of OS to cooperate and execute AML >>> provided by firmware, if it does not follow proper cpu hotplug flow >>> we can't guarantee that stolen CPU will work. >> >> This sounds convincing enough, for the hotplugged CPU; thanks. >> >> So now my concern is with step (01). While preparing for the initial >> relocation (of cold-plugged CPUs), the code assumes the memory at the >> default SMBASE (0x3) is normal RAM. >> >> Is it not a problem that the area is written initially while running in >> normal 32-bit or 64-bit mode, but then executed (in response to the >> first, synchronous, SMI) as SMRAM? > > currently there is no SMRAM at 0x3, so all access falls through > into RAM address space and we are about to change that. > > but firmware doesn't have to use it as RAM, it can check if QEMU > supports SMRAM at 0x3 and if supported map it to configure > and then lock it down. I'm sure you are *technically* right, but you seem to be assuming that I can modify or rearrange anything I want in edk2. :) If we can solve the above in OVMF platform code, that's great. If not (e.g. UefiCpuPkg code needs to be updated), then things will get tricky. If we can introduce another platform hook for this, that would help. I can't say before I try. > > >> Basically I'm confused by the alias. >> >> TSEG (and presumably, A/B seg) work like this: >> - when open, looks like RAM to normal mode and SMM >> - when closed, looks like black-hole to normal mode, and like RAM to SMM >> >> The generic edk2 code knows this, and manages the SMRAM areas accordingly. >> >> The area at 0x3 is different: >> - looks like RAM to both normal mode and SMM >> >> If we set up the alias at 0x3 into A/B seg, >> - will that *permanently* hide the normal RAM at 0x3? >> - will 0x3 start behaving like A/B seg? >> >> Basically my concern is that the universal code in edk2 might or might >> not keep A/B seg open while initially populating the area at the default >> SMBASE. Specifically, I can imagine two issues: >> >> - if the alias into A/B seg is inactive during the initial population, >> then the initial writes go to RAM, but the execution (the first SMBASE >> relocation) will occur from A/B seg through the alias >> >> - alternatively, if the alias is always active, but A/B seg is closed >> during initial population (which happens in normal mode), then the >> initial writes go to the black hole, and execution will occur from a >> "blank" A/B seg. >> >> Am I seeing things? (Sorry, I keep feeling dumber and dumber in this >> thread.) > > I don't really know how firmware uses A/B segments and I'm afraid that > cannibalizing one for configuring 0x3 might break something. > > Since we are inventing something out of q35 spec anyway, How about > leaving A/B/TSEG to be and using fwcfg to configure when/where >
Re: [edk2-devel] [edk2-platforms][PATCH V2 6/6] QuarkSocPkg: Clean up duplicated SmramMemoryReserve.h files
Reviewed-by: Kelly Steele
Thanks,
Kelly
> -Original Message-
> From: Chen, Marc W
> Sent: September 02, 2019 08:36
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Steele, Kelly
> ; Desimone, Nathaniel L
>
> Subject: [edk2-platforms][PATCH V2 6/6] QuarkSocPkg: Clean up duplicated
> SmramMemoryReserve.h files
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2108
>
> SmramMemoryReserve.h has been added into
> Edk2\MdePkg\Include\Guid\SmramMemoryReserve.h.
>
> The duplicated header file can be cleaned up.
> Edk2Platforms\Silicon\Intel\QuarkSocPkg\QuarkNorthCluster\Include\Guid\
> SmramMemoryReserve.h
>
> Cc: Michael D Kinney
> Cc: Kelly Steele
>
> Co-authored-by: Nate DeSimone
> Signed-off-by: Marc W Chen
> Signed-off-by: Nate DeSimone
> ---
> .../Include/Guid/SmramMemoryReserve.h | 54 ---
> .../Smm/Dxe/SmmAccessDxe/SmmAccess.inf| 2 +-
> .../Smm/Dxe/SmmAccessDxe/SmmAccessDriver.c| 2 +-
> .../Smm/Pei/SmmAccessPei/SmmAccessPei.c | 4 +-
> .../Smm/Pei/SmmAccessPei/SmmAccessPei.inf | 2 +-
> Silicon/Intel/QuarkSocPkg/QuarkSocPkg.dec | 1 -
> 6 files changed, 5 insertions(+), 60 deletions(-)
> delete mode 100644
> Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Include/Guid/SmramMemory
> Reserve.h
>
> diff --git
> a/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Include/Guid/SmramMemo
> ryReserve.h
> b/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Include/Guid/SmramMemo
> ryReserve.h
> deleted file mode 100644
> index d57dfbebf3..00
> ---
> a/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Include/Guid/SmramMemo
> ryReserve.h
> @@ -1,54 +0,0 @@
> -/** @file
> - Definition of GUIDed HOB for reserving SMRAM regions.
> -
> - This file defines:
> - * the GUID used to identify the GUID HOB for reserving SMRAM regions.
> - * the data structure of SMRAM descriptor to describe SMRAM candidate
> regions
> - * values of state of SMRAM candidate regions
> - * the GUID specific data structure of HOB for reserving SMRAM regions.
> - This GUIDed HOB can be used to convey the existence of the T-SEG
> reservation and H-SEG usage
> -
> -Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> - @par Revision Reference:
> - GUIDs defined in SmmCis spec version 0.9.
> -
> -**/
> -
> -#ifndef _EFI_SMM_PEI_SMRAM_MEMORY_RESERVE_H_
> -#define _EFI_SMM_PEI_SMRAM_MEMORY_RESERVE_H_
> -
> -#define EFI_SMM_PEI_SMRAM_MEMORY_RESERVE \
> - { \
> -0x6dadf1d1, 0xd4cc, 0x4910, {0xbb, 0x6e, 0x82, 0xb1, 0xfd, 0x80, 0xff,
> 0x3d
> } \
> - }
> -
> -/**
> -* GUID specific data structure of HOB for reserving SMRAM regions.
> -*
> -* Inconsistent with specification here:
> -* EFI_HOB_SMRAM_DESCRIPTOR_BLOCK has been changed to
> EFI_SMRAM_HOB_DESCRIPTOR_BLOCK.
> -* This inconsistency is kept in code in order for backward compatibility.
> -**/
> -typedef struct {
> - ///
> - /// Designates the number of possible regions in the system
> - /// that can be usable for SMRAM.
> - ///
> - /// Inconsistent with specification here:
> - /// In Framework SMM CIS 0.91 specification, it defines the field type as
> UINTN.
> - /// However, HOBs are supposed to be CPU neutral, so UINT32 should be
> used instead.
> - ///
> - UINT32NumberOfSmmReservedRegions;
> - ///
> - /// Used throughout this protocol to describe the candidate
> - /// regions for SMRAM that are supported by this platform.
> - ///
> - EFI_SMRAM_DESCRIPTOR Descriptor[1];
> -} EFI_SMRAM_HOB_DESCRIPTOR_BLOCK;
> -
> -extern EFI_GUID gEfiSmmPeiSmramMemoryReserveGuid;
> -
> -#endif
> -
> diff --git
> a/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/
> SmmAccess.inf
> b/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/
> SmmAccess.inf
> index bb555b4a2e..cf579efd02 100644
> ---
> a/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/
> SmmAccess.inf
> +++
> b/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/
> SmmAccess.inf
> @@ -42,7 +42,7 @@
>gEfiSmmAccess2ProtocolGuid
>
> [Guids]
> - gEfiSmmPeiSmramMemoryReserveGuid
> + gEfiSmmSmramMemoryGuid
>
> [Depex]
>gEfiPciRootBridgeIoProtocolGuid
> diff --git
> a/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/
> SmmAccessDriver.c
> b/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/
> SmmAccessDriver.c
> index 830f8b83c3..7992ef7ded 100644
> ---
> a/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/
> SmmAccessDriver.c
> +++
> b/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/
> SmmAccessDriver.c
> @@ -75,7 +75,7 @@ Returns:
>//
>// Get Hob list
>//
> - GuidHob= GetFirstGuidHob ();
> + GuidHob= GetFirstGuidHob ();
>DescriptorBlock = GET_GUID_HOB_DATA (GuidHob);
>ASSERT (DescriptorBlock);
>
> diff --git
> a/Silicon/Intel/QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmAccessPei/S
> mmAccessPei.c
>
Re: [Qemu-devel] [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF
On Mon, 2 Sep 2019 21:09:58 +0200 Laszlo Ersek wrote: > On 09/02/19 10:45, Igor Mammedov wrote: > > On Fri, 30 Aug 2019 20:46:14 +0200 > > Laszlo Ersek wrote: > > > >> On 08/30/19 16:48, Igor Mammedov wrote: > >> > >>> (01) On boot firmware maps and initializes SMI handler at default SMBASE > >>> (3) > >>> (using dedicated SMRAM at 3 would allow us to avoid save/restore > >>> steps and make SMM handler pointer not vulnerable to DMA attacks) > >>> > >>> (02) QEMU hotplugs a new CPU in reset-ed state and sends SCI > >>> > >>> (03) on receiving SCI, host CPU calls GPE cpu hotplug handler > >>> which writes to IO port 0xB2 (broadcast SMI) > >>> > >>> (04) firmware waits for all existing CPUs rendezvous in SMM mode, > >>> new CPU(s) have SMI pending but does nothing yet > >>> > >>> (05) host CPU wakes up one new CPU (INIT-INIT-SIPI) > >>> SIPI vector points to RO flash HLT loop. > >>> (how host CPU will know which new CPUs to relocate? > >>> possibly reuse QEMU CPU hotplug MMIO interface???) > >>> > >>> (06) new CPU does relocation. > >>> (in case of attacker sends SIPI to several new CPUs, > >>> open question how to detect collision of several CPUs at the same > >>> default SMBASE) > >>> > >>> (07) once new CPU relocated host CPU completes initialization, returns > >>> from IO port write and executes the rest of GPE handler, telling OS > >>> to online new CPU. > >> > >> In step (03), it is the OS that handles the SCI; it transfers control to > >> ACPI. The AML can write to IO port 0xB2 only because the OS allows it. > >> > >> If the OS decides to omit that step, and sends an INIT-SIPI-SIPI > >> directly to the new CPU, can it steal the CPU? > > It sure can but this way it won't get access to privileged SMRAM > > so OS can't subvert firmware. > > The next time SMI broadcast is sent the CPU will use SMI handler at > > default 3 SMBASE. It's up to us to define behavior here (for example > > relocation handler can put such CPU in shutdown state). > > > > It's in the best interest of OS to cooperate and execute AML > > provided by firmware, if it does not follow proper cpu hotplug flow > > we can't guarantee that stolen CPU will work. > > This sounds convincing enough, for the hotplugged CPU; thanks. > > So now my concern is with step (01). While preparing for the initial > relocation (of cold-plugged CPUs), the code assumes the memory at the > default SMBASE (0x3) is normal RAM. > > Is it not a problem that the area is written initially while running in > normal 32-bit or 64-bit mode, but then executed (in response to the > first, synchronous, SMI) as SMRAM? currently there is no SMRAM at 0x3, so all access falls through into RAM address space and we are about to change that. but firmware doesn't have to use it as RAM, it can check if QEMU supports SMRAM at 0x3 and if supported map it to configure and then lock it down. > Basically I'm confused by the alias. > > TSEG (and presumably, A/B seg) work like this: > - when open, looks like RAM to normal mode and SMM > - when closed, looks like black-hole to normal mode, and like RAM to SMM > > The generic edk2 code knows this, and manages the SMRAM areas accordingly. > > The area at 0x3 is different: > - looks like RAM to both normal mode and SMM > > If we set up the alias at 0x3 into A/B seg, > - will that *permanently* hide the normal RAM at 0x3? > - will 0x3 start behaving like A/B seg? > > Basically my concern is that the universal code in edk2 might or might > not keep A/B seg open while initially populating the area at the default > SMBASE. Specifically, I can imagine two issues: > > - if the alias into A/B seg is inactive during the initial population, > then the initial writes go to RAM, but the execution (the first SMBASE > relocation) will occur from A/B seg through the alias > > - alternatively, if the alias is always active, but A/B seg is closed > during initial population (which happens in normal mode), then the > initial writes go to the black hole, and execution will occur from a > "blank" A/B seg. > > Am I seeing things? (Sorry, I keep feeling dumber and dumber in this > thread.) I don't really know how firmware uses A/B segments and I'm afraid that cannibalizing one for configuring 0x3 might break something. Since we are inventing something out of q35 spec anyway, How about leaving A/B/TSEG to be and using fwcfg to configure when/where SMRAM(0x3+128K) should be mapped into RAM address space. I see a couple of options: 1: use identity mapping where SMRAM(0x3+128K) maps into the same range in RAM address space when firmware writes into fwcfg file and unmaps/locks on the second write (until HW reset) 2: let firmware choose where to map SMRAM(0x3+128K) in RAM address space, logic is essentially the same as above only firmware picks and writes into fwcfg an address where
Re: [edk2-devel] [edk2-platforms][PATCH V2 2/6] QuarkPlatformPkg: Clean up duplicated SmramMemoryReserve.h files
Reviewed-by: Kelly Steele
Thanks,
Kelly
> -Original Message-
> From: Chen, Marc W
> Sent: September 02, 2019 08:36
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D ; Steele, Kelly
> ; Desimone, Nathaniel L
>
> Subject: [edk2-platforms][PATCH V2 2/6] QuarkPlatformPkg: Clean up
> duplicated SmramMemoryReserve.h files
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2108
>
> SmramMemoryReserve.h has been added into
> Edk2\MdePkg\Include\Guid\SmramMemoryReserve.h.
>
> The duplicated header file can be cleaned up.
> Edk2Platforms\Silicon\Intel\QuarkSocPkg\QuarkNorthCluster\Include\Guid\
> SmramMemoryReserve.h
>
> Cc: Michael D Kinney
> Cc: Kelly Steele
>
> Co-authored-by: Nate DeSimone
> Signed-off-by: Marc W Chen
> Signed-off-by: Nate DeSimone
> ---
> .../Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatform.c | 4 ++--
> .../Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatform.inf | 2 +-
> .../Platform/Pei/PlatformInit/MrcWrapper.c| 8
> .../Platform/Pei/PlatformInit/PlatformEarlyInit.inf | 2 +-
> 4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git
> a/Platform/Intel/QuarkPlatformPkg/Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatf
> orm.c
> b/Platform/Intel/QuarkPlatformPkg/Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatf
> orm.c
> index f7f7ca3196..479459b801 100644
> ---
> a/Platform/Intel/QuarkPlatformPkg/Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatf
> orm.c
> +++
> b/Platform/Intel/QuarkPlatformPkg/Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatf
> orm.c
> @@ -3,7 +3,7 @@ ACPISMM Driver implementation file.
>
> This is QNC Smm platform driver
>
> -Copyright (c) 2013-2016 Intel Corporation.
> +Copyright (c) 2013-2019 Intel Corporation.
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -225,7 +225,7 @@ Returns:
>//
>// Get Hob list for SMRAM desc
>//
> - GuidHob= GetFirstGuidHob ();
> + GuidHob= GetFirstGuidHob ();
>ASSERT (GuidHob);
>DescriptorBlock = GET_GUID_HOB_DATA (GuidHob);
>ASSERT (DescriptorBlock);
> diff --git
> a/Platform/Intel/QuarkPlatformPkg/Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatf
> orm.inf
> b/Platform/Intel/QuarkPlatformPkg/Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatf
> orm.inf
> index be80c73528..5301eccc6e 100644
> ---
> a/Platform/Intel/QuarkPlatformPkg/Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatf
> orm.inf
> +++
> b/Platform/Intel/QuarkPlatformPkg/Acpi/DxeSmm/AcpiSmm/AcpiSmmPlatf
> orm.inf
> @@ -56,7 +56,7 @@
>gEfiSmmSwDispatch2ProtocolGuid
>
> [Guids]
> - gEfiSmmPeiSmramMemoryReserveGuid
> + gEfiSmmSmramMemoryGuid
>gQncS3CodeInLockBoxGuid
>gQncS3ContextInLockBoxGuid
>
> diff --git
> a/Platform/Intel/QuarkPlatformPkg/Platform/Pei/PlatformInit/MrcWrapper.
> c
> b/Platform/Intel/QuarkPlatformPkg/Platform/Pei/PlatformInit/MrcWrapper.
> c
> index fcb5c79aaf..1bb532acfd 100644
> ---
> a/Platform/Intel/QuarkPlatformPkg/Platform/Pei/PlatformInit/MrcWrapper.
> c
> +++
> b/Platform/Intel/QuarkPlatformPkg/Platform/Pei/PlatformInit/MrcWrapper.
> c
> @@ -1,7 +1,7 @@
> /** @file
> Framework PEIM to initialize memory on a Quark Memory Controller.
>
> -Copyright (c) 2013 - 2016, Intel Corporation.
> +Copyright (c) 2013 - 2019, Intel Corporation.
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -836,7 +836,7 @@ InstallEfiMemory (
>BufferSize += ((SmramRanges - 1) * sizeof (EFI_SMRAM_DESCRIPTOR));
>
>Hob.Raw = BuildGuidHob (
> - ,
> + ,
>BufferSize
>);
>ASSERT (Hob.Raw);
> @@ -958,7 +958,7 @@ InstallS3Memory (
>}
>
>Hob.Raw = BuildGuidHob (
> - ,
> + ,
>BufferSize
>);
>ASSERT (Hob.Raw);
> @@ -1546,7 +1546,7 @@ InfoPostInstallMemory (
> }
>}
> } else if (Hob.Header->HobType == EFI_HOB_TYPE_GUID_EXTENSION) {
> - if (CompareGuid (&(Hob.Guid->Name),
> )) {
> + if (CompareGuid (&(Hob.Guid->Name), ))
> {
> SmramHobDescriptorBlock = (VOID*) (Hob.Raw + sizeof
> (EFI_HOB_GUID_TYPE));
> if (SmramDescriptorPtr != NULL) {
>*SmramDescriptorPtr = SmramHobDescriptorBlock->Descriptor;
> diff --git
> a/Platform/Intel/QuarkPlatformPkg/Platform/Pei/PlatformInit/PlatformEarly
> Init.inf
> b/Platform/Intel/QuarkPlatformPkg/Platform/Pei/PlatformInit/PlatformEarly
> Init.inf
> index adec9e20eb..7910446402 100644
> ---
> a/Platform/Intel/QuarkPlatformPkg/Platform/Pei/PlatformInit/PlatformEarly
> Init.inf
> +++
> b/Platform/Intel/QuarkPlatformPkg/Platform/Pei/PlatformInit/PlatformEarly
> Init.inf
> @@ -108,7 +108,7 @@
>gEfiAcpiVariableGuid # ALWAYS_CONSUMED
> L"AcpiGlobalVariab"
>gEfiMemoryTypeInformationGuid # ALWAYS_CONSUMED
> L"MemoryTypeInformation"
>gEfiMemoryConfigDataGuid # SOMETIMES_PRODUCED Hob:
> GUID_EXTENSION
> - gEfiSmmPeiSmramMemoryReserveGuid # ALWAYS_PRODUCED
> Hob: GUID_EXTENSION
> + gEfiSmmSmramMemoryGuid# ALWAYS_PRODUCED Hob:
>
[edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Race condition between APHandler's release of Busy spinlock and user-triggered SmmStartupThisAP's
Race condition between APHandler's release of Busy spinlock and
user-triggered SmmStartupThisAP's acquisition attempt of the Busy spinlock
(non-blocking mode).
UserProc is the user's procedure to execute on an AP.
UserProcCompletion is the user procedure's completion spinlock.
All other variables are from EDK2.
BSP AP
=
APHandler ()
WaitForSemaphore (Run)
<< initial
state >>
AcquireSpinLock (UserProcCompletion)
SmmStartupThisAp (Procedure)
AcquireSpinLockOrFail (Busy)
ReleaseSemaphore (Run)
UserProc ()
DoStuff()DoSomeOtherStuff ()
AcquireSpinLockOrFail (UserProcCompletion) AcquireSpinLockOrFail
(UserProcCompletion)
^^ waiting in a loop for user procedure's
completion == these fail
ReleaseSpinLock (UserProcCompletion) AcquireSpinLockOrFail
(UserProcCompletion)
^^ this succeeds
ReleaseSpinLock (UserProcCompletion)
<< return control to the caller and
reenter the flow >>>
AcquireSpinLock (UserProcCompletion)
SmmStartupThisAp (Procedure)
AcquireSpinLockOrFail (Busy)
^^ this wins the race with AP's
ReleaseSpinLock and fails;
ReleaseSpinLock (Busy)
return EFI_INVALID_PARAMETER;
To remedy, if AcquireSpinLockOrFail (of the Busy spinlock) fails, perform
regular AcquireSpinLock -- this eliminates the race condition.
Signed-off-by: Damian Nikodem
Cc: Eric Dong
Cc: Ray Ni
Cc: Benjamin You
Cc: Laszlo Ersek
Cc: Krzysztof Rusocki
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index d8d2b6f444..206e196a76 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -1239,8 +1239,16 @@ InternalSmmStartupThisAp (
AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
} else {
if (!AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy)) {
- DEBUG((DEBUG_ERROR, "Can't acquire mSmmMpSyncData->CpuData[%d].Busy\n",
CpuIndex));
- return EFI_NOT_READY;
+ DEBUG ((DEBUG_INFO, "BSP[%d] finds AP[%d] busy at proc 0x%llX (param
0x%llX), ",
+mSmmMpSyncData->BspIndex,
+CpuIndex,
+*mSmmMpSyncData->CpuData[CpuIndex].Procedure,
+(VOID*)mSmmMpSyncData->CpuData[CpuIndex].Parameter));
+ DEBUG ((DEBUG_INFO, "new proc 0x%llX (param 0x%llX). Waiting for the
previous AP procedure to complete...\n",
+Procedure,
+ProcArguments));
+
+ AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
}
*Token = (MM_COMPLETION) CreateToken ();
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 |
Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie;
jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). If you are not the intended recipient, please
contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46716): https://edk2.groups.io/g/devel/message/46716
Mute This Topic: https://groups.io/mt/33128825/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous Integration Phase 1
Laszlo/Mike, The idea that the maintainer must create the PR is fighting the optimized github PR flow. Github and PRs process is optimized for letting everyone contribute from "their" fork while still protecting and putting process in place for the "upstream". Why not use github to assign maintainers to each package or filepath and then let contributors submit their own PR to edk2 after the mailing list has approved. Then the PR has a policy that someone that is the maintainer of all files changed must approve before the PR can be completed (or you could even set it so that maintainer must complete PR). This would have the benefit of less monotonous work for the maintainers and on rejected PRs the contributor could easily update their branch and resubmit their PR. Thanks Sean -Original Message- From: r...@edk2.groups.io On Behalf Of Laszlo Ersek via Groups.Io Sent: Tuesday, September 3, 2019 9:55 AM To: Ni, Ray ; devel@edk2.groups.io; r...@edk2.groups.io; Gao, Liming ; Kinney, Michael D Subject: Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous Integration Phase 1 On 09/03/19 18:41, Ni, Ray wrote: >> -Original Message- >> From: devel@edk2.groups.io On Behalf Of Laszlo >> Ersek >> Sent: Tuesday, September 3, 2019 6:20 AM >> To: Ni, Ray ; r...@edk2.groups.io; >> devel@edk2.groups.io; Gao, Liming ; Kinney, >> Michael D >> Subject: Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous >> Integration Phase 1 >> >> On 09/03/19 05:39, Ni, Ray wrote: >>> Can someone draw a flow chart of who does what to help clarify? >> >> I don't see this as a huge change, relative to the current process. >> >> Before, it's always been up to the subsys maintainer to apply & >> rebase the patches locally, pick up the feedback tags, and run at >> least *some* build tests before pushing. >> >> After, the final git push is not to "origin" but to a personal branch >> on github.com, and then a github PR is needed. If that fails, notify >> the submitter. That's all, as far as I can see. >> >>> It sounds like maintainers are going to be the very important bridges >>> between CI system and the patch owners (developers). >> Important it is I agree but boring it is as well I have to say. >> >> Oh, it *is* absolutely boring. >> >>> Are maintainers still needed to be picked up/chosen/promoted from >>> developers? >> >> Would you trust a person to apply, build-test, and push your >> UefiCpuPkg patches, if that person had *zero* UefiCpuPkg development >> experience? >> (Or even zero edk2 development experience?) > > Can we change the process a bit? > 1. maintainers created pull requests on behave of the patch owners 2. > patch owners can be notified automatically if pull requests fail I believe this can work if: - the submitter has a github account - the maintainer knows the submitter's account name - the maintainer manually subscribes the submitter to the PR (I have never tried this myself) > 3. patch owners update the pull requests > (I am not familiar to pull requests. I assume the rights of > modifying pull requests and creating pull requests are separated. Are > they?) PRs should not be updated. If there is a CI failure, the PR should be rejected. If that happens automatically, that's great. If not, then someone must close the PR manually. If the patch series submitter can do that, that's great (I have no idea if that works!) If only the PR owner (= maintainer) can close the PR, then they should close it. Either way, the patch author will have to submit a v2 to the list. When that is sufficiently reviewed, the same process starts (new PR opened by maintainer). > So, maintainers only need to initiate the pull requests. I hope this can actually work. We need to test this out first. > It assumes when pull requests are initiated, everyone at least agrees the > justifications of the changes are valid and the ways the changes are done. I agree fully about this. If the CI check completes, then the changes are pushed to master automatically! (Only if the push is a fast-forward -- otherwise the PR is rejected again. GitHub will not be allowed to merge or rebase withoout supervision.) Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46715): https://edk2.groups.io/g/devel/message/46715 Mute This Topic: https://groups.io/mt/33122986/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Race condition between APHandler's release of Busy spinlock and user-triggered SmmStartupThisAP's
1. can we directly call AcquireSpinLock()? *OrFail() can be removed IMO.
2. It's a patch to change the behavior of SmmStartupThisAP(). So that to reduce
the potential bugs in caller's code. Patch title is a bit mis-leading.
> -Original Message-
> From: Nikodem, Damian
> Sent: Tuesday, September 3, 2019 7:58 AM
> To: devel@edk2.groups.io
> Cc: Nikodem, Damian ; Dong, Eric
> ; Ni, Ray ; You,
> Benjamin ; Laszlo Ersek ; Rusocki,
> Krzysztof
> Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Race condition between
> APHandler's release of Busy spinlock and user-
> triggered SmmStartupThisAP's
>
> Race condition between APHandler's release of Busy spinlock and
> user-triggered SmmStartupThisAP's acquisition attempt of the Busy spinlock
> (non-blocking mode).
>
> UserProc is the user's procedure to execute on an AP.
> UserProcCompletion is the user procedure's completion spinlock.
> All other variables are from EDK2.
>
> BSP AP
> =
>
> APHandler ()
>
>WaitForSemaphore (Run)
>
> << initial
> state >>
>
> AcquireSpinLock (UserProcCompletion)
> SmmStartupThisAp (Procedure)
> AcquireSpinLockOrFail (Busy)
> ReleaseSemaphore (Run)
>
>UserProc ()
> DoStuff()DoSomeOtherStuff ()
>
> AcquireSpinLockOrFail (UserProcCompletion) AcquireSpinLockOrFail
> (UserProcCompletion)
>
> ^^ waiting in a loop for user procedure's
>completion == these fail
> ReleaseSpinLock (UserProcCompletion) AcquireSpinLockOrFail
> (UserProcCompletion)
>
> ^^ this succeeds
>
> ReleaseSpinLock (UserProcCompletion)
>
> << return control to the caller and
>reenter the flow >>>
>
> AcquireSpinLock (UserProcCompletion)
> SmmStartupThisAp (Procedure)
> AcquireSpinLockOrFail (Busy)
> ^^ this wins the race with AP's
>ReleaseSpinLock and fails;
>
>ReleaseSpinLock (Busy)
> return EFI_INVALID_PARAMETER;
>
> To remedy, if AcquireSpinLockOrFail (of the Busy spinlock) fails, perform
> regular AcquireSpinLock -- this eliminates the race
> condition.
>
> Signed-off-by: Damian Nikodem
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Benjamin You
> Cc: Laszlo Ersek
> Cc: Krzysztof Rusocki
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index d8d2b6f444..206e196a76 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -1239,8 +1239,16 @@ InternalSmmStartupThisAp (
> AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
>} else {
> if (!AcquireSpinLockOrFail (mSmmMpSyncData->CpuData[CpuIndex].Busy)) {
> - DEBUG((DEBUG_ERROR, "Can't acquire
> mSmmMpSyncData->CpuData[%d].Busy\n", CpuIndex));
> - return EFI_NOT_READY;
> + DEBUG ((DEBUG_INFO, "BSP[%d] finds AP[%d] busy at proc 0x%llX (param
> 0x%llX), ",
> +mSmmMpSyncData->BspIndex,
> +CpuIndex,
> +*mSmmMpSyncData->CpuData[CpuIndex].Procedure,
> +(VOID*)mSmmMpSyncData->CpuData[CpuIndex].Parameter));
> + DEBUG ((DEBUG_INFO, "new proc 0x%llX (param 0x%llX). Waiting for the
> previous AP procedure to complete...\n",
> +Procedure,
> +ProcArguments));
> +
> + AcquireSpinLock (mSmmMpSyncData->CpuData[CpuIndex].Busy);
> }
>
> *Token = (MM_COMPLETION) CreateToken ();
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46714): https://edk2.groups.io/g/devel/message/46714
Mute This Topic: https://groups.io/mt/33128825/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous Integration Phase 1
On 09/03/19 18:41, Ni, Ray wrote: >> -Original Message- >> From: devel@edk2.groups.io On Behalf Of Laszlo Ersek >> Sent: Tuesday, September 3, 2019 6:20 AM >> To: Ni, Ray ; r...@edk2.groups.io; devel@edk2.groups.io; >> Gao, Liming ; Kinney, >> Michael D >> Subject: Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous Integration >> Phase 1 >> >> On 09/03/19 05:39, Ni, Ray wrote: >>> Can someone draw a flow chart of who does what to help clarify? >> >> I don't see this as a huge change, relative to the current process. >> >> Before, it's always been up to the subsys maintainer to apply & rebase >> the patches locally, pick up the feedback tags, and run at least *some* >> build tests before pushing. >> >> After, the final git push is not to "origin" but to a personal branch on >> github.com, and then a github PR is needed. If that fails, notify the >> submitter. That's all, as far as I can see. >> >>> It sounds like maintainers are going to be the very important bridges >>> between CI system and the patch owners (developers). >> Important it is I agree but boring it is as well I have to say. >> >> Oh, it *is* absolutely boring. >> >>> Are maintainers still needed to be picked up/chosen/promoted from >>> developers? >> >> Would you trust a person to apply, build-test, and push your UefiCpuPkg >> patches, if that person had *zero* UefiCpuPkg development experience? >> (Or even zero edk2 development experience?) > > Can we change the process a bit? > 1. maintainers created pull requests on behave of the patch owners > 2. patch owners can be notified automatically if pull requests fail I believe this can work if: - the submitter has a github account - the maintainer knows the submitter's account name - the maintainer manually subscribes the submitter to the PR (I have never tried this myself) > 3. patch owners update the pull requests > (I am not familiar to pull requests. I assume the rights of modifying > pull requests and creating pull requests are separated. Are they?) PRs should not be updated. If there is a CI failure, the PR should be rejected. If that happens automatically, that's great. If not, then someone must close the PR manually. If the patch series submitter can do that, that's great (I have no idea if that works!) If only the PR owner (= maintainer) can close the PR, then they should close it. Either way, the patch author will have to submit a v2 to the list. When that is sufficiently reviewed, the same process starts (new PR opened by maintainer). > So, maintainers only need to initiate the pull requests. I hope this can actually work. We need to test this out first. > It assumes when pull requests are initiated, everyone at least agrees the > justifications of the changes are valid and the ways the changes are done. I agree fully about this. If the CI check completes, then the changes are pushed to master automatically! (Only if the push is a fast-forward -- otherwise the PR is rejected again. GitHub will not be allowed to merge or rebase withoout supervision.) Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46713): https://edk2.groups.io/g/devel/message/46713 Mute This Topic: https://groups.io/mt/33122986/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] ArmVirtPkg/PlatformBootManagerLib: unload image on EFI_SECURITY_VIOLATION
On Tue, 3 Sep 2019 at 09:38, Laszlo Ersek wrote:
>
> The LoadImage() boot service is a bit unusual in that it allocates
> resources in a particular failure case; namely, it produces a valid
> "ImageHandle" when it returns EFI_SECURITY_VIOLATION. This is supposed to
> happen e.g. when Secure Boot verification fails for the image, but the
> platform policy for the particular image origin (such as "fixed media" or
> "removable media") is DEFER_EXECUTE_ON_SECURITY_VIOLATION. The return code
> allows platform logic to selectively override the verification failure,
> and launch the image nonetheless.
>
> ArmVirtPkg/PlatformBootManagerLib does not override EFI_SECURITY_VIOLATION
> for the kernel image loaded from fw_cfg -- any LoadImage() error is
> considered fatal. When we simply treat EFI_SECURITY_VIOLATION like any
> other LoadImage() error, we leak the resources associated with
> "KernelImageHandle". From a resource usage perspective,
> EFI_SECURITY_VIOLATION must be considered "success", and rolled back.
>
> Implement this rollback, without breaking the proper "nesting" of error
> handling jumps and labels.
>
> Cc: Ard Biesheuvel
> Cc: Dandan Bi
> Cc: Leif Lindholm
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1992
> Fixes: 23d04b58e27b382bbd3f9b16ba9adb1cb203dad5
> Signed-off-by: Laszlo Ersek
Acked-by: Ard Biesheuvel
> ---
>
> Notes:
> Repo: https://github.com/lersek/edk2.git
> Branch: ldimg_armvirt_bz1992
>
> ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
> b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
> index 3cc83e3b7b95..d3851fd75fa5 100644
> --- a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
> +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
> @@ -968,53 +968,60 @@ TryRunningQemuKernel (
>
>//
>// Create a device path for the kernel image to be loaded from that will
> call
>// back into our file system.
>//
>KernelDevicePath = FileDevicePath (FileSystemHandle, KernelBlob->Name);
>if (KernelDevicePath == NULL) {
> DEBUG ((EFI_D_ERROR, "%a: failed to allocate kernel device path\n",
>__FUNCTION__));
> Status = EFI_OUT_OF_RESOURCES;
> goto UninstallProtocols;
>}
>
>//
>// Load the image. This should call back into our file system.
>//
>Status = gBS->LoadImage (
>FALSE, // BootPolicy: exact match required
>gImageHandle, // ParentImageHandle
>KernelDevicePath,
>NULL, // SourceBuffer
>0, // SourceSize
>
>);
>if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "%a: LoadImage(): %r\n", __FUNCTION__, Status));
> -goto FreeKernelDevicePath;
> +if (Status != EFI_SECURITY_VIOLATION) {
> + goto FreeKernelDevicePath;
> +}
> +//
> +// From the resource allocation perspective, EFI_SECURITY_VIOLATION means
> +// "success", so we must roll back the image loading.
> +//
> +goto UnloadKernelImage;
>}
>
>//
>// Construct the kernel command line.
>//
>Status = gBS->OpenProtocol (
>KernelImageHandle,
>,
>(VOID **),
>gImageHandle, // AgentHandle
>NULL, // ControllerHandle
>EFI_OPEN_PROTOCOL_GET_PROTOCOL
>);
>ASSERT_EFI_ERROR (Status);
>
>if (CommandLineBlob->Data == NULL) {
> KernelLoadedImage->LoadOptionsSize = 0;
>} else {
> //
> // Verify NUL-termination of the command line.
> //
> if (CommandLineBlob->Data[CommandLineBlob->Size - 1] != '\0') {
>DEBUG ((EFI_D_ERROR, "%a: kernel command line is not NUL-terminated\n",
> __FUNCTION__));
>Status = EFI_PROTOCOL_ERROR;
>goto UnloadKernelImage;
> --
> 2.19.1.3.g30247aa5d201
>
>
>
> Groups.io Links: You receive all messages sent to this group.
>
> View/Reply Online (#46710): https://edk2.groups.io/g/devel/message/46710
> Mute This Topic: https://groups.io/mt/33128626/1761188
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ard.biesheu...@linaro.org]
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46712): https://edk2.groups.io/g/devel/message/46712
Mute This Topic: https://groups.io/mt/33128626/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous Integration Phase 1
> -Original Message- > From: devel@edk2.groups.io On Behalf Of Laszlo Ersek > Sent: Tuesday, September 3, 2019 6:20 AM > To: Ni, Ray ; r...@edk2.groups.io; devel@edk2.groups.io; > Gao, Liming ; Kinney, > Michael D > Subject: Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous Integration > Phase 1 > > On 09/03/19 05:39, Ni, Ray wrote: > > Can someone draw a flow chart of who does what to help clarify? > > I don't see this as a huge change, relative to the current process. > > Before, it's always been up to the subsys maintainer to apply & rebase > the patches locally, pick up the feedback tags, and run at least *some* > build tests before pushing. > > After, the final git push is not to "origin" but to a personal branch on > github.com, and then a github PR is needed. If that fails, notify the > submitter. That's all, as far as I can see. > > > It sounds like maintainers are going to be the very important bridges > > between CI system and the patch owners (developers). > Important it is I agree but boring it is as well I have to say. > > Oh, it *is* absolutely boring. > > > Are maintainers still needed to be picked up/chosen/promoted from > > developers? > > Would you trust a person to apply, build-test, and push your UefiCpuPkg > patches, if that person had *zero* UefiCpuPkg development experience? > (Or even zero edk2 development experience?) Can we change the process a bit? 1. maintainers created pull requests on behave of the patch owners 2. patch owners can be notified automatically if pull requests fail 3. patch owners update the pull requests (I am not familiar to pull requests. I assume the rights of modifying pull requests and creating pull requests are separated. Are they?) So, maintainers only need to initiate the pull requests. It assumes when pull requests are initiated, everyone at least agrees the justifications of the changes are valid and the ways the changes are done. Thanks, Ray -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46711): https://edk2.groups.io/g/devel/message/46711 Mute This Topic: https://groups.io/mt/33122986/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] ArmVirtPkg/PlatformBootManagerLib: unload image on EFI_SECURITY_VIOLATION
The LoadImage() boot service is a bit unusual in that it allocates
resources in a particular failure case; namely, it produces a valid
"ImageHandle" when it returns EFI_SECURITY_VIOLATION. This is supposed to
happen e.g. when Secure Boot verification fails for the image, but the
platform policy for the particular image origin (such as "fixed media" or
"removable media") is DEFER_EXECUTE_ON_SECURITY_VIOLATION. The return code
allows platform logic to selectively override the verification failure,
and launch the image nonetheless.
ArmVirtPkg/PlatformBootManagerLib does not override EFI_SECURITY_VIOLATION
for the kernel image loaded from fw_cfg -- any LoadImage() error is
considered fatal. When we simply treat EFI_SECURITY_VIOLATION like any
other LoadImage() error, we leak the resources associated with
"KernelImageHandle". From a resource usage perspective,
EFI_SECURITY_VIOLATION must be considered "success", and rolled back.
Implement this rollback, without breaking the proper "nesting" of error
handling jumps and labels.
Cc: Ard Biesheuvel
Cc: Dandan Bi
Cc: Leif Lindholm
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1992
Fixes: 23d04b58e27b382bbd3f9b16ba9adb1cb203dad5
Signed-off-by: Laszlo Ersek
---
Notes:
Repo: https://github.com/lersek/edk2.git
Branch: ldimg_armvirt_bz1992
ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
index 3cc83e3b7b95..d3851fd75fa5 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
@@ -968,53 +968,60 @@ TryRunningQemuKernel (
//
// Create a device path for the kernel image to be loaded from that will call
// back into our file system.
//
KernelDevicePath = FileDevicePath (FileSystemHandle, KernelBlob->Name);
if (KernelDevicePath == NULL) {
DEBUG ((EFI_D_ERROR, "%a: failed to allocate kernel device path\n",
__FUNCTION__));
Status = EFI_OUT_OF_RESOURCES;
goto UninstallProtocols;
}
//
// Load the image. This should call back into our file system.
//
Status = gBS->LoadImage (
FALSE, // BootPolicy: exact match required
gImageHandle, // ParentImageHandle
KernelDevicePath,
NULL, // SourceBuffer
0, // SourceSize
);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "%a: LoadImage(): %r\n", __FUNCTION__, Status));
-goto FreeKernelDevicePath;
+if (Status != EFI_SECURITY_VIOLATION) {
+ goto FreeKernelDevicePath;
+}
+//
+// From the resource allocation perspective, EFI_SECURITY_VIOLATION means
+// "success", so we must roll back the image loading.
+//
+goto UnloadKernelImage;
}
//
// Construct the kernel command line.
//
Status = gBS->OpenProtocol (
KernelImageHandle,
,
(VOID **),
gImageHandle, // AgentHandle
NULL, // ControllerHandle
EFI_OPEN_PROTOCOL_GET_PROTOCOL
);
ASSERT_EFI_ERROR (Status);
if (CommandLineBlob->Data == NULL) {
KernelLoadedImage->LoadOptionsSize = 0;
} else {
//
// Verify NUL-termination of the command line.
//
if (CommandLineBlob->Data[CommandLineBlob->Size - 1] != '\0') {
DEBUG ((EFI_D_ERROR, "%a: kernel command line is not NUL-terminated\n",
__FUNCTION__));
Status = EFI_PROTOCOL_ERROR;
goto UnloadKernelImage;
--
2.19.1.3.g30247aa5d201
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46710): https://edk2.groups.io/g/devel/message/46710
Mute This Topic: https://groups.io/mt/33128626/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] Upcoming Event: TianoCore Design / Bug Triage - EMEA - Wed, 09/04/2019 8:00am-9:00am #cal-reminder
*Reminder:* TianoCore Design / Bug Triage - EMEA *When:* Wednesday, 4 September 2019, 8:00am to 9:00am, (GMT-07:00) America/Los Angeles *Where:* https://zoom.us/j/695893389 View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=503240 ) *Organizer:* Stephano Cetola stephano.cet...@linux.intel.com ( stephano.cet...@linux.intel.com?subject=Re:%20Event:%20TianoCore%20Design%20%2F%20Bug%20Triage%20-%20EMEA ) *Description:* Join Zoom Meeting https://zoom.us/j/695893389 One tap mobile +17207072699,,695893389# US +16465588656,,695893389# US (New York) Dial by your location +1 720 707 2699 US +1 646 558 8656 US (New York) Meeting ID: 695 893 389 Find your local number: https://zoom.us/u/abOtdJckxL -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46709): https://edk2.groups.io/g/devel/message/46709 Mute This Topic: https://groups.io/mt/33127452/21656 Mute #cal-reminder: https://groups.io/mk?hashtag=cal-reminder=3846945 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [edk2-test] [PATCH 1/1] uefi-sct/SctPkg: buffer overflow in NotifyFunctionTplEx()
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1976
CreateEventEx() may lead to a change in the memory map causing an
EFI_EVENT_GROUP_MEMORY_MAP_CHANGE. So in BBTestCreateEventEx_Func_Sub3() we
should only check for events triggered after the events have been set up.
Among other changes commit c093702f98ad (""uefi-sct/SctPkg:Fix flaw in
BBTestCreateEventEx_Func_Sub3) tried to adjust the event recording logic in
NotifyFunctionTplEx() to account for this.
The commit did not consider that CloseEvent() will release memory and
equally lead to EFI_EVENT_GROUP_MEMORY_MAP_CHANGE. NotifyFunctionTplEx()
does not check the limits of the buffer. So a buffer overrun occurs in this
case.
The easiest way to account for memory map changes by CreateEventEx() is to
initialize the event invocation records after setting up the events.
In function NotifyFunctionTplEx() check the index against the buffer
limits. Stop recording after MAX_TEST_EVENT_NUM events.
Fixes: c093702f98ad (""uefi-sct/SctPkg:Fix flaw in
BBTestCreateEventEx_Func_Sub3)
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heinrich Schuchardt
---
...rTaskPriorityServicesBBTestCreateEventEx.c | 19 +--
.../BlackBoxTest/Support.c| 55 +--
2 files changed, 27 insertions(+), 47 deletions(-)
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
index 4a8e44e2..40af6c07 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/EventTimerTaskPriorityServicesBBTestCreateEventEx.c
@@ -918,12 +918,11 @@ BBTestCreateEventEx_Func_Sub3 (
UINTN Buffer[MAX_TEST_EVENT_NUM + MAX_TEST_EVENT_NUM*2];
//
- // Initialize Buffer as SIGNAL_CONTEXT
+ // Initialize the event index. The event invocation records will be
+ // initialized later.
//
for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) {
Buffer[Index] = Index;
-Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(SIGNAL_CONTEXT);
-Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(SIGNAL_CONTEXT);
}
//
@@ -976,7 +975,17 @@ BBTestCreateEventEx_Func_Sub3 (
gtBS->CloseEvent (Event[1]);
return Status;
}
-
+
+ //
+ // CreateEventEx() may lead to a change in the memory map and trigger
+ // EFI_EVENT_GROUP_MEMORY_MAP_CHANGE itself. So initialize the event
+ // invocation records after creating the events.
+ //
+ for (Index = 0; Index < MAX_TEST_EVENT_NUM; Index ++) {
+Buffer[Index + MAX_TEST_EVENT_NUM + Index] = (UINTN)(SIGNAL_CONTEXT);
+Buffer[Index + MAX_TEST_EVENT_NUM + 1 + Index] = (UINTN)(SIGNAL_CONTEXT);
+ }
+
//
// Call AllocatePage to change the memorymap
//
@@ -1035,4 +1044,4 @@ BBTestCreateEventEx_Func_Sub3 (
//
return EFI_SUCCESS;
}
-#endif
\ No newline at end of file
+#endif
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
index c702f84d..0c900a3e 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/EventTimerTaskPriorityServices/BlackBoxTest/Support.c
@@ -58,58 +58,29 @@ NotifyFunctionTplEx(
EFI_TPL OldTpl;
UINTN EventIndex;
UINTN Index;
-
+
if (Context != NULL) {
Buffer = Context;
EventIndex = Buffer[0];
//
-// The special code check for the BBTestCreateEventEx_Func_Sub3
-// Besides AllocatePages(), CreateEventEx() may trigger the memorymap
-// change when it is out of resource in memory pool
-// Use SIGNAL_CONTEXT to block possible enter triggered by CreateEventEx
-//
-if (EventIndex != 2 && Buffer[4] == (UINTN)(SIGNAL_CONTEXT))
- return;
-
-//
-// It is the code execution path as expect
-// The overall layout buffer as below
-// Buffer[0] [1] [2] store 1st/2nd/3rd event index (start from 0)
-// Buffer[3] [5] [7] store the index of event notified
-// Buffer[4] [6] [8] store the tpl of notification function of 1st/2nd/3rd
event notified
+// The event's context is offset by EventIndex from the true buffer start.
+// Skip over the MAX_TEST_EVENT_NUM leading index entries.
+// A maximum of MAX_TEST_EVENT_NUM events can be recorded.
//
-// since 3rd event is created at notify tpl, 1nd/2rd event at callback
-// EventIndex should be 2 here for the first enter
-// Because Context points to Buffer[2] and
Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous Integration Phase 1
On 09/03/19 05:39, Ni, Ray wrote: > Can someone draw a flow chart of who does what to help clarify? I don't see this as a huge change, relative to the current process. Before, it's always been up to the subsys maintainer to apply & rebase the patches locally, pick up the feedback tags, and run at least *some* build tests before pushing. After, the final git push is not to "origin" but to a personal branch on github.com, and then a github PR is needed. If that fails, notify the submitter. That's all, as far as I can see. > It sounds like maintainers are going to be the very important bridges between > CI system and the patch owners (developers). Important it is I agree but > boring it is as well I have to say. Oh, it *is* absolutely boring. > Are maintainers still needed to be picked up/chosen/promoted from developers? Would you trust a person to apply, build-test, and push your UefiCpuPkg patches, if that person had *zero* UefiCpuPkg development experience? (Or even zero edk2 development experience?) Maintainers don't have to be intimately familiar with the subsystem that they are pushing a given set of patches for, but every bit of knowledge helps. So the answer is "technically no, but you'll regret it". Maintainership is *always* a chore. It always takes away resources from development activities, for the maintainer. In most projects, people alternate in a given maintainer role -- they keep replacing each other. The variance is mostly in how frequent this alternation is. In some projects, the role belongs to a group, and people cover the maintainer responsibilities in very quick succession. In other projects, people replace each other in maintainer roles when the current maintainer gets tired of the work, and steps down. Then someone volunteers to assume the role. Either way, the new subsys maintainer is almost always a seasoned developer in the subsystem. Thanks, Laszlo > >> -Original Message- >> From: r...@edk2.groups.io On Behalf Of Laszlo Ersek >> Sent: Friday, August 30, 2019 5:58 AM >> To: devel@edk2.groups.io; Gao, Liming ; >> r...@edk2.groups.io; Kinney, Michael D >> >> Subject: Re: [edk2-rfc] [edk2-devel] [RFC] EDK II Continuous Integration >> Phase 1 >> >> On 08/30/19 10:43, Liming Gao wrote: >>> Mike: >>> I add my comments. >>> -Original Message- From: r...@edk2.groups.io [mailto:r...@edk2.groups.io] On Behalf Of Michael D Kinney Sent: Friday, August 30, 2019 4:23 AM To: devel@edk2.groups.io; r...@edk2.groups.io Subject: [edk2-rfc] [RFC] EDK II Continuous Integration Phase 1 Hello, This is a proposal for a first step towards continuous integration for all TianoCore repositories to help improve to quality of commits and automate testing and release processes for all EDK II packages and platforms. This is based on work from a number of EDK II community members that have provide valuable input and evaluations. * Rebecca Cran Jenkins evaluation * Laszlo Ersek GitLab evaluation * Philippe Mathieu-Daudé GitLab evaluation * Sean Brogan Azure Pipelines and HBFA * Bret Barkelew Azure Pipelines and HBFA * Jiewen Yao HBFA The following link is a link to an EDK II WIKI page that contains a summary of the work to date. Please provide feedback in the EDK II mailing lists. The WIKI pages will be updated with input from the entire EDK II community. https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Continuous- Integration Proposal Phase 1 of adding continuous integration is limited to the edk2 repository. Additional repositories will be added later. The following changes are proposed: * Remove EDK II Maintainers write access to edk2 repository. Only EDK II Administrators will continue to have write access, and that should only be used to handle extraordinary events. * EDK II Maintainers use a GitHub Pull Request instead of push to commit a patch series to the edk2 repository. There are no other changes to the development and review process. The patch series is prepared in an EDK II maintainer branch with all commit message requirements met on each patch in the series. >>> >>> Will the maintainer manually provide pull request after the patch passes >>> the review? >> >> Yes. The maintainer will prepare a local branch that is rebased to >> master, and has all the mailing list feedback tags (Reviewed-by, etc) >> applied. The maintainer also does all the local testing that they >> usually do, just before the final "git push origin". >> >> Then, the final "git push origin" action is replaced by: >> (1) git push personal-repo topic-branch-pr >> (2) manual creation of a GitHub.com Pull Request, for the topic branch >> just pushed. >> >> That is, the final "git push origin" action is replaced with the
[edk2-devel] [edk2-test] [PATCH 1/1] uefi-sct/SctPkg: fix typo 'Remained test cases'
%s/Remained/Remaining/g
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heinrich Schuchardt
---
.../SctPkg/TestInfrastructure/SCT/Framework/Execute/Execute.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/Execute/Execute.c
b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/Execute/Execute.c
index 9e7f672e..6fd36914 100644
--- a/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/Execute/Execute.c
+++ b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/Execute/Execute.c
@@ -608,10 +608,10 @@ Routine Description:
//
while (TRUE) {
//
-// Calculate the number of remained cases
+// Calculate the number of remaining test cases
//
GetTestCaseRemainNum();
-SctPrint(L" Remained test cases: %d\n", Remain);
+SctPrint(L" Remaining test cases: %d\n", Remain);
//
// Send assertion to remotion computer in passive mode to inform case
begin.
--
2.23.0.rc1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46706): https://edk2.groups.io/g/devel/message/46706
Mute This Topic: https://groups.io/mt/33124786/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms][PATCH V2 4/6] KabylakeSiliconPkg: Clean up duplicated SmramMemoryReserve.h files
Reviewed-by: Chasel Chiu
> -Original Message-
> From: Chen, Marc W
> Sent: Monday, September 2, 2019 11:36 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Kubacki, Michael A
> ; Chaganty, Rangasai V
> ; Desimone, Nathaniel L
>
> Subject: [edk2-platforms][PATCH V2 4/6] KabylakeSiliconPkg: Clean up
> duplicated SmramMemoryReserve.h files
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2108
>
> SmramMemoryReserve.h has been added into
> Edk2\MdePkg\Include\Guid\SmramMemoryReserve.h.
>
> The duplicated header file can be cleaned up.
> Edk2Platforms\Silicon\Intel\KabylakeSiliconPkg\SampleCode\IntelFramework
> Pkg\Include\Guid\SmramMemoryReserve.h
>
> Cc: Chasel Chiu
> Cc: Michael Kubacki
> Cc: Sai Chaganty
>
> Co-authored-by: Nate DeSimone
> Signed-off-by: Marc W Chen
> Signed-off-by: Nate DeSimone
> ---
> .../Include/Guid/SmramMemoryReserve.h | 54 ---
> Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec| 4 --
> .../SystemAgent/SmmAccess/Dxe/SmmAccess.inf | 4 +-
> .../SmmAccess/Dxe/SmmAccessDriver.c | 4 +-
> 4 files changed, 4 insertions(+), 62 deletions(-) delete mode 100644
> Silicon/Intel/KabylakeSiliconPkg/SampleCode/IntelFrameworkPkg/Include/Gu
> id/SmramMemoryReserve.h
>
> diff --git
> a/Silicon/Intel/KabylakeSiliconPkg/SampleCode/IntelFrameworkPkg/Include/
> Guid/SmramMemoryReserve.h
> b/Silicon/Intel/KabylakeSiliconPkg/SampleCode/IntelFrameworkPkg/Include/
> Guid/SmramMemoryReserve.h
> deleted file mode 100644
> index 9918c768ba..00
> ---
> a/Silicon/Intel/KabylakeSiliconPkg/SampleCode/IntelFrameworkPkg/Include/
> Guid/SmramMemoryReserve.h
> @@ -1,54 +0,0 @@
> -/** @file
> - Definition of GUIDed HOB for reserving SMRAM regions.
> -
> - This file defines:
> - * the GUID used to identify the GUID HOB for reserving SMRAM regions.
> - * the data structure of SMRAM descriptor to describe SMRAM candidate
> regions
> - * values of state of SMRAM candidate regions
> - * the GUID specific data structure of HOB for reserving SMRAM regions.
> - This GUIDed HOB can be used to convey the existence of the T-SEG
> reservation and H-SEG usage
> -
> -Copyright (c) 2007 - 2010, Intel Corporation. All rights reserved.
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> - @par Revision Reference:
> - GUIDs defined in SmmCis spec version 0.9.
> -
> -**/
> -
> -#ifndef _EFI_SMM_PEI_SMRAM_MEMORY_RESERVE_H_
> -#define _EFI_SMM_PEI_SMRAM_MEMORY_RESERVE_H_
> -
> -#define EFI_SMM_PEI_SMRAM_MEMORY_RESERVE \
> - { \
> -0x6dadf1d1, 0xd4cc, 0x4910, {0xbb, 0x6e, 0x82, 0xb1, 0xfd, 0x80, 0xff,
> 0x3d } \
> - }
> -
> -/**
> -* GUID specific data structure of HOB for reserving SMRAM regions.
> -*
> -* Inconsistent with specification here:
> -* EFI_HOB_SMRAM_DESCRIPTOR_BLOCK has been changed to
> EFI_SMRAM_HOB_DESCRIPTOR_BLOCK.
> -* This inconsistency is kept in code in order for backward compatibility.
> -**/
> -typedef struct {
> - ///
> - /// Designates the number of possible regions in the system
> - /// that can be usable for SMRAM.
> - ///
> - /// Inconsistent with specification here:
> - /// In Framework SMM CIS 0.91 specification, it defines the field type as
> UINTN.
> - /// However, HOBs are supposed to be CPU neutral, so UINT32 should be
> used instead.
> - ///
> - UINT32NumberOfSmmReservedRegions;
> - ///
> - /// Used throughout this protocol to describe the candidate
> - /// regions for SMRAM that are supported by this platform.
> - ///
> - EFI_SMRAM_DESCRIPTOR Descriptor[1];
> -} EFI_SMRAM_HOB_DESCRIPTOR_BLOCK;
> -
> -extern EFI_GUID gEfiSmmPeiSmramMemoryReserveGuid;
> -
> -#endif
> -
> diff --git a/Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec
> b/Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec
> index a9f1c0f092..3881671757 100644
> --- a/Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec
> +++ b/Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec
> @@ -63,10 +63,6 @@ gEfiMemoryTypeInformationGuid = {0x4c19049f,
> 0x4137, 0x4dd3, {0x9c, 0x10, 0x8b
> gEfiCapsuleVendorGuid = {0x711c703f, 0xc285, 0x4b10, {0xa3,
> 0xb0, 0x36, 0xec, 0xbd, 0x3c, 0x8b, 0xe2}}
> gEfiConsoleOutDeviceGuid = {0xd3b36f2c, 0xd551, 0x11d4, {0x9a,
> 0x46, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d}}
> ##
> -## IntelFrameworkPkg
> -##
> -gEfiSmmPeiSmramMemoryReserveGuid = {0x6dadf1d1, 0xd4cc, 0x4910,
> {0xbb, 0x6e, 0x82, 0xb1, 0xfd, 0x80, 0xff, 0x3d}} -## ## ##
> gSmbiosProcessorInfoHobGuid = {0xe6d73d92, 0xff56, 0x4146, {0xaf, 0xac,
> 0x1c, 0x18, 0x81, 0x7d, 0x68, 0x71}} diff --git
> a/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/SmmAccess/Dxe/SmmAccess
> .inf
> b/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/SmmAccess/Dxe/SmmAccess
> .inf
> index 93ab408206..287e631689 100644
> ---
> a/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/SmmAccess/Dxe/SmmAccess
> .inf
> +++
> b/Silicon/Intel/KabylakeSiliconPkg/SystemAgent/SmmAccess/Dxe/SmmAcce
> +++ ss.inf
> @@ -3,7 +3,7 @@
> #
> # {1323C7F8-DAD5-4126-A54B-7A05FBF4151}
> #
> -#
Re: [edk2-devel] [edk2-platforms][PATCH V2 3/6] CoffeelakeSiliconPkg: Clean up duplicated SmramMemoryReserve.h files
Reviewed-by: Chasel Chiu
> -Original Message-
> From: Chen, Marc W
> Sent: Monday, September 2, 2019 11:36 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Kubacki, Michael A
> ; Chaganty, Rangasai V
> ; Desimone, Nathaniel L
>
> Subject: [edk2-platforms][PATCH V2 3/6] CoffeelakeSiliconPkg: Clean up
> duplicated SmramMemoryReserve.h files
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2108
>
> SmramMemoryReserve.h has been added into
> Edk2\MdePkg\Include\Guid\SmramMemoryReserve.h.
>
> The duplicated header file can be cleaned up.
> Edk2Platforms\Silicon\Intel\CoffeelakeSiliconPkg\SampleCode\IntelFramewor
> kPkg\Include\Guid\SmramMemoryReserve.h
>
> Cc: Chasel Chiu
> Cc: Michael Kubacki
> Cc: Sai Chaganty
>
> Co-authored-by: Nate DeSimone
> Signed-off-by: Marc W Chen
> Signed-off-by: Nate DeSimone
> ---
> .../Include/Guid/SmramMemoryReserve.h | 51 ---
> Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec | 5 --
> .../SystemAgent/SmmAccess/Dxe/SmmAccess.inf | 2 +-
> .../SmmAccess/Dxe/SmmAccessDriver.c | 2 +-
> 4 files changed, 2 insertions(+), 58 deletions(-) delete mode 100644
> Silicon/Intel/CoffeelakeSiliconPkg/SampleCode/IntelFrameworkPkg/Include/
> Guid/SmramMemoryReserve.h
>
> diff --git
> a/Silicon/Intel/CoffeelakeSiliconPkg/SampleCode/IntelFrameworkPkg/Include
> /Guid/SmramMemoryReserve.h
> b/Silicon/Intel/CoffeelakeSiliconPkg/SampleCode/IntelFrameworkPkg/Includ
> e/Guid/SmramMemoryReserve.h
> deleted file mode 100644
> index 862a7c8aea..00
> ---
> a/Silicon/Intel/CoffeelakeSiliconPkg/SampleCode/IntelFrameworkPkg/Include
> /Guid/SmramMemoryReserve.h
> @@ -1,51 +0,0 @@
> -/** @file
> - Definition of GUIDed HOB for reserving SMRAM regions.
> -
> - This file defines:
> - * the GUID used to identify the GUID HOB for reserving SMRAM regions.
> - * the data structure of SMRAM descriptor to describe SMRAM candidate
> regions
> - * values of state of SMRAM candidate regions
> - * the GUID specific data structure of HOB for reserving SMRAM regions.
> - This GUIDed HOB can be used to convey the existence of the T-SEG
> reservation and H-SEG usage
> -
> - Copyright (c) 2019 Intel Corporation. All rights reserved.
> -
> - SPDX-License-Identifier: BSD-2-Clause-Patent -**/
> -
> -#ifndef _EFI_SMM_PEI_SMRAM_MEMORY_RESERVE_H_
> -#define _EFI_SMM_PEI_SMRAM_MEMORY_RESERVE_H_
> -
> -#define EFI_SMM_PEI_SMRAM_MEMORY_RESERVE \
> - { \
> -0x6dadf1d1, 0xd4cc, 0x4910, {0xbb, 0x6e, 0x82, 0xb1, 0xfd, 0x80, 0xff,
> 0x3d } \
> - }
> -
> -/**
> -* GUID specific data structure of HOB for reserving SMRAM regions.
> -*
> -* Inconsistent with specification here:
> -* EFI_HOB_SMRAM_DESCRIPTOR_BLOCK has been changed to
> EFI_SMRAM_HOB_DESCRIPTOR_BLOCK.
> -* This inconsistency is kept in code in order for backward compatibility.
> -**/
> -typedef struct {
> - ///
> - /// Designates the number of possible regions in the system
> - /// that can be usable for SMRAM.
> - ///
> - /// Inconsistent with specification here:
> - /// In Framework SMM CIS 0.91 specification, it defines the field type as
> UINTN.
> - /// However, HOBs are supposed to be CPU neutral, so UINT32 should be
> used instead.
> - ///
> - UINT32NumberOfSmmReservedRegions;
> - ///
> - /// Used throughout this protocol to describe the candidate
> - /// regions for SMRAM that are supported by this platform.
> - ///
> - EFI_SMRAM_DESCRIPTOR Descriptor[1];
> -} EFI_SMRAM_HOB_DESCRIPTOR_BLOCK;
> -
> -extern EFI_GUID gEfiSmmPeiSmramMemoryReserveGuid;
> -
> -#endif
> -
> diff --git a/Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec
> b/Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec
> index fa8c11e93d..6cf894498d 100644
> --- a/Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec
> +++ b/Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec
> @@ -54,11 +54,6 @@ gEfiMemoryTypeInformationGuid = {0x4c19049f,
> 0x4137, 0x4dd3, {0x9c, 0x10, 0x8b gEfiCapsuleVendorGuid =
> {0x711c703f, 0xc285, 0x4b10, {0xa3, 0xb0, 0x36, 0xec, 0xbd, 0x3c, 0x8b, 0xe2}}
> gEfiConsoleOutDeviceGuid = { 0xd3b36f2c, 0xd551, 0x11d4, { 0x9a, 0x46, 0x0,
> 0x90, 0x27, 0x3f, 0xc1, 0x4d}}
>
> -##
> -## IntelFrameworkPkg
> -##
> -gEfiSmmPeiSmramMemoryReserveGuid = {0x6dadf1d1, 0xd4cc, 0x4910,
> {0xbb, 0x6e, 0x82, 0xb1, 0xfd, 0x80, 0xff, 0x3d}}
> -
> ##
> ## Common
> ##
> diff --git
> a/Silicon/Intel/CoffeelakeSiliconPkg/SystemAgent/SmmAccess/Dxe/SmmAcce
> ss.inf
> b/Silicon/Intel/CoffeelakeSiliconPkg/SystemAgent/SmmAccess/Dxe/SmmAcce
> ss.inf
> index 9356781c9e..bb1944c9ec 100644
> ---
> a/Silicon/Intel/CoffeelakeSiliconPkg/SystemAgent/SmmAccess/Dxe/SmmAcce
> ss.inf
> +++
> b/Silicon/Intel/CoffeelakeSiliconPkg/SystemAgent/SmmAccess/Dxe/SmmAc
> +++ cess.inf
> @@ -41,7 +41,7 @@ gEfiSmmAccess2ProtocolGuid ## PRODUCES
>
>
> [Guids]
> -gEfiSmmPeiSmramMemoryReserveGuid
> +gEfiSmmSmramMemoryGuid
>
>
> [Depex]
> diff --git
> a/Silicon/Intel/CoffeelakeSiliconPkg/SystemAgent/SmmAccess/Dxe/SmmAcce
>
Re: [edk2-devel] [edk2-platforms][PATCH V2 1/6] MinPlatformPkg: Clean up duplicated SmramMemoryReserve.h files
Reviewed-by: Chasel Chiu
> -Original Message-
> From: Chen, Marc W
> Sent: Monday, September 2, 2019 11:36 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A ; Chiu, Chasel
> ; Desimone, Nathaniel L
> ; Gao, Liming
> Subject: [edk2-platforms][PATCH V2 1/6] MinPlatformPkg: Clean up
> duplicated SmramMemoryReserve.h files
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2108
>
> SmramMemoryReserve.h has been added into
> Edk2\MdePkg\Include\Guid\SmramMemoryReserve.h.
>
> The duplicated header file can be clean up.
> Edk2Platforms\Platform\Intel\MinPlatformPkg\Include\Guid\SmramMemory
> Reserve.h
>
> Cc: Michael Kubacki
> Cc: Chasel Chiu
> Cc: Nate DeSimone
> Cc: Liming Gao
>
> Co-authored-by: Nate DeSimone
> Signed-off-by: Marc W Chen
> Signed-off-by: Nate DeSimone
> ---
> .../Include/Guid/SmramMemoryReserve.h | 54 ---
> .../Intel/MinPlatformPkg/MinPlatformPkg.dec | 5 --
> .../PlatformInitPei/PlatformInitPostMem.c | 4 +-
> .../PlatformInitPei/PlatformInitPostMem.inf | 4 +-
> .../TestPointCheckLib/PeiCheckSmmInfo.c | 6 +--
> 5 files changed, 7 insertions(+), 66 deletions(-) delete mode 100644
> Platform/Intel/MinPlatformPkg/Include/Guid/SmramMemoryReserve.h
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Include/Guid/SmramMemoryReserve.h
> b/Platform/Intel/MinPlatformPkg/Include/Guid/SmramMemoryReserve.h
> deleted file mode 100644
> index 9918c768ba..00
> --- a/Platform/Intel/MinPlatformPkg/Include/Guid/SmramMemoryReserve.h
> @@ -1,54 +0,0 @@
> -/** @file
> - Definition of GUIDed HOB for reserving SMRAM regions.
> -
> - This file defines:
> - * the GUID used to identify the GUID HOB for reserving SMRAM regions.
> - * the data structure of SMRAM descriptor to describe SMRAM candidate
> regions
> - * values of state of SMRAM candidate regions
> - * the GUID specific data structure of HOB for reserving SMRAM regions.
> - This GUIDed HOB can be used to convey the existence of the T-SEG
> reservation and H-SEG usage
> -
> -Copyright (c) 2007 - 2010, Intel Corporation. All rights reserved.
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> - @par Revision Reference:
> - GUIDs defined in SmmCis spec version 0.9.
> -
> -**/
> -
> -#ifndef _EFI_SMM_PEI_SMRAM_MEMORY_RESERVE_H_
> -#define _EFI_SMM_PEI_SMRAM_MEMORY_RESERVE_H_
> -
> -#define EFI_SMM_PEI_SMRAM_MEMORY_RESERVE \
> - { \
> -0x6dadf1d1, 0xd4cc, 0x4910, {0xbb, 0x6e, 0x82, 0xb1, 0xfd, 0x80, 0xff,
> 0x3d } \
> - }
> -
> -/**
> -* GUID specific data structure of HOB for reserving SMRAM regions.
> -*
> -* Inconsistent with specification here:
> -* EFI_HOB_SMRAM_DESCRIPTOR_BLOCK has been changed to
> EFI_SMRAM_HOB_DESCRIPTOR_BLOCK.
> -* This inconsistency is kept in code in order for backward compatibility.
> -**/
> -typedef struct {
> - ///
> - /// Designates the number of possible regions in the system
> - /// that can be usable for SMRAM.
> - ///
> - /// Inconsistent with specification here:
> - /// In Framework SMM CIS 0.91 specification, it defines the field type as
> UINTN.
> - /// However, HOBs are supposed to be CPU neutral, so UINT32 should be
> used instead.
> - ///
> - UINT32NumberOfSmmReservedRegions;
> - ///
> - /// Used throughout this protocol to describe the candidate
> - /// regions for SMRAM that are supported by this platform.
> - ///
> - EFI_SMRAM_DESCRIPTOR Descriptor[1];
> -} EFI_SMRAM_HOB_DESCRIPTOR_BLOCK;
> -
> -extern EFI_GUID gEfiSmmPeiSmramMemoryReserveGuid;
> -
> -#endif
> -
> diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> index a642f9f3a3..d79f5ec1bd 100644
> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> @@ -42,11 +42,6 @@ gBoardNotificationInitGuid = {0x78dbcabf, 0xc544,
> 0x4e6f, {0xaf, 0x3a, 0x71, 0x1
> gBoardAcpiTableGuid= {0xd70e9f57, 0x69f, 0x4bef, {0x96, 0xc0,
> 0x84, 0x74, 0xf4, 0xa2, 0x5f, 0x3a}}
> gBoardAcpiEnableGuid = {0x9727b610, 0xf645, 0x4429, {0x89, 0x21,
> 0x2c, 0x2b, 0x58, 0xdc, 0xbb, 0xa}}
>
> -##
> -## IntelFrameworkPkg
> -##
> -gEfiSmmPeiSmramMemoryReserveGuid = {0x6dadf1d1, 0xd4cc, 0x4910,
> {0xbb, 0x6e, 0x82, 0xb1, 0xfd, 0x80, 0xff, 0x3d}}
> -
> gDefaultDataFileGuid= { 0x1ae42876, 0x008f,
> 0x4161, { 0xb2, 0xb7, 0x1c, 0x0d, 0x15, 0xc5, 0xef, 0x43 }}
> gDefaultDataOptSizeFileGuid = { 0x003e7b41, 0x98a2,
> 0x4be2, { 0xb2, 0x7a, 0x6c, 0x30, 0xc7, 0x65, 0x52, 0x25 }}
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPo
> stMem.c
> b/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitP
> ostMem.c
> index 00877593bc..70e6b9a495 100644
> ---
> a/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPo
> stMem.c
> +++ b/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/Platfor
> +++ mInitPostMem.c
>
Re: [edk2-devel] [edk2-test] [PATCH] uefi-sct/SctPkg: StrUpr() test for cyrillic letters
Reviewed-by: Eric Jin
-Original Message-
From: Heinrich Schuchardt
Sent: Monday, September 2, 2019 5:57 PM
To: devel@edk2.groups.io
Cc: Jin, Eric ; Supreeth Venkatesh
; Stephano Cetola
; Heinrich Schuchardt
Subject: [edk2-test] [PATCH] uefi-sct/SctPkg: StrUpr() test for cyrillic letters
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1911
The test of StrUpr() of the UnicodeCollationProtocol2 uses the following test
string:
L"\x30\x50[A-D]\x40\x20\x30\x50f\x40\x20"
This string contains the Unicode code point 0x050f (CYRILLIC SMALL LETTER KOMI
TJE). The correct conversion to upper case is 0x050e (CYRILLIC CAPITAL LETTER
KOMI TJE).
The SCT code uses function ToUpper() which is not a part of the SCT code itself
for the verification. As the EDK2 implementation of StrUpr() does not support
upper-casing cyrillic letters it does not convert 0x050f but leaves it
unchanged.
This leads to rejecting the output of UEFI implementations fully supporting
cyrillic letters.
Replace cyrillic 0x50f by 0x50 ('P') to avoid false positives.
Signed-off-by: Heinrich Schuchardt
---
.../BlackBoxTest/UnicodeCollation2BBTestFunction.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 a8652a5c..653b263a 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/UnicodeCollation2/BlackBoxTest/UnicodeCollation2BBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/UnicodeCollation2/Black
+++ BoxTest/UnicodeCollation2BBTestFunction.c
@@ -451,7 +451,7 @@ BBTestStrUprFunctionAutoTest (
CHAR16 *TestData[] ={
L"\x21\x22\x31\x32\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4A\x4B\x4C\x4D\x4E\x4F\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5Ax61\x62\x7D\x7E",
L"\x30\x50[abcdzyxw!)(@#*]\x40\x20\x30\x50\ab\x40\x20",-
L"\x30\x50[A-D]\x40\x20\x30\x50f\x40\x20",+
L"\x30\x50[A-D]\x40\x20\x30\x50\x40\x20",
L""
}; --
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46702): https://edk2.groups.io/g/devel/message/46702
Mute This Topic: https://groups.io/mt/33109522/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 1/1] ShellPkg/CommandLib: avoid NULL derefence and memory leak
This patch has passed the package maintainer review. So, I push this change @8b8e91584555b6193f2099a36502763b47501533. Thanks Liming >-Original Message- >From: Desimone, Nathaniel L >Sent: Tuesday, September 03, 2019 1:00 PM >To: devel@edk2.groups.io; xypron.g...@gmx.de; Carsey, Jaben >; Gao, Zhichao >Cc: Ni, Ray ; Leif Lindholm ; Gao, >Liming ; Stephano Cetola > >Subject: RE: [edk2-devel] [PATCH v3 1/1] ShellPkg/CommandLib: avoid NULL >derefence and memory leak > >From https://github.com/tianocore/edk2/blob/master/Maintainers.txt: > >M: Jaben Carsey >M: Ray Ni > >-Original Message- >From: devel@edk2.groups.io On Behalf Of Heinrich >Schuchardt >Sent: Monday, September 2, 2019 3:15 AM >To: Carsey, Jaben ; devel@edk2.groups.io; Gao, >Zhichao >Cc: Ni, Ray ; Leif Lindholm ; Gao, >Liming ; Stephano Cetola > >Subject: Re: [edk2-devel] [PATCH v3 1/1] ShellPkg/CommandLib: avoid NULL >derefence and memory leak > >On 5/10/19 4:32 PM, Carsey, Jaben wrote: >> Reviewed-by: Jaben Carsey >> >> Code change looks good visually. > >Somehow this patch > >https://edk2.groups.io/g/devel/message/40395 > >was never merged. > >Who is the maintainer for the ShellPkg? > >Best regards > >Heinrich > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46701): https://edk2.groups.io/g/devel/message/46701 Mute This Topic: https://groups.io/mt/31573312/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
