[PATCH v4 0/6] hw/riscv: Add TPM support to the virt board

[PATCH v4 0/6] hw/riscv: Add TPM support to the virt board

[Alistair Francis]

From: Alistair Francis <alistair.francis@wdc.com>

This series adds support for connecting TPM devices to the RISC-V virt
board. This is similar to how it works for the ARM virt board.

This was tested by first creating an emulated TPM device:

    swtpm socket --tpm2 -t -d --tpmstate dir=/tmp/tpm \
        --ctrl type=unixio,path=swtpm-sock

Then launching QEMU with:

    -chardev socket,id=chrtpm,path=swtpm-sock \
    -tpmdev emulator,id=tpm0,chardev=chrtpm \
    -device tpm-tis-device,tpmdev=tpm0

The TPM device can be seen in the memory tree and the generated device
tree.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/942

Alistair Francis (6):
  hw/riscv: virt: Add a machine done notifier
  hw/core: Move the ARM sysbus-fdt to core
  hw/riscv: virt: Create a platform bus
  hw/riscv: virt: Add support for generating platform FDT entries
  hw/riscv: virt: Add device plug support
  hw/riscv: Enable TPM backends

 include/hw/{arm => core}/sysbus-fdt.h |   0
 include/hw/riscv/virt.h               |   8 +-
 hw/arm/virt.c                         |   2 +-
 hw/arm/xlnx-versal-virt.c             |   1 -
 hw/{arm => core}/sysbus-fdt.c         |   2 +-
 hw/riscv/virt.c                       | 312 +++++++++++++++++---------
 hw/arm/meson.build                    |   1 -
 hw/core/meson.build                   |   1 +
 hw/riscv/Kconfig                      |   2 +
 9 files changed, 221 insertions(+), 108 deletions(-)
 rename include/hw/{arm => core}/sysbus-fdt.h (100%)
 rename hw/{arm => core}/sysbus-fdt.c (99%)

-- 
2.35.1

[PATCH v4 1/6] hw/riscv: virt: Add a machine done notifier

[Alistair Francis]

From: Alistair Francis <alistair.francis@wdc.com>

Move the binary and device tree loading code to the machine done
notifier. This allows us to prepare for editing the device tree as part
of the notifier.

This is based on similar code in the ARM virt machine.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 include/hw/riscv/virt.h |   1 +
 hw/riscv/virt.c         | 180 +++++++++++++++++++++-------------------
 2 files changed, 97 insertions(+), 84 deletions(-)

diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 78b058ec86..8b8db3fb7c 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -45,6 +45,7 @@ struct RISCVVirtState {
     MachineState parent;
 
     /*< public >*/
+    Notifier machine_done;
     RISCVHartArrayState soc[VIRT_SOCKETS_MAX];
     DeviceState *irqchip[VIRT_SOCKETS_MAX];
     PFlashCFI01 *flash[2];
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index da50cbed43..3d48469367 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1156,6 +1156,95 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
     return aplic_m;
 }
 
+static
+void virt_machine_done(Notifier *notifier, void *data)
+{
+    RISCVVirtState *s = container_of(notifier, RISCVVirtState,
+                                     machine_done);
+    const MemMapEntry *memmap = virt_memmap;
+    MachineState *machine = MACHINE(s);
+    target_ulong start_addr = memmap[VIRT_DRAM].base;
+    target_ulong firmware_end_addr, kernel_start_addr;
+    uint32_t fdt_load_addr;
+    uint64_t kernel_entry;
+
+    /*
+     * Only direct boot kernel is currently supported for KVM VM,
+     * so the "-bios" parameter is ignored and treated like "-bios none"
+     * when KVM is enabled.
+     */
+    if (kvm_enabled()) {
+        g_free(machine->firmware);
+        machine->firmware = g_strdup("none");
+    }
+
+    if (riscv_is_32bit(&s->soc[0])) {
+        firmware_end_addr = riscv_find_and_load_firmware(machine,
+                                    RISCV32_BIOS_BIN, start_addr, NULL);
+    } else {
+        firmware_end_addr = riscv_find_and_load_firmware(machine,
+                                    RISCV64_BIOS_BIN, start_addr, NULL);
+    }
+
+    if (machine->kernel_filename) {
+        kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0],
+                                                         firmware_end_addr);
+
+        kernel_entry = riscv_load_kernel(machine->kernel_filename,
+                                         kernel_start_addr, NULL);
+
+        if (machine->initrd_filename) {
+            hwaddr start;
+            hwaddr end = riscv_load_initrd(machine->initrd_filename,
+                                           machine->ram_size, kernel_entry,
+                                           &start);
+            qemu_fdt_setprop_cell(machine->fdt, "/chosen",
+                                  "linux,initrd-start", start);
+            qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end",
+                                  end);
+        }
+    } else {
+       /*
+        * If dynamic firmware is used, it doesn't know where is the next mode
+        * if kernel argument is not set.
+        */
+        kernel_entry = 0;
+    }
+
+    if (drive_get(IF_PFLASH, 0, 0)) {
+        /*
+         * Pflash was supplied, let's overwrite the address we jump to after
+         * reset to the base of the flash.
+         */
+        start_addr = virt_memmap[VIRT_FLASH].base;
+    }
+
+    /*
+     * Init fw_cfg.  Must be done before riscv_load_fdt, otherwise the device
+     * tree cannot be altered and we get FDT_ERR_NOSPACE.
+     */
+    s->fw_cfg = create_fw_cfg(machine);
+    rom_set_fw(s->fw_cfg);
+
+    /* Compute the fdt load address in dram */
+    fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
+                                   machine->ram_size, machine->fdt);
+    /* load the reset vector */
+    riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr,
+                              virt_memmap[VIRT_MROM].base,
+                              virt_memmap[VIRT_MROM].size, kernel_entry,
+                              fdt_load_addr, machine->fdt);
+
+    /*
+     * Only direct boot kernel is currently supported for KVM VM,
+     * So here setup kernel start address and fdt address.
+     * TODO:Support firmware loading and integrate to TCG start
+     */
+    if (kvm_enabled()) {
+        riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
+    }
+}
+
 static void virt_machine_init(MachineState *machine)
 {
     const MemMapEntry *memmap = virt_memmap;
@@ -1163,10 +1252,6 @@ static void virt_machine_init(MachineState *machine)
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
     char *soc_name;
-    target_ulong start_addr = memmap[VIRT_DRAM].base;
-    target_ulong firmware_end_addr, kernel_start_addr;
-    uint32_t fdt_load_addr;
-    uint64_t kernel_entry;
     DeviceState *mmio_irqchip, *virtio_irqchip, *pcie_irqchip;
     int i, base_hartid, hart_count;
 
@@ -1296,92 +1381,12 @@ static void virt_machine_init(MachineState *machine)
     memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
         machine->ram);
 
-    /* create device tree */
-    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
-               riscv_is_32bit(&s->soc[0]));
-
     /* boot rom */
     memory_region_init_rom(mask_rom, NULL, "riscv_virt_board.mrom",
                            memmap[VIRT_MROM].size, &error_fatal);
     memory_region_add_subregion(system_memory, memmap[VIRT_MROM].base,
                                 mask_rom);
 
-    /*
-     * Only direct boot kernel is currently supported for KVM VM,
-     * so the "-bios" parameter is ignored and treated like "-bios none"
-     * when KVM is enabled.
-     */
-    if (kvm_enabled()) {
-        g_free(machine->firmware);
-        machine->firmware = g_strdup("none");
-    }
-
-    if (riscv_is_32bit(&s->soc[0])) {
-        firmware_end_addr = riscv_find_and_load_firmware(machine,
-                                    RISCV32_BIOS_BIN, start_addr, NULL);
-    } else {
-        firmware_end_addr = riscv_find_and_load_firmware(machine,
-                                    RISCV64_BIOS_BIN, start_addr, NULL);
-    }
-
-    if (machine->kernel_filename) {
-        kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0],
-                                                         firmware_end_addr);
-
-        kernel_entry = riscv_load_kernel(machine->kernel_filename,
-                                         kernel_start_addr, NULL);
-
-        if (machine->initrd_filename) {
-            hwaddr start;
-            hwaddr end = riscv_load_initrd(machine->initrd_filename,
-                                           machine->ram_size, kernel_entry,
-                                           &start);
-            qemu_fdt_setprop_cell(machine->fdt, "/chosen",
-                                  "linux,initrd-start", start);
-            qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end",
-                                  end);
-        }
-    } else {
-       /*
-        * If dynamic firmware is used, it doesn't know where is the next mode
-        * if kernel argument is not set.
-        */
-        kernel_entry = 0;
-    }
-
-    if (drive_get(IF_PFLASH, 0, 0)) {
-        /*
-         * Pflash was supplied, let's overwrite the address we jump to after
-         * reset to the base of the flash.
-         */
-        start_addr = virt_memmap[VIRT_FLASH].base;
-    }
-
-    /*
-     * Init fw_cfg.  Must be done before riscv_load_fdt, otherwise the device
-     * tree cannot be altered and we get FDT_ERR_NOSPACE.
-     */
-    s->fw_cfg = create_fw_cfg(machine);
-    rom_set_fw(s->fw_cfg);
-
-    /* Compute the fdt load address in dram */
-    fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
-                                   machine->ram_size, machine->fdt);
-    /* load the reset vector */
-    riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr,
-                              virt_memmap[VIRT_MROM].base,
-                              virt_memmap[VIRT_MROM].size, kernel_entry,
-                              fdt_load_addr, machine->fdt);
-
-    /*
-     * Only direct boot kernel is currently supported for KVM VM,
-     * So here setup kernel start address and fdt address.
-     * TODO:Support firmware loading and integrate to TCG start
-     */
-    if (kvm_enabled()) {
-        riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
-    }
-
     /* SiFive Test MMIO device */
     sifive_test_create(memmap[VIRT_TEST].base);
 
@@ -1417,6 +1422,13 @@ static void virt_machine_init(MachineState *machine)
                                   drive_get(IF_PFLASH, 0, i));
     }
     virt_flash_map(s, system_memory);
+
+    /* create device tree */
+    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
+               riscv_is_32bit(&s->soc[0]));
+
+    s->machine_done.notify = virt_machine_done;
+    qemu_add_machine_init_done_notifier(&s->machine_done);
 }
 
 static void virt_machine_instance_init(Object *obj)
-- 
2.35.1

[PATCH v4 2/6] hw/core: Move the ARM sysbus-fdt to core

[Alistair Francis]

From: Alistair Francis <alistair.francis@wdc.com>

The ARM virt machine currently uses sysbus-fdt to create device tree
entries for dynamically created MMIO devices.

The RISC-V virt machine can also benefit from this, so move the code to
the core directory.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 include/hw/{arm => core}/sysbus-fdt.h | 0
 hw/arm/virt.c                         | 2 +-
 hw/arm/xlnx-versal-virt.c             | 1 -
 hw/{arm => core}/sysbus-fdt.c         | 2 +-
 hw/arm/meson.build                    | 1 -
 hw/core/meson.build                   | 1 +
 6 files changed, 3 insertions(+), 4 deletions(-)
 rename include/hw/{arm => core}/sysbus-fdt.h (100%)
 rename hw/{arm => core}/sysbus-fdt.c (99%)

diff --git a/include/hw/arm/sysbus-fdt.h b/include/hw/core/sysbus-fdt.h
similarity index 100%
rename from include/hw/arm/sysbus-fdt.h
rename to include/hw/core/sysbus-fdt.h
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d2e5ecd234..cfababa292 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -56,7 +56,7 @@
 #include "qemu/module.h"
 #include "hw/pci-host/gpex.h"
 #include "hw/virtio/virtio-pci.h"
-#include "hw/arm/sysbus-fdt.h"
+#include "hw/core/sysbus-fdt.h"
 #include "hw/platform-bus.h"
 #include "hw/qdev-properties.h"
 #include "hw/arm/fdt.h"
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 7c7baff8b7..bc20af4ba8 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -15,7 +15,6 @@
 #include "sysemu/device_tree.h"
 #include "hw/boards.h"
 #include "hw/sysbus.h"
-#include "hw/arm/sysbus-fdt.h"
 #include "hw/arm/fdt.h"
 #include "cpu.h"
 #include "hw/qdev-properties.h"
diff --git a/hw/arm/sysbus-fdt.c b/hw/core/sysbus-fdt.c
similarity index 99%
rename from hw/arm/sysbus-fdt.c
rename to hw/core/sysbus-fdt.c
index 48c5fe9bf1..19d22cbe73 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/core/sysbus-fdt.c
@@ -27,7 +27,7 @@
 #ifdef CONFIG_LINUX
 #include <linux/vfio.h>
 #endif
-#include "hw/arm/sysbus-fdt.h"
+#include "hw/core/sysbus-fdt.h"
 #include "qemu/error-report.h"
 #include "sysemu/device_tree.h"
 #include "sysemu/tpm.h"
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 721a8eb8be..122e5dd992 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -1,6 +1,5 @@
 arm_ss = ss.source_set()
 arm_ss.add(files('boot.c'), fdt)
-arm_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true: files('sysbus-fdt.c'))
 arm_ss.add(when: 'CONFIG_ARM_VIRT', if_true: files('virt.c'))
 arm_ss.add(when: 'CONFIG_ACPI', if_true: files('virt-acpi-build.c'))
 arm_ss.add(when: 'CONFIG_DIGIC', if_true: files('digic_boards.c'))
diff --git a/hw/core/meson.build b/hw/core/meson.build
index 0f884d6fd4..7a4d02b6c0 100644
--- a/hw/core/meson.build
+++ b/hw/core/meson.build
@@ -33,6 +33,7 @@ softmmu_ss.add(when: 'CONFIG_PTIMER', if_true: files('ptimer.c'))
 softmmu_ss.add(when: 'CONFIG_REGISTER', if_true: files('register.c'))
 softmmu_ss.add(when: 'CONFIG_SPLIT_IRQ', if_true: files('split-irq.c'))
 softmmu_ss.add(when: 'CONFIG_XILINX_AXI', if_true: files('stream.c'))
+softmmu_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true: files('sysbus-fdt.c'))
 
 softmmu_ss.add(files(
   'cpu-sysemu.c',
-- 
2.35.1

[PATCH v4 3/6] hw/riscv: virt: Create a platform bus

[Alistair Francis]

From: Alistair Francis <alistair.francis@wdc.com>

Create a platform bus to allow dynamic devices to be connected. This is
based on the ARM implementation.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 include/hw/riscv/virt.h |  7 ++++-
 hw/riscv/virt.c         | 68 +++++++++++++++++++++++++++++------------
 hw/riscv/Kconfig        |  1 +
 3 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 8b8db3fb7c..984e55c77f 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -46,6 +46,7 @@ struct RISCVVirtState {
 
     /*< public >*/
     Notifier machine_done;
+    DeviceState *platform_bus_dev;
     RISCVHartArrayState soc[VIRT_SOCKETS_MAX];
     DeviceState *irqchip[VIRT_SOCKETS_MAX];
     PFlashCFI01 *flash[2];
@@ -76,6 +77,7 @@ enum {
     VIRT_DRAM,
     VIRT_PCIE_MMIO,
     VIRT_PCIE_PIO,
+    VIRT_PLATFORM_BUS,
     VIRT_PCIE_ECAM
 };
 
@@ -85,9 +87,12 @@ enum {
     VIRTIO_IRQ = 1, /* 1 to 8 */
     VIRTIO_COUNT = 8,
     PCIE_IRQ = 0x20, /* 32 to 35 */
-    VIRTIO_NDEV = 0x35 /* Arbitrary maximum number of interrupts */
+    VIRT_PLATFORM_BUS_IRQ = 64, /* 64 to 96 */
+    VIRTIO_NDEV = 96 /* Arbitrary maximum number of interrupts */
 };
 
+#define VIRT_PLATFORM_BUS_NUM_IRQS 32
+
 #define VIRT_IRQCHIP_IPI_MSI 1
 #define VIRT_IRQCHIP_NUM_MSIS 255
 #define VIRT_IRQCHIP_NUM_SOURCES VIRTIO_NDEV
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 3d48469367..169da70350 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -37,6 +37,7 @@
 #include "hw/intc/riscv_imsic.h"
 #include "hw/intc/sifive_plic.h"
 #include "hw/misc/sifive_test.h"
+#include "hw/platform-bus.h"
 #include "chardev/char.h"
 #include "sysemu/device_tree.h"
 #include "sysemu/sysemu.h"
@@ -68,25 +69,26 @@
 #endif
 
 static const MemMapEntry virt_memmap[] = {
-    [VIRT_DEBUG] =       {        0x0,         0x100 },
-    [VIRT_MROM] =        {     0x1000,        0xf000 },
-    [VIRT_TEST] =        {   0x100000,        0x1000 },
-    [VIRT_RTC] =         {   0x101000,        0x1000 },
-    [VIRT_CLINT] =       {  0x2000000,       0x10000 },
-    [VIRT_ACLINT_SSWI] = {  0x2F00000,        0x4000 },
-    [VIRT_PCIE_PIO] =    {  0x3000000,       0x10000 },
-    [VIRT_PLIC] =        {  0xc000000, VIRT_PLIC_SIZE(VIRT_CPUS_MAX * 2) },
-    [VIRT_APLIC_M] =     {  0xc000000, APLIC_SIZE(VIRT_CPUS_MAX) },
-    [VIRT_APLIC_S] =     {  0xd000000, APLIC_SIZE(VIRT_CPUS_MAX) },
-    [VIRT_UART0] =       { 0x10000000,         0x100 },
-    [VIRT_VIRTIO] =      { 0x10001000,        0x1000 },
-    [VIRT_FW_CFG] =      { 0x10100000,          0x18 },
-    [VIRT_FLASH] =       { 0x20000000,     0x4000000 },
-    [VIRT_IMSIC_M] =     { 0x24000000, VIRT_IMSIC_MAX_SIZE },
-    [VIRT_IMSIC_S] =     { 0x28000000, VIRT_IMSIC_MAX_SIZE },
-    [VIRT_PCIE_ECAM] =   { 0x30000000,    0x10000000 },
-    [VIRT_PCIE_MMIO] =   { 0x40000000,    0x40000000 },
-    [VIRT_DRAM] =        { 0x80000000,           0x0 },
+    [VIRT_DEBUG] =        {        0x0,         0x100 },
+    [VIRT_MROM] =         {     0x1000,        0xf000 },
+    [VIRT_TEST] =         {   0x100000,        0x1000 },
+    [VIRT_RTC] =          {   0x101000,        0x1000 },
+    [VIRT_CLINT] =        {  0x2000000,       0x10000 },
+    [VIRT_ACLINT_SSWI] =  {  0x2F00000,        0x4000 },
+    [VIRT_PCIE_PIO] =     {  0x3000000,       0x10000 },
+    [VIRT_PLATFORM_BUS] = {  0x4000000,     0x2000000 },
+    [VIRT_PLIC] =         {  0xc000000, VIRT_PLIC_SIZE(VIRT_CPUS_MAX * 2) },
+    [VIRT_APLIC_M] =      {  0xc000000, APLIC_SIZE(VIRT_CPUS_MAX) },
+    [VIRT_APLIC_S] =      {  0xd000000, APLIC_SIZE(VIRT_CPUS_MAX) },
+    [VIRT_UART0] =        { 0x10000000,         0x100 },
+    [VIRT_VIRTIO] =       { 0x10001000,        0x1000 },
+    [VIRT_FW_CFG] =       { 0x10100000,          0x18 },
+    [VIRT_FLASH] =        { 0x20000000,     0x4000000 },
+    [VIRT_IMSIC_M] =      { 0x24000000, VIRT_IMSIC_MAX_SIZE },
+    [VIRT_IMSIC_S] =      { 0x28000000, VIRT_IMSIC_MAX_SIZE },
+    [VIRT_PCIE_ECAM] =    { 0x30000000,    0x10000000 },
+    [VIRT_PCIE_MMIO] =    { 0x40000000,    0x40000000 },
+    [VIRT_DRAM] =         { 0x80000000,           0x0 },
 };
 
 /* PCIe high mmio is fixed for RV32 */
@@ -1156,6 +1158,32 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
     return aplic_m;
 }
 
+static void create_platform_bus(RISCVVirtState *s, DeviceState *irqchip)
+{
+    DeviceState *dev;
+    SysBusDevice *sysbus;
+    const MemMapEntry *memmap = virt_memmap;
+    int i;
+    MemoryRegion *sysmem = get_system_memory();
+
+    dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE);
+    dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE);
+    qdev_prop_set_uint32(dev, "num_irqs", VIRT_PLATFORM_BUS_NUM_IRQS);
+    qdev_prop_set_uint32(dev, "mmio_size", memmap[VIRT_PLATFORM_BUS].size);
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+    s->platform_bus_dev = dev;
+
+    sysbus = SYS_BUS_DEVICE(dev);
+    for (i = 0; i < VIRT_PLATFORM_BUS_NUM_IRQS; i++) {
+        int irq = VIRT_PLATFORM_BUS_IRQ + i;
+        sysbus_connect_irq(sysbus, i, qdev_get_gpio_in(irqchip, irq));
+    }
+
+    memory_region_add_subregion(sysmem,
+                                memmap[VIRT_PLATFORM_BUS].base,
+                                sysbus_mmio_get_region(sysbus, 0));
+}
+
 static
 void virt_machine_done(Notifier *notifier, void *data)
 {
@@ -1407,6 +1435,8 @@ static void virt_machine_init(MachineState *machine)
                    memmap[VIRT_PCIE_PIO].base,
                    DEVICE(pcie_irqchip));
 
+    create_platform_bus(s, DEVICE(mmio_irqchip));
+
     serial_mm_init(system_memory, memmap[VIRT_UART0].base,
         0, qdev_get_gpio_in(DEVICE(mmio_irqchip), UART0_IRQ), 399193,
         serial_hd(0), DEVICE_LITTLE_ENDIAN);
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index 91bb9d21c4..da790f5936 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -48,6 +48,7 @@ config RISCV_VIRT
     select SIFIVE_TEST
     select VIRTIO_MMIO
     select FW_CFG_DMA
+    select PLATFORM_BUS
 
 config SIFIVE_E
     bool
-- 
2.35.1

[PATCH v4 4/6] hw/riscv: virt: Add support for generating platform FDT entries

[Alistair Francis]

From: Alistair Francis <alistair.francis@wdc.com>

Similar to the ARM virt machine add support for adding device tree
entries for dynamically created devices.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 hw/riscv/virt.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 169da70350..e4a5c6c28b 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -28,6 +28,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/char/serial.h"
 #include "target/riscv/cpu.h"
+#include "hw/core/sysbus-fdt.h"
 #include "hw/riscv/riscv_hart.h"
 #include "hw/riscv/virt.h"
 #include "hw/riscv/boot.h"
@@ -411,6 +412,12 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
         qemu_fdt_setprop(mc->fdt, name, "interrupt-controller", NULL, 0);
         qemu_fdt_setprop_cell(mc->fdt, name, "#interrupt-cells", 0);
         riscv_socket_fdt_write_id(mc, mc->fdt, name, socket);
+
+        platform_bus_add_all_fdt_nodes(mc->fdt, name,
+                                       memmap[VIRT_PLATFORM_BUS].base,
+                                       memmap[VIRT_PLATFORM_BUS].size,
+                                       VIRT_PLATFORM_BUS_IRQ);
+
         g_free(name);
     }
 
@@ -469,6 +476,12 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
     riscv_socket_fdt_write_id(mc, mc->fdt, plic_name, socket);
     qemu_fdt_setprop_cell(mc->fdt, plic_name, "phandle",
         plic_phandles[socket]);
+
+    platform_bus_add_all_fdt_nodes(mc->fdt, plic_name,
+                                   memmap[VIRT_PLATFORM_BUS].base,
+                                   memmap[VIRT_PLATFORM_BUS].size,
+                                   VIRT_PLATFORM_BUS_IRQ);
+
     g_free(plic_name);
 
     g_free(plic_cells);
@@ -546,6 +559,12 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
             IMSIC_MMIO_GROUP_MIN_SHIFT);
     }
     qemu_fdt_setprop_cell(mc->fdt, imsic_name, "phandle", *msi_m_phandle);
+
+    platform_bus_add_all_fdt_nodes(mc->fdt, imsic_name,
+                                   memmap[VIRT_PLATFORM_BUS].base,
+                                   memmap[VIRT_PLATFORM_BUS].size,
+                                   VIRT_PLATFORM_BUS_IRQ);
+
     g_free(imsic_name);
 
     /* S-level IMSIC node */
@@ -683,6 +702,12 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
         VIRT_IRQCHIP_NUM_SOURCES);
     riscv_socket_fdt_write_id(mc, mc->fdt, aplic_name, socket);
     qemu_fdt_setprop_cell(mc->fdt, aplic_name, "phandle", aplic_s_phandle);
+
+    platform_bus_add_all_fdt_nodes(mc->fdt, aplic_name,
+                                   memmap[VIRT_PLATFORM_BUS].base,
+                                   memmap[VIRT_PLATFORM_BUS].size,
+                                   VIRT_PLATFORM_BUS_IRQ);
+
     g_free(aplic_name);
 
     g_free(aplic_cells);
-- 
2.35.1

[PATCH v4 5/6] hw/riscv: virt: Add device plug support

[Alistair Francis]

From: Alistair Francis <alistair.francis@wdc.com>

Add support for plugging in devices, this was tested with the TPM
device.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 hw/riscv/virt.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index e4a5c6c28b..6eed1f4d70 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1564,10 +1564,37 @@ static void virt_set_aclint(Object *obj, bool value, Error **errp)
     s->have_aclint = value;
 }
 
+static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
+                                                        DeviceState *dev)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+
+    if (device_is_dynamic_sysbus(mc, dev)) {
+        return HOTPLUG_HANDLER(machine);
+    }
+    return NULL;
+}
+
+static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
+                                        DeviceState *dev, Error **errp)
+{
+    RISCVVirtState *s = RISCV_VIRT_MACHINE(hotplug_dev);
+
+    if (s->platform_bus_dev) {
+        MachineClass *mc = MACHINE_GET_CLASS(s);
+
+        if (device_is_dynamic_sysbus(mc, dev)) {
+            platform_bus_link_device(PLATFORM_BUS_DEVICE(s->platform_bus_dev),
+                                     SYS_BUS_DEVICE(dev));
+        }
+    }
+}
+
 static void virt_machine_class_init(ObjectClass *oc, void *data)
 {
     char str[128];
     MachineClass *mc = MACHINE_CLASS(oc);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
     mc->desc = "RISC-V VirtIO board";
     mc->init = virt_machine_init;
@@ -1579,6 +1606,10 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id;
     mc->numa_mem_supported = true;
     mc->default_ram_id = "riscv_virt_board.ram";
+    assert(!mc->get_hotplug_handler);
+    mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
+
+    hc->plug = virt_machine_device_plug_cb;
 
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
 
@@ -1609,6 +1640,10 @@ static const TypeInfo virt_machine_typeinfo = {
     .class_init = virt_machine_class_init,
     .instance_init = virt_machine_instance_init,
     .instance_size = sizeof(RISCVVirtState),
+    .interfaces = (InterfaceInfo[]) {
+         { TYPE_HOTPLUG_HANDLER },
+         { }
+    },
 };
 
 static void virt_machine_init_register_types(void)
-- 
2.35.1

[PATCH v4 6/6] hw/riscv: Enable TPM backends

[Alistair Francis]

From: Alistair Francis <alistair.francis@wdc.com>

Imply the TPM sysbus devices. This allows users to add TPM devices to
the RISC-V virt board.

This was tested by first creating an emulated TPM device:

    swtpm socket --tpm2 -t -d --tpmstate dir=/tmp/tpm \
        --ctrl type=unixio,path=swtpm-sock

Then launching QEMU with:

    -chardev socket,id=chrtpm,path=swtpm-sock \
    -tpmdev emulator,id=tpm0,chardev=chrtpm \
    -device tpm-tis-device,tpmdev=tpm0

The TPM device can be seen in the memory tree and the generated device
tree.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/942
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 hw/riscv/virt.c  | 4 ++++
 hw/riscv/Kconfig | 1 +
 2 files changed, 5 insertions(+)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 6eed1f4d70..b62fd66a49 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -43,6 +43,7 @@
 #include "sysemu/device_tree.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
+#include "sysemu/tpm.h"
 #include "hw/pci/pci.h"
 #include "hw/pci-host/gpex.h"
 #include "hw/display/ramfb.h"
@@ -1612,6 +1613,9 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     hc->plug = virt_machine_device_plug_cb;
 
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
+#ifdef CONFIG_TPM
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
+#endif
 
     object_class_property_add_bool(oc, "aclint", virt_get_aclint,
                                    virt_set_aclint);
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index da790f5936..79ff61c464 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -34,6 +34,7 @@ config RISCV_VIRT
     imply PCI_DEVICES
     imply VIRTIO_VGA
     imply TEST_DEVICES
+    imply TPM_TIS_SYSBUS
     select RISCV_NUMA
     select GOLDFISH_RTC
     select MSI_NONBROKEN
-- 
2.35.1

Re: [PATCH v4 0/6] hw/riscv: Add TPM support to the virt board

[Bin Meng]

On Wed, Apr 20, 2022 at 1:52 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> This series adds support for connecting TPM devices to the RISC-V virt
> board. This is similar to how it works for the ARM virt board.
>
> This was tested by first creating an emulated TPM device:
>
>     swtpm socket --tpm2 -t -d --tpmstate dir=/tmp/tpm \
>         --ctrl type=unixio,path=swtpm-sock
>
> Then launching QEMU with:
>
>     -chardev socket,id=chrtpm,path=swtpm-sock \
>     -tpmdev emulator,id=tpm0,chardev=chrtpm \
>     -device tpm-tis-device,tpmdev=tpm0
>
> The TPM device can be seen in the memory tree and the generated device
> tree.

Please include a 'virt' board documentation update patch to mention
above usage for TPM, or dynamically instantiated devices with TPM as
an example.

>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/942
>
> Alistair Francis (6):
>   hw/riscv: virt: Add a machine done notifier
>   hw/core: Move the ARM sysbus-fdt to core
>   hw/riscv: virt: Create a platform bus
>   hw/riscv: virt: Add support for generating platform FDT entries
>   hw/riscv: virt: Add device plug support
>   hw/riscv: Enable TPM backends
>
>  include/hw/{arm => core}/sysbus-fdt.h |   0
>  include/hw/riscv/virt.h               |   8 +-
>  hw/arm/virt.c                         |   2 +-
>  hw/arm/xlnx-versal-virt.c             |   1 -
>  hw/{arm => core}/sysbus-fdt.c         |   2 +-
>  hw/riscv/virt.c                       | 312 +++++++++++++++++---------
>  hw/arm/meson.build                    |   1 -
>  hw/core/meson.build                   |   1 +
>  hw/riscv/Kconfig                      |   2 +
>  9 files changed, 221 insertions(+), 108 deletions(-)
>  rename include/hw/{arm => core}/sysbus-fdt.h (100%)
>  rename hw/{arm => core}/sysbus-fdt.c (99%)
>

Regards,
Bin

Re: [PATCH v4 0/6] hw/riscv: Add TPM support to the virt board

[Bin Meng]

On Wed, Apr 20, 2022 at 1:52 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> This series adds support for connecting TPM devices to the RISC-V virt
> board. This is similar to how it works for the ARM virt board.
>
> This was tested by first creating an emulated TPM device:
>
>     swtpm socket --tpm2 -t -d --tpmstate dir=/tmp/tpm \
>         --ctrl type=unixio,path=swtpm-sock
>
> Then launching QEMU with:
>
>     -chardev socket,id=chrtpm,path=swtpm-sock \
>     -tpmdev emulator,id=tpm0,chardev=chrtpm \
>     -device tpm-tis-device,tpmdev=tpm0
>
> The TPM device can be seen in the memory tree and the generated device
> tree.

Please include a 'virt' board documentation update patch to mention
above usage for TPM, or dynamically instantiated devices with TPM as
an example.

>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/942
>
> Alistair Francis (6):
>   hw/riscv: virt: Add a machine done notifier
>   hw/core: Move the ARM sysbus-fdt to core
>   hw/riscv: virt: Create a platform bus
>   hw/riscv: virt: Add support for generating platform FDT entries
>   hw/riscv: virt: Add device plug support
>   hw/riscv: Enable TPM backends
>
>  include/hw/{arm => core}/sysbus-fdt.h |   0
>  include/hw/riscv/virt.h               |   8 +-
>  hw/arm/virt.c                         |   2 +-
>  hw/arm/xlnx-versal-virt.c             |   1 -
>  hw/{arm => core}/sysbus-fdt.c         |   2 +-
>  hw/riscv/virt.c                       | 312 +++++++++++++++++---------
>  hw/arm/meson.build                    |   1 -
>  hw/core/meson.build                   |   1 +
>  hw/riscv/Kconfig                      |   2 +
>  9 files changed, 221 insertions(+), 108 deletions(-)
>  rename include/hw/{arm => core}/sysbus-fdt.h (100%)
>  rename hw/{arm => core}/sysbus-fdt.c (99%)
>

Regards,
Bin

Re: [PATCH v4 1/6] hw/riscv: virt: Add a machine done notifier

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Move the binary and device tree loading code to the machine done
> notifier. This allows us to prepare for editing the device tree as part
> of the notifier.
>
> This is based on similar code in the ARM virt machine.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  include/hw/riscv/virt.h |   1 +
>  hw/riscv/virt.c         | 180 +++++++++++++++++++++-------------------
>  2 files changed, 97 insertions(+), 84 deletions(-)
>
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 78b058ec86..8b8db3fb7c 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -45,6 +45,7 @@ struct RISCVVirtState {
>      MachineState parent;
>
>      /*< public >*/
> +    Notifier machine_done;
>      RISCVHartArrayState soc[VIRT_SOCKETS_MAX];
>      DeviceState *irqchip[VIRT_SOCKETS_MAX];
>      PFlashCFI01 *flash[2];
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index da50cbed43..3d48469367 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1156,6 +1156,95 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
>      return aplic_m;
>  }
>
> +static

nits: move this to the following line

> +void virt_machine_done(Notifier *notifier, void *data)
> +{
> +    RISCVVirtState *s = container_of(notifier, RISCVVirtState,
> +                                     machine_done);
> +    const MemMapEntry *memmap = virt_memmap;
> +    MachineState *machine = MACHINE(s);
> +    target_ulong start_addr = memmap[VIRT_DRAM].base;
> +    target_ulong firmware_end_addr, kernel_start_addr;
> +    uint32_t fdt_load_addr;
> +    uint64_t kernel_entry;
> +
> +    /*
> +     * Only direct boot kernel is currently supported for KVM VM,
> +     * so the "-bios" parameter is ignored and treated like "-bios none"
> +     * when KVM is enabled.
> +     */
> +    if (kvm_enabled()) {
> +        g_free(machine->firmware);
> +        machine->firmware = g_strdup("none");
> +    }
> +
> +    if (riscv_is_32bit(&s->soc[0])) {
> +        firmware_end_addr = riscv_find_and_load_firmware(machine,
> +                                    RISCV32_BIOS_BIN, start_addr, NULL);
> +    } else {
> +        firmware_end_addr = riscv_find_and_load_firmware(machine,
> +                                    RISCV64_BIOS_BIN, start_addr, NULL);
> +    }
> +
> +    if (machine->kernel_filename) {
> +        kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0],
> +                                                         firmware_end_addr);
> +
> +        kernel_entry = riscv_load_kernel(machine->kernel_filename,
> +                                         kernel_start_addr, NULL);
> +
> +        if (machine->initrd_filename) {
> +            hwaddr start;
> +            hwaddr end = riscv_load_initrd(machine->initrd_filename,
> +                                           machine->ram_size, kernel_entry,
> +                                           &start);
> +            qemu_fdt_setprop_cell(machine->fdt, "/chosen",
> +                                  "linux,initrd-start", start);
> +            qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end",
> +                                  end);
> +        }
> +    } else {
> +       /*
> +        * If dynamic firmware is used, it doesn't know where is the next mode
> +        * if kernel argument is not set.
> +        */
> +        kernel_entry = 0;
> +    }
> +
> +    if (drive_get(IF_PFLASH, 0, 0)) {
> +        /*
> +         * Pflash was supplied, let's overwrite the address we jump to after
> +         * reset to the base of the flash.
> +         */
> +        start_addr = virt_memmap[VIRT_FLASH].base;
> +    }
> +
> +    /*
> +     * Init fw_cfg.  Must be done before riscv_load_fdt, otherwise the device
> +     * tree cannot be altered and we get FDT_ERR_NOSPACE.
> +     */
> +    s->fw_cfg = create_fw_cfg(machine);
> +    rom_set_fw(s->fw_cfg);
> +
> +    /* Compute the fdt load address in dram */
> +    fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
> +                                   machine->ram_size, machine->fdt);
> +    /* load the reset vector */
> +    riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr,
> +                              virt_memmap[VIRT_MROM].base,
> +                              virt_memmap[VIRT_MROM].size, kernel_entry,
> +                              fdt_load_addr, machine->fdt);
> +
> +    /*
> +     * Only direct boot kernel is currently supported for KVM VM,
> +     * So here setup kernel start address and fdt address.
> +     * TODO:Support firmware loading and integrate to TCG start
> +     */
> +    if (kvm_enabled()) {
> +        riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
> +    }
> +}
> +
>  static void virt_machine_init(MachineState *machine)
>  {
>      const MemMapEntry *memmap = virt_memmap;
> @@ -1163,10 +1252,6 @@ static void virt_machine_init(MachineState *machine)
>      MemoryRegion *system_memory = get_system_memory();
>      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>      char *soc_name;
> -    target_ulong start_addr = memmap[VIRT_DRAM].base;
> -    target_ulong firmware_end_addr, kernel_start_addr;
> -    uint32_t fdt_load_addr;
> -    uint64_t kernel_entry;
>      DeviceState *mmio_irqchip, *virtio_irqchip, *pcie_irqchip;
>      int i, base_hartid, hart_count;
>
> @@ -1296,92 +1381,12 @@ static void virt_machine_init(MachineState *machine)
>      memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
>          machine->ram);
>
> -    /* create device tree */
> -    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> -               riscv_is_32bit(&s->soc[0]));
> -
>      /* boot rom */
>      memory_region_init_rom(mask_rom, NULL, "riscv_virt_board.mrom",
>                             memmap[VIRT_MROM].size, &error_fatal);
>      memory_region_add_subregion(system_memory, memmap[VIRT_MROM].base,
>                                  mask_rom);
>
> -    /*
> -     * Only direct boot kernel is currently supported for KVM VM,
> -     * so the "-bios" parameter is ignored and treated like "-bios none"
> -     * when KVM is enabled.
> -     */
> -    if (kvm_enabled()) {
> -        g_free(machine->firmware);
> -        machine->firmware = g_strdup("none");
> -    }
> -
> -    if (riscv_is_32bit(&s->soc[0])) {
> -        firmware_end_addr = riscv_find_and_load_firmware(machine,
> -                                    RISCV32_BIOS_BIN, start_addr, NULL);
> -    } else {
> -        firmware_end_addr = riscv_find_and_load_firmware(machine,
> -                                    RISCV64_BIOS_BIN, start_addr, NULL);
> -    }
> -
> -    if (machine->kernel_filename) {
> -        kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0],
> -                                                         firmware_end_addr);
> -
> -        kernel_entry = riscv_load_kernel(machine->kernel_filename,
> -                                         kernel_start_addr, NULL);
> -
> -        if (machine->initrd_filename) {
> -            hwaddr start;
> -            hwaddr end = riscv_load_initrd(machine->initrd_filename,
> -                                           machine->ram_size, kernel_entry,
> -                                           &start);
> -            qemu_fdt_setprop_cell(machine->fdt, "/chosen",
> -                                  "linux,initrd-start", start);
> -            qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end",
> -                                  end);
> -        }
> -    } else {
> -       /*
> -        * If dynamic firmware is used, it doesn't know where is the next mode
> -        * if kernel argument is not set.
> -        */
> -        kernel_entry = 0;
> -    }
> -
> -    if (drive_get(IF_PFLASH, 0, 0)) {
> -        /*
> -         * Pflash was supplied, let's overwrite the address we jump to after
> -         * reset to the base of the flash.
> -         */
> -        start_addr = virt_memmap[VIRT_FLASH].base;
> -    }
> -
> -    /*
> -     * Init fw_cfg.  Must be done before riscv_load_fdt, otherwise the device
> -     * tree cannot be altered and we get FDT_ERR_NOSPACE.
> -     */
> -    s->fw_cfg = create_fw_cfg(machine);
> -    rom_set_fw(s->fw_cfg);
> -
> -    /* Compute the fdt load address in dram */
> -    fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
> -                                   machine->ram_size, machine->fdt);
> -    /* load the reset vector */
> -    riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr,
> -                              virt_memmap[VIRT_MROM].base,
> -                              virt_memmap[VIRT_MROM].size, kernel_entry,
> -                              fdt_load_addr, machine->fdt);
> -
> -    /*
> -     * Only direct boot kernel is currently supported for KVM VM,
> -     * So here setup kernel start address and fdt address.
> -     * TODO:Support firmware loading and integrate to TCG start
> -     */
> -    if (kvm_enabled()) {
> -        riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
> -    }
> -
>      /* SiFive Test MMIO device */
>      sifive_test_create(memmap[VIRT_TEST].base);
>
> @@ -1417,6 +1422,13 @@ static void virt_machine_init(MachineState *machine)
>                                    drive_get(IF_PFLASH, 0, i));
>      }
>      virt_flash_map(s, system_memory);
> +
> +    /* create device tree */
> +    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> +               riscv_is_32bit(&s->soc[0]));
> +
> +    s->machine_done.notify = virt_machine_done;
> +    qemu_add_machine_init_done_notifier(&s->machine_done);
>  }
>
>  static void virt_machine_instance_init(Object *obj)
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH v4 1/6] hw/riscv: virt: Add a machine done notifier

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Move the binary and device tree loading code to the machine done
> notifier. This allows us to prepare for editing the device tree as part
> of the notifier.
>
> This is based on similar code in the ARM virt machine.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  include/hw/riscv/virt.h |   1 +
>  hw/riscv/virt.c         | 180 +++++++++++++++++++++-------------------
>  2 files changed, 97 insertions(+), 84 deletions(-)
>
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index 78b058ec86..8b8db3fb7c 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -45,6 +45,7 @@ struct RISCVVirtState {
>      MachineState parent;
>
>      /*< public >*/
> +    Notifier machine_done;
>      RISCVHartArrayState soc[VIRT_SOCKETS_MAX];
>      DeviceState *irqchip[VIRT_SOCKETS_MAX];
>      PFlashCFI01 *flash[2];
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index da50cbed43..3d48469367 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1156,6 +1156,95 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType aia_type, int aia_guests,
>      return aplic_m;
>  }
>
> +static

nits: move this to the following line

> +void virt_machine_done(Notifier *notifier, void *data)
> +{
> +    RISCVVirtState *s = container_of(notifier, RISCVVirtState,
> +                                     machine_done);
> +    const MemMapEntry *memmap = virt_memmap;
> +    MachineState *machine = MACHINE(s);
> +    target_ulong start_addr = memmap[VIRT_DRAM].base;
> +    target_ulong firmware_end_addr, kernel_start_addr;
> +    uint32_t fdt_load_addr;
> +    uint64_t kernel_entry;
> +
> +    /*
> +     * Only direct boot kernel is currently supported for KVM VM,
> +     * so the "-bios" parameter is ignored and treated like "-bios none"
> +     * when KVM is enabled.
> +     */
> +    if (kvm_enabled()) {
> +        g_free(machine->firmware);
> +        machine->firmware = g_strdup("none");
> +    }
> +
> +    if (riscv_is_32bit(&s->soc[0])) {
> +        firmware_end_addr = riscv_find_and_load_firmware(machine,
> +                                    RISCV32_BIOS_BIN, start_addr, NULL);
> +    } else {
> +        firmware_end_addr = riscv_find_and_load_firmware(machine,
> +                                    RISCV64_BIOS_BIN, start_addr, NULL);
> +    }
> +
> +    if (machine->kernel_filename) {
> +        kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0],
> +                                                         firmware_end_addr);
> +
> +        kernel_entry = riscv_load_kernel(machine->kernel_filename,
> +                                         kernel_start_addr, NULL);
> +
> +        if (machine->initrd_filename) {
> +            hwaddr start;
> +            hwaddr end = riscv_load_initrd(machine->initrd_filename,
> +                                           machine->ram_size, kernel_entry,
> +                                           &start);
> +            qemu_fdt_setprop_cell(machine->fdt, "/chosen",
> +                                  "linux,initrd-start", start);
> +            qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end",
> +                                  end);
> +        }
> +    } else {
> +       /*
> +        * If dynamic firmware is used, it doesn't know where is the next mode
> +        * if kernel argument is not set.
> +        */
> +        kernel_entry = 0;
> +    }
> +
> +    if (drive_get(IF_PFLASH, 0, 0)) {
> +        /*
> +         * Pflash was supplied, let's overwrite the address we jump to after
> +         * reset to the base of the flash.
> +         */
> +        start_addr = virt_memmap[VIRT_FLASH].base;
> +    }
> +
> +    /*
> +     * Init fw_cfg.  Must be done before riscv_load_fdt, otherwise the device
> +     * tree cannot be altered and we get FDT_ERR_NOSPACE.
> +     */
> +    s->fw_cfg = create_fw_cfg(machine);
> +    rom_set_fw(s->fw_cfg);
> +
> +    /* Compute the fdt load address in dram */
> +    fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
> +                                   machine->ram_size, machine->fdt);
> +    /* load the reset vector */
> +    riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr,
> +                              virt_memmap[VIRT_MROM].base,
> +                              virt_memmap[VIRT_MROM].size, kernel_entry,
> +                              fdt_load_addr, machine->fdt);
> +
> +    /*
> +     * Only direct boot kernel is currently supported for KVM VM,
> +     * So here setup kernel start address and fdt address.
> +     * TODO:Support firmware loading and integrate to TCG start
> +     */
> +    if (kvm_enabled()) {
> +        riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
> +    }
> +}
> +
>  static void virt_machine_init(MachineState *machine)
>  {
>      const MemMapEntry *memmap = virt_memmap;
> @@ -1163,10 +1252,6 @@ static void virt_machine_init(MachineState *machine)
>      MemoryRegion *system_memory = get_system_memory();
>      MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>      char *soc_name;
> -    target_ulong start_addr = memmap[VIRT_DRAM].base;
> -    target_ulong firmware_end_addr, kernel_start_addr;
> -    uint32_t fdt_load_addr;
> -    uint64_t kernel_entry;
>      DeviceState *mmio_irqchip, *virtio_irqchip, *pcie_irqchip;
>      int i, base_hartid, hart_count;
>
> @@ -1296,92 +1381,12 @@ static void virt_machine_init(MachineState *machine)
>      memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
>          machine->ram);
>
> -    /* create device tree */
> -    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> -               riscv_is_32bit(&s->soc[0]));
> -
>      /* boot rom */
>      memory_region_init_rom(mask_rom, NULL, "riscv_virt_board.mrom",
>                             memmap[VIRT_MROM].size, &error_fatal);
>      memory_region_add_subregion(system_memory, memmap[VIRT_MROM].base,
>                                  mask_rom);
>
> -    /*
> -     * Only direct boot kernel is currently supported for KVM VM,
> -     * so the "-bios" parameter is ignored and treated like "-bios none"
> -     * when KVM is enabled.
> -     */
> -    if (kvm_enabled()) {
> -        g_free(machine->firmware);
> -        machine->firmware = g_strdup("none");
> -    }
> -
> -    if (riscv_is_32bit(&s->soc[0])) {
> -        firmware_end_addr = riscv_find_and_load_firmware(machine,
> -                                    RISCV32_BIOS_BIN, start_addr, NULL);
> -    } else {
> -        firmware_end_addr = riscv_find_and_load_firmware(machine,
> -                                    RISCV64_BIOS_BIN, start_addr, NULL);
> -    }
> -
> -    if (machine->kernel_filename) {
> -        kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0],
> -                                                         firmware_end_addr);
> -
> -        kernel_entry = riscv_load_kernel(machine->kernel_filename,
> -                                         kernel_start_addr, NULL);
> -
> -        if (machine->initrd_filename) {
> -            hwaddr start;
> -            hwaddr end = riscv_load_initrd(machine->initrd_filename,
> -                                           machine->ram_size, kernel_entry,
> -                                           &start);
> -            qemu_fdt_setprop_cell(machine->fdt, "/chosen",
> -                                  "linux,initrd-start", start);
> -            qemu_fdt_setprop_cell(machine->fdt, "/chosen", "linux,initrd-end",
> -                                  end);
> -        }
> -    } else {
> -       /*
> -        * If dynamic firmware is used, it doesn't know where is the next mode
> -        * if kernel argument is not set.
> -        */
> -        kernel_entry = 0;
> -    }
> -
> -    if (drive_get(IF_PFLASH, 0, 0)) {
> -        /*
> -         * Pflash was supplied, let's overwrite the address we jump to after
> -         * reset to the base of the flash.
> -         */
> -        start_addr = virt_memmap[VIRT_FLASH].base;
> -    }
> -
> -    /*
> -     * Init fw_cfg.  Must be done before riscv_load_fdt, otherwise the device
> -     * tree cannot be altered and we get FDT_ERR_NOSPACE.
> -     */
> -    s->fw_cfg = create_fw_cfg(machine);
> -    rom_set_fw(s->fw_cfg);
> -
> -    /* Compute the fdt load address in dram */
> -    fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
> -                                   machine->ram_size, machine->fdt);
> -    /* load the reset vector */
> -    riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr,
> -                              virt_memmap[VIRT_MROM].base,
> -                              virt_memmap[VIRT_MROM].size, kernel_entry,
> -                              fdt_load_addr, machine->fdt);
> -
> -    /*
> -     * Only direct boot kernel is currently supported for KVM VM,
> -     * So here setup kernel start address and fdt address.
> -     * TODO:Support firmware loading and integrate to TCG start
> -     */
> -    if (kvm_enabled()) {
> -        riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
> -    }
> -
>      /* SiFive Test MMIO device */
>      sifive_test_create(memmap[VIRT_TEST].base);
>
> @@ -1417,6 +1422,13 @@ static void virt_machine_init(MachineState *machine)
>                                    drive_get(IF_PFLASH, 0, i));
>      }
>      virt_flash_map(s, system_memory);
> +
> +    /* create device tree */
> +    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> +               riscv_is_32bit(&s->soc[0]));
> +
> +    s->machine_done.notify = virt_machine_done;
> +    qemu_add_machine_init_done_notifier(&s->machine_done);
>  }
>
>  static void virt_machine_instance_init(Object *obj)
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH v4 2/6] hw/core: Move the ARM sysbus-fdt to core

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> The ARM virt machine currently uses sysbus-fdt to create device tree
> entries for dynamically created MMIO devices.
>
> The RISC-V virt machine can also benefit from this, so move the code to
> the core directory.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  include/hw/{arm => core}/sysbus-fdt.h | 0
>  hw/arm/virt.c                         | 2 +-
>  hw/arm/xlnx-versal-virt.c             | 1 -
>  hw/{arm => core}/sysbus-fdt.c         | 2 +-
>  hw/arm/meson.build                    | 1 -
>  hw/core/meson.build                   | 1 +
>  6 files changed, 3 insertions(+), 4 deletions(-)
>  rename include/hw/{arm => core}/sysbus-fdt.h (100%)
>  rename hw/{arm => core}/sysbus-fdt.c (99%)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

I believe you can do some refactor to let hw/ppc/e500.c to use this
platform bus too.

Regards,
Bin

Re: [PATCH v4 2/6] hw/core: Move the ARM sysbus-fdt to core

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> The ARM virt machine currently uses sysbus-fdt to create device tree
> entries for dynamically created MMIO devices.
>
> The RISC-V virt machine can also benefit from this, so move the code to
> the core directory.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  include/hw/{arm => core}/sysbus-fdt.h | 0
>  hw/arm/virt.c                         | 2 +-
>  hw/arm/xlnx-versal-virt.c             | 1 -
>  hw/{arm => core}/sysbus-fdt.c         | 2 +-
>  hw/arm/meson.build                    | 1 -
>  hw/core/meson.build                   | 1 +
>  6 files changed, 3 insertions(+), 4 deletions(-)
>  rename include/hw/{arm => core}/sysbus-fdt.h (100%)
>  rename hw/{arm => core}/sysbus-fdt.c (99%)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

I believe you can do some refactor to let hw/ppc/e500.c to use this
platform bus too.

Regards,
Bin

Re: [PATCH v4 3/6] hw/riscv: virt: Create a platform bus

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Create a platform bus to allow dynamic devices to be connected. This is
> based on the ARM implementation.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  include/hw/riscv/virt.h |  7 ++++-
>  hw/riscv/virt.c         | 68 +++++++++++++++++++++++++++++------------
>  hw/riscv/Kconfig        |  1 +
>  3 files changed, 56 insertions(+), 20 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH v4 3/6] hw/riscv: virt: Create a platform bus

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Create a platform bus to allow dynamic devices to be connected. This is
> based on the ARM implementation.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  include/hw/riscv/virt.h |  7 ++++-
>  hw/riscv/virt.c         | 68 +++++++++++++++++++++++++++++------------
>  hw/riscv/Kconfig        |  1 +
>  3 files changed, 56 insertions(+), 20 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH v4 4/6] hw/riscv: virt: Add support for generating platform FDT entries

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Similar to the ARM virt machine add support for adding device tree
> entries for dynamically created devices.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  hw/riscv/virt.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 169da70350..e4a5c6c28b 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -28,6 +28,7 @@
>  #include "hw/qdev-properties.h"
>  #include "hw/char/serial.h"
>  #include "target/riscv/cpu.h"
> +#include "hw/core/sysbus-fdt.h"
>  #include "hw/riscv/riscv_hart.h"
>  #include "hw/riscv/virt.h"
>  #include "hw/riscv/boot.h"
> @@ -411,6 +412,12 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
>          qemu_fdt_setprop(mc->fdt, name, "interrupt-controller", NULL, 0);
>          qemu_fdt_setprop_cell(mc->fdt, name, "#interrupt-cells", 0);
>          riscv_socket_fdt_write_id(mc, mc->fdt, name, socket);
> +
> +        platform_bus_add_all_fdt_nodes(mc->fdt, name,
> +                                       memmap[VIRT_PLATFORM_BUS].base,
> +                                       memmap[VIRT_PLATFORM_BUS].size,
> +                                       VIRT_PLATFORM_BUS_IRQ);

This one is not needed.

> +
>          g_free(name);
>      }
>
> @@ -469,6 +476,12 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>      riscv_socket_fdt_write_id(mc, mc->fdt, plic_name, socket);
>      qemu_fdt_setprop_cell(mc->fdt, plic_name, "phandle",
>          plic_phandles[socket]);
> +
> +    platform_bus_add_all_fdt_nodes(mc->fdt, plic_name,
> +                                   memmap[VIRT_PLATFORM_BUS].base,
> +                                   memmap[VIRT_PLATFORM_BUS].size,
> +                                   VIRT_PLATFORM_BUS_IRQ);
> +
>      g_free(plic_name);
>
>      g_free(plic_cells);
> @@ -546,6 +559,12 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
>              IMSIC_MMIO_GROUP_MIN_SHIFT);
>      }
>      qemu_fdt_setprop_cell(mc->fdt, imsic_name, "phandle", *msi_m_phandle);
> +
> +    platform_bus_add_all_fdt_nodes(mc->fdt, imsic_name,
> +                                   memmap[VIRT_PLATFORM_BUS].base,
> +                                   memmap[VIRT_PLATFORM_BUS].size,
> +                                   VIRT_PLATFORM_BUS_IRQ);
> +
>      g_free(imsic_name);
>
>      /* S-level IMSIC node */
> @@ -683,6 +702,12 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
>          VIRT_IRQCHIP_NUM_SOURCES);
>      riscv_socket_fdt_write_id(mc, mc->fdt, aplic_name, socket);
>      qemu_fdt_setprop_cell(mc->fdt, aplic_name, "phandle", aplic_s_phandle);
> +
> +    platform_bus_add_all_fdt_nodes(mc->fdt, aplic_name,
> +                                   memmap[VIRT_PLATFORM_BUS].base,
> +                                   memmap[VIRT_PLATFORM_BUS].size,
> +                                   VIRT_PLATFORM_BUS_IRQ);
> +
>      g_free(aplic_name);
>
>      g_free(aplic_cells);
> --

Regards,
Bin

Re: [PATCH v4 4/6] hw/riscv: virt: Add support for generating platform FDT entries

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Similar to the ARM virt machine add support for adding device tree
> entries for dynamically created devices.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  hw/riscv/virt.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 169da70350..e4a5c6c28b 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -28,6 +28,7 @@
>  #include "hw/qdev-properties.h"
>  #include "hw/char/serial.h"
>  #include "target/riscv/cpu.h"
> +#include "hw/core/sysbus-fdt.h"
>  #include "hw/riscv/riscv_hart.h"
>  #include "hw/riscv/virt.h"
>  #include "hw/riscv/boot.h"
> @@ -411,6 +412,12 @@ static void create_fdt_socket_aclint(RISCVVirtState *s,
>          qemu_fdt_setprop(mc->fdt, name, "interrupt-controller", NULL, 0);
>          qemu_fdt_setprop_cell(mc->fdt, name, "#interrupt-cells", 0);
>          riscv_socket_fdt_write_id(mc, mc->fdt, name, socket);
> +
> +        platform_bus_add_all_fdt_nodes(mc->fdt, name,
> +                                       memmap[VIRT_PLATFORM_BUS].base,
> +                                       memmap[VIRT_PLATFORM_BUS].size,
> +                                       VIRT_PLATFORM_BUS_IRQ);

This one is not needed.

> +
>          g_free(name);
>      }
>
> @@ -469,6 +476,12 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>      riscv_socket_fdt_write_id(mc, mc->fdt, plic_name, socket);
>      qemu_fdt_setprop_cell(mc->fdt, plic_name, "phandle",
>          plic_phandles[socket]);
> +
> +    platform_bus_add_all_fdt_nodes(mc->fdt, plic_name,
> +                                   memmap[VIRT_PLATFORM_BUS].base,
> +                                   memmap[VIRT_PLATFORM_BUS].size,
> +                                   VIRT_PLATFORM_BUS_IRQ);
> +
>      g_free(plic_name);
>
>      g_free(plic_cells);
> @@ -546,6 +559,12 @@ static void create_fdt_imsic(RISCVVirtState *s, const MemMapEntry *memmap,
>              IMSIC_MMIO_GROUP_MIN_SHIFT);
>      }
>      qemu_fdt_setprop_cell(mc->fdt, imsic_name, "phandle", *msi_m_phandle);
> +
> +    platform_bus_add_all_fdt_nodes(mc->fdt, imsic_name,
> +                                   memmap[VIRT_PLATFORM_BUS].base,
> +                                   memmap[VIRT_PLATFORM_BUS].size,
> +                                   VIRT_PLATFORM_BUS_IRQ);
> +
>      g_free(imsic_name);
>
>      /* S-level IMSIC node */
> @@ -683,6 +702,12 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
>          VIRT_IRQCHIP_NUM_SOURCES);
>      riscv_socket_fdt_write_id(mc, mc->fdt, aplic_name, socket);
>      qemu_fdt_setprop_cell(mc->fdt, aplic_name, "phandle", aplic_s_phandle);
> +
> +    platform_bus_add_all_fdt_nodes(mc->fdt, aplic_name,
> +                                   memmap[VIRT_PLATFORM_BUS].base,
> +                                   memmap[VIRT_PLATFORM_BUS].size,
> +                                   VIRT_PLATFORM_BUS_IRQ);
> +
>      g_free(aplic_name);
>
>      g_free(aplic_cells);
> --

Regards,
Bin

Re: [PATCH v4 5/6] hw/riscv: virt: Add device plug support

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Add support for plugging in devices, this was tested with the TPM
> device.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  hw/riscv/virt.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH v4 5/6] hw/riscv: virt: Add device plug support

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Add support for plugging in devices, this was tested with the TPM
> device.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  hw/riscv/virt.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH v4 6/6] hw/riscv: Enable TPM backends

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Imply the TPM sysbus devices. This allows users to add TPM devices to
> the RISC-V virt board.
>
> This was tested by first creating an emulated TPM device:
>
>     swtpm socket --tpm2 -t -d --tpmstate dir=/tmp/tpm \
>         --ctrl type=unixio,path=swtpm-sock
>
> Then launching QEMU with:
>
>     -chardev socket,id=chrtpm,path=swtpm-sock \
>     -tpmdev emulator,id=tpm0,chardev=chrtpm \
>     -device tpm-tis-device,tpmdev=tpm0
>
> The TPM device can be seen in the memory tree and the generated device
> tree.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/942
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  hw/riscv/virt.c  | 4 ++++
>  hw/riscv/Kconfig | 1 +
>  2 files changed, 5 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH v4 6/6] hw/riscv: Enable TPM backends

[Bin Meng]

On Wed, Apr 20, 2022 at 1:53 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@wdc.com>
>
> Imply the TPM sysbus devices. This allows users to add TPM devices to
> the RISC-V virt board.
>
> This was tested by first creating an emulated TPM device:
>
>     swtpm socket --tpm2 -t -d --tpmstate dir=/tmp/tpm \
>         --ctrl type=unixio,path=swtpm-sock
>
> Then launching QEMU with:
>
>     -chardev socket,id=chrtpm,path=swtpm-sock \
>     -tpmdev emulator,id=tpm0,chardev=chrtpm \
>     -device tpm-tis-device,tpmdev=tpm0
>
> The TPM device can be seen in the memory tree and the generated device
> tree.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/942
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  hw/riscv/virt.c  | 4 ++++
>  hw/riscv/Kconfig | 1 +
>  2 files changed, 5 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH v4 6/6] hw/riscv: Enable TPM backends

[Guenter Roeck]

Hi Alistair,

On Wed, Apr 20, 2022 at 03:52:48PM +1000, Alistair Francis wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> Imply the TPM sysbus devices. This allows users to add TPM devices to
> the RISC-V virt board.
> 
> This was tested by first creating an emulated TPM device:
> 
>     swtpm socket --tpm2 -t -d --tpmstate dir=/tmp/tpm \
>         --ctrl type=unixio,path=swtpm-sock
> 
> Then launching QEMU with:
> 
>     -chardev socket,id=chrtpm,path=swtpm-sock \
>     -tpmdev emulator,id=tpm0,chardev=chrtpm \
>     -device tpm-tis-device,tpmdev=tpm0
> 
> The TPM device can be seen in the memory tree and the generated device
> tree.
> 
I tried to get this working with qemu 8.0, but I did not have any success.
I am quite sure I have the above command line correctly, and it does work
with arm64. Any idea what I might be missing ?

Thanks,
Guenter

> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/942
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> ---
>  hw/riscv/virt.c  | 4 ++++
>  hw/riscv/Kconfig | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 6eed1f4d70..b62fd66a49 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -43,6 +43,7 @@
>  #include "sysemu/device_tree.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/kvm.h"
> +#include "sysemu/tpm.h"
>  #include "hw/pci/pci.h"
>  #include "hw/pci-host/gpex.h"
>  #include "hw/display/ramfb.h"
> @@ -1612,6 +1613,9 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>      hc->plug = virt_machine_device_plug_cb;
>  
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
> +#ifdef CONFIG_TPM
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> +#endif
>  
>      object_class_property_add_bool(oc, "aclint", virt_get_aclint,
>                                     virt_set_aclint);
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index da790f5936..79ff61c464 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -34,6 +34,7 @@ config RISCV_VIRT
>      imply PCI_DEVICES
>      imply VIRTIO_VGA
>      imply TEST_DEVICES
> +    imply TPM_TIS_SYSBUS
>      select RISCV_NUMA
>      select GOLDFISH_RTC
>      select MSI_NONBROKEN
> -- 
> 2.35.1
> 
> 

Re: [PATCH v4 6/6] hw/riscv: Enable TPM backends

[Guenter Roeck]

On Mon, Jun 19, 2023 at 01:32:34PM -0700, Guenter Roeck wrote:
> Hi Alistair,
> 
> On Wed, Apr 20, 2022 at 03:52:48PM +1000, Alistair Francis wrote:
> > From: Alistair Francis <alistair.francis@wdc.com>
> > 
> > Imply the TPM sysbus devices. This allows users to add TPM devices to
> > the RISC-V virt board.
> > 
> > This was tested by first creating an emulated TPM device:
> > 
> >     swtpm socket --tpm2 -t -d --tpmstate dir=/tmp/tpm \
> >         --ctrl type=unixio,path=swtpm-sock
> > 
> > Then launching QEMU with:
> > 
> >     -chardev socket,id=chrtpm,path=swtpm-sock \
> >     -tpmdev emulator,id=tpm0,chardev=chrtpm \
> >     -device tpm-tis-device,tpmdev=tpm0
> > 
> > The TPM device can be seen in the memory tree and the generated device
> > tree.
> > 
> I tried to get this working with qemu 8.0, but I did not have any success.
> I am quite sure I have the above command line correctly, and it does work
> with arm64. Any idea what I might be missing ?
> 

Answering my own question: Nothing. The problem is that the devicetree
is created too early, before the tpm device is instantiated/realized in
qemu. The tpm device therefore does not show up in devicetree, and the
tom device does not instantiate in Linux. The patch below fixes the problem
for me.

Any comments / thoughts ? Is that change acceptable, or should it be
implemented differently ?

Thanks,
Guenter

---
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4e3efbee16..ea259d7ade 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1247,6 +1247,11 @@ static void virt_machine_done(Notifier *notifier, void *data)
     uint32_t fdt_load_addr;
     uint64_t kernel_entry;
 
+    /* create devicetree if not provided */
+    if (!machine->dtb) {
+        create_fdt(s, memmap);
+    }
+
     /*
      * Only direct boot kernel is currently supported for KVM VM,
      * so the "-bios" parameter is not supported when KVM is enabled.
@@ -1519,15 +1524,13 @@ static void virt_machine_init(MachineState *machine)
     }
     virt_flash_map(s, system_memory);
 
-    /* load/create device tree */
+    /* load device tree */
     if (machine->dtb) {
         machine->fdt = load_device_tree(machine->dtb, &s->fdt_size);
         if (!machine->fdt) {
             error_report("load_device_tree() failed");
             exit(1);
         }
-    } else {
-        create_fdt(s, memmap);
     }
 
     s->machine_done.notify = virt_machine_done;

Re: [PATCH v4 6/6] hw/riscv: Enable TPM backends

[Alistair Francis]

On Wed, Jun 21, 2023 at 4:34 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Mon, Jun 19, 2023 at 01:32:34PM -0700, Guenter Roeck wrote:
> > Hi Alistair,
> >
> > On Wed, Apr 20, 2022 at 03:52:48PM +1000, Alistair Francis wrote:
> > > From: Alistair Francis <alistair.francis@wdc.com>
> > >
> > > Imply the TPM sysbus devices. This allows users to add TPM devices to
> > > the RISC-V virt board.
> > >
> > > This was tested by first creating an emulated TPM device:
> > >
> > >     swtpm socket --tpm2 -t -d --tpmstate dir=/tmp/tpm \
> > >         --ctrl type=unixio,path=swtpm-sock
> > >
> > > Then launching QEMU with:
> > >
> > >     -chardev socket,id=chrtpm,path=swtpm-sock \
> > >     -tpmdev emulator,id=tpm0,chardev=chrtpm \
> > >     -device tpm-tis-device,tpmdev=tpm0
> > >
> > > The TPM device can be seen in the memory tree and the generated device
> > > tree.
> > >
> > I tried to get this working with qemu 8.0, but I did not have any success.
> > I am quite sure I have the above command line correctly, and it does work
> > with arm64. Any idea what I might be missing ?
> >
>
> Answering my own question: Nothing. The problem is that the devicetree
> is created too early, before the tpm device is instantiated/realized in
> qemu. The tpm device therefore does not show up in devicetree, and the
> tom device does not instantiate in Linux. The patch below fixes the problem
> for me.

Thanks for looking into this!

>
> Any comments / thoughts ? Is that change acceptable, or should it be
> implemented differently ?

The diff below looks fine to me. Do you mind sending a patch?

Alistair

>
> Thanks,
> Guenter
>
> ---
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 4e3efbee16..ea259d7ade 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1247,6 +1247,11 @@ static void virt_machine_done(Notifier *notifier, void *data)
>      uint32_t fdt_load_addr;
>      uint64_t kernel_entry;
>
> +    /* create devicetree if not provided */
> +    if (!machine->dtb) {
> +        create_fdt(s, memmap);
> +    }
> +
>      /*
>       * Only direct boot kernel is currently supported for KVM VM,
>       * so the "-bios" parameter is not supported when KVM is enabled.
> @@ -1519,15 +1524,13 @@ static void virt_machine_init(MachineState *machine)
>      }
>      virt_flash_map(s, system_memory);
>
> -    /* load/create device tree */
> +    /* load device tree */
>      if (machine->dtb) {
>          machine->fdt = load_device_tree(machine->dtb, &s->fdt_size);
>          if (!machine->fdt) {
>              error_report("load_device_tree() failed");
>              exit(1);
>          }
> -    } else {
> -        create_fdt(s, memmap);
>      }
>
>      s->machine_done.notify = virt_machine_done;