Re: [PATCH v1 2/3] linux-user: deal with address wrap for ARM_COMMPAGE on 32 bit

[Aleksandar Markovic]

сре, 27. мај 2020. у 12:07 Alex Bennée <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:

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 <address@hidden>

> Cc: Bug 1880225 <address@hidden>
> Signed-off-by: Alex Bennée <address@hidden>
> Cc: Richard Henderson <address@hidden>
> Cc: Peter Maydell <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
>
>