[PATCH 00/17] For 9.2: A bunch of cleanups and work towards variable pagesize support

[PATCH 00/17] For 9.2: A bunch of cleanups and work towards variable pagesize support

[Warner Losh]

This series has a number of changes to reduce diffs between blitz and
qemu-project. These are minor and self-contined.

The second half of these changes are the next round of changes in the quest to
support variable page sizes. These are building towards the final set of changes
that will dynamically allocate arrays rather than have them be hard-coded in a

Stacey Son (1):
  bsd-user: Implement cpu_copy()

Warner Losh (16):
  bsd-user: Delete TaskState next member
  bsd-user: Make init_task_state global
  bsd-user: Make cpu_model and cpu_type file scope
  bsd-user: Eliminate unused regs arg in load_elf_binary
  bsd-user: Remove load_flt_binary prototype
  bsd-user: Remove deprecated -p argument
  bsd-user: Eliminate unused qemu_uname_release
  bsd-user: target_msync unused, remove it
  bsd-user: Pass image name down the stack
  bsd-user: Replace set_brk and padzero with zerobss from linux-user
  bsd-user: Use guest_range_valid_untagged to validate range
  bsd-user: target_mprotect: rename prot to target_prot
  bsd-user: target_mmap*: change prot to target_prot
  bsd-user: target_mprotect: use helper host_page_size local
  bsd-user: Define validate_prot_to_pageflags and use in mprotect
  bsd-user: copy linux-user target_mprotect impl

 bsd-user/bsdload.c |   2 +-
 bsd-user/elfload.c | 132 +++++++++++++------------
 bsd-user/main.c    |  48 +++++++---
 bsd-user/mmap.c    | 234 +++++++++++++++++++++++++++------------------
 bsd-user/qemu.h    |   9 +-
 5 files changed, 247 insertions(+), 178 deletions(-)

-- 
2.45.1

[PATCH 01/17] bsd-user: Delete TaskState next member

[Warner Losh]

The next struct member of TaskState is unused. Remove it.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/qemu.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index 3736c417860..4ccbee265a1 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -83,7 +83,6 @@ struct emulated_sigtable {
 struct TaskState {
     pid_t ts_tid;     /* tid (or pid) of this task */
 
-    struct TaskState *next;
     struct bsd_binprm *bprm;
     struct image_info *info;
 
-- 
2.45.1

[PATCH 02/17] bsd-user: Make init_task_state global

[Warner Losh]

Restore init_task_state to its global status. It's needed for threading
support outside of main.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/main.c | 2 +-
 bsd-user/qemu.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index cc980e6f401..4d29e13a8f5 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -213,7 +213,7 @@ void qemu_cpu_kick(CPUState *cpu)
 }
 
 /* Assumes contents are already zeroed.  */
-static void init_task_state(TaskState *ts)
+void init_task_state(TaskState *ts)
 {
     ts->sigaltstack_used = (struct target_sigaltstack) {
         .ss_sp = 0,
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index 4ccbee265a1..c7f78096734 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -119,6 +119,7 @@ struct TaskState {
     struct target_sigaltstack sigaltstack_used;
 } __attribute__((aligned(16)));
 
+void init_task_state(TaskState *ts);
 void stop_all_tasks(void);
 extern const char *interp_prefix;
 extern const char *qemu_uname_release;
-- 
2.45.1

[PATCH 03/17] bsd-user: Make cpu_model and cpu_type file scope

[Warner Losh]

linux-user already does this since 2278b93941d4. That same commit just
added them with main() scope to bsd-user. We need the cpu_type, like
linux-user does, to create new CPUs outside of main to support
threading. Move both cpu_model and cpu_type to mirror linux-user/main.c.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 4d29e13a8f5..1533fd51168 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -62,6 +62,8 @@ intptr_t qemu_host_page_mask;
 static bool opt_one_insn_per_tb;
 uintptr_t guest_base;
 bool have_guest_base;
+static const char *cpu_model;
+static const char *cpu_type;
 /*
  * When running 32-on-64 we should make sure we can fit all of the possible
  * guest address space into a contiguous chunk of virtual host memory.
@@ -251,8 +253,6 @@ adjust_ssize(void)
 int main(int argc, char **argv)
 {
     const char *filename;
-    const char *cpu_model;
-    const char *cpu_type;
     const char *log_file = NULL;
     const char *log_mask = NULL;
     const char *seed_optarg = NULL;
-- 
2.45.1

[PATCH 04/17] bsd-user: Implement cpu_copy()

[Warner Losh]

From: Stacey Son <sson@FreeBSD.org>

Catch up with 30ba0ee52d15 and implement cpu_copy(). It's needed for
threading. Stacey's original code, with bug fixes from Jessica, Justin
and myself.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
Signed-off-by: Justin Hibbits <chmeeedalf@gmail.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/main.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 1533fd51168..9ad31bd1efe 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -224,6 +224,37 @@ void init_task_state(TaskState *ts)
     };
 }
 
+CPUArchState *cpu_copy(CPUArchState *env)
+{
+    CPUState *cpu = env_cpu(env);
+    CPUState *new_cpu = cpu_create(cpu_type);
+    CPUArchState *new_env = cpu_env(new_cpu);
+    CPUBreakpoint *bp;
+    CPUWatchpoint *wp;
+
+    /* Reset non arch specific state */
+    cpu_reset(new_cpu);
+
+    new_cpu->tcg_cflags = cpu->tcg_cflags;
+    memcpy(new_env, env, sizeof(CPUArchState));
+
+    /*
+     * Clone all break/watchpoints.
+     * Note: Once we support ptrace with hw-debug register access, make sure
+     * BP_CPU break/watchpoints are handled correctly on clone.
+     */
+    QTAILQ_INIT(&cpu->breakpoints);
+    QTAILQ_INIT(&cpu->watchpoints);
+    QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
+        cpu_breakpoint_insert(new_cpu, bp->pc, bp->flags, NULL);
+    }
+    QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
+        cpu_watchpoint_insert(new_cpu, wp->vaddr, wp->len, wp->flags, NULL);
+    }
+
+    return new_env;
+}
+
 void gemu_log(const char *fmt, ...)
 {
     va_list ap;
-- 
2.45.1

[PATCH 05/17] bsd-user: Eliminate unused regs arg in load_elf_binary

[Warner Losh]

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/bsdload.c | 2 +-
 bsd-user/elfload.c | 3 +--
 bsd-user/qemu.h    | 3 +--
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/bsd-user/bsdload.c b/bsd-user/bsdload.c
index 5b3c061a452..dcf3ca14fcc 100644
--- a/bsd-user/bsdload.c
+++ b/bsd-user/bsdload.c
@@ -193,7 +193,7 @@ int loader_exec(const char *filename, char **argv, char **envp,
                 && bprm->buf[1] == 'E'
                 && bprm->buf[2] == 'L'
                 && bprm->buf[3] == 'F') {
-            retval = load_elf_binary(bprm, regs, infop);
+            retval = load_elf_binary(bprm, infop);
         } else {
             fprintf(stderr, "Unknown binary format\n");
             return -1;
diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index 833fa3bd057..caf8a1adf2d 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -597,8 +597,7 @@ load_elf_sections(const struct elfhdr *hdr, struct elf_phdr *phdr, int fd,
     return 0;
 }
 
-int load_elf_binary(struct bsd_binprm *bprm, struct target_pt_regs *regs,
-                    struct image_info *info)
+int load_elf_binary(struct bsd_binprm *bprm, struct image_info *info)
 {
     struct elfhdr elf_ex;
     struct elfhdr interp_elf_ex;
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index c7f78096734..f18a54cc933 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -163,8 +163,7 @@ int loader_exec(const char *filename, char **argv, char **envp,
                 struct target_pt_regs *regs, struct image_info *infop,
                 struct bsd_binprm *bprm);
 
-int load_elf_binary(struct bsd_binprm *bprm, struct target_pt_regs *regs,
-                    struct image_info *info);
+int load_elf_binary(struct bsd_binprm *bprm, struct image_info *info);
 int load_flt_binary(struct bsd_binprm *bprm, struct target_pt_regs *regs,
                     struct image_info *info);
 int is_target_elf_binary(int fd);
-- 
2.45.1

[PATCH 06/17] bsd-user: Remove load_flt_binary prototype

[Warner Losh]

bsd-user doesn't have support for loading FLT binaries.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/qemu.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index f18a54cc933..b97a902a4c2 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -164,8 +164,6 @@ int loader_exec(const char *filename, char **argv, char **envp,
                 struct bsd_binprm *bprm);
 
 int load_elf_binary(struct bsd_binprm *bprm, struct image_info *info);
-int load_flt_binary(struct bsd_binprm *bprm, struct target_pt_regs *regs,
-                    struct image_info *info);
 int is_target_elf_binary(int fd);
 
 abi_long memcpy_to_target(abi_ulong dest, const void *src,
-- 
2.45.1

[PATCH 07/17] bsd-user: Remove deprecated -p argument

[Warner Losh]

FreeBSD never really used the -p argument, so it's safe to remove
entirely.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/main.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 9ad31bd1efe..709ab10ddc1 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -388,14 +388,6 @@ int main(int argc, char **argv)
             }
         } else if (!strcmp(r, "L")) {
             interp_prefix = argv[optind++];
-        } else if (!strcmp(r, "p")) {
-            unsigned size, want = qemu_real_host_page_size();
-
-            r = argv[optind++];
-            if (qemu_strtoui(r, NULL, 10, &size) || size != want) {
-                warn_report("Deprecated page size option cannot "
-                            "change host page size (%u)", want);
-            }
         } else if (!strcmp(r, "g")) {
             gdbstub = g_strdup(argv[optind++]);
         } else if (!strcmp(r, "r")) {
-- 
2.45.1

[PATCH 08/17] bsd-user: Eliminate unused qemu_uname_release

[Warner Losh]

bsd-user has never supported this, and FreeBSD make it easy to set this
on a per-jail basis, so that the normal reporting routines that we pass
through just work. Since this was never used, and never even in the
usage(), retire it to cut down on the clutter. It was literally just a
write-only variable.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/main.c | 3 ---
 bsd-user/qemu.h | 1 -
 2 files changed, 4 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 709ab10ddc1..8c52fb43ff1 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -92,7 +92,6 @@ static const char *cpu_type;
 unsigned long reserved_va;
 
 const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
-const char *qemu_uname_release;
 
 unsigned long target_maxtsiz = TARGET_MAXTSIZ;   /* max text size */
 unsigned long target_dfldsiz = TARGET_DFLDSIZ;   /* initial data size limit */
@@ -390,8 +389,6 @@ int main(int argc, char **argv)
             interp_prefix = argv[optind++];
         } else if (!strcmp(r, "g")) {
             gdbstub = g_strdup(argv[optind++]);
-        } else if (!strcmp(r, "r")) {
-            qemu_uname_release = argv[optind++];
         } else if (!strcmp(r, "cpu")) {
             cpu_model = argv[optind++];
             if (is_help_option(cpu_model)) {
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index b97a902a4c2..ed6044cfdaf 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -122,7 +122,6 @@ struct TaskState {
 void init_task_state(TaskState *ts);
 void stop_all_tasks(void);
 extern const char *interp_prefix;
-extern const char *qemu_uname_release;
 
 /*
  * TARGET_ARG_MAX defines the number of bytes allocated for arguments
-- 
2.45.1

[PATCH 09/17] bsd-user: target_msync unused, remove it

[Warner Losh]

Nothing calls target_msync in the upstream or blitz fork, so remove it.
It will save us having to modernize it.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 17 -----------------
 bsd-user/qemu.h |  1 -
 2 files changed, 18 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index f3a4f1712da..fc69cb43ebd 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -739,20 +739,3 @@ int target_munmap(abi_ulong start, abi_ulong len)
     mmap_unlock();
     return ret;
 }
-
-int target_msync(abi_ulong start, abi_ulong len, int flags)
-{
-    abi_ulong end;
-
-    if (start & ~TARGET_PAGE_MASK)
-        return -EINVAL;
-    len = TARGET_PAGE_ALIGN(len);
-    end = start + len;
-    if (end < start)
-        return -EINVAL;
-    if (end == start)
-        return 0;
-
-    start &= qemu_host_page_mask;
-    return msync(g2h_untagged(start), end - start, flags);
-}
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index ed6044cfdaf..a2bc14eea50 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -236,7 +236,6 @@ int target_munmap(abi_ulong start, abi_ulong len);
 abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size,
                        abi_ulong new_size, unsigned long flags,
                        abi_ulong new_addr);
-int target_msync(abi_ulong start, abi_ulong len, int flags);
 extern abi_ulong mmap_next_start;
 abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size);
 void mmap_reserve(abi_ulong start, abi_ulong size);
-- 
2.45.1

[PATCH 10/17] bsd-user: Pass image name down the stack

[Warner Losh]

Pass the image name down the stack so that we can give better error
messages. Inspired by similar work in linux-user, and more likely to
come.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/elfload.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index caf8a1adf2d..dba03f17465 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -36,8 +36,8 @@ abi_ulong target_stksiz;
 abi_ulong target_stkbas;
 
 static int elf_core_dump(int signr, CPUArchState *env);
-static int load_elf_sections(const struct elfhdr *hdr, struct elf_phdr *phdr,
-    int fd, abi_ulong rbase, abi_ulong *baddrp);
+static int load_elf_sections(const char *image_name, const struct elfhdr *hdr,
+    struct elf_phdr *phdr, int fd, abi_ulong rbase, abi_ulong *baddrp);
 
 static inline void memcpy_fromfs(void *to, const void *from, unsigned long n)
 {
@@ -268,7 +268,8 @@ static void padzero(abi_ulong elf_bss, abi_ulong last_bss)
     }
 }
 
-static abi_ulong load_elf_interp(struct elfhdr *interp_elf_ex,
+static abi_ulong load_elf_interp(const char *elf_interpreter,
+                                 struct elfhdr *interp_elf_ex,
                                  int interpreter_fd,
                                  abi_ulong *interp_load_addr)
 {
@@ -335,7 +336,7 @@ static abi_ulong load_elf_interp(struct elfhdr *interp_elf_ex,
         }
     }
 
-    error = load_elf_sections(interp_elf_ex, elf_phdata, interpreter_fd, rbase,
+    error = load_elf_sections(elf_interpreter, interp_elf_ex, elf_phdata, interpreter_fd, rbase,
         &baddr);
     if (error != 0) {
         perror("load_elf_sections");
@@ -526,8 +527,9 @@ int is_target_elf_binary(int fd)
 }
 
 static int
-load_elf_sections(const struct elfhdr *hdr, struct elf_phdr *phdr, int fd,
-    abi_ulong rbase, abi_ulong *baddrp)
+load_elf_sections(const char *image_name, const struct elfhdr *hdr,
+                  struct elf_phdr *phdr, int fd, abi_ulong rbase,
+                  abi_ulong *baddrp)
 {
     struct elf_phdr *elf_ppnt;
     abi_ulong baddr;
@@ -764,7 +766,7 @@ int load_elf_binary(struct bsd_binprm *bprm, struct image_info *info)
 
     info->elf_flags = elf_ex.e_flags;
 
-    error = load_elf_sections(&elf_ex, elf_phdata, bprm->fd, et_dyn_addr,
+    error = load_elf_sections(bprm->filename, &elf_ex, elf_phdata, bprm->fd, et_dyn_addr,
         &load_addr);
     for (i = 0, elf_ppnt = elf_phdata; i < elf_ex.e_phnum; i++, elf_ppnt++) {
         if (elf_ppnt->p_type != PT_LOAD) {
@@ -780,7 +782,8 @@ int load_elf_binary(struct bsd_binprm *bprm, struct image_info *info)
     }
 
     if (elf_interpreter) {
-        elf_entry = load_elf_interp(&interp_elf_ex, interpreter_fd,
+        elf_entry = load_elf_interp(elf_interpreter,
+                                    &interp_elf_ex, interpreter_fd,
                                     &interp_load_addr);
         reloc_func_desc = interp_load_addr;
 
-- 
2.45.1

[PATCH 11/17] bsd-user: Replace set_brk and padzero with zerobss from linux-user

[Warner Losh]

The zero_bss interface from linux-user is much better at doing this. Use
it in preference to set_brk (badly named) and padzero. These both have
issues with the new variable page size code, so it's best to just retire
them and reuse the code from linux-user. Also start to use the error
reporting code that linux-user uses to give better error messages on
failure.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/elfload.c | 110 +++++++++++++++++++++++----------------------
 1 file changed, 57 insertions(+), 53 deletions(-)

diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index dba03f17465..0a2f2379c93 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -22,6 +22,7 @@
 #include "qemu.h"
 #include "disas/disas.h"
 #include "qemu/path.h"
+#include "qapi/error.h"
 
 static abi_ulong target_auxents;   /* Where the AUX entries are in target */
 static size_t target_auxents_sz;   /* Size of AUX entries including AT_NULL */
@@ -210,62 +211,63 @@ static void setup_arg_pages(struct bsd_binprm *bprm, struct image_info *info,
     }
 }
 
-static void set_brk(abi_ulong start, abi_ulong end)
+/**
+ * zero_bss:
+ *
+ * Map and zero the bss.  We need to explicitly zero any fractional pages
+ * after the data section (i.e. bss).  Return false on mapping failure.
+ */
+static bool zero_bss(abi_ulong start_bss, abi_ulong end_bss,
+                     int prot, Error **errp)
 {
-    /* page-align the start and end addresses... */
-    start = HOST_PAGE_ALIGN(start);
-    end = HOST_PAGE_ALIGN(end);
-    if (end <= start) {
-        return;
-    }
-    if (target_mmap(start, end - start, PROT_READ | PROT_WRITE | PROT_EXEC,
-        MAP_FIXED | MAP_PRIVATE | MAP_ANON, -1, 0) == -1) {
-        perror("cannot mmap brk");
-        exit(-1);
+    abi_ulong align_bss;
+
+    /* We only expect writable bss; the code segment shouldn't need this. */
+    if (!(prot & PROT_WRITE)) {
+        error_setg(errp, "PT_LOAD with non-writable bss");
+        return false;
     }
-}
 
+    align_bss = TARGET_PAGE_ALIGN(start_bss);
+    end_bss = TARGET_PAGE_ALIGN(end_bss);
 
-/*
- * We need to explicitly zero any fractional pages after the data
- * section (i.e. bss).  This would contain the junk from the file that
- * should not be in memory.
- */
-static void padzero(abi_ulong elf_bss, abi_ulong last_bss)
-{
-    abi_ulong nbyte;
+    if (start_bss < align_bss) {
+        int flags = page_get_flags(start_bss);
 
-    if (elf_bss >= last_bss) {
-        return;
-    }
+        if (!(flags & PAGE_RWX)) {
+            /*
+             * The whole address space of the executable was reserved
+             * at the start, therefore all pages will be VALID.
+             * But assuming there are no PROT_NONE PT_LOAD segments,
+             * a PROT_NONE page means no data all bss, and we can
+             * simply extend the new anon mapping back to the start
+             * of the page of bss.
+             */
+            align_bss -= TARGET_PAGE_SIZE;
+        } else {
+            /*
+             * The start of the bss shares a page with something.
+             * The only thing that we expect is the data section,
+             * which would already be marked writable.
+             * Overlapping the RX code segment seems malformed.
+             */
+            if (!(flags & PAGE_WRITE)) {
+                error_setg(errp, "PT_LOAD with bss overlapping "
+                           "non-writable page");
+                return false;
+            }
 
-    /*
-     * XXX: this is really a hack : if the real host page size is
-     * smaller than the target page size, some pages after the end
-     * of the file may not be mapped. A better fix would be to
-     * patch target_mmap(), but it is more complicated as the file
-     * size must be known.
-     */
-    if (qemu_real_host_page_size() < qemu_host_page_size) {
-        abi_ulong end_addr, end_addr1;
-        end_addr1 = REAL_HOST_PAGE_ALIGN(elf_bss);
-        end_addr = HOST_PAGE_ALIGN(elf_bss);
-        if (end_addr1 < end_addr) {
-            mmap((void *)g2h_untagged(end_addr1), end_addr - end_addr1,
-                 PROT_READ | PROT_WRITE | PROT_EXEC,
-                 MAP_FIXED | MAP_PRIVATE | MAP_ANON, -1, 0);
+            /* The page is already mapped and writable. */
+            memset(g2h_untagged(start_bss), 0, align_bss - start_bss);
         }
     }
-
-    nbyte = elf_bss & (qemu_host_page_size - 1);
-    if (nbyte) {
-        nbyte = qemu_host_page_size - nbyte;
-        do {
-            /* FIXME - what to do if put_user() fails? */
-            put_user_u8(0, elf_bss);
-            elf_bss++;
-        } while (--nbyte);
+    if (align_bss < end_bss &&
+        target_mmap(align_bss, end_bss - align_bss, prot,
+                    MAP_FIXED | MAP_PRIVATE | MAP_ANON, -1, 0) == -1) {
+        error_setg_errno(errp, errno, "Error mapping bss");
+        return false;
     }
+    return true;
 }
 
 static abi_ulong load_elf_interp(const char *elf_interpreter,
@@ -535,6 +537,7 @@ load_elf_sections(const char *image_name, const struct elfhdr *hdr,
     abi_ulong baddr;
     int i;
     bool first;
+    Error *err = NULL;
 
     /*
      * Now we do a little grungy work by mmaping the ELF image into
@@ -579,12 +582,10 @@ load_elf_sections(const char *image_name, const struct elfhdr *hdr,
             start_bss = rbase + elf_ppnt->p_vaddr + elf_ppnt->p_filesz;
             end_bss = rbase + elf_ppnt->p_vaddr + elf_ppnt->p_memsz;
 
-            /*
-             * Calling set_brk effectively mmaps the pages that we need for the
-             * bss and break sections.
-             */
-            set_brk(start_bss, end_bss);
-            padzero(start_bss, end_bss);
+            if (start_bss < end_bss &&
+                !zero_bss(start_bss, end_bss, elf_prot, &err)) {
+                goto exit_errmsg;
+            }
         }
 
         if (first) {
@@ -597,6 +598,9 @@ load_elf_sections(const char *image_name, const struct elfhdr *hdr,
         *baddrp = baddr;
     }
     return 0;
+exit_errmsg:
+    error_reportf_err(err, "%s: ", image_name);
+    exit(-1);
 }
 
 int load_elf_binary(struct bsd_binprm *bprm, struct image_info *info)
-- 
2.45.1

[PATCH 12/17] bsd-user: Use guest_range_valid_untagged to validate range

[Warner Losh]

This is the generic validation function, so remove some hand-rolled
ones.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index fc69cb43ebd..ed8d31a9048 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -74,9 +74,10 @@ int target_mprotect(abi_ulong start, abi_ulong len, int prot)
     if ((start & ~TARGET_PAGE_MASK) != 0)
         return -EINVAL;
     len = TARGET_PAGE_ALIGN(len);
+    if (!guest_range_valid_untagged(start, len)) {
+        return -ENOMEM;
+    }
     end = start + len;
-    if (end < start)
-        return -EINVAL;
     prot &= PROT_READ | PROT_WRITE | PROT_EXEC;
     if (len == 0)
         return 0;
@@ -689,11 +690,13 @@ int target_munmap(abi_ulong start, abi_ulong len)
            TARGET_ABI_FMT_lx "\n",
            start, len);
 #endif
-    if (start & ~TARGET_PAGE_MASK)
+    if (start & ~TARGET_PAGE_MASK) {
         return -EINVAL;
+    }
     len = TARGET_PAGE_ALIGN(len);
-    if (len == 0)
+    if (len == 0 || !guest_range_valid_untagged(start, len)) {
         return -EINVAL;
+    }
     mmap_lock();
     end = start + len;
     real_start = start & qemu_host_page_mask;
-- 
2.45.1

[PATCH 13/17] bsd-user: target_mprotect: rename prot to target_prot

[Warner Losh]

Linux-user's target_mprotect uses this convention, so move to it.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index ed8d31a9048..d34075c5c64 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -61,16 +61,16 @@ void mmap_fork_end(int child)
 }
 
 /* NOTE: all the constants are the HOST ones, but addresses are target. */
-int target_mprotect(abi_ulong start, abi_ulong len, int prot)
+int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
 {
     abi_ulong end, host_start, host_end, addr;
     int prot1, ret;
 
     qemu_log_mask(CPU_LOG_PAGE, "mprotect: start=0x" TARGET_ABI_FMT_lx
                   " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len,
-                  prot & PROT_READ ? 'r' : '-',
-                  prot & PROT_WRITE ? 'w' : '-',
-                  prot & PROT_EXEC ? 'x' : '-');
+                  target_prot & PROT_READ ? 'r' : '-',
+                  target_prot & PROT_WRITE ? 'w' : '-',
+                  target_prot & PROT_EXEC ? 'x' : '-');
     if ((start & ~TARGET_PAGE_MASK) != 0)
         return -EINVAL;
     len = TARGET_PAGE_ALIGN(len);
@@ -78,7 +78,7 @@ int target_mprotect(abi_ulong start, abi_ulong len, int prot)
         return -ENOMEM;
     }
     end = start + len;
-    prot &= PROT_READ | PROT_WRITE | PROT_EXEC;
+    target_prot &= PROT_READ | PROT_WRITE | PROT_EXEC;
     if (len == 0)
         return 0;
 
@@ -87,7 +87,7 @@ int target_mprotect(abi_ulong start, abi_ulong len, int prot)
     host_end = HOST_PAGE_ALIGN(end);
     if (start > host_start) {
         /* handle host page containing start */
-        prot1 = prot;
+        prot1 = target_prot;
         for (addr = host_start; addr < start; addr += TARGET_PAGE_SIZE) {
             prot1 |= page_get_flags(addr);
         }
@@ -104,7 +104,7 @@ int target_mprotect(abi_ulong start, abi_ulong len, int prot)
         host_start += qemu_host_page_size;
     }
     if (end < host_end) {
-        prot1 = prot;
+        prot1 = target_prot;
         for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
             prot1 |= page_get_flags(addr);
         }
@@ -117,11 +117,11 @@ int target_mprotect(abi_ulong start, abi_ulong len, int prot)
 
     /* handle the pages in the middle */
     if (host_start < host_end) {
-        ret = mprotect(g2h_untagged(host_start), host_end - host_start, prot);
+        ret = mprotect(g2h_untagged(host_start), host_end - host_start, target_prot);
         if (ret != 0)
             goto error;
     }
-    page_set_flags(start, start + len - 1, prot | PAGE_VALID);
+    page_set_flags(start, start + len - 1, target_prot | PAGE_VALID);
     mmap_unlock();
     return 0;
 error:
-- 
2.45.1

[PATCH 14/17] bsd-user: target_mmap*: change prot to target_prot

[Warner Losh]

Adopt the linux-user convention of using target_prot for passed in
protections. no functional change.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index d34075c5c64..2118972f073 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -152,7 +152,7 @@ error:
  */
 static int mmap_frag(abi_ulong real_start,
                      abi_ulong start, abi_ulong end,
-                     int prot, int flags, int fd, abi_ulong offset)
+                     int target_prot, int flags, int fd, abi_ulong offset)
 {
     abi_ulong real_end, addr;
     void *host_start;
@@ -170,20 +170,20 @@ static int mmap_frag(abi_ulong real_start,
 
     if (prot1 == 0) {
         /* no page was there, so we allocate one. See also above. */
-        void *p = mmap(host_start, qemu_host_page_size, prot,
+        void *p = mmap(host_start, qemu_host_page_size, target_prot,
                        flags | ((fd != -1) ? MAP_ANON : 0), -1, 0);
         if (p == MAP_FAILED)
             return -1;
-        prot1 = prot;
+        prot1 = target_prot;
     }
     prot1 &= PAGE_RWX;
 
-    prot_new = prot | prot1;
+    prot_new = target_prot | prot1;
     if (fd != -1) {
         /* msync() won't work here, so we return an error if write is
            possible while it is a shared mapping */
         if ((flags & TARGET_BSD_MAP_FLAGMASK) == MAP_SHARED &&
-            (prot & PROT_WRITE))
+            (target_prot & PROT_WRITE))
             return -1;
 
         /* adjust protection to be able to read */
@@ -367,7 +367,7 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size)
 }
 
 /* NOTE: all the constants are the HOST ones */
-abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
+abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
                      int flags, int fd, off_t offset)
 {
     abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
@@ -377,9 +377,9 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         qemu_log("mmap: start=0x" TARGET_ABI_FMT_lx
                  " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c flags=",
                  start, len,
-                 prot & PROT_READ ? 'r' : '-',
-                 prot & PROT_WRITE ? 'w' : '-',
-                 prot & PROT_EXEC ? 'x' : '-');
+                 target_prot & PROT_READ ? 'r' : '-',
+                 target_prot & PROT_WRITE ? 'w' : '-',
+                 target_prot & PROT_EXEC ? 'x' : '-');
         if (flags & MAP_ALIGNMENT_MASK) {
             qemu_log("MAP_ALIGNED(%u) ",
                      (flags & MAP_ALIGNMENT_MASK) >> MAP_ALIGNMENT_SHIFT);
@@ -416,13 +416,14 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         goto fail;
     }
     if (flags & MAP_STACK) {
-        if ((fd != -1) || ((prot & (PROT_READ | PROT_WRITE)) !=
-                    (PROT_READ | PROT_WRITE))) {
+        if (fd != -1 ||
+            ((target_prot & (PROT_READ | PROT_WRITE)) !=
+             (PROT_READ | PROT_WRITE))) {
             errno = EINVAL;
             goto fail;
         }
     }
-    if ((flags & MAP_GUARD) && (prot != PROT_NONE || fd != -1 ||
+    if ((flags & MAP_GUARD) && (target_prot != PROT_NONE || fd != -1 ||
         offset != 0 || (flags & (MAP_SHARED | MAP_PRIVATE |
         /* MAP_PREFAULT | */ /* MAP_PREFAULT not in mman.h */
         MAP_PREFAULT_READ | MAP_ANON | MAP_STACK)) != 0)) {
@@ -512,14 +513,14 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
          * especially important if qemu_host_page_size >
          * qemu_real_host_page_size
          */
-        p = mmap(g2h_untagged(start), host_len, prot,
+        p = mmap(g2h_untagged(start), host_len, target_prot,
                  flags | MAP_FIXED | ((fd != -1) ? MAP_ANON : 0), -1, 0);
         if (p == MAP_FAILED)
             goto fail;
         /* update start so that it points to the file position at 'offset' */
         host_start = (unsigned long)p;
         if (fd != -1) {
-            p = mmap(g2h_untagged(start), len, prot,
+            p = mmap(g2h_untagged(start), len, target_prot,
                      flags | MAP_FIXED, fd, host_offset);
             if (p == MAP_FAILED) {
                 munmap(g2h_untagged(start), host_len);
@@ -557,11 +558,11 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
              * possible while it is a shared mapping
              */
             if ((flags & TARGET_BSD_MAP_FLAGMASK) == MAP_SHARED &&
-                (prot & PROT_WRITE)) {
+                (target_prot & PROT_WRITE)) {
                 errno = EINVAL;
                 goto fail;
             }
-            retaddr = target_mmap(start, len, prot | PROT_WRITE,
+            retaddr = target_mmap(start, len, target_prot | PROT_WRITE,
                                   MAP_FIXED | MAP_PRIVATE | MAP_ANON,
                                   -1, 0);
             if (retaddr == -1)
@@ -569,8 +570,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             if (pread(fd, g2h_untagged(start), len, offset) == -1) {
                 goto fail;
             }
-            if (!(prot & PROT_WRITE)) {
-                ret = target_mprotect(start, len, prot);
+            if (!(target_prot & PROT_WRITE)) {
+                ret = target_mprotect(start, len, target_prot);
                 assert(ret == 0);
             }
             goto the_end;
@@ -587,13 +588,13 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             if (real_end == real_start + qemu_host_page_size) {
                 /* one single host page */
                 ret = mmap_frag(real_start, start, end,
-                                prot, flags, fd, offset);
+                                target_prot, flags, fd, offset);
                 if (ret == -1)
                     goto fail;
                 goto the_end1;
             }
             ret = mmap_frag(real_start, start, real_start + qemu_host_page_size,
-                            prot, flags, fd, offset);
+                            target_prot, flags, fd, offset);
             if (ret == -1)
                 goto fail;
             real_start += qemu_host_page_size;
@@ -602,7 +603,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         if (end < real_end) {
             ret = mmap_frag(real_end - qemu_host_page_size,
                             real_end - qemu_host_page_size, end,
-                            prot, flags, fd,
+                            target_prot, flags, fd,
                             offset + real_end - qemu_host_page_size - start);
             if (ret == -1)
                 goto fail;
@@ -618,13 +619,13 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             else
                 offset1 = offset + real_start - start;
             p = mmap(g2h_untagged(real_start), real_end - real_start,
-                     prot, flags, fd, offset1);
+                     target_prot, flags, fd, offset1);
             if (p == MAP_FAILED)
                 goto fail;
         }
     }
  the_end1:
-    page_set_flags(start, start + len - 1, prot | PAGE_VALID);
+    page_set_flags(start, start + len - 1, target_prot | PAGE_VALID);
  the_end:
 #ifdef DEBUG_MMAP
     printf("ret=0x" TARGET_ABI_FMT_lx "\n", start);
-- 
2.45.1

[PATCH 15/17] bsd-user: target_mprotect: use helper host_page_size local

[Warner Losh]

Use helper variable for host_page_size. Linux-user uses a similar helper
to make the code smaller after the multi-page-size migration.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 2118972f073..ffecf52a72a 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -63,6 +63,7 @@ void mmap_fork_end(int child)
 /* NOTE: all the constants are the HOST ones, but addresses are target. */
 int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
 {
+    int host_page_size = qemu_real_host_page_size();
     abi_ulong end, host_start, host_end, addr;
     int prot1, ret;
 
@@ -83,7 +84,7 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
         return 0;
 
     mmap_lock();
-    host_start = start & qemu_host_page_mask;
+    host_start = start & -host_page_size;
     host_end = HOST_PAGE_ALIGN(end);
     if (start > host_start) {
         /* handle host page containing start */
@@ -91,28 +92,28 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
         for (addr = host_start; addr < start; addr += TARGET_PAGE_SIZE) {
             prot1 |= page_get_flags(addr);
         }
-        if (host_end == host_start + qemu_host_page_size) {
+        if (host_end == host_start + host_page_size) {
             for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
                 prot1 |= page_get_flags(addr);
             }
             end = host_end;
         }
         ret = mprotect(g2h_untagged(host_start),
-                       qemu_host_page_size, prot1 & PAGE_RWX);
+                       host_page_size, prot1 & PAGE_RWX);
         if (ret != 0)
             goto error;
-        host_start += qemu_host_page_size;
+        host_start += host_page_size;
     }
     if (end < host_end) {
         prot1 = target_prot;
         for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
             prot1 |= page_get_flags(addr);
         }
-        ret = mprotect(g2h_untagged(host_end - qemu_host_page_size),
-                       qemu_host_page_size, prot1 & PAGE_RWX);
+        ret = mprotect(g2h_untagged(host_end - host_page_size),
+                       host_page_size, prot1 & PAGE_RWX);
         if (ret != 0)
             goto error;
-        host_end -= qemu_host_page_size;
+        host_end -= host_page_size;
     }
 
     /* handle the pages in the middle */
-- 
2.45.1

[PATCH 16/17] bsd-user: Define validate_prot_to_pageflags and use in mprotect

[Warner Losh]

Define validate_prot_to_pageflags. Use it in target_mprotect to validate
the flags. Our taraget_mmap needs more work before it can be used there,
do don't copy linux-user's use of it there. This should hvae no net
functional change, but does make target_mprotect more similar to
linux-user's.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index ffecf52a72a..3c48a188e88 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -60,12 +60,26 @@ void mmap_fork_end(int child)
         pthread_mutex_unlock(&mmap_mutex);
 }
 
+/*
+ * Validate target prot bitmask.
+ * Return the prot bitmask for the host in *HOST_PROT.
+ * Return 0 if the target prot bitmask is invalid, otherwise
+ * the internal qemu page_flags (which will include PAGE_VALID).
+ */
+static int validate_prot_to_pageflags(int prot)
+{
+    int valid = PROT_READ | PROT_WRITE | PROT_EXEC;
+    int page_flags = (prot & PAGE_RWX) | PAGE_VALID;
+
+    return prot & ~valid ? 0 : page_flags;
+}
+
 /* NOTE: all the constants are the HOST ones, but addresses are target. */
 int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
 {
     int host_page_size = qemu_real_host_page_size();
     abi_ulong end, host_start, host_end, addr;
-    int prot1, ret;
+    int prot1, ret, page_flags;
 
     qemu_log_mask(CPU_LOG_PAGE, "mprotect: start=0x" TARGET_ABI_FMT_lx
                   " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len,
@@ -74,14 +88,18 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
                   target_prot & PROT_EXEC ? 'x' : '-');
     if ((start & ~TARGET_PAGE_MASK) != 0)
         return -EINVAL;
+    page_flags = validate_prot_to_pageflags(target_prot);
+    if (!page_flags) {
+        return -TARGET_EINVAL;
+    }
     len = TARGET_PAGE_ALIGN(len);
+    if (len == 0)
+        return 0;
     if (!guest_range_valid_untagged(start, len)) {
         return -ENOMEM;
     }
-    end = start + len;
     target_prot &= PROT_READ | PROT_WRITE | PROT_EXEC;
-    if (len == 0)
-        return 0;
+    end = start + len;
 
     mmap_lock();
     host_start = start & -host_page_size;
@@ -122,7 +140,7 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
         if (ret != 0)
             goto error;
     }
-    page_set_flags(start, start + len - 1, target_prot | PAGE_VALID);
+    page_set_flags(start, start + len - 1, page_flags);
     mmap_unlock();
     return 0;
 error:
-- 
2.45.1

[PATCH 17/17] bsd-user: copy linux-user target_mprotect impl

[Warner Losh]

Now that we're closer to the linux-user target_mprotect code, go ahead
and grab the rest of the implementation. This moves from a stard, end
impl to a start, last which will allow last page mapping, etc. This also
moves to a more general algorithm. We're close enough that this jump
isn't so large, and doing it incrementally further has become too
much work for too little gain.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 138 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 90 insertions(+), 48 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 3c48a188e88..a4de7674bec 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -60,6 +60,17 @@ void mmap_fork_end(int child)
         pthread_mutex_unlock(&mmap_mutex);
 }
 
+/*
+ * Map target protection mask to host. Identity on FreeBSD.
+ */
+static abi_ulong target_to_host_prot(abi_ulong prot)
+{
+    return (prot);
+}
+
+/* Helpful temporary #define to reduce diffs with linux-user mmap.c */
+#define trace_target_mprotect(start, len, target_prot)
+
 /*
  * Validate target prot bitmask.
  * Return the prot bitmask for the host in *HOST_PROT.
@@ -78,72 +89,103 @@ static int validate_prot_to_pageflags(int prot)
 int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
 {
     int host_page_size = qemu_real_host_page_size();
-    abi_ulong end, host_start, host_end, addr;
-    int prot1, ret, page_flags;
-
-    qemu_log_mask(CPU_LOG_PAGE, "mprotect: start=0x" TARGET_ABI_FMT_lx
-                  " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len,
-                  target_prot & PROT_READ ? 'r' : '-',
-                  target_prot & PROT_WRITE ? 'w' : '-',
-                  target_prot & PROT_EXEC ? 'x' : '-');
-    if ((start & ~TARGET_PAGE_MASK) != 0)
-        return -EINVAL;
+    abi_ulong starts[3];
+    abi_ulong lens[3];
+    int prots[3];
+    abi_ulong host_start, host_last, last;
+    int prot1, ret, page_flags, nranges;
+
+    trace_target_mprotect(start, len, target_prot);
+
+    if ((start & ~TARGET_PAGE_MASK) != 0) {
+        return -TARGET_EINVAL;
+    }
     page_flags = validate_prot_to_pageflags(target_prot);
     if (!page_flags) {
         return -TARGET_EINVAL;
     }
-    len = TARGET_PAGE_ALIGN(len);
-    if (len == 0)
+    if (len == 0) {
         return 0;
+    }
+    len = TARGET_PAGE_ALIGN(len);
     if (!guest_range_valid_untagged(start, len)) {
-        return -ENOMEM;
+        return -TARGET_ENOMEM;
     }
-    target_prot &= PROT_READ | PROT_WRITE | PROT_EXEC;
-    end = start + len;
 
-    mmap_lock();
+    last = start + len - 1;
     host_start = start & -host_page_size;
-    host_end = HOST_PAGE_ALIGN(end);
-    if (start > host_start) {
-        /* handle host page containing start */
+    host_last = ROUND_UP(last, host_page_size) - 1;
+    nranges = 0;
+
+    mmap_lock();
+
+    if (host_last - host_start < host_page_size) {
+        /* Single host page contains all guest pages: sum the prot. */
         prot1 = target_prot;
-        for (addr = host_start; addr < start; addr += TARGET_PAGE_SIZE) {
-            prot1 |= page_get_flags(addr);
+        for (abi_ulong a = host_start; a < start; a += TARGET_PAGE_SIZE) {
+            prot1 |= page_get_flags(a);
+        }
+        for (abi_ulong a = last; a < host_last; a += TARGET_PAGE_SIZE) {
+            prot1 |= page_get_flags(a + 1);
         }
-        if (host_end == host_start + host_page_size) {
-            for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
-                prot1 |= page_get_flags(addr);
+        starts[nranges] = host_start;
+        lens[nranges] = host_page_size;
+        prots[nranges] = prot1;
+        nranges++;
+    } else {
+        if (host_start < start) {
+            /* Host page contains more than one guest page: sum the prot. */
+            prot1 = target_prot;
+            for (abi_ulong a = host_start; a < start; a += TARGET_PAGE_SIZE) {
+                prot1 |= page_get_flags(a);
+            }
+            /* If the resulting sum differs, create a new range. */
+            if (prot1 != target_prot) {
+                starts[nranges] = host_start;
+                lens[nranges] = host_page_size;
+                prots[nranges] = prot1;
+                nranges++;
+                host_start += host_page_size;
             }
-            end = host_end;
         }
-        ret = mprotect(g2h_untagged(host_start),
-                       host_page_size, prot1 & PAGE_RWX);
-        if (ret != 0)
-            goto error;
-        host_start += host_page_size;
-    }
-    if (end < host_end) {
-        prot1 = target_prot;
-        for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
-            prot1 |= page_get_flags(addr);
+
+        if (last < host_last) {
+            /* Host page contains more than one guest page: sum the prot. */
+            prot1 = target_prot;
+            for (abi_ulong a = last; a < host_last; a += TARGET_PAGE_SIZE) {
+                prot1 |= page_get_flags(a + 1);
+            }
+            /* If the resulting sum differs, create a new range. */
+            if (prot1 != target_prot) {
+                host_last -= host_page_size;
+                starts[nranges] = host_last + 1;
+                lens[nranges] = host_page_size;
+                prots[nranges] = prot1;
+                nranges++;
+            }
+        }
+
+        /* Create a range for the middle, if any remains. */
+        if (host_start < host_last) {
+            starts[nranges] = host_start;
+            lens[nranges] = host_last - host_start + 1;
+            prots[nranges] = target_prot;
+            nranges++;
         }
-        ret = mprotect(g2h_untagged(host_end - host_page_size),
-                       host_page_size, prot1 & PAGE_RWX);
-        if (ret != 0)
-            goto error;
-        host_end -= host_page_size;
     }
 
-    /* handle the pages in the middle */
-    if (host_start < host_end) {
-        ret = mprotect(g2h_untagged(host_start), host_end - host_start, target_prot);
-        if (ret != 0)
+    for (int i = 0; i < nranges; ++i) {
+        ret = mprotect(g2h_untagged(starts[i]), lens[i],
+                       target_to_host_prot(prots[i]));
+        if (ret != 0) {
             goto error;
+        }
     }
-    page_set_flags(start, start + len - 1, page_flags);
-    mmap_unlock();
-    return 0;
-error:
+
+    page_set_flags(start, last, page_flags);
+    ret = 0;
+
+ error:
     mmap_unlock();
     return ret;
 }
-- 
2.45.1

Re: [PATCH 01/17] bsd-user: Delete TaskState next member

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> The next struct member of TaskState is unused. Remove it.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/qemu.h | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 02/17] bsd-user: Make init_task_state global

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> Restore init_task_state to its global status. It's needed for threading
> support outside of main.
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   bsd-user/main.c | 2 +-
>   bsd-user/qemu.h | 1 +
>   2 files changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 03/17] bsd-user: Make cpu_model and cpu_type file scope

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> linux-user already does this since 2278b93941d4. That same commit just
> added them with main() scope to bsd-user. We need the cpu_type, like
> linux-user does, to create new CPUs outside of main to support
> threading. Move both cpu_model and cpu_type to mirror linux-user/main.c.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/main.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 04/17] bsd-user: Implement cpu_copy()

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> Catch up with 30ba0ee52d15 and implement cpu_copy(). It's needed for
> threading. Stacey's original code, with bug fixes from Jessica, Justin
> and myself.
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Jessica Clarke<jrtc27@jrtc27.com>
> Signed-off-by: Justin Hibbits<chmeeedalf@gmail.com>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/main.c | 31 +++++++++++++++++++++++++++++++
>   1 file changed, 31 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 05/17] bsd-user: Eliminate unused regs arg in load_elf_binary

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/bsdload.c | 2 +-
>   bsd-user/elfload.c | 3 +--
>   bsd-user/qemu.h    | 3 +--
>   3 files changed, 3 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 06/17] bsd-user: Remove load_flt_binary prototype

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> bsd-user doesn't have support for loading FLT binaries.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/qemu.h | 2 --
>   1 file changed, 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 07/17] bsd-user: Remove deprecated -p argument

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> FreeBSD never really used the -p argument, so it's safe to remove
> entirely.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/main.c | 8 --------
>   1 file changed, 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 08/17] bsd-user: Eliminate unused qemu_uname_release

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> bsd-user has never supported this, and FreeBSD make it easy to set this
> on a per-jail basis, so that the normal reporting routines that we pass
> through just work. Since this was never used, and never even in the
> usage(), retire it to cut down on the clutter. It was literally just a
> write-only variable.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/main.c | 3 ---
>   bsd-user/qemu.h | 1 -
>   2 files changed, 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 09/17] bsd-user: target_msync unused, remove it

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> Nothing calls target_msync in the upstream or blitz fork, so remove it.
> It will save us having to modernize it.
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   bsd-user/mmap.c | 17 -----------------
>   bsd-user/qemu.h |  1 -
>   2 files changed, 18 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 10/17] bsd-user: Pass image name down the stack

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> Pass the image name down the stack so that we can give better error
> messages. Inspired by similar work in linux-user, and more likely to
> come.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/elfload.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 11/17] bsd-user: Replace set_brk and padzero with zerobss from linux-user

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> The zero_bss interface from linux-user is much better at doing this. Use
> it in preference to set_brk (badly named) and padzero. These both have
> issues with the new variable page size code, so it's best to just retire
> them and reuse the code from linux-user. Also start to use the error
> reporting code that linux-user uses to give better error messages on
> failure.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/elfload.c | 110 +++++++++++++++++++++++----------------------
>   1 file changed, 57 insertions(+), 53 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 12/17] bsd-user: Use guest_range_valid_untagged to validate range

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> This is the generic validation function, so remove some hand-rolled
> ones.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/mmap.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 13/17] bsd-user: target_mprotect: rename prot to target_prot

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> Linux-user's target_mprotect uses this convention, so move to it.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/mmap.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 14/17] bsd-user: target_mmap*: change prot to target_prot

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> Adopt the linux-user convention of using target_prot for passed in
> protections. no functional change.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/mmap.c | 47 ++++++++++++++++++++++++-----------------------
>   1 file changed, 24 insertions(+), 23 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 15/17] bsd-user: target_mprotect: use helper host_page_size local

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> Use helper variable for host_page_size. Linux-user uses a similar helper
> to make the code smaller after the multi-page-size migration.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/mmap.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 16/17] bsd-user: Define validate_prot_to_pageflags and use in mprotect

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> Define validate_prot_to_pageflags. Use it in target_mprotect to validate
> the flags. Our taraget_mmap needs more work before it can be used there,
> do don't copy linux-user's use of it there. This should hvae no net
> functional change, but does make target_mprotect more similar to
> linux-user's.
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   bsd-user/mmap.c | 28 +++++++++++++++++++++++-----
>   1 file changed, 23 insertions(+), 5 deletions(-)

> +/*
> + * Validate target prot bitmask.
> + * Return the prot bitmask for the host in *HOST_PROT.
> + * Return 0 if the target prot bitmask is invalid, otherwise
> + * the internal qemu page_flags (which will include PAGE_VALID).
> + */
> +static int validate_prot_to_pageflags(int prot)
> +{
> +    int valid = PROT_READ | PROT_WRITE | PROT_EXEC;
> +    int page_flags = (prot & PAGE_RWX) | PAGE_VALID;
> +
> +    return prot & ~valid ? 0 : page_flags;
> +}

Comment still refers to @host_prot, which you removed.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 17/17] bsd-user: copy linux-user target_mprotect impl

[Richard Henderson]

On 8/3/24 09:56, Warner Losh wrote:
> Now that we're closer to the linux-user target_mprotect code, go ahead
> and grab the rest of the implementation. This moves from a stard, end
> impl to a start, last which will allow last page mapping, etc. This also
> moves to a more general algorithm. We're close enough that this jump
> isn't so large, and doing it incrementally further has become too
> much work for too little gain.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/mmap.c | 138 +++++++++++++++++++++++++++++++-----------------
>   1 file changed, 90 insertions(+), 48 deletions(-)

Acked-by: Richard Henderson <richard.henderson@linaro.org>

r~