* [Qemu-devel] [PATCH 1/7] hw/misc/platform_devices: helpers for dynamic instantiation of platform devices
2014-07-07 7:08 [Qemu-devel] [PATCH 0/7] machvirt dynamic sysbus device instantiation Eric Auger
@ 2014-07-07 7:08 ` Eric Auger
2014-07-08 13:43 ` Alexander Graf
2014-07-07 7:08 ` [Qemu-devel] [PATCH 2/7] hw/arm/boot: load_dtb becomes non static Eric Auger
` (5 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Eric Auger @ 2014-07-07 7:08 UTC (permalink / raw)
To: eric.auger, christoffer.dall, qemu-devel, kim.phillips, a.rigo
Cc: peter.maydell, eric.auger, patches, agraf, stuart.yoder,
alex.williamson, a.motakis, kvmarm
This new module implements routines which help in dynamic instantiation
of sysbus devices. Machine files can use those generic routines.
---
Dynamic sysbus device allocation fully written by Alex Graf.
[Eric Auger]
Those functions were initially in ppc e500 machine file. Now moved to a
separate module.
PPCE500Params is replaced by a generic struct named PlatformParams
Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
hw/misc/Makefile.objs | 1 +
hw/misc/platform_devices.c | 217 +++++++++++++++++++++++++++++++++++++
include/hw/misc/platform_devices.h | 61 +++++++++++
3 files changed, 279 insertions(+)
create mode 100644 hw/misc/platform_devices.c
create mode 100644 include/hw/misc/platform_devices.h
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index e47fea8..d081606 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -40,3 +40,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
obj-$(CONFIG_ZYNQ) += zynq_slcr.o
obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-y += platform_devices.o
diff --git a/hw/misc/platform_devices.c b/hw/misc/platform_devices.c
new file mode 100644
index 0000000..96ab272
--- /dev/null
+++ b/hw/misc/platform_devices.c
@@ -0,0 +1,217 @@
+#include "hw/misc/platform_devices.h"
+#include "hw/sysbus.h"
+#include "qemu/error-report.h"
+
+#define PAGE_SHIFT 12
+
+int sysbus_device_create_devtree(Object *obj, void *opaque)
+{
+ PlatformDevtreeData *data = opaque;
+ Object *dev;
+ SysBusDevice *sbdev;
+ bool matched = false;
+
+ dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
+ sbdev = (SysBusDevice *)dev;
+
+ if (!sbdev) {
+ /* Container, traverse it for children */
+ return object_child_foreach(obj, sysbus_device_create_devtree, data);
+ }
+
+ if (!matched) {
+ error_report("Device %s is not supported by this machine yet.",
+ qdev_fw_name(DEVICE(dev)));
+ exit(1);
+ }
+
+ return 0;
+}
+
+void platform_bus_create_devtree(PlatformParams *params, void *fdt,
+ const char *mpic)
+{
+ gchar *node = g_strdup_printf("/platform@%"PRIx64,
+ params->platform_bus_base);
+ const char platcomp[] = "qemu,platform\0simple-bus";
+ PlatformDevtreeData data;
+ Object *container;
+ uint64_t addr = params->platform_bus_base;
+ uint64_t size = params->platform_bus_size;
+ int irq_start = params->platform_bus_first_irq;
+
+ /* Create a /platform node that we can put all devices into */
+
+ qemu_fdt_add_subnode(fdt, node);
+ qemu_fdt_setprop(fdt, node, "compatible", platcomp, sizeof(platcomp));
+
+ /* Our platform bus region is less than 32bit big, so 1 cell is enough for
+ address and size */
+ qemu_fdt_setprop_cells(fdt, node, "#size-cells", 1);
+ qemu_fdt_setprop_cells(fdt, node, "#address-cells", 1);
+ qemu_fdt_setprop_cells(fdt, node, "ranges", 0, addr >> 32, addr, size);
+
+ qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", mpic);
+
+ /* Loop through all devices and create nodes for known ones */
+ data.fdt = fdt;
+ data.mpic = mpic;
+ data.irq_start = irq_start;
+ data.node = node;
+
+ container = container_get(qdev_get_machine(), "/peripheral");
+ sysbus_device_create_devtree(container, &data);
+ container = container_get(qdev_get_machine(), "/peripheral-anon");
+ sysbus_device_create_devtree(container, &data);
+
+ g_free(node);
+}
+
+int platform_bus_map_irq(PlatformParams *params, SysBusDevice *sbdev,
+ int n, unsigned long *used_irqs,
+ qemu_irq *platform_irqs)
+{
+ int max_irqs = params->platform_bus_num_irqs;
+ char *prop = g_strdup_printf("irq[%d]", n);
+ int irqn = object_property_get_int(OBJECT(sbdev), prop, NULL);
+
+ if (irqn == SYSBUS_DYNAMIC) {
+ /* Find the first available IRQ */
+ irqn = find_first_zero_bit(used_irqs, max_irqs);
+ }
+
+ if ((irqn >= max_irqs) || test_and_set_bit(irqn, used_irqs)) {
+ hw_error("IRQ %d is already allocated or no free IRQ left", irqn);
+ }
+
+ sysbus_connect_irq(sbdev, n, platform_irqs[irqn]);
+ object_property_set_int(OBJECT(sbdev), irqn, prop, NULL);
+
+ g_free(prop);
+ return 0;
+}
+
+int platform_bus_map_mmio(PlatformParams *params, SysBusDevice *sbdev,
+ int n, unsigned long *used_mem,
+ MemoryRegion *pmem)
+{
+ MemoryRegion *device_mem = sbdev->mmio[n].memory;
+ uint64_t size = memory_region_size(device_mem);
+ uint64_t page_size = (1 << PAGE_SHIFT);
+ uint64_t page_mask = page_size - 1;
+ uint64_t size_pages = (size + page_mask) >> PAGE_SHIFT;
+ uint64_t max_size = params->platform_bus_size;
+ uint64_t max_pages = max_size >> PAGE_SHIFT;
+ char *prop = g_strdup_printf("mmio[%d]", n);
+ hwaddr addr = object_property_get_int(OBJECT(sbdev), prop, NULL);
+ int page;
+ int i;
+
+ page = addr >> PAGE_SHIFT;
+ if (addr == SYSBUS_DYNAMIC) {
+ uint64_t size_pages_align;
+
+ /* Align the region to at least its own size granularity */
+ if (is_power_of_2(size_pages)) {
+ size_pages_align = size_pages;
+ } else {
+ size_pages_align = pow2floor(size_pages) << 1;
+ }
+
+ /* Find the first available region that fits */
+ page = bitmap_find_next_zero_area(used_mem, max_pages, 0, size_pages,
+ size_pages_align);
+
+ addr = (uint64_t)page << PAGE_SHIFT;
+ }
+
+ if (page >= max_pages || test_bit(page, used_mem) ||
+ (find_next_bit(used_mem, max_pages, page) < size_pages)) {
+ hw_error("Memory [%"PRIx64":%"PRIx64" is already allocated or "
+ "no slot left", addr, size);
+ }
+
+ for (i = page; i < (page + size_pages); i++) {
+ set_bit(i, used_mem);
+ }
+
+ memory_region_add_subregion(pmem, addr, device_mem);
+ sbdev->mmio[n].addr = addr;
+ object_property_set_int(OBJECT(sbdev), addr, prop, NULL);
+
+ g_free(prop);
+ return 0;
+}
+
+int sysbus_device_check(Object *obj, void *opaque)
+{
+ PlatformBusInitData *init = opaque;
+ Object *dev;
+ SysBusDevice *sbdev;
+ int i;
+
+ dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
+ sbdev = (SysBusDevice *)dev;
+
+ if (!sbdev) {
+ /* Container, traverse it for children */
+ return object_child_foreach(obj, sysbus_device_check, opaque);
+ }
+
+ /* Connect sysbus device to virtual platform bus */
+ for (i = 0; i < sbdev->num_irq; i++) {
+ if (!sbdev->irqp[i]) {
+ /* This IRQ is an incoming IRQ, we can't wire those here */
+ continue;
+ }
+ platform_bus_map_irq(init->params, sbdev, i,
+ init->used_irqs, init->irqs);
+ }
+
+ for (i = 0; i < sbdev->num_mmio; i++) {
+ platform_bus_map_mmio(init->params, sbdev, i,
+ init->used_mem, init->mem);
+ }
+
+ return 0;
+}
+
+void platform_bus_init(PlatformParams *params,
+ MemoryRegion *address_space_mem,
+ qemu_irq *mpic)
+{
+ uint64_t max_size = params->platform_bus_size;
+ uint64_t max_pages = max_size >> PAGE_SHIFT;
+ DECLARE_BITMAP(used_irqs, params->platform_bus_num_irqs);
+ DECLARE_BITMAP(used_mem, max_pages);
+ MemoryRegion *platform_region = g_new(MemoryRegion, 1);
+ Object *container;
+ PlatformBusInitData init = {
+ .used_irqs = used_irqs,
+ .used_mem = used_mem,
+ .mem = platform_region,
+ .irqs = &mpic[params->platform_bus_first_irq],
+ .params = params,
+ };
+
+ memory_region_init(platform_region, NULL, "platform devices",
+ params->platform_bus_size);
+
+ bitmap_clear(used_irqs, 0, params->platform_bus_num_irqs);
+ bitmap_clear(used_mem, 0, max_pages);
+
+ /* Loop through all sysbus devices that were spawened outside the machine */
+ container = container_get(qdev_get_machine(), "/peripheral");
+ sysbus_device_check(container, &init);
+ container = container_get(qdev_get_machine(), "/peripheral-anon");
+ sysbus_device_check(container, &init);
+
+ memory_region_add_subregion(address_space_mem, params->platform_bus_base,
+ platform_region);
+}
+
+void platform_bus_init_notify(Notifier *notifier, void *data)
+{
+ PlatformBusNotifier *pn = (PlatformBusNotifier *)notifier;
+ platform_bus_init(&pn->params, pn->address_space_mem, pn->mpic);
+}
diff --git a/include/hw/misc/platform_devices.h b/include/hw/misc/platform_devices.h
new file mode 100644
index 0000000..ab79346
--- /dev/null
+++ b/include/hw/misc/platform_devices.h
@@ -0,0 +1,61 @@
+#ifndef HW_MISC_PLATFORM_DEVICES_H
+#define HW_MISC_PLATFORM_DEVICES_H
+
+#include "qemu-common.h"
+#include "sysemu/device_tree.h"
+#include "hw/hw.h"
+#include "hw/sysbus.h"
+
+typedef struct PlatformDevtreeData {
+ void *fdt;
+ const char *mpic;
+ int irq_start;
+ const char *node;
+} PlatformDevtreeData;
+
+typedef struct {
+ bool has_platform_bus;
+ hwaddr platform_bus_base;
+ hwaddr platform_bus_size;
+ int platform_bus_first_irq;
+ int platform_bus_num_irqs;
+} PlatformParams;
+
+typedef struct PlatformBusNotifier {
+ Notifier notifier;
+ MemoryRegion *address_space_mem;
+ qemu_irq *mpic;
+ PlatformParams params;
+} PlatformBusNotifier;
+
+typedef struct PlatformBusInitData {
+ unsigned long *used_irqs;
+ unsigned long *used_mem;
+ MemoryRegion *mem;
+ qemu_irq *irqs;
+ int device_count;
+ PlatformParams *params;
+} PlatformBusInitData;
+
+
+int sysbus_device_create_devtree(Object *obj, void *opaque);
+
+void platform_bus_create_devtree(PlatformParams *params, void *fdt,
+ const char *mpic);
+
+int platform_bus_map_irq(PlatformParams *params, SysBusDevice *sbdev,
+ int n, unsigned long *used_irqs,
+ qemu_irq *platform_irqs);
+
+int platform_bus_map_mmio(PlatformParams *params, SysBusDevice *sbdev,
+ int n, unsigned long *used_mem,
+ MemoryRegion *pmem);
+
+int sysbus_device_check(Object *obj, void *opaque);
+
+void platform_bus_init(PlatformParams *params,
+ MemoryRegion *address_space_mem,
+ qemu_irq *mpic);
+
+void platform_bus_init_notify(Notifier *notifier, void *data);
+#endif
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH 1/7] hw/misc/platform_devices: helpers for dynamic instantiation of platform devices
2014-07-07 7:08 ` [Qemu-devel] [PATCH 1/7] hw/misc/platform_devices: helpers for dynamic instantiation of platform devices Eric Auger
@ 2014-07-08 13:43 ` Alexander Graf
2014-07-23 14:58 ` Eric Auger
0 siblings, 1 reply; 23+ messages in thread
From: Alexander Graf @ 2014-07-08 13:43 UTC (permalink / raw)
To: Eric Auger, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 07.07.14 09:08, Eric Auger wrote:
> This new module implements routines which help in dynamic instantiation
> of sysbus devices. Machine files can use those generic routines.
>
> ---
>
> Dynamic sysbus device allocation fully written by Alex Graf.
>
> [Eric Auger]
> Those functions were initially in ppc e500 machine file. Now moved to a
> separate module.
>
> PPCE500Params is replaced by a generic struct named PlatformParams
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
> hw/misc/Makefile.objs | 1 +
> hw/misc/platform_devices.c | 217 +++++++++++++++++++++++++++++++++++++
> include/hw/misc/platform_devices.h | 61 +++++++++++
> 3 files changed, 279 insertions(+)
> create mode 100644 hw/misc/platform_devices.c
> create mode 100644 include/hw/misc/platform_devices.h
>
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index e47fea8..d081606 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -40,3 +40,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
> obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>
> obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-y += platform_devices.o
> diff --git a/hw/misc/platform_devices.c b/hw/misc/platform_devices.c
> new file mode 100644
> index 0000000..96ab272
> --- /dev/null
> +++ b/hw/misc/platform_devices.c
> @@ -0,0 +1,217 @@
> +#include "hw/misc/platform_devices.h"
> +#include "hw/sysbus.h"
> +#include "qemu/error-report.h"
> +
> +#define PAGE_SHIFT 12
> +
> +int sysbus_device_create_devtree(Object *obj, void *opaque)
> +{
> + PlatformDevtreeData *data = opaque;
> + Object *dev;
> + SysBusDevice *sbdev;
> + bool matched = false;
> +
> + dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
> + sbdev = (SysBusDevice *)dev;
> +
> + if (!sbdev) {
> + /* Container, traverse it for children */
> + return object_child_foreach(obj, sysbus_device_create_devtree, data);
> + }
> +
> + if (!matched) {
> + error_report("Device %s is not supported by this machine yet.",
> + qdev_fw_name(DEVICE(dev)));
> + exit(1);
> + }
> +
> + return 0;
> +}
> +
> +void platform_bus_create_devtree(PlatformParams *params, void *fdt,
> + const char *mpic)
> +{
> + gchar *node = g_strdup_printf("/platform@%"PRIx64,
> + params->platform_bus_base);
> + const char platcomp[] = "qemu,platform\0simple-bus";
> + PlatformDevtreeData data;
> + Object *container;
> + uint64_t addr = params->platform_bus_base;
> + uint64_t size = params->platform_bus_size;
> + int irq_start = params->platform_bus_first_irq;
> +
> + /* Create a /platform node that we can put all devices into */
> +
> + qemu_fdt_add_subnode(fdt, node);
> + qemu_fdt_setprop(fdt, node, "compatible", platcomp, sizeof(platcomp));
> +
> + /* Our platform bus region is less than 32bit big, so 1 cell is enough for
> + address and size */
> + qemu_fdt_setprop_cells(fdt, node, "#size-cells", 1);
> + qemu_fdt_setprop_cells(fdt, node, "#address-cells", 1);
> + qemu_fdt_setprop_cells(fdt, node, "ranges", 0, addr >> 32, addr, size);
> +
> + qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", mpic);
> +
> + /* Loop through all devices and create nodes for known ones */
> + data.fdt = fdt;
> + data.mpic = mpic;
> + data.irq_start = irq_start;
> + data.node = node;
> +
> + container = container_get(qdev_get_machine(), "/peripheral");
> + sysbus_device_create_devtree(container, &data);
> + container = container_get(qdev_get_machine(), "/peripheral-anon");
> + sysbus_device_create_devtree(container, &data);
> +
> + g_free(node);
> +}
Device trees are pretty platform (and even machine) specific. Just to
give you an example - the interrupt specifier on most e500 systems
really is 4 cells big:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt#n80
| Interrupt specifiers consists of 4 cells encoded as
follows:
<1st-cell> interrupt-number
Identifies the interrupt source. The meaning
depends on the type of interrupt.
Note: If the interrupt-type cell is undefined
(i.e. #interrupt-cells = 2), this cell
should be interpreted the same as for
interrupt-type 0-- i.e. an external or
normal SoC device interrupt.
<2nd-cell> level-sense information, encoded as follows:
0 = low-to-high edge triggered
1 = active low level-sensitive
2 = active high level-sensitive
3 = high-to-low edge triggered
<3rd-cell> interrupt-type
The following types are supported:
0 = external or normal SoC device interrupt
The interrupt-number cell contains
the SoC device interrupt number. The
type-specific cell is undefined. The
interrupt-number is derived from the
MPIC a block of registers referred to as
the "Interrupt Source Configuration Registers".
Each source has 32-bytes of registers
(vector/priority and destination) in this
region. So interrupt 0 is at offset 0x0,
interrupt 1 is at offset 0x20, and so on.
1 = error interrupt
The interrupt-number cell contains
the SoC device interrupt number for
the error interrupt. The type-specific
cell identifies the specific error
interrupt number.
2 = MPIC inter-processor interrupt (IPI)
The interrupt-number cell identifies
the MPIC IPI number. The type-specific
cell is undefined.
3 = MPIC timer interrupt
The interrupt-number cell identifies
the MPIC timer number. The type-specific
cell is undefined.
<4th-cell> type-specific information
The type-specific cell is encoded as follows:
- For interrupt-type 1 (error interrupt),
the type-specific cell contains the
bit number of the error interrupt in the
Error Interrupt Summary Register.
|
while on ARM you have a GIC which works like this:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/gic.txt#n20
|- #interrupt-cells : Specifies the number of cells needed to encode an
interrupt source. The type shall be a <u32> and the value shall be 3.
The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI
interrupts.
The 2nd cell contains the interrupt number for the interrupt type.
SPI interrupts are in the range [0-987]. PPI interrupts are in the
range [0-15].
The 3rd cell is the flags, encoded as follows:
bits[3:0] trigger type and level flags.
1 = low-to-high edge triggered
2 = high-to-low edge triggered
4 = active high level-sensitive
8 = active low level-sensitive
bits[15:8] PPI interrupt cpu mask. Each bit corresponds to each of
the 8 possible cpus attached to the GIC. A bit set to '1' indicated
the interrupt is wired to that CPU. Only valid for PPI interrupts.
|
Both have vastly different semantics. The number of cells is different,
the value of the cells is different. Even the definition how to
represent edge vs level triggered interrupts differs.
I don't think this will stop with interrupts. Maybe someone wants to add
a special machine check flag to addresses on a platform and then
"ranges" and "regs" will have different semantics on different
platforms. There is a lot that can go wrong when you try to unify this code.
> +
> +int platform_bus_map_irq(PlatformParams *params, SysBusDevice *sbdev,
> + int n, unsigned long *used_irqs,
> + qemu_irq *platform_irqs)
> +{
> + int max_irqs = params->platform_bus_num_irqs;
> + char *prop = g_strdup_printf("irq[%d]", n);
> + int irqn = object_property_get_int(OBJECT(sbdev), prop, NULL);
> +
> + if (irqn == SYSBUS_DYNAMIC) {
> + /* Find the first available IRQ */
> + irqn = find_first_zero_bit(used_irqs, max_irqs);
> + }
> +
> + if ((irqn >= max_irqs) || test_and_set_bit(irqn, used_irqs)) {
> + hw_error("IRQ %d is already allocated or no free IRQ left", irqn);
> + }
> +
> + sysbus_connect_irq(sbdev, n, platform_irqs[irqn]);
> + object_property_set_int(OBJECT(sbdev), irqn, prop, NULL);
> +
> + g_free(prop);
> + return 0;
> +}
> +
> +int platform_bus_map_mmio(PlatformParams *params, SysBusDevice *sbdev,
> + int n, unsigned long *used_mem,
> + MemoryRegion *pmem)
> +{
> + MemoryRegion *device_mem = sbdev->mmio[n].memory;
> + uint64_t size = memory_region_size(device_mem);
> + uint64_t page_size = (1 << PAGE_SHIFT);
> + uint64_t page_mask = page_size - 1;
> + uint64_t size_pages = (size + page_mask) >> PAGE_SHIFT;
> + uint64_t max_size = params->platform_bus_size;
> + uint64_t max_pages = max_size >> PAGE_SHIFT;
> + char *prop = g_strdup_printf("mmio[%d]", n);
> + hwaddr addr = object_property_get_int(OBJECT(sbdev), prop, NULL);
> + int page;
> + int i;
> +
> + page = addr >> PAGE_SHIFT;
> + if (addr == SYSBUS_DYNAMIC) {
> + uint64_t size_pages_align;
> +
> + /* Align the region to at least its own size granularity */
> + if (is_power_of_2(size_pages)) {
> + size_pages_align = size_pages;
> + } else {
> + size_pages_align = pow2floor(size_pages) << 1;
> + }
> +
> + /* Find the first available region that fits */
> + page = bitmap_find_next_zero_area(used_mem, max_pages, 0, size_pages,
> + size_pages_align);
> +
> + addr = (uint64_t)page << PAGE_SHIFT;
> + }
> +
> + if (page >= max_pages || test_bit(page, used_mem) ||
> + (find_next_bit(used_mem, max_pages, page) < size_pages)) {
> + hw_error("Memory [%"PRIx64":%"PRIx64" is already allocated or "
> + "no slot left", addr, size);
> + }
> +
> + for (i = page; i < (page + size_pages); i++) {
> + set_bit(i, used_mem);
> + }
> +
> + memory_region_add_subregion(pmem, addr, device_mem);
> + sbdev->mmio[n].addr = addr;
> + object_property_set_int(OBJECT(sbdev), addr, prop, NULL);
> +
> + g_free(prop);
> + return 0;
> +}
> +
> +int sysbus_device_check(Object *obj, void *opaque)
> +{
> + PlatformBusInitData *init = opaque;
> + Object *dev;
> + SysBusDevice *sbdev;
> + int i;
> +
> + dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
> + sbdev = (SysBusDevice *)dev;
> +
> + if (!sbdev) {
> + /* Container, traverse it for children */
> + return object_child_foreach(obj, sysbus_device_check, opaque);
> + }
> +
> + /* Connect sysbus device to virtual platform bus */
> + for (i = 0; i < sbdev->num_irq; i++) {
> + if (!sbdev->irqp[i]) {
> + /* This IRQ is an incoming IRQ, we can't wire those here */
> + continue;
> + }
> + platform_bus_map_irq(init->params, sbdev, i,
> + init->used_irqs, init->irqs);
> + }
> +
> + for (i = 0; i < sbdev->num_mmio; i++) {
> + platform_bus_map_mmio(init->params, sbdev, i,
> + init->used_mem, init->mem);
> + }
> +
> + return 0;
> +}
> +
> +void platform_bus_init(PlatformParams *params,
> + MemoryRegion *address_space_mem,
> + qemu_irq *mpic)
> +{
> + uint64_t max_size = params->platform_bus_size;
> + uint64_t max_pages = max_size >> PAGE_SHIFT;
> + DECLARE_BITMAP(used_irqs, params->platform_bus_num_irqs);
> + DECLARE_BITMAP(used_mem, max_pages);
> + MemoryRegion *platform_region = g_new(MemoryRegion, 1);
> + Object *container;
> + PlatformBusInitData init = {
> + .used_irqs = used_irqs,
> + .used_mem = used_mem,
> + .mem = platform_region,
> + .irqs = &mpic[params->platform_bus_first_irq],
> + .params = params,
> + };
> +
> + memory_region_init(platform_region, NULL, "platform devices",
> + params->platform_bus_size);
> +
> + bitmap_clear(used_irqs, 0, params->platform_bus_num_irqs);
> + bitmap_clear(used_mem, 0, max_pages);
> +
> + /* Loop through all sysbus devices that were spawened outside the machine */
> + container = container_get(qdev_get_machine(), "/peripheral");
> + sysbus_device_check(container, &init);
> + container = container_get(qdev_get_machine(), "/peripheral-anon");
> + sysbus_device_check(container, &init);
> +
> + memory_region_add_subregion(address_space_mem, params->platform_bus_base,
> + platform_region);
> +}
However, I do think it's a good idea to generalize the "platform bus"
device if you want to reuse it on ARM. The mmio / irq allocator is
pretty straight forward and should be generic enough for you to use.
If you do this, please don't duplicate the code but rather move it from
the e500 file into your new one :).
Alex
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH 1/7] hw/misc/platform_devices: helpers for dynamic instantiation of platform devices
2014-07-08 13:43 ` Alexander Graf
@ 2014-07-23 14:58 ` Eric Auger
2014-07-23 23:07 ` Alexander Graf
0 siblings, 1 reply; 23+ messages in thread
From: Eric Auger @ 2014-07-23 14:58 UTC (permalink / raw)
To: Alexander Graf, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 07/08/2014 03:43 PM, Alexander Graf wrote:
>
> On 07.07.14 09:08, Eric Auger wrote:
>> This new module implements routines which help in dynamic instantiation
>> of sysbus devices. Machine files can use those generic routines.
>>
>> ---
>>
>> Dynamic sysbus device allocation fully written by Alex Graf.
>>
>> [Eric Auger]
>> Those functions were initially in ppc e500 machine file. Now moved to a
>> separate module.
>>
>> PPCE500Params is replaced by a generic struct named PlatformParams
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>> ---
>> hw/misc/Makefile.objs | 1 +
>> hw/misc/platform_devices.c | 217
>> +++++++++++++++++++++++++++++++++++++
>> include/hw/misc/platform_devices.h | 61 +++++++++++
>> 3 files changed, 279 insertions(+)
>> create mode 100644 hw/misc/platform_devices.c
>> create mode 100644 include/hw/misc/platform_devices.h
>>
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index e47fea8..d081606 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -40,3 +40,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>> obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>> obj-$(CONFIG_PVPANIC) += pvpanic.o
>> +obj-y += platform_devices.o
>> diff --git a/hw/misc/platform_devices.c b/hw/misc/platform_devices.c
>> new file mode 100644
>> index 0000000..96ab272
>> --- /dev/null
>> +++ b/hw/misc/platform_devices.c
>> @@ -0,0 +1,217 @@
>> +#include "hw/misc/platform_devices.h"
>> +#include "hw/sysbus.h"
>> +#include "qemu/error-report.h"
>> +
>> +#define PAGE_SHIFT 12
>> +
>> +int sysbus_device_create_devtree(Object *obj, void *opaque)
>> +{
>> + PlatformDevtreeData *data = opaque;
>> + Object *dev;
>> + SysBusDevice *sbdev;
>> + bool matched = false;
>> +
>> + dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
>> + sbdev = (SysBusDevice *)dev;
>> +
>> + if (!sbdev) {
>> + /* Container, traverse it for children */
>> + return object_child_foreach(obj,
>> sysbus_device_create_devtree, data);
>> + }
>> +
>> + if (!matched) {
>> + error_report("Device %s is not supported by this machine yet.",
>> + qdev_fw_name(DEVICE(dev)));
>> + exit(1);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +void platform_bus_create_devtree(PlatformParams *params, void *fdt,
>> + const char *mpic)
>> +{
>> + gchar *node = g_strdup_printf("/platform@%"PRIx64,
>> + params->platform_bus_base);
>> + const char platcomp[] = "qemu,platform\0simple-bus";
>> + PlatformDevtreeData data;
>> + Object *container;
>> + uint64_t addr = params->platform_bus_base;
>> + uint64_t size = params->platform_bus_size;
>> + int irq_start = params->platform_bus_first_irq;
>> +
>> + /* Create a /platform node that we can put all devices into */
>> +
>> + qemu_fdt_add_subnode(fdt, node);
>> + qemu_fdt_setprop(fdt, node, "compatible", platcomp,
>> sizeof(platcomp));
>> +
>> + /* Our platform bus region is less than 32bit big, so 1 cell is
>> enough for
>> + address and size */
>> + qemu_fdt_setprop_cells(fdt, node, "#size-cells", 1);
>> + qemu_fdt_setprop_cells(fdt, node, "#address-cells", 1);
>> + qemu_fdt_setprop_cells(fdt, node, "ranges", 0, addr >> 32, addr,
>> size);
>> +
>> + qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", mpic);
>> +
>> + /* Loop through all devices and create nodes for known ones */
>> + data.fdt = fdt;
>> + data.mpic = mpic;
>> + data.irq_start = irq_start;
>> + data.node = node;
>> +
>> + container = container_get(qdev_get_machine(), "/peripheral");
>> + sysbus_device_create_devtree(container, &data);
>> + container = container_get(qdev_get_machine(), "/peripheral-anon");
>> + sysbus_device_create_devtree(container, &data);
>> +
>> + g_free(node);
>> +}
>
> Device trees are pretty platform (and even machine) specific. Just to
> give you an example - the interrupt specifier on most e500 systems
> really is 4 cells big:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt#n80
>
>
> | Interrupt specifiers consists of 4 cells encoded as
> follows:
>
> <1st-cell> interrupt-number
>
> Identifies the interrupt source. The meaning
> depends on the type of interrupt.
>
> Note: If the interrupt-type cell is undefined
> (i.e. #interrupt-cells = 2), this cell
> should be interpreted the same as for
> interrupt-type 0-- i.e. an external or
> normal SoC device interrupt.
>
> <2nd-cell> level-sense information, encoded as follows:
> 0 = low-to-high edge triggered
> 1 = active low level-sensitive
> 2 = active high level-sensitive
> 3 = high-to-low edge triggered
>
> <3rd-cell> interrupt-type
>
> The following types are supported:
>
> 0 = external or normal SoC device interrupt
>
> The interrupt-number cell contains
> the SoC device interrupt number. The
> type-specific cell is undefined. The
> interrupt-number is derived from the
> MPIC a block of registers referred to as
> the "Interrupt Source Configuration Registers".
> Each source has 32-bytes of registers
> (vector/priority and destination) in this
> region. So interrupt 0 is at offset 0x0,
> interrupt 1 is at offset 0x20, and so on.
>
> 1 = error interrupt
>
> The interrupt-number cell contains
> the SoC device interrupt number for
> the error interrupt. The type-specific
> cell identifies the specific error
> interrupt number.
>
> 2 = MPIC inter-processor interrupt (IPI)
>
> The interrupt-number cell identifies
> the MPIC IPI number. The type-specific
> cell is undefined.
>
> 3 = MPIC timer interrupt
>
> The interrupt-number cell identifies
> the MPIC timer number. The type-specific
> cell is undefined.
>
> <4th-cell> type-specific information
>
> The type-specific cell is encoded as follows:
>
> - For interrupt-type 1 (error interrupt),
> the type-specific cell contains the
> bit number of the error interrupt in the
> Error Interrupt Summary Register.
> |
>
>
>
>
> while on ARM you have a GIC which works like this:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/gic.txt#n20
>
>
> |- #interrupt-cells : Specifies the number of cells needed to encode an
> interrupt source. The type shall be a <u32> and the value shall be 3.
>
> The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI
> interrupts.
>
> The 2nd cell contains the interrupt number for the interrupt type.
> SPI interrupts are in the range [0-987]. PPI interrupts are in the
> range [0-15].
>
> The 3rd cell is the flags, encoded as follows:
> bits[3:0] trigger type and level flags.
> 1 = low-to-high edge triggered
> 2 = high-to-low edge triggered
> 4 = active high level-sensitive
> 8 = active low level-sensitive
> bits[15:8] PPI interrupt cpu mask. Each bit corresponds to each of
> the 8 possible cpus attached to the GIC. A bit set to '1' indicated
> the interrupt is wired to that CPU. Only valid for PPI interrupts.
> |
>
>
>
> Both have vastly different semantics. The number of cells is different,
> the value of the cells is different. Even the definition how to
> represent edge vs level triggered interrupts differs.
>
> I don't think this will stop with interrupts. Maybe someone wants to add
> a special machine check flag to addresses on a platform and then
> "ranges" and "regs" will have different semantics on different
> platforms. There is a lot that can go wrong when you try to unify this
> code.
Hi Alex,
thank you for giving such an example. Indeed I was not aware there were
such huge discrepancies. I guess this comment mostly holds for the
actual device node generation (what I specialized in the parent QEMU
device) and not for the "qemu, platform simple-bus" node generation?
>
>> +
>> +int platform_bus_map_irq(PlatformParams *params, SysBusDevice *sbdev,
>> + int n, unsigned long *used_irqs,
>> + qemu_irq *platform_irqs)
>> +{
>> + int max_irqs = params->platform_bus_num_irqs;
>> + char *prop = g_strdup_printf("irq[%d]", n);
>> + int irqn = object_property_get_int(OBJECT(sbdev), prop, NULL);
>> +
>> + if (irqn == SYSBUS_DYNAMIC) {
>> + /* Find the first available IRQ */
>> + irqn = find_first_zero_bit(used_irqs, max_irqs);
>> + }
>> +
>> + if ((irqn >= max_irqs) || test_and_set_bit(irqn, used_irqs)) {
>> + hw_error("IRQ %d is already allocated or no free IRQ left",
>> irqn);
>> + }
>> +
>> + sysbus_connect_irq(sbdev, n, platform_irqs[irqn]);
>> + object_property_set_int(OBJECT(sbdev), irqn, prop, NULL);
>> +
>> + g_free(prop);
>> + return 0;
>> +}
>> +
>> +int platform_bus_map_mmio(PlatformParams *params, SysBusDevice *sbdev,
>> + int n, unsigned long *used_mem,
>> + MemoryRegion *pmem)
>> +{
>> + MemoryRegion *device_mem = sbdev->mmio[n].memory;
>> + uint64_t size = memory_region_size(device_mem);
>> + uint64_t page_size = (1 << PAGE_SHIFT);
>> + uint64_t page_mask = page_size - 1;
>> + uint64_t size_pages = (size + page_mask) >> PAGE_SHIFT;
>> + uint64_t max_size = params->platform_bus_size;
>> + uint64_t max_pages = max_size >> PAGE_SHIFT;
>> + char *prop = g_strdup_printf("mmio[%d]", n);
>> + hwaddr addr = object_property_get_int(OBJECT(sbdev), prop, NULL);
>> + int page;
>> + int i;
>> +
>> + page = addr >> PAGE_SHIFT;
>> + if (addr == SYSBUS_DYNAMIC) {
>> + uint64_t size_pages_align;
>> +
>> + /* Align the region to at least its own size granularity */
>> + if (is_power_of_2(size_pages)) {
>> + size_pages_align = size_pages;
>> + } else {
>> + size_pages_align = pow2floor(size_pages) << 1;
>> + }
>> +
>> + /* Find the first available region that fits */
>> + page = bitmap_find_next_zero_area(used_mem, max_pages, 0,
>> size_pages,
>> + size_pages_align);
>> +
>> + addr = (uint64_t)page << PAGE_SHIFT;
>> + }
>> +
>> + if (page >= max_pages || test_bit(page, used_mem) ||
>> + (find_next_bit(used_mem, max_pages, page) < size_pages)) {
>> + hw_error("Memory [%"PRIx64":%"PRIx64" is already allocated or "
>> + "no slot left", addr, size);
>> + }
>> +
>> + for (i = page; i < (page + size_pages); i++) {
>> + set_bit(i, used_mem);
>> + }
>> +
>> + memory_region_add_subregion(pmem, addr, device_mem);
>> + sbdev->mmio[n].addr = addr;
>> + object_property_set_int(OBJECT(sbdev), addr, prop, NULL);
>> +
>> + g_free(prop);
>> + return 0;
>> +}
>> +
>> +int sysbus_device_check(Object *obj, void *opaque)
>> +{
>> + PlatformBusInitData *init = opaque;
>> + Object *dev;
>> + SysBusDevice *sbdev;
>> + int i;
>> +
>> + dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
>> + sbdev = (SysBusDevice *)dev;
>> +
>> + if (!sbdev) {
>> + /* Container, traverse it for children */
>> + return object_child_foreach(obj, sysbus_device_check, opaque);
>> + }
>> +
>> + /* Connect sysbus device to virtual platform bus */
>> + for (i = 0; i < sbdev->num_irq; i++) {
>> + if (!sbdev->irqp[i]) {
>> + /* This IRQ is an incoming IRQ, we can't wire those here */
>> + continue;
>> + }
>> + platform_bus_map_irq(init->params, sbdev, i,
>> + init->used_irqs, init->irqs);
>> + }
>> +
>> + for (i = 0; i < sbdev->num_mmio; i++) {
>> + platform_bus_map_mmio(init->params, sbdev, i,
>> + init->used_mem, init->mem);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +void platform_bus_init(PlatformParams *params,
>> + MemoryRegion *address_space_mem,
>> + qemu_irq *mpic)
>> +{
>> + uint64_t max_size = params->platform_bus_size;
>> + uint64_t max_pages = max_size >> PAGE_SHIFT;
>> + DECLARE_BITMAP(used_irqs, params->platform_bus_num_irqs);
>> + DECLARE_BITMAP(used_mem, max_pages);
>> + MemoryRegion *platform_region = g_new(MemoryRegion, 1);
>> + Object *container;
>> + PlatformBusInitData init = {
>> + .used_irqs = used_irqs,
>> + .used_mem = used_mem,
>> + .mem = platform_region,
>> + .irqs = &mpic[params->platform_bus_first_irq],
>> + .params = params,
>> + };
>> +
>> + memory_region_init(platform_region, NULL, "platform devices",
>> + params->platform_bus_size);
>> +
>> + bitmap_clear(used_irqs, 0, params->platform_bus_num_irqs);
>> + bitmap_clear(used_mem, 0, max_pages);
>> +
>> + /* Loop through all sysbus devices that were spawened outside the
>> machine */
>> + container = container_get(qdev_get_machine(), "/peripheral");
>> + sysbus_device_check(container, &init);
>> + container = container_get(qdev_get_machine(), "/peripheral-anon");
>> + sysbus_device_check(container, &init);
>> +
>> + memory_region_add_subregion(address_space_mem,
>> params->platform_bus_base,
>> + platform_region);
>> +}
>
> However, I do think it's a good idea to generalize the "platform bus"
> device if you want to reuse it on ARM. The mmio / irq allocator is
> pretty straight forward and should be generic enough for you to use.
I need clarification here: do you talk about your very first patch
"Platform Device Support" or the code above with a proper solution for
device tree generation?
>
> If you do this, please don't duplicate the code but rather move it from
> the e500 file into your new one :).
OK. do you mean modifying the e500.c code to call those routines? My
concern is about testing.
Thanks again for your comments
Best Regards
Eric
>
>
> Alex
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH 1/7] hw/misc/platform_devices: helpers for dynamic instantiation of platform devices
2014-07-23 14:58 ` Eric Auger
@ 2014-07-23 23:07 ` Alexander Graf
2014-07-24 8:01 ` Eric Auger
0 siblings, 1 reply; 23+ messages in thread
From: Alexander Graf @ 2014-07-23 23:07 UTC (permalink / raw)
To: Eric Auger, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 23.07.14 16:58, Eric Auger wrote:
> On 07/08/2014 03:43 PM, Alexander Graf wrote:
>> On 07.07.14 09:08, Eric Auger wrote:
>>> This new module implements routines which help in dynamic instantiation
>>> of sysbus devices. Machine files can use those generic routines.
>>>
>>> ---
>>>
>>> Dynamic sysbus device allocation fully written by Alex Graf.
>>>
>>> [Eric Auger]
>>> Those functions were initially in ppc e500 machine file. Now moved to a
>>> separate module.
>>>
>>> PPCE500Params is replaced by a generic struct named PlatformParams
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>> ---
>>> hw/misc/Makefile.objs | 1 +
>>> hw/misc/platform_devices.c | 217
>>> +++++++++++++++++++++++++++++++++++++
>>> include/hw/misc/platform_devices.h | 61 +++++++++++
>>> 3 files changed, 279 insertions(+)
>>> create mode 100644 hw/misc/platform_devices.c
>>> create mode 100644 include/hw/misc/platform_devices.h
>>>
>>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>>> index e47fea8..d081606 100644
>>> --- a/hw/misc/Makefile.objs
>>> +++ b/hw/misc/Makefile.objs
>>> @@ -40,3 +40,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>>> obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>>> obj-$(CONFIG_PVPANIC) += pvpanic.o
>>> +obj-y += platform_devices.o
>>> diff --git a/hw/misc/platform_devices.c b/hw/misc/platform_devices.c
>>> new file mode 100644
>>> index 0000000..96ab272
>>> --- /dev/null
>>> +++ b/hw/misc/platform_devices.c
>>> @@ -0,0 +1,217 @@
>>> +#include "hw/misc/platform_devices.h"
>>> +#include "hw/sysbus.h"
>>> +#include "qemu/error-report.h"
>>> +
>>> +#define PAGE_SHIFT 12
>>> +
>>> +int sysbus_device_create_devtree(Object *obj, void *opaque)
>>> +{
>>> + PlatformDevtreeData *data = opaque;
>>> + Object *dev;
>>> + SysBusDevice *sbdev;
>>> + bool matched = false;
>>> +
>>> + dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
>>> + sbdev = (SysBusDevice *)dev;
>>> +
>>> + if (!sbdev) {
>>> + /* Container, traverse it for children */
>>> + return object_child_foreach(obj,
>>> sysbus_device_create_devtree, data);
>>> + }
>>> +
>>> + if (!matched) {
>>> + error_report("Device %s is not supported by this machine yet.",
>>> + qdev_fw_name(DEVICE(dev)));
>>> + exit(1);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +void platform_bus_create_devtree(PlatformParams *params, void *fdt,
>>> + const char *mpic)
>>> +{
>>> + gchar *node = g_strdup_printf("/platform@%"PRIx64,
>>> + params->platform_bus_base);
>>> + const char platcomp[] = "qemu,platform\0simple-bus";
>>> + PlatformDevtreeData data;
>>> + Object *container;
>>> + uint64_t addr = params->platform_bus_base;
>>> + uint64_t size = params->platform_bus_size;
>>> + int irq_start = params->platform_bus_first_irq;
>>> +
>>> + /* Create a /platform node that we can put all devices into */
>>> +
>>> + qemu_fdt_add_subnode(fdt, node);
>>> + qemu_fdt_setprop(fdt, node, "compatible", platcomp,
>>> sizeof(platcomp));
>>> +
>>> + /* Our platform bus region is less than 32bit big, so 1 cell is
>>> enough for
>>> + address and size */
>>> + qemu_fdt_setprop_cells(fdt, node, "#size-cells", 1);
>>> + qemu_fdt_setprop_cells(fdt, node, "#address-cells", 1);
>>> + qemu_fdt_setprop_cells(fdt, node, "ranges", 0, addr >> 32, addr,
>>> size);
>>> +
>>> + qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", mpic);
>>> +
>>> + /* Loop through all devices and create nodes for known ones */
>>> + data.fdt = fdt;
>>> + data.mpic = mpic;
>>> + data.irq_start = irq_start;
>>> + data.node = node;
>>> +
>>> + container = container_get(qdev_get_machine(), "/peripheral");
>>> + sysbus_device_create_devtree(container, &data);
>>> + container = container_get(qdev_get_machine(), "/peripheral-anon");
>>> + sysbus_device_create_devtree(container, &data);
>>> +
>>> + g_free(node);
>>> +}
>> Device trees are pretty platform (and even machine) specific. Just to
>> give you an example - the interrupt specifier on most e500 systems
>> really is 4 cells big:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt#n80
>>
>>
>> | Interrupt specifiers consists of 4 cells encoded as
>> follows:
>>
>> <1st-cell> interrupt-number
>>
>> Identifies the interrupt source. The meaning
>> depends on the type of interrupt.
>>
>> Note: If the interrupt-type cell is undefined
>> (i.e. #interrupt-cells = 2), this cell
>> should be interpreted the same as for
>> interrupt-type 0-- i.e. an external or
>> normal SoC device interrupt.
>>
>> <2nd-cell> level-sense information, encoded as follows:
>> 0 = low-to-high edge triggered
>> 1 = active low level-sensitive
>> 2 = active high level-sensitive
>> 3 = high-to-low edge triggered
>>
>> <3rd-cell> interrupt-type
>>
>> The following types are supported:
>>
>> 0 = external or normal SoC device interrupt
>>
>> The interrupt-number cell contains
>> the SoC device interrupt number. The
>> type-specific cell is undefined. The
>> interrupt-number is derived from the
>> MPIC a block of registers referred to as
>> the "Interrupt Source Configuration Registers".
>> Each source has 32-bytes of registers
>> (vector/priority and destination) in this
>> region. So interrupt 0 is at offset 0x0,
>> interrupt 1 is at offset 0x20, and so on.
>>
>> 1 = error interrupt
>>
>> The interrupt-number cell contains
>> the SoC device interrupt number for
>> the error interrupt. The type-specific
>> cell identifies the specific error
>> interrupt number.
>>
>> 2 = MPIC inter-processor interrupt (IPI)
>>
>> The interrupt-number cell identifies
>> the MPIC IPI number. The type-specific
>> cell is undefined.
>>
>> 3 = MPIC timer interrupt
>>
>> The interrupt-number cell identifies
>> the MPIC timer number. The type-specific
>> cell is undefined.
>>
>> <4th-cell> type-specific information
>>
>> The type-specific cell is encoded as follows:
>>
>> - For interrupt-type 1 (error interrupt),
>> the type-specific cell contains the
>> bit number of the error interrupt in the
>> Error Interrupt Summary Register.
>> |
>>
>>
>>
>>
>> while on ARM you have a GIC which works like this:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/gic.txt#n20
>>
>>
>> |- #interrupt-cells : Specifies the number of cells needed to encode an
>> interrupt source. The type shall be a <u32> and the value shall be 3.
>>
>> The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI
>> interrupts.
>>
>> The 2nd cell contains the interrupt number for the interrupt type.
>> SPI interrupts are in the range [0-987]. PPI interrupts are in the
>> range [0-15].
>>
>> The 3rd cell is the flags, encoded as follows:
>> bits[3:0] trigger type and level flags.
>> 1 = low-to-high edge triggered
>> 2 = high-to-low edge triggered
>> 4 = active high level-sensitive
>> 8 = active low level-sensitive
>> bits[15:8] PPI interrupt cpu mask. Each bit corresponds to each of
>> the 8 possible cpus attached to the GIC. A bit set to '1' indicated
>> the interrupt is wired to that CPU. Only valid for PPI interrupts.
>> |
>>
>>
>>
>> Both have vastly different semantics. The number of cells is different,
>> the value of the cells is different. Even the definition how to
>> represent edge vs level triggered interrupts differs.
>>
>> I don't think this will stop with interrupts. Maybe someone wants to add
>> a special machine check flag to addresses on a platform and then
>> "ranges" and "regs" will have different semantics on different
>> platforms. There is a lot that can go wrong when you try to unify this
>> code.
> Hi Alex,
>
> thank you for giving such an example. Indeed I was not aware there were
> such huge discrepancies. I guess this comment mostly holds for the
> actual device node generation (what I specialized in the parent QEMU
> device) and not for the "qemu, platform simple-bus" node generation?
Is the dt generation that much code? Just duplicate it for now - we can
generalize it later if we see how things work out.
>
>>> +
>>> +int platform_bus_map_irq(PlatformParams *params, SysBusDevice *sbdev,
>>> + int n, unsigned long *used_irqs,
>>> + qemu_irq *platform_irqs)
>>> +{
>>> + int max_irqs = params->platform_bus_num_irqs;
>>> + char *prop = g_strdup_printf("irq[%d]", n);
>>> + int irqn = object_property_get_int(OBJECT(sbdev), prop, NULL);
>>> +
>>> + if (irqn == SYSBUS_DYNAMIC) {
>>> + /* Find the first available IRQ */
>>> + irqn = find_first_zero_bit(used_irqs, max_irqs);
>>> + }
>>> +
>>> + if ((irqn >= max_irqs) || test_and_set_bit(irqn, used_irqs)) {
>>> + hw_error("IRQ %d is already allocated or no free IRQ left",
>>> irqn);
>>> + }
>>> +
>>> + sysbus_connect_irq(sbdev, n, platform_irqs[irqn]);
>>> + object_property_set_int(OBJECT(sbdev), irqn, prop, NULL);
>>> +
>>> + g_free(prop);
>>> + return 0;
>>> +}
>>> +
>>> +int platform_bus_map_mmio(PlatformParams *params, SysBusDevice *sbdev,
>>> + int n, unsigned long *used_mem,
>>> + MemoryRegion *pmem)
>>> +{
>>> + MemoryRegion *device_mem = sbdev->mmio[n].memory;
>>> + uint64_t size = memory_region_size(device_mem);
>>> + uint64_t page_size = (1 << PAGE_SHIFT);
>>> + uint64_t page_mask = page_size - 1;
>>> + uint64_t size_pages = (size + page_mask) >> PAGE_SHIFT;
>>> + uint64_t max_size = params->platform_bus_size;
>>> + uint64_t max_pages = max_size >> PAGE_SHIFT;
>>> + char *prop = g_strdup_printf("mmio[%d]", n);
>>> + hwaddr addr = object_property_get_int(OBJECT(sbdev), prop, NULL);
>>> + int page;
>>> + int i;
>>> +
>>> + page = addr >> PAGE_SHIFT;
>>> + if (addr == SYSBUS_DYNAMIC) {
>>> + uint64_t size_pages_align;
>>> +
>>> + /* Align the region to at least its own size granularity */
>>> + if (is_power_of_2(size_pages)) {
>>> + size_pages_align = size_pages;
>>> + } else {
>>> + size_pages_align = pow2floor(size_pages) << 1;
>>> + }
>>> +
>>> + /* Find the first available region that fits */
>>> + page = bitmap_find_next_zero_area(used_mem, max_pages, 0,
>>> size_pages,
>>> + size_pages_align);
>>> +
>>> + addr = (uint64_t)page << PAGE_SHIFT;
>>> + }
>>> +
>>> + if (page >= max_pages || test_bit(page, used_mem) ||
>>> + (find_next_bit(used_mem, max_pages, page) < size_pages)) {
>>> + hw_error("Memory [%"PRIx64":%"PRIx64" is already allocated or "
>>> + "no slot left", addr, size);
>>> + }
>>> +
>>> + for (i = page; i < (page + size_pages); i++) {
>>> + set_bit(i, used_mem);
>>> + }
>>> +
>>> + memory_region_add_subregion(pmem, addr, device_mem);
>>> + sbdev->mmio[n].addr = addr;
>>> + object_property_set_int(OBJECT(sbdev), addr, prop, NULL);
>>> +
>>> + g_free(prop);
>>> + return 0;
>>> +}
>>> +
>>> +int sysbus_device_check(Object *obj, void *opaque)
>>> +{
>>> + PlatformBusInitData *init = opaque;
>>> + Object *dev;
>>> + SysBusDevice *sbdev;
>>> + int i;
>>> +
>>> + dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
>>> + sbdev = (SysBusDevice *)dev;
>>> +
>>> + if (!sbdev) {
>>> + /* Container, traverse it for children */
>>> + return object_child_foreach(obj, sysbus_device_check, opaque);
>>> + }
>>> +
>>> + /* Connect sysbus device to virtual platform bus */
>>> + for (i = 0; i < sbdev->num_irq; i++) {
>>> + if (!sbdev->irqp[i]) {
>>> + /* This IRQ is an incoming IRQ, we can't wire those here */
>>> + continue;
>>> + }
>>> + platform_bus_map_irq(init->params, sbdev, i,
>>> + init->used_irqs, init->irqs);
>>> + }
>>> +
>>> + for (i = 0; i < sbdev->num_mmio; i++) {
>>> + platform_bus_map_mmio(init->params, sbdev, i,
>>> + init->used_mem, init->mem);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +void platform_bus_init(PlatformParams *params,
>>> + MemoryRegion *address_space_mem,
>>> + qemu_irq *mpic)
>>> +{
>>> + uint64_t max_size = params->platform_bus_size;
>>> + uint64_t max_pages = max_size >> PAGE_SHIFT;
>>> + DECLARE_BITMAP(used_irqs, params->platform_bus_num_irqs);
>>> + DECLARE_BITMAP(used_mem, max_pages);
>>> + MemoryRegion *platform_region = g_new(MemoryRegion, 1);
>>> + Object *container;
>>> + PlatformBusInitData init = {
>>> + .used_irqs = used_irqs,
>>> + .used_mem = used_mem,
>>> + .mem = platform_region,
>>> + .irqs = &mpic[params->platform_bus_first_irq],
>>> + .params = params,
>>> + };
>>> +
>>> + memory_region_init(platform_region, NULL, "platform devices",
>>> + params->platform_bus_size);
>>> +
>>> + bitmap_clear(used_irqs, 0, params->platform_bus_num_irqs);
>>> + bitmap_clear(used_mem, 0, max_pages);
>>> +
>>> + /* Loop through all sysbus devices that were spawened outside the
>>> machine */
>>> + container = container_get(qdev_get_machine(), "/peripheral");
>>> + sysbus_device_check(container, &init);
>>> + container = container_get(qdev_get_machine(), "/peripheral-anon");
>>> + sysbus_device_check(container, &init);
>>> +
>>> + memory_region_add_subregion(address_space_mem,
>>> params->platform_bus_base,
>>> + platform_region);
>>> +}
>> However, I do think it's a good idea to generalize the "platform bus"
>> device if you want to reuse it on ARM. The mmio / irq allocator is
>> pretty straight forward and should be generic enough for you to use.
> I need clarification here: do you talk about your very first patch
> "Platform Device Support" or the code above with a proper solution for
> device tree generation?
I'm talking about the actual implementation of the allocation logic. I
think we're better off to keep all the device tree logic purely in the
machine files for now.
>> If you do this, please don't duplicate the code but rather move it from
>> the e500 file into your new one :).
> OK. do you mean modifying the e500.c code to call those routines? My
> concern is about testing.
Why? We have a u-boot binary that starts up based on the device tree
with TCG if you just start the e500plat machine and if you like I can
easily give you a nice guest kernel and rootfs as well ;).
Alex
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH 1/7] hw/misc/platform_devices: helpers for dynamic instantiation of platform devices
2014-07-23 23:07 ` Alexander Graf
@ 2014-07-24 8:01 ` Eric Auger
0 siblings, 0 replies; 23+ messages in thread
From: Eric Auger @ 2014-07-24 8:01 UTC (permalink / raw)
To: Alexander Graf, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 07/24/2014 01:07 AM, Alexander Graf wrote:
>
> On 23.07.14 16:58, Eric Auger wrote:
>> On 07/08/2014 03:43 PM, Alexander Graf wrote:
>>> On 07.07.14 09:08, Eric Auger wrote:
>>>> This new module implements routines which help in dynamic instantiation
>>>> of sysbus devices. Machine files can use those generic routines.
>>>>
>>>> ---
>>>>
>>>> Dynamic sysbus device allocation fully written by Alex Graf.
>>>>
>>>> [Eric Auger]
>>>> Those functions were initially in ppc e500 machine file. Now moved to a
>>>> separate module.
>>>>
>>>> PPCE500Params is replaced by a generic struct named PlatformParams
>>>>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>> ---
>>>> hw/misc/Makefile.objs | 1 +
>>>> hw/misc/platform_devices.c | 217
>>>> +++++++++++++++++++++++++++++++++++++
>>>> include/hw/misc/platform_devices.h | 61 +++++++++++
>>>> 3 files changed, 279 insertions(+)
>>>> create mode 100644 hw/misc/platform_devices.c
>>>> create mode 100644 include/hw/misc/platform_devices.h
>>>>
>>>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>>>> index e47fea8..d081606 100644
>>>> --- a/hw/misc/Makefile.objs
>>>> +++ b/hw/misc/Makefile.objs
>>>> @@ -40,3 +40,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
>>>> obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>>>> obj-$(CONFIG_PVPANIC) += pvpanic.o
>>>> +obj-y += platform_devices.o
>>>> diff --git a/hw/misc/platform_devices.c b/hw/misc/platform_devices.c
>>>> new file mode 100644
>>>> index 0000000..96ab272
>>>> --- /dev/null
>>>> +++ b/hw/misc/platform_devices.c
>>>> @@ -0,0 +1,217 @@
>>>> +#include "hw/misc/platform_devices.h"
>>>> +#include "hw/sysbus.h"
>>>> +#include "qemu/error-report.h"
>>>> +
>>>> +#define PAGE_SHIFT 12
>>>> +
>>>> +int sysbus_device_create_devtree(Object *obj, void *opaque)
>>>> +{
>>>> + PlatformDevtreeData *data = opaque;
>>>> + Object *dev;
>>>> + SysBusDevice *sbdev;
>>>> + bool matched = false;
>>>> +
>>>> + dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
>>>> + sbdev = (SysBusDevice *)dev;
>>>> +
>>>> + if (!sbdev) {
>>>> + /* Container, traverse it for children */
>>>> + return object_child_foreach(obj,
>>>> sysbus_device_create_devtree, data);
>>>> + }
>>>> +
>>>> + if (!matched) {
>>>> + error_report("Device %s is not supported by this machine
>>>> yet.",
>>>> + qdev_fw_name(DEVICE(dev)));
>>>> + exit(1);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +void platform_bus_create_devtree(PlatformParams *params, void *fdt,
>>>> + const char *mpic)
>>>> +{
>>>> + gchar *node = g_strdup_printf("/platform@%"PRIx64,
>>>> + params->platform_bus_base);
>>>> + const char platcomp[] = "qemu,platform\0simple-bus";
>>>> + PlatformDevtreeData data;
>>>> + Object *container;
>>>> + uint64_t addr = params->platform_bus_base;
>>>> + uint64_t size = params->platform_bus_size;
>>>> + int irq_start = params->platform_bus_first_irq;
>>>> +
>>>> + /* Create a /platform node that we can put all devices into */
>>>> +
>>>> + qemu_fdt_add_subnode(fdt, node);
>>>> + qemu_fdt_setprop(fdt, node, "compatible", platcomp,
>>>> sizeof(platcomp));
>>>> +
>>>> + /* Our platform bus region is less than 32bit big, so 1 cell is
>>>> enough for
>>>> + address and size */
>>>> + qemu_fdt_setprop_cells(fdt, node, "#size-cells", 1);
>>>> + qemu_fdt_setprop_cells(fdt, node, "#address-cells", 1);
>>>> + qemu_fdt_setprop_cells(fdt, node, "ranges", 0, addr >> 32, addr,
>>>> size);
>>>> +
>>>> + qemu_fdt_setprop_phandle(fdt, node, "interrupt-parent", mpic);
>>>> +
>>>> + /* Loop through all devices and create nodes for known ones */
>>>> + data.fdt = fdt;
>>>> + data.mpic = mpic;
>>>> + data.irq_start = irq_start;
>>>> + data.node = node;
>>>> +
>>>> + container = container_get(qdev_get_machine(), "/peripheral");
>>>> + sysbus_device_create_devtree(container, &data);
>>>> + container = container_get(qdev_get_machine(), "/peripheral-anon");
>>>> + sysbus_device_create_devtree(container, &data);
>>>> +
>>>> + g_free(node);
>>>> +}
>>> Device trees are pretty platform (and even machine) specific. Just to
>>> give you an example - the interrupt specifier on most e500 systems
>>> really is 4 cells big:
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt#n80
>>>
>>>
>>>
>>> | Interrupt specifiers consists of 4 cells encoded as
>>> follows:
>>>
>>> <1st-cell> interrupt-number
>>>
>>> Identifies the interrupt source. The meaning
>>> depends on the type of interrupt.
>>>
>>> Note: If the interrupt-type cell is undefined
>>> (i.e. #interrupt-cells = 2), this cell
>>> should be interpreted the same as for
>>> interrupt-type 0-- i.e. an external or
>>> normal SoC device interrupt.
>>>
>>> <2nd-cell> level-sense information, encoded as follows:
>>> 0 = low-to-high edge triggered
>>> 1 = active low level-sensitive
>>> 2 = active high level-sensitive
>>> 3 = high-to-low edge triggered
>>>
>>> <3rd-cell> interrupt-type
>>>
>>> The following types are supported:
>>>
>>> 0 = external or normal SoC device interrupt
>>>
>>> The interrupt-number cell contains
>>> the SoC device interrupt number. The
>>> type-specific cell is undefined. The
>>> interrupt-number is derived from the
>>> MPIC a block of registers referred to as
>>> the "Interrupt Source Configuration Registers".
>>> Each source has 32-bytes of registers
>>> (vector/priority and destination) in this
>>> region. So interrupt 0 is at offset 0x0,
>>> interrupt 1 is at offset 0x20, and so on.
>>>
>>> 1 = error interrupt
>>>
>>> The interrupt-number cell contains
>>> the SoC device interrupt number for
>>> the error interrupt. The type-specific
>>> cell identifies the specific error
>>> interrupt number.
>>>
>>> 2 = MPIC inter-processor interrupt (IPI)
>>>
>>> The interrupt-number cell identifies
>>> the MPIC IPI number. The type-specific
>>> cell is undefined.
>>>
>>> 3 = MPIC timer interrupt
>>>
>>> The interrupt-number cell identifies
>>> the MPIC timer number. The type-specific
>>> cell is undefined.
>>>
>>> <4th-cell> type-specific information
>>>
>>> The type-specific cell is encoded as follows:
>>>
>>> - For interrupt-type 1 (error interrupt),
>>> the type-specific cell contains the
>>> bit number of the error interrupt in the
>>> Error Interrupt Summary Register.
>>> |
>>>
>>>
>>>
>>>
>>> while on ARM you have a GIC which works like this:
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/gic.txt#n20
>>>
>>>
>>>
>>> |- #interrupt-cells : Specifies the number of cells needed to encode an
>>> interrupt source. The type shall be a <u32> and the value shall
>>> be 3.
>>>
>>> The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI
>>> interrupts.
>>>
>>> The 2nd cell contains the interrupt number for the interrupt type.
>>> SPI interrupts are in the range [0-987]. PPI interrupts are in the
>>> range [0-15].
>>>
>>> The 3rd cell is the flags, encoded as follows:
>>> bits[3:0] trigger type and level flags.
>>> 1 = low-to-high edge triggered
>>> 2 = high-to-low edge triggered
>>> 4 = active high level-sensitive
>>> 8 = active low level-sensitive
>>> bits[15:8] PPI interrupt cpu mask. Each bit corresponds to each of
>>> the 8 possible cpus attached to the GIC. A bit set to '1'
>>> indicated
>>> the interrupt is wired to that CPU. Only valid for PPI interrupts.
>>> |
>>>
>>>
>>>
>>> Both have vastly different semantics. The number of cells is different,
>>> the value of the cells is different. Even the definition how to
>>> represent edge vs level triggered interrupts differs.
>>>
>>> I don't think this will stop with interrupts. Maybe someone wants to add
>>> a special machine check flag to addresses on a platform and then
>>> "ranges" and "regs" will have different semantics on different
>>> platforms. There is a lot that can go wrong when you try to unify this
>>> code.
>> Hi Alex,
>>
>> thank you for giving such an example. Indeed I was not aware there were
>> such huge discrepancies. I guess this comment mostly holds for the
>> actual device node generation (what I specialized in the parent QEMU
>> device) and not for the "qemu, platform simple-bus" node generation?
>
> Is the dt generation that much code? Just duplicate it for now - we can
> generalize it later if we see how things work out.
>
>>
>>>> +
>>>> +int platform_bus_map_irq(PlatformParams *params, SysBusDevice *sbdev,
>>>> + int n, unsigned long *used_irqs,
>>>> + qemu_irq *platform_irqs)
>>>> +{
>>>> + int max_irqs = params->platform_bus_num_irqs;
>>>> + char *prop = g_strdup_printf("irq[%d]", n);
>>>> + int irqn = object_property_get_int(OBJECT(sbdev), prop, NULL);
>>>> +
>>>> + if (irqn == SYSBUS_DYNAMIC) {
>>>> + /* Find the first available IRQ */
>>>> + irqn = find_first_zero_bit(used_irqs, max_irqs);
>>>> + }
>>>> +
>>>> + if ((irqn >= max_irqs) || test_and_set_bit(irqn, used_irqs)) {
>>>> + hw_error("IRQ %d is already allocated or no free IRQ left",
>>>> irqn);
>>>> + }
>>>> +
>>>> + sysbus_connect_irq(sbdev, n, platform_irqs[irqn]);
>>>> + object_property_set_int(OBJECT(sbdev), irqn, prop, NULL);
>>>> +
>>>> + g_free(prop);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +int platform_bus_map_mmio(PlatformParams *params, SysBusDevice *sbdev,
>>>> + int n, unsigned long *used_mem,
>>>> + MemoryRegion *pmem)
>>>> +{
>>>> + MemoryRegion *device_mem = sbdev->mmio[n].memory;
>>>> + uint64_t size = memory_region_size(device_mem);
>>>> + uint64_t page_size = (1 << PAGE_SHIFT);
>>>> + uint64_t page_mask = page_size - 1;
>>>> + uint64_t size_pages = (size + page_mask) >> PAGE_SHIFT;
>>>> + uint64_t max_size = params->platform_bus_size;
>>>> + uint64_t max_pages = max_size >> PAGE_SHIFT;
>>>> + char *prop = g_strdup_printf("mmio[%d]", n);
>>>> + hwaddr addr = object_property_get_int(OBJECT(sbdev), prop, NULL);
>>>> + int page;
>>>> + int i;
>>>> +
>>>> + page = addr >> PAGE_SHIFT;
>>>> + if (addr == SYSBUS_DYNAMIC) {
>>>> + uint64_t size_pages_align;
>>>> +
>>>> + /* Align the region to at least its own size granularity */
>>>> + if (is_power_of_2(size_pages)) {
>>>> + size_pages_align = size_pages;
>>>> + } else {
>>>> + size_pages_align = pow2floor(size_pages) << 1;
>>>> + }
>>>> +
>>>> + /* Find the first available region that fits */
>>>> + page = bitmap_find_next_zero_area(used_mem, max_pages, 0,
>>>> size_pages,
>>>> + size_pages_align);
>>>> +
>>>> + addr = (uint64_t)page << PAGE_SHIFT;
>>>> + }
>>>> +
>>>> + if (page >= max_pages || test_bit(page, used_mem) ||
>>>> + (find_next_bit(used_mem, max_pages, page) < size_pages)) {
>>>> + hw_error("Memory [%"PRIx64":%"PRIx64" is already allocated
>>>> or "
>>>> + "no slot left", addr, size);
>>>> + }
>>>> +
>>>> + for (i = page; i < (page + size_pages); i++) {
>>>> + set_bit(i, used_mem);
>>>> + }
>>>> +
>>>> + memory_region_add_subregion(pmem, addr, device_mem);
>>>> + sbdev->mmio[n].addr = addr;
>>>> + object_property_set_int(OBJECT(sbdev), addr, prop, NULL);
>>>> +
>>>> + g_free(prop);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +int sysbus_device_check(Object *obj, void *opaque)
>>>> +{
>>>> + PlatformBusInitData *init = opaque;
>>>> + Object *dev;
>>>> + SysBusDevice *sbdev;
>>>> + int i;
>>>> +
>>>> + dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
>>>> + sbdev = (SysBusDevice *)dev;
>>>> +
>>>> + if (!sbdev) {
>>>> + /* Container, traverse it for children */
>>>> + return object_child_foreach(obj, sysbus_device_check, opaque);
>>>> + }
>>>> +
>>>> + /* Connect sysbus device to virtual platform bus */
>>>> + for (i = 0; i < sbdev->num_irq; i++) {
>>>> + if (!sbdev->irqp[i]) {
>>>> + /* This IRQ is an incoming IRQ, we can't wire those
>>>> here */
>>>> + continue;
>>>> + }
>>>> + platform_bus_map_irq(init->params, sbdev, i,
>>>> + init->used_irqs, init->irqs);
>>>> + }
>>>> +
>>>> + for (i = 0; i < sbdev->num_mmio; i++) {
>>>> + platform_bus_map_mmio(init->params, sbdev, i,
>>>> + init->used_mem, init->mem);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +void platform_bus_init(PlatformParams *params,
>>>> + MemoryRegion *address_space_mem,
>>>> + qemu_irq *mpic)
>>>> +{
>>>> + uint64_t max_size = params->platform_bus_size;
>>>> + uint64_t max_pages = max_size >> PAGE_SHIFT;
>>>> + DECLARE_BITMAP(used_irqs, params->platform_bus_num_irqs);
>>>> + DECLARE_BITMAP(used_mem, max_pages);
>>>> + MemoryRegion *platform_region = g_new(MemoryRegion, 1);
>>>> + Object *container;
>>>> + PlatformBusInitData init = {
>>>> + .used_irqs = used_irqs,
>>>> + .used_mem = used_mem,
>>>> + .mem = platform_region,
>>>> + .irqs = &mpic[params->platform_bus_first_irq],
>>>> + .params = params,
>>>> + };
>>>> +
>>>> + memory_region_init(platform_region, NULL, "platform devices",
>>>> + params->platform_bus_size);
>>>> +
>>>> + bitmap_clear(used_irqs, 0, params->platform_bus_num_irqs);
>>>> + bitmap_clear(used_mem, 0, max_pages);
>>>> +
>>>> + /* Loop through all sysbus devices that were spawened outside the
>>>> machine */
>>>> + container = container_get(qdev_get_machine(), "/peripheral");
>>>> + sysbus_device_check(container, &init);
>>>> + container = container_get(qdev_get_machine(), "/peripheral-anon");
>>>> + sysbus_device_check(container, &init);
>>>> +
>>>> + memory_region_add_subregion(address_space_mem,
>>>> params->platform_bus_base,
>>>> + platform_region);
>>>> +}
>>> However, I do think it's a good idea to generalize the "platform bus"
>>> device if you want to reuse it on ARM. The mmio / irq allocator is
>>> pretty straight forward and should be generic enough for you to use.
>> I need clarification here: do you talk about your very first patch
>> "Platform Device Support" or the code above with a proper solution for
>> device tree generation?
>
> I'm talking about the actual implementation of the allocation logic. I
> think we're better off to keep all the device tree logic purely in the
> machine files for now.
ok
>
>>> If you do this, please don't duplicate the code but rather move it from
>>> the e500 file into your new one :).
>> OK. do you mean modifying the e500.c code to call those routines? My
>> concern is about testing.
>
> Why? We have a u-boot binary that starts up based on the device tree
> with TCG if you just start the e500plat machine and if you like I can
> easily give you a nice guest kernel and rootfs as well ;).
Definitively if you can afford to give this support, no problem.
Best Regards
Eric
>
>
> Alex
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 2/7] hw/arm/boot: load_dtb becomes non static
2014-07-07 7:08 [Qemu-devel] [PATCH 0/7] machvirt dynamic sysbus device instantiation Eric Auger
2014-07-07 7:08 ` [Qemu-devel] [PATCH 1/7] hw/misc/platform_devices: helpers for dynamic instantiation of platform devices Eric Auger
@ 2014-07-07 7:08 ` Eric Auger
2014-07-07 7:08 ` [Qemu-devel] [PATCH 3/7] hw/arm/virt: add new add_fdt_xxx_node functions Eric Auger
` (4 subsequent siblings)
6 siblings, 0 replies; 23+ messages in thread
From: Eric Auger @ 2014-07-07 7:08 UTC (permalink / raw)
To: eric.auger, christoffer.dall, qemu-devel, kim.phillips, a.rigo
Cc: peter.maydell, eric.auger, patches, agraf, stuart.yoder,
alex.williamson, a.motakis, kvmarm
load_dtb will be used by machvirt for dynamic instantiation of
platform devices
---
hw/arm/boot.c | 2 +-
include/hw/arm/arm.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 3d1f4a2..314bbfd 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -312,7 +312,7 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
}
}
-static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
+int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
{
void *fdt = NULL;
int size, rc;
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index cbbf4ca..fe58dc0 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -68,6 +68,7 @@ struct arm_boot_info {
hwaddr entry;
};
void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info);
+int load_dtb(hwaddr addr, const struct arm_boot_info *binfo);
/* Multiplication factor to convert from system clock ticks to qemu timer
ticks. */
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PATCH 3/7] hw/arm/virt: add new add_fdt_xxx_node functions
2014-07-07 7:08 [Qemu-devel] [PATCH 0/7] machvirt dynamic sysbus device instantiation Eric Auger
2014-07-07 7:08 ` [Qemu-devel] [PATCH 1/7] hw/misc/platform_devices: helpers for dynamic instantiation of platform devices Eric Auger
2014-07-07 7:08 ` [Qemu-devel] [PATCH 2/7] hw/arm/boot: load_dtb becomes non static Eric Auger
@ 2014-07-07 7:08 ` Eric Auger
2014-07-07 7:08 ` [Qemu-devel] [PATCH 4/7] hw/arm/virt: Support dynamically spawned sysbus devices Eric Auger
` (3 subsequent siblings)
6 siblings, 0 replies; 23+ messages in thread
From: Eric Auger @ 2014-07-07 7:08 UTC (permalink / raw)
To: eric.auger, christoffer.dall, qemu-devel, kim.phillips, a.rigo
Cc: peter.maydell, eric.auger, patches, agraf, stuart.yoder,
alex.williamson, a.motakis, kvmarm
Create new functions:
- add_fdt_uart_node
- add_fdt_rtc_node
- add_fdt_virtio_nodes
They will be used for dynamic sysbus instantiation.
Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
hw/arm/virt.c | 67 +++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 44 insertions(+), 23 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 405c61d..eeecdbf 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -330,18 +330,15 @@ static void create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
fdt_add_gic_node(vbi);
}
-static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
+static void fdt_add_uart_node(const VirtBoardInfo *vbi)
{
- char *nodename;
hwaddr base = vbi->memmap[VIRT_UART].base;
hwaddr size = vbi->memmap[VIRT_UART].size;
int irq = vbi->irqmap[VIRT_UART];
const char compat[] = "arm,pl011\0arm,primecell";
const char clocknames[] = "uartclk\0apb_pclk";
+ char *nodename = g_strdup_printf("/pl011@%" PRIx64, base);
- sysbus_create_simple("pl011", base, pic[irq]);
-
- nodename = g_strdup_printf("/pl011@%" PRIx64, base);
qemu_fdt_add_subnode(vbi->fdt, nodename);
/* Note that we can't use setprop_string because of the embedded NUL */
qemu_fdt_setprop(vbi->fdt, nodename, "compatible",
@@ -358,17 +355,23 @@ static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
g_free(nodename);
}
-static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
+static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
+{
+ hwaddr base = vbi->memmap[VIRT_UART].base;
+ int irq = vbi->irqmap[VIRT_UART];
+
+ sysbus_create_simple("pl011", base, pic[irq]);
+ fdt_add_uart_node(vbi);
+}
+
+static void fdt_add_rtc_node(const VirtBoardInfo *vbi)
{
- char *nodename;
hwaddr base = vbi->memmap[VIRT_RTC].base;
hwaddr size = vbi->memmap[VIRT_RTC].size;
int irq = vbi->irqmap[VIRT_RTC];
const char compat[] = "arm,pl031\0arm,primecell";
+ char *nodename = g_strdup_printf("/pl031@%" PRIx64, base);
- sysbus_create_simple("pl031", base, pic[irq]);
-
- nodename = g_strdup_printf("/pl031@%" PRIx64, base);
qemu_fdt_add_subnode(vbi->fdt, nodename);
qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat));
qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
@@ -381,22 +384,20 @@ static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
g_free(nodename);
}
-static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
+static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
{
- int i;
- hwaddr size = vbi->memmap[VIRT_MMIO].size;
+ hwaddr base = vbi->memmap[VIRT_RTC].base;
+ int irq = vbi->irqmap[VIRT_RTC];
- /* Note that we have to create the transports in forwards order
- * so that command line devices are inserted lowest address first,
- * and then add dtb nodes in reverse order so that they appear in
- * the finished device tree lowest address first.
- */
- for (i = 0; i < NUM_VIRTIO_TRANSPORTS; i++) {
- int irq = vbi->irqmap[VIRT_MMIO] + i;
- hwaddr base = vbi->memmap[VIRT_MMIO].base + i * size;
+ sysbus_create_simple("pl031", base, pic[irq]);
- sysbus_create_simple("virtio-mmio", base, pic[irq]);
- }
+ fdt_add_rtc_node(vbi);
+}
+
+static void fdt_add_virtio_nodes(const VirtBoardInfo *vbi)
+{
+ int i;
+ hwaddr size = vbi->memmap[VIRT_MMIO].size;
for (i = NUM_VIRTIO_TRANSPORTS - 1; i >= 0; i--) {
char *nodename;
@@ -416,6 +417,26 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
}
}
+static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
+{
+ int i;
+ hwaddr size = vbi->memmap[VIRT_MMIO].size;
+
+ /* Note that we have to create the transports in forwards order
+ * so that command line devices are inserted lowest address first,
+ * and then add dtb nodes in reverse order so that they appear in
+ * the finished device tree lowest address first.
+ */
+ for (i = 0; i < NUM_VIRTIO_TRANSPORTS; i++) {
+ int irq = vbi->irqmap[VIRT_MMIO] + i;
+ hwaddr base = vbi->memmap[VIRT_MMIO].base + i * size;
+
+ sysbus_create_simple("virtio-mmio", base, pic[irq]);
+ }
+
+ fdt_add_virtio_nodes(vbi);
+}
+
static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
{
const VirtBoardInfo *board = (const VirtBoardInfo *)binfo;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PATCH 4/7] hw/arm/virt: Support dynamically spawned sysbus devices
2014-07-07 7:08 [Qemu-devel] [PATCH 0/7] machvirt dynamic sysbus device instantiation Eric Auger
` (2 preceding siblings ...)
2014-07-07 7:08 ` [Qemu-devel] [PATCH 3/7] hw/arm/virt: add new add_fdt_xxx_node functions Eric Auger
@ 2014-07-07 7:08 ` Eric Auger
2014-07-08 13:51 ` Alexander Graf
2014-07-07 7:08 ` [Qemu-devel] [PATCH 5/7] hw/core/sysbus: add fdt_add_node method Eric Auger
` (2 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Eric Auger @ 2014-07-07 7:08 UTC (permalink / raw)
To: eric.auger, christoffer.dall, qemu-devel, kim.phillips, a.rigo
Cc: peter.maydell, eric.auger, patches, agraf, stuart.yoder,
alex.williamson, a.motakis, kvmarm
Allows sysbus devices to be instantiated from command line by
using -device option
---
Inspired from what Alex Graf did in ppc e500
https://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00012.html
Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
hw/arm/virt.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index eeecdbf..3a21db4 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -40,6 +40,8 @@
#include "exec/address-spaces.h"
#include "qemu/bitops.h"
#include "qemu/error-report.h"
+#include "hw/misc/platform_devices.h"
+#include "hw/vfio/vfio-platform.h"
#define NUM_VIRTIO_TRANSPORTS 32
@@ -57,6 +59,14 @@
#define GIC_FDT_IRQ_PPI_CPU_START 8
#define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
+#define MACHVIRT_PLATFORM_BASE 0xa004000
+#define MACHVIRT_PLATFORM_HOLE (128ULL * 1024 * 1024) /* 128 MB */
+#define MACHVIRT_PLATFORM_PAGE_SHIFT 12
+#define MACHVIRT_PLATFORM_HOLE_PAGES (MACHVIRT_PLATFORM_HOLE >> \
+ MACHVIRT_PLATFORM_PAGE_SHIFT)
+#define MACHVIRT_PLATFORM_FIRST_IRQ 48
+#define MACHVIRT_PLATFORM_NUM_IRQS 20
+
enum {
VIRT_FLASH,
VIRT_MEM,
@@ -66,6 +76,7 @@ enum {
VIRT_UART,
VIRT_MMIO,
VIRT_RTC,
+ VIRT_PLATFORM,
};
typedef struct MemMapEntry {
@@ -108,6 +119,7 @@ static const MemMapEntry a15memmap[] = {
[VIRT_MMIO] = { 0xa000000, 0x200 },
/* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
/* 0x10000000 .. 0x40000000 reserved for PCI */
+ [VIRT_PLATFORM] = {MACHVIRT_PLATFORM_BASE , MACHVIRT_PLATFORM_HOLE},
[VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
};
@@ -115,6 +127,15 @@ static const int a15irqmap[] = {
[VIRT_UART] = 1,
[VIRT_RTC] = 2,
[VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
+ [VIRT_PLATFORM] = MACHVIRT_PLATFORM_FIRST_IRQ,
+};
+
+static PlatformParams machvirt_params = {
+ .has_platform_bus = true,
+ .platform_bus_base = MACHVIRT_PLATFORM_BASE,
+ .platform_bus_size = MACHVIRT_PLATFORM_HOLE,
+ .platform_bus_first_irq = MACHVIRT_PLATFORM_FIRST_IRQ,
+ .platform_bus_num_irqs = MACHVIRT_PLATFORM_NUM_IRQS
};
static VirtBoardInfo machines[] = {
@@ -437,6 +458,18 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
fdt_add_virtio_nodes(vbi);
}
+static void machvirt_prep_device_tree(VirtBoardInfo *vbi)
+{
+ create_fdt(vbi);
+ fdt_add_timer_nodes(vbi);
+ fdt_add_cpu_nodes(vbi);
+ fdt_add_psci_node(vbi);
+ fdt_add_gic_node(vbi);
+ fdt_add_uart_node(vbi);
+ fdt_add_rtc_node(vbi);
+ fdt_add_virtio_nodes(vbi);
+}
+
static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
{
const VirtBoardInfo *board = (const VirtBoardInfo *)binfo;
@@ -445,14 +478,27 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
return board->fdt;
}
+static void machvirt_reset_device_tree(void *opaque)
+{
+ VirtBoardInfo *board = (VirtBoardInfo *)opaque;
+ struct arm_boot_info *info = &board->bootinfo;
+ hwaddr dtb_start = QEMU_ALIGN_UP(info->initrd_start + info->initrd_size,
+ 4096);
+ machvirt_prep_device_tree(board);
+ platform_bus_create_devtree(&machvirt_params, board->fdt, "/intc");
+
+ load_dtb(dtb_start, info);
+}
+
static void machvirt_init(MachineState *machine)
{
- qemu_irq pic[NUM_IRQS];
+ qemu_irq *pic = g_new(qemu_irq, NUM_IRQS);
MemoryRegion *sysmem = get_system_memory();
int n;
MemoryRegion *ram = g_new(MemoryRegion, 1);
const char *cpu_model = machine->cpu_model;
VirtBoardInfo *vbi;
+ PlatformBusNotifier *notifier;
if (!cpu_model) {
cpu_model = "cortex-a15";
@@ -526,6 +572,13 @@ static void machvirt_init(MachineState *machine)
*/
create_virtio_devices(vbi, pic);
+ notifier = g_new(PlatformBusNotifier, 1);
+ notifier->notifier.notify = platform_bus_init_notify;
+ notifier->address_space_mem = sysmem;
+ notifier->mpic = pic;
+ notifier->params = machvirt_params;
+ qemu_add_machine_init_done_notifier(¬ifier->notifier);
+
vbi->bootinfo.ram_size = machine->ram_size;
vbi->bootinfo.kernel_filename = machine->kernel_filename;
vbi->bootinfo.kernel_cmdline = machine->kernel_cmdline;
@@ -535,6 +588,8 @@ static void machvirt_init(MachineState *machine)
vbi->bootinfo.loader_start = vbi->memmap[VIRT_MEM].base;
vbi->bootinfo.get_dtb = machvirt_dtb;
arm_load_kernel(ARM_CPU(first_cpu), &vbi->bootinfo);
+
+ qemu_register_reset(machvirt_reset_device_tree, vbi);
}
static QEMUMachine machvirt_a15_machine = {
@@ -542,6 +597,7 @@ static QEMUMachine machvirt_a15_machine = {
.desc = "ARM Virtual Machine",
.init = machvirt_init,
.max_cpus = 4,
+ .has_dynamic_sysbus = true,
};
static void machvirt_machine_init(void)
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH 4/7] hw/arm/virt: Support dynamically spawned sysbus devices
2014-07-07 7:08 ` [Qemu-devel] [PATCH 4/7] hw/arm/virt: Support dynamically spawned sysbus devices Eric Auger
@ 2014-07-08 13:51 ` Alexander Graf
2014-07-08 13:55 ` Peter Maydell
2014-07-23 15:01 ` Eric Auger
0 siblings, 2 replies; 23+ messages in thread
From: Alexander Graf @ 2014-07-08 13:51 UTC (permalink / raw)
To: Eric Auger, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 07.07.14 09:08, Eric Auger wrote:
> Allows sysbus devices to be instantiated from command line by
> using -device option
>
> ---
>
> Inspired from what Alex Graf did in ppc e500
> https://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00012.html
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
> hw/arm/virt.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index eeecdbf..3a21db4 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -40,6 +40,8 @@
> #include "exec/address-spaces.h"
> #include "qemu/bitops.h"
> #include "qemu/error-report.h"
> +#include "hw/misc/platform_devices.h"
> +#include "hw/vfio/vfio-platform.h"
>
> #define NUM_VIRTIO_TRANSPORTS 32
>
> @@ -57,6 +59,14 @@
> #define GIC_FDT_IRQ_PPI_CPU_START 8
> #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
>
> +#define MACHVIRT_PLATFORM_BASE 0xa004000
That's an odd address for a 128MB window. Can you make it 128MB aligned?
Maybe move the virtio region behind this one?
With a bit of smartness we don't need a virtio-mmio region with this
patch set anymore btw. We could just generate the virtio-mmio devices on
our platform bus on the fly.
> +#define MACHVIRT_PLATFORM_HOLE (128ULL * 1024 * 1024) /* 128 MB */
As Scott mentioned in the e500 review round, "hole" is an odd name ;).
Alex
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 4/7] hw/arm/virt: Support dynamically spawned sysbus devices
2014-07-08 13:51 ` Alexander Graf
@ 2014-07-08 13:55 ` Peter Maydell
2014-07-23 15:01 ` Eric Auger
1 sibling, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-07-08 13:55 UTC (permalink / raw)
To: Alexander Graf
Cc: Kim Phillips, eric.auger, Eric Auger, Patch Tracking,
Alvise Rigo, QEMU Developers, Alex Williamson, Stuart Yoder,
Antonios Motakis, kvmarm, Christoffer Dall
On 8 July 2014 14:51, Alexander Graf <agraf@suse.de> wrote:
> On 07.07.14 09:08, Eric Auger wrote:
>> +#define MACHVIRT_PLATFORM_BASE 0xa004000
>
>
> That's an odd address for a 128MB window. Can you make it 128MB aligned?
> Maybe move the virtio region behind this one?
I'd rather not move things around if we can avoid it. I know in
theory the guest should only ever be looking at the DTB to
find things, but still...
> With a bit of smartness we don't need a virtio-mmio region with this patch
> set anymore btw. We could just generate the virtio-mmio devices on our
> platform bus on the fly.
Hmmm....
thanks
-- PMM
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 4/7] hw/arm/virt: Support dynamically spawned sysbus devices
2014-07-08 13:51 ` Alexander Graf
2014-07-08 13:55 ` Peter Maydell
@ 2014-07-23 15:01 ` Eric Auger
1 sibling, 0 replies; 23+ messages in thread
From: Eric Auger @ 2014-07-23 15:01 UTC (permalink / raw)
To: Alexander Graf, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 07/08/2014 03:51 PM, Alexander Graf wrote:
>
> On 07.07.14 09:08, Eric Auger wrote:
>> Allows sysbus devices to be instantiated from command line by
>> using -device option
>>
>> ---
>>
>> Inspired from what Alex Graf did in ppc e500
>> https://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00012.html
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>> ---
>> hw/arm/virt.c | 58
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 57 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index eeecdbf..3a21db4 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -40,6 +40,8 @@
>> #include "exec/address-spaces.h"
>> #include "qemu/bitops.h"
>> #include "qemu/error-report.h"
>> +#include "hw/misc/platform_devices.h"
>> +#include "hw/vfio/vfio-platform.h"
>> #define NUM_VIRTIO_TRANSPORTS 32
>> @@ -57,6 +59,14 @@
>> #define GIC_FDT_IRQ_PPI_CPU_START 8
>> #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
>> +#define MACHVIRT_PLATFORM_BASE 0xa004000
>
> That's an odd address for a 128MB window. Can you make it 128MB aligned?
> Maybe move the virtio region behind this one?
Yes you're right. I didn't pay attention to that. Now we have to find a
hole agreed with everybody if that's feasible ;-)
>
> With a bit of smartness we don't need a virtio-mmio region with this
> patch set anymore btw. We could just generate the virtio-mmio devices on
> our platform bus on the fly.
>
>> +#define MACHVIRT_PLATFORM_HOLE (128ULL * 1024 * 1024) /* 128
>> MB */
>
> As Scott mentioned in the e500 review round, "hole" is an odd name ;).
OK I will rename that.
>
>
> Alex
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 5/7] hw/core/sysbus: add fdt_add_node method
2014-07-07 7:08 [Qemu-devel] [PATCH 0/7] machvirt dynamic sysbus device instantiation Eric Auger
` (3 preceding siblings ...)
2014-07-07 7:08 ` [Qemu-devel] [PATCH 4/7] hw/arm/virt: Support dynamically spawned sysbus devices Eric Auger
@ 2014-07-07 7:08 ` Eric Auger
2014-07-08 13:52 ` Alexander Graf
2014-07-07 7:08 ` [Qemu-devel] [PATCH 6/7] hw/misc/platform_devices: add call to sysbus fdt_add_node Eric Auger
2014-07-07 7:08 ` [Qemu-devel] [PATCH 7/7] hw/misc/platform_devices: Add platform_bus_base to PlatformDevtreeData Eric Auger
6 siblings, 1 reply; 23+ messages in thread
From: Eric Auger @ 2014-07-07 7:08 UTC (permalink / raw)
To: eric.auger, christoffer.dall, qemu-devel, kim.phillips, a.rigo
Cc: peter.maydell, eric.auger, patches, agraf, stuart.yoder,
alex.williamson, a.motakis, kvmarm
This method is meant to be called on sysbus device dynamic
instantiation (-device option). Devices that support this
kind of instantiation must implement this method.
Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
hw/core/sysbus.c | 12 ++++++++++++
include/hw/sysbus.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index aacc446..c1c0009 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -289,11 +289,23 @@ MemoryRegion *sysbus_address_space(SysBusDevice *dev)
return get_system_memory();
}
+/*
+ * to be specialized in susbus devices that support dynamic instantiation
+ */
+void sysbus_fdt_add_node(SysBusDevice *dev, void *data)
+{
+ error_report("Dynamic instantiation of Device %s"
+ " is not implemented",
+ qdev_fw_name(DEVICE(dev)));
+}
+
static void sysbus_device_class_init(ObjectClass *klass, void *data)
{
DeviceClass *k = DEVICE_CLASS(klass);
+ SysBusDeviceClass *sbdevk = SYS_BUS_DEVICE_CLASS(klass);
k->init = sysbus_device_init;
k->bus_type = TYPE_SYSTEM_BUS;
+ sbdevk->fdt_add_node = sysbus_fdt_add_node;
}
static const TypeInfo sysbus_device_type_info = {
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index 533184a..df514f9 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -40,6 +40,7 @@ typedef struct SysBusDeviceClass {
/*< public >*/
int (*init)(SysBusDevice *dev);
+ void (*fdt_add_node)(SysBusDevice *dev, void *data);
} SysBusDeviceClass;
struct SysBusDevice {
@@ -73,6 +74,7 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr addr,
void sysbus_add_io(SysBusDevice *dev, hwaddr addr,
MemoryRegion *mem);
void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem);
+void sysbus_fdt_add_node(SysBusDevice *dev, void *mem);
MemoryRegion *sysbus_address_space(SysBusDevice *dev);
/* Legacy helper function for creating devices. */
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH 5/7] hw/core/sysbus: add fdt_add_node method
2014-07-07 7:08 ` [Qemu-devel] [PATCH 5/7] hw/core/sysbus: add fdt_add_node method Eric Auger
@ 2014-07-08 13:52 ` Alexander Graf
2014-07-23 15:33 ` Eric Auger
0 siblings, 1 reply; 23+ messages in thread
From: Alexander Graf @ 2014-07-08 13:52 UTC (permalink / raw)
To: Eric Auger, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 07.07.14 09:08, Eric Auger wrote:
> This method is meant to be called on sysbus device dynamic
> instantiation (-device option). Devices that support this
> kind of instantiation must implement this method.
>
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
For the reason I stated earlier, I don't think it's a good idea to put
device tree code into our device models.
Alex
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/core/sysbus: add fdt_add_node method
2014-07-08 13:52 ` Alexander Graf
@ 2014-07-23 15:33 ` Eric Auger
2014-07-23 23:02 ` Alexander Graf
0 siblings, 1 reply; 23+ messages in thread
From: Eric Auger @ 2014-07-23 15:33 UTC (permalink / raw)
To: Alexander Graf, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 07/08/2014 03:52 PM, Alexander Graf wrote:
>
> On 07.07.14 09:08, Eric Auger wrote:
>> This method is meant to be called on sysbus device dynamic
>> instantiation (-device option). Devices that support this
>> kind of instantiation must implement this method.
>>
>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>
> For the reason I stated earlier, I don't think it's a good idea to put
> device tree code into our device models.
Hi Alex,
I would propose we discuss that topic during next KVM call if you are
available. Hope Peter will be available to join too. Because I feel
stuck between not putting things in the machine file (1) - obviously we
could put them in a helper module (2) - and not putting them in the
device (3).
Whatever the solution I fear we are going to pollute something: Any time
a new device wants to support dynamic instantiation, we would need to
modify the machine file or the helper module with 1 and 2 resp. In case
we put it in the device we pollute this latter...
My hope was that quite few QEMU platform devices would need to support
that feature and hence would need to implement this dt node generation
method. To me dynamic instantiation of platform device was not the
mainstream solution.
Then there is the fundamental question of technical feasibility of
devising a generic PlatformParams that match all the specialization
needs? Here I miss experience. In case we know the machine type and a
small set of additional fields couldn't we do the adaptations you talked
about, related to IRQs?
Best Regards
Eric
>
>
> Alex
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/core/sysbus: add fdt_add_node method
2014-07-23 15:33 ` Eric Auger
@ 2014-07-23 23:02 ` Alexander Graf
2014-07-24 7:36 ` Eric Auger
0 siblings, 1 reply; 23+ messages in thread
From: Alexander Graf @ 2014-07-23 23:02 UTC (permalink / raw)
To: Eric Auger, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 23.07.14 17:33, Eric Auger wrote:
> On 07/08/2014 03:52 PM, Alexander Graf wrote:
>> On 07.07.14 09:08, Eric Auger wrote:
>>> This method is meant to be called on sysbus device dynamic
>>> instantiation (-device option). Devices that support this
>>> kind of instantiation must implement this method.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>> For the reason I stated earlier, I don't think it's a good idea to put
>> device tree code into our device models.
> Hi Alex,
>
> I would propose we discuss that topic during next KVM call if you are
> available.
I lost track when that would be. Next week would work fine, the week
after not :).
> Hope Peter will be available to join too. Because I feel
> stuck between not putting things in the machine file (1) - obviously we
> could put them in a helper module (2) - and not putting them in the
> device (3).
>
> Whatever the solution I fear we are going to pollute something: Any time
> a new device wants to support dynamic instantiation, we would need to
> modify the machine file or the helper module with 1 and 2 resp. In case
> we put it in the device we pollute this latter...
>
> My hope was that quite few QEMU platform devices would need to support
> that feature and hence would need to implement this dt node generation
> method. To me dynamic instantiation of platform device was not the
> mainstream solution.
Quite frankly I don't think it'd be that many. I think we'll cover 99.9%
of all use cases if we just enable it for the virt machines of e500 and arm.
> Then there is the fundamental question of technical feasibility of
> devising a generic PlatformParams that match all the specialization
> needs? Here I miss experience. In case we know the machine type and a
> small set of additional fields couldn't we do the adaptations you talked
> about, related to IRQs?
The problem is that I don't know all the boards and different things
people come up with either. There's also no reason machine files have to
stick to the "platform bus" model - they could just take those devices
and stick them into an existing other virtual bus.
I don't feel comfortable generalizing something where I'm pretty sure
things will blow up sooner or later.
Alex
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/core/sysbus: add fdt_add_node method
2014-07-23 23:02 ` Alexander Graf
@ 2014-07-24 7:36 ` Eric Auger
2014-07-24 11:25 ` Alexander Graf
0 siblings, 1 reply; 23+ messages in thread
From: Eric Auger @ 2014-07-24 7:36 UTC (permalink / raw)
To: Alexander Graf, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 07/24/2014 01:02 AM, Alexander Graf wrote:
>
> On 23.07.14 17:33, Eric Auger wrote:
>> On 07/08/2014 03:52 PM, Alexander Graf wrote:
>>> On 07.07.14 09:08, Eric Auger wrote:
>>>> This method is meant to be called on sysbus device dynamic
>>>> instantiation (-device option). Devices that support this
>>>> kind of instantiation must implement this method.
>>>>
>>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>> For the reason I stated earlier, I don't think it's a good idea to put
>>> device tree code into our device models.
>> Hi Alex,
>>
>> I would propose we discuss that topic during next KVM call if you are
>> available.
>
> I lost track when that would be. Next week would work fine, the week
> after not :).
Hi Alex,
Unfortunately I think the last one was this week. If you are available
next week I would propose to setup a short call next week. Who are the
required people in the call aside us and Peter?
>
>> Hope Peter will be available to join too. Because I feel
>> stuck between not putting things in the machine file (1) - obviously we
>> could put them in a helper module (2) - and not putting them in the
>> device (3).
>>
>> Whatever the solution I fear we are going to pollute something: Any time
>> a new device wants to support dynamic instantiation, we would need to
>> modify the machine file or the helper module with 1 and 2 resp. In case
>> we put it in the device we pollute this latter...
>>
>> My hope was that quite few QEMU platform devices would need to support
>> that feature and hence would need to implement this dt node generation
>> method. To me dynamic instantiation of platform device was not the
>> mainstream solution.
>
> Quite frankly I don't think it'd be that many. I think we'll cover 99.9%
> of all use cases if we just enable it for the virt machines of e500 and
> arm.
>
>> Then there is the fundamental question of technical feasibility of
>> devising a generic PlatformParams that match all the specialization
>> needs? Here I miss experience. In case we know the machine type and a
>> small set of additional fields couldn't we do the adaptations you talked
>> about, related to IRQs?
>
> The problem is that I don't know all the boards and different things
> people come up with either. There's also no reason machine files have to
> stick to the "platform bus" model - they could just take those devices
> and stick them into an existing other virtual bus.
>
> I don't feel comfortable generalizing something where I'm pretty sure
> things will blow up sooner or later.
ok
Best Regards
Eric
>
>
> Alex
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/core/sysbus: add fdt_add_node method
2014-07-24 7:36 ` Eric Auger
@ 2014-07-24 11:25 ` Alexander Graf
2014-07-24 12:42 ` Rob Herring
0 siblings, 1 reply; 23+ messages in thread
From: Alexander Graf @ 2014-07-24 11:25 UTC (permalink / raw)
To: Eric Auger, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson,
Scott Wood, a.motakis, Rob Herring, kvmarm
On 24.07.14 09:36, Eric Auger wrote:
> On 07/24/2014 01:02 AM, Alexander Graf wrote:
>> On 23.07.14 17:33, Eric Auger wrote:
>>> On 07/08/2014 03:52 PM, Alexander Graf wrote:
>>>> On 07.07.14 09:08, Eric Auger wrote:
>>>>> This method is meant to be called on sysbus device dynamic
>>>>> instantiation (-device option). Devices that support this
>>>>> kind of instantiation must implement this method.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>> For the reason I stated earlier, I don't think it's a good idea to put
>>>> device tree code into our device models.
>>> Hi Alex,
>>>
>>> I would propose we discuss that topic during next KVM call if you are
>>> available.
>> I lost track when that would be. Next week would work fine, the week
>> after not :).
> Hi Alex,
>
> Unfortunately I think the last one was this week. If you are available
> next week I would propose to setup a short call next week.
Sure!
> Who are the
> required people in the call aside us and Peter?
It would be good if we could have one person on the call who has a very
good understanding of cross-platform device trees. Scott Wood or Rob
Herring come to my mind here.
Scott, Rob, would either of you be available for a quick call on device
tree abstraction levels in QEMU Tuesday next week?
Alex
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] hw/core/sysbus: add fdt_add_node method
2014-07-24 11:25 ` Alexander Graf
@ 2014-07-24 12:42 ` Rob Herring
0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2014-07-24 12:42 UTC (permalink / raw)
To: Alexander Graf
Cc: alex.williamson, Kim Phillips, eric.auger, Eric Auger,
Linaro Patches, a.rigo, QEMU Developers, stuart.yoder,
Scott Wood, Rob Herring, kvmarm, Christoffer Dall
On Thu, Jul 24, 2014 at 6:25 AM, Alexander Graf <agraf@suse.de> wrote:
>
> On 24.07.14 09:36, Eric Auger wrote:
>>
>> On 07/24/2014 01:02 AM, Alexander Graf wrote:
>>>
>>> On 23.07.14 17:33, Eric Auger wrote:
>>>>
>>>> On 07/08/2014 03:52 PM, Alexander Graf wrote:
>>>>>
>>>>> On 07.07.14 09:08, Eric Auger wrote:
>>>>>>
>>>>>> This method is meant to be called on sysbus device dynamic
>>>>>> instantiation (-device option). Devices that support this
>>>>>> kind of instantiation must implement this method.
>>>>>>
>>>>>> Signed-off-by: Eric Auger <eric.auger@linaro.org>
>>>>>
>>>>> For the reason I stated earlier, I don't think it's a good idea to put
>>>>> device tree code into our device models.
>>>>
>>>> Hi Alex,
>>>>
>>>> I would propose we discuss that topic during next KVM call if you are
>>>> available.
>>>
>>> I lost track when that would be. Next week would work fine, the week
>>> after not :).
>>
>> Hi Alex,
>>
>> Unfortunately I think the last one was this week. If you are available
>> next week I would propose to setup a short call next week.
>
>
> Sure!
>
>
>> Who are the
>> required people in the call aside us and Peter?
>
>
> It would be good if we could have one person on the call who has a very good
> understanding of cross-platform device trees. Scott Wood or Rob Herring come
> to my mind here.
>
> Scott, Rob, would either of you be available for a quick call on device tree
> abstraction levels in QEMU Tuesday next week?
Yes, I can.
Rob
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 6/7] hw/misc/platform_devices: add call to sysbus fdt_add_node
2014-07-07 7:08 [Qemu-devel] [PATCH 0/7] machvirt dynamic sysbus device instantiation Eric Auger
` (4 preceding siblings ...)
2014-07-07 7:08 ` [Qemu-devel] [PATCH 5/7] hw/core/sysbus: add fdt_add_node method Eric Auger
@ 2014-07-07 7:08 ` Eric Auger
2014-07-07 7:08 ` [Qemu-devel] [PATCH 7/7] hw/misc/platform_devices: Add platform_bus_base to PlatformDevtreeData Eric Auger
6 siblings, 0 replies; 23+ messages in thread
From: Eric Auger @ 2014-07-07 7:08 UTC (permalink / raw)
To: eric.auger, christoffer.dall, qemu-devel, kim.phillips, a.rigo
Cc: peter.maydell, eric.auger, patches, agraf, stuart.yoder,
alex.williamson, a.motakis, kvmarm
Creation of the node in the device tree relies on the new sysbus
fdt_add_node method.
Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
hw/misc/platform_devices.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/hw/misc/platform_devices.c b/hw/misc/platform_devices.c
index 96ab272..a054606 100644
--- a/hw/misc/platform_devices.c
+++ b/hw/misc/platform_devices.c
@@ -9,7 +9,8 @@ int sysbus_device_create_devtree(Object *obj, void *opaque)
PlatformDevtreeData *data = opaque;
Object *dev;
SysBusDevice *sbdev;
- bool matched = false;
+ SysBusDeviceClass *k;
+
dev = object_dynamic_cast(obj, TYPE_SYS_BUS_DEVICE);
sbdev = (SysBusDevice *)dev;
@@ -19,12 +20,8 @@ int sysbus_device_create_devtree(Object *obj, void *opaque)
return object_child_foreach(obj, sysbus_device_create_devtree, data);
}
- if (!matched) {
- error_report("Device %s is not supported by this machine yet.",
- qdev_fw_name(DEVICE(dev)));
- exit(1);
- }
-
+ k = SYS_BUS_DEVICE_GET_CLASS(dev);
+ k->fdt_add_node(sbdev, data);
return 0;
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [Qemu-devel] [PATCH 7/7] hw/misc/platform_devices: Add platform_bus_base to PlatformDevtreeData
2014-07-07 7:08 [Qemu-devel] [PATCH 0/7] machvirt dynamic sysbus device instantiation Eric Auger
` (5 preceding siblings ...)
2014-07-07 7:08 ` [Qemu-devel] [PATCH 6/7] hw/misc/platform_devices: add call to sysbus fdt_add_node Eric Auger
@ 2014-07-07 7:08 ` Eric Auger
2014-07-08 13:53 ` Alexander Graf
6 siblings, 1 reply; 23+ messages in thread
From: Eric Auger @ 2014-07-07 7:08 UTC (permalink / raw)
To: eric.auger, christoffer.dall, qemu-devel, kim.phillips, a.rigo
Cc: peter.maydell, eric.auger, patches, agraf, stuart.yoder,
alex.williamson, a.motakis, kvmarm
The base address of the platform bus sometimes is used to build the
<reg> property.
---
Actually I did not succeed in doing it another way with Calxeda xgmac.
If someone knows how to do without, please advise.
Signed-off-by: Eric Auger <eric.auger@linaro.org>
---
hw/misc/platform_devices.c | 1 +
include/hw/misc/platform_devices.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/hw/misc/platform_devices.c b/hw/misc/platform_devices.c
index a054606..f194b1d 100644
--- a/hw/misc/platform_devices.c
+++ b/hw/misc/platform_devices.c
@@ -54,6 +54,7 @@ void platform_bus_create_devtree(PlatformParams *params, void *fdt,
data.fdt = fdt;
data.mpic = mpic;
data.irq_start = irq_start;
+ data.platform_bus_base = addr;
data.node = node;
container = container_get(qdev_get_machine(), "/peripheral");
diff --git a/include/hw/misc/platform_devices.h b/include/hw/misc/platform_devices.h
index ab79346..6d228ca 100644
--- a/include/hw/misc/platform_devices.h
+++ b/include/hw/misc/platform_devices.h
@@ -11,6 +11,7 @@ typedef struct PlatformDevtreeData {
const char *mpic;
int irq_start;
const char *node;
+ hwaddr platform_bus_base;
} PlatformDevtreeData;
typedef struct {
--
1.8.3.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Qemu-devel] [PATCH 7/7] hw/misc/platform_devices: Add platform_bus_base to PlatformDevtreeData
2014-07-07 7:08 ` [Qemu-devel] [PATCH 7/7] hw/misc/platform_devices: Add platform_bus_base to PlatformDevtreeData Eric Auger
@ 2014-07-08 13:53 ` Alexander Graf
2014-07-23 15:39 ` Eric Auger
0 siblings, 1 reply; 23+ messages in thread
From: Alexander Graf @ 2014-07-08 13:53 UTC (permalink / raw)
To: Eric Auger, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 07.07.14 09:08, Eric Auger wrote:
> The base address of the platform bus sometimes is used to build the
> <reg> property.
>
> ---
>
> Actually I did not succeed in doing it another way with Calxeda xgmac.
> If someone knows how to do without, please advise.
Not sure I understand. The "regs" properties live inside the parent's
"ranges". So in device tree the only thing that should be aware of the
bus offset is the platform bus node, no?
Alex
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] hw/misc/platform_devices: Add platform_bus_base to PlatformDevtreeData
2014-07-08 13:53 ` Alexander Graf
@ 2014-07-23 15:39 ` Eric Auger
0 siblings, 0 replies; 23+ messages in thread
From: Eric Auger @ 2014-07-23 15:39 UTC (permalink / raw)
To: Alexander Graf, eric.auger, christoffer.dall, qemu-devel,
kim.phillips, a.rigo
Cc: peter.maydell, patches, stuart.yoder, alex.williamson, a.motakis, kvmarm
On 07/08/2014 03:53 PM, Alexander Graf wrote:
>
> On 07.07.14 09:08, Eric Auger wrote:
>> The base address of the platform bus sometimes is used to build the
>> <reg> property.
>>
>> ---
>>
>> Actually I did not succeed in doing it another way with Calxeda xgmac.
>> If someone knows how to do without, please advise.
>
> Not sure I understand. The "regs" properties live inside the parent's
> "ranges". So in device tree the only thing that should be aware of the
> bus offset is the platform bus node, no?
yes I full agree with you as I mentioned in a previous email. I tried to
use <offset> instead of <range> but I never succeeded in making it work.
Maybe a syntax issue, ... I need to spend some more time on it/ get some
help and fix that ...
BR
Eric
>
>
> Alex
>
^ permalink raw reply [flat|nested] 23+ messages in thread