public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Jian J Wang <jian.j.wang@intel.com>
To: edk2-devel@lists.01.org
Cc: Liming Gao <liming.gao@intel.com>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Ruiyu Ni <ruiyu.ni@intel.com>
Subject: [PATCH v2 3/3] IntelFrameworkModulePkg/KeyboardDxe: Use macro to enable/disable page 0
Date: Thu,  7 Dec 2017 13:40:49 +0800	[thread overview]
Message-ID: <20171207054049.18140-4-jian.j.wang@intel.com> (raw)
In-Reply-To: <20171207054049.18140-1-jian.j.wang@intel.com>

> v2:
> a. Remove unnecessary braces enclosing code passed to ACCESS_PAGE0_CODE

Current implementation uses following two methods

    EnableNullDetection()
    DisableNullDetection()

to enable/disable page 0. These two methods will check PCD
PcdNullPointerDetectionPropertyMask to know if the page 0 is disabled or not.
This is due to the fact that old GCD service doesn't provide paging related
attributes of memory block. Since this issue has been fixed, GCD services
can be used to determine the paging status of page 0. This is also make it
possible to just use a new macro

    ACCESS_PAGE0_CODE(
      <code accessing page 0>
    );

to replace above methods to do the same job, which also makes code more
readability.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 .../Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c       | 118 ++-------------------
 .../Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf      |   1 -
 2 files changed, 10 insertions(+), 109 deletions(-)

diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
index ebf03d30c1..ec525891dc 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/BiosKeyboard.c
@@ -1732,98 +1732,6 @@ CheckKeyboardConnect (
   }
 }
 
-/**
-  Disable NULL pointer detection.
-**/
-VOID
-DisableNullDetection (
-  VOID
-  )
-{
-  EFI_STATUS                            Status;
-  EFI_GCD_MEMORY_SPACE_DESCRIPTOR       Desc;
-
-  if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0) {
-    return;
-  }
-
-  //
-  // Check current capabilities and attributes
-  //
-  Status = gDS->GetMemorySpaceDescriptor (0, &Desc);
-  ASSERT_EFI_ERROR (Status);
-
-  //
-  // Try to add EFI_MEMORY_RP support if necessary
-  //
-  if ((Desc.Capabilities & EFI_MEMORY_RP) == 0) {
-    Desc.Capabilities |= EFI_MEMORY_RP;
-    Status = gDS->SetMemorySpaceCapabilities (0, EFI_PAGES_TO_SIZE(1),
-                                              Desc.Capabilities);
-    ASSERT_EFI_ERROR (Status);
-    if (EFI_ERROR (Status)) {
-      return;
-    }
-  }
-
-  //
-  // Don't bother if EFI_MEMORY_RP is already cleared.
-  //
-  if ((Desc.Attributes & EFI_MEMORY_RP) != 0) {
-    Desc.Attributes &= ~EFI_MEMORY_RP;
-    Status = gDS->SetMemorySpaceAttributes (0, EFI_PAGES_TO_SIZE(1),
-                                            Desc.Attributes);
-    ASSERT_EFI_ERROR (Status);
-  } else {
-    DEBUG ((DEBUG_WARN, "!!! Page 0 is supposed to be disabled !!!\r\n"));
-  }
-}
-
-/**
-   Enable NULL pointer detection.
-**/
-VOID
-EnableNullDetection (
-  VOID
-  )
-{
-  EFI_STATUS                            Status;
-  EFI_GCD_MEMORY_SPACE_DESCRIPTOR       Desc;
-
-  if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0) {
-    return;
-  }
-
-  //
-  // Check current capabilities and attributes
-  //
-  Status = gDS->GetMemorySpaceDescriptor (0, &Desc);
-  ASSERT_EFI_ERROR (Status);
-
-  //
-  // Try to add EFI_MEMORY_RP support if necessary
-  //
-  if ((Desc.Capabilities & EFI_MEMORY_RP) == 0) {
-    Desc.Capabilities |= EFI_MEMORY_RP;
-    Status = gDS->SetMemorySpaceCapabilities (0, EFI_PAGES_TO_SIZE(1),
-                                              Desc.Capabilities);
-    ASSERT_EFI_ERROR (Status);
-    if (EFI_ERROR (Status)) {
-      return;
-    }
-  }
-
-  //
-  // Don't bother if EFI_MEMORY_RP is already set.
-  //
-  if ((Desc.Attributes & EFI_MEMORY_RP) == 0) {
-    Desc.Attributes |= EFI_MEMORY_RP;
-    Status = gDS->SetMemorySpaceAttributes (0, EFI_PAGES_TO_SIZE(1),
-                                            Desc.Attributes);
-    ASSERT_EFI_ERROR (Status);
-  }
-}
-
 /**
   Timer event handler: read a series of key stroke from 8042
   and put them into memory key buffer. 
@@ -1931,16 +1839,13 @@ BiosKeyboardTimerHandler (
   //   0 Right Shift pressed
 
 
-  //
-  // Disable NULL pointer detection temporarily
-  //
-  DisableNullDetection ();
-
   //
   // Clear the CTRL and ALT BDA flag
   //
-  KbFlag1 = *((UINT8 *) (UINTN) 0x417);  // read the STATUS FLAGS 1
-  KbFlag2 = *((UINT8 *) (UINTN) 0x418); // read STATUS FLAGS 2
+  ACCESS_PAGE0_CODE (
+    KbFlag1 = *((UINT8 *) (UINTN) 0x417); // read the STATUS FLAGS 1
+    KbFlag2 = *((UINT8 *) (UINTN) 0x418); // read STATUS FLAGS 2
+  );
 
   DEBUG_CODE (
     {
@@ -2008,15 +1913,12 @@ BiosKeyboardTimerHandler (
   //
   // Clear left alt and left ctrl BDA flag
   //
-  KbFlag2 &= ~(KB_LEFT_ALT_PRESSED | KB_LEFT_CTRL_PRESSED);
-  *((UINT8 *) (UINTN) 0x418) = KbFlag2;
-  KbFlag1 &= ~0x0C;                      
-  *((UINT8 *) (UINTN) 0x417) = KbFlag1; 
-
-  //
-  // Restore NULL pointer detection
-  //
-  EnableNullDetection ();
+  ACCESS_PAGE0_CODE (
+    KbFlag2 &= ~(KB_LEFT_ALT_PRESSED | KB_LEFT_CTRL_PRESSED);
+    *((UINT8 *) (UINTN) 0x418) = KbFlag2;
+    KbFlag1 &= ~0x0C;
+    *((UINT8 *) (UINTN) 0x417) = KbFlag1;
+  );
   
   //
   // Output EFI input key and shift/toggle state
diff --git a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
index 596f4ced44..eaaedbfa9c 100644
--- a/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
+++ b/IntelFrameworkModulePkg/Csm/BiosThunk/KeyboardDxe/KeyboardDxe.inf
@@ -74,7 +74,6 @@
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFastPS2Detection                  ## SOMETIMES_CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask  ## CONSUMES
 
 [UserExtensions.TianoCore."ExtraFiles"]
   KeyboardDxeExtra.uni
-- 
2.15.1.windows.2



  parent reply	other threads:[~2017-12-07  5:36 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-07  5:40 [PATCH v2 0/3] Remove dependency on PcdNullPointerDetectionPropertyMask Jian J Wang
2017-12-07  5:40 ` [PATCH v2 1/3] IntelFrameworkPkg/LegacyBios.h: Add a macro to guarantee page 0 access Jian J Wang
2017-12-07  7:00   ` Ni, Ruiyu
2017-12-07  8:03     ` Wang, Jian J
2017-12-07  5:40 ` [PATCH v2 2/3] IntelFrameworkModulePkg/LegacyBiosDxe: Use macro to enable/disable page 0 Jian J Wang
2017-12-07  7:01   ` Ni, Ruiyu
2017-12-07  5:40 ` Jian J Wang [this message]
2017-12-07  7:01   ` [PATCH v2 3/3] IntelFrameworkModulePkg/KeyboardDxe: " Ni, Ruiyu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171207054049.18140-4-jian.j.wang@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox