QOM prop overloading + ARM MPCore CPUs
cover QOM prop overloading + ARM MPCore CPUs
Hi All,
This series introduced support for multi QOM properties with the same name and then moves the ARM CPUs to the MPCore container objects (yes! they are related!)
The application of the QOM change is container objects passing through a single property on multiple same-type children as a single alias. The immediate use case, is the ARM MPCore where we want to add N cpus but pass through the CPU properties for all of them as an alias on the container itself. The container property setter should fan out to all the CPUs in the container.
Patches 1-5 implement overloaded properties as part of QOM. QOM properties do not allow overloading by default, the creator of the property has to switch it on.
Patch 6 switches this feature on for alias properties which handles the container use case.
Patch 8 is the feature presentation, pulling the CPUs into the ARM MPCore container. This is based on a series of Alistair's to do the same. This version does the extra refactoring to handle the case of multiple CPUs and the problems created around aliases.
Extra discussion points:
The QOM work will probably conflict with Pavel Fedin' work of arrayified properties. So I'll resolve that conflict in a future spin.
Liviu recently brought up a desire for arguments to QOM constructors. P8 would probably be cleaner if this feature existed, as the number of CPUs could be set as a constructor argument. There is no flexibility on when this has to be set, it must be done immediately after construction so it ideally should be part of construction.
My biggest fear is testing of the changes for the affected boards. Peter, do you much coverage of these boards in your regressions? Do you have automated tests in a git repo somewhere?
Regards,
Peter
Peter Crosthwaite (8):
qom: Refactor array property code path
qom: Add property overloading
qom: Implement overloaded property setters
qom: Delete all instances of an overloaded property
qom: Disallow getting/resolving an overloaded property
qom: Enable overloading of Alias properties
arm: realview: Factor out CPU property setters
arm: axxmpcore: Add CPUs to MPCore
hw/arm/exynos4210.c | 72 +++++++----------------------- hw/arm/highbank.c | 65 +++++---------------------- hw/arm/realview.c | 100 +++++++++++++++++++++++------------------ hw/arm/vexpress.c | 71 +++++++----------------------- hw/arm/xilinx_zynq.c | 65 ++++++++++----------------- hw/cpu/a15mpcore.c | 66 ++++++++++++++++++++++++---- hw/cpu/a9mpcore.c | 97 +++++++++++++++++++++++++++++++++++++--- hw/intc/exynos4210_gic.c | 105 -------------------------------------------- include/hw/arm/exynos4210.h | 2 - include/hw/cpu/a15mpcore.h | 2 + include/hw/cpu/a9mpcore.h | 6 +++ include/qom/object.h | 2 + qom/object.c | 96 ++++++++++++++++++++++++---------------- 13 files changed, 341 insertions(+), 408 deletions(-)
Signature · 1 line
2.4.3.3.g905f831
RFC PATCH 1/8 qom: Refactor array property code path
To not be a trial and error based approach. Rather, explicitly scan the existing property lists for the array format strings using a nested strcmp loop.
This prepares support for multiple properties with the same name.
--- qom/object.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
qom/object.c +9 -6
@@ -735,19 +735,22 @@ object_property_add(Object *obj, const char *name, const char *type,int i;ObjectProperty *ret;char *name_no_array =g_strdup (name); +char *full_name; name_no_array[name_len -3 ] ='\0' ; -for (i =0 ; ; ++i) { -char *full_name =g_strdup_printf ("%s[%d]" , name_no_array, i); +for (i =0 ;; ++i) { + full_name =g_strdup_printf ("%s[%d]" , name_no_array, i); - ret =object_property_add (obj, full_name, type, get, set, - release, opaque,NULL ); -g_free (full_name); -if (ret) { +if (!object_property_find (obj, full_name,NULL )) {break ; } +g_free (full_name); }g_free (name_no_array); + + ret =object_property_add (obj, full_name, type, get, set, + release, opaque, &error_abort); +g_free (full_name);return ret; }
Signature · 1 line
2.4.3.3.g905f831
RFC PATCH 2/8 qom: Add property overloading
In reply to Peter CrosthwaiteAdd a mechanism to allow property name overloading. The property being overloaded must explicitly allow it and the property types must match, otherwise an error is returned as normal.
Once the property has been overloaded, set a flag indicating as such, so operations that don't make sense for overloaded properties can raise an error at their time of invocation.
--- include/qom/object.h | 2 ++ qom/object.c | 20 ++++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-)
include/qom/object.h +2 -0
@@ -343,6 +343,8 @@ typedef struct ObjectPropertyObjectPropertyAccessor *set;ObjectPropertyResolve *resolve;ObjectPropertyRelease *release; +bool allows_overloading; +bool overloaded;void *opaque;QTAILQ_ENTRY (ObjectProperty) node;
qom/object.c +12 -8
@@ -728,7 +728,7 @@ object_property_add(Object *obj, const char *name, const char *type, ObjectPropertyRelease *release, void *opaque, Error **errp) { -ObjectProperty *prop; +ObjectProperty *first, *prop;size_t name_len =strlen (name);if (name_len >=3 && !memcmp (name + name_len -3 ,"[*]" ,4 )) {
@@ -754,13 +754,12 @@ object_property_add(Object *obj, const char *name, const char *type,return ret; } - QTAILQ_FOREACH(prop, &obj->properties, node) { -if (strcmp (prop->name, name) ==0 ) { -error_setg (errp,"attempt to add duplicate property '%s'" -" to object (type '%s')" , name, -object_get_typename (obj)); -return NULL ; - } + first =object_property_find (obj, name,NULL ); + +if (first && (!first->allows_overloading ||strcmp (type, first->type))) { +error_setg (errp,"attempt to add duplicate property '%s'" +" to object (type '%s')" , name,object_get_typename (obj)); +return NULL ; } prop =g_malloc0 (sizeof (*prop));
@@ -773,6 +772,11 @@ object_property_add(Object *obj, const char *name, const char *type, prop->release = release; prop->opaque = opaque; +if (first) { + first->overloaded = true; + prop->overloaded = true; + } +QTAILQ_INSERT_TAIL (&obj->properties, prop, node);return prop; }
Signature · 1 line
2.4.3.3.g905f831
RFC PATCH 3/8 qom: Implement overloaded property setters
In reply to Peter CrosthwaiteSet the description/value of all matching properties for a given name.
--- qom/object.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)
qom/object.c +19 -10
@@ -834,14 +834,24 @@ void object_property_set(Object *obj, Visitor *v, const char *name, Error **errp) {ObjectProperty *prop =object_property_find (obj, name, errp); +Error *local_err =NULL ; +if (prop ==NULL ) {return ; } -if (!prop->set) { -error_set (errp,QERR_PERMISSION_DENIED ); - }else { - prop->set (obj, v, prop->opaque, name, errp); + QTAILQ_FOREACH(prop, &obj->properties, node) { +if (strcmp (prop->name, name) ==0 ) { +if (!prop->set) { +error_set (&local_err,QERR_PERMISSION_DENIED ); + }else { + prop->set (obj, v, prop->opaque, name, &local_err); + } + } +if (local_err) { +error_propagate (errp, local_err); +return ; + } } }
@@ -1797,13 +1807,12 @@ void object_property_set_description(Object *obj, const char *name, {ObjectProperty *op; - op =object_property_find (obj, name, errp); -if (!op) { -return ; + QTAILQ_FOREACH(op, &obj->properties, node) { +if (strcmp (op->name, name) ==0 ) { +g_free (op->description); + op->description =g_strdup (description); + } } - -g_free (op->description); - op->description =g_strdup (description); }static void object_instance_init (Object *obj)
Signature · 1 line
2.4.3.3.g905f831
RFC PATCH 4/8 qom: Delete all instances of an overloaded property
In reply to Peter CrosthwaiteIf a property name is overloaded all instances should be deleted by the deleter.
--- qom/object.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)
qom/object.c +14 -12
@@ -798,21 +798,23 @@ ObjectProperty *object_property_find(Object *obj, const char *name,void object_property_del (Object *obj,const char *name,Error **errp) { -ObjectProperty *prop =object_property_find (obj, name, errp); -if (prop ==NULL ) { -return ; - } +for (;;) { +ObjectProperty *prop =object_property_find (obj, name, errp); +if (prop ==NULL ) { +return ; + } -if (prop->release) { - prop->release (obj, name, prop->opaque); - } +if (prop->release) { + prop->release (obj, name, prop->opaque); + } -QTAILQ_REMOVE (&obj->properties, prop, node); +QTAILQ_REMOVE (&obj->properties, prop, node); -g_free (prop->name); -g_free (prop->type); -g_free (prop->description); -g_free (prop); +g_free (prop->name); +g_free (prop->type); +g_free (prop->description); +g_free (prop); + } } void object_property_get(Object *obj, Visitor *v, const char *name,
Signature · 1 line
2.4.3.3.g905f831
RFC PATCH 5/8 qom: Disallow getting/resolving an overloaded property
In reply to Peter CrosthwaiteUsing a getter or trying to resolve an overloaded property is ambiguous. Disallow it.
--- qom/object.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
qom/object.c +4 -1
@@ -827,6 +827,9 @@ void object_property_get(Object *obj, Visitor *v, const char *name,if (!prop->get) {error_set (errp,QERR_PERMISSION_DENIED ); + }else if (prop->overloaded) { +error_setg (errp,"Overloaded property '.%s' getter not supported\n" , + name); }else { prop->get (obj, v, prop->opaque, name, errp); }
@@ -1352,7 +1355,7 @@ gchar *object_get_canonical_path(Object *obj)Object *object_resolve_path_component (Object *parent,const gchar *part) {ObjectProperty *prop =object_property_find (parent, part,NULL ); -if (prop ==NULL ) { +if (prop ==NULL || prop->overloaded) {return NULL ; }
Signature · 1 line
2.4.3.3.g905f831
RFC PATCH 6/8 qom: Enable overloading of Alias properties
In reply to Peter CrosthwaiteSo that container objects can implement multi-way setting aliases of their contained objects.
--- qom/object.c | 1 + 1 file changed, 1 insertion(+)
qom/object.c +1 -0
@@ -1798,6 +1798,7 @@ void object_property_add_alias(Object *obj, const char *name, goto out; } op->resolve = property_resolve_alias; + op->allows_overloading = true; object_property_set_description(obj, op->name, target_prop->description,
Signature · 1 line
2.4.3.3.g905f831
RFC PATCH 7/8 arm: realview: Factor out CPU property setters
In reply to Peter CrosthwaiteInto its own function. This prepares support for cpu-inclusive MPCores which may need to have these props set for them as well.
--- hw/arm/realview.c | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-)
hw/arm/realview.c +27 -19
@@ -45,6 +45,32 @@ static const int realview_board_id[] = { 0x76d }; +static inline void realview_init_cpu_props (Object *obj,hwaddr reset_cbar, +bool do_reset_cbar) +{ +Error *err =NULL ; + +/* By default A9,A15 and ARM1176 CPUs have EL3 enabled. This board +* does not currently support EL3 so the CPU EL3 property is disabled +* before realization. +*/ +if (object_property_find (obj,"has_el3" ,NULL )) { +object_property_set_bool (obj, false,"has_el3" , &err); +if (err) { +error_report_err (err); +exit (1 ); + } + } + +if (do_reset_cbar) { +object_property_set_int (obj, reset_cbar,"reset-cbar" , &err); +if (err) { +error_report_err (err); +exit (1 ); + } + } +} +static void realview_init (MachineState *machine,enum realview_board_type board_type) {
@@ -101,25 +127,7 @@ static void realview_init(MachineState *machine,Object *cpuobj =object_new (object_class_get_name (cpu_oc));Error *err =NULL ; -/* By default A9,A15 and ARM1176 CPUs have EL3 enabled. This board -* does not currently support EL3 so the CPU EL3 property is disabled -* before realization. -*/ -if (object_property_find (cpuobj,"has_el3" ,NULL )) { -object_property_set_bool (cpuobj, false,"has_el3" , &err); -if (err) { -error_report_err (err); -exit (1 ); - } - } - -if (is_pb && is_mpcore) { -object_property_set_int (cpuobj, periphbase,"reset-cbar" , &err); -if (err) { -error_report_err (err); -exit (1 ); - } - } +realview_init_cpu_props (cpuobj, periphbase, is_pb && is_mpcore);object_property_set_bool (cpuobj, true,"realized" , &err);if (err) {
Signature · 1 line
2.4.3.3.g905f831
RFC PATCH 8/8 arm: axxmpcore: Add CPUs to MPCore
In reply to Peter CrosthwaiteAdd the ARM a9/a15 Cortex CPUs to their respective MPCore containers. Update all users or MPCore to not instantiate CPUs on the machine level.
A9 MPCore needs to be extended with the external interrupt controller capability (which ors a set of pins with the GIC CPU IRQs). This is needed by Exynos, which uses the current MPCore/CPU split to manually implement an extra interrupt controller.
Change both a15 and a9 at once to the interface consistent for the Vexpress and Highbank boards.
This defeatures cpu_model override of some of the existing ARM boards. This is ok, as this is invalid for all of these platforms. A board including an A9 or A15 MPCore cannot validly have its CPU type overridden.
Promote property setters of some of the CPU properties to &error abort. With no CPU model flexibility, these shouldn't fail anymore for the way they are used by the fixed machine models.
As part of the change, A9MPCore now supports the oring in of external interrupt controller natively. This obsoletes the Exynos specific IRQ gate logic.
---
Sorry about the big patch. I can't see a way to split this without having to rewrite/delete added code.
hw/arm/exynos4210.c | 72 +++++++----------------------- hw/arm/highbank.c | 65 +++++---------------------- hw/arm/realview.c | 54 +++++++++++++---------- hw/arm/vexpress.c | 71 +++++++----------------------- hw/arm/xilinx_zynq.c | 65 ++++++++++----------------- hw/cpu/a15mpcore.c | 66 ++++++++++++++++++++++++---- hw/cpu/a9mpcore.c | 97 +++++++++++++++++++++++++++++++++++++--- hw/intc/exynos4210_gic.c | 105 -------------------------------------------- include/hw/arm/exynos4210.h | 2 - include/hw/cpu/a15mpcore.h | 2 + include/hw/cpu/a9mpcore.h | 6 +++ 11 files changed, 253 insertions(+), 352 deletions(-)
hw/arm/exynos4210.c +17 -55
@@ -137,73 +137,34 @@ void exynos4210_write_secondary(ARMCPU *cpu,Exynos4210State *exynos4210_init (MemoryRegion *system_mem,unsigned long ram_size) { -int i, n; +int n;Exynos4210State *s =g_new (Exynos4210State,1 ); -qemu_irq gate_irq[EXYNOS4210_NCPUS ][EXYNOS4210_IRQ_GATE_NINPUTS ];unsigned long mem_size; -DeviceState *dev; +DeviceState *dev, *mpcore;SysBusDevice *busdev;ObjectClass *cpu_oc; cpu_oc =cpu_class_by_name (TYPE_ARM_CPU ,"cortex-a9" );assert (cpu_oc); -for (n =0 ; n <EXYNOS4210_NCPUS ; n++) { -Object *cpuobj =object_new (object_class_get_name (cpu_oc)); -Error *err =NULL ; - -/* By default A9 CPUs have EL3 enabled. This board does not currently -* support EL3 so the CPU EL3 property is disabled before realization. -*/ -if (object_property_find (cpuobj,"has_el3" ,NULL )) { -object_property_set_bool (cpuobj, false,"has_el3" , &err); -if (err) { -error_report_err (err); -exit (1 ); - } - } - - s->cpu[n] =ARM_CPU (cpuobj); -object_property_set_int (cpuobj,EXYNOS4210_SMP_PRIVATE_BASE_ADDR , -"reset-cbar" , &error_abort); -object_property_set_bool (cpuobj, true,"realized" , &err); -if (err) { -error_report_err (err); -exit (1 ); - } - } -/*** IRQs ***/ s->irq_table =exynos4210_init_irq (&s->irqs); -/* IRQ Gate */ -for (i =0 ; i <EXYNOS4210_NCPUS ; i++) { - dev =qdev_create (NULL ,"exynos4210.irq_gate" ); -qdev_prop_set_uint32 (dev,"n_in" ,EXYNOS4210_IRQ_GATE_NINPUTS ); -qdev_init_nofail (dev); -/* Get IRQ Gate input in gate_irq */ -for (n =0 ; n <EXYNOS4210_IRQ_GATE_NINPUTS ; n++) { - gate_irq[i][n] =qdev_get_gpio_in (dev, n); - } - busdev =SYS_BUS_DEVICE (dev); - -/* Connect IRQ Gate output to CPU's IRQ line */ -sysbus_connect_irq (busdev,0 , -qdev_get_gpio_in (DEVICE (s->cpu[i]),ARM_CPU_IRQ )); - } - -/* Private memory region and Internal GIC */ - dev =qdev_create (NULL ,"a9mpcore_priv" ); -qdev_prop_set_uint32 (dev,"num-cpu" ,EXYNOS4210_NCPUS ); -qdev_init_nofail (dev); - busdev =SYS_BUS_DEVICE (dev); +/* MPCore (Contains internal GIC) */ + mpcore =qdev_create (NULL ,"a9mpcore_priv" ); +qdev_prop_set_uint32 (mpcore,"num-cpu" ,EXYNOS4210_NCPUS ); +/* By default A9 CPUs have EL3 enabled. This board does not currently +* support EL3 so the CPU EL3 property is disabled before realization. +*/ +object_property_set_bool (OBJECT (mpcore), false,"has_el3" , &error_abort); +object_property_set_int (OBJECT (mpcore),EXYNOS4210_SMP_PRIVATE_BASE_ADDR , +"reset-cbar" , &error_abort); +qdev_init_nofail (mpcore); + busdev =SYS_BUS_DEVICE (mpcore);sysbus_mmio_map (busdev,0 ,EXYNOS4210_SMP_PRIVATE_BASE_ADDR ); -for (n =0 ; n <EXYNOS4210_NCPUS ; n++) { -sysbus_connect_irq (busdev, n, gate_irq[n][0 ]); - }for (n =0 ; n <EXYNOS4210_INT_GIC_NIRQ ; n++) { - s->irqs.int_gic_irq[n] =qdev_get_gpio_in (dev, n); + s->irqs.int_gic_irq[n] =qdev_get_gpio_in (mpcore, n); }/* Cache controller */
@@ -216,10 +177,11 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem, busdev =SYS_BUS_DEVICE (dev);/* Map CPU interface */ sysbus_mmio_map (busdev,0 ,EXYNOS4210_EXT_GIC_CPU_BASE_ADDR ); -/* Map Distributer interface */ +/* Map Distributor interface */ sysbus_mmio_map (busdev,1 ,EXYNOS4210_EXT_GIC_DIST_BASE_ADDR );for (n =0 ; n <EXYNOS4210_NCPUS ; n++) { -sysbus_connect_irq (busdev, n, gate_irq[n][1 ]); +sysbus_connect_irq (busdev, n, +qdev_get_gpio_in_named (mpcore,"ext-interrupt" , n)); }for (n =0 ; n <EXYNOS4210_EXT_GIC_NIRQ ; n++) { s->irqs.ext_gic_irq[n] =qdev_get_gpio_in (dev, n);
hw/arm/highbank.c +10 -55
91 lines not on this page · raw message
hw/arm/realview.c +30 -24
96 lines not on this page · raw message
hw/arm/vexpress.c +17 -54
137 lines not on this page · raw message
hw/arm/xilinx_zynq.c +22 -43
95 lines not on this page · raw message
hw/cpu/a15mpcore.c +57 -9
113 lines not on this page · raw message
hw/cpu/a9mpcore.c +92 -5
141 lines not on this page · raw message
hw/intc/exynos4210_gic.c +0 -105
108 lines not on this page · raw message
include/hw/arm/exynos4210.h +0 -2
@@ -56,7 +56,6 @@/* * exynos4210 IRQ subsystem stub definitions. */ -#define EXYNOS4210_IRQ_GATE_NINPUTS 2/* Internal and External GIC */ #define EXYNOS4210_MAX_INT_COMBINER_OUT_IRQ 64#define EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ 16
@@ -85,7 +84,6 @@ typedef struct Exynos4210Irq { } Exynos4210Irq; typedef struct Exynos4210State { - ARMCPU *cpu[EXYNOS4210_NCPUS]; Exynos4210Irq irqs; qemu_irq *irq_table;
include/hw/cpu/a15mpcore.h +2 -0
@@ -34,6 +34,8 @@ typedef struct A15MPPrivState {SysBusDevice parent_obj;/*< public >*/ +ARMCPU *cpu; +uint32_t num_cpu;uint32_t num_irq;MemoryRegion container;
include/hw/cpu/a9mpcore.h +6 -0
@@ -29,11 +29,17 @@ typedef struct A9MPPrivState {MemoryRegion container;uint32_t num_irq; +ARMCPU *cpu; +A9SCUState scu;GICState gic;A9GTimerState gtimer;ARMMPTimerState mptimer;ARMMPTimerState wdt; + +qemu_irq *cpu_irq; +bool *gic_pin_states; + bool *ext_intc_states; } A9MPPrivState;#endif
Signature · 1 line
2.4.3.3.g905f831
Quoted 8 lines · Peter Crosthwaite
Liviu recently brought up a desire for arguments to QOM constructors. P8 would probably be cleaner if this feature existed, as the number of CPUs could be set as a constructor argument. There is no flexibility on when this has to be set, it must be done immediately after construction so it ideally should be part of construction.
as Peter mentioned in another thread, for some situations instance_init() is too early, realize() is to late.
here is another one, from the implementation of my Cortex-M framework:
DeviceState *mcu = qdev_alloc(NULL, TYPE_STM32F103RB);
{
STM32F103RB_GET_CLASS(mcu)->construct(OBJECT(mcu), machine);
/* Set the board specific oscillator frequencies. */
qdev_prop_set_uint32(mcu, "hse-freq-hz", 8000000); /* 8.0 MHz */
qdev_prop_set_uint32(mcu, "lse-freq-hz", 32768); /* 32 KHz */
}
qdev_realize(mcu);the two properties are in fact aliases to an internal object that handles clocks. doing the MCU construction in .instance_init() is not possible, because it depends on some data not available at that moment, and doing the construction at realize() is too late, because setting properties will no longer be possible after this point.
my solution was to add a custom constructor to each class, and manually chain them up to the parent, as in any OO implementation.
regards,
Liviu
Quoted 14 lines · Peter Crosthwaite
Hi All,
This series introduced support for multi QOM properties with the same name and then moves the ARM CPUs to the MPCore container objects (yes! they are related!)
The application of the QOM change is container objects passing through a single property on multiple same-type children as a single alias. The immediate use case, is the ARM MPCore where we want to add N cpus but pass through the CPU properties for all of them as an alias on the container itself. The container property setter should fan out to all the CPUs in the container.
Quoted 3 lines
Patches 1-5 implement overloaded properties as part of QOM. QOM properties do not allow overloading by default, the creator of the property has to switch it on.
Looks pretty good. I'm just wondering what the logic is for making it off by default. Why not make everything overloadable?
Thanks,
Alistair
Quoted 58 lines
Patch 6 switches this feature on for alias properties which handles the container use case.
Patch 8 is the feature presentation, pulling the CPUs into the ARM MPCore container. This is based on a series of Alistair's to do the same. This version does the extra refactoring to handle the case of multiple CPUs and the problems created around aliases.
Extra discussion points:
The QOM work will probably conflict with Pavel Fedin' work of arrayified properties. So I'll resolve that conflict in a future spin.
Liviu recently brought up a desire for arguments to QOM constructors. P8 would probably be cleaner if this feature existed, as the number of CPUs could be set as a constructor argument. There is no flexibility on when this has to be set, it must be done immediately after construction so it ideally should be part of construction.
My biggest fear is testing of the changes for the affected boards. Peter, do you much coverage of these boards in your regressions? Do you have automated tests in a git repo somewhere?
Regards,
PeterPeter Crosthwaite (8):
qom: Refactor array property code path
qom: Add property overloading
qom: Implement overloaded property setters
qom: Delete all instances of an overloaded property
qom: Disallow getting/resolving an overloaded property
qom: Enable overloading of Alias properties
arm: realview: Factor out CPU property setters
arm: axxmpcore: Add CPUs to MPCorehw/arm/exynos4210.c | 72 +++++++----------------------- hw/arm/highbank.c | 65 +++++---------------------- hw/arm/realview.c | 100 +++++++++++++++++++++++------------------ hw/arm/vexpress.c | 71 +++++++----------------------- hw/arm/xilinx_zynq.c | 65 ++++++++++----------------- hw/cpu/a15mpcore.c | 66 ++++++++++++++++++++++++---- hw/cpu/a9mpcore.c | 97 +++++++++++++++++++++++++++++++++++++--- hw/intc/exynos4210_gic.c | 105 -------------------------------------------- include/hw/arm/exynos4210.h | 2 - include/hw/cpu/a15mpcore.h | 2 + include/hw/cpu/a9mpcore.h | 6 +++ include/qom/object.h | 2 + qom/object.c | 96 ++++++++++++++++++++++++---------------- 13 files changed, 341 insertions(+), 408 deletions(-)--
2.4.3.3.g905f831
On 14 June 2015 at 23:36, Peter Crosthwaite <[email protected]> wrote
Quoted 15 lines
This series introduced support for multi QOM properties with the same name and then moves the ARM CPUs to the MPCore container objects (yes! they are related!)
The application of the QOM change is container objects passing through a single property on multiple same-type children as a single alias. The immediate use case, is the ARM MPCore where we want to add N cpus but pass through the CPU properties for all of them as an alias on the container itself. The container property setter should fan out to all the CPUs in the container.
Patches 1-5 implement overloaded properties as part of QOM. QOM properties do not allow overloading by default, the creator of the property has to switch it on.
Quoted 7 lines
Patch 6 switches this feature on for alias properties which handles the container use case.
Patch 8 is the feature presentation, pulling the CPUs into the ARM MPCore container. This is based on a series of Alistair's to do the same. This version does the extra refactoring to handle the case of multiple CPUs and the problems created around aliases.
Hi. I've been going through my to-review folder cleaning it out, and I found this patchset from back in June in it. Sorry I never got round to reviewing it back then. This is just a note to say that if you care about this series you should rebase it and resend it and I'll have a look at it then.
Quoted 3 lines
My biggest fear is testing of the changes for the affected boards. Peter, do you much coverage of these boards in your regressions? Do you have automated tests in a git repo somewhere?
The answers to these questions are "nowhere near enough" and "unfortunately not"...
thanks
-- PMM
Quoted 24 lines · Peter Maydell
On 14 June 2015 at 23:36, Peter Crosthwaite <[email protected]> wrote
Quoted 15 lines
This series introduced support for multi QOM properties with the same name and then moves the ARM CPUs to the MPCore container objects (yes! they are related!)
The application of the QOM change is container objects passing through a single property on multiple same-type children as a single alias. The immediate use case, is the ARM MPCore where we want to add N cpus but pass through the CPU properties for all of them as an alias on the container itself. The container property setter should fan out to all the CPUs in the container.
Patches 1-5 implement overloaded properties as part of QOM. QOM properties do not allow overloading by default, the creator of the property has to switch it on.
Quoted 7 lines
Patch 6 switches this feature on for alias properties which handles the container use case.
Patch 8 is the feature presentation, pulling the CPUs into the ARM MPCore container. This is based on a series of Alistair's to do the same. This version does the extra refactoring to handle the case of multiple CPUs and the problems created around aliases.
Quoted 6 lines
Hi. I've been going through my to-review folder cleaning it out, and I found this patchset from back in June in it. Sorry I never got round to reviewing it back then. This is just a note to say that if you care about this series you should rebase it and resend it and I'll have a look at it then.
Nice catch, A ping of this was on my todo. Ill add it to the queue of stuff. The other one I have in need of a review is:
http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg05891.html
Quoted 7 lines
Quoted 3 lines
My biggest fear is testing of the changes for the affected boards. Peter, do you much coverage of these boards in your regressions? Do you have automated tests in a git repo somewhere?
The answers to these questions are "nowhere near enough" and "unfortunately not"...
How hard would it be to do something Yocto powered? AFAIK Yocto only supports the one ARM board (Vexpress), three (+ZynqMP, +Zynq) with the Meta-Xilinx layer and there may be more with other layers (anything in meta-linaro?). Can we bitbake something that builds out a large number of ARM machines and tests them all on QEMU?
Regards,
Peter
Quoted 2 lines
thanks
-- PMM
Quoted 3 lines · Peter Crosthwaite
The other one I have in need of a review is:
http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg05891.html
Ah, data-driven device registers. I think I decided I didn't personally care enough about that to put it into my to-review queue in the first place; I was hoping maybe somebody else would tackle it instead...
-- PMM
Quoted 8 lines · Peter Crosthwaite
Quoted 7 lines
Quoted 3 lines
My biggest fear is testing of the changes for the affected boards. Peter, do you much coverage of these boards in your regressions? Do you have automated tests in a git repo somewhere?
The answers to these questions are "nowhere near enough" and "unfortunately not"...
Quoted 5 lines
How hard would it be to do something Yocto powered? AFAIK Yocto only supports the one ARM board (Vexpress), three (+ZynqMP, +Zynq) with the Meta-Xilinx layer and there may be more with other layers (anything in meta-linaro?). Can we bitbake something that builds out a large number of ARM machines and tests them all on QEMU?
Running our standard ARM board tests is a case of:
git clone http://git.yoctoproject.org/git/poky
cd poky
source oe-init-build-env
echo 'INHERIT += "testimage"' >> ./conf/local.conf MACHINE=qemuarm bitbake core-image-sato MACHINE=qemuarm bitbake core-image-sato -c testimage
You could replace core-image-sato -> core-image-minimal for a smaller image and fewer tests or try core-image-sato-sdk or core-image-lsb-sdk for more.
The Quick Start guide is at
http://www.yoctoproject.org/docs/1.8/yocto-project-qs/yocto-project-qs.html and has various things like precanned lists of prerequisites for the package manager.
Not sure which other boards you could try booting but I know the Zaurus machines did work a long time ago as we submitted the qemu code. They are now in their own layer and I've not tried them in a long time.
The above will build its own qemu-native as there are some patches we rely on (like the network fixes). You can point the qemu recipe at different source easily enough.
Cheers,
Richard
Quoted 15 lines · Richard Purdie
Quoted 8 lines · Peter Crosthwaite
Quoted 7 lines
Quoted 3 lines
My biggest fear is testing of the changes for the affected boards. Peter, do you much coverage of these boards in your regressions? Do you have automated tests in a git repo somewhere?
The answers to these questions are "nowhere near enough" and "unfortunately not"...
Quoted 5 lines
How hard would it be to do something Yocto powered? AFAIK Yocto only supports the one ARM board (Vexpress), three (+ZynqMP, +Zynq) with the Meta-Xilinx layer and there may be more with other layers (anything in meta-linaro?). Can we bitbake something that builds out a large number of ARM machines and tests them all on QEMU?
Quoted 9 lines
Running our standard ARM board tests is a case of:
git clone http://git.yoctoproject.org/git/poky
cd poky
source oe-init-build-env
echo 'INHERIT += "testimage"' >>./conf/local.confMACHINE=qemuarm bitbake core-image-sato MACHINE=qemuarm bitbake core-image-sato -c testimage
So qemuarm is implicitly vexpress, I guess we would want to add more, such as qemuarm-zynq, qemuarm-zaurus, qemuarm-virt etc. Can a single bitbake core-image-foo build out multiple MACHINEs or does it not work like that?
Quoted 11 lines
You could replace core-image-sato -> core-image-minimal for a smaller image and fewer tests or try core-image-sato-sdk or core-image-lsb-sdk for more.
The Quick Start guide is at
http://www.yoctoproject.org/docs/1.8/yocto-project-qs/yocto-project-qs.html and has various things like precanned lists of prerequisites for the package manager.Not sure which other boards you could try booting but I know the Zaurus machines did work a long time ago as we submitted the qemu code. They are now in their own layer and I've not tried them in a long time.
Do these multiple vendor layers conflict with each other and is merging all the different ARM machines to poky mainline feasible?
Something else that is on topic, is we should consider changing (subject to backwards compat) the default qemuarm machine to virt, as this machine is well maintained and intended for use as a pure virtual machine (which is intent of Yocto qemu specific target IIUC).
Regards,
Peter
Quoted 8 lines
The above will build its own qemu-native as there are some patches we rely on (like the network fixes). You can point the qemu recipe at different source easily enough.
Cheers,
Richard
Quoted 27 lines · Peter Crosthwaite
Quoted 15 lines · Richard Purdie
Quoted 8 lines · Peter Crosthwaite
Quoted 7 lines
Quoted 3 lines
My biggest fear is testing of the changes for the affected boards. Peter, do you much coverage of these boards in your regressions? Do you have automated tests in a git repo somewhere?
The answers to these questions are "nowhere near enough" and "unfortunately not"...
Quoted 5 lines
How hard would it be to do something Yocto powered? AFAIK Yocto only supports the one ARM board (Vexpress), three (+ZynqMP, +Zynq) with the Meta-Xilinx layer and there may be more with other layers (anything in meta-linaro?). Can we bitbake something that builds out a large number of ARM machines and tests them all on QEMU?
Quoted 9 lines
Running our standard ARM board tests is a case of:
git clone http://git.yoctoproject.org/git/poky
cd poky
source oe-init-build-env
echo 'INHERIT += "testimage"' >>./conf/local.confMACHINE=qemuarm bitbake core-image-sato MACHINE=qemuarm bitbake core-image-sato -c testimage
Quoted 4 lines
So qemuarm is implicitly vexpress, I guess we would want to add more, such as qemuarm-zynq, qemuarm-zaurus, qemuarm-virt etc. Can a single bitbake core-image-foo build out multiple MACHINEs or does it not work like that?
You'd usually just MACHINE=X bitbake <image>; MACHINE=Y bitbake <image>. If the configuration shares a common set of compiler optimisation flags, it will reuse the image binaries.
Quoted 7 lines
Quoted 7 lines
You could replace core-image-sato -> core-image-minimal for a smaller image and fewer tests or try core-image-sato-sdk or core-image-lsb-sdk for more.
The Quick Start guide is at
http://www.yoctoproject.org/docs/1.8/yocto-project-qs/yocto-project-qs.html and has various things like precanned lists of prerequisites for the package manager.
Quoted 6 lines
Quoted 3 lines
Not sure which other boards you could try booting but I know the Zaurus machines did work a long time ago as we submitted the qemu code. They are now in their own layer and I've not tried them in a long time.
Do these multiple vendor layers conflict with each other and is merging all the different ARM machines to poky mainline feasible?
The layer model is intentional like a plugin architecture. OE was once a monolithic repository, it grew too large and painful to work. We therefore now have layers which have separate maintainership. The quality does vary a bit but it did with the monolithic repo too.
In theory you can just plug the layers you want into the core and use them, they shouldn't conflict.
Quoted 4 lines
Something else that is on topic, is we should consider changing (subject to backwards compat) the default qemuarm machine to virt, as this machine is well maintained and intended for use as a pure virtual machine (which is intent of Yocto qemu specific target IIUC).
That would need some wider discussion with the OE community and our kernel maintainers since I know we build a specific qemuarm kernel but in principle it could be done.
There are also qemux86, qemux86-64, qemuppc, qemumips, qemuarm64 and qemumips64 fwiw. I'm not sure we make the best use of qemu in these so I'd be interested in any opinions on what we're doing there... :)
FWIW we did leave qemuarm as an armv5 cpu since we tend to test a lot of of armv7 on real hardware.
Cheers,
Richard