| Message ID | 1426785402-2091-8-git-send-email-eric.auger@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Vikram, On 04/25/2015 12:35 AM, Vikram Sethi wrote: > Hi Eric, > > On 03/19/15 12:16, Eric Auger wrote: >> This patch allows the instantiation of the vfio-calxeda-xgmac device >> from the QEMU command line (-device vfio-calxeda-xgmac,host="<device>"). >> >> A specialized device tree node is created for the guest, containing >> compat, dma-coherent, reg and interrupts properties. >> >> Signed-off-by: Eric Auger <eric.auger@linaro.org> >> >> --- >> v10 -> v11: >> - add dma-coherent property to calxeda midway xgmac node (fix) >> - use qemu_fdt_setprop to add reg property instead of >> qemu_fdt_setprop_sized_cells_from_array >> - commit message rewording >> >> 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 | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 72 insertions(+) >> >> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c >> index 3038b94..6e465b2 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,81 @@ typedef struct NodeCreationPair { >> int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque); >> } NodeCreationPair; >> >> +/* Device Specific Code */ >> + >> +/** >> + * add_calxeda_midway_xgmac_fdt_node >> + * >> + * Generates a simple node with following properties: >> + * compatible string, regs, interrupts, dma-coherent >> + */ > Since this function is applicable to any dma-coherent device with > only mmio and irq properties to be exposed, should it be named > something more generic so that it can be reused for other devices? Hi Vikram, well this is a discussion started a while ago. Alex requested the entry point not to be generic. I think when adding new entries in this lookup table we will bring helpers that can be used by several entries to generate most common props (irq, regions, ...). see [RFC PATCH v3 1/3] hw/vfio/sysbus-fdt: helper routines to create fdt nodes (https://lists.gnu.org/archive/html/qemu-devel/2015-02/msg02063.html) >> +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, i, ret = -1; >> + char *nodename; >> + uint32_t *irq_attr, *reg_attr; >> + uint64_t mmio_base, 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); >> + >> + qemu_fdt_setprop(fdt, nodename, "dma-coherent", "", 0); >> + >> + reg_attr = g_new(uint32_t, vbasedev->num_regions*2); >> + for (i = 0; i < vbasedev->num_regions; i++) { >> + mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i); > You are missing the cpu_to_be32 for mmio_base and size. > As a result, a 0x1000 size really gets encoded as 0x100000. > If you have a device with 2 MMIO regions, this results > in the second region getting a wrong starting address in > the guest kernel, which also doesn't match the 2nd stage > mapping done by KVM. With that fix, able to test this function > with a device with 2 regions as well. Thanks for reporting this. This will be corrected in v13 early this week. Best Regards Eric >> + reg_attr[2*i] = (uint32_t)mmio_base; >> + reg_attr[2*i+1] = (uint32_t)memory_region_size(&vdev->regions[i]->mem); >> + } >> + ret = qemu_fdt_setprop(fdt, nodename, "reg", reg_attr, >> + vbasedev->num_regions*2*sizeof(uint32_t)); >> + 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)); >> + 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 >> * >
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c index 3038b94..6e465b2 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,81 @@ typedef struct NodeCreationPair { int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque); } NodeCreationPair; +/* Device Specific Code */ + +/** + * add_calxeda_midway_xgmac_fdt_node + * + * Generates a simple node with following properties: + * compatible string, regs, interrupts, dma-coherent + */ +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, i, ret = -1; + char *nodename; + uint32_t *irq_attr, *reg_attr; + uint64_t mmio_base, 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); + + qemu_fdt_setprop(fdt, nodename, "dma-coherent", "", 0); + + reg_attr = g_new(uint32_t, vbasedev->num_regions*2); + for (i = 0; i < vbasedev->num_regions; i++) { + mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i); + reg_attr[2*i] = (uint32_t)mmio_base; + reg_attr[2*i+1] = (uint32_t)memory_region_size(&vdev->regions[i]->mem); + } + ret = qemu_fdt_setprop(fdt, nodename, "reg", reg_attr, + vbasedev->num_regions*2*sizeof(uint32_t)); + 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)); + 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 *
This patch allows the instantiation of the vfio-calxeda-xgmac device from the QEMU command line (-device vfio-calxeda-xgmac,host="<device>"). A specialized device tree node is created for the guest, containing compat, dma-coherent, reg and interrupts properties. Signed-off-by: Eric Auger <eric.auger@linaro.org> --- v10 -> v11: - add dma-coherent property to calxeda midway xgmac node (fix) - use qemu_fdt_setprop to add reg property instead of qemu_fdt_setprop_sized_cells_from_array - commit message rewording 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 | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)