[RFC,v2,1/7] hw/misc/dyn_sysbus_binding: helpers for sysbus device dynamic binding

This new module implements routines which help in dynamic device
binding (mmio regions, irq). They are supposed to be used by machine
files that support dynamic sysbus instantiation.

---

v1 -> v2:
- platform_devices renamed into dyn_sysbus_binding
- PlatformParams renamed into DynSysbusParams
- PlatformBusNotifier renamed into DynSysbusNotifier
- platform_bus_map_irq, platform_bus_map_mmio, sysbus_device_check,
  platform_bus_init become static
- PlatformBusInitData becomes private to the module
- page_shift becomes a member of DynSysbusParams

v1: Dynamic sysbus device allocation fully written by Alex Graf.
Those functions were initially in ppc e500 machine file. Now moved to a
separate module.
PPCE500Params is replaced by a generic struct named PlatformParams

Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
 hw/misc/Makefile.objs                |   1 +
 hw/misc/dyn_sysbus_binding.c         | 163 +++++++++++++++++++++++++++++++++++
 include/hw/misc/dyn_sysbus_binding.h |  24 ++++++
 3 files changed, 188 insertions(+)
 create mode 100644 hw/misc/dyn_sysbus_binding.c
 create mode 100644 include/hw/misc/dyn_sysbus_binding.h

On 08.08.14 17:03, Eric Auger wrote:
> This new module implements routines which help in dynamic device
> binding (mmio regions, irq). They are supposed to be used by machine
> files that support dynamic sysbus instantiation.
>
> ---
>
> v1 -> v2:
> - platform_devices renamed into dyn_sysbus_binding
> - PlatformParams renamed into DynSysbusParams
> - PlatformBusNotifier renamed into DynSysbusNotifier
> - platform_bus_map_irq, platform_bus_map_mmio, sysbus_device_check,
>    platform_bus_init become static
> - PlatformBusInitData becomes private to the module
> - page_shift becomes a member of DynSysbusParams
>
> v1: Dynamic sysbus device allocation fully written by Alex Graf.
> Those functions were initially in ppc e500 machine file. Now moved to a
> separate module.
> PPCE500Params is replaced by a generic struct named PlatformParams
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>   hw/misc/Makefile.objs                |   1 +
>   hw/misc/dyn_sysbus_binding.c         | 163 +++++++++++++++++++++++++++++++++++
>   include/hw/misc/dyn_sysbus_binding.h |  24 ++++++
>   3 files changed, 188 insertions(+)
>   create mode 100644 hw/misc/dyn_sysbus_binding.c
>   create mode 100644 include/hw/misc/dyn_sysbus_binding.h
>
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 979e532..86f6243 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>   obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>   
>   obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-y += dyn_sysbus_binding.o
> diff --git a/hw/misc/dyn_sysbus_binding.c b/hw/misc/dyn_sysbus_binding.c
> new file mode 100644
> index 0000000..0f34f0b
> --- /dev/null
> +++ b/hw/misc/dyn_sysbus_binding.c
> @@ -0,0 +1,163 @@

This file is missing a license header.

> +#include "hw/misc/dyn_sysbus_binding.h"
> +#include "qemu/error-report.h"
> +
> +typedef struct PlatformBusInitData {
> +    unsigned long *used_irqs;
> +    unsigned long *used_mem;
> +    MemoryRegion *mem;
> +    qemu_irq *irqs;
> +    int device_count;
> +    DynSysbusParams *params;
> +} PlatformBusInitData;
> +
> +
> +static int platform_bus_map_irq(DynSysbusParams *params,
> +                                SysBusDevice *sbdev,
> +                                int n, unsigned long *used_irqs,
> +                                qemu_irq *platform_irqs)
> +{
> +    int max_irqs = params->platform_bus_num_irqs;
> +    char *prop = g_strdup_printf("irq[%d]", n);
> +    int irqn = object_property_get_int(OBJECT(sbdev), prop, NULL);
> +
> +    if (irqn == SYSBUS_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("IRQ %d is already allocated or no free IRQ left", irqn);
> +    }
> +
> +    sysbus_connect_irq(sbdev, n, platform_irqs[irqn]);
> +    object_property_set_int(OBJECT(sbdev), irqn, prop, NULL);
> +
> +    g_free(prop);
> +    return 0;
> +}
> +
> +static int platform_bus_map_mmio(DynSysbusParams *params,
> +                                 SysBusDevice *sbdev,
> +                                 int n, unsigned long *used_mem,
> +                                 MemoryRegion *pmem)
> +{
> +    MemoryRegion *device_mem = sbdev->mmio[n].memory;
> +    uint64_t size = memory_region_size(device_mem);
> +    uint64_t page_size = (1 << params->page_shift);
> +    uint64_t page_mask = page_size - 1;
> +    uint64_t size_pages = (size + page_mask) >> params->page_shift;
> +    uint64_t max_size = params->platform_bus_size;
> +    uint64_t max_pages = max_size >> params->page_shift;
> +    char *prop = g_strdup_printf("mmio[%d]", n);
> +    hwaddr addr = object_property_get_int(OBJECT(sbdev), prop, NULL);
> +    int page;
> +    int i;
> +
> +    page = addr >> params->page_shift;
> +    if (addr == SYSBUS_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, max_pages, 0, size_pages,
> +                                          size_pages_align);
> +
> +        addr = (uint64_t)page << params->page_shift;
> +    }
> +
> +    if (page >= max_pages || test_bit(page, used_mem) ||
> +        (find_next_bit(used_mem, max_pages, page) < size_pages)) {
> +        hw_error("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);
> +    sbdev->mmio[n].addr = addr;
> +    object_property_set_int(OBJECT(sbdev), addr, prop, NULL);
> +
> +    g_free(prop);
> +    return 0;
> +}
> +
> +static int sysbus_device_check(Object *obj, void *opaque)
> +{
> +    PlatformBusInitData *init = opaque;
> +    Object *dev;
> +    SysBusDevice *sbdev;
> +    int i;
> +
> +    dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
> +    sbdev = (SysBusDevice *)dev;
> +
> +    if (!sbdev) {
> +        /* Container, traverse it for children */
> +        return object_child_foreach(obj, sysbus_device_check, opaque);
> +    }
> +
> +    /* Connect sysbus device to virtual platform bus */
> +    for (i = 0; i < sbdev->num_irq; i++) {
> +        if (!sbdev->irqp[i]) {
> +            /* This IRQ is an incoming IRQ, we can't wire those here */
> +            continue;
> +        }
> +        platform_bus_map_irq(init->params, sbdev, i,
> +                             init->used_irqs, init->irqs);
> +    }
> +
> +    for (i = 0; i < sbdev->num_mmio; i++) {
> +        platform_bus_map_mmio(init->params, sbdev, i,
> +                              init->used_mem, init->mem);
> +    }
> +
> +    return 0;
> +}
> +
> +static void platform_bus_init(DynSysbusParams *params,
> +                       MemoryRegion *address_space_mem,
> +                       qemu_irq *mpic)

You probably don't want to call it "mpic" in generic code - you're using 
a GIC after all on ARM.

Also I think it'd be better if you actually transform my patch set into 
the generic variant. That way I can push mine upstream and then your 
patch set makes that copy generic.


Alex

> +{
> +    uint64_t max_size = params->platform_bus_size;
> +    uint64_t max_pages = max_size >> params->page_shift;
> +    DECLARE_BITMAP(used_irqs, params->platform_bus_num_irqs);
> +    DECLARE_BITMAP(used_mem, max_pages);
> +    MemoryRegion *platform_region = g_new(MemoryRegion, 1);
> +    Object *container;
> +    PlatformBusInitData init = {
> +        .used_irqs = used_irqs,
> +        .used_mem = used_mem,
> +        .mem = platform_region,
> +        .irqs = &mpic[params->platform_bus_first_irq],
> +        .params = params,
> +    };
> +
> +    memory_region_init(platform_region, NULL, "platform devices",
> +                       params->platform_bus_size);
> +
> +    bitmap_clear(used_irqs, 0, params->platform_bus_num_irqs);
> +    bitmap_clear(used_mem, 0, max_pages);
> +
> +    /* Loop through all sysbus devices that were spawened outside the machine */
> +    container = container_get(qdev_get_machine(), "/peripheral");
> +    sysbus_device_check(container, &init);
> +    container = container_get(qdev_get_machine(), "/peripheral-anon");
> +    sysbus_device_check(container, &init);
> +
> +    memory_region_add_subregion(address_space_mem, params->platform_bus_base,
> +                                platform_region);
> +}
> +
> +void platform_bus_init_notify(Notifier *notifier, void *data)
> +{
> +    DynSysbusNotifier *pn = (DynSysbusNotifier *)notifier;
> +    platform_bus_init(&pn->params, pn->address_space_mem, pn->mpic);
> +}
> diff --git a/include/hw/misc/dyn_sysbus_binding.h b/include/hw/misc/dyn_sysbus_binding.h
> new file mode 100644
> index 0000000..961c9c7
> --- /dev/null
> +++ b/include/hw/misc/dyn_sysbus_binding.h
> @@ -0,0 +1,24 @@
> +#ifndef HW_MISC_PLATFORM_DEVICES_H
> +#define HW_MISC_PLATFORM_DEVICES_H
> +
> +#include "qemu-common.h"
> +#include "hw/sysbus.h"
> +
> +typedef struct {
> +    bool has_platform_bus;
> +    hwaddr platform_bus_base;
> +    hwaddr platform_bus_size;
> +    int platform_bus_first_irq;
> +    int platform_bus_num_irqs;
> +    int page_shift;
> +} DynSysbusParams;
> +
> +typedef struct DynSysbusNotifier {
> +    Notifier notifier;
> +    MemoryRegion *address_space_mem;
> +    qemu_irq *mpic;
> +    DynSysbusParams params;
> +} DynSysbusNotifier;
> +
> +void platform_bus_init_notify(Notifier *notifier, void *data);
> +#endif

On 08/11/2014 03:08 PM, Alexander Graf wrote:
> 
> On 08.08.14 17:03, Eric Auger wrote:
>> This new module implements routines which help in dynamic device
>> binding (mmio regions, irq). They are supposed to be used by machine
>> files that support dynamic sysbus instantiation.
>>
>> ---
>>
>> v1 -> v2:
>> - platform_devices renamed into dyn_sysbus_binding
>> - PlatformParams renamed into DynSysbusParams
>> - PlatformBusNotifier renamed into DynSysbusNotifier
>> - platform_bus_map_irq, platform_bus_map_mmio, sysbus_device_check,
>>    platform_bus_init become static
>> - PlatformBusInitData becomes private to the module
>> - page_shift becomes a member of DynSysbusParams
>>
>> v1: Dynamic sysbus device allocation fully written by Alex Graf.
>> Those functions were initially in ppc e500 machine file. Now moved to a
>> separate module.
>> PPCE500Params is replaced by a generic struct named PlatformParams
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>> ---
>>   hw/misc/Makefile.objs                |   1 +
>>   hw/misc/dyn_sysbus_binding.c         | 163
>> +++++++++++++++++++++++++++++++++++
>>   include/hw/misc/dyn_sysbus_binding.h |  24 ++++++
>>   3 files changed, 188 insertions(+)
>>   create mode 100644 hw/misc/dyn_sysbus_binding.c
>>   create mode 100644 include/hw/misc/dyn_sysbus_binding.h
>>
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index 979e532..86f6243 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>>   obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>>     obj-$(CONFIG_PVPANIC) += pvpanic.o
>> +obj-y += dyn_sysbus_binding.o
>> diff --git a/hw/misc/dyn_sysbus_binding.c b/hw/misc/dyn_sysbus_binding.c
>> new file mode 100644
>> index 0000000..0f34f0b
>> --- /dev/null
>> +++ b/hw/misc/dyn_sysbus_binding.c
>> @@ -0,0 +1,163 @@
> 
> This file is missing a license header.
OK
> 
>> +#include "hw/misc/dyn_sysbus_binding.h"
>> +#include "qemu/error-report.h"
>> +
>> +typedef struct PlatformBusInitData {
>> +    unsigned long *used_irqs;
>> +    unsigned long *used_mem;
>> +    MemoryRegion *mem;
>> +    qemu_irq *irqs;
>> +    int device_count;
>> +    DynSysbusParams *params;
>> +} PlatformBusInitData;
>> +
>> +
>> +static int platform_bus_map_irq(DynSysbusParams *params,
>> +                                SysBusDevice *sbdev,
>> +                                int n, unsigned long *used_irqs,
>> +                                qemu_irq *platform_irqs)
>> +{
>> +    int max_irqs = params->platform_bus_num_irqs;
>> +    char *prop = g_strdup_printf("irq[%d]", n);
>> +    int irqn = object_property_get_int(OBJECT(sbdev), prop, NULL);
>> +
>> +    if (irqn == SYSBUS_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("IRQ %d is already allocated or no free IRQ left",
>> irqn);
>> +    }
>> +
>> +    sysbus_connect_irq(sbdev, n, platform_irqs[irqn]);
>> +    object_property_set_int(OBJECT(sbdev), irqn, prop, NULL);
>> +
>> +    g_free(prop);
>> +    return 0;
>> +}
>> +
>> +static int platform_bus_map_mmio(DynSysbusParams *params,
>> +                                 SysBusDevice *sbdev,
>> +                                 int n, unsigned long *used_mem,
>> +                                 MemoryRegion *pmem)
>> +{
>> +    MemoryRegion *device_mem = sbdev->mmio[n].memory;
>> +    uint64_t size = memory_region_size(device_mem);
>> +    uint64_t page_size = (1 << params->page_shift);
>> +    uint64_t page_mask = page_size - 1;
>> +    uint64_t size_pages = (size + page_mask) >> params->page_shift;
>> +    uint64_t max_size = params->platform_bus_size;
>> +    uint64_t max_pages = max_size >> params->page_shift;
>> +    char *prop = g_strdup_printf("mmio[%d]", n);
>> +    hwaddr addr = object_property_get_int(OBJECT(sbdev), prop, NULL);
>> +    int page;
>> +    int i;
>> +
>> +    page = addr >> params->page_shift;
>> +    if (addr == SYSBUS_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, max_pages, 0,
>> size_pages,
>> +                                          size_pages_align);
>> +
>> +        addr = (uint64_t)page << params->page_shift;
>> +    }
>> +
>> +    if (page >= max_pages || test_bit(page, used_mem) ||
>> +        (find_next_bit(used_mem, max_pages, page) < size_pages)) {
>> +        hw_error("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);
>> +    sbdev->mmio[n].addr = addr;
>> +    object_property_set_int(OBJECT(sbdev), addr, prop, NULL);
>> +
>> +    g_free(prop);
>> +    return 0;
>> +}
>> +
>> +static int sysbus_device_check(Object *obj, void *opaque)
>> +{
>> +    PlatformBusInitData *init = opaque;
>> +    Object *dev;
>> +    SysBusDevice *sbdev;
>> +    int i;
>> +
>> +    dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
>> +    sbdev = (SysBusDevice *)dev;
>> +
>> +    if (!sbdev) {
>> +        /* Container, traverse it for children */
>> +        return object_child_foreach(obj, sysbus_device_check, opaque);
>> +    }
>> +
>> +    /* Connect sysbus device to virtual platform bus */
>> +    for (i = 0; i < sbdev->num_irq; i++) {
>> +        if (!sbdev->irqp[i]) {
>> +            /* This IRQ is an incoming IRQ, we can't wire those here */
>> +            continue;
>> +        }
>> +        platform_bus_map_irq(init->params, sbdev, i,
>> +                             init->used_irqs, init->irqs);
>> +    }
>> +
>> +    for (i = 0; i < sbdev->num_mmio; i++) {
>> +        platform_bus_map_mmio(init->params, sbdev, i,
>> +                              init->used_mem, init->mem);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void platform_bus_init(DynSysbusParams *params,
>> +                       MemoryRegion *address_space_mem,
>> +                       qemu_irq *mpic)
> 
> You probably don't want to call it "mpic" in generic code - you're using
> a GIC after all on ARM.
yes, will change that
> 
> Also I think it'd be better if you actually transform my patch set into
> the generic variant. That way I can push mine upstream and then your
> patch set makes that copy generic.
OK I will do that

thanks

Eric
> 
> 
> Alex
> 
>> +{
>> +    uint64_t max_size = params->platform_bus_size;
>> +    uint64_t max_pages = max_size >> params->page_shift;
>> +    DECLARE_BITMAP(used_irqs, params->platform_bus_num_irqs);
>> +    DECLARE_BITMAP(used_mem, max_pages);
>> +    MemoryRegion *platform_region = g_new(MemoryRegion, 1);
>> +    Object *container;
>> +    PlatformBusInitData init = {
>> +        .used_irqs = used_irqs,
>> +        .used_mem = used_mem,
>> +        .mem = platform_region,
>> +        .irqs = &mpic[params->platform_bus_first_irq],
>> +        .params = params,
>> +    };
>> +
>> +    memory_region_init(platform_region, NULL, "platform devices",
>> +                       params->platform_bus_size);
>> +
>> +    bitmap_clear(used_irqs, 0, params->platform_bus_num_irqs);
>> +    bitmap_clear(used_mem, 0, max_pages);
>> +
>> +    /* Loop through all sysbus devices that were spawened outside the
>> machine */
>> +    container = container_get(qdev_get_machine(), "/peripheral");
>> +    sysbus_device_check(container, &init);
>> +    container = container_get(qdev_get_machine(), "/peripheral-anon");
>> +    sysbus_device_check(container, &init);
>> +
>> +    memory_region_add_subregion(address_space_mem,
>> params->platform_bus_base,
>> +                                platform_region);
>> +}
>> +
>> +void platform_bus_init_notify(Notifier *notifier, void *data)
>> +{
>> +    DynSysbusNotifier *pn = (DynSysbusNotifier *)notifier;
>> +    platform_bus_init(&pn->params, pn->address_space_mem, pn->mpic);
>> +}
>> diff --git a/include/hw/misc/dyn_sysbus_binding.h
>> b/include/hw/misc/dyn_sysbus_binding.h
>> new file mode 100644
>> index 0000000..961c9c7
>> --- /dev/null
>> +++ b/include/hw/misc/dyn_sysbus_binding.h
>> @@ -0,0 +1,24 @@
>> +#ifndef HW_MISC_PLATFORM_DEVICES_H
>> +#define HW_MISC_PLATFORM_DEVICES_H
>> +
>> +#include "qemu-common.h"
>> +#include "hw/sysbus.h"
>> +
>> +typedef struct {
>> +    bool has_platform_bus;
>> +    hwaddr platform_bus_base;
>> +    hwaddr platform_bus_size;
>> +    int platform_bus_first_irq;
>> +    int platform_bus_num_irqs;
>> +    int page_shift;
>> +} DynSysbusParams;
>> +
>> +typedef struct DynSysbusNotifier {
>> +    Notifier notifier;
>> +    MemoryRegion *address_space_mem;
>> +    qemu_irq *mpic;
>> +    DynSysbusParams params;
>> +} DynSysbusNotifier;
>> +
>> +void platform_bus_init_notify(Notifier *notifier, void *data);
>> +#endif
>