| Message ID | 1404251378-5242-7-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
On Tue, 2014-07-01 at 23:49 +0200, Alexander Graf wrote: > This patch adds support to expose eTSEC devices in the dynamically created > guest facing device tree. This allows us to expose eTSEC devices into guests > without changes in the machine file. > > Because we can now tell the guest about eTSEC devices this patch allows the > user to specify eTSEC devices via -device at all. > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > hw/ppc/e500.c | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c > index bf704b0..bebff6f 100644 > --- a/hw/ppc/e500.c > +++ b/hw/ppc/e500.c > @@ -37,6 +37,7 @@ > #include "qemu/host-utils.h" > #include "hw/pci-host/ppce500.h" > #include "qemu/error-report.h" > +#include "hw/net/fsl_etsec/etsec.h" > > #define EPAPR_MAGIC (0x45504150) > #define BINARY_DEVICE_TREE_FILE "mpc8544ds.dtb" > @@ -139,6 +140,34 @@ typedef struct PlatformDevtreeData { > int id; > } PlatformDevtreeData; > > +static int create_devtree_etsec(eTSEC *etsec, PlatformDevtreeData *data) > +{ > + SysBusDevice *sbdev = &etsec->busdev; > + gchar *node = g_strdup_printf("/platform/ethernet@%d", data->id); The unit address is supposed to match reg. It's not an arbitrary disambiguator. > + gchar *group = g_strdup_printf("%s/queue-group", node); > + void *fdt = data->fdt; > + > + qemu_fdt_add_subnode(fdt, node); > + qemu_fdt_setprop_string(fdt, node, "device_type", "network"); > + qemu_fdt_setprop_string(fdt, node, "compatible", "fsl,etsec2"); > + qemu_fdt_setprop_string(fdt, node, "model", "eTSEC"); > + qemu_fdt_setprop(fdt, node, "local-mac-address", etsec->conf.macaddr.a, 6); > + qemu_fdt_setprop_cells(fdt, node, "fixed-link", 0, 1, 1000, 0, 0); > + > + qemu_fdt_add_subnode(fdt, group); > + qemu_fdt_setprop_cells(fdt, group, "reg", sbdev->user_mmios[0], 0x1000); > + qemu_fdt_setprop_phandle(fdt, group, "interrupt-parent", data->mpic); Why not do interrupt-parent in the parent node, or top of tree? > + qemu_fdt_setprop_cells(fdt, group, "interrupts", > + data->irq_start + sbdev->user_irqs[0], 0x0, > + data->irq_start + sbdev->user_irqs[1], 0x0, > + data->irq_start + sbdev->user_irqs[2], 0x0); Are we still using two-cell interrupt specifiers? If so, we should switch before the assumption gets encoded into random device files. Also, why are these interrupts edge triggered? -Scott
On 02.07.14 00:56, Scott Wood wrote: > On Tue, 2014-07-01 at 23:49 +0200, Alexander Graf wrote: >> This patch adds support to expose eTSEC devices in the dynamically created >> guest facing device tree. This allows us to expose eTSEC devices into guests >> without changes in the machine file. >> >> Because we can now tell the guest about eTSEC devices this patch allows the >> user to specify eTSEC devices via -device at all. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> hw/ppc/e500.c | 34 ++++++++++++++++++++++++++++++++++ >> 1 file changed, 34 insertions(+) >> >> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c >> index bf704b0..bebff6f 100644 >> --- a/hw/ppc/e500.c >> +++ b/hw/ppc/e500.c >> @@ -37,6 +37,7 @@ >> #include "qemu/host-utils.h" >> #include "hw/pci-host/ppce500.h" >> #include "qemu/error-report.h" >> +#include "hw/net/fsl_etsec/etsec.h" >> >> #define EPAPR_MAGIC (0x45504150) >> #define BINARY_DEVICE_TREE_FILE "mpc8544ds.dtb" >> @@ -139,6 +140,34 @@ typedef struct PlatformDevtreeData { >> int id; >> } PlatformDevtreeData; >> >> +static int create_devtree_etsec(eTSEC *etsec, PlatformDevtreeData *data) >> +{ >> + SysBusDevice *sbdev = &etsec->busdev; >> + gchar *node = g_strdup_printf("/platform/ethernet@%d", data->id); > The unit address is supposed to match reg. It's not an arbitrary > disambiguator. So what do we do in case we don't have any reg, but only an IRQ line? Oh well - I guess we can cross that line when we get to it. > >> + gchar *group = g_strdup_printf("%s/queue-group", node); >> + void *fdt = data->fdt; >> + >> + qemu_fdt_add_subnode(fdt, node); >> + qemu_fdt_setprop_string(fdt, node, "device_type", "network"); >> + qemu_fdt_setprop_string(fdt, node, "compatible", "fsl,etsec2"); >> + qemu_fdt_setprop_string(fdt, node, "model", "eTSEC"); >> + qemu_fdt_setprop(fdt, node, "local-mac-address", etsec->conf.macaddr.a, 6); >> + qemu_fdt_setprop_cells(fdt, node, "fixed-link", 0, 1, 1000, 0, 0); >> + >> + qemu_fdt_add_subnode(fdt, group); >> + qemu_fdt_setprop_cells(fdt, group, "reg", sbdev->user_mmios[0], 0x1000); >> + qemu_fdt_setprop_phandle(fdt, group, "interrupt-parent", data->mpic); > Why not do interrupt-parent in the parent node, or top of tree? Parent sounds appealing :). In fact, it's already there - this copy is simply useless. > >> + qemu_fdt_setprop_cells(fdt, group, "interrupts", >> + data->irq_start + sbdev->user_irqs[0], 0x0, >> + data->irq_start + sbdev->user_irqs[1], 0x0, >> + data->irq_start + sbdev->user_irqs[2], 0x0); > Are we still using two-cell interrupt specifiers? If so, we should > switch before the assumption gets encoded into random device files. Random device files should never get any device tree bits encoded. Device tree generation is responsibility of the machine file. So we can easily convert the whole thing to the 4-cell when we start to support different interrupt types :) > Also, why are these interrupts edge triggered? Good catch - they're 0x2 on real hardware. Alex
On Wed, 2014-07-02 at 19:24 +0200, Alexander Graf wrote: > On 02.07.14 00:56, Scott Wood wrote: > > On Tue, 2014-07-01 at 23:49 +0200, Alexander Graf wrote: > >> This patch adds support to expose eTSEC devices in the dynamically created > >> guest facing device tree. This allows us to expose eTSEC devices into guests > >> without changes in the machine file. > >> > >> Because we can now tell the guest about eTSEC devices this patch allows the > >> user to specify eTSEC devices via -device at all. > >> > >> Signed-off-by: Alexander Graf <agraf@suse.de> > >> --- > >> hw/ppc/e500.c | 34 ++++++++++++++++++++++++++++++++++ > >> 1 file changed, 34 insertions(+) > >> > >> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c > >> index bf704b0..bebff6f 100644 > >> --- a/hw/ppc/e500.c > >> +++ b/hw/ppc/e500.c > >> @@ -37,6 +37,7 @@ > >> #include "qemu/host-utils.h" > >> #include "hw/pci-host/ppce500.h" > >> #include "qemu/error-report.h" > >> +#include "hw/net/fsl_etsec/etsec.h" > >> > >> #define EPAPR_MAGIC (0x45504150) > >> #define BINARY_DEVICE_TREE_FILE "mpc8544ds.dtb" > >> @@ -139,6 +140,34 @@ typedef struct PlatformDevtreeData { > >> int id; > >> } PlatformDevtreeData; > >> > >> +static int create_devtree_etsec(eTSEC *etsec, PlatformDevtreeData *data) > >> +{ > >> + SysBusDevice *sbdev = &etsec->busdev; > >> + gchar *node = g_strdup_printf("/platform/ethernet@%d", data->id); > > The unit address is supposed to match reg. It's not an arbitrary > > disambiguator. > > So what do we do in case we don't have any reg, but only an IRQ line? Oh > well - I guess we can cross that line when we get to it. To be theoretically correct (i.e. something that wouldn't break if used in a real Open Firmware) you'd either leave out the unit address and put the disambiguation directly in the name, or have a zero-length reg that corresponds to ranges or a child node's reg. If you just want to match what we currently do in the real hardware fdt, use the reg of the first group node. > >> + qemu_fdt_setprop_cells(fdt, group, "interrupts", > >> + data->irq_start + sbdev->user_irqs[0], 0x0, > >> + data->irq_start + sbdev->user_irqs[1], 0x0, > >> + data->irq_start + sbdev->user_irqs[2], 0x0); > > Are we still using two-cell interrupt specifiers? If so, we should > > switch before the assumption gets encoded into random device files. > > Random device files should never get any device tree bits encoded. > Device tree generation is responsibility of the machine file. Sigh. I missed that this is in e500.c rather than the eTSEC file. This approach will not scale if we ever have multiple platforms wanting to create device trees with overlap in the devices they want to describe. -Scott
On 02.07.14 19:32, Scott Wood wrote: > On Wed, 2014-07-02 at 19:24 +0200, Alexander Graf wrote: >> On 02.07.14 00:56, Scott Wood wrote: >>> On Tue, 2014-07-01 at 23:49 +0200, Alexander Graf wrote: >>>> This patch adds support to expose eTSEC devices in the dynamically created >>>> guest facing device tree. This allows us to expose eTSEC devices into guests >>>> without changes in the machine file. >>>> >>>> Because we can now tell the guest about eTSEC devices this patch allows the >>>> user to specify eTSEC devices via -device at all. >>>> >>>> Signed-off-by: Alexander Graf <agraf@suse.de> >>>> --- >>>> hw/ppc/e500.c | 34 ++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 34 insertions(+) >>>> >>>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c >>>> index bf704b0..bebff6f 100644 >>>> --- a/hw/ppc/e500.c >>>> +++ b/hw/ppc/e500.c >>>> @@ -37,6 +37,7 @@ >>>> #include "qemu/host-utils.h" >>>> #include "hw/pci-host/ppce500.h" >>>> #include "qemu/error-report.h" >>>> +#include "hw/net/fsl_etsec/etsec.h" >>>> >>>> #define EPAPR_MAGIC (0x45504150) >>>> #define BINARY_DEVICE_TREE_FILE "mpc8544ds.dtb" >>>> @@ -139,6 +140,34 @@ typedef struct PlatformDevtreeData { >>>> int id; >>>> } PlatformDevtreeData; >>>> >>>> +static int create_devtree_etsec(eTSEC *etsec, PlatformDevtreeData *data) >>>> +{ >>>> + SysBusDevice *sbdev = &etsec->busdev; >>>> + gchar *node = g_strdup_printf("/platform/ethernet@%d", data->id); >>> The unit address is supposed to match reg. It's not an arbitrary >>> disambiguator. >> So what do we do in case we don't have any reg, but only an IRQ line? Oh >> well - I guess we can cross that line when we get to it. > To be theoretically correct (i.e. something that wouldn't break if used > in a real Open Firmware) you'd either leave out the unit address and put > the disambiguation directly in the name, or have a zero-length reg that > corresponds to ranges or a child node's reg. > > If you just want to match what we currently do in the real hardware fdt, > use the reg of the first group node. > >>>> + qemu_fdt_setprop_cells(fdt, group, "interrupts", >>>> + data->irq_start + sbdev->user_irqs[0], 0x0, >>>> + data->irq_start + sbdev->user_irqs[1], 0x0, >>>> + data->irq_start + sbdev->user_irqs[2], 0x0); >>> Are we still using two-cell interrupt specifiers? If so, we should >>> switch before the assumption gets encoded into random device files. >> Random device files should never get any device tree bits encoded. >> Device tree generation is responsibility of the machine file. > Sigh. I missed that this is in e500.c rather than the eTSEC file. This > approach will not scale if we ever have multiple platforms wanting to > create device trees with overlap in the devices they want to describe. It has to - device trees differ too much between architectures (and potentially even machines) to have them reasonably live in device files. Alex
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index bf704b0..bebff6f 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -37,6 +37,7 @@ #include "qemu/host-utils.h" #include "hw/pci-host/ppce500.h" #include "qemu/error-report.h" +#include "hw/net/fsl_etsec/etsec.h" #define EPAPR_MAGIC (0x45504150) #define BINARY_DEVICE_TREE_FILE "mpc8544ds.dtb" @@ -139,6 +140,34 @@ typedef struct PlatformDevtreeData { int id; } PlatformDevtreeData; +static int create_devtree_etsec(eTSEC *etsec, PlatformDevtreeData *data) +{ + SysBusDevice *sbdev = &etsec->busdev; + gchar *node = g_strdup_printf("/platform/ethernet@%d", data->id); + gchar *group = g_strdup_printf("%s/queue-group", node); + void *fdt = data->fdt; + + qemu_fdt_add_subnode(fdt, node); + qemu_fdt_setprop_string(fdt, node, "device_type", "network"); + qemu_fdt_setprop_string(fdt, node, "compatible", "fsl,etsec2"); + qemu_fdt_setprop_string(fdt, node, "model", "eTSEC"); + qemu_fdt_setprop(fdt, node, "local-mac-address", etsec->conf.macaddr.a, 6); + qemu_fdt_setprop_cells(fdt, node, "fixed-link", 0, 1, 1000, 0, 0); + + qemu_fdt_add_subnode(fdt, group); + qemu_fdt_setprop_cells(fdt, group, "reg", sbdev->user_mmios[0], 0x1000); + qemu_fdt_setprop_phandle(fdt, group, "interrupt-parent", data->mpic); + qemu_fdt_setprop_cells(fdt, group, "interrupts", + data->irq_start + sbdev->user_irqs[0], 0x0, + data->irq_start + sbdev->user_irqs[1], 0x0, + data->irq_start + sbdev->user_irqs[2], 0x0); + + g_free(node); + g_free(group); + + return 0; +} + static int sysbus_device_create_devtree(Object *obj, void *opaque) { PlatformDevtreeData *data = opaque; @@ -154,6 +183,11 @@ static int sysbus_device_create_devtree(Object *obj, void *opaque) return object_child_foreach(obj, sysbus_device_create_devtree, data); } + if (object_dynamic_cast(obj, TYPE_ETSEC_COMMON)) { + create_devtree_etsec(ETSEC_COMMON(dev), data); + matched = true; + } + if (matched) { data->id++; } else {
This patch adds support to expose eTSEC devices in the dynamically created guest facing device tree. This allows us to expose eTSEC devices into guests without changes in the machine file. Because we can now tell the guest about eTSEC devices this patch allows the user to specify eTSEC devices via -device at all. Signed-off-by: Alexander Graf <agraf@suse.de> --- hw/ppc/e500.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)