From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44020) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cCTeu-0005gd-1F for qemu-devel@nongnu.org; Thu, 01 Dec 2016 10:52:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cCTep-0001fO-6h for qemu-devel@nongnu.org; Thu, 01 Dec 2016 10:52:28 -0500 Received: from mail-qk0-x242.google.com ([2607:f8b0:400d:c09::242]:36532) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cCTep-0001fC-1o for qemu-devel@nongnu.org; Thu, 01 Dec 2016 10:52:23 -0500 Received: by mail-qk0-x242.google.com with SMTP id h201so26731818qke.3 for ; Thu, 01 Dec 2016 07:52:22 -0800 (PST) Sender: Richard Henderson References: <1480600329-16272-1-git-send-email-jinguojie@loongson.cn> From: Richard Henderson Message-ID: <7b7aaab8-f27e-4d0f-8864-302ac0ab4722@twiddle.net> Date: Thu, 1 Dec 2016 07:52:19 -0800 MIME-Version: 1.0 In-Reply-To: <1480600329-16272-1-git-send-email-jinguojie@loongson.cn> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 00/10] tcg mips64 and mips r6 improvements List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jin Guojie , qemu-devel@nongnu.org Cc: Aurelien Jarno , James Hogan On 12/01/2016 05:51 AM, Jin Guojie wrote: > Changes in v5: > * Update against master(v2.8.0-rc2) > * Fix a bug: 64-bit big-endian guests hang on mips64 little-endian > hosts, and vice versa. This bug was first introduced in v2 patch, > due to obvious misuse of ret/arg registers in tcg_out_bswap64(). > > tcg_out_opc_reg(s, OPC_DSBH, ret, 0, arg); > - tcg_out_opc_reg(s, OPC_DSHD, ret, 0, arg); > + tcg_out_opc_reg(s, OPC_DSHD, ret, 0, ret); Oops. Thanks. > * Fix a style problem: checkpatch.pl forbids 'extern' to be used in .c. > > ERROR: externs should be avoided in .c files > #28: FILE: tcg/mips/tcg-target.inc.c:39: > +extern int link_error(void); Hmph. This is checkpatch not being helpful, but your change is fine. I've looked at a diff between your current patch and the last update I had to my branch. This probably includes code that post-dates the v2 that you started with. There are two things I'd like to point out: @@ -1233,7 +1235,8 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg base, TCGReg addrl, if (a_bits < s_bits) { a_bits = s_bits; } - mask = (target_ulong)TARGET_PAGE_MASK | ((1 << a_bits) - 1); + + mask = TARGET_PAGE_MASK | ((1 << a_bits) - 1); You need the target_ulong cast here for mips64. See patch ebb90a005da67147245cd38fb04a965a87a961b7. if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) { - tcg_out_ext32u(s, base, addrl); - addrl = base; + tcg_out_ext32u(s, TCG_TMP0, TCG_TMP0); } Why did you make this change? It is definitely wrong. We should be zero-extending the guest address, not the address that we just loaded from CMP_OFF. This is because we need to add an unsigned 32-bit quantity to the full 64-bit host pointer that we load from ADD_OFF. Did you notice a compare problem between TMP1 and TMP0 below? If so, I believe that a partial solution is the TARGET_PAGE_MASK change above. I guess we also need to do tcg_out_ldst(s, (TARGET_LONG_BITS == 64 ? OPC_LD TCG_TARGET_REG_BITS == 64 : OPC_LWU : OPC_LW), TCG_TMP0, TCG_REG_A0, cmp_off); in the else section of the tlb comparator load above, since mask will be 32-bit unsigned for TARGET_LONG_BITS == 32, and we need an unsigned quantity to compare that against. r~ PS: Ignoring N32 for now. But as a note to ourselves for future: What is the proper combination of zero/sign-extend vs ADDU/DADDU for 32/64-bit guests and an N32 host? What we have now is technically illegal, with zero-extend + ADDU. But I can imagine several valid scenarios.