Re: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/KeyboardDxe: Use macro to enable/disable page 0
On 12/7/2017 1:40 PM, Jian J Wang wrote:
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(
);
to replace above methods to do the same job, which also makes code more
readability.
Cc: Liming Gao
Cc: Michael D Kinney
Cc: Ruiyu Ni
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang
---
.../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_STATUSStatus;
- 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_STATUSStatus;
- 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;
+K
[edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/KeyboardDxe: Use macro to enable/disable page 0
> 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(
);
to replace above methods to do the same job, which also makes code more
readability.
Cc: Liming Gao
Cc: Michael D Kinney
Cc: Ruiyu Ni
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang
---
.../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_STATUSStatus;
- 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_STATUSStatus;
- 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) 0x4
