[edk2-devel] [PATCH 4/5] ArmVirt/PlatformBootManagerLib: set up virtio serial as console

Gerd Hoffmann posted 5 patches 1 year, 1 month ago
There is a newer version of this series
[edk2-devel] [PATCH 4/5] ArmVirt/PlatformBootManagerLib: set up virtio serial as console
Posted by Gerd Hoffmann 1 year, 1 month ago
In case a virtio serial device is found in the system register the first
console port as EFI console, by updating ConIn, ConOut and ErrOut.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 .../PlatformBootManagerLib/PlatformBm.c       | 163 ++++++++++++++++++
 1 file changed, 163 insertions(+)

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
index ed38c42a43ee..7010d73c1388 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -312,6 +312,21 @@ IsVirtioRng (
   return IsVirtio (Handle, ReportText, VIRTIO_SUBSYSTEM_ENTROPY_SOURCE);
 }
 
+/**
+  This FILTER_FUNCTION checks if a handle corresponds to a Virtio serial device at
+  the VIRTIO_DEVICE_PROTOCOL level.
+**/
+STATIC
+BOOLEAN
+EFIAPI
+IsVirtioSerial (
+  IN EFI_HANDLE    Handle,
+  IN CONST CHAR16  *ReportText
+  )
+{
+  return IsVirtio (Handle, ReportText, VIRTIO_SUBSYSTEM_CONSOLE);
+}
+
 /**
   This function checks if a handle corresponds to the Virtio Device ID given
   at the EFI_PCI_IO_PROTOCOL level.
@@ -446,6 +461,21 @@ IsVirtioPciRng (
   return IsVirtioPci (Handle, ReportText, VIRTIO_SUBSYSTEM_ENTROPY_SOURCE);
 }
 
+/**
+  This FILTER_FUNCTION checks if a handle corresponds to a Virtio serial device at
+  the EFI_PCI_IO_PROTOCOL level.
+**/
+STATIC
+BOOLEAN
+EFIAPI
+IsVirtioPciSerial (
+  IN EFI_HANDLE    Handle,
+  IN CONST CHAR16  *ReportText
+  )
+{
+  return IsVirtioPci (Handle, ReportText, VIRTIO_SUBSYSTEM_CONSOLE);
+}
+
 /**
   This CALLBACK_FUNCTION attempts to connect a handle non-recursively, asking
   the matching driver to produce all first-level child handles.
@@ -534,6 +564,133 @@ AddOutput (
     ));
 }
 
+/**
+  This CALLBACK_FUNCTION retrieves the EFI_DEVICE_PATH_PROTOCOL from
+  the handle, appends serial, uart and terminal nodes, finally updates
+  ConIn, ConOut and ErrOut.
+**/
+STATIC
+VOID
+EFIAPI
+SetupVirtioSerial (
+  IN EFI_HANDLE    Handle,
+  IN CONST CHAR16  *ReportText
+  )
+{
+  STATIC ACPI_HID_DEVICE_PATH  SerialNode = {
+    {
+      ACPI_DEVICE_PATH,
+      ACPI_DP,
+      {
+        (UINT8)(sizeof (ACPI_HID_DEVICE_PATH)),
+        (UINT8)((sizeof (ACPI_HID_DEVICE_PATH)) >> 8)
+      },
+    },
+    EISA_PNP_ID (0x0501),
+    0
+  };
+
+  STATIC UART_DEVICE_PATH  UartNode = {
+    {
+      MESSAGING_DEVICE_PATH,
+      MSG_UART_DP,
+      {
+        (UINT8)(sizeof (UART_DEVICE_PATH)),
+        (UINT8)((sizeof (UART_DEVICE_PATH)) >> 8)
+      },
+    },
+    0,
+    115200,
+    8,
+    1,
+    1
+  };
+
+  STATIC VENDOR_DEVICE_PATH  TerminalNode = {
+    {
+      MESSAGING_DEVICE_PATH,
+      MSG_VENDOR_DP,
+      {
+        (UINT8)(sizeof (VENDOR_DEVICE_PATH)),
+        (UINT8)((sizeof (VENDOR_DEVICE_PATH)) >> 8)
+      },
+    },
+    DEVICE_PATH_MESSAGING_VT_UTF8
+  };
+
+  EFI_STATUS                Status;
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+
+  DevicePath = DevicePathFromHandle (Handle);
+
+  if (DevicePath == NULL) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: %s: handle %p: device path not found\n",
+      __func__,
+      ReportText,
+      Handle
+      ));
+    return;
+  }
+
+  DevicePath = AppendDevicePathNode (
+                 DevicePath,
+                 (EFI_DEVICE_PATH_PROTOCOL *)&SerialNode
+                 );
+  DevicePath = AppendDevicePathNode (
+                 DevicePath,
+                 (EFI_DEVICE_PATH_PROTOCOL *)&UartNode
+                 );
+  DevicePath = AppendDevicePathNode (
+                 DevicePath,
+                 (EFI_DEVICE_PATH_PROTOCOL *)&TerminalNode
+                 );
+
+  Status = EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: %s: adding to ConIn: %r\n",
+      __func__,
+      ReportText,
+      Status
+      ));
+    return;
+  }
+
+  Status = EfiBootManagerUpdateConsoleVariable (ConOut, DevicePath, NULL);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: %s: adding to ConOut: %r\n",
+      __func__,
+      ReportText,
+      Status
+      ));
+    return;
+  }
+
+  Status = EfiBootManagerUpdateConsoleVariable (ErrOut, DevicePath, NULL);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: %s: adding to ErrOut: %r\n",
+      __func__,
+      ReportText,
+      Status
+      ));
+    return;
+  }
+
+  DEBUG ((
+    DEBUG_VERBOSE,
+    "%a: %s: added to ConIn, ConOut and ErrOut\n",
+    __func__,
+    ReportText
+    ));
+}
+
 STATIC
 VOID
 PlatformRegisterFvBootOption (
@@ -932,6 +1089,12 @@ PlatformBootManagerBeforeConsole (
   // instances on Virtio PCI RNG devices.
   //
   FilterAndProcess (&gEfiPciIoProtocolGuid, IsVirtioPciRng, Connect);
+
+  //
+  // Register Virtio serial devices as console.
+  //
+  FilterAndProcess (&gVirtioDeviceProtocolGuid, IsVirtioSerial, SetupVirtioSerial);
+  FilterAndProcess (&gEfiPciIoProtocolGuid, IsVirtioPciSerial, SetupVirtioSerial);
 }
 
 /**
-- 
2.40.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104805): https://edk2.groups.io/g/devel/message/104805
Mute This Topic: https://groups.io/mt/98850085/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 4/5] ArmVirt/PlatformBootManagerLib: set up virtio serial as console
Posted by Ard Biesheuvel 1 year ago
On Fri, 12 May 2023 at 16:23, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> In case a virtio serial device is found in the system register the first
> console port as EFI console, by updating ConIn, ConOut and ErrOut.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  .../PlatformBootManagerLib/PlatformBm.c       | 163 ++++++++++++++++++
>  1 file changed, 163 insertions(+)
>
> diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index ed38c42a43ee..7010d73c1388 100644
> --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -312,6 +312,21 @@ IsVirtioRng (
>    return IsVirtio (Handle, ReportText, VIRTIO_SUBSYSTEM_ENTROPY_SOURCE);
>  }
>
> +/**
> +  This FILTER_FUNCTION checks if a handle corresponds to a Virtio serial device at
> +  the VIRTIO_DEVICE_PROTOCOL level.
> +**/
> +STATIC
> +BOOLEAN
> +EFIAPI
> +IsVirtioSerial (
> +  IN EFI_HANDLE    Handle,
> +  IN CONST CHAR16  *ReportText
> +  )
> +{
> +  return IsVirtio (Handle, ReportText, VIRTIO_SUBSYSTEM_CONSOLE);
> +}
> +
>  /**
>    This function checks if a handle corresponds to the Virtio Device ID given
>    at the EFI_PCI_IO_PROTOCOL level.
> @@ -446,6 +461,21 @@ IsVirtioPciRng (
>    return IsVirtioPci (Handle, ReportText, VIRTIO_SUBSYSTEM_ENTROPY_SOURCE);
>  }
>
> +/**
> +  This FILTER_FUNCTION checks if a handle corresponds to a Virtio serial device at
> +  the EFI_PCI_IO_PROTOCOL level.
> +**/
> +STATIC
> +BOOLEAN
> +EFIAPI
> +IsVirtioPciSerial (
> +  IN EFI_HANDLE    Handle,
> +  IN CONST CHAR16  *ReportText
> +  )
> +{
> +  return IsVirtioPci (Handle, ReportText, VIRTIO_SUBSYSTEM_CONSOLE);
> +}
> +
>  /**
>    This CALLBACK_FUNCTION attempts to connect a handle non-recursively, asking
>    the matching driver to produce all first-level child handles.
> @@ -534,6 +564,133 @@ AddOutput (
>      ));
>  }
>
> +/**
> +  This CALLBACK_FUNCTION retrieves the EFI_DEVICE_PATH_PROTOCOL from
> +  the handle, appends serial, uart and terminal nodes, finally updates
> +  ConIn, ConOut and ErrOut.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +SetupVirtioSerial (
> +  IN EFI_HANDLE    Handle,
> +  IN CONST CHAR16  *ReportText
> +  )
> +{
> +  STATIC ACPI_HID_DEVICE_PATH  SerialNode = {
> +    {
> +      ACPI_DEVICE_PATH,
> +      ACPI_DP,
> +      {
> +        (UINT8)(sizeof (ACPI_HID_DEVICE_PATH)),
> +        (UINT8)((sizeof (ACPI_HID_DEVICE_PATH)) >> 8)
> +      },
> +    },
> +    EISA_PNP_ID (0x0501),
> +    0
> +  };
> +
> +  STATIC UART_DEVICE_PATH  UartNode = {
> +    {
> +      MESSAGING_DEVICE_PATH,
> +      MSG_UART_DP,
> +      {
> +        (UINT8)(sizeof (UART_DEVICE_PATH)),
> +        (UINT8)((sizeof (UART_DEVICE_PATH)) >> 8)
> +      },
> +    },
> +    0,
> +    115200,
> +    8,
> +    1,
> +    1
> +  };
> +
> +  STATIC VENDOR_DEVICE_PATH  TerminalNode = {
> +    {
> +      MESSAGING_DEVICE_PATH,
> +      MSG_VENDOR_DP,
> +      {
> +        (UINT8)(sizeof (VENDOR_DEVICE_PATH)),
> +        (UINT8)((sizeof (VENDOR_DEVICE_PATH)) >> 8)
> +      },
> +    },
> +    DEVICE_PATH_MESSAGING_VT_UTF8
> +  };
> +

Please make these STATIC CONST

> +  EFI_STATUS                Status;
> +  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
> +
> +  DevicePath = DevicePathFromHandle (Handle);
> +
> +  if (DevicePath == NULL) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: %s: handle %p: device path not found\n",
> +      __func__,
> +      ReportText,
> +      Handle
> +      ));
> +    return;
> +  }
> +
> +  DevicePath = AppendDevicePathNode (
> +                 DevicePath,
> +                 (EFI_DEVICE_PATH_PROTOCOL *)&SerialNode

You can use &SerialNode.Header and drop the cast (same below)

> +                 );
> +  DevicePath = AppendDevicePathNode (
> +                 DevicePath,
> +                 (EFI_DEVICE_PATH_PROTOCOL *)&UartNode
> +                 );
> +  DevicePath = AppendDevicePathNode (
> +                 DevicePath,
> +                 (EFI_DEVICE_PATH_PROTOCOL *)&TerminalNode
> +                 );
> +

This leaks two copies of DevicePath - better to assign to a temp
variable and FreePool() it between calls.

DevicePath = DevicePathFromHandle()
DevicePath = AppendDevicePathNode(DevicePath)
TempDp = AppendDevicePathNode(DevicePath)
Free(DevicePath)
DevicePath = AppendDevicePathNode(TempDp)
Free(TempDp)

and then free DevicePath at the end too.


> +  Status = EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: %s: adding to ConIn: %r\n",
> +      __func__,
> +      ReportText,
> +      Status
> +      ));
> +    return;
> +  }
> +
> +  Status = EfiBootManagerUpdateConsoleVariable (ConOut, DevicePath, NULL);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: %s: adding to ConOut: %r\n",
> +      __func__,
> +      ReportText,
> +      Status
> +      ));
> +    return;
> +  }
> +
> +  Status = EfiBootManagerUpdateConsoleVariable (ErrOut, DevicePath, NULL);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: %s: adding to ErrOut: %r\n",
> +      __func__,
> +      ReportText,
> +      Status
> +      ));
> +    return;
> +  }
> +
> +  DEBUG ((
> +    DEBUG_VERBOSE,
> +    "%a: %s: added to ConIn, ConOut and ErrOut\n",
> +    __func__,
> +    ReportText
> +    ));
> +}
> +
>  STATIC
>  VOID
>  PlatformRegisterFvBootOption (
> @@ -932,6 +1089,12 @@ PlatformBootManagerBeforeConsole (
>    // instances on Virtio PCI RNG devices.
>    //
>    FilterAndProcess (&gEfiPciIoProtocolGuid, IsVirtioPciRng, Connect);
> +
> +  //
> +  // Register Virtio serial devices as console.
> +  //
> +  FilterAndProcess (&gVirtioDeviceProtocolGuid, IsVirtioSerial, SetupVirtioSerial);
> +  FilterAndProcess (&gEfiPciIoProtocolGuid, IsVirtioPciSerial, SetupVirtioSerial);
>  }
>
>  /**
> --
> 2.40.1
>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105539): https://edk2.groups.io/g/devel/message/105539
Mute This Topic: https://groups.io/mt/98850085/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-