| Message ID | 20230216142338.82982-3-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/cpu/cluster: Restrict CPU cluster to a particular CPU type | expand |
On Thu, 16 Feb 2023 at 14:23, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > Do not recursively add CPU and all their children objects. > Simply iterate on the cluster direct children, which must > be of TYPE_CPU. Otherwise raise an error. The documentation in include/hw/cpu/cluster.h says: * The CPUs may be either direct children of the cluster object, or indirect * children (e.g. children of children of the cluster object). If we want to change that we need to update the documentation too. I'm not sure why this doesn't hit the error on the armsse.c use of TYPE_CLUSTER -- there the objects we have to put in the cluster are TYPE_ARMV7M, which are not themselves TYPE_CPU. They're a container which contains a TYPE_CPU. This is why the docs say that it's OK to have the CPU be an indirect child. I think one of the riscv boards may be also using this facility, but I'm less sure there. thanks -- PMM
diff --git a/hw/cpu/cluster.c b/hw/cpu/cluster.c index bf3e27e945..b0cdf7d931 100644 --- a/hw/cpu/cluster.c +++ b/hw/cpu/cluster.c @@ -39,12 +39,19 @@ typedef struct CallbackData { static bool add_cpu_to_cluster(Object *obj, void *opaque, Error **errp) { CallbackData *cbdata = opaque; - CPUState *cpu = (CPUState *)object_dynamic_cast(obj, TYPE_CPU); + CPUState *cpu; - if (cpu) { - cpu->cluster_index = cbdata->cluster->cluster_id; - cbdata->cpu_count++; + cpu = (CPUState *)object_dynamic_cast(obj, TYPE_CPU); + if (!cpu) { + error_setg(errp, "cluster %s can only accept CPU types (got %s)", + object_get_canonical_path(OBJECT(cbdata->cluster)), + object_get_typename(obj)); + return false; } + + cpu->cluster_index = cbdata->cluster->cluster_id; + cbdata->cpu_count++; + return true; } @@ -63,8 +70,9 @@ static void cpu_cluster_realize(DeviceState *dev, Error **errp) return; } - object_child_foreach_recursive(cluster_obj, add_cpu_to_cluster, - &cbdata, NULL); + if (!object_child_foreach(cluster_obj, add_cpu_to_cluster, &cbdata, errp)) { + return; + } /* * A cluster with no CPUs is a bug in the board/SoC code that created it;
Do not recursively add CPU and all their children objects. Simply iterate on the cluster direct children, which must be of TYPE_CPU. Otherwise raise an error. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/cpu/cluster.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)