[Qemu-devel] [PATCH 0/2] target-mips: fix Windows NT support

[Qemu-devel] [PATCH 0/2] target-mips: fix Windows NT support

[Aurelien Jarno]

This patch set fix Windows NT support by correctly emulating the
LWL/LWR/LDL/LDR instructions. The first patch is the actual fix
and we might want to see it included in 2.4, though the problem
is not new to 2.4. The second patch is a small improvement in the
generated TCG code, so we probably want to wait for 2.5.

Aurelien Jarno (2):
  target-mips: fix page fault address for LWL/LWR/LDL/LDR
  target-mips: simplify LWL/LDL mask generation

 target-mips/translate.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH for-2.4 1/2] target-mips: fix page fault address for LWL/LWR/LDL/LDR

[Aurelien Jarno]

When a LWL, LWR, LDL or LDR instruction triggers a page fault, QEMU
currently reports the aligned address in CP0 BadVAddr, while the Windows
NT kernel expects the unaligned address.

This patch adds a byte access with the unaligned address at the
beginning of the LWL/LWR/LDL/LDR instructions to possibly trigger a page
fault and fill the QEMU TLB.

Cc: Leon Alrae <leon.alrae@imgtec.com>
Reported-by: Hervé Poussineau <hpoussin@reactos.org>
Tested-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-mips/translate.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 7302857..0ac3bd8 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -2142,6 +2142,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
         break;
     case OPC_LDL:
         t1 = tcg_temp_new();
+        /* Do a byte access to possibly trigger a page
+           fault with the unaligned address.  */
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
         tcg_gen_andi_tl(t1, t0, 7);
 #ifndef TARGET_WORDS_BIGENDIAN
         tcg_gen_xori_tl(t1, t1, 7);
@@ -2163,6 +2166,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
         break;
     case OPC_LDR:
         t1 = tcg_temp_new();
+        /* Do a byte access to possibly trigger a page
+           fault with the unaligned address.  */
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
         tcg_gen_andi_tl(t1, t0, 7);
 #ifdef TARGET_WORDS_BIGENDIAN
         tcg_gen_xori_tl(t1, t1, 7);
@@ -2229,6 +2235,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
         break;
     case OPC_LWL:
         t1 = tcg_temp_new();
+        /* Do a byte access to possibly trigger a page
+           fault with the unaligned address.  */
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
         tcg_gen_andi_tl(t1, t0, 3);
 #ifndef TARGET_WORDS_BIGENDIAN
         tcg_gen_xori_tl(t1, t1, 3);
@@ -2251,6 +2260,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
         break;
     case OPC_LWR:
         t1 = tcg_temp_new();
+        /* Do a byte access to possibly trigger a page
+           fault with the unaligned address.  */
+        tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx, MO_UB);
         tcg_gen_andi_tl(t1, t0, 3);
 #ifdef TARGET_WORDS_BIGENDIAN
         tcg_gen_xori_tl(t1, t1, 3);
-- 
2.1.4

Re: [Qemu-devel] [PATCH for-2.4 1/2] target-mips: fix page fault address for LWL/LWR/LDL/LDR

[Leon Alrae]

On 14/07/2015 16:45, Aurelien Jarno wrote:
> When a LWL, LWR, LDL or LDR instruction triggers a page fault, QEMU
> currently reports the aligned address in CP0 BadVAddr, while the Windows
> NT kernel expects the unaligned address.
> 
> This patch adds a byte access with the unaligned address at the
> beginning of the LWL/LWR/LDL/LDR instructions to possibly trigger a page
> fault and fill the QEMU TLB.
> 
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Reported-by: Hervé Poussineau <hpoussin@reactos.org>
> Tested-by: Hervé Poussineau <hpoussin@reactos.org>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target-mips/translate.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Thanks, applied to mips-next.

Leon

[Qemu-devel] [PATCH 2/2] target-mips: simplify LWL/LDL mask generation

[Aurelien Jarno]

The LWL/LDL instructions mask the GPR with a mask depending on the
address alignement. It is currently computed by doing:

    mask = 0x7fffffffffffffffull >> (t1 ^ 63)

It's simpler to generate it by doing:

    mask = (1 << t1) - 1

It uses the same number of TCG instructions, but it avoids a 32/64-bit
constant loading which can take a few instructions on RISC hosts.

Cc: Leon Alrae <leon.alrae@imgtec.com>
Tested-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-mips/translate.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 0ac3bd8..9891209 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -2153,9 +2153,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
         tcg_gen_andi_tl(t0, t0, ~7);
         tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ);
         tcg_gen_shl_tl(t0, t0, t1);
-        tcg_gen_xori_tl(t1, t1, 63);
-        t2 = tcg_const_tl(0x7fffffffffffffffull);
-        tcg_gen_shr_tl(t2, t2, t1);
+        t2 = tcg_const_tl(1);
+        tcg_gen_shl_tl(t2, t2, t1);
+        tcg_gen_subi_tl(t2, t2, 1);
         gen_load_gpr(t1, rt);
         tcg_gen_and_tl(t1, t1, t2);
         tcg_temp_free(t2);
@@ -2246,9 +2246,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
         tcg_gen_andi_tl(t0, t0, ~3);
         tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL);
         tcg_gen_shl_tl(t0, t0, t1);
-        tcg_gen_xori_tl(t1, t1, 31);
-        t2 = tcg_const_tl(0x7fffffffull);
-        tcg_gen_shr_tl(t2, t2, t1);
+        t2 = tcg_const_tl(1);
+        tcg_gen_shl_tl(t2, t2, t1);
+        tcg_gen_subi_tl(t2, t2, 1);
         gen_load_gpr(t1, rt);
         tcg_gen_and_tl(t1, t1, t2);
         tcg_temp_free(t2);
-- 
2.1.4

Re: [Qemu-devel] [PATCH 2/2] target-mips: simplify LWL/LDL mask generation

[Paolo Bonzini]

On 14/07/2015 17:45, Aurelien Jarno wrote:
> The LWL/LDL instructions mask the GPR with a mask depending on the
> address alignement. It is currently computed by doing:
> 
>     mask = 0x7fffffffffffffffull >> (t1 ^ 63)
> 
> It's simpler to generate it by doing:
> 
>     mask = (1 << t1) - 1

Using ~(-1 << t1) may let you use an ANDN instruction, and is also the
same number of instructions on x86.

Paolo

> It uses the same number of TCG instructions, but it avoids a 32/64-bit
> constant loading which can take a few instructions on RISC hosts.
> 
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> Tested-by: Hervé Poussineau <hpoussin@reactos.org>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target-mips/translate.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 0ac3bd8..9891209 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -2153,9 +2153,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
>          tcg_gen_andi_tl(t0, t0, ~7);
>          tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ);
>          tcg_gen_shl_tl(t0, t0, t1);
> -        tcg_gen_xori_tl(t1, t1, 63);
> -        t2 = tcg_const_tl(0x7fffffffffffffffull);
> -        tcg_gen_shr_tl(t2, t2, t1);
> +        t2 = tcg_const_tl(1);
> +        tcg_gen_shl_tl(t2, t2, t1);
> +        tcg_gen_subi_tl(t2, t2, 1);
>          gen_load_gpr(t1, rt);
>          tcg_gen_and_tl(t1, t1, t2);
>          tcg_temp_free(t2);
> @@ -2246,9 +2246,9 @@ static void gen_ld(DisasContext *ctx, uint32_t opc,
>          tcg_gen_andi_tl(t0, t0, ~3);
>          tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL);
>          tcg_gen_shl_tl(t0, t0, t1);
> -        tcg_gen_xori_tl(t1, t1, 31);
> -        t2 = tcg_const_tl(0x7fffffffull);
> -        tcg_gen_shr_tl(t2, t2, t1);
> +        t2 = tcg_const_tl(1);
> +        tcg_gen_shl_tl(t2, t2, t1);
> +        tcg_gen_subi_tl(t2, t2, 1);
>          gen_load_gpr(t1, rt);
>          tcg_gen_and_tl(t1, t1, t2);
>          tcg_temp_free(t2);
> 

Re: [Qemu-devel] [PATCH 2/2] target-mips: simplify LWL/LDL mask generation

[Aurelien Jarno]

On 2015-07-14 18:17, Paolo Bonzini wrote:
> 
> 
> On 14/07/2015 17:45, Aurelien Jarno wrote:
> > The LWL/LDL instructions mask the GPR with a mask depending on the
> > address alignement. It is currently computed by doing:
> > 
> >     mask = 0x7fffffffffffffffull >> (t1 ^ 63)
> > 
> > It's simpler to generate it by doing:
> > 
> >     mask = (1 << t1) - 1
> 
> Using ~(-1 << t1) may let you use an ANDN instruction, and is also the
> same number of instructions on x86.
> 

Indeed thanks for the hint. The generated code has the same size, but is
one instruction less:

   mov    0x88(%rsp),%r10
   shlx   %r10,%rbx,%rbx
-  mov    $0x1,%r11d
+  mov    $0xffffffffffffffff,%r11
   shlx   %r10,%r11,%r11
-  dec    %r11
   mov    0x18(%r14),%r10
-  and    %r11,%r10
+  andn   %r10,%r11,%r10
   or     %r10,%rbx
   movslq %ebx,%rbx

I'll send a new version of the patch.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 2/2] target-mips: simplify LWL/LDL mask generation

[Paolo Bonzini]

On 14/07/2015 18:45, Aurelien Jarno wrote:
>>> > > 
>>> > >     mask = 0x7fffffffffffffffull >> (t1 ^ 63)
>>> > > 
>>> > > It's simpler to generate it by doing:
>>> > > 
>>> > >     mask = (1 << t1) - 1
>> > 
>> > Using ~(-1 << t1) may let you use an ANDN instruction, and is also the
>> > same number of instructions on x86.
>> > 
> Indeed thanks for the hint. The generated code has the same size, but is
> one instruction less:
> 
>    mov    0x88(%rsp),%r10
>    shlx   %r10,%rbx,%rbx
> -  mov    $0x1,%r11d
> +  mov    $0xffffffffffffffff,%r11
>    shlx   %r10,%r11,%r11
> -  dec    %r11
>    mov    0x18(%r14),%r10
> -  and    %r11,%r10
> +  andn   %r10,%r11,%r10
>    or     %r10,%rbx
>    movslq %ebx,%rbx

Oh, indeed I forgot about the fancy new x86 bit manipulation
instructions!  Even better. :)

Paolo