From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46267) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZVMQr-0000HL-Fn for qemu-devel@nongnu.org; Fri, 28 Aug 2015 12:23:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZVMQn-0003HT-He for qemu-devel@nongnu.org; Fri, 28 Aug 2015 12:23:13 -0400 References: <1440719254-12349-1-git-send-email-afaerber@suse.de> <55DFE3CC.4030806@twiddle.net> From: =?UTF-8?Q?Andreas_F=c3=a4rber?= Message-ID: <55E08AEE.3060308@suse.de> Date: Fri, 28 Aug 2015 10:23:10 -0600 MIME-Version: 1.0 In-Reply-To: <55DFE3CC.4030806@twiddle.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] tcg/aarch64: Fix tcg_out_qemu_{ld, st} for linux-user List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson , qemu-devel@nongnu.org Cc: Paolo Bonzini , Claudio Fontana , qemu-stable , Aurelien Jarno Am 27.08.2015 um 22:30 schrieb Richard Henderson: > On 08/27/2015 04:47 PM, Andreas F=C3=A4rber wrote: >> The argument order for the !CONFIG_SOFTMMU case was jumbled up since >> ffc6372851d8631a9f9fa56ec613b3244dc635b9 ("tcg/aarch64: use 32-bit >> offset for 32-bit user-mode emulation"), regressing from -rc2 to v2.4.= 0. >> Fix their order to avoid segfaults, e.g., in openSUSE's GNU coreutils >> 8.24. >=20 > Nack. The argument order is correct, that is... >=20 >> - tcg_out_qemu_ld_direct(s, memop, ext, data_reg, >> - guest_base ? TCG_REG_GUEST_BASE : >> TCG_REG_XZR, >> - otype, addr_reg); >> + tcg_out_qemu_ld_direct(s, memop, ext, data_reg, addr_reg, otype, >> + guest_base ? TCG_REG_GUEST_BASE : >> TCG_REG_XZR); >=20 > TCG_REG_GUEST_BASE is definitely the "base" register, holding a 64-bit > host address, while addr_reg is the "offset" register, holding a > (potentially) 32-bit guest address. It is (supposed to be) the "offset= " > register to which the zero-extend is applied. Huh? Please see http://git.qemu-project.org/?p=3Dqemu.git;a=3Dblobdiff;f=3Dtcg/aarch64/tc= g-target.c;h=3D4aca883b8e78b2cae6ec22c19a74f0fa9bc22bfe;hp=3Dd1c550886a46= 1581f93c34d5beead2b3ddfebde5;hb=3Dffc6372851d8631a9f9fa56ec613b3244dc635b= 9;hpb=3D6c0f0c0f124718650a8d682ba275044fc02f6fe2 @@ -1210,12 +1210,15 @@ static void tcg_out_qemu_ld(TCGContext *s, TCGReg data_reg, TCGReg addr_reg, tcg_insn_unit *label_ptr; tcg_out_tlb_read(s, addr_reg, s_bits, &label_ptr, mem_index, 1); - tcg_out_qemu_ld_direct(s, memop, ext, data_reg, addr_reg, TCG_REG_X1); + tcg_out_qemu_ld_direct(s, memop, ext, data_reg, addr_reg, + TCG_TYPE_I64, TCG_REG_X1); add_qemu_ldst_label(s, true, oi, ext, data_reg, addr_reg, s->code_ptr, label_ptr); #else /* !CONFIG_SOFTMMU */ - tcg_out_qemu_ld_direct(s, memop, ext, data_reg, addr_reg, - GUEST_BASE ? TCG_REG_GUEST_BASE : TCG_REG_XZR); + const TCGType otype =3D TARGET_LONG_BITS =3D=3D 64 ? TCG_TYPE_I64 : TCG_TYPE_I32; + tcg_out_qemu_ld_direct(s, memop, ext, data_reg, + GUEST_BASE ? TCG_REG_GUEST_BASE : TCG_REG_XZR, + otype, addr_reg); #endif /* CONFIG_SOFTMMU */ } @@ -1229,12 +1232,15 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg, tcg_insn_unit *label_ptr; tcg_out_tlb_read(s, addr_reg, s_bits, &label_ptr, mem_index, 0); - tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg, TCG_REG_X1); + tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg, + TCG_TYPE_I64, TCG_REG_X1); add_qemu_ldst_label(s, false, oi, s_bits =3D=3D MO_64, data_reg, addr_reg= , s->code_ptr, label_ptr); #else /* !CONFIG_SOFTMMU */ - tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg, - GUEST_BASE ? TCG_REG_GUEST_BASE : TCG_REG_XZR); + const TCGType otype =3D TARGET_LONG_BITS =3D=3D 64 ? TCG_TYPE_I64 : TCG_TYPE_I32; + tcg_out_qemu_st_direct(s, memop, data_reg, + GUEST_BASE ? TCG_REG_GUEST_BASE : TCG_REG_XZR, + otype, addr_reg); #endif /* CONFIG_SOFTMMU */ } An otype argument is being inserted as next-to-last argument for the function definitions. Same for the softmmu callsites. Only in the *-user callsites the argument order is being changed with addr_reg and off_r switching order? I don't see why that should be done in this patch. If it was wrong before, it should've been done in a separate patch. > If something's wrong, and I'm not currently in a position to verify one > way or another, it's in tcg_out_insn_3310. git-bisect pointed to the above commit. You are referring to its predecessor http://git.qemu-project.org/?p=3Dqemu.git;a=3Dcommit;h=3D6c0f0c0f12471865= 0a8d682ba275044fc02f6fe2 ? Don't spot anything obviously wrong there immediately... Regards, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Felix Imend=C3=B6rffer, Jane Smithard, Graham Norton; HRB 21284 (AG N= =C3=BCrnberg)