Please see comments highlighted in green.
On Fri, Apr 29, 2011 at 2:01 PM, Aurelien Jarno <aurel...@aurel32.net>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 <aurel...@aurel32.net
> >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 <kha...@kics.edu.pk>
> > > > 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
> <
> > > > kha...@kics.edu.pk>
> > > > ---
> > > > 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.
>
It is the load linked address which is causing problem as it is full 64 bit
address
following is the QEMU code snippet, main.c:do_store_exclusive()
addr = env->lladdr;
page_addr = addr & TARGET_PAGE_MASK;
start_exclusive();
mmap_lock();
flags = page_get_flags(page_addr);
when 64 bit page_addr is sent to page_get_flags(), l1_map has zero entry for
this address and it returns flags=0.
what we can do is we can rounded lladdr to 32 bit as a workaround
addr = env->lladdr;
addr &= qemu_host_page_mask;
Is this solution acceptable for you?? please give your comments.
>
> 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
> aurel...@aurel32.net http://www.aurel32.net
>
