Re: [Qemu-devel] [PATCH 3/4] exynos4210: Set reset-cbar property of Cortex-A9 CPUs

[Peter Crosthwaite]

On Tue, Feb 25, 2014 at 6:24 AM, Peter Maydell <address@hidden> wrote:
> Set the reset-cbar property of the Exynos4210 SoC's Cortex-A9
> CPUs, so that Linux doesn't misrecognize them as a broken
> uniprocessor SoC.
>
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Peter Crosthwaite <address@hidden>

But a couple of random notes below ...

> ---
>  hw/arm/exynos4210.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index 9f137e9..a6f481a 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -143,11 +143,28 @@ Exynos4210State *exynos4210_init(MemoryRegion 
> *system_mem,
>      unsigned long mem_size;
>      DeviceState *dev;
>      SysBusDevice *busdev;
> +    ObjectClass *cpu_oc;
> +
> +    cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, "cortex-a9");
> +    if (!cpu_oc) {
> +        fprintf(stderr, "Unable to find CPU definition\n");
> +        exit(1);
> +    }

Now that this code is ignoring use -cpu I wonder if this should be
promoted to an assertion. The only way this can fail is if someone
rips A9 out of QEMU (or some other nasty fatal condition).

>
>      for (n = 0; n < EXYNOS4210_NCPUS; n++) {
> -        s->cpu[n] = cpu_arm_init("cortex-a9");
> -        if (!s->cpu[n]) {
> -            fprintf(stderr, "Unable to find CPU %d definition\n", n);
> +        Object *cpuobj = object_new(object_class_get_name(cpu_oc));
> +        Error *err = NULL;
> +
> +        s->cpu[n] = ARM_CPU(cpuobj);
> +        object_property_set_int(cpuobj, EXYNOS4210_SMP_PRIVATE_BASE_ADDR,
> +                                "reset-cbar", &err);
> +        if (err) {
> +            error_report("%s", error_get_pretty(err));
> +            exit(1);
> +        }

And this may even be convertible to error_abort as theres no user
accessible way to make this fail.

Regards,
Peter

> +        object_property_set_bool(cpuobj, true, "realized", &err);
> +        if (err) {
> +            error_report("%s", error_get_pretty(err));
>              exit(1);
>          }
>      }
> --
> 1.8.5
>
>