[PATCH v3] target/riscv: Fix shift count overflow

demin.han posted 1 patch 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240225174114.5298-1-demin.han@starfivetech.com
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
target/riscv/vector_helper.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
[PATCH v3] target/riscv: Fix shift count overflow
Posted by demin.han 9 months ago
The result of (8 - 3 - vlmul) is negative when vlmul >= 6,
and results in wrong vill.

Signed-off-by: demin.han <demin.han@starfivetech.com>
---
Changes in v2:
- Add vlen var

Changes in v3:
- Fix commit msg typo

 target/riscv/vector_helper.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 84cec73eb2..fe56c007d5 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -44,6 +44,7 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
     target_ulong reserved = s2 &
                             MAKE_64BIT_MASK(R_VTYPE_RESERVED_SHIFT,
                                             xlen - 1 - R_VTYPE_RESERVED_SHIFT);
+    uint16_t vlen = cpu->cfg.vlenb << 3;
     int8_t lmul;
 
     if (vlmul & 4) {
@@ -53,10 +54,8 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
          * VLEN * LMUL >= SEW
          * VLEN >> (8 - lmul) >= sew
          * (vlenb << 3) >> (8 - lmul) >= sew
-         * vlenb >> (8 - 3 - lmul) >= sew
          */
-        if (vlmul == 4 ||
-            cpu->cfg.vlenb >> (8 - 3 - vlmul) < sew) {
+        if (vlmul == 4 || (vlen >> (8 - vlmul)) < sew) {
             vill = true;
         }
     }
-- 
2.43.2
Re: [PATCH v3] target/riscv: Fix shift count overflow
Posted by Alistair Francis 8 months, 3 weeks ago
On Mon, Feb 26, 2024 at 3:42 AM demin.han <demin.han@starfivetech.com> wrote:
>
> The result of (8 - 3 - vlmul) is negative when vlmul >= 6,
> and results in wrong vill.
>
> Signed-off-by: demin.han <demin.han@starfivetech.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> Changes in v2:
> - Add vlen var
>
> Changes in v3:
> - Fix commit msg typo
>
>  target/riscv/vector_helper.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 84cec73eb2..fe56c007d5 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -44,6 +44,7 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
>      target_ulong reserved = s2 &
>                              MAKE_64BIT_MASK(R_VTYPE_RESERVED_SHIFT,
>                                              xlen - 1 - R_VTYPE_RESERVED_SHIFT);
> +    uint16_t vlen = cpu->cfg.vlenb << 3;
>      int8_t lmul;
>
>      if (vlmul & 4) {
> @@ -53,10 +54,8 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
>           * VLEN * LMUL >= SEW
>           * VLEN >> (8 - lmul) >= sew
>           * (vlenb << 3) >> (8 - lmul) >= sew
> -         * vlenb >> (8 - 3 - lmul) >= sew
>           */
> -        if (vlmul == 4 ||
> -            cpu->cfg.vlenb >> (8 - 3 - vlmul) < sew) {
> +        if (vlmul == 4 || (vlen >> (8 - vlmul)) < sew) {
>              vill = true;
>          }
>      }
> --
> 2.43.2
>
>
Re: [PATCH v3] target/riscv: Fix shift count overflow
Posted by Daniel Henrique Barboza 9 months ago

On 2/25/24 14:41, demin.han wrote:
> The result of (8 - 3 - vlmul) is negative when vlmul >= 6,
> and results in wrong vill.
> 
> Signed-off-by: demin.han <demin.han@starfivetech.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

> Changes in v2:
> - Add vlen var
> 
> Changes in v3:
> - Fix commit msg typo
> 
>   target/riscv/vector_helper.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 84cec73eb2..fe56c007d5 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -44,6 +44,7 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
>       target_ulong reserved = s2 &
>                               MAKE_64BIT_MASK(R_VTYPE_RESERVED_SHIFT,
>                                               xlen - 1 - R_VTYPE_RESERVED_SHIFT);
> +    uint16_t vlen = cpu->cfg.vlenb << 3;
>       int8_t lmul;
>   
>       if (vlmul & 4) {
> @@ -53,10 +54,8 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
>            * VLEN * LMUL >= SEW
>            * VLEN >> (8 - lmul) >= sew
>            * (vlenb << 3) >> (8 - lmul) >= sew
> -         * vlenb >> (8 - 3 - lmul) >= sew
>            */
> -        if (vlmul == 4 ||
> -            cpu->cfg.vlenb >> (8 - 3 - vlmul) < sew) {
> +        if (vlmul == 4 || (vlen >> (8 - vlmul)) < sew) {
>               vill = true;
>           }
>       }
Re: [PATCH v3] target/riscv: Fix shift count overflow
Posted by Daniel Henrique Barboza 8 months, 3 weeks ago
Hi Alistair,


Please don't forget to queue this up. Thanks,


Daniel

On 2/26/24 06:31, Daniel Henrique Barboza wrote:
> 
> 
> On 2/25/24 14:41, demin.han wrote:
>> The result of (8 - 3 - vlmul) is negative when vlmul >= 6,
>> and results in wrong vill.
>>
>> Signed-off-by: demin.han <demin.han@starfivetech.com>
>> ---
> 
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> 
>> Changes in v2:
>> - Add vlen var
>>
>> Changes in v3:
>> - Fix commit msg typo
>>
>>   target/riscv/vector_helper.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
>> index 84cec73eb2..fe56c007d5 100644
>> --- a/target/riscv/vector_helper.c
>> +++ b/target/riscv/vector_helper.c
>> @@ -44,6 +44,7 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
>>       target_ulong reserved = s2 &
>>                               MAKE_64BIT_MASK(R_VTYPE_RESERVED_SHIFT,
>>                                               xlen - 1 - R_VTYPE_RESERVED_SHIFT);
>> +    uint16_t vlen = cpu->cfg.vlenb << 3;
>>       int8_t lmul;
>>       if (vlmul & 4) {
>> @@ -53,10 +54,8 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
>>            * VLEN * LMUL >= SEW
>>            * VLEN >> (8 - lmul) >= sew
>>            * (vlenb << 3) >> (8 - lmul) >= sew
>> -         * vlenb >> (8 - 3 - lmul) >= sew
>>            */
>> -        if (vlmul == 4 ||
>> -            cpu->cfg.vlenb >> (8 - 3 - vlmul) < sew) {
>> +        if (vlmul == 4 || (vlen >> (8 - vlmul)) < sew) {
>>               vill = true;
>>           }
>>       }