From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42335) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XyI56-0005yv-UM for qemu-devel@nongnu.org; Tue, 09 Dec 2014 05:31:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XyI50-0003dq-R0 for qemu-devel@nongnu.org; Tue, 09 Dec 2014 05:31:48 -0500 Received: from mail-wg0-f42.google.com ([74.125.82.42]:37666) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XyI50-0003de-JB for qemu-devel@nongnu.org; Tue, 09 Dec 2014 05:31:42 -0500 Received: by mail-wg0-f42.google.com with SMTP id z12so405162wgg.29 for ; Tue, 09 Dec 2014 02:31:42 -0800 (PST) Message-ID: <5486CF3B.7000307@linaro.org> Date: Tue, 09 Dec 2014 11:30:19 +0100 From: Eric Auger MIME-Version: 1.0 References: <1417371570-11789-1-git-send-email-eric.auger@linaro.org> <1417371570-11789-7-git-send-email-eric.auger@linaro.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 6/6] hw/arm/virt: add dynamic sysbus device support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Joel Schopp , Kim Phillips , eric.auger@st.com, Antonios Motakis , Alvise Rigo , manish.jaggi@caviumnetworks.com, Ard Biesheuvel , Will Deacon , QEMU Developers , Alexander Graf , Bharat Bhushan , Alex Williamson , Patch Tracking , Shannon Zhao , Stuart Yoder , Paolo Bonzini , "kvmarm@lists.cs.columbia.edu" , Christoffer Dall On 12/05/2014 05:36 PM, Peter Maydell wrote: > On 30 November 2014 at 18:19, 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 >> Signed-off-by: Eric Auger >> >> --- >> 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 | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 57 insertions(+) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 314e55b..37326a9 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,11 @@ >> #define GIC_FDT_IRQ_PPI_CPU_START 8 >> #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8 >> >> +#define PLATFORM_BUS_BASE 0x9400000 >> +#define PLATFORM_BUS_SIZE (4ULL * 1024 * 1024) >> +#define PLATFORM_BUS_FIRST_IRQ 48 >> +#define PLATFORM_BUS_NUM_IRQS 20 >> + >> enum { >> VIRT_FLASH, >> VIRT_MEM, >> @@ -68,6 +75,7 @@ enum { >> VIRT_UART, >> VIRT_MMIO, >> VIRT_RTC, >> + VIRT_PLATFORM_BUS, >> }; >> >> typedef struct MemMapEntry { >> @@ -107,6 +115,7 @@ static const MemMapEntry a15memmap[] = { >> [VIRT_GIC_CPU] = { 0x08010000, 0x00010000 }, >> [VIRT_UART] = { 0x09000000, 0x00001000 }, >> [VIRT_RTC] = { 0x09010000, 0x00001000 }, >> + [VIRT_PLATFORM_BUS] = {PLATFORM_BUS_BASE , PLATFORM_BUS_SIZE}, > > This makes it pretty hard to read this -- you should use > the raw 0x numbers here. Anywhere else that wants to know > the base address etc should fish it out of the memory > map at runtime, as we do with the other devices. Hi Peter ok. As a side effect platform_bus_params will not be a const anymore, previously recommended by Alex. > >> [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, >> /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ >> /* 0x10000000 .. 0x40000000 reserved for PCI */ >> @@ -117,6 +126,14 @@ static const int a15irqmap[] = { >> [VIRT_UART] = 1, >> [VIRT_RTC] = 2, >> [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ >> + [VIRT_PLATFORM_BUS] = PLATFORM_BUS_FIRST_IRQ, > > Similarly with interrupt numbers. ok Thanks Eric > > -- PMM >