From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44287) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0STv-0004yZ-Fy for qemu-devel@nongnu.org; Fri, 27 Jun 2014 05:30:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X0STo-0002qK-Fe for qemu-devel@nongnu.org; Fri, 27 Jun 2014 05:30:07 -0400 Received: from mail-wg0-f49.google.com ([74.125.82.49]:38141) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0STo-0002px-7V for qemu-devel@nongnu.org; Fri, 27 Jun 2014 05:30:00 -0400 Received: by mail-wg0-f49.google.com with SMTP id y10so4859252wgg.20 for ; Fri, 27 Jun 2014 02:29:58 -0700 (PDT) Message-ID: <53AD3983.8040300@linaro.org> Date: Fri, 27 Jun 2014 11:29:39 +0200 From: Eric Auger MIME-Version: 1.0 References: <1401884936-12907-1-git-send-email-agraf@suse.de> <1401884936-12907-5-git-send-email-agraf@suse.de> In-Reply-To: <1401884936-12907-5-git-send-email-agraf@suse.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/5] PPC: e500: Support platform devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf , qemu-ppc@nongnu.org Cc: qemu-devel@nongnu.org On 06/04/2014 02:28 PM, Alexander Graf wrote: > For e500 our approach to supporting platform devices is to create a simple > bus from the guest's point of view within which we map platform devices > dynamically. > > We allocate memory regions always within the "platform" hole in address > space and map IRQs to predetermined IRQ lines that are reserved for platform > device usage. > > This maps really nicely into device tree logic, so we can just tell the > guest about our virtual simple bus in device tree as well. > > Signed-off-by: Alexander Graf > --- > default-configs/ppc-softmmu.mak | 1 + > default-configs/ppc64-softmmu.mak | 1 + > hw/ppc/e500.c | 221 ++++++++++++++++++++++++++++++++++++++ > hw/ppc/e500.h | 1 + > hw/ppc/e500plat.c | 1 + > 5 files changed, 225 insertions(+) > > diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak > index 33f8d84..d6ec8b9 100644 > --- a/default-configs/ppc-softmmu.mak > +++ b/default-configs/ppc-softmmu.mak > @@ -45,6 +45,7 @@ CONFIG_PREP=y > CONFIG_MAC=y > CONFIG_E500=y > CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM)) > +CONFIG_PLATFORM=y > # For PReP > CONFIG_MC146818RTC=y > CONFIG_ETSEC=y > diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak > index 37a15b7..06677bf 100644 > --- a/default-configs/ppc64-softmmu.mak > +++ b/default-configs/ppc64-softmmu.mak > @@ -45,6 +45,7 @@ CONFIG_PSERIES=y > CONFIG_PREP=y > CONFIG_MAC=y > CONFIG_E500=y > +CONFIG_PLATFORM=y > CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM)) > # For pSeries > CONFIG_XICS=$(CONFIG_PSERIES) > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c > index 33d54b3..bc26215 100644 > --- a/hw/ppc/e500.c > +++ b/hw/ppc/e500.c > @@ -36,6 +36,7 @@ > #include "exec/address-spaces.h" > #include "qemu/host-utils.h" > #include "hw/pci-host/ppce500.h" > +#include "hw/platform/device.h" > > #define EPAPR_MAGIC (0x45504150) > #define BINARY_DEVICE_TREE_FILE "mpc8544ds.dtb" > @@ -47,6 +48,14 @@ > > #define RAM_SIZES_ALIGN (64UL << 20) > > +#define E500_PLATFORM_BASE 0xF0000000ULL > +#define E500_PLATFORM_HOLE (128ULL * 1024 * 1024) /* 128 MB */ > +#define E500_PLATFORM_PAGE_SHIFT 12 > +#define E500_PLATFORM_HOLE_PAGES (E500_PLATFORM_HOLE >> \ > + E500_PLATFORM_PAGE_SHIFT) > +#define E500_PLATFORM_FIRST_IRQ 5 > +#define E500_PLATFORM_NUM_IRQS 10 > + > /* TODO: parameterize */ > #define MPC8544_CCSRBAR_BASE 0xE0000000ULL > #define MPC8544_CCSRBAR_SIZE 0x00100000ULL > @@ -122,6 +131,62 @@ static void dt_serial_create(void *fdt, unsigned long long offset, > } > } > > +typedef struct PlatformDevtreeData { > + void *fdt; > + const char *mpic; > + int irq_start; > + const char *node; > +} PlatformDevtreeData; > + > +static int platform_device_create_devtree(Object *obj, void *opaque) > +{ > + PlatformDevtreeData *data = opaque; > + Object *dev; > + PlatformDeviceState *pdev; > + > + dev = object_dynamic_cast(obj, TYPE_PLATFORM_DEVICE); > + pdev = (PlatformDeviceState *)dev; > + > + if (!pdev) { > + /* Container, traverse it for children */ > + return object_child_foreach(obj, platform_device_create_devtree, data); > + } > + > + return 0; > +} > + > +static void platform_create_devtree(void *fdt, const char *node, uint64_t addr, > + const char *mpic, int irq_start, > + int nr_irqs) > +{ > + const char platcomp[] = "qemu,platform\0simple-bus"; > + PlatformDevtreeData data; > + > + /* Create a /platform node that we can put all devices into */ > + > + qemu_fdt_add_subnode(fdt, node); > + qemu_fdt_setprop(fdt, node, "compatible", platcomp, sizeof(platcomp)); > + qemu_fdt_setprop_string(fdt, node, "device_type", "platform"); > + > + /* Our platform hole is less than 32bit big, so 1 cell is enough for address > + and size */ > + qemu_fdt_setprop_cells(fdt, node, "#size-cells", 1); > + qemu_fdt_setprop_cells(fdt, node, "#address-cells", 1); > + qemu_fdt_setprop_cells(fdt, node, "ranges", 0, addr >> 32, addr, > + E500_PLATFORM_HOLE); > + > + qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", mpic); > + > + /* Loop through all devices and create nodes for known ones */ > + > + data.fdt = fdt; > + data.mpic = mpic; > + data.irq_start = irq_start; > + data.node = node; > + > + platform_device_create_devtree(qdev_get_machine(), &data); > +} > + > static int ppce500_load_device_tree(MachineState *machine, > PPCE500Params *params, > hwaddr addr, > @@ -379,6 +444,12 @@ static int ppce500_load_device_tree(MachineState *machine, > qemu_fdt_setprop_cell(fdt, pci, "#address-cells", 3); > qemu_fdt_setprop_string(fdt, "/aliases", "pci0", pci); > > + if (params->has_platform) { > + platform_create_devtree(fdt, "/platform", E500_PLATFORM_BASE, > + mpic, E500_PLATFORM_FIRST_IRQ, > + E500_PLATFORM_NUM_IRQS); > + } > + > params->fixup_devtree(params, fdt); > > if (toplevel_compat) { > @@ -618,6 +689,147 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params *params, MemoryRegion *ccsr, > return mpic; > } > > +typedef struct PlatformNotifier { > + Notifier notifier; > + MemoryRegion *address_space_mem; > + qemu_irq *mpic; > +} PlatformNotifier; > + > +typedef struct PlatformInitData { > + unsigned long *used_irqs; > + unsigned long *used_mem; > + MemoryRegion *mem; > + qemu_irq *irqs; > + int device_count; > +} PlatformInitData; > + > +static int platform_map_irq(unsigned long *used_irqs, qemu_irq *platform_irqs, > + uint32_t *device_irqn, qemu_irq *device_irq) > +{ > + int max_irqs = E500_PLATFORM_NUM_IRQS; > + int irqn = *device_irqn; > + > + if (irqn == (uint32_t)PLATFORM_DYNAMIC) { > + /* Find the first available IRQ */ > + irqn = find_first_zero_bit(used_irqs, max_irqs); > + } > + > + if ((irqn >= max_irqs) || test_and_set_bit(irqn, used_irqs)) { > + hw_error("e500: IRQ %d is already allocated or no free IRQ left", irqn); > + } > + > + *device_irq = platform_irqs[irqn]; > + *device_irqn = irqn; Hi Alex, Wouldn' it be more natural to add IRQ_START here, given the semantic of plat_irqs? Besides, I saw you added it dt_serial_create. > + > + return 0; > +} > + > +static int platform_map_region(unsigned long *used_mem, MemoryRegion *pmem, > + uint64_t *device_addr, MemoryRegion *device_mem) > +{ > + uint64_t size = memory_region_size(device_mem); > + uint64_t page_size = (1 << E500_PLATFORM_PAGE_SHIFT); > + uint64_t page_mask = page_size - 1; > + uint64_t size_pages = (size + page_mask) >> E500_PLATFORM_PAGE_SHIFT; > + hwaddr addr = *device_addr; > + int page; > + int i; > + > + page = addr >> E500_PLATFORM_PAGE_SHIFT; > + if (addr == (uint64_t)PLATFORM_DYNAMIC) { > + uint64_t size_pages_align; > + > + /* Align the region to at least its own size granularity */ > + if (is_power_of_2(size_pages)) { > + size_pages_align = size_pages; > + } else { > + size_pages_align = pow2floor(size_pages) << 1; > + } > + > + /* Find the first available region that fits */ > + page = bitmap_find_next_zero_area(used_mem, E500_PLATFORM_HOLE_PAGES, 0, > + size_pages, size_pages_align); > + > + addr = (uint64_t)page << E500_PLATFORM_PAGE_SHIFT; > + } > + > + if (page >= E500_PLATFORM_HOLE_PAGES || test_bit(page, used_mem) || > + (find_next_bit(used_mem, E500_PLATFORM_HOLE_PAGES, page) < size_pages)) { > + hw_error("e500: Memory [%"PRIx64":%"PRIx64" is already allocated or " > + "no slot left", addr, size); > + } > + > + for (i = page; i < (page + size_pages); i++) { > + set_bit(i, used_mem); > + } > + > + memory_region_add_subregion(pmem, addr, device_mem); > + *device_addr = addr; same for E500_PLATFORM_BASE? Actually you use plat_region_addrs[0] as an offset wrt platorm parent region in dt_serial_create. The exact semantic may be clarified. Otherwise, the full patch works fine for vfio & virt too. I just moved that code in a separate file and moved E500_* in a PlatformSettings struct included in both PlatformNotifier and PlatformInitData. Best Regards Eric > + > + return 0; > +} > + > +static int platform_device_check(Object *obj, void *opaque) > +{ > + PlatformInitData *init = opaque; > + Object *dev; > + PlatformDeviceState *pdev; > + int i; > + > + dev = object_dynamic_cast(obj, TYPE_PLATFORM_DEVICE); > + pdev = (PlatformDeviceState *)dev; > + > + if (!pdev) { > + /* Container, traverse it for children */ > + return object_child_foreach(obj, platform_device_check, opaque); > + } > + > + /* Connect platform device to machine */ > + for (i = 0; i < pdev->num_irqs; i++) { > + platform_map_irq(init->used_irqs, init->irqs, &pdev->plat_irqs[i], > + pdev->irqs[i]); > + } > + > + for (i = 0; i < pdev->num_regions; i++) { > + platform_map_region(init->used_mem, init->mem, > + &pdev->plat_region_addrs[i], pdev->regions[i]); > + } > + > + return 0; > +} > + > +static void platform_devices_init(MemoryRegion *address_space_mem, > + qemu_irq *mpic) > +{ > + DECLARE_BITMAP(used_irqs, E500_PLATFORM_NUM_IRQS); > + DECLARE_BITMAP(used_mem, E500_PLATFORM_HOLE_PAGES); > + MemoryRegion *platform_region = g_new(MemoryRegion, 1); > + PlatformInitData init = { > + .used_irqs = used_irqs, > + .used_mem = used_mem, > + .mem = platform_region, > + .irqs = &mpic[E500_PLATFORM_FIRST_IRQ], > + }; > + > + memory_region_init(platform_region, NULL, "platform devices", > + E500_PLATFORM_HOLE); > + > + bitmap_clear(used_irqs, 0, E500_PLATFORM_NUM_IRQS); > + bitmap_clear(used_mem, 0, E500_PLATFORM_HOLE_PAGES); > + > + /* Loop through our internal device tree and attach any dangling device */ > + platform_device_check(qdev_get_machine(), &init); > + > + memory_region_add_subregion(address_space_mem, E500_PLATFORM_BASE, > + platform_region); > +} > + > +static void platform_devices_init_notify(Notifier *notifier, void *data) > +{ > + PlatformNotifier *pn = (PlatformNotifier *)notifier; > + platform_devices_init(pn->address_space_mem, pn->mpic); > +} > + > void ppce500_init(MachineState *machine, PPCE500Params *params) > { > MemoryRegion *address_space_mem = get_system_memory(); > @@ -770,6 +982,15 @@ void ppce500_init(MachineState *machine, PPCE500Params *params) > cur_base = (32 * 1024 * 1024); > } > > + /* Platform Devices */ > + if (params->has_platform) { > + PlatformNotifier *notifier = g_new(PlatformNotifier, 1); > + notifier->notifier.notify = platform_devices_init_notify; > + notifier->address_space_mem = address_space_mem; > + notifier->mpic = mpic; > + qemu_add_machine_init_done_notifier(¬ifier->notifier); > + } > + > /* Load kernel. */ > if (machine->kernel_filename) { > kernel_base = cur_base; > diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h > index 08b25fa..3a588ed 100644 > --- a/hw/ppc/e500.h > +++ b/hw/ppc/e500.h > @@ -11,6 +11,7 @@ typedef struct PPCE500Params { > void (*fixup_devtree)(struct PPCE500Params *params, void *fdt); > > int mpic_version; > + bool has_platform; > } PPCE500Params; > > void ppce500_init(MachineState *machine, PPCE500Params *params); > diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c > index 27df31d..bb84283 100644 > --- a/hw/ppc/e500plat.c > +++ b/hw/ppc/e500plat.c > @@ -35,6 +35,7 @@ static void e500plat_init(MachineState *machine) > .pci_nr_slots = PCI_SLOT_MAX - 1, > .fixup_devtree = e500plat_fixup_devtree, > .mpic_version = OPENPIC_MODEL_FSL_MPIC_42, > + .has_platform = true, > }; > > /* Older KVM versions don't support EPR which breaks guests when we announce >