* [PATCH 3/4] IntelFrameworkModulePkg/Csm: Update CSM code to temporarily bypass NULL pointer detection if enabled.
@ 2017-09-13 8:07 Wang, Jian J
0 siblings, 0 replies; 3+ messages in thread
From: Wang, Jian J @ 2017-09-13 8:07 UTC (permalink / raw)
To: edk2-devel
CSM code has to access memory below 4096 (BDA, int vector, etc.). If NULL pointer detection is enabled, the page 0 must be enabled temporarily before accessing it and disabled again afterwards. Otherwise page fault will be triggered.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Justen, Jordan L <jordan.l.justen@intel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Wolman, Ayellet <ayellet.wolman@intel.com>
Suggested-by: Wolman, Ayellet <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wang, Jian J <jian.j.wang@intel.com>
---
.../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c | 10 +++-
.../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h | 18 +++++++
.../Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf | 2 +
.../Csm/LegacyBiosDxe/LegacyBda.c | 4 ++
.../Csm/LegacyBiosDxe/LegacyBios.c | 55 ++++++++++++++++++----
.../Csm/LegacyBiosDxe/LegacyBiosDxe.inf | 2 +
.../Csm/LegacyBiosDxe/LegacyBiosInterface.h | 23 +++++++++
.../Csm/LegacyBiosDxe/LegacyBootSupport.c | 33 ++++++++++---
.../Csm/LegacyBiosDxe/LegacyPci.c | 17 ++++++-
IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c | 41 ++++++++++------
10 files changed, 173 insertions(+), 32 deletions(-)
diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
index 7308523ad8..96148ae367 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
@@ -248,7 +248,7 @@ BiosKeyboardDriverBindingStart (
//
// Allocate the private device structure
//
- BiosKeyboardPrivate = (BIOS_KEYBOARD_DEV *) AllocateZeroPool (sizeof (BIOS_KEYBOARD_DEV));
+ BiosKeyboardPrivate = (BIOS_KEYBOARD_DEV *) AllocateZeroPool (sizeof (BIOS_KEYBOARD_DEV));
if (NULL == BiosKeyboardPrivate) {
Status = EFI_OUT_OF_RESOURCES;
goto Done;
@@ -281,6 +281,9 @@ BiosKeyboardDriverBindingStart (
BiosKeyboardPrivate->SimpleTextInputEx.UnregisterKeyNotify = BiosKeyboardUnregisterKeyNotify;
InitializeListHead (&BiosKeyboardPrivate->NotifyList);
+ Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **) &BiosKeyboardPrivate->Cpu);
+ ASSERT_EFI_ERROR(Status);
+
//
// Report that the keyboard is being enabled
//
@@ -1842,7 +1845,9 @@ BiosKeyboardTimerHandler (
//
// Clear the CTRL and ALT BDA flag
//
- KbFlag1 = *((UINT8 *) (UINTN) 0x417); // read the STATUS FLAGS 1
+ DISABLE_NULL_DETECTION(BiosKeyboardPrivate);
+
+ KbFlag1 = *((UINT8 *) (UINTN) 0x417); // read the STATUS FLAGS 1
KbFlag2 = *((UINT8 *) (UINTN) 0x418); // read STATUS FLAGS 2
DEBUG_CODE (
@@ -1916,6 +1921,7 @@ BiosKeyboardTimerHandler (
KbFlag1 &= ~0x0C;
*((UINT8 *) (UINTN) 0x417) = KbFlag1;
+ ENABLE_NULL_DETECTION(BiosKeyboardPrivate);
//
// 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..b717ef676b 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
@@ -26,6 +26,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Protocol/IsaIo.h>
#include <Protocol/DevicePath.h>
#include <Protocol/Ps2Policy.h>
+#include <Protocol/Cpu.h>
#include <Library/DebugLib.h>
#include <Library/UefiLib.h>
@@ -212,6 +213,7 @@ typedef struct {
EFI_HANDLE Handle;
EFI_LEGACY_BIOS_PROTOCOL *LegacyBios;
EFI_ISA_IO_PROTOCOL *IsaIo;
+ EFI_CPU_ARCH_PROTOCOL *Cpu;
EFI_SIMPLE_TEXT_INPUT_PROTOCOL SimpleTextIn;
EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL SimpleTextInputEx;
UINT16 DataRegisterAddress;
@@ -242,6 +244,22 @@ typedef struct {
BIOS_KEYBOARD_DEV_SIGNATURE \
)
+//
+// CSM needs to access memory between 0-4095, which will cause page fault exception
+// if NULL pointer detection mechanism is enabled. Following macros can be used
+// to disable/enable NULL pointer detection before/after accessing those memory.
+//
+#define NULL_POINTER_DETECTION_ENABLED ((PcdGet8(PcdNullPointerDetectionPropertyMask) & BIT0) != 0)
+#define DISABLE_NULL_DETECTION(Instance) \
+ if (NULL_POINTER_DETECTION_ENABLED && (Instance)->Cpu != NULL) { \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, 0); \
+ }
+
+#define ENABLE_NULL_DETECTION(Instance) \
+ if (NULL_POINTER_DETECTION_ENABLED && (Instance)->Cpu != NULL) { \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, EFI_MEMORY_RP); \
+ }
+
//
// Global Variables
//
diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
index 4d4536466c..4291a10123 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
@@ -67,12 +67,14 @@
gEfiSimpleTextInputExProtocolGuid ## BY_START
gEfiLegacyBiosProtocolGuid ## CONSUMES
gEfiPs2PolicyProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiCpuArchProtocolGuid ## SOMETIMES_CONSUMES
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE ## CONSUMES
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFastPS2Detection ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES
[UserExtensions.TianoCore."ExtraFiles"]
KeyboardDxeExtra.uni
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
index c45d5d4c3e..e7cee4b8a3 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
@@ -34,6 +34,8 @@ LegacyBiosInitBda (
BDA_STRUC *Bda;
UINT8 *Ebda;
+ DISABLE_NULL_DETECTION(Private);
+
Bda = (BDA_STRUC *) ((UINTN) 0x400);
Ebda = (UINT8 *) ((UINTN) 0x9fc00);
@@ -62,5 +64,7 @@ LegacyBiosInitBda (
*Ebda = 0x01;
+ ENABLE_NULL_DETECTION(Private);
+
return EFI_SUCCESS;
}
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
index 3ead2d9828..c3ef542ea3 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
@@ -40,6 +40,7 @@ VOID *mRuntimeSmbiosEntryPoint = NULL;
EFI_PHYSICAL_ADDRESS mReserveSmbiosEntryPoint = 0;
EFI_PHYSICAL_ADDRESS mStructureTableAddress = 0;
UINTN mStructureTablePages = 0;
+BOOLEAN mEndOfDxe = FALSE;
/**
Do an AllocatePages () of type AllocateMaxAddress for EfiBootServicesCode
@@ -765,6 +766,26 @@ InstallSmbiosEventCallback (
}
}
+/**
+ Callback function to toggle EndOfDxe status. NULL pointer detection needs this
+ status to decide if it's necessary to change attributes of page 0.
+
+ @param Event Event whose notification function is being invoked.
+ @param Context The pointer to the notification function's context,
+ which is implementation-dependent.
+
+**/
+VOID
+EFIAPI
+ToggleEndOfDxeStatus (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ mEndOfDxe = TRUE;
+ return;
+}
+
/**
Install Driver to produce Legacy BIOS protocol.
@@ -802,6 +823,7 @@ LegacyBiosInstall (
UINT64 Length;
UINT8 *SecureBoot;
EFI_EVENT InstallSmbiosEvent;
+ EFI_EVENT EndOfDxeEvent;
//
// Load this driver's image to memory
@@ -964,8 +986,10 @@ LegacyBiosInstall (
// Initialize region from 0x0000 to 4k. This initializes interrupt vector
// range.
//
- gBS->SetMem ((VOID *) ClearPtr, 0x400, INITIAL_VALUE_BELOW_1K);
- ZeroMem ((VOID *) ((UINTN)ClearPtr + 0x400), 0xC00);
+ DISABLE_NULL_DETECTION(Private);
+ gBS->SetMem ((VOID *) ClearPtr, 0x400, INITIAL_VALUE_BELOW_1K);
+ ZeroMem ((VOID *) ((UINTN)ClearPtr + 0x400), 0xC00);
+ ENABLE_NULL_DETECTION(Private);
//
// Allocate pages for OPROM usage
@@ -1104,12 +1128,14 @@ LegacyBiosInstall (
//
// Save Unexpected interrupt vector so can restore it just prior to boot
//
- BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
- Private->BiosUnexpectedInt = BaseVectorMaster[0];
- IntRedirCode = (UINT32) (UINTN) Private->IntThunk->InterruptRedirectionCode;
- for (Index = 0; Index < 8; Index++) {
- BaseVectorMaster[Index] = (EFI_SEGMENT (IntRedirCode + Index * 4) << 16) | EFI_OFFSET (IntRedirCode + Index * 4);
- }
+ DISABLE_NULL_DETECTION(Private);
+ BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
+ Private->BiosUnexpectedInt = BaseVectorMaster[0];
+ IntRedirCode = (UINT32) (UINTN) Private->IntThunk->InterruptRedirectionCode;
+ for (Index = 0; Index < 8; Index++) {
+ BaseVectorMaster[Index] = (EFI_SEGMENT (IntRedirCode + Index * 4) << 16) | EFI_OFFSET (IntRedirCode + Index * 4);
+ }
+ ENABLE_NULL_DETECTION(Private);
//
// Save EFI value
//
@@ -1133,6 +1159,19 @@ LegacyBiosInstall (
);
ASSERT_EFI_ERROR (Status);
+ //
+ // Create callback to update status of EndOfDxe, which is needed by NULL pointer detection
+ //
+ Status = gBS->CreateEventEx (
+ EVT_NOTIFY_SIGNAL,
+ TPL_NOTIFY,
+ ToggleEndOfDxeStatus,
+ NULL,
+ &gEfiEndOfDxeEventGroupGuid,
+ &EndOfDxeEvent
+ );
+ ASSERT_EFI_ERROR (Status);
+
//
// Make a new handle and install the protocol
//
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
index 48473a0713..10dc392800 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
@@ -108,6 +108,7 @@
gEfiDiskInfoIdeInterfaceGuid ## SOMETIMES_CONSUMES ##GUID #Used in LegacyBiosBuildIdeData() to assure device is a disk
gEfiSmbiosTableGuid ## SOMETIMES_CONSUMES ##SystemTable
gEfiLegacyBiosGuid ## SOMETIMES_CONSUMES ##GUID #Used in LegacyBiosInstallVgaRom() to locate handle buffer
+ gEfiEndOfDxeEventGroupGuid
[Guids.IA32]
gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ##SystemTable
@@ -147,6 +148,7 @@
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdHighPmmMemorySize ## CONSUMES
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdOpromReservedMemoryBase ## CONSUMES
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdOpromReservedMemorySize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES
[Depex]
gEfiLegacyRegion2ProtocolGuid AND gEfiLegacyInterruptProtocolGuid AND gEfiLegacyBiosPlatformProtocolGuid AND gEfiLegacy8259ProtocolGuid AND gEfiGenericMemTestProtocolGuid AND gEfiCpuArchProtocolGuid AND gEfiTimerArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
index fe9dd7463a..9d479309a4 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
@@ -108,6 +108,27 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define NORMALIZE_EFI_SEGMENT(_Adr) (UINT16) (((UINTN) (_Adr)) >> 4)
#define NORMALIZE_EFI_OFFSET(_Adr) (UINT16) (((UINT16) ((UINTN) (_Adr))) & 0xf)
+//
+// CSM needs to access memory between 0-4095, which will cause page fault exception
+// if NULL pointer detection mechanism is enabled. Following macros can be used
+// to disable/enable NULL pointer detection before/after accessing those memory.
+//
+#define NULL_DETECTION_ENABLED \
+ ( ((mEndOfDxe == FALSE) && ((PcdGet8(PcdNullPointerDetectionPropertyMask) & BIT0) == BIT0)) \
+ || ((mEndOfDxe == TRUE) && ((PcdGet8(PcdNullPointerDetectionPropertyMask) & (BIT0|BIT7)) == BIT0)) \
+ )
+#define DISABLE_NULL_DETECTION(Instance) \
+ if (NULL_DETECTION_ENABLED) { \
+ DEBUG((DEBUG_INFO, "%a(): disable NULL detection\r\n", __func__)); \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, 0); \
+ }
+
+#define ENABLE_NULL_DETECTION(Instance) \
+ if (NULL_DETECTION_ENABLED) { \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, EFI_MEMORY_RP); \
+ DEBUG((DEBUG_INFO, "%a(): enable NULL detection\r\n", __func__)); \
+ }
+
//
// Trace defines
//
@@ -509,6 +530,8 @@ extern BBS_TABLE *mBbsTable;
extern EFI_GENERIC_MEMORY_TEST_PROTOCOL *gGenMemoryTest;
+extern BOOLEAN mEndOfDxe;
+
#define PORT_70 0x70
#define PORT_71 0x71
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
index 1e098b3726..d381c2f735 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
@@ -1073,8 +1073,10 @@ GenericLegacyBoot (
// Use 182/10 to avoid floating point math.
//
LocalTime = (LocalTime * 182) / 10;
- BdaPtr = (UINT32 *) (UINTN)0x46C;
- *BdaPtr = LocalTime;
+ DISABLE_NULL_DETECTION(Private);
+ BdaPtr = (UINT32 *) (UINTN)0x46C;
+ *BdaPtr = LocalTime;
+ ENABLE_NULL_DETECTION(Private);
//
// Shadow PCI ROMs. We must do this near the end since this will kick
@@ -1320,6 +1322,7 @@ GenericLegacyBoot (
// set of TIANO vectors) or takes it over.
//
//
+ DISABLE_NULL_DETECTION(Private);
BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
for (Index = 0; Index < 8; Index++) {
Private->ThunkSavedInt[Index] = BaseVectorMaster[Index];
@@ -1327,6 +1330,7 @@ GenericLegacyBoot (
BaseVectorMaster[Index] = (UINT32) (Private->BiosUnexpectedInt);
}
}
+ ENABLE_NULL_DETECTION(Private);
ZeroMem (&Regs, sizeof (EFI_IA32_REGISTER_SET));
Regs.X.AX = Legacy16Boot;
@@ -1340,10 +1344,12 @@ GenericLegacyBoot (
0
);
+ DISABLE_NULL_DETECTION(Private);
BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
for (Index = 0; Index < 8; Index++) {
BaseVectorMaster[Index] = Private->ThunkSavedInt[Index];
}
+ ENABLE_NULL_DETECTION(Private);
}
Private->LegacyBootEntered = TRUE;
if ((mBootMode == BOOT_LEGACY_OS) || (mBootMode == BOOT_UNCONVENTIONAL_DEVICE)) {
@@ -1731,9 +1737,11 @@ LegacyBiosBuildE820 (
//
// First entry is 0 to (640k - EBDA)
//
- E820Table[0].BaseAddr = 0;
- E820Table[0].Length = (UINT64) ((*(UINT16 *) (UINTN)0x40E) << 4);
- E820Table[0].Type = EfiAcpiAddressRangeMemory;
+ DISABLE_NULL_DETECTION(Private);
+ E820Table[0].BaseAddr = 0;
+ E820Table[0].Length = (UINT64) ((*(UINT16 *) (UINTN)0x40E) << 4);
+ E820Table[0].Type = EfiAcpiAddressRangeMemory;
+ ENABLE_NULL_DETECTION(Private);
//
// Second entry is (640k - EBDA) to 640k
@@ -1967,6 +1975,8 @@ LegacyBiosCompleteBdaBeforeBoot (
UINT16 MachineConfig;
DEVICE_PRODUCER_DATA_HEADER *SioPtr;
+ DISABLE_NULL_DETECTION(Private);
+
Bda = (BDA_STRUC *) ((UINTN) 0x400);
MachineConfig = 0;
@@ -2025,6 +2035,8 @@ LegacyBiosCompleteBdaBeforeBoot (
MachineConfig = (UINT16) (MachineConfig + 0x00 + 0x02 + (SioPtr->MousePresent * 0x04));
Bda->MachineConfig = MachineConfig;
+ ENABLE_NULL_DETECTION(Private);
+
return EFI_SUCCESS;
}
@@ -2049,15 +2061,20 @@ LegacyBiosUpdateKeyboardLedStatus (
UINT8 LocalLeds;
EFI_IA32_REGISTER_SET Regs;
- Bda = (BDA_STRUC *) ((UINTN) 0x400);
-
Private = LEGACY_BIOS_INSTANCE_FROM_THIS (This);
+
+ DISABLE_NULL_DETECTION(Private);
+
+ Bda = (BDA_STRUC *) ((UINTN) 0x400);
LocalLeds = Leds;
Bda->LedStatus = (UINT8) ((Bda->LedStatus &~0x07) | LocalLeds);
LocalLeds = (UINT8) (LocalLeds << 4);
Bda->ShiftStatus = (UINT8) ((Bda->ShiftStatus &~0x70) | LocalLeds);
LocalLeds = (UINT8) (Leds & 0x20);
Bda->KeyboardStatus = (UINT8) ((Bda->KeyboardStatus &~0x20) | LocalLeds);
+
+ ENABLE_NULL_DETECTION(Private);
+
//
// Call into Legacy16 code to allow it to do any processing
//
@@ -2102,7 +2119,9 @@ LegacyBiosCompleteStandardCmosBeforeBoot (
// to large capacity drives
// CMOS 14 = BDA 40:10 plus bit 3(display enabled)
//
+ DISABLE_NULL_DETECTION(Private);
Bda = (UINT8)(*((UINT8 *)((UINTN)0x410)) | BIT3);
+ ENABLE_NULL_DETECTION(Private);
//
// Force display enabled
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
index 8ffdf0c1ff..2ca5dddf00 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
@@ -2279,6 +2279,7 @@ LegacyBiosInstallRom (
UINTN Function;
EFI_IA32_REGISTER_SET Regs;
UINT8 VideoMode;
+ UINT8 OldVideoMode;
EFI_TIME BootTime;
UINT32 *BdaPtr;
UINT32 LocalTime;
@@ -2299,6 +2300,7 @@ LegacyBiosInstallRom (
Device = 0;
Function = 0;
VideoMode = 0;
+ OldVideoMode = 0;
PhysicalAddress = 0;
MaxRomAddr = PcdGet32 (PcdEndOpromShadowAddress);
@@ -2401,6 +2403,8 @@ LegacyBiosInstallRom (
// 2. BBS compliants drives will not change 40:75 until boot time.
// 3. Onboard IDE controllers will change 40:75
//
+ DISABLE_NULL_DETECTION(Private);
+
LocalDiskStart = (UINT8) ((*(UINT8 *) ((UINTN) 0x475)) + 0x80);
if ((Private->Disk4075 + 0x80) < LocalDiskStart) {
//
@@ -2426,6 +2430,9 @@ LegacyBiosInstallRom (
//
VideoMode = *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE));
}
+
+ ENABLE_NULL_DETECTION(Private);
+
//
// Notify the platform that we are about to scan the ROM
//
@@ -2466,9 +2473,11 @@ LegacyBiosInstallRom (
// Multiply result by 18.2 for number of ticks since midnight.
// Use 182/10 to avoid floating point math.
//
+ DISABLE_NULL_DETECTION(Private);
LocalTime = (LocalTime * 182) / 10;
BdaPtr = (UINT32 *) ((UINTN) 0x46C);
*BdaPtr = LocalTime;
+ ENABLE_NULL_DETECTION(Private);
//
// Pass in handoff data
@@ -2564,7 +2573,11 @@ LegacyBiosInstallRom (
//
// Set mode settings since PrepareToScanRom may change mode
//
- if (VideoMode != *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE))) {
+ DISABLE_NULL_DETECTION(Private);
+ OldVideoMode = *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE));
+ ENABLE_NULL_DETECTION(Private);
+
+ if (VideoMode != OldVideoMode) {
//
// The active video mode is changed, restore it to original mode.
//
@@ -2604,7 +2617,9 @@ LegacyBiosInstallRom (
}
}
+ DISABLE_NULL_DETECTION(Private);
LocalDiskEnd = (UINT8) ((*(UINT8 *) ((UINTN) 0x475)) + 0x80);
+ ENABLE_NULL_DETECTION(Private);
//
// Allow platform to perform any required actions after the
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
index 3d9a8b9649..50f6247a99 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
@@ -57,7 +57,11 @@ LegacyBiosInt86 (
IN EFI_IA32_REGISTER_SET *Regs
)
{
- UINT32 *VectorBase;
+ UINT16 Segment;
+ UINT16 Offset;
+ LEGACY_BIOS_INSTANCE *Private;
+
+ Private = LEGACY_BIOS_INSTANCE_FROM_THIS (This);
Regs->X.Flags.Reserved1 = 1;
Regs->X.Flags.Reserved2 = 0;
@@ -72,12 +76,15 @@ LegacyBiosInt86 (
// The base address of legacy interrupt vector table is 0.
// We use this base address to get the legacy interrupt handler.
//
- VectorBase = 0;
+ DISABLE_NULL_DETECTION(Private);
+ Segment = (UINT16)(((UINT32 *)0)[BiosInt] >> 16);
+ Offset = (UINT16)((UINT32 *)0)[BiosInt];
+ ENABLE_NULL_DETECTION(Private);
return InternalLegacyBiosFarCall (
This,
- (UINT16) ((VectorBase)[BiosInt] >> 16),
- (UINT16) (VectorBase)[BiosInt],
+ Segment,
+ Offset,
Regs,
&Regs->X.Flags,
sizeof (Regs->X.Flags)
@@ -288,16 +295,22 @@ InternalLegacyBiosFarCall (
// EBDA base address, if the current EBDA base address is smaller, it indicates
// PcdEbdaReservedMemorySize should be adjusted to larger for more OPROMs.
//
- DEBUG_CODE (
- {
- UINTN EbdaBaseAddress;
- UINTN ReservedEbdaBaseAddress;
-
- EbdaBaseAddress = (*(UINT16 *) (UINTN) 0x40E) << 4;
- ReservedEbdaBaseAddress = CONVENTIONAL_MEMORY_TOP - PcdGet32 (PcdEbdaReservedMemorySize);
- ASSERT (ReservedEbdaBaseAddress <= EbdaBaseAddress);
- }
- );
+ if (!NULL_DETECTION_ENABLED) {
+ //
+ // Only do following if NULL pointer detection is not enabled, because it cannot
+ // be disabled at this time due to current TPL(=TPL_HIGH_LEVEL).
+ //
+ DEBUG_CODE (
+ {
+ UINTN EbdaBaseAddress;
+ UINTN ReservedEbdaBaseAddress;
+
+ EbdaBaseAddress = (*(UINT16 *) (UINTN) 0x40E) << 4;
+ ReservedEbdaBaseAddress = CONVENTIONAL_MEMORY_TOP - PcdGet32 (PcdEbdaReservedMemorySize);
+ ASSERT (ReservedEbdaBaseAddress <= EbdaBaseAddress);
+ }
+ );
+ }
if (Stack != NULL && StackSize != 0) {
//
--
2.14.1.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 3/4] IntelFrameworkModulePkg/Csm: Update CSM code to temporarily bypass NULL pointer detection if enabled.
2017-09-13 9:25 ` [PATCH 0/4] Implement NULL pointer detection feature for special pool Wang, Jian J
@ 2017-09-13 9:25 ` Wang, Jian J
2017-09-13 16:33 ` Johnson, Brian (EXL - Eagan)
0 siblings, 1 reply; 3+ messages in thread
From: Wang, Jian J @ 2017-09-13 9:25 UTC (permalink / raw)
To: edk2-devel
Cc: Jiewen Yao, Eric Dong, Star Zeng, Laszlo Ersek, Justen, Jordan L,
Kinney, Michael D, Wolman, Ayellet
CSM code has to access memory below 4096 (BDA, int vector, etc.). If NULL pointer detection is enabled, the page 0 must be enabled temporarily before accessing it and disabled again afterwards. Otherwise page fault will be triggered.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Justen, Jordan L <jordan.l.justen@intel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Wolman, Ayellet <ayellet.wolman@intel.com>
Suggested-by: Wolman, Ayellet <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wang, Jian J <jian.j.wang@intel.com>
---
.../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c | 10 +++-
.../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h | 18 +++++++
.../Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf | 2 +
.../Csm/LegacyBiosDxe/LegacyBda.c | 4 ++
.../Csm/LegacyBiosDxe/LegacyBios.c | 55 ++++++++++++++++++----
.../Csm/LegacyBiosDxe/LegacyBiosDxe.inf | 2 +
.../Csm/LegacyBiosDxe/LegacyBiosInterface.h | 23 +++++++++
.../Csm/LegacyBiosDxe/LegacyBootSupport.c | 33 ++++++++++---
.../Csm/LegacyBiosDxe/LegacyPci.c | 17 ++++++-
IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c | 41 ++++++++++------
10 files changed, 173 insertions(+), 32 deletions(-)
diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
index 7308523ad8..96148ae367 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
@@ -248,7 +248,7 @@ BiosKeyboardDriverBindingStart (
//
// Allocate the private device structure
//
- BiosKeyboardPrivate = (BIOS_KEYBOARD_DEV *) AllocateZeroPool (sizeof (BIOS_KEYBOARD_DEV));
+ BiosKeyboardPrivate = (BIOS_KEYBOARD_DEV *) AllocateZeroPool (sizeof (BIOS_KEYBOARD_DEV));
if (NULL == BiosKeyboardPrivate) {
Status = EFI_OUT_OF_RESOURCES;
goto Done;
@@ -281,6 +281,9 @@ BiosKeyboardDriverBindingStart (
BiosKeyboardPrivate->SimpleTextInputEx.UnregisterKeyNotify = BiosKeyboardUnregisterKeyNotify;
InitializeListHead (&BiosKeyboardPrivate->NotifyList);
+ Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **) &BiosKeyboardPrivate->Cpu);
+ ASSERT_EFI_ERROR(Status);
+
//
// Report that the keyboard is being enabled
//
@@ -1842,7 +1845,9 @@ BiosKeyboardTimerHandler (
//
// Clear the CTRL and ALT BDA flag
//
- KbFlag1 = *((UINT8 *) (UINTN) 0x417); // read the STATUS FLAGS 1
+ DISABLE_NULL_DETECTION(BiosKeyboardPrivate);
+
+ KbFlag1 = *((UINT8 *) (UINTN) 0x417); // read the STATUS FLAGS 1
KbFlag2 = *((UINT8 *) (UINTN) 0x418); // read STATUS FLAGS 2
DEBUG_CODE (
@@ -1916,6 +1921,7 @@ BiosKeyboardTimerHandler (
KbFlag1 &= ~0x0C;
*((UINT8 *) (UINTN) 0x417) = KbFlag1;
+ ENABLE_NULL_DETECTION(BiosKeyboardPrivate);
//
// 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..b717ef676b 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
@@ -26,6 +26,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Protocol/IsaIo.h>
#include <Protocol/DevicePath.h>
#include <Protocol/Ps2Policy.h>
+#include <Protocol/Cpu.h>
#include <Library/DebugLib.h>
#include <Library/UefiLib.h>
@@ -212,6 +213,7 @@ typedef struct {
EFI_HANDLE Handle;
EFI_LEGACY_BIOS_PROTOCOL *LegacyBios;
EFI_ISA_IO_PROTOCOL *IsaIo;
+ EFI_CPU_ARCH_PROTOCOL *Cpu;
EFI_SIMPLE_TEXT_INPUT_PROTOCOL SimpleTextIn;
EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL SimpleTextInputEx;
UINT16 DataRegisterAddress;
@@ -242,6 +244,22 @@ typedef struct {
BIOS_KEYBOARD_DEV_SIGNATURE \
)
+//
+// CSM needs to access memory between 0-4095, which will cause page fault exception
+// if NULL pointer detection mechanism is enabled. Following macros can be used
+// to disable/enable NULL pointer detection before/after accessing those memory.
+//
+#define NULL_POINTER_DETECTION_ENABLED ((PcdGet8(PcdNullPointerDetectionPropertyMask) & BIT0) != 0)
+#define DISABLE_NULL_DETECTION(Instance) \
+ if (NULL_POINTER_DETECTION_ENABLED && (Instance)->Cpu != NULL) { \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, 0); \
+ }
+
+#define ENABLE_NULL_DETECTION(Instance) \
+ if (NULL_POINTER_DETECTION_ENABLED && (Instance)->Cpu != NULL) { \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, EFI_MEMORY_RP); \
+ }
+
//
// Global Variables
//
diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
index 4d4536466c..4291a10123 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
@@ -67,12 +67,14 @@
gEfiSimpleTextInputExProtocolGuid ## BY_START
gEfiLegacyBiosProtocolGuid ## CONSUMES
gEfiPs2PolicyProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiCpuArchProtocolGuid ## SOMETIMES_CONSUMES
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE ## CONSUMES
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFastPS2Detection ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES
[UserExtensions.TianoCore."ExtraFiles"]
KeyboardDxeExtra.uni
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
index c45d5d4c3e..e7cee4b8a3 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
@@ -34,6 +34,8 @@ LegacyBiosInitBda (
BDA_STRUC *Bda;
UINT8 *Ebda;
+ DISABLE_NULL_DETECTION(Private);
+
Bda = (BDA_STRUC *) ((UINTN) 0x400);
Ebda = (UINT8 *) ((UINTN) 0x9fc00);
@@ -62,5 +64,7 @@ LegacyBiosInitBda (
*Ebda = 0x01;
+ ENABLE_NULL_DETECTION(Private);
+
return EFI_SUCCESS;
}
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
index 3ead2d9828..c3ef542ea3 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
@@ -40,6 +40,7 @@ VOID *mRuntimeSmbiosEntryPoint = NULL;
EFI_PHYSICAL_ADDRESS mReserveSmbiosEntryPoint = 0;
EFI_PHYSICAL_ADDRESS mStructureTableAddress = 0;
UINTN mStructureTablePages = 0;
+BOOLEAN mEndOfDxe = FALSE;
/**
Do an AllocatePages () of type AllocateMaxAddress for EfiBootServicesCode
@@ -765,6 +766,26 @@ InstallSmbiosEventCallback (
}
}
+/**
+ Callback function to toggle EndOfDxe status. NULL pointer detection needs this
+ status to decide if it's necessary to change attributes of page 0.
+
+ @param Event Event whose notification function is being invoked.
+ @param Context The pointer to the notification function's context,
+ which is implementation-dependent.
+
+**/
+VOID
+EFIAPI
+ToggleEndOfDxeStatus (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ mEndOfDxe = TRUE;
+ return;
+}
+
/**
Install Driver to produce Legacy BIOS protocol.
@@ -802,6 +823,7 @@ LegacyBiosInstall (
UINT64 Length;
UINT8 *SecureBoot;
EFI_EVENT InstallSmbiosEvent;
+ EFI_EVENT EndOfDxeEvent;
//
// Load this driver's image to memory
@@ -964,8 +986,10 @@ LegacyBiosInstall (
// Initialize region from 0x0000 to 4k. This initializes interrupt vector
// range.
//
- gBS->SetMem ((VOID *) ClearPtr, 0x400, INITIAL_VALUE_BELOW_1K);
- ZeroMem ((VOID *) ((UINTN)ClearPtr + 0x400), 0xC00);
+ DISABLE_NULL_DETECTION(Private);
+ gBS->SetMem ((VOID *) ClearPtr, 0x400, INITIAL_VALUE_BELOW_1K);
+ ZeroMem ((VOID *) ((UINTN)ClearPtr + 0x400), 0xC00);
+ ENABLE_NULL_DETECTION(Private);
//
// Allocate pages for OPROM usage
@@ -1104,12 +1128,14 @@ LegacyBiosInstall (
//
// Save Unexpected interrupt vector so can restore it just prior to boot
//
- BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
- Private->BiosUnexpectedInt = BaseVectorMaster[0];
- IntRedirCode = (UINT32) (UINTN) Private->IntThunk->InterruptRedirectionCode;
- for (Index = 0; Index < 8; Index++) {
- BaseVectorMaster[Index] = (EFI_SEGMENT (IntRedirCode + Index * 4) << 16) | EFI_OFFSET (IntRedirCode + Index * 4);
- }
+ DISABLE_NULL_DETECTION(Private);
+ BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
+ Private->BiosUnexpectedInt = BaseVectorMaster[0];
+ IntRedirCode = (UINT32) (UINTN) Private->IntThunk->InterruptRedirectionCode;
+ for (Index = 0; Index < 8; Index++) {
+ BaseVectorMaster[Index] = (EFI_SEGMENT (IntRedirCode + Index * 4) << 16) | EFI_OFFSET (IntRedirCode + Index * 4);
+ }
+ ENABLE_NULL_DETECTION(Private);
//
// Save EFI value
//
@@ -1133,6 +1159,19 @@ LegacyBiosInstall (
);
ASSERT_EFI_ERROR (Status);
+ //
+ // Create callback to update status of EndOfDxe, which is needed by NULL pointer detection
+ //
+ Status = gBS->CreateEventEx (
+ EVT_NOTIFY_SIGNAL,
+ TPL_NOTIFY,
+ ToggleEndOfDxeStatus,
+ NULL,
+ &gEfiEndOfDxeEventGroupGuid,
+ &EndOfDxeEvent
+ );
+ ASSERT_EFI_ERROR (Status);
+
//
// Make a new handle and install the protocol
//
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
index 48473a0713..10dc392800 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
@@ -108,6 +108,7 @@
gEfiDiskInfoIdeInterfaceGuid ## SOMETIMES_CONSUMES ##GUID #Used in LegacyBiosBuildIdeData() to assure device is a disk
gEfiSmbiosTableGuid ## SOMETIMES_CONSUMES ##SystemTable
gEfiLegacyBiosGuid ## SOMETIMES_CONSUMES ##GUID #Used in LegacyBiosInstallVgaRom() to locate handle buffer
+ gEfiEndOfDxeEventGroupGuid
[Guids.IA32]
gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ##SystemTable
@@ -147,6 +148,7 @@
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdHighPmmMemorySize ## CONSUMES
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdOpromReservedMemoryBase ## CONSUMES
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdOpromReservedMemorySize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES
[Depex]
gEfiLegacyRegion2ProtocolGuid AND gEfiLegacyInterruptProtocolGuid AND gEfiLegacyBiosPlatformProtocolGuid AND gEfiLegacy8259ProtocolGuid AND gEfiGenericMemTestProtocolGuid AND gEfiCpuArchProtocolGuid AND gEfiTimerArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
index fe9dd7463a..9d479309a4 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
@@ -108,6 +108,27 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define NORMALIZE_EFI_SEGMENT(_Adr) (UINT16) (((UINTN) (_Adr)) >> 4)
#define NORMALIZE_EFI_OFFSET(_Adr) (UINT16) (((UINT16) ((UINTN) (_Adr))) & 0xf)
+//
+// CSM needs to access memory between 0-4095, which will cause page fault exception
+// if NULL pointer detection mechanism is enabled. Following macros can be used
+// to disable/enable NULL pointer detection before/after accessing those memory.
+//
+#define NULL_DETECTION_ENABLED \
+ ( ((mEndOfDxe == FALSE) && ((PcdGet8(PcdNullPointerDetectionPropertyMask) & BIT0) == BIT0)) \
+ || ((mEndOfDxe == TRUE) && ((PcdGet8(PcdNullPointerDetectionPropertyMask) & (BIT0|BIT7)) == BIT0)) \
+ )
+#define DISABLE_NULL_DETECTION(Instance) \
+ if (NULL_DETECTION_ENABLED) { \
+ DEBUG((DEBUG_INFO, "%a(): disable NULL detection\r\n", __func__)); \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, 0); \
+ }
+
+#define ENABLE_NULL_DETECTION(Instance) \
+ if (NULL_DETECTION_ENABLED) { \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, EFI_MEMORY_RP); \
+ DEBUG((DEBUG_INFO, "%a(): enable NULL detection\r\n", __func__)); \
+ }
+
//
// Trace defines
//
@@ -509,6 +530,8 @@ extern BBS_TABLE *mBbsTable;
extern EFI_GENERIC_MEMORY_TEST_PROTOCOL *gGenMemoryTest;
+extern BOOLEAN mEndOfDxe;
+
#define PORT_70 0x70
#define PORT_71 0x71
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
index 1e098b3726..d381c2f735 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
@@ -1073,8 +1073,10 @@ GenericLegacyBoot (
// Use 182/10 to avoid floating point math.
//
LocalTime = (LocalTime * 182) / 10;
- BdaPtr = (UINT32 *) (UINTN)0x46C;
- *BdaPtr = LocalTime;
+ DISABLE_NULL_DETECTION(Private);
+ BdaPtr = (UINT32 *) (UINTN)0x46C;
+ *BdaPtr = LocalTime;
+ ENABLE_NULL_DETECTION(Private);
//
// Shadow PCI ROMs. We must do this near the end since this will kick
@@ -1320,6 +1322,7 @@ GenericLegacyBoot (
// set of TIANO vectors) or takes it over.
//
//
+ DISABLE_NULL_DETECTION(Private);
BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
for (Index = 0; Index < 8; Index++) {
Private->ThunkSavedInt[Index] = BaseVectorMaster[Index];
@@ -1327,6 +1330,7 @@ GenericLegacyBoot (
BaseVectorMaster[Index] = (UINT32) (Private->BiosUnexpectedInt);
}
}
+ ENABLE_NULL_DETECTION(Private);
ZeroMem (&Regs, sizeof (EFI_IA32_REGISTER_SET));
Regs.X.AX = Legacy16Boot;
@@ -1340,10 +1344,12 @@ GenericLegacyBoot (
0
);
+ DISABLE_NULL_DETECTION(Private);
BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
for (Index = 0; Index < 8; Index++) {
BaseVectorMaster[Index] = Private->ThunkSavedInt[Index];
}
+ ENABLE_NULL_DETECTION(Private);
}
Private->LegacyBootEntered = TRUE;
if ((mBootMode == BOOT_LEGACY_OS) || (mBootMode == BOOT_UNCONVENTIONAL_DEVICE)) {
@@ -1731,9 +1737,11 @@ LegacyBiosBuildE820 (
//
// First entry is 0 to (640k - EBDA)
//
- E820Table[0].BaseAddr = 0;
- E820Table[0].Length = (UINT64) ((*(UINT16 *) (UINTN)0x40E) << 4);
- E820Table[0].Type = EfiAcpiAddressRangeMemory;
+ DISABLE_NULL_DETECTION(Private);
+ E820Table[0].BaseAddr = 0;
+ E820Table[0].Length = (UINT64) ((*(UINT16 *) (UINTN)0x40E) << 4);
+ E820Table[0].Type = EfiAcpiAddressRangeMemory;
+ ENABLE_NULL_DETECTION(Private);
//
// Second entry is (640k - EBDA) to 640k
@@ -1967,6 +1975,8 @@ LegacyBiosCompleteBdaBeforeBoot (
UINT16 MachineConfig;
DEVICE_PRODUCER_DATA_HEADER *SioPtr;
+ DISABLE_NULL_DETECTION(Private);
+
Bda = (BDA_STRUC *) ((UINTN) 0x400);
MachineConfig = 0;
@@ -2025,6 +2035,8 @@ LegacyBiosCompleteBdaBeforeBoot (
MachineConfig = (UINT16) (MachineConfig + 0x00 + 0x02 + (SioPtr->MousePresent * 0x04));
Bda->MachineConfig = MachineConfig;
+ ENABLE_NULL_DETECTION(Private);
+
return EFI_SUCCESS;
}
@@ -2049,15 +2061,20 @@ LegacyBiosUpdateKeyboardLedStatus (
UINT8 LocalLeds;
EFI_IA32_REGISTER_SET Regs;
- Bda = (BDA_STRUC *) ((UINTN) 0x400);
-
Private = LEGACY_BIOS_INSTANCE_FROM_THIS (This);
+
+ DISABLE_NULL_DETECTION(Private);
+
+ Bda = (BDA_STRUC *) ((UINTN) 0x400);
LocalLeds = Leds;
Bda->LedStatus = (UINT8) ((Bda->LedStatus &~0x07) | LocalLeds);
LocalLeds = (UINT8) (LocalLeds << 4);
Bda->ShiftStatus = (UINT8) ((Bda->ShiftStatus &~0x70) | LocalLeds);
LocalLeds = (UINT8) (Leds & 0x20);
Bda->KeyboardStatus = (UINT8) ((Bda->KeyboardStatus &~0x20) | LocalLeds);
+
+ ENABLE_NULL_DETECTION(Private);
+
//
// Call into Legacy16 code to allow it to do any processing
//
@@ -2102,7 +2119,9 @@ LegacyBiosCompleteStandardCmosBeforeBoot (
// to large capacity drives
// CMOS 14 = BDA 40:10 plus bit 3(display enabled)
//
+ DISABLE_NULL_DETECTION(Private);
Bda = (UINT8)(*((UINT8 *)((UINTN)0x410)) | BIT3);
+ ENABLE_NULL_DETECTION(Private);
//
// Force display enabled
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
index 8ffdf0c1ff..2ca5dddf00 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
@@ -2279,6 +2279,7 @@ LegacyBiosInstallRom (
UINTN Function;
EFI_IA32_REGISTER_SET Regs;
UINT8 VideoMode;
+ UINT8 OldVideoMode;
EFI_TIME BootTime;
UINT32 *BdaPtr;
UINT32 LocalTime;
@@ -2299,6 +2300,7 @@ LegacyBiosInstallRom (
Device = 0;
Function = 0;
VideoMode = 0;
+ OldVideoMode = 0;
PhysicalAddress = 0;
MaxRomAddr = PcdGet32 (PcdEndOpromShadowAddress);
@@ -2401,6 +2403,8 @@ LegacyBiosInstallRom (
// 2. BBS compliants drives will not change 40:75 until boot time.
// 3. Onboard IDE controllers will change 40:75
//
+ DISABLE_NULL_DETECTION(Private);
+
LocalDiskStart = (UINT8) ((*(UINT8 *) ((UINTN) 0x475)) + 0x80);
if ((Private->Disk4075 + 0x80) < LocalDiskStart) {
//
@@ -2426,6 +2430,9 @@ LegacyBiosInstallRom (
//
VideoMode = *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE));
}
+
+ ENABLE_NULL_DETECTION(Private);
+
//
// Notify the platform that we are about to scan the ROM
//
@@ -2466,9 +2473,11 @@ LegacyBiosInstallRom (
// Multiply result by 18.2 for number of ticks since midnight.
// Use 182/10 to avoid floating point math.
//
+ DISABLE_NULL_DETECTION(Private);
LocalTime = (LocalTime * 182) / 10;
BdaPtr = (UINT32 *) ((UINTN) 0x46C);
*BdaPtr = LocalTime;
+ ENABLE_NULL_DETECTION(Private);
//
// Pass in handoff data
@@ -2564,7 +2573,11 @@ LegacyBiosInstallRom (
//
// Set mode settings since PrepareToScanRom may change mode
//
- if (VideoMode != *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE))) {
+ DISABLE_NULL_DETECTION(Private);
+ OldVideoMode = *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE));
+ ENABLE_NULL_DETECTION(Private);
+
+ if (VideoMode != OldVideoMode) {
//
// The active video mode is changed, restore it to original mode.
//
@@ -2604,7 +2617,9 @@ LegacyBiosInstallRom (
}
}
+ DISABLE_NULL_DETECTION(Private);
LocalDiskEnd = (UINT8) ((*(UINT8 *) ((UINTN) 0x475)) + 0x80);
+ ENABLE_NULL_DETECTION(Private);
//
// Allow platform to perform any required actions after the
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
index 3d9a8b9649..50f6247a99 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
@@ -57,7 +57,11 @@ LegacyBiosInt86 (
IN EFI_IA32_REGISTER_SET *Regs
)
{
- UINT32 *VectorBase;
+ UINT16 Segment;
+ UINT16 Offset;
+ LEGACY_BIOS_INSTANCE *Private;
+
+ Private = LEGACY_BIOS_INSTANCE_FROM_THIS (This);
Regs->X.Flags.Reserved1 = 1;
Regs->X.Flags.Reserved2 = 0;
@@ -72,12 +76,15 @@ LegacyBiosInt86 (
// The base address of legacy interrupt vector table is 0.
// We use this base address to get the legacy interrupt handler.
//
- VectorBase = 0;
+ DISABLE_NULL_DETECTION(Private);
+ Segment = (UINT16)(((UINT32 *)0)[BiosInt] >> 16);
+ Offset = (UINT16)((UINT32 *)0)[BiosInt];
+ ENABLE_NULL_DETECTION(Private);
return InternalLegacyBiosFarCall (
This,
- (UINT16) ((VectorBase)[BiosInt] >> 16),
- (UINT16) (VectorBase)[BiosInt],
+ Segment,
+ Offset,
Regs,
&Regs->X.Flags,
sizeof (Regs->X.Flags)
@@ -288,16 +295,22 @@ InternalLegacyBiosFarCall (
// EBDA base address, if the current EBDA base address is smaller, it indicates
// PcdEbdaReservedMemorySize should be adjusted to larger for more OPROMs.
//
- DEBUG_CODE (
- {
- UINTN EbdaBaseAddress;
- UINTN ReservedEbdaBaseAddress;
-
- EbdaBaseAddress = (*(UINT16 *) (UINTN) 0x40E) << 4;
- ReservedEbdaBaseAddress = CONVENTIONAL_MEMORY_TOP - PcdGet32 (PcdEbdaReservedMemorySize);
- ASSERT (ReservedEbdaBaseAddress <= EbdaBaseAddress);
- }
- );
+ if (!NULL_DETECTION_ENABLED) {
+ //
+ // Only do following if NULL pointer detection is not enabled, because it cannot
+ // be disabled at this time due to current TPL(=TPL_HIGH_LEVEL).
+ //
+ DEBUG_CODE (
+ {
+ UINTN EbdaBaseAddress;
+ UINTN ReservedEbdaBaseAddress;
+
+ EbdaBaseAddress = (*(UINT16 *) (UINTN) 0x40E) << 4;
+ ReservedEbdaBaseAddress = CONVENTIONAL_MEMORY_TOP - PcdGet32 (PcdEbdaReservedMemorySize);
+ ASSERT (ReservedEbdaBaseAddress <= EbdaBaseAddress);
+ }
+ );
+ }
if (Stack != NULL && StackSize != 0) {
//
--
2.14.1.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 3/4] IntelFrameworkModulePkg/Csm: Update CSM code to temporarily bypass NULL pointer detection if enabled.
2017-09-13 9:25 ` [PATCH 3/4] IntelFrameworkModulePkg/Csm: Update CSM code to temporarily bypass NULL pointer detection if enabled Wang, Jian J
@ 2017-09-13 16:33 ` Johnson, Brian (EXL - Eagan)
0 siblings, 0 replies; 3+ messages in thread
From: Johnson, Brian (EXL - Eagan) @ 2017-09-13 16:33 UTC (permalink / raw)
To: Wang, Jian J, edk2-devel@lists.01.org
Cc: Justen@ml01.01.org, Eric Dong, Kinney@ml01.01.org, Jordan L,
Wolman@ml01.01.org, Jiewen Yao, Ayellet, Michael D, Laszlo Ersek,
Star Zeng
Acked-by: Brian J. Johnson <brian.johnson@hpe.com>
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Wang, Jian J
Sent: Wednesday, September 13, 2017 4:25 AM
To: edk2-devel@lists.01.org
Cc: Justen@ml01.01.org; Eric Dong <eric.dong@intel.com>; Kinney@ml01.01.org; Jordan L <jordan.l.justen@intel.com>; Wolman@ml01.01.org; Jiewen Yao <jiewen.yao@intel.com>; Ayellet <ayellet.wolman@intel.com>; Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Star Zeng <star.zeng@intel.com>
Subject: [edk2] [PATCH 3/4] IntelFrameworkModulePkg/Csm: Update CSM code to temporarily bypass NULL pointer detection if enabled.
CSM code has to access memory below 4096 (BDA, int vector, etc.). If NULL pointer detection is enabled, the page 0 must be enabled temporarily before accessing it and disabled again afterwards. Otherwise page fault will be triggered.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Justen, Jordan L <jordan.l.justen@intel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Wolman, Ayellet <ayellet.wolman@intel.com>
Suggested-by: Wolman, Ayellet <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wang, Jian J <jian.j.wang@intel.com>
---
.../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c | 10 +++-
.../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h | 18 +++++++
.../Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf | 2 +
.../Csm/LegacyBiosDxe/LegacyBda.c | 4 ++
.../Csm/LegacyBiosDxe/LegacyBios.c | 55 ++++++++++++++++++----
.../Csm/LegacyBiosDxe/LegacyBiosDxe.inf | 2 +
.../Csm/LegacyBiosDxe/LegacyBiosInterface.h | 23 +++++++++
.../Csm/LegacyBiosDxe/LegacyBootSupport.c | 33 ++++++++++---
.../Csm/LegacyBiosDxe/LegacyPci.c | 17 ++++++-
IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c | 41 ++++++++++------
10 files changed, 173 insertions(+), 32 deletions(-)
diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
index 7308523ad8..96148ae367 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
@@ -248,7 +248,7 @@ BiosKeyboardDriverBindingStart (
//
// Allocate the private device structure
//
- BiosKeyboardPrivate = (BIOS_KEYBOARD_DEV *) AllocateZeroPool (sizeof (BIOS_KEYBOARD_DEV));
+ BiosKeyboardPrivate = (BIOS_KEYBOARD_DEV *) AllocateZeroPool (sizeof (BIOS_KEYBOARD_DEV));
if (NULL == BiosKeyboardPrivate) {
Status = EFI_OUT_OF_RESOURCES;
goto Done;
@@ -281,6 +281,9 @@ BiosKeyboardDriverBindingStart (
BiosKeyboardPrivate->SimpleTextInputEx.UnregisterKeyNotify = BiosKeyboardUnregisterKeyNotify;
InitializeListHead (&BiosKeyboardPrivate->NotifyList);
+ Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **) &BiosKeyboardPrivate->Cpu);
+ ASSERT_EFI_ERROR(Status);
+
//
// Report that the keyboard is being enabled
//
@@ -1842,7 +1845,9 @@ BiosKeyboardTimerHandler (
//
// Clear the CTRL and ALT BDA flag
//
- KbFlag1 = *((UINT8 *) (UINTN) 0x417); // read the STATUS FLAGS 1
+ DISABLE_NULL_DETECTION(BiosKeyboardPrivate);
+
+ KbFlag1 = *((UINT8 *) (UINTN) 0x417); // read the STATUS FLAGS 1
KbFlag2 = *((UINT8 *) (UINTN) 0x418); // read STATUS FLAGS 2
DEBUG_CODE (
@@ -1916,6 +1921,7 @@ BiosKeyboardTimerHandler (
KbFlag1 &= ~0x0C;
*((UINT8 *) (UINTN) 0x417) = KbFlag1;
+ ENABLE_NULL_DETECTION(BiosKeyboardPrivate);
//
// 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..b717ef676b 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.h
@@ -26,6 +26,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Protocol/IsaIo.h>
#include <Protocol/DevicePath.h>
#include <Protocol/Ps2Policy.h>
+#include <Protocol/Cpu.h>
#include <Library/DebugLib.h>
#include <Library/UefiLib.h>
@@ -212,6 +213,7 @@ typedef struct {
EFI_HANDLE Handle;
EFI_LEGACY_BIOS_PROTOCOL *LegacyBios;
EFI_ISA_IO_PROTOCOL *IsaIo;
+ EFI_CPU_ARCH_PROTOCOL *Cpu;
EFI_SIMPLE_TEXT_INPUT_PROTOCOL SimpleTextIn;
EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL SimpleTextInputEx;
UINT16 DataRegisterAddress;
@@ -242,6 +244,22 @@ typedef struct {
BIOS_KEYBOARD_DEV_SIGNATURE \
)
+//
+// CSM needs to access memory between 0-4095, which will cause page fault exception
+// if NULL pointer detection mechanism is enabled. Following macros can be used
+// to disable/enable NULL pointer detection before/after accessing those memory.
+//
+#define NULL_POINTER_DETECTION_ENABLED ((PcdGet8(PcdNullPointerDetectionPropertyMask) & BIT0) != 0)
+#define DISABLE_NULL_DETECTION(Instance) \
+ if (NULL_POINTER_DETECTION_ENABLED && (Instance)->Cpu != NULL) { \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, 0); \
+ }
+
+#define ENABLE_NULL_DETECTION(Instance) \
+ if (NULL_POINTER_DETECTION_ENABLED && (Instance)->Cpu != NULL) { \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, EFI_MEMORY_RP); \
+ }
+
//
// Global Variables
//
diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
index 4d4536466c..4291a10123 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
@@ -67,12 +67,14 @@
gEfiSimpleTextInputExProtocolGuid ## BY_START
gEfiLegacyBiosProtocolGuid ## CONSUMES
gEfiPs2PolicyProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiCpuArchProtocolGuid ## SOMETIMES_CONSUMES
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE ## CONSUMES
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdFastPS2Detection ## SOMETIMES_CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES
[UserExtensions.TianoCore."ExtraFiles"]
KeyboardDxeExtra.uni
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
index c45d5d4c3e..e7cee4b8a3 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
@@ -34,6 +34,8 @@ LegacyBiosInitBda (
BDA_STRUC *Bda;
UINT8 *Ebda;
+ DISABLE_NULL_DETECTION(Private);
+
Bda = (BDA_STRUC *) ((UINTN) 0x400);
Ebda = (UINT8 *) ((UINTN) 0x9fc00);
@@ -62,5 +64,7 @@ LegacyBiosInitBda (
*Ebda = 0x01;
+ ENABLE_NULL_DETECTION(Private);
+
return EFI_SUCCESS;
}
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
index 3ead2d9828..c3ef542ea3 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
@@ -40,6 +40,7 @@ VOID *mRuntimeSmbiosEntryPoint = NULL;
EFI_PHYSICAL_ADDRESS mReserveSmbiosEntryPoint = 0;
EFI_PHYSICAL_ADDRESS mStructureTableAddress = 0;
UINTN mStructureTablePages = 0;
+BOOLEAN mEndOfDxe = FALSE;
/**
Do an AllocatePages () of type AllocateMaxAddress for EfiBootServicesCode
@@ -765,6 +766,26 @@ InstallSmbiosEventCallback (
}
}
+/**
+ Callback function to toggle EndOfDxe status. NULL pointer detection needs this
+ status to decide if it's necessary to change attributes of page 0.
+
+ @param Event Event whose notification function is being invoked.
+ @param Context The pointer to the notification function's context,
+ which is implementation-dependent.
+
+**/
+VOID
+EFIAPI
+ToggleEndOfDxeStatus (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ mEndOfDxe = TRUE;
+ return;
+}
+
/**
Install Driver to produce Legacy BIOS protocol.
@@ -802,6 +823,7 @@ LegacyBiosInstall (
UINT64 Length;
UINT8 *SecureBoot;
EFI_EVENT InstallSmbiosEvent;
+ EFI_EVENT EndOfDxeEvent;
//
// Load this driver's image to memory
@@ -964,8 +986,10 @@ LegacyBiosInstall (
// Initialize region from 0x0000 to 4k. This initializes interrupt vector
// range.
//
- gBS->SetMem ((VOID *) ClearPtr, 0x400, INITIAL_VALUE_BELOW_1K);
- ZeroMem ((VOID *) ((UINTN)ClearPtr + 0x400), 0xC00);
+ DISABLE_NULL_DETECTION(Private);
+ gBS->SetMem ((VOID *) ClearPtr, 0x400, INITIAL_VALUE_BELOW_1K);
+ ZeroMem ((VOID *) ((UINTN)ClearPtr + 0x400), 0xC00);
+ ENABLE_NULL_DETECTION(Private);
//
// Allocate pages for OPROM usage
@@ -1104,12 +1128,14 @@ LegacyBiosInstall (
//
// Save Unexpected interrupt vector so can restore it just prior to boot
//
- BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
- Private->BiosUnexpectedInt = BaseVectorMaster[0];
- IntRedirCode = (UINT32) (UINTN) Private->IntThunk->InterruptRedirectionCode;
- for (Index = 0; Index < 8; Index++) {
- BaseVectorMaster[Index] = (EFI_SEGMENT (IntRedirCode + Index * 4) << 16) | EFI_OFFSET (IntRedirCode + Index * 4);
- }
+ DISABLE_NULL_DETECTION(Private);
+ BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
+ Private->BiosUnexpectedInt = BaseVectorMaster[0];
+ IntRedirCode = (UINT32) (UINTN) Private->IntThunk->InterruptRedirectionCode;
+ for (Index = 0; Index < 8; Index++) {
+ BaseVectorMaster[Index] = (EFI_SEGMENT (IntRedirCode + Index * 4) << 16) | EFI_OFFSET (IntRedirCode + Index * 4);
+ }
+ ENABLE_NULL_DETECTION(Private);
//
// Save EFI value
//
@@ -1133,6 +1159,19 @@ LegacyBiosInstall (
);
ASSERT_EFI_ERROR (Status);
+ //
+ // Create callback to update status of EndOfDxe, which is needed by NULL pointer detection
+ //
+ Status = gBS->CreateEventEx (
+ EVT_NOTIFY_SIGNAL,
+ TPL_NOTIFY,
+ ToggleEndOfDxeStatus,
+ NULL,
+ &gEfiEndOfDxeEventGroupGuid,
+ &EndOfDxeEvent
+ );
+ ASSERT_EFI_ERROR (Status);
+
//
// Make a new handle and install the protocol
//
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
index 48473a0713..10dc392800 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
@@ -108,6 +108,7 @@
gEfiDiskInfoIdeInterfaceGuid ## SOMETIMES_CONSUMES ##GUID #Used in LegacyBiosBuildIdeData() to assure device is a disk
gEfiSmbiosTableGuid ## SOMETIMES_CONSUMES ##SystemTable
gEfiLegacyBiosGuid ## SOMETIMES_CONSUMES ##GUID #Used in LegacyBiosInstallVgaRom() to locate handle buffer
+ gEfiEndOfDxeEventGroupGuid
[Guids.IA32]
gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ##SystemTable
@@ -147,6 +148,7 @@
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdHighPmmMemorySize ## CONSUMES
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdOpromReservedMemoryBase ## CONSUMES
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdOpromReservedMemorySize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES
[Depex]
gEfiLegacyRegion2ProtocolGuid AND gEfiLegacyInterruptProtocolGuid AND gEfiLegacyBiosPlatformProtocolGuid AND gEfiLegacy8259ProtocolGuid AND gEfiGenericMemTestProtocolGuid AND gEfiCpuArchProtocolGuid AND gEfiTimerArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
index fe9dd7463a..9d479309a4 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
@@ -108,6 +108,27 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define NORMALIZE_EFI_SEGMENT(_Adr) (UINT16) (((UINTN) (_Adr)) >> 4)
#define NORMALIZE_EFI_OFFSET(_Adr) (UINT16) (((UINT16) ((UINTN) (_Adr))) & 0xf)
+//
+// CSM needs to access memory between 0-4095, which will cause page fault exception
+// if NULL pointer detection mechanism is enabled. Following macros can be used
+// to disable/enable NULL pointer detection before/after accessing those memory.
+//
+#define NULL_DETECTION_ENABLED \
+ ( ((mEndOfDxe == FALSE) && ((PcdGet8(PcdNullPointerDetectionPropertyMask) & BIT0) == BIT0)) \
+ || ((mEndOfDxe == TRUE) && ((PcdGet8(PcdNullPointerDetectionPropertyMask) & (BIT0|BIT7)) == BIT0)) \
+ )
+#define DISABLE_NULL_DETECTION(Instance) \
+ if (NULL_DETECTION_ENABLED) { \
+ DEBUG((DEBUG_INFO, "%a(): disable NULL detection\r\n", __func__)); \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, 0); \
+ }
+
+#define ENABLE_NULL_DETECTION(Instance) \
+ if (NULL_DETECTION_ENABLED) { \
+ (Instance)->Cpu->SetMemoryAttributes((Instance)->Cpu, 0, EFI_PAGE_SIZE, EFI_MEMORY_RP); \
+ DEBUG((DEBUG_INFO, "%a(): enable NULL detection\r\n", __func__)); \
+ }
+
//
// Trace defines
//
@@ -509,6 +530,8 @@ extern BBS_TABLE *mBbsTable;
extern EFI_GENERIC_MEMORY_TEST_PROTOCOL *gGenMemoryTest;
+extern BOOLEAN mEndOfDxe;
+
#define PORT_70 0x70
#define PORT_71 0x71
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
index 1e098b3726..d381c2f735 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
@@ -1073,8 +1073,10 @@ GenericLegacyBoot (
// Use 182/10 to avoid floating point math.
//
LocalTime = (LocalTime * 182) / 10;
- BdaPtr = (UINT32 *) (UINTN)0x46C;
- *BdaPtr = LocalTime;
+ DISABLE_NULL_DETECTION(Private);
+ BdaPtr = (UINT32 *) (UINTN)0x46C;
+ *BdaPtr = LocalTime;
+ ENABLE_NULL_DETECTION(Private);
//
// Shadow PCI ROMs. We must do this near the end since this will kick
@@ -1320,6 +1322,7 @@ GenericLegacyBoot (
// set of TIANO vectors) or takes it over.
//
//
+ DISABLE_NULL_DETECTION(Private);
BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
for (Index = 0; Index < 8; Index++) {
Private->ThunkSavedInt[Index] = BaseVectorMaster[Index];
@@ -1327,6 +1330,7 @@ GenericLegacyBoot (
BaseVectorMaster[Index] = (UINT32) (Private->BiosUnexpectedInt);
}
}
+ ENABLE_NULL_DETECTION(Private);
ZeroMem (&Regs, sizeof (EFI_IA32_REGISTER_SET));
Regs.X.AX = Legacy16Boot;
@@ -1340,10 +1344,12 @@ GenericLegacyBoot (
0
);
+ DISABLE_NULL_DETECTION(Private);
BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
for (Index = 0; Index < 8; Index++) {
BaseVectorMaster[Index] = Private->ThunkSavedInt[Index];
}
+ ENABLE_NULL_DETECTION(Private);
}
Private->LegacyBootEntered = TRUE;
if ((mBootMode == BOOT_LEGACY_OS) || (mBootMode == BOOT_UNCONVENTIONAL_DEVICE)) {
@@ -1731,9 +1737,11 @@ LegacyBiosBuildE820 (
//
// First entry is 0 to (640k - EBDA)
//
- E820Table[0].BaseAddr = 0;
- E820Table[0].Length = (UINT64) ((*(UINT16 *) (UINTN)0x40E) << 4);
- E820Table[0].Type = EfiAcpiAddressRangeMemory;
+ DISABLE_NULL_DETECTION(Private);
+ E820Table[0].BaseAddr = 0;
+ E820Table[0].Length = (UINT64) ((*(UINT16 *) (UINTN)0x40E) << 4);
+ E820Table[0].Type = EfiAcpiAddressRangeMemory;
+ ENABLE_NULL_DETECTION(Private);
//
// Second entry is (640k - EBDA) to 640k
@@ -1967,6 +1975,8 @@ LegacyBiosCompleteBdaBeforeBoot (
UINT16 MachineConfig;
DEVICE_PRODUCER_DATA_HEADER *SioPtr;
+ DISABLE_NULL_DETECTION(Private);
+
Bda = (BDA_STRUC *) ((UINTN) 0x400);
MachineConfig = 0;
@@ -2025,6 +2035,8 @@ LegacyBiosCompleteBdaBeforeBoot (
MachineConfig = (UINT16) (MachineConfig + 0x00 + 0x02 + (SioPtr->MousePresent * 0x04));
Bda->MachineConfig = MachineConfig;
+ ENABLE_NULL_DETECTION(Private);
+
return EFI_SUCCESS;
}
@@ -2049,15 +2061,20 @@ LegacyBiosUpdateKeyboardLedStatus (
UINT8 LocalLeds;
EFI_IA32_REGISTER_SET Regs;
- Bda = (BDA_STRUC *) ((UINTN) 0x400);
-
Private = LEGACY_BIOS_INSTANCE_FROM_THIS (This);
+
+ DISABLE_NULL_DETECTION(Private);
+
+ Bda = (BDA_STRUC *) ((UINTN) 0x400);
LocalLeds = Leds;
Bda->LedStatus = (UINT8) ((Bda->LedStatus &~0x07) | LocalLeds);
LocalLeds = (UINT8) (LocalLeds << 4);
Bda->ShiftStatus = (UINT8) ((Bda->ShiftStatus &~0x70) | LocalLeds);
LocalLeds = (UINT8) (Leds & 0x20);
Bda->KeyboardStatus = (UINT8) ((Bda->KeyboardStatus &~0x20) | LocalLeds);
+
+ ENABLE_NULL_DETECTION(Private);
+
//
// Call into Legacy16 code to allow it to do any processing
//
@@ -2102,7 +2119,9 @@ LegacyBiosCompleteStandardCmosBeforeBoot (
// to large capacity drives
// CMOS 14 = BDA 40:10 plus bit 3(display enabled)
//
+ DISABLE_NULL_DETECTION(Private);
Bda = (UINT8)(*((UINT8 *)((UINTN)0x410)) | BIT3);
+ ENABLE_NULL_DETECTION(Private);
//
// Force display enabled
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
index 8ffdf0c1ff..2ca5dddf00 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
@@ -2279,6 +2279,7 @@ LegacyBiosInstallRom (
UINTN Function;
EFI_IA32_REGISTER_SET Regs;
UINT8 VideoMode;
+ UINT8 OldVideoMode;
EFI_TIME BootTime;
UINT32 *BdaPtr;
UINT32 LocalTime;
@@ -2299,6 +2300,7 @@ LegacyBiosInstallRom (
Device = 0;
Function = 0;
VideoMode = 0;
+ OldVideoMode = 0;
PhysicalAddress = 0;
MaxRomAddr = PcdGet32 (PcdEndOpromShadowAddress);
@@ -2401,6 +2403,8 @@ LegacyBiosInstallRom (
// 2. BBS compliants drives will not change 40:75 until boot time.
// 3. Onboard IDE controllers will change 40:75
//
+ DISABLE_NULL_DETECTION(Private);
+
LocalDiskStart = (UINT8) ((*(UINT8 *) ((UINTN) 0x475)) + 0x80);
if ((Private->Disk4075 + 0x80) < LocalDiskStart) {
//
@@ -2426,6 +2430,9 @@ LegacyBiosInstallRom (
//
VideoMode = *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE));
}
+
+ ENABLE_NULL_DETECTION(Private);
+
//
// Notify the platform that we are about to scan the ROM
//
@@ -2466,9 +2473,11 @@ LegacyBiosInstallRom (
// Multiply result by 18.2 for number of ticks since midnight.
// Use 182/10 to avoid floating point math.
//
+ DISABLE_NULL_DETECTION(Private);
LocalTime = (LocalTime * 182) / 10;
BdaPtr = (UINT32 *) ((UINTN) 0x46C);
*BdaPtr = LocalTime;
+ ENABLE_NULL_DETECTION(Private);
//
// Pass in handoff data
@@ -2564,7 +2573,11 @@ LegacyBiosInstallRom (
//
// Set mode settings since PrepareToScanRom may change mode
//
- if (VideoMode != *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE))) {
+ DISABLE_NULL_DETECTION(Private);
+ OldVideoMode = *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE));
+ ENABLE_NULL_DETECTION(Private);
+
+ if (VideoMode != OldVideoMode) {
//
// The active video mode is changed, restore it to original mode.
//
@@ -2604,7 +2617,9 @@ LegacyBiosInstallRom (
}
}
+ DISABLE_NULL_DETECTION(Private);
LocalDiskEnd = (UINT8) ((*(UINT8 *) ((UINTN) 0x475)) + 0x80);
+ ENABLE_NULL_DETECTION(Private);
//
// Allow platform to perform any required actions after the
diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
index 3d9a8b9649..50f6247a99 100644
--- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
+++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
@@ -57,7 +57,11 @@ LegacyBiosInt86 (
IN EFI_IA32_REGISTER_SET *Regs
)
{
- UINT32 *VectorBase;
+ UINT16 Segment;
+ UINT16 Offset;
+ LEGACY_BIOS_INSTANCE *Private;
+
+ Private = LEGACY_BIOS_INSTANCE_FROM_THIS (This);
Regs->X.Flags.Reserved1 = 1;
Regs->X.Flags.Reserved2 = 0;
@@ -72,12 +76,15 @@ LegacyBiosInt86 (
// The base address of legacy interrupt vector table is 0.
// We use this base address to get the legacy interrupt handler.
//
- VectorBase = 0;
+ DISABLE_NULL_DETECTION(Private);
+ Segment = (UINT16)(((UINT32 *)0)[BiosInt] >> 16);
+ Offset = (UINT16)((UINT32 *)0)[BiosInt];
+ ENABLE_NULL_DETECTION(Private);
return InternalLegacyBiosFarCall (
This,
- (UINT16) ((VectorBase)[BiosInt] >> 16),
- (UINT16) (VectorBase)[BiosInt],
+ Segment,
+ Offset,
Regs,
&Regs->X.Flags,
sizeof (Regs->X.Flags)
@@ -288,16 +295,22 @@ InternalLegacyBiosFarCall (
// EBDA base address, if the current EBDA base address is smaller, it indicates
// PcdEbdaReservedMemorySize should be adjusted to larger for more OPROMs.
//
- DEBUG_CODE (
- {
- UINTN EbdaBaseAddress;
- UINTN ReservedEbdaBaseAddress;
-
- EbdaBaseAddress = (*(UINT16 *) (UINTN) 0x40E) << 4;
- ReservedEbdaBaseAddress = CONVENTIONAL_MEMORY_TOP - PcdGet32 (PcdEbdaReservedMemorySize);
- ASSERT (ReservedEbdaBaseAddress <= EbdaBaseAddress);
- }
- );
+ if (!NULL_DETECTION_ENABLED) {
+ //
+ // Only do following if NULL pointer detection is not enabled, because it cannot
+ // be disabled at this time due to current TPL(=TPL_HIGH_LEVEL).
+ //
+ DEBUG_CODE (
+ {
+ UINTN EbdaBaseAddress;
+ UINTN ReservedEbdaBaseAddress;
+
+ EbdaBaseAddress = (*(UINT16 *) (UINTN) 0x40E) << 4;
+ ReservedEbdaBaseAddress = CONVENTIONAL_MEMORY_TOP - PcdGet32 (PcdEbdaReservedMemorySize);
+ ASSERT (ReservedEbdaBaseAddress <= EbdaBaseAddress);
+ }
+ );
+ }
if (Stack != NULL && StackSize != 0) {
//
--
2.14.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-09-13 16:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-13 8:07 [PATCH 3/4] IntelFrameworkModulePkg/Csm: Update CSM code to temporarily bypass NULL pointer detection if enabled Wang, Jian J
[not found] <Implement NULL pointer detection feature>
2017-09-13 9:25 ` [PATCH 0/4] Implement NULL pointer detection feature for special pool Wang, Jian J
2017-09-13 9:25 ` [PATCH 3/4] IntelFrameworkModulePkg/Csm: Update CSM code to temporarily bypass NULL pointer detection if enabled Wang, Jian J
2017-09-13 16:33 ` Johnson, Brian (EXL - Eagan)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox