[PATCH v4] target/loongarch: fix vldi/xvldi raise wrong error

Song Gao posted 1 patch 5 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250603082510.353876-1-gaosong@loongson.cn
Maintainers: Song Gao <gaosong@loongson.cn>
There is a newer version of this series
target/loongarch/tcg/insn_trans/trans_vec.c.inc | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
[PATCH v4] target/loongarch: fix vldi/xvldi raise wrong error
Posted by Song Gao 5 months, 2 weeks ago
on qemu we got an aborted error
**
ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
Bail out! ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
Aborted (core dumped)
bu on 3A600/3A5000 we got a "Illegal instruction" error.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2971

Signed-off-by: Song Gao <gaosong@loongson.cn>
---
 target/loongarch/tcg/insn_trans/trans_vec.c.inc | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/target/loongarch/tcg/insn_trans/trans_vec.c.inc b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
index dff92772ad..9fb72fe914 100644
--- a/target/loongarch/tcg/insn_trans/trans_vec.c.inc
+++ b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
@@ -3465,7 +3465,7 @@ TRANS(xvmsknz_b, LASX, gen_xx, gen_helper_vmsknz_b)
 static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
 {
     int mode;
-    uint64_t data, t;
+    uint64_t data = 0, t;
 
     /*
      * imm bit [11:8] is mode, mode value is 0-12.
@@ -3568,19 +3568,27 @@ static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
             t1 = (b7 << 9) | ((1-b6) << 8) | (b6 ? 0xff : 0);
             data = (t1 << 54) | (t0 << 48);
         }
-        break;
     default:
-        generate_exception(ctx, EXCCODE_INE);
         g_assert_not_reached();
+        break;
     }
     return data;
 }
 
+static bool check_vldi_mode(arg_vldi *a)
+{
+   return (a->imm >>8 & 0xf) <= 12;
+}
 static bool gen_vldi(DisasContext *ctx, arg_vldi *a, uint32_t oprsz)
 {
     int sel, vece;
     uint64_t value;
 
+    if (!check_vldi_mode(a)){
+        generate_exception(ctx, EXCCODE_INE);
+        return true;
+    }
+
     if (!check_vec(ctx, oprsz)) {
         return true;
     }
-- 
2.34.1
Re: [PATCH v4] target/loongarch: fix vldi/xvldi raise wrong error
Posted by Philippe Mathieu-Daudé 5 months, 2 weeks ago
On 3/6/25 10:25, Song Gao wrote:
> on qemu we got an aborted error
> **
> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
> Bail out! ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
> Aborted (core dumped)
> bu on 3A600/3A5000 we got a "Illegal instruction" error.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2971
> 
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   target/loongarch/tcg/insn_trans/trans_vec.c.inc | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/target/loongarch/tcg/insn_trans/trans_vec.c.inc b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
> index dff92772ad..9fb72fe914 100644
> --- a/target/loongarch/tcg/insn_trans/trans_vec.c.inc
> +++ b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
> @@ -3465,7 +3465,7 @@ TRANS(xvmsknz_b, LASX, gen_xx, gen_helper_vmsknz_b)
>   static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
>   {
>       int mode;
> -    uint64_t data, t;
> +    uint64_t data = 0, t;
>   
>       /*
>        * imm bit [11:8] is mode, mode value is 0-12.
> @@ -3568,19 +3568,27 @@ static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
>               t1 = (b7 << 9) | ((1-b6) << 8) | (b6 ? 0xff : 0);
>               data = (t1 << 54) | (t0 << 48);
>           }
> -        break;

Dubious because previous 'data' would be dead...

>       default:
> -        generate_exception(ctx, EXCCODE_INE);
>           g_assert_not_reached();
> +        break;
>       }
>       return data;
>   }
>   
> +static bool check_vldi_mode(arg_vldi *a)
> +{
> +   return (a->imm >>8 & 0xf) <= 12;
> +}
>   static bool gen_vldi(DisasContext *ctx, arg_vldi *a, uint32_t oprsz)
>   {
>       int sel, vece;
>       uint64_t value;
>   
> +    if (!check_vldi_mode(a)){
> +        generate_exception(ctx, EXCCODE_INE);
> +        return true;
> +    }
> +
>       if (!check_vec(ctx, oprsz)) {
>           return true;
>       }
Re: [PATCH v4] target/loongarch: fix vldi/xvldi raise wrong error
Posted by gaosong 5 months, 2 weeks ago
在 2025/6/3 下午7:15, Philippe Mathieu-Daudé 写道:
> On 3/6/25 10:25, Song Gao wrote:
>> on qemu we got an aborted error
>> **
>> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: 
>> code should not be reached
>> Bail out! 
>> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: 
>> code should not be reached
>> Aborted (core dumped)
>> bu on 3A600/3A5000 we got a "Illegal instruction" error.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2971
>>
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> ---
>>   target/loongarch/tcg/insn_trans/trans_vec.c.inc | 14 +++++++++++---
>>   1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/loongarch/tcg/insn_trans/trans_vec.c.inc 
>> b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
>> index dff92772ad..9fb72fe914 100644
>> --- a/target/loongarch/tcg/insn_trans/trans_vec.c.inc
>> +++ b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
>> @@ -3465,7 +3465,7 @@ TRANS(xvmsknz_b, LASX, gen_xx, 
>> gen_helper_vmsknz_b)
>>   static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
>>   {
>>       int mode;
>> -    uint64_t data, t;
>> +    uint64_t data = 0, t;
>>         /*
>>        * imm bit [11:8] is mode, mode value is 0-12.
>> @@ -3568,19 +3568,27 @@ static uint64_t vldi_get_value(DisasContext 
>> *ctx, uint32_t imm)
>>               t1 = (b7 << 9) | ((1-b6) << 8) | (b6 ? 0xff : 0);
>>               data = (t1 << 54) | (t0 << 48);
>>           }
>> -        break;
>
> Dubious because previous 'data' would be dead...
I made a mistake here, thanks for  pointing it out

thanks.
Song Gao
>
>>       default:
>> -        generate_exception(ctx, EXCCODE_INE);
>>           g_assert_not_reached();
>> +        break;
>>       }
>>       return data;
>>   }
>>   +static bool check_vldi_mode(arg_vldi *a)
>> +{
>> +   return (a->imm >>8 & 0xf) <= 12;
>> +}
>>   static bool gen_vldi(DisasContext *ctx, arg_vldi *a, uint32_t oprsz)
>>   {
>>       int sel, vece;
>>       uint64_t value;
>>   +    if (!check_vldi_mode(a)){
>> +        generate_exception(ctx, EXCCODE_INE);
>> +        return true;
>> +    }
>> +
>>       if (!check_vec(ctx, oprsz)) {
>>           return true;
>>       }


Re: [PATCH v4] target/loongarch: fix vldi/xvldi raise wrong error
Posted by Bibo Mao 5 months, 2 weeks ago
Song,

It is a little strange that patch with three version is sent in one day.
Maybe we should keep careful and calm :-)

Regards
Bibo Mao

On 2025/6/3 下午4:25, Song Gao wrote:
> on qemu we got an aborted error
> **
> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
> Bail out! ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
> Aborted (core dumped)
> bu on 3A600/3A5000 we got a "Illegal instruction" error.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2971
> 
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   target/loongarch/tcg/insn_trans/trans_vec.c.inc | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/target/loongarch/tcg/insn_trans/trans_vec.c.inc b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
> index dff92772ad..9fb72fe914 100644
> --- a/target/loongarch/tcg/insn_trans/trans_vec.c.inc
> +++ b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
> @@ -3465,7 +3465,7 @@ TRANS(xvmsknz_b, LASX, gen_xx, gen_helper_vmsknz_b)
>   static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
>   {
>       int mode;
> -    uint64_t data, t;
> +    uint64_t data = 0, t;
>   
>       /*
>        * imm bit [11:8] is mode, mode value is 0-12.
> @@ -3568,19 +3568,27 @@ static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
>               t1 = (b7 << 9) | ((1-b6) << 8) | (b6 ? 0xff : 0);
>               data = (t1 << 54) | (t0 << 48);
>           }
> -        break;
>       default:
> -        generate_exception(ctx, EXCCODE_INE);
>           g_assert_not_reached();
> +        break;
>       }
>       return data;
>   }
>   
> +static bool check_vldi_mode(arg_vldi *a)
> +{
> +   return (a->imm >>8 & 0xf) <= 12;
> +}
>   static bool gen_vldi(DisasContext *ctx, arg_vldi *a, uint32_t oprsz)
>   {
>       int sel, vece;
>       uint64_t value;
>   
> +    if (!check_vldi_mode(a)){
> +        generate_exception(ctx, EXCCODE_INE);
> +        return true;
> +    }
> +
>       if (!check_vec(ctx, oprsz)) {
>           return true;
>       }
> 


Re: [PATCH v4] target/loongarch: fix vldi/xvldi raise wrong error
Posted by gaosong 5 months, 2 weeks ago
在 2025/6/3 下午5:59, Bibo Mao 写道:
> Song,
>
> It is a little strange that patch with three version is sent in one day.
> Maybe we should keep careful and calm :-)
>
yes  agreed,
I should be more careful and calm. :-)

thanks.
Song Gao
> Regards
> Bibo Mao
>
> On 2025/6/3 下午4:25, Song Gao wrote:
>> on qemu we got an aborted error
>> **
>> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: 
>> code should not be reached
>> Bail out! 
>> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: 
>> code should not be reached
>> Aborted (core dumped)
>> bu on 3A600/3A5000 we got a "Illegal instruction" error.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2971
>>
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> ---
>>   target/loongarch/tcg/insn_trans/trans_vec.c.inc | 14 +++++++++++---
>>   1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/loongarch/tcg/insn_trans/trans_vec.c.inc 
>> b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
>> index dff92772ad..9fb72fe914 100644
>> --- a/target/loongarch/tcg/insn_trans/trans_vec.c.inc
>> +++ b/target/loongarch/tcg/insn_trans/trans_vec.c.inc
>> @@ -3465,7 +3465,7 @@ TRANS(xvmsknz_b, LASX, gen_xx, 
>> gen_helper_vmsknz_b)
>>   static uint64_t vldi_get_value(DisasContext *ctx, uint32_t imm)
>>   {
>>       int mode;
>> -    uint64_t data, t;
>> +    uint64_t data = 0, t;
>>         /*
>>        * imm bit [11:8] is mode, mode value is 0-12.
>> @@ -3568,19 +3568,27 @@ static uint64_t vldi_get_value(DisasContext 
>> *ctx, uint32_t imm)
>>               t1 = (b7 << 9) | ((1-b6) << 8) | (b6 ? 0xff : 0);
>>               data = (t1 << 54) | (t0 << 48);
>>           }
>> -        break;
>>       default:
>> -        generate_exception(ctx, EXCCODE_INE);
>>           g_assert_not_reached();
>> +        break;
>>       }
>>       return data;
>>   }
>>   +static bool check_vldi_mode(arg_vldi *a)
>> +{
>> +   return (a->imm >>8 & 0xf) <= 12;
>> +}
>>   static bool gen_vldi(DisasContext *ctx, arg_vldi *a, uint32_t oprsz)
>>   {
>>       int sel, vece;
>>       uint64_t value;
>>   +    if (!check_vldi_mode(a)){
>> +        generate_exception(ctx, EXCCODE_INE);
>> +        return true;
>> +    }
>> +
>>       if (!check_vec(ctx, oprsz)) {
>>           return true;
>>       }
>>


Re: [PATCH v4] target/loongarch: fix vldi/xvldi raise wrong error
Posted by Michael Tokarev 5 months, 2 weeks ago
03.06.2025 11:25, Song Gao wrote:
> on qemu we got an aborted error
> **
> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
> Bail out! ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: code should not be reached
> Aborted (core dumped)
> bu on 3A600/3A5000 we got a "Illegal instruction" error.

What is "bu" ?  Maybe it meant to be "but"?

Thanks,

/mjt
Re: [PATCH v4] target/loongarch: fix vldi/xvldi raise wrong error
Posted by gaosong 5 months, 2 weeks ago
在 2025/6/3 下午5:44, Michael Tokarev 写道:
> 03.06.2025 11:25, Song Gao wrote:
>> on qemu we got an aborted error
>> **
>> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: 
>> code should not be reached
>> Bail out! 
>> ERROR:../target/loongarch/tcg/insn_trans/trans_vec.c.inc:3574:vldi_get_value: 
>> code should not be reached
>> Aborted (core dumped)
>> bu on 3A600/3A5000 we got a "Illegal instruction" error.
>
> What is "bu" ?  Maybe it meant to be "but"?
>
'but'.
thanks for pointint it out.

thanks.
Song Gao
> Thanks,
>
> /mjt