Comment 10 for bug 1880225

Revision history for this message
Aleksandar Markovic (aleksandar-markovic) wrote : Re: [PATCH v1 2/3] linux-user: deal with address wrap for ARM_COMMPAGE on 32 bit

сре, 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:

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
>
>