Re: [Qemu-trivial] [PATCH] target-mips: fix incorrect behaviour for INSV

[Michael Tokarev <mjt@tls.msk.ru>]

08.05.2013 15:17, Petar Jovanovic wrote:
> From: Petar Jovanovic <petar.jovanovic@imgtec.com>
> 
> Corner case for INSV instruction when size=32 has not been correctly
> implemented. The mask for size should be one bit wider, and preparing the
> filter variable should be aware of this case too.
> 
> The test for INSV has been extended to include the case that triggers the
> bug.

This isn't really a good fit for a trivial patch.  Cc'ing qemu-devel
and mips "odd-fixes" maintainer instead.

Thanks!

/mjt

> Signed-off-by: Petar Jovanovic <petar.jovanovic@imgtec.com>
> ---
>  target-mips/dsp_helper.c         |    4 ++--
>  tests/tcg/mips/mips32-dsp/insv.c |   13 +++++++++++++
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index 805247d..9212789 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -2921,7 +2921,7 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong rs,  \
>          return rt;                                              \
>      }                                                           \
>                                                                  \
> -    filter = ((int32_t)0x01 << size) - 1;                       \
> +    filter = ((int64_t)0x01 << size) - 1;                       \
>      filter = filter << pos;                                     \
>      temprs = (rs << pos) & filter;                              \
>      temprt = rt & ~filter;                                      \
> @@ -2930,7 +2930,7 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong rs,  \
>      return (target_long)(ret_type)temp;                         \
>  }
>  
> -BIT_INSV(insv, 0x1F, 0x1F, int32_t);
> +BIT_INSV(insv, 0x1F, 0x3F, int32_t);
>  #ifdef TARGET_MIPS64
>  BIT_INSV(dinsv, 0x7F, 0x3F, target_long);
>  #endif
> diff --git a/tests/tcg/mips/mips32-dsp/insv.c b/tests/tcg/mips/mips32-dsp/insv.c
> index 243b007..9d67469 100644
> --- a/tests/tcg/mips/mips32-dsp/insv.c
> +++ b/tests/tcg/mips/mips32-dsp/insv.c
> @@ -19,5 +19,18 @@ int main()
>          );
>      assert(rt == result);
>  
> +    dsp    = 0x1000;
> +    rt     = 0xF0F0F0F0;
> +    rs     = 0xA5A5A5A5;
> +    result = 0xA5A5A5A5;
> +
> +    __asm
> +        ("wrdsp %2\n\t"
> +         "insv  %0, %1\n\t"
> +         : "+r"(rt)
> +         : "r"(rs), "r"(dsp)
> +        );
> +    assert(rt == result);
> +
>      return 0;
>  }
>