* [Qemu-devel] [PATCH for-2.4 0/2] tcg/i386 address zero-extension
@ 2015-07-16 21:25 Richard Henderson
2015-07-16 21:25 ` [Qemu-devel] [PATCH 1/2] tcg/i386: Extend addresses for 32-bit guests Richard Henderson
2015-07-16 21:25 ` [Qemu-devel] [PATCH 2/2] tcg/i386: Reserve register for guest_base if a segment isn't available Richard Henderson
0 siblings, 2 replies; 9+ messages in thread
From: Richard Henderson @ 2015-07-16 21:25 UTC (permalink / raw)
To: qemu-devel; +Cc: leon.alrae, aurelien
This is an alternative to the patch that Aurelien posted yesterday.
r~
Richard Henderson (2):
tcg/i386: Extend addresses for 32-bit guests
tcg/i386: Reserve register for guest_base if a segment isn't available
tcg/i386/tcg-target.c | 163 ++++++++++++++++++++++++++++----------------------
1 file changed, 90 insertions(+), 73 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 1/2] tcg/i386: Extend addresses for 32-bit guests
2015-07-16 21:25 [Qemu-devel] [PATCH for-2.4 0/2] tcg/i386 address zero-extension Richard Henderson
@ 2015-07-16 21:25 ` Richard Henderson
2015-07-17 10:18 ` Aurelien Jarno
2015-07-16 21:25 ` [Qemu-devel] [PATCH 2/2] tcg/i386: Reserve register for guest_base if a segment isn't available Richard Henderson
1 sibling, 1 reply; 9+ messages in thread
From: Richard Henderson @ 2015-07-16 21:25 UTC (permalink / raw)
To: qemu-devel; +Cc: leon.alrae, aurelien
Removing the ??? comment explaining why it (mostly) worked.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
tcg/i386/tcg-target.c | 105 +++++++++++++++++++++++++++++++-------------------
1 file changed, 65 insertions(+), 40 deletions(-)
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index ff4d9cf..bbe2963 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -1434,8 +1434,8 @@ static inline void setup_guest_base_seg(void) { }
#endif /* SOFTMMU */
static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
- TCGReg base, intptr_t ofs, int seg,
- TCGMemOp memop)
+ TCGReg base, int index, intptr_t ofs,
+ int seg, TCGMemOp memop)
{
const TCGMemOp real_bswap = memop & MO_BSWAP;
TCGMemOp bswap = real_bswap;
@@ -1448,13 +1448,16 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
switch (memop & MO_SSIZE) {
case MO_UB:
- tcg_out_modrm_offset(s, OPC_MOVZBL + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, OPC_MOVZBL + seg, datalo,
+ base, index, 0, ofs);
break;
case MO_SB:
- tcg_out_modrm_offset(s, OPC_MOVSBL + P_REXW + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, OPC_MOVSBL + P_REXW + seg, datalo,
+ base, index, 0, ofs);
break;
case MO_UW:
- tcg_out_modrm_offset(s, OPC_MOVZWL + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, OPC_MOVZWL + seg, datalo,
+ base, index, 0, ofs);
if (real_bswap) {
tcg_out_rolw_8(s, datalo);
}
@@ -1462,20 +1465,21 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
case MO_SW:
if (real_bswap) {
if (have_movbe) {
- tcg_out_modrm_offset(s, OPC_MOVBE_GyMy + P_DATA16 + seg,
- datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, OPC_MOVBE_GyMy + P_DATA16 + seg,
+ datalo, base, index, 0, ofs);
} else {
- tcg_out_modrm_offset(s, OPC_MOVZWL + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, OPC_MOVZWL + seg, datalo,
+ base, index, 0, ofs);
tcg_out_rolw_8(s, datalo);
}
tcg_out_modrm(s, OPC_MOVSWL + P_REXW, datalo, datalo);
} else {
- tcg_out_modrm_offset(s, OPC_MOVSWL + P_REXW + seg,
- datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, OPC_MOVSWL + P_REXW + seg,
+ datalo, base, index, 0, ofs);
}
break;
case MO_UL:
- tcg_out_modrm_offset(s, movop + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, movop + seg, datalo, base, index, 0, ofs);
if (bswap) {
tcg_out_bswap32(s, datalo);
}
@@ -1483,19 +1487,22 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
#if TCG_TARGET_REG_BITS == 64
case MO_SL:
if (real_bswap) {
- tcg_out_modrm_offset(s, movop + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, movop + seg, datalo,
+ base, index, 0, ofs);
if (bswap) {
tcg_out_bswap32(s, datalo);
}
tcg_out_ext32s(s, datalo, datalo);
} else {
- tcg_out_modrm_offset(s, OPC_MOVSLQ + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, OPC_MOVSLQ + seg, datalo,
+ base, index, 0, ofs);
}
break;
#endif
case MO_Q:
if (TCG_TARGET_REG_BITS == 64) {
- tcg_out_modrm_offset(s, movop + P_REXW + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, movop + P_REXW + seg, datalo,
+ base, index, 0, ofs);
if (bswap) {
tcg_out_bswap64(s, datalo);
}
@@ -1506,11 +1513,15 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
datahi = t;
}
if (base != datalo) {
- tcg_out_modrm_offset(s, movop + seg, datalo, base, ofs);
- tcg_out_modrm_offset(s, movop + seg, datahi, base, ofs + 4);
+ tcg_out_modrm_sib_offset(s, movop + seg, datalo,
+ base, index, 0, ofs);
+ tcg_out_modrm_sib_offset(s, movop + seg, datahi,
+ base, index, 0, ofs + 4);
} else {
- tcg_out_modrm_offset(s, movop + seg, datahi, base, ofs + 4);
- tcg_out_modrm_offset(s, movop + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, movop + seg, datahi,
+ base, index, 0, ofs + 4);
+ tcg_out_modrm_sib_offset(s, movop + seg, datalo,
+ base, index, 0, ofs);
}
if (bswap) {
tcg_out_bswap32(s, datalo);
@@ -1553,7 +1564,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
label_ptr, offsetof(CPUTLBEntry, addr_read));
/* TLB Hit. */
- tcg_out_qemu_ld_direct(s, datalo, datahi, TCG_REG_L1, 0, 0, opc);
+ tcg_out_qemu_ld_direct(s, datalo, datahi, TCG_REG_L1, -1, 0, 0, opc);
/* Record the current context of a load into ldst label */
add_qemu_ldst_label(s, true, oi, datalo, datahi, addrlo, addrhi,
@@ -1562,24 +1573,29 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
{
int32_t offset = GUEST_BASE;
TCGReg base = addrlo;
+ int index = -1;
int seg = 0;
- /* ??? We assume all operations have left us with register contents
- that are zero extended. So far this appears to be true. If we
- want to enforce this, we can either do an explicit zero-extension
- here, or (if GUEST_BASE == 0, or a segment register is in use)
- use the ADDR32 prefix. For now, do nothing. */
if (GUEST_BASE && guest_base_flags) {
seg = guest_base_flags;
offset = 0;
- } else if (TCG_TARGET_REG_BITS == 64 && offset != GUEST_BASE) {
- tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L1, GUEST_BASE);
- tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L1, base);
- base = TCG_REG_L1;
- offset = 0;
+ if (TCG_TARGET_REG_BITS == 64 && TARGET_LONG_BITS == 32) {
+ seg |= P_ADDR32;
+ }
+ } else if (TCG_TARGET_REG_BITS == 64) {
+ if (TARGET_LONG_BITS == 32) {
+ tcg_out_ext32u(s, TCG_REG_L0, base);
+ base = TCG_REG_L0;
+ }
+ if (offset != GUEST_BASE) {
+ tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L1, GUEST_BASE);
+ index = TCG_REG_L1;
+ offset = 0;
+ }
}
- tcg_out_qemu_ld_direct(s, datalo, datahi, base, offset, seg, opc);
+ tcg_out_qemu_ld_direct(s, datalo, datahi,
+ base, index, offset, seg, opc);
}
#endif
}
@@ -1697,19 +1713,28 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is64)
TCGReg base = addrlo;
int seg = 0;
- /* ??? We assume all operations have left us with register contents
- that are zero extended. So far this appears to be true. If we
- want to enforce this, we can either do an explicit zero-extension
- here, or (if GUEST_BASE == 0, or a segment register is in use)
- use the ADDR32 prefix. For now, do nothing. */
if (GUEST_BASE && guest_base_flags) {
seg = guest_base_flags;
offset = 0;
- } else if (TCG_TARGET_REG_BITS == 64 && offset != GUEST_BASE) {
- tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L1, GUEST_BASE);
- tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L1, base);
- base = TCG_REG_L1;
- offset = 0;
+ if (TCG_TARGET_REG_BITS == 64 && TARGET_LONG_BITS == 32) {
+ seg |= P_ADDR32;
+ }
+ } else if (TCG_TARGET_REG_BITS == 64) {
+ /* ??? Note that we can't use the same SIB addressing scheme
+ as for loads, since we require L0 free for bswap. */
+ if (offset != GUEST_BASE) {
+ if (TARGET_LONG_BITS == 32) {
+ tcg_out_ext32u(s, TCG_REG_L0, base);
+ base = TCG_REG_L0;
+ }
+ tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L1, GUEST_BASE);
+ tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L1, base);
+ base = TCG_REG_L1;
+ offset = 0;
+ } else if (TARGET_LONG_BITS == 32) {
+ tcg_out_ext32u(s, TCG_REG_L1, base);
+ base = TCG_REG_L1;
+ }
}
tcg_out_qemu_st_direct(s, datalo, datahi, base, offset, seg, opc);
--
2.4.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 2/2] tcg/i386: Reserve register for guest_base if a segment isn't available
2015-07-16 21:25 [Qemu-devel] [PATCH for-2.4 0/2] tcg/i386 address zero-extension Richard Henderson
2015-07-16 21:25 ` [Qemu-devel] [PATCH 1/2] tcg/i386: Extend addresses for 32-bit guests Richard Henderson
@ 2015-07-16 21:25 ` Richard Henderson
2015-07-17 1:33 ` Peter Maydell
2015-07-17 10:18 ` Aurelien Jarno
1 sibling, 2 replies; 9+ messages in thread
From: Richard Henderson @ 2015-07-16 21:25 UTC (permalink / raw)
To: qemu-devel; +Cc: leon.alrae, aurelien
This saves 2 insns and 10 bytes from the implementation of
each memory operation.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
tcg/i386/tcg-target.c | 120 +++++++++++++++++++++++---------------------------
1 file changed, 56 insertions(+), 64 deletions(-)
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index bbe2963..beffbbe 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -1421,16 +1421,25 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
int arch_prctl(int code, unsigned long addr);
+static int32_t guest_base_ofs;
static int guest_base_flags;
-static inline void setup_guest_base_seg(void)
+static int guest_base_reg = -1;
+static inline void setup_guest_base(TCGContext *s)
{
if (arch_prctl(ARCH_SET_GS, GUEST_BASE) == 0) {
guest_base_flags = P_GS;
+ } else if (GUEST_BASE == (int32_t)GUEST_BASE) {
+ guest_base_ofs = GUEST_BASE;
+ } else {
+ guest_base_reg = TCG_REG_EBP;
+ tcg_regset_set_reg(s->reserved_regs, guest_base_reg);
+ tcg_out_movi(s, TCG_TYPE_PTR, guest_base_reg, GUEST_BASE);
}
}
#else
-# define guest_base_flags 0
-static inline void setup_guest_base_seg(void) { }
+# define guest_base_flags 0
+# define guest_base_reg -1
+# define guest_base_ofs GUEST_BASE
#endif /* SOFTMMU */
static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
@@ -1571,38 +1580,28 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
s->code_ptr, label_ptr);
#else
{
- int32_t offset = GUEST_BASE;
TCGReg base = addrlo;
- int index = -1;
- int seg = 0;
+ int flags = 0;
- if (GUEST_BASE && guest_base_flags) {
- seg = guest_base_flags;
- offset = 0;
+ if (GUEST_BASE == 0 || guest_base_flags) {
+ flags = guest_base_flags;
if (TCG_TARGET_REG_BITS == 64 && TARGET_LONG_BITS == 32) {
- seg |= P_ADDR32;
- }
- } else if (TCG_TARGET_REG_BITS == 64) {
- if (TARGET_LONG_BITS == 32) {
- tcg_out_ext32u(s, TCG_REG_L0, base);
- base = TCG_REG_L0;
- }
- if (offset != GUEST_BASE) {
- tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L1, GUEST_BASE);
- index = TCG_REG_L1;
- offset = 0;
+ flags |= P_ADDR32;
}
+ } else if (TCG_TARGET_REG_BITS == 64 && TARGET_LONG_BITS == 32) {
+ tcg_out_ext32u(s, TCG_REG_L1, base);
+ base = TCG_REG_L1;
}
- tcg_out_qemu_ld_direct(s, datalo, datahi,
- base, index, offset, seg, opc);
+ tcg_out_qemu_ld_direct(s, datalo, datahi, base, guest_base_reg,
+ guest_base_ofs, flags, opc);
}
#endif
}
static void tcg_out_qemu_st_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
- TCGReg base, intptr_t ofs, int seg,
- TCGMemOp memop)
+ TCGReg base, int index, intptr_t ofs,
+ int seg, TCGMemOp memop)
{
/* ??? Ideally we wouldn't need a scratch register. For user-only,
we could perform the bswap twice to restore the original value
@@ -1626,8 +1625,8 @@ static void tcg_out_qemu_st_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
tcg_out_mov(s, TCG_TYPE_I32, scratch, datalo);
datalo = scratch;
}
- tcg_out_modrm_offset(s, OPC_MOVB_EvGv + P_REXB_R + seg,
- datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, OPC_MOVB_EvGv + P_REXB_R + seg, datalo,
+ base, index, 0, ofs);
break;
case MO_16:
if (bswap) {
@@ -1635,7 +1634,8 @@ static void tcg_out_qemu_st_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
tcg_out_rolw_8(s, scratch);
datalo = scratch;
}
- tcg_out_modrm_offset(s, movop + P_DATA16 + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, movop + P_DATA16 + seg, datalo,
+ base, index, 0, ofs);
break;
case MO_32:
if (bswap) {
@@ -1643,7 +1643,7 @@ static void tcg_out_qemu_st_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
tcg_out_bswap32(s, scratch);
datalo = scratch;
}
- tcg_out_modrm_offset(s, movop + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, movop + seg, datalo, base, index, 0, ofs);
break;
case MO_64:
if (TCG_TARGET_REG_BITS == 64) {
@@ -1652,22 +1652,27 @@ static void tcg_out_qemu_st_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
tcg_out_bswap64(s, scratch);
datalo = scratch;
}
- tcg_out_modrm_offset(s, movop + P_REXW + seg, datalo, base, ofs);
+ tcg_out_modrm_sib_offset(s, movop + P_REXW + seg, datalo,
+ base, index, 0, ofs);
} else if (bswap) {
tcg_out_mov(s, TCG_TYPE_I32, scratch, datahi);
tcg_out_bswap32(s, scratch);
- tcg_out_modrm_offset(s, OPC_MOVL_EvGv + seg, scratch, base, ofs);
+ tcg_out_modrm_sib_offset(s, OPC_MOVL_EvGv + seg, scratch,
+ base, index, 0, ofs);
tcg_out_mov(s, TCG_TYPE_I32, scratch, datalo);
tcg_out_bswap32(s, scratch);
- tcg_out_modrm_offset(s, OPC_MOVL_EvGv + seg, scratch, base, ofs+4);
+ tcg_out_modrm_sib_offset(s, OPC_MOVL_EvGv + seg, scratch,
+ base, index, 0, ofs+4);
} else {
if (real_bswap) {
int t = datalo;
datalo = datahi;
datahi = t;
}
- tcg_out_modrm_offset(s, movop + seg, datalo, base, ofs);
- tcg_out_modrm_offset(s, movop + seg, datahi, base, ofs+4);
+ tcg_out_modrm_sib_offset(s, movop + seg, datalo,
+ base, index, 0, ofs);
+ tcg_out_modrm_sib_offset(s, movop + seg, datahi,
+ base, index, 0, ofs+4);
}
break;
default:
@@ -1702,42 +1707,28 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is64)
label_ptr, offsetof(CPUTLBEntry, addr_write));
/* TLB Hit. */
- tcg_out_qemu_st_direct(s, datalo, datahi, TCG_REG_L1, 0, 0, opc);
+ tcg_out_qemu_st_direct(s, datalo, datahi, TCG_REG_L1, -1, 0, 0, opc);
/* Record the current context of a store into ldst label */
add_qemu_ldst_label(s, false, oi, datalo, datahi, addrlo, addrhi,
s->code_ptr, label_ptr);
#else
{
- int32_t offset = GUEST_BASE;
TCGReg base = addrlo;
- int seg = 0;
+ int flags = 0;
- if (GUEST_BASE && guest_base_flags) {
- seg = guest_base_flags;
- offset = 0;
+ if (GUEST_BASE == 0 || guest_base_flags) {
+ flags = guest_base_flags;
if (TCG_TARGET_REG_BITS == 64 && TARGET_LONG_BITS == 32) {
- seg |= P_ADDR32;
- }
- } else if (TCG_TARGET_REG_BITS == 64) {
- /* ??? Note that we can't use the same SIB addressing scheme
- as for loads, since we require L0 free for bswap. */
- if (offset != GUEST_BASE) {
- if (TARGET_LONG_BITS == 32) {
- tcg_out_ext32u(s, TCG_REG_L0, base);
- base = TCG_REG_L0;
- }
- tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L1, GUEST_BASE);
- tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L1, base);
- base = TCG_REG_L1;
- offset = 0;
- } else if (TARGET_LONG_BITS == 32) {
- tcg_out_ext32u(s, TCG_REG_L1, base);
- base = TCG_REG_L1;
+ flags |= P_ADDR32;
}
+ } else if (TCG_TARGET_REG_BITS == 64 && TARGET_LONG_BITS == 32) {
+ tcg_out_ext32u(s, TCG_REG_L1, base);
+ base = TCG_REG_L1;
}
- tcg_out_qemu_st_direct(s, datalo, datahi, base, offset, seg, opc);
+ tcg_out_qemu_st_direct(s, datalo, datahi, base, guest_base_reg,
+ guest_base_ofs, flags, opc);
}
#endif
}
@@ -2286,6 +2277,14 @@ static void tcg_target_qemu_prologue(TCGContext *s)
#else
tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, tcg_target_call_iarg_regs[0]);
tcg_out_addi(s, TCG_REG_ESP, -stack_addend);
+
+# if !defined(CONFIG_SOFTMMU)
+ /* Try to set up a segment register to point to GUEST_BASE. */
+ if (GUEST_BASE) {
+ setup_guest_base(s);
+ }
+# endif
+
/* jmp *tb. */
tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, tcg_target_call_iarg_regs[1]);
#endif
@@ -2299,13 +2298,6 @@ static void tcg_target_qemu_prologue(TCGContext *s)
tcg_out_pop(s, tcg_target_callee_save_regs[i]);
}
tcg_out_opc(s, OPC_RET, 0, 0, 0);
-
-#if !defined(CONFIG_SOFTMMU)
- /* Try to set up a segment register to point to GUEST_BASE. */
- if (GUEST_BASE) {
- setup_guest_base_seg();
- }
-#endif
}
static void tcg_target_init(TCGContext *s)
--
2.4.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] tcg/i386: Reserve register for guest_base if a segment isn't available
2015-07-16 21:25 ` [Qemu-devel] [PATCH 2/2] tcg/i386: Reserve register for guest_base if a segment isn't available Richard Henderson
@ 2015-07-17 1:33 ` Peter Maydell
2015-07-17 6:02 ` Richard Henderson
2015-07-17 15:30 ` Paolo Bonzini
2015-07-17 10:18 ` Aurelien Jarno
1 sibling, 2 replies; 9+ messages in thread
From: Peter Maydell @ 2015-07-17 1:33 UTC (permalink / raw)
To: Richard Henderson; +Cc: Leon Alrae, QEMU Developers, Aurelien Jarno
On 16 July 2015 at 22:25, Richard Henderson <rth@twiddle.net> wrote:
> This saves 2 insns and 10 bytes from the implementation of
> each memory operation.
Do we have an idea of which platforms/configs don't let
us have a segment register for guest_base?
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] tcg/i386: Reserve register for guest_base if a segment isn't available
2015-07-17 1:33 ` Peter Maydell
@ 2015-07-17 6:02 ` Richard Henderson
2015-07-17 15:30 ` Paolo Bonzini
1 sibling, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2015-07-17 6:02 UTC (permalink / raw)
To: Peter Maydell; +Cc: Leon Alrae, QEMU Developers, Aurelien Jarno
On 07/17/2015 02:33 AM, Peter Maydell wrote:
> On 16 July 2015 at 22:25, Richard Henderson <rth@twiddle.net> wrote:
>> This saves 2 insns and 10 bytes from the implementation of
>> each memory operation.
>
> Do we have an idea of which platforms/configs don't let
> us have a segment register for guest_base?
We've only bothered to fill in code for Linux so far.
These days we should probably probe for fsgsbase insns and use those. Though I
don't know how many of the various operating systems enable that...
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] tcg/i386: Extend addresses for 32-bit guests
2015-07-16 21:25 ` [Qemu-devel] [PATCH 1/2] tcg/i386: Extend addresses for 32-bit guests Richard Henderson
@ 2015-07-17 10:18 ` Aurelien Jarno
0 siblings, 0 replies; 9+ messages in thread
From: Aurelien Jarno @ 2015-07-17 10:18 UTC (permalink / raw)
To: Richard Henderson; +Cc: leon.alrae, qemu-devel
On 2015-07-16 22:25, Richard Henderson wrote:
> Removing the ??? comment explaining why it (mostly) worked.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> tcg/i386/tcg-target.c | 105 +++++++++++++++++++++++++++++++-------------------
> 1 file changed, 65 insertions(+), 40 deletions(-)
>
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index ff4d9cf..bbe2963 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -1434,8 +1434,8 @@ static inline void setup_guest_base_seg(void) { }
> #endif /* SOFTMMU */
>
> static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
> - TCGReg base, intptr_t ofs, int seg,
> - TCGMemOp memop)
> + TCGReg base, int index, intptr_t ofs,
> + int seg, TCGMemOp memop)
> {
> const TCGMemOp real_bswap = memop & MO_BSWAP;
> TCGMemOp bswap = real_bswap;
> @@ -1448,13 +1448,16 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
>
> switch (memop & MO_SSIZE) {
> case MO_UB:
> - tcg_out_modrm_offset(s, OPC_MOVZBL + seg, datalo, base, ofs);
> + tcg_out_modrm_sib_offset(s, OPC_MOVZBL + seg, datalo,
> + base, index, 0, ofs);
> break;
> case MO_SB:
> - tcg_out_modrm_offset(s, OPC_MOVSBL + P_REXW + seg, datalo, base, ofs);
> + tcg_out_modrm_sib_offset(s, OPC_MOVSBL + P_REXW + seg, datalo,
> + base, index, 0, ofs);
> break;
> case MO_UW:
> - tcg_out_modrm_offset(s, OPC_MOVZWL + seg, datalo, base, ofs);
> + tcg_out_modrm_sib_offset(s, OPC_MOVZWL + seg, datalo,
> + base, index, 0, ofs);
> if (real_bswap) {
> tcg_out_rolw_8(s, datalo);
> }
> @@ -1462,20 +1465,21 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
> case MO_SW:
> if (real_bswap) {
> if (have_movbe) {
> - tcg_out_modrm_offset(s, OPC_MOVBE_GyMy + P_DATA16 + seg,
> - datalo, base, ofs);
> + tcg_out_modrm_sib_offset(s, OPC_MOVBE_GyMy + P_DATA16 + seg,
> + datalo, base, index, 0, ofs);
> } else {
> - tcg_out_modrm_offset(s, OPC_MOVZWL + seg, datalo, base, ofs);
> + tcg_out_modrm_sib_offset(s, OPC_MOVZWL + seg, datalo,
> + base, index, 0, ofs);
> tcg_out_rolw_8(s, datalo);
> }
> tcg_out_modrm(s, OPC_MOVSWL + P_REXW, datalo, datalo);
> } else {
> - tcg_out_modrm_offset(s, OPC_MOVSWL + P_REXW + seg,
> - datalo, base, ofs);
> + tcg_out_modrm_sib_offset(s, OPC_MOVSWL + P_REXW + seg,
> + datalo, base, index, 0, ofs);
> }
> break;
> case MO_UL:
> - tcg_out_modrm_offset(s, movop + seg, datalo, base, ofs);
> + tcg_out_modrm_sib_offset(s, movop + seg, datalo, base, index, 0, ofs);
> if (bswap) {
> tcg_out_bswap32(s, datalo);
> }
> @@ -1483,19 +1487,22 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
> #if TCG_TARGET_REG_BITS == 64
> case MO_SL:
> if (real_bswap) {
> - tcg_out_modrm_offset(s, movop + seg, datalo, base, ofs);
> + tcg_out_modrm_sib_offset(s, movop + seg, datalo,
> + base, index, 0, ofs);
> if (bswap) {
> tcg_out_bswap32(s, datalo);
> }
> tcg_out_ext32s(s, datalo, datalo);
> } else {
> - tcg_out_modrm_offset(s, OPC_MOVSLQ + seg, datalo, base, ofs);
> + tcg_out_modrm_sib_offset(s, OPC_MOVSLQ + seg, datalo,
> + base, index, 0, ofs);
> }
> break;
> #endif
> case MO_Q:
> if (TCG_TARGET_REG_BITS == 64) {
> - tcg_out_modrm_offset(s, movop + P_REXW + seg, datalo, base, ofs);
> + tcg_out_modrm_sib_offset(s, movop + P_REXW + seg, datalo,
> + base, index, 0, ofs);
> if (bswap) {
> tcg_out_bswap64(s, datalo);
> }
> @@ -1506,11 +1513,15 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi,
> datahi = t;
> }
> if (base != datalo) {
> - tcg_out_modrm_offset(s, movop + seg, datalo, base, ofs);
> - tcg_out_modrm_offset(s, movop + seg, datahi, base, ofs + 4);
> + tcg_out_modrm_sib_offset(s, movop + seg, datalo,
> + base, index, 0, ofs);
> + tcg_out_modrm_sib_offset(s, movop + seg, datahi,
> + base, index, 0, ofs + 4);
> } else {
> - tcg_out_modrm_offset(s, movop + seg, datahi, base, ofs + 4);
> - tcg_out_modrm_offset(s, movop + seg, datalo, base, ofs);
> + tcg_out_modrm_sib_offset(s, movop + seg, datahi,
> + base, index, 0, ofs + 4);
> + tcg_out_modrm_sib_offset(s, movop + seg, datalo,
> + base, index, 0, ofs);
> }
> if (bswap) {
> tcg_out_bswap32(s, datalo);
> @@ -1553,7 +1564,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
> label_ptr, offsetof(CPUTLBEntry, addr_read));
>
> /* TLB Hit. */
> - tcg_out_qemu_ld_direct(s, datalo, datahi, TCG_REG_L1, 0, 0, opc);
> + tcg_out_qemu_ld_direct(s, datalo, datahi, TCG_REG_L1, -1, 0, 0, opc);
>
> /* Record the current context of a load into ldst label */
> add_qemu_ldst_label(s, true, oi, datalo, datahi, addrlo, addrhi,
> @@ -1562,24 +1573,29 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
> {
> int32_t offset = GUEST_BASE;
> TCGReg base = addrlo;
> + int index = -1;
> int seg = 0;
>
> - /* ??? We assume all operations have left us with register contents
> - that are zero extended. So far this appears to be true. If we
> - want to enforce this, we can either do an explicit zero-extension
> - here, or (if GUEST_BASE == 0, or a segment register is in use)
> - use the ADDR32 prefix. For now, do nothing. */
I think we should keep a comment about why we need to use addr32 or
zero-extend the value.
> if (GUEST_BASE && guest_base_flags) {
> seg = guest_base_flags;
> offset = 0;
> - } else if (TCG_TARGET_REG_BITS == 64 && offset != GUEST_BASE) {
> - tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L1, GUEST_BASE);
> - tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L1, base);
> - base = TCG_REG_L1;
> - offset = 0;
> + if (TCG_TARGET_REG_BITS == 64 && TARGET_LONG_BITS == 32) {
You can write that as (TCG_TARGET_REG_BITS > TARGET_LONG_BITS)
> + seg |= P_ADDR32;
> + }
> + } else if (TCG_TARGET_REG_BITS == 64) {
> + if (TARGET_LONG_BITS == 32) {
> + tcg_out_ext32u(s, TCG_REG_L0, base);
> + base = TCG_REG_L0;
> + }
> + if (offset != GUEST_BASE) {
> + tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L1, GUEST_BASE);
> + index = TCG_REG_L1;
> + offset = 0;
> + }
> }
>
> - tcg_out_qemu_ld_direct(s, datalo, datahi, base, offset, seg, opc);
> + tcg_out_qemu_ld_direct(s, datalo, datahi,
> + base, index, offset, seg, opc);
> }
> #endif
> }
> @@ -1697,19 +1713,28 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is64)
> TCGReg base = addrlo;
> int seg = 0;
>
> - /* ??? We assume all operations have left us with register contents
> - that are zero extended. So far this appears to be true. If we
> - want to enforce this, we can either do an explicit zero-extension
> - here, or (if GUEST_BASE == 0, or a segment register is in use)
> - use the ADDR32 prefix. For now, do nothing. */
> if (GUEST_BASE && guest_base_flags) {
> seg = guest_base_flags;
> offset = 0;
> - } else if (TCG_TARGET_REG_BITS == 64 && offset != GUEST_BASE) {
> - tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L1, GUEST_BASE);
> - tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L1, base);
> - base = TCG_REG_L1;
> - offset = 0;
> + if (TCG_TARGET_REG_BITS == 64 && TARGET_LONG_BITS == 32) {
> + seg |= P_ADDR32;
> + }
> + } else if (TCG_TARGET_REG_BITS == 64) {
> + /* ??? Note that we can't use the same SIB addressing scheme
> + as for loads, since we require L0 free for bswap. */
> + if (offset != GUEST_BASE) {
> + if (TARGET_LONG_BITS == 32) {
> + tcg_out_ext32u(s, TCG_REG_L0, base);
> + base = TCG_REG_L0;
> + }
> + tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L1, GUEST_BASE);
> + tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L1, base);
> + base = TCG_REG_L1;
> + offset = 0;
> + } else if (TARGET_LONG_BITS == 32) {
> + tcg_out_ext32u(s, TCG_REG_L1, base);
> + base = TCG_REG_L1;
> + }
> }
>
> tcg_out_qemu_st_direct(s, datalo, datahi, base, offset, seg, opc);
Besides the small nitpicks above:
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] tcg/i386: Reserve register for guest_base if a segment isn't available
2015-07-16 21:25 ` [Qemu-devel] [PATCH 2/2] tcg/i386: Reserve register for guest_base if a segment isn't available Richard Henderson
2015-07-17 1:33 ` Peter Maydell
@ 2015-07-17 10:18 ` Aurelien Jarno
1 sibling, 0 replies; 9+ messages in thread
From: Aurelien Jarno @ 2015-07-17 10:18 UTC (permalink / raw)
To: Richard Henderson; +Cc: leon.alrae, qemu-devel
On 2015-07-16 22:25, Richard Henderson wrote:
> This saves 2 insns and 10 bytes from the implementation of
> each memory operation.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> tcg/i386/tcg-target.c | 120 +++++++++++++++++++++++---------------------------
> 1 file changed, 56 insertions(+), 64 deletions(-)
>
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
That said I do wonder if it is 2.4 material or if it should be deferred
to 2.5.
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] tcg/i386: Reserve register for guest_base if a segment isn't available
2015-07-17 1:33 ` Peter Maydell
2015-07-17 6:02 ` Richard Henderson
@ 2015-07-17 15:30 ` Paolo Bonzini
2015-07-17 15:56 ` Aurelien Jarno
1 sibling, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2015-07-17 15:30 UTC (permalink / raw)
To: Peter Maydell, Richard Henderson
Cc: Leon Alrae, QEMU Developers, Aurelien Jarno
On 17/07/2015 03:33, Peter Maydell wrote:
> On 16 July 2015 at 22:25, Richard Henderson <rth@twiddle.net> wrote:
> > This saves 2 insns and 10 bytes from the implementation of
> > each memory operation.
>
> Do we have an idea of which platforms/configs don't let
> us have a segment register for guest_base?
Everything non-Linux I think, which for user-mode emulation means bsd-user.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] tcg/i386: Reserve register for guest_base if a segment isn't available
2015-07-17 15:30 ` Paolo Bonzini
@ 2015-07-17 15:56 ` Aurelien Jarno
0 siblings, 0 replies; 9+ messages in thread
From: Aurelien Jarno @ 2015-07-17 15:56 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Maydell, Leon Alrae, QEMU Developers, Richard Henderson
On 2015-07-17 17:30, Paolo Bonzini wrote:
>
>
> On 17/07/2015 03:33, Peter Maydell wrote:
> > On 16 July 2015 at 22:25, Richard Henderson <rth@twiddle.net> wrote:
> > > This saves 2 insns and 10 bytes from the implementation of
> > > each memory operation.
> >
> > Do we have an idea of which platforms/configs don't let
> > us have a segment register for guest_base?
>
> Everything non-Linux I think, which for user-mode emulation means bsd-user.
Linux might also not allow to use a segment register if the prctl call
with ARCH_SET_GS fails. OTOH I have no idea why it can fail, but we
actually check that it doesn't fail before using %gs.
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-07-17 15:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-16 21:25 [Qemu-devel] [PATCH for-2.4 0/2] tcg/i386 address zero-extension Richard Henderson
2015-07-16 21:25 ` [Qemu-devel] [PATCH 1/2] tcg/i386: Extend addresses for 32-bit guests Richard Henderson
2015-07-17 10:18 ` Aurelien Jarno
2015-07-16 21:25 ` [Qemu-devel] [PATCH 2/2] tcg/i386: Reserve register for guest_base if a segment isn't available Richard Henderson
2015-07-17 1:33 ` Peter Maydell
2015-07-17 6:02 ` Richard Henderson
2015-07-17 15:30 ` Paolo Bonzini
2015-07-17 15:56 ` Aurelien Jarno
2015-07-17 10:18 ` Aurelien Jarno
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).