[edk2-devel] [PATCH v2] ArmPkg/PlatformBootManagerLib: connect non-discoverable USB hosts
Ard Biesheuvel
ard.biesheuvel at arm.com
Fri May 22 12:06:16 UTC 2020
On 5/22/20 2:03 PM, Leif Lindholm wrote:
> On Fri, May 22, 2020 at 10:40:06 +0200, Ard Biesheuvel wrote:
>> 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.
>
> The downside to doing it properly is that this now becomes another
> thing that needs to be copied around to every PlatformBootManagerLib
> implementation (i.e.
> Platform/RaspberryPi/Library/PlatformBootManagerLib/
> Silicon/Hisilicon/Library/PlatformBootManagerLib/, but also
> out-of-tree implementations).
>
> Is there any way of avoiding that?
>
Not really. We will get rid of the RPi one as soon as we can, though. If
the HiSilicon one keeps the ConnectAll() call [which is in the same
file], they don't need this change to begin with.
>
>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at 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
>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#60135): https://edk2.groups.io/g/devel/message/60135
Mute This Topic: https://groups.io/mt/74395026/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list