Check access permission to ADDVL/ADDPL/RDVL

[Qemu-devel] [PATCH] Check access permission to ADDVL/ADDPL/RDVL

Posted by Amir Charif 5 years, 1 month ago

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

Re: [Qemu-devel] [Qemu-arm] [PATCH] Check access permission to ADDVL/ADDPL/RDVL

Posted by Peter Maydell 5 years, 1 month ago

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

Re: [Qemu-devel] [Qemu-arm] [PATCH] Check access permission to ADDVL/ADDPL/RDVL

Posted by Richard Henderson 5 years, 1 month ago

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~

Re: [Qemu-devel] [Qemu-arm] [PATCH] Check access permission to ADDVL/ADDPL/RDVL

Posted by Eric Blake 5 years, 1 month ago

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

Re: [Qemu-devel] [Qemu-arm] [PATCH] Check access permission to ADDVL/ADDPL/RDVL

Posted by Peter Maydell 5 years, 1 month ago

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

Re: [Qemu-devel] [Qemu-arm] [PATCH] Check access permission to ADDVL/ADDPL/RDVL

Posted by Amir CHARIF 5 years, 1 month ago

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