[edk2] [PATCH v2] IntelSiliconPkg/MicrocodeUpdateDxe: Error message enhancement
The error message of ExtendedTableCount is not clear. Add the count
number into the debug message.
https://bugzilla.tianocore.org/show_bug.cgi?id=1500
v2: Change the judgment condition to return error message when
ExtendedTableCount is not equal to target value.
Cc: Ray Ni
Cc: Rangasai V Chaganty
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Shenglei Zhang
---
.../Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git
a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
index 9098712c2f..037b2433a6 100644
--- a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
+++ b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
@@ -517,8 +517,8 @@ VerifyMicrocode (
// Checksum correct
//
ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
- if (ExtendedTableCount > (ExtendedTableLength -
sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) /
sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
-DEBUG((DEBUG_ERROR, "VerifyMicrocode - ExtendedTableCount too
big\n"));
+ if (ExtendedTableCount != (ExtendedTableLength -
sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) /
sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
+DEBUG((DEBUG_ERROR, "VerifyMicrocode - ExtendedTableCount %d is
incorrect\n", ExtendedTableCount));
} else {
ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE
*)(ExtendedTableHeader + 1);
for (Index = 0; Index < ExtendedTableCount; Index++) {
--
2.18.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/SecCore: Wrong Debug Information for SecCore
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1533
When SecCore and PeiCore in different FV, current
implementation still assuming SecCore and PeiCore are in
the same FV.
To fix this issue 2 FVs will be input parameters for
FindAndReportEntryPoints () and SecCore and PeiCore will
be found in each FV and correct debug information will
be reported.
Test: Booted with internal platform successfully.
Cc: Eric Dong
Cc: Ray Ni
Cc: Laszlo Ersek
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chasel Chiu
---
UefiCpuPkg/SecCore/FindPeiCore.c | 60
++--
UefiCpuPkg/SecCore/SecMain.c | 10 --
UefiCpuPkg/SecCore/SecMain.h | 8 +---
3 files changed, 43 insertions(+), 35 deletions(-)
diff --git a/UefiCpuPkg/SecCore/FindPeiCore.c b/UefiCpuPkg/SecCore/FindPeiCore.c
index bb9c434d1e..6f2046ad95 100644
--- a/UefiCpuPkg/SecCore/FindPeiCore.c
+++ b/UefiCpuPkg/SecCore/FindPeiCore.c
@@ -1,7 +1,7 @@
/** @file
Locate the entry point for the PEI Core
- Copyright (c) 2008 - 2011, Intel Corporation. All rights reserved.
+ Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
which accompanies this distribution. The full text of the license may be
found at
@@ -19,17 +19,17 @@
/**
Find core image base.
- @param BootFirmwareVolumePtrPoint to the boot firmware volume.
- @param SecCoreImageBase The base address of the SEC core image.
- @param PeiCoreImageBase The base address of the PEI core image.
+ @param FirmwareVolumePtrPoint to the firmware volume for finding
core image.
+ @param FileType The FileType for searching, either SecCore
or PeiCore.
+ @param CoreImageBaseThe base address of the core image.
**/
EFI_STATUS
EFIAPI
FindImageBase (
- IN EFI_FIRMWARE_VOLUME_HEADER *BootFirmwareVolumePtr,
- OUT EFI_PHYSICAL_ADDRESS *SecCoreImageBase,
- OUT EFI_PHYSICAL_ADDRESS *PeiCoreImageBase
+ IN EFI_FIRMWARE_VOLUME_HEADER *FirmwareVolumePtr,
+ IN EFI_FV_FILETYPE FileType,
+ OUT EFI_PHYSICAL_ADDRESS *CoreImageBase
)
{
EFI_PHYSICAL_ADDRESSCurrentAddress;
@@ -40,16 +40,15 @@ FindImageBase (
EFI_COMMON_SECTION_HEADER *Section;
EFI_PHYSICAL_ADDRESSEndOfSection;
- *SecCoreImageBase = 0;
- *PeiCoreImageBase = 0;
+ *CoreImageBase = 0;
- CurrentAddress = (EFI_PHYSICAL_ADDRESS)(UINTN) BootFirmwareVolumePtr;
- EndOfFirmwareVolume = CurrentAddress + BootFirmwareVolumePtr->FvLength;
+ CurrentAddress = (EFI_PHYSICAL_ADDRESS)(UINTN) FirmwareVolumePtr;
+ EndOfFirmwareVolume = CurrentAddress + FirmwareVolumePtr->FvLength;
//
// Loop through the FFS files in the Boot Firmware Volume
//
- for (EndOfFile = CurrentAddress + BootFirmwareVolumePtr->HeaderLength; ; ) {
+ for (EndOfFile = CurrentAddress + FirmwareVolumePtr->HeaderLength; ; ) {
CurrentAddress = (EndOfFile + 7) & 0xfff8ULL;
if (CurrentAddress > EndOfFirmwareVolume) {
@@ -75,10 +74,9 @@ FindImageBase (
}
//
-// Look for SEC Core / PEI Core files
+// Look for particular Core file (either SEC Core or PEI Core)
//
-if (File->Type != EFI_FV_FILETYPE_SECURITY_CORE &&
-File->Type != EFI_FV_FILETYPE_PEI_CORE) {
+if (File->Type != FileType) {
continue;
}
@@ -115,17 +113,11 @@ FindImageBase (
// Look for executable sections
//
if (Section->Type == EFI_SECTION_PE32 || Section->Type ==
EFI_SECTION_TE) {
-if (File->Type == EFI_FV_FILETYPE_SECURITY_CORE) {
+if (File->Type == FileType) {
if (IS_SECTION2 (Section)) {
-*SecCoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) Section
+ sizeof (EFI_COMMON_SECTION_HEADER2));
+*CoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) Section +
sizeof (EFI_COMMON_SECTION_HEADER2));
} else {
-*SecCoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) Section
+ sizeof (EFI_COMMON_SECTION_HEADER));
- }
-} else {
- if (IS_SECTION2 (Section)) {
-*PeiCoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) Section
+ sizeof (EFI_COMMON_SECTION_HEADER2));
- } else {
-*PeiCoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) Section
+ sizeof (EFI_COMMON_SECTION_HEADER));
+*CoreImageBase = (PHYSICAL_ADDRESS) (UINTN) ((UINT8 *) Section +
sizeof (EFI_COMMON_SECTION_HEADER));
}
}
break;
@@ -133,9 +125,9 @@ FindImageBase (
}
//
-// Both SEC Core and PEI Core images found
+// Either SEC Core or PEI Core images found
//
-if (*SecCoreImageBase != 0 && *PeiCoreImageBase != 0) {
+if (*CoreImageBase != 0) {
[edk2] [PATCH] UefiCpuPkg/Microcode: Fix incorrect checksum issue for extended table
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
The following Microcode payload format is define in SDM spec.
Payload: |MicrocodeHeader|MicrocodeBinary|ExtendedHeader|ExtendedTable|.
When we verify the CheckSum32 with ExtendedTable, we should use the fields
of ExtendedTable to replace corresponding fields in MicrocodeHeader,
and then calculate the CheckSum32 with MicrocodeHeader+MicrocodeBinary.
This patch already verified on ICL platform.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen
Cc: Ray Ni
Cc: Eric Dong
Cc: Zhang Chao B
---
UefiCpuPkg/Library/MpInitLib/Microcode.c | 82
1 file changed, 73 insertions(+), 9 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index d84344c6f5..e1f661d6b1 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -35,6 +35,42 @@ GetCurrentMicrocodeSignature (
/**
Detect whether specified processor can find matching microcode patch and
load it.
+ Microcode Payload as the following format:
+ ++--+
+ | CPU_MICROCODE_HEADER | |
+ ++ CheckSum Part1 |
+ |Microcode Binary| |
+ ++--+
+ | CPU_MICROCODE_EXTENDED_TABLE_HEADER | |
+ ++ CheckSum Part2 |
+ | CPU_MICROCODE_EXTENDED_TABLE | |
+ | ... | |
+ ++--+
+
+ There may by multiple CPU_MICROCODE_EXTENDED_TABLE in this format.
+ The count of CPU_MICROCODE_EXTENDED_TABLE is indicated by
ExtendedSignatureCount
+ of CPU_MICROCODE_EXTENDED_TABLE_HEADER structure.
+
+ When we are trying to verify the CheckSum32 with extended table.
+ We should use the fields of exnteded table to replace the corresponding
+ fields in CPU_MICROCODE_HEADER structure, and recalculate the
+ CheckSum32 with CPU_MICROCODE_HEADER + Microcode Binary. We named
+ it as CheckSum Part3.
+
+ The CheckSum Part2 is used to verify the CPU_MICROCODE_EXTENDED_TABLE_HEADER
+ and CPU_MICROCODE_EXTENDED_TABLE parts. We should make sure CheckSum Part2
+ is correct before we are going to verify each CPU_MICROCODE_EXTENDED_TABLE.
+
+ Only ProcessorSignature, ProcessorFlag and CheckSum are different between
+ CheckSum Part1 and CheckSum Part3. To avoid multiple computing CheckSum
Part3.
+ Save an in-complete CheckSum32 from CheckSum Part1 for common parts.
+ When we are going to calculate CheckSum32, just should use the corresponding
part
+ of the ProcessorSignature, ProcessorFlag and CheckSum with in-complete
CheckSum32.
+
+ Notes: CheckSum32 is not a strong verification.
+ It does not guarantee that the data has not been modified.
+ CPU has its own mechanism to verify Microcode Binary part.
+
@param[in] CpuMpDataThe pointer to CPU MP Data structure.
@param[in] IsBspCallIn Indicate whether the caller is BSP or not.
**/
@@ -57,6 +93,7 @@ MicrocodeDetect (
UINT32 LatestRevision;
UINTN TotalSize;
UINT32 CheckSum32;
+ UINT32 InCompleteCheckSum32;
BOOLEAN CorrectMicrocode;
VOID*MicrocodeData;
MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr;
@@ -121,6 +158,25 @@ MicrocodeDetect (
MicrocodeData = NULL;
MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress +
CpuMpData->MicrocodePatchRegionSize);
MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN)
CpuMpData->MicrocodePatchAddress;
+
+ //
+ // Save an in-complete CheckSum32 from CheckSum Part1 for common parts.
+ //
+ if (MicrocodeEntryPoint->DataSize == 0) {
+InCompleteCheckSum32 = CalculateSum32 (
+ (UINT32 *) MicrocodeEntryPoint,
+ sizeof (CPU_MICROCODE_HEADER) + 2000
+ );
+ } else {
+InCompleteCheckSum32 = CalculateSum32 (
+ (UINT32 *) MicrocodeEntryPoint,
+ sizeof (CPU_MICROCODE_HEADER) +
MicrocodeEntryPoint->DataSize
+ );
+ }
+ InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorSignature.Uint32;
+ InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
+ InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
+
do {
//
// Check if the microcode is for the Cpu and the version is newer
@@ -137,14 +193,13 @@ MicrocodeDetect (
MicrocodeEntryPoint->UpdateRevision > LatestRevision &&
(MicrocodeEntryPo
Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set memory <1MB to UC
Hi Ray,
Sorry for later response.
Reviewed-by: Eric Dong
Thanks,
Eric
> -Original Message-
> From: Ni, Ray
> Sent: Monday, January 21, 2019 11:17 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray ; Dong, Eric ; Chiu,
> Chasel
> Subject: [PATCH] UefiCpuPkg/MtrrLib: Fix a bug that may wrongly set
> memory <1MB to UC
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1481
>
> Today's MtrrLib contains a bug, for example:
> when the original cache setting is WB for [0xF_, 0xF_8000) and, a new
> request to set [0xF_, 0xF_4000) to WP, the cache setting for [0xF_4000,
> 0xF_8000) is reset to UC.
>
> The reason is when MtrrLibSetBelow1MBMemoryAttribute() is called the
> WorkingFixedSettings doesn't contain the actual MSR value stored in
> hardware, but when writing the fixed MTRRs, the code logic assumes
> WorkingFixedSettings contains the actual MSR value.
>
> The new fix is to change MtrrLibSetBelow1MBMemoryAttribute() to
> calculate the correct ClearMasks[] and OrMasks[], and use them directly
> when writing the fixed MTRRs.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ray Ni
> Cc: Eric Dong
> Cc: Chasel Chiu
> ---
> UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 59 +---
> 1 file changed, 18 insertions(+), 41 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> index 086f7ad8f0..2cf7d092e8 100644
> --- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> +++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> @@ -5,7 +5,7 @@
> Most of services in this library instance are suggested to be invoked by
> BSP
> only,
> except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR setting to
> APs.
>
> - Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
> + Copyright (c) 2008 - 2019, Intel Corporation. All rights
> + reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution. The full text of the license may be
> found at @@ -2099,8 +2099,8 @@ MtrrLibSetMemoryRanges (
>Set the below-1MB memory attribute to fixed MTRR buffer.
>Modified flag array indicates which fixed MTRR is modified.
>
> - @param [in, out] FixedSettings Fixed MTRR buffer.
> - @param [out] Modified Flag array indicating which MTRR is
> modified.
> + @param [in, out] ClearMasksThe bits to clear in the fixed MTRR MSR.
> + @param [in, out] OrMasks The bits to set in the fixed MTRR MSR.
>@param [in] BaseAddress Base address.
>@param [in] LengthLength.
>@param [in] Type Memory type.
> @@ -2111,8 +2111,8 @@ MtrrLibSetMemoryRanges ( **/ RETURN_STATUS
> MtrrLibSetBelow1MBMemoryAttribute (
> - IN OUT MTRR_FIXED_SETTINGS *FixedSettings,
> - OUT BOOLEAN*Modified,
> + IN OUT UINT64 *ClearMasks,
> + IN OUT UINT64 *OrMasks,
>IN PHYSICAL_ADDRESSBaseAddress,
>IN UINT64 Length,
>IN MTRR_MEMORY_CACHE_TYPE Type
> @@ -2122,36 +2122,17 @@ MtrrLibSetBelow1MBMemoryAttribute (
>UINT32MsrIndex;
>UINT64ClearMask;
>UINT64OrMask;
> - UINT64ClearMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
> - UINT64OrMasks[ARRAY_SIZE (mMtrrLibFixedMtrrTable)];
> - BOOLEAN LocalModified[ARRAY_SIZE
> (mMtrrLibFixedMtrrTable)];
>
>ASSERT (BaseAddress < BASE_1MB);
>
> - SetMem (LocalModified, sizeof (LocalModified), FALSE);
> -
> - //
> - // (Value & ~0 | 0) still equals to (Value)
> - //
> - SetMem (ClearMasks, sizeof (ClearMasks), 0);
> - SetMem (OrMasks, sizeof (OrMasks), 0);
> -
>MsrIndex = (UINT32)-1;
>while ((BaseAddress < BASE_1MB) && (Length != 0)) {
> Status = MtrrLibProgramFixedMtrr (Type, &BaseAddress, &Length,
> &MsrIndex, &ClearMask, &OrMask);
> if (RETURN_ERROR (Status)) {
>return Status;
> }
> -ClearMasks[MsrIndex]= ClearMask;
> -OrMasks[MsrIndex] = OrMask;
> -Modified[MsrIndex] = TRUE;
> -LocalModified[MsrIndex] = TRUE;
> - }
> -
> - for (MsrIndex = 0; MsrIndex < ARRAY_SIZE (mMtrrLibFixedMtrrTable);
> MsrIndex++) {
> -if (LocalModified[MsrIndex]) {
> - FixedSettings->Mtrr[MsrIndex] = (FixedSettings->Mtrr[MsrIndex] &
> ~ClearMasks[MsrIndex]) | OrMasks[MsrIndex];
> -}
> +ClearMasks[MsrIndex] = ClearMasks[MsrIndex] | ClearMask;
> +OrMasks[MsrIndex]= (OrMasks[MsrIndex] & ~ClearMask) | OrMask;
>}
>return RETURN_SUCCESS;
> }
> @@ -2213,8 +2194,8 @@ MtrrSetMemoryAttributesInMtrrSettings (
>MTRR_MEMORY_RANGE WorkingVariableMtrr[ARRAY_SIZE
> (MtrrSetting->Variables.Mtrr)];
>BOOLEAN VariableSettingModified[ARRAY_SIZE (MtrrSetting-
> >Variables.Mtrr)];
>
Re: [edk2] [patch V4] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible
Reviewed-by: Liming Gao
> -Original Message-
> From: Zeng, Star
> Sent: Monday, February 18, 2019 10:22 PM
> To: Bi, Dandan ; edk2-devel@lists.01.org
> Cc: Max Knutsen ; Wang, Jian J
> ; Wu, Hao A ; Michael Turner
> ; Gao, Liming ; Zeng,
> Star
> Subject: RE: [patch V4] MdeModulePkg/ReportStatusCodeLib: Avoid using
> AllocatePool if possible
>
> Reviewed-by: Star Zeng .
>
> -Original Message-
> From: Bi, Dandan
> Sent: Monday, February 18, 2019 9:37 PM
> To: edk2-devel@lists.01.org
> Cc: Max Knutsen ; Zeng, Star ;
> Wang, Jian J ; Wu, Hao A
> ; Michael Turner ; Gao,
> Liming
> Subject: [patch V4] MdeModulePkg/ReportStatusCodeLib: Avoid using
> AllocatePool if possible
>
> From: Max Knutsen
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1114
>
> V2: simplify the code logic.
> update
> if (!mHaveExitedBootServices &&
> (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
> gBS->FreePool (StatusCodeData);
> }
> to
> if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
> gBS->FreePool (StatusCodeData);
> }
>
> V3:
> And the code below into the else condition (stack buffer is not enough) in
> /DxeReportStatusCodeLib/ReportStatusCodeLib.c
>
> if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
> return EFI_UNSUPPORTED;
> }
>
> V4:
> Refine code logic.
>
> When report status code with ExtendedData data, and the extended data can fit
> in the local static buffer, there is no need to use
> AllocatePool to hold the ExtendedData data.
>
> This patch is just to do the enhancement to avoid using AllocatePool.
>
> Cc: Star Zeng
> Cc: Jian J Wang
> Cc: Hao Wu
> Cc: Michael Turner
> Cc: Liming Gao
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi
> ---
> .../ReportStatusCodeLib.c | 45 +--
> .../ReportStatusCodeLib.c | 14 +++---
> 2 files changed, 30 insertions(+), 29 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> index b28dc5c3bb..d3cf8b1de3 100644
> --- a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> +++ b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> @@ -494,41 +494,38 @@ ReportStatusCodeEx (
>UINT64Buffer[(MAX_EXTENDED_DATA_SIZE / sizeof (UINT64)) +
> 1];
>
>ASSERT (!((ExtendedData == NULL) && (ExtendedDataSize != 0)));
>ASSERT (!((ExtendedData != NULL) && (ExtendedDataSize == 0)));
>
> - if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
> -return EFI_UNSUPPORTED;
> - }
> -
> - //
> - // Retrieve the current TPL
> - //
> - Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
> - gBS->RestoreTPL (Tpl);
> + if (ExtendedDataSize <= (MAX_EXTENDED_DATA_SIZE - sizeof
> (EFI_STATUS_CODE_DATA))) {
> +//
> +// Use Buffer instead of allocating if possible.
> +//
> +StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer; } else {
> +if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
> + return EFI_UNSUPPORTED;
> +}
>
> - StatusCodeData = NULL;
> - if (Tpl <= TPL_NOTIFY) {
> //
> -// Allocate space for the Status Code Header and its buffer
> +// Retrieve the current TPL
> //
> -gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) +
> ExtendedDataSize, (VOID **)&StatusCodeData);
> - }
> +Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
> +gBS->RestoreTPL (Tpl);
> +
> +if (Tpl > TPL_NOTIFY) {
> + return EFI_OUT_OF_RESOURCES;
> +}
>
> - if (StatusCodeData == NULL) {
> //
> -// If a buffer could not be allocated, then see if the local variable
> Buffer can be used
> +// Allocate space for the Status Code Header and its buffer
> //
> -if (ExtendedDataSize > (MAX_EXTENDED_DATA_SIZE - sizeof
> (EFI_STATUS_CODE_DATA))) {
> - //
> - // The local variable Buffer not large enough to hold the extended
> data associated
> - // with the status code being reported.
> - //
> - DEBUG ((EFI_D_ERROR, "Status code extended data is too large to be
> reported!\n"));
> +StatusCodeData = NULL;
> +gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) +
> ExtendedDataSize, (VOID **)&StatusCodeData);
> +if (StatusCodeData == NULL) {
>return EFI_OUT_OF_RESOURCES;
> }
> -StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer;
>}
>
>//
>// Fill in the extended data header
>//
> @@ -626,6 +623,6 @@ EFIAPI
> ReportDebugCodeEnabled (
>VOID
>)
> {
>return (BOOLEAN) ((PcdGet8 (PcdReportStatusCodePropertyMask) &
> REPORT_STATUS_CODE_PROPERTY_DEBUG_CODE_ENABLED) != 0);
> -}
> +}
> \ No newline at end of file
> diff --git
> a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
> b/MdeModulePkg/Library/Runti
Re: [edk2] [PATCH] BaseTools:Fix a ECC issue
Reviewed-by: Bob Feng -Original Message- From: Fan, ZhijuX Sent: Monday, February 18, 2019 2:24 PM To: edk2-devel@lists.01.org Cc: Gao, Liming ; Feng, Bob C Subject: [edk2][PATCH] BaseTools:Fix a ECC issue BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1522 A property error occurred because the property of the function was not defined. a property is now redefined. Cc: Bob Feng Cc: Liming Gao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Zhiju.Fan --- BaseTools/Source/Python/Ecc/CodeFragmentCollector.py | 2 +- BaseTools/Source/Python/Ecc/Configuration.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/BaseTools/Source/Python/Ecc/CodeFragmentCollector.py b/BaseTools/Source/Python/Ecc/CodeFragmentCollector.py index 21fed59cad..f844b4a0b3 100644 --- a/BaseTools/Source/Python/Ecc/CodeFragmentCollector.py +++ b/BaseTools/Source/Python/Ecc/CodeFragmentCollector.py @@ -27,7 +27,7 @@ if sys.version_info.major == 3: from Ecc.CParser4.CParser import CParser else: import antlr3 as antlr -antlr.InputString = antlr.StringStream +antlr.InputStream = antlr.StringStream from Ecc.CParser3.CLexer import CLexer from Ecc.CParser3.CParser import CParser diff --git a/BaseTools/Source/Python/Ecc/Configuration.py b/BaseTools/Source/Python/Ecc/Configuration.py index c19a3990c7..f2b2b86487 100644 --- a/BaseTools/Source/Python/Ecc/Configuration.py +++ b/BaseTools/Source/Python/Ecc/Configuration.py @@ -192,6 +192,8 @@ class Configuration(object): self.GeneralCheckLineEnding = 1 # Check if there is no trailing white space in one line. self.GeneralCheckTrailingWhiteSpaceLine = 1 + +self.CFunctionLayoutCheckNoDeprecated = 1 ## Space Checking self.SpaceCheckAll = 1 -- 2.14.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/PropertiesTableAttributesDxe: Remove this driver
Reviewed-by: Ray Ni
> -Original Message-
> From: Zeng, Star
> Sent: Monday, February 18, 2019 10:22 PM
> To: Wang, Jian J ; Zhang, Shenglei
> ; edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Ni, Ray
> Subject: RE: [PATCH] MdeModulePkg/PropertiesTableAttributesDxe:
> Remove this driver
>
> Reviewed-by: Star Zeng
>
> -Original Message-
> From: Wang, Jian J
> Sent: Monday, February 18, 2019 10:16 PM
> To: Zhang, Shenglei ; edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Ni, Ray ; Zeng,
> Star
> Subject: RE: [PATCH] MdeModulePkg/PropertiesTableAttributesDxe:
> Remove this driver
>
>
> Reviewed-by: Jian J Wang
>
> > -Original Message-
> > From: Zhang, Shenglei
> > Sent: Monday, February 18, 2019 4:53 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wang, Jian J ; Wu, Hao A
> > ; Ni, Ray ; Zeng, Star
> >
> > Subject: [PATCH] MdeModulePkg/PropertiesTableAttributesDxe: Remove
> > this driver
> >
> > This functionality of this driver has been deprecated and no platform
> > employs this driver. It can be removed completely.
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1475
> >
> > Cc: Jian J Wang
> > Cc: Hao Wu
> > Cc: Ray Ni
> > Cc: Star Zeng
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Shenglei Zhang
> > ---
> > MdeModulePkg/MdeModulePkg.dsc | 1 -
> > .../PropertiesTableAttributesDxe.c| 208 --
> > .../PropertiesTableAttributesDxe.inf | 56 -
> > .../PropertiesTableAttributesDxe.uni | 23 --
> > .../PropertiesTableAttributesDxeExtra.uni | 23 --
> > 5 files changed, 311 deletions(-)
> > delete mode 100644
> >
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAt
> t
> > ribu
> > tesDxe.c
> > delete mode 100644
> >
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAt
> t
> > ribu
> > tesDxe.inf
> > delete mode 100644
> >
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAt
> t
> > ribu
> > tesDxe.uni
> > delete mode 100644
> >
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAt
> t
> > ribu
> > tesDxeExtra.uni
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > b/MdeModulePkg/MdeModulePkg.dsc index 4f2ac8ae89..388bca25bd
> 100644
> > --- a/MdeModulePkg/MdeModulePkg.dsc
> > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > @@ -413,7 +413,6 @@
> >MdeModulePkg/Universal/EsrtDxe/EsrtDxe.inf
> >MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmpDxe.inf
> >
> > -
> >
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAt
> t
> > ribu
> > tesDxe.inf
> >MdeModulePkg/Universal/FileExplorerDxe/FileExplorerDxe.inf {
> >
> >
> > FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.i
> > nf
> > diff --git
> >
> a/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTable
> A
> > ttri
> > butesDxe.c
> >
> b/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTable
> A
> > ttri
> > butesDxe.c
> > deleted file mode 100644
> > index 4d1a46f64c..00
> > ---
> >
> a/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTable
> A
> > ttri
> > butesDxe.c
> > +++ /dev/null
> > @@ -1,208 +0,0 @@
> > -/** @file
> > - This module sets default policy for attributes of EfiACPIMemoryNVS
> > and EfiReservedMemoryType.
> > -
> > - This module sets EFI_MEMORY_XP for attributes of EfiACPIMemoryNVS
> > and EfiReservedMemoryType
> > - in UEFI memory map, if and only of PropertiesTable is published and
> > has BIT0 set.
> > -
> > -Copyright (c) 2015 - 2018, Intel Corporation. All rights
> > reserved. -This program and the accompanying materials -are
> > licensed and made available under the terms and conditions of the BSD
> > License -which accompanies this distribution. The full text of the
> > license may be found at
> > -http://opensource.org/licenses/bsd-license.php
> > -
> > -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR
> > IMPLIED.
> > -
> > -**/
> > -
> > -#include
> > -#include
> > -#include
> > -#include -#include
> > -#include
> > -#include -#include
> > -
> > -/**
> > - Converts a number of EFI_PAGEs to a size in bytes.
> > -
> > - NOTE: Do not use EFI_PAGES_TO_SIZE because it handles UINTN only.
> > -
> > - @param Pages The number of EFI_PAGES.
> > -
> > - @return The number of bytes associated with the number of
> > EFI_PAGEs specified
> > - by Pages.
> > -**/
> > -UINT64
> > -EfiPagesToSize (
> > - IN UINT64 Pages
> > - )
> > -{
> > - return LShiftU64 (Pages, EFI_PAGE_SHIFT); -}
> > -
> > -/**
> > - Set memory attributes according to default policy.
> > -
> > - @param MemoryMapA pointer to the buffer in which firmware
> places
> > the current memory map.
> > - @param MemoryMapSizeSize, in bytes, of the MemoryMap buffer.
> > - @param DescriptorSize size, in bytes, of an individual
> > EFI_MEMORY
Re: [edk2] [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration
Jordan, I find many real platforms do not implement the temporary ram migration PPI and rely on the PeiCore migration logic. So perhaps TemporaryRamMigration PPI was added to help platform to destroy the temporary RAM (CAR in x86 platform). But with the introduction of TemporaryRamDone PPI, maybe the TemporaryRamMigration PPI can be retired. The logic in PeiCore to call TemporaryRamMigration is just for backward compatibility. If that's true, do you still need to enhance PeiCore? For the Emulator case, I already found without TemporaryRamMigration the platform can still boot. Does OVMF hard-depend on TemporaryRamMigration? Or it can reply on the PeiCore migration logic + TemporaryDone PPI? Thanks, Ray > -Original Message- > From: Justen, Jordan L > Sent: Monday, February 18, 2019 12:12 PM > To: edk2-devel@lists.01.org > Cc: Justen, Jordan L ; Liu Yu > ; Andrew Fish ; Anthony > Perard ; Ard Biesheuvel > ; Wu, Hao A ; Wang, Jian > J ; Julien Grall ; Laszlo > Ersek > ; Ni, Ray ; Zeng, Star > > Subject: [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration > > https://github.com/jljusten/edk2.git temp-ram-support > > This series fixes an issue that, while rare, is possible based on the way the > TemporaryRamSupport PPI is defined along with how it is used by the PEI > Core. > > Liu Yu reported a boot issue with EmulatorPkg, which I believe was caused by > this issue. > > The details of the issue are described in the commit message of the > "MdeModePkg/Core/Pei: Add code path to allow assembly temp-ram > migration" patch. > > Along with this, I added a few Temporary RAM patches for EmulatorPkg and > OvmfPkg. > > Cc: Liu Yu > Cc: Andrew Fish > Cc: Anthony Perard > Cc: Ard Biesheuvel > Cc: Hao Wu > Cc: Jian J Wang > Cc: Julien Grall > Cc: Laszlo Ersek > Cc: Ray Ni > Cc: Star Zeng > > Jordan Justen (10): > EmulatorPkg/build.sh: Fix missing usage of -b BUILDTARGET parameter > EmulatorPkg/Unix/Host: Use PcdInitValueInTempStack to init temp-ram > EmulatorPkg/Sec: Replace assembly temp-ram support with C code > EmulatorPkg/Sec: Disable optimizations for TemporaryRamMigration > function > OvmfPkg/Sec: Swap TemporaryRam Stack and Heap locations > OvmfPkg/Sec: Disable optimizations for TemporaryRamMigration > MdeModePkg/Core/Pei: Add code path to allow assembly temp-ram > migration > MdeModulePkg/Core/Pei: Use assembly for X64 TemporaryRamMigration > MdeModulePkg/Core/Pei: Use assembly for IA32 TemporaryRamMigration > OvmfPkg/Sec: Fill Temp Ram after TemporaryRamMigration > > EmulatorPkg/Sec/Ia32/SwitchRam.S | 95 --- > EmulatorPkg/Sec/Ia32/SwitchRam.asm| 94 -- > EmulatorPkg/Sec/Ia32/TempRam.c| 65 - > EmulatorPkg/Sec/Sec.c | 76 ++- > EmulatorPkg/Sec/Sec.inf | 13 +-- > EmulatorPkg/Sec/X64/SwitchRam.S | 72 -- > EmulatorPkg/Sec/X64/SwitchRam.asm | 76 --- > EmulatorPkg/Unix/Host/Host.c | 2 +- > EmulatorPkg/Unix/Host/Host.inf| 1 + > EmulatorPkg/build.sh | 10 +- > MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 59 > .../Dispatcher/Ia32/TemporaryRamMigration.S | 72 ++ > .../Ia32/TemporaryRamMigration.nasm | 78 +++ > .../Pei/Dispatcher/TemporaryRamMigration.c| 52 ++ > .../Dispatcher/X64/TemporaryRamMigration.S| 69 ++ > .../Dispatcher/X64/TemporaryRamMigration.nasm | 75 +++ > MdeModulePkg/Core/Pei/PeiMain.h | 52 ++ > MdeModulePkg/Core/Pei/PeiMain.inf | 14 +++ > OvmfPkg/Sec/Ia32/SecEntry.nasm| 2 +- > OvmfPkg/Sec/SecMain.c | 59 > OvmfPkg/Sec/X64/SecEntry.nasm | 2 +- > 21 files changed, 577 insertions(+), 461 deletions(-) delete mode 100644 > EmulatorPkg/Sec/Ia32/SwitchRam.S delete mode 100644 > EmulatorPkg/Sec/Ia32/SwitchRam.asm > delete mode 100644 EmulatorPkg/Sec/Ia32/TempRam.c delete mode > 100644 EmulatorPkg/Sec/X64/SwitchRam.S delete mode 100644 > EmulatorPkg/Sec/X64/SwitchRam.asm create mode 100644 > MdeModulePkg/Core/Pei/Dispatcher/Ia32/TemporaryRamMigration.S > create mode 100644 > MdeModulePkg/Core/Pei/Dispatcher/Ia32/TemporaryRamMigration.nasm > create mode 100644 > MdeModulePkg/Core/Pei/Dispatcher/TemporaryRamMigration.c > create mode 100644 > MdeModulePkg/Core/Pei/Dispatcher/X64/TemporaryRamMigration.S > create mode 100644 > MdeModulePkg/Core/Pei/Dispatcher/X64/TemporaryRamMigration.nasm > > -- > 2.20.0.rc1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] UefiCpuPkg/Microcode: Fix incorrect checksum issue for extended table
I agree with Chao's comments.
Please add more code comments to:
1. describe the uCode format
2. explain all the 3 code blocks that checks the checksum.
> -Original Message-
> From: edk2-devel On Behalf Of Zhang,
> Chao B
> Sent: Tuesday, February 19, 2019 9:06 AM
> To: Chen, Chen A ; edk2-devel@lists.01.org
> Cc: Dong, Eric
> Subject: Re: [edk2] [PATCH] UefiCpuPkg/Microcode: Fix incorrect checksum
> issue for extended table
>
> Chen Chen:
>I think you can add uCode format info into comments. Also please highlight
> in comment Which part is header checksum calculation, which part is for
> extended header
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Chen A Chen
> Sent: Monday, February 18, 2019 1:54 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric
> Subject: [edk2] [PATCH] UefiCpuPkg/Microcode: Fix incorrect checksum
> issue for extended table
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
>
> The following Microcode payload format is define in SDM spec.
> Payload:
> |MicrocodeHeader|MicrocodeBinary|ExtendedHeader|ExtendedTable|.
> When we verify the CheckSum32 with ExtendedTable, we should use the
> fields of ExtendedTable to replace corresponding fields in MicrocodeHeader,
> and then calculate the CheckSum32 with MicrocodeHeader+MicrocodeBinary.
> This patch already verified on ICL platform.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chen A Chen
> Cc: Ray Ni
> Cc: Eric Dong
> ---
> UefiCpuPkg/Library/MpInitLib/Microcode.c | 38
>
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index d84344c6f5..38880cdbec 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -57,6 +57,7 @@ MicrocodeDetect (
>UINT32 LatestRevision;
>UINTN TotalSize;
>UINT32 CheckSum32;
> + UINT32 InCompleteCheckSum32;
>BOOLEAN CorrectMicrocode;
>VOID*MicrocodeData;
>MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr;
> @@ -121,6 +122,26 @@ MicrocodeDetect (
>MicrocodeData = NULL;
>MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress +
> CpuMpData->MicrocodePatchRegionSize);
>MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN)
> CpuMpData->MicrocodePatchAddress;
> +
> + //
> + // To avoid double calculate checksum32 value.
> + // Save the CheckSum32 of the common parts in advance.
> + //
> + if (MicrocodeEntryPoint->DataSize == 0) {
> +InCompleteCheckSum32 = CalculateSum32 (
> + (UINT32 *) MicrocodeEntryPoint,
> + sizeof (CPU_MICROCODE_HEADER) + 2000
> + );
> + } else {
> +InCompleteCheckSum32 = CalculateSum32 (
> + (UINT32 *) MicrocodeEntryPoint,
> + sizeof (CPU_MICROCODE_HEADER) +
> MicrocodeEntryPoint-
> >DataSize
> + );
> + }
> + InCompleteCheckSum32 -=
> + MicrocodeEntryPoint->ProcessorSignature.Uint32;
> + InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
> + InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
> +
>do {
> //
> // Check if the microcode is for the Cpu and the version is newer @@ -
> 137,14 +158,10 @@ MicrocodeDetect (
>MicrocodeEntryPoint->UpdateRevision > LatestRevision &&
>(MicrocodeEntryPoint->ProcessorFlags & (1 << PlatformId))
>) {
> -if (MicrocodeEntryPoint->DataSize == 0) {
> - CheckSum32 = CalculateSum32 ((UINT32 *) MicrocodeEntryPoint, 2048);
> -} else {
> - CheckSum32 = CalculateSum32 (
> - (UINT32 *) MicrocodeEntryPoint,
> - MicrocodeEntryPoint->DataSize + sizeof
> (CPU_MICROCODE_HEADER)
> - );
> -}
> +CheckSum32 = InCompleteCheckSum32;
> +CheckSum32 += MicrocodeEntryPoint->ProcessorSignature.Uint32;
> +CheckSum32 += MicrocodeEntryPoint->ProcessorFlags;
> +CheckSum32 += MicrocodeEntryPoint->Checksum;
> if (CheckSum32 == 0) {
>CorrectMicrocode = TRUE;
>ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
> @@ -171,7 +188,10 @@ MicrocodeDetect (
>ExtendedTableCount = ExtendedTableHeader-
> >ExtendedSignatureCount;
>ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE *)
> (ExtendedTableHeader + 1);
>for (Index = 0; Index < ExtendedTableCount; Index ++) {
> -CheckSum32 = CalculateSum32 ((UINT32 *) ExtendedTable,
> sizeof(CPU_MICROCODE_EXTENDED_TABLE));
> +
Re: [edk2] [PATCH] UefiCpuPkg/Microcode: Fix incorrect checksum issue for extended table
Chen Chen:
I think you can add uCode format info into comments. Also please highlight
in comment
Which part is header checksum calculation, which part is for extended header
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Chen A
Chen
Sent: Monday, February 18, 2019 1:54 PM
To: edk2-devel@lists.01.org
Cc: Dong, Eric
Subject: [edk2] [PATCH] UefiCpuPkg/Microcode: Fix incorrect checksum issue for
extended table
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
The following Microcode payload format is define in SDM spec.
Payload: |MicrocodeHeader|MicrocodeBinary|ExtendedHeader|ExtendedTable|.
When we verify the CheckSum32 with ExtendedTable, we should use the fields of
ExtendedTable to replace corresponding fields in MicrocodeHeader, and then
calculate the CheckSum32 with MicrocodeHeader+MicrocodeBinary.
This patch already verified on ICL platform.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen
Cc: Ray Ni
Cc: Eric Dong
---
UefiCpuPkg/Library/MpInitLib/Microcode.c | 38
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index d84344c6f5..38880cdbec 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -57,6 +57,7 @@ MicrocodeDetect (
UINT32 LatestRevision;
UINTN TotalSize;
UINT32 CheckSum32;
+ UINT32 InCompleteCheckSum32;
BOOLEAN CorrectMicrocode;
VOID*MicrocodeData;
MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr;
@@ -121,6 +122,26 @@ MicrocodeDetect (
MicrocodeData = NULL;
MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress +
CpuMpData->MicrocodePatchRegionSize);
MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN)
CpuMpData->MicrocodePatchAddress;
+
+ //
+ // To avoid double calculate checksum32 value.
+ // Save the CheckSum32 of the common parts in advance.
+ //
+ if (MicrocodeEntryPoint->DataSize == 0) {
+InCompleteCheckSum32 = CalculateSum32 (
+ (UINT32 *) MicrocodeEntryPoint,
+ sizeof (CPU_MICROCODE_HEADER) + 2000
+ );
+ } else {
+InCompleteCheckSum32 = CalculateSum32 (
+ (UINT32 *) MicrocodeEntryPoint,
+ sizeof (CPU_MICROCODE_HEADER) +
MicrocodeEntryPoint->DataSize
+ );
+ }
+ InCompleteCheckSum32 -=
+ MicrocodeEntryPoint->ProcessorSignature.Uint32;
+ InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
+ InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
+
do {
//
// Check if the microcode is for the Cpu and the version is newer @@
-137,14 +158,10 @@ MicrocodeDetect (
MicrocodeEntryPoint->UpdateRevision > LatestRevision &&
(MicrocodeEntryPoint->ProcessorFlags & (1 << PlatformId))
) {
-if (MicrocodeEntryPoint->DataSize == 0) {
- CheckSum32 = CalculateSum32 ((UINT32 *) MicrocodeEntryPoint, 2048);
-} else {
- CheckSum32 = CalculateSum32 (
- (UINT32 *) MicrocodeEntryPoint,
- MicrocodeEntryPoint->DataSize + sizeof
(CPU_MICROCODE_HEADER)
- );
-}
+CheckSum32 = InCompleteCheckSum32;
+CheckSum32 += MicrocodeEntryPoint->ProcessorSignature.Uint32;
+CheckSum32 += MicrocodeEntryPoint->ProcessorFlags;
+CheckSum32 += MicrocodeEntryPoint->Checksum;
if (CheckSum32 == 0) {
CorrectMicrocode = TRUE;
ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
@@ -171,7 +188,10 @@ MicrocodeDetect (
ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE *)
(ExtendedTableHeader + 1);
for (Index = 0; Index < ExtendedTableCount; Index ++) {
-CheckSum32 = CalculateSum32 ((UINT32 *) ExtendedTable,
sizeof(CPU_MICROCODE_EXTENDED_TABLE));
+CheckSum32 = InCompleteCheckSum32;
+CheckSum32 += ExtendedTable->ProcessorSignature.Uint32;
+CheckSum32 += ExtendedTable->ProcessorFlag;
+CheckSum32 += ExtendedTable->Checksum;
if (CheckSum32 == 0) {
//
// Verify Header
--
2.16.2.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-deve
Re: [edk2] [Patch] BaseTools: Correct the value assignment for StructurePcd
Hi Felix,
Good point. I'll update the patch.
Thanks,
Bob
-Original Message-
From: Felix Polyudov [mailto:fel...@ami.com]
Sent: Tuesday, February 19, 2019 12:57 AM
To: Feng, Bob C ; edk2-devel@lists.01.org
Cc: Gao, Liming
Subject: RE: [edk2] [Patch] BaseTools: Correct the value assignment for
StructurePcd
Bob,
I think silent trimming of input data is not a good idea.
The build tool should either report an error (my preference) or a warning that
DSC or DEC input value is too long.
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Feng,
Bob C
Sent: Saturday, February 16, 2019 12:55 AM
To: edk2-devel@lists.01.org
Cc: Liming Gao
Subject: [edk2] [Patch] BaseTools: Correct the value assignment for StructurePcd
This patch is to fix the code bug in StructurePcd overall value assignment
logic. If a Pcd Array size is fixed but the size of actual value in Dsc or Dec
is bigger than the Pcd array size, the tool only copy the data as same as that
Pcd Array size.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng
Cc: Liming Gao
---
.../Python/Workspace/BuildClassObject.py | 57 ---
.../Source/Python/Workspace/DscBuildData.py | 145 ++
2 files changed, 147 insertions(+), 55 deletions(-)
diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py
b/BaseTools/Source/Python/Workspace/BuildClassObject.py
index 1df042f41c..d6168b62d5 100644
--- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
+++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
@@ -76,34 +76,45 @@ class PcdClassObject(object):
self.UserDefinedDefaultStoresFlag = UserDefinedDefaultStoresFlag
self._Capacity = None
@property
def Capacity(self):
-self._Capacity = []
-dimension = ArrayIndex.findall(self._DatumType)
-for item in dimension:
-maxsize = item.lstrip("[").rstrip("]").strip()
-if not maxsize:
-maxsize = "-1"
-maxsize = str(int(maxsize,16)) if maxsize.startswith(("0x","0X"))
else maxsize
-self._Capacity.append(maxsize)
-if hasattr(self, "SkuOverrideValues"):
-for sku in self.SkuOverrideValues:
-for defaultstore in self.SkuOverrideValues[sku]:
-fields = self.SkuOverrideValues[sku][defaultstore]
-for demesionattr in fields:
-deme = ArrayIndex.findall(demesionattr)
-for i in range(len(deme)-1):
-if int(deme[i].lstrip("[").rstrip("]").strip()) >
int(self._Capacity[i]):
-print ("error")
-if hasattr(self,"DefaultValues"):
-for demesionattr in self.DefaultValues:
-deme = ArrayIndex.findall(demesionattr)
-for i in range(len(deme)-1):
-if int(deme[i].lstrip("[").rstrip("]").strip()) >
int(self._Capacity[i]):
-print ("error")
+if self._Capacity is None:
+self._Capacity = []
+dimension = ArrayIndex.findall(self._DatumType)
+for item in dimension:
+maxsize = item.lstrip("[").rstrip("]").strip()
+if not maxsize:
+maxsize = "-1"
+maxsize = str(int(maxsize,16)) if
maxsize.startswith(("0x","0X")) else maxsize
+self._Capacity.append(maxsize)
+if hasattr(self, "SkuOverrideValues"):
+for sku in self.SkuOverrideValues:
+for defaultstore in self.SkuOverrideValues[sku]:
+fields = self.SkuOverrideValues[sku][defaultstore]
+for demesionattr in fields:
+deme = ArrayIndex.findall(demesionattr)
+for i in range(len(deme)):
+if
int(deme[i].lstrip("[").rstrip("]").strip()) >= int(self._Capacity[i]):
+if self._Capacity[i] != "-1":
+raise ("error")
+if hasattr(self,"DefaultValues"):
+for demesionattr in self.DefaultValues:
+deme = ArrayIndex.findall(demesionattr)
+for i in range(len(deme)):
+if int(deme[i].lstrip("[").rstrip("]").strip()) >=
int(self._Capacity[i]):
+if self._Capacity[i] != "-1":
+raise ("error")
return self._Capacity
+
+def PcdArraySize(self):
+if self.Capacity[-1] == "-1":
+return -1
+size = 1
+for de in self.Capacity:
+size = size * int(de)
+return size
@property
def DatumType(self):
return self._DatumType
@DatumType.setter
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/
Re: [edk2] [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Error message enhancement
Reviewed-by: Chaganty, Rangasai V
-Original Message-
From: Zhang, Shenglei
Sent: Sunday, February 17, 2019 11:19 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ray ; Chaganty, Rangasai V
Subject: [PATCH] IntelSiliconPkg/MicrocodeUpdateDxe: Error message enhancement
The error message of ExtendedTableCount is not clear. Add the count number into
the debug message.
https://bugzilla.tianocore.org/show_bug.cgi?id=1500
Cc: Ray Ni
Cc: Rangasai V Chaganty
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Shenglei Zhang
---
.../Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c| 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
index 9098712c2f..9b5757da71 100644
--- a/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate.c
+++ b/IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdate
+++ .c
@@ -518,7 +518,7 @@ VerifyMicrocode (
//
ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
if (ExtendedTableCount > (ExtendedTableLength -
sizeof(CPU_MICROCODE_EXTENDED_TABLE_HEADER)) /
sizeof(CPU_MICROCODE_EXTENDED_TABLE)) {
-DEBUG((DEBUG_ERROR, "VerifyMicrocode - ExtendedTableCount too
big\n"));
+DEBUG((DEBUG_ERROR, "VerifyMicrocode - ExtendedTableCount
+ %d is too big\n", ExtendedTableCount));
} else {
ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE
*)(ExtendedTableHeader + 1);
for (Index = 0; Index < ExtendedTableCount; Index++) {
--
2.18.0.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] BaseTools: Correct the value assignment for StructurePcd
Bob,
I think silent trimming of input data is not a good idea.
The build tool should either report an error (my preference) or a warning that
DSC or DEC input value is too long.
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Feng,
Bob C
Sent: Saturday, February 16, 2019 12:55 AM
To: edk2-devel@lists.01.org
Cc: Liming Gao
Subject: [edk2] [Patch] BaseTools: Correct the value assignment for StructurePcd
This patch is to fix the code bug in StructurePcd overall
value assignment logic. If a Pcd Array size is fixed but the
size of actual value in Dsc or Dec is bigger than the Pcd
array size, the tool only copy the data as same as that
Pcd Array size.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng
Cc: Liming Gao
---
.../Python/Workspace/BuildClassObject.py | 57 ---
.../Source/Python/Workspace/DscBuildData.py | 145 ++
2 files changed, 147 insertions(+), 55 deletions(-)
diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py
b/BaseTools/Source/Python/Workspace/BuildClassObject.py
index 1df042f41c..d6168b62d5 100644
--- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
+++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
@@ -76,34 +76,45 @@ class PcdClassObject(object):
self.UserDefinedDefaultStoresFlag = UserDefinedDefaultStoresFlag
self._Capacity = None
@property
def Capacity(self):
-self._Capacity = []
-dimension = ArrayIndex.findall(self._DatumType)
-for item in dimension:
-maxsize = item.lstrip("[").rstrip("]").strip()
-if not maxsize:
-maxsize = "-1"
-maxsize = str(int(maxsize,16)) if maxsize.startswith(("0x","0X"))
else maxsize
-self._Capacity.append(maxsize)
-if hasattr(self, "SkuOverrideValues"):
-for sku in self.SkuOverrideValues:
-for defaultstore in self.SkuOverrideValues[sku]:
-fields = self.SkuOverrideValues[sku][defaultstore]
-for demesionattr in fields:
-deme = ArrayIndex.findall(demesionattr)
-for i in range(len(deme)-1):
-if int(deme[i].lstrip("[").rstrip("]").strip()) >
int(self._Capacity[i]):
-print ("error")
-if hasattr(self,"DefaultValues"):
-for demesionattr in self.DefaultValues:
-deme = ArrayIndex.findall(demesionattr)
-for i in range(len(deme)-1):
-if int(deme[i].lstrip("[").rstrip("]").strip()) >
int(self._Capacity[i]):
-print ("error")
+if self._Capacity is None:
+self._Capacity = []
+dimension = ArrayIndex.findall(self._DatumType)
+for item in dimension:
+maxsize = item.lstrip("[").rstrip("]").strip()
+if not maxsize:
+maxsize = "-1"
+maxsize = str(int(maxsize,16)) if
maxsize.startswith(("0x","0X")) else maxsize
+self._Capacity.append(maxsize)
+if hasattr(self, "SkuOverrideValues"):
+for sku in self.SkuOverrideValues:
+for defaultstore in self.SkuOverrideValues[sku]:
+fields = self.SkuOverrideValues[sku][defaultstore]
+for demesionattr in fields:
+deme = ArrayIndex.findall(demesionattr)
+for i in range(len(deme)):
+if
int(deme[i].lstrip("[").rstrip("]").strip()) >= int(self._Capacity[i]):
+if self._Capacity[i] != "-1":
+raise ("error")
+if hasattr(self,"DefaultValues"):
+for demesionattr in self.DefaultValues:
+deme = ArrayIndex.findall(demesionattr)
+for i in range(len(deme)):
+if int(deme[i].lstrip("[").rstrip("]").strip()) >=
int(self._Capacity[i]):
+if self._Capacity[i] != "-1":
+raise ("error")
return self._Capacity
+
+def PcdArraySize(self):
+if self.Capacity[-1] == "-1":
+return -1
+size = 1
+for de in self.Capacity:
+size = size * int(de)
+return size
@property
def DatumType(self):
return self._DatumType
@DatumType.setter
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index e45beb3924..09d917c5db 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1763,27 +1763,41 @@ class DscBuildData(PlatformBuildClassObject):
return Result
def GenerateSizeFunction(self, Pcd):
Re: [edk2] [PATCH] MdeModulePkg/PropertiesTableAttributesDxe: Remove this driver
Reviewed-by: Star Zeng
-Original Message-
From: Wang, Jian J
Sent: Monday, February 18, 2019 10:16 PM
To: Zhang, Shenglei ; edk2-devel@lists.01.org
Cc: Wu, Hao A ; Ni, Ray ; Zeng, Star
Subject: RE: [PATCH] MdeModulePkg/PropertiesTableAttributesDxe: Remove this
driver
Reviewed-by: Jian J Wang
> -Original Message-
> From: Zhang, Shenglei
> Sent: Monday, February 18, 2019 4:53 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Jian J ; Wu, Hao A
> ; Ni, Ray ; Zeng, Star
>
> Subject: [PATCH] MdeModulePkg/PropertiesTableAttributesDxe: Remove
> this driver
>
> This functionality of this driver has been deprecated and no platform
> employs this driver. It can be removed completely.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1475
>
> Cc: Jian J Wang
> Cc: Hao Wu
> Cc: Ray Ni
> Cc: Star Zeng
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Shenglei Zhang
> ---
> MdeModulePkg/MdeModulePkg.dsc | 1 -
> .../PropertiesTableAttributesDxe.c| 208 --
> .../PropertiesTableAttributesDxe.inf | 56 -
> .../PropertiesTableAttributesDxe.uni | 23 --
> .../PropertiesTableAttributesDxeExtra.uni | 23 --
> 5 files changed, 311 deletions(-)
> delete mode 100644
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAtt
> ribu
> tesDxe.c
> delete mode 100644
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAtt
> ribu
> tesDxe.inf
> delete mode 100644
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAtt
> ribu
> tesDxe.uni
> delete mode 100644
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAtt
> ribu
> tesDxeExtra.uni
>
> diff --git a/MdeModulePkg/MdeModulePkg.dsc
> b/MdeModulePkg/MdeModulePkg.dsc index 4f2ac8ae89..388bca25bd 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -413,7 +413,6 @@
>MdeModulePkg/Universal/EsrtDxe/EsrtDxe.inf
>MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmpDxe.inf
>
> -
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAtt
> ribu
> tesDxe.inf
>MdeModulePkg/Universal/FileExplorerDxe/FileExplorerDxe.inf {
>
>
> FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.i
> nf
> diff --git
> a/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableA
> ttri
> butesDxe.c
> b/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableA
> ttri
> butesDxe.c
> deleted file mode 100644
> index 4d1a46f64c..00
> ---
> a/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableA
> ttri
> butesDxe.c
> +++ /dev/null
> @@ -1,208 +0,0 @@
> -/** @file
> - This module sets default policy for attributes of EfiACPIMemoryNVS
> and EfiReservedMemoryType.
> -
> - This module sets EFI_MEMORY_XP for attributes of EfiACPIMemoryNVS
> and EfiReservedMemoryType
> - in UEFI memory map, if and only of PropertiesTable is published and
> has BIT0 set.
> -
> -Copyright (c) 2015 - 2018, Intel Corporation. All rights
> reserved. -This program and the accompanying materials -are
> licensed and made available under the terms and conditions of the BSD
> License -which accompanies this distribution. The full text of the
> license may be found at
> -http://opensource.org/licenses/bsd-license.php
> -
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
> -
> -**/
> -
> -#include
> -#include
> -#include
> -#include -#include
> -#include
> -#include -#include
> -
> -/**
> - Converts a number of EFI_PAGEs to a size in bytes.
> -
> - NOTE: Do not use EFI_PAGES_TO_SIZE because it handles UINTN only.
> -
> - @param Pages The number of EFI_PAGES.
> -
> - @return The number of bytes associated with the number of
> EFI_PAGEs specified
> - by Pages.
> -**/
> -UINT64
> -EfiPagesToSize (
> - IN UINT64 Pages
> - )
> -{
> - return LShiftU64 (Pages, EFI_PAGE_SHIFT); -}
> -
> -/**
> - Set memory attributes according to default policy.
> -
> - @param MemoryMapA pointer to the buffer in which firmware places
> the current memory map.
> - @param MemoryMapSizeSize, in bytes, of the MemoryMap buffer.
> - @param DescriptorSize size, in bytes, of an individual
> EFI_MEMORY_DESCRIPTOR.
> -**/
> -VOID
> -SetMemorySpaceAttributesDefault (
> - IN EFI_MEMORY_DESCRIPTOR *MemoryMap,
> - IN UINTN MemoryMapSize,
> - IN UINTN DescriptorSize
> - )
> -{
> - EFI_MEMORY_DESCRIPTOR *MemoryMapEntry;
> - EFI_MEMORY_DESCRIPTOR *MemoryMapEnd;
> - EFI_STATUS Status;
> -
> - DEBUG ((EFI_D_INFO, "SetMemorySpaceAttributesDefault\n"));
> -
> - MemoryMapEntry = MemoryMap;
> - MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) MemoryMap +
> MemoryMapSize);
> - while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd)
Re: [edk2] [patch V4] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible
Reviewed-by: Star Zeng .
-Original Message-
From: Bi, Dandan
Sent: Monday, February 18, 2019 9:37 PM
To: edk2-devel@lists.01.org
Cc: Max Knutsen ; Zeng, Star ;
Wang, Jian J ; Wu, Hao A ; Michael
Turner ; Gao, Liming
Subject: [patch V4] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool
if possible
From: Max Knutsen
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1114
V2: simplify the code logic.
update
if (!mHaveExitedBootServices &&
(StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
gBS->FreePool (StatusCodeData);
}
to
if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
gBS->FreePool (StatusCodeData);
}
V3:
And the code below into the else condition (stack buffer is not enough) in
/DxeReportStatusCodeLib/ReportStatusCodeLib.c
if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
return EFI_UNSUPPORTED;
}
V4:
Refine code logic.
When report status code with ExtendedData data, and the extended data can fit
in the local static buffer, there is no need to use AllocatePool to hold the
ExtendedData data.
This patch is just to do the enhancement to avoid using AllocatePool.
Cc: Star Zeng
Cc: Jian J Wang
Cc: Hao Wu
Cc: Michael Turner
Cc: Liming Gao
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi
---
.../ReportStatusCodeLib.c | 45 +--
.../ReportStatusCodeLib.c | 14 +++---
2 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
index b28dc5c3bb..d3cf8b1de3 100644
--- a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
+++ b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
@@ -494,41 +494,38 @@ ReportStatusCodeEx (
UINT64Buffer[(MAX_EXTENDED_DATA_SIZE / sizeof (UINT64)) + 1];
ASSERT (!((ExtendedData == NULL) && (ExtendedDataSize != 0)));
ASSERT (!((ExtendedData != NULL) && (ExtendedDataSize == 0)));
- if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
-return EFI_UNSUPPORTED;
- }
-
- //
- // Retrieve the current TPL
- //
- Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
- gBS->RestoreTPL (Tpl);
+ if (ExtendedDataSize <= (MAX_EXTENDED_DATA_SIZE - sizeof
(EFI_STATUS_CODE_DATA))) {
+//
+// Use Buffer instead of allocating if possible.
+//
+StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer; } else {
+if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
+ return EFI_UNSUPPORTED;
+}
- StatusCodeData = NULL;
- if (Tpl <= TPL_NOTIFY) {
//
-// Allocate space for the Status Code Header and its buffer
+// Retrieve the current TPL
//
-gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) +
ExtendedDataSize, (VOID **)&StatusCodeData);
- }
+Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
+gBS->RestoreTPL (Tpl);
+
+if (Tpl > TPL_NOTIFY) {
+ return EFI_OUT_OF_RESOURCES;
+}
- if (StatusCodeData == NULL) {
//
-// If a buffer could not be allocated, then see if the local variable
Buffer can be used
+// Allocate space for the Status Code Header and its buffer
//
-if (ExtendedDataSize > (MAX_EXTENDED_DATA_SIZE - sizeof
(EFI_STATUS_CODE_DATA))) {
- //
- // The local variable Buffer not large enough to hold the extended data
associated
- // with the status code being reported.
- //
- DEBUG ((EFI_D_ERROR, "Status code extended data is too large to be
reported!\n"));
+StatusCodeData = NULL;
+gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) +
ExtendedDataSize, (VOID **)&StatusCodeData);
+if (StatusCodeData == NULL) {
return EFI_OUT_OF_RESOURCES;
}
-StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer;
}
//
// Fill in the extended data header
//
@@ -626,6 +623,6 @@ EFIAPI
ReportDebugCodeEnabled (
VOID
)
{
return (BOOLEAN) ((PcdGet8 (PcdReportStatusCodePropertyMask) &
REPORT_STATUS_CODE_PROPERTY_DEBUG_CODE_ENABLED) != 0); -}
+}
\ No newline at end of file
diff --git
a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
index b73103517a..9b79854538 100644
--- a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
+++ b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCod
+++ eLib.c
@@ -630,16 +630,20 @@ ReportStatusCodeEx (
UINT64StatusCodeBuffer[(MAX_EXTENDED_DATA_SIZE / sizeof
(UINT64)) + 1];
ASSERT (!((ExtendedData == NULL) && (ExtendedDataSize != 0)));
ASSERT (!((ExtendedData != NULL) && (ExtendedDataSize == 0)));
- if (mHaveExitedBootServices) {
-if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize >
MAX_EXTENDED_DATA_SIZE) {
+
Re: [edk2] [PATCH] MdeModulePkg/PropertiesTableAttributesDxe: Remove this driver
Reviewed-by: Jian J Wang
> -Original Message-
> From: Zhang, Shenglei
> Sent: Monday, February 18, 2019 4:53 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Jian J ; Wu, Hao A ;
> Ni, Ray ; Zeng, Star
> Subject: [PATCH] MdeModulePkg/PropertiesTableAttributesDxe: Remove this
> driver
>
> This functionality of this driver has been deprecated and
> no platform employs this driver. It can be removed completely.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1475
>
> Cc: Jian J Wang
> Cc: Hao Wu
> Cc: Ray Ni
> Cc: Star Zeng
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Shenglei Zhang
> ---
> MdeModulePkg/MdeModulePkg.dsc | 1 -
> .../PropertiesTableAttributesDxe.c| 208 --
> .../PropertiesTableAttributesDxe.inf | 56 -
> .../PropertiesTableAttributesDxe.uni | 23 --
> .../PropertiesTableAttributesDxeExtra.uni | 23 --
> 5 files changed, 311 deletions(-)
> delete mode 100644
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttribu
> tesDxe.c
> delete mode 100644
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttribu
> tesDxe.inf
> delete mode 100644
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttribu
> tesDxe.uni
> delete mode 100644
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttribu
> tesDxeExtra.uni
>
> diff --git a/MdeModulePkg/MdeModulePkg.dsc
> b/MdeModulePkg/MdeModulePkg.dsc
> index 4f2ac8ae89..388bca25bd 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -413,7 +413,6 @@
>MdeModulePkg/Universal/EsrtDxe/EsrtDxe.inf
>MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmpDxe.inf
>
> -
> MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttribu
> tesDxe.inf
>MdeModulePkg/Universal/FileExplorerDxe/FileExplorerDxe.inf {
>
>
> FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> diff --git
> a/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttri
> butesDxe.c
> b/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttri
> butesDxe.c
> deleted file mode 100644
> index 4d1a46f64c..00
> ---
> a/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttri
> butesDxe.c
> +++ /dev/null
> @@ -1,208 +0,0 @@
> -/** @file
> - This module sets default policy for attributes of EfiACPIMemoryNVS and
> EfiReservedMemoryType.
> -
> - This module sets EFI_MEMORY_XP for attributes of EfiACPIMemoryNVS and
> EfiReservedMemoryType
> - in UEFI memory map, if and only of PropertiesTable is published and has
> BIT0
> set.
> -
> -Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
> -This program and the accompanying materials
> -are licensed and made available under the terms and conditions of the BSD
> License
> -which accompanies this distribution. The full text of the license may be
> found
> at
> -http://opensource.org/licenses/bsd-license.php
> -
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
> -
> -**/
> -
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -#include
> -
> -/**
> - Converts a number of EFI_PAGEs to a size in bytes.
> -
> - NOTE: Do not use EFI_PAGES_TO_SIZE because it handles UINTN only.
> -
> - @param Pages The number of EFI_PAGES.
> -
> - @return The number of bytes associated with the number of EFI_PAGEs
> specified
> - by Pages.
> -**/
> -UINT64
> -EfiPagesToSize (
> - IN UINT64 Pages
> - )
> -{
> - return LShiftU64 (Pages, EFI_PAGE_SHIFT);
> -}
> -
> -/**
> - Set memory attributes according to default policy.
> -
> - @param MemoryMapA pointer to the buffer in which firmware places
> the current memory map.
> - @param MemoryMapSizeSize, in bytes, of the MemoryMap buffer.
> - @param DescriptorSize size, in bytes, of an individual
> EFI_MEMORY_DESCRIPTOR.
> -**/
> -VOID
> -SetMemorySpaceAttributesDefault (
> - IN EFI_MEMORY_DESCRIPTOR *MemoryMap,
> - IN UINTN MemoryMapSize,
> - IN UINTN DescriptorSize
> - )
> -{
> - EFI_MEMORY_DESCRIPTOR *MemoryMapEntry;
> - EFI_MEMORY_DESCRIPTOR *MemoryMapEnd;
> - EFI_STATUS Status;
> -
> - DEBUG ((EFI_D_INFO, "SetMemorySpaceAttributesDefault\n"));
> -
> - MemoryMapEntry = MemoryMap;
> - MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) MemoryMap +
> MemoryMapSize);
> - while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) {
> -if (MemoryMapEntry->PhysicalStart < BASE_1MB) {
> - //
> - // Do not touch memory space below 1MB
> - //
> - MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry,
> DescriptorSize);
> - continue;
> -}
> -switch (MemoryMapEntry->Type) {
> -case EfiRuntimeServicesC
[edk2] [patch V4] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible
From: Max Knutsen
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1114
V2: simplify the code logic.
update
if (!mHaveExitedBootServices &&
(StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
gBS->FreePool (StatusCodeData);
}
to
if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
gBS->FreePool (StatusCodeData);
}
V3:
And the code below into the else condition (stack buffer is not enough)
in /DxeReportStatusCodeLib/ReportStatusCodeLib.c
if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
return EFI_UNSUPPORTED;
}
V4:
Refine code logic.
When report status code with ExtendedData data,
and the extended data can fit in the local static buffer,
there is no need to use AllocatePool to hold the ExtendedData data.
This patch is just to do the enhancement to avoid using AllocatePool.
Cc: Star Zeng
Cc: Jian J Wang
Cc: Hao Wu
Cc: Michael Turner
Cc: Liming Gao
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi
---
.../ReportStatusCodeLib.c | 45 +--
.../ReportStatusCodeLib.c | 14 +++---
2 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
index b28dc5c3bb..d3cf8b1de3 100644
--- a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
+++ b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
@@ -494,41 +494,38 @@ ReportStatusCodeEx (
UINT64Buffer[(MAX_EXTENDED_DATA_SIZE / sizeof (UINT64)) + 1];
ASSERT (!((ExtendedData == NULL) && (ExtendedDataSize != 0)));
ASSERT (!((ExtendedData != NULL) && (ExtendedDataSize == 0)));
- if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
-return EFI_UNSUPPORTED;
- }
-
- //
- // Retrieve the current TPL
- //
- Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
- gBS->RestoreTPL (Tpl);
+ if (ExtendedDataSize <= (MAX_EXTENDED_DATA_SIZE - sizeof
(EFI_STATUS_CODE_DATA))) {
+//
+// Use Buffer instead of allocating if possible.
+//
+StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer;
+ } else {
+if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
+ return EFI_UNSUPPORTED;
+}
- StatusCodeData = NULL;
- if (Tpl <= TPL_NOTIFY) {
//
-// Allocate space for the Status Code Header and its buffer
+// Retrieve the current TPL
//
-gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) +
ExtendedDataSize, (VOID **)&StatusCodeData);
- }
+Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
+gBS->RestoreTPL (Tpl);
+
+if (Tpl > TPL_NOTIFY) {
+ return EFI_OUT_OF_RESOURCES;
+}
- if (StatusCodeData == NULL) {
//
-// If a buffer could not be allocated, then see if the local variable
Buffer can be used
+// Allocate space for the Status Code Header and its buffer
//
-if (ExtendedDataSize > (MAX_EXTENDED_DATA_SIZE - sizeof
(EFI_STATUS_CODE_DATA))) {
- //
- // The local variable Buffer not large enough to hold the extended data
associated
- // with the status code being reported.
- //
- DEBUG ((EFI_D_ERROR, "Status code extended data is too large to be
reported!\n"));
+StatusCodeData = NULL;
+gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) +
ExtendedDataSize, (VOID **)&StatusCodeData);
+if (StatusCodeData == NULL) {
return EFI_OUT_OF_RESOURCES;
}
-StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer;
}
//
// Fill in the extended data header
//
@@ -626,6 +623,6 @@ EFIAPI
ReportDebugCodeEnabled (
VOID
)
{
return (BOOLEAN) ((PcdGet8 (PcdReportStatusCodePropertyMask) &
REPORT_STATUS_CODE_PROPERTY_DEBUG_CODE_ENABLED) != 0);
-}
+}
\ No newline at end of file
diff --git
a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
index b73103517a..9b79854538 100644
--- a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
+++ b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
@@ -630,16 +630,20 @@ ReportStatusCodeEx (
UINT64StatusCodeBuffer[(MAX_EXTENDED_DATA_SIZE / sizeof
(UINT64)) + 1];
ASSERT (!((ExtendedData == NULL) && (ExtendedDataSize != 0)));
ASSERT (!((ExtendedData != NULL) && (ExtendedDataSize == 0)));
- if (mHaveExitedBootServices) {
-if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize >
MAX_EXTENDED_DATA_SIZE) {
+ if (ExtendedDataSize <= (MAX_EXTENDED_DATA_SIZE - sizeof
(EFI_STATUS_CODE_DATA))) {
+//
+// Use Buffer instead of allocating if possible.
+//
+StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
+ } else {
+if (mHaveExitedBootServices) {
return EFI_OUT_OF_RESOURCES;
}
-StatusCodeDa
Re: [edk2] [PATCH] OvmfPkg/Sec: Clear the Cache Disable flag in the CR0 register
generic comment (applies to all NASM usage I guess): On 02/18/19 11:10, Jordan Justen wrote: > +mov eax, cr0 > +and eax, ~(1 << 30) > +mov cr0, eax > +mov rax, cr0 > +and eax, ~(1 << 30) > +mov cr0, rax I've read up on the << and ~ operators in the NASM documentation, and I think the above build-time calculations of the masks are well-defined and correct. - bit shifts are always unsigned - given bit position 30, ~(1 << 30) will be a value with 32 bits - bit-neg simply flips bits (one's complement) On the other hand, I find these NASM specifics counter-intuitive. The expression ~(1 << 30) looks like valid C, but in C, it means a quite different thing. I think calculating the mask with "strict dword" somehow (not exactly sure how) would make this more readable; or else the BTR instruction would. Opinions? (Again, pertaining to all NASM usage in edk2.) Thanks Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 10/10] OvmfPkg/Sec: Fill Temp Ram after TemporaryRamMigration
On 02/18/19 05:11, Jordan Justen wrote: > On some platforms, the TemporaryRamMigration PPI may cause Temporary > RAM to become inaccessible after the RAM is migrated. To emulate this > in OVMF, we should initialize memory to a bad state to make sure it > isn't accidentally being used after TemporaryRamMigration is called. > > I tested IA32 booting to shell and X64 booting to OS with this change. > With X64 I also tested S3 suspend/resume. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Anthony Perard > Cc: Julien Grall > --- > OvmfPkg/Sec/SecMain.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c > index 86c22a2ac9..72946e0eab 100644 > --- a/OvmfPkg/Sec/SecMain.c > +++ b/OvmfPkg/Sec/SecMain.c > @@ -948,6 +948,14 @@ TemporaryRamMigration ( > LongJump (&JumpBuffer, (UINTN)-1); >} > > + // > + // Initialize Temporary RAM to a bad value to make sure it will not > + // be used after migration. > + // > + SetMem32 ( > +(VOID*)(UINTN)TemporaryMemoryBase, CopySize, > +PcdGet32 (PcdInitValueInTempStack)); > + >SaveAndSetDebugTimerInterrupt (OldStatus); > >return EFI_SUCCESS; > Assuming the previous PEI Core patches in this series don't change the order, it looks like the PeiCheckAndSwitchStack() function [MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c] logs the "stack ever used" stat first, and calls the TemporaryRamMigration() PPI member second. Thus, by the time we overwrite the temp RAM in this patch, the statistics will have been printed in PeiCheckAndSwitchStack(). OK. Reviewed-by: Laszlo Ersek Thanks Laszlo ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 06/10] OvmfPkg/Sec: Disable optimizations for TemporaryRamMigration
On 02/18/19 10:32, Ard Biesheuvel wrote:
> On Mon, 18 Feb 2019 at 10:08, Jordan Justen wrote:
>>
>> On 2019-02-17 23:53:01, Ard Biesheuvel wrote:
>>> On Mon, 18 Feb 2019 at 05:12, Jordan Justen
>>> wrote:
>>>
>>> This needs an explanation why optimization needs to be disabled.
>>
>> I'm not sure this is required. The reason I added these patches is to
>> hopefully prevent the compiler from removing the frame pointer. We
>> adjust the frame pointer in the code, and that is a little sketchy if
>> the frame pointer isn't being used.
>>
>> Unfortunately, it can reasonably be argued that the
>> TemporaryRamSupport PPI definition ultimately makes it unsafe to write
>> the migration code in C.
>>
>> I tried reverting both the EmulatorPkg and OvmfPkg patches for
>> disabling the optimizations, and with my setup there was no impact. I
>> think there is a good change that we'd be pretty safe to just drop
>> these two patches to wait and see if someone encounters a situation
>> that requires it.
>>
>> Ok, so based on this explanation, do you think I should add info to
>> the commit message and keep the patches, or just drop them?
>>
>
> I think 'little sketchy' is an understatement here (as is
> setjmp/longjmp in general), but it is the reality we have to deal with
> when writing startup code in C. Looking at the code, I agree that the
> fact that [re]bp is assigned directly implies that we should not
> permit it to be used as a general purpose register, especially when
> you throw LTO into the mix, which could produce all kinds of
> surprising results when it decides to inline functions being called
> from here.
>
> For GCC/Clang, I don't think it is correct to assume that changing the
> optimization level will result in -fno-omit-frame-pointer to be set,
> so I'd prefer setting that option directly, either via the pragma, or
> for the whole file.
>
> For MSVC, I have no idea how to tweak the compiler to force it to emit
> frame pointers.
I think modifying the build options in the INF file would be more readable.
Thanks
Laszlo
>>>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jordan Justen
Cc: Laszlo Ersek
Cc: Ard Biesheuvel
Cc: Anthony Perard
Cc: Julien Grall
---
OvmfPkg/Sec/SecMain.c | 12
1 file changed, 12 insertions(+)
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 46ac739862..86c22a2ac9 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -873,6 +873,13 @@ SecStartupPhase2(
CpuDeadLoop ();
}
+#ifdef __GNUC__
+#pragma GCC push_options
+#pragma GCC optimize ("O0")
+#else
+#pragma optimize ("", off)
+#endif
+
EFI_STATUS
EFIAPI
TemporaryRamMigration (
@@ -946,3 +953,8 @@ TemporaryRamMigration (
return EFI_SUCCESS;
}
+#ifdef __GNUC__
+#pragma GCC pop_options
+#else
+#pragma optimize ("", on)
+#endif
>>>
>>> I can't tell from the context if this is the end of the file, but if
>>> it is not, aren't you turning on optimization here for non-GCC even if
>>> it was not enabled on the command line to begin with?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 05/10] OvmfPkg/Sec: Swap TemporaryRam Stack and Heap locations
On 02/18/19 05:11, Jordan Justen wrote: > Apparently something depends on the heap being above the stack after > TemporaryRamMigration. > > This is the opposite order that OVMF has always used for TempRam > before migration to permanent ram. In 42a83e80f37c (svn r10770), Mike > changed OVMF's TemporaryRamMigration to swap the locations of heap and > stack during the migration. > > Rather than doing the swap during TemporaryRamMigration, this change > causes OVMF to boot with TemporaryRam setup with heap being above the > stack. Then, during TemporaryRamMigration, we can directly copy all of > TemporaryRam. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Anthony Perard > Cc: Julien Grall > --- > OvmfPkg/Sec/Ia32/SecEntry.nasm | 2 +- > OvmfPkg/Sec/SecMain.c | 39 +- > OvmfPkg/Sec/X64/SecEntry.nasm | 2 +- > 3 files changed, 22 insertions(+), 21 deletions(-) > > diff --git a/OvmfPkg/Sec/Ia32/SecEntry.nasm b/OvmfPkg/Sec/Ia32/SecEntry.nasm > index 03501969eb..61917b9eef 100644 > --- a/OvmfPkg/Sec/Ia32/SecEntry.nasm > +++ b/OvmfPkg/Sec/Ia32/SecEntry.nasm > @@ -57,7 +57,7 @@ ASM_PFX(_ModuleEntryPoint): > ; Load temporary RAM stack based on PCDs > ; > %define SEC_TOP_OF_STACK (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) + \ > - FixedPcdGet32 (PcdOvmfSecPeiTempRamSize)) > + (FixedPcdGet32 (PcdOvmfSecPeiTempRamSize) / 2)) > mov eax, SEC_TOP_OF_STACK > mov esp, eax > nop > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c > index f7fec3d8c0..46ac739862 100644 > --- a/OvmfPkg/Sec/SecMain.c > +++ b/OvmfPkg/Sec/SecMain.c > @@ -1,7 +1,7 @@ > /** @file >Main SEC phase code. Transitions to PEI. > > - Copyright (c) 2008 - 2015, Intel Corporation. All rights reserved. > + Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved. >(C) Copyright 2016 Hewlett Packard Enterprise Development LP > >This program and the accompanying materials > @@ -779,15 +779,15 @@ SecCoreStartupWithStack ( > #endif > >// > - // |-| <-- TopOfCurrentStack > - // | Stack | 32k >// |-| >// |Heap | 32k > + // |-| <-- TopOfCurrentStack > + // | Stack | 32k >// |-| <-- SecCoreData.TemporaryRamBase >// > >ASSERT ((UINTN) (PcdGet32 (PcdOvmfSecPeiTempRamBase) + > - PcdGet32 (PcdOvmfSecPeiTempRamSize)) == > + (PcdGet32 (PcdOvmfSecPeiTempRamSize) / 2)) == >(UINTN) TopOfCurrentStack); > >// > @@ -795,14 +795,17 @@ SecCoreStartupWithStack ( >// >SecCoreData.DataSize = sizeof(EFI_SEC_PEI_HAND_OFF); > > - SecCoreData.TemporaryRamSize = (UINTN) PcdGet32 > (PcdOvmfSecPeiTempRamSize); > - SecCoreData.TemporaryRamBase = (VOID*)((UINT8 *)TopOfCurrentStack - > SecCoreData.TemporaryRamSize); > + SecCoreData.TemporaryRamBase = > +(VOID*)(UINTN) PcdGet32 (PcdOvmfSecPeiTempRamBase); > + SecCoreData.TemporaryRamSize = (UINTN) PcdGet32 (PcdOvmfSecPeiTempRamSize); > > - SecCoreData.PeiTemporaryRamBase= SecCoreData.TemporaryRamBase; > - SecCoreData.PeiTemporaryRamSize= SecCoreData.TemporaryRamSize >> 1; > + SecCoreData.PeiTemporaryRamBase = > +(UINT8*)(VOID*)(UINTN) PcdGet32 (PcdOvmfSecPeiTempRamBase) + > +((UINTN) PcdGet32 (PcdOvmfSecPeiTempRamSize) / 2); > + SecCoreData.PeiTemporaryRamSize = PcdGet32 (PcdOvmfSecPeiTempRamSize) / 2; > > - SecCoreData.StackBase = (UINT8 *)SecCoreData.TemporaryRamBase > + SecCoreData.PeiTemporaryRamSize; > - SecCoreData.StackSize = SecCoreData.TemporaryRamSize >> 1; > + SecCoreData.StackBase = (VOID*)(UINTN) PcdGet32 (PcdOvmfSecPeiTempRamBase); > + SecCoreData.StackSize = PcdGet32 (PcdOvmfSecPeiTempRamSize) / 2; > >SecCoreData.BootFirmwareVolumeBase = BootFv; >SecCoreData.BootFirmwareVolumeSize = (UINTN) BootFv->FvLength; > @@ -895,10 +898,10 @@ TemporaryRamMigration ( > (UINT64)CopySize > )); > > - OldHeap = (VOID*)(UINTN)TemporaryMemoryBase; > + OldHeap = (VOID*)((UINTN)TemporaryMemoryBase + (CopySize >> 1)); >NewHeap = (VOID*)((UINTN)PermanentMemoryBase + (CopySize >> 1)); > > - OldStack = (VOID*)((UINTN)TemporaryMemoryBase + (CopySize >> 1)); > + OldStack = (VOID*)(UINTN)TemporaryMemoryBase; >NewStack = (VOID*)(UINTN)PermanentMemoryBase; > >DebugAgentContext.HeapMigrateOffset = (UINTN)NewHeap - (UINTN)OldHeap; > @@ -908,15 +911,13 @@ TemporaryRamMigration ( >InitializeDebugAgent (DEBUG_AGENT_INIT_POSTMEM_SEC, (VOID *) > &DebugAgentContext, NULL); > >// > - // Migrate Heap > + // Migrate the whole temporary memory to permenent memory. s/permenent/permanent/ >// > - CopyMem (NewHeap, OldHeap, CopySize >> 1); > + CopyMem( > +(VOID*)(UINTN)PermanentMemoryBa
Re: [edk2] [PATCH] OvmfPkg/Sec: Clear the Cache Disable flag in the CR0 register
On 02/18/19 11:10, Jordan Justen wrote:
> Clear the CD (Cache Disable) flag in the CR0 register. When the VM
> implements the CD flag, this can substantially decrease the time it
> takes to decompress the firmware volumes.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jordan Justen
> Tested-by: Peter Fang
> Cc: Peter Fang
> Cc: Maurice Ma
> Cc: Laszlo Ersek
> Cc: Ard Biesheuvel
> Cc: Anthony Perard
> Cc: Julien Grall
> ---
> OvmfPkg/Sec/Ia32/SecEntry.nasm | 8 +++-
> OvmfPkg/Sec/X64/SecEntry.nasm | 8 +++-
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/Sec/Ia32/SecEntry.nasm b/OvmfPkg/Sec/Ia32/SecEntry.nasm
> index 03501969eb..fc7f47385a 100644
> --- a/OvmfPkg/Sec/Ia32/SecEntry.nasm
> +++ b/OvmfPkg/Sec/Ia32/SecEntry.nasm
> @@ -40,6 +40,13 @@ extern ASM_PFX(SecCoreStartupWithStack)
> global ASM_PFX(_ModuleEntryPoint)
> ASM_PFX(_ModuleEntryPoint):
>
> +;
> +; Clear the CD (Cache Disable) flag in the CR0 register.
> +;
> +mov eax, cr0
> +and eax, ~(1 << 30)
> +mov cr0, eax
> +
> ;
> ; Fill the temporary RAM with the initial stack value.
> ; The loop below will seed the heap as well, but that's harmless.
> @@ -71,4 +78,3 @@ ASM_PFX(_ModuleEntryPoint):
> pusheax
> pushebp
> callASM_PFX(SecCoreStartupWithStack)
> -
> diff --git a/OvmfPkg/Sec/X64/SecEntry.nasm b/OvmfPkg/Sec/X64/SecEntry.nasm
> index d76adcffd8..7471b3a3e3 100644
> --- a/OvmfPkg/Sec/X64/SecEntry.nasm
> +++ b/OvmfPkg/Sec/X64/SecEntry.nasm
> @@ -41,6 +41,13 @@ extern ASM_PFX(SecCoreStartupWithStack)
> global ASM_PFX(_ModuleEntryPoint)
> ASM_PFX(_ModuleEntryPoint):
>
> +;
> +; Clear the CD (Cache Disable) flag in the CR0 register.
> +;
> +mov rax, cr0
> +and eax, ~(1 << 30)
> +mov cr0, rax
> +
> ;
> ; Fill the temporary RAM with the initial stack value.
> ; The loop below will seed the heap as well, but that's harmless.
> @@ -72,4 +79,3 @@ ASM_PFX(_ModuleEntryPoint):
> mov rdx, rsp
> sub rsp, 0x20
> callASM_PFX(SecCoreStartupWithStack)
> -
>
In commit 98f378a7be12 ("OvmfPkg/ResetVector: enable caching in initial
page tables", 2013-09-24), we said
In UEFI X64 we use other mechanisms to disable caching.
(CD/NW in CR0 and MTRRs.)
That suggests that having caching disabled in SEC is a good thing.
What has changed? I assume Peter reported a problem.
... Is this by any chance related to Linux commit 879ae1880449 ("KVM:
x86: obey KVM_X86_QUIRK_CD_NW_CLEARED in kvm_set_cr0()", 2015-11-04) --
or CR0.CD virtualization in KVM, in general?
(The commit message says "When the VM implements the CD flag", and not
"When the *VMM* implements the CD flag", so I'm unsure.)
Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] OvmfPkg/build.sh: Enable flash for qemu 3 or later
On 02/16/19 09:18, Jordan Justen wrote:
> The check for 1.[1-9][0-9].* was removed since qemu jumped to 2.0
> after 1.7.
>
> Changed 2.*.* to [2-9].*.* to match major releases 3 - 9.
>
> Added [1-9][0-9]*.*.* to match major releses >= 10.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jordan Justen
> Cc: Laszlo Ersek
> Cc: Ard Biesheuvel
> Cc: Anthony Perard
> Cc: Julien Grall
> ---
> OvmfPkg/build.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
> index 6821742e7a..97863b09f2 100755
> --- a/OvmfPkg/build.sh
> +++ b/OvmfPkg/build.sh
> @@ -215,7 +215,7 @@ if [[ "$RUN_QEMU" == "yes" ]]; then
> grep -o -E 'version [0-9]+\.[0-9]+\.[0-9]+' | \
> awk '{print $2}')
>case $qemu_version in
> -1.[6-9].*|1.[1-9][0-9].*|2.*.*)
> +1.[6-9].*|[2-9].*.*|[1-9][0-9]*.*.*)
>ENABLE_FLASH=yes
>;;
>esac
>
Reviewed-by: Laszlo Ersek
Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1140
In eMMC HS400 switch sequence flow eMMC driver attempted
to execute SEND_STATUS just after switching bus timing to high
speed and before downgrading clock frequency to 52MHz. Since link
was at that time in incorrect state SEND_STATUS was failing which
made driver think switch to HS400 failed.
This change makes driver always change clock frequency after
switching bus timing and before executing SEND_STATUS.
Cc: Hao Wu
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Albecki Mateusz
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 39 +
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index 4ef849fd0962..15db8a87a5c4 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -642,7 +642,7 @@ EmmcSwitchBusWidth (
}
/**
- Switch the clock frequency to the specified value.
+ Switch the bus timing and clock frequency.
Refer to EMMC Electrical Standard Spec 5.1 Section 6.6 and SD Host Controller
Simplified Spec 3.0 Figure 3-3 for details.
@@ -660,7 +660,7 @@ EmmcSwitchBusWidth (
**/
EFI_STATUS
-EmmcSwitchClockFreq (
+EmmcSwitchBusTiming (
IN EFI_PCI_IO_PROTOCOL*PciIo,
IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru,
IN UINT8 Slot,
@@ -689,22 +689,10 @@ EmmcSwitchClockFreq (
Status = EmmcSwitch (PassThru, Slot, Access, Index, Value, CmdSet);
if (EFI_ERROR (Status)) {
-DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Switch to hstiming %d fails
with %r\n", HsTiming, Status));
+DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Switch to hstiming %d fails
with %r\n", HsTiming, Status));
return Status;
}
- Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus);
- if (EFI_ERROR (Status)) {
-DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Send status fails with %r\n",
Status));
-return Status;
- }
- //
- // Check the switch operation is really successful or not.
- //
- if ((DevStatus & BIT7) != 0) {
-DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: The switch operation fails as
DevStatus is 0x%08x\n", DevStatus));
-return EFI_DEVICE_ERROR;
- }
//
// Convert the clock freq unit from MHz to KHz.
//
@@ -713,6 +701,19 @@ EmmcSwitchClockFreq (
return Status;
}
+ Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus);
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Send status fails with %r\n",
Status));
+return Status;
+ }
+ //
+ // Check the switch operation is really successful or not.
+ //
+ if ((DevStatus & BIT7) != 0) {
+DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: The switch operation fails as
DevStatus is 0x%08x\n", DevStatus));
+return EFI_DEVICE_ERROR;
+ }
+
if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
Status = mOverride->NotifyPhase (
Private->ControllerHandle,
@@ -799,7 +800,7 @@ EmmcSwitchToHighSpeed (
}
HsTiming = 1;
- Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing,
ClockFreq);
+ Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing,
ClockFreq);
return Status;
}
@@ -887,7 +888,7 @@ EmmcSwitchToHS200 (
Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof
(ClockCtrl), &ClockCtrl);
HsTiming = 2;
- Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing,
ClockFreq);
+ Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing,
ClockFreq);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -937,7 +938,7 @@ EmmcSwitchToHS400 (
// Set to Hight Speed timing and set the clock frequency to a value less
than 52MHz.
//
HsTiming = 1;
- Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming,
SdMmcMmcHsSdr, 52);
+ Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming,
SdMmcMmcHsSdr, 52);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -957,7 +958,7 @@ EmmcSwitchToHS400 (
}
HsTiming = 3;
- Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing,
ClockFreq);
+ Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing,
ClockFreq);
return Status;
}
--
2.14.1.windows.1
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
przegladani
[edk2] [PATCH 2/4] MdeModulePkg: Add a new API ResetSystem for DXE ResetSystemLib
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1460
Add a new API ResetSystem for DXE ResetSystemLib. So the consumer of
ResetSystemLib can use this API to reset system with additional reset
data.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao
Cc: Ray Ni
Cc: Liming Gao
---
.../Library/DxeResetSystemLib/DxeResetSystemLib.c | 28 +-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
index ea4878cab1..f5c7386c9a 100644
--- a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
@@ -1,7 +1,7 @@
/** @file
DXE Reset System Library instance that calls gRT->ResetSystem().
- Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
which accompanies this distribution. The full text of the license may be
found at
@@ -96,3 +96,29 @@ ResetPlatformSpecific (
{
gRT->ResetSystem (EfiResetPlatformSpecific, EFI_SUCCESS, DataSize,
ResetData);
}
+
+/**
+ The ResetSystem function resets the entire platform.
+
+ @param[in] ResetType The type of reset to perform.
+ @param[in] ResetStatusThe status code for the reset.
+ @param[in] DataSize The size, in bytes, of ResetData.
+ @param[in] ResetData For a ResetType of EfiResetCold, EfiResetWarm, or
EfiResetShutdown
+the data buffer starts with a Null-terminated
string, optionally
+followed by additional binary data. The string is
a description
+that the caller may use to further indicate the
reason for the
+system reset. ResetData is only valid if
ResetStatus is something
+other than EFI_SUCCESS unless the ResetType is
EfiResetPlatformSpecific
+where a minimum amount of ResetData is always
required.
+**/
+VOID
+EFIAPI
+ResetSystem (
+ IN EFI_RESET_TYPE ResetType,
+ IN EFI_STATUS ResetStatus,
+ IN UINTNDataSize,
+ IN VOID *ResetData OPTIONAL
+ )
+{
+ gRT->ResetSystem (ResetType, ResetStatus, DataSize, ResetData);
+}
--
2.16.2.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/4] Add a new API ResetSystem for ResetSystemLib
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1460 Add a new API ResetSystem for ResetSystemLib so that the consumer can use this interface to reset system with additional reset data. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Zhichao Gao Cc: Ray Ni Cc: Liming Gao Zhichao Gao (4): MdeModulePkg: change the function name ResetSystem MdeModulePkg: Add a new API ResetSystem for DXE ResetSystemLib MdeModulePkg: Add a new API ResetSystem for PEI ResetSystemLib MdeModulePkg: Add the new API ResetSystem in the head file MdeModulePkg/Include/Library/ResetSystemLib.h | 25 ++- .../Library/DxeResetSystemLib/DxeResetSystemLib.c | 28 +- .../Library/PeiResetSystemLib/PeiResetSystemLib.c | 28 +- .../Universal/ResetSystemRuntimeDxe/ResetSystem.c | 8 +++ .../Universal/ResetSystemRuntimeDxe/ResetSystem.h | 4 ++-- 5 files changed, 84 insertions(+), 9 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 3/4] MdeModulePkg: Add a new API ResetSystem for PEI ResetSystemLib
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1460
Add a new API ResetSystem for PEI ResetSystemLib to be in accord with
DXE instance.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao
Cc: Ray Ni
Cc: Liming Gao
---
.../Library/PeiResetSystemLib/PeiResetSystemLib.c | 28 +-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
index d8219775d1..f35acb3c6e 100644
--- a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
+++ b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
@@ -1,7 +1,7 @@
/** @file
PEI Reset System Library instance that calls the ResetSystem2() PEI Service.
- Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
which accompanies this distribution. The full text of the license may be
found at
@@ -96,3 +96,29 @@ ResetPlatformSpecific (
{
PeiServicesResetSystem2 (EfiResetPlatformSpecific, EFI_SUCCESS, DataSize,
ResetData);
}
+
+/**
+ The ResetSystem function resets the entire platform.
+
+ @param[in] ResetType The type of reset to perform.
+ @param[in] ResetStatusThe status code for the reset.
+ @param[in] DataSize The size, in bytes, of ResetData.
+ @param[in] ResetData For a ResetType of EfiResetCold, EfiResetWarm, or
EfiResetShutdown
+the data buffer starts with a Null-terminated
string, optionally
+followed by additional binary data. The string is
a description
+that the caller may use to further indicate the
reason for the
+system reset. ResetData is only valid if
ResetStatus is something
+other than EFI_SUCCESS unless the ResetType is
EfiResetPlatformSpecific
+where a minimum amount of ResetData is always
required.
+**/
+VOID
+EFIAPI
+ResetSystem (
+ IN EFI_RESET_TYPE ResetType,
+ IN EFI_STATUS ResetStatus,
+ IN UINTNDataSize,
+ IN VOID *ResetData OPTIONAL
+ )
+{
+ PeiServicesResetSystem2 (ResetType, ResetStatus, DataSize, ResetData);
+}
--
2.16.2.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 4/4] MdeModulePkg: Add the new API ResetSystem in the head file
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1460 Add the new API ResetSystem in the related head file so that the consumer can use it through the combination of library instance and head file. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Zhichao Gao Cc: Ray Ni Cc: Liming Gao https://bugzilla.tianocore.org/show_bug.cgi?id=1460 --- MdeModulePkg/Include/Library/ResetSystemLib.h | 25 - 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Include/Library/ResetSystemLib.h b/MdeModulePkg/Include/Library/ResetSystemLib.h index 55d1923ae1..2f5d15ade8 100644 --- a/MdeModulePkg/Include/Library/ResetSystemLib.h +++ b/MdeModulePkg/Include/Library/ResetSystemLib.h @@ -2,7 +2,7 @@ System reset Library Services. This library class defines a set of methods that reset the whole system. -Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved. +Copyright (c) 2005 - 2019, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License that accompanies this distribution. The full text of the license may be found at @@ -83,4 +83,27 @@ ResetPlatformSpecific ( IN VOID*ResetData ); +/** + The ResetSystem function resets the entire platform. + + @param[in] ResetType The type of reset to perform. + @param[in] ResetStatusThe status code for the reset. + @param[in] DataSize The size, in bytes, of ResetData. + @param[in] ResetData For a ResetType of EfiResetCold, EfiResetWarm, or EfiResetShutdown +the data buffer starts with a Null-terminated string, optionally +followed by additional binary data. The string is a description +that the caller may use to further indicate the reason for the +system reset. ResetData is only valid if ResetStatus is something +other than EFI_SUCCESS unless the ResetType is EfiResetPlatformSpecific +where a minimum amount of ResetData is always required. +**/ +VOID +EFIAPI +ResetSystem ( + IN EFI_RESET_TYPE ResetType, + IN EFI_STATUS ResetStatus, + IN UINTNDataSize, + IN VOID *ResetData OPTIONAL + ); + #endif -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/4] MdeModulePkg: change the function name ResetSystem
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1460
Change the function name form ResetSystem to EfiRuntimeResetSystem.
Because ResetSystem and EfiResetSystem would be used in ResetSystemLib
and RuntimeLib.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao
Cc: Ray Ni
Cc: Liming Gao
---
MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c | 8
MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
index afc35587fc..e16b0cda7b 100644
--- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
+++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
@@ -1,7 +1,7 @@
/** @file
Reset Architectural and Reset Notification protocols implementation.
- Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -187,7 +187,7 @@ InitializeResetSystem (
//
// Hook the runtime service table
//
- gRT->ResetSystem = ResetSystem;
+ gRT->ResetSystem = EfiRuntimeResetSystem;
//
// Now install the Reset RT AP on a new handle
@@ -242,7 +242,7 @@ DoS3 (
**/
VOID
EFIAPI
-ResetSystem (
+EfiRuntimeResetSystem (
IN EFI_RESET_TYPE ResetType,
IN EFI_STATUS ResetStatus,
IN UINTNDataSize,
@@ -256,7 +256,7 @@ ResetSystem (
RESET_NOTIFY_ENTRY *Entry;
//
- // Only do REPORT_STATUS_CODE() on first call to ResetSystem()
+ // Only do REPORT_STATUS_CODE() on first call to EfiRuntimeResetSystem()
//
if (mResetNotifyDepth == 0) {
//
diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h
b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h
index 8529de675c..448e30f079 100644
--- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h
+++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
@@ -97,7 +97,7 @@ InitializeResetSystem (
**/
VOID
EFIAPI
-ResetSystem (
+EfiRuntimeResetSystem (
IN EFI_RESET_TYPE ResetType,
IN EFI_STATUS ResetStatus,
IN UINTNDataSize,
--
2.16.2.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] OvmfPkg/Sec: Clear the Cache Disable flag in the CR0 register
Clear the CD (Cache Disable) flag in the CR0 register. When the VM implements the CD flag, this can substantially decrease the time it takes to decompress the firmware volumes. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jordan Justen Tested-by: Peter Fang Cc: Peter Fang Cc: Maurice Ma Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc: Anthony Perard Cc: Julien Grall --- OvmfPkg/Sec/Ia32/SecEntry.nasm | 8 +++- OvmfPkg/Sec/X64/SecEntry.nasm | 8 +++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/OvmfPkg/Sec/Ia32/SecEntry.nasm b/OvmfPkg/Sec/Ia32/SecEntry.nasm index 03501969eb..fc7f47385a 100644 --- a/OvmfPkg/Sec/Ia32/SecEntry.nasm +++ b/OvmfPkg/Sec/Ia32/SecEntry.nasm @@ -40,6 +40,13 @@ extern ASM_PFX(SecCoreStartupWithStack) global ASM_PFX(_ModuleEntryPoint) ASM_PFX(_ModuleEntryPoint): +; +; Clear the CD (Cache Disable) flag in the CR0 register. +; +mov eax, cr0 +and eax, ~(1 << 30) +mov cr0, eax + ; ; Fill the temporary RAM with the initial stack value. ; The loop below will seed the heap as well, but that's harmless. @@ -71,4 +78,3 @@ ASM_PFX(_ModuleEntryPoint): pusheax pushebp callASM_PFX(SecCoreStartupWithStack) - diff --git a/OvmfPkg/Sec/X64/SecEntry.nasm b/OvmfPkg/Sec/X64/SecEntry.nasm index d76adcffd8..7471b3a3e3 100644 --- a/OvmfPkg/Sec/X64/SecEntry.nasm +++ b/OvmfPkg/Sec/X64/SecEntry.nasm @@ -41,6 +41,13 @@ extern ASM_PFX(SecCoreStartupWithStack) global ASM_PFX(_ModuleEntryPoint) ASM_PFX(_ModuleEntryPoint): +; +; Clear the CD (Cache Disable) flag in the CR0 register. +; +mov rax, cr0 +and eax, ~(1 << 30) +mov cr0, rax + ; ; Fill the temporary RAM with the initial stack value. ; The loop below will seed the heap as well, but that's harmless. @@ -72,4 +79,3 @@ ASM_PFX(_ModuleEntryPoint): mov rdx, rsp sub rsp, 0x20 callASM_PFX(SecCoreStartupWithStack) - -- 2.20.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] BaseTools:PCD value error in structure pcd sku case.
Defined 2 PCDs(Test4 & Test401) and 2 SKUs(DEFAULT & _),
then set "SKUID_Defines" to ALL, for FixedAtBuild
gEfiStructuredPcdPkgTokenSpaceGuid. Test401 in this case,
its value should get from "Default" SKU, not from "_" SKU,
but we does not set value in SKU "_" in dsc, so Test401
should only display the value get from dec.
Cc: Bob Feng
Cc: Liming Gao
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhiju.Fan
---
BaseTools/Source/Python/Common/GlobalData.py | 2 +-
BaseTools/Source/Python/Workspace/DscBuildData.py | 3 +++
BaseTools/Source/Python/build/BuildReport.py | 5 -
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Source/Python/Common/GlobalData.py
b/BaseTools/Source/Python/Common/GlobalData.py
index 5eaee06694..f117998b0b 100644
--- a/BaseTools/Source/Python/Common/GlobalData.py
+++ b/BaseTools/Source/Python/Common/GlobalData.py
@@ -103,7 +103,7 @@ MixedPcd = {}
# Structure Pcd dict
gStructurePcd = {}
-
+gPcdSkuOverrides={}
# Pcd name for the Pcd which used in the Conditional directives
gConditionalPcds = []
diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
b/BaseTools/Source/Python/Workspace/DscBuildData.py
index 1fd1639ab6..5daefe835e 100644
--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
@@ -1537,6 +1537,9 @@ class DscBuildData(PlatformBuildClassObject):
stru_pcd.SkuOverrideValues[skuid] =
copy.deepcopy(stru_pcd.SkuOverrideValues[nextskuid]) if not NoDefault else
copy.deepcopy({defaultstorename: stru_pcd.DefaultValues for defaultstorename in
DefaultStores} if DefaultStores else {})
#{TAB_DEFAULT_STORES_DEFAULT:stru_pcd.DefaultValues})
if not NoDefault:
stru_pcd.ValueChain.add((skuid, ''))
+if 'DEFAULT' in stru_pcd.SkuOverrideValues and not
GlobalData.gPcdSkuOverrides.get((stru_pcd.TokenCName,
stru_pcd.TokenSpaceGuidCName)):
+GlobalData.gPcdSkuOverrides.update(
+{(stru_pcd.TokenCName, stru_pcd.TokenSpaceGuidCName):
{'DEFAULT':stru_pcd.SkuOverrideValues['DEFAULT']}})
if stru_pcd.Type in
[self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_HII],
self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_EX_HII]]:
for skuid in SkuIds:
nextskuid = skuid
diff --git a/BaseTools/Source/Python/build/BuildReport.py
b/BaseTools/Source/Python/build/BuildReport.py
index 0b98d62cb6..358fdd82d6 100644
--- a/BaseTools/Source/Python/build/BuildReport.py
+++ b/BaseTools/Source/Python/build/BuildReport.py
@@ -1295,7 +1295,10 @@ class PcdReport(object):
FileWrite(File, ' %-*s : %6s %10s = %s' % (self.MaxLen, Flag
+ ' ' + PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', Value))
if IsStructure:
FiledOverrideFlag = False
-OverrideValues = Pcd.SkuOverrideValues
+if (Pcd.TokenCName,Pcd.TokenSpaceGuidCName) in
GlobalData.gPcdSkuOverrides:
+OverrideValues =
GlobalData.gPcdSkuOverrides[(Pcd.TokenCName,Pcd.TokenSpaceGuidCName)]
+else:
+OverrideValues = Pcd.SkuOverrideValues
if OverrideValues:
for Data in OverrideValues.values():
Struct = list(Data.values())
--
2.14.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch V3] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible
Hi Dandan,
/DxeReportStatusCodeLib/ReportStatusCodeLib.c:
The " Retrieve the current TPL " code block is not needed also when using stack
buffer, so it can be also moved to else condition.
And the code block below is redundant, so it can be removed.
if (StatusCodeData == NULL) {
//
// If a buffer could not be allocated, then see if the local variable
Buffer can be used
//
if (ExtendedDataSize > (MAX_EXTENDED_DATA_SIZE - sizeof
(EFI_STATUS_CODE_DATA))) {
//
// The local variable Buffer not large enough to hold the extended data
associated
// with the status code being reported.
//
DEBUG ((EFI_D_ERROR, "Status code extended data is too large to be
reported!\n"));
return EFI_OUT_OF_RESOURCES;
}
StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer;
}
The code for /RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c can be also
refined.
To be easily understand, check
https://github.com/lzeng14/edk2/tree/ReportStatusCode for reference.
Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dandan Bi
Sent: Monday, February 18, 2019 10:43 AM
To: edk2-devel@lists.01.org
Cc: Max Knutsen ; Wu, Hao A ;
Michael Turner ; Gao, Liming
; Zeng, Star
Subject: [edk2] [patch V3] MdeModulePkg/ReportStatusCodeLib: Avoid using
AllocatePool if possible
From: Max Knutsen
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1114
V2: simplify the code logic.
update
if (!mHaveExitedBootServices &&
(StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
gBS->FreePool (StatusCodeData);
}
to
if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
gBS->FreePool (StatusCodeData);
}
V3:
And the code below into the else condition (stack buffer is not enough) in
/DxeReportStatusCodeLib/ReportStatusCodeLib.c
if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
return EFI_UNSUPPORTED;
}
When report status code with ExtendedData data, and the extended data can fit
in the local static buffer, there is no need to use AllocatePool to hold the
ExtendedData data.
This patch is just to do the enhancement to avoid using AllocatePool.
Cc: Star Zeng
Cc: Jian J Wang
Cc: Hao Wu
Cc: Michael Turner
Cc: Liming Gao
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi
---
.../DxeReportStatusCodeLib/ReportStatusCodeLib.c | 15 +--
.../ReportStatusCodeLib.c | 9 +++--
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
index b28dc5c3bb..3bca0a94ee 100644
--- a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
+++ b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
@@ -494,24 +494,27 @@ ReportStatusCodeEx (
UINT64Buffer[(MAX_EXTENDED_DATA_SIZE / sizeof (UINT64)) + 1];
ASSERT (!((ExtendedData == NULL) && (ExtendedDataSize != 0)));
ASSERT (!((ExtendedData != NULL) && (ExtendedDataSize == 0)));
- if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
-return EFI_UNSUPPORTED;
- }
-
//
// Retrieve the current TPL
//
Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
gBS->RestoreTPL (Tpl);
StatusCodeData = NULL;
- if (Tpl <= TPL_NOTIFY) {
+ if (ExtendedDataSize <= (MAX_EXTENDED_DATA_SIZE - sizeof
(EFI_STATUS_CODE_DATA))) {
+//
+// Use Buffer instead of allocating if possible.
+//
+StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer; } else if (gBS ==
+ NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
+return EFI_UNSUPPORTED;
+ } else if (Tpl <= TPL_NOTIFY){
//
-// Allocate space for the Status Code Header and its buffer
+// If Buffer is not big enough, allocate space for the Status Code
+ Header and its buffer
//
gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) +
ExtendedDataSize, (VOID **)&StatusCodeData);
}
if (StatusCodeData == NULL) {
diff --git
a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
index b73103517a..75e2f88ad2 100644
--- a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
+++ b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCod
+++ eLib.c
@@ -635,11 +635,14 @@ ReportStatusCodeEx (
if (mHaveExitedBootServices) {
if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize >
MAX_EXTENDED_DATA_SIZE) {
return EFI_OUT_OF_RESOURCES;
}
StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
- } else {
+ } else if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize >
MAX_EXTENDED_DATA_SIZE) {
+//
+// Only use AllocatePool for larger ExtendedData.
+//
if (gBS == NULL || gBS->AllocatePool ==
[edk2] [Patch] BaseTools: Fixed a bug in Vpd handling
If there are multiple sku used in a platform and
gEfiMdeModulePkgTokenSpaceGuid.PcdNvStoreDefaultValueBuffer PCD
is used, build will fail.
This is a regression issue introduced by the commit:
5695877ec8f636bd4ad873ef50eceb9da7a0f382 which only update the
Vpd offset for default SKU but not other SKUs.
This patch is going to fix this issue.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng
Cc: Liming Gao
---
BaseTools/Source/Python/AutoGen/AutoGen.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 2452ecbcba..8370ee0c93 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -1686,11 +1686,11 @@ class PlatformAutoGen(AutoGen):
PcdNvStoreDfBuffer = [item for item in self._DynamicPcdList if
item.TokenCName == "PcdNvStoreDefaultValueBuffer" and item.TokenSpaceGuidCName
== "gEfiMdeModulePkgTokenSpaceGuid"]
if PcdNvStoreDfBuffer:
PcdName,PcdGuid = PcdNvStoreDfBuffer[0].TokenCName,
PcdNvStoreDfBuffer[0].TokenSpaceGuidCName
if (PcdName,PcdGuid) in VpdSkuMap:
DefaultSku =
PcdNvStoreDfBuffer[0].SkuInfoList.get(TAB_DEFAULT)
-VpdSkuMap[(PcdName,PcdGuid)] =
{DefaultSku.DefaultValue:[DefaultSku]}
+VpdSkuMap[(PcdName,PcdGuid)] =
{DefaultSku.DefaultValue:[SkuObj for SkuObj in
PcdNvStoreDfBuffer[0].SkuInfoList.values() ]}
# Process VPD map file generated by third party BPDG tool
if NeedProcessVpdMapFile:
VpdMapFilePath = os.path.join(self.BuildDir,
TAB_FV_DIRECTORY, "%s.map" % self.Platform.VpdToolGuid)
if os.path.exists(VpdMapFilePath):
--
2.20.1.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 06/10] OvmfPkg/Sec: Disable optimizations for TemporaryRamMigration
On Mon, 18 Feb 2019 at 10:08, Jordan Justen wrote:
>
> On 2019-02-17 23:53:01, Ard Biesheuvel wrote:
> > On Mon, 18 Feb 2019 at 05:12, Jordan Justen
> > wrote:
> > >
> >
> > This needs an explanation why optimization needs to be disabled.
>
> I'm not sure this is required. The reason I added these patches is to
> hopefully prevent the compiler from removing the frame pointer. We
> adjust the frame pointer in the code, and that is a little sketchy if
> the frame pointer isn't being used.
>
> Unfortunately, it can reasonably be argued that the
> TemporaryRamSupport PPI definition ultimately makes it unsafe to write
> the migration code in C.
>
> I tried reverting both the EmulatorPkg and OvmfPkg patches for
> disabling the optimizations, and with my setup there was no impact. I
> think there is a good change that we'd be pretty safe to just drop
> these two patches to wait and see if someone encounters a situation
> that requires it.
>
> Ok, so based on this explanation, do you think I should add info to
> the commit message and keep the patches, or just drop them?
>
I think 'little sketchy' is an understatement here (as is
setjmp/longjmp in general), but it is the reality we have to deal with
when writing startup code in C. Looking at the code, I agree that the
fact that [re]bp is assigned directly implies that we should not
permit it to be used as a general purpose register, especially when
you throw LTO into the mix, which could produce all kinds of
surprising results when it decides to inline functions being called
from here.
For GCC/Clang, I don't think it is correct to assume that changing the
optimization level will result in -fno-omit-frame-pointer to be set,
so I'd prefer setting that option directly, either via the pragma, or
for the whole file.
For MSVC, I have no idea how to tweak the compiler to force it to emit
frame pointers.
> >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Jordan Justen
> > > Cc: Laszlo Ersek
> > > Cc: Ard Biesheuvel
> > > Cc: Anthony Perard
> > > Cc: Julien Grall
> > > ---
> > > OvmfPkg/Sec/SecMain.c | 12
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
> > > index 46ac739862..86c22a2ac9 100644
> > > --- a/OvmfPkg/Sec/SecMain.c
> > > +++ b/OvmfPkg/Sec/SecMain.c
> > > @@ -873,6 +873,13 @@ SecStartupPhase2(
> > >CpuDeadLoop ();
> > > }
> > >
> > > +#ifdef __GNUC__
> > > +#pragma GCC push_options
> > > +#pragma GCC optimize ("O0")
> > > +#else
> > > +#pragma optimize ("", off)
> > > +#endif
> > > +
> > > EFI_STATUS
> > > EFIAPI
> > > TemporaryRamMigration (
> > > @@ -946,3 +953,8 @@ TemporaryRamMigration (
> > >return EFI_SUCCESS;
> > > }
> > >
> > > +#ifdef __GNUC__
> > > +#pragma GCC pop_options
> > > +#else
> > > +#pragma optimize ("", on)
> > > +#endif
> >
> > I can't tell from the context if this is the end of the file, but if
> > it is not, aren't you turning on optimization here for non-GCC even if
> > it was not enabled on the command line to begin with?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 06/10] OvmfPkg/Sec: Disable optimizations for TemporaryRamMigration
On 2019-02-17 23:53:01, Ard Biesheuvel wrote:
> On Mon, 18 Feb 2019 at 05:12, Jordan Justen wrote:
> >
>
> This needs an explanation why optimization needs to be disabled.
I'm not sure this is required. The reason I added these patches is to
hopefully prevent the compiler from removing the frame pointer. We
adjust the frame pointer in the code, and that is a little sketchy if
the frame pointer isn't being used.
Unfortunately, it can reasonably be argued that the
TemporaryRamSupport PPI definition ultimately makes it unsafe to write
the migration code in C.
I tried reverting both the EmulatorPkg and OvmfPkg patches for
disabling the optimizations, and with my setup there was no impact. I
think there is a good change that we'd be pretty safe to just drop
these two patches to wait and see if someone encounters a situation
that requires it.
Ok, so based on this explanation, do you think I should add info to
the commit message and keep the patches, or just drop them?
Thanks,
-Jordan
>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jordan Justen
> > Cc: Laszlo Ersek
> > Cc: Ard Biesheuvel
> > Cc: Anthony Perard
> > Cc: Julien Grall
> > ---
> > OvmfPkg/Sec/SecMain.c | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
> > index 46ac739862..86c22a2ac9 100644
> > --- a/OvmfPkg/Sec/SecMain.c
> > +++ b/OvmfPkg/Sec/SecMain.c
> > @@ -873,6 +873,13 @@ SecStartupPhase2(
> >CpuDeadLoop ();
> > }
> >
> > +#ifdef __GNUC__
> > +#pragma GCC push_options
> > +#pragma GCC optimize ("O0")
> > +#else
> > +#pragma optimize ("", off)
> > +#endif
> > +
> > EFI_STATUS
> > EFIAPI
> > TemporaryRamMigration (
> > @@ -946,3 +953,8 @@ TemporaryRamMigration (
> >return EFI_SUCCESS;
> > }
> >
> > +#ifdef __GNUC__
> > +#pragma GCC pop_options
> > +#else
> > +#pragma optimize ("", on)
> > +#endif
>
> I can't tell from the context if this is the end of the file, but if
> it is not, aren't you turning on optimization here for non-GCC even if
> it was not enabled on the command line to begin with?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] MdeModulePkg/PropertiesTableAttributesDxe: Remove this driver
This functionality of this driver has been deprecated and
no platform employs this driver. It can be removed completely.
https://bugzilla.tianocore.org/show_bug.cgi?id=1475
Cc: Jian J Wang
Cc: Hao Wu
Cc: Ray Ni
Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Shenglei Zhang
---
MdeModulePkg/MdeModulePkg.dsc | 1 -
.../PropertiesTableAttributesDxe.c| 208 --
.../PropertiesTableAttributesDxe.inf | 56 -
.../PropertiesTableAttributesDxe.uni | 23 --
.../PropertiesTableAttributesDxeExtra.uni | 23 --
5 files changed, 311 deletions(-)
delete mode 100644
MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.c
delete mode 100644
MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.inf
delete mode 100644
MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.uni
delete mode 100644
MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxeExtra.uni
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 4f2ac8ae89..388bca25bd 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -413,7 +413,6 @@
MdeModulePkg/Universal/EsrtDxe/EsrtDxe.inf
MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmpDxe.inf
-
MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.inf
MdeModulePkg/Universal/FileExplorerDxe/FileExplorerDxe.inf {
FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
diff --git
a/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.c
b/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.c
deleted file mode 100644
index 4d1a46f64c..00
---
a/MdeModulePkg/Universal/PropertiesTableAttributesDxe/PropertiesTableAttributesDxe.c
+++ /dev/null
@@ -1,208 +0,0 @@
-/** @file
- This module sets default policy for attributes of EfiACPIMemoryNVS and
EfiReservedMemoryType.
-
- This module sets EFI_MEMORY_XP for attributes of EfiACPIMemoryNVS and
EfiReservedMemoryType
- in UEFI memory map, if and only of PropertiesTable is published and has BIT0
set.
-
-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
-This program and the accompanying materials
-are licensed and made available under the terms and conditions of the BSD
License
-which accompanies this distribution. The full text of the license may be
found at
-http://opensource.org/licenses/bsd-license.php
-
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-/**
- Converts a number of EFI_PAGEs to a size in bytes.
-
- NOTE: Do not use EFI_PAGES_TO_SIZE because it handles UINTN only.
-
- @param Pages The number of EFI_PAGES.
-
- @return The number of bytes associated with the number of EFI_PAGEs
specified
- by Pages.
-**/
-UINT64
-EfiPagesToSize (
- IN UINT64 Pages
- )
-{
- return LShiftU64 (Pages, EFI_PAGE_SHIFT);
-}
-
-/**
- Set memory attributes according to default policy.
-
- @param MemoryMapA pointer to the buffer in which firmware places
the current memory map.
- @param MemoryMapSizeSize, in bytes, of the MemoryMap buffer.
- @param DescriptorSize size, in bytes, of an individual
EFI_MEMORY_DESCRIPTOR.
-**/
-VOID
-SetMemorySpaceAttributesDefault (
- IN EFI_MEMORY_DESCRIPTOR *MemoryMap,
- IN UINTN MemoryMapSize,
- IN UINTN DescriptorSize
- )
-{
- EFI_MEMORY_DESCRIPTOR *MemoryMapEntry;
- EFI_MEMORY_DESCRIPTOR *MemoryMapEnd;
- EFI_STATUS Status;
-
- DEBUG ((EFI_D_INFO, "SetMemorySpaceAttributesDefault\n"));
-
- MemoryMapEntry = MemoryMap;
- MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) MemoryMap +
MemoryMapSize);
- while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) {
-if (MemoryMapEntry->PhysicalStart < BASE_1MB) {
- //
- // Do not touch memory space below 1MB
- //
- MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
- continue;
-}
-switch (MemoryMapEntry->Type) {
-case EfiRuntimeServicesCode:
-case EfiRuntimeServicesData:
- //
- // should be handled later;
- //
- break;
-case EfiReservedMemoryType:
-case EfiACPIMemoryNVS:
- //
- // Handle EfiReservedMemoryType and EfiACPIMemoryNVS, because there
might be firmware executable there.
- //
- DEBUG ((EFI_D_INFO, "SetMemorySpaceAttributes - %016lx - %016lx (%016lx)
...\n",
-MemoryMapEntry->PhysicalStart,
-MemoryMapEntry->PhysicalStart + EfiPagesToSize
(MemoryMapEntry->NumberOfPages),
-MemoryMapEntry->Attribute
-));
- Status
Re: [edk2] [PATCH 2/2] MdeModulePkg: Add a runtime library instance of ResetSystemLib
For new added RuntimeResetSystemLib.inf, please add it into MdeModulePkg.dsc
and verify its build.
>-Original Message-
>From: Ni, Ray
>Sent: Monday, February 18, 2019 4:37 PM
>To: Gao, Zhichao ; edk2-devel@lists.01.org
>Cc: Gao, Liming
>Subject: RE: [PATCH 2/2] MdeModulePkg: Add a runtime library instance of
>ResetSystemLib
>
>Reviewed-by: Ray Ni
>
>> -Original Message-
>> From: Gao, Zhichao
>> Sent: Monday, February 18, 2019 3:33 PM
>> To: edk2-devel@lists.01.org
>> Cc: Ni, Ray ; Gao, Liming
>> Subject: [PATCH 2/2] MdeModulePkg: Add a runtime library instance of
>> ResetSystemLib
>>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1461
>>
>> For now there is no ResetSystemLib instance that can be linked against
>> Runtime driver. And it's improper to extend the existing DxeResetSystemLib
>> to support Runtime driver because no one converts gRT when entering RT
>> phase.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Zhichao Gao
>> Cc: Ray Ni
>> Cc: Liming Gao
>> ---
>> .../RuntimeResetSystemLib/RuntimeResetSystemLib.c | 216
>> +
>> .../RuntimeResetSystemLib.inf | 50 +
>> .../RuntimeResetSystemLib.uni | 21 ++
>> 3 files changed, 287 insertions(+)
>> create mode 100644
>>
>MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib.c
>> create mode 100644
>> MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib.i
>> nf
>> create mode 100644
>>
>MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib.u
>> ni
>>
>> diff --git
>>
>a/MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib
>> .c
>>
>b/MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib
>> .c
>> new file mode 100644
>> index 00..17826cd6a9
>> --- /dev/null
>> +++
>>
>b/MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib
>> .c
>> @@ -0,0 +1,216 @@
>> +/** @file
>> + DXE Reset System Library instance that calls gRT->ResetSystem().
>> +
>> + Copyright (c) 2017 - 2019, Intel Corporation. All rights
>> + reserved. This program and the accompanying materials are
>> + licensed and made available under the terms and conditions of the BSD
>> + License which accompanies this distribution. The full text of the
>> + license may be found at
>> + http://opensource.org/licenses/bsd-license.php
>> +
>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>> BASIS,
>> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>> EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +EFI_EVENT
>mRuntimeResetSystemLibVirtualAddressChangeEvent;
>> +EFI_RUNTIME_SERVICES *mInternalRT;
>> +
>> +/**
>> + This function causes a system-wide reset (cold reset), in which
>> + all circuitry within the system returns to its initial state. This
>> +type of reset
>> + is asynchronous to system operation and operates without regard to
>> + cycle boundaries.
>> +
>> + If this function returns, it means that the system does not support cold
>> reset.
>> +**/
>> +VOID
>> +EFIAPI
>> +ResetCold (
>> + VOID
>> + )
>> +{
>> + mInternalRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL); }
>> +
>> +/**
>> + This function causes a system-wide initialization (warm reset), in
>> +which all processors
>> + are set to their initial state. Pending cycles are not corrupted.
>> +
>> + If this function returns, it means that the system does not support warm
>> reset.
>> +**/
>> +VOID
>> +EFIAPI
>> +ResetWarm (
>> + VOID
>> + )
>> +{
>> + mInternalRT->ResetSystem (EfiResetWarm, EFI_SUCCESS, 0, NULL); }
>> +
>> +/**
>> + This function causes the system to enter a power state equivalent
>> + to the ACPI G2/S5 or G3 states.
>> +
>> + If this function returns, it means that the system does not support shut
>> down reset.
>> +**/
>> +VOID
>> +EFIAPI
>> +ResetShutdown (
>> + VOID
>> + )
>> +{
>> + mInternalRT->ResetSystem (EfiResetShutdown, EFI_SUCCESS, 0, NULL); }
>> +
>> +/**
>> + This function causes the system to enter S3 and then wake up
>immediately.
>> +
>> + If this function returns, it means that the system does not support S3
>> feature.
>> +**/
>> +VOID
>> +EFIAPI
>> +EnterS3WithImmediateWake (
>> + VOID
>> + )
>> +{
>> +}
>> +
>> +/**
>> + This function causes a systemwide reset. The exact type of the reset
>> +is
>> + defined by the EFI_GUID that follows the Null-terminated Unicode
>> +string passed
>> + into ResetData. If the platform does not recognize the EFI_GUID in
>> +ResetData
>> + the platform must pick a supported reset type to perform.The platform
>> +may
>> + optionally log the parameters from any non-normal reset that occurs.
>> +
>> + @param[in] DataSize The size, in bytes, of ResetData.
>> + @param[in] ResetData The data buffer starts with a Null-terminated
>> string,
>> + followed by the EFI_GUID.
>> +**/
>> +VOID
>> +EFI
Re: [edk2] [PATCH 1/2] MdeModulePkg: Add a new API ResetSystem
Zhichao,
Please separate the single patch to multiple patches so that every patch
just changes one component.
> -Original Message-
> From: Gao, Zhichao
> Sent: Monday, February 18, 2019 3:33 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray ; Gao, Liming
> Subject: [PATCH 1/2] MdeModulePkg: Add a new API ResetSystem
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1460
>
> Add a new API ResetSystem:
> VOID
> EFIAPI
> ResetSystem (
> IN EFI_RESET_TYPE ResetType,
> IN EFI_STATUS ResetStatus,
> IN UINTNDataSize,
> IN VOID *ResetData OPTIONAL
> )
> The Consumer of ResetSystemLib can use this new API to reset
> system with additional reset data.Because ResetSystemRuntimeDxe
> has a same function name with it, change the function name from
> ResetSystem to EfiRuntimeResetSystem(both ResetSystem and
> EfiResetSystem are used in ResetSystemLiband RuntimeLib, and the
> driver consumes these two library) in driver ResetSystemRuntimeDxe.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhichao Gao
> Cc: Ray Ni
> Cc: Liming Gao
> ---
> MdeModulePkg/Include/Library/ResetSystemLib.h | 25
> ++-
> .../Library/DxeResetSystemLib/DxeResetSystemLib.c | 28
> +-
> .../Library/PeiResetSystemLib/PeiResetSystemLib.c | 28
> +-
> .../Universal/ResetSystemRuntimeDxe/ResetSystem.c | 8 +++
> .../Universal/ResetSystemRuntimeDxe/ResetSystem.h | 4 ++--
> 5 files changed, 84 insertions(+), 9 deletions(-)
>
> diff --git a/MdeModulePkg/Include/Library/ResetSystemLib.h
> b/MdeModulePkg/Include/Library/ResetSystemLib.h
> index 55d1923ae1..2f5d15ade8 100644
> --- a/MdeModulePkg/Include/Library/ResetSystemLib.h
> +++ b/MdeModulePkg/Include/Library/ResetSystemLib.h
> @@ -2,7 +2,7 @@
>System reset Library Services. This library class defines a set of
>methods that reset the whole system.
>
> -Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2005 - 2019, Intel Corporation. All rights reserved.
> This program and the accompanying materials are licensed and made
> available under
> the terms and conditions of the BSD License that accompanies this
> distribution.
> The full text of the license may be found at
> @@ -83,4 +83,27 @@ ResetPlatformSpecific (
>IN VOID*ResetData
>);
>
> +/**
> + The ResetSystem function resets the entire platform.
> +
> + @param[in] ResetType The type of reset to perform.
> + @param[in] ResetStatusThe status code for the reset.
> + @param[in] DataSize The size, in bytes, of ResetData.
> + @param[in] ResetData For a ResetType of EfiResetCold, EfiResetWarm,
> or EfiResetShutdown
> +the data buffer starts with a Null-terminated
> string, optionally
> +followed by additional binary data. The string
> is a description
> +that the caller may use to further indicate the
> reason for the
> +system reset. ResetData is only valid if
> ResetStatus is
> something
> +other than EFI_SUCCESS unless the ResetType is
> EfiResetPlatformSpecific
> +where a minimum amount of ResetData is always
> required.
> +**/
> +VOID
> +EFIAPI
> +ResetSystem (
> + IN EFI_RESET_TYPE ResetType,
> + IN EFI_STATUS ResetStatus,
> + IN UINTNDataSize,
> + IN VOID *ResetData OPTIONAL
> + );
> +
> #endif
> diff --git
> a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
> b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
> index ea4878cab1..f5c7386c9a 100644
> --- a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
> +++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
> @@ -1,7 +1,7 @@
> /** @file
>DXE Reset System Library instance that calls gRT->ResetSystem().
>
> - Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.
> + Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution. The full text of the license may be
> found at
> @@ -96,3 +96,29 @@ ResetPlatformSpecific (
> {
>gRT->ResetSystem (EfiResetPlatformSpecific, EFI_SUCCESS, DataSize,
> ResetData);
> }
> +
> +/**
> + The ResetSystem function resets the entire platform.
> +
> + @param[in] ResetType The type of reset to perform.
> + @param[in] ResetStatusThe status code for the reset.
> + @param[in] DataSize The size, in bytes, of ResetData.
> + @param[in] ResetData For a ResetType of EfiResetCold, EfiResetWarm,
> or EfiResetShutdown
> +the data buffer s
Re: [edk2] [PATCH 2/2] MdeModulePkg: Add a runtime library instance of ResetSystemLib
Reviewed-by: Ray Ni
> -Original Message-
> From: Gao, Zhichao
> Sent: Monday, February 18, 2019 3:33 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray ; Gao, Liming
> Subject: [PATCH 2/2] MdeModulePkg: Add a runtime library instance of
> ResetSystemLib
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1461
>
> For now there is no ResetSystemLib instance that can be linked against
> Runtime driver. And it's improper to extend the existing DxeResetSystemLib
> to support Runtime driver because no one converts gRT when entering RT
> phase.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhichao Gao
> Cc: Ray Ni
> Cc: Liming Gao
> ---
> .../RuntimeResetSystemLib/RuntimeResetSystemLib.c | 216
> +
> .../RuntimeResetSystemLib.inf | 50 +
> .../RuntimeResetSystemLib.uni | 21 ++
> 3 files changed, 287 insertions(+)
> create mode 100644
> MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib.c
> create mode 100644
> MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib.i
> nf
> create mode 100644
> MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib.u
> ni
>
> diff --git
> a/MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib
> .c
> b/MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib
> .c
> new file mode 100644
> index 00..17826cd6a9
> --- /dev/null
> +++
> b/MdeModulePkg/Library/RuntimeResetSystemLib/RuntimeResetSystemLib
> .c
> @@ -0,0 +1,216 @@
> +/** @file
> + DXE Reset System Library instance that calls gRT->ResetSystem().
> +
> + Copyright (c) 2017 - 2019, Intel Corporation. All rights
> + reserved. This program and the accompanying materials are
> + licensed and made available under the terms and conditions of the BSD
> + License which accompanies this distribution. The full text of the
> + license may be found at
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +EFI_EVENT
> mRuntimeResetSystemLibVirtualAddressChangeEvent;
> +EFI_RUNTIME_SERVICES *mInternalRT;
> +
> +/**
> + This function causes a system-wide reset (cold reset), in which
> + all circuitry within the system returns to its initial state. This
> +type of reset
> + is asynchronous to system operation and operates without regard to
> + cycle boundaries.
> +
> + If this function returns, it means that the system does not support cold
> reset.
> +**/
> +VOID
> +EFIAPI
> +ResetCold (
> + VOID
> + )
> +{
> + mInternalRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL); }
> +
> +/**
> + This function causes a system-wide initialization (warm reset), in
> +which all processors
> + are set to their initial state. Pending cycles are not corrupted.
> +
> + If this function returns, it means that the system does not support warm
> reset.
> +**/
> +VOID
> +EFIAPI
> +ResetWarm (
> + VOID
> + )
> +{
> + mInternalRT->ResetSystem (EfiResetWarm, EFI_SUCCESS, 0, NULL); }
> +
> +/**
> + This function causes the system to enter a power state equivalent
> + to the ACPI G2/S5 or G3 states.
> +
> + If this function returns, it means that the system does not support shut
> down reset.
> +**/
> +VOID
> +EFIAPI
> +ResetShutdown (
> + VOID
> + )
> +{
> + mInternalRT->ResetSystem (EfiResetShutdown, EFI_SUCCESS, 0, NULL); }
> +
> +/**
> + This function causes the system to enter S3 and then wake up immediately.
> +
> + If this function returns, it means that the system does not support S3
> feature.
> +**/
> +VOID
> +EFIAPI
> +EnterS3WithImmediateWake (
> + VOID
> + )
> +{
> +}
> +
> +/**
> + This function causes a systemwide reset. The exact type of the reset
> +is
> + defined by the EFI_GUID that follows the Null-terminated Unicode
> +string passed
> + into ResetData. If the platform does not recognize the EFI_GUID in
> +ResetData
> + the platform must pick a supported reset type to perform.The platform
> +may
> + optionally log the parameters from any non-normal reset that occurs.
> +
> + @param[in] DataSize The size, in bytes, of ResetData.
> + @param[in] ResetData The data buffer starts with a Null-terminated
> string,
> + followed by the EFI_GUID.
> +**/
> +VOID
> +EFIAPI
> +ResetPlatformSpecific (
> + IN UINTN DataSize,
> + IN VOID*ResetData
> + )
> +{
> + mInternalRT->ResetSystem (EfiResetPlatformSpecific, EFI_SUCCESS,
> +DataSize, ResetData); }
> +
> +/**
> + The ResetSystem function resets the entire platform.
> +
> + @param[in] ResetType The type of reset to perform.
> + @param[in] ResetStatusThe status code for the reset.
> + @param[in] DataSize The size, in bytes, of ResetData.
> + @param[in] Reset
[edk2] [PATCH] ShellPkg: add array index check for shell delay option
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1528
Shell delay option without parameters do not check the
index of shell parameter argv. Add index check to avoid
invalid pointer references.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao
Cc: Liming Gao
Cc: Ray Ni
---
ShellPkg/Application/Shell/Shell.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/ShellPkg/Application/Shell/Shell.c
b/ShellPkg/Application/Shell/Shell.c
index 104f4c8961..ec344137d3 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -1,7 +1,7 @@
/** @file
This is THE shell (application)
- Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.
(C) Copyright 2013-2014 Hewlett-Packard Development Company, L.P.
Copyright 2015-2018 Dell Technologies.
This program and the accompanying materials
@@ -1002,7 +1002,11 @@ ProcessCommandLine(
) == 0) {
ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay= TRUE;
// Check for optional delay value following "-delay"
- DelayValueStr = gEfiShellParametersProtocol->Argv[LoopVar + 1];
+ if ((LoopVar + 1) >= gEfiShellParametersProtocol->Argc) {
+DelayValueStr = NULL;
+ } else {
+DelayValueStr = gEfiShellParametersProtocol->Argv[LoopVar + 1];
+ }
if (DelayValueStr != NULL){
if (*DelayValueStr == L':') {
DelayValueStr++;
--
2.16.2.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ShellPkg/Application/Shell: add array index check for shell delay option
Ignore this patch. The commit massage is not in compliance with
https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format.
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Zhichao Gao
> Sent: Thursday, February 14, 2019 10:55 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming
> Subject: [edk2] [PATCH] ShellPkg/Application/Shell: add array index check for
> shell delay option
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1528
> Shell delay option without parameters do not check the index of shell
> parameter argv. Add index check to avoid invalid pointer references.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhichao Gao
>
> Cc: Liming Gao
> Cc: Ray Ni
> ---
> ShellPkg/Application/Shell/Shell.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index 104f4c8961..f4d9668d81 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -1002,7 +1002,11 @@ ProcessCommandLine(
> ) == 0) {
>ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay= TRUE;
>// Check for optional delay value following "-delay"
> - DelayValueStr = gEfiShellParametersProtocol->Argv[LoopVar + 1];
> + if ((LoopVar + 1) >= gEfiShellParametersProtocol->Argc) {
> +DelayValueStr = NULL;
> + } else {
> +DelayValueStr = gEfiShellParametersProtocol->Argv[LoopVar + 1];
> + }
>if (DelayValueStr != NULL){
> if (*DelayValueStr == L':') {
>DelayValueStr++;
> --
> 2.16.2.windows.1
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
