Re: [edk2] [PATCH v2 3/4] MdePkg/UefiLib: Simplify protocol un/installation abstraction
Ashish: UefiLib implementation simplification doesn't require to change library APIs. UninstallApi() interfaces are not required to be updated. Below Install API can still be kept. I don't think we need to keep the same interfaces for Install and Uninstall APIs. EfiLibUninstallAllDriverProtocols2 ( IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,OPTIONAL IN CONST EFI_COMPONENT_NAME2_PROTOCOL *ComponentName2, OPTIONAL IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, OPTIONAL IN CONST EFI_DRIVER_CONFIGURATION2_PROTOCOL *DriverConfiguration2, OPTIONAL IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnostics,OPTIONAL IN CONST EFI_DRIVER_DIAGNOSTICS2_PROTOCOL *DriverDiagnostics2OPTIONAL ); Thanks Liming >-Original Message- >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >Ashish Singhal >Sent: Saturday, January 05, 2019 7:07 AM >To: edk2-devel@lists.01.org >Cc: Ashish Singhal >Subject: [edk2] [PATCH v2 3/4] MdePkg/UefiLib: Simplify protocol >un/installation abstraction > >Add a helper function to operate upon protocol installation and >uninstallation instead of every function doing it by itself. > >Contributed-under: TianoCore Contribution Agreement 1.1 >Signed-off-by: Ashish Singhal >--- > MdePkg/Include/Library/UefiLib.h | 26 +- > MdePkg/Library/UefiLib/UefiDriverModel.c | 1980 -- > 2 files changed, 270 insertions(+), 1736 deletions(-) > >diff --git a/MdePkg/Include/Library/UefiLib.h >b/MdePkg/Include/Library/UefiLib.h >index 08222d4..fbc9739 100644 >--- a/MdePkg/Include/Library/UefiLib.h >+++ b/MdePkg/Include/Library/UefiLib.h >@@ -1323,7 +1323,10 @@ EfiLibInstallDriverBinding ( > If DriverBinding is NULL, then ASSERT(). > If DriverBinding can not be uninstalled, then ASSERT(). > >+ @param ImageHandle The image handle of the driver. >+ @param SystemTable The EFI System Table that was passed to the >driver's entry point. > @param DriverBindingA Driver Binding Protocol instance that this > driver >produced. >+ @param DriverBindingHandle The handle that DriverBinding is to be >installed onto. > > @retval EFI_SUCCESS The protocol uninstallation successfully >completed. > @retval OthersStatus from gBS- >>UninstallMultipleProtocolInterfaces(). >@@ -1332,7 +1335,10 @@ EfiLibInstallDriverBinding ( > EFI_STATUS > EFIAPI > EfiLibUninstallDriverBinding ( >- IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding >+ IN CONST EFI_HANDLE ImageHandle, >+ IN CONST EFI_SYSTEM_TABLE *SystemTable, >+ IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, >+ IN EFI_HANDLE DriverBindingHandle > ); > > >@@ -1382,7 +1388,10 @@ EfiLibInstallAllDriverProtocols ( > If DriverBinding is NULL, then ASSERT(). > If the uninstallation fails, then ASSERT(). > >+ @param ImageHandle The image handle of the driver. >+ @param SystemTable The EFI System Table that was passed to the >driver's entry point. > @param DriverBindingA Driver Binding Protocol instance that this > driver >produced. >+ @param DriverBindingHandle The handle that DriverBinding is to be >installed onto. > @param ComponentNameA Component Name Protocol instance that >this driver produced. > @param DriverConfiguration A Driver Configuration Protocol instance that >this driver produced. > @param DriverDiagnosticsA Driver Diagnostics Protocol instance that > this >driver produced. >@@ -1394,7 +1403,10 @@ EfiLibInstallAllDriverProtocols ( > EFI_STATUS > EFIAPI > EfiLibUninstallAllDriverProtocols ( >+ IN CONST EFI_HANDLE ImageHandle, >+ IN CONST EFI_SYSTEM_TABLE *SystemTable, > IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, >+ IN EFI_HANDLE DriverBindingHandle, > IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, >OPTIONAL > IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, >OPTIONAL > IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnostics >OPTIONAL >@@ -1442,7 +1454,10 @@ EfiLibInstallDriverBindingComponentName2 ( > If DriverBinding is NULL, then ASSERT(). > If the uninstallation fails, then ASSERT(). > >+ @param ImageHandle The image handle of the driver. >+ @param SystemTable The EFI System Table that was passed to the >driver's entry point. > @param DriverBindingA Driver Binding Protocol instance that this > driver >produced. >+ @param DriverBindingHandle The handle that DriverBinding is to be >installed onto. > @param ComponentNameA Component Name Protocol instance that >this driver produced. > @param ComponentName2 A Component Name 2 Protocol instance that >this driver produced. > >@@ -1453,7 +1468,10 @@ EfiLibInst
[edk2] [Patch v2 0/2] Avoid AP calls PeiServices table.
AP should not use PeiServices. The patch serial fix one issue related to this. This serial also include one patch used to refine the debug message. Eric Dong (2): UefiCpuPkg/RegisterCpuFeaturesLib: Enhance debug message. UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiService. .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 39 +- .../DxeRegisterCpuFeaturesLib.c| 57 +++- .../PeiRegisterCpuFeaturesLib.c| 62 +- .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h | 16 +- 4 files changed, 96 insertions(+), 78 deletions(-) -- 2.15.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch v2 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Enhance debug message.
Enhance debug message format to let them easy to read. Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong --- .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c index 0a74d448c8..624ddee055 100644 --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c @@ -473,8 +473,9 @@ DumpRegisterTableOnProcessor ( case Msr: DEBUG (( DebugPrintErrorLevel, -"Processor: %d: MSR: %x, Bit Start: %d, Bit Length: %d, Value: %lx\r\n", +"Processor: %04d: Index %04d, MSR : %08x, Bit Start: %02d, Bit Length: %02d, Value: %016lx\r\n", ProcessorNumber, +FeatureIndex, RegisterTableEntry->Index, RegisterTableEntry->ValidBitStart, RegisterTableEntry->ValidBitLength, @@ -484,8 +485,9 @@ DumpRegisterTableOnProcessor ( case ControlRegister: DEBUG (( DebugPrintErrorLevel, -"Processor: %d:CR: %x, Bit Start: %d, Bit Length: %d, Value: %lx\r\n", +"Processor: %04d: Index %04d, CR : %08x, Bit Start: %02d, Bit Length: %02d, Value: %016lx\r\n", ProcessorNumber, +FeatureIndex, RegisterTableEntry->Index, RegisterTableEntry->ValidBitStart, RegisterTableEntry->ValidBitLength, @@ -495,8 +497,9 @@ DumpRegisterTableOnProcessor ( case MemoryMapped: DEBUG (( DebugPrintErrorLevel, -"Processor: %d: MMIO: %lx, Bit Start: %d, Bit Length: %d, Value: %lx\r\n", +"Processor: %04d: Index %04d, MMIO : %08lx, Bit Start: %02d, Bit Length: %02d, Value: %016lx\r\n", ProcessorNumber, +FeatureIndex, RegisterTableEntry->Index | LShiftU64 (RegisterTableEntry->HighIndex, 32), RegisterTableEntry->ValidBitStart, RegisterTableEntry->ValidBitLength, @@ -506,8 +509,9 @@ DumpRegisterTableOnProcessor ( case CacheControl: DEBUG (( DebugPrintErrorLevel, -"Processor: %d: CACHE: %x, Bit Start: %d, Bit Length: %d, Value: %lx\r\n", +"Processor: %04d: Index %04d, CACHE: %08lx, Bit Start: %02d, Bit Length: %02d, Value: %016lx\r\n", ProcessorNumber, +FeatureIndex, RegisterTableEntry->Index, RegisterTableEntry->ValidBitStart, RegisterTableEntry->ValidBitLength, @@ -517,8 +521,9 @@ DumpRegisterTableOnProcessor ( case Semaphore: DEBUG (( DebugPrintErrorLevel, -"Processor: %d: Semaphore: Scope Value: %s\r\n", +"Processor: %04d: Index %04d, SEMAP: %s\r\n", ProcessorNumber, +FeatureIndex, mDependTypeStr[MIN ((UINT32)RegisterTableEntry->Value, InvalidDepType)] )); break; @@ -833,7 +838,7 @@ ProgramProcessorRegister ( ApLocation->Thread; DEBUG (( DEBUG_INFO, -"Processor = %lu, Entry Index %lu, Type = %s!\n", +"Processor = %08lu, Index %08lu, Type = %s!\n", (UINT64)ThreadIndex, (UINT64)Index, mRegisterTypeStr[MIN ((REGISTER_TYPE)RegisterTableEntry->RegisterType, InvalidReg)] -- 2.15.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [Patch 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiService.
V2:
1. Initialize CpuFeaturesData->MpService in CpuInitDataInitialize
and make this function been called at the begin of the
initialization.
2. let all other functions use CpuFeaturesData->MpService install
of locate the protocol itself.
V1:
GetProcessorIndex function calls GetMpPpi to get the MP Ppi.
Ap will calls GetProcessorIndex function which final let AP calls
PeiService.
This patch avoid GetProcessorIndex call PeiService.
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1411
Cc: Ruiyu Ni
Cc: Laszlo Ersek
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong
---
.../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 22 +---
.../DxeRegisterCpuFeaturesLib.c| 57 +++-
.../PeiRegisterCpuFeaturesLib.c| 62 +-
.../RegisterCpuFeaturesLib/RegisterCpuFeatures.h | 16 +-
4 files changed, 85 insertions(+), 72 deletions(-)
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 624ddee055..5866e022f0 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -138,7 +138,7 @@ FillProcessorInfo (
**/
VOID
CpuInitDataInitialize (
- IN UINTN NumberOfCpus
+ VOID
)
{
EFI_STATUS Status;
@@ -157,12 +157,22 @@ CpuInitDataInitialize (
ACPI_CPU_DATA*AcpiCpuData;
CPU_STATUS_INFORMATION *CpuStatus;
UINT32 *ValidCoreCountPerPackage;
+ UINTNNumberOfCpus;
+ UINTNNumberOfEnabledProcessors;
Core= 0;
Package = 0;
Thread = 0;
CpuFeaturesData = GetCpuFeaturesData ();
+
+ //
+ // Initialize CpuFeaturesData->MpService as early as possile, so later
function can use it.
+ //
+ CpuFeaturesData->MpService = GetMpService ();
+
+ GetNumberOfProcessor (&NumberOfCpus, &NumberOfEnabledProcessors);
+
CpuFeaturesData->InitOrder = AllocateZeroPool (sizeof
(CPU_FEATURES_INIT_ORDER) * NumberOfCpus);
ASSERT (CpuFeaturesData->InitOrder != NULL);
@@ -409,7 +419,7 @@ CollectProcessorData (
CPU_FEATURES_DATA*CpuFeaturesData;
CpuFeaturesData = (CPU_FEATURES_DATA *)Buffer;
- ProcessorNumber = GetProcessorIndex ();
+ ProcessorNumber = GetProcessorIndex (CpuFeaturesData);
CpuInfo = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo;
//
// collect processor information
@@ -1105,15 +1115,11 @@ CpuFeaturesDetect (
VOID
)
{
- UINTN NumberOfCpus;
- UINTN NumberOfEnabledProcessors;
CPU_FEATURES_DATA *CpuFeaturesData;
CpuFeaturesData = GetCpuFeaturesData();
- GetNumberOfProcessor (&NumberOfCpus, &NumberOfEnabledProcessors);
-
- CpuInitDataInitialize (NumberOfCpus);
+ CpuInitDataInitialize ();
//
// Wakeup all APs for data collection.
@@ -1125,6 +1131,6 @@ CpuFeaturesDetect (
//
CollectProcessorData (CpuFeaturesData);
- AnalysisProcessorFeatures (NumberOfCpus);
+ AnalysisProcessorFeatures (CpuFeaturesData->NumberOfCpus);
}
diff --git
a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
index 926698dc95..2d2095e437 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
@@ -20,7 +20,6 @@
#include "RegisterCpuFeatures.h"
CPU_FEATURES_DATA mCpuFeaturesData = {0};
-EFI_MP_SERVICES_PROTOCOL *mCpuFeaturesMpServices = NULL;
/**
Worker function to get CPU_FEATURES_DATA pointer.
@@ -40,44 +39,44 @@ GetCpuFeaturesData (
@return Pointer to EFI_MP_SERVICES_PROTOCOL.
**/
-EFI_MP_SERVICES_PROTOCOL *
-GetMpProtocol (
+VOID *
+GetMpService (
VOID
)
{
- EFI_STATUS Status;
-
- if (mCpuFeaturesMpServices == NULL) {
-//
-// Get MP Services Protocol
-//
-Status = gBS->LocateProtocol (
- &gEfiMpServiceProtocolGuid,
- NULL,
- (VOID **)&mCpuFeaturesMpServices
- );
-ASSERT_EFI_ERROR (Status);
- }
+ EFI_STATUSStatus;
+ EFI_MP_SERVICES_PROTOCOL *MpServices;
- ASSERT (mCpuFeaturesMpServices != NULL);
- return mCpuFeaturesMpServices;
+ //
+ // Get MP Services Protocol
+ //
+ Status = gBS->LocateProtocol (
+&gEfiMpServiceProtocolGuid,
+NULL,
+(VOID **)&MpServices
+);
+ ASSERT_EFI_ERROR (Status);
+
+ return MpServices;
}
/**
Worker function to return processor index.
+ @param CpuFeaturesDataCpu Feature Data structure.
+
@return The processor index.
**/
UINTN
GetProcessorIndex
Re: [edk2] [PATCH v2 4/4] NetworkPkg/IScsiDxe: Update UEFILib Usage
Reviewed-by: Siyuan Fu > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ashish > Singhal > Sent: Saturday, January 5, 2019 7:07 AM > To: edk2-devel@lists.01.org > Cc: Ashish Singhal > Subject: [edk2] [PATCH v2 4/4] NetworkPkg/IScsiDxe: Update UEFILib Usage > > Update interfaces as exposed by UEFILib for protocol > installation and uninstallation abstraction. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ashish Singhal > --- > NetworkPkg/IScsiDxe/IScsiDriver.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c > b/NetworkPkg/IScsiDxe/IScsiDriver.c > index 8747de7..fa0ea00 100644 > --- a/NetworkPkg/IScsiDxe/IScsiDriver.c > +++ b/NetworkPkg/IScsiDxe/IScsiDriver.c > @@ -1863,14 +1863,20 @@ Error3: > > Error2: >EfiLibUninstallDriverBindingComponentName2 ( > +ImageHandle, > +SystemTable, > &gIScsiIp6DriverBinding, > +NULL, > &gIScsiComponentName, > &gIScsiComponentName2 > ); > > Error1: >EfiLibUninstallDriverBindingComponentName2 ( > +ImageHandle, > +SystemTable, > &gIScsiIp4DriverBinding, > +NULL, > &gIScsiComponentName, > &gIScsiComponentName2 > ); > -- > 2.7.4 > > ___ > 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 2/4] NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
Hi, Ashish The changes to NetworkPkg is good to me. Please add the package maintainer's name to the cc list of the patch mail in future, thanks. Reviewed-by: Fu Siyuan > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ashish > Singhal > Sent: Saturday, January 5, 2019 7:07 AM > To: edk2-devel@lists.01.org > Cc: Ashish Singhal > Subject: [edk2] [PATCH v2 2/4] NetworkPkg/IScsiDxe: Use UEFILib APIs to > uninstall protocols. > > During cleanup in case of initialization failure, some driver > bindings are not installed. Using abstractions in UEFILib takes > care of it. > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1428 > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ashish Singhal > --- > NetworkPkg/IScsiDxe/IScsiDriver.c | 31 +++ > 1 file changed, 11 insertions(+), 20 deletions(-) > > diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c > b/NetworkPkg/IScsiDxe/IScsiDriver.c > index 91176e6..8747de7 100644 > --- a/NetworkPkg/IScsiDxe/IScsiDriver.c > +++ b/NetworkPkg/IScsiDxe/IScsiDriver.c > @@ -1,6 +1,7 @@ > /** @file >The entry point of IScsi driver. > > +Copyright (c) 2019, NVIDIA Corporation. All rights reserved. > Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved. > (C) Copyright 2017 Hewlett Packard Enterprise Development LP > > @@ -1861,28 +1862,18 @@ Error3: > ); > > Error2: > - gBS->UninstallMultipleProtocolInterfaces ( > - gIScsiIp6DriverBinding.DriverBindingHandle, > - &gEfiDriverBindingProtocolGuid, > - &gIScsiIp6DriverBinding, > - &gEfiComponentName2ProtocolGuid, > - &gIScsiComponentName2, > - &gEfiComponentNameProtocolGuid, > - &gIScsiComponentName, > - NULL > - ); > + EfiLibUninstallDriverBindingComponentName2 ( > +&gIScsiIp6DriverBinding, > +&gIScsiComponentName, > +&gIScsiComponentName2 > +); > > Error1: > - gBS->UninstallMultipleProtocolInterfaces ( > - ImageHandle, > - &gEfiDriverBindingProtocolGuid, > - &gIScsiIp4DriverBinding, > - &gEfiComponentName2ProtocolGuid, > - &gIScsiComponentName2, > - &gEfiComponentNameProtocolGuid, > - &gIScsiComponentName, > - NULL > - ); > + EfiLibUninstallDriverBindingComponentName2 ( > +&gIScsiIp4DriverBinding, > +&gIScsiComponentName, > +&gIScsiComponentName2 > +); > >return Status; > } > -- > 2.7.4 > > ___ > 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 3/4] MdePkg/UefiLib: Simplify protocol un/installation abstraction
Hello Liming, I am not touching APIs for Install and am OK keeping Uninstall API same as what I had in patch 1/4. I thought it would be easier for the developer to keep the interface similar to install but I do not have a strong preference either way. If you are OK with the Uninstall API as in patch 1/4, I am OK submitting a new patch where I can squash 1/4 and 3/4 together into a single commit and keep API as in 1/4. Thanks Ashish -Original Message- From: Gao, Liming Sent: Sunday, January 6, 2019 5:33 PM To: Ashish Singhal ; edk2-devel@lists.01.org Subject: RE: [edk2] [PATCH v2 3/4] MdePkg/UefiLib: Simplify protocol un/installation abstraction Ashish: UefiLib implementation simplification doesn't require to change library APIs. UninstallApi() interfaces are not required to be updated. Below Install API can still be kept. I don't think we need to keep the same interfaces for Install and Uninstall APIs. EfiLibUninstallAllDriverProtocols2 ( IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,OPTIONAL IN CONST EFI_COMPONENT_NAME2_PROTOCOL *ComponentName2, OPTIONAL IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, OPTIONAL IN CONST EFI_DRIVER_CONFIGURATION2_PROTOCOL *DriverConfiguration2, OPTIONAL IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnostics,OPTIONAL IN CONST EFI_DRIVER_DIAGNOSTICS2_PROTOCOL *DriverDiagnostics2OPTIONAL ); Thanks Liming >-Original Message- >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >Ashish Singhal >Sent: Saturday, January 05, 2019 7:07 AM >To: edk2-devel@lists.01.org >Cc: Ashish Singhal >Subject: [edk2] [PATCH v2 3/4] MdePkg/UefiLib: Simplify protocol >un/installation abstraction > >Add a helper function to operate upon protocol installation and >uninstallation instead of every function doing it by itself. > >Contributed-under: TianoCore Contribution Agreement 1.1 >Signed-off-by: Ashish Singhal >--- > MdePkg/Include/Library/UefiLib.h | 26 +- > MdePkg/Library/UefiLib/UefiDriverModel.c | 1980 -- > 2 files changed, 270 insertions(+), 1736 deletions(-) > >diff --git a/MdePkg/Include/Library/UefiLib.h >b/MdePkg/Include/Library/UefiLib.h >index 08222d4..fbc9739 100644 >--- a/MdePkg/Include/Library/UefiLib.h >+++ b/MdePkg/Include/Library/UefiLib.h >@@ -1323,7 +1323,10 @@ EfiLibInstallDriverBinding ( > If DriverBinding is NULL, then ASSERT(). > If DriverBinding can not be uninstalled, then ASSERT(). > >+ @param ImageHandle The image handle of the driver. >+ @param SystemTable The EFI System Table that was passed to the >driver's entry point. > @param DriverBindingA Driver Binding Protocol instance that this > driver >produced. >+ @param DriverBindingHandle The handle that DriverBinding is to be >installed onto. > > @retval EFI_SUCCESS The protocol uninstallation successfully >completed. > @retval OthersStatus from gBS- >>UninstallMultipleProtocolInterfaces(). >@@ -1332,7 +1335,10 @@ EfiLibInstallDriverBinding ( > EFI_STATUS > EFIAPI > EfiLibUninstallDriverBinding ( >- IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding >+ IN CONST EFI_HANDLE ImageHandle, >+ IN CONST EFI_SYSTEM_TABLE *SystemTable, >+ IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, >+ IN EFI_HANDLE DriverBindingHandle > ); > > >@@ -1382,7 +1388,10 @@ EfiLibInstallAllDriverProtocols ( > If DriverBinding is NULL, then ASSERT(). > If the uninstallation fails, then ASSERT(). > >+ @param ImageHandle The image handle of the driver. >+ @param SystemTable The EFI System Table that was passed to the >driver's entry point. > @param DriverBindingA Driver Binding Protocol instance that this > driver >produced. >+ @param DriverBindingHandle The handle that DriverBinding is to be >installed onto. > @param ComponentNameA Component Name Protocol instance that >this driver produced. > @param DriverConfiguration A Driver Configuration Protocol instance that >this driver produced. > @param DriverDiagnosticsA Driver Diagnostics Protocol instance that > this >driver produced. >@@ -1394,7 +1403,10 @@ EfiLibInstallAllDriverProtocols ( > EFI_STATUS > EFIAPI > EfiLibUninstallAllDriverProtocols ( >+ IN CONST EFI_HANDLE ImageHandle, >+ IN CONST EFI_SYSTEM_TABLE *SystemTable, > IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, >+ IN EFI_HANDLE DriverBindingHandle, > IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, >OPTIONAL > IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, >OPTIONAL > IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnostics >OPTIONAL >@@ -1442,7 +1454,10 @@ EfiLibInstallDriverBindingComponentName2 ( > I
[edk2] [PATCH] MdePkg/UefiLib: Add a new API GetVariable3
From: jiansonx
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1396
Add a new API GetVariable3, which can return the attributes of a variable
during reading it.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiansong Xu
Cc: Liming Gao GetVariable.
+
+**/
+EFI_STATUS
+EFIAPI
+GetVariable3(
+ IN CONST CHAR16 *Name,
+ IN CONST EFI_GUID *Guid,
+ OUT VOID **Value,
+ OUT UINTN *Size OPTIONAL,
+ OUT UINT32 *Attr OPTIONAL
+ );
+
/**
Returns a pointer to an allocated buffer that contains the best matching
language
from a set of supported languages.
diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
index 39d0ce2b21..475017690c 100644
--- a/MdePkg/Library/UefiLib/UefiLib.c
+++ b/MdePkg/Library/UefiLib/UefiLib.c
@@ -1439,6 +1439,87 @@ GetVariable2 (
return Status;
}
+/** Return the attributes of the variable.
+
+ Returns the status whether get the variable success. The function retrieves
+ variable through the UEFI Runtime Service GetVariable(). The
+ returned buffer is allocated using AllocatePool(). The caller is responsible
+ for freeing this buffer with FreePool(). The attributes are returned if
+ the caller provides a valid Attribute parameter.
+
+ If Name is NULL, then ASSERT().
+ If Guid is NULL, then ASSERT().
+ If Value is NULL, then ASSERT().
+
+ @param[in] Name The pointer to a Null-terminated Unicode string.
+ @param[in] Guid The pointer to an EFI_GUID structure
+ @param[out] Value The buffer point saved the variable info.
+ @param[out] Size The buffer size of the variable.
+ @param[out] Attr The pointer to the variable attributes as found in var
store
+
+ @retval EFI_OUT_OF_RESOURCES Allocate buffer failed.
+ @retval EFI_SUCCESS Find the specified variable.
+ @retval Others Errors Return errors from call to
gRT->GetVariable.
+
+**/
+EFI_STATUS
+EFIAPI
+GetVariable3(
+ IN CONST CHAR16 *Name,
+ IN CONST EFI_GUID *Guid,
+ OUT VOID **Value,
+ OUT UINTN *Size OPTIONAL,
+ OUT UINT32 *Attr OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ UINTN BufferSize;
+
+ ASSERT(Name != NULL && Guid != NULL && Value != NULL);
+
+ //
+ // Try to get the variable size.
+ //
+ BufferSize = 0;
+ *Value = NULL;
+ if (Size != NULL) {
+*Size = 0;
+ }
+
+ if (Attr != NULL) {
+*Attr = 0;
+ }
+
+ Status = gRT->GetVariable((CHAR16 *)Name, (EFI_GUID *)Guid, Attr,
&BufferSize, *Value);
+ if (Status != EFI_BUFFER_TOO_SMALL) {
+return Status;
+ }
+
+ //
+ // Allocate buffer to get the variable.
+ //
+ *Value = AllocatePool(BufferSize);
+ ASSERT(*Value != NULL);
+ if (*Value == NULL) {
+return EFI_OUT_OF_RESOURCES;
+ }
+
+ //
+ // Get the variable data.
+ //
+ Status = gRT->GetVariable((CHAR16 *)Name, (EFI_GUID *)Guid, Attr,
&BufferSize, *Value);
+ if (EFI_ERROR(Status)) {
+FreePool(*Value);
+*Value = NULL;
+ }
+
+ if (Size != NULL) {
+*Size = BufferSize;
+ }
+
+ return Status;
+}
+
/**
Returns a pointer to an allocated buffer that contains the contents of a
variable retrieved through the UEFI Runtime Service GetVariable(). This
--
2.16.2.windows.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 2/2] NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
During cleanup in case of initialization failure, some driver bindings are not installed. Using abstractions in UEFILib takes care of it. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1428 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- NetworkPkg/IScsiDxe/IScsiDriver.c | 31 +++ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c b/NetworkPkg/IScsiDxe/IScsiDriver.c index 91176e6..8747de7 100644 --- a/NetworkPkg/IScsiDxe/IScsiDriver.c +++ b/NetworkPkg/IScsiDxe/IScsiDriver.c @@ -1,6 +1,7 @@ /** @file The entry point of IScsi driver. +Copyright (c) 2019, NVIDIA Corporation. All rights reserved. Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved. (C) Copyright 2017 Hewlett Packard Enterprise Development LP @@ -1861,28 +1862,18 @@ Error3: ); Error2: - gBS->UninstallMultipleProtocolInterfaces ( - gIScsiIp6DriverBinding.DriverBindingHandle, - &gEfiDriverBindingProtocolGuid, - &gIScsiIp6DriverBinding, - &gEfiComponentName2ProtocolGuid, - &gIScsiComponentName2, - &gEfiComponentNameProtocolGuid, - &gIScsiComponentName, - NULL - ); + EfiLibUninstallDriverBindingComponentName2 ( +&gIScsiIp6DriverBinding, +&gIScsiComponentName, +&gIScsiComponentName2 +); Error1: - gBS->UninstallMultipleProtocolInterfaces ( - ImageHandle, - &gEfiDriverBindingProtocolGuid, - &gIScsiIp4DriverBinding, - &gEfiComponentName2ProtocolGuid, - &gIScsiComponentName2, - &gEfiComponentNameProtocolGuid, - &gIScsiComponentName, - NULL - ); + EfiLibUninstallDriverBindingComponentName2 ( +&gIScsiIp4DriverBinding, +&gIScsiComponentName, +&gIScsiComponentName2 +); return Status; } -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 0/2] Provide UEFILib functions for protocol uninstallation.
An issue was seen in IScsiDxe in NetworkPkg where driver cleanup after initialization failure was not done right. Bug 1428 was filed in this regard. As per discussions with Mike, it was also discussed that having UEFILib provide protocol uninstallation abstraction would help to avoid these issues in the future. Bug 1429 was found to track this. The first 2 patches take care of this. Patch number 1 also simplifies the UEFILib protocol installation and uninstallation abstraction by adding a helper function doing operations instead of every public function. Ashish Singhal (2): MdePkg/UefiLib: Abstract driver model protocol uninstallation NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols. MdePkg/Include/Library/UefiLib.h | 103 +++ MdePkg/Library/UefiLib/UefiDriverModel.c | 1186 -- NetworkPkg/IScsiDxe/IScsiDriver.c| 31 +- 3 files changed, 435 insertions(+), 885 deletions(-) -- 2.7.4 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 1/2] MdePkg/UefiLib: Abstract driver model protocol uninstallation
Provided functions in UEFILib that abstract driver model protocol uninstallation. This helps drivers to install and uninstall protocols using a library to keep things seemless. Also, add a helper function to operate upon protocol installation and uninstallation instead of every function doing it by itself. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1429 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ashish Singhal --- MdePkg/Include/Library/UefiLib.h | 103 +++ MdePkg/Library/UefiLib/UefiDriverModel.c | 1186 -- 2 files changed, 424 insertions(+), 865 deletions(-) diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h index 468bffc..08222d4 100644 --- a/MdePkg/Include/Library/UefiLib.h +++ b/MdePkg/Include/Library/UefiLib.h @@ -12,6 +12,7 @@ of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is defined, then debug and assert related macros wrapped by it are the NULL implementations. +Copyright (c) 2019, NVIDIA Corporation. All rights reserved. Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License that accompanies this distribution. @@ -1283,6 +1284,7 @@ AsciiPrintXY ( ... ); + /** Installs and completes the initialization of a Driver Binding Protocol instance. @@ -1316,6 +1318,25 @@ EfiLibInstallDriverBinding ( /** + Uninstalls a Driver Binding Protocol instance. + + If DriverBinding is NULL, then ASSERT(). + If DriverBinding can not be uninstalled, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol uninstallation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallDriverBinding ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding + ); + + +/** Installs and completes the initialization of a Driver Binding Protocol instance and optionally installs the Component Name, Driver Configuration and Driver Diagnostics Protocols. @@ -1354,6 +1375,31 @@ EfiLibInstallAllDriverProtocols ( ); +/** + Uninstalls a Driver Binding Protocol instance and optionally uninstalls the + Component Name, Driver Configuration and Driver Diagnostics Protocols. + + If DriverBinding is NULL, then ASSERT(). + If the uninstallation fails, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param ComponentNameA Component Name Protocol instance that this driver produced. + @param DriverConfiguration A Driver Configuration Protocol instance that this driver produced. + @param DriverDiagnosticsA Driver Diagnostics Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol uninstallation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallAllDriverProtocols ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL + IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL *DriverConfiguration, OPTIONAL + IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnosticsOPTIONAL + ); + /** Installs Driver Binding Protocol with optional Component Name and Component Name 2 Protocols. @@ -1391,6 +1437,29 @@ EfiLibInstallDriverBindingComponentName2 ( /** + Uninstalls Driver Binding Protocol with optional Component Name and Component Name 2 Protocols. + + If DriverBinding is NULL, then ASSERT(). + If the uninstallation fails, then ASSERT(). + + @param DriverBindingA Driver Binding Protocol instance that this driver produced. + @param ComponentNameA Component Name Protocol instance that this driver produced. + @param ComponentName2 A Component Name 2 Protocol instance that this driver produced. + + @retval EFI_SUCCESS The protocol installation successfully completed. + @retval OthersStatus from gBS->UninstallMultipleProtocolInterfaces(). + +**/ +EFI_STATUS +EFIAPI +EfiLibUninstallDriverBindingComponentName2 ( + IN EFI_DRIVER_BINDING_PROTOCOL *DriverBinding, + IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName, OPTIONAL + IN CONST EFI_COMPONENT_NAME2_PROTOCOL *ComponentName2 OPTIONAL + ); + + +/** Installs Driver Binding Protocol with optional Component Name, Component Name 2, Driver Configuration, Driver Configuration 2, Driver Diagnostics, and Driver Diagnostics 2 Protocols. @@ -1434,6 +1503,40 @@ EfiLibInstallAllDriverProtocols2 ( IN CONST EFI_DRIVER_DIAGNOSTICS2_PROTOCOL *DriverDiagnostics2
[edk2] [PATCH] BaseTools/tools_def ARM GCC5: disable LTO for ASLC invocations
GCC for 32-bit ARM chokes on .aslc files when running with LTO enabled. Since LTO has no benefit whatsoever here, just disable it. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- BaseTools/Conf/tools_def.template | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template index f7eb87af14c2..e68cfd9a4997 100755 --- a/BaseTools/Conf/tools_def.template +++ b/BaseTools/Conf/tools_def.template @@ -5145,7 +5145,7 @@ RELEASE_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -flto -Os *_GCC5_ARM_RC_PATH = ENV(GCC5_ARM_PREFIX)objcopy *_GCC5_ARM_ARCHCC_FLAGS = -mthumb -*_GCC5_ARM_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) +*_GCC5_ARM_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -fno-lto *_GCC5_ARM_ASLDLINK_FLAGS= DEF(GCC5_ARM_ASLDLINK_FLAGS) *_GCC5_ARM_ASM_FLAGS = DEF(GCC5_ARM_ASM_FLAGS) *_GCC5_ARM_DLINK2_FLAGS = DEF(GCC5_ARM_DLINK2_FLAGS) -- 2.20.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] ArmPkg/ArmMmuLib ARM: disregard high memory when setting permissions
Ignore calls to ArmSetMemoryAttributes () when the region described
is outside of the 32-bit addressable range. This memory is not
mapped in the first place, and the current code does not deal with
the high bits correctly, resulting in hangs.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel
---
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 4
1 file changed, 4 insertions(+)
diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index 3b3b20aa9b78..bffab83d4fd0 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -744,6 +744,10 @@ ArmSetMemoryAttributes (
UINT64ChunkLength;
BOOLEAN FlushTlbs;
+ if (BaseAddress > (UINT64)MAX_ADDRESS - Length + 1) {
+return EFI_UNSUPPORTED;
+ }
+
if (Length == 0) {
return EFI_SUCCESS;
}
--
2.20.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 4/5] ArmPkg/ArmMmuLib AARCH64: add support for read-only page tables
As a hardening measure, implement support for remapping all page
tables read-only at a certain point during the boot (end of DXE
is the most appropriate trigger).
This should make it a lot more difficult to take advantage of
write exploits to defeat authentication checks, since the
attacker can no longer manipulate the page tables directly.
To allow the page tables to still be manipulated, make use
of the existing code to manipulate live entries: this drops
into assembler with interrupts off, and disables the MMU for
a brief moment to avoid causing TLB conflicts. Since page
tables are writable with the MMU off, we can reuse this code
to still manipulate the page tables after we updated the CPU
mappings to be read-only.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel
---
ArmPkg/Include/Library/ArmMmuLib.h | 6 +
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 119 ++--
ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 8 ++
3 files changed, 123 insertions(+), 10 deletions(-)
diff --git a/ArmPkg/Include/Library/ArmMmuLib.h
b/ArmPkg/Include/Library/ArmMmuLib.h
index d2725810f1c6..f0832b91bf17 100644
--- a/ArmPkg/Include/Library/ArmMmuLib.h
+++ b/ArmPkg/Include/Library/ArmMmuLib.h
@@ -70,4 +70,10 @@ ArmSetMemoryAttributes (
IN UINT64Attributes
);
+VOID
+EFIAPI
+MapAllPageTablesReadOnly (
+ VOID
+ );
+
#endif
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index b59c081a7e49..cefaad9961ea 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -28,6 +28,8 @@
// We use this index definition to define an invalid block entry
#define TT_ATTR_INDX_INVALID((UINT32)~0)
+STATIC BOOLEAN mReadOnlyPageTables;
+
STATIC
UINT64
ArmMemoryAttributeToPageAttribute (
@@ -137,6 +139,9 @@ ReplaceLiveEntry (
IN UINT64 Address
)
{
+ if (*Entry == Value) {
+return;
+ }
if (!ArmMmuEnabled ()) {
*Entry = Value;
} else {
@@ -181,7 +186,8 @@ GetBlockEntryListFromAddress (
IN UINT64RegionStart,
OUT UINTN*TableLevel,
IN OUT UINT64*BlockEntrySize,
- OUT UINT64 **LastBlockEntry
+ OUT UINT64 **LastBlockEntry,
+ OUT BOOLEAN *NewPageTablesAllocated
)
{
UINTN RootTableLevel;
@@ -292,6 +298,8 @@ GetBlockEntryListFromAddress (
return NULL;
}
+*NewPageTablesAllocated = TRUE;
+
// Populate the newly created lower level table
SubTableBlockEntry = TranslationTable;
for (Index = 0; Index < TT_ENTRY_COUNT; Index++) {
@@ -316,10 +324,18 @@ GetBlockEntryListFromAddress (
return NULL;
}
+*NewPageTablesAllocated = TRUE;
+
ZeroMem (TranslationTable, TT_ENTRY_COUNT * sizeof(UINT64));
// Fill the new BlockEntry with the TranslationTable
-*BlockEntry = ((UINTN)TranslationTable &
TT_ADDRESS_MASK_DESCRIPTION_TABLE) | TT_TYPE_TABLE_ENTRY;
+if (!mReadOnlyPageTables) {
+ *BlockEntry = (UINTN)TranslationTable | TT_TYPE_TABLE_ENTRY;
+} else {
+ ReplaceLiveEntry (BlockEntry,
+(UINTN)TranslationTable | TT_TYPE_TABLE_ENTRY,
+RegionStart);
+}
}
}
}
@@ -345,7 +361,8 @@ UpdateRegionMapping (
IN UINT64 RegionStart,
IN UINT64 RegionLength,
IN UINT64 Attributes,
- IN UINT64 BlockEntryMask
+ IN UINT64 BlockEntryMask,
+ OUT BOOLEAN *ReadOnlyRemapDone
)
{
UINT32 Type;
@@ -353,6 +370,7 @@ UpdateRegionMapping (
UINT64 *LastBlockEntry;
UINT64 BlockEntrySize;
UINTN TableLevel;
+ BOOLEAN NewPageTablesAllocated;
// Ensure the Length is aligned on 4KB boundary
if ((RegionLength == 0) || ((RegionLength & (SIZE_4KB - 1)) != 0)) {
@@ -360,11 +378,13 @@ UpdateRegionMapping (
return EFI_INVALID_PARAMETER;
}
+ NewPageTablesAllocated = FALSE;
do {
// Get the first Block Entry that matches the Virtual Address and also the
information on the Table Descriptor
// such as the the size of the Block Entry and the address of the last
BlockEntry of the Table Descriptor
BlockEntrySize = RegionLength;
-BlockEntry = GetBlockEntryListFromAddress (RootTable, RegionStart,
&TableLevel, &BlockEntrySize, &LastBlockEntry);
+BlockEntry = GetBlockEntryListFromAddress (RootTable, RegionStart,
+ &TableLevel, &BlockEntrySize, &LastBlockEntry, &NewPageTablesAllocated);
if (BlockEntry == NULL) {
// GetBlockEntryListFromAddress() return NULL when it fails to allocate
new pages from the Translation Tables
return EFI_OUT_OF_RESOURCES;
@@ -378,10 +398,16 @@ UpdateRegionMapping (
do {
// Fill the Block Entry with attribute and output block address
- *BlockEntry &= BlockEntryMask;
- *BlockEntry |= (RegionStart & TT_ADDRESS_MASK_BLOCK_ENTRY) | Attribut
[edk2] [PATCH 2/5] ArmPkg/ArmMmuLib AARCH64: get rid of needless TLB invalidation
Currently, we always invalidate the TLBs entirely after making
any modification to the page tables. Now that we have introduced
strict memory permissions in quite a number of places, such
modifications occur much more often, and it is better for performance
to flush only those TLB entries that are actually affected by
the changes.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel
---
ArmPkg/Include/Library/ArmMmuLib.h | 3 ++-
ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S| 6 +++---
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 16 +++-
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 14 --
4 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/ArmPkg/Include/Library/ArmMmuLib.h
b/ArmPkg/Include/Library/ArmMmuLib.h
index fb7fd006417c..d2725810f1c6 100644
--- a/ArmPkg/Include/Library/ArmMmuLib.h
+++ b/ArmPkg/Include/Library/ArmMmuLib.h
@@ -59,7 +59,8 @@ VOID
EFIAPI
ArmReplaceLiveTranslationEntry (
IN UINT64 *Entry,
- IN UINT64 Value
+ IN UINT64 Value,
+ IN UINT64 Address
);
EFI_STATUS
diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
index b7173e00b039..175fb58206b6 100644
--- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
@@ -124,15 +124,15 @@ ASM_FUNC(ArmSetMAIR)
// IN VOID *MVA// X1
// );
ASM_FUNC(ArmUpdateTranslationTableEntry)
- dc civac, x0 // Clean and invalidate data line
- dsb sy
+ dsb nshst
+ lsr x1, x1, #12
EL1_OR_EL2_OR_EL3(x0)
1: tlbivaae1, x1 // TLB Invalidate VA , EL1
b 4f
2: tlbivae2, x1 // TLB Invalidate VA , EL2
b 4f
3: tlbivae3, x1 // TLB Invalidate VA , EL3
-4: dsb sy
+4: dsb nsh
isb
ret
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index d66df3e17a02..e1fabfcbea14 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -129,13 +129,14 @@ STATIC
VOID
ReplaceLiveEntry (
IN UINT64 *Entry,
- IN UINT64 Value
+ IN UINT64 Value,
+ IN UINT64 Address
)
{
if (!ArmMmuEnabled ()) {
*Entry = Value;
} else {
-ArmReplaceLiveTranslationEntry (Entry, Value);
+ArmReplaceLiveTranslationEntry (Entry, Value, Address);
}
}
@@ -296,7 +297,8 @@ GetBlockEntryListFromAddress (
// Fill the BlockEntry with the new TranslationTable
ReplaceLiveEntry (BlockEntry,
- ((UINTN)TranslationTable & TT_ADDRESS_MASK_DESCRIPTION_TABLE) |
TableAttributes | TT_TYPE_TABLE_ENTRY);
+ (UINTN)TranslationTable | TableAttributes | TT_TYPE_TABLE_ENTRY,
+ RegionStart);
}
} else {
if (IndexLevel != PageLevel) {
@@ -375,6 +377,8 @@ UpdateRegionMapping (
*BlockEntry &= BlockEntryMask;
*BlockEntry |= (RegionStart & TT_ADDRESS_MASK_BLOCK_ENTRY) | Attributes
| Type;
+ ArmUpdateTranslationTableEntry (BlockEntry, (VOID *)RegionStart);
+
// Go to the next BlockEntry
RegionStart += BlockEntrySize;
RegionLength -= BlockEntrySize;
@@ -487,9 +491,6 @@ ArmSetMemoryAttributes (
return Status;
}
- // Invalidate all TLB entries so changes are synced
- ArmInvalidateTlb ();
-
return EFI_SUCCESS;
}
@@ -512,9 +513,6 @@ SetMemoryRegionAttribute (
return Status;
}
- // Invalidate all TLB entries so changes are synced
- ArmInvalidateTlb ();
-
return EFI_SUCCESS;
}
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
index 90192df24f55..d40c19b2e3e5 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S
@@ -32,11 +32,12 @@
dmb sy
dcivac, x0
- // flush the TLBs
+ // flush translations for the target address from the TLBs
+ lsr x2, x2, #12
.if \el == 1
- tlbi vmalle1
+ tlbi vaae1, x2
.else
- tlbi alle\el
+ tlbi vae\el, x2
.endif
dsb sy
@@ -48,12 +49,13 @@
//VOID
//ArmReplaceLiveTranslationEntry (
// IN UINT64 *Entry,
-// IN UINT64 Value
+// IN UINT64 Value,
+// IN UINT64 Address
// )
ASM_FUNC(ArmReplaceLiveTranslationEntry)
// disable interrupts
- mrs x2, daif
+ mrs x4, daif
msr daifset, #0xf
isb
@@ -69,7 +71,7 @@ ASM_FUNC(ArmReplaceLiveTranslationEntry)
b 4f
3:__replace_entry 3
-4:msr daif, x2
+4:msr daif, x4
ret
ASM_GLOBAL ASM_PFX(ArmReplaceLiveTranslationEntrySize)
--
2.20.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/5] memory/MMU hardening for AArch64
Now that we are getting more serious about implementing secure boot on ARM systems, by putting the code that manipulated the variable store in a secure partition, it makes sense to give some attention to the non-secure side as well, since having secure authenticated variables is moot if we can just nop out the authentication check in the image loader. Patch #1 fixes an issue in ArmMmuLib that is triggered when HeapGuard is enabled. Patch #2 optimizes TLB management so that we don't flush all of it every time. This is a performance optimization as well as a hardening measure, since it makes it more difficult to trigger a flush of all TLBs, which is needed when abusing a write exploit to change memory permissions. Patch #3 is a prerequisite for enabling StackGuard and HeapGuard, which make use of the EFI_MEMORY_RP attribute and this wasn't wired up yet. Patch #4 adds support to ArmMmuLib to remap all page tables read-only, so that they are no longer vulnerable to rogue writes. Patch #5 enables the feature added in #4 at EndOfDxe. Ard Biesheuvel (5): ArmPkg/ArmMmuLib AARCH64: fix out of bounds access ArmPkg/ArmMmuLib AARCH64: get rid of needless TLB invalidation ArmPkg/ArmMmuLib AARCH64: implement support for EFI_MEMORY_RP permissions ArmPkg/ArmMmuLib AARCH64: add support for read-only page tables ArmPkg/CpuDxe: switch to read-only page tables at EndOfDxe ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 5 +- ArmPkg/Drivers/CpuDxe/CpuDxe.c | 23 +++ ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 1 + ArmPkg/Include/Library/ArmMmuLib.h | 9 +- ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S| 6 +- ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 149 +--- ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S | 14 +- ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 8 ++ 8 files changed, 181 insertions(+), 34 deletions(-) -- 2.20.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/5] ArmPkg/ArmMmuLib AARCH64: fix out of bounds access
Take care not to dereference BlockEntry if it may be pointing past
the end of the page table we are manipulating. It is only a read,
and thus harmless, but HeapGuard triggers on it so let's fix it.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel
---
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index e41044142ef4..d66df3e17a02 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -382,7 +382,7 @@ UpdateRegionMapping (
// Break the inner loop when next block is a table
// Rerun GetBlockEntryListFromAddress to avoid page table memory leak
- if (TableLevel != 3 &&
+ if (TableLevel != 3 && BlockEntry <= LastBlockEntry &&
(*BlockEntry & TT_TYPE_MASK) == TT_TYPE_TABLE_ENTRY) {
break;
}
--
2.20.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 3/5] ArmPkg/ArmMmuLib AARCH64: implement support for EFI_MEMORY_RP permissions
Wire up the access flag (AF) page table attribute to the EFI_MEMORY_RP
permission attribute, so that attempts to read from such a region will
trigger an access flag fault.
Note that this is a stronger notion than just read protection, since
it now implies that any write or execute attempt is trapped as well.
However, this does not really matter in practice since we never assume
that a read protected page is writable or executable, and StackGuard
and HeapGuard (which are the primary users of this facility) certainly
don't care.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel
---
ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 5 +++--
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 14 +++---
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
index 3e216c7cb235..e62e3fa87112 100644
--- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
@@ -223,8 +223,9 @@ EfiAttributeToArmAttribute (
ArmAttributes = TT_ATTR_INDX_MASK;
}
- // Set the access flag to match the block attributes
- ArmAttributes |= TT_AF;
+ if ((EfiAttributes & EFI_MEMORY_RP) == 0) {
+ArmAttributes |= TT_AF;
+ }
// Determine protection attributes
if (EfiAttributes & EFI_MEMORY_RO) {
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index e1fabfcbea14..b59c081a7e49 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -102,6 +102,10 @@ PageAttributeToGcdAttribute (
GcdAttributes |= EFI_MEMORY_XP;
}
+ if ((PageAttributes & TT_AF) == 0) {
+GcdAttributes |= EFI_MEMORY_RP;
+ }
+
return GcdAttributes;
}
@@ -451,7 +455,11 @@ GcdAttributeToPageAttribute (
PageAttributes |= TT_AP_RO_RO;
}
- return PageAttributes | TT_AF;
+ if ((GcdAttributes & EFI_MEMORY_RP) == 0) {
+PageAttributes |= TT_AF;
+ }
+
+ return PageAttributes;
}
EFI_STATUS
@@ -474,9 +482,9 @@ ArmSetMemoryAttributes (
// No memory type was set in Attributes, so we are going to update the
// permissions only.
//
-PageAttributes &= TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK;
+PageAttributes &= TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK | TT_AF;
PageAttributeMask = ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK |
- TT_PXN_MASK | TT_XN_MASK);
+ TT_PXN_MASK | TT_XN_MASK | TT_AF);
}
TranslationTable = ArmGetTTBR0BaseAddress ();
--
2.20.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 5/5] ArmPkg/CpuDxe: switch to read-only page tables at EndOfDxe
Register for the EndOfDxe event, and use it to invoke the new
ArmMmuLib code that remaps all page tables as read-only. This
should limit the impact of arbitrary write exploits, since they
can no longer be abused to modify tightened memory permissions.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel
---
ArmPkg/Drivers/CpuDxe/CpuDxe.c | 23
ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 1 +
2 files changed, 24 insertions(+)
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
index 5e923d45b715..11f4a2ccf5c8 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
@@ -238,6 +238,17 @@ InitializeDma (
CpuArchProtocol->DmaBufferAlignment = ArmCacheWritebackGranule ();
}
+STATIC
+VOID
+EFIAPI
+OnEndOfDxe (
+ IN EFI_EVENTEvent,
+ IN VOID *Context
+ )
+{
+ MapAllPageTablesReadOnly ();
+}
+
EFI_STATUS
CpuDxeInitialize (
IN EFI_HANDLE ImageHandle,
@@ -246,6 +257,7 @@ CpuDxeInitialize (
{
EFI_STATUS Status;
EFI_EVENTIdleLoopEvent;
+ EFI_EVENTEndOfDxeEvent;
InitializeExceptions (&mCpu);
@@ -285,5 +297,16 @@ CpuDxeInitialize (
);
ASSERT_EFI_ERROR (Status);
+
+ Status = gBS->CreateEventEx (
+ EVT_NOTIFY_SIGNAL,
+ TPL_CALLBACK,
+ OnEndOfDxe,
+ NULL,
+ &gEfiEndOfDxeEventGroupGuid,
+ &EndOfDxeEvent
+ );
+ ASSERT_EFI_ERROR (Status);
+
return Status;
}
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
index c32d2cb9c7d4..0788a2ab27c0 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
@@ -63,6 +63,7 @@
[Guids]
gEfiDebugImageInfoTableGuid
+ gEfiEndOfDxeEventGroupGuid
gArmMpCoreInfoGuid
gIdleLoopEventGuid
gEfiVectorHandoffTableGuid
--
2.20.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch v2 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Enhance debug message.
On 1/7/2019 9:05 AM, Eric Dong wrote: Enhance debug message format to let them easy to read. Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong --- .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c index 0a74d448c8..624ddee055 100644 --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c @@ -473,8 +473,9 @@ DumpRegisterTableOnProcessor ( case Msr: DEBUG (( DebugPrintErrorLevel, -"Processor: %d: MSR: %x, Bit Start: %d, Bit Length: %d, Value: %lx\r\n", +"Processor: %04d: Index %04d, MSR : %08x, Bit Start: %02d, Bit Length: %02d, Value: %016lx\r\n", ProcessorNumber, +FeatureIndex, RegisterTableEntry->Index, RegisterTableEntry->ValidBitStart, RegisterTableEntry->ValidBitLength, @@ -484,8 +485,9 @@ DumpRegisterTableOnProcessor ( case ControlRegister: DEBUG (( DebugPrintErrorLevel, -"Processor: %d:CR: %x, Bit Start: %d, Bit Length: %d, Value: %lx\r\n", +"Processor: %04d: Index %04d, CR : %08x, Bit Start: %02d, Bit Length: %02d, Value: %016lx\r\n", ProcessorNumber, +FeatureIndex, RegisterTableEntry->Index, RegisterTableEntry->ValidBitStart, RegisterTableEntry->ValidBitLength, @@ -495,8 +497,9 @@ DumpRegisterTableOnProcessor ( case MemoryMapped: DEBUG (( DebugPrintErrorLevel, -"Processor: %d: MMIO: %lx, Bit Start: %d, Bit Length: %d, Value: %lx\r\n", +"Processor: %04d: Index %04d, MMIO : %08lx, Bit Start: %02d, Bit Length: %02d, Value: %016lx\r\n", ProcessorNumber, +FeatureIndex, RegisterTableEntry->Index | LShiftU64 (RegisterTableEntry->HighIndex, 32), RegisterTableEntry->ValidBitStart, RegisterTableEntry->ValidBitLength, @@ -506,8 +509,9 @@ DumpRegisterTableOnProcessor ( case CacheControl: DEBUG (( DebugPrintErrorLevel, -"Processor: %d: CACHE: %x, Bit Start: %d, Bit Length: %d, Value: %lx\r\n", +"Processor: %04d: Index %04d, CACHE: %08lx, Bit Start: %02d, Bit Length: %02d, Value: %016lx\r\n", ProcessorNumber, +FeatureIndex, RegisterTableEntry->Index, RegisterTableEntry->ValidBitStart, RegisterTableEntry->ValidBitLength, @@ -517,8 +521,9 @@ DumpRegisterTableOnProcessor ( case Semaphore: DEBUG (( DebugPrintErrorLevel, -"Processor: %d: Semaphore: Scope Value: %s\r\n", +"Processor: %04d: Index %04d, SEMAP: %s\r\n", ProcessorNumber, +FeatureIndex, mDependTypeStr[MIN ((UINT32)RegisterTableEntry->Value, InvalidDepType)] )); break; @@ -833,7 +838,7 @@ ProgramProcessorRegister ( ApLocation->Thread; DEBUG (( DEBUG_INFO, -"Processor = %lu, Entry Index %lu, Type = %s!\n", +"Processor = %08lu, Index %08lu, Type = %s!\n", (UINT64)ThreadIndex, (UINT64)Index, mRegisterTypeStr[MIN ((REGISTER_TYPE)RegisterTableEntry->RegisterType, InvalidReg)] Reviewed-by: Ray Ni -- Thanks, Ray ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
