diff mbox

MIPS simulator is broken

Message ID 20160112010025.GE4894@vapier.lan
State Committed
Headers show

Commit Message

Mike Frysinger Jan. 12, 2016, 1 a.m. UTC
On 08 Jan 2016 14:30, Steve Ellcey  wrote:
> Your December 26th change to the mips simulator has broken it, at least
> for me.  I am trying to compile and run a program with mips32r2 big-endian
> and am using the mti32.ld linker script.
> 
> After your commit my attempts to run a 'hello world' program die with:
> 
> 
> mips-core: 4 byte read to unmapped address 0xffffffff80020004 at 0xffffffff80020004
> program stopped with signal 10 (User defined signal 1).

the mips address logic was fundamentally broken.  my changes merely
uncovered that braindeadness rather than caused it.  if you look at
that commit and the AddressTranslation definition, you'll see:
  /* For a simple (flat) memory model, we simply pass virtual
     addressess through (mostly) unchanged. */
  vAddr &= 0xFFFFFFFF;

this is forcing all addresses to be 32-bit.  when i saw this, i assumed
it was just a dummy function someone didn't get around to finishing.  i
didn't notice that the sim explicitly sets the address space to be 64-bit
in configure.ac for many targets:
mips_addr_bitsize=
case "${target}" in
  mips*-sde-elf*)       mips_bitsize=64 ; mips_msb=63 ;;
...
  mips*-*-*)            mips_bitsize=32 ; mips_msb=31 ;;
...
SIM_AC_OPTION_BITSIZE($mips_bitsize,$mips_msb,$mips_addr_bitsize)

it passes for me because mips-elf is a 32-bit target where that mask is
actually a nop.  but for targets w/64-bit address space, it's getting a
bad 32-bit sign extended address.  the sim is behaving correctly here:
it should be throwing an error because 0xffffffff80010000 is not mapped.

digging further, it looks like the bug is in bfd, not the sim.  the pc
is set to 0xffffffff80010000 during init:
sim-main.c:sim_create_inferior:
  CPU_PC_SET (cpu, (unsigned64) bfd_get_start_address (abfd));

when we look at the bfd init, bfd_set_start_address is called with that
value as well:
elfcode.h:elf_object_p:
  bfd_set_start_address (abfd, i_ehdrp->e_entry);
(gdb) p *i_ehdrp
$2 = {
  e_ident = "\177ELF\001\002\001\000\000\000\000\000\000\000\000",
  e_entry = 0xffffffff80010000,

e_ehdrp is supposed to be representing an Elf32_Ehdr (by way of the
bfd Elf_Internal_Ehdr structure), and e_entry is supposed to be an
Elf32_Addr, and that's supposed to be uint32_t.

looking at include/elf/internal.h, Elf_Internal_Ehdr is:
  bfd_vma e_entry;    /* Entry point virtual address */
and bfd_vma is set to:
  bfd-in.h:typedef bfd_vma bfd_uint64_t;

so that all looks correct.  poking elf_object_p further, the x_ehdr
(Elf_External_Ehdr) is read in correctly too.

the badness starts here:
  elf_swap_ehdr_in (abfd, &x_ehdr, i_ehdrp);
which does:
  int signed_vma = get_elf_backend_data (abfd)->sign_extend_vma;
  if (signed_vma)
    dst->e_entry = H_GET_SIGNED_WORD (abfd, src->e_entry);
  else
    dst->e_entry = H_GET_WORD (abfd, src->e_entry);

and indeed, the mips bfds (for whatever reason) do:
elf32-mips.c:#define elf_backend_sign_extend_vma    TRUE
elf64-mips.c:#define elf_backend_sign_extend_vma    TRUE
elfn32-mips.c:#define elf_backend_sign_extend_vma   TRUE

i don't know why mips wants to do this, and considering literally no
other target does it (ok, sh64 does it, but let's ignore them since
that target is dead & being removed), it's probably not an accident.
so we have to hack around it in the sim:


that allows most tests to pass, but 6 (out of 69) are still failing.
fpu64-ps.s for example has:
mips-core: 8 byte read to unmapped address 0xffffffff80010b18 at 0x80010070

poking this a bit, i'm not sure if the code is bad (it's actually
doing a load from that address), or the mips decode logic has a bad
sign extension in there.  i'll let someone else figure it out.
-mike

Comments

Mike Frysinger Jan. 12, 2016, 6:49 a.m. UTC | #1
On 11 Jan 2016 20:00, Mike Frysinger wrote:
> i don't know why mips wants to do this, and considering literally no
> other target does it (ok, sh64 does it, but let's ignore them since
> that target is dead & being removed), it's probably not an accident.
> so we have to hack around it in the sim:

i've pushed that fix now

> that allows most tests to pass, but 6 (out of 69) are still failing.
> fpu64-ps.s for example has:
> mips-core: 8 byte read to unmapped address 0xffffffff80010b18 at 0x80010070

this one i punted to a bug:
https://sourceware.org/bugzilla/show_bug.cgi?id=19447

someone who cares about the mips port will have to take a look
-mike
Pedro Alves Jan. 12, 2016, 10:25 a.m. UTC | #2
On 01/12/2016 01:00 AM, Mike Frysinger wrote:

> and indeed, the mips bfds (for whatever reason) do:
> elf32-mips.c:#define elf_backend_sign_extend_vma    TRUE
> elf64-mips.c:#define elf_backend_sign_extend_vma    TRUE
> elfn32-mips.c:#define elf_backend_sign_extend_vma   TRUE
> 
> i don't know why mips wants to do this, and considering literally no
> other target does it (ok, sh64 does it, but let's ignore them since
> that target is dead & being removed), it's probably not an accident.
> so we have to hack around it in the sim:

AFAIK, that's just how the machine works:

  https://sourceware.org/ml/binutils/2002-09/msg00135.html

Thanks,
Pedro Alves
Maciej W. Rozycki Jan. 30, 2016, 4:09 p.m. UTC | #3
Mike,

> > mips-core: 4 byte read to unmapped address 0xffffffff80020004 at 0xffffffff80020004
> > program stopped with signal 10 (User defined signal 1).
> 
> the mips address logic was fundamentally broken.  my changes merely
> uncovered that braindeadness rather than caused it.  if you look at
> that commit and the AddressTranslation definition, you'll see:
>   /* For a simple (flat) memory model, we simply pass virtual
>      addressess through (mostly) unchanged. */
>   vAddr &= 0xFFFFFFFF;
> 
> this is forcing all addresses to be 32-bit.  when i saw this, i assumed
> it was just a dummy function someone didn't get around to finishing.

 Not unreasonable unless you want to simulate gigabytes of memory (or a 
TLB -- does the MIPS port of sim support it?).  Most programs run under 
sim won't need that though.  There is a mistake there however, you really 
want the mask to be 0x1FFFFFFF, to handle the architecture's 512MB KSEG0 
and KSEG1 unmapped segments and their aliasing correctly -- some programs 
rely on that.  Beyond that you need some more intelligence.

>  i
> didn't notice that the sim explicitly sets the address space to be 64-bit
> in configure.ac for many targets:
> mips_addr_bitsize=
> case "${target}" in
>   mips*-sde-elf*)       mips_bitsize=64 ; mips_msb=63 ;;
> ...
>   mips*-*-*)            mips_bitsize=32 ; mips_msb=31 ;;
> ...
> SIM_AC_OPTION_BITSIZE($mips_bitsize,$mips_msb,$mips_addr_bitsize)
> 
> it passes for me because mips-elf is a 32-bit target where that mask is
> actually a nop.  but for targets w/64-bit address space, it's getting a
> bad 32-bit sign extended address.

 The masking is still right, even for 64-bit targets.  It does shrink the 
size of XKPHYS segments to an unusually small size (a typical is at least 
36 bits), but this is actually permitted by the architecture, there's no 
explicit low limit set on the number of the physical address bits (PABITS) 
supported, though there is no means provided for software to detect it if 
it was below 12 (if such a low number was practical in the first place, 
that is)[1].

>  the sim is behaving correctly here:
> it should be throwing an error because 0xffffffff80010000 is not mapped.

 The mapping of the 0xffffffff80010000 virtual address is well defined in 
the architecture.  As this is within the KSEG0 unmapped segment, to map it  
to a physical address of you need to AND it with 0x1ffffff.  Therefore the
resulting bus address is 0x10000.

> digging further, it looks like the bug is in bfd, not the sim.  the pc
> is set to 0xffffffff80010000 during init:
> sim-main.c:sim_create_inferior:
>   CPU_PC_SET (cpu, (unsigned64) bfd_get_start_address (abfd));
> 
> when we look at the bfd init, bfd_set_start_address is called with that
> value as well:
> elfcode.h:elf_object_p:
>   bfd_set_start_address (abfd, i_ehdrp->e_entry);
> (gdb) p *i_ehdrp
> $2 = {
>   e_ident = "\177ELF\001\002\001\000\000\000\000\000\000\000\000",
>   e_entry = 0xffffffff80010000,
> 
> e_ehdrp is supposed to be representing an Elf32_Ehdr (by way of the
> bfd Elf_Internal_Ehdr structure), and e_entry is supposed to be an
> Elf32_Addr, and that's supposed to be uint32_t.
> 
> looking at include/elf/internal.h, Elf_Internal_Ehdr is:
>   bfd_vma e_entry;    /* Entry point virtual address */
> and bfd_vma is set to:
>   bfd-in.h:typedef bfd_vma bfd_uint64_t;
> 
> so that all looks correct.  poking elf_object_p further, the x_ehdr
> (Elf_External_Ehdr) is read in correctly too.
> 
> the badness starts here:
>   elf_swap_ehdr_in (abfd, &x_ehdr, i_ehdrp);
> which does:
>   int signed_vma = get_elf_backend_data (abfd)->sign_extend_vma;
>   if (signed_vma)
>     dst->e_entry = H_GET_SIGNED_WORD (abfd, src->e_entry);
>   else
>     dst->e_entry = H_GET_WORD (abfd, src->e_entry);
> 
> and indeed, the mips bfds (for whatever reason) do:
> elf32-mips.c:#define elf_backend_sign_extend_vma    TRUE
> elf64-mips.c:#define elf_backend_sign_extend_vma    TRUE
> elfn32-mips.c:#define elf_backend_sign_extend_vma   TRUE
> 
> i don't know why mips wants to do this, and considering literally no
> other target does it (ok, sh64 does it, but let's ignore them since
> that target is dead & being removed), it's probably not an accident.
> so we have to hack around it in the sim:

 This is how the 64-bit MIPS architecture has been defined for backwards 
compatibility.  All 32-bit operations previously defined operate such that 
results are sign-extended from bit 31 in processor registers.  For example 
a 32-bit load from memory of a value that has bit 31 set will result in 
bits 63:31 set in the destination register.  It's only 64-bit operations 
that may produce results that are not sign-extended from bit 31[2].

 So e.g. a 32-bit virtual address of 0xbfc00000 becomes 0xffffffffbfc00000 
in a 64-bit MIPS processor (and maps to a physical address of 0x1fc00000); 
as this is the reset vector, this is how the architectural notion of the 
PC register will be set at the exit from the reset state in 32-bit and 
64-bit MIPS processors, respectively.  Any simulator has to reproduce this 
behaviour.

 Similarly if a 32-bit program has an entry point of 0x80010000, then 
execution has to start from 0xffffffff80010000 on a 64-bit processor.

> --- a/sim/mips/interp.c
> +++ b/sim/mips/interp.c
> @@ -55,6 +55,7 @@ code on the hardware.
>  #include "getopt.h"
>  #include "libiberty.h"
>  #include "bfd.h"
> +#include "elf-bfd.h"
>  #include "gdb/callback.h"   /* GDB simulator callback interface */
>  #include "gdb/remote-sim.h" /* GDB simulator interface */
>  
> @@ -1020,7 +1021,17 @@ sim_create_inferior (SIM_DESC sd, struct bfd *abfd,
>        for (cpu_nr = 0; cpu_nr < sim_engine_nr_cpus (sd); cpu_nr++)
>  	{
>  	  sim_cpu *cpu = STATE_CPU (sd, cpu_nr);
> -	  CPU_PC_SET (cpu, (unsigned64) bfd_get_start_address (abfd));
> +	  sim_cia pc = bfd_get_start_address (abfd);
> +
> +	  /* We need to undo brain-dead bfd behavior where it sign-extends
> +	     addresses that are supposed to be unsigned.  See the mips bfd
> +	     sign_extend_vma setting.  We have to check the ELF data itself
> +	     in order to handle o32 & n32 ABIs.  */
> +	  if (abfd->tdata.elf_obj_data->elf_header->e_ident[EI_CLASS] ==
> +	      ELFCLASS32)
> +	    pc = (unsigned32) pc;
> +
> +	  CPU_PC_SET (cpu, pc);
>  	}
>      }

 So this change is wrong, at least as far as the comment is concerned, if 
not the actual calculation.  I suggest that you read the chapter on 
virtual addressing in the MIPS architecture[3].  This may help you to come 
up with the correct calculation here.

> that allows most tests to pass, but 6 (out of 69) are still failing.
> fpu64-ps.s for example has:
> mips-core: 8 byte read to unmapped address 0xffffffff80010b18 at 0x80010070
> 
> poking this a bit, i'm not sure if the code is bad (it's actually
> doing a load from that address), or the mips decode logic has a bad
> sign extension in there.  i'll let someone else figure it out.

 No idea offhand, that just ought to map to 0x10b18.  The test case uses 
LUXC1, unusually, so maybe that has something to do with the instruction?

References:

[1] "MIPS Architecture For Programmers, Volume III: The MIPS64 and
    microMIPS64 Privileged Resource Architecture", MIPS Technologies, 
    Inc., Document Number: MD00091, Revision 5.04, January 15, 2014, 
    Section 4.2.3 "Physical Address Size (PABITS)", p. 28
    <https://imgtec.com/mips/architectures/mips64/>

[2] "MIPS Architecture For Programmers, Volume I-A: Introduction to the 
    MIPS64 Architecture", MIPS Technologies, Inc., Document Number: 
    MD00083, Revision 5.04, November 20, 2013, Section 2.8.4 "CPU 
    Registers", p. 32
    <https://imgtec.com/mips/architectures/mips64/>

[3] same as [1], Section 4.3 "Virtual Address Spaces", pp. 28-31

 HTH,

  Maciej
Mike Frysinger Feb. 10, 2016, 7:28 a.m. UTC | #4
On 30 Jan 2016 16:09, Maciej W. Rozycki wrote:
> > > mips-core: 4 byte read to unmapped address 0xffffffff80020004 at 0xffffffff80020004
> > > program stopped with signal 10 (User defined signal 1).
> > 
> > the mips address logic was fundamentally broken.  my changes merely
> > uncovered that braindeadness rather than caused it.  if you look at
> > that commit and the AddressTranslation definition, you'll see:
> >   /* For a simple (flat) memory model, we simply pass virtual
> >      addressess through (mostly) unchanged. */
> >   vAddr &= 0xFFFFFFFF;
> > 
> > this is forcing all addresses to be 32-bit.  when i saw this, i assumed
> > it was just a dummy function someone didn't get around to finishing.
> 
>  Not unreasonable unless you want to simulate gigabytes of memory (or a 
> TLB -- does the MIPS port of sim support it?).  Most programs run under 
> sim won't need that though.  There is a mistake there however, you really 
> want the mask to be 0x1FFFFFFF, to handle the architecture's 512MB KSEG0 
> and KSEG1 unmapped segments and their aliasing correctly -- some programs 
> rely on that.  Beyond that you need some more intelligence.

the sim uses a sparse memory map, so it doesn't need to fill the address
space.  it's pretty common for some arches to allocate maps at the top
(for things like hardware) while the low addresses have the RAM.  i don't
know much this applies to existing mips usage.

for this specific case, i don't see how the mask as implemented could ever
be correct.  it always threw away the top 32-bits, so when you try to use
any address above 0xffffffff, it'd be silently changed to the lower 32-bits.
playing with some n32 programs on a mips64 system shows that it often gets
maps created both above and below that point.  i don't see how chopping the
top bits could possibly yield correct behavior.

let me outline a few general points i think apply here:
i see the simulator core as a bunch of generic building blocks.  the arch port
itself only adds support for the ISA (insns & registers) to the equation.  once
you get beyond that (e.g. the memory or devices), now you're talking about more
building blocks than stuff that should be baked in/architecture defaults.  so
even if today all of the systems that have a mips cpu also wire up the system
mem in a specific way, the mips arch core should not be doing any of that.
this makes it very easy to take just the mips ISA and construct a new cpu from
scratch that has new/different behavior and play around with things.  when you
do want behavior that matches an existing board, that's where the model
framework comes into play.  you can define specific cpu/system combinations
that match existing products and users can pick those via the --model flag.

it is a little difficult currently to reconcile virtual-vs-physical behavior
in the sim as there is only a single physical map and no virt-to-phys trans.
we've just ignored this so far and said virtual==physical.  which is why the
sim throws an error when you try to put a virtual 0xffffffff80000000 address
onto the physical bus.

since 64-bit address aren't actually being used in the 32-bit env, why bother
using them ?  seems like it'd be much easier to just use 32-bit addresses and
be done.
-mike
Maciej W. Rozycki Feb. 11, 2016, 4:54 p.m. UTC | #5
On Wed, 10 Feb 2016, Mike Frysinger wrote:

> > > the mips address logic was fundamentally broken.  my changes merely
> > > uncovered that braindeadness rather than caused it.  if you look at
> > > that commit and the AddressTranslation definition, you'll see:
> > >   /* For a simple (flat) memory model, we simply pass virtual
> > >      addressess through (mostly) unchanged. */
> > >   vAddr &= 0xFFFFFFFF;
> > > 
> > > this is forcing all addresses to be 32-bit.  when i saw this, i assumed
> > > it was just a dummy function someone didn't get around to finishing.
> > 
> >  Not unreasonable unless you want to simulate gigabytes of memory (or a 
> > TLB -- does the MIPS port of sim support it?).  Most programs run under 
> > sim won't need that though.  There is a mistake there however, you really 
> > want the mask to be 0x1FFFFFFF, to handle the architecture's 512MB KSEG0 
> > and KSEG1 unmapped segments and their aliasing correctly -- some programs 
> > rely on that.  Beyond that you need some more intelligence.
> 
> the sim uses a sparse memory map, so it doesn't need to fill the address
> space.  it's pretty common for some arches to allocate maps at the top
> (for things like hardware) while the low addresses have the RAM.  i don't
> know much this applies to existing mips usage.

 Due to how kernel segments, exception handler locations and the reset 
vector have been defined in the MIPS architecture, there's RAM expected to 
be there from the physical address 0 up, and ROM from the physical address 
0x1fc00000 up.  These will normally be accessed via the KSEG0 and KSEG1 
fixed virtual mappings respectively.  Specifically 32-bit MIPS processors 
will access exception handlers from 0x80000000 virtual/0 physical up and 
the reset vector 0xbfc00000 virtual/0x1fc00000 physical up.  And likewise 
64-bit MIPS processors will access these from 0xffffffff80000000 virtual/0 
physical and 0xffffffffbfc00000 virtual/0x1fc00000 physical respectively.  

 A bootstrap setting is available so that exception handlers are initially 
in ROM, so that any initialisation firmware can handle exceptional 
conditions before caches and any DRAM controller has been initialised, 
however applications will necessarily install their handlers in RAM so 
that they have control over them.

 Recently a possibility to relocate application exception handlers has 
been added, mainly to allow distinct handlers in SMP systems, however as a 
side effect there's no need for RAM at 0 physical anymore.

 NB I've mentioned it for illustration purposes only, as all except the 
virtual-to-physical mappings quoted above is external to the CPU.  It does 
have implications on how the mappings have been made in the first place 
though.

> for this specific case, i don't see how the mask as implemented could ever
> be correct.  it always threw away the top 32-bits, so when you try to use
> any address above 0xffffffff, it'd be silently changed to the lower 32-bits.

 Few bare-metal apps require or expect physical memory beyond 4GB, so this 
is mostly correct, but see below for the exact mapping details.

> playing with some n32 programs on a mips64 system shows that it often gets
> maps created both above and below that point.  i don't see how chopping the
> top bits could possibly yield correct behavior.

 N32 by definition uses 32-bit addressing and cannot ever go beyond 512MB 
via KSEG0/KSEG1 (because they strip the 3 MSBs of the address).  It could 
go via TLB, but this I gather is not implemented in GNU sim.  The 64-bit 
XKPHYS segment is by definition inaccesible to n32 programs, though indeed 
this is seldom enforced in hardware with the use of the CP0 Status.PX bit, 
due to historical baggage.  The XKPHYS mapping chops off the 5 MSBs BTW, 
that is bits 63:59.

 Can you give me an example of an n32 program going beyond 4GB?

 NB if you have bare-metal programs using both user (positive) and kernel 
(negative) addresses, then you really need to implement some sort of MMU 
to make them work correctly.

 Actually I think I wasn't explicit enough about this previously: the sign 
bit of any address is really the user/kernel* bit in the MIPS architecture
-- negative addresses are accessible to the kernel only, while positive 
addresses can be accessed both from the user and the kernel mode.  The 
implication of this arrangement and the observation that in 32-bit 
implementations it is bit #31 that is the sign bit and in 64-bit ones
it is bit #63 that is the sign bit is that you need to sign-extend 32-bit 
kernel addresses so that bit #31 is propagated to bit #63 in hardware.  
For 32-bit user addresses it does not matter if you sign- or zero-extend 
addresses, because bit #31 is zero anyway.  NB user addresses are mapped 
via the MMU, except when CP0 Status.ERL bit is set.

* There's some complication due to the optional supervisor mode, invented 
  miserably to support Windows NT on MIPS processors (ironically enough, 
  the architecture the OS was originally written for), but let's keep 
  things simple for the purpose of this consideration -- except from the 
  oddball Windows NT port for the DECstation (little-endian, of course!), 
  I haven't heard of any other use of the supervisor mode and the kernel 
  can treat any supervisor-mode addresses as plain kernel-mode addresses; 
  they're inaccessible to user-mode software.

  And yes, there's a further recent complication where you can shift the
  user/kernel split to 0xc0000000 instead in 32-bit processors; more on 
  this below.

> let me outline a few general points i think apply here:
> i see the simulator core as a bunch of generic building blocks.  the arch port
> itself only adds support for the ISA (insns & registers) to the equation.  once
> you get beyond that (e.g. the memory or devices), now you're talking about more
> building blocks than stuff that should be baked in/architecture defaults.  so
> even if today all of the systems that have a mips cpu also wire up the system
> mem in a specific way, the mips arch core should not be doing any of that.
> this makes it very easy to take just the mips ISA and construct a new cpu from
> scratch that has new/different behavior and play around with things.  when you
> do want behavior that matches an existing board, that's where the model
> framework comes into play.  you can define specific cpu/system combinations
> that match existing products and users can pick those via the --model flag.

 Listen, in the MIPS platform the memory architecture is wired to the CPU, 
not any external arrangement like straps, board circuitry or whatever; CP0 
is a part of the architecture, the privileged side.  The MIPS architecture 
may be doing it a bit differently from other architectures, but that's 
just how things are, every architecture is a bit unique in some respect.  
You can't have a MIPS implementation without these mappings within the CPU 
itself, whether externally there's memory, MMIO or nothing.

 Yes, you can wire different types of MMU, which will do different address 
mappings, e.g. BAT rather than TLB, but segments are going to stay there.  
In more recent architecture revisions there is an optional extension 
available to set segment attributes more flexibly, e.g. you can control 
privilege levels required for access or enable/disable mapping via the 
MMU.  This is how the user/kernel split at 0xc0000000 has been 
implemented.

 If you don't feel like implementing TLB or any fancy MMU for the MIPS 
port, then it's fine, such MIPS processors even have been made.  But then 
you need to provide at least the simplest KSEG0/KSEG1/XKPHYS fixed mapping 
I outlined for any MIPS software to work correctly.

 To give you a more illustrative example: ignoring the presence of MIPS 
segments is like ignoring the presence of segment registers on x86 even in 
their simplest real-mode form.  Yes, you can have an x86 simulation with 
the 8/16 GPRs only (and the PC aka IP), but is it still a proper x86 
implementation?

> it is a little difficult currently to reconcile virtual-vs-physical behavior
> in the sim as there is only a single physical map and no virt-to-phys trans.
> we've just ignored this so far and said virtual==physical.  which is why the
> sim throws an error when you try to put a virtual 0xffffffff80000000 address
> onto the physical bus.

 So yes, if you're missing a building block for what is an MMU in any 
processor that implements virtual addressing, then yes, I think you need 
to add it at least conceptually, and let ports wire it as they see it fit.  
Ones that have real addressing only will map 1-to-1, keeping this block 
null.  Other ones will wire it as appropriate, maybe in a generic way if 
there's any, or maybe in an implementation-dependent way, different for 
individual processor models simulated.

> since 64-bit address aren't actually being used in the 32-bit env, why bother
> using them ?  seems like it'd be much easier to just use 32-bit addresses and
> be done.

 This is why I suggested taking the simplest approach and masking off 
anything outside low 512MB, that is applying a mask of 0x1FFFFFFF still 
quoted at the top, matching KSEG0 vs KSEG1 semantics in real hardware and 
satisfying virtually all bare-metal software (let's exclude OS kernels 
such as Linux from here even though technically they're bare-metal apps 
too).  As a side effect you'll avoid the sign-extension that is 
architectural and which BFD gets right and you're so unhappy about.  
Merely chopping off upper 32 bits from a 64-bit virtual address isn't 
compliant with any architecturally permitted address mapping and won't 
work with most MIPS software.

 So what's wrong with this proposal?

 This isn't correct for user-mode addresses that need to go via the MMU, 
but GNU sim does not implement any MIPS MMU, so this will be as good as 
any.  In fact a mapping like this just could be treated as a simple fixed 
MMU, though to be fully compliant, user-mode addresses would have to be 
remapped according to the architecture manual[1], i.e. conceptually 
(ignoring CP0 Status.ERL for simplicity; trivial to add):

	if (addr >= 0)
		addr = (addr & 0x7fffffff) + 0x40000000;
	else if (addr <= -0x40000000)
		addr &= 0x1fffffff;
	else
		addr &= 0xffffffff;

Maybe we really ought to do this instead until we have a proper wireable 
MMU building block.  It is permitted by the architecture to implement a 
64-bit processor with 32-bit addressing only.

References:

[1] "MIPS Architecture For Programmers, Volume III: The MIPS64 and
    microMIPS64 Privileged Resource Architecture", MIPS Technologies,
    Inc., Document Number: MD00091, Revision 5.04, January 15, 2014,
    Section A.1 "Fixed Mapping MMU", p. 286
    <https://imgtec.com/mips/architectures/mips64/>

  Maciej
Matthew Fortune March 21, 2016, 11:54 a.m. UTC | #6
Ping.

Matthew Fortune <matthew.fortune@imgtec.com> writes:
> Mike Frysinger <vapier@gentoo.org> writes:

> > since 64-bit address aren't actually being used in the 32-bit env, why

> > bother using them ?  seems like it'd be much easier to just use 32-bit

> > addresses and be done.

> 

> Hi Mike,

> 

> The problem here is fairly common and seems to boil down to a

> misunderstanding at some level of the MIPS trick for 32-bit running on

> 64-bit architectures.

> 

> I agree that the address translation logic for MIPS seems weird but I

> also don’t think it should not get changed just because it looks odd

> without understanding why it is that way. As such for the time being I

> propose reverting both changes to MIPS sim to get it working again:

> 

>     Revert "sim: mips: delete mmu stubs to move to common

> sim_{read,write}"

> 

>     This reverts commit 26f8bf63bf36f9062a5cc1afacf71462a4abe0c8.

> 

>     Revert "sim: mips: workaround 32-bit addr sign extensions"

> 

>     This reverts commit b36d953bced0a4fecdde1823abac70ed7038ee95.

> 

> I'd assume this is OK given it 'fixes' the regression despite taking the

> code back to its unusual, but working, state.

> 

> I don't fully understand GNUSIM internals so please bear with me while I

> get up to speed...

> 

> Let's assume we just delete the masking of address in

> address_translation:

> 

> diff --git a/sim/mips/sim-main.c b/sim/mips/sim-main.c index

> 916769e..8cf5743 100644

> --- a/sim/mips/sim-main.c

> +++ b/sim/mips/sim-main.c

> @@ -68,7 +68,7 @@ address_translation (SIM_DESC sd,

> 

>    /* For a simple (flat) memory model, we simply pass virtual

>       addressess through (mostly) unchanged. */

> -  vAddr &= 0xFFFFFFFF;

> +//  vAddr &= 0xFFFFFFFF;

> 

>    *pAddr = vAddr;              /* default for isTARGET */

>    *CCA = Uncached;             /* not used for isHOST */

> 

> Where we could aim for is that when simulating a 64-bit architecture

> then all addresses (including those coming from o32 or n32 applications)

> should be seen as 64-bit and sign extended (NOT zero extended) from the

> 32-bit values seen in the ELF.

> 

> This means code in an o32 ELF with address 0x80010000 should be loaded

> at 0xffffffff80010000 and executed from that 64-bit address. When

> presenting addresses to the user the upper 32-bits can be discarded as

> they are irrelevant but internally in the sim they could be represented.

> 

> It seems this is how things work and I see sections being loaded at sign

> extended 64-bit addresses addresses but even when I claim to have a

> memory region at that 64-bit address I still get the read to unmapped

> address error as the code does not appear to get loaded:

> 

> run --memory-region 0xffffffff80010000,0x10000  sanity.s.x Loading

> section .text, size 0x60 lma 0xffffffff80010000 Loading section

> .MIPS.abiflags, size 0x18 lma 0x400098 Loading section .data, size 0x1a

> lma 0xffffffff80010060

> mips-core: 4 byte read to unmapped address 0xffffffff80020000 at

> 0xffffffff80020000 program stopped with signal 10 (User defined signal

> 1).

> 

> The trace output shows this:

> 

> insn:     0x80010000 ---   _start         nop              - SLLb

> insn:     0x80010004 ---   _start         nop              - SLLb

> insn:     0x80010008 ---   _fail          nop              - SLLb

> insn:     0x8001000c ---   _fail          nop              - SLLb

> insn:     0x80010010 ---   _fail          nop              - SLLb

> insn:     0x80010014 ---   _fail          nop              - SLLb

> 

> Can you help me understand why the code does not get loaded and/or if

> there is somewhere else we may need to educate about sign extended

> addresses?

> 

> Thanks,

> Matthew
Matthew Fortune April 5, 2016, 8:33 a.m. UTC | #7
Ping^2

> Ping.

> 

> Matthew Fortune <matthew.fortune@imgtec.com> writes:

> > Mike Frysinger <vapier@gentoo.org> writes:

> > > since 64-bit address aren't actually being used in the 32-bit env, why

> > > bother using them ?  seems like it'd be much easier to just use 32-bit

> > > addresses and be done.

> >

> > Hi Mike,

> >

> > The problem here is fairly common and seems to boil down to a

> > misunderstanding at some level of the MIPS trick for 32-bit running on

> > 64-bit architectures.

> >

> > I agree that the address translation logic for MIPS seems weird but I

> > also don’t think it should not get changed just because it looks odd

> > without understanding why it is that way. As such for the time being I

> > propose reverting both changes to MIPS sim to get it working again:

> >

> >     Revert "sim: mips: delete mmu stubs to move to common

> > sim_{read,write}"

> >

> >     This reverts commit 26f8bf63bf36f9062a5cc1afacf71462a4abe0c8.

> >

> >     Revert "sim: mips: workaround 32-bit addr sign extensions"

> >

> >     This reverts commit b36d953bced0a4fecdde1823abac70ed7038ee95.

> >

> > I'd assume this is OK given it 'fixes' the regression despite taking the

> > code back to its unusual, but working, state.

> >

> > I don't fully understand GNUSIM internals so please bear with me while I

> > get up to speed...

> >

> > Let's assume we just delete the masking of address in

> > address_translation:

> >

> > diff --git a/sim/mips/sim-main.c b/sim/mips/sim-main.c index

> > 916769e..8cf5743 100644

> > --- a/sim/mips/sim-main.c

> > +++ b/sim/mips/sim-main.c

> > @@ -68,7 +68,7 @@ address_translation (SIM_DESC sd,

> >

> >    /* For a simple (flat) memory model, we simply pass virtual

> >       addressess through (mostly) unchanged. */

> > -  vAddr &= 0xFFFFFFFF;

> > +//  vAddr &= 0xFFFFFFFF;

> >

> >    *pAddr = vAddr;              /* default for isTARGET */

> >    *CCA = Uncached;             /* not used for isHOST */

> >

> > Where we could aim for is that when simulating a 64-bit architecture

> > then all addresses (including those coming from o32 or n32 applications)

> > should be seen as 64-bit and sign extended (NOT zero extended) from the

> > 32-bit values seen in the ELF.

> >

> > This means code in an o32 ELF with address 0x80010000 should be loaded

> > at 0xffffffff80010000 and executed from that 64-bit address. When

> > presenting addresses to the user the upper 32-bits can be discarded as

> > they are irrelevant but internally in the sim they could be represented.

> >

> > It seems this is how things work and I see sections being loaded at sign

> > extended 64-bit addresses addresses but even when I claim to have a

> > memory region at that 64-bit address I still get the read to unmapped

> > address error as the code does not appear to get loaded:

> >

> > run --memory-region 0xffffffff80010000,0x10000  sanity.s.x Loading

> > section .text, size 0x60 lma 0xffffffff80010000 Loading section

> > .MIPS.abiflags, size 0x18 lma 0x400098 Loading section .data, size 0x1a

> > lma 0xffffffff80010060

> > mips-core: 4 byte read to unmapped address 0xffffffff80020000 at

> > 0xffffffff80020000 program stopped with signal 10 (User defined signal

> > 1).

> >

> > The trace output shows this:

> >

> > insn:     0x80010000 ---   _start         nop              - SLLb

> > insn:     0x80010004 ---   _start         nop              - SLLb

> > insn:     0x80010008 ---   _fail          nop              - SLLb

> > insn:     0x8001000c ---   _fail          nop              - SLLb

> > insn:     0x80010010 ---   _fail          nop              - SLLb

> > insn:     0x80010014 ---   _fail          nop              - SLLb

> >

> > Can you help me understand why the code does not get loaded and/or if

> > there is somewhere else we may need to educate about sign extended

> > addresses?

> >

> > Thanks,

> > Matthew
diff mbox

Patch

--- a/sim/mips/interp.c
+++ b/sim/mips/interp.c
@@ -55,6 +55,7 @@  code on the hardware.
 #include "getopt.h"
 #include "libiberty.h"
 #include "bfd.h"
+#include "elf-bfd.h"
 #include "gdb/callback.h"   /* GDB simulator callback interface */
 #include "gdb/remote-sim.h" /* GDB simulator interface */
 
@@ -1020,7 +1021,17 @@  sim_create_inferior (SIM_DESC sd, struct bfd *abfd,
       for (cpu_nr = 0; cpu_nr < sim_engine_nr_cpus (sd); cpu_nr++)
 	{
 	  sim_cpu *cpu = STATE_CPU (sd, cpu_nr);
-	  CPU_PC_SET (cpu, (unsigned64) bfd_get_start_address (abfd));
+	  sim_cia pc = bfd_get_start_address (abfd);
+
+	  /* We need to undo brain-dead bfd behavior where it sign-extends
+	     addresses that are supposed to be unsigned.  See the mips bfd
+	     sign_extend_vma setting.  We have to check the ELF data itself
+	     in order to handle o32 & n32 ABIs.  */
+	  if (abfd->tdata.elf_obj_data->elf_header->e_ident[EI_CLASS] ==
+	      ELFCLASS32)
+	    pc = (unsigned32) pc;
+
+	  CPU_PC_SET (cpu, pc);
 	}
     }