Re: [PATCH v3] riscv/virt: Add optional UART1

On 7/17/2026 11:16 PM, Yanfeng Liu wrote:
> This adds optional UART1 to RiscV virt board if required at
> runtime to simplify multicore development.

Can you please elaborate on the use case?  We can add multiple serial
devices already.  E.g. this will add 4 serials:


$ ./build/qemu-system-riscv64 -M virt --nodefaults \
          -serial stdio \
          -serial tcp::4444,server,nowait \
          -serial pty  \
          -serial file:uart4.log \
          --nographic


The difference here is that the additional serials won't have memory mappings
and fdt nodes.


Thanks,
Daniel

> 
> Note that UART0 remains default serial_hd(0) and it is:
> 
> - the lowest address UART
> - first serial in DTB
> - behind /aliases/serial0 in DTB
> - the /chosen/stdout-path in DTB
> 
> Note that UART1 is placed at different page from UART0 to
> support page level isolation.
> 
> Signed-off-by: Yanfeng Liu <[email protected]>
> ---
>   docs/system/riscv/virt.rst |  5 ++++-
>   hw/riscv/virt-acpi-build.c | 12 ++++++++----
>   hw/riscv/virt.c            | 40 ++++++++++++++++++++++++++++----------
>   include/hw/riscv/virt.h    |  3 +++
>   4 files changed, 45 insertions(+), 15 deletions(-)
> 
> diff --git a/docs/system/riscv/virt.rst b/docs/system/riscv/virt.rst
> index 60850970ce..45ac63ac77 100644
> --- a/docs/system/riscv/virt.rst
> +++ b/docs/system/riscv/virt.rst
> @@ -16,7 +16,7 @@ The ``virt`` machine supports the following devices:
>   * Core Local Interruptor (CLINT)
>   * Platform-Level Interrupt Controller (PLIC)
>   * CFI parallel NOR flash memory
> -* 1 NS16550 compatible UART
> +* Either 1 or 2 NS16550 compatible UARTs
>   * 1 Google Goldfish RTC
>   * 1 SiFive Test device
>   * 8 virtio-mmio transport devices
> @@ -27,6 +27,9 @@ The hypervisor extension has been enabled for the default CPU, so virtual
>   machines with hypervisor extension can simply be used without explicitly
>   declaring.
>   
> +The second UART only exists if a backend is configured explicitly (e.g.
> +with a second `-serial` command line option).
> +
>   Hardware configuration information
>   ----------------------------------
>   
> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> index 413d47d70e..597751c84a 100644
> --- a/hw/riscv/virt-acpi-build.c
> +++ b/hw/riscv/virt-acpi-build.c
> @@ -169,11 +169,11 @@ static void acpi_dsdt_add_plic_aplic(Aml *scope, uint8_t socket_count,
>   
>   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("RSCV0003")));
> -    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,
> @@ -479,7 +479,11 @@ static void build_dsdt(GArray *table_data,
>                                    memmap[VIRT_APLIC_S].size, "RSCV0002");
>       }
>   
> -    acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], UART0_IRQ);
> +    acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0], UART0_IRQ, 0);
> +    if (s->uart1_present) {
> +        acpi_dsdt_add_uart(scope, &memmap[VIRT_UART1], UART1_IRQ, 1);
> +    }
> +
>       if (virt_is_iommu_sys_enabled(s)) {
>           acpi_dsdt_add_iommu_sys(scope, &memmap[VIRT_IOMMU_SYS], IOMMU_SYS_IRQ);
>       }
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 33775a61fd..4703ac58f8 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -96,6 +96,8 @@ static const MemMapEntry virt_memmap[] = {
>       [VIRT_APLIC_S] =      {  0xd000000, APLIC_SIZE(VIRT_CPUS_MAX) },
>       [VIRT_UART0] =        { 0x10000000,         0x100 },
>       [VIRT_VIRTIO] =       { 0x10001000,        0x1000 },
> +    /* UART1 supports page isolation from UART0 */
> +    [VIRT_UART1] =        { 0x1000a000,         0x100 },
>       [VIRT_FW_CFG] =       { 0x10100000,          0x18 },
>       [VIRT_FLASH] =        { 0x20000000,     0x4000000 },
>       [VIRT_IMSIC_M] =      { 0x24000000, VIRT_IMSIC_MAX_SIZE },
> @@ -186,7 +188,8 @@ static void create_pcie_irq_map(RISCVVirtState *s, void *fdt, char *nodename,
>                             FDT_MAX_INT_MAP_WIDTH] = {};
>       uint32_t *irq_map = full_irq_map;
>   
> -    /* This code creates a standard swizzle of interrupts such that
> +    /*
> +     * This code creates a standard swizzle of interrupts such that
>        * each device's first interrupt is based on it's PCI_SLOT number.
>        * (See pci_swizzle_map_irq_fn())
>        *
> @@ -832,28 +835,38 @@ static void create_fdt_reset(RISCVVirtState *s, uint32_t *phandle)
>   }
>   
>   static void create_fdt_uart(RISCVVirtState *s,
> -                            uint32_t irq_mmio_phandle)
> +                            uint32_t irq_mmio_phandle, int memId, int irqNo)
>   {
>       g_autofree char *name = NULL;
>       MachineState *ms = MACHINE(s);
>   
>       name = g_strdup_printf("/soc/serial@%"HWADDR_PRIx,
> -                           s->memmap[VIRT_UART0].base);
> +                           s->memmap[memId].base);
>       qemu_fdt_add_subnode(ms->fdt, name);
>       qemu_fdt_setprop_string(ms->fdt, name, "compatible", "ns16550a");
>       qemu_fdt_setprop_sized_cells(ms->fdt, name, "reg",
> -                                 2, s->memmap[VIRT_UART0].base,
> -                                 2, s->memmap[VIRT_UART0].size);
> +                                 2, s->memmap[memId].base,
> +                                 2, s->memmap[memId].size);
>       qemu_fdt_setprop_cell(ms->fdt, name, "clock-frequency", 3686400);
>       qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent", irq_mmio_phandle);
>       if (s->aia_type == VIRT_AIA_TYPE_NONE) {
> -        qemu_fdt_setprop_cell(ms->fdt, name, "interrupts", UART0_IRQ);
> +        qemu_fdt_setprop_cell(ms->fdt, name, "interrupts", irqNo);
>       } else {
> -        qemu_fdt_setprop_cells(ms->fdt, name, "interrupts", UART0_IRQ, 0x4);
> +        qemu_fdt_setprop_cells(ms->fdt, name, "interrupts", irqNo, 0x4);
> +    }
> +
> +    if (VIRT_UART0 == memId) {
> +        qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", name);
> +        qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial0", name);
>       }
> +}
>   
> -    qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", name);
> -    qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial0", name);
> +static void create_fdt_uarts(RISCVVirtState *s, uint32_t irq_mmio_phandle)
> +{
> +    if (s->uart1_present) {
> +        create_fdt_uart(s, irq_mmio_phandle, VIRT_UART1, UART1_IRQ);
> +    }
> +    create_fdt_uart(s, irq_mmio_phandle, VIRT_UART0, UART0_IRQ);
>   }
>   
>   static void create_fdt_rtc(RISCVVirtState *s,
> @@ -1023,7 +1036,7 @@ static void finalize_fdt(RISCVVirtState *s)
>   
>       create_fdt_reset(s, &phandle);
>   
> -    create_fdt_uart(s, irq_mmio_phandle);
> +    create_fdt_uarts(s, irq_mmio_phandle);
>   
>       create_fdt_rtc(s, irq_mmio_phandle);
>   }
> @@ -1567,6 +1580,13 @@ static void virt_machine_init(MachineState *machine)
>           0, qdev_get_gpio_in(mmio_irqchip, UART0_IRQ), 399193,
>           serial_hd(0), DEVICE_LITTLE_ENDIAN);
>   
> +    if (serial_hd(1)) {
> +        serial_mm_init(system_memory, s->memmap[VIRT_UART1].base,
> +            0, qdev_get_gpio_in(mmio_irqchip, UART1_IRQ), 399193,
> +            serial_hd(1), DEVICE_LITTLE_ENDIAN);
> +        s->uart1_present = true;
> +    }
> +
>       sysbus_create_simple("goldfish_rtc", s->memmap[VIRT_RTC].base,
>           qdev_get_gpio_in(mmio_irqchip, RTC_IRQ));
>   
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 18a2a323a3..7d57c8292e 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -59,6 +59,7 @@ struct RISCVVirtState {
>       int aia_guests;
>       char *oem_id;
>       char *oem_table_id;
> +    bool uart1_present;
>       OnOffAuto acpi;
>       const MemMapEntry *memmap;
>       struct GPEXHost *gpex_host;
> @@ -78,6 +79,7 @@ enum {
>       VIRT_APLIC_S,
>       VIRT_UART0,
>       VIRT_VIRTIO,
> +    VIRT_UART1,
>       VIRT_FW_CFG,
>       VIRT_IMSIC_M,
>       VIRT_IMSIC_S,
> @@ -93,6 +95,7 @@ enum {
>   enum {
>       UART0_IRQ = 10,
>       RTC_IRQ = 11,
> +    UART1_IRQ = 12,
>       VIRTIO_IRQ = 1, /* 1 to 8 */
>       VIRTIO_COUNT = 8,
>       PCIE_IRQ = 0x20, /* 32 to 35 */