* [Qemu-devel] [PATCH 0/7] target/arm: Fix various underdecodings
@ 2019-01-25 18:26 Peter Maydell
2019-01-25 18:26 ` [Qemu-devel] [PATCH 1/7] target/arm/translate-a64: Don't underdecode system instructions Peter Maydell
` (6 more replies)
0 siblings, 7 replies; 16+ messages in thread
From: Peter Maydell @ 2019-01-25 18:26 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Laurent Desnogues
This patchset fixes the various cases of underdecoded instructions
that Laurent spotted and sent a bug report for. (The exception
is "missing default in disas_data_proc_1src", which got fixed in
commit 18de2813c35e359621a.)
thanks
-- PMM
Peter Maydell (7):
target/arm/translate-a64: Don't underdecode system instructions
target/arm/translate-a64: Don't underdecode PRFM
target/arm/translate-a64: Don't underdecode SIMD ld/st multiple
target/arm/translate-a64: Don't underdecode SIMD ld/st single
target/arm/translate-a64: Don't underdecode add/sub extended register
target/arm/translate-a64: Don't underdecode FP insns
target/arm/translate-a64: Don't underdecode SDOT and UDOT
target/arm/translate-a64.c | 53 +++++++++++++++++++++++++++++++++-----
1 file changed, 46 insertions(+), 7 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 1/7] target/arm/translate-a64: Don't underdecode system instructions
2019-01-25 18:26 [Qemu-devel] [PATCH 0/7] target/arm: Fix various underdecodings Peter Maydell
@ 2019-01-25 18:26 ` Peter Maydell
2019-01-28 11:09 ` Laurent Desnogues
2019-01-25 18:26 ` [Qemu-devel] [PATCH 2/7] target/arm/translate-a64: Don't underdecode PRFM Peter Maydell
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2019-01-25 18:26 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Laurent Desnogues
The "system instructions" and "system register move" subcategories
of "branches, exception generating and system instructions" for A64
only apply if bits [23:22] are zero; other values are currently
unallocated. Correctly UNDEF these unallocated encodings.
Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/translate-a64.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 4d28a27c3bd..e6df303e321 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -2144,7 +2144,11 @@ static void disas_b_exc_sys(DisasContext *s, uint32_t insn)
break;
case 0x6a: /* Exception generation / System */
if (insn & (1 << 24)) {
- disas_system(s, insn);
+ if (extract32(insn, 22, 2) == 0) {
+ disas_system(s, insn);
+ } else {
+ unallocated_encoding(s);
+ }
} else {
disas_exc(s, insn);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 2/7] target/arm/translate-a64: Don't underdecode PRFM
2019-01-25 18:26 [Qemu-devel] [PATCH 0/7] target/arm: Fix various underdecodings Peter Maydell
2019-01-25 18:26 ` [Qemu-devel] [PATCH 1/7] target/arm/translate-a64: Don't underdecode system instructions Peter Maydell
@ 2019-01-25 18:26 ` Peter Maydell
2019-01-28 11:10 ` Laurent Desnogues
2019-01-25 18:26 ` [Qemu-devel] [PATCH 3/7] target/arm/translate-a64: Don't underdecode SIMD ld/st multiple Peter Maydell
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2019-01-25 18:26 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Laurent Desnogues
The PRFM prefetch insn in the load/store with imm9 encodings
requires idx field 0b00; we were underdecoding this by
only checking !is_unpriv (which is equivalent to idx != 2).
Correctly UNDEF the unallocated encodings where idx == 0b01
and 0b11 as well as 0b10.
Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/translate-a64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index e6df303e321..8e081758e03 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -2803,7 +2803,7 @@ static void disas_ldst_reg_imm9(DisasContext *s, uint32_t insn,
} else {
if (size == 3 && opc == 2) {
/* PRFM - prefetch */
- if (is_unpriv) {
+ if (idx != 0) {
unallocated_encoding(s);
return;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 3/7] target/arm/translate-a64: Don't underdecode SIMD ld/st multiple
2019-01-25 18:26 [Qemu-devel] [PATCH 0/7] target/arm: Fix various underdecodings Peter Maydell
2019-01-25 18:26 ` [Qemu-devel] [PATCH 1/7] target/arm/translate-a64: Don't underdecode system instructions Peter Maydell
2019-01-25 18:26 ` [Qemu-devel] [PATCH 2/7] target/arm/translate-a64: Don't underdecode PRFM Peter Maydell
@ 2019-01-25 18:26 ` Peter Maydell
2019-01-28 11:11 ` Laurent Desnogues
2019-01-25 18:26 ` [Qemu-devel] [PATCH 4/7] target/arm/translate-a64: Don't underdecode SIMD ld/st single Peter Maydell
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2019-01-25 18:26 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Laurent Desnogues
In the AdvSIMD load/store multiple structures encodings,
the non-post-indexed case should have zeroes in [20:16]
(which is the Rm field for the post-indexed case).
Correctly UNDEF the currently unallocated encodings which
have non-zeroes in those bits.
Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/translate-a64.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 8e081758e03..c1f0cad7691 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -3249,6 +3249,7 @@ static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn)
{
int rt = extract32(insn, 0, 5);
int rn = extract32(insn, 5, 5);
+ int rm = extract32(insn, 16, 5);
int size = extract32(insn, 10, 2);
int opcode = extract32(insn, 12, 4);
bool is_store = !extract32(insn, 22, 1);
@@ -3268,6 +3269,11 @@ static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn)
return;
}
+ if (!is_postidx && rm != 0) {
+ unallocated_encoding(s);
+ return;
+ }
+
/* From the shared decode logic */
switch (opcode) {
case 0x0:
@@ -3367,7 +3373,6 @@ static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn)
}
if (is_postidx) {
- int rm = extract32(insn, 16, 5);
if (rm == 31) {
tcg_gen_mov_i64(tcg_rn, tcg_addr);
} else {
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 4/7] target/arm/translate-a64: Don't underdecode SIMD ld/st single
2019-01-25 18:26 [Qemu-devel] [PATCH 0/7] target/arm: Fix various underdecodings Peter Maydell
` (2 preceding siblings ...)
2019-01-25 18:26 ` [Qemu-devel] [PATCH 3/7] target/arm/translate-a64: Don't underdecode SIMD ld/st multiple Peter Maydell
@ 2019-01-25 18:26 ` Peter Maydell
2019-01-28 11:13 ` Laurent Desnogues
2019-01-25 18:26 ` [Qemu-devel] [PATCH 5/7] target/arm/translate-a64: Don't underdecode add/sub extended register Peter Maydell
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2019-01-25 18:26 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Laurent Desnogues
In the AdvSIMD load/store single structure encodings, the
non-post-indexed case should have zeroes in [20:16] (which is the
Rm field for the post-indexed case). Bit 31 must also be zero
(a check we got right in ldst_multiple but not here). Correctly
UNDEF these unallocated encodings.
Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/translate-a64.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index c1f0cad7691..2cade64ed25 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -3409,6 +3409,7 @@ static void disas_ldst_single_struct(DisasContext *s, uint32_t insn)
{
int rt = extract32(insn, 0, 5);
int rn = extract32(insn, 5, 5);
+ int rm = extract32(insn, 16, 5);
int size = extract32(insn, 10, 2);
int S = extract32(insn, 12, 1);
int opc = extract32(insn, 13, 3);
@@ -3424,6 +3425,15 @@ static void disas_ldst_single_struct(DisasContext *s, uint32_t insn)
int ebytes, xs;
TCGv_i64 tcg_addr, tcg_rn, tcg_ebytes;
+ if (extract32(insn, 31, 1)) {
+ unallocated_encoding(s);
+ return;
+ }
+ if (!is_postidx && rm != 0) {
+ unallocated_encoding(s);
+ return;
+ }
+
switch (scale) {
case 3:
if (!is_load || S) {
@@ -3501,7 +3511,6 @@ static void disas_ldst_single_struct(DisasContext *s, uint32_t insn)
}
if (is_postidx) {
- int rm = extract32(insn, 16, 5);
if (rm == 31) {
tcg_gen_mov_i64(tcg_rn, tcg_addr);
} else {
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 5/7] target/arm/translate-a64: Don't underdecode add/sub extended register
2019-01-25 18:26 [Qemu-devel] [PATCH 0/7] target/arm: Fix various underdecodings Peter Maydell
` (3 preceding siblings ...)
2019-01-25 18:26 ` [Qemu-devel] [PATCH 4/7] target/arm/translate-a64: Don't underdecode SIMD ld/st single Peter Maydell
@ 2019-01-25 18:26 ` Peter Maydell
2019-01-28 11:16 ` Laurent Desnogues
2019-01-25 18:26 ` [Qemu-devel] [PATCH 6/7] target/arm/translate-a64: Don't underdecode FP insns Peter Maydell
2019-01-25 18:26 ` [Qemu-devel] [PATCH 7/7] target/arm/translate-a64: Don't underdecode SDOT and UDOT Peter Maydell
6 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2019-01-25 18:26 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Laurent Desnogues
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 <laurent.desnogues@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 6/7] target/arm/translate-a64: Don't underdecode FP insns
2019-01-25 18:26 [Qemu-devel] [PATCH 0/7] target/arm: Fix various underdecodings Peter Maydell
` (4 preceding siblings ...)
2019-01-25 18:26 ` [Qemu-devel] [PATCH 5/7] target/arm/translate-a64: Don't underdecode add/sub extended register Peter Maydell
@ 2019-01-25 18:26 ` Peter Maydell
2019-01-28 11:19 ` Laurent Desnogues
2019-01-25 18:26 ` [Qemu-devel] [PATCH 7/7] target/arm/translate-a64: Don't underdecode SDOT and UDOT Peter Maydell
6 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2019-01-25 18:26 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Laurent Desnogues
In the encoding groups
* floating-point data-processing (1 source)
* floating-point data-processing (2 source)
* floating-point data-processing (3 source)
* floating-point immediate
* floating-point compare
* floating-ponit conditional compare
* floating-point conditional select
bit 31 is M and bit 29 is S (and bit 30 is 0, already checked at
this point in the decode). None of these groups allocate any
encoding for M=1 or S=1. We checked this in disas_fp_compare(),
disas_fp_ccomp() and disas_fp_csel(), but missed it in disas_fp_1src(),
disas_fp_2src(), disas_fp_3src() and disas_fp_imm().
We also missed that in the fp immediate encoding the imm5 field
must be all zeroes.
Correctly UNDEF the unallocated encodings here.
Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/translate-a64.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index efd2f6490b5..474d9bfb5f0 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -5636,11 +5636,17 @@ static void handle_fp_fcvt(DisasContext *s, int opcode,
*/
static void disas_fp_1src(DisasContext *s, uint32_t insn)
{
+ int mos = extract32(insn, 29, 3);
int type = extract32(insn, 22, 2);
int opcode = extract32(insn, 15, 6);
int rn = extract32(insn, 5, 5);
int rd = extract32(insn, 0, 5);
+ if (mos) {
+ unallocated_encoding(s);
+ return;
+ }
+
switch (opcode) {
case 0x4: case 0x5: case 0x7:
{
@@ -5867,13 +5873,14 @@ static void handle_fp_2src_half(DisasContext *s, int opcode,
*/
static void disas_fp_2src(DisasContext *s, uint32_t insn)
{
+ int mos = extract32(insn, 29, 3);
int type = extract32(insn, 22, 2);
int rd = extract32(insn, 0, 5);
int rn = extract32(insn, 5, 5);
int rm = extract32(insn, 16, 5);
int opcode = extract32(insn, 12, 4);
- if (opcode > 8) {
+ if (opcode > 8 || mos) {
unallocated_encoding(s);
return;
}
@@ -6028,6 +6035,7 @@ static void handle_fp_3src_half(DisasContext *s, bool o0, bool o1,
*/
static void disas_fp_3src(DisasContext *s, uint32_t insn)
{
+ int mos = extract32(insn, 29, 3);
int type = extract32(insn, 22, 2);
int rd = extract32(insn, 0, 5);
int rn = extract32(insn, 5, 5);
@@ -6036,6 +6044,11 @@ static void disas_fp_3src(DisasContext *s, uint32_t insn)
bool o0 = extract32(insn, 15, 1);
bool o1 = extract32(insn, 21, 1);
+ if (mos) {
+ unallocated_encoding(s);
+ return;
+ }
+
switch (type) {
case 0:
if (!fp_access_check(s)) {
@@ -6105,12 +6118,19 @@ uint64_t vfp_expand_imm(int size, uint8_t imm8)
static void disas_fp_imm(DisasContext *s, uint32_t insn)
{
int rd = extract32(insn, 0, 5);
+ int imm5 = extract32(insn, 5, 5);
int imm8 = extract32(insn, 13, 8);
int type = extract32(insn, 22, 2);
+ int mos = extract32(insn, 29, 3);
uint64_t imm;
TCGv_i64 tcg_res;
TCGMemOp sz;
+ if (mos || imm5) {
+ unallocated_encoding(s);
+ return;
+ }
+
switch (type) {
case 0:
sz = MO_32;
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 7/7] target/arm/translate-a64: Don't underdecode SDOT and UDOT
2019-01-25 18:26 [Qemu-devel] [PATCH 0/7] target/arm: Fix various underdecodings Peter Maydell
` (5 preceding siblings ...)
2019-01-25 18:26 ` [Qemu-devel] [PATCH 6/7] target/arm/translate-a64: Don't underdecode FP insns Peter Maydell
@ 2019-01-25 18:26 ` Peter Maydell
2019-01-28 11:20 ` Laurent Desnogues
6 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2019-01-25 18:26 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: patches, Laurent Desnogues
In the AdvSIMD scalar x indexed element and vector x indexed element
encoding group, the SDOT and UDOT instructions are vector only,
and their opcode is unallocated in the scalar group. Correctly
UNDEF this unallocated encoding.
Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/translate-a64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 474d9bfb5f0..30bc2412fc0 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -12641,7 +12641,7 @@ static void disas_simd_indexed(DisasContext *s, uint32_t insn)
break;
case 0x0e: /* SDOT */
case 0x1e: /* UDOT */
- if (size != MO_32 || !dc_isar_feature(aa64_dp, s)) {
+ if (is_scalar || size != MO_32 || !dc_isar_feature(aa64_dp, s)) {
unallocated_encoding(s);
return;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/7] target/arm/translate-a64: Don't underdecode system instructions
2019-01-25 18:26 ` [Qemu-devel] [PATCH 1/7] target/arm/translate-a64: Don't underdecode system instructions Peter Maydell
@ 2019-01-28 11:09 ` Laurent Desnogues
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Desnogues @ 2019-01-28 11:09 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, Patch Tracking
On Fri, Jan 25, 2019 at 7:26 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The "system instructions" and "system register move" subcategories
> of "branches, exception generating and system instructions" for A64
> only apply if bits [23:22] are zero; other values are currently
> unallocated. Correctly UNDEF these unallocated encodings.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Thanks,
Laurent
> ---
> target/arm/translate-a64.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 4d28a27c3bd..e6df303e321 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -2144,7 +2144,11 @@ static void disas_b_exc_sys(DisasContext *s, uint32_t insn)
> break;
> case 0x6a: /* Exception generation / System */
> if (insn & (1 << 24)) {
> - disas_system(s, insn);
> + if (extract32(insn, 22, 2) == 0) {
> + disas_system(s, insn);
> + } else {
> + unallocated_encoding(s);
> + }
> } else {
> disas_exc(s, insn);
> }
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target/arm/translate-a64: Don't underdecode PRFM
2019-01-25 18:26 ` [Qemu-devel] [PATCH 2/7] target/arm/translate-a64: Don't underdecode PRFM Peter Maydell
@ 2019-01-28 11:10 ` Laurent Desnogues
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Desnogues @ 2019-01-28 11:10 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, Patch Tracking
On Fri, Jan 25, 2019 at 7:26 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The PRFM prefetch insn in the load/store with imm9 encodings
> requires idx field 0b00; we were underdecoding this by
> only checking !is_unpriv (which is equivalent to idx != 2).
> Correctly UNDEF the unallocated encodings where idx == 0b01
> and 0b11 as well as 0b10.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Thanks,
Laurent
> ---
> target/arm/translate-a64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index e6df303e321..8e081758e03 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -2803,7 +2803,7 @@ static void disas_ldst_reg_imm9(DisasContext *s, uint32_t insn,
> } else {
> if (size == 3 && opc == 2) {
> /* PRFM - prefetch */
> - if (is_unpriv) {
> + if (idx != 0) {
> unallocated_encoding(s);
> return;
> }
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 3/7] target/arm/translate-a64: Don't underdecode SIMD ld/st multiple
2019-01-25 18:26 ` [Qemu-devel] [PATCH 3/7] target/arm/translate-a64: Don't underdecode SIMD ld/st multiple Peter Maydell
@ 2019-01-28 11:11 ` Laurent Desnogues
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Desnogues @ 2019-01-28 11:11 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, Patch Tracking
On Fri, Jan 25, 2019 at 7:26 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In the AdvSIMD load/store multiple structures encodings,
> the non-post-indexed case should have zeroes in [20:16]
> (which is the Rm field for the post-indexed case).
> Correctly UNDEF the currently unallocated encodings which
> have non-zeroes in those bits.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Thanks,
Laurent
> ---
> target/arm/translate-a64.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 8e081758e03..c1f0cad7691 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -3249,6 +3249,7 @@ static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn)
> {
> int rt = extract32(insn, 0, 5);
> int rn = extract32(insn, 5, 5);
> + int rm = extract32(insn, 16, 5);
> int size = extract32(insn, 10, 2);
> int opcode = extract32(insn, 12, 4);
> bool is_store = !extract32(insn, 22, 1);
> @@ -3268,6 +3269,11 @@ static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn)
> return;
> }
>
> + if (!is_postidx && rm != 0) {
> + unallocated_encoding(s);
> + return;
> + }
> +
> /* From the shared decode logic */
> switch (opcode) {
> case 0x0:
> @@ -3367,7 +3373,6 @@ static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn)
> }
>
> if (is_postidx) {
> - int rm = extract32(insn, 16, 5);
> if (rm == 31) {
> tcg_gen_mov_i64(tcg_rn, tcg_addr);
> } else {
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 4/7] target/arm/translate-a64: Don't underdecode SIMD ld/st single
2019-01-25 18:26 ` [Qemu-devel] [PATCH 4/7] target/arm/translate-a64: Don't underdecode SIMD ld/st single Peter Maydell
@ 2019-01-28 11:13 ` Laurent Desnogues
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Desnogues @ 2019-01-28 11:13 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, Patch Tracking
On Fri, Jan 25, 2019 at 7:26 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In the AdvSIMD load/store single structure encodings, the
> non-post-indexed case should have zeroes in [20:16] (which is the
> Rm field for the post-indexed case). Bit 31 must also be zero
> (a check we got right in ldst_multiple but not here). Correctly
> UNDEF these unallocated encodings.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Thanks,
Laurent
> ---
> target/arm/translate-a64.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index c1f0cad7691..2cade64ed25 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -3409,6 +3409,7 @@ static void disas_ldst_single_struct(DisasContext *s, uint32_t insn)
> {
> int rt = extract32(insn, 0, 5);
> int rn = extract32(insn, 5, 5);
> + int rm = extract32(insn, 16, 5);
> int size = extract32(insn, 10, 2);
> int S = extract32(insn, 12, 1);
> int opc = extract32(insn, 13, 3);
> @@ -3424,6 +3425,15 @@ static void disas_ldst_single_struct(DisasContext *s, uint32_t insn)
> int ebytes, xs;
> TCGv_i64 tcg_addr, tcg_rn, tcg_ebytes;
>
> + if (extract32(insn, 31, 1)) {
> + unallocated_encoding(s);
> + return;
> + }
> + if (!is_postidx && rm != 0) {
> + unallocated_encoding(s);
> + return;
> + }
> +
> switch (scale) {
> case 3:
> if (!is_load || S) {
> @@ -3501,7 +3511,6 @@ static void disas_ldst_single_struct(DisasContext *s, uint32_t insn)
> }
>
> if (is_postidx) {
> - int rm = extract32(insn, 16, 5);
> if (rm == 31) {
> tcg_gen_mov_i64(tcg_rn, tcg_addr);
> } else {
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] target/arm/translate-a64: Don't underdecode add/sub extended register
2019-01-25 18:26 ` [Qemu-devel] [PATCH 5/7] target/arm/translate-a64: Don't underdecode add/sub extended register Peter Maydell
@ 2019-01-28 11:16 ` Laurent Desnogues
2019-01-28 11:17 ` Peter Maydell
0 siblings, 1 reply; 16+ messages in thread
From: Laurent Desnogues @ 2019-01-28 11:16 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, Patch Tracking
On Fri, Jan 25, 2019 at 7:26 PM Peter Maydell <peter.maydell@linaro.org> 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 <laurent.desnogues@gmail.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> 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 <laurent.desnogues@gmail.com>
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] target/arm/translate-a64: Don't underdecode add/sub extended register
2019-01-28 11:16 ` Laurent Desnogues
@ 2019-01-28 11:17 ` Peter Maydell
0 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2019-01-28 11:17 UTC (permalink / raw)
To: Laurent Desnogues; +Cc: qemu-arm, qemu-devel, Patch Tracking
On Mon, 28 Jan 2019 at 11:16, Laurent Desnogues
<laurent.desnogues@gmail.com> wrote:
>
> On Fri, Jan 25, 2019 at 7:26 PM Peter Maydell <peter.maydell@linaro.org> 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 <laurent.desnogues@gmail.com>
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 6/7] target/arm/translate-a64: Don't underdecode FP insns
2019-01-25 18:26 ` [Qemu-devel] [PATCH 6/7] target/arm/translate-a64: Don't underdecode FP insns Peter Maydell
@ 2019-01-28 11:19 ` Laurent Desnogues
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Desnogues @ 2019-01-28 11:19 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, Patch Tracking
On Fri, Jan 25, 2019 at 7:26 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In the encoding groups
> * floating-point data-processing (1 source)
> * floating-point data-processing (2 source)
> * floating-point data-processing (3 source)
> * floating-point immediate
> * floating-point compare
> * floating-ponit conditional compare
> * floating-point conditional select
>
>
> bit 31 is M and bit 29 is S (and bit 30 is 0, already checked at
> this point in the decode). None of these groups allocate any
> encoding for M=1 or S=1. We checked this in disas_fp_compare(),
> disas_fp_ccomp() and disas_fp_csel(), but missed it in disas_fp_1src(),
> disas_fp_2src(), disas_fp_3src() and disas_fp_imm().
>
> We also missed that in the fp immediate encoding the imm5 field
> must be all zeroes.
>
> Correctly UNDEF the unallocated encodings here.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Thanks,
Laurent
> ---
> target/arm/translate-a64.c | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index efd2f6490b5..474d9bfb5f0 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -5636,11 +5636,17 @@ static void handle_fp_fcvt(DisasContext *s, int opcode,
> */
> static void disas_fp_1src(DisasContext *s, uint32_t insn)
> {
> + int mos = extract32(insn, 29, 3);
> int type = extract32(insn, 22, 2);
> int opcode = extract32(insn, 15, 6);
> int rn = extract32(insn, 5, 5);
> int rd = extract32(insn, 0, 5);
>
> + if (mos) {
> + unallocated_encoding(s);
> + return;
> + }
> +
> switch (opcode) {
> case 0x4: case 0x5: case 0x7:
> {
> @@ -5867,13 +5873,14 @@ static void handle_fp_2src_half(DisasContext *s, int opcode,
> */
> static void disas_fp_2src(DisasContext *s, uint32_t insn)
> {
> + int mos = extract32(insn, 29, 3);
> int type = extract32(insn, 22, 2);
> int rd = extract32(insn, 0, 5);
> int rn = extract32(insn, 5, 5);
> int rm = extract32(insn, 16, 5);
> int opcode = extract32(insn, 12, 4);
>
> - if (opcode > 8) {
> + if (opcode > 8 || mos) {
> unallocated_encoding(s);
> return;
> }
> @@ -6028,6 +6035,7 @@ static void handle_fp_3src_half(DisasContext *s, bool o0, bool o1,
> */
> static void disas_fp_3src(DisasContext *s, uint32_t insn)
> {
> + int mos = extract32(insn, 29, 3);
> int type = extract32(insn, 22, 2);
> int rd = extract32(insn, 0, 5);
> int rn = extract32(insn, 5, 5);
> @@ -6036,6 +6044,11 @@ static void disas_fp_3src(DisasContext *s, uint32_t insn)
> bool o0 = extract32(insn, 15, 1);
> bool o1 = extract32(insn, 21, 1);
>
> + if (mos) {
> + unallocated_encoding(s);
> + return;
> + }
> +
> switch (type) {
> case 0:
> if (!fp_access_check(s)) {
> @@ -6105,12 +6118,19 @@ uint64_t vfp_expand_imm(int size, uint8_t imm8)
> static void disas_fp_imm(DisasContext *s, uint32_t insn)
> {
> int rd = extract32(insn, 0, 5);
> + int imm5 = extract32(insn, 5, 5);
> int imm8 = extract32(insn, 13, 8);
> int type = extract32(insn, 22, 2);
> + int mos = extract32(insn, 29, 3);
> uint64_t imm;
> TCGv_i64 tcg_res;
> TCGMemOp sz;
>
> + if (mos || imm5) {
> + unallocated_encoding(s);
> + return;
> + }
> +
> switch (type) {
> case 0:
> sz = MO_32;
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] target/arm/translate-a64: Don't underdecode SDOT and UDOT
2019-01-25 18:26 ` [Qemu-devel] [PATCH 7/7] target/arm/translate-a64: Don't underdecode SDOT and UDOT Peter Maydell
@ 2019-01-28 11:20 ` Laurent Desnogues
0 siblings, 0 replies; 16+ messages in thread
From: Laurent Desnogues @ 2019-01-28 11:20 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, Patch Tracking
On Fri, Jan 25, 2019 at 7:26 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In the AdvSIMD scalar x indexed element and vector x indexed element
> encoding group, the SDOT and UDOT instructions are vector only,
> and their opcode is unallocated in the scalar group. Correctly
> UNDEF this unallocated encoding.
>
> Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Thanks,
Laurent
> ---
> target/arm/translate-a64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 474d9bfb5f0..30bc2412fc0 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -12641,7 +12641,7 @@ static void disas_simd_indexed(DisasContext *s, uint32_t insn)
> break;
> case 0x0e: /* SDOT */
> case 0x1e: /* UDOT */
> - if (size != MO_32 || !dc_isar_feature(aa64_dp, s)) {
> + if (is_scalar || size != MO_32 || !dc_isar_feature(aa64_dp, s)) {
> unallocated_encoding(s);
> return;
> }
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-01-28 11:20 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-25 18:26 [Qemu-devel] [PATCH 0/7] target/arm: Fix various underdecodings Peter Maydell
2019-01-25 18:26 ` [Qemu-devel] [PATCH 1/7] target/arm/translate-a64: Don't underdecode system instructions Peter Maydell
2019-01-28 11:09 ` Laurent Desnogues
2019-01-25 18:26 ` [Qemu-devel] [PATCH 2/7] target/arm/translate-a64: Don't underdecode PRFM Peter Maydell
2019-01-28 11:10 ` Laurent Desnogues
2019-01-25 18:26 ` [Qemu-devel] [PATCH 3/7] target/arm/translate-a64: Don't underdecode SIMD ld/st multiple Peter Maydell
2019-01-28 11:11 ` Laurent Desnogues
2019-01-25 18:26 ` [Qemu-devel] [PATCH 4/7] target/arm/translate-a64: Don't underdecode SIMD ld/st single Peter Maydell
2019-01-28 11:13 ` Laurent Desnogues
2019-01-25 18:26 ` [Qemu-devel] [PATCH 5/7] target/arm/translate-a64: Don't underdecode add/sub extended register Peter Maydell
2019-01-28 11:16 ` Laurent Desnogues
2019-01-28 11:17 ` Peter Maydell
2019-01-25 18:26 ` [Qemu-devel] [PATCH 6/7] target/arm/translate-a64: Don't underdecode FP insns Peter Maydell
2019-01-28 11:19 ` Laurent Desnogues
2019-01-25 18:26 ` [Qemu-devel] [PATCH 7/7] target/arm/translate-a64: Don't underdecode SDOT and UDOT Peter Maydell
2019-01-28 11:20 ` Laurent Desnogues
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).