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

[Eric Auger <eric.auger@linaro.org>]

On 08/11/2014 11:40 AM, Alexander Graf wrote:
> 
> On 09.08.14 16:25, Eric Auger wrote:
>> Generates the device node of VFIO devices, if any are invoked in
>> -device option. In case VFIO devices require more complex node
>> generation, they can be handled before.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>> ---
>>   hw/arm/dyn_sysbus_devtree.c | 138
>> ++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 138 insertions(+)
>>
>> diff --git a/hw/arm/dyn_sysbus_devtree.c b/hw/arm/dyn_sysbus_devtree.c
>> index 56af62f..ac34f07 100644
>> --- a/hw/arm/dyn_sysbus_devtree.c
>> +++ b/hw/arm/dyn_sysbus_devtree.c
>> @@ -1,6 +1,139 @@
>>   #include "hw/arm/dyn_sysbus_devtree.h"
>>   #include "qemu/error-report.h"
>>   #include "sysemu/device_tree.h"
>> +#include "hw/vfio/vfio-platform.h"
>> +
>> +static void vfio_fdt_add_device_node(SysBusDevice *sbdev, void *opaque);
>> +
>> +static char *format_compat(char * compat)
>> +{
>> +    char *str_ptr, *corrected_compat;
>> +    /*
>> +     * process compatibility property string passed by end-user
>> +     * replaces / by , and ; by NUL character
>> +     */
>> +    corrected_compat = g_strdup(compat);
>> +    /*
>> +     * the total length of the string has to include also the last
>> +     * NUL char.
>> +     */
>> +
>> +    str_ptr = corrected_compat;
>> +    while ((str_ptr = strchr(str_ptr, '/')) != NULL) {
>> +        *str_ptr = ',';
>> +    }
>> +
>> +    /* substitute ";" with the NUL char */
>> +    str_ptr = corrected_compat;
>> +    while ((str_ptr = strchr(str_ptr, ';')) != NULL) {
>> +        *str_ptr = '\0';
>> +    }
>> +
>> +    return corrected_compat;
>> +}
>> +
>> +static void wrap_vfio_fdt_add_node(SysBusDevice *sbdev, void *opaque)
>> +{
>> +    PlatformDevtreeData *data = opaque;
>> +    VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
>> +    VFIODevice *vbasedev = &vdev->vbasedev;
>> +    gchar irq_number_prop[8];
>> +    Object *obj = OBJECT(sbdev);
>> +    char *corrected_compat;
>> +    uint64_t irq_number;
>> +    int compat_str_len = strlen(vdev->compat)+1;
>> +    int i;
>> +
>> +    corrected_compat = format_compat(vdev->compat);
>> +    snprintf(vdev->compat, compat_str_len, "%s", corrected_compat);
>> +    g_free(corrected_compat);
>> +
>> +    vfio_fdt_add_device_node(sbdev, opaque);
>> +
>> +    for (i = 0; i < vbasedev->num_irqs; i++) {
>> +        snprintf(irq_number_prop, sizeof(irq_number_prop), "irq[%d]",
>> i);
>> +        irq_number = object_property_get_int(obj, irq_number_prop, NULL)
>> +                                                 + data->irq_start;
>> +        /*
>> +         * for setting irqfd up we must provide the virtual IRQ number
>> +         * which is the sum of irq_start and actual platform bus irq
>> +         * index. At realize point we do not have this info.
>> +         */
>> +        if (vdev->irqfd_allowed) {
>> +            vfio_setup_irqfd(sbdev, i, irq_number);
>> +        }
>> +    }
>> +}
>> +
>> +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/%s@%" PRIx64, parent_node,
>> +                               vbasedev->name,
>> +                               mmio_base);
>> +
>> +    qemu_fdt_add_subnode(fdt, nodename);
>> +
>> +    compat_str_len = strlen(vdev->compat) + 1;
>> +    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);
>> +    }
>> +
>> +    ret = qemu_fdt_setprop_sized_cells_from_array(fdt, nodename, "reg",
>> +                     vbasedev->num_regions*2, reg_attr);
>> +    if (ret < 0) {
>> +        error_report("could not set reg property of node %s", nodename);
>> +    }
>> +
>> +    irq_attr = g_new(uint32_t, vbasedev->num_irqs*3);
>> +
>> +    for (i = 0; i < vbasedev->num_irqs; i++) {
>> +        snprintf(irq_number_prop, sizeof(irq_number_prop), "irq[%d]",
>> i);
>> +        irq_number = object_property_get_int(obj, irq_number_prop, NULL)
>> +                                                 + data->irq_start;
>> +        irq_attr[3*i] = cpu_to_be32(0);
>> +        irq_attr[3*i+1] = cpu_to_be32(irq_number);
>> +        irq_attr[3*i+2] = cpu_to_be32(0x4);
>> +    }
>> +
>> +   ret = qemu_fdt_setprop(fdt, nodename, "interrupts",
>> +                     irq_attr, vbasedev->num_irqs*3*sizeof(uint32_t));
>> +    if (ret < 0) {
>> +        error_report("could not set interrupts property of node %s",
>> +                     nodename);
>> +    }
>> +
>> +    g_free(nodename);
>> +    g_free(irq_attr);
>> +    g_free(reg_attr);
>> +}
>>     int sysbus_device_create_devtree(Object *obj, void *opaque)
>>   {
>> @@ -17,6 +150,11 @@ int sysbus_device_create_devtree(Object *obj, void
>> *opaque)
>>           return object_child_foreach(obj,
>> sysbus_device_create_devtree, data);
>>       }
>>   +    if (object_dynamic_cast(obj, TYPE_VFIO_PLATFORM)) {
> 
> You should only ever check for specific VFIO device types. A generic
> "vfio-platform" device type will not work, since you won't have enough
> auxiliary information once devices become more complicated.

Hi Alex,

I will re-submit this week with calxeda-xgmac derived device back again
and abstract class too.

Thanks

Eric
> 
> 
> Alex
>