qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
  2010-02-12  0:15 [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Richard Henderson
@ 2010-02-11 22:20 ` Richard Henderson
  2010-02-12 20:01   ` Blue Swirl
  2010-02-11 22:47 ` [Qemu-devel] [PATCH 2/6] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid Richard Henderson
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2010-02-11 22:20 UTC (permalink / raw)
  To: qemu-devel

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.
---
 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 8389c54..766568b 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 c0dff4b..c144b4b 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 0626cd8..26171ca 100644
--- a/target-cris/cpu.h
+++ b/target-cris/cpu.h
@@ -195,6 +195,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 216b00e..7fb84db 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -872,6 +872,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 d15bba1..c91f8fe 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 0e75e1c..56fc083 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -100,6 +100,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..a9d61f6 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
+
+/* ??? This is a guess based on PAGE_OFFSET in the Linux kernel.  */
+#define TARGET_VIRT_ADDR_SPACE_BITS 41
+
 #else
 #define TARGET_LONG_BITS 64
 #define TARGET_FPREGS 64
 #define TARGET_PAGE_BITS 13 /* 8k */
+#define TARGET_PHYS_ADDR_SPACE_BITS 32
+#define TARGET_VIRT_ADDR_SPACE_BITS 32
 #endif
 
 #define CPUState struct CPUSPARCState
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 2/6] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid.
  2010-02-12  0:15 [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Richard Henderson
  2010-02-11 22:20 ` [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h Richard Henderson
@ 2010-02-11 22:47 ` Richard Henderson
  2010-02-11 22:57 ` [Qemu-devel] [PATCH 3/6] Fix last page errors in page_set_flags and page_check_range Richard Henderson
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-11 22:47 UTC (permalink / raw)
  To: qemu-devel

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.
---
 cpu-all.h |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 1ccc9a8..b81641f 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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 2/7] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid.
  2010-02-15 20:59 ` [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2 Richard Henderson
@ 2010-02-11 22:47   ` Richard Henderson
  2010-02-28 14:11     ` Paul Brook
  2010-02-11 23:11   ` [Qemu-devel] [PATCH 5/7] linux-user: Use h2g_valid in qemu_vmalloc Richard Henderson
                     ` (5 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2010-02-11 22:47 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel

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.
---
 cpu-all.h |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 1ccc9a8..b81641f 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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 3/6] Fix last page errors in page_set_flags and page_check_range.
  2010-02-12  0:15 [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Richard Henderson
  2010-02-11 22:20 ` [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h Richard Henderson
  2010-02-11 22:47 ` [Qemu-devel] [PATCH 2/6] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid Richard Henderson
@ 2010-02-11 22:57 ` Richard Henderson
  2010-02-12 19:47   ` Blue Swirl
  2010-02-11 23:11 ` [Qemu-devel] [PATCH 5/6] linux-user: Use h2g_valid in qemu_vmalloc Richard Henderson
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2010-02-11 22:57 UTC (permalink / raw)
  To: qemu-devel

The addr < end comparison prevents the last page from being
iterated; an iteration based on length avoids this problem.
---
 exec.c |   54 +++++++++++++++++++++++++++---------------------------
 1 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/exec.c b/exec.c
index 766568b..ebbe6d0 100644
--- a/exec.c
+++ b/exec.c
@@ -2222,35 +2222,31 @@ void page_dump(FILE *f)
 
 int page_get_flags(target_ulong address)
 {
-    PageDesc *p;
-
-    p = page_find(address >> TARGET_PAGE_BITS);
-    if (!p)
-        return 0;
-    return p->flags;
+    PageDesc *p = page_find(address >> TARGET_PAGE_BITS);
+    return p ? p->flags : 0;
 }
 
-/* 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;
 
-    /* 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) {
@@ -2266,18 +2262,22 @@ 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 */
+    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 */
+    /* END must be computed before we drop bits from START.  */
+    end = TARGET_PAGE_ALIGN(start + len);
     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 )
+        if (!p)
             return -1;
-        if( !(p->flags & PAGE_VALID) )
+        if (!(p->flags & PAGE_VALID))
             return -1;
 
         if ((flags & PAGE_READ) && !(p->flags & PAGE_READ))
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 5/6] linux-user: Use h2g_valid in qemu_vmalloc.
  2010-02-12  0:15 [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Richard Henderson
                   ` (2 preceding siblings ...)
  2010-02-11 22:57 ` [Qemu-devel] [PATCH 3/6] Fix last page errors in page_set_flags and page_check_range Richard Henderson
@ 2010-02-11 23:11 ` Richard Henderson
  2010-02-11 23:29 ` [Qemu-devel] [PATCH 6/6] linux-user: Fix mmap_find_vma returning invalid addresses Richard Henderson
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-11 23:11 UTC (permalink / raw)
  To: qemu-devel

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 5/7] linux-user: Use h2g_valid in qemu_vmalloc.
  2010-02-15 20:59 ` [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2 Richard Henderson
  2010-02-11 22:47   ` [Qemu-devel] [PATCH 2/7] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid Richard Henderson
@ 2010-02-11 23:11   ` Richard Henderson
  2010-02-11 23:29   ` [Qemu-devel] [PATCH 6/7] linux-user: Fix mmap_find_vma returning invalid addresses Richard Henderson
                     ` (4 subsequent siblings)
  6 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-11 23:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 6/6] linux-user: Fix mmap_find_vma returning invalid addresses.
  2010-02-12  0:15 [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Richard Henderson
                   ` (3 preceding siblings ...)
  2010-02-11 23:11 ` [Qemu-devel] [PATCH 5/6] linux-user: Use h2g_valid in qemu_vmalloc Richard Henderson
@ 2010-02-11 23:29 ` Richard Henderson
  2010-02-11 23:51 ` [Qemu-devel] [PATCH 4/6] Implement multi-level page tables Richard Henderson
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-11 23:29 UTC (permalink / raw)
  To: qemu-devel

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.
---
 linux-user/main.c |    7 +---
 linux-user/mmap.c |  102 ++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 80 insertions(+), 29 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index a0d8ce7..7db9fc3 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2725,12 +2725,9 @@ int main(int argc, char **argv, char **envp)
     /*
      * 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) {
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 */
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 6/7] linux-user: Fix mmap_find_vma returning invalid addresses.
  2010-02-15 20:59 ` [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2 Richard Henderson
  2010-02-11 22:47   ` [Qemu-devel] [PATCH 2/7] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid Richard Henderson
  2010-02-11 23:11   ` [Qemu-devel] [PATCH 5/7] linux-user: Use h2g_valid in qemu_vmalloc Richard Henderson
@ 2010-02-11 23:29   ` Richard Henderson
  2010-02-11 23:51   ` [Qemu-devel] [PATCH 4/7] Implement multi-level page tables Richard Henderson
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-11 23:29 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel

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.
---
 linux-user/main.c |    7 +---
 linux-user/mmap.c |  102 ++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 80 insertions(+), 29 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index a0d8ce7..7db9fc3 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2725,12 +2725,9 @@ int main(int argc, char **argv, char **envp)
     /*
      * 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) {
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 */
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 4/6] Implement multi-level page tables.
  2010-02-12  0:15 [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Richard Henderson
                   ` (4 preceding siblings ...)
  2010-02-11 23:29 ` [Qemu-devel] [PATCH 6/6] linux-user: Fix mmap_find_vma returning invalid addresses Richard Henderson
@ 2010-02-11 23:51 ` Richard Henderson
  2010-02-15 20:59 ` [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2 Richard Henderson
  2010-02-28 23:23 ` [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Paul Brook
  7 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-11 23:51 UTC (permalink / raw)
  To: qemu-devel

Use TARGET_VIRT_ADDR_SPACE_BITS for the virtual memory map based off
of 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.
---
 cpu-all.h |    7 +-
 exec.c    |  442 +++++++++++++++++++++++++++++++++++++------------------------
 2 files changed, 271 insertions(+), 178 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index b81641f..510f0b4 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 ebbe6d0..14f15a6 100644
--- a/exec.c
+++ b/exec.c
@@ -141,28 +141,47 @@ typedef struct PhysPageDesc {
     ram_addr_t region_offset;
 } PhysPageDesc;
 
+/* Size of the L2 (and L3, etc) page tables.  */
 #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)
+#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 \
+    ((TARGET_VIRT_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 L1_BITS (32 - L2_BITS - TARGET_PAGE_BITS)
+#define P_L1_BITS  P_L1_BITS_REM
 #endif
 
-#define L1_SIZE (1 << L1_BITS)
-#define L2_SIZE (1 << L2_BITS)
+#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 (TARGET_VIRT_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];
-static PhysPageDesc **l1_phys_map;
+/* 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];
+
+/* 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];
 
 #if !defined(CONFIG_USER_ONLY)
 static void io_mem_init(void);
@@ -247,130 +266,158 @@ 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);
-    l1_phys_map = qemu_vmalloc(L1_SIZE * sizeof(void *));
-    memset(l1_phys_map, 0, L1_SIZE * sizeof(void *));
 
 #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));
+    }
+
+    pd = *lp;
+    if (pd == NULL) {
+        if (!alloc) {
+            return NULL;
+        }
+        ALLOC(pd, sizeof(PageDesc) * L2_SIZE);
+        *lp = pd;
     }
-    return p + (index & (L2_SIZE - 1));
+
+#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);
 }
 
 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)
@@ -596,23 +643,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 */
@@ -1104,7 +1164,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);
@@ -1644,50 +1704,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)
@@ -2158,44 +2205,87 @@ int tlb_set_page_exec(CPUState *env, target_ulong vaddr,
  * 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;
+
+    if (*lp == NULL) {
+        return walk_memory_regions_end(data, base, 0);
+    }
 
-    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 (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 (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 walk_memory_regions_end(&data, 0, 0);
 }
 
 static int dump_region(void *priv, unsigned long start,
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 4/7] Implement multi-level page tables.
  2010-02-15 20:59 ` [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2 Richard Henderson
                     ` (2 preceding siblings ...)
  2010-02-11 23:29   ` [Qemu-devel] [PATCH 6/7] linux-user: Fix mmap_find_vma returning invalid addresses Richard Henderson
@ 2010-02-11 23:51   ` Richard Henderson
  2010-02-12 21:38   ` [Qemu-devel] [PATCH 1/7] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h Richard Henderson
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-11 23:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel

Use TARGET_VIRT_ADDR_SPACE_BITS for the virtual memory map based off
of 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.
---
 cpu-all.h |    7 +-
 exec.c    |  442 +++++++++++++++++++++++++++++++++++++------------------------
 2 files changed, 271 insertions(+), 178 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index b81641f..510f0b4 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 1521ce3..bb712ec 100644
--- a/exec.c
+++ b/exec.c
@@ -141,28 +141,47 @@ typedef struct PhysPageDesc {
     ram_addr_t region_offset;
 } PhysPageDesc;
 
+/* Size of the L2 (and L3, etc) page tables.  */
 #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)
+#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 \
+    ((TARGET_VIRT_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 L1_BITS (32 - L2_BITS - TARGET_PAGE_BITS)
+#define P_L1_BITS  P_L1_BITS_REM
 #endif
 
-#define L1_SIZE (1 << L1_BITS)
-#define L2_SIZE (1 << L2_BITS)
+#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 (TARGET_VIRT_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];
-static PhysPageDesc **l1_phys_map;
+/* 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];
+
+/* 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];
 
 #if !defined(CONFIG_USER_ONLY)
 static void io_mem_init(void);
@@ -247,130 +266,158 @@ 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);
-    l1_phys_map = qemu_vmalloc(L1_SIZE * sizeof(void *));
-    memset(l1_phys_map, 0, L1_SIZE * sizeof(void *));
 
 #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));
+    }
+
+    pd = *lp;
+    if (pd == NULL) {
+        if (!alloc) {
+            return NULL;
+        }
+        ALLOC(pd, sizeof(PageDesc) * L2_SIZE);
+        *lp = pd;
     }
-    return p + (index & (L2_SIZE - 1));
+
+#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);
 }
 
 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)
@@ -596,23 +643,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 */
@@ -1104,7 +1164,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);
@@ -1644,50 +1704,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)
@@ -2158,44 +2205,87 @@ int tlb_set_page_exec(CPUState *env, target_ulong vaddr,
  * 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;
+
+    if (*lp == NULL) {
+        return walk_memory_regions_end(data, base, 0);
+    }
 
-    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 (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 (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 walk_memory_regions_end(&data, 0, 0);
 }
 
 static int dump_region(void *priv, unsigned long start,
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes.
@ 2010-02-12  0:15 Richard Henderson
  2010-02-11 22:20 ` [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h Richard Henderson
                   ` (7 more replies)
  0 siblings, 8 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-12  0:15 UTC (permalink / raw)
  To: qemu-devel

I have previously posted a variant of part 6, to address the problem
of the host returning mmap results that are not page aligned for the
guest.  That, however, led me to the fact that we could also return
addresses that are outside the guest's virtual address space.

Which raises the question of what *is* the guest's virtual address
space?  For a 32-bit guest, clearly we cannot return anything outside
GUEST_BASE through GUEST_BASE+4G.  For a 64-bit guest, the question
is less clear.  One thing is certain: the guest's virtual address space
had better not be anything outside what page_find can support.

Which brings us to the problem of exec.c and the address spaces therein.
First, there was the fact that TARGET_PHYS_ADDR_SPACE_BITS was constrained
to be no larger than 32 (with a partial hack for Alpha to extend this to
42 bits).  Second, that this physical address space value was applied to
virtual addresses via page_find.

This patch series untangles this somewhat.

First, define separate physical and virtual address spaces for each cpu.
This allows the page tables used to be no deeper than necessary in order 
to support what the native hardware does.  E.g. 3 level page tables for
Alpha's 43-bit virtual address space, rather than the 5 levels required
for a full 64-bit space.  I've looked up proper values for x86_64 and 
ppc64; I couldn't find the correct values for mips64 and sparc64, so I
guessed.  Certainly the guess is no worse than what is supported by
the current exec.c values.

Second, implement the multi-level search within exec.c.  The form of
this multi-level search is taken from Tristan Gingold's es40 patches.
However, he only addressed the physical address space and ignored the
virtual; this patch handles both.  I tried to arrange things as 
readably as possible here; getting too clever here is a sure-fire
recipe for confusion.

Third, re-apply the mmap address fixes.  This time, as promised, with
a clear division between host and guest address space -- the last 
variant that I posted could return addresses below GUEST_BASE.



r~

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

 cpu-all.h               |   23 ++-
 exec.c                  |  513 +++++++++++++++++++++++++++--------------------
 linux-user/main.c       |    7 +-
 linux-user/mmap.c       |  111 ++++++++---
 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 +
 15 files changed, 456 insertions(+), 262 deletions(-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] Fix last page errors in page_set_flags and page_check_range.
  2010-02-11 22:57 ` [Qemu-devel] [PATCH 3/6] Fix last page errors in page_set_flags and page_check_range Richard Henderson
@ 2010-02-12 19:47   ` Blue Swirl
  2010-02-12 20:16     ` Richard Henderson
  0 siblings, 1 reply; 25+ messages in thread
From: Blue Swirl @ 2010-02-12 19:47 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Fri, Feb 12, 2010 at 12:57 AM, Richard Henderson <rth@twiddle.net> wrote:
> The addr < end comparison prevents the last page from being
> iterated; an iteration based on length avoids this problem.

Please make separate patches for unrelated changes. Now the essence of
the patch is very hard to see. Also pure formatting changes are not
very useful.

> ---
>  exec.c |   54 +++++++++++++++++++++++++++---------------------------
>  1 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 766568b..ebbe6d0 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2222,35 +2222,31 @@ void page_dump(FILE *f)
>
>  int page_get_flags(target_ulong address)
>  {
> -    PageDesc *p;
> -
> -    p = page_find(address >> TARGET_PAGE_BITS);
> -    if (!p)
> -        return 0;
> -    return p->flags;
> +    PageDesc *p = page_find(address >> TARGET_PAGE_BITS);
> +    return p ? p->flags : 0;
>  }

For example this change does not make any difference but just add confusion.

>
> -/* 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;
>
> -    /* 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.  */

Why remove the comment, is it no longer true?

> -        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) {
> @@ -2266,18 +2262,22 @@ 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 */
> +    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 */
> +    /* END must be computed before we drop bits from START.  */
> +    end = TARGET_PAGE_ALIGN(start + len);
>     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 )
> +        if (!p)
>             return -1;
> -        if( !(p->flags & PAGE_VALID) )
> +        if (!(p->flags & PAGE_VALID))
>             return -1;
>
>         if ((flags & PAGE_READ) && !(p->flags & PAGE_READ))
> --
> 1.6.6
>
>
>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
  2010-02-11 22:20 ` [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h Richard Henderson
@ 2010-02-12 20:01   ` Blue Swirl
  2010-02-12 20:25     ` Richard Henderson
  0 siblings, 1 reply; 25+ messages in thread
From: Blue Swirl @ 2010-02-12 20:01 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Fri, Feb 12, 2010 at 12:20 AM, Richard Henderson <rth@twiddle.net> 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.
> ---
>  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 8389c54..766568b 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 c0dff4b..c144b4b 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 0626cd8..26171ca 100644
> --- a/target-cris/cpu.h
> +++ b/target-cris/cpu.h
> @@ -195,6 +195,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 216b00e..7fb84db 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -872,6 +872,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 d15bba1..c91f8fe 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

I'd suppose this change applies to all targets with ABI32, not just PPC.

> +
>  #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 0e75e1c..56fc083 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -100,6 +100,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..a9d61f6 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
> +
> +/* ??? This is a guess based on PAGE_OFFSET in the Linux kernel.  */
> +#define TARGET_VIRT_ADDR_SPACE_BITS 41

No need to guess, the value for UltraSparc IIi is 44.

> +
>  #else
>  #define TARGET_LONG_BITS 64
>  #define TARGET_FPREGS 64
>  #define TARGET_PAGE_BITS 13 /* 8k */
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32

No. Please use the original value.

> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
>  #endif
>
>  #define CPUState struct CPUSPARCState
> --
> 1.6.6
>
>
>
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] Fix last page errors in page_set_flags and page_check_range.
  2010-02-12 19:47   ` Blue Swirl
@ 2010-02-12 20:16     ` Richard Henderson
  2010-02-12 20:37       ` Blue Swirl
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2010-02-12 20:16 UTC (permalink / raw)
  To: Blue Swirl; +Cc: qemu-devel

On 02/12/2010 11:47 AM, Blue Swirl wrote:
> Please make separate patches for unrelated changes. Now the essence of
> the patch is very hard to see. Also pure formatting changes are not
> very useful.

Is this just about page_get_flags, or was there some other pure 
formatting change to which you object?  For instance:

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

Only the first line is required to fix an off-by-one error.  But if I 
don't add the braces, generally the patch will be bounced for that.

>> -        /* We may be called for host regions that are outside guest
>> -           address space.  */
>
> Why remove the comment, is it no longer true?

Yes, after the entire patch series is applied.  Indeed, I believe that 
many of the addresses rejected here were *not* in fact outside the guest 
address space, merely that page_find_alloc did not accurately know what 
the guest address space was.

I think it was a mistake to ever consider usage of this function with 
out-of-band addresses as valid -- it adds a great deal of confusion.

I could add an assertion here to make sure, if you like.

If this were C++, it might have been interesting to try to use the type 
system to keep the host and guest address spaces forcibly separate.  But 
since this is plain C, I think wrapping everything in structures and 
access macros would just be too ugly.


r~

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
  2010-02-12 20:01   ` Blue Swirl
@ 2010-02-12 20:25     ` Richard Henderson
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-12 20:25 UTC (permalink / raw)
  To: Blue Swirl; +Cc: qemu-devel

On 02/12/2010 12:01 PM, Blue Swirl wrote:
>> +#ifdef TARGET_ABI32
>> +# define TARGET_VIRT_ADDR_SPACE_BITS 32
>> +#else
>> +# define TARGET_VIRT_ADDR_SPACE_BITS 64
>> +#endif
>
> I'd suppose this change applies to all targets with ABI32, not just PPC.

Indeed.  Odd that sparc32plus didn't yield errors building, while ppc 
did -- that's how I noticed the funny config macro in the first place.

>> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
>
> No. Please use the original value.

Oops.  Thanks for the catch.


r~

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 3/6] Fix last page errors in page_set_flags and page_check_range.
  2010-02-12 20:16     ` Richard Henderson
@ 2010-02-12 20:37       ` Blue Swirl
  0 siblings, 0 replies; 25+ messages in thread
From: Blue Swirl @ 2010-02-12 20:37 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Fri, Feb 12, 2010 at 10:16 PM, Richard Henderson <rth@twiddle.net> wrote:
> On 02/12/2010 11:47 AM, Blue Swirl wrote:
>>
>> Please make separate patches for unrelated changes. Now the essence of
>> the patch is very hard to see. Also pure formatting changes are not
>> very useful.
>
> Is this just about page_get_flags, or was there some other pure formatting
> change to which you object?  For instance:

Also this one:
-    end = TARGET_PAGE_ALIGN(start+len); /* must do before we loose
bits in the next step */
+    /* END must be computed before we drop bits from START.  */
+    end = TARGET_PAGE_ALIGN(start + len);
    start = start & TARGET_PAGE_MASK;

and these:

-        if( !p )
+        if (!p)
            return -1;
-        if( !(p->flags & PAGE_VALID) )
+        if (!(p->flags & PAGE_VALID))

>
>>> -    if (start + len < start)
>>> -        /* we've wrapped around */
>>> +    if (start + len - 1 < start) {
>>> +        /* We've wrapped around.  */
>>>        return -1;
>>> +    }
>
> Only the first line is required to fix an off-by-one error.  But if I don't
> add the braces, generally the patch will be bounced for that.

That change is OK.

>>> -        /* We may be called for host regions that are outside guest
>>> -           address space.  */
>>
>> Why remove the comment, is it no longer true?
>
> Yes, after the entire patch series is applied.  Indeed, I believe that many
> of the addresses rejected here were *not* in fact outside the guest address
> space, merely that page_find_alloc did not accurately know what the guest
> address space was.
>
> I think it was a mistake to ever consider usage of this function with
> out-of-band addresses as valid -- it adds a great deal of confusion.
>
> I could add an assertion here to make sure, if you like.
>
> If this were C++, it might have been interesting to try to use the type
> system to keep the host and guest address spaces forcibly separate.  But
> since this is plain C, I think wrapping everything in structures and access
> macros would just be too ugly.
>
>
> r~
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 1/7] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
  2010-02-15 20:59 ` [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2 Richard Henderson
                     ` (3 preceding siblings ...)
  2010-02-11 23:51   ` [Qemu-devel] [PATCH 4/7] Implement multi-level page tables Richard Henderson
@ 2010-02-12 21:38   ` Richard Henderson
  2010-02-12 22:03   ` [Qemu-devel] [PATCH 3/7] Fix last page errors in page_set_flags and page_check_range Richard Henderson
  2010-02-15 19:58   ` [Qemu-devel] [PATCH 7/7] Assert arguments in range for guest address space Richard Henderson
  6 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-12 21:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel

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.
---
 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 8389c54..766568b 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 c0dff4b..c144b4b 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 216b00e..7fb84db 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -872,6 +872,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 d15bba1..c91f8fe 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 0e75e1c..56fc083 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -100,6 +100,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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 3/7] Fix last page errors in page_set_flags and page_check_range.
  2010-02-15 20:59 ` [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2 Richard Henderson
                     ` (4 preceding siblings ...)
  2010-02-12 21:38   ` [Qemu-devel] [PATCH 1/7] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h Richard Henderson
@ 2010-02-12 22:03   ` Richard Henderson
  2010-02-15 19:58   ` [Qemu-devel] [PATCH 7/7] Assert arguments in range for guest address space Richard Henderson
  6 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-12 22:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel

The addr < end comparison prevents the last page from being
iterated; an iteration based on length avoids this problem.
---
 exec.c |   39 +++++++++++++++++++++------------------
 1 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/exec.c b/exec.c
index 766568b..1521ce3 100644
--- a/exec.c
+++ b/exec.c
@@ -2230,27 +2230,27 @@ 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;
 
-    /* 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) {
@@ -2266,14 +2266,17 @@ 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 */
+    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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 7/7] Assert arguments in range for guest address space.
  2010-02-15 20:59 ` [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2 Richard Henderson
                     ` (5 preceding siblings ...)
  2010-02-12 22:03   ` [Qemu-devel] [PATCH 3/7] Fix last page errors in page_set_flags and page_check_range Richard Henderson
@ 2010-02-15 19:58   ` Richard Henderson
  6 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-15 19:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel

We don't expect host addresses within page_set_flags or
page_check_range.
---
 exec.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/exec.c b/exec.c
index bb712ec..10673fc 100644
--- a/exec.c
+++ b/exec.c
@@ -2327,6 +2327,14 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
 {
     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 > TARGET_VIRT_ADDR_SPACE_BITS
+    assert(end < (1ul << TARGET_VIRT_ADDR_SPACE_BITS));
+#endif
+    assert(start < end);
+
     start = start & TARGET_PAGE_MASK;
     end = TARGET_PAGE_ALIGN(end);
 
@@ -2356,6 +2364,13 @@ int page_check_range(target_ulong start, target_ulong len, int flags)
     target_ulong end;
     target_ulong addr;
 
+    /* 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 > TARGET_VIRT_ADDR_SPACE_BITS
+    assert(start < (1ul << TARGET_VIRT_ADDR_SPACE_BITS));
+#endif
+
     if (start + len - 1 < start) {
         /* We've wrapped around.  */
         return -1;
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2
  2010-02-12  0:15 [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Richard Henderson
                   ` (5 preceding siblings ...)
  2010-02-11 23:51 ` [Qemu-devel] [PATCH 4/6] Implement multi-level page tables Richard Henderson
@ 2010-02-15 20:59 ` Richard Henderson
  2010-02-11 22:47   ` [Qemu-devel] [PATCH 2/7] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid Richard Henderson
                     ` (6 more replies)
  2010-02-28 23:23 ` [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Paul Brook
  7 siblings, 7 replies; 25+ messages in thread
From: Richard Henderson @ 2010-02-15 20:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel

Changes since v1:
  * Sparc virt and phys address range corrections.
  * Unrelated changes removed.
  * Assertions added for guest address space.


r~



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

 cpu-all.h               |   23 ++-
 exec.c                  |  513 ++++++++++++++++++++++++++++-------------------
 linux-user/main.c       |    7 +-
 linux-user/mmap.c       |  111 ++++++++---
 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 +
 15 files changed, 465 insertions(+), 253 deletions(-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 2/7] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid.
  2010-02-11 22:47   ` [Qemu-devel] [PATCH 2/7] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid Richard Henderson
@ 2010-02-28 14:11     ` Paul Brook
  0 siblings, 0 replies; 25+ messages in thread
From: Paul Brook @ 2010-02-28 14:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: blauwirbel, Richard Henderson


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

Shouldn't this be <= ? 
1ul << T_V_A_S_B is undefined for 64-bit guests on 32-bit hosts.

> +#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); \


Paul

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes.
  2010-02-12  0:15 [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Richard Henderson
                   ` (6 preceding siblings ...)
  2010-02-15 20:59 ` [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2 Richard Henderson
@ 2010-02-28 23:23 ` Paul Brook
  7 siblings, 0 replies; 25+ messages in thread
From: Paul Brook @ 2010-02-28 23:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Henderson

> Which brings us to the problem of exec.c and the address spaces therein.
> First, there was the fact that TARGET_PHYS_ADDR_SPACE_BITS was constrained
> to be no larger than 32 (with a partial hack for Alpha to extend this to
> 42 bits).  Second, that this physical address space value was applied to
> virtual addresses via page_find.

On further investigation (I missed this the first time round) it gets worse 
than that :-(  While userspace emulation uses PageDesc to describe virtual 
pages, system emulation uses it to track physical pages. This probably helps 
explain why the existing code is so confused.

I'm not sure why we have separate l1_map and l1_phys_map. My guess is it's an 
attempt to save memory, on the theory that typically only a small faction of 
ram will be used to hold code.
 
> This patch series untangles this somewhat.

Looks like a fairly good start, however they're missing a Signed-off-by.

Paul

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
  2010-03-10 23:59 [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3 Richard Henderson
@ 2010-03-10 22:33 ` Richard Henderson
  2010-03-11 11:11   ` Aurelien Jarno
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Henderson @ 2010-03-10 22:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: paul

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
  2010-03-10 22:33 ` [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h Richard Henderson
@ 2010-03-11 11:11   ` Aurelien Jarno
  2010-03-11 15:19     ` Richard Henderson
  0 siblings, 1 reply; 25+ messages in thread
From: Aurelien Jarno @ 2010-03-11 11:11 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel, paul

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h.
  2010-03-11 11:11   ` Aurelien Jarno
@ 2010-03-11 15:19     ` Richard Henderson
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Henderson @ 2010-03-11 15:19 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: qemu-devel, paul

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~

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2010-03-11 15:19 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-12  0:15 [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Richard Henderson
2010-02-11 22:20 ` [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h Richard Henderson
2010-02-12 20:01   ` Blue Swirl
2010-02-12 20:25     ` Richard Henderson
2010-02-11 22:47 ` [Qemu-devel] [PATCH 2/6] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid Richard Henderson
2010-02-11 22:57 ` [Qemu-devel] [PATCH 3/6] Fix last page errors in page_set_flags and page_check_range Richard Henderson
2010-02-12 19:47   ` Blue Swirl
2010-02-12 20:16     ` Richard Henderson
2010-02-12 20:37       ` Blue Swirl
2010-02-11 23:11 ` [Qemu-devel] [PATCH 5/6] linux-user: Use h2g_valid in qemu_vmalloc Richard Henderson
2010-02-11 23:29 ` [Qemu-devel] [PATCH 6/6] linux-user: Fix mmap_find_vma returning invalid addresses Richard Henderson
2010-02-11 23:51 ` [Qemu-devel] [PATCH 4/6] Implement multi-level page tables Richard Henderson
2010-02-15 20:59 ` [Qemu-devel] [PATCH 0/7] Multi-level page tables and userland mapping fixes, v2 Richard Henderson
2010-02-11 22:47   ` [Qemu-devel] [PATCH 2/7] Use TARGET_VIRT_ADDR_SPACE_BITS in h2g_valid Richard Henderson
2010-02-28 14:11     ` Paul Brook
2010-02-11 23:11   ` [Qemu-devel] [PATCH 5/7] linux-user: Use h2g_valid in qemu_vmalloc Richard Henderson
2010-02-11 23:29   ` [Qemu-devel] [PATCH 6/7] linux-user: Fix mmap_find_vma returning invalid addresses Richard Henderson
2010-02-11 23:51   ` [Qemu-devel] [PATCH 4/7] Implement multi-level page tables Richard Henderson
2010-02-12 21:38   ` [Qemu-devel] [PATCH 1/7] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h Richard Henderson
2010-02-12 22:03   ` [Qemu-devel] [PATCH 3/7] Fix last page errors in page_set_flags and page_check_range Richard Henderson
2010-02-15 19:58   ` [Qemu-devel] [PATCH 7/7] Assert arguments in range for guest address space Richard Henderson
2010-02-28 23:23 ` [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes Paul Brook
  -- strict thread matches above, loose matches on Subject: below --
2010-03-10 23:59 [Qemu-devel] [PATCH 0/6] Multi-level page tables and userland mapping fixes, v3 Richard Henderson
2010-03-10 22:33 ` [Qemu-devel] [PATCH 1/6] Move TARGET_PHYS_ADDR_SPACE_BITS to target-*/cpu.h Richard Henderson
2010-03-11 11:11   ` Aurelien Jarno
2010-03-11 15:19     ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).