[Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Alistair Francis]

This allows the board to set the reset address, which is required
for some boards (the Netduino Plus 2 for example)

Signed-off-by: Alistair Francis <alistair23@gmail.com>
---
At the moment nothing requires this change, but I have a machine
model that I'm working on that requires this

Thanks to Peter C for spotting this issue

 target-arm/cpu-qom.h | 1 +
 target-arm/cpu.c     | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 07f3c9e..7e415f5 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -138,6 +138,7 @@ typedef struct ARMCPU {
     uint32_t id_isar3;
     uint32_t id_isar4;
     uint32_t id_isar5;
+    uint32_t rom_address;
     uint64_t id_aa64pfr0;
     uint64_t id_aa64pfr1;
     uint64_t id_aa64dfr0;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 8199f32..29f9473 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -134,7 +134,7 @@ static void arm_cpu_reset(CPUState *s)
         uint32_t pc;
         uint8_t *rom;
         env->daif &= ~PSTATE_I;
-        rom = rom_ptr(0);
+        rom = rom_ptr(cpu->rom_address);
         if (rom) {
             /* We should really use ldl_phys here, in case the guest
                modified flash and reset itself.  However images
@@ -1020,6 +1020,7 @@ static const ARMCPUInfo arm_cpus[] = {
 static Property arm_cpu_properties[] = {
     DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false),
     DEFINE_PROP_UINT32("midr", ARMCPU, midr, 0),
+    DEFINE_PROP_UINT32("rom-address", ARMCPU, rom_address, 0),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Alistair Francis]

Add Konstanty


On Thu, Aug 21, 2014 at 11:36 AM, Alistair Francis <alistair23@gmail.com> wrote:
> This allows the board to set the reset address, which is required
> for some boards (the Netduino Plus 2 for example)
>
> Signed-off-by: Alistair Francis <alistair23@gmail.com>
> ---
> At the moment nothing requires this change, but I have a machine
> model that I'm working on that requires this
>
> Thanks to Peter C for spotting this issue
>
>  target-arm/cpu-qom.h | 1 +
>  target-arm/cpu.c     | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 07f3c9e..7e415f5 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -138,6 +138,7 @@ typedef struct ARMCPU {
>      uint32_t id_isar3;
>      uint32_t id_isar4;
>      uint32_t id_isar5;
> +    uint32_t rom_address;
>      uint64_t id_aa64pfr0;
>      uint64_t id_aa64pfr1;
>      uint64_t id_aa64dfr0;
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 8199f32..29f9473 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -134,7 +134,7 @@ static void arm_cpu_reset(CPUState *s)
>          uint32_t pc;
>          uint8_t *rom;
>          env->daif &= ~PSTATE_I;
> -        rom = rom_ptr(0);
> +        rom = rom_ptr(cpu->rom_address);
>          if (rom) {
>              /* We should really use ldl_phys here, in case the guest
>                 modified flash and reset itself.  However images
> @@ -1020,6 +1020,7 @@ static const ARMCPUInfo arm_cpus[] = {
>  static Property arm_cpu_properties[] = {
>      DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false),
>      DEFINE_PROP_UINT32("midr", ARMCPU, midr, 0),
> +    DEFINE_PROP_UINT32("rom-address", ARMCPU, rom_address, 0),
>      DEFINE_PROP_END_OF_LIST()
>  };
>
> --
> 1.9.1
>

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Peter Crosthwaite]

On Thu, Aug 21, 2014 at 11:40 AM, Alistair Francis <alistair23@gmail.com> wrote:
> Add Konstanty
>
>
> On Thu, Aug 21, 2014 at 11:36 AM, Alistair Francis <alistair23@gmail.com> wrote:
>> This allows the board to set the reset address, which is required
>> for some boards (the Netduino Plus 2 for example)
>>

The change is armv7m specific right? You should mention that in the
commit message.

This also obsoletes my attempt at correcting armv7m elf loading which
was trying to solve this issue a different way.

>> Signed-off-by: Alistair Francis <alistair23@gmail.com>

Suggested-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

>> ---
>> At the moment nothing requires this change, but I have a machine
>> model that I'm working on that requires this
>>
>> Thanks to Peter C for spotting this issue
>>
>>  target-arm/cpu-qom.h | 1 +
>>  target-arm/cpu.c     | 3 ++-
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>> index 07f3c9e..7e415f5 100644
>> --- a/target-arm/cpu-qom.h
>> +++ b/target-arm/cpu-qom.h
>> @@ -138,6 +138,7 @@ typedef struct ARMCPU {
>>      uint32_t id_isar3;
>>      uint32_t id_isar4;
>>      uint32_t id_isar5;
>> +    uint32_t rom_address;

Any reason for this to go in amongst the ID register fields? It seems
in a class of its own, and the closest thing to it would be rvbar
which is down the bottom. My gut says it should be last field in the
struct.

>>      uint64_t id_aa64pfr0;
>>      uint64_t id_aa64pfr1;
>>      uint64_t id_aa64dfr0;
>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> index 8199f32..29f9473 100644
>> --- a/target-arm/cpu.c
>> +++ b/target-arm/cpu.c
>> @@ -134,7 +134,7 @@ static void arm_cpu_reset(CPUState *s)
>>          uint32_t pc;
>>          uint8_t *rom;
>>          env->daif &= ~PSTATE_I;
>> -        rom = rom_ptr(0);
>> +        rom = rom_ptr(cpu->rom_address);
>>          if (rom) {
>>              /* We should really use ldl_phys here, in case the guest
>>                 modified flash and reset itself.  However images
>> @@ -1020,6 +1020,7 @@ static const ARMCPUInfo arm_cpus[] = {
>>  static Property arm_cpu_properties[] = {
>>      DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false),
>>      DEFINE_PROP_UINT32("midr", ARMCPU, midr, 0),
>> +    DEFINE_PROP_UINT32("rom-address", ARMCPU, rom_address, 0),

This should be an armv7m only property added in arm_cpu_post_init().

Regards,
Peter

>>      DEFINE_PROP_END_OF_LIST()
>>  };
>>
>> --
>> 1.9.1
>>
>

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Alistair Francis]

On Thu, Aug 21, 2014 at 1:37 PM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Thu, Aug 21, 2014 at 11:40 AM, Alistair Francis <alistair23@gmail.com> wrote:
>> Add Konstanty
>>
>>
>> On Thu, Aug 21, 2014 at 11:36 AM, Alistair Francis <alistair23@gmail.com> wrote:
>>> This allows the board to set the reset address, which is required
>>> for some boards (the Netduino Plus 2 for example)
>>>
>
> The change is armv7m specific right? You should mention that in the
> commit message.
>
> This also obsoletes my attempt at correcting armv7m elf loading which
> was trying to solve this issue a different way.
>
>>> Signed-off-by: Alistair Francis <alistair23@gmail.com>
>
> Suggested-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
>>> ---
>>> At the moment nothing requires this change, but I have a machine
>>> model that I'm working on that requires this
>>>
>>> Thanks to Peter C for spotting this issue
>>>
>>>  target-arm/cpu-qom.h | 1 +
>>>  target-arm/cpu.c     | 3 ++-
>>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>>> index 07f3c9e..7e415f5 100644
>>> --- a/target-arm/cpu-qom.h
>>> +++ b/target-arm/cpu-qom.h
>>> @@ -138,6 +138,7 @@ typedef struct ARMCPU {
>>>      uint32_t id_isar3;
>>>      uint32_t id_isar4;
>>>      uint32_t id_isar5;
>>> +    uint32_t rom_address;
>
> Any reason for this to go in amongst the ID register fields? It seems
> in a class of its own, and the closest thing to it would be rvbar
> which is down the bottom. My gut says it should be last field in the
> struct.
>
>>>      uint64_t id_aa64pfr0;
>>>      uint64_t id_aa64pfr1;
>>>      uint64_t id_aa64dfr0;
>>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>>> index 8199f32..29f9473 100644
>>> --- a/target-arm/cpu.c
>>> +++ b/target-arm/cpu.c
>>> @@ -134,7 +134,7 @@ static void arm_cpu_reset(CPUState *s)
>>>          uint32_t pc;
>>>          uint8_t *rom;
>>>          env->daif &= ~PSTATE_I;
>>> -        rom = rom_ptr(0);
>>> +        rom = rom_ptr(cpu->rom_address);
>>>          if (rom) {
>>>              /* We should really use ldl_phys here, in case the guest
>>>                 modified flash and reset itself.  However images
>>> @@ -1020,6 +1020,7 @@ static const ARMCPUInfo arm_cpus[] = {
>>>  static Property arm_cpu_properties[] = {
>>>      DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false),
>>>      DEFINE_PROP_UINT32("midr", ARMCPU, midr, 0),
>>> +    DEFINE_PROP_UINT32("rom-address", ARMCPU, rom_address, 0),
>
> This should be an armv7m only property added in arm_cpu_post_init().
>
> Regards,
> Peter

I agree with all of your comments, will fix in the next version. I'll
leave it a day or so to see if anyone else has any input

Thanks,

Alistair

>
>>>      DEFINE_PROP_END_OF_LIST()
>>>  };
>>>
>>> --
>>> 1.9.1
>>>
>>

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Andreas Färber]

Am 21.08.2014 05:37, schrieb Peter Crosthwaite:
> On Thu, Aug 21, 2014 at 11:40 AM, Alistair Francis <alistair23@gmail.com> wrote:
>> On Thu, Aug 21, 2014 at 11:36 AM, Alistair Francis <alistair23@gmail.com> wrote:
>>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>>> index 07f3c9e..7e415f5 100644
>>> --- a/target-arm/cpu-qom.h
>>> +++ b/target-arm/cpu-qom.h
>>> @@ -138,6 +138,7 @@ typedef struct ARMCPU {
>>>      uint32_t id_isar3;
>>>      uint32_t id_isar4;
>>>      uint32_t id_isar5;
>>> +    uint32_t rom_address;
> 
> Any reason for this to go in amongst the ID register fields? It seems
> in a class of its own, and the closest thing to it would be rvbar
> which is down the bottom. My gut says it should be last field in the
> struct.

Whether it is zero'ed on reset here would be another concern.

Regards,
Andreas

>>>      uint64_t id_aa64pfr0;
>>>      uint64_t id_aa64pfr1;
>>>      uint64_t id_aa64dfr0;
[snip]

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Peter Maydell]

On 21 August 2014 04:37, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> On Thu, Aug 21, 2014 at 11:40 AM, Alistair Francis <alistair23@gmail.com> wrote:
>> On Thu, Aug 21, 2014 at 11:36 AM, Alistair Francis <alistair23@gmail.com> wrote:
>>> This allows the board to set the reset address, which is required
>>> for some boards (the Netduino Plus 2 for example)

> The change is armv7m specific right? You should mention that in the
> commit message.
>
> This also obsoletes my attempt at correcting armv7m elf loading which
> was trying to solve this issue a different way.

I think the ELF loading thing is a separate issue -- if we're loading
an ELF file we should honour its entry point address instead of
trying to load from the vector table. (The question of what we
do about SP is an interesting one...)

PS: is the patch you refer to currently with you for rework? I don't
see anything relevant in my to-review queue...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Peter Crosthwaite]

On Thu, Aug 21, 2014 at 10:33 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 21 August 2014 04:37, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> On Thu, Aug 21, 2014 at 11:40 AM, Alistair Francis <alistair23@gmail.com> wrote:
>>> On Thu, Aug 21, 2014 at 11:36 AM, Alistair Francis <alistair23@gmail.com> wrote:
>>>> This allows the board to set the reset address, which is required
>>>> for some boards (the Netduino Plus 2 for example)
>
>> The change is armv7m specific right? You should mention that in the
>> commit message.
>>
>> This also obsoletes my attempt at correcting armv7m elf loading which
>> was trying to solve this issue a different way.
>
> I think the ELF loading thing is a separate issue -- if we're loading
> an ELF file we should honour its entry point address instead of
> trying to load from the vector table. (The question of what we
> do about SP is an interesting one...)
>

Well for an elf load you can grab just the SP from VTOR and not the
PC? You will still need to solve the problem of this patch however.

> PS: is the patch you refer to currently with you for rework? I don't
> see anything relevant in my to-review queue...
>

No itis dropped for the moment. I was hoping for the issue to go away
with whatever solution we come up for this.

Regards,
Peter

> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Peter Maydell]

On 21 August 2014 13:37, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> On Thu, Aug 21, 2014 at 10:33 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> I think the ELF loading thing is a separate issue -- if we're loading
>> an ELF file we should honour its entry point address instead of
>> trying to load from the vector table. (The question of what we
>> do about SP is an interesting one...)
>>
>
> Well for an elf load you can grab just the SP from VTOR and not the
> PC? You will still need to solve the problem of this patch however.

As you say, it requires that you can at least load from the vector
table, which is problematic if there's nothing there. It's probably
reasonable to require than M profile boards have something in
the memory map where the vector table lives, though.

-- PMM

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Peter Crosthwaite]

On Thu, Aug 21, 2014 at 10:40 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 21 August 2014 13:37, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> On Thu, Aug 21, 2014 at 10:33 PM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> I think the ELF loading thing is a separate issue -- if we're loading
>>> an ELF file we should honour its entry point address instead of
>>> trying to load from the vector table. (The question of what we
>>> do about SP is an interesting one...)
>>>
>>
>> Well for an elf load you can grab just the SP from VTOR and not the
>> PC? You will still need to solve the problem of this patch however.
>
> As you say, it requires that you can at least load from the vector
> table, which is problematic if there's nothing there. It's probably
> reasonable to require than M profile boards have something in
> the memory map where the vector table lives, though.
>

Yes. I think for Netduino, that will be an alias of another already
existing memory region.

Regards,
Peter

> -- PMM
>

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Christopher Covington]

On 08/21/2014 08:33 AM, Peter Maydell wrote:
> On 21 August 2014 04:37, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> On Thu, Aug 21, 2014 at 11:40 AM, Alistair Francis <alistair23@gmail.com> wrote:
>>> On Thu, Aug 21, 2014 at 11:36 AM, Alistair Francis <alistair23@gmail.com> wrote:
>>>> This allows the board to set the reset address, which is required
>>>> for some boards (the Netduino Plus 2 for example)
> 
>> The change is armv7m specific right? You should mention that in the
>> commit message.
>>
>> This also obsoletes my attempt at correcting armv7m elf loading which
>> was trying to solve this issue a different way.
> 
> I think the ELF loading thing is a separate issue -- if we're loading
> an ELF file we should honour its entry point address instead of
> trying to load from the vector table. (The question of what we
> do about SP is an interesting one...)

In my experience with A-profile, software is expected to either know enough
about the platform to set the SP to something sane itself or make the
SYS_HEAPINFO semihosting call, which despite the name returns both stack and
heap information.

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0471j/pge1358787054284.html

An example of the former option is the semihosting bootwrapper, which
designates an address in RAM as the top of the stack.

https://github.com/virtualopensystems/boot-wrapper/blob/master/model.lds.S#L57

For the latter option, my recollection is that ELF binaries built with Newlib
and ARM's C library will make the semihosting call.

Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Peter Maydell]

On 21 August 2014 14:15, Christopher Covington <cov@codeaurora.org> wrote:
> On 08/21/2014 08:33 AM, Peter Maydell wrote:
>> I think the ELF loading thing is a separate issue -- if we're loading
>> an ELF file we should honour its entry point address instead of
>> trying to load from the vector table. (The question of what we
>> do about SP is an interesting one...)
>
> In my experience with A-profile, software is expected to either know enough
> about the platform to set the SP to something sane itself or make the
> SYS_HEAPINFO semihosting call, which despite the name returns both stack and
> heap information.

My point was that M profile is significantly different from
A profile here. In A profile on CPU reset only the PC is
set to a known value, and therefore any bare metal code
that gets run will always set up SP itself in some way.
In M profile, on CPU reset both PC and SP are read from
the vector table, and so it's reasonable to have bare
metal code for M profile whose entry point assumes that
SP has been initialized.

-- PMM

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Peter Maydell]

On 21 August 2014 02:36, Alistair Francis <alistair23@gmail.com> wrote:
> This allows the board to set the reset address, which is required
> for some boards (the Netduino Plus 2 for example)
>
> Signed-off-by: Alistair Francis <alistair23@gmail.com>

This looks a bit odd -- I'm pretty sure the hardware doesn't
get only the reset address from an IMPDEF location.
Are you sure this board doesn't actually have an IMPDEF
reset value for the vector table offset register VTOR?

Which CPU core is this, anyway? Googling suggests
it's a Cortex-M4, and the M4 TRM suggests that VTOR
always resets to 0.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Alistair Francis]

On Thu, Aug 21, 2014 at 6:01 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 21 August 2014 02:36, Alistair Francis <alistair23@gmail.com> wrote:
>> This allows the board to set the reset address, which is required
>> for some boards (the Netduino Plus 2 for example)
>>
>> Signed-off-by: Alistair Francis <alistair23@gmail.com>
>
> This looks a bit odd -- I'm pretty sure the hardware doesn't
> get only the reset address from an IMPDEF location.
> Are you sure this board doesn't actually have an IMPDEF
> reset value for the vector table offset register VTOR?
>
> Which CPU core is this, anyway? Googling suggests
> it's a Cortex-M4, and the M4 TRM suggests that VTOR
> always resets to 0.

I'm not sure what you mean about the IMPDEF reset value
in the VTOR. The actual board is a Netduino Plus 2, with an
STM32F405RG (which is a Cortex-M4, but I'm making do with
the M3 for the moment).

Not sure if this helps, but to quote the datasheet:
"The Cortex ® -M4 with FPU CPU always fetches the reset
vector on the ICode bus, which implies to have the boot space
available only in the code area (typically, Flash memory)."

The reason this is such a problem is that the Netduino Plus 2
maps memory to 0, but the memory that is mapped there can be changed.
The actual memory mappings are in different locations, the 0 address is more
of a pointer.

Thanks,

Alistair

>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Peter Maydell]

On 21 August 2014 09:35, Alistair Francis <alistair23@gmail.com> wrote:
> On Thu, Aug 21, 2014 at 6:01 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 21 August 2014 02:36, Alistair Francis <alistair23@gmail.com> wrote:
>>> This allows the board to set the reset address, which is required
>>> for some boards (the Netduino Plus 2 for example)
>>>
>>> Signed-off-by: Alistair Francis <alistair23@gmail.com>
>>
>> This looks a bit odd -- I'm pretty sure the hardware doesn't
>> get only the reset address from an IMPDEF location.
>> Are you sure this board doesn't actually have an IMPDEF
>> reset value for the vector table offset register VTOR?
>>
>> Which CPU core is this, anyway? Googling suggests
>> it's a Cortex-M4, and the M4 TRM suggests that VTOR
>> always resets to 0.
>
> I'm not sure what you mean about the IMPDEF reset value
> in the VTOR. The actual board is a Netduino Plus 2, with an
> STM32F405RG (which is a Cortex-M4, but I'm making do with
> the M3 for the moment).

Well, the underlying question is, what is the hardware actually
doing, because I'm 99% certain it's not "magically use a
different address only for reading the reset SP and PC and
nothing else". We need to identify what the h/w behaviour
is that we're not getting right, and implement that.

The suggestion I was making was that one way the CPU
might be reading SP/PC from something other than 0
was if it took the permitted IMPDEF behaviour of having
VTOR reset to something non-zero, in which case that's
the address that SP/PC get read from. (I don't think the
M4 does this, though, so it's probably not the right answer.)

> Not sure if this helps, but to quote the datasheet:
> "The Cortex ® -M4 with FPU CPU always fetches the reset
> vector on the ICode bus, which implies to have the boot space
> available only in the code area (typically, Flash memory)."

Yes, that's standard Cortex-M* behaviour. Are you saying
that this board actually maps different devices at address
zero on the icode and dcode buses (so that an instruction
fetch from address 0 reads different data from a data load
from address 0)? That would be awkward to model for us,
we assume the system design is more conventional and
doesn't show the iside and the dside different views of the
world.

(If it is doing this, then we have problems for instruction
fetch, not just for reset SP and PC loading.)

> The reason this is such a problem is that the Netduino Plus 2
> maps memory to 0, but the memory that is mapped there can be changed.
> The actual memory mappings are in different locations, the 0 address is more
> of a pointer.

This isn't making a great deal of sense to me. If you
mean that the board design allows some sort of
programmable or configurable remapping of the
address space, we should implement that by
mapping and remapping MemoryRegions into place
as appropriate, not by messing with the CPU reset.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Alistair Francis]

On Thu, Aug 21, 2014 at 7:14 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 21 August 2014 09:35, Alistair Francis <alistair23@gmail.com> wrote:
>> On Thu, Aug 21, 2014 at 6:01 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 21 August 2014 02:36, Alistair Francis <alistair23@gmail.com> wrote:
>>>> This allows the board to set the reset address, which is required
>>>> for some boards (the Netduino Plus 2 for example)
>>>>
>>>> Signed-off-by: Alistair Francis <alistair23@gmail.com>
>>>
>>> This looks a bit odd -- I'm pretty sure the hardware doesn't
>>> get only the reset address from an IMPDEF location.
>>> Are you sure this board doesn't actually have an IMPDEF
>>> reset value for the vector table offset register VTOR?
>>>
>>> Which CPU core is this, anyway? Googling suggests
>>> it's a Cortex-M4, and the M4 TRM suggests that VTOR
>>> always resets to 0.
>>
>> I'm not sure what you mean about the IMPDEF reset value
>> in the VTOR. The actual board is a Netduino Plus 2, with an
>> STM32F405RG (which is a Cortex-M4, but I'm making do with
>> the M3 for the moment).
>
> Well, the underlying question is, what is the hardware actually
> doing, because I'm 99% certain it's not "magically use a
> different address only for reading the reset SP and PC and
> nothing else". We need to identify what the h/w behaviour
> is that we're not getting right, and implement that.
>
> The suggestion I was making was that one way the CPU
> might be reading SP/PC from something other than 0
> was if it took the permitted IMPDEF behaviour of having
> VTOR reset to something non-zero, in which case that's
> the address that SP/PC get read from. (I don't think the
> M4 does this, though, so it's probably not the right answer.)
>
>> Not sure if this helps, but to quote the datasheet:
>> "The Cortex ® -M4 with FPU CPU always fetches the reset
>> vector on the ICode bus, which implies to have the boot space
>> available only in the code area (typically, Flash memory)."
>
> Yes, that's standard Cortex-M* behaviour. Are you saying
> that this board actually maps different devices at address
> zero on the icode and dcode buses (so that an instruction
> fetch from address 0 reads different data from a data load
> from address 0)? That would be awkward to model for us,
> we assume the system design is more conventional and
> doesn't show the iside and the dside different views of the
> world.
>
> (If it is doing this, then we have problems for instruction
> fetch, not just for reset SP and PC loading.)
>
>> The reason this is such a problem is that the Netduino Plus 2
>> maps memory to 0, but the memory that is mapped there can be changed.
>> The actual memory mappings are in different locations, the 0 address is more
>> of a pointer.
>
> This isn't making a great deal of sense to me. If you
> mean that the board design allows some sort of
> programmable or configurable remapping of the
> address space, we should implement that by
> mapping and remapping MemoryRegions into place
> as appropriate, not by messing with the CPU reset.

This is the one that I mean.

Either Main Flash, System Flash, FSMC Bank1 or SRAM
are mapped to address 0 based on the value of a register.

This is checked after reset or when the device exits standby.

If trying to run any code on QEMU, it will immediately error with
executing code out of memory at address 0 (if compiled without
this change). What do you think would be the best method to
implement this in QEMU?

Thanks,

Alistair

>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Peter Crosthwaite]

On Thu, Aug 21, 2014 at 10:09 PM, Alistair Francis <alistair23@gmail.com> wrote:
> On Thu, Aug 21, 2014 at 7:14 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 21 August 2014 09:35, Alistair Francis <alistair23@gmail.com> wrote:
>>> On Thu, Aug 21, 2014 at 6:01 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> On 21 August 2014 02:36, Alistair Francis <alistair23@gmail.com> wrote:
>>>>> This allows the board to set the reset address, which is required
>>>>> for some boards (the Netduino Plus 2 for example)
>>>>>
>>>>> Signed-off-by: Alistair Francis <alistair23@gmail.com>
>>>>
>>>> This looks a bit odd -- I'm pretty sure the hardware doesn't
>>>> get only the reset address from an IMPDEF location.
>>>> Are you sure this board doesn't actually have an IMPDEF
>>>> reset value for the vector table offset register VTOR?
>>>>
>>>> Which CPU core is this, anyway? Googling suggests
>>>> it's a Cortex-M4, and the M4 TRM suggests that VTOR
>>>> always resets to 0.
>>>
>>> I'm not sure what you mean about the IMPDEF reset value
>>> in the VTOR. The actual board is a Netduino Plus 2, with an
>>> STM32F405RG (which is a Cortex-M4, but I'm making do with
>>> the M3 for the moment).
>>
>> Well, the underlying question is, what is the hardware actually
>> doing, because I'm 99% certain it's not "magically use a
>> different address only for reading the reset SP and PC and
>> nothing else". We need to identify what the h/w behaviour
>> is that we're not getting right, and implement that.
>>
>> The suggestion I was making was that one way the CPU
>> might be reading SP/PC from something other than 0
>> was if it took the permitted IMPDEF behaviour of having
>> VTOR reset to something non-zero, in which case that's
>> the address that SP/PC get read from. (I don't think the
>> M4 does this, though, so it's probably not the right answer.)
>>
>>> Not sure if this helps, but to quote the datasheet:
>>> "The Cortex ® -M4 with FPU CPU always fetches the reset
>>> vector on the ICode bus, which implies to have the boot space
>>> available only in the code area (typically, Flash memory)."
>>
>> Yes, that's standard Cortex-M* behaviour. Are you saying
>> that this board actually maps different devices at address
>> zero on the icode and dcode buses (so that an instruction
>> fetch from address 0 reads different data from a data load
>> from address 0)? That would be awkward to model for us,
>> we assume the system design is more conventional and
>> doesn't show the iside and the dside different views of the
>> world.
>>
>> (If it is doing this, then we have problems for instruction
>> fetch, not just for reset SP and PC loading.)
>>
>>> The reason this is such a problem is that the Netduino Plus 2
>>> maps memory to 0, but the memory that is mapped there can be changed.
>>> The actual memory mappings are in different locations, the 0 address is more
>>> of a pointer.
>>
>> This isn't making a great deal of sense to me. If you
>> mean that the board design allows some sort of
>> programmable or configurable remapping of the
>> address space, we should implement that by
>> mapping and remapping MemoryRegions into place
>> as appropriate, not by messing with the CPU reset.
>
> This is the one that I mean.
>
> Either Main Flash, System Flash, FSMC Bank1 or SRAM
> are mapped to address 0 based on the value of a register.
>

What's that register? Looking through docs now.

Regards,
Peter

> This is checked after reset or when the device exits standby.
>
> If trying to run any code on QEMU, it will immediately error with
> executing code out of memory at address 0 (if compiled without
> this change). What do you think would be the best method to
> implement this in QEMU?
>
> Thanks,
>
> Alistair
>
>>
>> thanks
>> -- PMM
>

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Alistair Francis]

On Thu, Aug 21, 2014 at 10:22 PM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Thu, Aug 21, 2014 at 10:09 PM, Alistair Francis <alistair23@gmail.com> wrote:
>> On Thu, Aug 21, 2014 at 7:14 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 21 August 2014 09:35, Alistair Francis <alistair23@gmail.com> wrote:
>>>> On Thu, Aug 21, 2014 at 6:01 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>>> On 21 August 2014 02:36, Alistair Francis <alistair23@gmail.com> wrote:
>>>>>> This allows the board to set the reset address, which is required
>>>>>> for some boards (the Netduino Plus 2 for example)
>>>>>>
>>>>>> Signed-off-by: Alistair Francis <alistair23@gmail.com>
>>>>>
>>>>> This looks a bit odd -- I'm pretty sure the hardware doesn't
>>>>> get only the reset address from an IMPDEF location.
>>>>> Are you sure this board doesn't actually have an IMPDEF
>>>>> reset value for the vector table offset register VTOR?
>>>>>
>>>>> Which CPU core is this, anyway? Googling suggests
>>>>> it's a Cortex-M4, and the M4 TRM suggests that VTOR
>>>>> always resets to 0.
>>>>
>>>> I'm not sure what you mean about the IMPDEF reset value
>>>> in the VTOR. The actual board is a Netduino Plus 2, with an
>>>> STM32F405RG (which is a Cortex-M4, but I'm making do with
>>>> the M3 for the moment).
>>>
>>> Well, the underlying question is, what is the hardware actually
>>> doing, because I'm 99% certain it's not "magically use a
>>> different address only for reading the reset SP and PC and
>>> nothing else". We need to identify what the h/w behaviour
>>> is that we're not getting right, and implement that.
>>>
>>> The suggestion I was making was that one way the CPU
>>> might be reading SP/PC from something other than 0
>>> was if it took the permitted IMPDEF behaviour of having
>>> VTOR reset to something non-zero, in which case that's
>>> the address that SP/PC get read from. (I don't think the
>>> M4 does this, though, so it's probably not the right answer.)
>>>
>>>> Not sure if this helps, but to quote the datasheet:
>>>> "The Cortex ® -M4 with FPU CPU always fetches the reset
>>>> vector on the ICode bus, which implies to have the boot space
>>>> available only in the code area (typically, Flash memory)."
>>>
>>> Yes, that's standard Cortex-M* behaviour. Are you saying
>>> that this board actually maps different devices at address
>>> zero on the icode and dcode buses (so that an instruction
>>> fetch from address 0 reads different data from a data load
>>> from address 0)? That would be awkward to model for us,
>>> we assume the system design is more conventional and
>>> doesn't show the iside and the dside different views of the
>>> world.
>>>
>>> (If it is doing this, then we have problems for instruction
>>> fetch, not just for reset SP and PC loading.)
>>>
>>>> The reason this is such a problem is that the Netduino Plus 2
>>>> maps memory to 0, but the memory that is mapped there can be changed.
>>>> The actual memory mappings are in different locations, the 0 address is more
>>>> of a pointer.
>>>
>>> This isn't making a great deal of sense to me. If you
>>> mean that the board design allows some sort of
>>> programmable or configurable remapping of the
>>> address space, we should implement that by
>>> mapping and remapping MemoryRegions into place
>>> as appropriate, not by messing with the CPU reset.
>>
>> This is the one that I mean.
>>
>> Either Main Flash, System Flash, FSMC Bank1 or SRAM
>> are mapped to address 0 based on the value of a register.
>>
>
> What's that register? Looking through docs now.

The register is: SYSCFG_MEMRMP

>
> Regards,
> Peter
>
>> This is checked after reset or when the device exits standby.
>>
>> If trying to run any code on QEMU, it will immediately error with
>> executing code out of memory at address 0 (if compiled without
>> this change). What do you think would be the best method to
>> implement this in QEMU?
>>
>> Thanks,
>>
>> Alistair
>>
>>>
>>> thanks
>>> -- PMM
>>

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Peter Maydell]

On 21 August 2014 13:09, Alistair Francis <alistair23@gmail.com> wrote:
> On Thu, Aug 21, 2014 at 7:14 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This isn't making a great deal of sense to me. If you
>> mean that the board design allows some sort of
>> programmable or configurable remapping of the
>> address space, we should implement that by
>> mapping and remapping MemoryRegions into place
>> as appropriate, not by messing with the CPU reset.
>
> This is the one that I mean.
>
> Either Main Flash, System Flash, FSMC Bank1 or SRAM
> are mapped to address 0 based on the value of a register.
>
> This is checked after reset or when the device exits standby.
>
> If trying to run any code on QEMU, it will immediately error with
> executing code out of memory at address 0 (if compiled without
> this change). What do you think would be the best method to
> implement this in QEMU?

You need to model the behaviour of the board in the
QEMU machine model, so that the right kind of
thing appears at address zero. It doesn't sound like
you need to change the behaviour of the CPU emulation
at all, because reading the PC and SP from 0 is the
correct behaviour.

Since we don't support this board upstream I assume
you're talking about some out-of-tree board model, so
it's a bit difficult to give exact suggestions.

Does the guest code actually make use of the dynamic
remapping, or is it sufficient to implement your board
as "always flash at 0" or "always RAM at 0" ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Peter Crosthwaite]

On Thu, Aug 21, 2014 at 10:24 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 21 August 2014 13:09, Alistair Francis <alistair23@gmail.com> wrote:
>> On Thu, Aug 21, 2014 at 7:14 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> This isn't making a great deal of sense to me. If you
>>> mean that the board design allows some sort of
>>> programmable or configurable remapping of the
>>> address space, we should implement that by
>>> mapping and remapping MemoryRegions into place
>>> as appropriate, not by messing with the CPU reset.
>>
>> This is the one that I mean.
>>
>> Either Main Flash, System Flash, FSMC Bank1 or SRAM
>> are mapped to address 0 based on the value of a register.
>>
>> This is checked after reset or when the device exits standby.
>>
>> If trying to run any code on QEMU, it will immediately error with
>> executing code out of memory at address 0 (if compiled without
>> this change). What do you think would be the best method to
>> implement this in QEMU?
>
> You need to model the behaviour of the board in the
> QEMU machine model, so that the right kind of
> thing appears at address zero. It doesn't sound like
> you need to change the behaviour of the CPU emulation
> at all, because reading the PC and SP from 0 is the
> correct behaviour.
>
> Since we don't support this board upstream I assume
> you're talking about some out-of-tree board model, so
> it's a bit difficult to give exact suggestions.
>
> Does the guest code actually make use of the dynamic
> remapping, or is it sufficient to implement your board
> as "always flash at 0" or "always RAM at 0" ?
>

Well the problem started with elfs that try and load at 0x08000000. I
think I've found some relevant documentation though.

http://www.st.com/st-web-ui/static/active/en/resource/technical/document/datasheet/DM00037051.pdf

page 69.

http://www.st.com/st-web-ui/static/active/en/resource/technical/document/reference_manual/DM00031020.pdf

page 287 (thanks Alistair).

It's a chip-level memory region aliasing system. The elfs are loading
via the actual address but the code or at least the initial PC/SP
retrieval them executes via the @0 alias.

I think the best answer is probably then a board level memory_region
alias. Check memory_region_init_alias.

Regards,
Peter

> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v1 1/1] target_arm: Make the reset rom_ptr a property

[Alistair Francis]

On Thu, Aug 21, 2014 at 10:24 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 21 August 2014 13:09, Alistair Francis <alistair23@gmail.com> wrote:
>> On Thu, Aug 21, 2014 at 7:14 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> This isn't making a great deal of sense to me. If you
>>> mean that the board design allows some sort of
>>> programmable or configurable remapping of the
>>> address space, we should implement that by
>>> mapping and remapping MemoryRegions into place
>>> as appropriate, not by messing with the CPU reset.
>>
>> This is the one that I mean.
>>
>> Either Main Flash, System Flash, FSMC Bank1 or SRAM
>> are mapped to address 0 based on the value of a register.
>>
>> This is checked after reset or when the device exits standby.
>>
>> If trying to run any code on QEMU, it will immediately error with
>> executing code out of memory at address 0 (if compiled without
>> this change). What do you think would be the best method to
>> implement this in QEMU?
>
> You need to model the behaviour of the board in the
> QEMU machine model, so that the right kind of
> thing appears at address zero. It doesn't sound like
> you need to change the behaviour of the CPU emulation
> at all, because reading the PC and SP from 0 is the
> correct behaviour.
>
> Since we don't support this board upstream I assume
> you're talking about some out-of-tree board model, so
> it's a bit difficult to give exact suggestions.
>
> Does the guest code actually make use of the dynamic
> remapping, or is it sufficient to implement your board
> as "always flash at 0" or "always RAM at 0" ?

The guest code sort of uses the dynamic mapping.

If any of the memory regions are moved to zero (instead
of where they should be) the guest fails to run (although
QEMU doesn't return an error).

So it can't just use "always flash at 0", there would need
to be two copies of the same memory.

If the reset address is changed by this patch it runs well.

The board I'm working on can be seen at:
https://github.com/alistair23/qemu

>
> thanks
> -- PMM