| Message ID | 1429888773-11730-4-git-send-email-eric.auger@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Alex On 04/27/2015 03:41 PM, Alexander Graf wrote: > On 04/24/2015 05:19 PM, 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@suse.de> > > Hrm, are you sure this code is from me? :) Well I just wanted to emphasize this originates from your e500.c etsec integration. through this is true most of it now is in sysbus-fdt.c. I will remove your Signed-off-by if this is your will ;-) Best Regards Eric > >> Signed-off-by: Eric Auger <eric.auger@linaro.org> >> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> >> >> --- >> v11 -> v12: >> - resize PLATFORM_BUS_NUM_IRQS to 64 instead of 32 >> - overall NUM_IRQS changed to 256. This leaves space for MSI >> looming addition >> - VIRT_PLATFORM_BUS size changed from 4MB to 32MB and base set at >> 0xc000000 >> - Add Alex R-b >> >> v8 -> v9: >> - PLATFORM_BUS_NUM_IRQS set to 32 instead of 20 >> - platform bus irq now start at 64 instead of 48 >> - remove change of indentation in a15memmap >> - correct misc style issues >> >> v7 -> v8: >> - rebase on 2.2.0 >> - in machvirt_init, create_platform_bus simply is added >> after the arm_load_kernel call instead of moving this latter. >> Related comment slighly reworded. >> - Due to those changes I dropped Alex and Shannon's Reviewed-by >> >> v6 -> v7: >> Take into account Shannon comments: >> - remove PLATFORM_BUS_FIRST_IRQ macro >> - correct platform bus size to 0x400000 >> - add an additional comment in a15irqmap related to >> PLATFORM_BUS_NUM_IRQS >> >> 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 >> --- >> hw/arm/virt.c | 61 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 60 insertions(+), 1 deletion(-) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 565f573..efa3216 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -43,11 +43,13 @@ >> #include "qemu/bitops.h" >> #include "qemu/error-report.h" >> #include "hw/pci-host/gpex.h" >> +#include "hw/arm/sysbus-fdt.h" >> +#include "hw/platform-bus.h" >> #define NUM_VIRTIO_TRANSPORTS 32 >> /* Number of external interrupt lines to configure the GIC with */ >> -#define NUM_IRQS 128 >> +#define NUM_IRQS 256 >> #define GIC_FDT_IRQ_TYPE_SPI 0 >> #define GIC_FDT_IRQ_TYPE_PPI 1 >> @@ -60,6 +62,8 @@ >> #define GIC_FDT_IRQ_PPI_CPU_START 8 >> #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8 >> +#define PLATFORM_BUS_NUM_IRQS 64 >> + >> enum { >> VIRT_FLASH, >> VIRT_MEM, >> @@ -71,8 +75,11 @@ enum { >> VIRT_RTC, >> VIRT_FW_CFG, >> VIRT_PCIE, >> + VIRT_PLATFORM_BUS, >> }; >> +static ARMPlatformBusSystemParams platform_bus_params; >> + >> typedef struct MemMapEntry { >> hwaddr base; >> hwaddr size; >> @@ -131,6 +138,7 @@ static const MemMapEntry a15memmap[] = { >> [VIRT_FW_CFG] = { 0x09020000, 0x0000000a }, >> [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, >> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of >> that size */ >> + [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 }, > > Peter, would you have a hard time if we just get rid of VIRT_MMIO > completely and allow users to create the mmio-virtio bridges using > -device for -M virt-2.4 and above? > > At the end of the day, I'm fairly sure people will end up virtio-pci > anyway and it's just a big waste of address space to keep VIRT_MMIO > around, no? > > That said, if you want to keep virtio-mmio support in the way it is > today, I won't object. The rest of the patch looks good to me. > > > Alex >
On 27 April 2015 at 14:41, Alexander Graf <agraf@suse.de> wrote: > Peter, would you have a hard time if we just get rid of VIRT_MMIO completely > and allow users to create the mmio-virtio bridges using -device for -M > virt-2.4 and above? I would strongly prefer not -- it breaks existing working command lines. I agree that we are going to want to move to virtio-pci and virtio-mmio is just legacy, but retaining legacy is what makes QEMU not kvmtool, right? :-) -- PMM
On 27 April 2015 at 18:04, Alexander Graf <agraf@suse.de> wrote: > On 04/27/2015 06:58 PM, Christopher Covington wrote: >> I'm not sure I have an opinion one way or the other, but I would like to >> understand the "big waste" argument. Is there something that users are >> eager >> to reuse this address space for, like more RAM? > > > It will get used for platform device assignment. If we only reserve 32MB we > will quickly run out of address space to map devices into. Personally I am hoping that platform device assignment will also turn out to be mostly for legacy reasons and anybody wanting to do serious device passthrough will be doing it with PCI... -- PMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 565f573..efa3216 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -43,11 +43,13 @@ #include "qemu/bitops.h" #include "qemu/error-report.h" #include "hw/pci-host/gpex.h" +#include "hw/arm/sysbus-fdt.h" +#include "hw/platform-bus.h" #define NUM_VIRTIO_TRANSPORTS 32 /* Number of external interrupt lines to configure the GIC with */ -#define NUM_IRQS 128 +#define NUM_IRQS 256 #define GIC_FDT_IRQ_TYPE_SPI 0 #define GIC_FDT_IRQ_TYPE_PPI 1 @@ -60,6 +62,8 @@ #define GIC_FDT_IRQ_PPI_CPU_START 8 #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8 +#define PLATFORM_BUS_NUM_IRQS 64 + enum { VIRT_FLASH, VIRT_MEM, @@ -71,8 +75,11 @@ enum { VIRT_RTC, VIRT_FW_CFG, VIRT_PCIE, + VIRT_PLATFORM_BUS, }; +static ARMPlatformBusSystemParams platform_bus_params; + typedef struct MemMapEntry { hwaddr base; hwaddr size; @@ -131,6 +138,7 @@ static const MemMapEntry a15memmap[] = { [VIRT_FW_CFG] = { 0x09020000, 0x0000000a }, [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ + [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 }, /* * PCIE verbose map: * @@ -147,6 +155,7 @@ static const int a15irqmap[] = { [VIRT_RTC] = 2, [VIRT_PCIE] = 3, /* ... to 6 */ [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ + [VIRT_PLATFORM_BUS] = 64, /* ... to 64 + PLATFORM_BUS_NUM_IRQS -1 */ }; static VirtBoardInfo machines[] = { @@ -717,6 +726,47 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic, 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; @@ -837,6 +887,14 @@ static void machvirt_init(MachineState *machine) vbi->bootinfo.get_dtb = machvirt_dtb; vbi->bootinfo.firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0); arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo); + + /* + * arm_load_kernel machine init done notifier registration must + * happen before the platform_bus_create call. In this latter, + * another notifier is registered which adds platform bus nodes. + * Notifiers are executed in registration reverse order. + */ + create_platform_bus(vbi, pic); } static bool virt_get_secure(Object *obj, Error **errp) @@ -875,6 +933,7 @@ static void virt_class_init(ObjectClass *oc, void *data) mc->desc = "ARM Virtual Machine", mc->init = machvirt_init; mc->max_cpus = 8; + mc->has_dynamic_sysbus = true; } static const TypeInfo machvirt_info = {