From dc3ab8ba5e3c5403ae6644f68d0474ba0d1b190c Mon Sep 17 00:00:00 2001
From: John Schock <joschock@microsoft.com>
Date: Wed, 25 Oct 2023 20:48:01 -0700
Subject: [PATCH] Add PCD to allow device exclusions for UsbHidDxe (#340)

## Description

Adds a PCD for configuring device exclusions for UsbHidDxe. Prior to
this change, the UsbHidDxe driver included logic to exclude keyboard
devices from being handled by HID, due to other parts of the stack not
being fully implemented yet. This removes that code and replaces it with
a PCD list that the platform integrator can use to disable particular
devices as desired for the platform.

- [x] Impacts functionality?
  - Changes how UsbHidDxe exclusions are handled.
- [ ] Impacts security?
- [x] Breaking change?
Previously keyboard exclusion was hard-coded; to replicate this behavior
platforms will need to add a PCD specification in the DSC files to
exclude keyboards. See Integration Instructions below for more details.
- [ ] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

Verified with functional testing of several different exclusions in Qemu
Q35.

## Integration Instructions

To replicate existing behavior, platforms will need to add PCD
specification to the DSC like the following to exclude USB keyboards:

```
[PcdsFixedAtBuild]
  gHidPkgTokenSpaceGuid.PcdExcludedHidDevices|{0x3, 0x1, 0x1, 0x0, 0x0, 0x0}
---
 HidPkg/HidPkg.dec              | 12 +++++++++++
 HidPkg/UsbHidDxe/UsbHidDxe.c   | 39 ++++++++++++++++++++++++----------
 HidPkg/UsbHidDxe/UsbHidDxe.inf |  7 +++---
 3 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/HidPkg/HidPkg.dec b/HidPkg/HidPkg.dec
index c756d692af..2e8da12dce 100644
--- a/HidPkg/HidPkg.dec
+++ b/HidPkg/HidPkg.dec
@@ -35,6 +35,18 @@
   gHidKeyboardLayoutPackageGuid      = {0x57adcfa9, 0xc08a, 0x40b9, {0x86, 0x87, 0x65, 0xe2, 0xec, 0xe0, 0xe5, 0xe1}}
   gHidKeyboardLayoutKeyGuid          = {0xdcafaba8, 0xcde5, 0x40a1, {0x9a, 0xd3, 0x48, 0x76, 0x44, 0x71, 0x7e, 0x47}}
 
+[PcdsFixedAtBuild, PcdsPatchableInModule]
+  ## List of {InterfaceClass, InterfaceSubClass, InterfaceProtocol} tuples that should be excluded from being managed by
+  # the UsbHidDxe driver. The list is terminated by a record of all zeros.
+  # Only interface class 3 (HID) is supported; other interface classes are ignored regardless of this list.
+  # SubClass and Protocol are defined by the USB HID 1.11 spec, section 4.2 and 4.3 respectively.
+  # - SubClass may be 0 - No Subclass, or 1 - Boot Interface Subclass.
+  # - If SubClass is 0, Protocol must be 0.
+  # - If SubClass is 1, Protocol must be 1 - Keyboard, or 2 - Mouse.
+  # For example, to exclude USB Boot Interface Keyboards, this would be set to {0x3, 0x1, 0x1, 0x0, 0x0, 0x0}
+  # @Prompt Exclude devices from UsbHidDxe management.
+  gHidPkgTokenSpaceGuid.PcdExcludedHidDevices|{0x0, 0x0, 0x0}|VOID*|0x00010100
+
 [PcdsFeatureFlag]
   ## Indicates if HID KeyBoard Driver disables the default keyboard layout.
   #  The default keyboard layout serves as the backup when no keyboard layout can be retrieved
diff --git a/HidPkg/UsbHidDxe/UsbHidDxe.c b/HidPkg/UsbHidDxe/UsbHidDxe.c
index 631478668f..8d9945ae28 100644
--- a/HidPkg/UsbHidDxe/UsbHidDxe.c
+++ b/HidPkg/UsbHidDxe/UsbHidDxe.c
@@ -26,9 +26,7 @@
 
 #include <IndustryStandard/Usb.h>
 
-#define CLASS_HID          3
-#define SUBCLASS_BOOT      1
-#define PROTOCOL_KEYBOARD  1
+#define CLASS_HID  3
 
 //
 // A common header for usb standard descriptor.
@@ -567,6 +565,9 @@ IsUsbHid (
 {
   EFI_STATUS                    Status;
   EFI_USB_INTERFACE_DESCRIPTOR  InterfaceDescriptor;
+  UINTN                         ExcludeListSize;
+  UINTN                         ExcludeListIndex;
+  UINT8                         *ExcludeList;
 
   //
   // Get the default interface descriptor
@@ -580,17 +581,33 @@ IsUsbHid (
     return FALSE;
   }
 
-  // Note: for now we exclude keyboards as they are not yet supported by
-  // UefiHidDxe. ADO #3570600 covers implementing keyboard support.
-  if ((InterfaceDescriptor.InterfaceClass == CLASS_HID) &&
-      (InterfaceDescriptor.InterfaceSubClass == SUBCLASS_BOOT) &&
-      (InterfaceDescriptor.InterfaceProtocol == PROTOCOL_KEYBOARD)
-      )
-  {
+  if (InterfaceDescriptor.InterfaceClass != CLASS_HID) {
     return FALSE;
   }
 
-  return (InterfaceDescriptor.InterfaceClass == CLASS_HID);
+  ExcludeListSize  = PcdGetSize (PcdExcludedHidDevices);
+  ExcludeList      = (UINT8 *)PcdGetPtr (PcdExcludedHidDevices);
+  ExcludeListIndex = 0;
+
+  for (ExcludeListIndex = 0; ExcludeListIndex + 2 < ExcludeListSize; ExcludeListIndex += 3) {
+    if ((ExcludeList[ExcludeListIndex] == 0) &&
+        (ExcludeList[ExcludeListIndex + 1] == 0) &&
+        (ExcludeList[ExcludeListIndex + 2] == 0))
+    {
+      // end of exclude list, and haven't found exclusion - device is supported.
+      break;
+    }
+
+    if ((ExcludeList[ExcludeListIndex] == InterfaceDescriptor.InterfaceClass) &&
+        (ExcludeList[ExcludeListIndex + 1] == InterfaceDescriptor.InterfaceSubClass) &&
+        (ExcludeList[ExcludeListIndex + 2] == InterfaceDescriptor.InterfaceProtocol))
+    {
+      // current entry matches an exclude list element - device not supported.
+      return FALSE;
+    }
+  }
+
+  return TRUE;
 }
 
 /**
diff --git a/HidPkg/UsbHidDxe/UsbHidDxe.inf b/HidPkg/UsbHidDxe/UsbHidDxe.inf
index a0a273884a..09b8fa03c3 100644
--- a/HidPkg/UsbHidDxe/UsbHidDxe.inf
+++ b/HidPkg/UsbHidDxe/UsbHidDxe.inf
@@ -35,8 +35,9 @@
   UefiUsbLib
 
 [Protocols]
-  gHidIoProtocolGuid
-  gEfiUsbIoProtocolGuid
+  gHidIoProtocolGuid    ## PRODUCES
+  gEfiUsbIoProtocolGuid ## CONSUMES
 
 [Pcd]
-  gEfiMdePkgTokenSpaceGuid.PcdUsbTransferTimeoutValue  ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdUsbTransferTimeoutValue
+  gHidPkgTokenSpaceGuid.PcdExcludedHidDevices