Re: [RFC v1 1/1] hw/riscv/server_platform_ref.c: add virtio bus

[Chao Liu <chao.liu.riscv@isrc.iscas.ac.cn>]

On 11/4/2025 10:19 PM, Andrew Jones wrote:
> On Tue, Nov 04, 2025 at 09:55:40AM -0300, Daniel Henrique Barboza wrote:
>> CCing Drew since he's the TG chair for this spec
>>
>> On 11/3/25 9:39 AM, Chao Liu wrote:
>>> Add an always present virtio bus for the rvsp-ref board.
>>>
>>> The IRQs being used are similar to what the 'virt' board is using: IRQs
>>> 40 to 47, one IRQ for queue.
>>>
>>> Signed-off-by: Chao Liu <chao.liu.riscv@isrc.iscas.ac.cn>
>>> ---
>>>    hw/riscv/server_platform_ref.c | 42 ++++++++++++++++++++++++++++++++--
>>>    1 file changed, 40 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/riscv/server_platform_ref.c b/hw/riscv/server_platform_ref.c
>>> index 975f07ddfb..24c3fc2e41 100644
>>> --- a/hw/riscv/server_platform_ref.c
>>> +++ b/hw/riscv/server_platform_ref.c
>>> @@ -46,6 +46,7 @@
>>>    #include "target/riscv/cpu.h"
>>>    #include "target/riscv/pmu.h"
>>>    #include "net/net.h"
>>> +#include "hw/virtio/virtio-mmio.h"
>>>    #define RVSP_CPUS_MAX_BITS             9
>>>    #define RVSP_CPUS_MAX                  (1 << RVSP_CPUS_MAX_BITS)
>>> @@ -108,16 +109,20 @@ enum {
>>>        RVSP_PCIE_MMIO,
>>>        RVSP_PCIE_PIO,
>>>        RVSP_PCIE_ECAM,
>>> -    RVSP_PCIE_MMIO_HIGH
>>> +    RVSP_PCIE_MMIO_HIGH,
>>> +    RVSP_VIRTIO
>>>    };
>>>    enum {
>>>        RVSP_UART0_IRQ = 10,
>>>        RVSP_RTC_IRQ = 11,
>>>        RVSP_PCIE_IRQ = 0x20, /* 32 to 35 */
>>> -    IOMMU_SYS_IRQ = 0x24 /* 36 to 39 */
>>> +    IOMMU_SYS_IRQ = 0x24, /* 36 to 39 */
>>> +    RVSP_VIRTIO_IRQ = 0x28 /* 40 to 47 */
>>>    };
>>> +#define RVSP_VIRTIO_COUNT 8
>>> +
>>>    /*
>>>     * The server soc reference machine physical address space used by some of the
>>>     * devices namely ACLINT, APLIC and IMSIC depend on number of Sockets, number
>>> @@ -151,6 +156,7 @@ static const MemMapEntry rvsp_ref_memmap[] = {
>>>        [RVSP_APLIC_M] =        {  0xc000000, APLIC_SIZE(RVSP_CPUS_MAX) },
>>>        [RVSP_APLIC_S] =        {  0xd000000, APLIC_SIZE(RVSP_CPUS_MAX) },
>>>        [RVSP_UART0] =          { 0x10000000,         0x100 },
>>> +    [RVSP_VIRTIO] =         { 0x10001000,        0x1000 },
>>>        [RVSP_FLASH] =          { 0x20000000,     0x4000000 },
>>>        [RVSP_IMSIC_M] =        { 0x24000000, RVSP_IMSIC_MAX_SIZE },
>>>        [RVSP_IMSIC_S] =        { 0x28000000, RVSP_IMSIC_MAX_SIZE },
>>> @@ -816,6 +822,29 @@ static void create_fdt_flash(RVSPMachineState *s, const MemMapEntry *memmap)
>>>        qemu_fdt_setprop_cell(ms->fdt, name, "bank-width", 4);
>>>    }
>>> +static void create_fdt_virtio(RVSPMachineState *s, uint32_t irq_virtio_phandle)
>>> +{
>>> +    int i;
>>> +    MachineState *ms = MACHINE(s);
>>> +    hwaddr virtio_base = rvsp_ref_memmap[RVSP_VIRTIO].base;
>>> +
>>> +    for (i = 0; i < RVSP_VIRTIO_COUNT; i++) {
>>> +        g_autofree char *name = NULL;
>>> +        uint64_t size = rvsp_ref_memmap[RVSP_VIRTIO].size;
>>> +        hwaddr addr = virtio_base + i * size;
>>> +
>>> +        name = g_strdup_printf("/soc/virtio_mmio@%"HWADDR_PRIx, addr);
>>> +
>>> +        qemu_fdt_add_subnode(ms->fdt, name);
>>> +        qemu_fdt_setprop_string(ms->fdt, name, "compatible", "virtio,mmio");
>>> +        qemu_fdt_setprop_sized_cells(ms->fdt, name, "reg", 2, addr, 2, size);
>>> +        qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent",
>>> +            irq_virtio_phandle);
>>> +        qemu_fdt_setprop_cells(ms->fdt, name, "interrupts",
>>> +                            RVSP_VIRTIO_IRQ + i, 0x4);
>>> +    }
>>> +}
>>> +
>>>    static void finalize_fdt(RVSPMachineState *s)
>>>    {
>>>        uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1;
>>> @@ -835,6 +864,8 @@ static void finalize_fdt(RVSPMachineState *s)
>>>        create_fdt_uart(s, rvsp_ref_memmap, irq_mmio_phandle);
>>>        create_fdt_rtc(s, rvsp_ref_memmap, irq_mmio_phandle);
>>> +
>>> +    create_fdt_virtio(s, irq_mmio_phandle);
>>>    }
>>>    static void create_fdt(RVSPMachineState *s, const MemMapEntry *memmap)
>>> @@ -1205,6 +1236,13 @@ static void rvsp_ref_machine_init(MachineState *machine)
>>>            }
>>>        }
>>> +    /* VirtIO MMIO devices */
>>> +    for (i = 0; i < RVSP_VIRTIO_COUNT; i++) {
>>> +        sysbus_create_simple("virtio-mmio",
>>> +            memmap[RVSP_VIRTIO].base + i * memmap[RVSP_VIRTIO].size,
>>> +            qdev_get_gpio_in(mmio_irqchip, RVSP_VIRTIO_IRQ + i));
>>> +    }
>>> +
>>
>> When you said virtio support I thought about virtio-pci and virtio-net-pci. I'm not sure if
>> we need a virtio-mmio for that.
>>
>> Also, I can't say if adding a device like virtio-mmio might conflict with something else from
>> the server plat specification. Drew, care to comment?
> 
> I'd rather we don't add virtio-mmio to the reference platform. In
> hindsight, I guess the only reason virtio was brought up was for
> virtio-mmio, since virtio-pci devices could already have been used.
> So I apologize for misleading by saying adding virtio support was
> fine.
> 
> Let's discuss the motivation. Which virtio devices were intended to
> be added to these virtio-mmio transports?
> 

Looking at it this way, we really don’t need to add virtio-mmio. At first, I added it to help
EDK II recognize the OS image.

Thanks for Drew’s reminder. With the command below, I can now make EDK II recognize the OS image:

```
   $QEMU ... \
   -drive file=$drive,format=qcow2,id=virtio-drive,if=none \
   -device virtio-blk-pci,drive=virtio-drive,id=virtio-blk-0
```

Thanks,
Chao