On 12/7/2017 1:40 PM, Jian J Wang wrote:
v2:
a. Fix a typo in expression in the macro ACCESS_PAGE0_CODE
b. Fix GCC49 build error

ue to the introduction of NULL pointer detection feature, page 0 will be
disabled if the feature is enabled, which will cause legacy code failed to
update legacy data in page 0. This macro is introduced to make sure the
page 0 is enabled before those code and restore the original status of it
afterwards.

Another reason to introduce this macro is to eliminate the dependency on
the PcdNullPointerDetectionPropertyMask. Because this is a new PCD, it
could cause some backward compatibility issue for some old packages.

This macro will simply check if the page 0 is disabled or not. If it's
disabled, it will enable it before code updating page 0 and disable it
afterwards. Otherwise, this macro will do nothing to page 0.

The usage of the macro will be look like (similar to DEBUG_CODE macro):

     ACCESS_PAGE0_CODE(
       <code accessing page 0>
     );

Cc: Liming Gao <liming....@intel.com>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Cc: Ruiyu Ni <ruiyu...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.w...@intel.com>
---
  IntelFrameworkPkg/Include/Protocol/LegacyBios.h | 34 +++++++++++++++++++++++++
  1 file changed, 34 insertions(+)

diff --git a/IntelFrameworkPkg/Include/Protocol/LegacyBios.h 
b/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
index 641f101bce..6a5f5464e7 100644
--- a/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
+++ b/IntelFrameworkPkg/Include/Protocol/LegacyBios.h
@@ -1518,6 +1518,40 @@ struct _EFI_LEGACY_BIOS_PROTOCOL {
    EFI_LEGACY_BIOS_BOOT_UNCONVENTIONAL_DEVICE  BootUnconventionalDevice;
  };
+//
+// Legacy BIOS needs to access memory in page 0 (0-4095), which is disabled if
+// NULL pointer detection feature is enabled. Following macro can be used to
+// enable/disable page 0 before/after accessing it.
+//
+#define ACCESS_PAGE0_CODE(statements)                           \
+  do {                                                          \
+    EFI_STATUS                            Status_;              \
+    EFI_GCD_MEMORY_SPACE_DESCRIPTOR       Desc_;                \
+                                                                \
+    Desc_.Attributes = 0;                                       \
+    Status_ = gDS->GetMemorySpaceDescriptor (0, &Desc_);        \
+    ASSERT_EFI_ERROR (Status_);                                 \
+    if ((Desc_.Attributes & EFI_MEMORY_RP) != 0) {              \
+      Status_ = gDS->SetMemorySpaceAttributes (                 \
+                      0,                                        \
+                      EFI_PAGES_TO_SIZE(1),                     \
+                      Desc_.Attributes & ~(UINT64)EFI_MEMORY_RP \
+                      );                                        \
+      ASSERT_EFI_ERROR (Status_);                               \
+    }                                                           \
+                                                                \
+    statements;                                                 \

It's better to surrounded statements with {}.
So that when statements contains variable declaration, C compiler
doesn't complain.

+                                                                \
+    if ((Desc_.Attributes & EFI_MEMORY_RP) != 0) {              \
+      Status_ = gDS->SetMemorySpaceAttributes (                 \
+                      0,                                        \
+                      EFI_PAGES_TO_SIZE(1),                     \
+                      Desc_.Attributes                          \
+                      );                                        \
+      ASSERT_EFI_ERROR (Status_);                               \
+    }                                                           \
+  } while (FALSE)
+
  extern EFI_GUID gEfiLegacyBiosProtocolGuid;
#endif


With the above suggested changes,
  Reviewed-by: Ruiyu Ni <ruiyu...@intel.com>

--
Thanks,
Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to