| Message ID | 1420652355-31847-2-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jan 07, 2015 at 06:39:12PM +0100, Paolo Bonzini wrote: > This was accessing an XMM register's low half without going through XMM_Q. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
On Wed, Jan 07, 2015 at 06:39:12PM +0100, Paolo Bonzini wrote: > This was accessing an XMM register's low half without going through XMM_Q. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target-i386/translate.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/target-i386/translate.c b/target-i386/translate.c > index ebdc350..5af4300 100644 > --- a/target-i386/translate.c > +++ b/target-i386/translate.c > @@ -3074,7 +3074,8 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b, > goto illegal_op; > gen_lea_modrm(env, s, modrm); > if (b1 & 1) { > - gen_stq_env_A0(s, offsetof(CPUX86State, xmm_regs[reg])); > + gen_stq_env_A0(s, offsetof(CPUX86State, > + xmm_regs[reg].XMM_Q(0))); Do we have (or will patch 4/4 introduce) the same bug on the tcg_gen_addi_ptr() calls that don't use the XMM_Q macro?
On 13/01/2015 19:48, Eduardo Habkost wrote: >> > if (b1 & 1) { >> > - gen_stq_env_A0(s, offsetof(CPUX86State, xmm_regs[reg])); >> > + gen_stq_env_A0(s, offsetof(CPUX86State, >> > + xmm_regs[reg].XMM_Q(0))); > Do we have (or will patch 4/4 introduce) the same bug on the > tcg_gen_addi_ptr() calls that don't use the XMM_Q macro? No, they all call into helpers that use the XMM_Q macro themselves. Paolo
On Tue, Jan 13, 2015 at 08:49:19PM +0100, Paolo Bonzini wrote: > > > On 13/01/2015 19:48, Eduardo Habkost wrote: > >> > if (b1 & 1) { > >> > - gen_stq_env_A0(s, offsetof(CPUX86State, xmm_regs[reg])); > >> > + gen_stq_env_A0(s, offsetof(CPUX86State, > >> > + xmm_regs[reg].XMM_Q(0))); > > Do we have (or will patch 4/4 introduce) the same bug on the > > tcg_gen_addi_ptr() calls that don't use the XMM_Q macro? > > No, they all call into helpers that use the XMM_Q macro themselves. tcg_gen_addi_ptr() is called sometimes using the fpregs[reg].mmx offset, and sometimes using the xmm_regs[reg] offset. How can it know if the XMM_Q macro is necessary or not?
On 14/01/2015 14:17, Eduardo Habkost wrote: >>> > > Do we have (or will patch 4/4 introduce) the same bug on the >>> > > tcg_gen_addi_ptr() calls that don't use the XMM_Q macro? >> > >> > No, they all call into helpers that use the XMM_Q macro themselves. > tcg_gen_addi_ptr() is called sometimes using the fpregs[reg].mmx offset, > and sometimes using the xmm_regs[reg] offset. How can it know if the > XMM_Q macro is necessary or not? It can't, but I audited the calls. Note that one helper is foo_xmm, the other is foo_mmx: tcg_gen_addi_ptr(cpu_ptr0, cpu_env, offsetof(CPUX86State,xmm_regs[rm])); gen_helper_pmovmskb_xmm(cpu_tmp2_i32, cpu_env, cpu_ptr0); } else { rm = (modrm & 7); tcg_gen_addi_ptr(cpu_ptr0, cpu_env, offsetof(CPUX86State,fpregs[rm].mmx)); gen_helper_pmovmskb_mmx(cpu_tmp2_i32, cpu_env, cpu_ptr0); Paolo
On Wed, Jan 14, 2015 at 02:24:57PM +0100, Paolo Bonzini wrote: > On 14/01/2015 14:17, Eduardo Habkost wrote: > >>> > > Do we have (or will patch 4/4 introduce) the same bug on the > >>> > > tcg_gen_addi_ptr() calls that don't use the XMM_Q macro? > >> > > >> > No, they all call into helpers that use the XMM_Q macro themselves. > > tcg_gen_addi_ptr() is called sometimes using the fpregs[reg].mmx offset, > > and sometimes using the xmm_regs[reg] offset. How can it know if the > > XMM_Q macro is necessary or not? > > It can't, but I audited the calls. > > Note that one helper is foo_xmm, the other is foo_mmx: > > tcg_gen_addi_ptr(cpu_ptr0, cpu_env, offsetof(CPUX86State,xmm_regs[rm])); > gen_helper_pmovmskb_xmm(cpu_tmp2_i32, cpu_env, cpu_ptr0); > } else { > rm = (modrm & 7); > tcg_gen_addi_ptr(cpu_ptr0, cpu_env, offsetof(CPUX86State,fpregs[rm].mmx)); > gen_helper_pmovmskb_mmx(cpu_tmp2_i32, cpu_env, cpu_ptr0); Oh, I was assuming tcg_gen_addi_ptr() would reference data at that offset somehow, but now I see that it will just add the pointer to the offset. Looks OK to me.
diff --git a/target-i386/translate.c b/target-i386/translate.c index ebdc350..5af4300 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -3074,7 +3074,8 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b, goto illegal_op; gen_lea_modrm(env, s, modrm); if (b1 & 1) { - gen_stq_env_A0(s, offsetof(CPUX86State, xmm_regs[reg])); + gen_stq_env_A0(s, offsetof(CPUX86State, + xmm_regs[reg].XMM_Q(0))); } else { tcg_gen_ld32u_tl(cpu_T[0], cpu_env, offsetof(CPUX86State, xmm_regs[reg].XMM_L(0)));
This was accessing an XMM register's low half without going through XMM_Q. Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- target-i386/translate.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)