[Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.

[Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.

[Richard Henderson]

Removes a set of ifdefs from exec.c.

Introduce TARGET_VIRT_ADDR_SPACE_BITS for all targets other
than Alpha.  This will be used for page_find_alloc, which is
supposed to be using virtual addresses in the first place.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 exec.c                  |   17 -----------------
 target-alpha/cpu.h      |    4 +++-
 target-arm/cpu.h        |    3 +++
 target-cris/cpu.h       |    3 +++
 target-i386/cpu.h       |   11 +++++++++++
 target-m68k/cpu.h       |    3 +++
 target-microblaze/cpu.h |    3 +++
 target-mips/mips-defs.h |    4 ++++
 target-ppc/cpu.h        |   17 +++++++++++++++++
 target-s390x/cpu.h      |    5 +++++
 target-sh4/cpu.h        |    3 +++
 target-sparc/cpu.h      |    8 ++++++++
 12 files changed, 63 insertions(+), 18 deletions(-)

diff --git a/exec.c b/exec.c
index 891e0ee..431f5b2 100644
--- a/exec.c
+++ b/exec.c
@@ -62,23 +62,6 @@
 
 #define SMC_BITMAP_USE_THRESHOLD 10
 
-#if defined(TARGET_SPARC64)
-#define TARGET_PHYS_ADDR_SPACE_BITS 41
-#elif defined(TARGET_SPARC)
-#define TARGET_PHYS_ADDR_SPACE_BITS 36
-#elif defined(TARGET_ALPHA)
-#define TARGET_PHYS_ADDR_SPACE_BITS 42
-#define TARGET_VIRT_ADDR_SPACE_BITS 42
-#elif defined(TARGET_PPC64)
-#define TARGET_PHYS_ADDR_SPACE_BITS 42
-#elif defined(TARGET_X86_64)
-#define TARGET_PHYS_ADDR_SPACE_BITS 42
-#elif defined(TARGET_I386)
-#define TARGET_PHYS_ADDR_SPACE_BITS 36
-#else
-#define TARGET_PHYS_ADDR_SPACE_BITS 32
-#endif
-
 static TranslationBlock *tbs;
 int code_gen_max_blocks;
 TranslationBlock *tb_phys_hash[CODE_GEN_PHYS_HASH_SIZE];
diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index 617f55c..8afe16d 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -41,7 +41,9 @@
 
 #define TARGET_PAGE_BITS 13
 
-#define VA_BITS 43
+/* ??? 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)
 
 /* Alpha major type */
 enum {
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 4a1c53f..3892db4 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -405,6 +405,9 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define TARGET_PAGE_BITS 10
 #endif
 
+#define TARGET_PHYS_ADDR_SPACE_BITS 32
+#define TARGET_VIRT_ADDR_SPACE_BITS 32
+
 #define cpu_init cpu_arm_init
 #define cpu_exec cpu_arm_exec
 #define cpu_gen_code cpu_arm_gen_code
diff --git a/target-cris/cpu.h b/target-cris/cpu.h
index 8ff86d9..063a240 100644
--- a/target-cris/cpu.h
+++ b/target-cris/cpu.h
@@ -200,6 +200,9 @@ enum {
 #define TARGET_PAGE_BITS 13
 #define MMAP_SHIFT TARGET_PAGE_BITS
 
+#define TARGET_PHYS_ADDR_SPACE_BITS 32
+#define TARGET_VIRT_ADDR_SPACE_BITS 32
+
 #define cpu_init cpu_cris_init
 #define cpu_exec cpu_cris_exec
 #define cpu_gen_code cpu_cris_gen_code
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index ef7d951..a9dab6f 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -874,6 +874,17 @@ uint64_t cpu_get_tsc(CPUX86State *env);
 
 #define TARGET_PAGE_BITS 12
 
+#ifdef TARGET_X86_64
+#define TARGET_PHYS_ADDR_SPACE_BITS 52
+/* ??? This is really 48 bits, sign-extended, but the only thing 
+   accessible to userland with bit 48 set is the VSYSCALL, and that
+   is handled via other mechanisms.  */
+#define TARGET_VIRT_ADDR_SPACE_BITS 47
+#else
+#define TARGET_PHYS_ADDR_SPACE_BITS 36
+#define TARGET_VIRT_ADDR_SPACE_BITS 32
+#endif
+
 #define cpu_init cpu_x86_init
 #define cpu_exec cpu_x86_exec
 #define cpu_gen_code cpu_x86_gen_code
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index 68a7e41..b2f37ec 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -210,6 +210,9 @@ void register_m68k_insns (CPUM68KState *env);
 #define TARGET_PAGE_BITS 10
 #endif
 
+#define TARGET_PHYS_ADDR_SPACE_BITS 32
+#define TARGET_VIRT_ADDR_SPACE_BITS 32
+
 #define cpu_init cpu_m68k_init
 #define cpu_exec cpu_m68k_exec
 #define cpu_gen_code cpu_m68k_gen_code
diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index 1bf4875..5999386 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -253,6 +253,9 @@ enum {
 #define TARGET_PAGE_BITS 12
 #define MMAP_SHIFT TARGET_PAGE_BITS
 
+#define TARGET_PHYS_ADDR_SPACE_BITS 32
+#define TARGET_VIRT_ADDR_SPACE_BITS 32
+
 #define cpu_init cpu_mb_init
 #define cpu_exec cpu_mb_exec
 #define cpu_gen_code cpu_mb_gen_code
diff --git a/target-mips/mips-defs.h b/target-mips/mips-defs.h
index 54e80f1..0f6a956 100644
--- a/target-mips/mips-defs.h
+++ b/target-mips/mips-defs.h
@@ -8,6 +8,10 @@
 #define TARGET_PAGE_BITS 12
 #define MIPS_TLB_MAX 128
 
+/* ??? MIPS64 no doubt has a larger address space.  */
+#define TARGET_PHYS_ADDR_SPACE_BITS 32
+#define TARGET_VIRT_ADDR_SPACE_BITS 32
+
 #if defined(TARGET_MIPS64)
 #define TARGET_LONG_BITS 64
 #else
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 63aeb86..2550186 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -29,6 +29,20 @@
 #define TARGET_LONG_BITS 64
 #define TARGET_PAGE_BITS 12
 
+/* Note that the official physical address space bits is 62-M where M
+   is implementation dependent.  I've not looked up M for the set of
+   cpus we emulate at the system level.  */
+#define TARGET_PHYS_ADDR_SPACE_BITS 62
+ 
+/* Note that the PPC environment architecture talks about 80 bit virtual
+   addresses, with segmentation.  Obviously that's not all visible to a
+   single process, which is all we're concerned with here.  */
+#ifdef TARGET_ABI32
+# define TARGET_VIRT_ADDR_SPACE_BITS 32
+#else
+# define TARGET_VIRT_ADDR_SPACE_BITS 64
+#endif
+
 #else /* defined (TARGET_PPC64) */
 /* PowerPC 32 definitions */
 #define TARGET_LONG_BITS 32
@@ -50,6 +64,9 @@
 #define TARGET_PAGE_BITS 12
 #endif /* defined(TARGET_PPCEMB) */
 
+#define TARGET_PHYS_ADDR_SPACE_BITS 32
+#define TARGET_VIRT_ADDR_SPACE_BITS 32
+
 #endif /* defined (TARGET_PPC64) */
 
 #define CPUState struct CPUPPCState
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 827f4e3..dd407b2 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -99,6 +99,11 @@ int cpu_s390x_handle_mmu_fault (CPUS390XState *env, target_ulong address, int rw
 
 #define TARGET_PAGE_BITS 12
 
+/* ??? This is certainly wrong for 64-bit s390x, but given that only KVM
+   emulation actually works, this is good enough for a placeholder.  */
+#define TARGET_PHYS_ADDR_SPACE_BITS 32
+#define TARGET_VIRT_ADDR_SPACE_BITS 32
+
 #ifndef CONFIG_USER_ONLY
 extern int s390_virtio_hypercall(CPUState *env);
 extern void kvm_s390_virtio_irq(CPUState *env, int config_change, uint64_t token);
diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
index 85f221d..18a5532 100644
--- a/target-sh4/cpu.h
+++ b/target-sh4/cpu.h
@@ -44,6 +44,9 @@
 
 #define TARGET_PAGE_BITS 12	/* 4k XXXXX */
 
+#define TARGET_PHYS_ADDR_SPACE_BITS 32
+#define TARGET_VIRT_ADDR_SPACE_BITS 32
+
 #define SR_MD (1 << 30)
 #define SR_RB (1 << 29)
 #define SR_BL (1 << 28)
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 5980deb..0c5a7ef 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -7,10 +7,18 @@
 #define TARGET_LONG_BITS 32
 #define TARGET_FPREGS 32
 #define TARGET_PAGE_BITS 12 /* 4k */
+#define TARGET_PHYS_ADDR_SPACE_BITS 41
+# ifdef TARGET_ABI32
+#  define TARGET_VIRT_ADDR_SPACE_BITS 32
+# else
+#  define TARGET_VIRT_ADDR_SPACE_BITS 44
+# endif
 #else
 #define TARGET_LONG_BITS 64
 #define TARGET_FPREGS 64
 #define TARGET_PAGE_BITS 13 /* 8k */
+#define TARGET_PHYS_ADDR_SPACE_BITS 36
+#define TARGET_VIRT_ADDR_SPACE_BITS 32
 #endif
 
 #define CPUState struct CPUSPARCState
-- 
1.6.6.1

[Qemu-devel] [PATCH 2/6] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid.

[Richard Henderson]

Previously, only 32-bit guests had a proper check for the
validity of the virtual address.  Extend that check to 64-bit
guests with a restricted virtual address space.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 cpu-all.h |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 9823c24..68848e9 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -634,16 +634,22 @@ extern int have_guest_base;
 
 /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
 #define g2h(x) ((void *)((unsigned long)(x) + GUEST_BASE))
+
+#if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
+#define h2g_valid(x) 1
+#else
+#define h2g_valid(x) ({ \
+    unsigned long __guest = (unsigned long)(x) - GUEST_BASE; \
+    __guest < (1ul << TARGET_VIRT_ADDR_SPACE_BITS); \
+})
+#endif
+
 #define h2g(x) ({ \
     unsigned long __ret = (unsigned long)(x) - GUEST_BASE; \
     /* Check if given address fits target address space */ \
-    assert(__ret == (abi_ulong)__ret); \
+    assert(h2g_valid(x)); \
     (abi_ulong)__ret; \
 })
-#define h2g_valid(x) ({ \
-    unsigned long __guest = (unsigned long)(x) - GUEST_BASE; \
-    (__guest == (abi_ulong)__guest); \
-})
 
 #define saddr(x) g2h(x)
 #define laddr(x) g2h(x)
-- 
1.6.6.1

[Qemu-devel] [PATCH 3/6] linux-user: Use h2g_valid in qemu_vmalloc.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/mmap.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 25fc0b2..65fdc33 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -80,16 +80,15 @@ void mmap_unlock(void)
 void *qemu_vmalloc(size_t size)
 {
     void *p;
-    unsigned long addr;
+
     mmap_lock();
     /* Use map and mark the pages as used.  */
     p = mmap(NULL, size, PROT_READ | PROT_WRITE,
              MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
-    addr = (unsigned long)p;
-    if (addr == (target_ulong) addr) {
-        /* Allocated region overlaps guest address space.
-           This may recurse.  */
+    if (h2g_valid(p)) {
+        /* Allocated region overlaps guest address space. This may recurse.  */
+        unsigned long addr = h2g(p);
         page_set_flags(addr & TARGET_PAGE_MASK, TARGET_PAGE_ALIGN(addr + size),
                        PAGE_RESERVED);
     }
-- 
1.6.6.1

[Qemu-devel] [PATCH 4/6] linux-user: Fix mmap_find_vma returning invalid addresses.

[Richard Henderson]

Don't return addresses that aren't properly aligned for the guest,
e.g. when the guest has a larger page size than the host.  Don't
return addresses that are outside the virtual address space for the
target, by paying proper attention to the h2g/g2h macros.

At the same time, place the default mapping base for 64-bit guests
(on 64-bit hosts) outside the low 4G.  Consistently interpret
mmap_next_start in the guest address space.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/main.c |   11 ++----
 linux-user/mmap.c |  102 ++++++++++++++++++++++++++++++++++++++++------------
 linux-user/qemu.h |    2 -
 3 files changed, 82 insertions(+), 33 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index eeae22e..4614e3c 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -39,8 +39,8 @@
 char *exec_path;
 
 int singlestep;
-#if defined(CONFIG_USE_GUEST_BASE)
 unsigned long mmap_min_addr;
+#if defined(CONFIG_USE_GUEST_BASE)
 unsigned long guest_base;
 int have_guest_base;
 #endif
@@ -2812,16 +2812,14 @@ int main(int argc, char **argv, char **envp)
      * proper page alignment for guest_base.
      */
     guest_base = HOST_PAGE_ALIGN(guest_base);
+#endif /* CONFIG_USE_GUEST_BASE */
 
     /*
      * Read in mmap_min_addr kernel parameter.  This value is used
      * When loading the ELF image to determine whether guest_base
-     * is needed.
-     *
-     * When user has explicitly set the quest base, we skip this
-     * test.
+     * is needed.  It is also used in mmap_find_vma.
      */
-    if (!have_guest_base) {
+    {
         FILE *fp;
 
         if ((fp = fopen("/proc/sys/vm/mmap_min_addr", "r")) != NULL) {
@@ -2833,7 +2831,6 @@ int main(int argc, char **argv, char **envp)
             fclose(fp);
         }
     }
-#endif /* CONFIG_USE_GUEST_BASE */
 
     /*
      * Prepare copy of argv vector for target.
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 65fdc33..ad00b6f 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -264,12 +264,15 @@ static int mmap_frag(abi_ulong real_start,
     return 0;
 }
 
-#if defined(__CYGWIN__)
+#if HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
+# define TASK_UNMAPPED_BASE  (1ul << 38)
+#elif defined(__CYGWIN__)
 /* Cygwin doesn't have a whole lot of address space.  */
-static abi_ulong mmap_next_start = 0x18000000;
+# define TASK_UNMAPPED_BASE  0x18000000
 #else
-static abi_ulong mmap_next_start = 0x40000000;
+# define TASK_UNMAPPED_BASE  0x40000000
 #endif
+static abi_ulong mmap_next_start = TASK_UNMAPPED_BASE;
 
 unsigned long last_brk;
 
@@ -281,19 +284,24 @@ unsigned long last_brk;
  */
 abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size)
 {
-    void *ptr;
+    void *ptr, *prev;
     abi_ulong addr;
-
-    size = HOST_PAGE_ALIGN(size);
-    start &= qemu_host_page_mask;
+    int wrapped, repeat;
 
     /* If 'start' == 0, then a default start address is used. */
-    if (start == 0)
+    if (start == 0) {
         start = mmap_next_start;
+    } else {
+        start &= qemu_host_page_mask;
+    }
+
+    size = HOST_PAGE_ALIGN(size);
 
     addr = start;
+    wrapped = repeat = 0;
+    prev = 0;
 
-    for(;;) {
+    for (;; prev = ptr) {
         /*
          * Reserve needed memory area to avoid a race.
          * It should be discarded using:
@@ -301,31 +309,77 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size)
          *  - mremap() with MREMAP_FIXED flag
          *  - shmat() with SHM_REMAP flag
          */
-        ptr = mmap((void *)(unsigned long)addr, size, PROT_NONE,
+        ptr = mmap(g2h(addr), size, PROT_NONE,
                    MAP_ANONYMOUS|MAP_PRIVATE|MAP_NORESERVE, -1, 0);
 
         /* ENOMEM, if host address space has no memory */
-        if (ptr == MAP_FAILED)
+        if (ptr == MAP_FAILED) {
             return (abi_ulong)-1;
+        }
 
-        /* If address fits target address space we've found what we need */
-        if ((unsigned long)ptr + size - 1 <= (abi_ulong)-1)
-            break;
+        /* Count the number of sequential returns of the same address.
+           This is used to modify the search algorithm below.  */
+        repeat = (ptr == prev ? repeat + 1 : 0);
 
-        /* Unmap and try again with new page */
+        if (h2g_valid(ptr + size - 1)) {
+            addr = h2g(ptr);
+
+            if ((addr & ~TARGET_PAGE_MASK) == 0) {
+                /* Success.  */
+                if (start == mmap_next_start && addr >= TASK_UNMAPPED_BASE) {
+                    mmap_next_start = addr + size;
+                }
+                return addr;
+            }
+
+            /* The address is not properly aligned for the target.  */
+            switch (repeat) {
+            case 0:
+                /* Assume the result that the kernel gave us is the
+                   first with enough free space, so start again at the
+                   next higher target page.  */
+                addr = TARGET_PAGE_ALIGN(addr);
+                break;
+            case 1:
+                /* Sometimes the kernel decides to perform the allocation
+                   at the top end of memory instead.  */
+                addr &= TARGET_PAGE_MASK;
+                break;
+            case 2:
+                /* Start over at low memory.  */
+                addr = 0;
+                break;
+            default:
+                /* Fail.  This unaligned block must the last.  */
+                addr = -1;
+                break;
+            }
+        } else {
+            /* Since the result the kernel gave didn't fit, start
+               again at low memory.  If any repetition, fail.  */
+            addr = (repeat ? -1 : 0);
+        }
+
+        /* Unmap and try again.  */
         munmap(ptr, size);
-        addr += qemu_host_page_size;
 
-        /* ENOMEM if we check whole of target address space */
-        if (addr == start)
+        /* ENOMEM if we checked the whole of the target address space.  */
+        if (addr == -1ul) {
             return (abi_ulong)-1;
+        } else if (addr == 0) {
+            if (wrapped) {
+                return (abi_ulong)-1;
+            }
+            wrapped = 1;
+            /* Don't actually use 0 when wrapping, instead indicate
+               that we'd truely like an allocation in low memory.  */
+            addr = (mmap_min_addr > TARGET_PAGE_SIZE
+                     ? TARGET_PAGE_ALIGN(mmap_min_addr)
+                     : TARGET_PAGE_SIZE);
+        } else if (wrapped && addr >= start) {
+            return (abi_ulong)-1;
+        }
     }
-
-    /* Update default start address */
-    if (start == mmap_next_start)
-        mmap_next_start = (unsigned long)ptr + size;
-
-    return h2g(ptr);
 }
 
 /* NOTE: all the constants are the HOST ones */
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index d129deb..6ab9517 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -133,9 +133,7 @@ void init_task_state(TaskState *ts);
 void task_settid(TaskState *);
 void stop_all_tasks(void);
 extern const char *qemu_uname_release;
-#if defined(CONFIG_USE_GUEST_BASE)
 extern unsigned long mmap_min_addr;
-#endif
 
 /* ??? See if we can avoid exposing so much of the loader internals.  */
 /*
-- 
1.6.6.1

[Qemu-devel] [PATCH 5/6] Implement multi-level page tables.

[Richard Henderson]

Define L1_MAP_ADDR_SPACE_BITS to be either the virtual address size
(in user mode) or physical address size (in system mode), and use
that to size l1_map.  This rewrites page_find_alloc, page_flush_tb,
and walk_memory_regions.

Use TARGET_PHYS_ADDR_SPACE_BITS for the physical memory map based
off of l1_phys_map.  This rewrites page_phys_find_alloc and
phys_page_for_each.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 cpu-all.h |    7 +-
 exec.c    |  451 +++++++++++++++++++++++++++++++++++++------------------------
 2 files changed, 278 insertions(+), 180 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 68848e9..a51b301 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -745,8 +745,11 @@ extern unsigned long qemu_host_page_mask;
 #define PAGE_RESERVED  0x0020
 
 void page_dump(FILE *f);
-int walk_memory_regions(void *,
-    int (*fn)(void *, unsigned long, unsigned long, unsigned long));
+
+typedef int (*walk_memory_regions_fn)(void *, unsigned long,
+                                      unsigned long, unsigned long);
+int walk_memory_regions(void *, walk_memory_regions_fn);
+
 int page_get_flags(target_ulong address);
 void page_set_flags(target_ulong start, target_ulong end, int flags);
 int page_check_range(target_ulong start, target_ulong len, int flags);
diff --git a/exec.c b/exec.c
index 431f5b2..5455a74 100644
--- a/exec.c
+++ b/exec.c
@@ -141,30 +141,56 @@ typedef struct PhysPageDesc {
     ram_addr_t region_offset;
 } PhysPageDesc;
 
-#define L2_BITS 10
-#if defined(CONFIG_USER_ONLY) && defined(TARGET_VIRT_ADDR_SPACE_BITS)
-/* XXX: this is a temporary hack for alpha target.
- *      In the future, this is to be replaced by a multi-level table
- *      to actually be able to handle the complete 64 bits address space.
- */
-#define L1_BITS (TARGET_VIRT_ADDR_SPACE_BITS - L2_BITS - TARGET_PAGE_BITS)
+/* In system mode we want L1_MAP to be based on physical addresses,
+   while in user mode we want it to be based on virtual addresses.  */
+#if !defined(CONFIG_USER_ONLY)
+# define L1_MAP_ADDR_SPACE_BITS  TARGET_PHYS_ADDR_SPACE_BITS
 #else
-#define L1_BITS (32 - L2_BITS - TARGET_PAGE_BITS)
+# define L1_MAP_ADDR_SPACE_BITS  TARGET_VIRT_ADDR_SPACE_BITS
 #endif
 
-#define L1_SIZE (1 << L1_BITS)
+/* Size of the L2 (and L3, etc) page tables.  */
+#define L2_BITS 10
 #define L2_SIZE (1 << L2_BITS)
 
+/* The bits remaining after N lower levels of page tables.  */
+#define P_L1_BITS_REM \
+    ((TARGET_PHYS_ADDR_SPACE_BITS - TARGET_PAGE_BITS) % L2_BITS)
+#define V_L1_BITS_REM \
+    ((L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS) % L2_BITS)
+
+/* Size of the L1 page table.  Avoid silly small sizes.  */
+#if P_L1_BITS_REM < 4
+#define P_L1_BITS  (P_L1_BITS_REM + L2_BITS)
+#else
+#define P_L1_BITS  P_L1_BITS_REM
+#endif
+
+#if V_L1_BITS_REM < 4
+#define V_L1_BITS  (V_L1_BITS_REM + L2_BITS)
+#else
+#define V_L1_BITS  V_L1_BITS_REM
+#endif
+
+#define P_L1_SIZE  ((target_phys_addr_t)1 << P_L1_BITS)
+#define V_L1_SIZE  ((target_ulong)1 << V_L1_BITS)
+
+#define P_L1_SHIFT (TARGET_PHYS_ADDR_SPACE_BITS - TARGET_PAGE_BITS - P_L1_BITS)
+#define V_L1_SHIFT (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - V_L1_BITS)
+
 unsigned long qemu_real_host_page_size;
 unsigned long qemu_host_page_bits;
 unsigned long qemu_host_page_size;
 unsigned long qemu_host_page_mask;
 
-/* XXX: for system emulation, it could just be an array */
-static PageDesc *l1_map[L1_SIZE];
+/* This is a multi-level map on the virtual address space.
+   The bottom level has pointers to PageDesc.  */
+static void *l1_map[V_L1_SIZE];
 
 #if !defined(CONFIG_USER_ONLY)
-static PhysPageDesc **l1_phys_map;
+/* This is a multi-level map on the physical address space.
+   The bottom level has pointers to PhysPageDesc.  */
+static void *l1_phys_map[P_L1_SIZE];
 
 static void io_mem_init(void);
 
@@ -239,133 +265,159 @@ static void page_init(void)
     while ((1 << qemu_host_page_bits) < qemu_host_page_size)
         qemu_host_page_bits++;
     qemu_host_page_mask = ~(qemu_host_page_size - 1);
-#if !defined(CONFIG_USER_ONLY)
-    l1_phys_map = qemu_vmalloc(L1_SIZE * sizeof(void *));
-    memset(l1_phys_map, 0, L1_SIZE * sizeof(void *));
-#endif
 
 #if !defined(_WIN32) && defined(CONFIG_USER_ONLY)
     {
-        long long startaddr, endaddr;
         FILE *f;
-        int n;
 
-        mmap_lock();
         last_brk = (unsigned long)sbrk(0);
+
         f = fopen("/proc/self/maps", "r");
         if (f) {
+            mmap_lock();
+
             do {
-                n = fscanf (f, "%llx-%llx %*[^\n]\n", &startaddr, &endaddr);
-                if (n == 2) {
-                    startaddr = MIN(startaddr,
-                                    (1ULL << TARGET_PHYS_ADDR_SPACE_BITS) - 1);
-                    endaddr = MIN(endaddr,
-                                    (1ULL << TARGET_PHYS_ADDR_SPACE_BITS) - 1);
-                    page_set_flags(startaddr & TARGET_PAGE_MASK,
-                                   TARGET_PAGE_ALIGN(endaddr),
-                                   PAGE_RESERVED); 
+                unsigned long startaddr, endaddr;
+                int n;
+
+                n = fscanf (f, "%lx-%lx %*[^\n]\n", &startaddr, &endaddr);
+
+                if (n == 2 && h2g_valid(startaddr)) {
+                    startaddr = h2g(startaddr) & TARGET_PAGE_MASK;
+
+                    if (h2g_valid(endaddr)) {
+                        endaddr = h2g(endaddr);
+                    } else {
+                        endaddr = ~0ul;
+                    }
+                    page_set_flags(startaddr, endaddr, PAGE_RESERVED); 
                 }
             } while (!feof(f));
+
             fclose(f);
+            mmap_unlock();
         }
-        mmap_unlock();
     }
 #endif
 }
 
-static inline PageDesc **page_l1_map(target_ulong index)
+static PageDesc *page_find_alloc(target_ulong index, int alloc)
 {
-#if TARGET_LONG_BITS > 32
-    /* Host memory outside guest VM.  For 32-bit targets we have already
-       excluded high addresses.  */
-    if (index > ((target_ulong)L2_SIZE * L1_SIZE))
-        return NULL;
+#if defined(CONFIG_USER_ONLY)
+    /* We can't use qemu_malloc because it may recurse into a locked mutex.
+       Neither can we record the new pages we reserve while allocating a
+       given page because that may recurse into an unallocated page table
+       entry.  Stuff the allocations we do make into a queue and process
+       them after having completed one entire page table allocation.  */
+
+    unsigned long reserve[2 * (V_L1_SHIFT / L2_BITS)];
+    int reserve_idx = 0;
+
+# define ALLOC(P, SIZE)                                 \
+    do {                                                \
+        P = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,    \
+                 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);   \
+        if (h2g_valid(P)) {                             \
+            reserve[reserve_idx] = h2g(P);              \
+            reserve[reserve_idx + 1] = SIZE;            \
+            reserve_idx += 2;                           \
+        }                                               \
+    } while (0)
+#else
+# define ALLOC(P, SIZE) \
+    do { P = qemu_mallocz(SIZE); } while (0)
 #endif
-    return &l1_map[index >> L2_BITS];
-}
 
-static inline PageDesc *page_find_alloc(target_ulong index)
-{
-    PageDesc **lp, *p;
-    lp = page_l1_map(index);
-    if (!lp)
-        return NULL;
+    PageDesc *pd;
+    void **lp;
+    int i;
 
-    p = *lp;
-    if (!p) {
-        /* allocate if not found */
-#if defined(CONFIG_USER_ONLY)
-        size_t len = sizeof(PageDesc) * L2_SIZE;
-        /* Don't use qemu_malloc because it may recurse.  */
-        p = mmap(NULL, len, PROT_READ | PROT_WRITE,
-                 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-        *lp = p;
-        if (h2g_valid(p)) {
-            unsigned long addr = h2g(p);
-            page_set_flags(addr & TARGET_PAGE_MASK,
-                           TARGET_PAGE_ALIGN(addr + len),
-                           PAGE_RESERVED); 
+    /* Level 1.  Always allocated.  */
+    lp = l1_map + ((index >> V_L1_SHIFT) & (V_L1_SIZE - 1));
+
+    /* Level 2..N-1.  */
+    for (i = V_L1_SHIFT / L2_BITS - 1; i > 0; i--) {
+        void **p = *lp;
+
+        if (p == NULL) {
+            if (!alloc) {
+                return NULL;
+            }
+            ALLOC(p, sizeof(void *) * L2_SIZE);
+            *lp = p;
         }
-#else
-        p = qemu_mallocz(sizeof(PageDesc) * L2_SIZE);
-        *lp = p;
-#endif
+
+        lp = p + ((index >> (i * L2_BITS)) & (L2_SIZE - 1));
     }
-    return p + (index & (L2_SIZE - 1));
+
+    pd = *lp;
+    if (pd == NULL) {
+        if (!alloc) {
+            return NULL;
+        }
+        ALLOC(pd, sizeof(PageDesc) * L2_SIZE);
+        *lp = pd;
+    }
+
+#undef ALLOC
+#if defined(CONFIG_USER_ONLY)
+    for (i = 0; i < reserve_idx; i += 2) {
+        unsigned long addr = reserve[i];
+        unsigned long len = reserve[i + 1];
+
+        page_set_flags(addr & TARGET_PAGE_MASK,
+                       TARGET_PAGE_ALIGN(addr + len),
+                       PAGE_RESERVED);
+    }
+#endif
+
+    return pd + (index & (L2_SIZE - 1));
 }
 
 static inline PageDesc *page_find(target_ulong index)
 {
-    PageDesc **lp, *p;
-    lp = page_l1_map(index);
-    if (!lp)
-        return NULL;
-
-    p = *lp;
-    if (!p) {
-        return NULL;
-    }
-    return p + (index & (L2_SIZE - 1));
+    return page_find_alloc(index, 0);
 }
 
 #if !defined(CONFIG_USER_ONLY)
 static PhysPageDesc *phys_page_find_alloc(target_phys_addr_t index, int alloc)
 {
-    void **lp, **p;
     PhysPageDesc *pd;
+    void **lp;
+    int i;
 
-    p = (void **)l1_phys_map;
-#if TARGET_PHYS_ADDR_SPACE_BITS > 32
+    /* Level 1.  Always allocated.  */
+    lp = l1_phys_map + ((index >> P_L1_SHIFT) & (P_L1_SIZE - 1));
 
-#if TARGET_PHYS_ADDR_SPACE_BITS > (32 + L1_BITS)
-#error unsupported TARGET_PHYS_ADDR_SPACE_BITS
-#endif
-    lp = p + ((index >> (L1_BITS + L2_BITS)) & (L1_SIZE - 1));
-    p = *lp;
-    if (!p) {
-        /* allocate if not found */
-        if (!alloc)
-            return NULL;
-        p = qemu_vmalloc(sizeof(void *) * L1_SIZE);
-        memset(p, 0, sizeof(void *) * L1_SIZE);
-        *lp = p;
+    /* Level 2..N-1.  */
+    for (i = P_L1_SHIFT / L2_BITS - 1; i > 0; i--) {
+        void **p = *lp;
+        if (p == NULL) {
+            if (!alloc) {
+                return NULL;
+            }
+            *lp = p = qemu_mallocz(sizeof(void *) * L2_SIZE);
+        }
+        lp = p + ((index >> (i * L2_BITS)) & (L2_SIZE - 1));
     }
-#endif
-    lp = p + ((index >> L2_BITS) & (L1_SIZE - 1));
+
     pd = *lp;
-    if (!pd) {
+    if (pd == NULL) {
         int i;
-        /* allocate if not found */
-        if (!alloc)
+
+        if (!alloc) {
             return NULL;
-        pd = qemu_vmalloc(sizeof(PhysPageDesc) * L2_SIZE);
-        *lp = pd;
+        }
+
+        *lp = pd = qemu_malloc(sizeof(PhysPageDesc) * L2_SIZE);
+
         for (i = 0; i < L2_SIZE; i++) {
-          pd[i].phys_offset = IO_MEM_UNASSIGNED;
-          pd[i].region_offset = (index + i) << TARGET_PAGE_BITS;
+            pd[i].phys_offset = IO_MEM_UNASSIGNED;
+            pd[i].region_offset = (index + i) << TARGET_PAGE_BITS;
         }
     }
-    return ((PhysPageDesc *)pd) + (index & (L2_SIZE - 1));
+
+    return pd + (index & (L2_SIZE - 1));
 }
 
 static inline PhysPageDesc *phys_page_find(target_phys_addr_t index)
@@ -573,23 +625,36 @@ static inline void invalidate_page_bitmap(PageDesc *p)
     p->code_write_count = 0;
 }
 
-/* set to NULL all the 'first_tb' fields in all PageDescs */
-static void page_flush_tb(void)
+/* Set to NULL all the 'first_tb' fields in all PageDescs. */
+
+static void page_flush_tb_1 (int level, void **lp)
 {
-    int i, j;
-    PageDesc *p;
+    int i;
 
-    for(i = 0; i < L1_SIZE; i++) {
-        p = l1_map[i];
-        if (p) {
-            for(j = 0; j < L2_SIZE; j++) {
-                p->first_tb = NULL;
-                invalidate_page_bitmap(p);
-                p++;
-            }
+    if (*lp == NULL) {
+        return;
+    }
+    if (level == 0) {
+        PageDesc *pd = *lp;
+        for (i = 0; i < L2_BITS; ++i) {
+            pd[i].first_tb = NULL;
+            invalidate_page_bitmap(pd + i);
+        }
+    } else {
+        void **pp = *lp;
+        for (i = 0; i < L2_BITS; ++i) {
+            page_flush_tb_1 (level - 1, pp + i);
         }
     }
 }
+            
+static void page_flush_tb(void)
+{
+    int i;
+    for (i = 0; i < V_L1_SIZE; i++) {
+        page_flush_tb_1(V_L1_SHIFT / L2_BITS - 1, l1_map + i);
+    }
+}
 
 /* flush all the translation blocks */
 /* XXX: tb_flush is currently not thread safe */
@@ -1081,7 +1146,7 @@ static inline void tb_alloc_page(TranslationBlock *tb,
     TranslationBlock *last_first_tb;
 
     tb->page_addr[n] = page_addr;
-    p = page_find_alloc(page_addr >> TARGET_PAGE_BITS);
+    p = page_find_alloc(page_addr >> TARGET_PAGE_BITS, 1);
     tb->page_next[n] = p->first_tb;
     last_first_tb = p->first_tb;
     p->first_tb = (TranslationBlock *)((long)tb | n);
@@ -1641,50 +1706,37 @@ static int cpu_notify_migration_log(int enable)
     return 0;
 }
 
-static void phys_page_for_each_in_l1_map(PhysPageDesc **phys_map,
-                                         CPUPhysMemoryClient *client)
+static void phys_page_for_each_1(CPUPhysMemoryClient *client,
+                                 int level, void **lp)
 {
-    PhysPageDesc *pd;
-    int l1, l2;
+    int i;
 
-    for (l1 = 0; l1 < L1_SIZE; ++l1) {
-        pd = phys_map[l1];
-        if (!pd) {
-            continue;
-        }
-        for (l2 = 0; l2 < L2_SIZE; ++l2) {
-            if (pd[l2].phys_offset == IO_MEM_UNASSIGNED) {
-                continue;
+    if (*lp == NULL) {
+        return;
+    }
+    if (level == 0) {
+        PhysPageDesc *pd = *lp;
+        for (i = 0; i < L2_BITS; ++i) {
+            if (pd[i].phys_offset != IO_MEM_UNASSIGNED) {
+                client->set_memory(client, pd[i].region_offset,
+                                   TARGET_PAGE_SIZE, pd[i].phys_offset);
             }
-            client->set_memory(client, pd[l2].region_offset,
-                               TARGET_PAGE_SIZE, pd[l2].phys_offset);
+        }
+    } else {
+        void **pp = *lp;
+        for (i = 0; i < L2_BITS; ++i) {
+            phys_page_for_each_1(client, level - 1, pp + i);
         }
     }
 }
 
 static void phys_page_for_each(CPUPhysMemoryClient *client)
 {
-#if TARGET_PHYS_ADDR_SPACE_BITS > 32
-
-#if TARGET_PHYS_ADDR_SPACE_BITS > (32 + L1_BITS)
-#error unsupported TARGET_PHYS_ADDR_SPACE_BITS
-#endif
-    void **phys_map = (void **)l1_phys_map;
-    int l1;
-    if (!l1_phys_map) {
-        return;
-    }
-    for (l1 = 0; l1 < L1_SIZE; ++l1) {
-        if (phys_map[l1]) {
-            phys_page_for_each_in_l1_map(phys_map[l1], client);
-        }
-    }
-#else
-    if (!l1_phys_map) {
-        return;
+    int i;
+    for (i = 0; i < P_L1_SIZE; ++i) {
+        phys_page_for_each_1(client, P_L1_SHIFT / L2_BITS - 1,
+                             l1_phys_map + 1);
     }
-    phys_page_for_each_in_l1_map(l1_phys_map, client);
-#endif
 }
 
 void cpu_register_phys_memory_client(CPUPhysMemoryClient *client)
@@ -2148,44 +2200,87 @@ void tlb_flush_page(CPUState *env, target_ulong addr)
  * Walks guest process memory "regions" one by one
  * and calls callback function 'fn' for each region.
  */
-int walk_memory_regions(void *priv,
-    int (*fn)(void *, unsigned long, unsigned long, unsigned long))
+
+struct walk_memory_regions_data
 {
-    unsigned long start, end;
-    PageDesc *p = NULL;
-    int i, j, prot, prot1;
-    int rc = 0;
+    walk_memory_regions_fn fn;
+    void *priv;
+    unsigned long start;
+    int prot;
+};
 
-    start = end = -1;
-    prot = 0;
+static int walk_memory_regions_end(struct walk_memory_regions_data *data,
+                                   unsigned long end, int new_prot)
+{
+    if (data->start != -1ul) {
+        int rc = data->fn(data->priv, data->start, end, data->prot);
+        if (rc != 0) {
+            return rc;
+        }
+    }
+
+    data->start = (new_prot ? end : -1ul);
+    data->prot = new_prot;
+
+    return 0;
+}
+
+static int walk_memory_regions_1(struct walk_memory_regions_data *data,
+                                 unsigned long base, int level, void **lp)
+{
+    unsigned long pa;
+    int i, rc;
 
-    for (i = 0; i <= L1_SIZE; i++) {
-        p = (i < L1_SIZE) ? l1_map[i] : NULL;
-        for (j = 0; j < L2_SIZE; j++) {
-            prot1 = (p == NULL) ? 0 : p[j].flags;
-            /*
-             * "region" is one continuous chunk of memory
-             * that has same protection flags set.
-             */
-            if (prot1 != prot) {
-                end = (i << (32 - L1_BITS)) | (j << TARGET_PAGE_BITS);
-                if (start != -1) {
-                    rc = (*fn)(priv, start, end, prot);
-                    /* callback can stop iteration by returning != 0 */
-                    if (rc != 0)
-                        return (rc);
+    if (*lp == NULL) {
+        return walk_memory_regions_end(data, base, 0);
+    }
+
+    if (level == 0) {
+        PageDesc *pd = *lp;
+        for (i = 0; i < L2_BITS; ++i) {
+            int prot = pd[i].flags;
+
+            pa = base | (i << TARGET_PAGE_BITS);
+            if (prot != data->prot) {
+                rc = walk_memory_regions_end(data, pa, prot);
+                if (rc != 0) {
+                    return rc;
                 }
-                if (prot1 != 0)
-                    start = end;
-                else
-                    start = -1;
-                prot = prot1;
             }
-            if (p == NULL)
-                break;
+        }
+    } else {
+        void **pp = *lp;
+        for (i = 0; i < L2_BITS; ++i) {
+            pa = base | (i << (TARGET_PAGE_BITS + L2_BITS * level));
+            rc = walk_memory_regions_1(data, pa, level - 1, pp + i);
+            if (rc != 0) {
+                return rc;
+            }
+        }
+    }
+
+    return 0;
+}
+
+int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
+{
+    struct walk_memory_regions_data data;
+    unsigned long i;
+
+    data.fn = fn;
+    data.priv = priv;
+    data.start = -1ul;
+    data.prot = 0;
+
+    for (i = 0; i < V_L1_SIZE; i++) {
+        int rc = walk_memory_regions_1(&data, i << V_L1_SHIFT,
+                                       V_L1_SHIFT / L2_BITS - 1, l1_map + i);
+        if (rc != 0) {
+            return rc;
         }
     }
-    return (rc);
+
+    return walk_memory_regions_end(&data, 0, 0);
 }
 
 static int dump_region(void *priv, unsigned long start,
-- 
1.6.6.1

[Qemu-devel] [PATCH 6/6] Fix last page errors in page_check_range and page_set_flags.

[Richard Henderson]

The addr < end comparison prevents iterating over the last
page in the guest address space; an iteration based on
length avoids this problem.

At the same time, assert that the given address is in the
guest address space.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 exec.c |   54 ++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/exec.c b/exec.c
index 5455a74..245535a 100644
--- a/exec.c
+++ b/exec.c
@@ -2315,27 +2315,35 @@ int page_get_flags(target_ulong address)
     return p->flags;
 }
 
-/* modify the flags of a page and invalidate the code if
-   necessary. The flag PAGE_WRITE_ORG is positioned automatically
-   depending on PAGE_WRITE */
+/* Modify the flags of a page and invalidate the code if necessary.
+   The flag PAGE_WRITE_ORG is positioned automatically depending
+   on PAGE_WRITE.  The mmap_lock should already be held.  */
 void page_set_flags(target_ulong start, target_ulong end, int flags)
 {
-    PageDesc *p;
-    target_ulong addr;
+    target_ulong addr, len;
+
+    /* 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 HOST_LONG_BITS > L1_MAP_ADDR_SPACE_BITS
+    assert(end < (1ul << L1_MAP_ADDR_SPACE_BITS));
+#endif
+    assert(start < end);
 
-    /* mmap_lock should already be held.  */
     start = start & TARGET_PAGE_MASK;
     end = TARGET_PAGE_ALIGN(end);
-    if (flags & PAGE_WRITE)
+
+    if (flags & PAGE_WRITE) {
         flags |= PAGE_WRITE_ORG;
-    for(addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
-        p = page_find_alloc(addr >> TARGET_PAGE_BITS);
-        /* We may be called for host regions that are outside guest
-           address space.  */
-        if (!p)
-            return;
-        /* if the write protection is set, then we invalidate the code
-           inside */
+    }
+
+    for (addr = start, len = end - start;
+         len != 0;
+         len -= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
+        PageDesc *p = page_find_alloc(addr >> TARGET_PAGE_BITS, 1);
+
+        /* If the write protection bit is set, then we invalidate
+           the code inside.  */
         if (!(p->flags & PAGE_WRITE) &&
             (flags & PAGE_WRITE) &&
             p->first_tb) {
@@ -2351,14 +2359,24 @@ int page_check_range(target_ulong start, target_ulong len, int flags)
     target_ulong end;
     target_ulong addr;
 
-    if (start + len < start)
-        /* we've wrapped around */
+    /* 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 HOST_LONG_BITS > L1_MAP_ADDR_SPACE_BITS
+    assert(start < (1ul << L1_MAP_ADDR_SPACE_BITS));
+#endif
+
+    if (start + len - 1 < start) {
+        /* We've wrapped around.  */
         return -1;
+    }
 
     end = TARGET_PAGE_ALIGN(start+len); /* must do before we loose bits in the next step */
     start = start & TARGET_PAGE_MASK;
 
-    for(addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
+    for (addr = start, len = end - start;
+         len != 0;
+         len -= TARGET_PAGE_SIZE, addr += TARGET_PAGE_SIZE) {
         p = page_find(addr >> TARGET_PAGE_BITS);
         if( !p )
             return -1;
-- 
1.6.6.1

[Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Richard Henderson]

Changes since v2:
  * Fix two errors pointed out by pbrook: l1_map size in system mode,
    and h2g_valid definition for 64-on-32-bit.
  * Assertions that addresses are not outside the bounds of the 
    guest address space.
  * Re-base vs master, after l1_map_phys changes.


r~


Richard Henderson (6):
  Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
  Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid.
  linux-user: Use h2g_valid in qemu_vmalloc.
  linux-user: Fix mmap_find_vma returning invalid addresses.
  Implement multi-level page tables.
  Fix last page errors in page_check_range and page_set_flags.

 cpu-all.h               |   23 ++-
 exec.c                  |  522 ++++++++++++++++++++++++++++-------------------
 linux-user/main.c       |   11 +-
 linux-user/mmap.c       |  111 ++++++++---
 linux-user/qemu.h       |    2 -
 target-alpha/cpu.h      |    4 +-
 target-arm/cpu.h        |    3 +
 target-cris/cpu.h       |    3 +
 target-i386/cpu.h       |   11 +
 target-m68k/cpu.h       |    3 +
 target-microblaze/cpu.h |    3 +
 target-mips/mips-defs.h |    4 +
 target-ppc/cpu.h        |   17 ++
 target-s390x/cpu.h      |    5 +
 target-sh4/cpu.h        |    3 +
 target-sparc/cpu.h      |    8 +
 16 files changed, 474 insertions(+), 259 deletions(-)

Re: [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.

[Aurelien Jarno]

On Wed, Mar 10, 2010 at 02:33:23PM -0800, Richard Henderson wrote:
> Removes a set of ifdefs from exec.c.
> 
> Introduce TARGET_VIRT_ADDR_SPACE_BITS for all targets other
> than Alpha.  This will be used for page_find_alloc, which is
> supposed to be using virtual addresses in the first place.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  exec.c                  |   17 -----------------
>  target-alpha/cpu.h      |    4 +++-
>  target-arm/cpu.h        |    3 +++
>  target-cris/cpu.h       |    3 +++
>  target-i386/cpu.h       |   11 +++++++++++
>  target-m68k/cpu.h       |    3 +++
>  target-microblaze/cpu.h |    3 +++
>  target-mips/mips-defs.h |    4 ++++
>  target-ppc/cpu.h        |   17 +++++++++++++++++
>  target-s390x/cpu.h      |    5 +++++
>  target-sh4/cpu.h        |    3 +++
>  target-sparc/cpu.h      |    8 ++++++++
>  12 files changed, 63 insertions(+), 18 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 891e0ee..431f5b2 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -62,23 +62,6 @@
>  
>  #define SMC_BITMAP_USE_THRESHOLD 10
>  
> -#if defined(TARGET_SPARC64)
> -#define TARGET_PHYS_ADDR_SPACE_BITS 41
> -#elif defined(TARGET_SPARC)
> -#define TARGET_PHYS_ADDR_SPACE_BITS 36
> -#elif defined(TARGET_ALPHA)
> -#define TARGET_PHYS_ADDR_SPACE_BITS 42
> -#define TARGET_VIRT_ADDR_SPACE_BITS 42
> -#elif defined(TARGET_PPC64)
> -#define TARGET_PHYS_ADDR_SPACE_BITS 42
> -#elif defined(TARGET_X86_64)
> -#define TARGET_PHYS_ADDR_SPACE_BITS 42
> -#elif defined(TARGET_I386)
> -#define TARGET_PHYS_ADDR_SPACE_BITS 36
> -#else
> -#define TARGET_PHYS_ADDR_SPACE_BITS 32
> -#endif
> -
>  static TranslationBlock *tbs;
>  int code_gen_max_blocks;
>  TranslationBlock *tb_phys_hash[CODE_GEN_PHYS_HASH_SIZE];
> diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
> index 617f55c..8afe16d 100644
> --- a/target-alpha/cpu.h
> +++ b/target-alpha/cpu.h
> @@ -41,7 +41,9 @@
>  
>  #define TARGET_PAGE_BITS 13
>  
> -#define VA_BITS 43
> +/* ??? 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)
>  
>  /* Alpha major type */
>  enum {
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 4a1c53f..3892db4 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -405,6 +405,9 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
>  #define TARGET_PAGE_BITS 10
>  #endif
>  
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +
>  #define cpu_init cpu_arm_init
>  #define cpu_exec cpu_arm_exec
>  #define cpu_gen_code cpu_arm_gen_code
> diff --git a/target-cris/cpu.h b/target-cris/cpu.h
> index 8ff86d9..063a240 100644
> --- a/target-cris/cpu.h
> +++ b/target-cris/cpu.h
> @@ -200,6 +200,9 @@ enum {
>  #define TARGET_PAGE_BITS 13
>  #define MMAP_SHIFT TARGET_PAGE_BITS
>  
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +
>  #define cpu_init cpu_cris_init
>  #define cpu_exec cpu_cris_exec
>  #define cpu_gen_code cpu_cris_gen_code
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index ef7d951..a9dab6f 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -874,6 +874,17 @@ uint64_t cpu_get_tsc(CPUX86State *env);
>  
>  #define TARGET_PAGE_BITS 12
>  
> +#ifdef TARGET_X86_64
> +#define TARGET_PHYS_ADDR_SPACE_BITS 52
> +/* ??? This is really 48 bits, sign-extended, but the only thing 
> +   accessible to userland with bit 48 set is the VSYSCALL, and that
> +   is handled via other mechanisms.  */
> +#define TARGET_VIRT_ADDR_SPACE_BITS 47
> +#else
> +#define TARGET_PHYS_ADDR_SPACE_BITS 36
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +#endif
> +
>  #define cpu_init cpu_x86_init
>  #define cpu_exec cpu_x86_exec
>  #define cpu_gen_code cpu_x86_gen_code
> diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
> index 68a7e41..b2f37ec 100644
> --- a/target-m68k/cpu.h
> +++ b/target-m68k/cpu.h
> @@ -210,6 +210,9 @@ void register_m68k_insns (CPUM68KState *env);
>  #define TARGET_PAGE_BITS 10
>  #endif
>  
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +
>  #define cpu_init cpu_m68k_init
>  #define cpu_exec cpu_m68k_exec
>  #define cpu_gen_code cpu_m68k_gen_code
> diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
> index 1bf4875..5999386 100644
> --- a/target-microblaze/cpu.h
> +++ b/target-microblaze/cpu.h
> @@ -253,6 +253,9 @@ enum {
>  #define TARGET_PAGE_BITS 12
>  #define MMAP_SHIFT TARGET_PAGE_BITS
>  
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +
>  #define cpu_init cpu_mb_init
>  #define cpu_exec cpu_mb_exec
>  #define cpu_gen_code cpu_mb_gen_code
> diff --git a/target-mips/mips-defs.h b/target-mips/mips-defs.h
> index 54e80f1..0f6a956 100644
> --- a/target-mips/mips-defs.h
> +++ b/target-mips/mips-defs.h
> @@ -8,6 +8,10 @@
>  #define TARGET_PAGE_BITS 12
>  #define MIPS_TLB_MAX 128
>  
> +/* ??? MIPS64 no doubt has a larger address space.  */
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +

In 32-bit mode, TARGET_VIRT_ADDR_SPACE_BITS is 32 bits, and
TARGET_PHYS_ADDR_SPACE_BITS is 36 bits.

In 64-bit mode, TARGET_VIRT_ADDR_SPACE_BITS is 42 bits, and
TARGET_PHYS_ADDR_SPACE_BITS is 36 bits for the CPUs emulated by QEMU,
though the architecture specification limits those value to respectively
62 and 59 bits.

>  #if defined(TARGET_MIPS64)
>  #define TARGET_LONG_BITS 64
>  #else
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 63aeb86..2550186 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -29,6 +29,20 @@
>  #define TARGET_LONG_BITS 64
>  #define TARGET_PAGE_BITS 12
>  
> +/* Note that the official physical address space bits is 62-M where M
> +   is implementation dependent.  I've not looked up M for the set of
> +   cpus we emulate at the system level.  */
> +#define TARGET_PHYS_ADDR_SPACE_BITS 62
> + 
> +/* Note that the PPC environment architecture talks about 80 bit virtual
> +   addresses, with segmentation.  Obviously that's not all visible to a
> +   single process, which is all we're concerned with here.  */
> +#ifdef TARGET_ABI32
> +# define TARGET_VIRT_ADDR_SPACE_BITS 32
> +#else
> +# define TARGET_VIRT_ADDR_SPACE_BITS 64
> +#endif
> +
>  #else /* defined (TARGET_PPC64) */
>  /* PowerPC 32 definitions */
>  #define TARGET_LONG_BITS 32
> @@ -50,6 +64,9 @@
>  #define TARGET_PAGE_BITS 12
>  #endif /* defined(TARGET_PPCEMB) */
>  
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +
>  #endif /* defined (TARGET_PPC64) */
>  
>  #define CPUState struct CPUPPCState
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 827f4e3..dd407b2 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -99,6 +99,11 @@ int cpu_s390x_handle_mmu_fault (CPUS390XState *env, target_ulong address, int rw
>  
>  #define TARGET_PAGE_BITS 12
>  
> +/* ??? This is certainly wrong for 64-bit s390x, but given that only KVM
> +   emulation actually works, this is good enough for a placeholder.  */
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +
>  #ifndef CONFIG_USER_ONLY
>  extern int s390_virtio_hypercall(CPUState *env);
>  extern void kvm_s390_virtio_irq(CPUState *env, int config_change, uint64_t token);
> diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
> index 85f221d..18a5532 100644
> --- a/target-sh4/cpu.h
> +++ b/target-sh4/cpu.h
> @@ -44,6 +44,9 @@
>  
>  #define TARGET_PAGE_BITS 12	/* 4k XXXXX */
>  
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +
>  #define SR_MD (1 << 30)
>  #define SR_RB (1 << 29)
>  #define SR_BL (1 << 28)
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 5980deb..0c5a7ef 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -7,10 +7,18 @@
>  #define TARGET_LONG_BITS 32
>  #define TARGET_FPREGS 32
>  #define TARGET_PAGE_BITS 12 /* 4k */
> +#define TARGET_PHYS_ADDR_SPACE_BITS 41
> +# ifdef TARGET_ABI32
> +#  define TARGET_VIRT_ADDR_SPACE_BITS 32
> +# else
> +#  define TARGET_VIRT_ADDR_SPACE_BITS 44
> +# endif
>  #else
>  #define TARGET_LONG_BITS 64
>  #define TARGET_FPREGS 64
>  #define TARGET_PAGE_BITS 13 /* 8k */
> +#define TARGET_PHYS_ADDR_SPACE_BITS 36
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
>  #endif
>  
>  #define CPUState struct CPUSPARCState
> -- 
> 1.6.6.1
> 
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.

[Richard Henderson]

On 03/11/2010 03:11 AM, Aurelien Jarno wrote:
>> +/* ??? MIPS64 no doubt has a larger address space.  */
>> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
>> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
>> +
> 
> In 32-bit mode, TARGET_VIRT_ADDR_SPACE_BITS is 32 bits, and
> TARGET_PHYS_ADDR_SPACE_BITS is 36 bits.
> 
> In 64-bit mode, TARGET_VIRT_ADDR_SPACE_BITS is 42 bits, and
> TARGET_PHYS_ADDR_SPACE_BITS is 36 bits for the CPUs emulated by QEMU,
> though the architecture specification limits those value to respectively
> 62 and 59 bits.

Thanks.  I'll queue that either for a subsequent round or a 
follow-up patch (since the current tree doesn't use anything
but the 32 above).


r~

Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Paul Brook]

> Richard Henderson (6):
>   Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
>   Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid.
>   linux-user: Use h2g_valid in qemu_vmalloc.
>   linux-user: Fix mmap_find_vma returning invalid addresses.
>   Implement multi-level page tables.
>   Fix last page errors in page_check_range and page_set_flags.

Applied, plus a bunch of follow-on patches that isolate the userspace 
emulation from any concept of physical address space.

Paul

Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Blue Swirl]

On 3/12/10, Paul Brook <paul@codesourcery.com> wrote:
> > Richard Henderson (6):
>  >   Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
>  >   Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid.
>  >   linux-user: Use h2g_valid in qemu_vmalloc.
>  >   linux-user: Fix mmap_find_vma returning invalid addresses.
>  >   Implement multi-level page tables.
>  >   Fix last page errors in page_check_range and page_set_flags.
>
>
> Applied, plus a bunch of follow-on patches that isolate the userspace
>  emulation from any concept of physical address space.

Now I get these warnings on OpenBSD/Sparc64:
  CC    sparc-bsd-user/exec.o
/src/qemu/exec.c: In function `page_set_flags':
/src/qemu/exec.c:2336: warning: comparison is always true due to
limited range of data type
/src/qemu/exec.c: In function `page_check_range':
/src/qemu/exec.c:2373: warning: comparison is always true due to
limited range of data type

Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Paul Brook]

> /src/qemu/exec.c: In function `page_set_flags':
> /src/qemu/exec.c:2336: warning: comparison is always true due to
> limited range of data type

Should be fixed by b480d9b74dfd1efd29026b7dc6438364a633ee99

Paul

Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Aurelien Jarno]

On Fri, Mar 12, 2010 at 06:51:41PM +0000, Paul Brook wrote:
> > Richard Henderson (6):
> >   Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
> >   Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid.
> >   linux-user: Use h2g_valid in qemu_vmalloc.
> >   linux-user: Fix mmap_find_vma returning invalid addresses.
> >   Implement multi-level page tables.
> >   Fix last page errors in page_check_range and page_set_flags.
> 
> Applied, plus a bunch of follow-on patches that isolate the userspace 
> emulation from any concept of physical address space.
> 

The patch "Implement multi-level page tables" breaks i386/x86_64 system
emulation. Using the image on [1], qemu now segfaults during the initrd
parts. This is not 100% reproducible, a few times it hanged instead of
crashing.

[1] http://people.debian.org/~aurel32/qemu/i386/debian_lenny_i386_small.qcow2

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Martin Mohring]

I am sorry to bring us all down to earth, we all had wished that this stuff gets finally fixed, but it seems that those patches applied to QEMU master have killed QEMU user mode.

I am referring to the user mode fixes after commit 0aef4261ac0ec9089ade0e3a92f986cb4ba7317e

I had QEMU working on PowerPC and ARM in chroots from the Linux versions:

- Fedora 11 / 12
- Ubuntu 9.04, 9.10, 10.04
- Debian 4, 5 and Sid

for ARM and PowerPC. My host OS is openSUSE 11.2 using a 2.6.31 kernel, or openSUSE 11.1. All machines are 64 Bit machines.

Now I have the situation that all the listed target chroots fail, except:
- Fedora 11 / 12 @ ARM

As you might guess (or even know), I am one of the Open Build Service (http://en.opensuse.org/Build_Service/Testing) developers and can automatedly setup and test such a big number of Targets and automatically run testsuites or compile jobs on them. The last fixes in the QEMU user mode memory allocator before your rewrite came from Jan-Simon and me.

I think its long overdue to fix the user mode stuff, so I am happy that now someone that knows more about QEMU than me cares about. SO my offering is to help to track this down and fix it. But I would be happy if you give me a hint about what your changes are really doing to track down the problem.

Aurelien Jarno wrote:

> On Fri, Mar 12, 2010 at 06:51:41PM +0000, Paul Brook wrote:
>   
>>> Richard Henderson (6):
>>>   Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
>>>   Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid.
>>>   linux-user: Use h2g_valid in qemu_vmalloc.
>>>   linux-user: Fix mmap_find_vma returning invalid addresses.
>>>   Implement multi-level page tables.
>>>   Fix last page errors in page_check_range and page_set_flags.
>>>       
>> Applied, plus a bunch of follow-on patches that isolate the userspace 
>> emulation from any concept of physical address space.
>>
>>     
>
> The patch "Implement multi-level page tables" breaks i386/x86_64 system
> emulation. Using the image on [1], qemu now segfaults during the initrd
> parts. This is not 100% reproducible, a few times it hanged instead of
> crashing.
>
> [1] http://people.debian.org/~aurel32/qemu/i386/debian_lenny_i386_small.qcow2
>
>   

Re: [Qemu-devel] [PATCH 5/6] Implement multi-level page tables.

[Stefan Weil]

Richard Henderson schrieb:
> Define L1_MAP_ADDR_SPACE_BITS to be either the virtual address size
> (in user mode) or physical address size (in system mode), and use
> that to size l1_map.  This rewrites page_find_alloc, page_flush_tb,
> and walk_memory_regions.
>
> Use TARGET_PHYS_ADDR_SPACE_BITS for the physical memory map based
> off of l1_phys_map.  This rewrites page_phys_find_alloc and
> phys_page_for_each.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  cpu-all.h |    7 +-
>  exec.c    |  451 +++++++++++++++++++++++++++++++++++++------------------------
>  2 files changed, 278 insertions(+), 180 deletions(-)

This patch breaks remote debugging (QEMU crash).

Test scenario:

x86_64 debian host, mips malta (32/64 bit, big/little endian) target

start qemu system emulation with a malta kernel and options -s -S,
attach remote debugger, start mips kernel with "c" in debugger =>
qemu crash:

page_flush_tb is filled with zero in page_flush_tb
tb_phys_invalidate is called, access fault in tb_remove.

Other targets might be affected, too.

Regards,
Stefan Weil

Re: [Qemu-devel] [PATCH 5/6] Implement multi-level page tables.

[Paul Brook]

> Richard Henderson schrieb:
> > Define L1_MAP_ADDR_SPACE_BITS to be either the virtual address size
> > (in user mode) or physical address size (in system mode), and use
> > that to size l1_map.  This rewrites page_find_alloc, page_flush_tb,
> > and walk_memory_regions.
> >
> > Use TARGET_PHYS_ADDR_SPACE_BITS for the physical memory map based
> > off of l1_phys_map.  This rewrites page_phys_find_alloc and
> > phys_page_for_each.
> >
> > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > ---
> >  cpu-all.h |    7 +-
> >  exec.c    |  451
> > +++++++++++++++++++++++++++++++++++++------------------------ 2 files
> > changed, 278 insertions(+), 180 deletions(-)
> 
> This patch breaks remote debugging (QEMU crash).

Probably nothing to do with debugging.  Should be fixed by 
7296abaccc98872e28cec50091dbf26d38e4f062

Paul

Re: [Qemu-devel] [PATCH 5/6] Implement multi-level page tables.

[Aurelien Jarno]

On Sun, Mar 14, 2010 at 03:02:11PM +0000, Paul Brook wrote:
> > Richard Henderson schrieb:
> > > Define L1_MAP_ADDR_SPACE_BITS to be either the virtual address size
> > > (in user mode) or physical address size (in system mode), and use
> > > that to size l1_map.  This rewrites page_find_alloc, page_flush_tb,
> > > and walk_memory_regions.
> > >
> > > Use TARGET_PHYS_ADDR_SPACE_BITS for the physical memory map based
> > > off of l1_phys_map.  This rewrites page_phys_find_alloc and
> > > phys_page_for_each.
> > >
> > > Signed-off-by: Richard Henderson <rth@twiddle.net>
> > > ---
> > >  cpu-all.h |    7 +-
> > >  exec.c    |  451
> > > +++++++++++++++++++++++++++++++++++++------------------------ 2 files
> > > changed, 278 insertions(+), 180 deletions(-)
> > 
> > This patch breaks remote debugging (QEMU crash).
> 
> Probably nothing to do with debugging.  Should be fixed by 
> 7296abaccc98872e28cec50091dbf26d38e4f062
> 

Thanks it also fixes the problem I have reported.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Riku Voipio]

On Sat, Mar 13, 2010 at 03:00:52PM +0100, Martin Mohring wrote:
> I am sorry to bring us all down to earth, we all had wished that this stuff gets finally fixed, but it seems that those patches applied to QEMU master have killed QEMU user mode.
 
sid and maemo userlands work for me like before (64bit and 32bit hosts).

> I am referring to the user mode fixes after commit 0aef4261ac0ec9089ade0e3a92f986cb4ba7317e
> 
> I had QEMU working on PowerPC and ARM in chroots from the Linux versions:
> 
> - Fedora 11 / 12
> - Ubuntu 9.04, 9.10, 10.04
> - Debian 4, 5 and Sid
> 
> for ARM and PowerPC. My host OS is openSUSE 11.2 using a 2.6.31 kernel, or openSUSE 11.1. All machines are 64 Bit machines.
> 
> Now I have the situation that all the listed target chroots fail, except:
> - Fedora 11 / 12 @ ARM

Please try to be more specific. What is killed? what do you mean by 
"chroots fail". qemu segfault? some binary doesn't work that before did?
some syscall doesn't work anymore?

Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Jan-Simon Möller]

> > I had QEMU working on PowerPC and ARM in chroots from the Linux versions:
> >
> > - Fedora 11 / 12
> > - Ubuntu 9.04, 9.10, 10.04
> > - Debian 4, 5 and Sid
> >
> > for ARM and PowerPC. My host OS is openSUSE 11.2 using a 2.6.31 kernel,
> > or openSUSE 11.1. All machines are 64 Bit machines.
> >
> > Now I have the situation that all the listed target chroots fail, except:
> > - Fedora 11 / 12 @ ARM
> 
> Please try to be more specific. What is killed? what do you mean by
> "chroots fail". qemu segfault? some binary doesn't work that before did?
> some syscall doesn't work anymore?
> 

We're still investigating the topic. So far, we tracked one failure down to
ldconfig.real which is a static arm binary executed after the chroot got the 
basic packages installed. If we setup a chroot with an older qemu-arm and 
exchange it afterwards with the new qemu and rerun just the build step, it 
works. Thus it seems to be an issue with static arm binaries atm.

Strace of a call of ldconfig.real with the qemu-arm failing:

http://pastie.org/870189

Sort version:

root@frodo:/# qemu-arm -strace /sbin/ldconfig.real                  
16359 uname(0x403fef78) = 0                                         
16359 brk(NULL) = 0x000a9000                                        
16359 brk(0x000a9d08) = 0x000a9d08                                  
16359 open("/dev/urandom",O_RDONLY) = 3                             
16359 read(3,0x403ff27d,3) = 3                                      
16359 close(3) = 0                                                  
[...]
16359 stat64("/usr/lib/libgettextlib.so",0x403fdf28) = 0
16359 stat64("/usr/lib/libgettextpo.so.0",0x403fdec0) = 0
16359 stat64("/usr/lib/libgettextpo.so.0.4.0",0x403fdf28) = 0
16359 stat64("/usr/lib/libpython2.6.so.1.0",0x403fdec0) = 0
16359 stat64("/usr/lib/libpython2.6.so.1.0",0x403fdf28) = 0
16359 open("/etc/ld.so.cache~",O_WRONLY|O_CREAT|O_NOFOLLOW|O_TRUNC,0600) = 3
16359 write(3,0xb03d0,1288) = 1288
16359 write(3,0x403ff0a0,0) = -1 errno=14 (Bad address)
16359 write(2,0x403fca08,21)/sbin/ldconfig.real:  = 21
16359 write(2,0x403fc9e8,28)Writing of cache data failed = 28
16359 write(2,0x403fc5b8,13): Bad address = 13
16359 write(2,0x403fc9c0,1)
 = 1
16359 exit_group(1)

Best,
Jan-Simon

Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Riku Voipio]

On Mon, Mar 15, 2010 at 01:46:10PM +0100, Jan-Simon Möller wrote:
> We're still investigating the topic. So far, we tracked one failure down to
> ldconfig.real which is a static arm binary executed after the chroot got the 
> basic packages installed. If we setup a chroot with an older qemu-arm and 
> exchange it afterwards with the new qemu and rerun just the build step, it 
> works. Thus it seems to be an issue with static arm binaries atm.
> 
> Strace of a call of ldconfig.real with the qemu-arm failing:
> 
> http://pastie.org/870189
> 
> Sort version:
> 
> root@frodo:/# qemu-arm -strace /sbin/ldconfig.real                  
> 16359 uname(0x403fef78) = 0                                         
> 16359 brk(NULL) = 0x000a9000                                        
> 16359 brk(0x000a9d08) = 0x000a9d08                                  
> 16359 open("/dev/urandom",O_RDONLY) = 3                             
> 16359 read(3,0x403ff27d,3) = 3                                      
> 16359 close(3) = 0                                                  
> [...]
> 16359 stat64("/usr/lib/libgettextlib.so",0x403fdf28) = 0
> 16359 stat64("/usr/lib/libgettextpo.so.0",0x403fdec0) = 0
> 16359 stat64("/usr/lib/libgettextpo.so.0.4.0",0x403fdf28) = 0
> 16359 stat64("/usr/lib/libpython2.6.so.1.0",0x403fdec0) = 0
> 16359 stat64("/usr/lib/libpython2.6.so.1.0",0x403fdf28) = 0
> 16359 open("/etc/ld.so.cache~",O_WRONLY|O_CREAT|O_NOFOLLOW|O_TRUNC,0600) = 3
> 16359 write(3,0xb03d0,1288) = 1288
> 16359 write(3,0x403ff0a0,0) = -1 errno=14 (Bad address)

A zero sized write. According to manpage ok. 

In qemu we do a lock_user to to get the string to write. Richards change changes
the access checks the get called by lock_user:

page_check_range:

-    if (start + len < start)
-        /* we've wrapped around */
...
+    if (start + len - 1 < start) {
+        /* We've wrapped around.  */

This now blows up with len = 0;

> 16359 write(2,0x403fca08,21)/sbin/ldconfig.real:  = 21
> 16359 write(2,0x403fc9e8,28)Writing of cache data failed = 28
> 16359 write(2,0x403fc5b8,13): Bad address = 13
> 16359 write(2,0x403fc9c0,1)
>  = 1
> 16359 exit_group(1)
> 
> Best,
> Jan-Simon
> 

Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Jan-Simon Möller]

Am Montag, 15. März 2010 15:48:03 schrieb Riku Voipio:
> On Mon, Mar 15, 2010 at 01:46:10PM +0100, Jan-Simon Möller wrote:
> > We're still investigating the topic. So far, we tracked one failure down
> > to ldconfig.real which is a static arm binary executed after the chroot
> > got the basic packages installed. If we setup a chroot with an older
> > qemu-arm and exchange it afterwards with the new qemu and rerun just the
> > build step, it works. Thus it seems to be an issue with static arm
> > binaries atm.
> >
> > Strace of a call of ldconfig.real with the qemu-arm failing:
> >
> > http://pastie.org/870189
> >
> > Sort version:
> >
> > root@frodo:/# qemu-arm -strace /sbin/ldconfig.real
> > 16359 uname(0x403fef78) = 0
> > 16359 brk(NULL) = 0x000a9000
> > 16359 brk(0x000a9d08) = 0x000a9d08
> > 16359 open("/dev/urandom",O_RDONLY) = 3
> > 16359 read(3,0x403ff27d,3) = 3
> > 16359 close(3) = 0
> > [...]
> > 16359 stat64("/usr/lib/libgettextlib.so",0x403fdf28) = 0
> > 16359 stat64("/usr/lib/libgettextpo.so.0",0x403fdec0) = 0
> > 16359 stat64("/usr/lib/libgettextpo.so.0.4.0",0x403fdf28) = 0
> > 16359 stat64("/usr/lib/libpython2.6.so.1.0",0x403fdec0) = 0
> > 16359 stat64("/usr/lib/libpython2.6.so.1.0",0x403fdf28) = 0
> > 16359 open("/etc/ld.so.cache~",O_WRONLY|O_CREAT|O_NOFOLLOW|O_TRUNC,0600)
> > = 3 16359 write(3,0xb03d0,1288) = 1288
> > 16359 write(3,0x403ff0a0,0) = -1 errno=14 (Bad address)
> 
> A zero sized write. According to manpage ok.
> 
> In qemu we do a lock_user to to get the string to write. Richards change
>  changes the access checks the get called by lock_user:
> 
> page_check_range:
> 
> -    if (start + len < start)
> -        /* we've wrapped around */
> ...
> +    if (start + len - 1 < start) {
> +        /* We've wrapped around.  */
> 
> This now blows up with len = 0;

Confirmed. A quick test with  if (len > 0) around and ldconfig.real runs.

Best,
Jan-Simon

Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Riku Voipio]

On Mon, Mar 15, 2010 at 04:08:46PM +0100, Jan-Simon Möller wrote:
> Am Montag, 15. März 2010 15:48:03 schrieb Riku Voipio:
> > On Mon, Mar 15, 2010 at 01:46:10PM +0100, Jan-Simon Möller wrote:
> > > root@frodo:/# qemu-arm -strace /sbin/ldconfig.real
> > > 16359 uname(0x403fef78) = 0
> > > 16359 brk(NULL) = 0x000a9000
> > > 16359 brk(0x000a9d08) = 0x000a9d08
> > > 16359 open("/dev/urandom",O_RDONLY) = 3
> > > 16359 read(3,0x403ff27d,3) = 3
> > > 16359 close(3) = 0
> > > [...]
> > > 16359 stat64("/usr/lib/libgettextlib.so",0x403fdf28) = 0
> > > 16359 stat64("/usr/lib/libgettextpo.so.0",0x403fdec0) = 0
> > > 16359 stat64("/usr/lib/libgettextpo.so.0.4.0",0x403fdf28) = 0
> > > 16359 stat64("/usr/lib/libpython2.6.so.1.0",0x403fdec0) = 0
> > > 16359 stat64("/usr/lib/libpython2.6.so.1.0",0x403fdf28) = 0
> > > 16359 open("/etc/ld.so.cache~",O_WRONLY|O_CREAT|O_NOFOLLOW|O_TRUNC,0600)
> > > = 3 16359 write(3,0xb03d0,1288) = 1288
> > > 16359 write(3,0x403ff0a0,0) = -1 errno=14 (Bad address)
 
> > A zero sized write. According to manpage ok.
 
> > In qemu we do a lock_user to to get the string to write. Richards change
> >  changes the access checks the get called by lock_user:

> > page_check_range:
> > 
> > -    if (start + len < start)
> > -        /* we've wrapped around */
> > ...
> > +    if (start + len - 1 < start) {
> > +        /* We've wrapped around.  */
> > 
> > This now blows up with len = 0;

> Confirmed. A quick test with  if (len > 0) around and ldconfig.real runs.

Richard, would you be ok with that change? Or should we rather have explicit 

if (len == 0) return 0;

?

Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3

[Richard Henderson]

On 03/17/2010 04:48 AM, Riku Voipio wrote:
>>> page_check_range:
>>>
>>> -    if (start + len < start)
>>> -        /* we've wrapped around */
>>> ...
>>> +    if (start + len - 1 < start) {
>>> +        /* We've wrapped around.  */
>>>
>>> This now blows up with len = 0;
> 
>> Confirmed. A quick test with  if (len > 0) around and ldconfig.real runs.
> 
> Richard, would you be ok with that change? Or should we rather have explicit 
> 
> if (len == 0) return 0;

My personal preference is 

  if (len != 0 && start + len - 1 < start)

but don't let that stop anyone checking in any correct variant.


r~