| Message ID | 20220303113741.2156877-2-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Check Neon VLD1/VST1 stride bits are zero | expand |
On Thu, 3 Mar 2022 at 11:37, Peter Maydell <peter.maydell@linaro.org> wrote: > > For VLD1/VST1 (single element to one lane) we are only accessing one > register, and so the 'stride' is meaningless. The bits that would > specify stride (insn bit [4] for size=1, bit [6] for size=2) are This should say "bit [5] for size=1". > specified to be zero in the encoding (which would correspond to a > stride of 1 for VLD2/VLD3/VLD4 etc), and we must UNDEF if they are > not. -- PMM
On 3/3/22 01:37, Peter Maydell wrote: > For VLD1/VST1 (single element to one lane) we are only accessing one > register, and so the 'stride' is meaningless. The bits that would > specify stride (insn bit [4] for size=1, bit [6] for size=2) are > specified to be zero in the encoding (which would correspond to a > stride of 1 for VLD2/VLD3/VLD4 etc), and we must UNDEF if they are > not. > > We failed to make this check, which meant that we would incorrectly > handle some instruction patterns as loads or stores instead of > UNDEFing them. Enforce that stride == 1 for the nregs == 1 case. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/890 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target/arm/translate-neon.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/arm/translate-neon.c b/target/arm/translate-neon.c > index 3854dd35163..072fdc1e6ee 100644 > --- a/target/arm/translate-neon.c > +++ b/target/arm/translate-neon.c > @@ -657,6 +657,9 @@ static bool trans_VLDST_single(DisasContext *s, arg_VLDST_single *a) > /* Catch the UNDEF cases. This is unavoidably a bit messy. */ > switch (nregs) { > case 1: > + if (a->stride != 1) { > + return false; > + } > if (((a->align & (1 << a->size)) != 0) || > (a->size == 2 && (a->align == 1 || a->align == 2))) { > return false; Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/arm/translate-neon.c b/target/arm/translate-neon.c index 3854dd35163..072fdc1e6ee 100644 --- a/target/arm/translate-neon.c +++ b/target/arm/translate-neon.c @@ -657,6 +657,9 @@ static bool trans_VLDST_single(DisasContext *s, arg_VLDST_single *a) /* Catch the UNDEF cases. This is unavoidably a bit messy. */ switch (nregs) { case 1: + if (a->stride != 1) { + return false; + } if (((a->align & (1 << a->size)) != 0) || (a->size == 2 && (a->align == 1 || a->align == 2))) { return false;
For VLD1/VST1 (single element to one lane) we are only accessing one register, and so the 'stride' is meaningless. The bits that would specify stride (insn bit [4] for size=1, bit [6] for size=2) are specified to be zero in the encoding (which would correspond to a stride of 1 for VLD2/VLD3/VLD4 etc), and we must UNDEF if they are not. We failed to make this check, which meant that we would incorrectly handle some instruction patterns as loads or stores instead of UNDEFing them. Enforce that stride == 1 for the nregs == 1 case. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/890 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-neon.c | 3 +++ 1 file changed, 3 insertions(+)