qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] tcg: support 32-bit guest addresses as signed
@ 2021-10-10 17:43 Richard Henderson
  2021-10-10 17:43 ` [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32 Richard Henderson
                   ` (7 more replies)
  0 siblings, 8 replies; 30+ messages in thread
From: Richard Henderson @ 2021-10-10 17:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: git, Alistair.Francis, f4bug

We have 2, and nearly 3, hosts that naturally produce sign-extended
values, and have to work extra hard (with 1 or 2 insns) to produce
the zero-extended address that we expect today.

However, it's a simple matter of arithmetic for the middle-end to
require sign-extended addresses instead.  For user-only, we do have
to be careful not to allow a guest object to wrap around the signed
boundary, but that's fairly easily done.

Tested with aarch64, as that's the best hw currently available.


r~


Richard Henderson (8):
  tcg: Add TCG_TARGET_SIGNED_ADDR32
  accel/tcg: Split out g2h_tlbe
  accel/tcg: Support TCG_TARGET_SIGNED_ADDR32 for softmmu
  accel/tcg: Add guest_base_signed_addr32 for user-only
  linux-user: Support TCG_TARGET_SIGNED_ADDR32
  tcg/aarch64: Support TCG_TARGET_SIGNED_ADDR32
  target/mips: Support TCG_TARGET_SIGNED_ADDR32
  target/riscv: Support TCG_TARGET_SIGNED_ADDR32

 include/exec/cpu-all.h        | 20 ++++++++---
 include/exec/cpu_ldst.h       |  3 +-
 tcg/aarch64/tcg-target-sa32.h |  7 ++++
 tcg/arm/tcg-target-sa32.h     |  1 +
 tcg/i386/tcg-target-sa32.h    |  1 +
 tcg/mips/tcg-target-sa32.h    |  9 +++++
 tcg/ppc/tcg-target-sa32.h     |  1 +
 tcg/riscv/tcg-target-sa32.h   |  5 +++
 tcg/s390x/tcg-target-sa32.h   |  1 +
 tcg/sparc/tcg-target-sa32.h   |  1 +
 tcg/tci/tcg-target-sa32.h     |  1 +
 accel/tcg/cputlb.c            | 36 +++++++++++++------
 bsd-user/main.c               |  4 +++
 linux-user/elfload.c          | 62 +++++++++++++++++++++++++-------
 linux-user/main.c             |  3 ++
 tcg/aarch64/tcg-target.c.inc  | 68 ++++++++++++++++++++++-------------
 tcg/mips/tcg-target.c.inc     | 13 ++-----
 tcg/riscv/tcg-target.c.inc    |  8 ++---
 18 files changed, 176 insertions(+), 68 deletions(-)
 create mode 100644 tcg/aarch64/tcg-target-sa32.h
 create mode 100644 tcg/arm/tcg-target-sa32.h
 create mode 100644 tcg/i386/tcg-target-sa32.h
 create mode 100644 tcg/mips/tcg-target-sa32.h
 create mode 100644 tcg/ppc/tcg-target-sa32.h
 create mode 100644 tcg/riscv/tcg-target-sa32.h
 create mode 100644 tcg/s390x/tcg-target-sa32.h
 create mode 100644 tcg/sparc/tcg-target-sa32.h
 create mode 100644 tcg/tci/tcg-target-sa32.h

-- 
2.25.1



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

* [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:43 [PATCH 0/8] tcg: support 32-bit guest addresses as signed Richard Henderson
@ 2021-10-10 17:43 ` Richard Henderson
  2021-10-11  4:21   ` WANG Xuerui
                     ` (3 more replies)
  2021-10-10 17:43 ` [PATCH 2/8] accel/tcg: Split out g2h_tlbe Richard Henderson
                   ` (6 subsequent siblings)
  7 siblings, 4 replies; 30+ messages in thread
From: Richard Henderson @ 2021-10-10 17:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: git, Alistair.Francis, f4bug

Define as 0 for all tcg hosts.  Put this in a separate header,
because we'll want this in places that do not ordinarily have
access to all of tcg/tcg.h.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/aarch64/tcg-target-sa32.h | 1 +
 tcg/arm/tcg-target-sa32.h     | 1 +
 tcg/i386/tcg-target-sa32.h    | 1 +
 tcg/mips/tcg-target-sa32.h    | 1 +
 tcg/ppc/tcg-target-sa32.h     | 1 +
 tcg/riscv/tcg-target-sa32.h   | 1 +
 tcg/s390x/tcg-target-sa32.h   | 1 +
 tcg/sparc/tcg-target-sa32.h   | 1 +
 tcg/tci/tcg-target-sa32.h     | 1 +
 9 files changed, 9 insertions(+)
 create mode 100644 tcg/aarch64/tcg-target-sa32.h
 create mode 100644 tcg/arm/tcg-target-sa32.h
 create mode 100644 tcg/i386/tcg-target-sa32.h
 create mode 100644 tcg/mips/tcg-target-sa32.h
 create mode 100644 tcg/ppc/tcg-target-sa32.h
 create mode 100644 tcg/riscv/tcg-target-sa32.h
 create mode 100644 tcg/s390x/tcg-target-sa32.h
 create mode 100644 tcg/sparc/tcg-target-sa32.h
 create mode 100644 tcg/tci/tcg-target-sa32.h

diff --git a/tcg/aarch64/tcg-target-sa32.h b/tcg/aarch64/tcg-target-sa32.h
new file mode 100644
index 0000000000..cb185b1526
--- /dev/null
+++ b/tcg/aarch64/tcg-target-sa32.h
@@ -0,0 +1 @@
+#define TCG_TARGET_SIGNED_ADDR32 0
diff --git a/tcg/arm/tcg-target-sa32.h b/tcg/arm/tcg-target-sa32.h
new file mode 100644
index 0000000000..cb185b1526
--- /dev/null
+++ b/tcg/arm/tcg-target-sa32.h
@@ -0,0 +1 @@
+#define TCG_TARGET_SIGNED_ADDR32 0
diff --git a/tcg/i386/tcg-target-sa32.h b/tcg/i386/tcg-target-sa32.h
new file mode 100644
index 0000000000..cb185b1526
--- /dev/null
+++ b/tcg/i386/tcg-target-sa32.h
@@ -0,0 +1 @@
+#define TCG_TARGET_SIGNED_ADDR32 0
diff --git a/tcg/mips/tcg-target-sa32.h b/tcg/mips/tcg-target-sa32.h
new file mode 100644
index 0000000000..cb185b1526
--- /dev/null
+++ b/tcg/mips/tcg-target-sa32.h
@@ -0,0 +1 @@
+#define TCG_TARGET_SIGNED_ADDR32 0
diff --git a/tcg/ppc/tcg-target-sa32.h b/tcg/ppc/tcg-target-sa32.h
new file mode 100644
index 0000000000..cb185b1526
--- /dev/null
+++ b/tcg/ppc/tcg-target-sa32.h
@@ -0,0 +1 @@
+#define TCG_TARGET_SIGNED_ADDR32 0
diff --git a/tcg/riscv/tcg-target-sa32.h b/tcg/riscv/tcg-target-sa32.h
new file mode 100644
index 0000000000..cb185b1526
--- /dev/null
+++ b/tcg/riscv/tcg-target-sa32.h
@@ -0,0 +1 @@
+#define TCG_TARGET_SIGNED_ADDR32 0
diff --git a/tcg/s390x/tcg-target-sa32.h b/tcg/s390x/tcg-target-sa32.h
new file mode 100644
index 0000000000..cb185b1526
--- /dev/null
+++ b/tcg/s390x/tcg-target-sa32.h
@@ -0,0 +1 @@
+#define TCG_TARGET_SIGNED_ADDR32 0
diff --git a/tcg/sparc/tcg-target-sa32.h b/tcg/sparc/tcg-target-sa32.h
new file mode 100644
index 0000000000..cb185b1526
--- /dev/null
+++ b/tcg/sparc/tcg-target-sa32.h
@@ -0,0 +1 @@
+#define TCG_TARGET_SIGNED_ADDR32 0
diff --git a/tcg/tci/tcg-target-sa32.h b/tcg/tci/tcg-target-sa32.h
new file mode 100644
index 0000000000..cb185b1526
--- /dev/null
+++ b/tcg/tci/tcg-target-sa32.h
@@ -0,0 +1 @@
+#define TCG_TARGET_SIGNED_ADDR32 0
-- 
2.25.1



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

* [PATCH 2/8] accel/tcg: Split out g2h_tlbe
  2021-10-10 17:43 [PATCH 0/8] tcg: support 32-bit guest addresses as signed Richard Henderson
  2021-10-10 17:43 ` [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32 Richard Henderson
@ 2021-10-10 17:43 ` Richard Henderson
  2021-10-11  4:22   ` WANG Xuerui
                     ` (3 more replies)
  2021-10-10 17:43 ` [PATCH 3/8] accel/tcg: Support TCG_TARGET_SIGNED_ADDR32 for softmmu Richard Henderson
                   ` (5 subsequent siblings)
  7 siblings, 4 replies; 30+ messages in thread
From: Richard Henderson @ 2021-10-10 17:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: git, Alistair.Francis, f4bug

Create a new function to combine a CPUTLBEntry addend
with the guest address to form a host address.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cputlb.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 46140ccff3..761f726722 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -90,6 +90,11 @@ static inline size_t sizeof_tlb(CPUTLBDescFast *fast)
     return fast->mask + (1 << CPU_TLB_ENTRY_BITS);
 }
 
+static inline uintptr_t g2h_tlbe(const CPUTLBEntry *tlb, target_ulong gaddr)
+{
+    return tlb->addend + (uintptr_t)gaddr;
+}
+
 static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
                              size_t max_entries)
 {
@@ -976,8 +981,7 @@ static void tlb_reset_dirty_range_locked(CPUTLBEntry *tlb_entry,
 
     if ((addr & (TLB_INVALID_MASK | TLB_MMIO |
                  TLB_DISCARD_WRITE | TLB_NOTDIRTY)) == 0) {
-        addr &= TARGET_PAGE_MASK;
-        addr += tlb_entry->addend;
+        addr = g2h_tlbe(tlb_entry, addr & TARGET_PAGE_MASK);
         if ((addr - start) < length) {
 #if TCG_OVERSIZED_GUEST
             tlb_entry->addr_write |= TLB_NOTDIRTY;
@@ -1527,7 +1531,7 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
         return -1;
     }
 
-    p = (void *)((uintptr_t)addr + entry->addend);
+    p = (void *)g2h_tlbe(entry, addr);
     if (hostp) {
         *hostp = p;
     }
@@ -1619,7 +1623,7 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr,
     }
 
     /* Everything else is RAM. */
-    *phost = (void *)((uintptr_t)addr + entry->addend);
+    *phost = (void *)g2h_tlbe(entry, addr);
     return flags;
 }
 
@@ -1727,7 +1731,7 @@ bool tlb_plugin_lookup(CPUState *cpu, target_ulong addr, int mmu_idx,
             data->v.io.offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr;
         } else {
             data->is_io = false;
-            data->v.ram.hostaddr = (void *)((uintptr_t)addr + tlbe->addend);
+            data->v.ram.hostaddr = (void *)g2h_tlbe(tlbe, addr);
         }
         return true;
     } else {
@@ -1826,7 +1830,7 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
         goto stop_the_world;
     }
 
-    hostaddr = (void *)((uintptr_t)addr + tlbe->addend);
+    hostaddr = (void *)g2h_tlbe(tlbe, addr);
 
     if (unlikely(tlb_addr & TLB_NOTDIRTY)) {
         notdirty_write(env_cpu(env), addr, size,
@@ -1938,7 +1942,7 @@ load_helper(CPUArchState *env, target_ulong addr, MemOpIdx oi,
                             access_type, op ^ (need_swap * MO_BSWAP));
         }
 
-        haddr = (void *)((uintptr_t)addr + entry->addend);
+        haddr = (void *)g2h_tlbe(entry, addr);
 
         /*
          * Keep these two load_memop separate to ensure that the compiler
@@ -1975,7 +1979,7 @@ load_helper(CPUArchState *env, target_ulong addr, MemOpIdx oi,
         return res & MAKE_64BIT_MASK(0, size * 8);
     }
 
-    haddr = (void *)((uintptr_t)addr + entry->addend);
+    haddr = (void *)g2h_tlbe(entry, addr);
     return load_memop(haddr, op);
 }
 
@@ -2467,7 +2471,7 @@ store_helper(CPUArchState *env, target_ulong addr, uint64_t val,
             notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr);
         }
 
-        haddr = (void *)((uintptr_t)addr + entry->addend);
+        haddr = (void *)g2h_tlbe(entry, addr);
 
         /*
          * Keep these two store_memop separate to ensure that the compiler
@@ -2492,7 +2496,7 @@ store_helper(CPUArchState *env, target_ulong addr, uint64_t val,
         return;
     }
 
-    haddr = (void *)((uintptr_t)addr + entry->addend);
+    haddr = (void *)g2h_tlbe(entry, addr);
     store_memop(haddr, val, op);
 }
 
-- 
2.25.1



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

* [PATCH 3/8] accel/tcg: Support TCG_TARGET_SIGNED_ADDR32 for softmmu
  2021-10-10 17:43 [PATCH 0/8] tcg: support 32-bit guest addresses as signed Richard Henderson
  2021-10-10 17:43 ` [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32 Richard Henderson
  2021-10-10 17:43 ` [PATCH 2/8] accel/tcg: Split out g2h_tlbe Richard Henderson
@ 2021-10-10 17:43 ` Richard Henderson
  2021-10-11  4:30   ` WANG Xuerui
  2021-10-10 17:43 ` [PATCH 4/8] accel/tcg: Add guest_base_signed_addr32 for user-only Richard Henderson
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2021-10-10 17:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: git, Alistair.Francis, f4bug

When TCG_TARGET_SIGNED_ADDR32 is set, adjust the tlb addend to
allow the 32-bit guest address to be sign extended within the
64-bit host register instead of zero extended.

This will simplify tcg hosts like MIPS, RISC-V, and LoongArch,
which naturally sign-extend 32-bit values, in contrast to x86_64
and AArch64 which zero-extend them.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cputlb.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 761f726722..d12621c60e 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -39,6 +39,7 @@
 #ifdef CONFIG_PLUGIN
 #include "qemu/plugin-memory.h"
 #endif
+#include "tcg-target-sa32.h"
 
 /* DEBUG defines, enable DEBUG_TLB_LOG to log to the CPU_LOG_MMU target */
 /* #define DEBUG_TLB */
@@ -92,6 +93,9 @@ static inline size_t sizeof_tlb(CPUTLBDescFast *fast)
 
 static inline uintptr_t g2h_tlbe(const CPUTLBEntry *tlb, target_ulong gaddr)
 {
+    if (TCG_TARGET_SIGNED_ADDR32 && TARGET_LONG_BITS == 32) {
+        return tlb->addend + (int32_t)gaddr;
+    }
     return tlb->addend + (uintptr_t)gaddr;
 }
 
@@ -1234,7 +1238,13 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
     desc->iotlb[index].attrs = attrs;
 
     /* Now calculate the new entry */
-    tn.addend = addend - vaddr_page;
+
+    if (TCG_TARGET_SIGNED_ADDR32 && TARGET_LONG_BITS < TCG_TARGET_REG_BITS) {
+        tn.addend = addend - (int32_t)vaddr_page;
+    } else {
+        tn.addend = addend - vaddr_page;
+    }
+
     if (prot & PAGE_READ) {
         tn.addr_read = address;
         if (wp_flags & BP_MEM_READ) {
-- 
2.25.1



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

* [PATCH 4/8] accel/tcg: Add guest_base_signed_addr32 for user-only
  2021-10-10 17:43 [PATCH 0/8] tcg: support 32-bit guest addresses as signed Richard Henderson
                   ` (2 preceding siblings ...)
  2021-10-10 17:43 ` [PATCH 3/8] accel/tcg: Support TCG_TARGET_SIGNED_ADDR32 for softmmu Richard Henderson
@ 2021-10-10 17:43 ` Richard Henderson
  2021-10-11 22:06   ` Philippe Mathieu-Daudé
  2021-10-13  7:07   ` Alistair Francis
  2021-10-10 17:43 ` [PATCH 5/8] linux-user: Support TCG_TARGET_SIGNED_ADDR32 Richard Henderson
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 30+ messages in thread
From: Richard Henderson @ 2021-10-10 17:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: git, Alistair.Francis, f4bug

While the host may prefer to treat 32-bit addresses as signed,
there are edge cases of guests that cannot be implemented with
addresses 0x7fff_ffff and 0x8000_0000 being non-consecutive.

Therefore, default to guest_base_signed_addr32 false, and allow
probe_guest_base to determine whether it is possible to set it
to true.  A tcg backend which sets TCG_TARGET_SIGNED_ADDR32 will
have to cope with either setting for user-only.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu-all.h  | 16 ++++++++++++++++
 include/exec/cpu_ldst.h |  3 ++-
 bsd-user/main.c         |  4 ++++
 linux-user/main.c       |  3 +++
 4 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 32cfb634c6..80b5e17329 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -146,6 +146,7 @@ static inline void tswap64s(uint64_t *s)
 
 #if defined(CONFIG_USER_ONLY)
 #include "exec/user/abitypes.h"
+#include "tcg-target-sa32.h"
 
 /* On some host systems the guest address space is reserved on the host.
  * This allows the guest address space to be offset to a convenient location.
@@ -154,6 +155,21 @@ extern uintptr_t guest_base;
 extern bool have_guest_base;
 extern unsigned long reserved_va;
 
+#if TCG_TARGET_SIGNED_ADDR32 && TARGET_LONG_BITS == 32
+extern bool guest_base_signed_addr32;
+#else
+#define guest_base_signed_addr32  false
+#endif
+
+static inline void set_guest_base_signed_addr32(void)
+{
+#ifdef guest_base_signed_addr32
+    qemu_build_not_reached();
+#else
+    guest_base_signed_addr32 = true;
+#endif
+}
+
 /*
  * Limit the guest addresses as best we can.
  *
diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index ce6ce82618..db760ff5c2 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -79,7 +79,8 @@ static inline abi_ptr cpu_untagged_addr(CPUState *cs, abi_ptr x)
 /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
 static inline void *g2h_untagged(abi_ptr x)
 {
-    return (void *)((uintptr_t)(x) + guest_base);
+    uintptr_t hx = guest_base_signed_addr32 ? (int32_t)x : (uintptr_t)x;
+    return (void *)(guest_base + hx);
 }
 
 static inline void *g2h(CPUState *cs, abi_ptr x)
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 48643eeabc..4fef0520da 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -54,6 +54,10 @@
 int singlestep;
 uintptr_t guest_base;
 bool have_guest_base;
+#ifndef guest_base_signed_addr32
+bool guest_base_signed_addr32;
+#endif
+
 /*
  * When running 32-on-64 we should make sure we can fit all of the possible
  * guest address space into a contiguous chunk of virtual host memory.
diff --git a/linux-user/main.c b/linux-user/main.c
index 16def5215d..ed7a88c195 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -72,6 +72,9 @@ static const char *seed_optarg;
 unsigned long mmap_min_addr;
 uintptr_t guest_base;
 bool have_guest_base;
+#ifndef guest_base_signed_addr32
+bool guest_base_signed_addr32;
+#endif
 
 /*
  * Used to implement backwards-compatibility for the `-strace`, and
-- 
2.25.1



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

* [PATCH 5/8] linux-user: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:43 [PATCH 0/8] tcg: support 32-bit guest addresses as signed Richard Henderson
                   ` (3 preceding siblings ...)
  2021-10-10 17:43 ` [PATCH 4/8] accel/tcg: Add guest_base_signed_addr32 for user-only Richard Henderson
@ 2021-10-10 17:43 ` Richard Henderson
  2021-10-11 10:22   ` Alex Bennée
  2021-10-10 17:43 ` [PATCH 6/8] tcg/aarch64: " Richard Henderson
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2021-10-10 17:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: git, Alistair.Francis, f4bug

When using reserved_va, which is the default for a 64-bit host
and a 32-bit guest, set guest_base_signed_addr32 if requested
by TCG_TARGET_SIGNED_ADDR32, and the executable layout allows.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu-all.h |  4 ---
 linux-user/elfload.c   | 62 ++++++++++++++++++++++++++++++++++--------
 2 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 80b5e17329..71d8e1de7a 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -278,11 +278,7 @@ extern intptr_t qemu_host_page_mask;
 #define PAGE_RESET     0x0040
 /* For linux-user, indicates that the page is MAP_ANON. */
 #define PAGE_ANON      0x0080
-
-#if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
-/* FIXME: Code that sets/uses this is broken and needs to go away.  */
 #define PAGE_RESERVED  0x0100
-#endif
 /* Target-specific bits that will be used via page_get_flags().  */
 #define PAGE_TARGET_1  0x0200
 #define PAGE_TARGET_2  0x0400
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 2404d482ba..4a3d339cf1 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2422,33 +2422,71 @@ static void pgb_dynamic(const char *image_name, long align)
 static void pgb_reserved_va(const char *image_name, abi_ulong guest_loaddr,
                             abi_ulong guest_hiaddr, long align)
 {
-    int flags = MAP_ANONYMOUS | MAP_PRIVATE | MAP_NORESERVE;
+    int flags = (MAP_ANONYMOUS | MAP_PRIVATE |
+                 MAP_NORESERVE | MAP_FIXED_NOREPLACE);
+    unsigned long local_rva = reserved_va;
+    bool protect_wrap = false;
     void *addr, *test;
 
-    if (guest_hiaddr > reserved_va) {
+    if (guest_hiaddr > local_rva) {
         error_report("%s: requires more than reserved virtual "
                      "address space (0x%" PRIx64 " > 0x%lx)",
-                     image_name, (uint64_t)guest_hiaddr, reserved_va);
+                     image_name, (uint64_t)guest_hiaddr, local_rva);
         exit(EXIT_FAILURE);
     }
 
-    /* Widen the "image" to the entire reserved address space. */
-    pgb_static(image_name, 0, reserved_va, align);
+    if (TCG_TARGET_SIGNED_ADDR32 && TARGET_LONG_BITS == 32) {
+        if (guest_loaddr < 0x80000000u && guest_hiaddr > 0x80000000u) {
+            /*
+             * The executable itself wraps on signed addresses.
+             * Without per-page translation, we must keep the
+             * guest address 0x7fff_ffff adjacent to 0x8000_0000
+             * consecutive in host memory: unsigned addresses.
+             */
+        } else {
+            set_guest_base_signed_addr32();
+            if (local_rva <= 0x80000000u) {
+                /* No guest addresses are "negative": win! */
+            } else {
+                /* Begin by allocating the entire address space. */
+                local_rva = 0xfffffffful + 1;
+                protect_wrap = true;
+            }
+        }
+    }
 
-    /* osdep.h defines this as 0 if it's missing */
-    flags |= MAP_FIXED_NOREPLACE;
+    /* Widen the "image" to the entire reserved address space. */
+    pgb_static(image_name, 0, local_rva, align);
+    assert(guest_base != 0);
 
     /* Reserve the memory on the host. */
-    assert(guest_base != 0);
     test = g2h_untagged(0);
-    addr = mmap(test, reserved_va, PROT_NONE, flags, -1, 0);
+    addr = mmap(test, local_rva, PROT_NONE, flags, -1, 0);
     if (addr == MAP_FAILED || addr != test) {
+        /*
+         * If protect_wrap, we could try again with the original reserved_va
+         * setting, but the edge case of low ulimit vm setting on a 64-bit
+         * host is probably useless.
+         */
         error_report("Unable to reserve 0x%lx bytes of virtual address "
-                     "space at %p (%s) for use as guest address space (check your"
-                     "virtual memory ulimit setting, min_mmap_addr or reserve less "
-                     "using -R option)", reserved_va, test, strerror(errno));
+                     "space at %p (%s) for use as guest address space "
+                     "(check your virtual memory ulimit setting, "
+                     "min_mmap_addr or reserve less using -R option)",
+                     local_rva, test, strerror(errno));
         exit(EXIT_FAILURE);
     }
+
+    if (protect_wrap) {
+        /*
+         * Prevent the page just before 0x80000000 from being allocated.
+         * This prevents a single guest object/allocation from crossing
+         * the signed wrap, and thus being discontiguous in host memory.
+         */
+        page_set_flags(0x7fffffff & TARGET_PAGE_MASK, 0x80000000u,
+                       PAGE_RESERVED);
+        /* Adjust guest_base so that 0 is in the middle of the reservation. */
+        guest_base += 0x80000000ul;
+    }
 }
 
 void probe_guest_base(const char *image_name, abi_ulong guest_loaddr,
-- 
2.25.1



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

* [PATCH 6/8] tcg/aarch64: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:43 [PATCH 0/8] tcg: support 32-bit guest addresses as signed Richard Henderson
                   ` (4 preceding siblings ...)
  2021-10-10 17:43 ` [PATCH 5/8] linux-user: Support TCG_TARGET_SIGNED_ADDR32 Richard Henderson
@ 2021-10-10 17:43 ` Richard Henderson
  2021-10-11 10:28   ` Alex Bennée
  2021-10-10 17:44 ` [PATCH 7/8] target/mips: " Richard Henderson
  2021-10-10 17:44 ` [PATCH 8/8] target/riscv: " Richard Henderson
  7 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2021-10-10 17:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: git, Alistair.Francis, f4bug

AArch64 has both sign and zero-extending addressing modes, which
means that either treatment of guest addresses is equally efficient.
Enabling this for AArch64 gives us testing of the feature in CI.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/aarch64/tcg-target-sa32.h |  8 ++++-
 tcg/aarch64/tcg-target.c.inc  | 68 ++++++++++++++++++++++-------------
 2 files changed, 51 insertions(+), 25 deletions(-)

diff --git a/tcg/aarch64/tcg-target-sa32.h b/tcg/aarch64/tcg-target-sa32.h
index cb185b1526..c99e502e4c 100644
--- a/tcg/aarch64/tcg-target-sa32.h
+++ b/tcg/aarch64/tcg-target-sa32.h
@@ -1 +1,7 @@
-#define TCG_TARGET_SIGNED_ADDR32 0
+/*
+ * AArch64 has both SXTW and UXTW addressing modes, which means that
+ * it is agnostic to how guest addresses should be represented.
+ * Because aarch64 is more common than the other hosts that will
+ * want to use this feature, enable it for continuous testing.
+ */
+#define TCG_TARGET_SIGNED_ADDR32 1
diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
index 5edca8d44d..88b2963f9d 100644
--- a/tcg/aarch64/tcg-target.c.inc
+++ b/tcg/aarch64/tcg-target.c.inc
@@ -12,6 +12,7 @@
 
 #include "../tcg-pool.c.inc"
 #include "qemu/bitops.h"
+#include "tcg-target-sa32.h"
 
 /* We're going to re-use TCGType in setting of the SF bit, which controls
    the size of the operation performed.  If we know the values match, it
@@ -804,12 +805,12 @@ static void tcg_out_insn_3617(TCGContext *s, AArch64Insn insn, bool q,
 }
 
 static void tcg_out_insn_3310(TCGContext *s, AArch64Insn insn,
-                              TCGReg rd, TCGReg base, TCGType ext,
+                              TCGReg rd, TCGReg base, int option,
                               TCGReg regoff)
 {
     /* Note the AArch64Insn constants above are for C3.3.12.  Adjust.  */
     tcg_out32(s, insn | I3312_TO_I3310 | regoff << 16 |
-              0x4000 | ext << 13 | base << 5 | (rd & 0x1f));
+              option << 13 | base << 5 | (rd & 0x1f));
 }
 
 static void tcg_out_insn_3312(TCGContext *s, AArch64Insn insn,
@@ -1124,7 +1125,7 @@ static void tcg_out_ldst(TCGContext *s, AArch64Insn insn, TCGReg rd,
 
     /* Worst-case scenario, move offset to temp register, use reg offset.  */
     tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_TMP, offset);
-    tcg_out_ldst_r(s, insn, rd, rn, TCG_TYPE_I64, TCG_REG_TMP);
+    tcg_out_ldst_r(s, insn, rd, rn, 3 /* LSL #0 */, TCG_REG_TMP);
 }
 
 static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
@@ -1718,34 +1719,34 @@ static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg, MemOp opc,
 
 static void tcg_out_qemu_ld_direct(TCGContext *s, MemOp memop, TCGType ext,
                                    TCGReg data_r, TCGReg addr_r,
-                                   TCGType otype, TCGReg off_r)
+                                   int option, TCGReg off_r)
 {
     /* Byte swapping is left to middle-end expansion. */
     tcg_debug_assert((memop & MO_BSWAP) == 0);
 
     switch (memop & MO_SSIZE) {
     case MO_UB:
-        tcg_out_ldst_r(s, I3312_LDRB, data_r, addr_r, otype, off_r);
+        tcg_out_ldst_r(s, I3312_LDRB, data_r, addr_r, option, off_r);
         break;
     case MO_SB:
         tcg_out_ldst_r(s, ext ? I3312_LDRSBX : I3312_LDRSBW,
-                       data_r, addr_r, otype, off_r);
+                       data_r, addr_r, option, off_r);
         break;
     case MO_UW:
-        tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, otype, off_r);
+        tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, option, off_r);
         break;
     case MO_SW:
         tcg_out_ldst_r(s, (ext ? I3312_LDRSHX : I3312_LDRSHW),
-                       data_r, addr_r, otype, off_r);
+                       data_r, addr_r, option, off_r);
         break;
     case MO_UL:
-        tcg_out_ldst_r(s, I3312_LDRW, data_r, addr_r, otype, off_r);
+        tcg_out_ldst_r(s, I3312_LDRW, data_r, addr_r, option, off_r);
         break;
     case MO_SL:
-        tcg_out_ldst_r(s, I3312_LDRSWX, data_r, addr_r, otype, off_r);
+        tcg_out_ldst_r(s, I3312_LDRSWX, data_r, addr_r, option, off_r);
         break;
     case MO_Q:
-        tcg_out_ldst_r(s, I3312_LDRX, data_r, addr_r, otype, off_r);
+        tcg_out_ldst_r(s, I3312_LDRX, data_r, addr_r, option, off_r);
         break;
     default:
         tcg_abort();
@@ -1754,50 +1755,68 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, MemOp memop, TCGType ext,
 
 static void tcg_out_qemu_st_direct(TCGContext *s, MemOp memop,
                                    TCGReg data_r, TCGReg addr_r,
-                                   TCGType otype, TCGReg off_r)
+                                   int option, TCGReg off_r)
 {
     /* Byte swapping is left to middle-end expansion. */
     tcg_debug_assert((memop & MO_BSWAP) == 0);
 
     switch (memop & MO_SIZE) {
     case MO_8:
-        tcg_out_ldst_r(s, I3312_STRB, data_r, addr_r, otype, off_r);
+        tcg_out_ldst_r(s, I3312_STRB, data_r, addr_r, option, off_r);
         break;
     case MO_16:
-        tcg_out_ldst_r(s, I3312_STRH, data_r, addr_r, otype, off_r);
+        tcg_out_ldst_r(s, I3312_STRH, data_r, addr_r, option, off_r);
         break;
     case MO_32:
-        tcg_out_ldst_r(s, I3312_STRW, data_r, addr_r, otype, off_r);
+        tcg_out_ldst_r(s, I3312_STRW, data_r, addr_r, option, off_r);
         break;
     case MO_64:
-        tcg_out_ldst_r(s, I3312_STRX, data_r, addr_r, otype, off_r);
+        tcg_out_ldst_r(s, I3312_STRX, data_r, addr_r, option, off_r);
         break;
     default:
         tcg_abort();
     }
 }
 
+static int guest_ext_option(void)
+{
+#ifdef CONFIG_USER_ONLY
+    bool signed_addr32 = guest_base_signed_addr32;
+#else
+    bool signed_addr32 = TCG_TARGET_SIGNED_ADDR32;
+#endif
+
+    if (TARGET_LONG_BITS == 64) {
+        return 3; /* LSL #0 */
+    } else if (signed_addr32) {
+        return 6; /* SXTW */
+    } else {
+        return 2; /* UXTW */
+    }
+}
+
 static void tcg_out_qemu_ld(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
                             MemOpIdx oi, TCGType ext)
 {
     MemOp memop = get_memop(oi);
-    const TCGType otype = TARGET_LONG_BITS == 64 ? TCG_TYPE_I64 : TCG_TYPE_I32;
+    int option = guest_ext_option();
+
 #ifdef CONFIG_SOFTMMU
     unsigned mem_index = get_mmuidx(oi);
     tcg_insn_unit *label_ptr;
 
     tcg_out_tlb_read(s, addr_reg, memop, &label_ptr, mem_index, 1);
     tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
-                           TCG_REG_X1, otype, addr_reg);
+                           TCG_REG_X1, option, addr_reg);
     add_qemu_ldst_label(s, true, oi, ext, data_reg, addr_reg,
                         s->code_ptr, label_ptr);
 #else /* !CONFIG_SOFTMMU */
     if (USE_GUEST_BASE) {
         tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
-                               TCG_REG_GUEST_BASE, otype, addr_reg);
+                               TCG_REG_GUEST_BASE, option, addr_reg);
     } else {
         tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
-                               addr_reg, TCG_TYPE_I64, TCG_REG_XZR);
+                               addr_reg, option, TCG_REG_XZR);
     }
 #endif /* CONFIG_SOFTMMU */
 }
@@ -1806,23 +1825,24 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
                             MemOpIdx oi)
 {
     MemOp memop = get_memop(oi);
-    const TCGType otype = TARGET_LONG_BITS == 64 ? TCG_TYPE_I64 : TCG_TYPE_I32;
+    int option = guest_ext_option();
+
 #ifdef CONFIG_SOFTMMU
     unsigned mem_index = get_mmuidx(oi);
     tcg_insn_unit *label_ptr;
 
     tcg_out_tlb_read(s, addr_reg, memop, &label_ptr, mem_index, 0);
     tcg_out_qemu_st_direct(s, memop, data_reg,
-                           TCG_REG_X1, otype, addr_reg);
+                           TCG_REG_X1, option, addr_reg);
     add_qemu_ldst_label(s, false, oi, (memop & MO_SIZE)== MO_64,
                         data_reg, addr_reg, s->code_ptr, label_ptr);
 #else /* !CONFIG_SOFTMMU */
     if (USE_GUEST_BASE) {
         tcg_out_qemu_st_direct(s, memop, data_reg,
-                               TCG_REG_GUEST_BASE, otype, addr_reg);
+                               TCG_REG_GUEST_BASE, option, addr_reg);
     } else {
         tcg_out_qemu_st_direct(s, memop, data_reg,
-                               addr_reg, TCG_TYPE_I64, TCG_REG_XZR);
+                               addr_reg, option, TCG_REG_XZR);
     }
 #endif /* CONFIG_SOFTMMU */
 }
-- 
2.25.1



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

* [PATCH 7/8] target/mips: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:43 [PATCH 0/8] tcg: support 32-bit guest addresses as signed Richard Henderson
                   ` (5 preceding siblings ...)
  2021-10-10 17:43 ` [PATCH 6/8] tcg/aarch64: " Richard Henderson
@ 2021-10-10 17:44 ` Richard Henderson
  2021-10-11  4:20   ` WANG Xuerui
  2021-10-10 17:44 ` [PATCH 8/8] target/riscv: " Richard Henderson
  7 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2021-10-10 17:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: git, Alistair.Francis, f4bug

All 32-bit mips operations sign-extend the output, so we are easily
able to keep TCG_TYPE_I32 values sign-extended in host registers.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/mips/tcg-target-sa32.h |  8 ++++++++
 tcg/mips/tcg-target.c.inc  | 13 +++----------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/tcg/mips/tcg-target-sa32.h b/tcg/mips/tcg-target-sa32.h
index cb185b1526..51255e7cba 100644
--- a/tcg/mips/tcg-target-sa32.h
+++ b/tcg/mips/tcg-target-sa32.h
@@ -1 +1,9 @@
+/*
+ * Do not set TCG_TARGET_SIGNED_ADDR32 for mips32;
+ * TCG expects this to only be set for 64-bit hosts.
+ */
+#ifdef __mips64
+#define TCG_TARGET_SIGNED_ADDR32 1
+#else
 #define TCG_TARGET_SIGNED_ADDR32 0
+#endif
diff --git a/tcg/mips/tcg-target.c.inc b/tcg/mips/tcg-target.c.inc
index d8f6914f03..cc3b4d5b90 100644
--- a/tcg/mips/tcg-target.c.inc
+++ b/tcg/mips/tcg-target.c.inc
@@ -1161,20 +1161,13 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg base, TCGReg addrl,
         tcg_out_ld(s, TCG_TYPE_I32, TCG_TMP0, TCG_TMP3, cmp_off + LO_OFF);
         tcg_out_movi(s, TCG_TYPE_I32, TCG_TMP1, mask);
     } else {
-        tcg_out_ldst(s, (TARGET_LONG_BITS == 64 ? OPC_LD
-                         : TCG_TARGET_REG_BITS == 64 ? OPC_LWU : OPC_LW),
-                     TCG_TMP0, TCG_TMP3, cmp_off);
+        tcg_out_ld(s, TCG_TYPE_TL, TCG_TMP0, TCG_TMP3, cmp_off);
         tcg_out_movi(s, TCG_TYPE_TL, TCG_TMP1, mask);
         /* No second compare is required here;
            load the tlb addend for the fast path.  */
         tcg_out_ld(s, TCG_TYPE_PTR, TCG_TMP2, TCG_TMP3, add_off);
     }
 
-    /* Zero extend a 32-bit guest address for a 64-bit host. */
-    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
-        tcg_out_ext32u(s, base, addrl);
-        addrl = base;
-    }
     tcg_out_opc_reg(s, OPC_AND, TCG_TMP1, TCG_TMP1, addrl);
 
     label_ptr[0] = s->code_ptr;
@@ -1456,7 +1449,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is_64)
                         data_regl, data_regh, addr_regl, addr_regh,
                         s->code_ptr, label_ptr);
 #else
-    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
+    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) {
         tcg_out_ext32u(s, base, addr_regl);
         addr_regl = base;
     }
@@ -1559,7 +1552,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is_64)
                         s->code_ptr, label_ptr);
 #else
     base = TCG_REG_A0;
-    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
+    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) {
         tcg_out_ext32u(s, base, addr_regl);
         addr_regl = base;
     }
-- 
2.25.1



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

* [PATCH 8/8] target/riscv: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:43 [PATCH 0/8] tcg: support 32-bit guest addresses as signed Richard Henderson
                   ` (6 preceding siblings ...)
  2021-10-10 17:44 ` [PATCH 7/8] target/mips: " Richard Henderson
@ 2021-10-10 17:44 ` Richard Henderson
  2021-10-11 22:00   ` Philippe Mathieu-Daudé
  2021-10-13  7:08   ` Alistair Francis
  7 siblings, 2 replies; 30+ messages in thread
From: Richard Henderson @ 2021-10-10 17:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: git, Alistair.Francis, f4bug

All RV64 32-bit operations sign-extend the output, so we are easily
able to keep TCG_TYPE_I32 values sign-extended in host registers.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/riscv/tcg-target-sa32.h | 6 +++++-
 tcg/riscv/tcg-target.c.inc  | 8 ++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tcg/riscv/tcg-target-sa32.h b/tcg/riscv/tcg-target-sa32.h
index cb185b1526..703467b37a 100644
--- a/tcg/riscv/tcg-target-sa32.h
+++ b/tcg/riscv/tcg-target-sa32.h
@@ -1 +1,5 @@
-#define TCG_TARGET_SIGNED_ADDR32 0
+/*
+ * Do not set TCG_TARGET_SIGNED_ADDR32 for RV32;
+ * TCG expects this to only be set for 64-bit hosts.
+ */
+#define TCG_TARGET_SIGNED_ADDR32  (__riscv_xlen == 64)
diff --git a/tcg/riscv/tcg-target.c.inc b/tcg/riscv/tcg-target.c.inc
index 9b13a46fb4..9426ef8926 100644
--- a/tcg/riscv/tcg-target.c.inc
+++ b/tcg/riscv/tcg-target.c.inc
@@ -952,10 +952,6 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg addrl,
     tcg_out_opc_branch(s, OPC_BNE, TCG_REG_TMP0, TCG_REG_TMP1, 0);
 
     /* TLB Hit - translate address using addend.  */
-    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
-        tcg_out_ext32u(s, TCG_REG_TMP0, addrl);
-        addrl = TCG_REG_TMP0;
-    }
     tcg_out_opc_reg(s, OPC_ADD, TCG_REG_TMP0, TCG_REG_TMP2, addrl);
 }
 
@@ -1126,7 +1122,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is_64)
                         data_regl, data_regh, addr_regl, addr_regh,
                         s->code_ptr, label_ptr);
 #else
-    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
+    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) {
         tcg_out_ext32u(s, base, addr_regl);
         addr_regl = base;
     }
@@ -1192,7 +1188,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is_64)
                         data_regl, data_regh, addr_regl, addr_regh,
                         s->code_ptr, label_ptr);
 #else
-    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
+    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) {
         tcg_out_ext32u(s, base, addr_regl);
         addr_regl = base;
     }
-- 
2.25.1



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

* Re: [PATCH 7/8] target/mips: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:44 ` [PATCH 7/8] target/mips: " Richard Henderson
@ 2021-10-11  4:20   ` WANG Xuerui
  2021-10-13 22:24     ` Richard Henderson
  0 siblings, 1 reply; 30+ messages in thread
From: WANG Xuerui @ 2021-10-11  4:20 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: Alistair.Francis, f4bug

Hi Richard,

On 2021/10/11 01:44, Richard Henderson wrote:
> All 32-bit mips operations sign-extend the output, so we are easily
> able to keep TCG_TYPE_I32 values sign-extended in host registers.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/mips/tcg-target-sa32.h |  8 ++++++++
>  tcg/mips/tcg-target.c.inc  | 13 +++----------
>  2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/tcg/mips/tcg-target-sa32.h b/tcg/mips/tcg-target-sa32.h
> index cb185b1526..51255e7cba 100644
> --- a/tcg/mips/tcg-target-sa32.h
> +++ b/tcg/mips/tcg-target-sa32.h
> @@ -1 +1,9 @@
> +/*
> + * Do not set TCG_TARGET_SIGNED_ADDR32 for mips32;
> + * TCG expects this to only be set for 64-bit hosts.
> + */
> +#ifdef __mips64
> +#define TCG_TARGET_SIGNED_ADDR32 1
> +#else
>  #define TCG_TARGET_SIGNED_ADDR32 0
> +#endif
It looks like we never want to set TCG_TARGET_SIGNED_ADDR32 on 32-bit
hosts; maybe a compile-time assert could be added somewhere for
statically guaranteeing this?
> diff --git a/tcg/mips/tcg-target.c.inc b/tcg/mips/tcg-target.c.inc
> index d8f6914f03..cc3b4d5b90 100644
> --- a/tcg/mips/tcg-target.c.inc
> +++ b/tcg/mips/tcg-target.c.inc
> @@ -1161,20 +1161,13 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg base, TCGReg addrl,
>          tcg_out_ld(s, TCG_TYPE_I32, TCG_TMP0, TCG_TMP3, cmp_off + LO_OFF);
>          tcg_out_movi(s, TCG_TYPE_I32, TCG_TMP1, mask);
>      } else {
> -        tcg_out_ldst(s, (TARGET_LONG_BITS == 64 ? OPC_LD
> -                         : TCG_TARGET_REG_BITS == 64 ? OPC_LWU : OPC_LW),
> -                     TCG_TMP0, TCG_TMP3, cmp_off);
> +        tcg_out_ld(s, TCG_TYPE_TL, TCG_TMP0, TCG_TMP3, cmp_off);
>          tcg_out_movi(s, TCG_TYPE_TL, TCG_TMP1, mask);
>          /* No second compare is required here;
>             load the tlb addend for the fast path.  */
>          tcg_out_ld(s, TCG_TYPE_PTR, TCG_TMP2, TCG_TMP3, add_off);
>      }
>  
> -    /* Zero extend a 32-bit guest address for a 64-bit host. */
> -    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
> -        tcg_out_ext32u(s, base, addrl);
> -        addrl = base;
> -    }
>      tcg_out_opc_reg(s, OPC_AND, TCG_TMP1, TCG_TMP1, addrl);
>  
>      label_ptr[0] = s->code_ptr;
> @@ -1456,7 +1449,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is_64)
>                          data_regl, data_regh, addr_regl, addr_regh,
>                          s->code_ptr, label_ptr);
>  #else
> -    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
> +    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) {
Is there precedent of extracting predicates like this into some header
for reuse? However I agree that the current expression conveys enough
meaning without being overly complicated.
>          tcg_out_ext32u(s, base, addr_regl);
>          addr_regl = base;
>      }
> @@ -1559,7 +1552,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is_64)
>                          s->code_ptr, label_ptr);
>  #else
>      base = TCG_REG_A0;
> -    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
> +    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) {
>          tcg_out_ext32u(s, base, addr_regl);
>          addr_regl = base;
>      }


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

* Re: [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:43 ` [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32 Richard Henderson
@ 2021-10-11  4:21   ` WANG Xuerui
  2021-10-11  9:55   ` Alex Bennée
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 30+ messages in thread
From: WANG Xuerui @ 2021-10-11  4:21 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: Alistair.Francis, f4bug

On 2021/10/11 01:43, Richard Henderson wrote:
> Define as 0 for all tcg hosts.  Put this in a separate header,
> because we'll want this in places that do not ordinarily have
> access to all of tcg/tcg.h.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/aarch64/tcg-target-sa32.h | 1 +
>  tcg/arm/tcg-target-sa32.h     | 1 +
>  tcg/i386/tcg-target-sa32.h    | 1 +
>  tcg/mips/tcg-target-sa32.h    | 1 +
>  tcg/ppc/tcg-target-sa32.h     | 1 +
>  tcg/riscv/tcg-target-sa32.h   | 1 +
>  tcg/s390x/tcg-target-sa32.h   | 1 +
>  tcg/sparc/tcg-target-sa32.h   | 1 +
>  tcg/tci/tcg-target-sa32.h     | 1 +
>  9 files changed, 9 insertions(+)
>  create mode 100644 tcg/aarch64/tcg-target-sa32.h
>  create mode 100644 tcg/arm/tcg-target-sa32.h
>  create mode 100644 tcg/i386/tcg-target-sa32.h
>  create mode 100644 tcg/mips/tcg-target-sa32.h
>  create mode 100644 tcg/ppc/tcg-target-sa32.h
>  create mode 100644 tcg/riscv/tcg-target-sa32.h
>  create mode 100644 tcg/s390x/tcg-target-sa32.h
>  create mode 100644 tcg/sparc/tcg-target-sa32.h
>  create mode 100644 tcg/tci/tcg-target-sa32.h
Reviewed-by: WANG Xuerui <git@xen0n.name>


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

* Re: [PATCH 2/8] accel/tcg: Split out g2h_tlbe
  2021-10-10 17:43 ` [PATCH 2/8] accel/tcg: Split out g2h_tlbe Richard Henderson
@ 2021-10-11  4:22   ` WANG Xuerui
  2021-10-11  9:55   ` Alex Bennée
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 30+ messages in thread
From: WANG Xuerui @ 2021-10-11  4:22 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: Alistair.Francis, f4bug

On 2021/10/11 01:43, Richard Henderson wrote:
> Create a new function to combine a CPUTLBEntry addend
> with the guest address to form a host address.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/cputlb.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
Reviewed-by: WANG Xuerui <git@xen0n.name>


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

* Re: [PATCH 3/8] accel/tcg: Support TCG_TARGET_SIGNED_ADDR32 for softmmu
  2021-10-10 17:43 ` [PATCH 3/8] accel/tcg: Support TCG_TARGET_SIGNED_ADDR32 for softmmu Richard Henderson
@ 2021-10-11  4:30   ` WANG Xuerui
  2021-10-11 15:27     ` Richard Henderson
  0 siblings, 1 reply; 30+ messages in thread
From: WANG Xuerui @ 2021-10-11  4:30 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: Alistair.Francis, f4bug

Hi Richard,

On 2021/10/11 01:43, Richard Henderson wrote:
> When TCG_TARGET_SIGNED_ADDR32 is set, adjust the tlb addend to
> allow the 32-bit guest address to be sign extended within the
> 64-bit host register instead of zero extended.
>
> This will simplify tcg hosts like MIPS, RISC-V, and LoongArch,
> which naturally sign-extend 32-bit values, in contrast to x86_64
> and AArch64 which zero-extend them.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/cputlb.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 761f726722..d12621c60e 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -39,6 +39,7 @@
>  #ifdef CONFIG_PLUGIN
>  #include "qemu/plugin-memory.h"
>  #endif
> +#include "tcg-target-sa32.h"
>  
>  /* DEBUG defines, enable DEBUG_TLB_LOG to log to the CPU_LOG_MMU target */
>  /* #define DEBUG_TLB */
> @@ -92,6 +93,9 @@ static inline size_t sizeof_tlb(CPUTLBDescFast *fast)
>  
>  static inline uintptr_t g2h_tlbe(const CPUTLBEntry *tlb, target_ulong gaddr)
>  {
> +    if (TCG_TARGET_SIGNED_ADDR32 && TARGET_LONG_BITS == 32) {
It seems this branch's direction should always match that of the branch
added below, so if TARGET_LONG_BITS == TARGET_LONG_BITS == 32 this
invariant is broken? Or is this expected behavior?
> +        return tlb->addend + (int32_t)gaddr;
> +    }
>      return tlb->addend + (uintptr_t)gaddr;
>  }
>  
> @@ -1234,7 +1238,13 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
>      desc->iotlb[index].attrs = attrs;
>  
>      /* Now calculate the new entry */
> -    tn.addend = addend - vaddr_page;
> +
> +    if (TCG_TARGET_SIGNED_ADDR32 && TARGET_LONG_BITS < TCG_TARGET_REG_BITS) {
> +        tn.addend = addend - (int32_t)vaddr_page;
> +    } else {
> +        tn.addend = addend - vaddr_page;
> +    }
> +
>      if (prot & PAGE_READ) {
>          tn.addr_read = address;
>          if (wp_flags & BP_MEM_READ) {


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

* Re: [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:43 ` [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32 Richard Henderson
  2021-10-11  4:21   ` WANG Xuerui
@ 2021-10-11  9:55   ` Alex Bennée
  2021-10-11 22:07   ` Philippe Mathieu-Daudé
  2021-10-11 23:16   ` Alistair Francis
  3 siblings, 0 replies; 30+ messages in thread
From: Alex Bennée @ 2021-10-11  9:55 UTC (permalink / raw)
  To: Richard Henderson; +Cc: git, Alistair.Francis, f4bug, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> Define as 0 for all tcg hosts.  Put this in a separate header,
> because we'll want this in places that do not ordinarily have
> access to all of tcg/tcg.h.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


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

* Re: [PATCH 2/8] accel/tcg: Split out g2h_tlbe
  2021-10-10 17:43 ` [PATCH 2/8] accel/tcg: Split out g2h_tlbe Richard Henderson
  2021-10-11  4:22   ` WANG Xuerui
@ 2021-10-11  9:55   ` Alex Bennée
  2021-10-11 21:48   ` Philippe Mathieu-Daudé
  2021-10-11 23:19   ` Alistair Francis
  3 siblings, 0 replies; 30+ messages in thread
From: Alex Bennée @ 2021-10-11  9:55 UTC (permalink / raw)
  To: Richard Henderson; +Cc: git, Alistair.Francis, f4bug, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> Create a new function to combine a CPUTLBEntry addend
> with the guest address to form a host address.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


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

* Re: [PATCH 5/8] linux-user: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:43 ` [PATCH 5/8] linux-user: Support TCG_TARGET_SIGNED_ADDR32 Richard Henderson
@ 2021-10-11 10:22   ` Alex Bennée
  2021-10-11 15:32     ` Richard Henderson
  0 siblings, 1 reply; 30+ messages in thread
From: Alex Bennée @ 2021-10-11 10:22 UTC (permalink / raw)
  To: Richard Henderson; +Cc: git, Alistair.Francis, f4bug, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> When using reserved_va, which is the default for a 64-bit host
> and a 32-bit guest, set guest_base_signed_addr32 if requested
> by TCG_TARGET_SIGNED_ADDR32, and the executable layout allows.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/cpu-all.h |  4 ---
>  linux-user/elfload.c   | 62 ++++++++++++++++++++++++++++++++++--------
>  2 files changed, 50 insertions(+), 16 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 80b5e17329..71d8e1de7a 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -278,11 +278,7 @@ extern intptr_t qemu_host_page_mask;
>  #define PAGE_RESET     0x0040
>  /* For linux-user, indicates that the page is MAP_ANON. */
>  #define PAGE_ANON      0x0080
> -
> -#if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
> -/* FIXME: Code that sets/uses this is broken and needs to go away.  */
>  #define PAGE_RESERVED  0x0100
> -#endif

Can we reference why this FIXME is being dropped in the commit message?
Looking at the current tree state I can see several uses of it due to
moves in 5b6dd8683d (exec: move TB handling to translate-all.c) which
post-date 2e9a5713f0 (Remove PAGE_RESERVED).

Otherwise looks reasonable:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


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

* Re: [PATCH 6/8] tcg/aarch64: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:43 ` [PATCH 6/8] tcg/aarch64: " Richard Henderson
@ 2021-10-11 10:28   ` Alex Bennée
  2021-10-11 15:24     ` Richard Henderson
  2021-10-13 21:05     ` Richard Henderson
  0 siblings, 2 replies; 30+ messages in thread
From: Alex Bennée @ 2021-10-11 10:28 UTC (permalink / raw)
  To: Richard Henderson; +Cc: git, Alistair.Francis, f4bug, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> AArch64 has both sign and zero-extending addressing modes, which
> means that either treatment of guest addresses is equally efficient.
> Enabling this for AArch64 gives us testing of the feature in CI.

So which guests front ends will exercise this backend? I realise you
never mentioned it in the cover letter. Is this something we can
exercise in 32 bit user mode tests?

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/aarch64/tcg-target-sa32.h |  8 ++++-
>  tcg/aarch64/tcg-target.c.inc  | 68 ++++++++++++++++++++++-------------
>  2 files changed, 51 insertions(+), 25 deletions(-)
>
> diff --git a/tcg/aarch64/tcg-target-sa32.h b/tcg/aarch64/tcg-target-sa32.h
> index cb185b1526..c99e502e4c 100644
> --- a/tcg/aarch64/tcg-target-sa32.h
> +++ b/tcg/aarch64/tcg-target-sa32.h
> @@ -1 +1,7 @@
> -#define TCG_TARGET_SIGNED_ADDR32 0
> +/*
> + * AArch64 has both SXTW and UXTW addressing modes, which means that
> + * it is agnostic to how guest addresses should be represented.
> + * Because aarch64 is more common than the other hosts that will
> + * want to use this feature, enable it for continuous testing.
> + */
> +#define TCG_TARGET_SIGNED_ADDR32 1
> diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
> index 5edca8d44d..88b2963f9d 100644
> --- a/tcg/aarch64/tcg-target.c.inc
> +++ b/tcg/aarch64/tcg-target.c.inc
> @@ -12,6 +12,7 @@
>  
>  #include "../tcg-pool.c.inc"
>  #include "qemu/bitops.h"
> +#include "tcg-target-sa32.h"
>  
>  /* We're going to re-use TCGType in setting of the SF bit, which controls
>     the size of the operation performed.  If we know the values match, it
> @@ -804,12 +805,12 @@ static void tcg_out_insn_3617(TCGContext *s, AArch64Insn insn, bool q,
>  }
>  
>  static void tcg_out_insn_3310(TCGContext *s, AArch64Insn insn,
> -                              TCGReg rd, TCGReg base, TCGType ext,
> +                              TCGReg rd, TCGReg base, int option,
>                                TCGReg regoff)
>  {
>      /* Note the AArch64Insn constants above are for C3.3.12.  Adjust.  */
>      tcg_out32(s, insn | I3312_TO_I3310 | regoff << 16 |
> -              0x4000 | ext << 13 | base << 5 | (rd & 0x1f));
> +              option << 13 | base << 5 | (rd & 0x1f));
>  }
>  
>  static void tcg_out_insn_3312(TCGContext *s, AArch64Insn insn,
> @@ -1124,7 +1125,7 @@ static void tcg_out_ldst(TCGContext *s, AArch64Insn insn, TCGReg rd,
>  
>      /* Worst-case scenario, move offset to temp register, use reg offset.  */
>      tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_TMP, offset);
> -    tcg_out_ldst_r(s, insn, rd, rn, TCG_TYPE_I64, TCG_REG_TMP);
> +    tcg_out_ldst_r(s, insn, rd, rn, 3 /* LSL #0 */, TCG_REG_TMP);
>  }
>  
>  static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
> @@ -1718,34 +1719,34 @@ static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg, MemOp opc,
>  
>  static void tcg_out_qemu_ld_direct(TCGContext *s, MemOp memop, TCGType ext,
>                                     TCGReg data_r, TCGReg addr_r,
> -                                   TCGType otype, TCGReg off_r)
> +                                   int option, TCGReg off_r)
>  {
>      /* Byte swapping is left to middle-end expansion. */
>      tcg_debug_assert((memop & MO_BSWAP) == 0);
>  
>      switch (memop & MO_SSIZE) {
>      case MO_UB:
> -        tcg_out_ldst_r(s, I3312_LDRB, data_r, addr_r, otype, off_r);
> +        tcg_out_ldst_r(s, I3312_LDRB, data_r, addr_r, option, off_r);
>          break;
>      case MO_SB:
>          tcg_out_ldst_r(s, ext ? I3312_LDRSBX : I3312_LDRSBW,
> -                       data_r, addr_r, otype, off_r);
> +                       data_r, addr_r, option, off_r);
>          break;
>      case MO_UW:
> -        tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, otype, off_r);
> +        tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, option, off_r);
>          break;
>      case MO_SW:
>          tcg_out_ldst_r(s, (ext ? I3312_LDRSHX : I3312_LDRSHW),
> -                       data_r, addr_r, otype, off_r);
> +                       data_r, addr_r, option, off_r);
>          break;
>      case MO_UL:
> -        tcg_out_ldst_r(s, I3312_LDRW, data_r, addr_r, otype, off_r);
> +        tcg_out_ldst_r(s, I3312_LDRW, data_r, addr_r, option, off_r);
>          break;
>      case MO_SL:
> -        tcg_out_ldst_r(s, I3312_LDRSWX, data_r, addr_r, otype, off_r);
> +        tcg_out_ldst_r(s, I3312_LDRSWX, data_r, addr_r, option, off_r);
>          break;
>      case MO_Q:
> -        tcg_out_ldst_r(s, I3312_LDRX, data_r, addr_r, otype, off_r);
> +        tcg_out_ldst_r(s, I3312_LDRX, data_r, addr_r, option, off_r);
>          break;
>      default:
>          tcg_abort();
> @@ -1754,50 +1755,68 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, MemOp memop, TCGType ext,
>  
>  static void tcg_out_qemu_st_direct(TCGContext *s, MemOp memop,
>                                     TCGReg data_r, TCGReg addr_r,
> -                                   TCGType otype, TCGReg off_r)
> +                                   int option, TCGReg off_r)
>  {
>      /* Byte swapping is left to middle-end expansion. */
>      tcg_debug_assert((memop & MO_BSWAP) == 0);
>  
>      switch (memop & MO_SIZE) {
>      case MO_8:
> -        tcg_out_ldst_r(s, I3312_STRB, data_r, addr_r, otype, off_r);
> +        tcg_out_ldst_r(s, I3312_STRB, data_r, addr_r, option, off_r);
>          break;
>      case MO_16:
> -        tcg_out_ldst_r(s, I3312_STRH, data_r, addr_r, otype, off_r);
> +        tcg_out_ldst_r(s, I3312_STRH, data_r, addr_r, option, off_r);
>          break;
>      case MO_32:
> -        tcg_out_ldst_r(s, I3312_STRW, data_r, addr_r, otype, off_r);
> +        tcg_out_ldst_r(s, I3312_STRW, data_r, addr_r, option, off_r);
>          break;
>      case MO_64:
> -        tcg_out_ldst_r(s, I3312_STRX, data_r, addr_r, otype, off_r);
> +        tcg_out_ldst_r(s, I3312_STRX, data_r, addr_r, option, off_r);
>          break;
>      default:
>          tcg_abort();
>      }
>  }
>  
> +static int guest_ext_option(void)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    bool signed_addr32 = guest_base_signed_addr32;
> +#else
> +    bool signed_addr32 = TCG_TARGET_SIGNED_ADDR32;
> +#endif
> +
> +    if (TARGET_LONG_BITS == 64) {
> +        return 3; /* LSL #0 */
> +    } else if (signed_addr32) {
> +        return 6; /* SXTW */
> +    } else {
> +        return 2; /* UXTW */
> +    }
> +}

If this is is going to be a magic number we pass into our code
generation we could at least wrap it in a confined enum rather than a
bare int we chuck around.

> +
>  static void tcg_out_qemu_ld(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
>                              MemOpIdx oi, TCGType ext)
>  {
>      MemOp memop = get_memop(oi);
> -    const TCGType otype = TARGET_LONG_BITS == 64 ? TCG_TYPE_I64 : TCG_TYPE_I32;
> +    int option = guest_ext_option();
> +
>  #ifdef CONFIG_SOFTMMU
>      unsigned mem_index = get_mmuidx(oi);
>      tcg_insn_unit *label_ptr;
>  
>      tcg_out_tlb_read(s, addr_reg, memop, &label_ptr, mem_index, 1);
>      tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
> -                           TCG_REG_X1, otype, addr_reg);
> +                           TCG_REG_X1, option, addr_reg);
>      add_qemu_ldst_label(s, true, oi, ext, data_reg, addr_reg,
>                          s->code_ptr, label_ptr);
>  #else /* !CONFIG_SOFTMMU */
>      if (USE_GUEST_BASE) {
>          tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
> -                               TCG_REG_GUEST_BASE, otype, addr_reg);
> +                               TCG_REG_GUEST_BASE, option, addr_reg);
>      } else {
>          tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
> -                               addr_reg, TCG_TYPE_I64, TCG_REG_XZR);
> +                               addr_reg, option, TCG_REG_XZR);
>      }
>  #endif /* CONFIG_SOFTMMU */
>  }
> @@ -1806,23 +1825,24 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
>                              MemOpIdx oi)
>  {
>      MemOp memop = get_memop(oi);
> -    const TCGType otype = TARGET_LONG_BITS == 64 ? TCG_TYPE_I64 : TCG_TYPE_I32;
> +    int option = guest_ext_option();
> +
>  #ifdef CONFIG_SOFTMMU
>      unsigned mem_index = get_mmuidx(oi);
>      tcg_insn_unit *label_ptr;
>  
>      tcg_out_tlb_read(s, addr_reg, memop, &label_ptr, mem_index, 0);
>      tcg_out_qemu_st_direct(s, memop, data_reg,
> -                           TCG_REG_X1, otype, addr_reg);
> +                           TCG_REG_X1, option, addr_reg);
>      add_qemu_ldst_label(s, false, oi, (memop & MO_SIZE)== MO_64,
>                          data_reg, addr_reg, s->code_ptr, label_ptr);
>  #else /* !CONFIG_SOFTMMU */
>      if (USE_GUEST_BASE) {
>          tcg_out_qemu_st_direct(s, memop, data_reg,
> -                               TCG_REG_GUEST_BASE, otype, addr_reg);
> +                               TCG_REG_GUEST_BASE, option, addr_reg);
>      } else {
>          tcg_out_qemu_st_direct(s, memop, data_reg,
> -                               addr_reg, TCG_TYPE_I64, TCG_REG_XZR);
> +                               addr_reg, option, TCG_REG_XZR);
>      }
>  #endif /* CONFIG_SOFTMMU */
>  }


-- 
Alex Bennée


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

* Re: [PATCH 6/8] tcg/aarch64: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-11 10:28   ` Alex Bennée
@ 2021-10-11 15:24     ` Richard Henderson
  2021-10-13 21:05     ` Richard Henderson
  1 sibling, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-10-11 15:24 UTC (permalink / raw)
  To: Alex Bennée; +Cc: git, Alistair.Francis, f4bug, qemu-devel

On 10/11/21 3:28 AM, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> AArch64 has both sign and zero-extending addressing modes, which
>> means that either treatment of guest addresses is equally efficient.
>> Enabling this for AArch64 gives us testing of the feature in CI.
> 
> So which guests front ends will exercise this backend?

All 32-bit guests.

> Is this something we can exercise in 32 bit user mode tests?

Yes.

Which is why I enabled this for aarch64, so that we'd have a major platform testing it daily.


r~


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

* Re: [PATCH 3/8] accel/tcg: Support TCG_TARGET_SIGNED_ADDR32 for softmmu
  2021-10-11  4:30   ` WANG Xuerui
@ 2021-10-11 15:27     ` Richard Henderson
  0 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-10-11 15:27 UTC (permalink / raw)
  To: WANG Xuerui, qemu-devel; +Cc: Alistair.Francis, f4bug

On 10/10/21 9:30 PM, WANG Xuerui wrote:
>> @@ -92,6 +93,9 @@ static inline size_t sizeof_tlb(CPUTLBDescFast *fast)
>>   
>>   static inline uintptr_t g2h_tlbe(const CPUTLBEntry *tlb, target_ulong gaddr)
>>   {
>> +    if (TCG_TARGET_SIGNED_ADDR32 && TARGET_LONG_BITS == 32) {
> It seems this branch's direction should always match that of the branch
> added below, so if TARGET_LONG_BITS == TARGET_LONG_BITS == 32 this
> invariant is broken? Or is this expected behavior?

The conditions should match, yes.

In revising the patch set I decided that the tcg backend should simply not set this flag 
for a 32-bit host.


r~


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

* Re: [PATCH 5/8] linux-user: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-11 10:22   ` Alex Bennée
@ 2021-10-11 15:32     ` Richard Henderson
  0 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-10-11 15:32 UTC (permalink / raw)
  To: Alex Bennée; +Cc: git, Alistair.Francis, f4bug, qemu-devel

On 10/11/21 3:22 AM, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> When using reserved_va, which is the default for a 64-bit host
>> and a 32-bit guest, set guest_base_signed_addr32 if requested
>> by TCG_TARGET_SIGNED_ADDR32, and the executable layout allows.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   include/exec/cpu-all.h |  4 ---
>>   linux-user/elfload.c   | 62 ++++++++++++++++++++++++++++++++++--------
>>   2 files changed, 50 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
>> index 80b5e17329..71d8e1de7a 100644
>> --- a/include/exec/cpu-all.h
>> +++ b/include/exec/cpu-all.h
>> @@ -278,11 +278,7 @@ extern intptr_t qemu_host_page_mask;
>>   #define PAGE_RESET     0x0040
>>   /* For linux-user, indicates that the page is MAP_ANON. */
>>   #define PAGE_ANON      0x0080
>> -
>> -#if defined(CONFIG_BSD) && defined(CONFIG_USER_ONLY)
>> -/* FIXME: Code that sets/uses this is broken and needs to go away.  */
>>   #define PAGE_RESERVED  0x0100
>> -#endif
> 
> Can we reference why this FIXME is being dropped in the commit message?

I'm not sure to what pbrook was referring with "... and is already broken" there.  I need 
something here to reserve a page, PAGE_RESERVED seems like a good name, so I took it out 
of the cupboard.

I'll do some archaeology.


r~

> Looking at the current tree state I can see several uses of it due to
> moves in 5b6dd8683d (exec: move TB handling to translate-all.c) which
> post-date 2e9a5713f0 (Remove PAGE_RESERVED).
> 
> Otherwise looks reasonable:
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 



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

* Re: [PATCH 2/8] accel/tcg: Split out g2h_tlbe
  2021-10-10 17:43 ` [PATCH 2/8] accel/tcg: Split out g2h_tlbe Richard Henderson
  2021-10-11  4:22   ` WANG Xuerui
  2021-10-11  9:55   ` Alex Bennée
@ 2021-10-11 21:48   ` Philippe Mathieu-Daudé
  2021-10-11 23:19   ` Alistair Francis
  3 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-11 21:48 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: git, Alistair.Francis

On 10/10/21 19:43, Richard Henderson wrote:
> Create a new function to combine a CPUTLBEntry addend
> with the guest address to form a host address.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/cputlb.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


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

* Re: [PATCH 8/8] target/riscv: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:44 ` [PATCH 8/8] target/riscv: " Richard Henderson
@ 2021-10-11 22:00   ` Philippe Mathieu-Daudé
  2021-10-13  7:08   ` Alistair Francis
  1 sibling, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-11 22:00 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: git, Alistair.Francis

On 10/10/21 19:44, Richard Henderson wrote:
> All RV64 32-bit operations sign-extend the output, so we are easily
> able to keep TCG_TYPE_I32 values sign-extended in host registers.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/riscv/tcg-target-sa32.h | 6 +++++-
>  tcg/riscv/tcg-target.c.inc  | 8 ++------
>  2 files changed, 7 insertions(+), 7 deletions(-)

Nice.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


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

* Re: [PATCH 4/8] accel/tcg: Add guest_base_signed_addr32 for user-only
  2021-10-10 17:43 ` [PATCH 4/8] accel/tcg: Add guest_base_signed_addr32 for user-only Richard Henderson
@ 2021-10-11 22:06   ` Philippe Mathieu-Daudé
  2021-10-13  7:07   ` Alistair Francis
  1 sibling, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-11 22:06 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: git, Alistair.Francis

On 10/10/21 19:43, Richard Henderson wrote:
> While the host may prefer to treat 32-bit addresses as signed,
> there are edge cases of guests that cannot be implemented with
> addresses 0x7fff_ffff and 0x8000_0000 being non-consecutive.
> 
> Therefore, default to guest_base_signed_addr32 false, and allow
> probe_guest_base to determine whether it is possible to set it
> to true.  A tcg backend which sets TCG_TARGET_SIGNED_ADDR32 will
> have to cope with either setting for user-only.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/cpu-all.h  | 16 ++++++++++++++++
>  include/exec/cpu_ldst.h |  3 ++-
>  bsd-user/main.c         |  4 ++++
>  linux-user/main.c       |  3 +++
>  4 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 32cfb634c6..80b5e17329 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -146,6 +146,7 @@ static inline void tswap64s(uint64_t *s)
>  
>  #if defined(CONFIG_USER_ONLY)
>  #include "exec/user/abitypes.h"
> +#include "tcg-target-sa32.h"

Unrelated but this header could be simplified by moving this
block to a new header such "exec/user/address.h".

>  
>  /* On some host systems the guest address space is reserved on the host.
>   * This allows the guest address space to be offset to a convenient location.
> @@ -154,6 +155,21 @@ extern uintptr_t guest_base;
>  extern bool have_guest_base;
>  extern unsigned long reserved_va;
>  
> +#if TCG_TARGET_SIGNED_ADDR32 && TARGET_LONG_BITS == 32
> +extern bool guest_base_signed_addr32;
> +#else
> +#define guest_base_signed_addr32  false
> +#endif
> +
> +static inline void set_guest_base_signed_addr32(void)
> +{
> +#ifdef guest_base_signed_addr32
> +    qemu_build_not_reached();
> +#else
> +    guest_base_signed_addr32 = true;
> +#endif
> +}
> +
>  /*
>   * Limit the guest addresses as best we can.
>   *

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


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

* Re: [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:43 ` [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32 Richard Henderson
  2021-10-11  4:21   ` WANG Xuerui
  2021-10-11  9:55   ` Alex Bennée
@ 2021-10-11 22:07   ` Philippe Mathieu-Daudé
  2021-10-11 23:16   ` Alistair Francis
  3 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-10-11 22:07 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: git, Alistair.Francis

On 10/10/21 19:43, Richard Henderson wrote:
> Define as 0 for all tcg hosts.  Put this in a separate header,
> because we'll want this in places that do not ordinarily have
> access to all of tcg/tcg.h.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/aarch64/tcg-target-sa32.h | 1 +
>  tcg/arm/tcg-target-sa32.h     | 1 +
>  tcg/i386/tcg-target-sa32.h    | 1 +
>  tcg/mips/tcg-target-sa32.h    | 1 +
>  tcg/ppc/tcg-target-sa32.h     | 1 +
>  tcg/riscv/tcg-target-sa32.h   | 1 +
>  tcg/s390x/tcg-target-sa32.h   | 1 +
>  tcg/sparc/tcg-target-sa32.h   | 1 +
>  tcg/tci/tcg-target-sa32.h     | 1 +
>  9 files changed, 9 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


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

* Re: [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:43 ` [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32 Richard Henderson
                     ` (2 preceding siblings ...)
  2021-10-11 22:07   ` Philippe Mathieu-Daudé
@ 2021-10-11 23:16   ` Alistair Francis
  3 siblings, 0 replies; 30+ messages in thread
From: Alistair Francis @ 2021-10-11 23:16 UTC (permalink / raw)
  To: Richard Henderson
  Cc: git, Alistair Francis, qemu-devel@nongnu.org Developers,
	Philippe Mathieu-Daudé

On Mon, Oct 11, 2021 at 3:49 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Define as 0 for all tcg hosts.  Put this in a separate header,
> because we'll want this in places that do not ordinarily have
> access to all of tcg/tcg.h.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  tcg/aarch64/tcg-target-sa32.h | 1 +
>  tcg/arm/tcg-target-sa32.h     | 1 +
>  tcg/i386/tcg-target-sa32.h    | 1 +
>  tcg/mips/tcg-target-sa32.h    | 1 +
>  tcg/ppc/tcg-target-sa32.h     | 1 +
>  tcg/riscv/tcg-target-sa32.h   | 1 +
>  tcg/s390x/tcg-target-sa32.h   | 1 +
>  tcg/sparc/tcg-target-sa32.h   | 1 +
>  tcg/tci/tcg-target-sa32.h     | 1 +
>  9 files changed, 9 insertions(+)
>  create mode 100644 tcg/aarch64/tcg-target-sa32.h
>  create mode 100644 tcg/arm/tcg-target-sa32.h
>  create mode 100644 tcg/i386/tcg-target-sa32.h
>  create mode 100644 tcg/mips/tcg-target-sa32.h
>  create mode 100644 tcg/ppc/tcg-target-sa32.h
>  create mode 100644 tcg/riscv/tcg-target-sa32.h
>  create mode 100644 tcg/s390x/tcg-target-sa32.h
>  create mode 100644 tcg/sparc/tcg-target-sa32.h
>  create mode 100644 tcg/tci/tcg-target-sa32.h
>
> diff --git a/tcg/aarch64/tcg-target-sa32.h b/tcg/aarch64/tcg-target-sa32.h
> new file mode 100644
> index 0000000000..cb185b1526
> --- /dev/null
> +++ b/tcg/aarch64/tcg-target-sa32.h
> @@ -0,0 +1 @@
> +#define TCG_TARGET_SIGNED_ADDR32 0
> diff --git a/tcg/arm/tcg-target-sa32.h b/tcg/arm/tcg-target-sa32.h
> new file mode 100644
> index 0000000000..cb185b1526
> --- /dev/null
> +++ b/tcg/arm/tcg-target-sa32.h
> @@ -0,0 +1 @@
> +#define TCG_TARGET_SIGNED_ADDR32 0
> diff --git a/tcg/i386/tcg-target-sa32.h b/tcg/i386/tcg-target-sa32.h
> new file mode 100644
> index 0000000000..cb185b1526
> --- /dev/null
> +++ b/tcg/i386/tcg-target-sa32.h
> @@ -0,0 +1 @@
> +#define TCG_TARGET_SIGNED_ADDR32 0
> diff --git a/tcg/mips/tcg-target-sa32.h b/tcg/mips/tcg-target-sa32.h
> new file mode 100644
> index 0000000000..cb185b1526
> --- /dev/null
> +++ b/tcg/mips/tcg-target-sa32.h
> @@ -0,0 +1 @@
> +#define TCG_TARGET_SIGNED_ADDR32 0
> diff --git a/tcg/ppc/tcg-target-sa32.h b/tcg/ppc/tcg-target-sa32.h
> new file mode 100644
> index 0000000000..cb185b1526
> --- /dev/null
> +++ b/tcg/ppc/tcg-target-sa32.h
> @@ -0,0 +1 @@
> +#define TCG_TARGET_SIGNED_ADDR32 0
> diff --git a/tcg/riscv/tcg-target-sa32.h b/tcg/riscv/tcg-target-sa32.h
> new file mode 100644
> index 0000000000..cb185b1526
> --- /dev/null
> +++ b/tcg/riscv/tcg-target-sa32.h
> @@ -0,0 +1 @@
> +#define TCG_TARGET_SIGNED_ADDR32 0
> diff --git a/tcg/s390x/tcg-target-sa32.h b/tcg/s390x/tcg-target-sa32.h
> new file mode 100644
> index 0000000000..cb185b1526
> --- /dev/null
> +++ b/tcg/s390x/tcg-target-sa32.h
> @@ -0,0 +1 @@
> +#define TCG_TARGET_SIGNED_ADDR32 0
> diff --git a/tcg/sparc/tcg-target-sa32.h b/tcg/sparc/tcg-target-sa32.h
> new file mode 100644
> index 0000000000..cb185b1526
> --- /dev/null
> +++ b/tcg/sparc/tcg-target-sa32.h
> @@ -0,0 +1 @@
> +#define TCG_TARGET_SIGNED_ADDR32 0
> diff --git a/tcg/tci/tcg-target-sa32.h b/tcg/tci/tcg-target-sa32.h
> new file mode 100644
> index 0000000000..cb185b1526
> --- /dev/null
> +++ b/tcg/tci/tcg-target-sa32.h
> @@ -0,0 +1 @@
> +#define TCG_TARGET_SIGNED_ADDR32 0
> --
> 2.25.1
>
>


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

* Re: [PATCH 2/8] accel/tcg: Split out g2h_tlbe
  2021-10-10 17:43 ` [PATCH 2/8] accel/tcg: Split out g2h_tlbe Richard Henderson
                     ` (2 preceding siblings ...)
  2021-10-11 21:48   ` Philippe Mathieu-Daudé
@ 2021-10-11 23:19   ` Alistair Francis
  3 siblings, 0 replies; 30+ messages in thread
From: Alistair Francis @ 2021-10-11 23:19 UTC (permalink / raw)
  To: Richard Henderson
  Cc: git, Alistair Francis, qemu-devel@nongnu.org Developers,
	Philippe Mathieu-Daudé

On Mon, Oct 11, 2021 at 3:44 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Create a new function to combine a CPUTLBEntry addend
> with the guest address to form a host address.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/tcg/cputlb.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 46140ccff3..761f726722 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -90,6 +90,11 @@ static inline size_t sizeof_tlb(CPUTLBDescFast *fast)
>      return fast->mask + (1 << CPU_TLB_ENTRY_BITS);
>  }
>
> +static inline uintptr_t g2h_tlbe(const CPUTLBEntry *tlb, target_ulong gaddr)
> +{
> +    return tlb->addend + (uintptr_t)gaddr;
> +}
> +
>  static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
>                               size_t max_entries)
>  {
> @@ -976,8 +981,7 @@ static void tlb_reset_dirty_range_locked(CPUTLBEntry *tlb_entry,
>
>      if ((addr & (TLB_INVALID_MASK | TLB_MMIO |
>                   TLB_DISCARD_WRITE | TLB_NOTDIRTY)) == 0) {
> -        addr &= TARGET_PAGE_MASK;
> -        addr += tlb_entry->addend;
> +        addr = g2h_tlbe(tlb_entry, addr & TARGET_PAGE_MASK);
>          if ((addr - start) < length) {
>  #if TCG_OVERSIZED_GUEST
>              tlb_entry->addr_write |= TLB_NOTDIRTY;
> @@ -1527,7 +1531,7 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
>          return -1;
>      }
>
> -    p = (void *)((uintptr_t)addr + entry->addend);
> +    p = (void *)g2h_tlbe(entry, addr);
>      if (hostp) {
>          *hostp = p;
>      }
> @@ -1619,7 +1623,7 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr,
>      }
>
>      /* Everything else is RAM. */
> -    *phost = (void *)((uintptr_t)addr + entry->addend);
> +    *phost = (void *)g2h_tlbe(entry, addr);
>      return flags;
>  }
>
> @@ -1727,7 +1731,7 @@ bool tlb_plugin_lookup(CPUState *cpu, target_ulong addr, int mmu_idx,
>              data->v.io.offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr;
>          } else {
>              data->is_io = false;
> -            data->v.ram.hostaddr = (void *)((uintptr_t)addr + tlbe->addend);
> +            data->v.ram.hostaddr = (void *)g2h_tlbe(tlbe, addr);
>          }
>          return true;
>      } else {
> @@ -1826,7 +1830,7 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
>          goto stop_the_world;
>      }
>
> -    hostaddr = (void *)((uintptr_t)addr + tlbe->addend);
> +    hostaddr = (void *)g2h_tlbe(tlbe, addr);
>
>      if (unlikely(tlb_addr & TLB_NOTDIRTY)) {
>          notdirty_write(env_cpu(env), addr, size,
> @@ -1938,7 +1942,7 @@ load_helper(CPUArchState *env, target_ulong addr, MemOpIdx oi,
>                              access_type, op ^ (need_swap * MO_BSWAP));
>          }
>
> -        haddr = (void *)((uintptr_t)addr + entry->addend);
> +        haddr = (void *)g2h_tlbe(entry, addr);
>
>          /*
>           * Keep these two load_memop separate to ensure that the compiler
> @@ -1975,7 +1979,7 @@ load_helper(CPUArchState *env, target_ulong addr, MemOpIdx oi,
>          return res & MAKE_64BIT_MASK(0, size * 8);
>      }
>
> -    haddr = (void *)((uintptr_t)addr + entry->addend);
> +    haddr = (void *)g2h_tlbe(entry, addr);
>      return load_memop(haddr, op);
>  }
>
> @@ -2467,7 +2471,7 @@ store_helper(CPUArchState *env, target_ulong addr, uint64_t val,
>              notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr);
>          }
>
> -        haddr = (void *)((uintptr_t)addr + entry->addend);
> +        haddr = (void *)g2h_tlbe(entry, addr);
>
>          /*
>           * Keep these two store_memop separate to ensure that the compiler
> @@ -2492,7 +2496,7 @@ store_helper(CPUArchState *env, target_ulong addr, uint64_t val,
>          return;
>      }
>
> -    haddr = (void *)((uintptr_t)addr + entry->addend);
> +    haddr = (void *)g2h_tlbe(entry, addr);
>      store_memop(haddr, val, op);
>  }
>
> --
> 2.25.1
>
>


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

* Re: [PATCH 4/8] accel/tcg: Add guest_base_signed_addr32 for user-only
  2021-10-10 17:43 ` [PATCH 4/8] accel/tcg: Add guest_base_signed_addr32 for user-only Richard Henderson
  2021-10-11 22:06   ` Philippe Mathieu-Daudé
@ 2021-10-13  7:07   ` Alistair Francis
  1 sibling, 0 replies; 30+ messages in thread
From: Alistair Francis @ 2021-10-13  7:07 UTC (permalink / raw)
  To: Richard Henderson
  Cc: git, Alistair Francis, qemu-devel@nongnu.org Developers,
	Philippe Mathieu-Daudé

On Mon, Oct 11, 2021 at 3:52 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> While the host may prefer to treat 32-bit addresses as signed,
> there are edge cases of guests that cannot be implemented with
> addresses 0x7fff_ffff and 0x8000_0000 being non-consecutive.
>
> Therefore, default to guest_base_signed_addr32 false, and allow
> probe_guest_base to determine whether it is possible to set it
> to true.  A tcg backend which sets TCG_TARGET_SIGNED_ADDR32 will
> have to cope with either setting for user-only.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair


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

* Re: [PATCH 8/8] target/riscv: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-10 17:44 ` [PATCH 8/8] target/riscv: " Richard Henderson
  2021-10-11 22:00   ` Philippe Mathieu-Daudé
@ 2021-10-13  7:08   ` Alistair Francis
  1 sibling, 0 replies; 30+ messages in thread
From: Alistair Francis @ 2021-10-13  7:08 UTC (permalink / raw)
  To: Richard Henderson
  Cc: git, Alistair Francis, qemu-devel@nongnu.org Developers,
	Philippe Mathieu-Daudé

On Mon, Oct 11, 2021 at 3:50 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> All RV64 32-bit operations sign-extend the output, so we are easily
> able to keep TCG_TYPE_I32 values sign-extended in host registers.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  tcg/riscv/tcg-target-sa32.h | 6 +++++-
>  tcg/riscv/tcg-target.c.inc  | 8 ++------
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tcg/riscv/tcg-target-sa32.h b/tcg/riscv/tcg-target-sa32.h
> index cb185b1526..703467b37a 100644
> --- a/tcg/riscv/tcg-target-sa32.h
> +++ b/tcg/riscv/tcg-target-sa32.h
> @@ -1 +1,5 @@
> -#define TCG_TARGET_SIGNED_ADDR32 0
> +/*
> + * Do not set TCG_TARGET_SIGNED_ADDR32 for RV32;
> + * TCG expects this to only be set for 64-bit hosts.
> + */
> +#define TCG_TARGET_SIGNED_ADDR32  (__riscv_xlen == 64)
> diff --git a/tcg/riscv/tcg-target.c.inc b/tcg/riscv/tcg-target.c.inc
> index 9b13a46fb4..9426ef8926 100644
> --- a/tcg/riscv/tcg-target.c.inc
> +++ b/tcg/riscv/tcg-target.c.inc
> @@ -952,10 +952,6 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg addrl,
>      tcg_out_opc_branch(s, OPC_BNE, TCG_REG_TMP0, TCG_REG_TMP1, 0);
>
>      /* TLB Hit - translate address using addend.  */
> -    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
> -        tcg_out_ext32u(s, TCG_REG_TMP0, addrl);
> -        addrl = TCG_REG_TMP0;
> -    }
>      tcg_out_opc_reg(s, OPC_ADD, TCG_REG_TMP0, TCG_REG_TMP2, addrl);
>  }
>
> @@ -1126,7 +1122,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is_64)
>                          data_regl, data_regh, addr_regl, addr_regh,
>                          s->code_ptr, label_ptr);
>  #else
> -    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
> +    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) {
>          tcg_out_ext32u(s, base, addr_regl);
>          addr_regl = base;
>      }
> @@ -1192,7 +1188,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is_64)
>                          data_regl, data_regh, addr_regl, addr_regh,
>                          s->code_ptr, label_ptr);
>  #else
> -    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
> +    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) {
>          tcg_out_ext32u(s, base, addr_regl);
>          addr_regl = base;
>      }
> --
> 2.25.1
>
>


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

* Re: [PATCH 6/8] tcg/aarch64: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-11 10:28   ` Alex Bennée
  2021-10-11 15:24     ` Richard Henderson
@ 2021-10-13 21:05     ` Richard Henderson
  1 sibling, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-10-13 21:05 UTC (permalink / raw)
  To: Alex Bennée; +Cc: git, Alistair.Francis, f4bug, qemu-devel

On 10/11/21 3:28 AM, Alex Bennée wrote:
>> +    if (TARGET_LONG_BITS == 64) {
>> +        return 3; /* LSL #0 */
>> +    } else if (signed_addr32) {
>> +        return 6; /* SXTW */
>> +    } else {
>> +        return 2; /* UXTW */
>> +    }
>> +}
> 
> If this is is going to be a magic number we pass into our code
> generation we could at least wrap it in a confined enum rather than a
> bare int we chuck around.

Given that it's used exactly one, and commented, and matches the ARM, do we really need an 
enum?


r~


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

* Re: [PATCH 7/8] target/mips: Support TCG_TARGET_SIGNED_ADDR32
  2021-10-11  4:20   ` WANG Xuerui
@ 2021-10-13 22:24     ` Richard Henderson
  0 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-10-13 22:24 UTC (permalink / raw)
  To: WANG Xuerui, qemu-devel; +Cc: Alistair.Francis, f4bug

On 10/10/21 9:20 PM, WANG Xuerui wrote:
>> +++ b/tcg/mips/tcg-target-sa32.h
>> @@ -1 +1,9 @@
>> +/*
>> + * Do not set TCG_TARGET_SIGNED_ADDR32 for mips32;
>> + * TCG expects this to only be set for 64-bit hosts.
>> + */
>> +#ifdef __mips64
>> +#define TCG_TARGET_SIGNED_ADDR32 1
>> +#else
>>   #define TCG_TARGET_SIGNED_ADDR32 0
>> +#endif
> It looks like we never want to set TCG_TARGET_SIGNED_ADDR32 on 32-bit
> hosts; maybe a compile-time assert could be added somewhere for
> statically guaranteeing this?

I've placed a build-time assert in tcg/tcg.c.

>> -    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
>> +    if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS && !guest_base_signed_addr32) {
> Is there precedent of extracting predicates like this into some header
> for reuse? However I agree that the current expression conveys enough
> meaning without being overly complicated.

Depends on the expected scope of the predicate, I guess.

If it's private to tcg-target.c.inc, I'd put it in tcg.c.
If it's private to tcg in general, I'd put it in tcg-internal.h.
Beyond that, I guess it depends.

For this, I don't know what I'd call it that isn't more verbose than the expression itself.

r~


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

end of thread, other threads:[~2021-10-13 22:25 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-10 17:43 [PATCH 0/8] tcg: support 32-bit guest addresses as signed Richard Henderson
2021-10-10 17:43 ` [PATCH 1/8] tcg: Add TCG_TARGET_SIGNED_ADDR32 Richard Henderson
2021-10-11  4:21   ` WANG Xuerui
2021-10-11  9:55   ` Alex Bennée
2021-10-11 22:07   ` Philippe Mathieu-Daudé
2021-10-11 23:16   ` Alistair Francis
2021-10-10 17:43 ` [PATCH 2/8] accel/tcg: Split out g2h_tlbe Richard Henderson
2021-10-11  4:22   ` WANG Xuerui
2021-10-11  9:55   ` Alex Bennée
2021-10-11 21:48   ` Philippe Mathieu-Daudé
2021-10-11 23:19   ` Alistair Francis
2021-10-10 17:43 ` [PATCH 3/8] accel/tcg: Support TCG_TARGET_SIGNED_ADDR32 for softmmu Richard Henderson
2021-10-11  4:30   ` WANG Xuerui
2021-10-11 15:27     ` Richard Henderson
2021-10-10 17:43 ` [PATCH 4/8] accel/tcg: Add guest_base_signed_addr32 for user-only Richard Henderson
2021-10-11 22:06   ` Philippe Mathieu-Daudé
2021-10-13  7:07   ` Alistair Francis
2021-10-10 17:43 ` [PATCH 5/8] linux-user: Support TCG_TARGET_SIGNED_ADDR32 Richard Henderson
2021-10-11 10:22   ` Alex Bennée
2021-10-11 15:32     ` Richard Henderson
2021-10-10 17:43 ` [PATCH 6/8] tcg/aarch64: " Richard Henderson
2021-10-11 10:28   ` Alex Bennée
2021-10-11 15:24     ` Richard Henderson
2021-10-13 21:05     ` Richard Henderson
2021-10-10 17:44 ` [PATCH 7/8] target/mips: " Richard Henderson
2021-10-11  4:20   ` WANG Xuerui
2021-10-13 22:24     ` Richard Henderson
2021-10-10 17:44 ` [PATCH 8/8] target/riscv: " Richard Henderson
2021-10-11 22:00   ` Philippe Mathieu-Daudé
2021-10-13  7:08   ` Alistair Francis

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