[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] tcg/aarch64: Fix tcg_out_qemu_{ld, st} for linu
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH] tcg/aarch64: Fix tcg_out_qemu_{ld, st} for linux-user |
Date: |
Fri, 28 Aug 2015 10:23:10 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
Am 27.08.2015 um 22:30 schrieb Richard Henderson:
> On 08/27/2015 04:47 PM, Andreas Färber 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.
>
> Nack. The argument order is correct, that is...
>
>> - 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);
>
> 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=qemu.git;a=blobdiff;f=tcg/aarch64/tcg-target.c;h=4aca883b8e78b2cae6ec22c19a74f0fa9bc22bfe;hp=d1c550886a461581f93c34d5beead2b3ddfebde5;hb=ffc6372851d8631a9f9fa56ec613b3244dc635b9;hpb=6c0f0c0f124718650a8d682ba275044fc02f6fe2
@@ -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 = TARGET_LONG_BITS == 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 == 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 = TARGET_LONG_BITS == 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=qemu.git;a=commit;h=6c0f0c0f124718650a8d682ba275044fc02f6fe2
? Don't spot anything obviously wrong there immediately...
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)