On Fri, 12 May 2023 at 16:23, Gerd Hoffmann <kra...@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 <kra...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
