Hi Alistair,

On Tue, 2026-07-14 at 01:13 +0000, Alistair Francis wrote:
> On Fri, 2026-07-03 at 12:24 +0800, Yanfeng Liu wrote:
> > This adds optional UART1 to RiscV virt board if required at
> > runtime to simplify multicore development.
> > 
> > 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
> > 
> > Signed-off-by: Yanfeng Liu <[email protected]>
> > ---
> >  docs/system/riscv/virt.rst |  5 ++++-
> >  hw/riscv/virt-acpi-build.c | 12 ++++++++----
> >  hw/riscv/virt.c            | 39 ++++++++++++++++++++++++++++------
> > --
> > --
> >  include/hw/riscv/virt.h    |  3 +++
> >  4 files changed, 44 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..9a112ccdef 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -96,6 +96,7 @@ static const MemMapEntry virt_memmap[] = {
> >      [VIRT_APLIC_S] =      {  0xd000000, APLIC_SIZE(VIRT_CPUS_MAX)
> > },
> >      [VIRT_UART0] =        { 0x10000000,         0x100 },
> >      [VIRT_VIRTIO] =       { 0x10001000,        0x1000 },
> > +    [VIRT_UART1] =        { 0x1000a000,         0x100 },
> 
> This could be at address 0x10000100 instead and squish up against
> UART0.

Actually I used 0x10000100 at the begining, but soon realized it is in
the same 4K page of UART0, thus the hypervisor refuses to assign it to
another VM for isolation purpose.  Thus current address was chosen to
be flexible. 


Regards,
Yanfeng

> 
> Otherwise:
> 
> Reviewed-by: Alistair Francis <[email protected]>
> 
> Alistair
> 
> >      [VIRT_FW_CFG] =       { 0x10100000,          0x18 },
> >      [VIRT_FLASH] =        { 0x20000000,     0x4000000 },
> >      [VIRT_IMSIC_M] =      { 0x24000000, VIRT_IMSIC_MAX_SIZE },
> > @@ -186,7 +187,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 +834,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 +1035,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 +1579,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 */

Reply via email to