On Mon, May 13, 2013 at 03:20:27PM +0200, Petar Jovanovic wrote: > From: Petar Jovanovic <petar.jovano...@imgtec.com> > > This change makes sure that modifications of pos field in the DSPControl > register do not trash other bits in the register. This bug can be triggered > with the additional test case in mips32-dsp/extpdp.c in this commit. > > In addition to this, this change corrects incorrect calculation of the mask > for EXTPDP. > > Signed-off-by: Petar Jovanovic <petar.jovano...@imgtec.com> > --- > target-mips/dsp_helper.c | 9 ++++----- > tests/tcg/mips/mips32-dsp/extpdp.c | 18 ++++++++++++++++++ > 2 files changed, 22 insertions(+), 5 deletions(-) > > diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c > index a55f866..27283ed 100644 > --- a/target-mips/dsp_helper.c > +++ b/target-mips/dsp_helper.c > @@ -90,10 +90,10 @@ static inline void set_DSPControl_pos(uint32_t pos, > CPUMIPSState *env) > dspc = env->active_tc.DSPControl; > #ifndef TARGET_MIPS64 > dspc = dspc & 0xFFFFFFC0; > - dspc |= pos; > + dspc |= (pos & 0x3F); > #else > dspc = dspc & 0xFFFFFF80; > - dspc |= pos; > + dspc |= (pos & 0x7F); > #endif > env->active_tc.DSPControl = dspc; > } > @@ -3439,10 +3439,9 @@ target_ulong helper_extpdp(target_ulong ac, > target_ulong size, > if (sub >= -1) { > acc = ((uint64_t)env->active_tc.HI[ac] << 32) | > ((uint64_t)env->active_tc.LO[ac] & MIPSDSP_LLO); > - temp = (acc >> (start_pos - size)) & > - (((uint32_t)0x01 << (size + 1)) - 1); > + temp = (acc >> (start_pos - size)) & (~0U >> (31 - size)); > > - set_DSPControl_pos(start_pos - (size + 1), env); > + set_DSPControl_pos(sub & 0x3F, env);
I am not sure it is correct there for the 64-bit mode. The value that should be written should be DSPcontrolpos6..0 - (size + 1), therefore keeeping only last 6 bits doesn't seems correct to me. Given the change above, I think it should simply be set_DSPControl_pos(sub, env); > set_DSPControl_efi(0, env); > } else { > set_DSPControl_efi(1, env); > diff --git a/tests/tcg/mips/mips32-dsp/extpdp.c > b/tests/tcg/mips/mips32-dsp/extpdp.c > index 15ba082..79ee16e 100644 > --- a/tests/tcg/mips/mips32-dsp/extpdp.c > +++ b/tests/tcg/mips/mips32-dsp/extpdp.c > @@ -42,5 +42,23 @@ int main() > efi = (dsp >> 14) & 0x01; > assert(efi == 1); > > + > + ach = 0; > + acl = 0; > + dsp = 0; > + result = 0; > + > + __asm > + ("wrdsp %1\n\t" > + "mthi %2, $ac1\n\t" > + "mtlo %3, $ac1\n\t" > + "extpdp %0, $ac1, 0x00\n\t" > + "rddsp %1\n\t" > + : "=r"(rt), "+r"(dsp) > + : "r"(ach), "r"(acl) > + ); > + assert(dsp == 0x3F); > + assert(result == rt); > + > return 0; > } > -- > 1.7.9.5 > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net