Some comments to this patch.

1. How about using lower TPL TPL_CALLBACK instead of TPL_NOTIFY for the 
notification?
2. Should GCD SetMemorySpaceCapabilities + SetMemorySpaceAttributes be used 
instead of gCpu->SetMemoryAttributes()?

Thanks,
Star
-----Original Message-----
From: Wang, Jian J 
Sent: Thursday, September 28, 2017 9:04 AM
To: [email protected]
Cc: Zeng, Star <[email protected]>; Dong, Eric <[email protected]>; Laszlo 
Ersek <[email protected]>; Yao, Jiewen <[email protected]>; Kinney, Michael 
D <[email protected]>; Justen, Jordan L <[email protected]>; 
Wolman, Ayellet <[email protected]>
Subject: [PATCH v3 3/6] MdeModulePkg/Core/Dxe: Add EndOfDxe workaround

One of issue caused by enabling NULL pointer detection is that some PCI device 
OptionROM, binary drivers and binary OS boot loaders may have NULL pointer 
access bugs, which will prevent BIOS from booting and is almost impossible to 
fix. BIT7 of PCD PcdNullPointerDetectionPropertyMask is used as a workaround to 
indicate BIOS to disable NULL pointer detection right after event 
gEfiEndOfDxeEventGroupGuid, and then let boot continue.

Cc: Star Zeng <[email protected]>
Cc: Eric Dong <[email protected]>
Cc: Laszlo Ersek <[email protected]>
Cc: Jiewen Yao <[email protected]>
Cc: Michael Kinney <[email protected]>
Cc: Jordan Justen <[email protected]>
Cc: Ayellet Wolman <[email protected]>
Suggested-by: Ayellet Wolman <[email protected]>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <[email protected]>
---
 MdeModulePkg/Core/Dxe/DxeMain.inf             |  1 +
 MdeModulePkg/Core/Dxe/Mem/Page.c              |  4 ++-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 48 +++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf 
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 30d5984f7c..0a161ffd71 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -192,6 +192,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable                   ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy                   ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy             ## 
CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask        ## 
CONSUMES
 
 # [Hob]
 # RESOURCE_DESCRIPTOR   ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index a142c79ee2..0468df3171 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -188,7 +188,9 @@ CoreAddRange (
   // used for other purposes.
   //  
   if (Type == EfiConventionalMemory && Start == 0 && (End >= EFI_PAGE_SIZE - 
1)) {
-    SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0);
+    if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0) {
+      SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0);
+    }
   }
   
   //
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index a73c4ccd64..73e3b269f3 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -995,6 +995,36 @@ MemoryProtectionExitBootServicesCallback (
   }
 }
 
+/**
+  Disable NULL pointer detection after EndOfDxe. This is a workaround 
+resort in
+  order to skip unfixable NULL pointer access issues detected in 
+OptionROM or
+  boot loaders.
+
+  @param[in]  Event     The Event this notify function registered to.
+  @param[in]  Context   Pointer to the context data registered to the Event.
+**/
+VOID
+EFIAPI
+DisableNullDetectionAtTheEndOfDxe (
+  EFI_EVENT                               Event,
+  VOID                                    *Context
+  )
+{
+  EFI_STATUS                        Status;
+
+  DEBUG ((DEBUG_INFO, "DisableNullDetectionAtTheEndOfDxe(): 
+ start\r\n"));  //  // Disable NULL pointer detection by enabling first 
+ 4K page  //  Status = gCpu->SetMemoryAttributes (gCpu, 0, 
+ EFI_PAGE_SIZE, 0);  ASSERT_EFI_ERROR (Status);
+
+  CoreCloseEvent (Event);
+  DEBUG ((DEBUG_INFO, "DisableNullDetectionAtTheEndOfDxe(): end\r\n"));
+
+  return;
+}
+
 /**
   Initialize Memory Protection support.
 **/
@@ -1006,6 +1036,7 @@ CoreInitializeMemoryProtection (  {
   EFI_STATUS  Status;
   EFI_EVENT   Event;
+  EFI_EVENT   EndOfDxeEvent;
   VOID        *Registration;
 
   mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy);
@@ -1044,6 +1075,23 @@ CoreInitializeMemoryProtection (
                );
     ASSERT_EFI_ERROR(Status);
   }
+
+  //
+  // Register a callback to disable NULL pointer detection at EndOfDxe  
+ //  if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT0|BIT7))
+       == (BIT0|BIT7)) {
+    Status = CoreCreateEventEx (
+                    EVT_NOTIFY_SIGNAL,
+                    TPL_NOTIFY,
+                    DisableNullDetectionAtTheEndOfDxe,
+                    NULL,
+                    &gEfiEndOfDxeEventGroupGuid,
+                    &EndOfDxeEvent
+                    );
+    ASSERT_EFI_ERROR (Status);
+  }
+
   return ;
 }
 
--
2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to