public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
To: devel@edk2.groups.io
Cc: liming.gao@intel.com, leif@nuviainc.com, lersek@redhat.com,
	philmd@redhat.com, Ard Biesheuvel <ard.biesheuvel@arm.com>
Subject: [PATCH v2] ArmPkg/PlatformBootManagerLib: connect non-discoverable USB hosts
Date: Fri, 22 May 2020 10:40:06 +0200	[thread overview]
Message-ID: <20200522084006.32006-1-ard.biesheuvel@arm.com> (raw)

The way the BDS handles the short-form USB device path of the console
keyboard relies on USB host controllers to be locatable via their PCI
metadata, which implies that these controllers already have a PCI I/O
protocol installed on their handle.

This is not the case for non-discoverable USB host controllers that are
supported by the NonDiscoverable PCI device driver. These controllers
must be connected first, or the BDS will never notice their existence,
and will not enable any USB keyboards connected through them.

Let's work around this by connecting these handles explicitly. This is
a bit of a stopgap, but it is the cleanest way of dealing with this
without violating the UEFI driver model entirely. This ensures that
platforms that do not rely on ConnectAll() will keep working as
expected.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---

This is tagged as v2 since it is a followup to

  'MdeModulePkg/NonDiscoverablePciDeviceDxe: connect handles on registration'

that I sent out yesterday. Laslzo made the point that it would be
unfortunate to add yet another hack that defeats the UEFI driver model
entirely, so this time, the change is in the BDS where it belongs.

Note that only USB console devices are affected by this: short form
block IO device paths used for booting are expanded and cached in a
"HDDP" variable, and so they work as expected with fast boot without
connect all.

 ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf |  4 ++
 ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c               | 42 ++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 87c9b1150c54..2f726d117d7d 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -66,6 +66,9 @@ [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
 
 [Guids]
+  gEdkiiNonDiscoverableEhciDeviceGuid
+  gEdkiiNonDiscoverableUhciDeviceGuid
+  gEdkiiNonDiscoverableXhciDeviceGuid
   gEfiFileInfoGuid
   gEfiFileSystemInfoGuid
   gEfiFileSystemVolumeLabelInfoIdGuid
@@ -74,6 +77,7 @@ [Guids]
   gUefiShellFileGuid
 
 [Protocols]
+  gEdkiiNonDiscoverableDeviceProtocolGuid
   gEfiDevicePathProtocolGuid
   gEfiGraphicsOutputProtocolGuid
   gEfiLoadedImageProtocolGuid
diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 3411219fbfdb..4aca1382b042 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -23,10 +23,12 @@
 #include <Protocol/EsrtManagement.h>
 #include <Protocol/GraphicsOutput.h>
 #include <Protocol/LoadedImage.h>
+#include <Protocol/NonDiscoverableDevice.h>
 #include <Protocol/PciIo.h>
 #include <Protocol/PciRootBridgeIo.h>
 #include <Protocol/PlatformBootManager.h>
 #include <Guid/EventGroup.h>
+#include <Guid/NonDiscoverableDevice.h>
 #include <Guid/TtyTerm.h>
 #include <Guid/SerialPortLibVendor.h>
 
@@ -254,6 +256,37 @@ IsPciDisplay (
 }
 
 
+/**
+  This FILTER_FUNCTION checks if a handle corresponds to a non-discoverable
+  USB host controller.
+**/
+STATIC
+BOOLEAN
+EFIAPI
+IsUsbHost (
+  IN EFI_HANDLE   Handle,
+  IN CONST CHAR16 *ReportText
+  )
+{
+  NON_DISCOVERABLE_DEVICE   *Device;
+  EFI_STATUS                Status;
+
+  Status = gBS->HandleProtocol (Handle,
+                  &gEdkiiNonDiscoverableDeviceProtocolGuid,
+                  (VOID **)&Device);
+  if (EFI_ERROR (Status)) {
+    return FALSE;
+  }
+
+  if (CompareGuid (Device->Type, &gEdkiiNonDiscoverableUhciDeviceGuid) ||
+      CompareGuid (Device->Type, &gEdkiiNonDiscoverableEhciDeviceGuid) ||
+      CompareGuid (Device->Type, &gEdkiiNonDiscoverableXhciDeviceGuid)) {
+    return TRUE;
+  }
+  return FALSE;
+}
+
+
 /**
   This CALLBACK_FUNCTION attempts to connect a handle non-recursively, asking
   the matching driver to produce all first-level child handles.
@@ -574,6 +607,15 @@ PlatformBootManagerBeforeConsole (
   //
   FilterAndProcess (&gEfiGraphicsOutputProtocolGuid, NULL, AddOutput);
 
+  //
+  // The core BDS code connects short-form USB device paths by explicitly
+  // looking for handles with PCI I/O installed, and checking the PCI class
+  // code whether it matches the one for a USB host controller. This means
+  // non-discoverable USB host controllers need to have the non-discoverable
+  // PCI driver attached first.
+  //
+  FilterAndProcess (&gEdkiiNonDiscoverableDeviceProtocolGuid, IsUsbHost, Connect);
+
   //
   // Add the hardcoded short-form USB keyboard device path to ConIn.
   //
-- 
2.17.1


             reply	other threads:[~2020-05-22  8:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-22  8:40 Ard Biesheuvel [this message]
2020-05-22 12:03 ` [PATCH v2] ArmPkg/PlatformBootManagerLib: connect non-discoverable USB hosts Leif Lindholm
2020-05-22 12:06   ` Ard Biesheuvel
2020-05-22 12:15     ` Leif Lindholm
2020-05-22 18:57 ` Laszlo Ersek
2020-05-22 19:03   ` [edk2-devel] " Ard Biesheuvel

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=20200522084006.32006-1-ard.biesheuvel@arm.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