Re: [Qemu-devel] [PATCH 5/7] target/arm/translate-a64: Don't underdecode add/sub extended register

[Laurent Desnogues]

On Fri, Jan 25, 2019 at 7:26 PM Peter Maydell  wrote:
>
> In the "add/subtract (extended register)" encoding group, the "opt"
> field in bits [23:22] must be zero. Correctly UNDEF the unallocated
> encodings where this field is not zero.
>
> Reported-by: Laurent Desnogues 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/translate-a64.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 2cade64ed25..efd2f6490b5 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -4204,12 +4204,13 @@ static void disas_add_sub_ext_reg(DisasContext *s, 
> uint32_t insn)
>  bool setflags = extract32(insn, 29, 1);
>  bool sub_op = extract32(insn, 30, 1);
>  bool sf = extract32(insn, 31, 1);
> +bool opt = extract32(insn, 22, 2);

I'd prefer an int to a bool.

Otherwise:

Reviewed-by: Laurent Desnogues 

Thanks,

Laurent

>  TCGv_i64 tcg_rm, tcg_rn; /* temps */
>  TCGv_i64 tcg_rd;
>  TCGv_i64 tcg_result;
>
> -if (imm3 > 4) {
> +if (imm3 > 4 || opt != 0) {
>  unallocated_encoding(s);
>  return;
>  }
> --
> 2.20.1
>

Re: [Qemu-devel] [PATCH 5/7] target/arm/translate-a64: Don't underdecode add/sub extended register

[Peter Maydell]

On Mon, 28 Jan 2019 at 11:16, Laurent Desnogues
 wrote:
>
> On Fri, Jan 25, 2019 at 7:26 PM Peter Maydell  
> wrote:
> >
> > In the "add/subtract (extended register)" encoding group, the "opt"
> > field in bits [23:22] must be zero. Correctly UNDEF the unallocated
> > encodings where this field is not zero.
> >
> > Reported-by: Laurent Desnogues 
> > Signed-off-by: Peter Maydell 
> > ---
> >  target/arm/translate-a64.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> > index 2cade64ed25..efd2f6490b5 100644
> > --- a/target/arm/translate-a64.c
> > +++ b/target/arm/translate-a64.c
> > @@ -4204,12 +4204,13 @@ static void disas_add_sub_ext_reg(DisasContext *s, 
> > uint32_t insn)
> >  bool setflags = extract32(insn, 29, 1);
> >  bool sub_op = extract32(insn, 30, 1);
> >  bool sf = extract32(insn, 31, 1);
> > +bool opt = extract32(insn, 22, 2);
>
> I'd prefer an int to a bool.

Oops, yes. I think I got mixed up with all the other bool
flags here, but it's a 2 bit field so definitely should be int.

thanks
-- PMM

[Qemu-devel] [PATCH 5/7] target/arm/translate-a64: Don't underdecode add/sub extended register

[Peter Maydell]

In the "add/subtract (extended register)" encoding group, the "opt"
field in bits [23:22] must be zero. Correctly UNDEF the unallocated
encodings where this field is not zero.

Reported-by: Laurent Desnogues 
Signed-off-by: Peter Maydell 
---
 target/arm/translate-a64.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 2cade64ed25..efd2f6490b5 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -4204,12 +4204,13 @@ static void disas_add_sub_ext_reg(DisasContext *s, 
uint32_t insn)
 bool setflags = extract32(insn, 29, 1);
 bool sub_op = extract32(insn, 30, 1);
 bool sf = extract32(insn, 31, 1);
+bool opt = extract32(insn, 22, 2);
 
 TCGv_i64 tcg_rm, tcg_rn; /* temps */
 TCGv_i64 tcg_rd;
 TCGv_i64 tcg_result;
 
-if (imm3 > 4) {
+if (imm3 > 4 || opt != 0) {
 unallocated_encoding(s);
 return;
 }
-- 
2.20.1