Re: [Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[Aurelien Jarno <aurelien@aurel32.net>]

On Wed, Nov 14, 2012 at 01:38:44PM +0800, Jia Liu wrote:
> Hi guys,
> 
> On Nov 14, 2012, at 10:49 AM, 陳韋任 (Wei-Ren Chen) <chenwj@iis.sinica.edu.tw> wrote:
> 
> > Hi all,
> > 
> >  While reading microMIPS decoding, I found a possible wrong opcode
> > encoding. According to [1] page 337, the bits 13..12 for MULTU is
> > 0x01 rather than 0x00. Please review, thanks.
> > 
> > [1] MIPS Architecture for Programmers VolumeIV-e: The MIPS DSP
> >    Application-Specific Extension to the microMIPS32 Architecture
> > 
> > Signed-off-by: Chen Wei-Ren <chenwj@iis.sinica.edu.tw>
> > ---
> > target-mips/translate.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/target-mips/translate.c b/target-mips/translate.c
> > index f6fc0c2..01b48fa 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -10385,7 +10385,7 @@ enum {
> > 
> >     /* bits 13..12 for 0x32 */
> >     MULT_ACC = 0x0,
> > -    MULTU_ACC = 0x0,
> > +    MULTU_ACC = 0x1,
> > 
> >     /* bits 15..12 for 0x2c */
> >     SEB = 0x2,
> > -- 
> > 1.7.12.3
> 
> 
> try this, it is a example using exist frame.
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index f6fc0c2..04b2aae 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -10371,6 +10371,8 @@ enum {
>      MFC0 = 0x03,
>      MTC0 = 0x0b,
>  
> +    MICRODSP_ABSQ_S_QB_OP = 0x04;
> +
>      /* bits 13..12 for 0x01 */
>      MFHI_ACC = 0x0,
>      MFLO_ACC = 0x1,
> @@ -10452,6 +10454,9 @@ enum {
>      MFLO32 = 0x1,
>      MTHI32 = 0x2,
>      MTLO32 = 0x3,
> +
> +    /* bits 15..12 for MICRODSP_ABSQ_S_QB_OP */
> +    MICRODSP_ABSQ_S_QB = 0x00;
>  };
>  
>  /* POOL32B encoding of minor opcode field (bits 15..12) */
> @@ -11274,6 +11279,14 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs,
>              goto pool32axf_invalid;
>          }
>          break;
> +    case MICRODSP_ABSQ_S_QB_OP:
> +        switch (minor) {
> +        case MICRODSP_ABSQ_S_QB: // ABSQ_S_QB
> +            break;
> +        default:
> +            goto pool32axf_invalid;
> +        }
> +        break;
>      default:
>      pool32axf_invalid:
>          MIPS_INVAL("pool32axf");
> 

I don't really understand what this patch is supposed to do compared to
the original bug report and patch.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net