Re: [Qemu-devel] qemu git head 20100323 on FreeBSD - qemu-devel port update for testing

[Juergen Lock]

On Tue, Mar 30, 2010 at 10:54:03PM +0300, Blue Swirl wrote:
> On 3/30/10, Juergen Lock <address@hidden> wrote:
> > On Tue, Mar 30, 2010 at 09:04:28PM +0300, Blue Swirl wrote:
> >  > On 3/25/10, Juergen Lock <address@hidden> wrote:
> >  > > Hi!
> >  > >
> >  > >   Now that qemu git head works again (thanx Aurelien! :) I've finished
> >  > >  the FreeBSD qemu-devel port update patch/shar that made me uncover
> >  > >  the bug:
> >  > >         http://people.freebsd.org/~nox/qemu/qemu-devel-20100323.patch
> >  > >  resp.
> >  > >         http://people.freebsd.org/~nox/qemu/qemu-devel-20100323.shar
> >  > >
> >  > >   This also adds a few misc fixes (that I'll submit on the qemu list
> >  > >  seperately), I have...
> >  > >
> >  > >  . Fixed the FreeBSD executable path detection to work without /proc
> >  > >   mounted (it usually isn't on FreeBSD), so you now no longer have to
> >  > >   pass the path to the pc-bios dir with -L if you run qemu out of the
> >  > >   build dir when another version is installed, like,
> >  > >         work/qemu-snapshot-20100323_20/i386-softmmu/qemu ...
> >  > >
> >  > >   (files/patch-vl.c in the shar/patch)
> >  > >
> >  > >  . Fixed some more bsd-user bugs so all of i386-bsd-user, 
> > x86_64-bsd-user,
> >  > >   and sparc64-bsd-user now run for me again on FreeBSD stable/8 amd64.
> >  > >   (I didn't test sparc-bsd-user as I only tried -bsd freebsd and 
> > FreeBSD
> >  > >   doesn't run on 32bit sparc.) - Yes bsd-user still needs more work but
> >  > >   at least simple exectuables run.
> >  > >
> >  > >   (files/patch-bsd-user-mmap.c, files/patch-exec.c)
> >  > >
> >  > >  . Fixed the bsd-user host page protection code for FreeBSD hosts
> >  > >   (using kinfo_getvmmap(3) on FeeBSD >= 7.x and /compat/linux/proc
> >  > >   on older FreeBSD.)
> >  > >
> >  > >   (files/patch-bsd-user-linproc)
> >  > >
> >  > >  . Fixed some compilation warnings and a missing #include.
> >  > >
> >  > >   (files/patch-qemu-char.c, files/patch-qemu-timer.c)
> >  > >
> >  >
> >  > Thanks, applied all except exec.c one.
> >
> >
> > Oh, is there something wrong with it?  You mean this one, right?
> >
> >  Subject: [PATCH] Avoid page_set_flags() assert in qemu-user host page
> >   protection code
> >  Message-ID: <address@hidden>
> >  [...]
> >
> >  --- a/exec.c
> >  +++ b/exec.c
> >  @@ -293,10 +293,13 @@ static void page_init(void)
> >
> >                      if (h2g_valid(endaddr)) {
> >                          endaddr = h2g(endaddr);
> >  +                        page_set_flags(startaddr, endaddr, PAGE_RESERVED);
> >                      } else {
> >  +#if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS
> >                          endaddr = ~0ul;
> >  +                        page_set_flags(startaddr, endaddr, PAGE_RESERVED);
> >  +#endif
> >                      }
> >  -                    page_set_flags(startaddr, endaddr, PAGE_RESERVED);
> >                  }
> >              } while (!feof(f));
> >
> >   I first tried to replace the endaddr in the !h2g_valid(endaddr) case with
> >         ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS) - 1
> >  if TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS (which comes from the condition
> >  of the assert in page_set_flags() that was triggered on the ~0ul value),
> >  but that caused the qemu process to grow into swap and made the box
> >  usuable when that code was reached and I had to kill qemu.  (The box has
> >  8 GB RAM.)  And so I thought just leaving that page range unprotected
> >  if only the start address is valid was the lesser evil...
> 
> I was thinking something like (abi_ulong)-1 but maybe that isn't any
> more correct.

Oh this is happening with x86_64-bsd-user on the same arch so I'd say
(abi_ulong)-1 would be the same as ~0ul (and still cause the assert.)

 Cheers,
        Juergen