Re: [edk2] [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
Yes, they are legacy version with old style alignment. It's first try to address this return status change in this new API. We may update some APIs depending on requirement and impacts evaluations later. Best Regards & Thanks, LONG, Qin -Original Message- From: Zhang, Chao B Sent: Thursday, September 28, 2017 2:03 PM To: Long, Qin ; ler...@redhat.com; Ye, Ting Cc: edk2-devel@lists.01.org Subject: RE: [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 certificate Qin: What about other X509 related interface, such as X509GetTBSCert, X509GetSubjectName. They all return TRUE/FALSE. It looks inconsistent between these interfaces -Original Message- From: Long, Qin Sent: Thursday, September 21, 2017 10:48 AM To: ler...@redhat.com; Ye, Ting ; Zhang, Chao B Cc: edk2-devel@lists.01.org Subject: [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 certificate v3: Add extra CommonNameSize check since OpenSSL didn't check this input parameter. (One openssl issue was filed to address this risk: https://github.com/openssl/openssl/issues/4392) v2: Update function interface to return RETURN_STATUS to represent different error cases. Add one new API (X509GetCommonName()) to retrieve the subject commonName string from one X.509 certificate. Cc: Laszlo Ersek Cc: Ting Ye Cc: Chao Zhang Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Qin Long --- CryptoPkg/Application/Cryptest/RsaVerify2.c| 32 -- CryptoPkg/Include/Library/BaseCryptLib.h | 35 +++ CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 109 + CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c | 32 ++ .../Pk/CryptX509Null.c | 34 ++- 5 files changed, 234 insertions(+), 8 deletions(-) diff --git a/CryptoPkg/Application/Cryptest/RsaVerify2.c b/CryptoPkg/Application/Cryptest/RsaVerify2.c index 98b5aad900..9db43d6eef 100644 --- a/CryptoPkg/Application/Cryptest/RsaVerify2.c +++ b/CryptoPkg/Application/Cryptest/RsaVerify2.c @@ -204,13 +204,17 @@ ValidateCryptRsa2 ( VOID ) { - BOOLEAN Status; - VOID *RsaPrivKey; - VOID *RsaPubKey; - UINT8*Signature; - UINTNSigSize; - UINT8*Subject; - UINTNSubjectSize; + BOOLEANStatus; + VOID *RsaPrivKey; + VOID *RsaPubKey; + UINT8 *Signature; + UINTN SigSize; + UINT8 *Subject; + UINTN SubjectSize; + RETURN_STATUS ReturnStatus; + CHAR8 CommonName[64]; + CHAR16 CommonNameUnicode[64]; + UINTN CommonNameSize; Print (L"\nUEFI-OpenSSL RSA Key Retrieving Testing: "); @@ -286,6 +290,20 @@ ValidateCryptRsa2 ( Print (L"[Pass]"); } + // + // Get CommonName from X509 Certificate Subject // CommonNameSize = + 64; ZeroMem (CommonName, CommonNameSize); ReturnStatus = + X509GetCommonName (TestCert, sizeof (TestCert), CommonName, + &CommonNameSize); if (RETURN_ERROR (ReturnStatus)) { +Print (L"\n - Retrieving Common Name - [Fail]"); +return EFI_ABORTED; + } else { +AsciiStrToUnicodeStrS (CommonName, CommonNameUnicode, CommonNameSize); +Print (L"\n - Retrieving Common Name = \"%s\" (Size = %d)", + CommonNameUnicode, CommonNameSize); } + // // X509 Certificate Verification. // diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h index 9c5ffcd9cf..2366a0218d 100644 --- a/CryptoPkg/Include/Library/BaseCryptLib.h +++ b/CryptoPkg/Include/Library/BaseCryptLib.h @@ -2171,6 +2171,41 @@ X509GetSubjectName ( IN OUT UINTN*SubjectSize ); +/** + Retrieve the common name (CN) string from one X.509 certificate. + + @param[in] Cert Pointer to the DER-encoded X509 certificate. + @param[in] CertSize Size of the X509 certificate in bytes. + @param[out] CommonName Buffer to contain the retrieved certificate common + name string. At most CommonNameSize bytes will be + written and the string will be null terminated. May be + NULL in order to determine the size buffer needed. + @param[in,out] CommonNameSize The size in bytes of the CommonName buffer on input, + and the size of buffer returned CommonName on output. + If CommonName is NULL then the amount of space needed + in buffer (including the final null) is returned. + + @retval RETURN_SUCCESS The certificate CommonName retrieved successfully. + @retval RETURN_INVALID_PARAMETER If Cert is NULL. + If CommonNameSize is NULL. + If CommonName is not NULL and CommonNameSize is 0. + If Certificate is invalid.
Re: [edk2] [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it
The change Looks good to me. Also, Good to see you adding the comment about why we need to cache BootNext before all the platform hook function calls. :) Reviewed-by: Sunny Wang Regards, Sunny Wang -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni Sent: Thursday, September 28, 2017 1:50 PM To: edk2-devel@lists.01.org Cc: Eric Dong ; Star Zeng Subject: [edk2] [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it Current implementation deletes the "BootNext" before calling any PlatformBootManagerLib APIs, but if system resets in PlatformBootManagerLib APIs, "BootNext" is not consumed but lost. The patch defers the deletion of "BootNext" to before booting it. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni Cc: Eric Dong Cc: Star Zeng --- MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35 ++-- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c index ac5f9088dd..a6fe617b56 100644 --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c @@ -808,7 +808,8 @@ BdsEntry ( ASSERT_EFI_ERROR (Status); // - // Cache and remove the "BootNext" NV variable. + // Cache the "BootNext" NV variable before calling any + PlatformBootManagerLib APIs // This could avoid the "BootNext" set by PlatformBootManagerLib be consumed in this boot. // GetEfiGlobalVariable2 (EFI_BOOT_NEXT_VARIABLE_NAME, (VOID **) &BootNext, &DataSize); if (DataSize != sizeof (UINT16)) { @@ -817,17 +818,6 @@ BdsEntry ( } BootNext = NULL; } - Status = gRT->SetVariable ( - EFI_BOOT_NEXT_VARIABLE_NAME, - &gEfiGlobalVariableGuid, - 0, - 0, - NULL - ); - // - // Deleting NV variable shouldn't fail unless it doesn't exist. - // - ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); // // Initialize the platform language variables @@ -1052,10 +1042,25 @@ BdsEntry ( EfiBootManagerHotkeyBoot (); -// -// Boot to "BootNext" -// if (BootNext != NULL) { + // + // Delete "BootNext" NV variable before transferring control to it to prevent loops. + // + Status = gRT->SetVariable ( + EFI_BOOT_NEXT_VARIABLE_NAME, + &gEfiGlobalVariableGuid, + 0, + 0, + NULL + ); + // + // Deleting NV variable shouldn't fail unless it doesn't exist. + // + ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); + + // + // Boot to "BootNext" + // UnicodeSPrint (BootNextVariableName, sizeof (BootNextVariableName), L"Boot%04x", *BootNext); Status = EfiBootManagerVariableToLoadOption (BootNextVariableName, &LoadOption); if (!EFI_ERROR (Status)) { -- 2.12.2.windows.2 ___ 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
[edk2] [PATCH 1/2] ShellPkg/dh: Add mapping of new UEFI/PI protocols
From: Huajing Li Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Huajing Li Reviewed-by: Ruiyu Ni Cc: Jaben Carsey --- .../UefiHandleParsingLib/UefiHandleParsingLib.c| 56 ++ .../UefiHandleParsingLib/UefiHandleParsingLib.inf | 40 .../UefiHandleParsingLib/UefiHandleParsingLib.uni | 44 + 3 files changed, 140 insertions(+) diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c index d12466c7b0..06f9d26e5f 100644 --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c @@ -2147,6 +2147,37 @@ STATIC CONST GUID_INFO_BLOCK mGuidStringList[] = { {STRING_TOKEN(STR_ADAPTER_INFO), &gEfiAdapterInformationProtocolGuid, AdapterInformationDumpInformation}, // +// UEFI2.5 +// + {STRING_TOKEN(STR_TLS_SB), &gEfiTlsServiceBindingProtocolGuid, NULL}, + {STRING_TOKEN(STR_TLS), &gEfiTlsProtocolGuid, NULL}, + {STRING_TOKEN(STR_TLS_CONFIG),&gEfiTlsConfigurationProtocolGuid, NULL}, + {STRING_TOKEN(STR_SUPPLICANT_SB), &gEfiSupplicantServiceBindingProtocolGuid, NULL}, + {STRING_TOKEN(STR_SUPPLICANT),&gEfiSupplicantProtocolGuid, NULL}, + +// +// UEFI2.6 +// + {STRING_TOKEN(STR_WIFI2), &gEfiWiFi2ProtocolGuid, NULL}, + {STRING_TOKEN(STR_RAMDISK), &gEfiRamDiskProtocolGuid, NULL}, + {STRING_TOKEN(STR_HII_ID),&gEfiHiiImageDecoderProtocolGuid, NULL}, + {STRING_TOKEN(STR_HII_IE),&gEfiHiiImageExProtocolGuid, NULL}, + {STRING_TOKEN(STR_SD_MPT),&gEfiSdMmcPassThruProtocolGuid, NULL}, + {STRING_TOKEN(STR_ERASE_BLOCK), &gEfiEraseBlockProtocolGuid, NULL}, + +// +// UEFI2.7 +// + {STRING_TOKEN(STR_BLUETOOTH_ATTR), &gEfiBluetoothAttributeProtocolGuid, NULL}, + {STRING_TOKEN(STR_BLUETOOTH_ATTR_SB), &gEfiBluetoothAttributeServiceBindingProtocolGuid, NULL}, + {STRING_TOKEN(STR_BLUETOOTH_LE_CONFIG), &gEfiBluetoothLeConfigProtocolGuid,NULL}, + {STRING_TOKEN(STR_UFS_DEV_CONFIG),&gEfiUfsDeviceConfigProtocolGuid, NULL}, + {STRING_TOKEN(STR_HTTP_BOOT_CALL),&gEfiHttpBootCallbackProtocolGuid, NULL}, + {STRING_TOKEN(STR_RESET_NOTI), &gEfiResetNotificationProtocolGuid,NULL}, + {STRING_TOKEN(STR_PARTITION_INFO),&gEfiPartitionInfoProtocolGuid, NULL}, + {STRING_TOKEN(STR_HII_POPUP), &gEfiHiiPopupProtocolGuid, NULL}, + +// // PI Spec ones // {STRING_TOKEN(STR_IDE_CONT_INIT), &gEfiIdeControllerInitProtocolGuid, NULL}, @@ -2271,6 +2302,31 @@ STATIC CONST GUID_INFO_BLOCK mGuidStringList[] = { {STRING_TOKEN(STR_REST), &gEfiRestProtocolGuid, NULL}, // +// PI 1.5 +// + {STRING_TOKEN(STR_MM_EOD),&gEfiMmEndOfDxeProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_ITD),&gEfiMmIoTrapDispatchProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_PBD), &gEfiMmPowerButtonDispatchProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_SBD), &gEfiMmStandbyButtonDispatchProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_GD), &gEfiMmGpiDispatchProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_UD), &gEfiMmUsbDispatchProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_PTD), &gEfiMmPeriodicTimerDispatchProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_SXD),&gEfiMmSxDispatchProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_SWD),&gEfiMmSwDispatchProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_PRBI), &gEfiMmPciRootBridgeIoProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_CPU),&gEfiMmCpuProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_STACODE),&gEfiMmStatusCodeProtocolGuid, NULL}, + {STRING_TOKEN(STR_DXEMM_RTL), &gEfiDxeMmReadyToLockProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_CONFIG), &gEfiMmConfigurationProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_RTL),&gEfiMmReadyToLockProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_CONTROL),&gEfiMmControlProtocolGuid, NULL}, + {STRING_TOKEN(STR_MM_ACCESS), &gEfiMmAccessProtocolGuid,
[edk2] [PATCH 2/2] ShellPkg/UefiHandleParsingLib.c: Map SmmPciRootBridgeIo correctly
From: Huajing Li The current implementation has a typo that maps SmmPciRootBridgeIo to "PciRootBridgeIo". Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Huajing Li Reviewed-by: Ruiyu Ni Cc: Jaben Carsey --- ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c index 06f9d26e5f..489decfaf8 100644 --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c @@ -2229,7 +2229,7 @@ STATIC CONST GUID_INFO_BLOCK mGuidStringList[] = { {STRING_TOKEN(STR_S_COMM),&gEfiSmmCommunicationProtocolGuid, NULL}, {STRING_TOKEN(STR_S_STAT),&gEfiSmmStatusCodeProtocolGuid, NULL}, {STRING_TOKEN(STR_S_CPU), &gEfiSmmCpuProtocolGuid, NULL}, - {STRING_TOKEN(STR_S_PCIRBIO), &gEfiPciRootBridgeIoProtocolGuid, NULL}, + {STRING_TOKEN(STR_S_PCIRBIO), &gEfiSmmPciRootBridgeIoProtocolGuid, NULL}, {STRING_TOKEN(STR_S_SWD), &gEfiSmmSwDispatch2ProtocolGuid, NULL}, {STRING_TOKEN(STR_S_SXD), &gEfiSmmSxDispatch2ProtocolGuid, NULL}, {STRING_TOKEN(STR_S_PTD2), &gEfiSmmPeriodicTimerDispatch2ProtocolGuid, NULL}, -- 2.12.2.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/2] Add mapping of new UEFI/PI protocols
Huajing Li (2): ShellPkg/dh: Add mapping of new UEFI/PI protocols ShellPkg/UefiHandleParsingLib.c: Map SmmPciRootBridgeIo correctly .../UefiHandleParsingLib/UefiHandleParsingLib.c| 58 +- .../UefiHandleParsingLib/UefiHandleParsingLib.inf | 40 +++ .../UefiHandleParsingLib/UefiHandleParsingLib.uni | 44 3 files changed, 141 insertions(+), 1 deletion(-) -- 2.12.2.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch] FDF Spec: Per PI 1.6 to support FV extended header contain FV used size
Cc: Liming Gao Cc: Michael Kinney Cc: Kevin W Shaw Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Yonghong Zhu --- 2_fdf_design_discussion/25_[fv]_sections.md | 2 +- 3_edk_ii_fdf_file_format/36_[fv]_sections.md | 2 ++ README.md| 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/2_fdf_design_discussion/25_[fv]_sections.md b/2_fdf_design_discussion/25_[fv]_sections.md index 82755dd..7b50017 100644 --- a/2_fdf_design_discussion/25_[fv]_sections.md +++ b/2_fdf_design_discussion/25_[fv]_sections.md @@ -57,11 +57,11 @@ previously defined sections within another FV section. This eliminates the need to re-specify components or modules in multiple places. When the `FvNameString` entry is present and set to TRUE in an `[FV]` section, the tools will generate an `FvNameString` entry in FV EXT header using the `UiFvName`. This section also specifies how to define content for PI FV Extensions which -provides a mapping between a GUID and an OEM file type. The size of +provides a mapping for a GUID, an OEM file type and FV used size. The size of `EFI_FIRMWARE_VOLUME_EXT_HEADER` and `EFI_FIRMWARE_VOLUME_EXT_ENTRY` sizes will be calculated based on content, while the `EFI_FIRMWARE_VOLUME_EXT_ENTRY` type must be defined by the platform integrator based on the PI specification, volume 3 The content is limited to the contents of a binary file specified by a FILE statement or a data array specified by a `DATA` statement. diff --git a/3_edk_ii_fdf_file_format/36_[fv]_sections.md b/3_edk_ii_fdf_file_format/36_[fv]_sections.md index f2d34cf..b4f292a 100644 --- a/3_edk_ii_fdf_file_format/36_[fv]_sections.md +++ b/3_edk_ii_fdf_file_format/36_[fv]_sections.md @@ -74,10 +74,11 @@ Conditional statements may be used anywhere within this section. ::= [] [] [] [] [] +[] [] [] [] * * @@ -108,10 +109,11 @@ Conditional statements may be used anywhere within this section. [ "READ_DISABLED_CAP" ] [ "READ_STATUS" ] [ "ERASE_POLARITY" {"0"} {"1"} ] ::= "FileSystemGuid" ::= "FvNameGuid" +::= "FvUsedSizeEnable" ::= "FvNameString" ::= "APRIORI" "PEI" "{" * * diff --git a/README.md b/README.md index 71c92fc..4e15a73 100644 --- a/README.md +++ b/README.md @@ -207,5 +207,6 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved. || [#478](https://bugzilla.tianocore.org/show_bug.cgi?id=478) FDF spec: extend the to support and | | || [#353](https://bugzilla.tianocore.org/show_bug.cgi?id=353) Build spec: Allow nested includes in DSC and FDF files | | || [#520](https://bugzilla.tianocore.org/show_bug.cgi?id=520) FDF spec: Update Precedence of PCD Values | | || [#585](https://bugzilla.tianocore.org/show_bug.cgi?id=585) FDF Spec: Update the FDF_SPECIFICATION version to 0x0001001B or 1.27 | | || Per PI 1.6 to extend FFS alignment to 16M | | +|| Per PI 1.6 to support FV extended header entry contain the used size of FV | | -- 2.6.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it
Ok, got it, tks. The new comment is showing why cache "BootNext" logic is still kept there, the commit log is showing the reason of the patch change. Make sense. :) Reviewed-by: Star Zeng Thanks, Star -Original Message- From: Ni, Ruiyu Sent: Thursday, September 28, 2017 2:18 PM To: Zeng, Star ; edk2-devel@lists.01.org Cc: Dong, Eric Subject: RE: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it I didn't change the position of code to cache "BootNext", because: This could avoid the "BootNext" (set by PlatformBootManagerLib) be consumed in *this* boot. Maybe this time it's more clear. Thanks/Ray > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 2:13 PM > To: Ni, Ruiyu ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Zeng, Star > Subject: RE: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" > until booting it > > The commit log is saying the "BootNext" *DELETED* (before > PlatformBootManagerLib) may be *LOST* if there is reset during > PlatformBootManagerLib. I realized it. > > The comment is saying to avoid the "BootNext" "SET" by > PlatformBootManagerLib. Sorry I am not getting the point. > > Thanks, > Star > -Original Message- > From: Ni, Ruiyu > Sent: Thursday, September 28, 2017 2:02 PM > To: Zeng, Star ; edk2-devel@lists.01.org > Cc: Dong, Eric > Subject: RE: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" > until booting it > > This could avoid the "BootNext" set by PlatformBootManagerLib be > consumed in *this* boot. > > If I add "*" around "this", is it more clear? > > Thanks/Ray > > > -Original Message- > > From: Zeng, Star > > Sent: Thursday, September 28, 2017 2:00 PM > > To: Ni, Ruiyu ; edk2-devel@lists.01.org > > Cc: Dong, Eric ; Zeng, Star > > > > Subject: RE: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" > > until booting it > > > > I am ok with the code logic change, but a little confused by the new > > comment. It seems not match with the commit log. > > > > " This could avoid the "BootNext" set by PlatformBootManagerLib be > > consumed in this boot. " > > > > > > Thanks, > > Star > > -Original Message- > > From: Ni, Ruiyu > > Sent: Thursday, September 28, 2017 1:50 PM > > To: edk2-devel@lists.01.org > > Cc: Dong, Eric ; Zeng, Star > > > > Subject: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until > > booting it > > > > Current implementation deletes the "BootNext" before calling any > > PlatformBootManagerLib APIs, but if system resets in > > PlatformBootManagerLib APIs, "BootNext" is not consumed but lost. > > > > The patch defers the deletion of "BootNext" to before booting it. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Ruiyu Ni > > Cc: Eric Dong > > Cc: Star Zeng > > --- > > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35 > > ++ > > -- > > 1 file changed, 20 insertions(+), 15 deletions(-) > > > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > index ac5f9088dd..a6fe617b56 100644 > > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > @@ -808,7 +808,8 @@ BdsEntry ( > >ASSERT_EFI_ERROR (Status); > > > >// > > - // Cache and remove the "BootNext" NV variable. > > + // Cache the "BootNext" NV variable before calling any > > + PlatformBootManagerLib APIs // This could avoid the "BootNext" > > + set by > > PlatformBootManagerLib be consumed in this boot. > >// > >GetEfiGlobalVariable2 (EFI_BOOT_NEXT_VARIABLE_NAME, (VOID **) > > &BootNext, &DataSize); > >if (DataSize != sizeof (UINT16)) { @@ -817,17 +818,6 @@ BdsEntry > > ( > > } > > BootNext = NULL; > >} > > - Status = gRT->SetVariable ( > > - EFI_BOOT_NEXT_VARIABLE_NAME, > > - &gEfiGlobalVariableGuid, > > - 0, > > - 0, > > - NULL > > - ); > > - // > > - // Deleting NV variable shouldn't fail unless it doesn't exist. > > - // > > - ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); > > > >// > >// Initialize the platform language variables @@ -1052,10 > > +1042,25 @@ BdsEntry ( > > > > EfiBootManagerHotkeyBoot (); > > > > -// > > -// Boot to "BootNext" > > -// > > if (BootNext != NULL) { > > + // > > + // Delete "BootNext" NV variable before transferring control > > + to it to > > prevent loops. > > + // > > + Status = gRT->SetVariable ( > > + EFI_BOOT_NEXT_VARIABLE_NAME, > > + &gEfiGlobalVariableGuid, > > + 0, > > + 0, > > + NULL > > + ); > > + // > > + // Deleting NV variable shouldn't fail unless it doesn't exist. > > + // > > + ASSERT (Status == EFI_SUCCESS || Status == EFI_N
Re: [edk2] [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it
I didn't change the position of code to cache "BootNext", because: This could avoid the "BootNext" (set by PlatformBootManagerLib) be consumed in *this* boot. Maybe this time it's more clear. Thanks/Ray > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 2:13 PM > To: Ni, Ruiyu ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Zeng, Star > Subject: RE: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until > booting it > > The commit log is saying the "BootNext" *DELETED* (before > PlatformBootManagerLib) may be *LOST* if there is reset during > PlatformBootManagerLib. I realized it. > > The comment is saying to avoid the "BootNext" "SET" by > PlatformBootManagerLib. Sorry I am not getting the point. > > Thanks, > Star > -Original Message- > From: Ni, Ruiyu > Sent: Thursday, September 28, 2017 2:02 PM > To: Zeng, Star ; edk2-devel@lists.01.org > Cc: Dong, Eric > Subject: RE: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until > booting it > > This could avoid the "BootNext" set by PlatformBootManagerLib be consumed > in *this* boot. > > If I add "*" around "this", is it more clear? > > Thanks/Ray > > > -Original Message- > > From: Zeng, Star > > Sent: Thursday, September 28, 2017 2:00 PM > > To: Ni, Ruiyu ; edk2-devel@lists.01.org > > Cc: Dong, Eric ; Zeng, Star > > Subject: RE: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" > > until booting it > > > > I am ok with the code logic change, but a little confused by the new > > comment. It seems not match with the commit log. > > > > " This could avoid the "BootNext" set by PlatformBootManagerLib be > > consumed in this boot. " > > > > > > Thanks, > > Star > > -Original Message- > > From: Ni, Ruiyu > > Sent: Thursday, September 28, 2017 1:50 PM > > To: edk2-devel@lists.01.org > > Cc: Dong, Eric ; Zeng, Star > > Subject: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until > > booting it > > > > Current implementation deletes the "BootNext" before calling any > > PlatformBootManagerLib APIs, but if system resets in > > PlatformBootManagerLib APIs, "BootNext" is not consumed but lost. > > > > The patch defers the deletion of "BootNext" to before booting it. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Ruiyu Ni > > Cc: Eric Dong > > Cc: Star Zeng > > --- > > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35 > > ++ > > -- > > 1 file changed, 20 insertions(+), 15 deletions(-) > > > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > index ac5f9088dd..a6fe617b56 100644 > > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > @@ -808,7 +808,8 @@ BdsEntry ( > >ASSERT_EFI_ERROR (Status); > > > >// > > - // Cache and remove the "BootNext" NV variable. > > + // Cache the "BootNext" NV variable before calling any > > + PlatformBootManagerLib APIs // This could avoid the "BootNext" set > > + by > > PlatformBootManagerLib be consumed in this boot. > >// > >GetEfiGlobalVariable2 (EFI_BOOT_NEXT_VARIABLE_NAME, (VOID **) > > &BootNext, &DataSize); > >if (DataSize != sizeof (UINT16)) { > > @@ -817,17 +818,6 @@ BdsEntry ( > > } > > BootNext = NULL; > >} > > - Status = gRT->SetVariable ( > > - EFI_BOOT_NEXT_VARIABLE_NAME, > > - &gEfiGlobalVariableGuid, > > - 0, > > - 0, > > - NULL > > - ); > > - // > > - // Deleting NV variable shouldn't fail unless it doesn't exist. > > - // > > - ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); > > > >// > >// Initialize the platform language variables @@ -1052,10 +1042,25 > > @@ BdsEntry ( > > > > EfiBootManagerHotkeyBoot (); > > > > -// > > -// Boot to "BootNext" > > -// > > if (BootNext != NULL) { > > + // > > + // Delete "BootNext" NV variable before transferring control to > > + it to > > prevent loops. > > + // > > + Status = gRT->SetVariable ( > > + EFI_BOOT_NEXT_VARIABLE_NAME, > > + &gEfiGlobalVariableGuid, > > + 0, > > + 0, > > + NULL > > + ); > > + // > > + // Deleting NV variable shouldn't fail unless it doesn't exist. > > + // > > + ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); > > + > > + // > > + // Boot to "BootNext" > > + // > >UnicodeSPrint (BootNextVariableName, sizeof > > (BootNextVariableName), L"Boot%04x", *BootNext); > >Status = EfiBootManagerVariableToLoadOption > > (BootNextVariableName, &LoadOption); > >if (!EFI_ERROR (Status)) { > > -- > > 2.12.2.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.
Re: [edk2] [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it
The commit log is saying the "BootNext" *DELETED* (before PlatformBootManagerLib) may be *LOST* if there is reset during PlatformBootManagerLib. I realized it. The comment is saying to avoid the "BootNext" "SET" by PlatformBootManagerLib. Sorry I am not getting the point. Thanks, Star -Original Message- From: Ni, Ruiyu Sent: Thursday, September 28, 2017 2:02 PM To: Zeng, Star ; edk2-devel@lists.01.org Cc: Dong, Eric Subject: RE: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it This could avoid the "BootNext" set by PlatformBootManagerLib be consumed in *this* boot. If I add "*" around "this", is it more clear? Thanks/Ray > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 2:00 PM > To: Ni, Ruiyu ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Zeng, Star > Subject: RE: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" > until booting it > > I am ok with the code logic change, but a little confused by the new > comment. It seems not match with the commit log. > > " This could avoid the "BootNext" set by PlatformBootManagerLib be > consumed in this boot. " > > > Thanks, > Star > -Original Message- > From: Ni, Ruiyu > Sent: Thursday, September 28, 2017 1:50 PM > To: edk2-devel@lists.01.org > Cc: Dong, Eric ; Zeng, Star > Subject: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until > booting it > > Current implementation deletes the "BootNext" before calling any > PlatformBootManagerLib APIs, but if system resets in > PlatformBootManagerLib APIs, "BootNext" is not consumed but lost. > > The patch defers the deletion of "BootNext" to before booting it. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Cc: Eric Dong > Cc: Star Zeng > --- > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35 > ++ > -- > 1 file changed, 20 insertions(+), 15 deletions(-) > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > index ac5f9088dd..a6fe617b56 100644 > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > @@ -808,7 +808,8 @@ BdsEntry ( >ASSERT_EFI_ERROR (Status); > >// > - // Cache and remove the "BootNext" NV variable. > + // Cache the "BootNext" NV variable before calling any > + PlatformBootManagerLib APIs // This could avoid the "BootNext" set > + by > PlatformBootManagerLib be consumed in this boot. >// >GetEfiGlobalVariable2 (EFI_BOOT_NEXT_VARIABLE_NAME, (VOID **) > &BootNext, &DataSize); >if (DataSize != sizeof (UINT16)) { > @@ -817,17 +818,6 @@ BdsEntry ( > } > BootNext = NULL; >} > - Status = gRT->SetVariable ( > - EFI_BOOT_NEXT_VARIABLE_NAME, > - &gEfiGlobalVariableGuid, > - 0, > - 0, > - NULL > - ); > - // > - // Deleting NV variable shouldn't fail unless it doesn't exist. > - // > - ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); > >// >// Initialize the platform language variables @@ -1052,10 +1042,25 > @@ BdsEntry ( > > EfiBootManagerHotkeyBoot (); > > -// > -// Boot to "BootNext" > -// > if (BootNext != NULL) { > + // > + // Delete "BootNext" NV variable before transferring control to > + it to > prevent loops. > + // > + Status = gRT->SetVariable ( > + EFI_BOOT_NEXT_VARIABLE_NAME, > + &gEfiGlobalVariableGuid, > + 0, > + 0, > + NULL > + ); > + // > + // Deleting NV variable shouldn't fail unless it doesn't exist. > + // > + ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); > + > + // > + // Boot to "BootNext" > + // >UnicodeSPrint (BootNextVariableName, sizeof > (BootNextVariableName), L"Boot%04x", *BootNext); >Status = EfiBootManagerVariableToLoadOption > (BootNextVariableName, &LoadOption); >if (!EFI_ERROR (Status)) { > -- > 2.12.2.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 certificate
Qin: What about other X509 related interface, such as X509GetTBSCert, X509GetSubjectName. They all return TRUE/FALSE. It looks inconsistent between these interfaces -Original Message- From: Long, Qin Sent: Thursday, September 21, 2017 10:48 AM To: ler...@redhat.com; Ye, Ting ; Zhang, Chao B Cc: edk2-devel@lists.01.org Subject: [PATCH v3] CryptoPkg: Add new API to retrieve commonName of X.509 certificate v3: Add extra CommonNameSize check since OpenSSL didn't check this input parameter. (One openssl issue was filed to address this risk: https://github.com/openssl/openssl/issues/4392) v2: Update function interface to return RETURN_STATUS to represent different error cases. Add one new API (X509GetCommonName()) to retrieve the subject commonName string from one X.509 certificate. Cc: Laszlo Ersek Cc: Ting Ye Cc: Chao Zhang Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Qin Long --- CryptoPkg/Application/Cryptest/RsaVerify2.c| 32 -- CryptoPkg/Include/Library/BaseCryptLib.h | 35 +++ CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 109 + CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c | 32 ++ .../Pk/CryptX509Null.c | 34 ++- 5 files changed, 234 insertions(+), 8 deletions(-) diff --git a/CryptoPkg/Application/Cryptest/RsaVerify2.c b/CryptoPkg/Application/Cryptest/RsaVerify2.c index 98b5aad900..9db43d6eef 100644 --- a/CryptoPkg/Application/Cryptest/RsaVerify2.c +++ b/CryptoPkg/Application/Cryptest/RsaVerify2.c @@ -204,13 +204,17 @@ ValidateCryptRsa2 ( VOID ) { - BOOLEAN Status; - VOID *RsaPrivKey; - VOID *RsaPubKey; - UINT8*Signature; - UINTNSigSize; - UINT8*Subject; - UINTNSubjectSize; + BOOLEANStatus; + VOID *RsaPrivKey; + VOID *RsaPubKey; + UINT8 *Signature; + UINTN SigSize; + UINT8 *Subject; + UINTN SubjectSize; + RETURN_STATUS ReturnStatus; + CHAR8 CommonName[64]; + CHAR16 CommonNameUnicode[64]; + UINTN CommonNameSize; Print (L"\nUEFI-OpenSSL RSA Key Retrieving Testing: "); @@ -286,6 +290,20 @@ ValidateCryptRsa2 ( Print (L"[Pass]"); } + // + // Get CommonName from X509 Certificate Subject // CommonNameSize = + 64; ZeroMem (CommonName, CommonNameSize); ReturnStatus = + X509GetCommonName (TestCert, sizeof (TestCert), CommonName, + &CommonNameSize); if (RETURN_ERROR (ReturnStatus)) { +Print (L"\n - Retrieving Common Name - [Fail]"); +return EFI_ABORTED; + } else { +AsciiStrToUnicodeStrS (CommonName, CommonNameUnicode, CommonNameSize); +Print (L"\n - Retrieving Common Name = \"%s\" (Size = %d)", + CommonNameUnicode, CommonNameSize); } + // // X509 Certificate Verification. // diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h index 9c5ffcd9cf..2366a0218d 100644 --- a/CryptoPkg/Include/Library/BaseCryptLib.h +++ b/CryptoPkg/Include/Library/BaseCryptLib.h @@ -2171,6 +2171,41 @@ X509GetSubjectName ( IN OUT UINTN*SubjectSize ); +/** + Retrieve the common name (CN) string from one X.509 certificate. + + @param[in] Cert Pointer to the DER-encoded X509 certificate. + @param[in] CertSize Size of the X509 certificate in bytes. + @param[out] CommonName Buffer to contain the retrieved certificate common + name string. At most CommonNameSize bytes will be + written and the string will be null terminated. May be + NULL in order to determine the size buffer needed. + @param[in,out] CommonNameSize The size in bytes of the CommonName buffer on input, + and the size of buffer returned CommonName on output. + If CommonName is NULL then the amount of space needed + in buffer (including the final null) is returned. + + @retval RETURN_SUCCESS The certificate CommonName retrieved successfully. + @retval RETURN_INVALID_PARAMETER If Cert is NULL. + If CommonNameSize is NULL. + If CommonName is not NULL and CommonNameSize is 0. + If Certificate is invalid. + @retval RETURN_NOT_FOUND If no CommonName entry exists. + @retval RETURN_BUFFER_TOO_SMALL If the CommonName is NULL. The required buffer size + (including the final null) is returned in the + CommonNameSize parameter. + @retval RETURN_UNSUPPORTED The operation is not supported. + +**/ +RETURN_STATUS +EFIAPI +X509GetCommonName ( + IN CONST UINT8 *Cert, + IN UINTNCertSize, + OUT CHA
Re: [edk2] [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it
This could avoid the "BootNext" set by PlatformBootManagerLib be consumed in *this* boot. If I add "*" around "this", is it more clear? Thanks/Ray > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 2:00 PM > To: Ni, Ruiyu ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Zeng, Star > Subject: RE: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until > booting it > > I am ok with the code logic change, but a little confused by the new comment. > It > seems not match with the commit log. > > " This could avoid the "BootNext" set by PlatformBootManagerLib be consumed > in this boot. " > > > Thanks, > Star > -Original Message- > From: Ni, Ruiyu > Sent: Thursday, September 28, 2017 1:50 PM > To: edk2-devel@lists.01.org > Cc: Dong, Eric ; Zeng, Star > Subject: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting > it > > Current implementation deletes the "BootNext" before calling any > PlatformBootManagerLib APIs, but if system resets in PlatformBootManagerLib > APIs, "BootNext" is not consumed but lost. > > The patch defers the deletion of "BootNext" to before booting it. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni > Cc: Eric Dong > Cc: Star Zeng > --- > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35 ++ > -- > 1 file changed, 20 insertions(+), 15 deletions(-) > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > index ac5f9088dd..a6fe617b56 100644 > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > @@ -808,7 +808,8 @@ BdsEntry ( >ASSERT_EFI_ERROR (Status); > >// > - // Cache and remove the "BootNext" NV variable. > + // Cache the "BootNext" NV variable before calling any > + PlatformBootManagerLib APIs // This could avoid the "BootNext" set by > PlatformBootManagerLib be consumed in this boot. >// >GetEfiGlobalVariable2 (EFI_BOOT_NEXT_VARIABLE_NAME, (VOID **) > &BootNext, &DataSize); >if (DataSize != sizeof (UINT16)) { > @@ -817,17 +818,6 @@ BdsEntry ( > } > BootNext = NULL; >} > - Status = gRT->SetVariable ( > - EFI_BOOT_NEXT_VARIABLE_NAME, > - &gEfiGlobalVariableGuid, > - 0, > - 0, > - NULL > - ); > - // > - // Deleting NV variable shouldn't fail unless it doesn't exist. > - // > - ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); > >// >// Initialize the platform language variables @@ -1052,10 +1042,25 @@ > BdsEntry ( > > EfiBootManagerHotkeyBoot (); > > -// > -// Boot to "BootNext" > -// > if (BootNext != NULL) { > + // > + // Delete "BootNext" NV variable before transferring control to it to > prevent loops. > + // > + Status = gRT->SetVariable ( > + EFI_BOOT_NEXT_VARIABLE_NAME, > + &gEfiGlobalVariableGuid, > + 0, > + 0, > + NULL > + ); > + // > + // Deleting NV variable shouldn't fail unless it doesn't exist. > + // > + ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); > + > + // > + // Boot to "BootNext" > + // >UnicodeSPrint (BootNextVariableName, sizeof (BootNextVariableName), > L"Boot%04x", *BootNext); >Status = EfiBootManagerVariableToLoadOption (BootNextVariableName, > &LoadOption); >if (!EFI_ERROR (Status)) { > -- > 2.12.2.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it
I am ok with the code logic change, but a little confused by the new comment. It seems not match with the commit log. " This could avoid the "BootNext" set by PlatformBootManagerLib be consumed in this boot. " Thanks, Star -Original Message- From: Ni, Ruiyu Sent: Thursday, September 28, 2017 1:50 PM To: edk2-devel@lists.01.org Cc: Dong, Eric ; Zeng, Star Subject: [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it Current implementation deletes the "BootNext" before calling any PlatformBootManagerLib APIs, but if system resets in PlatformBootManagerLib APIs, "BootNext" is not consumed but lost. The patch defers the deletion of "BootNext" to before booting it. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni Cc: Eric Dong Cc: Star Zeng --- MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35 ++-- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c index ac5f9088dd..a6fe617b56 100644 --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c @@ -808,7 +808,8 @@ BdsEntry ( ASSERT_EFI_ERROR (Status); // - // Cache and remove the "BootNext" NV variable. + // Cache the "BootNext" NV variable before calling any + PlatformBootManagerLib APIs // This could avoid the "BootNext" set by PlatformBootManagerLib be consumed in this boot. // GetEfiGlobalVariable2 (EFI_BOOT_NEXT_VARIABLE_NAME, (VOID **) &BootNext, &DataSize); if (DataSize != sizeof (UINT16)) { @@ -817,17 +818,6 @@ BdsEntry ( } BootNext = NULL; } - Status = gRT->SetVariable ( - EFI_BOOT_NEXT_VARIABLE_NAME, - &gEfiGlobalVariableGuid, - 0, - 0, - NULL - ); - // - // Deleting NV variable shouldn't fail unless it doesn't exist. - // - ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); // // Initialize the platform language variables @@ -1052,10 +1042,25 @@ BdsEntry ( EfiBootManagerHotkeyBoot (); -// -// Boot to "BootNext" -// if (BootNext != NULL) { + // + // Delete "BootNext" NV variable before transferring control to it to prevent loops. + // + Status = gRT->SetVariable ( + EFI_BOOT_NEXT_VARIABLE_NAME, + &gEfiGlobalVariableGuid, + 0, + 0, + NULL + ); + // + // Deleting NV variable shouldn't fail unless it doesn't exist. + // + ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); + + // + // Boot to "BootNext" + // UnicodeSPrint (BootNextVariableName, sizeof (BootNextVariableName), L"Boot%04x", *BootNext); Status = EfiBootManagerVariableToLoadOption (BootNextVariableName, &LoadOption); if (!EFI_ERROR (Status)) { -- 2.12.2.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch] ShellPkg/Dh: Refine variable naming style
Reviewed-by: Ruiyu Ni Thanks/Ray > -Original Message- > From: Bi, Dandan > Sent: Thursday, September 28, 2017 10:45 AM > To: edk2-devel@lists.01.org > Cc: Ni, Ruiyu ; Carsey, Jaben > Subject: [patch] ShellPkg/Dh: Refine variable naming style > > Avoid using only lower-case characters for variable name. > > Cc: Ruiyu Ni > Cc: Jaben Carsey > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Dandan Bi > --- > ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c > b/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c > index 7d06163..a7bd251 100644 > --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c > +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c > @@ -284,11 +284,11 @@ GetProtocolInfoString( >EFI_STATUSStatus; >CHAR16*RetVal; >UINTN Size; >CHAR16*Temp; >CHAR16GuidStr[40]; > - VOID *instance; > + VOID *Instance; >CHAR16InstanceStr[17]; > >ProtocolGuidArray = NULL; >RetVal= NULL; >Size = 0; > @@ -314,14 +314,14 @@ GetProtocolInfoString( > FreePool(Temp); >} >StrnCatGrow(&RetVal, &Size, L"%N", 0); > >if(Verbose) { > -Status = gBS->HandleProtocol (TheHandle, > ProtocolGuidArray[ProtocolIndex], &instance); > +Status = gBS->HandleProtocol (TheHandle, > ProtocolGuidArray[ProtocolIndex], &Instance); > if (!EFI_ERROR (Status)) { >StrnCatGrow (&RetVal, &Size, L"(%H", 0); > - UnicodeSPrint (InstanceStr, sizeof (InstanceStr), L"%x", instance); > + UnicodeSPrint (InstanceStr, sizeof (InstanceStr), L"%x", Instance); >StrnCatGrow (&RetVal, &Size, InstanceStr, 0); >StrnCatGrow (&RetVal, &Size, L"%N)", 0); > } >} > > -- > 1.9.5.msysgit.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] MdeModulePkg/BdsDxe: Don't delete "BootNext" until booting it
Current implementation deletes the "BootNext" before calling any PlatformBootManagerLib APIs, but if system resets in PlatformBootManagerLib APIs, "BootNext" is not consumed but lost. The patch defers the deletion of "BootNext" to before booting it. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni Cc: Eric Dong Cc: Star Zeng --- MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 35 ++-- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c index ac5f9088dd..a6fe617b56 100644 --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c @@ -808,7 +808,8 @@ BdsEntry ( ASSERT_EFI_ERROR (Status); // - // Cache and remove the "BootNext" NV variable. + // Cache the "BootNext" NV variable before calling any PlatformBootManagerLib APIs + // This could avoid the "BootNext" set by PlatformBootManagerLib be consumed in this boot. // GetEfiGlobalVariable2 (EFI_BOOT_NEXT_VARIABLE_NAME, (VOID **) &BootNext, &DataSize); if (DataSize != sizeof (UINT16)) { @@ -817,17 +818,6 @@ BdsEntry ( } BootNext = NULL; } - Status = gRT->SetVariable ( - EFI_BOOT_NEXT_VARIABLE_NAME, - &gEfiGlobalVariableGuid, - 0, - 0, - NULL - ); - // - // Deleting NV variable shouldn't fail unless it doesn't exist. - // - ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); // // Initialize the platform language variables @@ -1052,10 +1042,25 @@ BdsEntry ( EfiBootManagerHotkeyBoot (); -// -// Boot to "BootNext" -// if (BootNext != NULL) { + // + // Delete "BootNext" NV variable before transferring control to it to prevent loops. + // + Status = gRT->SetVariable ( + EFI_BOOT_NEXT_VARIABLE_NAME, + &gEfiGlobalVariableGuid, + 0, + 0, + NULL + ); + // + // Deleting NV variable shouldn't fail unless it doesn't exist. + // + ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_FOUND); + + // + // Boot to "BootNext" + // UnicodeSPrint (BootNextVariableName, sizeof (BootNextVariableName), L"Boot%04x", *BootNext); Status = EfiBootManagerVariableToLoadOption (BootNextVariableName, &LoadOption); if (!EFI_ERROR (Status)) { -- 2.12.2.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch] NetworkPkg/UefiPxeBcDxe: Fix the redundant condition check
Cc: Santhapur Naveen Cc: Ye Ting Cc: Fu Siyuan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Wu Jiaxin --- NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c index 568360d..9f5be15 100644 --- a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c @@ -1,9 +1,9 @@ /** @file Support functions implementation for UefiPxeBc Driver. - Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved. + Copyright (c) 2007 - 2017, 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. @@ -422,11 +422,11 @@ PxeBcIcmp6ErrorDpcHandle ( Type = *((UINT8 *) RxData->FragmentTable[0].FragmentBuffer); if (Type != ICMP_V6_DEST_UNREACHABLE && Type != ICMP_V6_PACKET_TOO_BIG && - Type != ICMP_V6_PACKET_TOO_BIG && + Type != ICMP_V6_TIME_EXCEEDED && Type != ICMP_V6_PARAMETER_PROBLEM) { // // The type of the receveid packet should be an ICMP6 error message. // gBS->SignalEvent (RxData->RecycleSignal); -- 1.9.5.msysgit.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3 4/5] MdeModulePkg/DxeNetLib: Fix negative value left shift
Reviewed-by: Wu Jiaxin > -Original Message- > From: Wu, Hao A > Sent: Thursday, September 28, 2017 12:32 PM > To: edk2-devel@lists.01.org > Cc: Wu, Hao A ; Shi, Steven ; > Fu, Siyuan ; Ye, Ting ; Wu, Jiaxin > ; Long, Qin ; Zeng, Star > ; Dong, Eric ; Paolo Bonzini > > Subject: [PATCH v3 4/5] MdeModulePkg/DxeNetLib: Fix negative value left > shift > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=698 > > Within function NetRandomInitSeed(), left shift a negative value is used > in: > "~Time.Hour << 24" > > which involves undefined behavior. > > Since Time.Hour is of type UINT8 (range from 0 to 23), hence ~Time.Hour > will be a negative value (of type int, signed). > > According to the C11 spec, Section 6.5.7: > > 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated > > bits are filled with zeros. If E1 has an unsigned type, the value > > of the result is E1 * 2^E2 , reduced modulo one more than the > > maximum value representable in the result type. If E1 has a signed > > type and nonnegative value, and E1 * 2^E2 is representable in the > > result type, then that is the resulting value; otherwise, the > > behavior is undefined. > > This commit will remove the '~' operator before 'Time.Hour', since it > seems like an implementation choice for generating the seed. > > Cc: Steven Shi > Cc: Fu Siyuan > Cc: Ye Ting > Cc: Wu Jiaxin > Cc: Qin Long > Cc: Star Zeng > Cc: Eric Dong > Cc: Paolo Bonzini > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Hao Wu > --- > MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c > b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c > index 7cd7e3aca0..1ee77fe036 100644 > --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c > +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c > @@ -872,7 +872,7 @@ NetRandomInitSeed ( >UINT64MonotonicCount; > >gRT->GetTime (&Time, NULL); > - Seed = (~Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | > Time.Second); > + Seed = (Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | > Time.Second); >Seed ^= Time.Nanosecond; >Seed ^= Time.Year << 7; > > -- > 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch 2/2] NetworkPkg/HttpDxe: Clarify the usage of HttpConfigData in HTTP protocol
Cc: Ye Ting Cc: Fu Siyuan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Wu Jiaxin --- NetworkPkg/HttpDxe/HttpImpl.c | 20 +++- NetworkPkg/HttpDxe/HttpImpl.h | 18 ++ 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c index c104b61..46d0323 100644 --- a/NetworkPkg/HttpDxe/HttpImpl.c +++ b/NetworkPkg/HttpDxe/HttpImpl.c @@ -31,20 +31,22 @@ EFI_HTTP_PROTOCOL mEfiHttpTemplate = { The GetModeData() function is used to read the current mode data (operational parameters) for this HTTP protocol instance. @param[in] ThisPointer to EFI_HTTP_PROTOCOL instance. @param[out] HttpConfigData Point to buffer for operational parameters of this - HTTP instance. + HTTP instance. It is the responsibility of the caller + to allocate the memory for HttpConfigData and + HttpConfigData->AccessPoint.IPv6Node/IPv4Node. In fact, + it is recommended to allocate sufficient memory to record + IPv6Node since it is big enough for all possibilities. @retval EFI_SUCCESS Operation succeeded. @retval EFI_INVALID_PARAMETER One or more of the following conditions is TRUE: This is NULL. HttpConfigData is NULL. - HttpInstance->LocalAddressIsIPv6 is FALSE and - HttpConfigData->IPv4Node is NULL. - HttpInstance->LocalAddressIsIPv6 is TRUE and - HttpConfigData->IPv6Node is NULL. + HttpConfigData->AccessPoint.IPv4Node or + HttpConfigData->AccessPoint.IPv6Node is NULL. @retval EFI_NOT_STARTED This EFI HTTP Protocol instance has not been started. **/ EFI_STATUS EFIAPI @@ -63,12 +65,12 @@ EfiHttpGetModeData ( } HttpInstance = HTTP_INSTANCE_FROM_PROTOCOL (This); ASSERT (HttpInstance != NULL); - if ((HttpInstance->LocalAddressIsIPv6 && HttpConfigData->AccessPoint.IPv6Node == NULL) || - (!HttpInstance->LocalAddressIsIPv6 && HttpConfigData->AccessPoint.IPv4Node == NULL)) { + if ((HttpConfigData->AccessPoint.IPv6Node == NULL) || + (HttpConfigData->AccessPoint.IPv4Node == NULL)) { return EFI_INVALID_PARAMETER; } if (HttpInstance->State < HTTP_STATE_HTTP_CONFIGED) { return EFI_NOT_STARTED; @@ -113,13 +115,13 @@ EfiHttpGetModeData ( @retval EFI_SUCCESS Operation succeeded. @retval EFI_INVALID_PARAMETER One or more of the following conditions is TRUE: This is NULL. HttpConfigData->LocalAddressIsIPv6 is FALSE and - HttpConfigData->IPv4Node is NULL. + HttpConfigData->AccessPoint.IPv4Node is NULL. HttpConfigData->LocalAddressIsIPv6 is TRUE and - HttpConfigData->IPv6Node is NULL. + HttpConfigData->AccessPoint.IPv6Node is NULL. @retval EFI_ALREADY_STARTED Reinitialize this HTTP instance without calling Configure() with NULL to reset it. @retval EFI_DEVICE_ERRORAn unexpected system or network error occurred. @retval EFI_OUT_OF_RESOURCESCould not allocate enough system resources when executing Configure(). diff --git a/NetworkPkg/HttpDxe/HttpImpl.h b/NetworkPkg/HttpDxe/HttpImpl.h index 40b2504..6550ce0 100644 --- a/NetworkPkg/HttpDxe/HttpImpl.h +++ b/NetworkPkg/HttpDxe/HttpImpl.h @@ -1,9 +1,9 @@ /** @file The header files of implementation of EFI_HTTP_PROTOCOL protocol interfaces. - Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved. + Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved. (C) Copyright 2016 Hewlett Packard Enterprise Development LP 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 @@ -31,20 +31,22 @@ The GetModeData() function is used to read the current mode data (operational parameters) for this HTTP protocol instance. @param[in] ThisPointer to EFI_HTTP_PROTOCOL instance. @param[out] HttpConfigData Point to buffer for operational parameters of this - HTTP instance. + HTTP instance. It is the responsibility of the caller + to allocat
[edk2] [Patch 0/2] Clarify the usage of HttpConfigData in HTTP protocol
Cc: Ye Ting Cc: Fu Siyuan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Wu Jiaxin Jiaxin Wu (2): MdePkg/Http.h: Clarify the usage of HttpConfigData in HTTP protocol NetworkPkg/HttpDxe: Clarify the usage of HttpConfigData in HTTP protocol MdePkg/Include/Protocol/Http.h | 16 +--- NetworkPkg/HttpDxe/HttpImpl.c | 20 +++- NetworkPkg/HttpDxe/HttpImpl.h | 18 ++ 3 files changed, 30 insertions(+), 24 deletions(-) -- 1.9.5.msysgit.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch 1/2] MdePkg/Http.h: Clarify the usage of HttpConfigData in HTTP protocol
Cc: Ye Ting Cc: Fu Siyuan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Wu Jiaxin --- MdePkg/Include/Protocol/Http.h | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/MdePkg/Include/Protocol/Http.h b/MdePkg/Include/Protocol/Http.h index 297d9c3..bdcd7b1 100644 --- a/MdePkg/Include/Protocol/Http.h +++ b/MdePkg/Include/Protocol/Http.h @@ -303,19 +303,21 @@ typedef struct { The GetModeData() function is used to read the current mode data (operational parameters) for this HTTP protocol instance. @param[in] ThisPointer to EFI_HTTP_PROTOCOL instance. @param[out] HttpConfigData Point to buffer for operational parameters of this - HTTP instance. + HTTP instance. It is the responsibility of the caller + to allocate the memory for HttpConfigData and + HttpConfigData->AccessPoint.IPv6Node/IPv4Node. In fact, + it is recommended to allocate sufficient memory to record + IPv6Node since it is big enough for all possibilities. @retval EFI_SUCCESS Operation succeeded. @retval EFI_INVALID_PARAMETER This is NULL. HttpConfigData is NULL. - HttpInstance->LocalAddressIsIPv6 is FALSE and - HttpConfigData->IPv4Node is NULL. - HttpInstance->LocalAddressIsIPv6 is TRUE and - HttpConfigData->IPv6Node is NULL. + HttpConfigData->AccessPoint.IPv4Node or + HttpConfigData->AccessPoint.IPv6Node is NULL. @retval EFI_NOT_STARTED This EFI HTTP Protocol instance has not been started. **/ typedef EFI_STATUS (EFIAPI *EFI_HTTP_GET_MODE_DATA)( @@ -341,13 +343,13 @@ EFI_STATUS @retval EFI_SUCCESS Operation succeeded. @retval EFI_INVALID_PARAMETER One or more of the following conditions is TRUE: This is NULL. HttpConfigData->LocalAddressIsIPv6 is FALSE and - HttpConfigData->IPv4Node is NULL. + HttpConfigData->AccessPoint.IPv4Node is NULL. HttpConfigData->LocalAddressIsIPv6 is TRUE and - HttpConfigData->IPv6Node is NULL. + HttpConfigData->AccessPoint.IPv6Node is NULL. @retval EFI_ALREADY_STARTED Reinitialize this HTTP instance without calling Configure() with NULL to reset it. @retval EFI_DEVICE_ERRORAn unexpected system or network error occurred. @retval EFI_OUT_OF_RESOURCESCould not allocate enough system resources when executing Configure(). -- 1.9.5.msysgit.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection
>From this perspective, you're right. > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 1:10 PM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Laszlo Ersek ; Yao, > Jiewen ; Kinney, Michael D > ; Justen, Jordan L ; > Wolman, Ayellet ; Zeng, Star > Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer > detection > > If NULL pointer detection is disabled, the code should be behave same with > before, and ClearLegacyMemory() is not needed to be called because DxeCore > will behave same with before to handle the first 4K page clear , right? > > > Thanks, > Star > -Original Message- > From: Wang, Jian J > Sent: Thursday, September 28, 2017 11:55 AM > To: Zeng, Star ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Laszlo Ersek ; Yao, > Jiewen ; Kinney, Michael D > ; Justen, Jordan L ; > Wolman, Ayellet > Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer > detection > > Clearing this block of memory has nothing to do with NULL pointer detection. > I'm not sure the extra check is necessary. > > > -Original Message- > > From: Zeng, Star > > Sent: Thursday, September 28, 2017 11:31 AM > > To: Wang, Jian J ; edk2-devel@lists.01.org > > Cc: Dong, Eric ; Laszlo Ersek > > ; Yao, Jiewen ; Kinney, > > Michael D ; Justen, Jordan L > > ; Wolman, Ayellet > > ; Zeng, Star > > Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL > > pointer detection > > > > Another comment. > > Should IsNullDetectionEnabled() be checked before calling > > ClearLegacyMemory()? > > > > > > Thanks, > > Star > > -Original Message- > > From: Zeng, Star > > Sent: Thursday, September 28, 2017 11:24 AM > > To: Wang, Jian J ; edk2-devel@lists.01.org > > Cc: Dong, Eric ; Laszlo Ersek > > ; Yao, Jiewen ; Kinney, > > Michael D ; Justen, Jordan L > > ; Wolman, Ayellet > > ; Zeng, Star > > Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL > > pointer detection > > > > Minor comments to this patch. > > > > 1. IsNullDetectionEnabled() function is put in DxeLoad.c that is > > shared by all ARCHs, and the function is consuming > > PcdNullPointerDetectionPropertyMask, > > but PcdNullPointerDetectionPropertyMask is only declared in > > "[Pcd.IA32,Pcd.X64]" in inf. > > I am not sure whether it will cause build failure or not for non-IA32/X64 > > ARCHs. > > > > 2. How about using name "ClearFirst4KPage" instead of "ClearLegacyMemory" > > to be more clear? > > > > > > Thanks, > > Star > > -Original Message- > > From: Wang, Jian J > > Sent: Thursday, September 28, 2017 9:04 AM > > To: edk2-devel@lists.01.org > > Cc: Zeng, Star ; Dong, Eric > > ; Laszlo Ersek ; Yao, Jiewen > > ; Kinney, Michael D > > ; Justen, Jordan L > > ; Wolman, Ayellet > > > > Subject: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer > > detection > > > > > According to Jiewen's feedback, change the page split condition for > > > NULL pointer detection to exclude IsExecuteDisableBitAvailable() > > > (Ia32/DxeLoadFunc.c) > > > > NULL pointer detection is done by making use of paging mechanism of CPU. > > During page table setup, if enabled, the first 4-K page (0-4095) will > > be marked as NOT PRESENT. Any code which unintentionally access memory > > between > > 0-4095 will trigger a Page Fault exception which warns users that > > there's potential illegal code in BIOS. > > > > This also means that legacy code which has to access memory between > > 0-4095 should be cautious to temporarily disable this feature before > > the access and re- enable it afterwards; or disalbe this feature at all. > > > > Cc: Star Zeng > > Cc: Eric Dong > > Cc: Laszlo Ersek > > Cc: Jiewen Yao > > Cc: Michael Kinney > > Cc: Jordan Justen > > Cc: Ayellet Wolman > > Suggested-by: Ayellet Wolman > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jian J Wang > > --- > > MdeModulePkg/Core/DxeIplPeim/DxeIpl.h| 25 + > > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 + > > MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 65 > > > > MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 11 +++- > > MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 + > > MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 31 --- > > 6 files changed, 126 insertions(+), 9 deletions(-) > > > > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > > b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > > index 72d2532f50..1654bcd2dc 100644 > > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > > @@ -240,4 +240,29 @@ Decompress ( > >OUT UINTN *OutputSize > >); > > > > +/** > > + Clear legacy memory located at the first 4K-page. > > + > > + This function traverses the whole HOB list to check if memory from 0 to > 4095 > > + exists and has not been allocated, and then clear it if so. > > + > > + @param HoStart
Re: [edk2] [PATCH v2 6/6] MdeModulePkg/AtaAtapiPassThru: Fix possible out of range left shift
Got it, thanks for the information. :) Star -Original Message- From: Gao, Liming Sent: Thursday, September 28, 2017 11:57 AM To: Zeng, Star ; Wu, Hao A ; edk2-devel@lists.01.org Cc: Wu, Hao A ; Dong, Eric Subject: RE: [edk2] [PATCH v2 6/6] MdeModulePkg/AtaAtapiPassThru: Fix possible out of range left shift Star: Crc32 change is not required . I just gives my comment on it. So, there is no consistent issue here. We can keep this patch. Thanks Liming >-Original Message- >From: Zeng, Star >Sent: Monday, September 25, 2017 2:25 PM >To: Wu, Hao A ; edk2-devel@lists.01.org >Cc: Wu, Hao A ; Dong, Eric ; >Gao, Liming ; Zeng, Star >Subject: RE: [edk2] [PATCH v2 6/6] MdeModulePkg/AtaAtapiPassThru: Fix >possible out of range left shift > >I prefer to have the code consistent between this patch with [PATCH v2 >5/6] >MdeModulePkg/Crc32: Fix possible out of range left shift. > >Both to use (UINT32) typecast or 1U. > >Liming, Do you have any comment? > > >Thanks, >Star >-Original Message- >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >Hao Wu >Sent: Thursday, September 21, 2017 2:46 PM >To: edk2-devel@lists.01.org >Cc: Wu, Hao A ; Dong, Eric ; >Zeng, Star >Subject: [edk2] [PATCH v2 6/6] MdeModulePkg/AtaAtapiPassThru: Fix >possible out of range left shift > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=699 > >Within function AhciModeInitialization(), left shift operations of 'BIT0' >in the following statements: >"if ((PortImplementBitMap & (BIT0 << Port)) != 0) {" > >will incur possible out of range left shift when Port is 31, since >"1 << 31" is possible to exceed the range of type 'int' (signed). > >According to the C11 spec, Section 6.5.7: >> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated >> bits are filled with zeros. If E1 has an unsigned type, the value >> of the result is E1 * 2^E2 , reduced modulo one more than the >> maximum value representable in the result type. If E1 has a signed >> type and nonnegative value, and E1 * 2^E2 is representable in the >> result type, then that is the resulting value; otherwise, the >> behavior is undefined. > >This commit explicitly cast 'BIT0' with UINT32 to resolve this issue. > >Cc: Steven Shi >Cc: Star Zeng >Cc: Eric Dong >Contributed-under: TianoCore Contribution Agreement 1.1 >Signed-off-by: Hao Wu >--- > MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > >diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c >b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c >index b954de8101..e6de5d65bc 100644 >--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c >+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c >@@ -1,7 +1,7 @@ > /** @file > The file for AHCI mode of ATA host controller. > >- Copyright (c) 2010 - 2016, Intel Corporation. All rights >reserved. >+ Copyright (c) 2010 - 2017, Intel Corporation. All rights >+ reserved. > (C) Copyright 2015 Hewlett Packard Enterprise Development LP > This program and the accompanying materials > are licensed and made available under the terms and conditions of >the BSD License @@ -2314,7 +2314,7 @@ AhciModeInitialization ( > } > > for (Port = 0; Port < EFI_AHCI_MAX_PORTS; Port ++) { >-if ((PortImplementBitMap & (BIT0 << Port)) != 0) { >+if ((PortImplementBitMap & (((UINT32)BIT0) << Port)) != 0) { > // > // According to AHCI spec, MaxPortNumber should be equal or >greater than the number of implemented ports. > // >-- >2.12.0.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
Re: [edk2] [PATCH v3 5/5] MdeModulePkg/AtaAtapiPassThru: Fix possible out of range left shift
Reviewed-by: Star Zeng -Original Message- From: Wu, Hao A Sent: Thursday, September 28, 2017 12:32 PM To: edk2-devel@lists.01.org Cc: Wu, Hao A ; Shi, Steven ; Zeng, Star ; Dong, Eric Subject: [PATCH v3 5/5] MdeModulePkg/AtaAtapiPassThru: Fix possible out of range left shift REF: https://bugzilla.tianocore.org/show_bug.cgi?id=699 Within function AhciModeInitialization(), left shift operations of 'BIT0' in the following statements: "if ((PortImplementBitMap & (BIT0 << Port)) != 0) {" will incur possible out of range left shift when Port is 31, since "1 << 31" is possible to exceed the range of type 'int' (signed). According to the C11 spec, Section 6.5.7: > 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated > bits are filled with zeros. If E1 has an unsigned type, the value > of the result is E1 * 2^E2 , reduced modulo one more than the > maximum value representable in the result type. If E1 has a signed > type and nonnegative value, and E1 * 2^E2 is representable in the > result type, then that is the resulting value; otherwise, the > behavior is undefined. This commit explicitly cast 'BIT0' with UINT32 to resolve this issue. Cc: Steven Shi Cc: Star Zeng Cc: Eric Dong Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu --- MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c index b954de8101..e6de5d65bc 100644 --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c @@ -1,7 +1,7 @@ /** @file The file for AHCI mode of ATA host controller. - Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved. + Copyright (c) 2010 - 2017, Intel Corporation. All rights + reserved. (C) Copyright 2015 Hewlett Packard Enterprise Development LP This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -2314,7 +2314,7 @@ AhciModeInitialization ( } for (Port = 0; Port < EFI_AHCI_MAX_PORTS; Port ++) { -if ((PortImplementBitMap & (BIT0 << Port)) != 0) { +if ((PortImplementBitMap & (((UINT32)BIT0) << Port)) != 0) { // // According to AHCI spec, MaxPortNumber should be equal or greater than the number of implemented ports. // -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection
I am curious about what is potential future change. Thanks, Star -Original Message- From: Wang, Jian J Sent: Thursday, September 28, 2017 11:50 AM To: Zeng, Star ; edk2-devel@lists.01.org Cc: Dong, Eric ; Laszlo Ersek ; Yao, Jiewen ; Kinney, Michael D ; Justen, Jordan L ; Wolman, Ayellet Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection Please see my comments inline below. > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 11:24 AM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Laszlo Ersek > ; Yao, Jiewen ; Kinney, > Michael D ; Justen, Jordan L > ; Wolman, Ayellet > ; Zeng, Star > Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL > pointer detection > > Minor comments to this patch. > > 1. IsNullDetectionEnabled() function is put in DxeLoad.c that is > shared by all ARCHs, and the function is consuming > PcdNullPointerDetectionPropertyMask, > but PcdNullPointerDetectionPropertyMask is only declared in > "[Pcd.IA32,Pcd.X64]" in inf. > I am not sure whether it will cause build failure or not for non-IA32/X64 > ARCHs. > You're right. The PCD should not be under this section. > 2. How about using name "ClearFirst4KPage" instead of "ClearLegacyMemory" > to be more clear? > I prefer a bit more general name in case of potential future change. > > Thanks, > Star > -Original Message- > From: Wang, Jian J > Sent: Thursday, September 28, 2017 9:04 AM > To: edk2-devel@lists.01.org > Cc: Zeng, Star ; Dong, Eric > ; Laszlo Ersek ; Yao, Jiewen > ; Kinney, Michael D > ; Justen, Jordan L > ; Wolman, Ayellet > > Subject: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer > detection > > > According to Jiewen's feedback, change the page split condition for > > NULL pointer detection to exclude IsExecuteDisableBitAvailable() > > (Ia32/DxeLoadFunc.c) > > NULL pointer detection is done by making use of paging mechanism of CPU. > During page table setup, if enabled, the first 4-K page (0-4095) will > be marked as NOT PRESENT. Any code which unintentionally access memory > between > 0-4095 will trigger a Page Fault exception which warns users that > there's potential illegal code in BIOS. > > This also means that legacy code which has to access memory between > 0-4095 should be cautious to temporarily disable this feature before > the access and re- enable it afterwards; or disalbe this feature at all. > > Cc: Star Zeng > Cc: Eric Dong > Cc: Laszlo Ersek > Cc: Jiewen Yao > Cc: Michael Kinney > Cc: Jordan Justen > Cc: Ayellet Wolman > Suggested-by: Ayellet Wolman > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang > --- > MdeModulePkg/Core/DxeIplPeim/DxeIpl.h| 25 + > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 + > MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 65 > > MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 11 +++- > MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 + > MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 31 --- > 6 files changed, 126 insertions(+), 9 deletions(-) > > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > index 72d2532f50..1654bcd2dc 100644 > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > @@ -240,4 +240,29 @@ Decompress ( >OUT UINTN *OutputSize >); > > +/** > + Clear legacy memory located at the first 4K-page. > + > + This function traverses the whole HOB list to check if memory from 0 to > 4095 > + exists and has not been allocated, and then clear it if so. > + > + @param HoStart The start of HobList passed to DxeCore. > + > +**/ > +VOID > +ClearLegacyMemory ( > + IN VOID *HobStart > + ); > + > +/** > + Return configure status of NULL pointer detection feature > + > + @return TRUE NULL pointer detection feature is enabled > + @return FALSE NULL pointer detection feature is disabled **/ > +BOOLEAN IsNullDetectionEnabled ( > + VOID > + ); > + > #endif > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > index c54afe4aa6..9d0e76a293 100644 > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > @@ -115,6 +115,7 @@ > [Pcd.IA32,Pcd.X64] >gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## > SOMETIMES_CONSUMES >gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask > ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask > ## CONSUMES > > [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] >gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## > SOMETIMES_CONSUMES > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > index 50b5440d15..0a71b1f3de 100644 >
Re: [edk2] [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection
If NULL pointer detection is disabled, the code should be behave same with before, and ClearLegacyMemory() is not needed to be called because DxeCore will behave same with before to handle the first 4K page clear , right? Thanks, Star -Original Message- From: Wang, Jian J Sent: Thursday, September 28, 2017 11:55 AM To: Zeng, Star ; edk2-devel@lists.01.org Cc: Dong, Eric ; Laszlo Ersek ; Yao, Jiewen ; Kinney, Michael D ; Justen, Jordan L ; Wolman, Ayellet Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection Clearing this block of memory has nothing to do with NULL pointer detection. I'm not sure the extra check is necessary. > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 11:31 AM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Laszlo Ersek > ; Yao, Jiewen ; Kinney, > Michael D ; Justen, Jordan L > ; Wolman, Ayellet > ; Zeng, Star > Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL > pointer detection > > Another comment. > Should IsNullDetectionEnabled() be checked before calling > ClearLegacyMemory()? > > > Thanks, > Star > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 11:24 AM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Laszlo Ersek > ; Yao, Jiewen ; Kinney, > Michael D ; Justen, Jordan L > ; Wolman, Ayellet > ; Zeng, Star > Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL > pointer detection > > Minor comments to this patch. > > 1. IsNullDetectionEnabled() function is put in DxeLoad.c that is > shared by all ARCHs, and the function is consuming > PcdNullPointerDetectionPropertyMask, > but PcdNullPointerDetectionPropertyMask is only declared in > "[Pcd.IA32,Pcd.X64]" in inf. > I am not sure whether it will cause build failure or not for non-IA32/X64 > ARCHs. > > 2. How about using name "ClearFirst4KPage" instead of "ClearLegacyMemory" > to be more clear? > > > Thanks, > Star > -Original Message- > From: Wang, Jian J > Sent: Thursday, September 28, 2017 9:04 AM > To: edk2-devel@lists.01.org > Cc: Zeng, Star ; Dong, Eric > ; Laszlo Ersek ; Yao, Jiewen > ; Kinney, Michael D > ; Justen, Jordan L > ; Wolman, Ayellet > > Subject: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer > detection > > > According to Jiewen's feedback, change the page split condition for > > NULL pointer detection to exclude IsExecuteDisableBitAvailable() > > (Ia32/DxeLoadFunc.c) > > NULL pointer detection is done by making use of paging mechanism of CPU. > During page table setup, if enabled, the first 4-K page (0-4095) will > be marked as NOT PRESENT. Any code which unintentionally access memory > between > 0-4095 will trigger a Page Fault exception which warns users that > there's potential illegal code in BIOS. > > This also means that legacy code which has to access memory between > 0-4095 should be cautious to temporarily disable this feature before > the access and re- enable it afterwards; or disalbe this feature at all. > > Cc: Star Zeng > Cc: Eric Dong > Cc: Laszlo Ersek > Cc: Jiewen Yao > Cc: Michael Kinney > Cc: Jordan Justen > Cc: Ayellet Wolman > Suggested-by: Ayellet Wolman > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang > --- > MdeModulePkg/Core/DxeIplPeim/DxeIpl.h| 25 + > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 + > MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 65 > > MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 11 +++- > MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 + > MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 31 --- > 6 files changed, 126 insertions(+), 9 deletions(-) > > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > index 72d2532f50..1654bcd2dc 100644 > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > @@ -240,4 +240,29 @@ Decompress ( >OUT UINTN *OutputSize >); > > +/** > + Clear legacy memory located at the first 4K-page. > + > + This function traverses the whole HOB list to check if memory from 0 to > 4095 > + exists and has not been allocated, and then clear it if so. > + > + @param HoStart The start of HobList passed to DxeCore. > + > +**/ > +VOID > +ClearLegacyMemory ( > + IN VOID *HobStart > + ); > + > +/** > + Return configure status of NULL pointer detection feature > + > + @return TRUE NULL pointer detection feature is enabled > + @return FALSE NULL pointer detection feature is disabled **/ > +BOOLEAN IsNullDetectionEnabled ( > + VOID > + ); > + > #endif > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > index c54afe4aa6..9d0e76a293 100644 > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > +++ b/MdeModulePkg/Core
Re: [edk2] [PATCH v3 3/6] MdeModulePkg/Core/Dxe: Add EndOfDxe workaround
Thanks for the feedback. Please see my comments below. > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 11:35 AM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Laszlo Ersek ; Yao, > Jiewen ; Kinney, Michael D > ; Justen, Jordan L ; > Wolman, Ayellet ; Zeng, Star > Subject: RE: [PATCH v3 3/6] MdeModulePkg/Core/Dxe: Add EndOfDxe > workaround > > Some comments to this patch. > > 1. How about using lower TPL TPL_CALLBACK instead of TPL_NOTIFY for the > notification? I think it's safe to use TPL_CALLBACK. > 2. Should GCD SetMemorySpaceCapabilities + SetMemorySpaceAttributes be > used instead of gCpu->SetMemoryAttributes()? Yes. Since the GCG out-of-sync issue has been fixed, GCD service should be used instead. > > Thanks, > Star > -Original Message- > From: Wang, Jian J > Sent: Thursday, September 28, 2017 9:04 AM > To: edk2-devel@lists.01.org > Cc: Zeng, Star ; Dong, Eric ; Laszlo > Ersek ; Yao, Jiewen ; Kinney, > Michael D ; Justen, Jordan L > ; Wolman, Ayellet > Subject: [PATCH v3 3/6] MdeModulePkg/Core/Dxe: Add EndOfDxe workaround > > One of issue caused by enabling NULL pointer detection is that some PCI device > OptionROM, binary drivers and binary OS boot loaders may have NULL pointer > access bugs, which will prevent BIOS from booting and is almost impossible to > fix. BIT7 of PCD PcdNullPointerDetectionPropertyMask is used as a workaround > to indicate BIOS to disable NULL pointer detection right after event > gEfiEndOfDxeEventGroupGuid, and then let boot continue. > > Cc: Star Zeng > Cc: Eric Dong > Cc: Laszlo Ersek > Cc: Jiewen Yao > Cc: Michael Kinney > Cc: Jordan Justen > Cc: Ayellet Wolman > Suggested-by: Ayellet Wolman > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang > --- > MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + > MdeModulePkg/Core/Dxe/Mem/Page.c | 4 ++- > MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 48 > +++ > 3 files changed, 52 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf > b/MdeModulePkg/Core/Dxe/DxeMain.inf > index 30d5984f7c..0a161ffd71 100644 > --- a/MdeModulePkg/Core/Dxe/DxeMain.inf > +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf > @@ -192,6 +192,7 @@ >gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable > ## > CONSUMES >gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy > ## > CONSUMES >gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy > ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask > ## CONSUMES > > # [Hob] > # RESOURCE_DESCRIPTOR ## CONSUMES > diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c > b/MdeModulePkg/Core/Dxe/Mem/Page.c > index a142c79ee2..0468df3171 100644 > --- a/MdeModulePkg/Core/Dxe/Mem/Page.c > +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c > @@ -188,7 +188,9 @@ CoreAddRange ( >// used for other purposes. >// >if (Type == EfiConventionalMemory && Start == 0 && (End >= EFI_PAGE_SIZE - > 1)) { > -SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0); > +if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0) { > + SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0); > +} >} > >// > diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > index a73c4ccd64..73e3b269f3 100644 > --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > @@ -995,6 +995,36 @@ MemoryProtectionExitBootServicesCallback ( >} > } > > +/** > + Disable NULL pointer detection after EndOfDxe. This is a workaround > +resort in > + order to skip unfixable NULL pointer access issues detected in > +OptionROM or > + boot loaders. > + > + @param[in] Event The Event this notify function registered to. > + @param[in] Context Pointer to the context data registered to the Event. > +**/ > +VOID > +EFIAPI > +DisableNullDetectionAtTheEndOfDxe ( > + EFI_EVENT Event, > + VOID*Context > + ) > +{ > + EFI_STATUSStatus; > + > + DEBUG ((DEBUG_INFO, "DisableNullDetectionAtTheEndOfDxe(): > + start\r\n")); // // Disable NULL pointer detection by enabling first > + 4K page // Status = gCpu->SetMemoryAttributes (gCpu, 0, > + EFI_PAGE_SIZE, 0); ASSERT_EFI_ERROR (Status); > + > + CoreCloseEvent (Event); > + DEBUG ((DEBUG_INFO, "DisableNullDetectionAtTheEndOfDxe(): end\r\n")); > + > + return; > +} > + > /** >Initialize Memory Protection support. > **/ > @@ -1006,6 +1036,7 @@ CoreInitializeMemoryProtection ( { >EFI_STATUS Status; >EFI_EVENT Event; > + EFI_EVENT EndOfDxeEvent; >VOID*Registration; > >mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy); > @@ -1044,6 +1075,23 @@ CoreInitializeMemoryProtection ( > )
[edk2] [PATCH v3 1/5] MdePkg/PrintLib: Fix possible negative value left shift
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=702 Within function InternalPrintLibSPrintMarker(), possible left shift of a negative value is found in: "(*(ArgumentString + 1) << 8)" which involves undefined behavior. Since '*(ArgumentString + 1)' is of type CONST CHAR8 (signed), it will be promoted to type int (signed) during the left shift operation. If '*(ArgumentString + 1)' is a negative value, the behavior will be undefined. According to the C11 spec, Section 6.5.7: > 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated > bits are filled with zeros. If E1 has an unsigned type, the value > of the result is E1 * 2^E2 , reduced modulo one more than the > maximum value representable in the result type. If E1 has a signed > type and nonnegative value, and E1 * 2^E2 is representable in the > result type, then that is the resulting value; otherwise, the > behavior is undefined. This commit explicitly cast '*(ArgumentString + 1)' with UINT8 to resolve this issue. Cc: Steven Shi Cc: Michael Kinney Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu Reviewed-by: Liming Gao --- MdePkg/Library/BasePrintLib/PrintLibInternal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c b/MdePkg/Library/BasePrintLib/PrintLibInternal.c index cec5b3bc99..28d946472f 100644 --- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c +++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c @@ -1165,7 +1165,7 @@ BasePrintLibSPrintMarker ( // Copy the string into the output buffer performing the required type conversions // while (Index < Count) { - ArgumentCharacter = ((*ArgumentString & 0xff) | (*(ArgumentString + 1) << 8)) & ArgumentMask; + ArgumentCharacter = ((*ArgumentString & 0xff) | (((UINT8)*(ArgumentString + 1)) << 8)) & ArgumentMask; LengthToReturn += (1 * BytesPerOutputCharacter); if ((Flags & COUNT_ONLY_NO_PRINT) == 0 && Buffer != NULL) { -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 4/5] MdeModulePkg/DxeNetLib: Fix negative value left shift
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=698 Within function NetRandomInitSeed(), left shift a negative value is used in: "~Time.Hour << 24" which involves undefined behavior. Since Time.Hour is of type UINT8 (range from 0 to 23), hence ~Time.Hour will be a negative value (of type int, signed). According to the C11 spec, Section 6.5.7: > 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated > bits are filled with zeros. If E1 has an unsigned type, the value > of the result is E1 * 2^E2 , reduced modulo one more than the > maximum value representable in the result type. If E1 has a signed > type and nonnegative value, and E1 * 2^E2 is representable in the > result type, then that is the resulting value; otherwise, the > behavior is undefined. This commit will remove the '~' operator before 'Time.Hour', since it seems like an implementation choice for generating the seed. Cc: Steven Shi Cc: Fu Siyuan Cc: Ye Ting Cc: Wu Jiaxin Cc: Qin Long Cc: Star Zeng Cc: Eric Dong Cc: Paolo Bonzini Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu --- MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c index 7cd7e3aca0..1ee77fe036 100644 --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c @@ -872,7 +872,7 @@ NetRandomInitSeed ( UINT64MonotonicCount; gRT->GetTime (&Time, NULL); - Seed = (~Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | Time.Second); + Seed = (Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | Time.Second); Seed ^= Time.Nanosecond; Seed ^= Time.Year << 7; -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 0/5] Resolve undefined behaviours in left shift OPs
Based on the feedbacks from Liming, the V3 series drops the patch for MdeModulePkg/Crc32. Since the left shift operation "1 << Index" will be removed by an under-reviewing patch series: [Patch 0/2] Add CalculateCrc32() API in MdePkg BaseLib V2 history: According to the feebacks from Paolo, the following changes are made in this version of patch series: a. Refine the code logic in Tpl.c to void left shifting the negative value. Also makes the code more readable; b. Remove the '~' operator before 'Time.Hour' in DxeNetLib.c, since it seems like an implementation choice for generating the seed; c. Use '1U' instead of '(UINT32)1' in Crc32.c. V1 history: The series resolves two kinds of undefined behaviours in left shift operations: a. Left-shifting negative values; b. Left-shifting that incurs the result being out of range. Cc: Steven Shi Cc: Michael Kinney Cc: Liming Gao Cc: Star Zeng Cc: Eric Dong Cc: Fu Siyuan Cc: Ye Ting Cc: Wu Jiaxin Cc: Qin Long Cc: Paolo Bonzini Hao Wu (5): MdePkg/PrintLib: Fix possible negative value left shift MdeModulePkg/PrintLib: Fix possible negative value left shift MdeModulePkg/Tpl: Fix negative value left shift MdeModulePkg/DxeNetLib: Fix negative value left shift MdeModulePkg/AtaAtapiPassThru: Fix possible out of range left shift MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 4 ++-- MdeModulePkg/Core/Dxe/Event/Tpl.c | 12 +--- MdeModulePkg/Library/DxeNetLib/DxeNetLib.c| 2 +- MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 2 +- MdePkg/Library/BasePrintLib/PrintLibInternal.c| 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 2/5] MdeModulePkg/PrintLib: Fix possible negative value left shift
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=702 Within function InternalPrintLibSPrintMarker(), possible left shift of a negative value is found in: "(*(ArgumentString + 1) << 8)" which involves undefined behavior. Since '*(ArgumentString + 1)' is of type CONST CHAR8 (signed), it will be promoted to type int (signed) during the left shift operation. If '*(ArgumentString + 1)' is a negative value, the behavior will be undefined. According to the C11 spec, Section 6.5.7: > 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated > bits are filled with zeros. If E1 has an unsigned type, the value > of the result is E1 * 2^E2 , reduced modulo one more than the > maximum value representable in the result type. If E1 has a signed > type and nonnegative value, and E1 * 2^E2 is representable in the > result type, then that is the resulting value; otherwise, the > behavior is undefined. This commit explicitly cast '*(ArgumentString + 1)' with UINT8 to resolve this issue. Cc: Steven Shi Cc: Michael Kinney Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu Reviewed-by: Liming Gao --- MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c index b58db8e011..56534e56c3 100644 --- a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c +++ b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c @@ -2108,7 +2108,7 @@ InternalPrintLibSPrintMarker ( // Copy the string into the output buffer performing the required type conversions // while (Index < Count) { - ArgumentCharacter = ((*ArgumentString & 0xff) | (*(ArgumentString + 1) << 8)) & ArgumentMask; + ArgumentCharacter = ((*ArgumentString & 0xff) | (((UINT8)*(ArgumentString + 1)) << 8)) & ArgumentMask; LengthToReturn += (1 * BytesPerOutputCharacter); if ((Flags & COUNT_ONLY_NO_PRINT) == 0 && Buffer != NULL) { -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 5/5] MdeModulePkg/AtaAtapiPassThru: Fix possible out of range left shift
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=699 Within function AhciModeInitialization(), left shift operations of 'BIT0' in the following statements: "if ((PortImplementBitMap & (BIT0 << Port)) != 0) {" will incur possible out of range left shift when Port is 31, since "1 << 31" is possible to exceed the range of type 'int' (signed). According to the C11 spec, Section 6.5.7: > 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated > bits are filled with zeros. If E1 has an unsigned type, the value > of the result is E1 * 2^E2 , reduced modulo one more than the > maximum value representable in the result type. If E1 has a signed > type and nonnegative value, and E1 * 2^E2 is representable in the > result type, then that is the resulting value; otherwise, the > behavior is undefined. This commit explicitly cast 'BIT0' with UINT32 to resolve this issue. Cc: Steven Shi Cc: Star Zeng Cc: Eric Dong Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu --- MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c index b954de8101..e6de5d65bc 100644 --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c @@ -1,7 +1,7 @@ /** @file The file for AHCI mode of ATA host controller. - Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved. + Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved. (C) Copyright 2015 Hewlett Packard Enterprise Development LP This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -2314,7 +2314,7 @@ AhciModeInitialization ( } for (Port = 0; Port < EFI_AHCI_MAX_PORTS; Port ++) { -if ((PortImplementBitMap & (BIT0 << Port)) != 0) { +if ((PortImplementBitMap & (((UINT32)BIT0) << Port)) != 0) { // // According to AHCI spec, MaxPortNumber should be equal or greater than the number of implemented ports. // -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 3/5] MdeModulePkg/Tpl: Fix negative value left shift
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=695 Within function CoreRestoreTpl(), left shift a negative value -2 is used in: "while (((-2 << NewTpl) & gEventPending) != 0) {" which involves undefined behavior. According to the C11 spec, Section 6.5.7: > 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated > bits are filled with zeros. If E1 has an unsigned type, the value > of the result is E1 * 2^E2 , reduced modulo one more than the > maximum value representable in the result type. If E1 has a signed > type and nonnegative value, and E1 * 2^E2 is representable in the > result type, then that is the resulting value; otherwise, the > behavior is undefined. This commit refines the code logic to avoid left shifting the negative value. Cc: Steven Shi Cc: Eric Dong Cc: Paolo Bonzini Cc: Michael Kinney Cc: Liming Gao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu Reviewed-by: Star Zeng --- MdeModulePkg/Core/Dxe/Event/Tpl.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Event/Tpl.c b/MdeModulePkg/Core/Dxe/Event/Tpl.c index 8ad0a33701..e3caf832b8 100644 --- a/MdeModulePkg/Core/Dxe/Event/Tpl.c +++ b/MdeModulePkg/Core/Dxe/Event/Tpl.c @@ -1,7 +1,7 @@ /** @file Task priority (TPL) functions. -Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved. +Copyright (c) 2006 - 2017, 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 @@ -103,6 +103,7 @@ CoreRestoreTpl ( ) { EFI_TPL OldTpl; + EFI_TPL PendingTpl; OldTpl = gEfiCurrentTpl; if (NewTpl > OldTpl) { @@ -123,8 +124,13 @@ CoreRestoreTpl ( // // Dispatch any pending events // - while (((-2 << NewTpl) & gEventPending) != 0) { -gEfiCurrentTpl = (UINTN) HighBitSet64 (gEventPending); + while (gEventPending != 0) { +PendingTpl = (UINTN) HighBitSet64 (gEventPending); +if (PendingTpl <= NewTpl) { + break; +} + +gEfiCurrentTpl = PendingTpl; if (gEfiCurrentTpl < TPL_HIGH_LEVEL) { CoreSetInterruptState (TRUE); } -- 2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 2/6] MdeModulePkg/PrintLib: Fix possible negative value left shift
Reviewed-by: Liming Gao >-Original Message- >From: Wu, Hao A >Sent: Thursday, September 21, 2017 2:46 PM >To: edk2-devel@lists.01.org >Cc: Wu, Hao A ; Shi, Steven ; >Kinney, Michael D ; Gao, Liming > >Subject: [PATCH v2 2/6] MdeModulePkg/PrintLib: Fix possible negative value >left shift > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=702 > >Within function InternalPrintLibSPrintMarker(), possible left shift of a >negative value is found in: >"(*(ArgumentString + 1) << 8)" > >which involves undefined behavior. > >Since '*(ArgumentString + 1)' is of type CONST CHAR8 (signed), it will be >promoted to type int (signed) during the left shift operation. If >'*(ArgumentString + 1)' is a negative value, the behavior will be >undefined. > >According to the C11 spec, Section 6.5.7: >> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated >> bits are filled with zeros. If E1 has an unsigned type, the value >> of the result is E1 * 2^E2 , reduced modulo one more than the >> maximum value representable in the result type. If E1 has a signed >> type and nonnegative value, and E1 * 2^E2 is representable in the >> result type, then that is the resulting value; otherwise, the >> behavior is undefined. > >This commit explicitly cast '*(ArgumentString + 1)' with UINT8 to resolve >this issue. > >Cc: Steven Shi >Cc: Michael Kinney >Cc: Liming Gao >Contributed-under: TianoCore Contribution Agreement 1.1 >Signed-off-by: Hao Wu >--- > MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c >b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c >index b58db8e011..56534e56c3 100644 >--- a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c >+++ b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c >@@ -2108,7 +2108,7 @@ InternalPrintLibSPrintMarker ( > // Copy the string into the output buffer performing the required type >conversions > // > while (Index < Count) { >- ArgumentCharacter = ((*ArgumentString & 0xff) | (*(ArgumentString + 1) ><< 8)) & ArgumentMask; >+ ArgumentCharacter = ((*ArgumentString & 0xff) | >(((UINT8)*(ArgumentString + 1)) << 8)) & ArgumentMask; > > LengthToReturn += (1 * BytesPerOutputCharacter); > if ((Flags & COUNT_ONLY_NO_PRINT) == 0 && Buffer != NULL) { >-- >2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 1/6] MdePkg/PrintLib: Fix possible negative value left shift
Reviewed-by: Liming Gao >-Original Message- >From: Wu, Hao A >Sent: Thursday, September 21, 2017 2:46 PM >To: edk2-devel@lists.01.org >Cc: Wu, Hao A ; Shi, Steven ; >Kinney, Michael D ; Gao, Liming > >Subject: [PATCH v2 1/6] MdePkg/PrintLib: Fix possible negative value left shift > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=702 > >Within function InternalPrintLibSPrintMarker(), possible left shift of a >negative value is found in: >"(*(ArgumentString + 1) << 8)" > >which involves undefined behavior. > >Since '*(ArgumentString + 1)' is of type CONST CHAR8 (signed), it will be >promoted to type int (signed) during the left shift operation. If >'*(ArgumentString + 1)' is a negative value, the behavior will be >undefined. > >According to the C11 spec, Section 6.5.7: >> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated >> bits are filled with zeros. If E1 has an unsigned type, the value >> of the result is E1 * 2^E2 , reduced modulo one more than the >> maximum value representable in the result type. If E1 has a signed >> type and nonnegative value, and E1 * 2^E2 is representable in the >> result type, then that is the resulting value; otherwise, the >> behavior is undefined. > >This commit explicitly cast '*(ArgumentString + 1)' with UINT8 to resolve >this issue. > >Cc: Steven Shi >Cc: Michael Kinney >Cc: Liming Gao >Contributed-under: TianoCore Contribution Agreement 1.1 >Signed-off-by: Hao Wu >--- > MdePkg/Library/BasePrintLib/PrintLibInternal.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c >b/MdePkg/Library/BasePrintLib/PrintLibInternal.c >index cec5b3bc99..28d946472f 100644 >--- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c >+++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c >@@ -1165,7 +1165,7 @@ BasePrintLibSPrintMarker ( > // Copy the string into the output buffer performing the required type >conversions > // > while (Index < Count) { >- ArgumentCharacter = ((*ArgumentString & 0xff) | (*(ArgumentString + 1) ><< 8)) & ArgumentMask; >+ ArgumentCharacter = ((*ArgumentString & 0xff) | >(((UINT8)*(ArgumentString + 1)) << 8)) & ArgumentMask; > > LengthToReturn += (1 * BytesPerOutputCharacter); > if ((Flags & COUNT_ONLY_NO_PRINT) == 0 && Buffer != NULL) { >-- >2.12.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 5/6] MdeModulePkg/Crc32: Fix possible out of range left shift
Got it. I will send a new series to drop this patch. Best Regards, Hao Wu > -Original Message- > From: Gao, Liming > Sent: Thursday, September 28, 2017 11:54 AM > To: Wu, Hao A; edk2-devel@lists.01.org > Cc: Wu, Hao A; Paolo Bonzini; Dong, Eric; Zeng, Star > Subject: RE: [edk2] [PATCH v2 5/6] MdeModulePkg/Crc32: Fix possible out of > range left shift > > Hao: > I sent another patch to introduce CacluateCrc32() API in BaseLib. It will > update > MdeModulePkg Crc32 to depend on BaseLib. And, BaseLib CacluateCrc32() has > no such logic. So, I think this patch is not necessary. > > Thanks > Liming > >-Original Message- > >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao > >Wu > >Sent: Thursday, September 21, 2017 2:46 PM > >To: edk2-devel@lists.01.org > >Cc: Wu, Hao A ; Paolo Bonzini ; > >Dong, Eric ; Zeng, Star > >Subject: [edk2] [PATCH v2 5/6] MdeModulePkg/Crc32: Fix possible out of > >range left shift > > > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=697 > > > >Within function ReverseBits(), left shift operations of 1 in the > >following statements: > >"(1 << Index)" and "(1 << (31 - Index))" > > > >will incur possible out of range left shift when Index is either 0 or > >31, since "1 << 31" is possible to exceed the range of type 'int' > >(signed). > > > >According to the C11 spec, Section 6.5.7: > >> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated > >> bits are filled with zeros. If E1 has an unsigned type, the value > >> of the result is E1 * 2^E2 , reduced modulo one more than the > >> maximum value representable in the result type. If E1 has a signed > >> type and nonnegative value, and E1 * 2^E2 is representable in the > >> result type, then that is the resulting value; otherwise, the > >> behavior is undefined. > > > >This commit uses '1U' instead of '1' to resolve this issue. > > > >Cc: Steven Shi > >Cc: Star Zeng > >Cc: Eric Dong > >Cc: Paolo Bonzini > >Contributed-under: TianoCore Contribution Agreement 1.1 > >Signed-off-by: Hao Wu > >--- > > MdeModulePkg/Core/RuntimeDxe/Crc32.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > >diff --git a/MdeModulePkg/Core/RuntimeDxe/Crc32.c > >b/MdeModulePkg/Core/RuntimeDxe/Crc32.c > >index a6fe77fa34..2cd234b562 100644 > >--- a/MdeModulePkg/Core/RuntimeDxe/Crc32.c > >+++ b/MdeModulePkg/Core/RuntimeDxe/Crc32.c > >@@ -7,7 +7,7 @@ > > EFI Runtime Services Table are converted from physical address to > > virtual addresses. This requires that the 32-bit CRC be recomputed. > > > >-Copyright (c) 2006 - 2008, Intel Corporation. All rights reserved. > >+Copyright (c) 2006 - 2017, 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 > >@@ -79,8 +79,8 @@ ReverseBits ( > > > > NewValue = 0; > > for (Index = 0; Index < 32; Index++) { > >-if ((Value & (1 << Index)) != 0) { > >- NewValue = NewValue | (1 << (31 - Index)); > >+if ((Value & (1U << Index)) != 0) { > >+ NewValue = NewValue | (1U << (31 - Index)); > > } > > } > > > >-- > >2.12.0.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
Re: [edk2] [PATCH v2 6/6] MdeModulePkg/AtaAtapiPassThru: Fix possible out of range left shift
Star: Crc32 change is not required . I just gives my comment on it. So, there is no consistent issue here. We can keep this patch. Thanks Liming >-Original Message- >From: Zeng, Star >Sent: Monday, September 25, 2017 2:25 PM >To: Wu, Hao A ; edk2-devel@lists.01.org >Cc: Wu, Hao A ; Dong, Eric ; Gao, >Liming ; Zeng, Star >Subject: RE: [edk2] [PATCH v2 6/6] MdeModulePkg/AtaAtapiPassThru: Fix >possible out of range left shift > >I prefer to have the code consistent between this patch with [PATCH v2 5/6] >MdeModulePkg/Crc32: Fix possible out of range left shift. > >Both to use (UINT32) typecast or 1U. > >Liming, Do you have any comment? > > >Thanks, >Star >-Original Message- >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao >Wu >Sent: Thursday, September 21, 2017 2:46 PM >To: edk2-devel@lists.01.org >Cc: Wu, Hao A ; Dong, Eric ; >Zeng, Star >Subject: [edk2] [PATCH v2 6/6] MdeModulePkg/AtaAtapiPassThru: Fix >possible out of range left shift > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=699 > >Within function AhciModeInitialization(), left shift operations of 'BIT0' >in the following statements: >"if ((PortImplementBitMap & (BIT0 << Port)) != 0) {" > >will incur possible out of range left shift when Port is 31, since >"1 << 31" is possible to exceed the range of type 'int' (signed). > >According to the C11 spec, Section 6.5.7: >> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated >> bits are filled with zeros. If E1 has an unsigned type, the value >> of the result is E1 * 2^E2 , reduced modulo one more than the >> maximum value representable in the result type. If E1 has a signed >> type and nonnegative value, and E1 * 2^E2 is representable in the >> result type, then that is the resulting value; otherwise, the >> behavior is undefined. > >This commit explicitly cast 'BIT0' with UINT32 to resolve this issue. > >Cc: Steven Shi >Cc: Star Zeng >Cc: Eric Dong >Contributed-under: TianoCore Contribution Agreement 1.1 >Signed-off-by: Hao Wu >--- > MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > >diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c >b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c >index b954de8101..e6de5d65bc 100644 >--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c >+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c >@@ -1,7 +1,7 @@ > /** @file > The file for AHCI mode of ATA host controller. > >- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved. >+ Copyright (c) 2010 - 2017, Intel Corporation. All rights >+ reserved. > (C) Copyright 2015 Hewlett Packard Enterprise Development LP > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD >License @@ -2314,7 +2314,7 @@ AhciModeInitialization ( > } > > for (Port = 0; Port < EFI_AHCI_MAX_PORTS; Port ++) { >-if ((PortImplementBitMap & (BIT0 << Port)) != 0) { >+if ((PortImplementBitMap & (((UINT32)BIT0) << Port)) != 0) { > // > // According to AHCI spec, MaxPortNumber should be equal or greater >than the number of implemented ports. > // >-- >2.12.0.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
Re: [edk2] [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection
Clearing this block of memory has nothing to do with NULL pointer detection. I'm not sure the extra check is necessary. > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 11:31 AM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Laszlo Ersek ; Yao, > Jiewen ; Kinney, Michael D > ; Justen, Jordan L ; > Wolman, Ayellet ; Zeng, Star > Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer > detection > > Another comment. > Should IsNullDetectionEnabled() be checked before calling > ClearLegacyMemory()? > > > Thanks, > Star > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 11:24 AM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Laszlo Ersek ; Yao, > Jiewen ; Kinney, Michael D > ; Justen, Jordan L ; > Wolman, Ayellet ; Zeng, Star > Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer > detection > > Minor comments to this patch. > > 1. IsNullDetectionEnabled() function is put in DxeLoad.c that is shared by all > ARCHs, and the function is consuming PcdNullPointerDetectionPropertyMask, > but PcdNullPointerDetectionPropertyMask is only declared in > "[Pcd.IA32,Pcd.X64]" in inf. > I am not sure whether it will cause build failure or not for non-IA32/X64 > ARCHs. > > 2. How about using name "ClearFirst4KPage" instead of "ClearLegacyMemory" > to be more clear? > > > Thanks, > Star > -Original Message- > From: Wang, Jian J > Sent: Thursday, September 28, 2017 9:04 AM > To: edk2-devel@lists.01.org > Cc: Zeng, Star ; Dong, Eric ; Laszlo > Ersek ; Yao, Jiewen ; Kinney, > Michael D ; Justen, Jordan L > ; Wolman, Ayellet > Subject: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer > detection > > > According to Jiewen's feedback, change the page split condition for > > NULL pointer detection to exclude IsExecuteDisableBitAvailable() > > (Ia32/DxeLoadFunc.c) > > NULL pointer detection is done by making use of paging mechanism of CPU. > During page table setup, if enabled, the first 4-K page (0-4095) will be > marked as > NOT PRESENT. Any code which unintentionally access memory between > 0-4095 will trigger a Page Fault exception which warns users that there's > potential illegal code in BIOS. > > This also means that legacy code which has to access memory between 0-4095 > should be cautious to temporarily disable this feature before the access and > re- > enable it afterwards; or disalbe this feature at all. > > Cc: Star Zeng > Cc: Eric Dong > Cc: Laszlo Ersek > Cc: Jiewen Yao > Cc: Michael Kinney > Cc: Jordan Justen > Cc: Ayellet Wolman > Suggested-by: Ayellet Wolman > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang > --- > MdeModulePkg/Core/DxeIplPeim/DxeIpl.h| 25 + > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 + > MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 65 > > MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 11 +++- > MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 + > MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 31 --- > 6 files changed, 126 insertions(+), 9 deletions(-) > > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > index 72d2532f50..1654bcd2dc 100644 > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > @@ -240,4 +240,29 @@ Decompress ( >OUT UINTN *OutputSize >); > > +/** > + Clear legacy memory located at the first 4K-page. > + > + This function traverses the whole HOB list to check if memory from 0 to > 4095 > + exists and has not been allocated, and then clear it if so. > + > + @param HoStart The start of HobList passed to DxeCore. > + > +**/ > +VOID > +ClearLegacyMemory ( > + IN VOID *HobStart > + ); > + > +/** > + Return configure status of NULL pointer detection feature > + > + @return TRUE NULL pointer detection feature is enabled > + @return FALSE NULL pointer detection feature is disabled **/ > +BOOLEAN IsNullDetectionEnabled ( > + VOID > + ); > + > #endif > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > index c54afe4aa6..9d0e76a293 100644 > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > @@ -115,6 +115,7 @@ > [Pcd.IA32,Pcd.X64] >gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## > SOMETIMES_CONSUMES >gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask > ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask > ## CONSUMES > > [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] >gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## > SOMETIMES_CONSUMES > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > index 50b5440d15..0a
Re: [edk2] [PATCH v2 5/6] MdeModulePkg/Crc32: Fix possible out of range left shift
Hao: I sent another patch to introduce CacluateCrc32() API in BaseLib. It will update MdeModulePkg Crc32 to depend on BaseLib. And, BaseLib CacluateCrc32() has no such logic. So, I think this patch is not necessary. Thanks Liming >-Original Message- >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Hao >Wu >Sent: Thursday, September 21, 2017 2:46 PM >To: edk2-devel@lists.01.org >Cc: Wu, Hao A ; Paolo Bonzini ; >Dong, Eric ; Zeng, Star >Subject: [edk2] [PATCH v2 5/6] MdeModulePkg/Crc32: Fix possible out of >range left shift > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=697 > >Within function ReverseBits(), left shift operations of 1 in the >following statements: >"(1 << Index)" and "(1 << (31 - Index))" > >will incur possible out of range left shift when Index is either 0 or >31, since "1 << 31" is possible to exceed the range of type 'int' >(signed). > >According to the C11 spec, Section 6.5.7: >> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated >> bits are filled with zeros. If E1 has an unsigned type, the value >> of the result is E1 * 2^E2 , reduced modulo one more than the >> maximum value representable in the result type. If E1 has a signed >> type and nonnegative value, and E1 * 2^E2 is representable in the >> result type, then that is the resulting value; otherwise, the >> behavior is undefined. > >This commit uses '1U' instead of '1' to resolve this issue. > >Cc: Steven Shi >Cc: Star Zeng >Cc: Eric Dong >Cc: Paolo Bonzini >Contributed-under: TianoCore Contribution Agreement 1.1 >Signed-off-by: Hao Wu >--- > MdeModulePkg/Core/RuntimeDxe/Crc32.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > >diff --git a/MdeModulePkg/Core/RuntimeDxe/Crc32.c >b/MdeModulePkg/Core/RuntimeDxe/Crc32.c >index a6fe77fa34..2cd234b562 100644 >--- a/MdeModulePkg/Core/RuntimeDxe/Crc32.c >+++ b/MdeModulePkg/Core/RuntimeDxe/Crc32.c >@@ -7,7 +7,7 @@ > EFI Runtime Services Table are converted from physical address to > virtual addresses. This requires that the 32-bit CRC be recomputed. > >-Copyright (c) 2006 - 2008, Intel Corporation. All rights reserved. >+Copyright (c) 2006 - 2017, 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 >@@ -79,8 +79,8 @@ ReverseBits ( > > NewValue = 0; > for (Index = 0; Index < 32; Index++) { >-if ((Value & (1 << Index)) != 0) { >- NewValue = NewValue | (1 << (31 - Index)); >+if ((Value & (1U << Index)) != 0) { >+ NewValue = NewValue | (1U << (31 - Index)); > } > } > >-- >2.12.0.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
Re: [edk2] [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection
Please see my comments inline below. > -Original Message- > From: Zeng, Star > Sent: Thursday, September 28, 2017 11:24 AM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Laszlo Ersek ; Yao, > Jiewen ; Kinney, Michael D > ; Justen, Jordan L ; > Wolman, Ayellet ; Zeng, Star > Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer > detection > > Minor comments to this patch. > > 1. IsNullDetectionEnabled() function is put in DxeLoad.c that is shared by all > ARCHs, and the function is consuming PcdNullPointerDetectionPropertyMask, > but PcdNullPointerDetectionPropertyMask is only declared in > "[Pcd.IA32,Pcd.X64]" in inf. > I am not sure whether it will cause build failure or not for non-IA32/X64 > ARCHs. > You're right. The PCD should not be under this section. > 2. How about using name "ClearFirst4KPage" instead of "ClearLegacyMemory" > to be more clear? > I prefer a bit more general name in case of potential future change. > > Thanks, > Star > -Original Message- > From: Wang, Jian J > Sent: Thursday, September 28, 2017 9:04 AM > To: edk2-devel@lists.01.org > Cc: Zeng, Star ; Dong, Eric ; Laszlo > Ersek ; Yao, Jiewen ; Kinney, > Michael D ; Justen, Jordan L > ; Wolman, Ayellet > Subject: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer > detection > > > According to Jiewen's feedback, change the page split condition for > > NULL pointer detection to exclude IsExecuteDisableBitAvailable() > > (Ia32/DxeLoadFunc.c) > > NULL pointer detection is done by making use of paging mechanism of CPU. > During page table setup, if enabled, the first 4-K page (0-4095) will be > marked as > NOT PRESENT. Any code which unintentionally access memory between > 0-4095 will trigger a Page Fault exception which warns users that there's > potential illegal code in BIOS. > > This also means that legacy code which has to access memory between 0-4095 > should be cautious to temporarily disable this feature before the access and > re- > enable it afterwards; or disalbe this feature at all. > > Cc: Star Zeng > Cc: Eric Dong > Cc: Laszlo Ersek > Cc: Jiewen Yao > Cc: Michael Kinney > Cc: Jordan Justen > Cc: Ayellet Wolman > Suggested-by: Ayellet Wolman > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang > --- > MdeModulePkg/Core/DxeIplPeim/DxeIpl.h| 25 + > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 + > MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 65 > > MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 11 +++- > MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 + > MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 31 --- > 6 files changed, 126 insertions(+), 9 deletions(-) > > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > index 72d2532f50..1654bcd2dc 100644 > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h > @@ -240,4 +240,29 @@ Decompress ( >OUT UINTN *OutputSize >); > > +/** > + Clear legacy memory located at the first 4K-page. > + > + This function traverses the whole HOB list to check if memory from 0 to > 4095 > + exists and has not been allocated, and then clear it if so. > + > + @param HoStart The start of HobList passed to DxeCore. > + > +**/ > +VOID > +ClearLegacyMemory ( > + IN VOID *HobStart > + ); > + > +/** > + Return configure status of NULL pointer detection feature > + > + @return TRUE NULL pointer detection feature is enabled > + @return FALSE NULL pointer detection feature is disabled **/ > +BOOLEAN IsNullDetectionEnabled ( > + VOID > + ); > + > #endif > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > index c54afe4aa6..9d0e76a293 100644 > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf > @@ -115,6 +115,7 @@ > [Pcd.IA32,Pcd.X64] >gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## > SOMETIMES_CONSUMES >gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask > ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask > ## CONSUMES > > [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] >gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## > SOMETIMES_CONSUMES > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > index 50b5440d15..0a71b1f3de 100644 > --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c > @@ -825,3 +825,68 @@ UpdateStackHob ( > Hob.Raw = GET_NEXT_HOB (Hob); >} > } > + > +/** > + Clear legacy memory located at the first 4K-page, if available. > + > + This function traverses the whole HOB list to check if memory from 0 to > 4095 > + exists and has not been allocated, and then clear it i
Re: [edk2] [PATCH v3 1/6] MdeModulePkg/MdeModulePkg.dec, .uni: Add NULL pointer detection PCD
Reviewed-by: Star Zeng -Original Message- From: Wang, Jian J Sent: Thursday, September 28, 2017 9:04 AM To: edk2-devel@lists.01.org Cc: Wang, Jian J ; Zeng, Star ; Dong, Eric ; Laszlo Ersek ; Yao, Jiewen ; Kinney, Michael D ; Justen, Jordan L ; Wolman, Ayellet Subject: [PATCH v3 1/6] MdeModulePkg/MdeModulePkg.dec,.uni: Add NULL pointer detection PCD From: "Wang, Jian J" > According to Star's feedback, add prompt and help string in uni file PCD PcdNullPointerDetectionPropertyMask is a bitmask used to control the NULL address detection functionality in code for different phases. If enabled, accessing NULL address in UEFI or SMM code can be caught as a page fault exception. BIT0- Enable NULL pointer detection for UEFI. BIT1- Enable NULL pointer detection for SMM. BIT2..6 - Reserved for future uses. BIT7- Disable NULL pointer detection just after EndOfDxe. This is a workaround for those unsolvable NULL access issues in OptionROM, boot loader, etc. It can also help to avoid unnecessary exception caused by legacy memory (0-4095) access after EndOfDxe, such as Windows 7 boot on Qemu. Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Michael Kinney Cc: Jordan Justen Cc: Ayellet Wolman Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/MdeModulePkg.dec | 13 + MdeModulePkg/MdeModulePkg.uni | 13 + 2 files changed, 26 insertions(+) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index a3c0633ee1..9248d10da8 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -867,6 +867,19 @@ # @ValidList 0x8006 | 0x03058002 gEfiMdeModulePkgTokenSpaceGuid.PcdErrorCodeSetVariable|0x03058002|UINT32|0x30001040 + ## Mask to control the NULL address detection in code for different phases. + # If enabled, accessing NULL address in UEFI or SMM code can be caught. + #BIT0- Enable NULL pointer detection for UEFI. + #BIT1- Enable NULL pointer detection for SMM. + #BIT2..6 - Reserved for future uses. + #BIT7- Disable NULL pointer detection just after EndOfDxe. + # This is a workaround for those unsolvable NULL access issues in + # OptionROM, boot loader, etc. It can also help to avoid unnecessary + # exception caused by legacy memory (0-4095) access after EndOfDxe, + # such as Windows 7 boot on Qemu. + # @Prompt Enable NULL address detection. + + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask|0x0 + |UINT8|0x30001050 + [PcdsFixedAtBuild, PcdsPatchableInModule] ## Dynamic type PCD can be registered callback function for Pcd setting action. # PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of callback function diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index d6015de75f..f8b31694ba 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1127,3 +1127,16 @@ "enabled on AMD processors supporting the Secure Encrypted Virtualization (SEV) feature.\n" "This mask should be applied when creating 1:1 virtual to physical mapping tables." +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNullPointerDetectionPropertyMask_PROMPT #language en-US "Enable NULL pointer detection" + +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNullPointerDetectionPropertyMask_HELP #language en-US "Mask to control the NULL address detection in code for different phases.\n" + " If enabled, accessing NULL address in UEFI or SMM code can be caught.\n\n" + " BIT0- Enable NULL pointer detection for UEFI.\n" + " BIT1- Enable NULL pointer detection for SMM.\n" + " BIT2..6 - Reserved for future uses.\n" + " BIT7- Disable NULL pointer detection just after EndOfDxe." + " This is a workaround for those unsolvable NULL access issues in" + " Opt
Re: [edk2] [PATCH v3 3/6] MdeModulePkg/Core/Dxe: Add EndOfDxe workaround
Some comments to this patch. 1. How about using lower TPL TPL_CALLBACK instead of TPL_NOTIFY for the notification? 2. Should GCD SetMemorySpaceCapabilities + SetMemorySpaceAttributes be used instead of gCpu->SetMemoryAttributes()? Thanks, Star -Original Message- From: Wang, Jian J Sent: Thursday, September 28, 2017 9:04 AM To: edk2-devel@lists.01.org Cc: Zeng, Star ; Dong, Eric ; Laszlo Ersek ; Yao, Jiewen ; Kinney, Michael D ; Justen, Jordan L ; Wolman, Ayellet Subject: [PATCH v3 3/6] MdeModulePkg/Core/Dxe: Add EndOfDxe workaround One of issue caused by enabling NULL pointer detection is that some PCI device OptionROM, binary drivers and binary OS boot loaders may have NULL pointer access bugs, which will prevent BIOS from booting and is almost impossible to fix. BIT7 of PCD PcdNullPointerDetectionPropertyMask is used as a workaround to indicate BIOS to disable NULL pointer detection right after event gEfiEndOfDxeEventGroupGuid, and then let boot continue. Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Michael Kinney Cc: Jordan Justen Cc: Ayellet Wolman Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + MdeModulePkg/Core/Dxe/Mem/Page.c | 4 ++- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 48 +++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf index 30d5984f7c..0a161ffd71 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.inf +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf @@ -192,6 +192,7 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask## CONSUMES # [Hob] # RESOURCE_DESCRIPTOR ## CONSUMES diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c index a142c79ee2..0468df3171 100644 --- a/MdeModulePkg/Core/Dxe/Mem/Page.c +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c @@ -188,7 +188,9 @@ CoreAddRange ( // used for other purposes. // if (Type == EfiConventionalMemory && Start == 0 && (End >= EFI_PAGE_SIZE - 1)) { -SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0); +if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0) { + SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0); +} } // diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index a73c4ccd64..73e3b269f3 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -995,6 +995,36 @@ MemoryProtectionExitBootServicesCallback ( } } +/** + Disable NULL pointer detection after EndOfDxe. This is a workaround +resort in + order to skip unfixable NULL pointer access issues detected in +OptionROM or + boot loaders. + + @param[in] Event The Event this notify function registered to. + @param[in] Context Pointer to the context data registered to the Event. +**/ +VOID +EFIAPI +DisableNullDetectionAtTheEndOfDxe ( + EFI_EVENT Event, + VOID*Context + ) +{ + EFI_STATUSStatus; + + DEBUG ((DEBUG_INFO, "DisableNullDetectionAtTheEndOfDxe(): + start\r\n")); // // Disable NULL pointer detection by enabling first + 4K page // Status = gCpu->SetMemoryAttributes (gCpu, 0, + EFI_PAGE_SIZE, 0); ASSERT_EFI_ERROR (Status); + + CoreCloseEvent (Event); + DEBUG ((DEBUG_INFO, "DisableNullDetectionAtTheEndOfDxe(): end\r\n")); + + return; +} + /** Initialize Memory Protection support. **/ @@ -1006,6 +1036,7 @@ CoreInitializeMemoryProtection ( { EFI_STATUS Status; EFI_EVENT Event; + EFI_EVENT EndOfDxeEvent; VOID*Registration; mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy); @@ -1044,6 +1075,23 @@ CoreInitializeMemoryProtection ( ); ASSERT_EFI_ERROR(Status); } + + // + // Register a callback to disable NULL pointer detection at EndOfDxe + // if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT0|BIT7)) + == (BIT0|BIT7)) { +Status = CoreCreateEventEx ( +EVT_NOTIFY_SIGNAL, +TPL_NOTIFY, +DisableNullDetectionAtTheEndOfDxe, +NULL, +&gEfiEndOfDxeEventGroupGuid, +&EndOfDxeEvent +); +ASSERT_EFI_ERROR (Status); + } + return ; } -- 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 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection
Another comment. Should IsNullDetectionEnabled() be checked before calling ClearLegacyMemory()? Thanks, Star -Original Message- From: Zeng, Star Sent: Thursday, September 28, 2017 11:24 AM To: Wang, Jian J ; edk2-devel@lists.01.org Cc: Dong, Eric ; Laszlo Ersek ; Yao, Jiewen ; Kinney, Michael D ; Justen, Jordan L ; Wolman, Ayellet ; Zeng, Star Subject: RE: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection Minor comments to this patch. 1. IsNullDetectionEnabled() function is put in DxeLoad.c that is shared by all ARCHs, and the function is consuming PcdNullPointerDetectionPropertyMask, but PcdNullPointerDetectionPropertyMask is only declared in "[Pcd.IA32,Pcd.X64]" in inf. I am not sure whether it will cause build failure or not for non-IA32/X64 ARCHs. 2. How about using name "ClearFirst4KPage" instead of "ClearLegacyMemory" to be more clear? Thanks, Star -Original Message- From: Wang, Jian J Sent: Thursday, September 28, 2017 9:04 AM To: edk2-devel@lists.01.org Cc: Zeng, Star ; Dong, Eric ; Laszlo Ersek ; Yao, Jiewen ; Kinney, Michael D ; Justen, Jordan L ; Wolman, Ayellet Subject: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection > According to Jiewen's feedback, change the page split condition for > NULL pointer detection to exclude IsExecuteDisableBitAvailable() > (Ia32/DxeLoadFunc.c) NULL pointer detection is done by making use of paging mechanism of CPU. During page table setup, if enabled, the first 4-K page (0-4095) will be marked as NOT PRESENT. Any code which unintentionally access memory between 0-4095 will trigger a Page Fault exception which warns users that there's potential illegal code in BIOS. This also means that legacy code which has to access memory between 0-4095 should be cautious to temporarily disable this feature before the access and re-enable it afterwards; or disalbe this feature at all. Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Michael Kinney Cc: Jordan Justen Cc: Ayellet Wolman Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/DxeIplPeim/DxeIpl.h| 25 + MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 + MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 65 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 11 +++- MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 + MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 31 --- 6 files changed, 126 insertions(+), 9 deletions(-) diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h index 72d2532f50..1654bcd2dc 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h @@ -240,4 +240,29 @@ Decompress ( OUT UINTN *OutputSize ); +/** + Clear legacy memory located at the first 4K-page. + + This function traverses the whole HOB list to check if memory from 0 to 4095 + exists and has not been allocated, and then clear it if so. + + @param HoStart The start of HobList passed to DxeCore. + +**/ +VOID +ClearLegacyMemory ( + IN VOID *HobStart + ); + +/** + Return configure status of NULL pointer detection feature + + @return TRUE NULL pointer detection feature is enabled + @return FALSE NULL pointer detection feature is disabled **/ +BOOLEAN IsNullDetectionEnabled ( + VOID + ); + #endif diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf index c54afe4aa6..9d0e76a293 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf @@ -115,6 +115,7 @@ [Pcd.IA32,Pcd.X64] gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## SOMETIMES_CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask## CONSUMES [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c index 50b5440d15..0a71b1f3de 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c @@ -825,3 +825,68 @@ UpdateStackHob ( Hob.Raw = GET_NEXT_HOB (Hob); } } + +/** + Clear legacy memory located at the first 4K-page, if available. + + This function traverses the whole HOB list to check if memory from 0 to 4095 + exists and has not been allocated, and then clear it if so. + + @param HoStart The start of HobList passed to DxeCore. + +**/ +VOID +ClearLegacyMemory ( + IN VOID *HobStart + ) +{ + EFI_PEI_HOB_POINTERS RscHob; + EFI_PEI_HOB_POINTERS MemHob; + BOOLEAN DoClear; + + RscHob.Raw = HobStart; + MemHo
Re: [edk2] [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection
Minor comments to this patch. 1. IsNullDetectionEnabled() function is put in DxeLoad.c that is shared by all ARCHs, and the function is consuming PcdNullPointerDetectionPropertyMask, but PcdNullPointerDetectionPropertyMask is only declared in "[Pcd.IA32,Pcd.X64]" in inf. I am not sure whether it will cause build failure or not for non-IA32/X64 ARCHs. 2. How about using name "ClearFirst4KPage" instead of "ClearLegacyMemory" to be more clear? Thanks, Star -Original Message- From: Wang, Jian J Sent: Thursday, September 28, 2017 9:04 AM To: edk2-devel@lists.01.org Cc: Zeng, Star ; Dong, Eric ; Laszlo Ersek ; Yao, Jiewen ; Kinney, Michael D ; Justen, Jordan L ; Wolman, Ayellet Subject: [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection > According to Jiewen's feedback, change the page split condition for > NULL pointer detection to exclude IsExecuteDisableBitAvailable() > (Ia32/DxeLoadFunc.c) NULL pointer detection is done by making use of paging mechanism of CPU. During page table setup, if enabled, the first 4-K page (0-4095) will be marked as NOT PRESENT. Any code which unintentionally access memory between 0-4095 will trigger a Page Fault exception which warns users that there's potential illegal code in BIOS. This also means that legacy code which has to access memory between 0-4095 should be cautious to temporarily disable this feature before the access and re-enable it afterwards; or disalbe this feature at all. Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Michael Kinney Cc: Jordan Justen Cc: Ayellet Wolman Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/DxeIplPeim/DxeIpl.h| 25 + MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 + MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 65 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 11 +++- MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 + MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 31 --- 6 files changed, 126 insertions(+), 9 deletions(-) diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h index 72d2532f50..1654bcd2dc 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h @@ -240,4 +240,29 @@ Decompress ( OUT UINTN *OutputSize ); +/** + Clear legacy memory located at the first 4K-page. + + This function traverses the whole HOB list to check if memory from 0 to 4095 + exists and has not been allocated, and then clear it if so. + + @param HoStart The start of HobList passed to DxeCore. + +**/ +VOID +ClearLegacyMemory ( + IN VOID *HobStart + ); + +/** + Return configure status of NULL pointer detection feature + + @return TRUE NULL pointer detection feature is enabled + @return FALSE NULL pointer detection feature is disabled **/ +BOOLEAN IsNullDetectionEnabled ( + VOID + ); + #endif diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf index c54afe4aa6..9d0e76a293 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf @@ -115,6 +115,7 @@ [Pcd.IA32,Pcd.X64] gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## SOMETIMES_CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask## CONSUMES [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c index 50b5440d15..0a71b1f3de 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c @@ -825,3 +825,68 @@ UpdateStackHob ( Hob.Raw = GET_NEXT_HOB (Hob); } } + +/** + Clear legacy memory located at the first 4K-page, if available. + + This function traverses the whole HOB list to check if memory from 0 to 4095 + exists and has not been allocated, and then clear it if so. + + @param HoStart The start of HobList passed to DxeCore. + +**/ +VOID +ClearLegacyMemory ( + IN VOID *HobStart + ) +{ + EFI_PEI_HOB_POINTERS RscHob; + EFI_PEI_HOB_POINTERS MemHob; + BOOLEAN DoClear; + + RscHob.Raw = HobStart; + MemHob.Raw = HobStart; + DoClear = FALSE; + + // + // Check if page 0 exists and free + // + while ((RscHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, + RscHob.Raw)) != NULL) { +if (RscHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY && +RscHob.ResourceDescriptor->PhysicalStart == 0) { + DoClear = TRUE; + // + // Make sure memory at 0-4095 has not been
[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] Update Minor Version of BIOS ID.
Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: zwei4 --- Platform/BroxtonPlatformPkg/BiosId.env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Platform/BroxtonPlatformPkg/BiosId.env b/Platform/BroxtonPlatformPkg/BiosId.env index 8de9eb87f..13b7d3be9 100644 --- a/Platform/BroxtonPlatformPkg/BiosId.env +++ b/Platform/BroxtonPlatformPkg/BiosId.env @@ -31,5 +31,5 @@ BOARD_ID = APLKRVP BOARD_REV = 3 BUILD_TYPE= D VERSION_MAJOR = 0066 -VERSION_MINOR = 02 +VERSION_MINOR = 03 BOARD_EXT = X64 -- 2.14.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [patch] ShellPkg/Dh: Refine variable naming style
Avoid using only lower-case characters for variable name. Cc: Ruiyu Ni Cc: Jaben Carsey Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Dandan Bi --- ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c b/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c index 7d06163..a7bd251 100644 --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c @@ -284,11 +284,11 @@ GetProtocolInfoString( EFI_STATUSStatus; CHAR16*RetVal; UINTN Size; CHAR16*Temp; CHAR16GuidStr[40]; - VOID *instance; + VOID *Instance; CHAR16InstanceStr[17]; ProtocolGuidArray = NULL; RetVal= NULL; Size = 0; @@ -314,14 +314,14 @@ GetProtocolInfoString( FreePool(Temp); } StrnCatGrow(&RetVal, &Size, L"%N", 0); if(Verbose) { -Status = gBS->HandleProtocol (TheHandle, ProtocolGuidArray[ProtocolIndex], &instance); +Status = gBS->HandleProtocol (TheHandle, ProtocolGuidArray[ProtocolIndex], &Instance); if (!EFI_ERROR (Status)) { StrnCatGrow (&RetVal, &Size, L"(%H", 0); - UnicodeSPrint (InstanceStr, sizeof (InstanceStr), L"%x", instance); + UnicodeSPrint (InstanceStr, sizeof (InstanceStr), L"%x", Instance); StrnCatGrow (&RetVal, &Size, InstanceStr, 0); StrnCatGrow (&RetVal, &Size, L"%N)", 0); } } -- 1.9.5.msysgit.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] [edk2-platforms/devel-MinnowBoard3-UDK2017] Enable SueCreek
Yeonsil: Please send the patch by git send-email way. We don't use attachment for patch review. Thanks Liming > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wei, > David > Sent: Thursday, September 28, 2017 9:50 AM > To: Yoon, Yeon Sil ; 'edk2-devel@lists.01.org' > > Cc: Rytkonen, Teemu S ; Loeppert, Anthony > > Subject: [edk2] [PATCH] [edk2-platforms/devel-MinnowBoard3-UDK2017] Enable > SueCreek > > Hi Yeon Sil, > > Only one comment: We need to add some code in _STA to make sure that this > device is only reported to OS when it is actually present > on the board. We will make some change to your patch as reference. > > Thanks, > David Wei > > Intel SSG/STO/UEFI BIOS > > From: Yoon, Yeon Sil > Sent: Thursday, September 28, 2017 6:32 AM > To: 'edk2-devel@lists.01.org' > Cc: Rytkonen, Teemu S ; Loeppert, Anthony > ; Wei, David > > Subject: Review request for patch to enable SueCreek on Benson Glacier > > Hello, > > This is a patch to enable SueCreek module on Benson Glacier h/w. > > Can you please review it? > > Thanks! > > Regards, > Yeonsil > ___ > 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
Re: [edk2] [PATCH] MdeModulePkg/PciBusDxe: Enable Bus Master on P2P bridges on demand
Reviewed-by: jiewen@intel.com > -Original Message- > From: Ni, Ruiyu > Sent: Friday, August 25, 2017 3:43 PM > To: edk2-devel@lists.01.org > Cc: Ruiyu Ni ; Sean Brogan > ; Yao, Jiewen ; Kinney, > Michael D > Subject: [PATCH] MdeModulePkg/PciBusDxe: Enable Bus Master on P2P bridges > on demand > > From: Ruiyu Ni > > The patch dynamically enables Bus Master on P2P bridges only > when requested by a device driver through PciIo.Attribute() to enable > the Bus Master. > > Signed-off-by: Sean Brogan > Signed-off-by: Ruiyu Ni > Cc: Jiewen Yao > Cc: Michael D Kinney > --- > MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 16 > +--- > MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 18 > +++--- > MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c| 8 > 3 files changed, 32 insertions(+), 10 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c > index c0227fa2b6..359b9ded6d 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c > @@ -1,7 +1,7 @@ > /** @file >Supporting functions implementaion for PCI devices management. > > -Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved. > +Copyright (c) 2006 - 2017, 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 > @@ -711,7 +711,12 @@ StartPciDevicesOnBridge ( > 0, > &Supports > ); > -Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE; > +// > +// By default every bridge's IO and MMIO spaces are enabled. > +// Bridge's Bus Master will be enabled when any device behind it > requests > +// to enable Bus Master. > +// > +Supports &= (UINT64) (EFI_PCI_IO_ATTRIBUTE_IO | > EFI_PCI_IO_ATTRIBUTE_MEMORY); > PciIoDevice->PciIo.Attributes ( > &(PciIoDevice->PciIo), > EfiPciIoAttributeOperationEnable, > @@ -763,7 +768,12 @@ StartPciDevicesOnBridge ( > 0, > &Supports > ); > -Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE; > +// > +// By default every bridge's IO and MMIO spaces are enabled. > +// Bridge's Bus Master will be enabled when any device behind it > requests > +// to enable Bus Master. > +// > +Supports &= (UINT64) (EFI_PCI_IO_ATTRIBUTE_IO | > EFI_PCI_IO_ATTRIBUTE_MEMORY); > PciIoDevice->PciIo.Attributes ( > &(PciIoDevice->PciIo), > EfiPciIoAttributeOperationEnable, > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > index 81171c82d9..f73756a31e 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > @@ -1218,11 +1218,12 @@ DetermineDeviceAttribute ( >return Status; > } > // > -// Assume the PCI Root Bridge supports DAC > +// Assume the PCI Root Bridge supports DAC and Bus Master. > // > PciIoDevice->Supports |= > (UINT64)(EFI_PCI_IO_ATTRIBUTE_EMBEDDED_DEVICE | >EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM | > - > EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE); > + > EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE | > + EFI_PCI_IO_ATTRIBUTE_BUS_MASTER); > >} else { > > @@ -1233,9 +1234,16 @@ DetermineDeviceAttribute ( > // > Command = EFI_PCI_COMMAND_IO_SPACE | >EFI_PCI_COMMAND_MEMORY_SPACE | > - EFI_PCI_COMMAND_BUS_MASTER | >EFI_PCI_COMMAND_VGA_PALETTE_SNOOP; > > +// > +// Per PCI-to-PCI Bridge Architecture all PCI-to-PCI bridges are Bus > Master > capable. > +// So only test the Bus Master capability for PCI devices. > +// > +if (!IS_PCI_BRIDGE(&PciIoDevice->Pci)) { > + Command |= EFI_PCI_COMMAND_BUS_MASTER; > +} > + > BridgeControl = EFI_PCI_BRIDGE_CONTROL_ISA | > EFI_PCI_BRIDGE_CONTROL_VGA | EFI_PCI_BRIDGE_CONTROL_VGA_16; > > // > @@ -1245,7 +1253,11 @@ DetermineDeviceAttribute ( > > // > // Set the supported attributes for specified PCI device > +// Per PCI-to-PCI Bridge Architecture all PCI-to-PCI bridges are Bus > Master > capable. > // > +if (IS_PCI_BRIDGE(&PciIoDevice->Pci)) { > + Command |= EFI_PCI_COMMAND_BUS_MASTER; > +} > PciSetDeviceAttribute (PciIoDevice, Command, BridgeControl, > EFI_SET_SUPPORTS); > > // > diff --git a/MdeModu
[edk2] [PATCH] UefiCpuPkg/SmmCpuFeaturesLib: replace hard-coded machine code
Replace hard-coded machine code with equivalent assembly source code. Changes tested by checking for machine code equivalence by disassembling the original and changed code. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chris Ruffin Cc: Jiewen Yao Cc: Michael D Kinney --- .../Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm | 24 ++ .../SmmCpuFeaturesLib/Ia32/SmiException.nasm | 5 ++-- .../Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm| 29 -- .../SmmCpuFeaturesLib/X64/SmiException.nasm| 5 ++-- 4 files changed, 34 insertions(+), 29 deletions(-) diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm index b1c84a494f..00c0f0672c 100644 --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiEntry.nasm @@ -1,5 +1,5 @@ ;-- ; -; Copyright (c) 2016, Intel Corporation. All rights reserved. +; Copyright (c) 2016 - 2017, 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 @@ -51,6 +51,11 @@ global ASM_PFX(gStmSmbase) global ASM_PFX(gStmXdSupported) extern ASM_PFX(gStmSmiHandlerIdtr) +ASM_PFX(gStmSmiCr3) EQU StmSmiCr3Patch - 4 +ASM_PFX(gStmSmiStack)EQU StmSmiStackPatch - 4 +ASM_PFX(gStmSmbase) EQU StmSmbasePatch - 4 +ASM_PFX(gStmXdSupported) EQU StmXdSupportedPatch - 1 + SECTION .text BITS 16 @@ -66,8 +71,8 @@ _StmSmiEntryPoint: o32 lgdt[cs:bx] ; lgdt fword ptr cs:[bx] mov ax, PROTECT_MODE_CS mov [cs:bx-0x2],ax -DB 0x66, 0xbf ; mov edi, SMBASE -ASM_PFX(gStmSmbase): DD 0 +o32 mov edi, strict dword 0 +StmSmbasePatch: lea eax, [edi + (@32bit - _StmSmiEntryPoint) + 0x8000] mov [cs:bx-0x6],eax mov ebx, cr0 @@ -87,15 +92,15 @@ o16 mov es, ax o16 mov fs, ax o16 mov gs, ax o16 mov ss, ax -DB 0xbc ; mov esp, imm32 -ASM_PFX(gStmSmiStack): DD 0 +mov esp, strict dword 0 +StmSmiStackPatch: mov eax, ASM_PFX(gStmSmiHandlerIdtr) lidt[eax] jmp ProtFlatMode ProtFlatMode: -DB 0xb8; mov eax, imm32 -ASM_PFX(gStmSmiCr3): DD 0 +mov eax, strict dword 0 +StmSmiCr3Patch: mov cr3, eax ; ; Need to test for CR4 specific bit support @@ -134,8 +139,8 @@ ASM_PFX(gStmSmiCr3): DD 0 .6: ; enable NXE if supported -DB 0b0h; mov al, imm8 -ASM_PFX(gStmXdSupported): DB 1 +mov al, strict byte 1 +StmXdSupportedPatch: cmp al, 0 jz @SkipXd ; @@ -268,4 +273,3 @@ _StmSmiHandler: ASM_PFX(gcStmSmiHandlerSize) : DW$ - _StmSmiEntryPoint ASM_PFX(gcStmSmiHandlerOffset) : DW_StmSmiHandler - _StmSmiEntryPoint - diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiException.nasm b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiException.nasm index 0ce8501ba9..93dc3005b7 100644 --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiException.nasm +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmiException.nasm @@ -1,5 +1,5 @@ ;-- ; -; Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved. +; Copyright (c) 2009 - 2017, 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 @@ -87,7 +87,7 @@ ASM_PFX(OnException): mov ebx, eax mov eax, 4 -DB 0x0f, 0x01, 0x0c1 ; VMCALL +vmcall jmp $ global ASM_PFX(OnStmSetup) @@ -173,4 +173,3 @@ ASM_PFX(OnStmTeardown): .72: rsm - diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm index c801591fc7..bcac643e96 100644 --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmiEntry.nasm @@ -1,5 +1,5 @@ ;-- ; -; Copyright (c) 2016, Intel Corporation. All rights reserved. +; Copyright (c) 2016 - 2017, 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 @@ -61,6 +61,11 @@ global ASM_PFX(gcStmSmiHandlerTemplate) global ASM_PFX(gcStmSmiHandlerSize) global ASM_PFX(gcStmSmiH
[edk2] [PATCH] [edk2-platforms/devel-MinnowBoard3-UDK2017] Enable SueCreek
Hi Yeon Sil, Only one comment: We need to add some code in _STA to make sure that this device is only reported to OS when it is actually present on the board. We will make some change to your patch as reference. Thanks, David Wei Intel SSG/STO/UEFI BIOS From: Yoon, Yeon Sil Sent: Thursday, September 28, 2017 6:32 AM To: 'edk2-devel@lists.01.org' Cc: Rytkonen, Teemu S ; Loeppert, Anthony ; Wei, David Subject: Review request for patch to enable SueCreek on Benson Glacier Hello, This is a patch to enable SueCreek module on Benson Glacier h/w. Can you please review it? Thanks! Regards, Yeonsil ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing
QemuVideoDxe driver will link VBE SHIM into page 0. If NULL pointer detection is enabled, this driver will fail to load. NULL pointer detection bypassing code is added to prevent such problem during boot. Please note that Windows 7 will try to access VBE SHIM during boot if it's installed, and then cause boot failure. This can be fixed by setting BIT7 of PcdNullPointerDetectionPropertyMask to disable NULL pointer detection after EndOfDxe. As far as we know, there's no other OSs has such issue. > According to Laszlo, remove the code disabling/enabling the NULL pointer > detection but just ignore the installing if it's enabled Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Michael Kinney Cc: Jordan Justen Cc: Ayellet Wolman Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 1 + OvmfPkg/QemuVideoDxe/VbeShim.c| 14 ++ 2 files changed, 15 insertions(+) diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf index 577e07b0a8..ff68c99e96 100644 --- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf +++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf @@ -77,3 +77,4 @@ [Pcd] gOptionRomPkgTokenSpaceGuid.PcdDriverSupportedEfiVersion gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask diff --git a/OvmfPkg/QemuVideoDxe/VbeShim.c b/OvmfPkg/QemuVideoDxe/VbeShim.c index e45a08e887..8ba5522cde 100644 --- a/OvmfPkg/QemuVideoDxe/VbeShim.c +++ b/OvmfPkg/QemuVideoDxe/VbeShim.c @@ -75,6 +75,20 @@ InstallVbeShim ( UINTNPrinted; VBE_MODE_INFO*VbeModeInfo; + if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT0|BIT7)) == BIT0) { +DEBUG (( + DEBUG_WARN, + "%a: page 0 protected, not installing VBE shim\n", + __FUNCTION__ + )); +DEBUG (( + DEBUG_WARN, + "%a: page 0 protection prevents Windows 7 from booting anyway\n", + __FUNCTION__ + )); +return; + } + Segment0 = 0x0; SegmentC = 0xC; SegmentF = 0xF; -- 2.14.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 5/6] IntelFrameworkModulePkg/Csm: Add code to bypass NULL pointer detection
Legacy has to access interrupt vector, BDA, etc. located in memory between 0-4095. To allow as much code as possible to be monitered by NULL pointer detection, we add code to temporarily disable this feature right before those memory access and enable it again afterwards. > According to Laszlo, get memory attributes before changing them. Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Michael Kinney Cc: Jordan Justen Cc: Ayellet Wolman Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c | 101 ++ .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h | 2 + .../Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf | 2 + .../Csm/LegacyBiosDxe/LegacyBda.c | 4 + .../Csm/LegacyBiosDxe/LegacyBios.c | 152 + .../Csm/LegacyBiosDxe/LegacyBiosDxe.inf| 2 + .../Csm/LegacyBiosDxe/LegacyBiosInterface.h| 18 +++ .../Csm/LegacyBiosDxe/LegacyBootSupport.c | 23 +++- .../Csm/LegacyBiosDxe/LegacyPci.c | 17 ++- IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c | 27 +++- 10 files changed, 338 insertions(+), 10 deletions(-) diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c index 7308523ad8..d2224a20aa 100644 --- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c +++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c @@ -1732,6 +1732,98 @@ CheckKeyboardConnect ( } } +/** + Disable NULL pointer detection +*/ +VOID +DisableNullDetection ( + VOID + ) +{ + EFI_STATUSStatus; + EFI_GCD_MEMORY_SPACE_DESCRIPTOR Desc; + + if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0) { +return; + } + + // + // Check current capabilities and attributes + // + Status = gDS->GetMemorySpaceDescriptor (0, &Desc); + ASSERT_EFI_ERROR (Status); + + // + // Try to add EFI_MEMORY_RP support if necessary + // + if ((Desc.Capabilities & EFI_MEMORY_RP) == 0) { +Desc.Capabilities |= EFI_MEMORY_RP; +Status = gDS->SetMemorySpaceCapabilities (0, EFI_PAGES_TO_SIZE(1), + Desc.Capabilities); +ASSERT_EFI_ERROR (Status); +if (EFI_ERROR (Status)) { + return; +} + } + + // + // Don't bother if EFI_MEMORY_RP is already cleared. + // + if ((Desc.Attributes & EFI_MEMORY_RP) != 0) { +Desc.Attributes &= ~EFI_MEMORY_RP; +Status = gDS->SetMemorySpaceAttributes (0, EFI_PAGES_TO_SIZE(1), +Desc.Attributes); +ASSERT_EFI_ERROR (Status); + } else { +DEBUG ((DEBUG_WARN, "!!! Page 0 is supposed to be disabled !!!\r\n")); + } +} + +/** + Enable NULL pointer detection +*/ +VOID +EnableNullDetection ( + VOID + ) +{ + EFI_STATUSStatus; + EFI_GCD_MEMORY_SPACE_DESCRIPTOR Desc; + + if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0) { +return; + } + + // + // Check current capabilities and attributes + // + Status = gDS->GetMemorySpaceDescriptor (0, &Desc); + ASSERT_EFI_ERROR (Status); + + // + // Try to add EFI_MEMORY_RP support if necessary + // + if ((Desc.Capabilities & EFI_MEMORY_RP) == 0) { +Desc.Capabilities |= EFI_MEMORY_RP; +Status = gDS->SetMemorySpaceCapabilities (0, EFI_PAGES_TO_SIZE(1), + Desc.Capabilities); +ASSERT_EFI_ERROR (Status); +if (EFI_ERROR (Status)) { + return; +} + } + + // + // Don't bother if EFI_MEMORY_RP is already set. + // + if ((Desc.Attributes & EFI_MEMORY_RP) == 0) { +Desc.Attributes |= EFI_MEMORY_RP; +Status = gDS->SetMemorySpaceAttributes (0, EFI_PAGES_TO_SIZE(1), +Desc.Attributes); +ASSERT_EFI_ERROR (Status); + } +} + /** Timer event handler: read a series of key stroke from 8042 and put them into memory key buffer. @@ -1839,6 +1931,11 @@ BiosKeyboardTimerHandler ( // 0 Right Shift pressed + // + // Disable NULL pointer detection temporarily + // + DisableNullDetection (); + // // Clear the CTRL and ALT BDA flag // @@ -1916,6 +2013,10 @@ BiosKeyboardTimerHandler ( KbFlag1 &= ~0x0C; *((UINT8 *) (UINTN) 0x417) = KbFlag1; + // + // Restore NULL pointer detection + // + EnableNullDetection (); // // Output EFI input key and shift/toggle state diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h index 0bf28ea140..c64ec0095e 100644 --- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h +++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h @@ -18,6 +18,7 @@ WITHOUT WARRANTIES OR R
[edk2] [PATCH v3 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Implement NULL pointer detection for SMM code
The mechanism behind is the same as NULL pointer detection enabled in EDK-II core. SMM has its own page table and we have to disable page 0 again in SMM mode. Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Michael Kinney Cc: Jordan Justen Cc: Ayellet Wolman Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 12 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c| 25 - UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 + UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 12 4 files changed, 49 insertions(+), 1 deletion(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c index f295c2ebf2..1c9e239a34 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c @@ -155,6 +155,18 @@ SmiPFHandler ( } } + // + // If NULL pointer was just accessed + // + if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0 && + (PFAddress < EFI_PAGE_SIZE)) { +DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n")); +DEBUG_CODE ( + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Rip); +); +CpuDeadLoop (); + } + if (FeaturePcdGet (PcdCpuSmmProfileEnable)) { SmmProfilePFHandler ( SystemContext.SystemContextIa32->Eip, diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c index f086b97c30..ed2afadb21 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c @@ -855,10 +855,10 @@ Gen4GPageTable ( Pte[Index] = (Index << 21) | mAddressEncMask | IA32_PG_PS | PAGE_ATTRIBUTE_BITS; } + Pdpte = (UINT64*)PageTable; if (FeaturePcdGet (PcdCpuSmmStackGuard)) { Pages = (UINTN)PageTable + EFI_PAGES_TO_SIZE (5); GuardPage = mSmmStackArrayBase + EFI_PAGE_SIZE; -Pdpte = (UINT64*)PageTable; for (PageIndex = Low2MBoundary; PageIndex <= High2MBoundary; PageIndex += SIZE_2MB) { Pte = (UINT64*)(UINTN)(Pdpte[BitFieldRead32 ((UINT32)PageIndex, 30, 31)] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 1)); Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] = (UINT64)Pages | mAddressEncMask | PAGE_ATTRIBUTE_BITS; @@ -886,6 +886,29 @@ Gen4GPageTable ( } } + if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0) { +Pte = (UINT64*)(UINTN)(Pdpte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 1)); +if ((Pte[0] & IA32_PG_PS) == 0) { + // 4K-page entries are already mapped. Just hide the first one anyway. + Pte = (UINT64*)(UINTN)(Pte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 1)); + Pte[0] &= ~1; // Hide page 0 +} else { + // Create 4K-page entries + Pages = (UINTN)AllocatePageTableMemory (1); + ASSERT (Pages != 0); + + Pte[0] = (UINT64)(Pages | mAddressEncMask | PAGE_ATTRIBUTE_BITS); + + Pte = (UINT64*)Pages; + PageAddress = 0; + Pte[0] = PageAddress | mAddressEncMask; // Hide page 0 but present left + for (Index = 1; Index < EFI_PAGE_SIZE / sizeof (*Pte); Index++) { +PageAddress += EFI_PAGE_SIZE; +Pte[Index] = PageAddress | mAddressEncMask | PAGE_ATTRIBUTE_BITS; + } +} + } + return (UINT32)(UINTN)PageTable; } diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf index 099792e6ce..31cb215342 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf @@ -159,6 +159,7 @@ gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask## CONSUMES [Depex] gEfiMpServiceProtocolGuid diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c index 3dde80f9ba..f3791ce897 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c @@ -872,6 +872,18 @@ SmiPFHandler ( } } + // + // If NULL pointer was just accessed + // + if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0 && + (PFAddress < EFI_PAGE_SIZE)) { +DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n")); +DEBUG_CODE ( + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip); +); +CpuDeadLoop (); + } + if (FeaturePcdGet (PcdCpuSmmProfileEnable)) { SmmProfilePFHandler ( SystemContext.SystemContextX64->Rip, -- 2.14.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 1/6] MdeModulePkg/MdeModulePkg.dec, .uni: Add NULL pointer detection PCD
From: "Wang, Jian J" > According to Star's feedback, add prompt and help string in uni file PCD PcdNullPointerDetectionPropertyMask is a bitmask used to control the NULL address detection functionality in code for different phases. If enabled, accessing NULL address in UEFI or SMM code can be caught as a page fault exception. BIT0- Enable NULL pointer detection for UEFI. BIT1- Enable NULL pointer detection for SMM. BIT2..6 - Reserved for future uses. BIT7- Disable NULL pointer detection just after EndOfDxe. This is a workaround for those unsolvable NULL access issues in OptionROM, boot loader, etc. It can also help to avoid unnecessary exception caused by legacy memory (0-4095) access after EndOfDxe, such as Windows 7 boot on Qemu. Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Michael Kinney Cc: Jordan Justen Cc: Ayellet Wolman Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/MdeModulePkg.dec | 13 + MdeModulePkg/MdeModulePkg.uni | 13 + 2 files changed, 26 insertions(+) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index a3c0633ee1..9248d10da8 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -867,6 +867,19 @@ # @ValidList 0x8006 | 0x03058002 gEfiMdeModulePkgTokenSpaceGuid.PcdErrorCodeSetVariable|0x03058002|UINT32|0x30001040 + ## Mask to control the NULL address detection in code for different phases. + # If enabled, accessing NULL address in UEFI or SMM code can be caught. + #BIT0- Enable NULL pointer detection for UEFI. + #BIT1- Enable NULL pointer detection for SMM. + #BIT2..6 - Reserved for future uses. + #BIT7- Disable NULL pointer detection just after EndOfDxe. + # This is a workaround for those unsolvable NULL access issues in + # OptionROM, boot loader, etc. It can also help to avoid unnecessary + # exception caused by legacy memory (0-4095) access after EndOfDxe, + # such as Windows 7 boot on Qemu. + # @Prompt Enable NULL address detection. + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask|0x0|UINT8|0x30001050 + [PcdsFixedAtBuild, PcdsPatchableInModule] ## Dynamic type PCD can be registered callback function for Pcd setting action. # PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of callback function diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index d6015de75f..f8b31694ba 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1127,3 +1127,16 @@ "enabled on AMD processors supporting the Secure Encrypted Virtualization (SEV) feature.\n" "This mask should be applied when creating 1:1 virtual to physical mapping tables." +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNullPointerDetectionPropertyMask_PROMPT #language en-US "Enable NULL pointer detection" + +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNullPointerDetectionPropertyMask_HELP #language en-US "Mask to control the NULL address detection in code for different phases.\n" + " If enabled, accessing NULL address in UEFI or SMM code can be caught.\n\n" + " BIT0- Enable NULL pointer detection for UEFI.\n" + " BIT1- Enable NULL pointer detection for SMM.\n" + " BIT2..6 - Reserved for future uses.\n" + " BIT7- Disable NULL pointer detection just after EndOfDxe." + " This is a workaround for those unsolvable NULL access issues in" + " OptionROM, boot loader, etc. It can also help to avoid unnecessary" + " exception caused by legacy memory (0-4095) access after EndOfDxe," + " such as Windows 7 boot on Qemu.
[edk2] [PATCH v3 0/6] Add NULL pointer detection feature
The mechanism behind is to trigger a page fault exception at address 0. This can be made by disabling page 0 (0-4095) during page table setup. So this feature can only be available on platform with paging enabled. Once this feature is enabled, any code, like CSM, which has to access memory in page 0 needs to enable this page temporarily in advance and disable it afterwards. PcdNullPointerDetectionPropertyMask is used to control and elaborate the use cases. For example, BIT7 of this PCD must be set for Windows 7 boot on Qemu if BIT0 set; or boot will fail. Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Michael Kinney Cc: Jordan Justen Cc: Ayellet Wolman Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang Jian J Wang (5): MdeModulePkg/DxeIpl: Implement NULL pointer detection MdeModulePkg/Core/Dxe: Add EndOfDxe workaround for NULL pointer detection UefiCpuPkg/PiSmmCpuDxeSmm: Implement NULL pointer detection for SMM code IntelFrameworkModulePkg/Csm: Add code to bypass NULL pointer detection OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing Wang, Jian J (1): MdeModulePkg/MdeModulePkg.dec,.uni: Add NULL pointer detection PCD .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c | 101 ++ .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h | 2 + .../Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf | 2 + .../Csm/LegacyBiosDxe/LegacyBda.c | 4 + .../Csm/LegacyBiosDxe/LegacyBios.c | 152 + .../Csm/LegacyBiosDxe/LegacyBiosDxe.inf| 2 + .../Csm/LegacyBiosDxe/LegacyBiosInterface.h| 18 +++ .../Csm/LegacyBiosDxe/LegacyBootSupport.c | 23 +++- .../Csm/LegacyBiosDxe/LegacyPci.c | 17 ++- IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c | 27 +++- MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + MdeModulePkg/Core/Dxe/Mem/Page.c | 4 +- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 48 +++ MdeModulePkg/Core/DxeIplPeim/DxeIpl.h | 25 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf| 1 + MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 65 + MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c| 11 +- MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 + MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 31 - MdeModulePkg/MdeModulePkg.dec | 13 ++ MdeModulePkg/MdeModulePkg.uni | 13 ++ OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 1 + OvmfPkg/QemuVideoDxe/VbeShim.c | 14 ++ UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 12 ++ UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 25 +++- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 + UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c| 12 ++ 27 files changed, 606 insertions(+), 21 deletions(-) -- 2.14.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection
> According to Jiewen's feedback, change the page split condition > for NULL pointer detection to exclude IsExecuteDisableBitAvailable() > (Ia32/DxeLoadFunc.c) NULL pointer detection is done by making use of paging mechanism of CPU. During page table setup, if enabled, the first 4-K page (0-4095) will be marked as NOT PRESENT. Any code which unintentionally access memory between 0-4095 will trigger a Page Fault exception which warns users that there's potential illegal code in BIOS. This also means that legacy code which has to access memory between 0-4095 should be cautious to temporarily disable this feature before the access and re-enable it afterwards; or disalbe this feature at all. Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Michael Kinney Cc: Jordan Justen Cc: Ayellet Wolman Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/DxeIplPeim/DxeIpl.h| 25 + MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 + MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 65 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 11 +++- MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 + MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 31 --- 6 files changed, 126 insertions(+), 9 deletions(-) diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h index 72d2532f50..1654bcd2dc 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h @@ -240,4 +240,29 @@ Decompress ( OUT UINTN *OutputSize ); +/** + Clear legacy memory located at the first 4K-page. + + This function traverses the whole HOB list to check if memory from 0 to 4095 + exists and has not been allocated, and then clear it if so. + + @param HoStart The start of HobList passed to DxeCore. + +**/ +VOID +ClearLegacyMemory ( + IN VOID *HobStart + ); + +/** + Return configure status of NULL pointer detection feature + + @return TRUE NULL pointer detection feature is enabled + @return FALSE NULL pointer detection feature is disabled +**/ +BOOLEAN +IsNullDetectionEnabled ( + VOID + ); + #endif diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf index c54afe4aa6..9d0e76a293 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf @@ -115,6 +115,7 @@ [Pcd.IA32,Pcd.X64] gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## SOMETIMES_CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask## CONSUMES [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c index 50b5440d15..0a71b1f3de 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c @@ -825,3 +825,68 @@ UpdateStackHob ( Hob.Raw = GET_NEXT_HOB (Hob); } } + +/** + Clear legacy memory located at the first 4K-page, if available. + + This function traverses the whole HOB list to check if memory from 0 to 4095 + exists and has not been allocated, and then clear it if so. + + @param HoStart The start of HobList passed to DxeCore. + +**/ +VOID +ClearLegacyMemory ( + IN VOID *HobStart + ) +{ + EFI_PEI_HOB_POINTERS RscHob; + EFI_PEI_HOB_POINTERS MemHob; + BOOLEAN DoClear; + + RscHob.Raw = HobStart; + MemHob.Raw = HobStart; + DoClear = FALSE; + + // + // Check if page 0 exists and free + // + while ((RscHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, + RscHob.Raw)) != NULL) { +if (RscHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY && +RscHob.ResourceDescriptor->PhysicalStart == 0) { + DoClear = TRUE; + // + // Make sure memory at 0-4095 has not been allocated. + // + while ((MemHob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, + MemHob.Raw)) != NULL) { +if (MemHob.MemoryAllocation->AllocDescriptor.MemoryBaseAddress +< EFI_PAGE_SIZE) { + DoClear = FALSE; + break; +} +MemHob.Raw = GET_NEXT_HOB (MemHob); + } + break; +} +RscHob.Raw = GET_NEXT_HOB (RscHob); + } + + if (DoClear) { +DEBUG ((DEBUG_INFO, "Clearing first 4K-page!\r\n")); +SetMem (NULL, EFI_PAGE_SIZE, 0); + } + + return; +} + +BOOLEAN +IsNullDetectionEnabled ( + VOID + ) +{ + return (((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) != 0) ? + TRUE : FALSE); +} + diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/Dxe
[edk2] [PATCH v3 3/6] MdeModulePkg/Core/Dxe: Add EndOfDxe workaround
One of issue caused by enabling NULL pointer detection is that some PCI device OptionROM, binary drivers and binary OS boot loaders may have NULL pointer access bugs, which will prevent BIOS from booting and is almost impossible to fix. BIT7 of PCD PcdNullPointerDetectionPropertyMask is used as a workaround to indicate BIOS to disable NULL pointer detection right after event gEfiEndOfDxeEventGroupGuid, and then let boot continue. Cc: Star Zeng Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Michael Kinney Cc: Jordan Justen Cc: Ayellet Wolman Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/DxeMain.inf | 1 + MdeModulePkg/Core/Dxe/Mem/Page.c | 4 ++- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 48 +++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf index 30d5984f7c..0a161ffd71 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.inf +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf @@ -192,6 +192,7 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask## CONSUMES # [Hob] # RESOURCE_DESCRIPTOR ## CONSUMES diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c index a142c79ee2..0468df3171 100644 --- a/MdeModulePkg/Core/Dxe/Mem/Page.c +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c @@ -188,7 +188,9 @@ CoreAddRange ( // used for other purposes. // if (Type == EfiConventionalMemory && Start == 0 && (End >= EFI_PAGE_SIZE - 1)) { -SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0); +if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0) { + SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0); +} } // diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index a73c4ccd64..73e3b269f3 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -995,6 +995,36 @@ MemoryProtectionExitBootServicesCallback ( } } +/** + Disable NULL pointer detection after EndOfDxe. This is a workaround resort in + order to skip unfixable NULL pointer access issues detected in OptionROM or + boot loaders. + + @param[in] Event The Event this notify function registered to. + @param[in] Context Pointer to the context data registered to the Event. +**/ +VOID +EFIAPI +DisableNullDetectionAtTheEndOfDxe ( + EFI_EVENT Event, + VOID*Context + ) +{ + EFI_STATUSStatus; + + DEBUG ((DEBUG_INFO, "DisableNullDetectionAtTheEndOfDxe(): start\r\n")); + // + // Disable NULL pointer detection by enabling first 4K page + // + Status = gCpu->SetMemoryAttributes (gCpu, 0, EFI_PAGE_SIZE, 0); + ASSERT_EFI_ERROR (Status); + + CoreCloseEvent (Event); + DEBUG ((DEBUG_INFO, "DisableNullDetectionAtTheEndOfDxe(): end\r\n")); + + return; +} + /** Initialize Memory Protection support. **/ @@ -1006,6 +1036,7 @@ CoreInitializeMemoryProtection ( { EFI_STATUS Status; EFI_EVENT Event; + EFI_EVENT EndOfDxeEvent; VOID*Registration; mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy); @@ -1044,6 +1075,23 @@ CoreInitializeMemoryProtection ( ); ASSERT_EFI_ERROR(Status); } + + // + // Register a callback to disable NULL pointer detection at EndOfDxe + // + if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT0|BIT7)) + == (BIT0|BIT7)) { +Status = CoreCreateEventEx ( +EVT_NOTIFY_SIGNAL, +TPL_NOTIFY, +DisableNullDetectionAtTheEndOfDxe, +NULL, +&gEfiEndOfDxeEventGroupGuid, +&EndOfDxeEvent +); +ASSERT_EFI_ERROR (Status); + } + return ; } -- 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][edk2-platforms/devel-MinnowBoardMax-UDK2017] Change GCC shell
On 9/27/2017 3:09 AM, Guo, Mang wrote: Change GCC shell from MinimumShell to UefiShell in ShellBinPkg. Thanks. I've been wondering about this now that GCC correctly sets -Os in the build options to reduce the binary size to around the same as Visual C++. -- Rebecca ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v4 00/11] Update D03/D05 binary for edk2 update and bug fix.
Really sorry for that. We made some stupid mistake to cause the patches in a mess. Please ignore them and we will send a new series. Regards, Heyi On 2017/9/28 2:27, Leif Lindholm wrote: Hi Heyi, Apologies for delay, Connect is keeping me busy. First of all, please ensure to generate these patches with --no-binary. Secondly, something has gone wrong with this series: there are binary files being added to edk2-platforms and source code to edk2-non-osi. There is even the addition of Juno binaries. Can you please regenerate, and resend with a separate cover letter for the edk2-non-osi set and the edk2-platfoms set? Best Regards, Leif On Fri, Sep 22, 2017 at 09:58:28PM +0800, Heyi Guo wrote: Code can also be found in github: https://github.com/hisilicon/OpenPlatformPkg.git branch: rp-1710-platforms-v4 Note: Maybe There are some bug in NonDiscoverablePciDeviceDxe. D05/3 console hang at boot, It's boot successfully by switch the VirtualEhciPciIo with old one. Heyi Guo (5): Hisilicon/D03/Net: Update Snp driver Hisilicon/D03/Sas: Add SasPlatform Hisilicon/D05/Net: Update Snp driver Hisilicon/D05/Sas: Add SasPlatform Hisilicon: Fix the drivers use the same GUID issue Leif Lindholm (1): Platforms: add ARM Juno system controller firmware Ming Huang (5): Hisilicon/D03: Update binary file Hisilicon: Update Trusted Firmware binary Hisilicon/D05: Update binary file Hisilicon/D05: Update Trusted Firmware binary Hisilicon/D05: Fix bug 3061 Platform/ARM/Juno/License.txt | 361 Platform/ARM/Juno/bl0.bin | Bin 0 -> 4172 bytes Platform/ARM/Juno/bl30.bin | Bin 0 -> 55204 bytes Platform/Hisilicon/D02/Drivers/Net/SnpPV600Dxe/SnpPV600Dxe.inf | 2 +- Platform/Hisilicon/D02/Drivers/SFC/SfcDxeDriver.inf | 2 +- Platform/Hisilicon/D02/MemoryInitPei/MemoryInitPeim.inf | 2 +- Platform/Hisilicon/D03/Drivers/GetInfoFromBmc/GetInfoFromBmc.efi | Bin 21696 -> 4768 bytes Platform/Hisilicon/D03/Drivers/Ipmi/IpmiInterfacePei/IpmiInterfacePei.efi | Bin 22208 -> 4672 bytes Platform/Hisilicon/D03/Drivers/Ipmi/ipmiInterfaceDxe/IpmiInterfaceDxe.efi | Bin 25440 -> 6784 bytes Platform/Hisilicon/D03/Drivers/IpmiMiscOpDxe/IpmiMiscOp.efi | Bin 23712 -> 4896 bytes Platform/Hisilicon/D03/Drivers/IpmiWatchdogDxe/IpmiWatchdogDxe.efi | Bin 18080 -> 2304 bytes Platform/Hisilicon/D03/Drivers/Net/SnpPV600Dxe/SnpPV600Dxe.efi | Bin 0 -> 26688 bytes Platform/Hisilicon/D03/Drivers/Net/SnpPV600Dxe/SnpPV600Dxe.inf | 24 ++ Platform/Hisilicon/D03/Drivers/Net/SnpPV660Dxe/SnpPV600Dxe.efi | Bin 56832 -> 0 bytes Platform/Hisilicon/D03/Drivers/Net/SnpPV660Dxe/SnpPV600Dxe.inf | 27 -- Platform/Hisilicon/D03/Drivers/Net/SnpPlatform/SnpPlatform.efi | Bin 0 -> 3040 bytes Platform/Hisilicon/D03/Drivers/Net/SnpPlatform/SnpPlatform.inf | 24 ++ Platform/Hisilicon/D03/Drivers/OhciDxe/NativeOhci.efi | Bin 48352 -> 21664 bytes Platform/Hisilicon/D03/Drivers/ReportPciePlugDidVidToBmc/ReportPciePlugDidVidToBmc.efi | Bin 22112 -> 3712 bytes Platform/Hisilicon/D03/Drivers/SFC/SFCDriver.efi | Bin 262144 -> 262144 bytes Platform/Hisilicon/D03/Drivers/Sas/SasDriverDxe.efi | Bin 208288 -> 98144 bytes Platform/Hisilicon/D03/Drivers/SasPlatform/SasPlatform.efi | Bin 0 -> 3040 bytes Platform/Hisilicon/D03/Drivers/SasPlatform/SasPlatform.inf | 24 ++ Platform/Hisilicon/D03/Drivers/Sm750Dxe/SmiGraphicsOutput.efi | Bin 36480 -> 17728 bytes Platform/Hisilicon/D03/Drivers/TransferSmbiosInfo/TransSmbiosInfo.efi | Bin 21408 -> 4000 bytes Platform/Hisilicon/D03/Library/OemAddressMap2P/OemAddressMap2P.lib | Bin 19486 -> 20550 bytes Platform/Hisilicon/D03/MemoryInitPei/MemoryInit.efi | Bin 161280 -> 90272 bytes Platform/Hisilicon/D03/Sec/FVMAIN_SEC.Fv | Bin 262144 -> 262144 bytes Platform/Hisilicon/D03/bl1.bin | Bin 14336 -> 14336 bytes Platform/Hisilicon/D03/fip.bin | Bin 45601 -> 62513 bytes Platform/Hisilicon/D05/Drivers/GetInfoFromBmc/GetInfoFromBmc.efi | Bin 19552 -> 5024 bytes Platform/Hisilicon/D05/
[edk2] [PATCH] Platform/ARM: Reorganize Lcd Graphics Output
From: Girish Pathak This corresponds to the recently submitted edk2 change "ArmPlatformPkg: Reorganize Lcd Graphics Output". This change enables building of a common LcdGraphicsOutputDxe, replacing PL111LcdGraphicsOutputDxe and HdLcdGraphicsOutputDxe. One of the different hardware implementations (PL111Lcd, HdLcd, and MaliDp) is included as a LcdHwLib library. NOTE: The FVP changes include framework for HdLcd and MaliDp builds. These are not part of the supported FVP model, but may be added as customisations. Because the parameters that might be used are not known, some PCD settings are commented out. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Girish Pathak Signed-off-by: Evan Lloyd --- Code can be examined at: https://github.com/EvanLloyd/edk2-platforms/tree/166_gop_v1 Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc| 7 --- Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 5 +++-- Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.fdf| 6 +++--- Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.fdf | 4 ++-- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc b/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc index efa41165e4ad8a16eacc9c707e0d1b5b60e89b1d..cabae1c7610183046220868776d20ae4e6bfa161 100644 --- a/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc +++ b/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.dsc @@ -1,5 +1,5 @@ # -# Copyright (c) 2012-2015, ARM Limited. All rights reserved. +# Copyright (c) 2012-2017, ARM Limited. All rights reserved. # Copyright (c) 2015, Intel Corporation. All rights reserved. # # This program and the accompanying materials @@ -51,6 +51,7 @@ [LibraryClasses.common] ArmGicLib|ArmPkg/Drivers/ArmGic/ArmGicLib.inf LcdPlatformLib|ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf + LcdHwLib|ArmPlatformPkg/Drivers/HdLcd/HdLcd.inf TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf @@ -236,8 +237,8 @@ [Components.common] ArmPkg/Drivers/ArmGic/ArmGicDxe.inf ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf - #ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.inf - ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf + + ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf ArmPkg/Drivers/TimerDxe/TimerDxe.inf ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc index e9f954d926ac25a2abd2f97a4141267927dfc0a3..2f1811cc68a9a4c3d8f71520533f69b8eafef3f6 100644 --- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc +++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc @@ -1,5 +1,5 @@ # -# Copyright (c) 2011-2015, ARM Limited. All rights reserved. +# Copyright (c) 2011-2017, ARM Limited. All rights reserved. # # This program and the accompanying materials # are licensed and made available under the terms and conditions of the BSD License @@ -48,6 +48,7 @@ [LibraryClasses.common] NorFlashPlatformLib|ArmPlatformPkg/ArmVExpressPkg/Library/NorFlashArmVExpressLib/NorFlashArmVExpressLib.inf !ifdef EDK2_ENABLE_PL111 LcdPlatformLib|ArmPlatformPkg/ArmVExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdArmVExpressLib.inf + LcdHwLib|ArmPlatformPkg/Drivers/PL111Lcd/PL111Lcd.inf !endif TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf @@ -269,7 +270,7 @@ [Components.common] ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf ArmPkg/Drivers/TimerDxe/TimerDxe.inf !ifdef EDK2_ENABLE_PL111 - ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.inf + ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf !endif ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.fdf b/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.fdf index cb9a89ef0c7f9930c4e78148c90072e364c4fa2e..9bdd71b21acd0d4bc2a5bf6947b649d7cce57861 100644 --- a/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.fdf +++ b/Platform/ARM/VExpressPkg/ArmVExpress-CTA15-A7.fdf @@ -1,5 +1,5 @@ # -# Copyright (c) 2012-2015, ARM Limited. All rights reserved. +# Copyright (c) 2012-2017, ARM Limited. All rights reserved. # Copyright (c) 2015, Intel Corporation. All rights reserved. # # This program and the accompanying materials @@ -97,8 +97,8 @@ [FV.FvMain] INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf INF ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf - #INF ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.inf - INF ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf + + INF ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf INF ArmPlatformPkg/Driver
[edk2] [PATCH v2 2/2] ArmPkg/Include: Add SVC function IDs for Management Mode.
SVCs are in the range 0xC460 - 0xC47f. The functions available to the secure MM partition: 1. Signal completion of MM event handling. 2. Set/Get memory attributes for a memory region at runtime. 3. Get version number of secure partition manager. Also, it defines memory attributes required for set/get operations. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Supreeth Venkatesh --- ArmPkg/Include/IndustryStandard/ArmMmSvc.h | 43 ++ 1 file changed, 43 insertions(+) create mode 100644 ArmPkg/Include/IndustryStandard/ArmMmSvc.h diff --git a/ArmPkg/Include/IndustryStandard/ArmMmSvc.h b/ArmPkg/Include/IndustryStandard/ArmMmSvc.h new file mode 100644 index 00..4c7b6c3386 --- /dev/null +++ b/ArmPkg/Include/IndustryStandard/ArmMmSvc.h @@ -0,0 +1,43 @@ +/** @file +* +* Copyright (c) 2012-2017, ARM Limited. All rights reserved. +* +* This program and the accompanying materials +* are licensed and made available under the terms and conditions of the BSD License +* which accompanies this distribution. The full text of the license may be found at +* http://opensource.org/licenses/bsd-license.php +* +* THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, +* WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. +* +**/ + +#ifndef __ARM_MM_SVC_H__ +#define __ARM_MM_SVC_H__ + +/* + * SVC IDs to allow the MM secure partition to initialise itself, handle + * delegated events and request the Secure partition manager to perform + * privileged operations on its behalf. + */ +#define ARM_SVC_ID_SPM_VERSION_AARCH64 0xC460 +#define ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64 0xC461 +#define ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64 0xC464 +#define ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64 0xC465 + +#define SET_MEM_ATTR_DATA_PERM_MASK 0x3 +#define SET_MEM_ATTR_DATA_PERM_SHIFT0 +#define SET_MEM_ATTR_DATA_PERM_NO_ACCESS0 +#define SET_MEM_ATTR_DATA_PERM_RW 1 +#define SET_MEM_ATTR_DATA_PERM_RO 3 + +#define SET_MEM_ATTR_CODE_PERM_MASK 0x1 +#define SET_MEM_ATTR_CODE_PERM_SHIFT2 +#define SET_MEM_ATTR_CODE_PERM_X0 +#define SET_MEM_ATTR_CODE_PERM_XN 1 + +#define SET_MEM_ATTR_MAKE_PERM_REQUEST(d_perm, c_perm) \ +c_perm) & SET_MEM_ATTR_CODE_PERM_MASK) << SET_MEM_ATTR_CODE_PERM_SHIFT) | \ +(( (d_perm) & SET_MEM_ATTR_DATA_PERM_MASK) << SET_MEM_ATTR_DATA_PERM_SHIFT)) + +#endif -- 2.14.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 1/2] ArmPkg/Include: Add standard SMC function IDs for MM interface.
This patch adds a list of function IDs that fall under the standard SMC range as defined in http://infocenter.arm.com/help/topic/com.arm.doc.den0060a/DEN0060A_ARM_MM_Interface_Specification.pdf. SMCs associated with Management Mode are in the range 0xC440 - 0xC45f (64 bit) and 0x8440 - 0x845f (32 bit). The function(s) available to the normal world: 1. Request services from the secure MM environment using MM_COMMUNICATE. It also defines MM return codes. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Achin Gupta Signed-off-by: Supreeth Venkatesh --- ArmPkg/Include/IndustryStandard/ArmStdSmc.h | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/ArmPkg/Include/IndustryStandard/ArmStdSmc.h b/ArmPkg/Include/IndustryStandard/ArmStdSmc.h index 593a3ce729..37d0796649 100644 --- a/ArmPkg/Include/IndustryStandard/ArmStdSmc.h +++ b/ArmPkg/Include/IndustryStandard/ArmStdSmc.h @@ -1,6 +1,6 @@ /** @file * -* Copyright (c) 2012-2014, ARM Limited. All rights reserved. +* Copyright (c) 2012-2017, ARM Limited. All rights reserved. * * This program and the accompanying materials * are licensed and made available under the terms and conditions of the BSD License @@ -40,6 +40,24 @@ #define ARM_SMC_STD_REVISION_MAJOR0x0 #define ARM_SMC_STD_REVISION_MINOR0x1 +/* + * Management Mode (MM) calls cover a subset of the Standard Service Call range. + * The list below is not exhaustive. + */ +#define ARM_SMC_ID_MM_VERSION_AARCH32 0x8440 +#define ARM_SMC_ID_MM_VERSION_AARCH64 0xC440 + +// Request service from secure standalone MM environment +#define ARM_SMC_ID_MM_COMMUNICATE_AARCH32 0x8441 +#define ARM_SMC_ID_MM_COMMUNICATE_AARCH64 0xC441 + +/* MM return error codes */ +#define ARM_SMC_MM_RET_SUCCESS 0 +#define ARM_SMC_MM_RET_NOT_SUPPORTED -1 +#define ARM_SMC_MM_RET_INVALID_PARAMS -2 +#define ARM_SMC_MM_RET_DENIED -3 +#define ARM_SMC_MM_RET_NO_MEMORY -4 + /* * Power State Coordination Interface (PSCI) calls cover a subset of the * Standard Service Call range. -- 2.14.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v4 00/11] Update D03/D05 binary for edk2 update and bug fix.
Hi Heyi, Apologies for delay, Connect is keeping me busy. First of all, please ensure to generate these patches with --no-binary. Secondly, something has gone wrong with this series: there are binary files being added to edk2-platforms and source code to edk2-non-osi. There is even the addition of Juno binaries. Can you please regenerate, and resend with a separate cover letter for the edk2-non-osi set and the edk2-platfoms set? Best Regards, Leif On Fri, Sep 22, 2017 at 09:58:28PM +0800, Heyi Guo wrote: > Code can also be found in github: > https://github.com/hisilicon/OpenPlatformPkg.git > branch: rp-1710-platforms-v4 > > Note: Maybe There are some bug in NonDiscoverablePciDeviceDxe. > D05/3 console hang at boot, It's boot successfully by switch the > VirtualEhciPciIo with old one. > > Heyi Guo (5): > Hisilicon/D03/Net: Update Snp driver > Hisilicon/D03/Sas: Add SasPlatform > Hisilicon/D05/Net: Update Snp driver > Hisilicon/D05/Sas: Add SasPlatform > Hisilicon: Fix the drivers use the same GUID issue > > Leif Lindholm (1): > Platforms: add ARM Juno system controller firmware > > Ming Huang (5): > Hisilicon/D03: Update binary file > Hisilicon: Update Trusted Firmware binary > Hisilicon/D05: Update binary file > Hisilicon/D05: Update Trusted Firmware binary > Hisilicon/D05: Fix bug 3061 > > Platform/ARM/Juno/License.txt > | 361 > Platform/ARM/Juno/bl0.bin > | Bin 0 -> 4172 bytes > Platform/ARM/Juno/bl30.bin > | Bin 0 -> 55204 bytes > Platform/Hisilicon/D02/Drivers/Net/SnpPV600Dxe/SnpPV600Dxe.inf > | 2 +- > Platform/Hisilicon/D02/Drivers/SFC/SfcDxeDriver.inf > | 2 +- > Platform/Hisilicon/D02/MemoryInitPei/MemoryInitPeim.inf > | 2 +- > Platform/Hisilicon/D03/Drivers/GetInfoFromBmc/GetInfoFromBmc.efi > | Bin 21696 -> 4768 bytes > Platform/Hisilicon/D03/Drivers/Ipmi/IpmiInterfacePei/IpmiInterfacePei.efi > | Bin 22208 -> 4672 bytes > Platform/Hisilicon/D03/Drivers/Ipmi/ipmiInterfaceDxe/IpmiInterfaceDxe.efi > | Bin 25440 -> 6784 bytes > Platform/Hisilicon/D03/Drivers/IpmiMiscOpDxe/IpmiMiscOp.efi > | Bin 23712 -> 4896 bytes > Platform/Hisilicon/D03/Drivers/IpmiWatchdogDxe/IpmiWatchdogDxe.efi > | Bin 18080 -> 2304 bytes > Platform/Hisilicon/D03/Drivers/Net/SnpPV600Dxe/SnpPV600Dxe.efi > | Bin 0 -> 26688 bytes > Platform/Hisilicon/D03/Drivers/Net/SnpPV600Dxe/SnpPV600Dxe.inf > | 24 ++ > Platform/Hisilicon/D03/Drivers/Net/SnpPV660Dxe/SnpPV600Dxe.efi > | Bin 56832 -> 0 bytes > Platform/Hisilicon/D03/Drivers/Net/SnpPV660Dxe/SnpPV600Dxe.inf > | 27 -- > Platform/Hisilicon/D03/Drivers/Net/SnpPlatform/SnpPlatform.efi > | Bin 0 -> 3040 bytes > Platform/Hisilicon/D03/Drivers/Net/SnpPlatform/SnpPlatform.inf > | 24 ++ > Platform/Hisilicon/D03/Drivers/OhciDxe/NativeOhci.efi > | Bin 48352 -> 21664 bytes > > Platform/Hisilicon/D03/Drivers/ReportPciePlugDidVidToBmc/ReportPciePlugDidVidToBmc.efi > | Bin 22112 -> 3712 bytes > Platform/Hisilicon/D03/Drivers/SFC/SFCDriver.efi > | Bin 262144 -> 262144 bytes > Platform/Hisilicon/D03/Drivers/Sas/SasDriverDxe.efi > | Bin 208288 -> 98144 bytes > Platform/Hisilicon/D03/Drivers/SasPlatform/SasPlatform.efi > | Bin 0 -> 3040 bytes > Platform/Hisilicon/D03/Drivers/SasPlatform/SasPlatform.inf > | 24 ++ > Platform/Hisilicon/D03/Drivers/Sm750Dxe/SmiGraphicsOutput.efi > | Bin 36480 -> 17728 bytes > Platform/Hisilicon/D03/Drivers/TransferSmbiosInfo/TransSmbiosInfo.efi > | Bin 21408 -> 4000 bytes > Platform/Hisilicon/D03/Library/OemAddressMap2P/OemAddressMap2P.lib > | Bin 19486 -> 20550 bytes > Platform/Hisilicon/D03/MemoryInitPei/MemoryInit.efi > | Bin 161280 -> 90272 bytes > Platform/Hisilicon/D03/Sec/FVMAIN_SEC.Fv > | Bin 262144 -> 262144 bytes > Platform/Hisilicon/D03/bl1.bin > | Bin 14336 -> 14336 bytes > Platform/Hisilicon/D03/fip.bin > | Bin 45601 -> 62513 bytes > Platform/Hisilicon/D05/Drivers/GetInfoFromBmc/GetInfoFromBmc.efi > | Bin 19552 -> 5024 bytes > Platform/Hisilicon/D05/Drivers/GetInfoFromBmc/GetInfoFromBmc.inf >
Re: [edk2] [PATCH] ShellPkg/dh: Add the 'dh' dump support for Partition Info protocol
> -Original Message- > From: Wu, Hao A > Sent: Wednesday, September 27, 2017 5:43 AM > To: edk2-devel@lists.01.org > Cc: Wu, Hao A ; Ni, Ruiyu ; > Carsey, Jaben > Subject: [PATCH] ShellPkg/dh: Add the 'dh' dump support for Partition Info > protocol > Importance: High > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=655 > > The dump information will include: > a. The type of the partition (Mbr, Gpt or Other); > b. Whether the partition is an EFI System Partition. > > Cc: Ruiyu Ni > Cc: Jaben Carsey > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Hao Wu > --- > .../UefiHandleParsingLib/UefiHandleParsingLib.c| 68 > ++ > .../UefiHandleParsingLib/UefiHandleParsingLib.h| 1 + > .../UefiHandleParsingLib/UefiHandleParsingLib.inf | 1 + > .../UefiHandleParsingLib/UefiHandleParsingLib.uni | 6 ++ > 4 files changed, 76 insertions(+) > > diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c > b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c > index d12466c7b0..1a56a699ba 100644 > --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c > +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c > @@ -1933,6 +1933,69 @@ ERROR_EXIT: >return NULL; > } > > +/** > + Function to dump information about Partition Information protocol. > + > + This will allocate the return buffer from boot services pool. > + > + @param[in] TheHandle The handle that has the protocol installed. > + @param[in] VerboseTRUE for additional information, FALSE otherwise. > + > + @retval A pointer to a string containing the information. > +**/ > +CHAR16* > +EFIAPI > +PartitionInfoProtocolDumpInformation ( > + IN CONST EFI_HANDLE TheHandle, > + IN CONST BOOLEANVerbose > + ) > +{ > + EFI_STATUS Status; > + EFI_PARTITION_INFO_PROTOCOL *PartitionInfo; > + CHAR16 *GetString; > + CHAR16 *RetVal; > + CONST CHAR16*PartitionType; > + > + if (!Verbose) { > +return NULL; > + } > + > + Status = gBS->OpenProtocol ( > +TheHandle, > +&gEfiPartitionInfoProtocolGuid, > +(VOID**)&PartitionInfo, > +gImageHandle, > +NULL, > +EFI_OPEN_PROTOCOL_GET_PROTOCOL > +); > + if (EFI_ERROR (Status)) { > +return NULL; > + } > + > + HandleParsingHiiInit (); > + GetString = HiiGetString (mHandleParsingHiiHandle, > STRING_TOKEN(STR_PARTINFO_DUMP_MAIN), NULL); > + if (GetString == NULL) { > +return NULL; > + } > + > + switch (PartitionInfo->Type) { > + case PARTITION_TYPE_OTHER: PartitionType = L"Other"; break; > + case PARTITION_TYPE_MBR:PartitionType = L"Mbr";break; > + case PARTITION_TYPE_GPT:PartitionType = L"Gpt";break; > + default: return NULL; > + } > + > + RetVal = CatSPrint ( > + NULL, > + GetString, > + PartitionType, > + PartitionInfo->System == 1 ? L"Yes" : L"No" > + ); This section needs to use proper localization. The strings should be from UNI files, not hard coded here. > + > + SHELL_FREE_NON_NULL (GetString); > + return RetVal; > +} > + > // > // Put the information on the NT32 protocol GUIDs here so we are not > dependant on the Nt32Pkg > // > @@ -2147,6 +2210,11 @@ STATIC CONST GUID_INFO_BLOCK > mGuidStringList[] = { >{STRING_TOKEN(STR_ADAPTER_INFO), > &gEfiAdapterInformationProtocolGuid, > AdapterInformationDumpInformation}, > > // > +// UEFI 2.7 > +// > + {STRING_TOKEN(STR_PARTITION_INFO), > &gEfiPartitionInfoProtocolGuid, > PartitionInfoProtocolDumpInformation}, > + > +// > // PI Spec ones > // >{STRING_TOKEN(STR_IDE_CONT_INIT), > &gEfiIdeControllerInitProtocolGuid, NULL}, > diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.h > b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.h > index cf849658aa..68bb00c620 100644 > --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.h > +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.h > @@ -138,6 +138,7 @@ > #include > #include > #include > +#include > > #include > #include > diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf > b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf > index 4c1c3d3846..ee1b85552b 100644 > --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf > +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf > @@ -292,6 +292,7 @@ >gEfiHttpProtocolGuid## UNDEFINED >gEfiHttpUtilitiesProtocolGuid ## UNDEFINED >gEfiRestProtocolGuid## UNDEFINED > + gEfiPartitionInfoProtocolGuid ## CONSUMES > > [Guids] >gEfiFileInfoGui
Re: [edk2] [Patch] BaseTools: Fix the regression bug to build single module
Reviewed-by: Liming Gao > -Original Message- > From: Zhu, Yonghong > Sent: Wednesday, September 27, 2017 9:22 PM > To: edk2-devel@lists.01.org > Cc: Gao, Liming > Subject: [Patch] BaseTools: Fix the regression bug to build single module > > The bug is introduced by 1b8eca to collect single module's build time. > Now the fix solution is copied from Platform build. > > Cc: Liming Gao > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Yonghong Zhu > --- > BaseTools/Source/Python/build/build.py | 12 > 1 file changed, 12 insertions(+) > > diff --git a/BaseTools/Source/Python/build/build.py > b/BaseTools/Source/Python/build/build.py > index dd65c66..13d8e50 100644 > --- a/BaseTools/Source/Python/build/build.py > +++ b/BaseTools/Source/Python/build/build.py > @@ -1830,10 +1830,22 @@ class Build(): > Pa = PlatformAutoGen(Wa, self.PlatformFile, BuildTarget, > ToolChain, Arch) > for Module in Pa.Platform.Modules: > if self.ModuleFile.Dir == Module.Dir and > self.ModuleFile.File == Module.File: > Ma = ModuleAutoGen(Wa, Module, BuildTarget, > ToolChain, Arch, self.PlatformFile) > if Ma == None: continue > +# Not to auto-gen for targets 'clean', > 'cleanlib', 'cleanall', 'run', 'fds' > +if self.Target not in ['clean', 'cleanlib', > 'cleanall', 'run', 'fds']: > +# for target which must generate AutoGen > code and makefile > +if not self.SkipAutoGen or self.Target == > 'genc': > +Ma.CreateCodeFile(True) > +if self.Target == "genc": > +continue > + > +if not self.SkipAutoGen or self.Target == > 'genmake': > +Ma.CreateMakeFile(True) > +if self.Target == "genmake": > +continue > MaList.append(Ma) > self.BuildModules.append(Ma) > self.AutoGenTime += int(round((time.time() - > AutoGenStart))) > MakeStart = time.time() > for Ma in self.BuildModules: > -- > 2.6.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch] BaseTools: Fix the regression bug to build single module
The bug is introduced by 1b8eca to collect single module's build time. Now the fix solution is copied from Platform build. Cc: Liming Gao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Yonghong Zhu --- BaseTools/Source/Python/build/build.py | 12 1 file changed, 12 insertions(+) diff --git a/BaseTools/Source/Python/build/build.py b/BaseTools/Source/Python/build/build.py index dd65c66..13d8e50 100644 --- a/BaseTools/Source/Python/build/build.py +++ b/BaseTools/Source/Python/build/build.py @@ -1830,10 +1830,22 @@ class Build(): Pa = PlatformAutoGen(Wa, self.PlatformFile, BuildTarget, ToolChain, Arch) for Module in Pa.Platform.Modules: if self.ModuleFile.Dir == Module.Dir and self.ModuleFile.File == Module.File: Ma = ModuleAutoGen(Wa, Module, BuildTarget, ToolChain, Arch, self.PlatformFile) if Ma == None: continue +# Not to auto-gen for targets 'clean', 'cleanlib', 'cleanall', 'run', 'fds' +if self.Target not in ['clean', 'cleanlib', 'cleanall', 'run', 'fds']: +# for target which must generate AutoGen code and makefile +if not self.SkipAutoGen or self.Target == 'genc': +Ma.CreateCodeFile(True) +if self.Target == "genc": +continue + +if not self.SkipAutoGen or self.Target == 'genmake': +Ma.CreateMakeFile(True) +if self.Target == "genmake": +continue MaList.append(Ma) self.BuildModules.append(Ma) self.AutoGenTime += int(round((time.time() - AutoGenStart))) MakeStart = time.time() for Ma in self.BuildModules: -- 2.6.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] ShellPkg/dh: Add the 'dh' dump support for Partition Info protocol
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=655 The dump information will include: a. The type of the partition (Mbr, Gpt or Other); b. Whether the partition is an EFI System Partition. Cc: Ruiyu Ni Cc: Jaben Carsey Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu --- .../UefiHandleParsingLib/UefiHandleParsingLib.c| 68 ++ .../UefiHandleParsingLib/UefiHandleParsingLib.h| 1 + .../UefiHandleParsingLib/UefiHandleParsingLib.inf | 1 + .../UefiHandleParsingLib/UefiHandleParsingLib.uni | 6 ++ 4 files changed, 76 insertions(+) diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c index d12466c7b0..1a56a699ba 100644 --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c @@ -1933,6 +1933,69 @@ ERROR_EXIT: return NULL; } +/** + Function to dump information about Partition Information protocol. + + This will allocate the return buffer from boot services pool. + + @param[in] TheHandle The handle that has the protocol installed. + @param[in] VerboseTRUE for additional information, FALSE otherwise. + + @retval A pointer to a string containing the information. +**/ +CHAR16* +EFIAPI +PartitionInfoProtocolDumpInformation ( + IN CONST EFI_HANDLE TheHandle, + IN CONST BOOLEANVerbose + ) +{ + EFI_STATUS Status; + EFI_PARTITION_INFO_PROTOCOL *PartitionInfo; + CHAR16 *GetString; + CHAR16 *RetVal; + CONST CHAR16*PartitionType; + + if (!Verbose) { +return NULL; + } + + Status = gBS->OpenProtocol ( +TheHandle, +&gEfiPartitionInfoProtocolGuid, +(VOID**)&PartitionInfo, +gImageHandle, +NULL, +EFI_OPEN_PROTOCOL_GET_PROTOCOL +); + if (EFI_ERROR (Status)) { +return NULL; + } + + HandleParsingHiiInit (); + GetString = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN(STR_PARTINFO_DUMP_MAIN), NULL); + if (GetString == NULL) { +return NULL; + } + + switch (PartitionInfo->Type) { + case PARTITION_TYPE_OTHER: PartitionType = L"Other"; break; + case PARTITION_TYPE_MBR:PartitionType = L"Mbr";break; + case PARTITION_TYPE_GPT:PartitionType = L"Gpt";break; + default: return NULL; + } + + RetVal = CatSPrint ( + NULL, + GetString, + PartitionType, + PartitionInfo->System == 1 ? L"Yes" : L"No" + ); + + SHELL_FREE_NON_NULL (GetString); + return RetVal; +} + // // Put the information on the NT32 protocol GUIDs here so we are not dependant on the Nt32Pkg // @@ -2147,6 +2210,11 @@ STATIC CONST GUID_INFO_BLOCK mGuidStringList[] = { {STRING_TOKEN(STR_ADAPTER_INFO), &gEfiAdapterInformationProtocolGuid, AdapterInformationDumpInformation}, // +// UEFI 2.7 +// + {STRING_TOKEN(STR_PARTITION_INFO),&gEfiPartitionInfoProtocolGuid, PartitionInfoProtocolDumpInformation}, + +// // PI Spec ones // {STRING_TOKEN(STR_IDE_CONT_INIT), &gEfiIdeControllerInitProtocolGuid, NULL}, diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.h b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.h index cf849658aa..68bb00c620 100644 --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.h +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.h @@ -138,6 +138,7 @@ #include #include #include +#include #include #include diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf index 4c1c3d3846..ee1b85552b 100644 --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf @@ -292,6 +292,7 @@ gEfiHttpProtocolGuid## UNDEFINED gEfiHttpUtilitiesProtocolGuid ## UNDEFINED gEfiRestProtocolGuid## UNDEFINED + gEfiPartitionInfoProtocolGuid ## CONSUMES [Guids] gEfiFileInfoGuid## UNDEFINED diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni index f49ca94623..1f2cc4a6f9 100644 --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni @@ -152,6 +152,9 @@ #string STR_SHELL #language en-US "Shell" #string STR_SHELL_DYNAMIC #language en-US "ShellDynamicCommand" +// Partition Information +#string STR_PARTITION_INFO#language en-US "Par
Re: [edk2] Linux CentOS 7.3 can get DHCP IPv4 IP address with configuring DHCP server as per RFC3021
Hi Siyuan, I've filled a New Bug in Bugzilla and following are the details. https://bugzilla.tianocore.org/show_bug.cgi?id=722 Thanks, Karunakar From: Fu, Siyuan [mailto:siyuan...@intel.com] Sent: Wednesday, September 27, 2017 1:46 PM To: Karunakar P; 'edk2-devel@lists.01.org' Cc: Wu, Jiaxin; Ye, Ting Subject: RE: Linux CentOS 7.3 can get DHCP IPv4 IP address with configuring DHCP server as per RFC3021 Hi, Karunakar We haven't received requirement for this feature before so we don't have plan now. I think you can submit a Bugzilla ticket for this feature request, we will follow up to investigate it. BestRegards Fu Siyuan From: Karunakar P [mailto:karunak...@amiindia.co.in] Sent: Monday, September 25, 2017 4:54 PM To: Fu, Siyuan mailto:siyuan...@intel.com>>; 'edk2-devel@lists.01.org' mailto:edk2-devel@lists.01.org>> Cc: Wu, Jiaxin mailto:jiaxin...@intel.com>>; Ye, Ting mailto:ting...@intel.com>> Subject: RE: Linux CentOS 7.3 can get DHCP IPv4 IP address with configuring DHCP server as per RFC3021 Hi Fu Siyuan, Thanks for your conformation. We would like to use /31s to save IPv4 addresses. When it's a point-to-point link (server to router), the extra network address and broadcast address is wasted. RFC 3021 solves the problem by using a broadcast address of 255.255.255.255 on such subnets. We have a requirement to support this, do you have any plan to support RFC3021 ? Given the requirement time consuming how hard to implement this ? Thanks, karunakar From: Fu, Siyuan [mailto:siyuan...@intel.com] Sent: Monday, September 25, 2017 1:28 PM To: Karunakar P; 'edk2-devel@lists.01.org' Cc: Wu, Jiaxin; Ye, Ting Subject: RE: Linux CentOS 7.3 can get DHCP IPv4 IP address with configuring DHCP server as per RFC3021 Hi, Karunakar May I know that whether you have a real requirement that must use the point-2-point link in your environment, or you just found this problem in your test? BestRegards Fu Siyuan From: Fu, Siyuan Sent: Monday, September 25, 2017 3:50 PM To: Karunakar P mailto:karunak...@amiindia.co.in>>; 'edk2-devel@lists.01.org' mailto:edk2-devel@lists.01.org>> Cc: Wu, Jiaxin mailto:jiaxin...@intel.com>>; Ye, Ting mailto:ting...@intel.com>> Subject: RE: Linux CentOS 7.3 can get DHCP IPv4 IP address with configuring DHCP server as per RFC3021 Hi, Karunakar You are correct that EDK2 doesn't support rfc3201. The most obvious problem that come to my mind is the NetIp4IsUnicast() function in NetLib, which has the assumption that the host address part should not be all zero or all one (or to say, -1 in the rfc3201). I think that's why the PXE failed, but Cent OS could use IP address from the same DHCP server. This is just an example, there may be some other places in edk2 network stack which have the same assumption, I'm not sure about this. Anyway, we never considered the point-2-point link in edk2. BestRegards Fu Siyuan From: Karunakar P [mailto:karunak...@amiindia.co.in] Sent: Monday, September 25, 2017 3:02 PM To: 'edk2-devel@lists.01.org' mailto:edk2-devel@lists.01.org>> Cc: Wu, Jiaxin mailto:jiaxin...@intel.com>>; Fu, Siyuan mailto:siyuan...@intel.com>>; Ye, Ting mailto:ting...@intel.com>> Subject: Re: Linux CentOS 7.3 can get DHCP IPv4 IP address with configuring DHCP server as per RFC3021 Hello All, It is known that current EDKII doesn't support RFC3021, We could see the following behavior which is PXE boot fails whereas Cent OS can get IP address from the same BIOS. [Configuration Used] DHCP server setting under Ubuntu: 1. /etc/dhcp/dhcpd.conf # RFC3021-Using 31-bit perfixes on IPv4 Point-to-Point Links subnet 192.168.1.0 netmask 255.255.255.254 { range 192.168.1.1 192.168.1.1; next-server 192.168.1.0; filename "EFI/BOOT/BOOTAA64.EFI"; option subnet-mask 255.255.255.254; option routers 192.168.1.0; } 2. /etc/network/interfaces auto eth0 iface eth0 inet static address 192.168.1.0 netmask 255.255.255.254 network 192.168.1.0 If the DHCP server and network interface are set up above configuration. Below are my test results and questions 1. CentOS 7.3 (pre-installed) was able to retrieve IP through DHCP when they connect HDD to the SUT where PXE is failing. 2. Could you please suggest what could be the reason behind this? Thanks, karunakar ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch][edk2-platforms/devel-MinnowBoardMax-UDK2017] Change GCC shell
Change GCC shell from MinimumShell to UefiShell in ShellBinPkg. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Guo Mang --- Vlv2TbltDevicePkg/PlatformPkgGcc.fdf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf index 2c10c11..78103d3 100644 --- a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf +++ b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf @@ -638,7 +638,7 @@ FILE DRIVER = 961578FE-B6B7-44c3-AF35-6BC705CD2B1F { # FILE APPLICATION = PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile) { #SECTION PE32 = EdkShellBinPkg/FullShell/$(EDK_DXE_ARCHITECTURE)/Shell_Full.efi - SECTION PE32 = EdkShellBinPkg/MinimumShell/$(EDK_DXE_ARCHITECTURE)/Shell.efi +SECTION PE32 = ShellBinPkg/UefiShell/$(EDK_DXE_ARCHITECTURE)/Shell.efi } -- 2.10.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] Linux CentOS 7.3 can get DHCP IPv4 IP address with configuring DHCP server as per RFC3021
Hi, Karunakar We haven't received requirement for this feature before so we don't have plan now. I think you can submit a Bugzilla ticket for this feature request, we will follow up to investigate it. BestRegards Fu Siyuan From: Karunakar P [mailto:karunak...@amiindia.co.in] Sent: Monday, September 25, 2017 4:54 PM To: Fu, Siyuan ; 'edk2-devel@lists.01.org' Cc: Wu, Jiaxin ; Ye, Ting Subject: RE: Linux CentOS 7.3 can get DHCP IPv4 IP address with configuring DHCP server as per RFC3021 Hi Fu Siyuan, Thanks for your conformation. We would like to use /31s to save IPv4 addresses. When it's a point-to-point link (server to router), the extra network address and broadcast address is wasted. RFC 3021 solves the problem by using a broadcast address of 255.255.255.255 on such subnets. We have a requirement to support this, do you have any plan to support RFC3021 ? Given the requirement time consuming how hard to implement this ? Thanks, karunakar From: Fu, Siyuan [mailto:siyuan...@intel.com] Sent: Monday, September 25, 2017 1:28 PM To: Karunakar P; 'edk2-devel@lists.01.org' Cc: Wu, Jiaxin; Ye, Ting Subject: RE: Linux CentOS 7.3 can get DHCP IPv4 IP address with configuring DHCP server as per RFC3021 Hi, Karunakar May I know that whether you have a real requirement that must use the point-2-point link in your environment, or you just found this problem in your test? BestRegards Fu Siyuan From: Fu, Siyuan Sent: Monday, September 25, 2017 3:50 PM To: Karunakar P mailto:karunak...@amiindia.co.in>>; 'edk2-devel@lists.01.org' mailto:edk2-devel@lists.01.org>> Cc: Wu, Jiaxin mailto:jiaxin...@intel.com>>; Ye, Ting mailto:ting...@intel.com>> Subject: RE: Linux CentOS 7.3 can get DHCP IPv4 IP address with configuring DHCP server as per RFC3021 Hi, Karunakar You are correct that EDK2 doesn't support rfc3201. The most obvious problem that come to my mind is the NetIp4IsUnicast() function in NetLib, which has the assumption that the host address part should not be all zero or all one (or to say, -1 in the rfc3201). I think that's why the PXE failed, but Cent OS could use IP address from the same DHCP server. This is just an example, there may be some other places in edk2 network stack which have the same assumption, I'm not sure about this. Anyway, we never considered the point-2-point link in edk2. BestRegards Fu Siyuan From: Karunakar P [mailto:karunak...@amiindia.co.in] Sent: Monday, September 25, 2017 3:02 PM To: 'edk2-devel@lists.01.org' mailto:edk2-devel@lists.01.org>> Cc: Wu, Jiaxin mailto:jiaxin...@intel.com>>; Fu, Siyuan mailto:siyuan...@intel.com>>; Ye, Ting mailto:ting...@intel.com>> Subject: Re: Linux CentOS 7.3 can get DHCP IPv4 IP address with configuring DHCP server as per RFC3021 Hello All, It is known that current EDKII doesn't support RFC3021, We could see the following behavior which is PXE boot fails whereas Cent OS can get IP address from the same BIOS. [Configuration Used] DHCP server setting under Ubuntu: 1. /etc/dhcp/dhcpd.conf # RFC3021-Using 31-bit perfixes on IPv4 Point-to-Point Links subnet 192.168.1.0 netmask 255.255.255.254 { range 192.168.1.1 192.168.1.1; next-server 192.168.1.0; filename "EFI/BOOT/BOOTAA64.EFI"; option subnet-mask 255.255.255.254; option routers 192.168.1.0; } 2. /etc/network/interfaces auto eth0 iface eth0 inet static address 192.168.1.0 netmask 255.255.255.254 network 192.168.1.0 If the DHCP server and network interface are set up above configuration. Below are my test results and questions 1. CentOS 7.3 (pre-installed) was able to retrieve IP through DHCP when they connect HDD to the SUT where PXE is failing. 2. Could you please suggest what could be the reason behind this? Thanks, karunakar ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] SecurityPkg\SmmTcg2PhysicalPresenceLib.c Handle reserved or unimplemented PP Operation
Reviewed-by: jiewen@intel.com > -Original Message- > From: Zeng, Star > Sent: Wednesday, September 27, 2017 3:57 PM > To: Zhang, Chao B ; edk2-devel@lists.01.org > Cc: Yao, Jiewen ; Zeng, Star > Subject: RE: [PATCH] SecurityPkg\SmmTcg2PhysicalPresenceLib.c Handle > reserved or unimplemented PP Operation > > Reviewed-by: Star Zeng > > -Original Message- > From: Zhang, Chao B > Sent: Friday, September 22, 2017 2:54 PM > To: edk2-devel@lists.01.org > Cc: Yao, Jiewen ; Zeng, Star ; > Zhang, Chao B > Subject: [PATCH] SecurityPkg\SmmTcg2PhysicalPresenceLib.c Handle reserved or > unimplemented PP Operation > > Several PP operations < 128(Vendor Specific) are reserved or unimplemented. > Follow TCG PC Client Platform Physical Presence Interface Specification to > return > not implemented. > https://trustedcomputinggroup.org/wp-content/uploads/Physical-Presence-Int > erface_1-30_0-52.pdf > > Cc: Jiewen Yao > Cc: Star Zeng > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Chao Zhang > --- > .../SmmTcg2PhysicalPresenceLib.c | 14 > +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git > a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLi > b.c > b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLi > b.c > index ba4db11..6061453 100644 > --- > a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLi > b.c > +++ b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPres > +++ enceLib.c > @@ -10,7 +10,7 @@ >Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction() and > Tcg2PhysicalPresenceLibGetUserConfirmationStatusFunction() >will receive untrusted input and do validation. > > -Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved. > +Copyright (c) 2015 - 2017, 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 @@ -291,6 +291,7 > @@ > Tcg2PhysicalPresenceLibGetUserConfirmationStatusFunction ( >} >break; > > +case TCG2_PHYSICAL_PRESENCE_NO_ACTION: > case TCG2_PHYSICAL_PRESENCE_SET_PP_REQUIRED_FOR_CLEAR_TRUE: >RequestConfirmed = TRUE; >break; > @@ -336,12 +337,11 @@ > Tcg2PhysicalPresenceLibGetUserConfirmationStatusFunction ( >break; > > default: > - if (OperationRequest <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) > { > -RequestConfirmed = TRUE; > - } else { > -if (OperationRequest < > TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) { > - return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED; > -} > + if (OperationRequest < > TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) { > +// > +// TCG PP spec defined operations that are reserved or > un-implemented > +// > +return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED; >} >break; >} > -- > 1.9.5.msysgit.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] SecurityPkg\SmmTcg2PhysicalPresenceLib.c Handle reserved or unimplemented PP Operation
Reviewed-by: Star Zeng -Original Message- From: Zhang, Chao B Sent: Friday, September 22, 2017 2:54 PM To: edk2-devel@lists.01.org Cc: Yao, Jiewen ; Zeng, Star ; Zhang, Chao B Subject: [PATCH] SecurityPkg\SmmTcg2PhysicalPresenceLib.c Handle reserved or unimplemented PP Operation Several PP operations < 128(Vendor Specific) are reserved or unimplemented. Follow TCG PC Client Platform Physical Presence Interface Specification to return not implemented. https://trustedcomputinggroup.org/wp-content/uploads/Physical-Presence-Interface_1-30_0-52.pdf Cc: Jiewen Yao Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chao Zhang --- .../SmmTcg2PhysicalPresenceLib.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c index ba4db11..6061453 100644 --- a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c +++ b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPres +++ enceLib.c @@ -10,7 +10,7 @@ Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction() and Tcg2PhysicalPresenceLibGetUserConfirmationStatusFunction() will receive untrusted input and do validation. -Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved. +Copyright (c) 2015 - 2017, 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 @@ -291,6 +291,7 @@ Tcg2PhysicalPresenceLibGetUserConfirmationStatusFunction ( } break; +case TCG2_PHYSICAL_PRESENCE_NO_ACTION: case TCG2_PHYSICAL_PRESENCE_SET_PP_REQUIRED_FOR_CLEAR_TRUE: RequestConfirmed = TRUE; break; @@ -336,12 +337,11 @@ Tcg2PhysicalPresenceLibGetUserConfirmationStatusFunction ( break; default: - if (OperationRequest <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) { -RequestConfirmed = TRUE; - } else { -if (OperationRequest < TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) { - return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED; -} + if (OperationRequest < TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) { +// +// TCG PP spec defined operations that are reserved or un-implemented +// +return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED; } break; } -- 1.9.5.msysgit.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] Changed Max Baud Rate
Change Max Baud Rate to 115200. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Guo Mang --- .../BroxtonSiPkg/SouthCluster/Include/ScRegs/RegsLpss.h | 9 +++-- .../PeiDxeSmmPchSerialIoUartLib/PeiDxeSmmPchSerialIoUartLib.c| 3 +-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/ScRegs/RegsLpss.h b/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/ScRegs/RegsLpss.h index 51e4e95..6f2f91d 100644 --- a/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/ScRegs/RegsLpss.h +++ b/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Include/ScRegs/RegsLpss.h @@ -17,7 +17,7 @@ - Registers / bits of new devices introduced in a SC generation will be just named as "_PCH_" without inserted. - Copyright (c) 1999 - 2016, Intel Corporation. All rights reserved. + Copyright (c) 1999 - 2017, 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 @@ -124,8 +124,13 @@ #define B_LPSS_IO_MEM_PCP_N_VAL 0x7FFF ///< N value for the M over N divider #define B_LPSS_IO_MEM_PCP_M_VAL 0xFFFE ///< M value for the M over N divider #define B_LPSS_IO_MEM_PCP_CLK_EN BIT0 ///< Clock Enable -#define V_LPSS_IO_PPR_CLK_M_DIV 1152 ///< Max Baudrate = (100MHz * (1152 / 15625)) / 16 = 460800Hz +#define V_LPSS_IO_PPR_CLK_M_DIV 288 ///< Max Baudrate = (100MHz * (288 / 15625)) / 16 = 115200Hz + /// 2304 -> 921600 + /// 1152 -> 460800 + /// 576 -> 230400 + /// 288 -> 115200 #define V_LPSS_IO_PPR_CLK_N_DIV 15625 +#define MAX_BAUD_RATE 115200 /// ((1 * (V_LPSS_IO_PPR_CLK_M_DIV / V_LPSS_IO_PPR_CLK_N_DIV)) / 16) #define R_LPSS_IO_MEM_RESETS 0x204///< Software Reset #define B_LPSS_IO_MEM_HC_RESET_REL(BIT0|BIT1) ///< LPSS IO Host Controller Reset Release diff --git a/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Library/PeiDxeSmmPchSerialIoUartLib/PeiDxeSmmPchSerialIoUartLib.c b/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Library/PeiDxeSmmPchSerialIoUartLib/PeiDxeSmmPchSerialIoUartLib.c index ed34140..4a3bdcc 100644 --- a/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Library/PeiDxeSmmPchSerialIoUartLib/PeiDxeSmmPchSerialIoUartLib.c +++ b/Silicon/BroxtonSoC/BroxtonSiPkg/SouthCluster/Library/PeiDxeSmmPchSerialIoUartLib/PeiDxeSmmPchSerialIoUartLib.c @@ -3,7 +3,7 @@ All function in this library is available for PEI, DXE, and SMM, But do not support UEFI RUNTIME environment call. - Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved. + Copyright (c) 2014 - 2017, 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 @@ -24,7 +24,6 @@ #include #include -#define MAX_BAUD_RATE 460800 // Maximum Baud per SoC spec #define R_PCH_SERIAL_IO_8BIT_UART_RXBUF 0x00 #define R_PCH_SERIAL_IO_8BIT_UART_TXBUF 0x00 -- 2.10.1.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch V2] Build spec: add description for build with binary cache
Reviewed-by: Liming Gao >-Original Message- >From: Zhu, Yonghong >Sent: Tuesday, September 19, 2017 2:48 PM >To: edk2-devel@lists.01.org >Cc: Gao, Liming ; Kinney, Michael D >; Shaw, Kevin W >Subject: [Patch V2] Build spec: add description for build with binary cache > >V2: >change the option name to --binary-destination and --binary-source. > >fixes:https://bugzilla.tianocore.org/show_bug.cgi?id=689 >Cc: Liming Gao >Cc: Michael Kinney >Cc: Kevin W Shaw >Contributed-under: TianoCore Contribution Agreement 1.1 >Signed-off-by: Yonghong Zhu >--- > .../82_auto-generation_process.md| 20 > README.md| 1 + > appendix_d_buildexe_command/d4_usage.md | 19 >+++ > 3 files changed, 36 insertions(+), 4 deletions(-) > >diff --git a/8_pre-build_autogen_stage/82_auto-generation_process.md >b/8_pre-build_autogen_stage/82_auto-generation_process.md >index 671a7d5..f2ddf32 100644 >--- a/8_pre-build_autogen_stage/82_auto-generation_process.md >+++ b/8_pre-build_autogen_stage/82_auto-generation_process.md >@@ -1031,10 +1031,30 @@ maximum command line length. The default >value is 4096. > **Note:** The following `FLAGS` options are included in the response file: > `PP_FLAGS`, `CC_FLAGS`, `VFRPP_FLAGS`, `APP_FLAGS`, `ASLPP_FLAGS`, >`ASLCC_FLAGS`, > and `ASM_FLAGS`. > ** > >+ 8.2.4.15 Build with Binary Cache >+ >+**build** tool provides three new options for binary cache feature. >+--hash enables hash-based caching during build process. when --hash is >enabled, >+build tool will base on the module hash value to do the incremental build, >without >+--hash, build tool will base on the timestamp to do the incremental build. -- >hash >+option use md5 method to get every hash value, DSC/FDF, tools_def.txt, >build_rule.txt >+and build command are calculated as global hash value, Package DEC and its >include >+header files are calculated as package hash value, Module source files and its >INF >+file are calculated as module hash value. Library hash value will combine the >global >+hash value and its dependent package hash value. Driver hash value will >combine the >+global hash value, its dependent package hash value and its linked library >hash value. >+When --hash and --binary-destination are specified, build tool will copy the >generated >+binary files for each module into the directory specified by binary- >destination at the >+build phase. Binary-destination directory caches all the generated binary >files. >+When --hash and --binary-source are specified, build tool will try to get the >binary >+files from the binary source directory at the build phase. If the cached >binary >has >+the same hash value, it will be directly used. Otherwise, build tool will >compile the >+source files and generate the binary files. >+ > ### 8.2.5 Post processing > > Once all files are parsed, the build tools will do following work for each EDK > II module: > >diff --git a/README.md b/README.md >index 52abb6a..ca59a35 100644 >--- a/README.md >+++ b/README.md >@@ -215,5 +215,6 @@ Copyright (c) 2008-2017, Intel Corporation. All rights >reserved. > || [#523](https://bugzilla.tianocore.org/show_bug.cgi?id=523) > Build >spec: add EBNF for the --pcd syntax in the Section D.4 >| | > || [#517](https://bugzilla.tianocore.org/show_bug.cgi?id=517) > Build >spec: chapter 5.2.2 Guided Tools add description for Pkcs7Sign tool and >BrotliCompress tool >| | > || [#481](https://bugzilla.tianocore.org/show_bug.cgi?id=481) > Build >Spec: add clarification for not used Pcd that build tool will not do additional >checks on its value >| | > || [#518](https://bugzilla.tianocore.org/show_bug.cgi?id=518) > Build >Spec: Update Precedence of PCD Values >| | > || [#669](https://bugzilla.tianocore.org/show_bug.cgi?id=669) > Build >Spec: Add multi-arg support to PREBUILD/POSTBUILD >| | >+|| [#689](https://bugzilla.tianocore.org/show_bug.cgi?id=689) >Build >spec: add description for build with binary cache >| | >diff --git a/appendix_d_buildexe_command/d4_usage.md >b/appendix_d_buildexe_command/d4_usage.md >index b71f2d0..c901266 100644 >--- a/appendix_d_buildexe_command/d4_usage.md >+++ b/appendix_d_buildexe_command/d4_usage.md >@@ -32,19 +32,20 @@ > ## D.4 Usage > > ```ini > Usage: build.exe [options] > [all|fds|genc|genmake|clean|cleanall|cleanlib|modules|libraries|run] >-Copyright (c) 2007 - 2014, Intel Corporation All rights reserved. >+Copyright (c) 2007 - 2017, Intel Corporation All rights reserved. > > Options: > --version show program's version number and exit > -h, --helpshow this help message and exit > -a TARGETARCH, --arch=TARGETARCH >-ARCHS is one of list: IA32, X64, IPF, ARM, or EBC, >-which