Re: [Qemu-devel] [PATCH v3 2/6] target/arm: Allow setting M mode entry and sp

[Alistair Francis]

On Fri, Jun 21, 2019 at 9:38 AM Philippe Mathieu-Daudé
<address@hidden> wrote:
>
> Hi Alistair,
>
> On 6/19/19 6:54 AM, Alistair Francis wrote:
> > Add M mode initial entry PC and SP properties.
> >
> > Signed-off-by: Alistair Francis <address@hidden>
> > ---
> >  target/arm/cpu.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> >  target/arm/cpu.h |  3 +++
> >  2 files changed, 50 insertions(+)
> >
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index 376db154f0..1d83972ab1 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -301,6 +301,9 @@ static void arm_cpu_reset(CPUState *s)
> >               */
> >              initial_msp = ldl_p(rom);
> >              initial_pc = ldl_p(rom + 4);
> > +        } else if (cpu->init_sp || cpu->init_entry) {
> > +            initial_msp = cpu->init_sp;
> > +            initial_pc = cpu->init_entry;
> >          } else {
> >              /* Address zero not covered by a ROM blob, or the ROM blob
> >               * is in non-modifiable memory and this is a second reset after
> > @@ -801,6 +804,38 @@ static void arm_set_init_svtor(Object *obj, Visitor 
> > *v, const char *name,
> >      visit_type_uint32(v, name, &cpu->init_svtor, errp);
> >  }
> >
> > +static void arm_get_init_sp(Object *obj, Visitor *v, const char *name,
> > +                            void *opaque, Error **errp)
> > +{
> > +    ARMCPU *cpu = ARM_CPU(obj);
> > +
> > +    visit_type_uint32(v, name, &cpu->init_sp, errp);
> > +}
> > +
> > +static void arm_set_init_sp(Object *obj, Visitor *v, const char *name,
> > +                            void *opaque, Error **errp)
> > +{
> > +    ARMCPU *cpu = ARM_CPU(obj);
> > +
> > +    visit_type_uint32(v, name, &cpu->init_sp, errp);
> > +}
> > +
> > +static void arm_get_init_entry(Object *obj, Visitor *v, const char *name,
> > +                            void *opaque, Error **errp)
> > +{
> > +    ARMCPU *cpu = ARM_CPU(obj);
> > +
> > +    visit_type_uint32(v, name, &cpu->init_entry, errp);
> > +}
> > +
> > +static void arm_set_init_entry(Object *obj, Visitor *v, const char *name,
> > +                            void *opaque, Error **errp)
> > +{
> > +    ARMCPU *cpu = ARM_CPU(obj);
> > +
> > +    visit_type_uint32(v, name, &cpu->init_entry, errp);
> > +}
> > +
> >  void arm_cpu_post_init(Object *obj)
> >  {
> >      ARMCPU *cpu = ARM_CPU(obj);
> > @@ -913,6 +948,18 @@ void arm_cpu_post_init(Object *obj)
> >          object_property_add(obj, "init-svtor", "uint32",
> >                              arm_get_init_svtor, arm_set_init_svtor,
> >                              NULL, NULL, &error_abort);
> > +    } else {
> > +        /*
> > +         * M profile: initial value of the SP and entry. We can't just use
> > +         * a simple DEFINE_PROP_UINT32 for this because we want to permit
> > +         * the property to be set after realize.
> > +         */
>
> This comment is mostly a copy of the other if() branch, maybe you can
> extract one generic comment for the 2 cases.

Good point, I have updated it.

>
> > +        object_property_add(obj, "init-sp", "uint32",
> > +                            arm_get_init_sp, arm_set_init_sp,
> > +                            NULL, NULL, &error_abort);
> > +        object_property_add(obj, "init-entry", "uint32",
> > +                            arm_get_init_entry, arm_set_init_entry,
> > +                            NULL, NULL, &error_abort);
>
> I'm having difficulties to test your patch :( I tried:
>
> $ arm-softmmu/qemu-system-arm -M emcraft-sf2 \
>   -device loader,file=/networking.uImage,cpu-num=0 \
>   -d in_asm,int,mmu \
>   -global cpu.init-sp=0x2000fff0 \
>   -global cpu.init-entry=0xa0008001
> PMSA MPU lookup for execute at 0xa0008000 mmu_idx 65 -> Miss (prot rw-)
> Taking exception 3 [Prefetch Abort]
> ...with CFSR.IACCVIOL
> PMSA MPU lookup for writing at 0x2000ffd0 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x2000ffd4 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x2000ffd8 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x2000ffdc mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x2000ffe0 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x2000ffe4 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x2000ffe8 mmu_idx 65 -> Hit (prot rwx)
> PMSA MPU lookup for writing at 0x2000ffec mmu_idx 65 -> Hit (prot rwx)
> ...taking pending nonsecure exception 3
> PMSA MPU lookup for execute at 0x00000000 mmu_idx 67 -> Hit (prot rwx)
> ----------------
> IN:
> PMSA MPU lookup for reading at 0x00000000 mmu_idx 67 -> Hit (prot rwx)
> 0x00000000:  00000000  andeq    r0, r0, r0
>
> Taking exception 18 [v7M INVSTATE UsageFault]
> qemu: fatal: Lockup: can't escalate 3 to HardFault (current priority -1)
>
> R00=00000000 R01=00000000 R02=00000000 R03=00000000
> R04=00000000 R05=00000000 R06=00000000 R07=00000000
> R08=00000000 R09=00000000 R10=00000000 R11=00000000
> R12=00000000 R13=2000ffd0 R14=fffffff9 R15=00000000
> XPSR=40000003 -Z-- A handler
> FPSCR: 00000000
> Aborted (core dumped)
>
> (same without setting cpu.init-entry).
>
> Downloaded "Prebuilt Linux image ready to be loaded to the M2S-FG484
> SOM" here: https://emcraft.com/products/255#software
>
> $ file networking.uImage
> networking.uImage: u-boot legacy uImage, Linux-2.6.33-cortexm-1.14.3,
> Linux/ARM, OS Kernel Image (Not compressed), 2299232 bytes, Wed Nov 11
> 14:19:53 2015, Load Address: 0xA0008000, Entry Point: 0xA0008001, Header
> CRC: 0x419AA120, Data CRC: 0x1C34C4BE
>
> This board memory map is:
>
> (qemu) info mtree
> address-space: memory
>   0000000000000000-ffffffffffffffff (prio -1, i/o): system
>     0000000000000000-000000000003ffff (prio 0, i/o): alias MSF2.eNVM
>     0000000020000000-000000002000ffff (prio 0, ram): MSF2.eSRAM
>     0000000040000000-000000004000001f (prio 0, i/o): serial
>     0000000040001000-000000004000103f (prio 0, i/o): mss-spi
>     0000000040002000-0000000040002fff (prio -1000, i/o): i2c_0
>     0000000040003000-0000000040003fff (prio -1000, i/o): dma
>     0000000040004000-000000004000402f (prio 0, i/o): mss-timer
>     0000000040005000-0000000040005fff (prio -1000, i/o): watchdog
>     0000000040011000-000000004001103f (prio 0, i/o): mss-spi
>     0000000040012000-0000000040012fff (prio -1000, i/o): i2c_1
>     0000000040013000-0000000040013fff (prio -1000, i/o): gpio
>     0000000040014000-0000000040014fff (prio -1000, i/o): hs-dma
>     0000000040015000-0000000040015fff (prio -1000, i/o): can
>     0000000040017000-0000000040017fff (prio -1000, i/o): rtc
>     0000000040020000-000000004002ffff (prio -1000, i/o): apb_config
>     0000000040038000-00000000400382ff (prio 0, i/o): msf2-sysreg
>     0000000040041000-0000000040041fff (prio -1000, i/o): emac
>     0000000040043000-0000000040043fff (prio -1000, i/o): usb
>     0000000060000000-000000006003ffff (prio 0, rom): MSF2.eNVM
>     00000000a0000000-00000000a3ffffff (prio 0, ram): ddr-ram
>
> So I set cpu.init-sp close to the end of the SRAM (0x2000fff0).
>
> Without your patch:
>
> PMSA MPU lookup for execute at 0xa0008000 mmu_idx 65 -> Miss (prot rw-)
> Taking exception 3 [Prefetch Abort]
> ...with CFSR.IACCVIOL
> PMSA MPU lookup for writing at 0xffffffe0 mmu_idx 65 -> Hit (prot rw-)
> ...BusFault with BFSR.STKERR
> ...taking pending nonsecure exception 3
> PMSA MPU lookup for execute at 0x00000000 mmu_idx 67 -> Hit (prot rwx)
> ----------------
> IN:
> PMSA MPU lookup for reading at 0x00000000 mmu_idx 67 -> Hit (prot rwx)
> 0x00000000:  00000000  andeq    r0, r0, r0
>
> Taking exception 18 [v7M INVSTATE UsageFault]
> qemu: fatal: Lockup: can't escalate 3 to HardFault (current priority -1)
>
> R00=00000000 R01=00000000 R02=00000000 R03=00000000
> R04=00000000 R05=00000000 R06=00000000 R07=00000000
> R08=00000000 R09=00000000 R10=00000000 R11=00000000
> R12=00000000 R13=ffffffe0 R14=fffffff9 R15=00000000
> XPSR=40000003 -Z-- A handler
> FPSCR: 00000000
> Aborted (core dumped)
>
> 304             } else if (cpu->init_sp || cpu->init_entry) {
> (gdb)
> 305                 initial_msp = cpu->init_sp;
> (gdb)
> 306                 initial_pc = cpu->init_entry;
> (gdb)
> 317             env->regs[13] = initial_msp & 0xFFFFFFFC;
> (gdb) p/x initial_msp
> $1 = 0x2000fff0
> (gdb) p/x initial_pc
> $2 = 0xa0008001
> (gdb) n
> 318             env->regs[15] = initial_pc & ~1;
> (gdb)
> 319             env->thumb = initial_pc & 1;
>
> I don't understand where I get $pc reset...

Thanks for testing!

I'm a little confused with whats going on here, it doesn't work
without my patch and with my patch it still doesn't work? From the GDB
output it looks like the value setting is working in the reset, are
you sure those are the correct values?

I haven't tested with the emcraft-sf2 machine as I am focused on the
STM. For the STM this is all read when the elf is loaded and I'm not
manually setting anything.

Alistair

>
> >      }
> >
> >      qdev_property_add_static(DEVICE(obj), &arm_cpu_cfgend_property,
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index f9da672be5..290fac19d3 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -805,6 +805,9 @@ struct ARMCPU {
> >       */
> >      uint32_t psci_conduit;
> >
> > +    /* For M, initial value of the entry and SP */
> > +    uint32_t init_sp, init_entry;
> > +
> >      /* For v8M, initial value of the Secure VTOR */
> >      uint32_t init_svtor;
> >
> >