Re: [Qemu-devel] [PATCH v8 2/8] ARM: Samsung exynos4210-based boards emulation

[Peter Maydell]

On 19 January 2012 08:31, Evgeny Voevodin <address@hidden> wrote:

> +    /*
> +     * Secondary CPU startup code will be placed here.
> +     */
> +    memory_region_init_ram(&s->hack_mem, "exynos4210.hack", 0x1000);
> +    memory_region_add_subregion(system_mem, EXYNOS4210_SMP_BOOT_ADDR,
> +            &s->hack_mem);

I've been thinking about this 'hack' memory region, because I figured
out that we didn't actually need it on vexpress (or realview, though
I haven't submitted a patch to fix that yet). Basically the idea is that
we need to put the secondary CPU startup code somewhere where Linux
won't trash it before the secondary cores get started properly. However
this requirement exists also for the real hardware. So the right thing
to do here is identify where the real hardware's boot ROM puts the
secondary CPU holding pen code, and use the same thing. (On vexpress
this is in an area of SDRAM). What does the Exynos4 hardware/bootrom do
here?

> +    /*
> +     * Hack: Map SECOND_CPU_BOOTREG, because it is in PMU USER5 register.
> +     */
> +    memory_region_init_ram(&s->bootreg_mem, "exynos4210.bootreg", 0x4);
> +    memory_region_add_subregion(system_mem, EXYNOS4210_SECOND_CPU_BOOTREG,
> +            &s->bootreg_mem);

If this was modelled as an actual register in a device model with a reset
function we wouldn't have needed the code in arm_boot.c:do_cpu_reset()
that clears smp_bootreg_addr. I think that's an argument for implementing
this as an actual qdev device, however minimal (one that implements
exactly one register would do) rather than as a bit of RAM.

> +static QEMUMachine nuri_machine = {
> +        .name = "nuri",
> +        .desc = "Samsung NURI board (Exynos4210)",
> +        .init = nuri_init,
> +        .max_cpus = EXYNOS4210_MAX_CPUS,
> +};
> +
> +static QEMUMachine smdkc210_machine = {
> +        .name = "smdkc210",
> +        .desc = "Samsung SMDKC210 board (Exynos4210)",
> +        .init = smdkc210_init,
> +        .max_cpus = EXYNOS4210_MAX_CPUS,
> +};

Indentation in these is still wrong.

-- PMM