From e7100972f2df313d1e47a0714aed968991437e86 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 10 Jun 2024 17:23:43 +0100 Subject: [PATCH] hw/arm/virt: allow creation of a second NonSecure UART MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For some use-cases, it is helpful to have more than one UART available to the guest. If the second UART slot is not already used for a TrustZone Secure-World-only UART, create it as a NonSecure UART only when the user provides a serial backend (e.g. via a second -serial command line option). This avoids problems where existing guest software only expects a single UART, and gets confused by the second UART in the DTB. The major example of this is older EDK2 firmware, which will send the GRUB bootloader output to UART1 and the guest serial output to UART0. Users who want to use both UARTs with a guest setup including EDK2 are advised to update to EDK2 release edk2-stable202311 or newer. (The prebuilt EDK2 blobs QEMU upstream provides are new enough.) The relevant EDK2 changes are the ones described here: https://bugzilla.tianocore.org/show_bug.cgi?id=4577 Inspired-by: Axel Heider Signed-off-by: Peter Maydell Tested-by: Laszlo Ersek Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240610162343.2131524-4-peter.maydell@linaro.org --- docs/system/arm/virt.rst | 6 +++++- hw/arm/virt-acpi-build.c | 12 ++++++++---- hw/arm/virt.c | 38 +++++++++++++++++++++++++++++++++++--- include/hw/arm/virt.h | 1 + 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst index 26fcba00b7..e67e7f0f7c 100644 --- a/docs/system/arm/virt.rst +++ b/docs/system/arm/virt.rst @@ -26,7 +26,7 @@ The virt board supports: - PCI/PCIe devices - Flash memory -- One PL011 UART +- Either one or two PL011 UARTs for the NonSecure World - An RTC - The fw_cfg device that allows a guest to obtain data from QEMU - A PL061 GPIO controller @@ -48,6 +48,10 @@ The virt board supports: - A secure flash memory - 16MB of secure RAM +The second NonSecure UART only exists if a backend is configured +explicitly (e.g. with a second -serial command line option) and +TrustZone emulation is not enabled. + Supported guest CPU types: - ``cortex-a7`` (32-bit) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index eb5796e309..b2366f24f9 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -79,11 +79,11 @@ static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState *vms) } static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap, - uint32_t uart_irq) + uint32_t uart_irq, int uartidx) { - Aml *dev = aml_device("COM0"); + Aml *dev = aml_device("COM%d", uartidx); aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011"))); - aml_append(dev, aml_name_decl("_UID", aml_int(0))); + aml_append(dev, aml_name_decl("_UID", aml_int(uartidx))); Aml *crs = aml_resource_template(); aml_append(crs, aml_memory32_fixed(uart_memmap->base, @@ -817,7 +817,11 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) scope = aml_scope("\\_SB"); acpi_dsdt_add_cpus(scope, vms); acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], - (irqmap[VIRT_UART0] + ARM_SPI_BASE)); + (irqmap[VIRT_UART0] + ARM_SPI_BASE), 0); + if (vms->second_ns_uart_present) { + acpi_dsdt_add_uart(scope, &memmap[VIRT_UART1], + (irqmap[VIRT_UART1] + ARM_SPI_BASE), 1); + } if (vmc->acpi_expose_flash) { acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]); } diff --git a/hw/arm/virt.c b/hw/arm/virt.c index ffb4983885..8555615256 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -906,7 +906,7 @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem) } static void create_uart(const VirtMachineState *vms, int uart, - MemoryRegion *mem, Chardev *chr) + MemoryRegion *mem, Chardev *chr, bool secure) { char *nodename; hwaddr base = vms->memmap[uart].base; @@ -944,6 +944,8 @@ static void create_uart(const VirtMachineState *vms, int uart, qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial0", nodename); } else { qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial1", nodename); + } + if (secure) { /* Mark as not usable by the normal world */ qemu_fdt_setprop_string(ms->fdt, nodename, "status", "disabled"); qemu_fdt_setprop_string(ms->fdt, nodename, "secure-status", "okay"); @@ -2317,11 +2319,41 @@ static void machvirt_init(MachineState *machine) fdt_add_pmu_nodes(vms); - create_uart(vms, VIRT_UART0, sysmem, serial_hd(0)); + /* + * The first UART always exists. If the security extensions are + * enabled, the second UART also always exists. Otherwise, it only exists + * if a backend is configured explicitly via '-serial '. + * This avoids potentially breaking existing user setups that expect + * only one NonSecure UART to be present (for instance, older EDK2 + * binaries). + * + * The nodes end up in the DTB in reverse order of creation, so we must + * create UART0 last to ensure it appears as the first node in the DTB, + * for compatibility with guest software that just iterates through the + * DTB to find the first UART, as older versions of EDK2 do. + * DTB readers that follow the spec, as Linux does, should honour the + * aliases node information and /chosen/stdout-path regardless of + * the order that nodes appear in the DTB. + * + * For similar back-compatibility reasons, if UART1 is the secure UART + * we create it second (and so it appears first in the DTB), because + * that's what QEMU has always done. + */ + if (!vms->secure) { + Chardev *serial1 = serial_hd(1); + + if (serial1) { + vms->second_ns_uart_present = true; + create_uart(vms, VIRT_UART1, sysmem, serial1, false); + } + } + create_uart(vms, VIRT_UART0, sysmem, serial_hd(0), false); + if (vms->secure) { + create_uart(vms, VIRT_UART1, secure_sysmem, serial_hd(1), true); + } if (vms->secure) { create_secure_ram(vms, secure_sysmem, secure_tag_sysmem); - create_uart(vms, VIRT_UART1, secure_sysmem, serial_hd(1)); } if (tag_sysmem) { diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index 1227e7f7f0..ab961bb6a9 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -151,6 +151,7 @@ struct VirtMachineState { bool ras; bool mte; bool dtb_randomness; + bool second_ns_uart_present; OnOffAuto acpi; VirtGICType gic_version; VirtIOMMUType iommu;