OcInputLib: Added support for KeyFiltering for buggy KB drivers

closes acidanthera/bugtracker#457

Application/KeyTester/KeyTester.c

+/** @file
+  Test keyboard input through standard protocol.
+
+Copyright (c) 2020, vit9696. All rights reserved.<BR>
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution.  The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Uefi.h>
+#include <Library/UefiLib.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiApplicationEntryPoint.h>
+
+
+EFI_STATUS
+EFIAPI
+UefiMain (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS     Status;
+  UINTN          EventIndex;
+  EFI_INPUT_KEY  Key;
+
+  STATIC CHAR16  Output[] = L"Received symbol:  \r\n";
+
+  do {
+    //
+    // Read a key
+    //
+    gBS->WaitForEvent (1, &gST->ConIn->WaitForKey, &EventIndex);
+    ZeroMem (&Key, sizeof (Key));
+    Status = gST->ConIn->ReadKeyStroke (gST->ConIn, &Key);
+    if (EFI_ERROR (Status)) {
+
+      if (Status == EFI_NOT_READY) {
+        continue;
+      }
+
+      break;
+    }
+
+    Output[ARRAY_SIZE (Output) - 4] = Key.UnicodeChar;
+    gST->ConOut->OutputString (gST->ConOut, Output);
+    DEBUG ((DEBUG_INFO, "KKT: Received %X %X\n", Key.ScanCode, Key.UnicodeChar));
+  } while (TRUE);
+
+  return EFI_SUCCESS;
+}

Application/KeyTester/KeyTester.inf

+## @file
+#  Test keyboard input through standard protocol.
+#
+#  Copyright (c) 2020, vit9696. All rights reserved.<BR>
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = KeyTester
+  FILE_GUID                      = 78E262F9-4438-49E0-9C53-0CA377B4029D
+  MODULE_TYPE                    = UEFI_APPLICATION
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = UefiMain
+
+#
+#  This flag specifies whether HII resource section is generated into PE image.
+#
+  UEFI_HII_RESOURCE_SECTION      = TRUE
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
+#
+
+[Sources]
+  KeyTester.c
+
+[Packages]
+  EfiPkg/EfiPkg.dec
+  MdePkg/MdePkg.dec
+  OpenCorePkg/OpenCorePkg.dec
+
+[LibraryClasses]
+  DebugLib
+  OcConsoleControlEntryModeLib
+  UefiApplicationEntryPoint
+  UefiBootServicesTableLib
+  UefiLib

Application/VerifyMsrE2/VerifyMsrE2.inf

 ## @file
-#  Viery MSR 0xE2 status on all the processors.
+#  Verify MSR 0xE2 status on all the processors.
 #
 #  Copyright (c) 2018, vit9696. All rights reserved.<BR>
 #

Changelog.md

@@ -5,7 +5,8 @@ OpenCore Changelog
 - Added TimeMachine detection to picker
 - Added early preview version of BootLiquor
 - Fixed FS discovery on NVMe with legacy drivers
-- Added DirectGopCacheMode option for FB cache policy
+- Added `DirectGopCacheMode` option for FB cache policy
+- Added `KeyFiltering` option to workaround buggy KB drivers
 
 #### v0.5.6
 - Various improvements to builtin text renderer

Docs/Configuration.tex

@@ -4050,6 +4050,17 @@ build -a X64 -b RELEASE -t XCODE5 -p MdeModulePkg/MdeModulePkg.dsc
 
 \begin{enumerate}
 
+\item
+  \texttt{KeyFiltering}\\
+  \textbf{Type}: \texttt{plist\ boolean}\\
+  \textbf{Failsafe}: \texttt{false}\\
+  \textbf{Description}: Enable keyboard input sanity checking.
+
+  Apparently some boards like GA Z77P-D3 may return uninitialised data
+  in \texttt{EFI\_INPUT\_KEY} with all input protocols.
+  This option discards keys that are neither ASCII, nor are defined
+  in the UEFI specification (see tables 107 and 108 in version 2.8).
+
 \item
   \texttt{KeyForgetThreshold}\\
   \textbf{Type}: \texttt{plist\ integer}\\
@@ -4116,6 +4127,9 @@ build -a X64 -b RELEASE -t XCODE5 -p MdeModulePkg/MdeModulePkg.dsc
   \item \texttt{AMI} --- Uses APTIO input protocol \texttt{AMI\_EFIKEYCODE\_PROTOCOL}.
   \end{itemize}
 
+  \emph{Note}: Currently \texttt{V1}, \texttt{V2}, and \texttt{AMI} unlike \texttt{Auto} only do filtering of
+  the particular specified protocol. This may change in the future versions.
+
 \item
   \texttt{KeySwap}\\
   \textbf{Type}: \texttt{plist\ boolean}\\
@@ -4269,6 +4283,21 @@ build -a X64 -b RELEASE -t XCODE5 -p MdeModulePkg/MdeModulePkg.dsc
 
   \emph{Note}: This option only applies to \texttt{System} renderer.
 
+\item
+  \texttt{DirectGopCacheMode}\\
+  \textbf{Type}: \texttt{plist\ string}\\
+  \textbf{Failsafe}: Empty string\\
+  \textbf{Description}: Cache mode for builtin graphics output protocol framebuffer.
+
+  Tuning cache mode may provide better rendering performance on some firmwares.
+  Providing empty string leaves cache control settings to the firmware.
+  Valid non-empty values are: \texttt{Uncacheable}, \texttt{WriteCombining}, and
+  \texttt{WriteThrough}.
+
+  \emph{Note}: This option is not supported on most hardware (see
+  \href{https://github.com/acidanthera/bugtracker/issues/755}{acidanthera/bugtracker\#755}
+  for more details).
+
 \item
   \texttt{DirectGopRendering}\\
   \textbf{Type}: \texttt{plist\ boolean}\\

Docs/Differences/Differences.tex

 \documentclass[]{article}
 %DIF LATEXDIFF DIFFERENCE FILE
 %DIF DEL PreviousConfiguration.tex   Fri Mar  6 09:43:05 2020
-%DIF ADD ../Configuration.tex        Fri Mar  6 23:22:27 2020
+%DIF ADD ../Configuration.tex        Sun Mar  8 15:47:32 2020
 
 \usepackage{lmodern}
 \usepackage{amssymb,amsmath}
@@ -4112,7 +4112,20 @@ build -a X64 -b RELEASE -t XCODE5 -p MdeModulePkg/MdeModulePkg.dsc
 \begin{enumerate}
 
 \item
-  \texttt{KeyForgetThreshold}\\
+  \DIFaddbegin \texttt{\DIFadd{KeyFiltering}}\\
+  \textbf{\DIFadd{Type}}\DIFadd{: }\texttt{\DIFadd{plist\ boolean}}\\
+  \textbf{\DIFadd{Failsafe}}\DIFadd{: }\texttt{\DIFadd{false}}\\
+  \textbf{\DIFadd{Description}}\DIFadd{: Enable keyboard input sanity checking.
+}
+
+  \DIFadd{Apparently some boards like GA Z77P-D3 may return uninitialised data
+  in }\texttt{\DIFadd{EFI\_INPUT\_KEY}} \DIFadd{with all input protocols.
+  This option discards keys that are neither ASCII, nor are defined
+  in the UEFI specification (see tables 107 and 108 in version 2.8).
+}
+
+\item
+  \DIFaddend \texttt{KeyForgetThreshold}\\
   \textbf{Type}: \texttt{plist\ integer}\\
   \textbf{Failsafe}: \texttt{0}\\
   \textbf{Description}: Remove key unless it was submitted during this timeout in milliseconds.
@@ -4177,7 +4190,11 @@ build -a X64 -b RELEASE -t XCODE5 -p MdeModulePkg/MdeModulePkg.dsc
   \item \texttt{AMI} --- Uses APTIO input protocol \texttt{AMI\_EFIKEYCODE\_PROTOCOL}.
   \end{itemize}
 
-\item
+  \DIFaddbegin \emph{\DIFadd{Note}}\DIFadd{: Currently }\texttt{\DIFadd{V1}}\DIFadd{, }\texttt{\DIFadd{V2}}\DIFadd{, and }\texttt{\DIFadd{AMI}} \DIFadd{unlike }\texttt{\DIFadd{Auto}} \DIFadd{only do filtering of
+  the particular specified protocol. This may change in the future versions.
+}
+
+\DIFaddend \item
   \texttt{KeySwap}\\
   \textbf{Type}: \texttt{plist\ boolean}\\
   \textbf{Failsafe}: \texttt{false}\\
@@ -4328,7 +4345,25 @@ build -a X64 -b RELEASE -t XCODE5 -p MdeModulePkg/MdeModulePkg.dsc
   This option fills the entire graphics screen with black color before switching to
   text mode.
 
-  \emph{Note}: This option only applies to \texttt{System} renderer.
+  \emph{Note}: This option only applies to \texttt{System} renderer\DIFaddbegin \DIFadd{.
+}
+
+\item
+  \texttt{\DIFadd{DirectGopCacheMode}}\\
+  \textbf{\DIFadd{Type}}\DIFadd{: }\texttt{\DIFadd{plist\ string}}\\
+  \textbf{\DIFadd{Failsafe}}\DIFadd{: Empty string}\\
+  \textbf{\DIFadd{Description}}\DIFadd{: Cache mode for builtin graphics output protocol framebuffer.
+}
+
+  \DIFadd{Tuning cache mode may provide better rendering performance on some firmwares.
+  Providing empty string leaves cache control settings to the firmware.
+  Valid non-empty values are: }\texttt{\DIFadd{Uncacheable}}\DIFadd{, }\texttt{\DIFadd{WriteCombining}}\DIFadd{, and
+  }\texttt{\DIFadd{WriteThrough}}\DIFadd{.
+}
+
+  \emph{\DIFadd{Note}}\DIFadd{: This option is not supported on most hardware (see
+  }\href{https://github.com/acidanthera/bugtracker/issues/755}{acidanthera/bugtracker\#755}
+  \DIFadd{for more details)}\DIFaddend .
 
 \item
   \texttt{DirectGopRendering}\\

Include/Library/OcConfigurationLib.h

@@ -481,14 +481,15 @@ typedef enum {
 /// Input is a set of options to support advanced input.
 ///
 #define OC_UEFI_INPUT_FIELDS(_, __) \
+  _(OC_STRING                   , KeySupportMode     ,     , OC_STRING_CONSTR ("", _, __)  , OC_DESTR (OC_STRING)) \
+  _(OC_STRING                   , PointerSupportMode ,     , OC_STRING_CONSTR ("", _, __)  , OC_DESTR (OC_STRING)) \
+  _(UINT32                      , TimerResolution    ,     , 0                             , ()) \
   _(UINT8                       , KeyForgetThreshold ,     , 0                             , ()) \
   _(UINT8                       , KeyMergeThreshold  ,     , 0                             , ()) \
   _(BOOLEAN                     , KeySupport         ,     , FALSE                         , ()) \
-  _(OC_STRING                   , KeySupportMode     ,     , OC_STRING_CONSTR ("", _, __)  , OC_DESTR (OC_STRING)) \
+  _(BOOLEAN                     , KeyFiltering       ,     , FALSE                         , ()) \
   _(BOOLEAN                     , KeySwap            ,     , FALSE                         , ()) \
-  _(BOOLEAN                     , PointerSupport     ,     , FALSE                         , ()) \
-  _(OC_STRING                   , PointerSupportMode ,     , OC_STRING_CONSTR ("", _, __)  , OC_DESTR (OC_STRING)) \
-  _(UINT32                      , TimerResolution    ,     , 0                             , ())
+  _(BOOLEAN                     , PointerSupport     ,     , FALSE                         , ())
   OC_DECLARE (OC_UEFI_INPUT)
 
 ///

Include/Library/OcInputLib.h

@@ -53,7 +53,8 @@ OcAppleGenericInputKeycodeInit (
   IN OC_INPUT_KEY_MODE  Mode,
   IN UINT8              KeyForgotThreshold,
   IN UINT8              KeyMergeThreshold,
-  IN BOOLEAN            KeySwap
+  IN BOOLEAN            KeySwap,
+  IN BOOLEAN            KeyFiltering
   );
 
 EFI_STATUS

Library/OcConfigurationLib/OcConfigurationLib.c

@@ -563,6 +563,7 @@ mUefiAudioSchema[] = {
 STATIC
 OC_SCHEMA
 mUefiInputSchema[] = {
+  OC_SCHEMA_BOOLEAN_IN ("KeyFiltering",       OC_GLOBAL_CONFIG, Uefi.Input.KeyFiltering),
   OC_SCHEMA_INTEGER_IN ("KeyForgetThreshold", OC_GLOBAL_CONFIG, Uefi.Input.KeyForgetThreshold),
   OC_SCHEMA_INTEGER_IN ("KeyMergeThreshold",  OC_GLOBAL_CONFIG, Uefi.Input.KeyMergeThreshold),
   OC_SCHEMA_BOOLEAN_IN ("KeySupport",         OC_GLOBAL_CONFIG, Uefi.Input.KeySupport),

Library/OcInputLib/Keycode/AIK.c

@@ -127,7 +127,8 @@ AIKPollKeyboardHandler (
   do {
     Status = AIKSourceGrabEfiKey (
       &Keycode->Source,
-      &KeyData
+      &KeyData,
+      Keycode->KeyFiltering
       );
     if (!EFI_ERROR (Status)) {
       DEBUG ((
@@ -232,7 +233,8 @@ OcAppleGenericInputKeycodeInit (
   IN OC_INPUT_KEY_MODE  Mode,
   IN UINT8              KeyForgotThreshold,
   IN UINT8              KeyMergeThreshold,
-  IN BOOLEAN            KeySwap
+  IN BOOLEAN            KeySwap,
+  IN BOOLEAN            KeyFiltering
   )
 {
   EFI_STATUS                Status;
@@ -242,6 +244,7 @@ OcAppleGenericInputKeycodeInit (
   gAikSelf.Mode               = Mode;
   gAikSelf.KeyForgotThreshold = KeyForgotThreshold;
   gAikSelf.KeyMergeThreshold  = KeyMergeThreshold;
+  gAikSelf.KeyFiltering       = KeyFiltering;
   Status = AIKInstall (&gAikSelf);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_INFO, "AIKInstall failed - %r\n", Status));

Library/OcInputLib/Keycode/AIK.h

@@ -48,6 +48,11 @@ typedef struct {
   //
   UINT8                 KeyMergeThreshold;
 
+  //
+  // Perform ASCII and scan code input key filtering.
+  //
+  BOOLEAN               KeyFiltering;
+
   //
   // Input sources
   //

Library/OcInputLib/Keycode/AIKSource.c

@@ -23,7 +23,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 EFI_STATUS
 AIKSourceGrabEfiKey (
   AIK_SOURCE        *Source,
-  AMI_EFI_KEY_DATA  *KeyData
+  AMI_EFI_KEY_DATA  *KeyData,
+  BOOLEAN           KeyFiltering
   )
 {
   EFI_STATUS      Status;
@@ -48,11 +49,78 @@ AIKSourceGrabEfiKey (
     Event  = NULL;
   }
 
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  //
+  // Some boards like GA Z77P-D3 may return uninitialised data in EFI_INPUT_KEY.
+  // Try to reduce the effects by discarding definitely invalid symbols.
+  // See: tables 107 and 108 in UEFI spec describing EFI Scan Codes.
+  //
+  if (KeyFiltering) {
+    if ((KeyData->Key.UnicodeChar & ~0xFFU) != 0) {
+      return EFI_INVALID_PARAMETER;
+    }
+
+    switch (KeyData->Key.ScanCode) {
+      case SCAN_NULL:
+      case SCAN_UP:
+      case SCAN_DOWN:
+      case SCAN_RIGHT:
+      case SCAN_LEFT:
+      case SCAN_HOME:
+      case SCAN_END:
+      case SCAN_INSERT:
+      case SCAN_DELETE:
+      case SCAN_PAGE_UP:
+      case SCAN_PAGE_DOWN:
+      case SCAN_F1:
+      case SCAN_F2:
+      case SCAN_F3:
+      case SCAN_F4:
+      case SCAN_F5:
+      case SCAN_F6:
+      case SCAN_F7:
+      case SCAN_F8:
+      case SCAN_F9:
+      case SCAN_F10:
+      case SCAN_ESC:
+      case SCAN_F11:
+      case SCAN_F12:
+      case SCAN_PAUSE:
+      case SCAN_F13:
+      case SCAN_F14:
+      case SCAN_F15:
+      case SCAN_F16:
+      case SCAN_F17:
+      case SCAN_F18:
+      case SCAN_F19:
+      case SCAN_F20:
+      case SCAN_F21:
+      case SCAN_F22:
+      case SCAN_F23:
+      case SCAN_F24:
+      case SCAN_MUTE:
+      case SCAN_VOLUME_UP:
+      case SCAN_VOLUME_DOWN:
+      case SCAN_BRIGHTNESS_UP:
+      case SCAN_BRIGHTNESS_DOWN:
+      case SCAN_SUSPEND:
+      case SCAN_HIBERNATE:
+      case SCAN_TOGGLE_DISPLAY:
+      case SCAN_RECOVERY:
+      case SCAN_EJECT:
+        break;
+      default:
+        return EFI_INVALID_PARAMETER;
+    }
+  }
+
   if (Event != NULL) {
     gBS->SignalEvent (Event);
   }
-
-  return Status;
+  return EFI_SUCCESS;
 }
 
 EFI_STATUS

Library/OcInputLib/Keycode/AIKSource.h

@@ -54,7 +54,8 @@ typedef struct {
 EFI_STATUS
 AIKSourceGrabEfiKey (
   AIK_SOURCE        *Source,
-  AMI_EFI_KEY_DATA  *KeyData
+  AMI_EFI_KEY_DATA  *KeyData,
+  BOOLEAN           KeyFiltering
   );
 
 EFI_STATUS

OpenCorePkg.dsc

@@ -129,6 +129,7 @@
   OpenCorePkg/Application/CleanNvram/CleanNvram.inf
   OpenCorePkg/Application/GopStop/GopStop.inf
   OpenCorePkg/Application/HdaCodecDump/HdaCodecDump.inf
+  OpenCorePkg/Application/KeyTester/KeyTester.inf
   OpenCorePkg/Application/PavpProvision/PavpProvision.inf
   OpenCorePkg/Application/VerifyMsrE2/VerifyMsrE2.inf
   OpenCorePkg/Debug/GdbSyms/GdbSyms.inf

Platform/OpenCore/OpenCoreUefiInOut.c

@@ -163,7 +163,8 @@ OcLoadUefiInputSupport (
                  KeyMode,
                  Config->Uefi.Input.KeyForgetThreshold,
                  Config->Uefi.Input.KeyMergeThreshold,
-                 Config->Uefi.Input.KeySwap
+                 Config->Uefi.Input.KeySwap,
+                 Config->Uefi.Input.KeyFiltering
                  );
       if (EFI_ERROR (Status)) {
         DEBUG ((DEBUG_ERROR, "OC: Failed to initialize keycode\n"));