Re: [PATCH v2 2/2] hw/arm/armv7m: Downgrade CPU reset handler priority

[Alistair Francis]

On Thu, Feb 27, 2020 at 5:32 AM Philippe Mathieu-Daudé
<address@hidden> wrote:
>
> Hi Stephanos,
>
> On 2/27/20 12:51 PM, Stephanos Ioannidis wrote:
> > The ARMv7-M CPU reset handler, which loads the initial SP and PC
> > register values from the vector table, is currently executed before
> > the ROM reset handler (rom_reset), and this causes the devices that
> > alias low memory region (e.g. STM32F405 that aliases the flash memory
> > located at 0x8000000 to 0x0) to load an invalid reset vector of 0 when
> > the kernel image is linked to be loaded at the high memory address.
>
> So we have armv7m_load_kernel -> load_elf_as -> rom_add_blob_fixed_as ->
> rom_add_blob -> rom_insert.
>
> arm_cpu_reset is called before rom_reset, rom_ptr is NULL, we call
> initial_pc = ldl_phys(cpu_as) from an empty flash.
>
> Then later rom_reset -> address_space_write_rom.
>
> I think Alistair and myself use the 'loader' device with Cortex-M boards
> and never hit this problem.

I do hit this problem, Peter described a workaround in the previous
version of this patch, that is to link at address 0 instead of the
alias address.

Alistair

>
> >
> > For instance, it is norm for the STM32F405 firmware ELF image to have
> > the text and rodata sections linked at 0x8000000, as this facilitates
> > proper image loading by the firmware burning utility, and the processor
> > can execute in place from the high flash memory address region as well.
> >
> > In order to resolve this issue, this commit downgrades the ARMCPU reset
> > handler invocation priority level to -1 such that it is always executed
> > after the ROM reset handler, which has a priority level of 0.
> >
> > Signed-off-by: Stephanos Ioannidis <address@hidden>
> > ---
> >   hw/arm/armv7m.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> > index 7531b97ccd..8b7c4b12a6 100644
> > --- a/hw/arm/armv7m.c
> > +++ b/hw/arm/armv7m.c
> > @@ -352,7 +352,8 @@ void armv7m_load_kernel(ARMCPU *cpu, const char 
> > *kernel_filename, int mem_size)
> >        * way A-profile does it. Note that this means that every M profile
> >        * board must call this function!
> >        */
> > -    qemu_register_reset(armv7m_reset, cpu);
> > +    qemu_register_reset_with_priority(
> > +        QEMU_RESET_PRIORITY_LEVEL(-1), armv7m_reset, cpu);
> >   }
> >
> >   static Property bitband_properties[] = {
> >
>
>