Re: [PATCH] MdeModulePkg/ConPlatform: Support short-form USB device path

[Laszlo Ersek <lersek@redhat.com>]

Hello Ray,

On 04/25/18 07:44, Ruiyu Ni wrote:
> Today's implementation does an exact device path match to check
> whether the device path of a console is in ConIn/ConOut/ErrOut.
> But that doesn't work for the USB keyboard.
> Because when a platform have multiple USB port, ConIn needs to
> carry all device paths corresponding to each port.
> Even worse, today's BDS core logic removes the device path from
> ConIn/ConOut/ErrOut when the connection to that device path fails.
> So if user switches the USB keyboard from one port to another across
> boot, the USB keyboard doesn't work in the second boot.
>
> ConPlatform driver solved this problem by adding the
> IsHotPlugDevice() function. So that for USB keyboard, ConPlatform
> doesn't care whether its device path is in ConIn or not.
> But the rule is too loose, and now causes platform BDS cannot control
> whether to enable USB keyboard as an active console.
>
> The patch changes ConPlatform to support USB short-form device path
> when checking whether the device path of a console is in
> ConIn/ConOut/ErrOut.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> ---
>  .../Universal/Console/ConPlatformDxe/ConPlatform.c | 526 ++++++++++++++-------
>  .../Universal/Console/ConPlatformDxe/ConPlatform.h |  20 +-
>  2 files changed, 353 insertions(+), 193 deletions(-)

just a quick question: in OvmfPkg and ArmVirtPkg (more precisely:
ArmVirtQemu*), we add a "wild card" USB keyboard to ConIn:

> STATIC PLATFORM_USB_KEYBOARD mUsbKeyboard = {
>   //
>   // USB_CLASS_DEVICE_PATH Keyboard
>   //
>   {
>     {
>       MESSAGING_DEVICE_PATH, MSG_USB_CLASS_DP,
>       DP_NODE_LEN (USB_CLASS_DEVICE_PATH)
>     },
>     0xFFFF, // VendorId: any
>     0xFFFF, // ProductId: any
>     3,      // DeviceClass: HID
>     1,      // DeviceSubClass: boot
>     1       // DeviceProtocol: keyboard
>   },
>
>   //
>   // EFI_DEVICE_PATH_PROTOCOL End
>   //
>   {
>     END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
>     DP_NODE_LEN (EFI_DEVICE_PATH_PROTOCOL)
>   }
> };
>
> [...]
>
>   //
>   // Add the hardcoded short-form USB keyboard device path to ConIn.
>   //
>   EfiBootManagerUpdateConsoleVariable (ConIn,
>     (EFI_DEVICE_PATH_PROTOCOL *)&mUsbKeyboard, NULL);

The idea is to connect any USB keyboard(s) that the virtual machine
might have.

This patch for ConPlatformDxe will keep that working, right?

Thanks!
Laszlo