[PATCH v2 0/4] ArmVirt: add VirtioSerialDxe

[PATCH v2 0/4] ArmVirt: add VirtioSerialDxe

[Gerd Hoffmann]

v2:
 - address review comments (Ard).
 - drop merged patch.

Gerd Hoffmann (4):
  ArmVirt: add VirtioSerialDxe to ArmVirtQemu builds
  ArmVirt/PlatformBootManagerLib: factor out IsVirtio()
  ArmVirt/PlatformBootManagerLib: factor out IsVirtioPci()
  ArmVirt/PlatformBootManagerLib: set up virtio serial as console

 ArmVirtPkg/ArmVirtQemu.dsc                    |   1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc              |   1 +
 .../PlatformBootManagerLib/PlatformBm.c       | 224 +++++++++++++++++-
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc          |   1 +
 4 files changed, 217 insertions(+), 10 deletions(-)

-- 
2.40.1

[PATCH v2 1/4] ArmVirt: add VirtioSerialDxe to ArmVirtQemu builds

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/ArmVirtQemu.dsc           | 1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc     | 1 +
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 +
 3 files changed, 3 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 449e73b9e132..d998f816b2b3 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -448,6 +448,7 @@ [Components.common]
   OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
   OvmfPkg/VirtioNetDxe/VirtioNet.inf
   OvmfPkg/VirtioRngDxe/VirtioRng.inf
+  OvmfPkg/VirtioSerialDxe/VirtioSerial.inf
 
   #
   # FAT filesystem + GPT/MBR partitioning + UDF filesystem + virtio-fs
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 3cb9120e4e10..c9d2b18392b9 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -353,6 +353,7 @@ [Components.common]
   OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
   OvmfPkg/VirtioNetDxe/VirtioNet.inf
   OvmfPkg/VirtioRngDxe/VirtioRng.inf
+  OvmfPkg/VirtioSerialDxe/VirtioSerial.inf
 
   #
   # FAT filesystem + GPT/MBR partitioning + UDF filesystem + virtio-fs
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index 8a063bac04ac..2894bc853a46 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -98,6 +98,7 @@ [FV.FvMain]
   INF OvmfPkg/VirtioNetDxe/VirtioNet.inf
   INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
   INF OvmfPkg/VirtioRngDxe/VirtioRng.inf
+  INF OvmfPkg/VirtioSerialDxe/VirtioSerial.inf
 
   INF ShellPkg/Application/Shell/Shell.inf
   INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
-- 
2.40.1

[PATCH v2 2/4] ArmVirt/PlatformBootManagerLib: factor out IsVirtio()

[Gerd Hoffmann]

IsVirtioRng() becomes just a thin wrapper for IsVirtio().
This allows to add similar thin wrappers for other virtio
devices in the future.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
---
 .../PlatformBootManagerLib/PlatformBm.c       | 26 +++++++++++++++----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 10c815378cd5..5eb6f0f9c14a 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -269,15 +269,16 @@ IsPciDisplay (
 }
 
 /**
-  This FILTER_FUNCTION checks if a handle corresponds to a Virtio RNG device at
-  the VIRTIO_DEVICE_PROTOCOL level.
+  This function checks if a handle corresponds to the Virtio Device ID given
+  at the VIRTIO_DEVICE_PROTOCOL level.
 **/
 STATIC
 BOOLEAN
 EFIAPI
-IsVirtioRng (
+IsVirtio (
   IN EFI_HANDLE    Handle,
-  IN CONST CHAR16  *ReportText
+  IN CONST CHAR16  *ReportText,
+  IN UINT16        VirtIoDeviceId
   )
 {
   EFI_STATUS              Status;
@@ -293,7 +294,22 @@ IsVirtioRng (
   }
 
   return (BOOLEAN)(VirtIo->SubSystemDeviceId ==
-                   VIRTIO_SUBSYSTEM_ENTROPY_SOURCE);
+                   VirtIoDeviceId);
+}
+
+/**
+  This FILTER_FUNCTION checks if a handle corresponds to a Virtio RNG device at
+  the VIRTIO_DEVICE_PROTOCOL level.
+**/
+STATIC
+BOOLEAN
+EFIAPI
+IsVirtioRng (
+  IN EFI_HANDLE    Handle,
+  IN CONST CHAR16  *ReportText
+  )
+{
+  return IsVirtio (Handle, ReportText, VIRTIO_SUBSYSTEM_ENTROPY_SOURCE);
 }
 
 /**
-- 
2.40.1

[PATCH v2 3/4] ArmVirt/PlatformBootManagerLib: factor out IsVirtioPci()

[Gerd Hoffmann]

IsVirtioPciRng() becomes just a thin wrapper for IsVirtioPci().
This allows to add similar thin wrappers for other virtio
devices in the future.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
---
 .../PlatformBootManagerLib/PlatformBm.c       | 30 ++++++++++++++-----
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 5eb6f0f9c14a..ed38c42a43ee 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -313,15 +313,16 @@ IsVirtioRng (
 }
 
 /**
-  This FILTER_FUNCTION checks if a handle corresponds to a Virtio RNG device at
-  the EFI_PCI_IO_PROTOCOL level.
+  This function checks if a handle corresponds to the Virtio Device ID given
+  at the EFI_PCI_IO_PROTOCOL level.
 **/
 STATIC
 BOOLEAN
 EFIAPI
-IsVirtioPciRng (
+IsVirtioPci (
   IN EFI_HANDLE    Handle,
-  IN CONST CHAR16  *ReportText
+  IN CONST CHAR16  *ReportText,
+  IN UINT16        VirtIoDeviceId
   )
 {
   EFI_STATUS           Status;
@@ -387,11 +388,11 @@ IsVirtioPciRng (
   //
   // From DeviceId and RevisionId, determine whether the device is a
   // modern-only Virtio 1.0 device. In case of Virtio 1.0, DeviceId can
-  // immediately be restricted to VIRTIO_SUBSYSTEM_ENTROPY_SOURCE, and
+  // immediately be restricted to VirtIoDeviceId, and
   // SubsystemId will only play a sanity-check role. Otherwise, DeviceId can
   // only be sanity-checked, and SubsystemId will decide.
   //
-  if ((DeviceId == 0x1040 + VIRTIO_SUBSYSTEM_ENTROPY_SOURCE) &&
+  if ((DeviceId == 0x1040 + VirtIoDeviceId) &&
       (RevisionId >= 0x01))
   {
     Virtio10 = TRUE;
@@ -419,7 +420,7 @@ IsVirtioPciRng (
     return TRUE;
   }
 
-  if (!Virtio10 && (SubsystemId == VIRTIO_SUBSYSTEM_ENTROPY_SOURCE)) {
+  if (!Virtio10 && (SubsystemId == VirtIoDeviceId)) {
     return TRUE;
   }
 
@@ -430,6 +431,21 @@ IsVirtioPciRng (
   return FALSE;
 }
 
+/**
+  This FILTER_FUNCTION checks if a handle corresponds to a Virtio RNG device at
+  the EFI_PCI_IO_PROTOCOL level.
+**/
+STATIC
+BOOLEAN
+EFIAPI
+IsVirtioPciRng (
+  IN EFI_HANDLE    Handle,
+  IN CONST CHAR16  *ReportText
+  )
+{
+  return IsVirtioPci (Handle, ReportText, VIRTIO_SUBSYSTEM_ENTROPY_SOURCE);
+}
+
 /**
   This CALLBACK_FUNCTION attempts to connect a handle non-recursively, asking
   the matching driver to produce all first-level child handles.
-- 
2.40.1

[PATCH v2 4/4] ArmVirt/PlatformBootManagerLib: set up virtio serial as console

[Gerd Hoffmann]

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       | 172 ++++++++++++++++++
 1 file changed, 172 insertions(+)

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
index ed38c42a43ee..b92a916f7eec 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,142 @@ 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 CONST 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 CONST 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 CONST 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, *OldDevicePath;
+
+  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,
+                 &SerialNode.Header
+                 );
+
+  OldDevicePath = DevicePath;
+  DevicePath    = AppendDevicePathNode (
+                    DevicePath,
+                    &UartNode.Header
+                    );
+  FreePool (OldDevicePath);
+
+  OldDevicePath = DevicePath;
+  DevicePath    = AppendDevicePathNode (
+                    DevicePath,
+                    &TerminalNode.Header
+                    );
+  FreePool (OldDevicePath);
+
+  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;
+  }
+
+  FreePool (DevicePath);
+
+  DEBUG ((
+    DEBUG_VERBOSE,
+    "%a: %s: added to ConIn, ConOut and ErrOut\n",
+    __func__,
+    ReportText
+    ));
+}
+
 STATIC
 VOID
 PlatformRegisterFvBootOption (
@@ -932,6 +1098,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

Re: [PATCH v2 0/4] ArmVirt: add VirtioSerialDxe

[Ard Biesheuvel]

On Thu, 1 Jun 2023 at 13:57, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> v2:
>  - address review comments (Ard).
>  - drop merged patch.
>
> Gerd Hoffmann (4):
>   ArmVirt: add VirtioSerialDxe to ArmVirtQemu builds
>   ArmVirt/PlatformBootManagerLib: factor out IsVirtio()
>   ArmVirt/PlatformBootManagerLib: factor out IsVirtioPci()
>   ArmVirt/PlatformBootManagerLib: set up virtio serial as console
>

Merged as #4463

Thanks

>  ArmVirtPkg/ArmVirtQemu.dsc                    |   1 +
>  ArmVirtPkg/ArmVirtQemuKernel.dsc              |   1 +
>  .../PlatformBootManagerLib/PlatformBm.c       | 224 +++++++++++++++++-
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc          |   1 +
>  4 files changed, 217 insertions(+), 10 deletions(-)
>
> --
> 2.40.1
>