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";<aurel...@aurel32.net>; Date: Nov 25, 2016 To: "Richard Henderson"<r...@twiddle.net>; Cc: "Jin Guojie"<jinguo...@loongson.cn>; "qemu-devel"<qemu-devel@nongnu.org>; "James Hogan"<james.ho...@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 aurel...@aurel32.net http://www.aurel32.net