diff mbox series

Check access permission to ADDVL/ADDPL/RDVL

Message ID 1552579248-31025-1-git-send-email-amir.charif@cea.fr (mailing list archive)
State New, archived
Headers show
Series Check access permission to ADDVL/ADDPL/RDVL | expand

Commit Message

Amir Charif March 14, 2019, 4 p.m. UTC
These instructions do not trap when SVE is disabled in EL0,
causing them to be executed with wrong size information.

Signed-off-by: Amir Charif <amir.charif@cea.fr>
---
 target/arm/translate-sve.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

Comments

Peter Maydell March 14, 2019, 5:21 p.m. UTC | #1
Richard, Amir -- it looks like you both sent exactly the
same patch at pretty much exactly the same time. Any
preferences for whose version I apply ? :-)

thanks
-- PMM

On Thu, 14 Mar 2019 at 16:41, Amir Charif <amir.charif@cea.fr> wrote:
>
> These instructions do not trap when SVE is disabled in EL0,
> causing them to be executed with wrong size information.
>
> Signed-off-by: Amir Charif <amir.charif@cea.fr>
> ---
>  target/arm/translate-sve.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
> index 3a2eb51..245cd82 100644
> --- a/target/arm/translate-sve.c
> +++ b/target/arm/translate-sve.c
> @@ -943,24 +943,30 @@ static bool trans_INDEX_rr(DisasContext *s, arg_INDEX_rr *a)
>
>  static bool trans_ADDVL(DisasContext *s, arg_ADDVL *a)
>  {
> -    TCGv_i64 rd = cpu_reg_sp(s, a->rd);
> -    TCGv_i64 rn = cpu_reg_sp(s, a->rn);
> -    tcg_gen_addi_i64(rd, rn, a->imm * vec_full_reg_size(s));
> +    if (sve_access_check(s)) {
> +        TCGv_i64 rd = cpu_reg_sp(s, a->rd);
> +        TCGv_i64 rn = cpu_reg_sp(s, a->rn);
> +        tcg_gen_addi_i64(rd, rn, a->imm * vec_full_reg_size(s));
> +    }
>      return true;
>  }
>
>  static bool trans_ADDPL(DisasContext *s, arg_ADDPL *a)
>  {
> -    TCGv_i64 rd = cpu_reg_sp(s, a->rd);
> -    TCGv_i64 rn = cpu_reg_sp(s, a->rn);
> -    tcg_gen_addi_i64(rd, rn, a->imm * pred_full_reg_size(s));
> +    if (sve_access_check(s)) {
> +        TCGv_i64 rd = cpu_reg_sp(s, a->rd);
> +        TCGv_i64 rn = cpu_reg_sp(s, a->rn);
> +        tcg_gen_addi_i64(rd, rn, a->imm * pred_full_reg_size(s));
> +    }
>      return true;
>  }
>
>  static bool trans_RDVL(DisasContext *s, arg_RDVL *a)
>  {
> -    TCGv_i64 reg = cpu_reg(s, a->rd);
> -    tcg_gen_movi_i64(reg, a->imm * vec_full_reg_size(s));
> +    if (sve_access_check(s)) {
> +        TCGv_i64 reg = cpu_reg(s, a->rd);
> +        tcg_gen_movi_i64(reg, a->imm * vec_full_reg_size(s));
> +    }
>      return true;
>  }
>
> --
> 2.7.4
Richard Henderson March 14, 2019, 5:34 p.m. UTC | #2
On 3/14/19 10:21 AM, Peter Maydell wrote:
> Richard, Amir -- it looks like you both sent exactly the
> same patch at pretty much exactly the same time. Any
> preferences for whose version I apply ? :-)
> 
> thanks
> -- PMM

Hah.  Bit of miscommunication there.  Might as well apply Amir's and have my
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Eric Blake March 14, 2019, 5:44 p.m. UTC | #3
On 3/14/19 12:34 PM, Richard Henderson wrote:
> On 3/14/19 10:21 AM, Peter Maydell wrote:
>> Richard, Amir -- it looks like you both sent exactly the
>> same patch at pretty much exactly the same time. Any
>> preferences for whose version I apply ? :-)
>>
>> thanks
>> -- PMM
> 
> Hah.  Bit of miscommunication there.  Might as well apply Amir's and have my
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Great minds think alike!

My personal preference, if it counts for anything, is to apply Amir's
patch, because it's always nice to have first-time contributors added in
a release.  Welcome to the community, Amir!
Peter Maydell March 14, 2019, 5:48 p.m. UTC | #4
On Thu, 14 Mar 2019 at 17:44, Eric Blake <eblake@redhat.com> wrote:
>
> On 3/14/19 12:34 PM, Richard Henderson wrote:
> > On 3/14/19 10:21 AM, Peter Maydell wrote:
> >> Richard, Amir -- it looks like you both sent exactly the
> >> same patch at pretty much exactly the same time. Any
> >> preferences for whose version I apply ? :-)
> >>
> >> thanks
> >> -- PMM
> >
> > Hah.  Bit of miscommunication there.  Might as well apply Amir's and have my
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> Great minds think alike!
>
> My personal preference, if it counts for anything, is to apply Amir's
> patch, because it's always nice to have first-time contributors added in
> a release.  Welcome to the community, Amir!

I usually tie-break by whoever sent the patch first, which in
this case is also Amir.

Applied to target-arm.next, thanks.

-- PMM
Amir Charif March 15, 2019, 8:50 a.m. UTC | #5
Thanks Eric, happy to join ^^ 

-----Message d'origine-----
De : Eric Blake <eblake@redhat.com> 
Envoyé : jeudi 14 mars 2019 18:44
À : Richard Henderson <richard.henderson@linaro.org>; Peter Maydell <peter.maydell@linaro.org>; Amir CHARIF <amir.charif@cea.fr>
Cc : qemu-arm <qemu-arm@nongnu.org>; QEMU Developers <qemu-devel@nongnu.org>
Objet : Re: [Qemu-devel] [Qemu-arm] [PATCH] Check access permission to ADDVL/ADDPL/RDVL

On 3/14/19 12:34 PM, Richard Henderson wrote:
> On 3/14/19 10:21 AM, Peter Maydell wrote:
>> Richard, Amir -- it looks like you both sent exactly the same patch 
>> at pretty much exactly the same time. Any preferences for whose 
>> version I apply ? :-)
>>
>> thanks
>> -- PMM
> 
> Hah.  Bit of miscommunication there.  Might as well apply Amir's and 
> have my
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Great minds think alike!

My personal preference, if it counts for anything, is to apply Amir's patch, because it's always nice to have first-time contributors added in a release.  Welcome to the community, Amir!

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
diff mbox series

Patch

diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 3a2eb51..245cd82 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -943,24 +943,30 @@  static bool trans_INDEX_rr(DisasContext *s, arg_INDEX_rr *a)
 
 static bool trans_ADDVL(DisasContext *s, arg_ADDVL *a)
 {
-    TCGv_i64 rd = cpu_reg_sp(s, a->rd);
-    TCGv_i64 rn = cpu_reg_sp(s, a->rn);
-    tcg_gen_addi_i64(rd, rn, a->imm * vec_full_reg_size(s));
+    if (sve_access_check(s)) {
+        TCGv_i64 rd = cpu_reg_sp(s, a->rd);
+        TCGv_i64 rn = cpu_reg_sp(s, a->rn);
+        tcg_gen_addi_i64(rd, rn, a->imm * vec_full_reg_size(s));
+    }
     return true;
 }
 
 static bool trans_ADDPL(DisasContext *s, arg_ADDPL *a)
 {
-    TCGv_i64 rd = cpu_reg_sp(s, a->rd);
-    TCGv_i64 rn = cpu_reg_sp(s, a->rn);
-    tcg_gen_addi_i64(rd, rn, a->imm * pred_full_reg_size(s));
+    if (sve_access_check(s)) {
+        TCGv_i64 rd = cpu_reg_sp(s, a->rd);
+        TCGv_i64 rn = cpu_reg_sp(s, a->rn);
+        tcg_gen_addi_i64(rd, rn, a->imm * pred_full_reg_size(s));
+    }
     return true;
 }
 
 static bool trans_RDVL(DisasContext *s, arg_RDVL *a)
 {
-    TCGv_i64 reg = cpu_reg(s, a->rd);
-    tcg_gen_movi_i64(reg, a->imm * vec_full_reg_size(s));
+    if (sve_access_check(s)) {
+        TCGv_i64 reg = cpu_reg(s, a->rd);
+        tcg_gen_movi_i64(reg, a->imm * vec_full_reg_size(s));
+    }
     return true;
 }