From: Aurelien Jarno <aurelien@aurel32.net>
To: Richard Henderson <rth@twiddle.net>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2] tcg-i386: Use %gs prefixes for x86_64 GUEST_BASE
Date: Sun, 21 Oct 2012 06:26:02 +0200 [thread overview]
Message-ID: <20121021042602.GA15193@ohm.aurel32.net> (raw)
In-Reply-To: <1350531365-29257-1-git-send-email-rth@twiddle.net>
On Thu, Oct 18, 2012 at 01:36:05PM +1000, Richard Henderson wrote:
> When we allocate a reserved_va for the guest, the kernel will likely
> choose an address well above 4G. At which point we must use a pair
> of movabsq+addq to form the host address. If we have OS support,
> set up a segment register to point to guest_base instead.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> tcg/i386/tcg-target.c | 141 ++++++++++++++++++++++++++++++--------------------
> 1 file changed, 85 insertions(+), 56 deletions(-)
>
> Dang it, left some duplicate includes in there...
>
>
> r~
>
>
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index 4952c05..0d8855a 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -236,11 +236,13 @@ static inline int tcg_target_const_match(tcg_target_long val,
> # define P_REXW 0x800 /* Set REX.W = 1 */
> # define P_REXB_R 0x1000 /* REG field as byte register */
> # define P_REXB_RM 0x2000 /* R/M field as byte register */
> +# define P_GS 0x4000 /* gs segment override */
> #else
> # define P_ADDR32 0
> # define P_REXW 0
> # define P_REXB_R 0
> # define P_REXB_RM 0
> +# define P_GS 0
> #endif
>
> #define OPC_ARITH_EvIz (0x81)
> @@ -356,6 +358,9 @@ static void tcg_out_opc(TCGContext *s, int opc, int r, int rm, int x)
> {
> int rex;
>
> + if (opc & P_GS) {
> + tcg_out8(s, 0x65);
> + }
> if (opc & P_DATA16) {
> /* We should never be asking for both 16 and 64-bit operation. */
> assert((opc & P_REXW) == 0);
> @@ -1080,10 +1085,25 @@ static inline void tcg_out_tlb_load(TCGContext *s, int addrlo_idx,
> tcg_out_modrm_offset(s, OPC_ADD_GvEv + P_REXW, r0, r1,
> offsetof(CPUTLBEntry, addend) - which);
> }
> -#endif
> +#elif defined(__x86_64__) && defined(__linux__)
> +# include <sys/syscall.h>
> +# include <asm/prctl.h>
> +
> +static int guest_base_flags;
> +static inline void setup_guest_base_seg(void)
> +{
> + if (syscall(__NR_arch_prctl, ARCH_SET_GS, GUEST_BASE) == 0) {
> + guest_base_flags = P_GS;
> + }
Why calling the syscall directly instead of using arch_prctl(2)?
> +}
> +#else
> +# define guest_base_flags 0
> +static inline void setup_guest_base_seg(void) { }
> +#endif /* SOFTMMU */
>
> static void tcg_out_qemu_ld_direct(TCGContext *s, int datalo, int datahi,
> - int base, tcg_target_long ofs, int sizeop)
> + int base, tcg_target_long ofs, int seg,
> + int sizeop)
> {
> #ifdef TARGET_WORDS_BIGENDIAN
> const int bswap = 1;
> @@ -1092,28 +1112,29 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, int datalo, int datahi,
> #endif
> switch (sizeop) {
> case 0:
> - tcg_out_modrm_offset(s, OPC_MOVZBL, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVZBL + seg, datalo, base, ofs);
> break;
> case 0 | 4:
> - tcg_out_modrm_offset(s, OPC_MOVSBL + P_REXW, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVSBL + P_REXW + seg, datalo, base, ofs);
> break;
> case 1:
> - tcg_out_modrm_offset(s, OPC_MOVZWL, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVZWL + seg, datalo, base, ofs);
> if (bswap) {
> tcg_out_rolw_8(s, datalo);
> }
> break;
> case 1 | 4:
> if (bswap) {
> - tcg_out_modrm_offset(s, OPC_MOVZWL, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVZWL + seg, datalo, base, 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, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVSWL + P_REXW + seg,
> + datalo, base, ofs);
> }
> break;
> case 2:
> - tcg_out_ld(s, TCG_TYPE_I32, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVL_GvEv + seg, datalo, base, ofs);
> if (bswap) {
> tcg_out_bswap32(s, datalo);
> }
> @@ -1121,17 +1142,18 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, int datalo, int datahi,
> #if TCG_TARGET_REG_BITS == 64
> case 2 | 4:
> if (bswap) {
> - tcg_out_ld(s, TCG_TYPE_I32, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVL_GvEv + seg, datalo, base, ofs);
> tcg_out_bswap32(s, datalo);
> tcg_out_ext32s(s, datalo, datalo);
> } else {
> - tcg_out_modrm_offset(s, OPC_MOVSLQ, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVSLQ + seg, datalo, base, ofs);
> }
> break;
> #endif
> case 3:
> if (TCG_TARGET_REG_BITS == 64) {
> - tcg_out_ld(s, TCG_TYPE_I64, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVL_GvEv + P_REXW + seg,
> + datalo, base, ofs);
> if (bswap) {
> tcg_out_bswap64(s, datalo);
> }
> @@ -1142,11 +1164,15 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, int datalo, int datahi,
> datahi = t;
> }
> if (base != datalo) {
> - tcg_out_ld(s, TCG_TYPE_I32, datalo, base, ofs);
> - tcg_out_ld(s, TCG_TYPE_I32, datahi, base, ofs + 4);
> + tcg_out_modrm_offset(s, OPC_MOVL_GvEv + seg,
> + datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVL_GvEv + seg,
> + datahi, base, ofs + 4);
> } else {
> - tcg_out_ld(s, TCG_TYPE_I32, datahi, base, ofs + 4);
> - tcg_out_ld(s, TCG_TYPE_I32, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVL_GvEv + seg,
> + datahi, base, ofs + 4);
> + tcg_out_modrm_offset(s, OPC_MOVL_GvEv + seg,
> + datalo, base, ofs);
> }
> if (bswap) {
> tcg_out_bswap32(s, datalo);
> @@ -1192,7 +1218,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args,
> label_ptr, offsetof(CPUTLBEntry, addr_read));
>
> /* TLB Hit. */
> - tcg_out_qemu_ld_direct(s, data_reg, data_reg2, TCG_REG_L0, 0, opc);
> + tcg_out_qemu_ld_direct(s, data_reg, data_reg2, TCG_REG_L0, 0, 0, opc);
>
> /* jmp label2 */
> tcg_out8(s, OPC_JMP_short);
> @@ -1285,29 +1311,26 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args,
> {
> int32_t offset = GUEST_BASE;
> int base = args[addrlo_idx];
> -
> - if (TCG_TARGET_REG_BITS == 64) {
> - /* ??? 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)
> - use the ADDR32 prefix. For now, do nothing. */
> -
AFAIU this comment is still valid when %gs is/can not be used. Why
removing it?
> - if (offset != GUEST_BASE) {
> - tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L0, GUEST_BASE);
> - tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L0, base);
> - base = TCG_REG_L0;
> - offset = 0;
> - }
> + int seg = 0;
> +
> + 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_L0, GUEST_BASE);
> + tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L0, base);
> + base = TCG_REG_L0;
> + offset = 0;
> }
>
> - tcg_out_qemu_ld_direct(s, data_reg, data_reg2, base, offset, opc);
> + tcg_out_qemu_ld_direct(s, data_reg, data_reg2, base, offset, seg, opc);
> }
> #endif
> }
>
> static void tcg_out_qemu_st_direct(TCGContext *s, int datalo, int datahi,
> - int base, tcg_target_long ofs, int sizeop)
> + int base, tcg_target_long ofs, int seg,
> + int sizeop)
> {
> #ifdef TARGET_WORDS_BIGENDIAN
> const int bswap = 1;
> @@ -1322,7 +1345,8 @@ static void tcg_out_qemu_st_direct(TCGContext *s, int datalo, int datahi,
>
> switch (sizeop) {
> case 0:
> - tcg_out_modrm_offset(s, OPC_MOVB_EvGv + P_REXB_R, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVB_EvGv + P_REXB_R + seg,
> + datalo, base, ofs);
> break;
> case 1:
> if (bswap) {
> @@ -1330,7 +1354,8 @@ static void tcg_out_qemu_st_direct(TCGContext *s, int datalo, int datahi,
> tcg_out_rolw_8(s, scratch);
> datalo = scratch;
> }
> - tcg_out_modrm_offset(s, OPC_MOVL_EvGv + P_DATA16, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVL_EvGv + P_DATA16 + seg,
> + datalo, base, ofs);
> break;
> case 2:
> if (bswap) {
> @@ -1338,7 +1363,7 @@ static void tcg_out_qemu_st_direct(TCGContext *s, int datalo, int datahi,
> tcg_out_bswap32(s, scratch);
> datalo = scratch;
> }
> - tcg_out_st(s, TCG_TYPE_I32, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVL_EvGv + seg, datalo, base, ofs);
> break;
> case 3:
> if (TCG_TARGET_REG_BITS == 64) {
> @@ -1347,17 +1372,18 @@ static void tcg_out_qemu_st_direct(TCGContext *s, int datalo, int datahi,
> tcg_out_bswap64(s, scratch);
> datalo = scratch;
> }
> - tcg_out_st(s, TCG_TYPE_I64, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVL_EvGv + P_REXW + seg,
> + datalo, base, ofs);
> } else if (bswap) {
> tcg_out_mov(s, TCG_TYPE_I32, scratch, datahi);
> tcg_out_bswap32(s, scratch);
> - tcg_out_st(s, TCG_TYPE_I32, scratch, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVL_EvGv + seg, scratch, base, ofs);
> tcg_out_mov(s, TCG_TYPE_I32, scratch, datalo);
> tcg_out_bswap32(s, scratch);
> - tcg_out_st(s, TCG_TYPE_I32, scratch, base, ofs + 4);
> + tcg_out_modrm_offset(s, OPC_MOVL_EvGv + seg, scratch, base, ofs+4);
> } else {
> - tcg_out_st(s, TCG_TYPE_I32, datalo, base, ofs);
> - tcg_out_st(s, TCG_TYPE_I32, datahi, base, ofs + 4);
> + tcg_out_modrm_offset(s, OPC_MOVL_EvGv + seg, datalo, base, ofs);
> + tcg_out_modrm_offset(s, OPC_MOVL_EvGv + seg, datahi, base, ofs+4);
> }
> break;
> default:
> @@ -1391,7 +1417,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args,
> label_ptr, offsetof(CPUTLBEntry, addr_write));
>
> /* TLB Hit. */
> - tcg_out_qemu_st_direct(s, data_reg, data_reg2, TCG_REG_L0, 0, opc);
> + tcg_out_qemu_st_direct(s, data_reg, data_reg2, TCG_REG_L0, 0, 0, opc);
>
> /* jmp label2 */
> tcg_out8(s, OPC_JMP_short);
> @@ -1451,23 +1477,19 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args,
> {
> int32_t offset = GUEST_BASE;
> int base = args[addrlo_idx];
> -
> - if (TCG_TARGET_REG_BITS == 64) {
> - /* ??? 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)
> - use the ADDR32 prefix. For now, do nothing. */
> -
Same here.
> - if (offset != GUEST_BASE) {
> - tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L0, GUEST_BASE);
> - tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L0, base);
> - base = TCG_REG_L0;
> - offset = 0;
> - }
> + int seg = 0;
> +
> + 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_L0, GUEST_BASE);
> + tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L0, base);
> + base = TCG_REG_L0;
> + offset = 0;
> }
>
> - tcg_out_qemu_st_direct(s, data_reg, data_reg2, base, offset, opc);
> + tcg_out_qemu_st_direct(s, data_reg, data_reg2, base, offset, seg, opc);
> }
> #endif
> }
> @@ -2061,6 +2083,13 @@ 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)
Otherwise looks fine, though I haven't done any benchmark.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
next prev parent reply other threads:[~2012-10-21 4:26 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-18 3:28 [Qemu-devel] [PATCH] tcg-i386: Use %gs prefixes for x86_64 GUEST_BASE Richard Henderson
2012-10-18 3:36 ` [Qemu-devel] [PATCH v2] " Richard Henderson
2012-10-21 4:26 ` Aurelien Jarno [this message]
2012-10-21 6:24 ` Richard Henderson
2012-10-21 20:43 ` Richard Henderson
-- strict thread matches above, loose matches on Subject: below --
2012-10-22 2:11 Richard Henderson
2012-10-22 5:59 ` Aurelien Jarno
2012-10-22 21:19 ` Richard Henderson
2012-10-29 14:34 ` Aurelien Jarno
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20121021042602.GA15193@ohm.aurel32.net \
--to=aurelien@aurel32.net \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.