On 12/16/2014 06:41 AM, Shannon Zhao wrote: > On 2014/12/9 18:29, Eric Auger wrote: >> Allows sysbus devices to be instantiated from command line by >> using -device option. Machvirt creates a platform bus at init. >> The dynamic sysbus devices are attached to this platform bus device. >> >> The platform bus device registers a machine init done notifier >> whose role will be to bind the dynamic sysbus devices. Indeed >> dynamic sysbus devices are created after machine init. >> >> machvirt also registers a notifier that will build the device >> tree nodes for the platform bus and its children dynamic sysbus >> devices. >> >> Signed-off-by: Alexander Graf <agraf@xxxxxxx> >> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx> >> >> --- >> v5 -> v6: >> - Take into account Peter's comments: >> - platform_bus_params initialized from vbi->memmap and vbi->irqmap. >> As a consequence, const is removed. Also alignment in a15memmap >> is slightly changed. >> - ARMPlatformBusSystemParams handle removed from create_platform_bus >> prototype >> - arm_load_kernel has become a machine init done notifier registration. >> It must be called before platform_bus_create to guarantee the correct >> notifier execution sequence >> >> v4 -> v5: >> - platform_bus_params becomes static const >> - reword comment in create_platform_bus >> - reword the commit message >> >> v3 -> v4: >> - use platform bus object, instantiated in create_platform_bus >> - device tree generation for platform bus and children dynamic >> sysbus devices is no more handled at reset but in a >> machine_init_done_notifier (due to the change in implementaion >> of ARM load dtb using rom_add_blob_fixed). >> - device tree enhancement now takes into account the case of >> user provided dtb. Before the user dtb was overwritten which >> was wrong. However in case the dtb is provided by the user, >> dynamic sysbus nodes are not added there. >> - renaming of MACHVIRT_PLATFORM defines >> - MACHVIRT_PLATFORM_PAGE_SHIFT and SIZE_PAGES not needed anymore, >> hence removed. >> - DynSysbusParams struct renamed into ARMPlatformBusSystemParams >> and above params removed. >> - separation of dt creation and QEMU binding is not mandated anymore >> since the device tree is not created from scratch anymore. Instead >> the modify_dtb function is used. >> - create_platform_bus registers another machine init done notifier >> to start VFIO IRQ handling. This latter executes after the >> dynamic sysbus device binding. >> >> v2 -> v3: >> - renaming of arm_platform_bus_create_devtree and arm_load_dtb >> - add copyright in hw/arm/dyn_sysbus_devtree.c >> >> v1 -> v2: >> - remove useless vfio-platform.h include file >> - s/MACHVIRT_PLATFORM_HOLE/MACHVIRT_PLATFORM_SIZE >> - use dyn_sysbus_binding and dyn_sysbus_devtree >> - dynamic sysbus platform buse size shrinked to 4MB and >> moved between RTC and MMIO >> >> v1: >> >> Inspired from what Alex Graf did in ppc e500 >> https://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00012.html >> >> Conflicts: >> hw/arm/sysbus-fdt.c >> --- >> hw/arm/virt.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++----------- >> 1 file changed, 80 insertions(+), 17 deletions(-) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 314e55b..366083a 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -42,6 +42,8 @@ >> #include "exec/address-spaces.h" >> #include "qemu/bitops.h" >> #include "qemu/error-report.h" >> +#include "hw/arm/sysbus-fdt.h" >> +#include "hw/platform-bus.h" >> >> #define NUM_VIRTIO_TRANSPORTS 32 >> >> @@ -59,6 +61,9 @@ >> #define GIC_FDT_IRQ_PPI_CPU_START 8 >> #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8 >> >> +#define PLATFORM_BUS_FIRST_IRQ 48 > > This macro is not used any more, right? Hi Shannon, yes you're right. I am going to remove it. > >> +#define PLATFORM_BUS_NUM_IRQS 20 >> + >> enum { >> VIRT_FLASH, >> VIRT_MEM, >> @@ -68,8 +73,11 @@ enum { >> VIRT_UART, >> VIRT_MMIO, >> VIRT_RTC, >> + VIRT_PLATFORM_BUS, >> }; >> >> +static ARMPlatformBusSystemParams platform_bus_params; >> + >> typedef struct MemMapEntry { >> hwaddr base; >> hwaddr size; >> @@ -100,23 +108,25 @@ typedef struct VirtBoardInfo { >> */ >> static const MemMapEntry a15memmap[] = { >> /* Space up to 0x8000000 is reserved for a boot ROM */ >> - [VIRT_FLASH] = { 0, 0x08000000 }, >> - [VIRT_CPUPERIPHS] = { 0x08000000, 0x00020000 }, >> + [VIRT_FLASH] = { 0, 0x08000000 }, >> + [VIRT_CPUPERIPHS] = { 0x08000000, 0x00020000 }, >> /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */ >> - [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 }, >> - [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 }, >> - [VIRT_UART] = { 0x09000000, 0x00001000 }, >> - [VIRT_RTC] = { 0x09010000, 0x00001000 }, >> - [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, >> + [VIRT_GIC_DIST] = { 0x08000000, 0x00010000 }, >> + [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 }, >> + [VIRT_UART] = { 0x09000000, 0x00001000 }, >> + [VIRT_RTC] = { 0x09010000, 0x00001000 }, >> + [VIRT_PLATFORM_BUS] = { 0x09400000, 0x04000000 }, > > The number of platform_bus_size is not right. Maybe it should be 0x00400000. you're fully correct! thanks for spotting this mistake. Best Regards Eric > > Thanks, > Shannon > >> + [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, >> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ >> /* 0x10000000 .. 0x40000000 reserved for PCI */ >> - [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 }, >> + [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 }, >> }; >> >> static const int a15irqmap[] = { >> [VIRT_UART] = 1, >> [VIRT_RTC] = 2, >> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ >> + [VIRT_PLATFORM_BUS] = 48, >> }; >> >> static VirtBoardInfo machines[] = { >> @@ -519,6 +529,47 @@ static void create_flash(const VirtBoardInfo *vbi) >> g_free(nodename); >> } >> >> +static void create_platform_bus(VirtBoardInfo *vbi, qemu_irq *pic) >> +{ >> + DeviceState *dev; >> + SysBusDevice *s; >> + int i; >> + ARMPlatformBusFdtParams *fdt_params = g_new(ARMPlatformBusFdtParams, 1); >> + MemoryRegion *sysmem = get_system_memory(); >> + >> + platform_bus_params.platform_bus_base = vbi->memmap[VIRT_PLATFORM_BUS].base; >> + platform_bus_params.platform_bus_size = vbi->memmap[VIRT_PLATFORM_BUS].size; >> + platform_bus_params.platform_bus_first_irq = vbi->irqmap[VIRT_PLATFORM_BUS]; >> + platform_bus_params.platform_bus_num_irqs = PLATFORM_BUS_NUM_IRQS; >> + >> + fdt_params->system_params = &platform_bus_params; >> + fdt_params->binfo = &vbi->bootinfo; >> + fdt_params->intc = "/intc"; >> + /* >> + * register a machine init done notifier that creates the device tree >> + * nodes of the platform bus and its children dynamic sysbus devices >> + */ >> + arm_register_platform_bus_fdt_creator(fdt_params); >> + >> + dev = qdev_create(NULL, TYPE_PLATFORM_BUS_DEVICE); >> + dev->id = TYPE_PLATFORM_BUS_DEVICE; >> + qdev_prop_set_uint32(dev, "num_irqs", >> + platform_bus_params.platform_bus_num_irqs); >> + qdev_prop_set_uint32(dev, "mmio_size", >> + platform_bus_params.platform_bus_size); >> + qdev_init_nofail(dev); >> + s = SYS_BUS_DEVICE(dev); >> + >> + for (i = 0; i < platform_bus_params.platform_bus_num_irqs; i++) { >> + int irqn = platform_bus_params.platform_bus_first_irq + i; >> + sysbus_connect_irq(s, i, pic[irqn]); >> + } >> + >> + memory_region_add_subregion(sysmem, >> + platform_bus_params.platform_bus_base, >> + sysbus_mmio_get_region(s, 0)); >> +} >> + >> static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size) >> { >> const VirtBoardInfo *board = (const VirtBoardInfo *)binfo; >> @@ -581,6 +632,24 @@ static void machvirt_init(MachineState *machine) >> >> object_property_set_bool(cpuobj, true, "realized", NULL); >> } >> + >> + /* >> + * register the arm_load_kernel machine init done notifier >> + * before platform_bus_create call. >> + * There, arm_register_platform_bus_fdt_creator registers another >> + * notifier that adds platform bus nodes. Notifiers are executed >> + * in registration reverse order. >> + */ >> + vbi->bootinfo.ram_size = machine->ram_size; >> + vbi->bootinfo.kernel_filename = machine->kernel_filename; >> + vbi->bootinfo.kernel_cmdline = machine->kernel_cmdline; >> + vbi->bootinfo.initrd_filename = machine->initrd_filename; >> + vbi->bootinfo.nb_cpus = smp_cpus; >> + vbi->bootinfo.board_id = -1; >> + vbi->bootinfo.loader_start = vbi->memmap[VIRT_MEM].base; >> + vbi->bootinfo.get_dtb = machvirt_dtb; >> + arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo); >> + >> fdt_add_timer_nodes(vbi); >> fdt_add_cpu_nodes(vbi); >> fdt_add_psci_node(vbi); >> @@ -604,15 +673,8 @@ static void machvirt_init(MachineState *machine) >> */ >> create_virtio_devices(vbi, pic); >> >> - vbi->bootinfo.ram_size = machine->ram_size; >> - vbi->bootinfo.kernel_filename = machine->kernel_filename; >> - vbi->bootinfo.kernel_cmdline = machine->kernel_cmdline; >> - vbi->bootinfo.initrd_filename = machine->initrd_filename; >> - vbi->bootinfo.nb_cpus = smp_cpus; >> - vbi->bootinfo.board_id = -1; >> - vbi->bootinfo.loader_start = vbi->memmap[VIRT_MEM].base; >> - vbi->bootinfo.get_dtb = machvirt_dtb; >> - arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo); >> + create_platform_bus(vbi, pic); >> + >> } >> >> static QEMUMachine machvirt_a15_machine = { >> @@ -620,6 +682,7 @@ static QEMUMachine machvirt_a15_machine = { >> .desc = "ARM Virtual Machine", >> .init = machvirt_init, >> .max_cpus = 8, >> + .has_dynamic_sysbus = true, >> }; >> >> static void machvirt_machine_init(void) >> > > _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm