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

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

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 <alistair@alistair23.me>
> ---
>  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.

> +        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...

>      }
>  
>      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;
>  
>