Re: [Qemu-devel] [PATCH 1/2] Support for MIPS64 user mode emulation

[Khansa Butt]

I have made following changes

 addr = env->lladdr;

addr &= qemu_host_page_mask;

 page_addr = addr & TARGET_PAGE_MASK;

 start_exclusive();

 mmap_lock();

 flags = page_get_flags(page_addr);

now return to elfload.c

I have a simple hello world mips64 binary for which I have two loadable segments
so following rounded off address ranges passed to page_set_flags() in target_mmap()
1) 0x20000000 - 0x2008d000
2) 0x2009c000 - 0x200a6000
the last addresses of these ranges are not included in l1_map
because of the for loop condition in page_set_flags()

for (addr = start, len = end - start;

    len != 0;
    len -= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
while env->lladdr after rounding off belong to 0x200a6000 so in order to
include last address of above range i made following change
error = target_mmap(vaddr_ps, eppnt->p_memsz + v addr_po,
                                elf_prot, MAP_PRIVATE | MAP_FIXED,
                                image_fd, eppnt->p_offset - vaddr_po);
as mem size of a segment is greater than its file size but you told me that it will cause SIGBUS
please suggest some solution for me in order to avoid target_mmap() change(i.e. filesz to memsz)
or can I change condition of for loop some how so that one more iteration will run for the last address.

On Fri, Apr 29, 2011 at 2:01 PM, Aurelien Jarno <address@hidden> wrote:

On Mon, Apr 25, 2011 at 04:54:19PM +0500, Khansa Butt wrote:
> please see inline comments highlighted in red color.
>
> On Wed, Apr 13, 2011 at 2:32 AM, Aurelien Jarno <address@hidden>wrote:
>
> > [I don't know very well linux-user, it would be nice to Cc: Riku Voipio,
> >  the linux-user maintainer for the next version.]
> >
> > On Sat, Apr 09, 2011 at 04:02:31PM +0500, Khansa Butt wrote:
> > > From e96e20e50cada1c9e1b65de5925281cdd5659746 Mon Sep 17 00:00:00 2001
> > > From: Ehsan-ul-Haq & Khansa Butt <address@hidden>
> > > Date: Sat, 9 Apr 2011 10:51:22 +0500
> > > Subject: [PATCH 1/2] Support for MIPS64 user mode emulation
> > >
> > >
> > > Signed-off-by: Ehsan-ul-Haq, Abdul Qadeer, Abdul Waheed, Khansa Butt <
> > > address@hidden>
> > > ---
> > >  configure                             |    1 +
> > >  default-configs/mips64-linux-user.mak |    1 +
> > >  linux-user/elfload.c                  |    2 +-
> > >  linux-user/main.c                     |   29
> > +++++++++++++++++++++++++++--
> > >  linux-user/mips64/syscall.h           |    3 +++
> > >  linux-user/signal.c                   |    3 ++-
> > >  target-mips/translate.c               |    1 +
> > >  7 files changed, 36 insertions(+), 4 deletions(-)
> > >  create mode 100644 default-configs/mips64-linux-user.mak
> > >
> > > diff --git a/configure b/configure
> > > index ae97e11..d1f7867 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -1039,6 +1039,7 @@ m68k-linux-user \
> > >  microblaze-linux-user \
> > >  microblazeel-linux-user \
> > >  mips-linux-user \
> > > +mips64-linux-user \
> > >  mipsel-linux-user \
> > >  ppc-linux-user \
> > >  ppc64-linux-user \
> > > diff --git a/default-configs/mips64-linux-user.mak
> > > b/default-configs/mips64-linux-user.mak
> > > new file mode 100644
> > > index 0000000..1598bfc
> > > --- /dev/null
> > > +++ b/default-configs/mips64-linux-user.mak
> > > @@ -0,0 +1 @@
> > > +# Default configuration for mips64-linux-user
> > > diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> > > index fe5410e..2832a33 100644
> > > --- a/linux-user/elfload.c
> > > +++ b/linux-user/elfload.c
> > > @@ -1384,7 +1384,7 @@ static void load_elf_image(const char *image_name,
> > int
> > > image_fd,
> > >              vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
> > >              vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
> > >
> > > -            error = target_mmap(vaddr_ps, eppnt->p_filesz + vaddr_po,
> > > +            error = target_mmap(vaddr_ps, eppnt->p_memsz + vaddr_po,
> >
> > What is the goal of this change? If the mmapped aread is bigger than the
> > file size rounded up to te page size, it will cause a SIGBUS.
> >
> > >                                  elf_prot, MAP_PRIVATE | MAP_FIXED,
> > >                                  image_fd, eppnt->p_offset - vaddr_po);
> > >              if (error == -1) {
> > > diff --git a/linux-user/main.c b/linux-user/main.c
> > > index e651bfd..a7f4955 100644
> > > --- a/linux-user/main.c
> > > +++ b/linux-user/main.c
> > > @@ -1937,6 +1937,14 @@ static int do_store_exclusive(CPUMIPSState *env)
> > >      int d;
> > >
> > >      addr = env->lladdr;
> > > +#if defined(TARGET_MIPS64)
> > > +/* For MIPS64 on 32 bit host there is a need to make
> > > +* the page accessible to which the above 'addr' is belonged */
> > > +#if HOST_LONG_BITS == 32
> > > +    int flag = PAGE_VALID | PAGE_READ | PAGE_WRITE | PAGE_WRITE_ORG;
> > > +    page_set_flags(addr, addr + 4096, flag);
> > > +#endif
> > > +#endif
> >
> > I don't really see the reason why this should be done that way. Are you
> > trying to run MIPS32 binaries compiled for 8kB page size?
> >
>
>
>
> this change is needed when we run MIPS64 ELF on 32 bit x86 host. MIPS64 ELF
> contains 36 bit address.

Actually it can contains up to 62-bit address there (all the user mapped
space).

>  load_elf_image() at /home/khansa/testpatch/qemu/linux-user/elfload.c: QEMU
>  contains these lines
>            /* Round addresses to page boundaries.  */
>             loaddr &= qemu_host_page_mask;
>             hiaddr = HOST_PAGE_ALIGN(hiaddr);
> when QEMU run on 32 bit x86 the above two variables are rounded to 32 bit
> value while these should be 36 bits as these come from MIPS64 ELF.and then

It is correct to truncate them, as the address space of the host is
smaller. It's just based on the fact that programs only need a subset of
the 62 bit address space.

> for these rounded address l1_map is initialized in page_find_alloc().
> in case of SCD(store condition double ) instruction of MIPS64r2 when we have
> to check load linked address its again 36 bit so it will make an index(addr
> >> TARGET_PAGE_BITS) for which l1_map is no valid entry, returning 0 value
> and we got segmentation fault. this is the reason we did following changes
> in main.c do_store_exclusive()

No, the load linked address register size, as well as the shift is
actually implementation dependent. On old 64-bit MIPS implementation
and MIPS32 core it is a 32-bit register and a 4-bit shift, where as
on MIPS64 cores it is a 64-bit register and a 0-bit shift.

In any case this value is the *physical address*, not the *virtual
address*, hence we have to workaround that by saving the virtual
address in the linux-user code. For linux-user, the full 64-bit address
is saved in env->lladdr.

What I don't understand is why the address passed to the scd instruction
is not an address rounder to 32-bit, it should match the rounded address
from elfload.c.

Anyway, I don't doubt there is a problem, but clearly the solution of
creating a fake page at this address is clearly the wrong way to go.

--

Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net