1. Patchew
  2. QEMU
  3. View series

[PATCH] target/loongarch: fix vcpu reset command word issue

Xianglai Li posted 1 patch 1 year ago
Failed in applying to current master (apply log)
There is a newer version of this series
target/loongarch/kvm/kvm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] target/loongarch: fix vcpu reset command word issue
Posted by Xianglai Li 1 year ago 
When the KVM_REG_LOONGARCH_VCPU_RESET command word
is sent to the kernel through the kvm_set_one_reg interface,
the parameter source needs to be a legal address,
otherwise the kernel will return an error and the command word
will fail to be sent.

Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
---
Cc: Bibo Mao <Maobibo@loongson.cn>
Cc: Song Gao <gaosong@loongson.cn>

 target/loongarch/kvm/kvm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
index a3f55155b0..01cddb7012 100644
--- a/target/loongarch/kvm/kvm.c
+++ b/target/loongarch/kvm/kvm.c
@@ -581,9 +581,10 @@ static int kvm_loongarch_get_lbt(CPUState *cs)
 void kvm_arch_reset_vcpu(CPUState *cs)
 {
     CPULoongArchState *env = cpu_env(cs);
+    uint64_t val;
 
     env->mp_state = KVM_MP_STATE_RUNNABLE;
-    kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, 0);
+    kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
 }
 
 static int kvm_loongarch_get_mpstate(CPUState *cs)
-- 
2.39.1
Re: [PATCH] target/loongarch: fix vcpu reset command word issue
Posted by bibo mao 1 year ago 
On 2025/2/5 下午8:06, Xianglai Li wrote:
> When the KVM_REG_LOONGARCH_VCPU_RESET command word
> is sent to the kernel through the kvm_set_one_reg interface,
> the parameter source needs to be a legal address,
> otherwise the kernel will return an error and the command word
> will fail to be sent.
Hi Xianglai,

Good catch, it is actually one problem and thanks for reporting it.

> 
> Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
> ---
> Cc: Bibo Mao <Maobibo@loongson.cn>
> Cc: Song Gao <gaosong@loongson.cn>
> 
>   target/loongarch/kvm/kvm.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
> index a3f55155b0..01cddb7012 100644
> --- a/target/loongarch/kvm/kvm.c
> +++ b/target/loongarch/kvm/kvm.c
> @@ -581,9 +581,10 @@ static int kvm_loongarch_get_lbt(CPUState *cs)
>   void kvm_arch_reset_vcpu(CPUState *cs)
>   {
>       CPULoongArchState *env = cpu_env(cs);
> +    uint64_t val;
How about set initial value here although it is not used? such as
        uint64_t val = 0;
>   
>       env->mp_state = KVM_MP_STATE_RUNNABLE;
> -    kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, 0);
> +    kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
Can we add return value checking here? such as
        ret = kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
        if (ret) {
            error_report("... %s", strerror(errno));
        }

Regards
Bibo Mao
>   }
>   
>   static int kvm_loongarch_get_mpstate(CPUState *cs)
>
Re: [PATCH] target/loongarch: fix vcpu reset command word issue
Posted by Philippe Mathieu-Daudé 1 year ago 
On 6/2/25 03:34, bibo mao wrote:
> On 2025/2/5 下午8:06, Xianglai Li wrote:
>> When the KVM_REG_LOONGARCH_VCPU_RESET command word
>> is sent to the kernel through the kvm_set_one_reg interface,
>> the parameter source needs to be a legal address,
>> otherwise the kernel will return an error and the command word
>> will fail to be sent.
> Hi Xianglai,
> 
> Good catch, it is actually one problem and thanks for reporting it.
> 
>>
>> Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
>> ---
>> Cc: Bibo Mao <Maobibo@loongson.cn>
>> Cc: Song Gao <gaosong@loongson.cn>
>>
>>   target/loongarch/kvm/kvm.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
>> index a3f55155b0..01cddb7012 100644
>> --- a/target/loongarch/kvm/kvm.c
>> +++ b/target/loongarch/kvm/kvm.c
>> @@ -581,9 +581,10 @@ static int kvm_loongarch_get_lbt(CPUState *cs)
>>   void kvm_arch_reset_vcpu(CPUState *cs)
>>   {
>>       CPULoongArchState *env = cpu_env(cs);
>> +    uint64_t val;
> How about set initial value here although it is not used? such as
>         uint64_t val = 0;

Or        uint64_t unused = 0;

>>       env->mp_state = KVM_MP_STATE_RUNNABLE;
>> -    kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, 0);
>> +    kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
> Can we add return value checking here? such as
>         ret = kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
>         if (ret) {
>             error_report("... %s", strerror(errno));
>         }
> 
> Regards
> Bibo Mao
>>   }
>>   static int kvm_loongarch_get_mpstate(CPUState *cs)
>>
> 
>
Re: [PATCH] target/loongarch: fix vcpu reset command word issue
Posted by bibo mao 1 year ago 

On 2025/2/6 下午5:02, Philippe Mathieu-Daudé wrote:
> On 6/2/25 03:34, bibo mao wrote:
>> On 2025/2/5 下午8:06, Xianglai Li wrote:
>>> When the KVM_REG_LOONGARCH_VCPU_RESET command word
>>> is sent to the kernel through the kvm_set_one_reg interface,
>>> the parameter source needs to be a legal address,
>>> otherwise the kernel will return an error and the command word
>>> will fail to be sent.
>> Hi Xianglai,
>>
>> Good catch, it is actually one problem and thanks for reporting it.
>>
>>>
>>> Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
>>> ---
>>> Cc: Bibo Mao <Maobibo@loongson.cn>
>>> Cc: Song Gao <gaosong@loongson.cn>
>>>
>>>   target/loongarch/kvm/kvm.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
>>> index a3f55155b0..01cddb7012 100644
>>> --- a/target/loongarch/kvm/kvm.c
>>> +++ b/target/loongarch/kvm/kvm.c
>>> @@ -581,9 +581,10 @@ static int kvm_loongarch_get_lbt(CPUState *cs)
>>>   void kvm_arch_reset_vcpu(CPUState *cs)
>>>   {
>>>       CPULoongArchState *env = cpu_env(cs);
>>> +    uint64_t val;
>> How about set initial value here although it is not used? such as
>>         uint64_t val = 0;
> 
> Or        uint64_t unused = 0;
yes, that is better and easier to understand -:)

> 
>>>       env->mp_state = KVM_MP_STATE_RUNNABLE;
>>> -    kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, 0);
>>> +    kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
>> Can we add return value checking here? such as
>>         ret = kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
>>         if (ret) {
>>             error_report("... %s", strerror(errno));
>>         }
>>
>> Regards
>> Bibo Mao
>>>   }
>>>   static int kvm_loongarch_get_mpstate(CPUState *cs)
>>>
>>
>>
Re: [PATCH] target/loongarch: fix vcpu reset command word issue
Posted by lixianglai 1 year ago 
Ok! I will modify it according to the suggestion.

Thanks,
Xianglai.

> On 2025/2/5 下午8:06, Xianglai Li wrote:
>> When the KVM_REG_LOONGARCH_VCPU_RESET command word
>> is sent to the kernel through the kvm_set_one_reg interface,
>> the parameter source needs to be a legal address,
>> otherwise the kernel will return an error and the command word
>> will fail to be sent.
> Hi Xianglai,
>
> Good catch, it is actually one problem and thanks for reporting it.
>
>>
>> Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
>> ---
>> Cc: Bibo Mao <Maobibo@loongson.cn>
>> Cc: Song Gao <gaosong@loongson.cn>
>>
>>   target/loongarch/kvm/kvm.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
>> index a3f55155b0..01cddb7012 100644
>> --- a/target/loongarch/kvm/kvm.c
>> +++ b/target/loongarch/kvm/kvm.c
>> @@ -581,9 +581,10 @@ static int kvm_loongarch_get_lbt(CPUState *cs)
>>   void kvm_arch_reset_vcpu(CPUState *cs)
>>   {
>>       CPULoongArchState *env = cpu_env(cs);
>> +    uint64_t val;
> How about set initial value here although it is not used? such as
>        uint64_t val = 0;
>>         env->mp_state = KVM_MP_STATE_RUNNABLE;
>> -    kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, 0);
>> +    kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
> Can we add return value checking here? such as
>        ret = kvm_set_one_reg(cs, KVM_REG_LOONGARCH_VCPU_RESET, &val);
>        if (ret) {
>            error_report("... %s", strerror(errno));
>        }
>
> Regards
> Bibo Mao
>>   }
>>     static int kvm_loongarch_get_mpstate(CPUState *cs)
>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.