Re: [Qemu-devel] [PULL 10/10] hw/arm: versal: Add a virtual Xilinx Versal board

[Peter Maydell]

On Fri, 2 Nov 2018 at 17:24, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add a virtual Xilinx Versal board.
>
> This board is based on the Xilinx Versal SoC. The exact
> details of what peripherals are attached to this board
> will remain in control of QEMU. QEMU will generate an
> FDT on the fly for Linux and other software to auto-discover
> peripherals.

Hi Edgar; I was just looking at the Versal board code for
something else I was working on, and I noticed a bug that's been
there since it was added in this patch:

> +    s->binfo.ram_size = machine->ram_size;
> +    s->binfo.kernel_filename = machine->kernel_filename;
> +    s->binfo.kernel_cmdline = machine->kernel_cmdline;
> +    s->binfo.initrd_filename = machine->initrd_filename;
> +    s->binfo.loader_start = 0x0;
> +    s->binfo.get_dtb = versal_virt_get_dtb;
> +    s->binfo.modify_dtb = versal_virt_modify_dtb;
> +    if (machine->kernel_filename) {
> +        arm_load_kernel(s->soc.fpd.apu.cpu[0], &s->binfo);
> +    } else {
> +        AddressSpace *as = arm_boot_address_space(s->soc.fpd.apu.cpu[0],
> +                                                  &s->binfo);
> +        /* Some boot-loaders (e.g u-boot) don't like blobs at address 0 
> (NULL).
> +         * Offset things by 4K.  */
> +        s->binfo.loader_start = 0x1000;
> +        s->binfo.dtb_limit = 0x1000000;
> +        if (arm_load_dtb(s->binfo.loader_start,
> +                         &s->binfo, s->binfo.dtb_limit, as) < 0) {
> +            exit(EXIT_FAILURE);
> +        }
> +    }

The board init code only calls arm_load_kernel() if machine->kernel_filename
is set. This is a bug, because calling arm_load_kernel() is mandatory
for board code -- it is the place where we set up the reset handler
that resets the CPUs, so if you don't call it the CPU objects don't
get reset.

thanks
-- PMM