[PATCH 0/4] target/loongarch/kvm: cpucfg and device attr fixes

[PATCH 0/4] target/loongarch/kvm: cpucfg and device attr fixes

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

A few small robustness fixes around cpucfg register sync and kvm device
attribute handling in the LoongArch KVM target.

  1/4: kvm_check_cpucfg2() discarded the return value of
       KVM_GET_DEVICE_ATTR and used an uninitialized val to mask
       cpucfg[2]; check the return value and initialize val.

  2/4: kvm_get_stealtime(), kvm_set_stealtime() and kvm_set_pv_features()
       pass a struct kvm_device_attr by value to the variadic
       kvm_vcpu_ioctl(), which expects a pointer. Pass &attr.

  3/4: drop the redundant trace_kvm_failed_get/put_cpucfg() calls and
       their now-unused trace events; kvm_get/set_one_reg() already trace
       on failure.

  4/4: kvm_loongarch_get/put_cpucfg() overwrite ret on each iteration, so
       only the last register's result is returned and earlier failures
       are lost; on a failed read, get_cpucfg() also stores a stale val.
       Accumulate errors with ret |= and only store on a successful read.

Compiled and boot-tested on a loongarch64 KVM host.

Tao Cui (4):
  target/loongarch/kvm: fix uninitialized val and unchecked GET in
    cpucfg2 check
  target/loongarch/kvm: pass device attr by reference to kvm_vcpu_ioctl
  target/loongarch/kvm: remove redundant cpucfg failure traces
  target/loongarch/kvm: fix cpucfg sync error handling

 target/loongarch/kvm/kvm.c    | 39 ++++++++++++++++++-----------------
 target/loongarch/trace-events |  2 --
 2 files changed, 20 insertions(+), 21 deletions(-)

-- 
2.43.0

[PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

kvm_check_cpucfg2() discards the return value of KVM_GET_DEVICE_ATTR and
then uses the local val (the host cpucfg2 mask) without checking whether
the read succeeded. val is also declared without an initializer.

If GET fails, env->cpucfg[2] &= val uses an uninitialized value and can
silently clear feature bits (FP / LLFTP / LSX / LASX), since bitwise-AND
can only turn bits off.

Check the GET return value, report the failure with error_report(), and
initialize val to 0.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 target/loongarch/kvm/kvm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
index d6539c12ac..b7176ce53a 100644
--- a/target/loongarch/kvm/kvm.c
+++ b/target/loongarch/kvm/kvm.c
@@ -725,7 +725,7 @@ static int kvm_loongarch_get_cpucfg(CPUState *cs)
 static int kvm_check_cpucfg2(CPUState *cs)
 {
     int ret;
-    uint64_t val;
+    uint64_t val = 0;
     struct kvm_device_attr attr = {
         .group = KVM_LOONGARCH_VCPU_CPUCFG,
         .attr = 2,
@@ -736,7 +736,11 @@ static int kvm_check_cpucfg2(CPUState *cs)
     ret = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, &attr);
 
     if (!ret) {
-        kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
+        ret = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
+        if (ret) {
+            error_report("CPUCFG2: KVM_GET_DEVICE_ATTR: %s", strerror(errno));
+            return ret;
+        }
         env->cpucfg[2] &= val;
 
         if (FIELD_EX32(env->cpucfg[2], CPUCFG2, FP)) {
-- 
2.43.0

Re: [PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check

[Bibo Mao]

On 2026/6/25 上午9:58, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> kvm_check_cpucfg2() discards the return value of KVM_GET_DEVICE_ATTR and
> then uses the local val (the host cpucfg2 mask) without checking whether
> the read succeeded. val is also declared without an initializer.
> 
> If GET fails, env->cpucfg[2] &= val uses an uninitialized value and can
> silently clear feature bits (FP / LLFTP / LSX / LASX), since bitwise-AND
> can only turn bits off.
> 
> Check the GET return value, report the failure with error_report(), and
> initialize val to 0.
> 
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> ---
>   target/loongarch/kvm/kvm.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
> index d6539c12ac..b7176ce53a 100644
> --- a/target/loongarch/kvm/kvm.c
> +++ b/target/loongarch/kvm/kvm.c
> @@ -725,7 +725,7 @@ static int kvm_loongarch_get_cpucfg(CPUState *cs)
>   static int kvm_check_cpucfg2(CPUState *cs)
>   {
>       int ret;
> -    uint64_t val;
> +    uint64_t val = 0;
>       struct kvm_device_attr attr = {
>           .group = KVM_LOONGARCH_VCPU_CPUCFG,
>           .attr = 2,
> @@ -736,7 +736,11 @@ static int kvm_check_cpucfg2(CPUState *cs)
>       ret = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, &attr);
>   
>       if (!ret) {
> -        kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
> +        ret = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
> +        if (ret) {
> +            error_report("CPUCFG2: KVM_GET_DEVICE_ATTR: %s", strerror(errno));
> +            return ret;
if it is successful with KVM_HAS_DEVICE_ATTR, however error with 
KVM_GET_DEVICE_ATTR. There should be mempy_from/to_user problem, maybe 
VM can continue to run without the following logic and operation. How 
about something like this?
-        kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
-        env->cpucfg[2] &= val;
+        ret = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
+        if (!ret)
+            env->cpucfg[2] &= val;

Regards
Bibo Mao


sentence
feature and operation env->cpucfg[2] &= val;
> +        }
>           env->cpucfg[2] &= val;
>   
>           if (FIELD_EX32(env->cpucfg[2], CPUCFG2, FP)) {
> 

Re: [PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check

[Tao Cui]

在 2026/6/25 10:48, Bibo Mao 写道:
> 
> 
> On 2026/6/25 上午9:58, Tao Cui wrote:
>> From: Tao Cui <cuitao@kylinos.cn>
>>
>> kvm_check_cpucfg2() discards the return value of KVM_GET_DEVICE_ATTR and
>> then uses the local val (the host cpucfg2 mask) without checking whether
>> the read succeeded. val is also declared without an initializer.
>>
>> If GET fails, env->cpucfg[2] &= val uses an uninitialized value and can
>> silently clear feature bits (FP / LLFTP / LSX / LASX), since bitwise-AND
>> can only turn bits off.
>>
>> Check the GET return value, report the failure with error_report(), and
>> initialize val to 0.
>>
>> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>> ---
>>   target/loongarch/kvm/kvm.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
>> index d6539c12ac..b7176ce53a 100644
>> --- a/target/loongarch/kvm/kvm.c
>> +++ b/target/loongarch/kvm/kvm.c
>> @@ -725,7 +725,7 @@ static int kvm_loongarch_get_cpucfg(CPUState *cs)
>>   static int kvm_check_cpucfg2(CPUState *cs)
>>   {
>>       int ret;
>> -    uint64_t val;
>> +    uint64_t val = 0;
>>       struct kvm_device_attr attr = {
>>           .group = KVM_LOONGARCH_VCPU_CPUCFG,
>>           .attr = 2,
>> @@ -736,7 +736,11 @@ static int kvm_check_cpucfg2(CPUState *cs)
>>       ret = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, &attr);
>>         if (!ret) {
>> -        kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
>> +        ret = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
>> +        if (ret) {
>> +            error_report("CPUCFG2: KVM_GET_DEVICE_ATTR: %s", strerror(errno));
>> +            return ret;
> if it is successful with KVM_HAS_DEVICE_ATTR, however error with KVM_GET_DEVICE_ATTR. There should be mempy_from/to_user problem, maybe VM can continue to run without the following logic and operation. How about something like this?
> -        kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
> -        env->cpucfg[2] &= val;
> +        ret = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
> +        if (!ret)
> +            env->cpucfg[2] &= val;
> 
Right, the &= mask is best-effort negotiation, so failing the whole register
sync over it is heavier than needed. I'll rework 1/4 so a GET failure only
skips the mask, using a local variable so the GET error no longer propagates:

    if (!ret) {
        int r = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
        if (r) {
            warn_report("CPUCFG2: KVM_GET_DEVICE_ATTR: %s", strerror(errno));
        } else {
            env->cpucfg[2] &= val;
        }
        ...
    }
    return ret;

The guest then keeps running with the cpucfg2 it already has. val is still
initialized to 0.

Thanks,
Tao
> 
> sentence
> feature and operation env->cpucfg[2] &= val;
>> +        }
>>           env->cpucfg[2] &= val;
>>             if (FIELD_EX32(env->cpucfg[2], CPUCFG2, FP)) {
>>
> 

[PATCH 2/4] target/loongarch/kvm: pass device attr by reference to kvm_vcpu_ioctl

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

kvm_vcpu_ioctl() is variadic and reads its argument as a pointer, but
kvm_get_stealtime(), kvm_set_stealtime() and kvm_set_pv_features() pass
the local struct kvm_device_attr by value. It currently works because of
how the calling convention passes large structs; pass &attr so the
argument is passed as intended.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 target/loongarch/kvm/kvm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
index b7176ce53a..fffb2d431a 100644
--- a/target/loongarch/kvm/kvm.c
+++ b/target/loongarch/kvm/kvm.c
@@ -46,12 +46,12 @@ static int kvm_get_stealtime(CPUState *cs)
         .addr = (uint64_t)&env->stealtime.guest_addr,
     };
 
-    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr);
+    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, &attr);
     if (err) {
         return 0;
     }
 
-    err = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, attr);
+    err = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
     if (err) {
         error_report("PVTIME: KVM_GET_DEVICE_ATTR: %s", strerror(errno));
         return err;
@@ -70,12 +70,12 @@ static int kvm_set_stealtime(CPUState *cs)
         .addr = (uint64_t)&env->stealtime.guest_addr,
     };
 
-    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr);
+    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, &attr);
     if (err) {
         return 0;
     }
 
-    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr);
+    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, &attr);
     if (err) {
         error_report("PVTIME: KVM_SET_DEVICE_ATTR %s with gpa "TARGET_FMT_lx,
                       strerror(errno), env->stealtime.guest_addr);
@@ -96,13 +96,13 @@ static int kvm_set_pv_features(CPUState *cs)
         .addr = (uint64_t)&val,
     };
 
-    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr);
+    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, &attr);
     if (err) {
         return 0;
     }
 
     val = env->pv_features;
-    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr);
+    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, &attr);
     if (err) {
         error_report("Fail to set pv feature "TARGET_FMT_lx " with error %s",
                       val, strerror(errno));
-- 
2.43.0

Re: [PATCH 2/4] target/loongarch/kvm: pass device attr by reference to kvm_vcpu_ioctl

[Bibo Mao]

On 2026/6/25 上午9:58, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> kvm_vcpu_ioctl() is variadic and reads its argument as a pointer, but
> kvm_get_stealtime(), kvm_set_stealtime() and kvm_set_pv_features() pass
> the local struct kvm_device_attr by value. It currently works because of
> how the calling convention passes large structs; pass &attr so the
> argument is passed as intended.
> 
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> ---
>   target/loongarch/kvm/kvm.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
> index b7176ce53a..fffb2d431a 100644
> --- a/target/loongarch/kvm/kvm.c
> +++ b/target/loongarch/kvm/kvm.c
> @@ -46,12 +46,12 @@ static int kvm_get_stealtime(CPUState *cs)
>           .addr = (uint64_t)&env->stealtime.guest_addr,
>       };
>   
> -    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr);
> +    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, &attr);
>       if (err) {
>           return 0;
>       }
>   
> -    err = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, attr);
> +    err = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
>       if (err) {
>           error_report("PVTIME: KVM_GET_DEVICE_ATTR: %s", strerror(errno));
>           return err;
> @@ -70,12 +70,12 @@ static int kvm_set_stealtime(CPUState *cs)
>           .addr = (uint64_t)&env->stealtime.guest_addr,
>       };
>   
> -    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr);
> +    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, &attr);
>       if (err) {
>           return 0;
>       }
>   
> -    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr);
> +    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, &attr);
>       if (err) {
>           error_report("PVTIME: KVM_SET_DEVICE_ATTR %s with gpa "TARGET_FMT_lx,
>                         strerror(errno), env->stealtime.guest_addr);
> @@ -96,13 +96,13 @@ static int kvm_set_pv_features(CPUState *cs)
>           .addr = (uint64_t)&val,
>       };
>   
> -    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr);
> +    err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, &attr);
>       if (err) {
>           return 0;
>       }
>   
>       val = env->pv_features;
> -    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr);
> +    err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, &attr);
>       if (err) {
>           error_report("Fail to set pv feature "TARGET_FMT_lx " with error %s",
>                         val, strerror(errno));
> 
Reviewed-by: Bibo Mao <maobibo@loongson.cn>

[PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

kvm_get_one_reg() and kvm_set_one_reg() already trace on failure, so the
trace_kvm_failed_get_cpucfg()/trace_kvm_failed_put_cpucfg() calls in
kvm_loongarch_get_cpucfg() and kvm_loongarch_put_cpucfg() duplicate that.
Remove the calls and the now-unused trace events.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 target/loongarch/kvm/kvm.c    | 6 ------
 target/loongarch/trace-events | 2 --
 2 files changed, 8 deletions(-)

diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
index fffb2d431a..6c3eea87b0 100644
--- a/target/loongarch/kvm/kvm.c
+++ b/target/loongarch/kvm/kvm.c
@@ -714,9 +714,6 @@ static int kvm_loongarch_get_cpucfg(CPUState *cs)
 
     for (i = 0; i < 21; i++) {
         ret = kvm_get_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
-        if (ret < 0) {
-            trace_kvm_failed_get_cpucfg(strerror(errno));
-        }
         env->cpucfg[i] = (uint32_t)val;
     }
     return ret;
@@ -772,9 +769,6 @@ static int kvm_loongarch_put_cpucfg(CPUState *cs)
 	}
         val = env->cpucfg[i];
         ret = kvm_set_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
-        if (ret < 0) {
-            trace_kvm_failed_put_cpucfg(strerror(errno));
-        }
     }
     return ret;
 }
diff --git a/target/loongarch/trace-events b/target/loongarch/trace-events
index dea11edc0f..829d8e0f53 100644
--- a/target/loongarch/trace-events
+++ b/target/loongarch/trace-events
@@ -9,7 +9,5 @@ kvm_failed_get_mpstate(const char *msg) "Failed to get mp_state from KVM: %s"
 kvm_failed_put_mpstate(const char *msg) "Failed to put mp_state into KVM: %s"
 kvm_failed_get_counter(const char *msg) "Failed to get counter from KVM: %s"
 kvm_failed_put_counter(const char *msg) "Failed to put counter into KVM: %s"
-kvm_failed_get_cpucfg(const char *msg) "Failed to get cpucfg from KVM: %s"
-kvm_failed_put_cpucfg(const char *msg) "Failed to put cpucfg into KVM: %s"
 kvm_arch_handle_exit(int num) "kvm arch handle exit, the reason number: %d"
 kvm_set_intr(int irq, int level) "kvm set interrupt, irq num: %d, level: %d"
-- 
2.43.0

Re: [PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces

[Bibo Mao]

On 2026/6/25 上午9:58, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> kvm_get_one_reg() and kvm_set_one_reg() already trace on failure, so the
> trace_kvm_failed_get_cpucfg()/trace_kvm_failed_put_cpucfg() calls in
> kvm_loongarch_get_cpucfg() and kvm_loongarch_put_cpucfg() duplicate that.
> Remove the calls and the now-unused trace events.
> 
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> ---
>   target/loongarch/kvm/kvm.c    | 6 ------
>   target/loongarch/trace-events | 2 --
>   2 files changed, 8 deletions(-)
> 
> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
> index fffb2d431a..6c3eea87b0 100644
> --- a/target/loongarch/kvm/kvm.c
> +++ b/target/loongarch/kvm/kvm.c
> @@ -714,9 +714,6 @@ static int kvm_loongarch_get_cpucfg(CPUState *cs)
>   
>       for (i = 0; i < 21; i++) {
>           ret = kvm_get_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
> -        if (ret < 0) {
> -            trace_kvm_failed_get_cpucfg(strerror(errno));
yeap, the trace is unnecessary, should we return immediately if there is 
error? otherwise the previous error will be overwritten.

Regards
Bibo Mao
> -        }
>           env->cpucfg[i] = (uint32_t)val;
>       }
>       return ret;
> @@ -772,9 +769,6 @@ static int kvm_loongarch_put_cpucfg(CPUState *cs)
>   	}
>           val = env->cpucfg[i];
>           ret = kvm_set_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
> -        if (ret < 0) {
> -            trace_kvm_failed_put_cpucfg(strerror(errno));
> -        }
>       }
>       return ret;
>   }
> diff --git a/target/loongarch/trace-events b/target/loongarch/trace-events
> index dea11edc0f..829d8e0f53 100644
> --- a/target/loongarch/trace-events
> +++ b/target/loongarch/trace-events
> @@ -9,7 +9,5 @@ kvm_failed_get_mpstate(const char *msg) "Failed to get mp_state from KVM: %s"
>   kvm_failed_put_mpstate(const char *msg) "Failed to put mp_state into KVM: %s"
>   kvm_failed_get_counter(const char *msg) "Failed to get counter from KVM: %s"
>   kvm_failed_put_counter(const char *msg) "Failed to put counter into KVM: %s"
> -kvm_failed_get_cpucfg(const char *msg) "Failed to get cpucfg from KVM: %s"
> -kvm_failed_put_cpucfg(const char *msg) "Failed to put cpucfg into KVM: %s"
>   kvm_arch_handle_exit(int num) "kvm arch handle exit, the reason number: %d"
>   kvm_set_intr(int irq, int level) "kvm set interrupt, irq num: %d, level: %d"
> 

Re: [PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces

[Tao Cui]

在 2026/6/25 10:38, Bibo Mao 写道:
> 
> 
> On 2026/6/25 上午9:58, Tao Cui wrote:
>> From: Tao Cui <cuitao@kylinos.cn>
>>
>> kvm_get_one_reg() and kvm_set_one_reg() already trace on failure, so the
>> trace_kvm_failed_get_cpucfg()/trace_kvm_failed_put_cpucfg() calls in
>> kvm_loongarch_get_cpucfg() and kvm_loongarch_put_cpucfg() duplicate that.
>> Remove the calls and the now-unused trace events.
>>
>> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>> ---
>>   target/loongarch/kvm/kvm.c    | 6 ------
>>   target/loongarch/trace-events | 2 --
>>   2 files changed, 8 deletions(-)
>>
>> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
>> index fffb2d431a..6c3eea87b0 100644
>> --- a/target/loongarch/kvm/kvm.c
>> +++ b/target/loongarch/kvm/kvm.c
>> @@ -714,9 +714,6 @@ static int kvm_loongarch_get_cpucfg(CPUState *cs)
>>         for (i = 0; i < 21; i++) {
>>           ret = kvm_get_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
>> -        if (ret < 0) {
>> -            trace_kvm_failed_get_cpucfg(strerror(errno));
> yeap, the trace is unnecessary, should we return immediately if there is error? otherwise the previous error will be overwritten.
> 

You're right that the per-iteration ret assignmentoverwrites earlier errors. That's addressed in patch 4/4 ("fix cpucfg sync
error handling") of this series:

    int r = kvm_get_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
    ret |= r;
    if (!r) {
        env->cpucfg[i] = (uint32_t)val;
    }

I went with ret |= (accumulate) rather than returning immediately, to match
the existing kvm_loongarch_get_csr()/put_csr() loops in the same file which
also accumulate; it additionally skips storing cpucfg[i] on a failed read.

Happy to switch to an early return if you prefer that style.

Thanks,
Tao

> 
>> -        }
>>           env->cpucfg[i] = (uint32_t)val;
>>       }
>>       return ret;
>> @@ -772,9 +769,6 @@ static int kvm_loongarch_put_cpucfg(CPUState *cs)
>>       }
>>           val = env->cpucfg[i];
>>           ret = kvm_set_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
>> -        if (ret < 0) {
>> -            trace_kvm_failed_put_cpucfg(strerror(errno));
>> -        }
>>       }
>>       return ret;
>>   }
>> diff --git a/target/loongarch/trace-events b/target/loongarch/trace-events
>> index dea11edc0f..829d8e0f53 100644
>> --- a/target/loongarch/trace-events
>> +++ b/target/loongarch/trace-events
>> @@ -9,7 +9,5 @@ kvm_failed_get_mpstate(const char *msg) "Failed to get mp_state from KVM: %s"
>>   kvm_failed_put_mpstate(const char *msg) "Failed to put mp_state into KVM: %s"
>>   kvm_failed_get_counter(const char *msg) "Failed to get counter from KVM: %s"
>>   kvm_failed_put_counter(const char *msg) "Failed to put counter into KVM: %s"
>> -kvm_failed_get_cpucfg(const char *msg) "Failed to get cpucfg from KVM: %s"
>> -kvm_failed_put_cpucfg(const char *msg) "Failed to put cpucfg into KVM: %s"
>>   kvm_arch_handle_exit(int num) "kvm arch handle exit, the reason number: %d"
>>   kvm_set_intr(int irq, int level) "kvm set interrupt, irq num: %d, level: %d"
>>
> 

Re: [PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces

[Bibo Mao]

On 2026/6/25 上午11:33, Tao Cui wrote:
> 
> 
> 在 2026/6/25 10:38, Bibo Mao 写道:
>>
>>
>> On 2026/6/25 上午9:58, Tao Cui wrote:
>>> From: Tao Cui <cuitao@kylinos.cn>
>>>
>>> kvm_get_one_reg() and kvm_set_one_reg() already trace on failure, so the
>>> trace_kvm_failed_get_cpucfg()/trace_kvm_failed_put_cpucfg() calls in
>>> kvm_loongarch_get_cpucfg() and kvm_loongarch_put_cpucfg() duplicate that.
>>> Remove the calls and the now-unused trace events.
>>>
>>> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>>> ---
>>>    target/loongarch/kvm/kvm.c    | 6 ------
>>>    target/loongarch/trace-events | 2 --
>>>    2 files changed, 8 deletions(-)
>>>
>>> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
>>> index fffb2d431a..6c3eea87b0 100644
>>> --- a/target/loongarch/kvm/kvm.c
>>> +++ b/target/loongarch/kvm/kvm.c
>>> @@ -714,9 +714,6 @@ static int kvm_loongarch_get_cpucfg(CPUState *cs)
>>>          for (i = 0; i < 21; i++) {
>>>            ret = kvm_get_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
>>> -        if (ret < 0) {
>>> -            trace_kvm_failed_get_cpucfg(strerror(errno));
>> yeap, the trace is unnecessary, should we return immediately if there is error? otherwise the previous error will be overwritten.
>>
> 
> You're right that the per-iteration ret assignmentoverwrites earlier errors. That's addressed in patch 4/4 ("fix cpucfg sync
> error handling") of this series:
> 
>      int r = kvm_get_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
>      ret |= r;
>      if (!r) {
>          env->cpucfg[i] = (uint32_t)val;
>      }
> 
> I went with ret |= (accumulate) rather than returning immediately, to match
> the existing kvm_loongarch_get_csr()/put_csr() loops in the same file which
> also accumulate; it additionally skips storing cpucfg[i] on a failed read.
> 
> Happy to switch to an early return if you prefer that style.
Both are ok for me, no special points here :)

Regards
Bibo Mao
> 
> Thanks,
> Tao
> 
>>
>>> -        }
>>>            env->cpucfg[i] = (uint32_t)val;
>>>        }
>>>        return ret;
>>> @@ -772,9 +769,6 @@ static int kvm_loongarch_put_cpucfg(CPUState *cs)
>>>        }
>>>            val = env->cpucfg[i];
>>>            ret = kvm_set_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
>>> -        if (ret < 0) {
>>> -            trace_kvm_failed_put_cpucfg(strerror(errno));
>>> -        }
>>>        }
>>>        return ret;
>>>    }
>>> diff --git a/target/loongarch/trace-events b/target/loongarch/trace-events
>>> index dea11edc0f..829d8e0f53 100644
>>> --- a/target/loongarch/trace-events
>>> +++ b/target/loongarch/trace-events
>>> @@ -9,7 +9,5 @@ kvm_failed_get_mpstate(const char *msg) "Failed to get mp_state from KVM: %s"
>>>    kvm_failed_put_mpstate(const char *msg) "Failed to put mp_state into KVM: %s"
>>>    kvm_failed_get_counter(const char *msg) "Failed to get counter from KVM: %s"
>>>    kvm_failed_put_counter(const char *msg) "Failed to put counter into KVM: %s"
>>> -kvm_failed_get_cpucfg(const char *msg) "Failed to get cpucfg from KVM: %s"
>>> -kvm_failed_put_cpucfg(const char *msg) "Failed to put cpucfg into KVM: %s"
>>>    kvm_arch_handle_exit(int num) "kvm arch handle exit, the reason number: %d"
>>>    kvm_set_intr(int irq, int level) "kvm set interrupt, irq num: %d, level: %d"
>>>
>>
> 

[PATCH 4/4] target/loongarch/kvm: fix cpucfg sync error handling

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

In kvm_loongarch_get_cpucfg() and kvm_loongarch_put_cpucfg(), ret is
overwritten on each iteration, so only the last register's result is
returned and earlier failures are lost. On a failed read, env->cpucfg[i]
is stored from a stale or uninitialized val.

Accumulate errors with ret |=, matching kvm_loongarch_get_csr()/put_csr(),
and only update env->cpucfg[i] on a successful read. Keep the cpucfg2
negotiation check in put_cpucfg() on a separate variable so its early
return does not overwrite the accumulated result.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 target/loongarch/kvm/kvm.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
index 6c3eea87b0..a60c99227f 100644
--- a/target/loongarch/kvm/kvm.c
+++ b/target/loongarch/kvm/kvm.c
@@ -713,8 +713,11 @@ static int kvm_loongarch_get_cpucfg(CPUState *cs)
     CPULoongArchState *env = cpu_env(cs);
 
     for (i = 0; i < 21; i++) {
-        ret = kvm_get_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
-        env->cpucfg[i] = (uint32_t)val;
+        int r = kvm_get_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
+        ret |= r;
+        if (!r) {
+            env->cpucfg[i] = (uint32_t)val;
+        }
     }
     return ret;
 }
@@ -762,13 +765,13 @@ static int kvm_loongarch_put_cpucfg(CPUState *cs)
 
     for (i = 0; i < 21; i++) {
 	if (i == 2) {
-            ret = kvm_check_cpucfg2(cs);
-            if (ret) {
-                return ret;
+            int r = kvm_check_cpucfg2(cs);
+            if (r) {
+                return r;
             }
 	}
         val = env->cpucfg[i];
-        ret = kvm_set_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
+        ret |= kvm_set_one_reg(cs, KVM_IOC_CPUCFG(i), &val);
     }
     return ret;
 }
-- 
2.43.0