* [PATCH v1 01/14] linux-user: completely re-write init_guest_space
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 17:05 ` [PATCH v1 02/14] exec/cpu-all: Use bool for have_guest_base Alex Bennée
` (14 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Riku Voipio, Alex Bennée, Laurent Vivier
First we ensure all guest space initialisation logic comes through
probe_guest_base once we understand the nature of the binary we are
loading. The convoluted init_guest_space routine is removed and
replaced with a number of pgb_* helpers which are called depending on
what requirements we have when loading the binary.
We first try to do what is requested by the host. Failing that we try
and satisfy the guest requested base address. If all those options
fail we fall back to finding a space in the memory map using our
recently written read_self_maps() helper.
There are some additional complications we try and take into account
when looking for holes in the address space. We try not to go directly
after the system brk() space so there is space for a little growth. We
also don't want to have to use negative offsets which would result in
slightly less efficient code on x86 when it's unable to use the
segment offset register.
Less mind-binding gotos and hopefully clearer logic throughout.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v3
- include rth updates that
- split probe_guest_base into multiple functions
- more heuristics on gap finding
v4
- whitespace fix
---
linux-user/qemu.h | 31 ++-
linux-user/elfload.c | 503 +++++++++++++++++++++---------------------
linux-user/flatload.c | 6 +
linux-user/main.c | 23 +-
4 files changed, 277 insertions(+), 286 deletions(-)
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 792c74290f..ce902f5132 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -219,18 +219,27 @@ void init_qemu_uname_release(void);
void fork_start(void);
void fork_end(int child);
-/* Creates the initial guest address space in the host memory space using
- * the given host start address hint and size. The guest_start parameter
- * specifies the start address of the guest space. guest_base will be the
- * difference between the host start address computed by this function and
- * guest_start. If fixed is specified, then the mapped address space must
- * start at host_start. The real start address of the mapped memory space is
- * returned or -1 if there was an error.
+/**
+ * probe_guest_base:
+ * @image_name: the executable being loaded
+ * @loaddr: the lowest fixed address in the executable
+ * @hiaddr: the highest fixed address in the executable
+ *
+ * Creates the initial guest address space in the host memory space.
+ *
+ * If @loaddr == 0, then no address in the executable is fixed,
+ * i.e. it is fully relocatable. In that case @hiaddr is the size
+ * of the executable.
+ *
+ * This function will not return if a valid value for guest_base
+ * cannot be chosen. On return, the executable loader can expect
+ *
+ * target_mmap(loaddr, hiaddr - loaddr, ...)
+ *
+ * to succeed.
*/
-unsigned long init_guest_space(unsigned long host_start,
- unsigned long host_size,
- unsigned long guest_start,
- bool fixed);
+void probe_guest_base(const char *image_name,
+ abi_ulong loaddr, abi_ulong hiaddr);
#include "qemu/log.h"
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 619c054cc4..01a9323a63 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -11,6 +11,7 @@
#include "qemu/queue.h"
#include "qemu/guest-random.h"
#include "qemu/units.h"
+#include "qemu/selfmap.h"
#ifdef _ARCH_PPC64
#undef ARCH_DLINFO
@@ -382,68 +383,30 @@ enum {
/* The commpage only exists for 32 bit kernels */
-/* Return 1 if the proposed guest space is suitable for the guest.
- * Return 0 if the proposed guest space isn't suitable, but another
- * address space should be tried.
- * Return -1 if there is no way the proposed guest space can be
- * valid regardless of the base.
- * The guest code may leave a page mapped and populate it if the
- * address is suitable.
- */
-static int init_guest_commpage(unsigned long guest_base,
- unsigned long guest_size)
-{
- unsigned long real_start, test_page_addr;
-
- /* We need to check that we can force a fault on access to the
- * commpage at 0xffff0fxx
- */
- test_page_addr = guest_base + (0xffff0f00 & qemu_host_page_mask);
-
- /* If the commpage lies within the already allocated guest space,
- * then there is no way we can allocate it.
- *
- * You may be thinking that that this check is redundant because
- * we already validated the guest size against MAX_RESERVED_VA;
- * but if qemu_host_page_mask is unusually large, then
- * test_page_addr may be lower.
- */
- if (test_page_addr >= guest_base
- && test_page_addr < (guest_base + guest_size)) {
- return -1;
- }
+#define ARM_COMMPAGE (intptr_t)0xffff0f00u
- /* Note it needs to be writeable to let us initialise it */
- real_start = (unsigned long)
- mmap((void *)test_page_addr, qemu_host_page_size,
- PROT_READ | PROT_WRITE,
- MAP_ANONYMOUS | MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+static bool init_guest_commpage(void)
+{
+ void *want = g2h(ARM_COMMPAGE & -qemu_host_page_size);
+ void *addr = mmap(want, qemu_host_page_size, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
- /* If we can't map it then try another address */
- if (real_start == -1ul) {
- return 0;
+ if (addr == MAP_FAILED) {
+ perror("Allocating guest commpage");
+ exit(EXIT_FAILURE);
}
-
- if (real_start != test_page_addr) {
- /* OS didn't put the page where we asked - unmap and reject */
- munmap((void *)real_start, qemu_host_page_size);
- return 0;
+ if (addr != want) {
+ return false;
}
- /* Leave the page mapped
- * Populate it (mmap should have left it all 0'd)
- */
-
- /* Kernel helper versions */
- __put_user(5, (uint32_t *)g2h(0xffff0ffcul));
+ /* Set kernel helper versions; rest of page is 0. */
+ __put_user(5, (uint32_t *)g2h(0xffff0ffcu));
- /* Now it's populated make it RO */
- if (mprotect((void *)test_page_addr, qemu_host_page_size, PROT_READ)) {
+ if (mprotect(addr, qemu_host_page_size, PROT_READ)) {
perror("Protecting guest commpage");
- exit(-1);
+ exit(EXIT_FAILURE);
}
-
- return 1; /* All good */
+ return true;
}
#define ELF_HWCAP get_elf_hwcap()
@@ -2075,239 +2038,267 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
return sp;
}
-unsigned long init_guest_space(unsigned long host_start,
- unsigned long host_size,
- unsigned long guest_start,
- bool fixed)
-{
- /* In order to use host shmat, we must be able to honor SHMLBA. */
- unsigned long align = MAX(SHMLBA, qemu_host_page_size);
- unsigned long current_start, aligned_start;
- int flags;
-
- assert(host_start || host_size);
-
- /* If just a starting address is given, then just verify that
- * address. */
- if (host_start && !host_size) {
-#if defined(TARGET_ARM) && !defined(TARGET_AARCH64)
- if (init_guest_commpage(host_start, host_size) != 1) {
- return (unsigned long)-1;
- }
+#ifndef ARM_COMMPAGE
+#define ARM_COMMPAGE 0
+#define init_guest_commpage() true
#endif
- return host_start;
- }
- /* Setup the initial flags and start address. */
- current_start = host_start & -align;
- flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_NORESERVE;
- if (fixed) {
- flags |= MAP_FIXED;
- }
+static void pgb_fail_in_use(const char *image_name)
+{
+ error_report("%s: requires virtual address space that is in use "
+ "(omit the -B option or choose a different value)",
+ image_name);
+ exit(EXIT_FAILURE);
+}
- /* Otherwise, a non-zero size region of memory needs to be mapped
- * and validated. */
+static void pgb_have_guest_base(const char *image_name, abi_ulong guest_loaddr,
+ abi_ulong guest_hiaddr, long align)
+{
+ const int flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_NORESERVE;
+ void *addr, *test;
-#if defined(TARGET_ARM) && !defined(TARGET_AARCH64)
- /* On 32-bit ARM, we need to map not just the usable memory, but
- * also the commpage. Try to find a suitable place by allocating
- * a big chunk for all of it. If host_start, then the naive
- * strategy probably does good enough.
- */
- if (!host_start) {
- unsigned long guest_full_size, host_full_size, real_start;
-
- guest_full_size =
- (0xffff0f00 & qemu_host_page_mask) + qemu_host_page_size;
- host_full_size = guest_full_size - guest_start;
- real_start = (unsigned long)
- mmap(NULL, host_full_size, PROT_NONE, flags, -1, 0);
- if (real_start == (unsigned long)-1) {
- if (host_size < host_full_size - qemu_host_page_size) {
- /* We failed to map a continous segment, but we're
- * allowed to have a gap between the usable memory and
- * the commpage where other things can be mapped.
- * This sparseness gives us more flexibility to find
- * an address range.
- */
- goto naive;
- }
- return (unsigned long)-1;
+ if (!QEMU_IS_ALIGNED(guest_base, align)) {
+ fprintf(stderr, "Requested guest base 0x%lx does not satisfy "
+ "host minimum alignment (0x%lx)\n",
+ guest_base, align);
+ exit(EXIT_FAILURE);
+ }
+
+ /* Sanity check the guest binary. */
+ if (reserved_va) {
+ if (guest_hiaddr > reserved_va) {
+ error_report("%s: requires more than reserved virtual "
+ "address space (0x%" PRIx64 " > 0x%lx)",
+ image_name, (uint64_t)guest_hiaddr, reserved_va);
+ exit(EXIT_FAILURE);
}
- munmap((void *)real_start, host_full_size);
- if (real_start & (align - 1)) {
- /* The same thing again, but with extra
- * so that we can shift around alignment.
- */
- unsigned long real_size = host_full_size + qemu_host_page_size;
- real_start = (unsigned long)
- mmap(NULL, real_size, PROT_NONE, flags, -1, 0);
- if (real_start == (unsigned long)-1) {
- if (host_size < host_full_size - qemu_host_page_size) {
- goto naive;
- }
- return (unsigned long)-1;
- }
- munmap((void *)real_start, real_size);
- real_start = ROUND_UP(real_start, align);
+ } else {
+ if ((guest_hiaddr - guest_base) > ~(uintptr_t)0) {
+ error_report("%s: requires more virtual address space "
+ "than the host can provide (0x%" PRIx64 ")",
+ image_name, (uint64_t)guest_hiaddr - guest_base);
+ exit(EXIT_FAILURE);
}
- current_start = real_start;
}
- naive:
-#endif
- while (1) {
- unsigned long real_start, real_size, aligned_size;
- aligned_size = real_size = host_size;
+ /*
+ * Expand the allocation to the entire reserved_va.
+ * Exclude the mmap_min_addr hole.
+ */
+ if (reserved_va) {
+ guest_loaddr = (guest_base >= mmap_min_addr ? 0
+ : mmap_min_addr - guest_base);
+ guest_hiaddr = reserved_va;
+ }
- /* Do not use mmap_find_vma here because that is limited to the
- * guest address space. We are going to make the
- * guest address space fit whatever we're given.
- */
- real_start = (unsigned long)
- mmap((void *)current_start, host_size, PROT_NONE, flags, -1, 0);
- if (real_start == (unsigned long)-1) {
- return (unsigned long)-1;
- }
+ /* Reserve the address space for the binary, or reserved_va. */
+ test = g2h(guest_loaddr);
+ addr = mmap(test, guest_hiaddr - guest_loaddr, PROT_NONE, flags, -1, 0);
+ if (test != addr) {
+ pgb_fail_in_use(image_name);
+ }
+}
- /* Check to see if the address is valid. */
- if (host_start && real_start != current_start) {
- qemu_log_mask(CPU_LOG_PAGE, "invalid %lx && %lx != %lx\n",
- host_start, real_start, current_start);
- goto try_again;
+/* 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)
+{
+ GSList *maps, *iter;
+ uintptr_t this_start, this_end, next_start, brk;
+ intptr_t ret = -1;
+
+ assert(QEMU_IS_ALIGNED(guest_loaddr, align));
+
+ maps = read_self_maps();
+
+ /* Read brk after we've read the maps, which will malloc. */
+ brk = (uintptr_t)sbrk(0);
+
+ /* The first hole is before the first map entry. */
+ this_start = mmap_min_addr;
+
+ for (iter = maps; iter;
+ this_start = next_start, iter = g_slist_next(iter)) {
+ uintptr_t align_start, hole_size;
+
+ this_end = ((MapInfo *)iter->data)->start;
+ next_start = ((MapInfo *)iter->data)->end;
+ align_start = ROUND_UP(this_start, align);
+
+ /* Skip holes that are too small. */
+ if (align_start >= this_end) {
+ continue;
+ }
+ hole_size = this_end - align_start;
+ if (hole_size < guest_size) {
+ continue;
}
- /* Ensure the address is properly aligned. */
- if (real_start & (align - 1)) {
- /* Ideally, we adjust like
- *
- * pages: [ ][ ][ ][ ][ ]
- * old: [ real ]
- * [ aligned ]
- * new: [ real ]
- * [ aligned ]
- *
- * But if there is something else mapped right after it,
- * then obviously it won't have room to grow, and the
- * kernel will put the new larger real someplace else with
- * unknown alignment (if we made it to here, then
- * fixed=false). Which is why we grow real by a full page
- * size, instead of by part of one; so that even if we get
- * moved, we can still guarantee alignment. But this does
- * mean that there is a padding of < 1 page both before
- * and after the aligned range; the "after" could could
- * cause problems for ARM emulation where it could butt in
- * to where we need to put the commpage.
- */
- munmap((void *)real_start, host_size);
- real_size = aligned_size + align;
- real_start = (unsigned long)
- mmap((void *)real_start, real_size, PROT_NONE, flags, -1, 0);
- if (real_start == (unsigned long)-1) {
- return (unsigned long)-1;
+ /* If this hole contains brk, give ourselves some room to grow. */
+ if (this_start <= brk && brk < this_end) {
+ hole_size -= guest_size;
+ if (sizeof(uintptr_t) == 8 && hole_size >= 1 * GiB) {
+ align_start += 1 * GiB;
+ } else if (hole_size >= 16 * MiB) {
+ align_start += 16 * MiB;
+ } else {
+ align_start = (this_end - guest_size) & -align;
+ if (align_start < this_start) {
+ continue;
+ }
}
- aligned_start = ROUND_UP(real_start, align);
- } else {
- aligned_start = real_start;
}
-#if defined(TARGET_ARM) && !defined(TARGET_AARCH64)
- /* On 32-bit ARM, we need to also be able to map the commpage. */
- int valid = init_guest_commpage(aligned_start - guest_start,
- aligned_size + guest_start);
- if (valid == -1) {
- munmap((void *)real_start, real_size);
- return (unsigned long)-1;
- } else if (valid == 0) {
- goto try_again;
+ /* Record the lowest successful match. */
+ if (ret < 0) {
+ ret = align_start - guest_loaddr;
}
-#endif
-
- /* If nothing has said `return -1` or `goto try_again` yet,
- * then the address we have is good.
- */
- break;
-
- try_again:
- /* That address didn't work. Unmap and try a different one.
- * The address the host picked because is typically right at
- * the top of the host address space and leaves the guest with
- * no usable address space. Resort to a linear search. We
- * already compensated for mmap_min_addr, so this should not
- * happen often. Probably means we got unlucky and host
- * address space randomization put a shared library somewhere
- * inconvenient.
- *
- * This is probably a good strategy if host_start, but is
- * probably a bad strategy if not, which means we got here
- * because of trouble with ARM commpage setup.
- */
- if (munmap((void *)real_start, real_size) != 0) {
- error_report("%s: failed to unmap %lx:%lx (%s)", __func__,
- real_start, real_size, strerror(errno));
- abort();
+ /* If this hole contains the identity map, select it. */
+ if (align_start <= guest_loaddr &&
+ guest_loaddr + guest_size <= this_end) {
+ ret = 0;
}
- current_start += align;
- if (host_start == current_start) {
- /* Theoretically possible if host doesn't have any suitably
- * aligned areas. Normally the first mmap will fail.
- */
- return (unsigned long)-1;
+ /* If this hole ends above the identity map, stop looking. */
+ if (this_end >= guest_loaddr) {
+ break;
}
}
+ free_self_maps(maps);
- qemu_log_mask(CPU_LOG_PAGE, "Reserved 0x%lx bytes of guest address space\n", host_size);
-
- return aligned_start;
+ return ret;
}
-static void probe_guest_base(const char *image_name,
- abi_ulong loaddr, abi_ulong hiaddr)
+static void pgb_static(const char *image_name, abi_ulong orig_loaddr,
+ abi_ulong orig_hiaddr, long align)
{
- /* Probe for a suitable guest base address, if the user has not set
- * it explicitly, and set guest_base appropriately.
- * In case of error we will print a suitable message and exit.
- */
- const char *errmsg;
- if (!have_guest_base && !reserved_va) {
- unsigned long host_start, real_start, host_size;
+ uintptr_t loaddr = orig_loaddr;
+ uintptr_t hiaddr = orig_hiaddr;
+ uintptr_t addr;
- /* Round addresses to page boundaries. */
- loaddr &= qemu_host_page_mask;
- hiaddr = HOST_PAGE_ALIGN(hiaddr);
+ if (hiaddr != orig_hiaddr) {
+ error_report("%s: requires virtual address space that the "
+ "host cannot provide (0x%" PRIx64 ")",
+ image_name, (uint64_t)orig_hiaddr);
+ exit(EXIT_FAILURE);
+ }
- if (loaddr < mmap_min_addr) {
- host_start = HOST_PAGE_ALIGN(mmap_min_addr);
+ loaddr &= -align;
+ 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.
+ */
+ if (sizeof(uintptr_t) == 8 || loaddr >= 0x80000000u) {
+ hiaddr = (uintptr_t)4 << 30;
} else {
- host_start = loaddr;
- if (host_start != loaddr) {
- errmsg = "Address overflow loading ELF binary";
- goto exit_errmsg;
- }
+ loaddr = ARM_COMMPAGE & -align;
}
- host_size = hiaddr - loaddr;
+ }
- /* Setup the initial guest memory space with ranges gleaned from
- * the ELF image that is being loaded.
+ addr = pgb_find_hole(loaddr, hiaddr - loaddr, align);
+ if (addr == -1) {
+ /*
+ * If ARM_COMMPAGE, there *might* be a non-consecutive allocation
+ * that can satisfy both. But as the normal arm32 link base address
+ * is ~32k, and we extend down to include the commpage, making the
+ * overhead only ~96k, this is unlikely.
*/
- real_start = init_guest_space(host_start, host_size, loaddr, false);
- if (real_start == (unsigned long)-1) {
- errmsg = "Unable to find space for application";
- goto exit_errmsg;
- }
- guest_base = real_start - loaddr;
+ error_report("%s: Unable to allocate %#zx bytes of "
+ "virtual address space", image_name,
+ (size_t)(hiaddr - loaddr));
+ exit(EXIT_FAILURE);
+ }
+
+ guest_base = addr;
+}
+
+static void pgb_dynamic(const char *image_name, long align)
+{
+ /*
+ * The executable is dynamic and does not require a fixed address.
+ * All we need is a commpage that satisfies align.
+ * If we do not need a commpage, leave guest_base == 0.
+ */
+ if (ARM_COMMPAGE) {
+ uintptr_t addr, commpage;
- qemu_log_mask(CPU_LOG_PAGE, "Relocating guest address space from 0x"
- TARGET_ABI_FMT_lx " to 0x%lx\n",
- loaddr, real_start);
+ /* 64-bit hosts should have used reserved_va. */
+ assert(sizeof(uintptr_t) == 4);
+
+ /*
+ * By putting the commpage at the first hole, that puts guest_base
+ * just above that, and maximises the positive guest addresses.
+ */
+ commpage = ARM_COMMPAGE & -align;
+ addr = pgb_find_hole(commpage, -commpage, align);
+ assert(addr != -1);
+ guest_base = addr;
}
- return;
+}
-exit_errmsg:
- fprintf(stderr, "%s: %s\n", image_name, errmsg);
- exit(-1);
+static void pgb_reserved_va(const char *image_name, abi_ulong guest_loaddr,
+ abi_ulong guest_hiaddr, long align)
+{
+ const int flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_NORESERVE;
+ void *addr, *test;
+
+ if (guest_hiaddr > reserved_va) {
+ error_report("%s: requires more than reserved virtual "
+ "address space (0x%" PRIx64 " > 0x%lx)",
+ image_name, (uint64_t)guest_hiaddr, reserved_va);
+ exit(EXIT_FAILURE);
+ }
+
+ /* Widen the "image" to the entire reserved address space. */
+ pgb_static(image_name, 0, reserved_va, align);
+
+ /* Reserve the memory on the host. */
+ assert(guest_base != 0);
+ test = g2h(0);
+ addr = mmap(test, reserved_va, PROT_NONE, flags, -1, 0);
+ if (addr == MAP_FAILED) {
+ error_report("Unable to reserve 0x%lx bytes of virtual address "
+ "space for use as guest address space (check your "
+ "virtual memory ulimit setting or reserve less "
+ "using -R option)", reserved_va);
+ exit(EXIT_FAILURE);
+ }
+ assert(addr == test);
}
+void probe_guest_base(const char *image_name, abi_ulong guest_loaddr,
+ abi_ulong guest_hiaddr)
+{
+ /* In order to use host shmat, we must be able to honor SHMLBA. */
+ uintptr_t align = MAX(SHMLBA, qemu_host_page_size);
+
+ if (have_guest_base) {
+ pgb_have_guest_base(image_name, guest_loaddr, guest_hiaddr, align);
+ } else if (reserved_va) {
+ pgb_reserved_va(image_name, guest_loaddr, guest_hiaddr, align);
+ } else if (guest_loaddr) {
+ pgb_static(image_name, guest_loaddr, guest_hiaddr, align);
+ } else {
+ pgb_dynamic(image_name, align);
+ }
+
+ /* Reserve and initialize the commpage. */
+ if (!init_guest_commpage()) {
+ /*
+ * With have_guest_base, the user has selected the address and
+ * we are trying to work with that. Otherwise, we have selected
+ * free space and init_guest_commpage must succeeded.
+ */
+ assert(have_guest_base);
+ pgb_fail_in_use(image_name);
+ }
+
+ assert(QEMU_IS_ALIGNED(guest_base, align));
+ qemu_log_mask(CPU_LOG_PAGE, "Locating guest address space "
+ "@ 0x%" PRIx64 "\n", (uint64_t)guest_base);
+}
/* Load an ELF image into the address space.
@@ -2399,6 +2390,12 @@ static void load_elf_image(const char *image_name, int image_fd,
* MMAP_MIN_ADDR or the QEMU application itself.
*/
probe_guest_base(image_name, loaddr, hiaddr);
+ } else {
+ /*
+ * The binary is dynamic, but we still need to
+ * select guest_base. In this case we pass a size.
+ */
+ probe_guest_base(image_name, 0, hiaddr - loaddr);
}
}
diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index 66901f39cc..8fb448f0bf 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -441,6 +441,12 @@ static int load_flat_file(struct linux_binprm * bprm,
indx_len = MAX_SHARED_LIBS * sizeof(abi_ulong);
indx_len = (indx_len + 15) & ~(abi_ulong)15;
+ /*
+ * Alloate the address space.
+ */
+ probe_guest_base(bprm->filename, 0,
+ text_len + data_len + extra + indx_len);
+
/*
* there are a couple of cases here, the separate code/data
* case, and then the fully copied to RAM case which lumps
diff --git a/linux-user/main.c b/linux-user/main.c
index 22578b1633..1d20a83d4e 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -24,6 +24,7 @@
#include "qemu-version.h"
#include <sys/syscall.h>
#include <sys/resource.h>
+#include <sys/shm.h>
#include "qapi/error.h"
#include "qemu.h"
@@ -747,28 +748,6 @@ int main(int argc, char **argv, char **envp)
target_environ = envlist_to_environ(envlist, NULL);
envlist_free(envlist);
- /*
- * Now that page sizes are configured in tcg_exec_init() we can do
- * proper page alignment for guest_base.
- */
- guest_base = HOST_PAGE_ALIGN(guest_base);
-
- if (reserved_va || have_guest_base) {
- guest_base = init_guest_space(guest_base, reserved_va, 0,
- have_guest_base);
- if (guest_base == (unsigned long)-1) {
- fprintf(stderr, "Unable to reserve 0x%lx bytes of virtual address "
- "space for use as guest address space (check your virtual "
- "memory ulimit setting or reserve less using -R option)\n",
- reserved_va);
- exit(EXIT_FAILURE);
- }
-
- if (reserved_va) {
- mmap_next_start = reserved_va;
- }
- }
-
/*
* Read in mmap_min_addr kernel parameter. This value is used
* When loading the ELF image to determine whether guest_base
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 02/14] exec/cpu-all: Use bool for have_guest_base
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
2020-04-23 17:05 ` [PATCH v1 01/14] linux-user: completely re-write init_guest_space Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 17:05 ` [PATCH v1 03/14] accel/tcg: Relax va restrictions on 64-bit guests Alex Bennée
` (13 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Riku Voipio, Richard Henderson,
Laurent Vivier, Paolo Bonzini, Alex Bennée,
Richard Henderson
From: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/exec/cpu-all.h | 2 +-
bsd-user/main.c | 4 ++--
linux-user/main.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 49384bb66a..b4fb5832c4 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -159,7 +159,7 @@ static inline void tswap64s(uint64_t *s)
* This allows the guest address space to be offset to a convenient location.
*/
extern unsigned long guest_base;
-extern int have_guest_base;
+extern bool have_guest_base;
extern unsigned long reserved_va;
#if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 770c2b267a..aef5531628 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -42,7 +42,7 @@
int singlestep;
unsigned long mmap_min_addr;
unsigned long guest_base;
-int have_guest_base;
+bool have_guest_base;
unsigned long reserved_va;
static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
@@ -828,7 +828,7 @@ int main(int argc, char **argv)
}
} else if (!strcmp(r, "B")) {
guest_base = strtol(argv[optind++], NULL, 0);
- have_guest_base = 1;
+ have_guest_base = true;
} else if (!strcmp(r, "drop-ld-preload")) {
(void) envlist_unsetenv(envlist, "LD_PRELOAD");
} else if (!strcmp(r, "bsd")) {
diff --git a/linux-user/main.c b/linux-user/main.c
index 1d20a83d4e..90ad365b43 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -59,7 +59,7 @@ static const char *cpu_type;
static const char *seed_optarg;
unsigned long mmap_min_addr;
unsigned long guest_base;
-int have_guest_base;
+bool have_guest_base;
/*
* Used to implement backwards-compatibility for the `-strace`, and
@@ -334,7 +334,7 @@ static void handle_arg_cpu(const char *arg)
static void handle_arg_guest_base(const char *arg)
{
guest_base = strtol(arg, NULL, 0);
- have_guest_base = 1;
+ have_guest_base = true;
}
static void handle_arg_reserved_va(const char *arg)
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 03/14] accel/tcg: Relax va restrictions on 64-bit guests
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
2020-04-23 17:05 ` [PATCH v1 01/14] linux-user: completely re-write init_guest_space Alex Bennée
2020-04-23 17:05 ` [PATCH v1 02/14] exec/cpu-all: Use bool for have_guest_base Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 17:05 ` [PATCH v1 04/14] .gitignore: include common build sub-directories Alex Bennée
` (12 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Paolo Bonzini, Richard Henderson,
Richard Henderson
From: Richard Henderson <richard.henderson@linaro.org>
We cannot at present limit a 64-bit guest to a virtual address
space smaller than the host. It will mostly work to ignore this
limitation, except if the guest uses high bits of the address
space for tags. But it will certainly work better, as presently
we can wind up failing to allocate the guest stack.
Widen our user-only page tree to the host or abi pointer width.
Remove the workaround for this problem from target/alpha.
Always validate guest addresses vs reserved_va, as there we
control allocation ourselves.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/exec/cpu-all.h | 23 +++++++++++++++++++----
target/alpha/cpu-param.h | 15 ++-------------
accel/tcg/translate-all.c | 15 +++++++++------
3 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index b4fb5832c4..c0c2fa3cc5 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -162,12 +162,27 @@ extern unsigned long guest_base;
extern bool have_guest_base;
extern unsigned long reserved_va;
-#if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
-#define GUEST_ADDR_MAX (~0ul)
+/*
+ * Limit the guest addresses as best we can.
+ *
+ * When not using -R reserved_va, we cannot really limit the guest
+ * to less address space than the host. For 32-bit guests, this
+ * acts as a sanity check that we're not giving the guest an address
+ * that it cannot even represent. For 64-bit guests... the address
+ * might not be what the real kernel would give, but it is at least
+ * representable in the guest.
+ *
+ * TODO: Improve address allocation to avoid this problem, and to
+ * avoid setting bits at the top of guest addresses that might need
+ * to be used for tags.
+ */
+#if MIN(TARGET_VIRT_ADDR_SPACE_BITS, TARGET_ABI_BITS) <= 32
+# define GUEST_ADDR_MAX_ UINT32_MAX
#else
-#define GUEST_ADDR_MAX (reserved_va ? reserved_va - 1 : \
- (1ul << TARGET_VIRT_ADDR_SPACE_BITS) - 1)
+# define GUEST_ADDR_MAX_ ~0ul
#endif
+#define GUEST_ADDR_MAX (reserved_va ? reserved_va - 1 : GUEST_ADDR_MAX_)
+
#else
#include "exec/hwaddr.h"
diff --git a/target/alpha/cpu-param.h b/target/alpha/cpu-param.h
index 692aee27ca..1153992e42 100644
--- a/target/alpha/cpu-param.h
+++ b/target/alpha/cpu-param.h
@@ -10,22 +10,11 @@
#define TARGET_LONG_BITS 64
#define TARGET_PAGE_BITS 13
-#ifdef CONFIG_USER_ONLY
-/*
- * ??? The kernel likes to give addresses in high memory. If the host has
- * more virtual address space than the guest, this can lead to impossible
- * allocations. Honor the long-standing assumption that only kernel addrs
- * are negative, but otherwise allow allocations anywhere. This could lead
- * to tricky emulation problems for programs doing tagged addressing, but
- * that's far fewer than encounter the impossible allocation problem.
- */
-#define TARGET_PHYS_ADDR_SPACE_BITS 63
-#define TARGET_VIRT_ADDR_SPACE_BITS 63
-#else
+
/* ??? EV4 has 34 phys addr bits, EV5 has 40, EV6 has 44. */
#define TARGET_PHYS_ADDR_SPACE_BITS 44
#define TARGET_VIRT_ADDR_SPACE_BITS (30 + TARGET_PAGE_BITS)
-#endif
+
#define NB_MMU_MODES 3
#endif
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 9924e66d1f..e4f703a7e6 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -173,8 +173,13 @@ struct page_collection {
#define TB_FOR_EACH_JMP(head_tb, tb, n) \
TB_FOR_EACH_TAGGED((head_tb)->jmp_list_head, tb, n, jmp_list_next)
-/* In system mode we want L1_MAP to be based on ram offsets,
- while in user mode we want it to be based on virtual addresses. */
+/*
+ * In system mode we want L1_MAP to be based on ram offsets,
+ * while in user mode we want it to be based on virtual addresses.
+ *
+ * TODO: For user mode, see the caveat re host vs guest virtual
+ * address spaces near GUEST_ADDR_MAX.
+ */
#if !defined(CONFIG_USER_ONLY)
#if HOST_LONG_BITS < TARGET_PHYS_ADDR_SPACE_BITS
# define L1_MAP_ADDR_SPACE_BITS HOST_LONG_BITS
@@ -182,7 +187,7 @@ struct page_collection {
# define L1_MAP_ADDR_SPACE_BITS TARGET_PHYS_ADDR_SPACE_BITS
#endif
#else
-# define L1_MAP_ADDR_SPACE_BITS TARGET_VIRT_ADDR_SPACE_BITS
+# define L1_MAP_ADDR_SPACE_BITS MIN(HOST_LONG_BITS, TARGET_ABI_BITS)
#endif
/* Size of the L2 (and L3, etc) page tables. */
@@ -2497,9 +2502,7 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
/* This function should never be called with addresses outside the
guest address space. If this assert fires, it probably indicates
a missing call to h2g_valid. */
-#if TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS
- assert(end <= ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
-#endif
+ assert(end - 1 <= GUEST_ADDR_MAX);
assert(start < end);
assert_memory_lock();
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 04/14] .gitignore: include common build sub-directories
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (2 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 03/14] accel/tcg: Relax va restrictions on 64-bit guests Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 17:05 ` [PATCH v1 05/14] configure: favour gdb-multiarch if we have it Alex Bennée
` (11 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée, Philippe Mathieu-Daudé
As out-of-tree builds become more common (or rather building in a
subdir) we can add a lot of load to "git ls-files" as it hunts down
sub-directories that are irrelevant to the source tree. This is
especially annoying if you have a prompt that attempts to summarise
the current git status on command completion.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v2
- use build*/ to capture build and it's variants
---
.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/.gitignore b/.gitignore
index 0c5af83aa7..8dccb61a44 100644
--- a/.gitignore
+++ b/.gitignore
@@ -141,6 +141,7 @@ cscope.*
tags
TAGS
docker-src.*
+build*/
*~
*.ast_raw
*.depend_raw
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 05/14] configure: favour gdb-multiarch if we have it
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (3 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 04/14] .gitignore: include common build sub-directories Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 20:28 ` Philippe Mathieu-Daudé
2020-04-23 17:05 ` [PATCH v1 06/14] gdbstub: Introduce gdb_get_float64() to get 64-bit float registers Alex Bennée
` (10 subsequent siblings)
15 siblings, 1 reply; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée
As gdb will generally be talking to "foreign" guests lets use that if
we can. Otherwise the chances of gdb barfing are considerably higher.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 23b5e93752..c58787100f 100755
--- a/configure
+++ b/configure
@@ -303,7 +303,7 @@ libs_qga=""
debug_info="yes"
stack_protector=""
use_containers="yes"
-gdb_bin=$(command -v "gdb")
+gdb_bin=$(command -v "gdb-multiarch" || command -v "gdb")
if test -e "$source_path/.git"
then
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 05/14] configure: favour gdb-multiarch if we have it
2020-04-23 17:05 ` [PATCH v1 05/14] configure: favour gdb-multiarch if we have it Alex Bennée
@ 2020-04-23 20:28 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-23 20:28 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
On 4/23/20 7:05 PM, Alex Bennée wrote:
> As gdb will generally be talking to "foreign" guests lets use that if
> we can. Otherwise the chances of gdb barfing are considerably higher.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> configure | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 23b5e93752..c58787100f 100755
> --- a/configure
> +++ b/configure
> @@ -303,7 +303,7 @@ libs_qga=""
> debug_info="yes"
> stack_protector=""
> use_containers="yes"
> -gdb_bin=$(command -v "gdb")
> +gdb_bin=$(command -v "gdb-multiarch" || command -v "gdb")
>
Good one!
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> if test -e "$source_path/.git"
> then
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 06/14] gdbstub: Introduce gdb_get_float64() to get 64-bit float registers
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (4 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 05/14] configure: favour gdb-multiarch if we have it Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 17:05 ` [PATCH v1 07/14] tests/tcg: better trap gdb failures Alex Bennée
` (9 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: open list:PowerPC TCG CPUs, Alex Bennée,
Philippe Mathieu-Daudé, Laurent Vivier, David Gibson
From: Philippe Mathieu-Daudé <philmd@redhat.com>
When converted to use GByteArray in commits 462474d760c and
a010bdbe719, the call to stfq_p() was removed. This call
serialize a float.
Since we now use a GByteArray, we can not use stfq_p() directly.
Introduce the gdb_get_float64() helper to load a float64 register.
Fixes: 462474d760c ("target/m68k: use gdb_get_reg helpers")
Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200414163853.12164-3-philmd@redhat.com>
---
include/exec/gdbstub.h | 11 +++++++++++
target/m68k/helper.c | 3 ++-
target/ppc/gdbstub.c | 4 ++--
target/ppc/translate_init.inc.c | 2 +-
4 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 20e1072692..4a2b8e3089 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -134,6 +134,17 @@ static inline int gdb_get_float32(GByteArray *array, float32 val)
return sizeof(buf);
}
+
+static inline int gdb_get_float64(GByteArray *array, float64 val)
+{
+ uint8_t buf[sizeof(CPU_DoubleU)];
+
+ stfq_p(buf, val);
+ g_byte_array_append(array, buf, sizeof(buf));
+
+ return sizeof(buf);
+}
+
static inline int gdb_get_zeroes(GByteArray *array, size_t len)
{
guint oldlen = array->len;
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index cad4083895..79b0b10ea9 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -72,7 +72,8 @@ static int cf_fpu_gdb_get_reg(CPUM68KState *env, GByteArray *mem_buf, int n)
{
if (n < 8) {
float_status s;
- return gdb_get_reg64(mem_buf, floatx80_to_float64(env->fregs[n].d, &s));
+ return gdb_get_float64(mem_buf,
+ floatx80_to_float64(env->fregs[n].d, &s));
}
switch (n) {
case 8: /* fpcontrol */
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index eb362dd9ae..5c11c88b2a 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -130,7 +130,7 @@ int ppc_cpu_gdb_read_register(CPUState *cs, GByteArray *buf, int n)
gdb_get_regl(buf, env->gpr[n]);
} else if (n < 64) {
/* fprs */
- gdb_get_reg64(buf, *cpu_fpr_ptr(env, n - 32));
+ gdb_get_float64(buf, *cpu_fpr_ptr(env, n - 32));
} else {
switch (n) {
case 64:
@@ -184,7 +184,7 @@ int ppc_cpu_gdb_read_register_apple(CPUState *cs, GByteArray *buf, int n)
gdb_get_reg64(buf, env->gpr[n]);
} else if (n < 64) {
/* fprs */
- gdb_get_reg64(buf, *cpu_fpr_ptr(env, n - 32));
+ gdb_get_float64(buf, *cpu_fpr_ptr(env, n - 32));
} else if (n < 96) {
/* Altivec */
gdb_get_reg64(buf, n - 64);
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index e853164a86..d825cb5975 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -9881,7 +9881,7 @@ static int gdb_get_float_reg(CPUPPCState *env, GByteArray *buf, int n)
{
uint8_t *mem_buf;
if (n < 32) {
- gdb_get_reg64(buf, *cpu_fpr_ptr(env, n));
+ gdb_get_float64(buf, *cpu_fpr_ptr(env, n));
mem_buf = gdb_get_reg_ptr(buf, 8);
ppc_maybe_bswap_register(env, mem_buf, 8);
return 8;
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 07/14] tests/tcg: better trap gdb failures
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (5 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 06/14] gdbstub: Introduce gdb_get_float64() to get 64-bit float registers Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 17:05 ` [PATCH v1 08/14] tests/tcg: drop inferior.was_attached() test Alex Bennée
` (8 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, open list:ARM TCG CPUs, Alex Bennée
It seems older and non-multiarach aware GDBs might not fail gracefully
when faced with something they don't know. For example when faced with
a target XML for s390x the Ubuntu 18.04 gdb will generate an internal
fault and prompt for a core dump.
Work around this by invoking GDB in a more batch orientated way and
then trying to filter out between test failures and gdb failures.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
tests/guest-debug/run-test.py | 19 ++++++++++++++++++-
tests/tcg/aarch64/gdbstub/test-sve-ioctl.py | 1 -
tests/tcg/aarch64/gdbstub/test-sve.py | 1 -
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/tests/guest-debug/run-test.py b/tests/guest-debug/run-test.py
index 8c49ee2f22..2bbb8fbaa3 100755
--- a/tests/guest-debug/run-test.py
+++ b/tests/guest-debug/run-test.py
@@ -50,8 +50,25 @@ if __name__ == '__main__':
inferior = subprocess.Popen(shlex.split(cmd))
# Now launch gdb with our test and collect the result
- gdb_cmd = "%s %s -ex 'target remote localhost:1234' -x %s" % (args.gdb, args.binary, args.test)
+ gdb_cmd = "%s %s" % (args.gdb, args.binary)
+ # run quietly and ignore .gdbinit
+ gdb_cmd += " -q -n -batch"
+ # disable prompts in case of crash
+ gdb_cmd += " -ex 'set confirm off'"
+ # connect to remote
+ gdb_cmd += " -ex 'target remote localhost:1234'"
+ # finally the test script itself
+ gdb_cmd += " -x %s" % (args.test)
+
+ print("GDB CMD: %s" % (gdb_cmd))
result = subprocess.call(gdb_cmd, shell=True);
+ # A negative result is the result of an internal gdb failure like
+ # a crash. We force a return of 0 so we don't fail the test on
+ # account of broken external tools.
+ if result < 0:
+ print("GDB crashed? SKIPPING")
+ exit(0)
+
exit(result)
diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
index 984fbeb277..387b2fc20a 100644
--- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
+++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
@@ -70,7 +70,6 @@ except (gdb.error, AttributeError):
try:
# These are not very useful in scripts
gdb.execute("set pagination off")
- gdb.execute("set confirm off")
# Run the actual tests
run_test()
diff --git a/tests/tcg/aarch64/gdbstub/test-sve.py b/tests/tcg/aarch64/gdbstub/test-sve.py
index dbe7f2aa93..5995689625 100644
--- a/tests/tcg/aarch64/gdbstub/test-sve.py
+++ b/tests/tcg/aarch64/gdbstub/test-sve.py
@@ -71,7 +71,6 @@ except (gdb.error, AttributeError):
try:
# These are not very useful in scripts
gdb.execute("set pagination off")
- gdb.execute("set confirm off")
# Run the actual tests
run_test()
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 08/14] tests/tcg: drop inferior.was_attached() test
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (6 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 07/14] tests/tcg: better trap gdb failures Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 17:05 ` [PATCH v1 09/14] gdbstub: eliminate gdbserver_fd global Alex Bennée
` (7 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, open list:ARM TCG CPUs, Alex Bennée
This test seems flaky and reports attachment even when we failed to
negotiate the architecture. However the fetching of the guest
architecture will fail tripping up the gdb AttributeError which will
trigger our early no error status exit from the test
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
tests/tcg/aarch64/gdbstub/test-sve-ioctl.py | 3 ---
tests/tcg/aarch64/gdbstub/test-sve.py | 3 ---
2 files changed, 6 deletions(-)
diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
index 387b2fc20a..972cf73c31 100644
--- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
+++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
@@ -58,9 +58,6 @@ def run_test():
#
try:
inferior = gdb.selected_inferior()
- if inferior.was_attached == False:
- print("SKIPPING (failed to attach)", file=sys.stderr)
- exit(0)
arch = inferior.architecture()
report(arch.name() == "aarch64", "connected to aarch64")
except (gdb.error, AttributeError):
diff --git a/tests/tcg/aarch64/gdbstub/test-sve.py b/tests/tcg/aarch64/gdbstub/test-sve.py
index 5995689625..b96bdbb99a 100644
--- a/tests/tcg/aarch64/gdbstub/test-sve.py
+++ b/tests/tcg/aarch64/gdbstub/test-sve.py
@@ -59,9 +59,6 @@ def run_test():
#
try:
inferior = gdb.selected_inferior()
- if inferior.was_attached == False:
- print("SKIPPING (failed to attach)", file=sys.stderr)
- exit(0)
arch = inferior.architecture()
report(arch.name() == "aarch64", "connected to aarch64")
except (gdb.error, AttributeError):
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 09/14] gdbstub: eliminate gdbserver_fd global
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (7 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 08/14] tests/tcg: drop inferior.was_attached() test Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 20:30 ` Philippe Mathieu-Daudé
2020-04-23 17:05 ` [PATCH v1 10/14] gdbstub/linux-user: support debugging over a unix socket Alex Bennée
` (6 subsequent siblings)
15 siblings, 1 reply; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Alex Bennée
We don't really need to track this fd beyond the initial creation of
the socket. We already know if the system has been initialised by
virtue of the gdbserver_state so lets remove it. This makes the later
re-factoring easier.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
gdbstub.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 171e150950..8c53cc0e1c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -398,8 +398,6 @@ static void reset_gdbserver_state(void)
bool gdb_has_xml;
#ifdef CONFIG_USER_ONLY
-/* XXX: This is not thread safe. Do we care? */
-static int gdbserver_fd = -1;
static int get_char(void)
{
@@ -2964,7 +2962,7 @@ void gdb_exit(CPUArchState *env, int code)
return;
}
#ifdef CONFIG_USER_ONLY
- if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
+ if (gdbserver_state.fd < 0) {
return;
}
#endif
@@ -3011,7 +3009,7 @@ gdb_handlesig(CPUState *cpu, int sig)
char buf[256];
int n;
- if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
+ if (!gdbserver_state.init || gdbserver_state.fd < 0) {
return sig;
}
@@ -3060,7 +3058,7 @@ void gdb_signalled(CPUArchState *env, int sig)
{
char buf[4];
- if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
+ if (!gdbserver_state.init || gdbserver_state.fd < 0) {
return;
}
@@ -3068,7 +3066,7 @@ void gdb_signalled(CPUArchState *env, int sig)
put_packet(buf);
}
-static bool gdb_accept(void)
+static bool gdb_accept(int gdb_fd)
{
struct sockaddr_in sockaddr;
socklen_t len;
@@ -3076,7 +3074,7 @@ static bool gdb_accept(void)
for(;;) {
len = sizeof(sockaddr);
- fd = accept(gdbserver_fd, (struct sockaddr *)&sockaddr, &len);
+ fd = accept(gdb_fd, (struct sockaddr *)&sockaddr, &len);
if (fd < 0 && errno != EINTR) {
perror("accept");
return false;
@@ -3137,13 +3135,12 @@ static int gdbserver_open(int port)
int gdbserver_start(int port)
{
- gdbserver_fd = gdbserver_open(port);
- if (gdbserver_fd < 0)
+ int gdb_fd = gdbserver_open(port);
+ if (gdb_fd < 0)
return -1;
/* accept connections */
- if (!gdb_accept()) {
- close(gdbserver_fd);
- gdbserver_fd = -1;
+ if (!gdb_accept(gdb_fd)) {
+ close(gdb_fd);
return -1;
}
return 0;
@@ -3152,7 +3149,7 @@ int gdbserver_start(int port)
/* Disable gdb stub for child processes. */
void gdbserver_fork(CPUState *cpu)
{
- if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
+ if (!gdbserver_state.init || gdbserver_state.fd < 0) {
return;
}
close(gdbserver_state.fd);
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 09/14] gdbstub: eliminate gdbserver_fd global
2020-04-23 17:05 ` [PATCH v1 09/14] gdbstub: eliminate gdbserver_fd global Alex Bennée
@ 2020-04-23 20:30 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-23 20:30 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
On 4/23/20 7:05 PM, Alex Bennée wrote:
> We don't really need to track this fd beyond the initial creation of
> the socket. We already know if the system has been initialised by
> virtue of the gdbserver_state so lets remove it. This makes the later
> re-factoring easier.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> gdbstub.c | 23 ++++++++++-------------
> 1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 171e150950..8c53cc0e1c 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -398,8 +398,6 @@ static void reset_gdbserver_state(void)
> bool gdb_has_xml;
>
> #ifdef CONFIG_USER_ONLY
> -/* XXX: This is not thread safe. Do we care? */
> -static int gdbserver_fd = -1;
>
> static int get_char(void)
> {
> @@ -2964,7 +2962,7 @@ void gdb_exit(CPUArchState *env, int code)
> return;
> }
> #ifdef CONFIG_USER_ONLY
> - if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
> + if (gdbserver_state.fd < 0) {
> return;
> }
> #endif
> @@ -3011,7 +3009,7 @@ gdb_handlesig(CPUState *cpu, int sig)
> char buf[256];
> int n;
>
> - if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
> + if (!gdbserver_state.init || gdbserver_state.fd < 0) {
> return sig;
> }
>
> @@ -3060,7 +3058,7 @@ void gdb_signalled(CPUArchState *env, int sig)
> {
> char buf[4];
>
> - if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
> + if (!gdbserver_state.init || gdbserver_state.fd < 0) {
> return;
> }
>
> @@ -3068,7 +3066,7 @@ void gdb_signalled(CPUArchState *env, int sig)
> put_packet(buf);
> }
>
> -static bool gdb_accept(void)
> +static bool gdb_accept(int gdb_fd)
> {
> struct sockaddr_in sockaddr;
> socklen_t len;
> @@ -3076,7 +3074,7 @@ static bool gdb_accept(void)
>
> for(;;) {
> len = sizeof(sockaddr);
> - fd = accept(gdbserver_fd, (struct sockaddr *)&sockaddr, &len);
> + fd = accept(gdb_fd, (struct sockaddr *)&sockaddr, &len);
> if (fd < 0 && errno != EINTR) {
> perror("accept");
> return false;
> @@ -3137,13 +3135,12 @@ static int gdbserver_open(int port)
>
> int gdbserver_start(int port)
> {
> - gdbserver_fd = gdbserver_open(port);
> - if (gdbserver_fd < 0)
> + int gdb_fd = gdbserver_open(port);
> + if (gdb_fd < 0)
> return -1;
> /* accept connections */
> - if (!gdb_accept()) {
> - close(gdbserver_fd);
> - gdbserver_fd = -1;
> + if (!gdb_accept(gdb_fd)) {
> + close(gdb_fd);
> return -1;
> }
> return 0;
> @@ -3152,7 +3149,7 @@ int gdbserver_start(int port)
> /* Disable gdb stub for child processes. */
> void gdbserver_fork(CPUState *cpu)
> {
> - if (gdbserver_fd < 0 || gdbserver_state.fd < 0) {
> + if (!gdbserver_state.init || gdbserver_state.fd < 0) {
> return;
> }
> close(gdbserver_state.fd);
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 10/14] gdbstub/linux-user: support debugging over a unix socket
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (8 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 09/14] gdbstub: eliminate gdbserver_fd global Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 17:05 ` [PATCH v1 11/14] tests/guest-debug: use the unix socket for linux-user tests Alex Bennée
` (5 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé, Riku Voipio, Alex Bennée,
Laurent Vivier
While debugging over TCP is fairly straightforward now we have test
cases that want to orchestrate via make and currently a parallel build
fails as two processes can't use the same listening port. While system
emulation offers a wide cornucopia of connection methods thanks to the
chardev abstraction we are a little more limited for linux user.
Thankfully the programming API for a TCP socket and a local UNIX
socket is pretty much the same once it's set up.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/exec/gdbstub.h | 14 ++++--
bsd-user/main.c | 8 +--
gdbstub.c | 107 ++++++++++++++++++++++++++++++++++-------
linux-user/main.c | 12 ++---
4 files changed, 108 insertions(+), 33 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 4a2b8e3089..94d8f83e92 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -177,11 +177,15 @@ static inline uint8_t * gdb_get_reg_ptr(GByteArray *buf, int len)
#endif
-#ifdef CONFIG_USER_ONLY
-int gdbserver_start(int);
-#else
-int gdbserver_start(const char *port);
-#endif
+/**
+ * gdbserver_start: start the gdb server
+ * @port_or_device: connection spec for gdb
+ *
+ * For CONFIG_USER this is either a tcp port or a path to a fifo. For
+ * system emulation you can use a full chardev spec for your gdbserver
+ * port.
+ */
+int gdbserver_start(const char *port_or_device);
void gdbserver_cleanup(void);
diff --git a/bsd-user/main.c b/bsd-user/main.c
index aef5531628..0bfe46cff9 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -738,7 +738,7 @@ int main(int argc, char **argv)
CPUState *cpu;
int optind;
const char *r;
- int gdbstub_port = 0;
+ const char *gdbstub = NULL;
char **target_environ, **wrk;
envlist_t *envlist = NULL;
char *trace_file = NULL;
@@ -814,7 +814,7 @@ int main(int argc, char **argv)
exit(1);
}
} else if (!strcmp(r, "g")) {
- gdbstub_port = atoi(argv[optind++]);
+ gdbstub = g_strdup(argv[optind++]);
} else if (!strcmp(r, "r")) {
qemu_uname_release = argv[optind++];
} else if (!strcmp(r, "cpu")) {
@@ -1124,8 +1124,8 @@ int main(int argc, char **argv)
#error unsupported target CPU
#endif
- if (gdbstub_port) {
- gdbserver_start (gdbstub_port);
+ if (gdbstub) {
+ gdbserver_start(gdbstub);
gdb_handlesig(cpu, 0);
}
cpu_loop(env);
diff --git a/gdbstub.c b/gdbstub.c
index 8c53cc0e1c..51e2ab9110 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -355,6 +355,7 @@ typedef struct GDBState {
int signal;
#ifdef CONFIG_USER_ONLY
int fd;
+ char *socket_path;
int running_state;
#else
CharBackend chr;
@@ -2962,6 +2963,9 @@ void gdb_exit(CPUArchState *env, int code)
return;
}
#ifdef CONFIG_USER_ONLY
+ if (gdbserver_state.socket_path) {
+ unlink(gdbserver_state.socket_path);
+ }
if (gdbserver_state.fd < 0) {
return;
}
@@ -3034,7 +3038,6 @@ gdb_handlesig(CPUState *cpu, int sig)
n = read(gdbserver_state.fd, buf, 256);
if (n > 0) {
int i;
-
for (i = 0; i < n; i++) {
gdb_read_byte(buf[i]);
}
@@ -3066,7 +3069,66 @@ void gdb_signalled(CPUArchState *env, int sig)
put_packet(buf);
}
-static bool gdb_accept(int gdb_fd)
+static void gdb_accept_init(int fd)
+{
+ init_gdbserver_state();
+ create_default_process(&gdbserver_state);
+ gdbserver_state.processes[0].attached = true;
+ gdbserver_state.c_cpu = gdb_first_attached_cpu();
+ gdbserver_state.g_cpu = gdbserver_state.c_cpu;
+ gdbserver_state.fd = fd;
+ gdb_has_xml = false;
+}
+
+static bool gdb_accept_socket(int gdb_fd)
+{
+ int fd;
+
+ for(;;) {
+ fd = accept(gdb_fd, NULL, NULL);
+ if (fd < 0 && errno != EINTR) {
+ perror("accept socket");
+ return false;
+ } else if (fd >= 0) {
+ qemu_set_cloexec(fd);
+ break;
+ }
+ }
+
+ gdb_accept_init(fd);
+ return true;
+}
+
+static int gdbserver_open_socket(const char *path)
+{
+ struct sockaddr_un sockaddr;
+ int fd, ret;
+
+ fd = socket(AF_UNIX, SOCK_STREAM, 0);
+ if (fd < 0) {
+ perror("create socket");
+ return -1;
+ }
+
+ sockaddr.sun_family = AF_UNIX;
+ pstrcpy(sockaddr.sun_path, sizeof(sockaddr.sun_path)-1, path);
+ ret = bind(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr));
+ if (ret < 0) {
+ perror("bind socket");
+ close(fd);
+ return -1;
+ }
+ ret = listen(fd, 1);
+ if (ret < 0) {
+ perror("listen socket");
+ close(fd);
+ return -1;
+ }
+
+ return fd;
+}
+
+static bool gdb_accept_tcp(int gdb_fd)
{
struct sockaddr_in sockaddr;
socklen_t len;
@@ -3091,17 +3153,11 @@ static bool gdb_accept(int gdb_fd)
return false;
}
- init_gdbserver_state();
- create_default_process(&gdbserver_state);
- gdbserver_state.processes[0].attached = true;
- gdbserver_state.c_cpu = gdb_first_attached_cpu();
- gdbserver_state.g_cpu = gdbserver_state.c_cpu;
- gdbserver_state.fd = fd;
- gdb_has_xml = false;
+ gdb_accept_init(fd);
return true;
}
-static int gdbserver_open(int port)
+static int gdbserver_open_port(int port)
{
struct sockaddr_in sockaddr;
int fd, ret;
@@ -3130,20 +3186,35 @@ static int gdbserver_open(int port)
close(fd);
return -1;
}
+
return fd;
}
-int gdbserver_start(int port)
+int gdbserver_start(const char *port_or_path)
{
- int gdb_fd = gdbserver_open(port);
- if (gdb_fd < 0)
- return -1;
- /* accept connections */
- if (!gdb_accept(gdb_fd)) {
- close(gdb_fd);
+ int port = g_ascii_strtoull(port_or_path, NULL, 10);
+ int gdb_fd;
+
+ if (port > 0) {
+ gdb_fd = gdbserver_open_port(port);
+ } else {
+ gdb_fd = gdbserver_open_socket(port_or_path);
+ }
+
+ if (gdb_fd < 0) {
return -1;
}
- return 0;
+
+ if (port > 0 && gdb_accept_tcp(gdb_fd)) {
+ return 0;
+ } else if (gdb_accept_socket(gdb_fd)) {
+ gdbserver_state.socket_path = g_strdup(port_or_path);
+ return 0;
+ }
+
+ /* gone wrong */
+ close(gdb_fd);
+ return -1;
}
/* Disable gdb stub for child processes. */
diff --git a/linux-user/main.c b/linux-user/main.c
index 90ad365b43..3597e99bb1 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -52,7 +52,7 @@ char *exec_path;
int singlestep;
static const char *argv0;
-static int gdbstub_port;
+static const char *gdbstub;
static envlist_t *envlist;
static const char *cpu_model;
static const char *cpu_type;
@@ -311,7 +311,7 @@ static void handle_arg_seed(const char *arg)
static void handle_arg_gdb(const char *arg)
{
- gdbstub_port = atoi(arg);
+ gdbstub = g_strdup(arg);
}
static void handle_arg_uname(const char *arg)
@@ -840,10 +840,10 @@ int main(int argc, char **argv, char **envp)
target_cpu_copy_regs(env, regs);
- if (gdbstub_port) {
- if (gdbserver_start(gdbstub_port) < 0) {
- fprintf(stderr, "qemu: could not open gdbserver on port %d\n",
- gdbstub_port);
+ if (gdbstub) {
+ if (gdbserver_start(gdbstub) < 0) {
+ fprintf(stderr, "qemu: could not open gdbserver on %s\n",
+ gdbstub);
exit(EXIT_FAILURE);
}
gdb_handlesig(cpu, 0);
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 11/14] tests/guest-debug: use the unix socket for linux-user tests
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (9 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 10/14] gdbstub/linux-user: support debugging over a unix socket Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 17:05 ` [PATCH v1 12/14] tests/tcg: add a multiarch linux-user gdb test Alex Bennée
` (4 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée
Now we have support for debugging over a unix socket for linux-user
lets use it in our test harness.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
tests/guest-debug/run-test.py | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tests/guest-debug/run-test.py b/tests/guest-debug/run-test.py
index 2bbb8fbaa3..2441a1d623 100755
--- a/tests/guest-debug/run-test.py
+++ b/tests/guest-debug/run-test.py
@@ -15,6 +15,8 @@ import argparse
import subprocess
import shutil
import shlex
+import os
+from tempfile import TemporaryDirectory
def get_args():
parser = argparse.ArgumentParser(description="A gdbstub test runner")
@@ -41,11 +43,14 @@ if __name__ == '__main__':
print("We need gdb to run the test")
exit(-1)
+ socket_dir = TemporaryDirectory("qemu-gdbstub")
+ socket_name = os.path.join(socket_dir.name, "gdbstub.socket")
+
# Launch QEMU with binary
if "system" in args.qemu:
cmd = "%s %s %s -s -S" % (args.qemu, args.qargs, args.binary)
else:
- cmd = "%s %s -g 1234 %s" % (args.qemu, args.qargs, args.binary)
+ cmd = "%s %s -g %s %s" % (args.qemu, args.qargs, socket_name, args.binary)
inferior = subprocess.Popen(shlex.split(cmd))
@@ -56,7 +61,10 @@ if __name__ == '__main__':
# disable prompts in case of crash
gdb_cmd += " -ex 'set confirm off'"
# connect to remote
- gdb_cmd += " -ex 'target remote localhost:1234'"
+ if "system" in args.qemu:
+ gdb_cmd += " -ex 'target remote localhost:1234'"
+ else:
+ gdb_cmd += " -ex 'target remote %s'" % (socket_name)
# finally the test script itself
gdb_cmd += " -x %s" % (args.test)
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 12/14] tests/tcg: add a multiarch linux-user gdb test
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (10 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 11/14] tests/guest-debug: use the unix socket for linux-user tests Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 17:05 ` [PATCH v1 13/14] .travis.yml: show free disk space at end of run Alex Bennée
` (3 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, open list:ARM TCG CPUs, Alex Bennée,
Edgar E. Iglesias
When the gdbstub code was converted to the new API we missed a few
snafus in the various guests. Add a simple gdb test script which can
be used on all our linux-user guests to check for obvious failures.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- use EXTRA_RUNS to queue the tests so as not to break plugins
---
tests/tcg/aarch64/Makefile.target | 5 +-
tests/tcg/cris/Makefile.target | 1 +
tests/tcg/multiarch/Makefile.target | 14 +++++
tests/tcg/multiarch/gdbstub/sha1.py | 81 +++++++++++++++++++++++++++++
4 files changed, 98 insertions(+), 3 deletions(-)
create mode 100644 tests/tcg/multiarch/gdbstub/sha1.py
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index d99b2a9ece..312f36cde5 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -54,9 +54,6 @@ sve-ioctls: CFLAGS+=-march=armv8.1-a+sve
ifneq ($(HAVE_GDB_BIN),)
GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py
-AARCH64_TESTS += gdbstub-sysregs gdbstub-sve-ioctls
-
-.PHONY: gdbstub-sysregs gdbstub-sve-ioctls
run-gdbstub-sysregs: sysregs
$(call run-test, $@, $(GDB_SCRIPT) \
--gdb $(HAVE_GDB_BIN) \
@@ -70,6 +67,8 @@ run-gdbstub-sve-ioctls: sve-ioctls
--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
--bin $< --test $(AARCH64_SRC)/gdbstub/test-sve-ioctl.py, \
"basic gdbstub SVE ZLEN support")
+
+EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls
endif
endif
diff --git a/tests/tcg/cris/Makefile.target b/tests/tcg/cris/Makefile.target
index 24c7f2e761..e72d3cbdb2 100644
--- a/tests/tcg/cris/Makefile.target
+++ b/tests/tcg/cris/Makefile.target
@@ -23,6 +23,7 @@ CRIS_RUNS = $(patsubst %, run-%, $(CRIS_USABLE_TESTS))
# override the list of tests, as we can't build the multiarch tests
TESTS = $(CRIS_USABLE_TESTS)
+EXTRA_RUNS =
VPATH = $(CRIS_SRC)
AS = $(CC) -x assembler-with-cpp
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 035b09c853..51fb75ecfd 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -42,5 +42,19 @@ run-test-mmap-%: test-mmap
$(call run-test, test-mmap-$*, $(QEMU) -p $* $<,\
"$< ($* byte pages) on $(TARGET_NAME)")
+ifneq ($(HAVE_GDB_BIN),)
+GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py
+
+run-gdbstub-sha1: sha1
+ $(call run-test, $@, $(GDB_SCRIPT) \
+ --gdb $(HAVE_GDB_BIN) \
+ --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+ --bin $< --test $(MULTIARCH_SRC)/gdbstub/sha1.py, \
+ "basic gdbstub support")
+
+EXTRA_RUNS += run-gdbstub-sha1
+endif
+
+
# Update TESTS
TESTS += $(MULTIARCH_TESTS)
diff --git a/tests/tcg/multiarch/gdbstub/sha1.py b/tests/tcg/multiarch/gdbstub/sha1.py
new file mode 100644
index 0000000000..734553b98b
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/sha1.py
@@ -0,0 +1,81 @@
+from __future__ import print_function
+#
+# A very simple smoke test for debugging the SHA1 userspace test on
+# each target.
+#
+# This is launched via tests/guest-debug/run-test.py
+#
+
+import gdb
+import sys
+
+initial_vlen = 0
+failcount = 0
+
+def report(cond, msg):
+ "Report success/fail of test"
+ if cond:
+ print("PASS: %s" % (msg))
+ else:
+ print("FAIL: %s" % (msg))
+ global failcount
+ failcount += 1
+
+def check_break(sym_name):
+ "Setup breakpoint, continue and check we stopped."
+ sym, ok = gdb.lookup_symbol(sym_name)
+ bp = gdb.Breakpoint(sym_name)
+
+ gdb.execute("c")
+
+ # hopefully we came back
+ end_pc = gdb.parse_and_eval('$pc')
+ report(bp.hit_count == 1,
+ "break @ %s (%s %d hits)" % (end_pc, sym.value(), bp.hit_count))
+
+ bp.delete()
+
+def run_test():
+ "Run through the tests one by one"
+
+ check_break("SHA1Init")
+
+ # check step and inspect values
+ gdb.execute("next")
+ val_ctx = gdb.parse_and_eval("context->state[0]")
+ exp_ctx = 0x67452301
+ report(int(val_ctx) == exp_ctx, "context->state[0] == %x" % exp_ctx);
+
+ gdb.execute("next")
+ val_ctx = gdb.parse_and_eval("context->state[1]")
+ exp_ctx = 0xEFCDAB89
+ report(int(val_ctx) == exp_ctx, "context->state[1] == %x" % exp_ctx);
+
+ # finally check we don't barf inspecting registers
+ gdb.execute("info registers")
+
+#
+# This runs as the script it sourced (via -x, via run-test.py)
+#
+try:
+ inferior = gdb.selected_inferior()
+ arch = inferior.architecture()
+ print("ATTACHED: %s" % arch.name())
+except (gdb.error, AttributeError):
+ print("SKIPPING (not connected)", file=sys.stderr)
+ exit(0)
+
+try:
+ # These are not very useful in scripts
+ gdb.execute("set pagination off")
+ gdb.execute("set confirm off")
+
+ # Run the actual tests
+ run_test()
+except (gdb.error):
+ print ("GDB Exception: %s" % (sys.exc_info()[0]))
+ failcount += 1
+ pass
+
+print("All tests complete: %d failures" % failcount)
+exit(failcount)
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v1 13/14] .travis.yml: show free disk space at end of run
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (11 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 12/14] tests/tcg: add a multiarch linux-user gdb test Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 20:27 ` Philippe Mathieu-Daudé
2020-04-23 17:05 ` [PATCH v1 14/14] .travis.yml: drop MacOSX Alex Bennée
` (2 subsequent siblings)
15 siblings, 1 reply; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Philippe Mathieu-Daudé, Alex Bennée
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
.travis.yml | 1 +
1 file changed, 1 insertion(+)
diff --git a/.travis.yml b/.travis.yml
index 2fd63eceaa..a4c3c6c805 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -113,6 +113,7 @@ script:
$(exit $BUILD_RC);
fi
after_script:
+ - df -h
- if command -v ccache ; then ccache --show-stats ; fi
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 13/14] .travis.yml: show free disk space at end of run
2020-04-23 17:05 ` [PATCH v1 13/14] .travis.yml: show free disk space at end of run Alex Bennée
@ 2020-04-23 20:27 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-23 20:27 UTC (permalink / raw)
To: Alex Bennée, qemu-devel; +Cc: Fam Zheng
On 4/23/20 7:05 PM, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> .travis.yml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/.travis.yml b/.travis.yml
> index 2fd63eceaa..a4c3c6c805 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -113,6 +113,7 @@ script:
> $(exit $BUILD_RC);
> fi
> after_script:
> + - df -h
Well, I suppose it helps to realize we didn't break the runner, it was
already dying...
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> - if command -v ccache ; then ccache --show-stats ; fi
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 14/14] .travis.yml: drop MacOSX
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (12 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 13/14] .travis.yml: show free disk space at end of run Alex Bennée
@ 2020-04-23 17:05 ` Alex Bennée
2020-04-23 20:28 ` Philippe Mathieu-Daudé
2020-04-23 18:15 ` [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis no-reply
2020-04-23 18:17 ` no-reply
15 siblings, 1 reply; 21+ messages in thread
From: Alex Bennée @ 2020-04-23 17:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Philippe Mathieu-Daudé, Alex Bennée
This keeps breaking on Travis so lets just fall back to the Cirrus CI
builds which seem to be better maintained. Fix up the comments while
we are doing this as we never had a windows build.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
.travis.yml | 28 +---------------------------
1 file changed, 1 insertion(+), 27 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index a4c3c6c805..49267b73b3 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -9,9 +9,8 @@ compiler:
cache:
# There is one cache per branch and compiler version.
# characteristics of each job are used to identify the cache:
- # - OS name (currently, linux, osx, or windows)
+ # - OS name (currently only linux)
# - OS distribution (for Linux, xenial, trusty, or precise)
- # - macOS image name (e.g., xcode7.2)
# - Names and values of visible environment variables set in .travis.yml or Settings panel
timeout: 1200
ccache: true
@@ -271,31 +270,6 @@ jobs:
- TEST_CMD=""
- # MacOSX builds - cirrus.yml also tests some MacOS builds including latest Xcode
-
- - name: "OSX Xcode 10.3"
- env:
- - BASE_CONFIG="--disable-docs --enable-tools"
- - CONFIG="--target-list=i386-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu,x86_64-softmmu"
- os: osx
- osx_image: xcode10.3
- compiler: clang
- addons:
- homebrew:
- packages:
- - ccache
- - glib
- - pixman
- - gnu-sed
- - python
- update: true
- before_script:
- - brew link --overwrite python
- - export PATH="/usr/local/opt/ccache/libexec:$PATH"
- - mkdir -p ${BUILD_DIR} && cd ${BUILD_DIR}
- - ${SRC_DIR}/configure ${BASE_CONFIG} ${CONFIG} || { cat config.log && exit 1; }
-
-
# Python builds
- name: "GCC Python 3.5 (x86_64-softmmu)"
env:
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 14/14] .travis.yml: drop MacOSX
2020-04-23 17:05 ` [PATCH v1 14/14] .travis.yml: drop MacOSX Alex Bennée
@ 2020-04-23 20:28 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-23 20:28 UTC (permalink / raw)
To: Alex Bennée, qemu-devel; +Cc: Fam Zheng
On 4/23/20 7:05 PM, Alex Bennée wrote:
> This keeps breaking on Travis so lets just fall back to the Cirrus CI
> builds which seem to be better maintained. Fix up the comments while
> we are doing this as we never had a windows build.
I certainly vouch this.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> .travis.yml | 28 +---------------------------
> 1 file changed, 1 insertion(+), 27 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index a4c3c6c805..49267b73b3 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -9,9 +9,8 @@ compiler:
> cache:
> # There is one cache per branch and compiler version.
> # characteristics of each job are used to identify the cache:
> - # - OS name (currently, linux, osx, or windows)
> + # - OS name (currently only linux)
> # - OS distribution (for Linux, xenial, trusty, or precise)
> - # - macOS image name (e.g., xcode7.2)
> # - Names and values of visible environment variables set in .travis.yml or Settings panel
> timeout: 1200
> ccache: true
> @@ -271,31 +270,6 @@ jobs:
> - TEST_CMD=""
>
>
> - # MacOSX builds - cirrus.yml also tests some MacOS builds including latest Xcode
> -
> - - name: "OSX Xcode 10.3"
> - env:
> - - BASE_CONFIG="--disable-docs --enable-tools"
> - - CONFIG="--target-list=i386-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu,x86_64-softmmu"
> - os: osx
> - osx_image: xcode10.3
> - compiler: clang
> - addons:
> - homebrew:
> - packages:
> - - ccache
> - - glib
> - - pixman
> - - gnu-sed
> - - python
> - update: true
> - before_script:
> - - brew link --overwrite python
> - - export PATH="/usr/local/opt/ccache/libexec:$PATH"
> - - mkdir -p ${BUILD_DIR} && cd ${BUILD_DIR}
> - - ${SRC_DIR}/configure ${BASE_CONFIG} ${CONFIG} || { cat config.log && exit 1; }
> -
> -
> # Python builds
> - name: "GCC Python 3.5 (x86_64-softmmu)"
> env:
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (13 preceding siblings ...)
2020-04-23 17:05 ` [PATCH v1 14/14] .travis.yml: drop MacOSX Alex Bennée
@ 2020-04-23 18:15 ` no-reply
2020-04-23 18:17 ` no-reply
15 siblings, 0 replies; 21+ messages in thread
From: no-reply @ 2020-04-23 18:15 UTC (permalink / raw)
To: alex.bennee; +Cc: alex.bennee, qemu-devel
Patchew URL: https://patchew.org/QEMU/20200423170557.31106-1-alex.bennee@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis
Message-id: 20200423170557.31106-1-alex.bennee@linaro.org
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/20200423150127.142609-1-kwolf@redhat.com -> patchew/20200423150127.142609-1-kwolf@redhat.com
- [tag update] patchew/20200423160036.7048-1-armbru@redhat.com -> patchew/20200423160036.7048-1-armbru@redhat.com
Switched to a new branch 'test'
689e6e8 .travis.yml: drop MacOSX
f7c8828 .travis.yml: show free disk space at end of run
c2d0ee7 tests/tcg: add a multiarch linux-user gdb test
5002abd tests/guest-debug: use the unix socket for linux-user tests
98d25bf gdbstub/linux-user: support debugging over a unix socket
37544e1 gdbstub: eliminate gdbserver_fd global
852d87f tests/tcg: drop inferior.was_attached() test
72e9645 tests/tcg: better trap gdb failures
8a161b1 gdbstub: Introduce gdb_get_float64() to get 64-bit float registers
d0f142f configure: favour gdb-multiarch if we have it
bc6da7bb .gitignore: include common build sub-directories
ad7e15b accel/tcg: Relax va restrictions on 64-bit guests
55ee255 exec/cpu-all: Use bool for have_guest_base
55dbacd linux-user: completely re-write init_guest_space
=== OUTPUT BEGIN ===
1/14 Checking commit 55dbacd6b444 (linux-user: completely re-write init_guest_space)
2/14 Checking commit 55ee255da7f6 (exec/cpu-all: Use bool for have_guest_base)
3/14 Checking commit ad7e15b42dc2 (accel/tcg: Relax va restrictions on 64-bit guests)
ERROR: Macros with complex values should be enclosed in parenthesis
#91: FILE: include/exec/cpu-all.h:182:
+# define GUEST_ADDR_MAX_ ~0ul
total: 1 errors, 0 warnings, 88 lines checked
Patch 3/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/14 Checking commit bc6da7bb0a03 (.gitignore: include common build sub-directories)
5/14 Checking commit d0f142fa8cfd (configure: favour gdb-multiarch if we have it)
6/14 Checking commit 8a161b12a736 (gdbstub: Introduce gdb_get_float64() to get 64-bit float registers)
7/14 Checking commit 72e964519511 (tests/tcg: better trap gdb failures)
8/14 Checking commit 852d87fc501d (tests/tcg: drop inferior.was_attached() test)
9/14 Checking commit 37544e18de5c (gdbstub: eliminate gdbserver_fd global)
ERROR: suspect code indent for conditional statements (2, 6)
#33: FILE: gdbstub.c:2965:
+ if (gdbserver_state.fd < 0) {
return;
ERROR: braces {} are necessary for all arms of this statement
#80: FILE: gdbstub.c:3139:
+ if (gdb_fd < 0)
[...]
total: 2 errors, 0 warnings, 73 lines checked
Patch 9/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/14 Checking commit 98d25bf64760 (gdbstub/linux-user: support debugging over a unix socket)
ERROR: suspect code indent for conditional statements (2, 6)
#67: FILE: gdbstub.c:2966:
+ if (gdbserver_state.socket_path) {
+ unlink(gdbserver_state.socket_path);
ERROR: space required before the open parenthesis '('
#101: FILE: gdbstub.c:3087:
+ for(;;) {
ERROR: spaces required around that '-' (ctx:VxV)
#128: FILE: gdbstub.c:3114:
+ pstrcpy(sockaddr.sun_path, sizeof(sockaddr.sun_path)-1, path);
^
total: 3 errors, 0 warnings, 228 lines checked
Patch 10/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/14 Checking commit 5002abd35782 (tests/guest-debug: use the unix socket for linux-user tests)
WARNING: line over 80 characters
#38: FILE: tests/guest-debug/run-test.py:53:
+ cmd = "%s %s -g %s %s" % (args.qemu, args.qargs, socket_name, args.binary)
total: 0 errors, 1 warnings, 34 lines checked
Patch 11/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/14 Checking commit c2d0ee78fa1d (tests/tcg: add a multiarch linux-user gdb test)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#75:
new file mode 100644
total: 0 errors, 1 warnings, 124 lines checked
Patch 12/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/14 Checking commit f7c88287e116 (.travis.yml: show free disk space at end of run)
14/14 Checking commit 689e6e8641c4 (.travis.yml: drop MacOSX)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200423170557.31106-1-alex.bennee@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis
2020-04-23 17:05 [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis Alex Bennée
` (14 preceding siblings ...)
2020-04-23 18:15 ` [PATCH for 5.1 v1 00/14] guest_base, gdbstub and Travis no-reply
@ 2020-04-23 18:17 ` no-reply
15 siblings, 0 replies; 21+ messages in thread
From: no-reply @ 2020-04-23 18:17 UTC (permalink / raw)
To: alex.bennee; +Cc: alex.bennee, qemu-devel
Patchew URL: https://patchew.org/QEMU/20200423170557.31106-1-alex.bennee@linaro.org/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
Cleared directory 'slirp'
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) unregistered for path 'slirp'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) unregistered for path 'ui/keycodemapdb'
make[1]: *** [/var/tmp/patchew-tester-tmp-rhig99s_/src/docker-src.2020-04-23-14.15.56.12776] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-rhig99s_/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 2m0.927s
user 0m5.275s
The full log is available at
http://patchew.org/logs/20200423170557.31106-1-alex.bennee@linaro.org/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 21+ messages in thread