All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 00/11] tcg mips64 and mips r6 improvements
@ 2016-11-25  3:31 Jin Guojie
  2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 01/11] tcg-mips: Move bswap code to a subroutine Jin Guojie
                   ` (12 more replies)
  0 siblings, 13 replies; 21+ messages in thread
From: Jin Guojie @ 2016-11-25  3:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jin Guojie, Aurelien Jarno, James Hogan, Richard Henderson

Changes since v2:
  * Update against master(v2.8.0-rc1)
  * Tested on Loongson as mips32r2(el) and mips64r2(el) hosts.
    Loongson only implements little-endian mips32/mips64 ISA.
  * Fully work for 32-bit and 64-bit guests.
    Fix two bugs:segmentation fault on mips64el with 32-bit guests,
                  blocking when emulating i386 kernel on mips64el.
  * Fix some minor style problems.
  * PATCH v2 12~16 are not examined due to the lack of R6 machine. 

To be tested:
  * big-endian mips32 and mips64 hosts.
    I have tried running qemu-system-mips on an X86. The speed is awful. 
    The compilation of qemu did not complete over a night until I gave up.
    A better way is needed to do this test.
  * MIPS R6.
    
Summary of changes on v2 patch:

  | tcg-mips: Always use tcg_debug_assert      | merged previously   |
  | tcg-mips: Move bswap code to a subroutine  | no change           |
  | tcg-mips: Add mips64 opcodes               | no change           |
  | tcg-mips: Support 64-bit opcodes           | no change           |
  | tcg-mips: Add bswap32u and bswap64         | no change           |
  | tcg-mips: Adjust move functions for mips64 | no change           |
  | tcg-mips: Adjust load/store functions for  | no change           |
  | tcg-mips: Adjust prologue for mips64       | no change           |
  | tcg-mips: Add tcg unwind info              | fix a style problem |
  | tcg-mips: Adjust calling conventions for   | no change           |
  | tcg-mips: Adjust qemu_ld/st for mips64                           | 
  |   (1) tcg_out_qemu_st_slow_path:fix the crash on mips64el with  |
  |       i386 guest, when executing seabios                         | 
  |   (2) tlb_load:simplify ifdefs                                  |
  | tcg-mips: Adjust condition functions for mips64 (newly added)    | 
  |   fix the blocking when emulating i386 kernel on mips64el        |

Jin Guojie (11):
  tcg-mips: Move bswap code to a subroutine
  tcg-mips: Add mips64 opcodes
  tcg-mips: Support 64-bit opcodes
  tcg-mips: Add bswap32u and bswap64
  tcg-mips: Adjust move functions for mips64
  tcg-mips: Adjust load/store functions for mips64
  tcg-mips: Adjust prologue for mips64
  tcg-mips: Add tcg unwind info
  tcg-mips: Adjust calling conventions for mips64
  tcg-mips: Adjust qemu_ld/st for mips64
  tcg-mips: Adjust condition functions for mips64

Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: James Hogan <james.hogan@imgtec.com>
Signed-off-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Jin Guojie <jinguojie@loongson.cn>

 tcg/mips/tcg-target.h     |   60 ++-
 tcg/mips/tcg-target.inc.c | 1156 +++++++++++++++++++++++++++++++++++----------
 2 files changed, 965 insertions(+), 251 deletions(-)

-- 
2.1.0

^ permalink raw reply	[flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v3 11/11] tcg-mips: Adjust condition functions for mips64
@ 2016-11-28  7:42 Jin Guojie
  2016-11-28  8:11 ` Richard Henderson
  0 siblings, 1 reply; 21+ messages in thread
From: Jin Guojie @ 2016-11-28  7:42 UTC (permalink / raw)
  To: Aurelien Jarno, Richard Henderson; +Cc: qemu-devel, James Hogan

Here I can describe the problem when patch 11 is not applied.

When booting Linux kernel with qemu-system-i386 on mips64el host, the
guest CPU enters infinite loop:


0xc01f3b90:  cmp    0x14(%esp),%esi
0xc01f3b94:  jae    0xc01f3b99

 qemu_ld_i32 tmp1,tmp2,leul,2
 mov_i32 tmp0,esi
 mov_i32 cc_src,tmp1
 brcond_i32 tmp0,cc_src,geu,$L1

0xffd64e8430:  lw       s2,24(s0)    <-- esi
0xffd64e8444:  sltu     at,s2,s1

s2(representing ESI) is loaded via lw, which always do sign-extension.
This is just the same case as Richard and Aurelien commented.

However, qemu_ld_i32 returns through slow path.
The memory address 0x14(%esp) is read by calling helper_le_ldul_mmu(). The
returned value V0 is then assigned to s1.

The helper function is defined in softmmu_template.h as:

#define WORD_TYPE tcg_target_ulong   // same as uint64_t
#define DATA_TYPE  uint32_t

WORD_TYPE helper_le_ld_name(CPUArchState *env, target_ulong addr,
                            TCGMemOpIdx oi, uintptr_t retaddr)
{
    DATA_TYPE res;
    ......
}

By adding printf(), I noticed that res is returned without high 32-bit extended.

That means: if %esi and 0x14(%esp) has the same 32-bit value 0xcccccccc, they 
have different 64-bit value when being compared by sltu:

s2: 0xffffffffcccccccc
s1: 0x00000000cccccccc

To verify this, I wrote another small C program:

[a.c]
unsigned long add()
{
  return (unsigned int)0xcccccccc;
}

main()
{
  printf("%lx\n", add());
}

$ gcc a.c -mabi=64
$ ./a.out
cccccccc

This is just the value which helper_le_ld_name() returns.

To fix the comparision result, I introduced patch 11, in which the 32-bit registers in condition functions
are forced to be signed-extended, and then kernel booting is OK.

By reading Richard and Aurelien's comment, I realized now the best way to solve this problem
is not to add ext32s in brcond_32i, but to fix the helper function. In another word,
the register value should be 32-bit sign-extened at where it's being *created*, not where
it's being *utilized*. Maybe I can do this ext32s after helper_le_ld_name() is call back to ensure 
 V0 to be sign-extended. 

I notice that, in tcg/sparc/tcg-target.inc.c, there is a similar comment "We let the helper sign-extend SB and SW, 
but leave SL for here". This may be a good evidence supporting my idea.

If you think my idea is reasonable, I will submit a better solution in v4 patch soon.

Jin Guojie
 
------------------ Original ------------------
From:  "Aurelien Jarno";<aurelien@aurel32.net>;
Date:  Nov 25, 2016
To:  "Richard Henderson"<rth@twiddle.net>; 
Cc:  "Jin Guojie"<jinguojie@loongson.cn>; "qemu-devel"<qemu-devel@nongnu.org>; "James Hogan"<james.hogan@imgtec.com>; 
Subject:  Re: [PATCH v3 11/11] tcg-mips: Adjust condition functions for mips64



On 2016-11-25 13:06, Richard Henderson wrote:
> On 11/25/2016 04:31 AM, Jin Guojie wrote:
> > 32-bit condition functions(like brcond_i32) should only
> > compare the low half parts of two 64-bit host registers.
> > However, MIPS64 does not have distinct instruction for
> > such operation. The operands should be sign extended
> > to fit the case.
> > 
> > Gcc handles 32-bit comparison in the same way, as the
> > following example shows:
> > 
> > [a.c]
> > main()
> > {
> >   long a = 0xcccccccc;
> >   long b = 0xdddddddd;
> >   int c = (int)a > (int)b;
> > }
> 
> This problem is why opcodes like
> 
>    OPC_INDEX_extrl_i64_i32
>    OPC_INDEX_extrh_i64_i32
>    OPC_INDEX_ext_i32_i64
>    OPC_INDEX_extu_i32_i64
> 
> exist.  The intention is to keep 32-bit values in their sign-extended form,
> exactly as the mips hardware manual requires.  At which point all 32-bit
> opcodes (ADDIU, SLL, etc) will preserve the 32-bit sign extension property.

It's even stronger than that. It's required for 32-bit opcodes to work
correctly. The manual says:

| If GPR rs does not contain a sign-extended 32-bit value (bits 63..31
| equal), then the result of the operation is UNPREDICTABLE.

Aurelien

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

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

end of thread, other threads:[~2016-11-30 19:39 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-25  3:31 [Qemu-devel] [PATCH v3 00/11] tcg mips64 and mips r6 improvements Jin Guojie
2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 01/11] tcg-mips: Move bswap code to a subroutine Jin Guojie
2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 02/11] tcg-mips: Add mips64 opcodes Jin Guojie
2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 03/11] tcg-mips: Support 64-bit opcodes Jin Guojie
2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 04/11] tcg-mips: Add bswap32u and bswap64 Jin Guojie
2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 05/11] tcg-mips: Adjust move functions for mips64 Jin Guojie
2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 06/11] tcg-mips: Adjust load/store " Jin Guojie
2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 07/11] tcg-mips: Adjust prologue " Jin Guojie
2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 08/11] tcg-mips: Add tcg unwind info Jin Guojie
2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 09/11] tcg-mips: Adjust calling conventions for mips64 Jin Guojie
2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 10/11] tcg-mips: Adjust qemu_ld/st " Jin Guojie
2016-11-25 11:55   ` Richard Henderson
2016-11-25  3:31 ` [Qemu-devel] [PATCH v3 11/11] tcg-mips: Adjust condition functions " Jin Guojie
2016-11-25 12:06   ` Richard Henderson
2016-11-25 14:25     ` Aurelien Jarno
2016-11-25 14:21 ` [Qemu-devel] [PATCH v3 00/11] tcg mips64 and mips r6 improvements Aurelien Jarno
2016-11-30 17:19 ` Aurelien Jarno
2016-11-30 18:33   ` =?gb18030?B?SmluIEd1b2ppZQ==?=
2016-11-30 19:39     ` Aurelien Jarno
2016-11-28  7:42 [Qemu-devel] [PATCH v3 11/11] tcg-mips: Adjust condition functions for mips64 Jin Guojie
2016-11-28  8:11 ` Richard Henderson

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.