[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] tcg/mips: Bugfix for crash whenrunningprogram
From: |
jiang.biao2 |
Subject: |
Re: [Qemu-devel] [PATCH] tcg/mips: Bugfix for crash whenrunningprogram with qemu-i386. |
Date: |
Mon, 10 Jul 2017 11:15:00 +0800 (CST) |
> On 07/09/2017 04:04 PM, address@hidden wrote:
> > >> if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
> > >> tcg_out_ext32u(s, base, addr_regl)
> > >> - addr_regl = base
> > >> + tcg_out_mov(s, TCG_TYPE_PTR, addr_regl, base)
> > >> }
> > >> if (guest_base == 0 && data_regl != addr_regl) {
> > >> base = addr_regl
> > >
> > > This is wrong, because you're not allowed to modify the input operands.
> > >
> > > Try this, just a few lines lower in the function:
> >
> > > - tcg_out_movi(s, TCG_TYPE_PTR, base, guest_base)
> > > - tcg_out_opc_reg(s, ALIAS_PADD, base, base, addr_regl)
> > > + tcg_out_movi(s, TCG_TYPE_PTR, TCG_TMP0, guest_base)
> > > + tcg_out_opc_reg(s, ALIAS_PADD, base, TCG_TMP0, addr_regl)
> >
> > >
> >
> > Got it, but the real problem is for addr_regl instead of guest_base.
>
> Guest base is a problem simply because we require a temporary for it, and we
> were trying to put two temporaries into the same register.
>
> If we retain guest_base in a register all of the time, then (1) we do not
> have
> to recompute it for every memory load and (2) we do not need a temporary for
> it.
>
> > > > Better would be to reserve a register for the guest_base, like we do
> > > for ppc.
> > > See all of the uses of TCG_GUEST_BASE_REG in tcg/ppc/tcg-target.inc.c.
> >
> > It uses base(TCG_REG_A0) for temperary use for guest_base in this case.
>
> No it doesn't. It computes guest_base into a register in the prologue:
>
> > #ifndef CONFIG_SOFTMMU
> > if (guest_base) {
> > tcg_out_movi(s, TCG_TYPE_PTR, TCG_GUEST_BASE_REG, guest_base)
> > tcg_regset_set_reg(s->reserved_regs, TCG_GUEST_BASE_REG)
> > }
> > #endif
>
> and then it uses a reg+reg addressing mode during qemu_ld/st:
>
> > rbase = guest_base ? TCG_GUEST_BASE_REG : 0
> ....
> > insn = qemu_ldx_opc[opc & (MO_SIZE | MO_BSWAP)]
> > tcg_out32(s, insn | TAB(datalo, rbase, addrlo))
>
> Obviously mips doesn't have a reg+reg addressing mode, so a PADD instruction
> is
> required, but otherwise you can use the same scheme. Using TCG_REG_S1 on
> mips
> for TCG_GUEST_BASE_REG would be fine.
Understood. guest_base is a problem indeed, and it would be better to be like
that.
However, the bug I mentioned here seems to have no direct connect with the
guest_base problem. It lies in the following code,
> if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
> tcg_out_ext32u(s, base, addr_regl)
> addr_regl = base //problem is here.
> }
this section of code is to extend the addr_regl to 64bit, and use *base* as
temp
intermedia. The real intention could be to extend addr_regl into base, and then
move base back to addr_regl for later use, but it wrongly assigning base to
addr_regl directly, which will cause crash for every use of tcg_out_qemu_st.
My intention is to fix this bug, and intend to use TCG_TMP0 for temporary use
for addr_regl.
As for the guest_base problem, I'll try to optimize it later, taking ppc code
for
reference.
thanks for your advise and help.
- Re: [Qemu-devel] [PATCH] tcg/mips: Bugfix for crash whenrunningprogram with qemu-i386.,
jiang.biao2 <=