Re: [Qemu-devel] [PATCH v5 10/10] hw/arm/dyn_sysbus_devtree: enable simple VFIO dynamic instantiation

[Peter Maydell]

On 18 August 2014 22:54, Joel Schopp <address@hidden> wrote:
>
> +static void vfio_fdt_add_device_node(SysBusDevice *sbdev, void *opaque)
> +{
> +    PlatformDevtreeData *data = opaque;
> +    void *fdt = data->fdt;
> +    const char *parent_node = data->node;
> +    int compat_str_len;
> +    char *nodename;
> +    int i, ret;
> +    uint32_t *irq_attr;
> +    uint64_t *reg_attr;
> +    uint64_t mmio_base;
> +    uint64_t irq_number;
> +    gchar mmio_base_prop[8];
> +    gchar irq_number_prop[8];
> +    VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
> +    VFIODevice *vbasedev = &vdev->vbasedev;
> +    Object *obj = OBJECT(sbdev);
> +
> +    mmio_base = object_property_get_int(obj, "mmio[0]", NULL);
> +
> +    nodename = g_strdup_printf("%s/address@hidden" PRIx64, parent_node,
> +                               vbasedev->name,
> +                               mmio_base);
> +
> +    qemu_fdt_add_subnode(fdt, nodename);
> +
> +    compat_str_len = strlen(vdev->compat) + 1;

At this point you've already substituted the NULs in,
so you can't call strlen(), I think.

> +    qemu_fdt_setprop(fdt, nodename, "compatible",
> +                            vdev->compat, compat_str_len);
> +
> +    reg_attr = g_new(uint64_t, vbasedev->num_regions*4);
> +
> +    for (i = 0; i < vbasedev->num_regions; i++) {
> +        snprintf(mmio_base_prop, sizeof(mmio_base_prop), "mmio[%d]", i);
> +        mmio_base = object_property_get_int(obj, mmio_base_prop, NULL);
> +        reg_attr[2*i] = 1;
> +        reg_attr[2*i+1] = mmio_base;
> +        reg_attr[2*i+2] = 1;
> +        reg_attr[2*i+3] = memory_region_size(&vdev->regions[i]->mem);
> +    }
>
> This should be 4 instead of 2.
> Also, to support 64 bit systems I think this should be 2 instead of 1.

Actually it depends entirely on what the board has done to
create the device tree node that we're inserting this child
node into. For ARM boot.c sets both #address-cells and
#size-cells to 2 regardless of whether the system is 32
or 64 bits, for simplicity. I imagine PPC does something
different. If we're editing a dtb that the user passed in (which
I think would be pretty lunatic so we shouldn't do this)
we'd have to actually walk the dtb to try to figure out what
the semantics of the reg property should be.

thanks
-- PMM