Re: [Qemu-devel] big endian arm.

[KONRAD Frederic]

On 08/11/2017 12:18 PM, Peter Maydell wrote:
> On 11 August 2017 at 10:59, KONRAD Frederic <address@hidden> wrote:
> > Hi Peters,
> >
> > I got some strange results since this commit:
> >
> > commit 9776f636455b6f0d9c14dce112242ed653f954b4
> > Author: Peter Crosthwaite <address@hidden>
> > Date:   Fri Mar 4 11:30:21 2016 +0000
> >
> >      arm: boot: Support big-endian elfs
> >
> >      Support ARM big-endian ELF files in system-mode emulation. When loading
> >      an elf, determine the endianness mode expected by the elf, and set the
> >      relevant CPU state accordingly.
> >
> >      With this, big-endian modes are now fully supported via system-mode LE,
> >      so there is no need to restrict the elf loading to the TARGET
> >      endianness so the ifdeffery on TARGET_WORDS_BIGENDIAN goes away.
> >
> >      Signed-off-by: Peter Crosthwaite <address@hidden>
> >      Reviewed-by: Peter Maydell <address@hidden>
> >      [PMM: fix typo in comments]
> >      Signed-off-by: Peter Maydell <address@hidden>
> >
> > It seems that when I try to load a big endian image on a
> > Cortex-R5 it gets confused:
> >   * the instructions are fine it executes some code.
> >   * GDB address / insns are wrong endianness.
> >   * in monitor x command is good endianness.
> >   * the data order seems to be wrong endianness:
> >     Here is my Hello World: lleHlroW
>
> R profile bigendian is weird (ie not like A profile) because
> it has a special fudge: SCTLR.IE is an Instruction Endianness
> bit which lets you specify big-endian instruction order (IMPDEF
> how it's set, but for the Cortex-R5 it's an external config
> signal so the SoC can hardwire it as it likes). For A profile
> this bit is always 0 and instructions are LE always. We
> don't implement letting the board model set SCTLR.IE (or
> the code needed to honour it being non-zero).
>
> If your code requires SCTLR.IE to be 1 then you'll need
> to implement that handling (if the instructions are being
> correctly executed then that suggests it doesn't, but
> you should check).

Yes it seems the instruction are OK as they are swapped during
the load.
> > We used to add a armeb-softmmu/qemu-system-armeb target for the
> > big endian cpu but it doesn't work anymore as it's double
> > reversed by the elf endianness (data_swab <= 2).
>
> This was never the right way to do system BE :-)  We should
> fix up whatever the R profile issues you see are...
>
> > And I'm surprised we don't set the CPSR_E and SCTLR_EE bits
> > accordingly?
>
> Setting SCTLR_EE is the job of the board model: a BE board
> model sets the 'cfgend' property on the CPU, which configures
> the reset SCTLR.EE (or SCTLR.B for v5/v6 cores). (You may
> also be able to set -cpu=something,cfgend=true on the command
> line but that's kind of a hack.) SCTLR.EE is then supposed
> to set the CPSR.E on exception entry including reset.
>
> There is I think a bug here though: we set CPSR.E on
> exception entry in arm_cpu_do_interrupt_aarch32():
>
>      /* Set new mode endianness */
>      env->uncached_cpsr &= ~CPSR_E;
>      if (env->cp15.sctlr_el[arm_current_el(env)] & SCTLR_EE) {
>          env->uncached_cpsr |= CPSR_E;
>      }
>
> but we forgot to do anything in arm_cpu_reset() to get
> the CPSR.E bit right on initial reset.
>
> That said, if you specify a BE elf file then we do
> set the SCTLR.EE and CPSR.E bits on reset in do_cpu_reset()
> (a change added in the commit you quote), which is probably
> why we haven't noticed the arm_cpu_reset() bug.

Yes that was what I saw. But seems the bits are not reset anymore
in do_cpu_reset() we probably lost that change.

Fred

> Dunno about the gdbstub stuff.
>
> thanks
> -- PMM