[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 6/6] hw/arm/sysbus-fdt: enable amd-xgbe dynamic
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 6/6] hw/arm/sysbus-fdt: enable amd-xgbe dynamic instantiation |
Date: |
Fri, 18 Dec 2015 15:05:10 +0000 |
On 17 December 2015 at 12:29, Eric Auger <address@hidden> wrote:
> This patch allows the instantiation of the vfio-amd-xgbe device
> from the QEMU command line (-device vfio-amd-xgbe,host="<device>").
>
> The guest is exposed with a device tree node that combines the description
> of both XGBE and PHY (representation supported from 4.2 onwards kernel):
> Documentation/devicetree/bindings/net/amd-xgbe.txt.
>
> There are 5 register regions, 6 interrupts including 4 optional
> edge-sensitive per-channel interrupts.
>
> Some property values are inherited from host device tree. Host device tree
> must feature a combined XGBE/PHY representation (>= 4.2 host kernel).
>
> 2 clock nodes (dma and ptp) also are created. It is checked those clocks
> are fixed on host side.
>
> Signed-off-by: Eric Auger <address@hidden>
>
> ---
>
> RFC -> v1:
> - use qemu_fdt_getprop with Error **
> - free substrings in sysfs_to_dt_name
> - add some comments related to endianess in add_amd_xgbe_fdt_node
> - reword commit message (dtc binary dependency has disappeared)
> - check the host device has 5 regions meaning this is a combined
> XGBE/PHY device
> ---
> hw/arm/sysbus-fdt.c | 184
> ++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 178 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> index c2d813b..7ec5623 100644
> --- a/hw/arm/sysbus-fdt.c
> +++ b/hw/arm/sysbus-fdt.c
> @@ -22,6 +22,7 @@
> */
>
> #include <libfdt.h>
> +#include <linux/vfio.h>
> #include "hw/arm/sysbus-fdt.h"
> #include "qemu/error-report.h"
> #include "sysemu/device_tree.h"
> @@ -29,6 +30,7 @@
> #include "sysemu/sysemu.h"
> #include "hw/vfio/vfio-platform.h"
> #include "hw/vfio/vfio-calxeda-xgmac.h"
> +#include "hw/vfio/vfio-amd-xgbe.h"
> #include "hw/arm/fdt.h"
>
> /*
> @@ -120,12 +122,9 @@ static HostProperty clock_inherited_properties[] = {
> * @host_phandle: phandle of the clock in host device tree
> * @guest_phandle: phandle to assign to the guest node
> */
> -int fdt_build_clock_node(void *host_fdt, void *guest_fdt,
> - uint32_t host_phandle,
> - uint32_t guest_phandle);
> -int fdt_build_clock_node(void *host_fdt, void *guest_fdt,
> - uint32_t host_phandle,
> - uint32_t guest_phandle)
> +static int fdt_build_clock_node(void *host_fdt, void *guest_fdt,
> + uint32_t host_phandle,
> + uint32_t guest_phandle)
> {
> char node_path[256];
> char *nodename;
> @@ -167,6 +166,22 @@ out:
> return ret;
> }
>
> +/**
> + * sysfs_to_dt_name
> + *
> + * convert the name found in sysfs into the node name
> + * for instance e0900000.xgmac is converted into address@hidden
> + */
> +static char *sysfs_to_dt_name(const char *sysfs_name)
> +{
> + gchar **substrings = g_strsplit(sysfs_name, ".", 2);
> + char *dt_name;
> +
> + dt_name = g_strdup_printf("address@hidden", substrings[1],
> substrings[0]);
> + g_strfreev(substrings);
> + return dt_name;
This will crash if you feed it a string without a "." in it.
A bit more robustness would be a good idea.
> +}
> +
> /* Device Specific Code */
>
> /**
> @@ -234,9 +249,166 @@ fail_reg:
> return ret;
> }
>
> +
> +/* AMD xgbe properties whose values are copied/pasted from host */
> +static HostProperty amd_xgbe_inherited_properties[] = {
> + {"compatible", 0},
> + {"dma-coherent", 1},
> + {"amd,per-channel-interrupt", 1},
> + {"phy-mode", 0},
> + {"mac-address", 1},
> + {"amd,speed-set", 0},
> + {"amd,serdes-blwc", 1},
> + {"amd,serdes-cdr-rate", 1},
> + {"amd,serdes-pq-skew", 1},
> + {"amd,serdes-tx-amp", 1},
> + {"amd,serdes-dfe-tap-config", 1},
> + {"amd,serdes-dfe-tap-enable", 1},
> + {"clock-names", 0},
HostProperty's second field is type 'bool' so these initializers
should be using 'true' and 'false' rather than 1 and 0.
> +};
> +
> +/**
> + * add_amd_xgbe_fdt_node
> + *
> + * Generates the combined xgbe/phy node following kernel >=4.2
> + * binding documentation:
> + * Documentation/devicetree/bindings/net/amd-xgbe.txt:
> + * Also 2 clock nodes are created (dma and ptp)
> + */
> +static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, void *opaque)
> +{
> + PlatformBusFDTData *data = opaque;
> + PlatformBusDevice *pbus = data->pbus;
> + VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
> + VFIODevice *vbasedev = &vdev->vbasedev;
> + VFIOINTp *intp;
> + const char *parent_node = data->pbus_node_name;
> + char *node_path, *nodename, *dt_name;
> + void *guest_fdt = data->fdt, *host_fdt;
> + const void *r;
> + int i, ret = -1, prop_len;
> + uint32_t *irq_attr, *reg_attr, *host_clock_phandles;
> + uint64_t mmio_base, irq_number;
> + uint32_t guest_clock_phandles[2];
> +
> + host_fdt = load_device_tree_from_sysfs();
> + if (!host_fdt) {
> + goto stop;
> + }
> + dt_name = sysfs_to_dt_name(vbasedev->name);
> + ret = qemu_fdt_node_path(host_fdt, dt_name, vdev->compat, &node_path);
> + g_free(dt_name);
> +
> + if (ret) {
> + goto stop;
> + }
> +
> + /* make sure the asssigned device is a combined XGBE/PHY device */
"assigned"
> + if (vbasedev->num_regions != 5) {
> + goto stop;
in this error case will we ever print a useful error message
to the user? (More generally I think the error reporting should
probably pass up Errors and then you can add a hint at the
end when you report it about the host dt blob being wrong
and perhaps how the user might deal with that.)
> + }
> +
> + /* generate nodes for DMA_CLK and PTP_CLK */
> + r = qemu_fdt_getprop(host_fdt, node_path, "clocks",
> + &prop_len, &error_fatal);
> + if (prop_len != 8) {
> + goto stop;
> + }
> + host_clock_phandles = (uint32_t *)r;
> + guest_clock_phandles[0] = qemu_fdt_alloc_phandle(guest_fdt);
> + guest_clock_phandles[1] = qemu_fdt_alloc_phandle(guest_fdt);
> +
> + /**
> + * clock handles fetched from host dt are in be32 layout whereas
> + * rest of the code uses cpu layout. Also guest clock handles are
> + * in cpu layout.
> + */
> + ret = fdt_build_clock_node(host_fdt, guest_fdt,
> + be32_to_cpu(host_clock_phandles[0]),
> + guest_clock_phandles[0]);
> + if (ret) {
> + goto stop;
> + }
> +
> + ret = fdt_build_clock_node(host_fdt, guest_fdt,
> + be32_to_cpu(host_clock_phandles[1]),
> + guest_clock_phandles[1]);
> + if (ret) {
> + goto stop;
> + }
> +
> + /* combined XGBE/PHY node */
> + 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(guest_fdt, nodename);
> +
> + inherit_properties(amd_xgbe_inherited_properties,
> + ARRAY_SIZE(amd_xgbe_inherited_properties),
> + host_fdt, guest_fdt,
> + node_path, nodename);
> +
> + qemu_fdt_setprop_cells(guest_fdt, nodename, "clocks",
> + guest_clock_phandles[0],
> + guest_clock_phandles[1]);
> +
> + 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] = cpu_to_be32(mmio_base);
> + reg_attr[2 * i + 1] = cpu_to_be32(
> + memory_region_size(&vdev->regions[i]->mem));
> + }
> + ret = qemu_fdt_setprop(guest_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(GIC_FDT_IRQ_TYPE_SPI);
> + irq_attr[3 * i + 1] = cpu_to_be32(irq_number);
> + /*
> + * General device interrupt and PCS auto-negociation interrupts are
> + * level-sensitive while the 4 per-channel interrupts are edge
> + * sensitive
> + */
> + QLIST_FOREACH(intp, &vdev->intp_list, next) {
> + if (intp->pin == i) {
> + break;
> + }
> + }
> + if (intp->flags & VFIO_IRQ_INFO_AUTOMASKED) {
> + irq_attr[3 * i + 2] = cpu_to_be32(GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> + } else {
> + irq_attr[3 * i + 2] = cpu_to_be32(GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
> + }
> + }
> + ret = qemu_fdt_setprop(guest_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(host_fdt);
> + g_free(node_path);
> + g_free(irq_attr);
> +fail_reg:
> + g_free(reg_attr);
> + g_free(nodename);
> + return ret;
> +stop:
> + exit(1);
> +}
> +
> /* list of supported dynamic sysbus devices */
> static const NodeCreationPair add_fdt_node_functions[] = {
> {TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
> + {TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node},
> {"", NULL}, /* last element */
> };
thanks
-- PMM
- Re: [Qemu-devel] [PATCH 1/6] hw/vfio/platform: amd-xgbe device, (continued)
- [Qemu-devel] [PATCH 2/6] device_tree: introduce load_device_tree_from_sysfs, Eric Auger, 2015/12/17
- [Qemu-devel] [PATCH 3/6] device_tree: introduce qemu_fdt_node_path, Eric Auger, 2015/12/17
- [Qemu-devel] [PATCH 4/6] device_tree: qemu_fdt_getprop converted to use the error API, Eric Auger, 2015/12/17
- [Qemu-devel] [PATCH 5/6] hw/arm/sysbus-fdt: helpers for clock node generation, Eric Auger, 2015/12/17
- [Qemu-devel] [PATCH 6/6] hw/arm/sysbus-fdt: enable amd-xgbe dynamic instantiation, Eric Auger, 2015/12/17
- Re: [Qemu-devel] [PATCH 6/6] hw/arm/sysbus-fdt: enable amd-xgbe dynamic instantiation,
Peter Maydell <=