| Message ID | 7dadfa99c7a31615e6df922fb6e2084837e08033.1408584788.git.alistair23@gmail.com |
|---|---|
| State | New |
| Headers | show |
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 >
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 >> >
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 >>> >>
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
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
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
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]
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
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 >
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
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 >>
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
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 >
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
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 >
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
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 >
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
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
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() };
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(-)