Alex, On 02/17/2015 12:36 PM, Alex Bennée wrote: > > Eric Auger <eric.auger@xxxxxxxxxx> writes: > >> vfio-calxeda-xgmac now can be instantiated using the -device option. >> The node creation function generates a very basic dt node composed >> of the compat, reg and interrupts properties >> >> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx> >> >> --- >> v8 -> v9: >> - properly free resources in case of errors in >> add_calxeda_midway_xgmac_fdt_node >> >> v7 -> v8: >> - move the add_fdt_node_functions array declaration between the device >> specific code and the generic code to avoid forward declarations of >> decice specific functions >> - rename add_basic_vfio_fdt_node into >> add_calxeda_midway_xgmac_fdt_node >> >> v6 -> v7: >> - compat string re-formatting removed since compat string is not exposed >> anymore as a user option >> - VFIO IRQ kick-off removed from sysbus-fdt and moved to VFIO platform >> device >> --- >> hw/arm/sysbus-fdt.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 83 insertions(+) >> >> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c >> index 3038b94..d4f97f5 100644 >> --- a/hw/arm/sysbus-fdt.c >> +++ b/hw/arm/sysbus-fdt.c >> @@ -26,6 +26,8 @@ >> #include "sysemu/device_tree.h" >> #include "hw/platform-bus.h" >> #include "sysemu/sysemu.h" >> +#include "hw/vfio/vfio-platform.h" >> +#include "hw/vfio/vfio-calxeda-xgmac.h" >> >> /* >> * internal struct that contains the information to create dynamic >> @@ -53,11 +55,92 @@ typedef struct NodeCreationPair { >> int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque); >> } NodeCreationPair; >> >> +/* Device Specific Code */ >> + >> +/** >> + * add_calxeda_midway_xgmac_fdt_node >> + * >> + * Generates a very simple node with following properties: >> + * compatible string, regs, interrupts >> + */ >> +static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque) >> +{ >> + PlatformBusFDTData *data = opaque; >> + PlatformBusDevice *pbus = data->pbus; >> + void *fdt = data->fdt; >> + const char *parent_node = data->pbus_node_name; >> + int compat_str_len; >> + char *nodename; >> + int i, ret = -1; >> + uint32_t *irq_attr; >> + uint64_t *reg_attr; >> + uint64_t mmio_base; >> + uint64_t irq_number; >> + 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++) { >> + mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i); >> + reg_attr[4*i] = 1; >> + reg_attr[4*i+1] = mmio_base; >> + reg_attr[4*i+2] = 1; >> + reg_attr[4*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); > > Could we use qemu_fdt_setprop_sized_cells() like everyone else to hide > the uglyness of the _from_array? Due to the fact I need to handle 'num_regions' regions, I need to setup the array programmatically. I could use qemu_fdt_setprop instead since the cell size always is 1 but I currently do not see how I could easily take benefit from qemu_fdt_setprop_sized_cells. > >> + if (ret) { >> + error_report("could not set reg property of node %s", nodename); >> + goto fail_reg; >> + } >> + >> + irq_attr = g_new(uint32_t, vbasedev->num_irqs*3); >> + >> + for (i = 0; i < vbasedev->num_irqs; i++) { >> + irq_number = platform_bus_get_irqn(pbus, sbdev , i) >> + + 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)); > > Ditto. Here also I need to handle num_irqs. But maybe I misunderstand your comment here. Best Regards Eric > >> + if (ret) { >> + error_report("could not set interrupts property of node %s", >> + nodename); >> + } >> + >> + g_free(irq_attr); >> +fail_reg: >> + g_free(reg_attr); >> + g_free(nodename); >> + return ret; >> +} >> + >> /* list of supported dynamic sysbus devices */ >> static const NodeCreationPair add_fdt_node_functions[] = { >> + {TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node}, >> {"", NULL}, /* last element */ >> }; >> >> +/* Generic Code */ >> + >> /** >> * add_fdt_node - add the device tree node of a dynamic sysbus device >> * > _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm