| Message ID | 1453130204-655-8-git-send-email-eric.auger@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 18 January 2016 at 15:16, Eric Auger <eric.auger@linaro.org> 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. > > AMD XGBE node creation function has a dependency on vfio Linux header and > more generally node creation function for VFIO platform devices only make > sense with CONFIG_LINUX so let's protect this code with #ifdef CONFIG_LINUX. > > Signed-off-by: Eric Auger <eric.auger@linaro.org> > hw/arm/sysbus-fdt.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 189 insertions(+), 6 deletions(-) I'll let it pass for this patchset, but this file is starting to get big, and probably needs some kind of split into common functions in one file and then a separate file for each pass-through device, if they're all going to require 200-odd lines to deal with. > +/** > + * sysfs_to_dt_name: convert the name found in sysfs into the node name > + * for instance e0900000.xgmac is converted into xgmac@e0900000 > + * @sysfs_name: directory name in sysfs > + * > + * returns the device tree name upon success or NULL in case the sysfs name > + * does not match the expected format > + */ > +static char *sysfs_to_dt_name(const char *sysfs_name) > +{ > + gchar **substrings = g_strsplit(sysfs_name, ".", 2); > + char *dt_name = NULL; > + > + if (!substrings || !substrings[1] || !substrings[0]) { We should check substrings[0] before substrings[1], as otherwise if we're passed the empty string we'll index off the end of the vector. > + goto out; > + } > + dt_name = g_strdup_printf("%s@%s", substrings[1], substrings[0]); > +out: > + g_strfreev(substrings); > + return dt_name; > +} > + > /* Device Specific Code */ > > /** > @@ -243,9 +269,166 @@ fail_reg: > return ret; > } > > + > +/* AMD xgbe properties whose values are copied/pasted from host */ > +static HostProperty amd_xgbe_copied_properties[] = { > + {"compatible", false}, > + {"dma-coherent", true}, > + {"amd,per-channel-interrupt", true}, > + {"phy-mode", false}, > + {"mac-address", true}, > + {"amd,speed-set", false}, > + {"amd,serdes-blwc", true}, > + {"amd,serdes-cdr-rate", true}, > + {"amd,serdes-pq-skew", true}, > + {"amd,serdes-tx-amp", true}, > + {"amd,serdes-dfe-tap-config", true}, > + {"amd,serdes-dfe-tap-enable", true}, > + {"clock-names", false}, > +}; > + > +/** > + * 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) > + * > + * self-asserts in case of error "asserts". > + 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 "negotiation" > + * 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); > + } > + } > + qemu_fdt_setprop(guest_fdt, nodename, "interrupts", > + irq_attr, vbasedev->num_irqs * 3 * sizeof(uint32_t)); > + > + g_free(host_fdt); > + g_strfreev(node_path); > + g_free(irq_attr); > + g_free(reg_attr); > + g_free(nodename); > + return 0; > +} > + > +#endif /* CONFIG_LINUX */ > + > /* list of supported dynamic sysbus devices */ > static const NodeCreationPair add_fdt_node_functions[] = { > +#ifdef CONFIG_LINUX > {TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node}, > + {TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node}, > +#endif > {"", NULL}, /* last element */ > }; thanks -- PMM
Hi Peter, On 01/25/2016 03:33 PM, Peter Maydell wrote: > On 18 January 2016 at 15:16, Eric Auger <eric.auger@linaro.org> 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. >> >> AMD XGBE node creation function has a dependency on vfio Linux header and >> more generally node creation function for VFIO platform devices only make >> sense with CONFIG_LINUX so let's protect this code with #ifdef CONFIG_LINUX. >> >> Signed-off-by: Eric Auger <eric.auger@linaro.org> > >> hw/arm/sysbus-fdt.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 189 insertions(+), 6 deletions(-) > > I'll let it pass for this patchset, but this file is starting to > get big, and probably needs some kind of split into common functions > in one file and then a separate file for each pass-through device, > if they're all going to require 200-odd lines to deal with. That's understood. If you don't mind i would rather introduce that move in a separate series then. Thanks Eric > >> +/** >> + * sysfs_to_dt_name: convert the name found in sysfs into the node name >> + * for instance e0900000.xgmac is converted into xgmac@e0900000 >> + * @sysfs_name: directory name in sysfs >> + * >> + * returns the device tree name upon success or NULL in case the sysfs name >> + * does not match the expected format >> + */ >> +static char *sysfs_to_dt_name(const char *sysfs_name) >> +{ >> + gchar **substrings = g_strsplit(sysfs_name, ".", 2); >> + char *dt_name = NULL; >> + >> + if (!substrings || !substrings[1] || !substrings[0]) { > > We should check substrings[0] before substrings[1], as otherwise > if we're passed the empty string we'll index off the end of the > vector. > >> + goto out; >> + } >> + dt_name = g_strdup_printf("%s@%s", substrings[1], substrings[0]); >> +out: >> + g_strfreev(substrings); >> + return dt_name; >> +} >> + >> /* Device Specific Code */ >> >> /** >> @@ -243,9 +269,166 @@ fail_reg: >> return ret; >> } >> >> + >> +/* AMD xgbe properties whose values are copied/pasted from host */ >> +static HostProperty amd_xgbe_copied_properties[] = { >> + {"compatible", false}, >> + {"dma-coherent", true}, >> + {"amd,per-channel-interrupt", true}, >> + {"phy-mode", false}, >> + {"mac-address", true}, >> + {"amd,speed-set", false}, >> + {"amd,serdes-blwc", true}, >> + {"amd,serdes-cdr-rate", true}, >> + {"amd,serdes-pq-skew", true}, >> + {"amd,serdes-tx-amp", true}, >> + {"amd,serdes-dfe-tap-config", true}, >> + {"amd,serdes-dfe-tap-enable", true}, >> + {"clock-names", false}, >> +}; >> + >> +/** >> + * 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) >> + * >> + * self-asserts in case of error > > "asserts". > >> + 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 > > "negotiation" > >> + * 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); >> + } >> + } >> + qemu_fdt_setprop(guest_fdt, nodename, "interrupts", >> + irq_attr, vbasedev->num_irqs * 3 * sizeof(uint32_t)); >> + >> + g_free(host_fdt); >> + g_strfreev(node_path); >> + g_free(irq_attr); >> + g_free(reg_attr); >> + g_free(nodename); >> + return 0; >> +} >> + >> +#endif /* CONFIG_LINUX */ >> + >> /* list of supported dynamic sysbus devices */ >> static const NodeCreationPair add_fdt_node_functions[] = { >> +#ifdef CONFIG_LINUX >> {TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node}, >> + {TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node}, >> +#endif >> {"", NULL}, /* last element */ >> }; > > thanks > -- PMM >
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c index d85c9e6..bf6ec24 100644 --- a/hw/arm/sysbus-fdt.c +++ b/hw/arm/sysbus-fdt.c @@ -22,6 +22,10 @@ */ #include <libfdt.h> +#include "qemu-common.h" +#ifdef CONFIG_LINUX +#include <linux/vfio.h> +#endif #include "hw/arm/sysbus-fdt.h" #include "qemu/error-report.h" #include "sysemu/device_tree.h" @@ -29,6 +33,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" /* @@ -64,6 +69,8 @@ typedef struct HostProperty { bool optional; } HostProperty; +#ifdef CONFIG_LINUX + /** * copy_properties_from_host * @@ -126,12 +133,9 @@ static HostProperty clock_copied_properties[] = { * @host_phandle: phandle of the clock in host device tree * @guest_phandle: phandle to assign to the guest node */ -void fdt_build_clock_node(void *host_fdt, void *guest_fdt, - uint32_t host_phandle, - uint32_t guest_phandle); -void fdt_build_clock_node(void *host_fdt, void *guest_fdt, - uint32_t host_phandle, - uint32_t guest_phandle) +static void fdt_build_clock_node(void *host_fdt, void *guest_fdt, + uint32_t host_phandle, + uint32_t guest_phandle) { char *node_path = NULL; char *nodename; @@ -176,6 +180,28 @@ void fdt_build_clock_node(void *host_fdt, void *guest_fdt, g_free(node_path); } +/** + * sysfs_to_dt_name: convert the name found in sysfs into the node name + * for instance e0900000.xgmac is converted into xgmac@e0900000 + * @sysfs_name: directory name in sysfs + * + * returns the device tree name upon success or NULL in case the sysfs name + * does not match the expected format + */ +static char *sysfs_to_dt_name(const char *sysfs_name) +{ + gchar **substrings = g_strsplit(sysfs_name, ".", 2); + char *dt_name = NULL; + + if (!substrings || !substrings[1] || !substrings[0]) { + goto out; + } + dt_name = g_strdup_printf("%s@%s", substrings[1], substrings[0]); +out: + g_strfreev(substrings); + return dt_name; +} + /* Device Specific Code */ /** @@ -243,9 +269,166 @@ fail_reg: return ret; } + +/* AMD xgbe properties whose values are copied/pasted from host */ +static HostProperty amd_xgbe_copied_properties[] = { + {"compatible", false}, + {"dma-coherent", true}, + {"amd,per-channel-interrupt", true}, + {"phy-mode", false}, + {"mac-address", true}, + {"amd,speed-set", false}, + {"amd,serdes-blwc", true}, + {"amd,serdes-cdr-rate", true}, + {"amd,serdes-pq-skew", true}, + {"amd,serdes-tx-amp", true}, + {"amd,serdes-dfe-tap-config", true}, + {"amd,serdes-dfe-tap-enable", true}, + {"clock-names", false}, +}; + +/** + * 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) + * + * self-asserts in case of error + */ +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, 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(); + + dt_name = sysfs_to_dt_name(vbasedev->name); + if (!dt_name) { + error_setg(&error_fatal, "%s incorrect sysfs device name %s", + __func__, vbasedev->name); + } + node_path = qemu_fdt_node_path(host_fdt, dt_name, vdev->compat, + &error_fatal); + if (!node_path[0]) { + error_setg(&error_fatal, "%s unable to retrieve node path for %s/%s", + __func__, dt_name, vdev->compat); + } + + if (node_path[1]) { + error_setg(&error_fatal, "%s more than one node matching %s/%s!", + __func__, dt_name, vdev->compat); + } + + g_free(dt_name); + + if (vbasedev->num_regions != 5) { + error_setg(&error_fatal, "%s Does the host dt node combine XGBE/PHY?", + __func__); + } + + /* generate nodes for DMA_CLK and PTP_CLK */ + r = qemu_fdt_getprop(host_fdt, node_path[0], "clocks", + &prop_len, &error_fatal); + if (prop_len != 8) { + error_setg(&error_fatal, "%s clocks property should contain 2 handles", + __func__); + } + 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. + */ + fdt_build_clock_node(host_fdt, guest_fdt, + be32_to_cpu(host_clock_phandles[0]), + guest_clock_phandles[0]); + + fdt_build_clock_node(host_fdt, guest_fdt, + be32_to_cpu(host_clock_phandles[1]), + guest_clock_phandles[1]); + + /* combined XGBE/PHY node */ + mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, 0); + nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node, + vbasedev->name, mmio_base); + qemu_fdt_add_subnode(guest_fdt, nodename); + + copy_properties_from_host(amd_xgbe_copied_properties, + ARRAY_SIZE(amd_xgbe_copied_properties), + host_fdt, guest_fdt, + node_path[0], 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)); + } + qemu_fdt_setprop(guest_fdt, nodename, "reg", reg_attr, + vbasedev->num_regions * 2 * sizeof(uint32_t)); + + 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); + } + } + qemu_fdt_setprop(guest_fdt, nodename, "interrupts", + irq_attr, vbasedev->num_irqs * 3 * sizeof(uint32_t)); + + g_free(host_fdt); + g_strfreev(node_path); + g_free(irq_attr); + g_free(reg_attr); + g_free(nodename); + return 0; +} + +#endif /* CONFIG_LINUX */ + /* list of supported dynamic sysbus devices */ static const NodeCreationPair add_fdt_node_functions[] = { +#ifdef CONFIG_LINUX {TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node}, + {TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node}, +#endif {"", NULL}, /* last element */ };
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. AMD XGBE node creation function has a dependency on vfio Linux header and more generally node creation function for VFIO platform devices only make sense with CONFIG_LINUX so let's protect this code with #ifdef CONFIG_LINUX. Signed-off-by: Eric Auger <eric.auger@linaro.org> --- v4 -> v5: - use new proto for qemu_fdt_node_path, check there is only 1 node matching the name/compat. - renamed inherit_properties* v3 -> v4: - add_amd_xgbe_fdt_node explicitly returns 0. Reword comment to emphasize the fact the function self-asserts in case of error v1 -> v2: - add CONFIG_LINUX protection - improves robustness in sysfs_to_dt_name - output messages to end-user on misc failures and self-exits: error management becomes more violent than before assuming if the end-user wants passthrough we must exit if the device cannot be instantiated - fix misc style issues - remove qemu_fdt_setprop returned value check since it self-asserts 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 | 195 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 189 insertions(+), 6 deletions(-)