[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 11/11] tcg-mips: Adjust condition functions f
From: |
Jin Guojie |
Subject: |
Re: [Qemu-devel] [PATCH v3 11/11] tcg-mips: Adjust condition functions for mips64 |
Date: |
Mon, 28 Nov 2016 15:42:10 +0800 |
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";<address@hidden>;
Date: Nov 25, 2016
To: "Richard Henderson"<address@hidden>;
Cc: "Jin Guojie"<address@hidden>; "qemu-devel"<address@hidden>; "James
Hogan"<address@hidden>;
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
address@hidden http://www.aurel32.net
- Re: [Qemu-devel] [PATCH v3 11/11] tcg-mips: Adjust condition functions for mips64,
Jin Guojie <=