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

On 8/24/2026 10:37 AM, Yanfeng Liu wrote:
> On Sun, 2026-08-23 at 18:55 -0300, Daniel Henrique Barboza wrote:
>>
>>
>> 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:
>>
> I was playing Bao hypervisor and found that having a 2nd NMIO UART can
> ba handy.  Previoously when trying AMP I added 2nd MMIO UART to aid
> debugging.
> 
> Since this happened to me at least twice, I am wondering someone else
> might also need it. Besides many real devices have muitlple UARTs.

And that's fine.  In fact I think this patch is already queued for upstream
so we're going forward with the change.

I was just curious about the second UART mapping being added.  I assume that
this would be required for OpenSBI and etc to recognize both of them since
it'll read via DT.


Thanks,
Daniel



> 
>>
>>
>> $ ./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.
>>
> 
> how these additional UARTs can be used by guests if they dont have
> MMIO?
> 
> 
> Regards,
> Yanfeng
> 
> 
>>
>> 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 */
>