[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 3/3] hw/arm/virt: add dynamic sysbus device s
From: |
Shannon Zhao |
Subject: |
Re: [Qemu-devel] [PATCH v6 3/3] hw/arm/virt: add dynamic sysbus device support |
Date: |
Tue, 16 Dec 2014 13:41:49 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 |
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 <address@hidden>
> Signed-off-by: Eric Auger <address@hidden>
>
> ---
> 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?
> +#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.
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)
>