Linaro-uefi November 2017

linaro-uefi@lists.linaro.org

5 participants
8 discussions

[PATCH] MdeModulePkg/PerformanceLib: add lock protection

by Heyi Guo

DXE performance gauge record access functions might be reentered since we are supporting something like USB hot-plug, which is a timer event where gBS->ConnectController might be called and then PERF will be called in CoreConnectSingleController. When StartGaugeEx is being reentered, not only the gauge record might be overwritten, more serious situation will be caused if gauge data buffer reallocation procedure is interrupted, between line 180 and 187 in DxeCorePerformanceLib.c specifically. There, mMaxGaugeRecords will be doubled twice (denoted as 4X), but mGaugeData only points to a buffer of size 2X, which will probably cause the following 2X memory to be overflowed when gauge records are increased. So we add EFI lock with with TPL_NOTIFY in StartGaugeEx/EndGaugeEx/GetGaugeEx to avoid memory overflow and gauge data corruption. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Heyi Guo <heyi.guo(a)linaro.org> Cc: Star Zeng <star.zeng(a)intel.com> Cc: Eric Dong <eric.dong(a)intel.com> Cc: Liming Gao <liming.gao(a)intel.com> --- .../DxeCorePerformanceLib/DxeCorePerformanceLib.c | 67 +++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c index 51f488a..7c0e207 100644 --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c @@ -63,6 +63,11 @@ PERFORMANCE_EX_PROTOCOL mPerformanceExInterface = { PERFORMANCE_PROPERTY mPerformanceProperty; +// +// Gauge record lock to avoid data corruption or even memory overflow +// +STATIC EFI_LOCK mPerfRecordLock = EFI_INITIALIZE_LOCK_VARIABLE (TPL_NOTIFY); + /** Searches in the gauge array with keyword Handle, Token, Module and Identifier. @@ -162,6 +167,12 @@ StartGaugeEx ( UINTN OldGaugeDataSize; GAUGE_DATA_HEADER *OldGaugeData; UINT32 Index; + EFI_STATUS Status; + + Status = EfiAcquireLockOrFail (&mPerfRecordLock); + if (EFI_ERROR (Status)) { + return Status; + } Index = mGaugeData->NumberOfEntries; if (Index >= mMaxGaugeRecords) { @@ -175,6 +186,7 @@ StartGaugeEx ( NewGaugeData = AllocateZeroPool (GaugeDataSize); if (NewGaugeData == NULL) { + EfiReleaseLock (&mPerfRecordLock); return EFI_OUT_OF_RESOURCES; } @@ -209,6 +221,8 @@ StartGaugeEx ( mGaugeData->NumberOfEntries++; + EfiReleaseLock (&mPerfRecordLock); + return EFI_SUCCESS; } @@ -250,6 +264,12 @@ EndGaugeEx ( { GAUGE_DATA_ENTRY_EX *GaugeEntryExArray; UINT32 Index; + EFI_STATUS Status; + + Status = EfiAcquireLockOrFail (&mPerfRecordLock); + if (EFI_ERROR (Status)) { + return Status; + } if (TimeStamp == 0) { TimeStamp = GetPerformanceCounter (); @@ -257,11 +277,13 @@ EndGaugeEx ( Index = InternalSearchForGaugeEntry (Handle, Token, Module, Identifier); if (Index >= mGaugeData->NumberOfEntries) { + EfiReleaseLock (&mPerfRecordLock); return EFI_NOT_FOUND; } GaugeEntryExArray = (GAUGE_DATA_ENTRY_EX *) (mGaugeData + 1); GaugeEntryExArray[Index].EndTimeStamp = TimeStamp; + EfiReleaseLock (&mPerfRecordLock); return EFI_SUCCESS; } @@ -274,6 +296,8 @@ EndGaugeEx ( If it stands for a valid entry, then EFI_SUCCESS is returned and GaugeDataEntryEx stores the pointer to that entry. + This internal function is added to avoid releasing lock before each return statement. + @param LogEntryKey The key for the previous performance measurement log entry. If 0, then the first performance measurement log entry is retrieved. @param GaugeDataEntryEx The indirect pointer to the extended gauge data entry specified by LogEntryKey @@ -287,7 +311,7 @@ EndGaugeEx ( **/ EFI_STATUS EFIAPI -GetGaugeEx ( +InternalGetGaugeEx ( IN UINTN LogEntryKey, OUT GAUGE_DATA_ENTRY_EX **GaugeDataEntryEx ) @@ -314,6 +338,47 @@ GetGaugeEx ( } /** + Retrieves a previously logged performance measurement. + It can also retrieve the log created by StartGauge and EndGauge of PERFORMANCE_PROTOCOL, + and then assign the Identifier with 0. + + Retrieves the performance log entry from the performance log specified by LogEntryKey. + If it stands for a valid entry, then EFI_SUCCESS is returned and + GaugeDataEntryEx stores the pointer to that entry. + + @param LogEntryKey The key for the previous performance measurement log entry. + If 0, then the first performance measurement log entry is retrieved. + @param GaugeDataEntryEx The indirect pointer to the extended gauge data entry specified by LogEntryKey + if the retrieval is successful. + + @retval EFI_SUCCESS The GuageDataEntryEx is successfully found based on LogEntryKey. + @retval EFI_NOT_FOUND The LogEntryKey is the last entry (equals to the total entry number). + @retval EFI_INVALIDE_PARAMETER The LogEntryKey is not a valid entry (greater than the total entry number). + @retval EFI_INVALIDE_PARAMETER GaugeDataEntryEx is NULL. + +**/ +EFI_STATUS +EFIAPI +GetGaugeEx ( + IN UINTN LogEntryKey, + OUT GAUGE_DATA_ENTRY_EX **GaugeDataEntryEx + ) +{ + EFI_STATUS Status; + + Status = EfiAcquireLockOrFail (&mPerfRecordLock); + if (EFI_ERROR (Status)) { + return Status; + } + + Status = InternalGetGaugeEx (LogEntryKey, GaugeDataEntryEx); + + EfiReleaseLock (&mPerfRecordLock); + + return Status; +} + +/** Adds a record at the end of the performance measurement log that records the start time of a performance measurement. -- 2.7.4

6 years, 4 months

[PATCH edk2-non-osi 0/2] Fix Linaro RP bug 3464

by Heyi Guo

We have found 2 bugs with Linaro Reference Platform bug 3464: https://bugs.linaro.org/show_bug.cgi?id=3464 1. SAS driver might use uninitialized value which would cause system exception; 2. LpcSerialPortLib used on D03 will cause SerialDxe initialized with failure and exited immediately, which would cause no serial terminal in BDS. Patches with binary files can also be found in https://github.com/hisilicon/OpenPlatformPkg/commits/rp-osi-bug-v4 Cc: Leif Lindholm <leif.lindholm(a)linaro.org> Cc: Ard Biesheuvel <ard.biesheuvel(a)linaro.org> Heyi Guo (2): Hisilicon/D0x/Sas: fix occasional exception Hisilicon/LpcSerialPortLib: return SUCCESS for SetAttributes Platform/Hisilicon/D03/Drivers/Sas/SasDriverDxe.efi | Bin 98144 -> 98112 bytes Platform/Hisilicon/D05/Drivers/Sas/SasDriverDxe.efi | Bin 116288 -> 112832 bytes Silicon/Hisilicon/Hi1610/Library/Uart/LpcSerialPortLib/LpcSerialPortLib.lib | Bin 16942 -> 16950 bytes 3 files changed, 0 insertions(+), 0 deletions(-) -- 1.9.1

6 years, 5 months

[PATCH edk2-non-osi v3] Update D03/D05 MemoryInit.efi binary for bug 3419

by Heyi Guo

https://bugs.linaro.org/show_bug.cgi?id=3061 For fix this bug,the function PciIoPciRead of NonDiscoverablePciDeviceDxe should be modified also. Code can also be found in github: https://github.com/hisilicon/OpenPlatformPkg.git branch: rp-osi-bug-v3 Ming Huang (1): Hisilicon D0x: Remove uncacheable attribute from memory resource HOB Platform/Hisilicon/D03/MemoryInitPei/MemoryInit.efi | Bin 90272 -> 90336 bytes Platform/Hisilicon/D05/MemoryInitPei/MemoryInit.efi | Bin 152576 -> 152480 bytes 2 files changed, 0 insertions(+), 0 deletions(-) -- 1.9.1

6 years, 5 months

[PATCH edk2-non-osi v2] Update D03/D05 MemoryInit.efi binary for bug 3419

by Ming Huang

https://bugs.linaro.org/show_bug.cgi?id=3061 For fix this bug,the function PciIoPciRead of NonDiscoverablePciDeviceDxe should be modified also. Code can also be found in github: https://github.com/hisilicon/OpenPlatformPkg.git branch: rp-osi-bug-v2 Ming Huang (1): Hisilicon D0x: Remove uncacheable attribute from memory resource HOB Platform/Hisilicon/D03/MemoryInitPei/MemoryInit.efi | Bin 90272 -> 90336 bytes Platform/Hisilicon/D05/MemoryInitPei/MemoryInit.efi | Bin 152576 -> 152480 bytes 2 files changed, 0 insertions(+), 0 deletions(-) -- 1.9.1

6 years, 5 months

[PATCH edk2-non-osi v1] Update D03/D05 MemoryInit.efi binary for bug 3419

by Ming Huang

https://bugs.linaro.org/show_bug.cgi?id=3061 For fix this bug,the function PciIoPciRead of NonDiscoverablePciDeviceDxe should be modifyed also. Code can also be found in github: https://github.com/hisilicon/OpenPlatformPkg.git branch: rp-osi-bug-v1 Ming Huang (1): Hisilicon D0x: Remove uncacheable attribute from memory resource HOB Platform/Hisilicon/D03/MemoryInitPei/MemoryInit.efi | Bin 90272 -> 90336 bytes Platform/Hisilicon/D05/MemoryInitPei/MemoryInit.efi | Bin 152576 -> 152480 bytes 2 files changed, 0 insertions(+), 0 deletions(-) -- 1.9.1

6 years, 5 months

[edk2][PATCH] MdeModulePkg/NonDiscoverable: fix memory override bug

by Heyi Guo

For PciIoPciRead interface, memory prior to Buffer would be written with zeros if Offset was larger than sizeof (Dev->ConfigSpace), which would cause serious system exception. So we add a pre-check branch to avoid memory override. Cc: Star Zeng <star.zeng(a)intel.com> Cc: Eric Dong <eric.dong(a)intel.com> Cc: Ard Biesheuvel <ard.biesheuvel(a)linaro.org> Cc: Ruiyu Ni <ruiyu.ni(a)intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Heyi Guo <heyi.guo(a)linaro.org> --- .../Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c index c836ad6..0e42ae4 100644 --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c @@ -465,6 +465,11 @@ PciIoPciRead ( Address = (UINT8 *)&Dev->ConfigSpace + Offset; Length = Count << ((UINTN)Width & 0x3); + if (Offset >= sizeof (Dev->ConfigSpace)) { + ZeroMem (Buffer, Length); + return EFI_SUCCESS; + } + if (Offset + Length > sizeof (Dev->ConfigSpace)) { // // Read all zeroes for config space accesses beyond the first -- 1.9.1

6 years, 5 months

[PATCH] ArmPkg/ArmMmuLib: Add new attribute

by Heyi Guo

V1 changes upon RFC version: 1. Rename NONSHARE to NONSHAREABLE per review comments. 2. Place these two uncommon attributes after normal WRITE_BACK attributes to imply they should not be used by default. 3. Add warnning comments. Peicong Li (1): ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHAREABLE ArmPkg/Include/Library/ArmLib.h | 7 +++++++ ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 ++++ 2 files changed, 11 insertions(+) -- 2.7.2.windows.1

6 years, 5 months

[RFC] ArmPkg/ArmMmuLib: Add new attribute WRITE_BACK_NONSHARE

by Heyi Guo

From: Peicong Li <lipeicong(a)huawei.com> Flash region needs to be set as cacheable (write back) to increase performance, if PEI is still XIP on flash or DXE FV is decompressed from flash FV. However some ARM platforms do not support to set flash as inner shareable since flash is not normal DDR memory and it will not respond to cache snoop request, which will causes system hang after MMU is enabled. So we need a new ARM memory region attribute WRITE_BACK_NONSHARE for flash region on these platforms specifically. This attribute will set the region as write back but not inner shared. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Peicong Li <lipeicong(a)huawei.com> Signed-off-by: Heyi Guo <heyi.guo(a)linaro.org> Cc: Leif Lindholm <leif.lindholm(a)linaro.org> Cc: Ard Biesheuvel <ard.biesheuvel(a)linaro.org> --- ArmPkg/Include/Library/ArmLib.h | 2 ++ ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h index 24ffe9f..e43e375 100644 --- a/ArmPkg/Include/Library/ArmLib.h +++ b/ArmPkg/Include/Library/ArmLib.h @@ -39,6 +39,8 @@ typedef enum { ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED = 0, ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED, + ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE, + ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE, ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK, ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK, ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH, diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c index 8bd1c6f..cc10143 100644 --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c @@ -35,6 +35,10 @@ ArmMemoryAttributeToPageAttribute ( ) { switch (Attributes) { + case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHARE: + case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHARE: + return TT_ATTR_INDX_MEMORY_WRITE_BACK; + case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK: case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK: return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE; -- 2.7.2.windows.1

6 years, 5 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

Linaro-uefi November 2017