Comment 11 for bug 1880225

сре, 27. мај 2020. у 14:05 Aleksandar Markovic
<email address hidden> је написао/ла:
>
> сре, 27. мај 2020. у 12:07 Alex Bennée <email address hidden> је написао/ла:
> >
> > We rely on the pointer to wrap when accessing the high address of the
> > COMMPAGE so it lands somewhere reasonable. However on 32 bit hosts we
> > cannot afford just to map the entire 4gb address range. The old mmap
> > trial and error code handled this by just checking we could map both
> > the guest_base and the computed COMMPAGE address.
> >
> > We can't just manipulate loadaddr to get what we want so we introduce
> > an offset which pgb_find_hole can apply when looking for a gap for
> > guest_base that ensures there is space left to map the COMMPAGE
> > afterwards.
> >
> > This is arguably a little inefficient for the one 32 bit
> > value (kuser_helper_version) we need to keep there given all the
> > actual code entries are picked up during the translation phase.
> >
> > Fixes: ee94743034b
> > Bug: https://bugs.launchpad.net/qemu/+bug/1880225
>
> For the scenario in this bug report, for today's master, before and after
> applying this patch:
>

I am not sure how significant is this info, but I executed the test
without applying patch 1/3, so only 2/3 was applied in the case
"AFTER".

Aleksandar

> BEFORE:
>
> $ ~/Build/qemu-master/build-gcc/arm-linux-user/qemu-arm ./toupper_string-arm
> qemu-arm: /home/rtrk/Build/qemu-master/linux-user/elfload.c:2294:
> probe_guest_base: Assertion `have_guest_base' failed.
> Aborted
>
> AFTER:
>
> $ ~/Build/qemu-master/build-gcc/arm-linux-user/qemu-arm ./toupper_string-arm
> CONTROL RESULT: (toupper_string)
> nwlrbbmqbhcdarz owkkyhiddqscdxr jmowfrxsjybldbe fsarcbynecdyggx xpklorellnmpapq
> NWLRBBMQBHCDARZ OWKKYHIDDQSCDXR JMOWFRXSJYBLDBE FSARCBYNECDYGGX XPKLORELLNMPAPQ
>
> So, it works in my test bed.
>
> Tested-by: Aleksandar Markovic <email address hidden>
>
> > Cc: Bug 1880225 <email address hidden>
> > Signed-off-by: Alex Bennée <email address hidden>
> > Cc: Richard Henderson <email address hidden>
> > Cc: Peter Maydell <email address hidden>
> > ---
> > linux-user/elfload.c | 18 ++++++++++--------
> > 1 file changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> > index d6027867a1a..31defce95b5 100644
> > --- a/linux-user/elfload.c
> > +++ b/linux-user/elfload.c
> > @@ -2145,7 +2145,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, lon
> >
> > /* Return value for guest_base, or -1 if no hole found. */
> > static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size,
> > - long align)
> > + long align, uintptr_t offset)
> > {
> > GSList *maps, *iter;
> > uintptr_t this_start, this_end, next_start, brk;
> > @@ -2171,7 +2171,7 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size,
> >
> > this_end = ((MapInfo *)iter->data)->start;
> > next_start = ((MapInfo *)iter->data)->end;
> > - align_start = ROUND_UP(this_start, align);
> > + align_start = ROUND_UP(this_start + offset, align);
> >
> > /* Skip holes that are too small. */
> > if (align_start >= this_end) {
> > @@ -2221,6 +2221,7 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr,
> > {
> > uintptr_t loaddr = orig_loaddr;
> > uintptr_t hiaddr = orig_hiaddr;
> > + uintptr_t offset = 0;
> > uintptr_t addr;
> >
> > if (hiaddr != orig_hiaddr) {
> > @@ -2234,18 +2235,19 @@ static void pgb_static(const char *image_name, abi_ulong orig_loaddr,
> > if (ARM_COMMPAGE) {
> > /*
> > * Extend the allocation to include the commpage.
> > - * For a 64-bit host, this is just 4GiB; for a 32-bit host,
> > - * the address arithmetic will wrap around, but the difference
> > - * will produce the correct allocation size.
> > + * For a 64-bit host, this is just 4GiB; for a 32-bit host we
> > + * need to ensure there is space bellow the guest_base so we
> > + * can map the commpage in the place needed when the address
> > + * arithmetic wraps around.
> > */
> > if (sizeof(uintptr_t) == 8 || loaddr >= 0x80000000u) {
> > hiaddr = (uintptr_t)4 << 30;
> > } else {
> > - loaddr = ARM_COMMPAGE & -align;
> > + offset = (128 * KiB);
> > }
> > }
> >
> > - addr = pgb_find_hole(loaddr, hiaddr - loaddr, align);
> > + addr = pgb_find_hole(loaddr, hiaddr - loaddr, align, offset);
> > if (addr == -1) {
> > /*
> > * If ARM_COMMPAGE, there *might* be a non-consecutive allocation
> > @@ -2280,7 +2282,7 @@ static void pgb_dynamic(const char *image_name, long align)
> > * just above that, and maximises the positive guest addresses.
> > */
> > commpage = ARM_COMMPAGE & -align;
> > - addr = pgb_find_hole(commpage, -commpage, align);
> > + addr = pgb_find_hole(commpage, -commpage, align, 0);
> > assert(addr != -1);
> > guest_base = addr;
> > }
> > --
> > 2.20.1
> >
> >