[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RESEND PATCH v16 1/6] hw/arm/sysbus-fdt: enable vfio-c
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [RESEND PATCH v16 1/6] hw/arm/sysbus-fdt: enable vfio-calxeda-xgmac dynamic instantiation |
Date: |
Tue, 16 Jun 2015 09:29:09 +0100 |
On 15 June 2015 at 17:33, Eric Auger <address@hidden> 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 <address@hidden>
>
Minor nits:
> +/* 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;
> +
> + mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
> + 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;
> + 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);
You should have spaces around the '*' operator here (and in a
couple of other places below).
> + for (i = 0; i < vbasedev->num_regions; i++) {
> + mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i);
> + reg_attr[2*i] = cpu_to_be32(mmio_base);
> + reg_attr[2*i+1] = cpu_to_be32(
> + 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);
In virt.c the magic numbers in cells 0 and 2 have #defines
(GIC_FDT_IRQ_TYPE_SPI and GIC_FDT_IRQ_FLAGS_LEVEL_HI).
(1) maybe we should pull those out into a header so you can use them here
(2) is this device really using a level-triggered interrupt?
> + }
> + 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 */
> };
This file could get big if we have a lot of platform passthrough
devices, but let's hope we don't...
Otherwise:
Acked-by: Peter Maydell <address@hidden>
-- PMM