* [PATCH 0/4] target/loongarch/kvm: cpucfg and device attr fixes
@ 2026-06-25 1:58 Tao Cui
2026-06-25 1:58 ` [PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check Tao Cui
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Tao Cui @ 2026-06-25 1:58 UTC (permalink / raw)
To: qemu-devel
Cc: Song Gao, Bibo Mao, Paolo Bonzini, Philippe Mathieu-Daudé,
Qiang Ma, 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check
2026-06-25 1:58 [PATCH 0/4] target/loongarch/kvm: cpucfg and device attr fixes Tao Cui
@ 2026-06-25 1:58 ` Tao Cui
2026-06-25 2:48 ` Bibo Mao
2026-06-25 1:58 ` [PATCH 2/4] target/loongarch/kvm: pass device attr by reference to kvm_vcpu_ioctl Tao Cui
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Tao Cui @ 2026-06-25 1:58 UTC (permalink / raw)
To: qemu-devel
Cc: Song Gao, Bibo Mao, Paolo Bonzini, Philippe Mathieu-Daudé,
Qiang Ma, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] target/loongarch/kvm: pass device attr by reference to kvm_vcpu_ioctl
2026-06-25 1:58 [PATCH 0/4] target/loongarch/kvm: cpucfg and device attr fixes Tao Cui
2026-06-25 1:58 ` [PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check Tao Cui
@ 2026-06-25 1:58 ` Tao Cui
2026-06-25 2:32 ` Bibo Mao
2026-06-25 1:58 ` [PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces Tao Cui
2026-06-25 1:58 ` [PATCH 4/4] target/loongarch/kvm: fix cpucfg sync error handling Tao Cui
3 siblings, 1 reply; 11+ messages in thread
From: Tao Cui @ 2026-06-25 1:58 UTC (permalink / raw)
To: qemu-devel
Cc: Song Gao, Bibo Mao, Paolo Bonzini, Philippe Mathieu-Daudé,
Qiang Ma, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces
2026-06-25 1:58 [PATCH 0/4] target/loongarch/kvm: cpucfg and device attr fixes Tao Cui
2026-06-25 1:58 ` [PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check Tao Cui
2026-06-25 1:58 ` [PATCH 2/4] target/loongarch/kvm: pass device attr by reference to kvm_vcpu_ioctl Tao Cui
@ 2026-06-25 1:58 ` Tao Cui
2026-06-25 2:38 ` Bibo Mao
2026-06-25 1:58 ` [PATCH 4/4] target/loongarch/kvm: fix cpucfg sync error handling Tao Cui
3 siblings, 1 reply; 11+ messages in thread
From: Tao Cui @ 2026-06-25 1:58 UTC (permalink / raw)
To: qemu-devel
Cc: Song Gao, Bibo Mao, Paolo Bonzini, Philippe Mathieu-Daudé,
Qiang Ma, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] target/loongarch/kvm: fix cpucfg sync error handling
2026-06-25 1:58 [PATCH 0/4] target/loongarch/kvm: cpucfg and device attr fixes Tao Cui
` (2 preceding siblings ...)
2026-06-25 1:58 ` [PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces Tao Cui
@ 2026-06-25 1:58 ` Tao Cui
3 siblings, 0 replies; 11+ messages in thread
From: Tao Cui @ 2026-06-25 1:58 UTC (permalink / raw)
To: qemu-devel
Cc: Song Gao, Bibo Mao, Paolo Bonzini, Philippe Mathieu-Daudé,
Qiang Ma, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] target/loongarch/kvm: pass device attr by reference to kvm_vcpu_ioctl
2026-06-25 1:58 ` [PATCH 2/4] target/loongarch/kvm: pass device attr by reference to kvm_vcpu_ioctl Tao Cui
@ 2026-06-25 2:32 ` Bibo Mao
0 siblings, 0 replies; 11+ messages in thread
From: Bibo Mao @ 2026-06-25 2:32 UTC (permalink / raw)
To: Tao Cui, qemu-devel
Cc: Song Gao, Paolo Bonzini, Philippe Mathieu-Daudé, Qiang Ma,
Tao Cui
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>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces
2026-06-25 1:58 ` [PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces Tao Cui
@ 2026-06-25 2:38 ` Bibo Mao
2026-06-25 3:33 ` Tao Cui
0 siblings, 1 reply; 11+ messages in thread
From: Bibo Mao @ 2026-06-25 2:38 UTC (permalink / raw)
To: Tao Cui, qemu-devel
Cc: Song Gao, Paolo Bonzini, Philippe Mathieu-Daudé, Qiang Ma,
Tao Cui
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"
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check
2026-06-25 1:58 ` [PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check Tao Cui
@ 2026-06-25 2:48 ` Bibo Mao
2026-06-25 3:24 ` Tao Cui
0 siblings, 1 reply; 11+ messages in thread
From: Bibo Mao @ 2026-06-25 2:48 UTC (permalink / raw)
To: Tao Cui, qemu-devel
Cc: Song Gao, Paolo Bonzini, Philippe Mathieu-Daudé, Qiang Ma,
Tao Cui
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)) {
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check
2026-06-25 2:48 ` Bibo Mao
@ 2026-06-25 3:24 ` Tao Cui
0 siblings, 0 replies; 11+ messages in thread
From: Tao Cui @ 2026-06-25 3:24 UTC (permalink / raw)
To: Bibo Mao, qemu-devel
Cc: cui.tao, Song Gao, Paolo Bonzini, Philippe Mathieu-Daudé,
Qiang Ma, 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)) {
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces
2026-06-25 2:38 ` Bibo Mao
@ 2026-06-25 3:33 ` Tao Cui
2026-06-25 3:58 ` Bibo Mao
0 siblings, 1 reply; 11+ messages in thread
From: Tao Cui @ 2026-06-25 3:33 UTC (permalink / raw)
To: Bibo Mao, qemu-devel
Cc: cui.tao, Song Gao, Paolo Bonzini, Philippe Mathieu-Daudé,
Qiang Ma, 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"
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces
2026-06-25 3:33 ` Tao Cui
@ 2026-06-25 3:58 ` Bibo Mao
0 siblings, 0 replies; 11+ messages in thread
From: Bibo Mao @ 2026-06-25 3:58 UTC (permalink / raw)
To: Tao Cui, qemu-devel
Cc: Song Gao, Paolo Bonzini, Philippe Mathieu-Daudé, Qiang Ma,
Tao Cui
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"
>>>
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-06-25 4:03 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-25 1:58 [PATCH 0/4] target/loongarch/kvm: cpucfg and device attr fixes Tao Cui
2026-06-25 1:58 ` [PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check Tao Cui
2026-06-25 2:48 ` Bibo Mao
2026-06-25 3:24 ` Tao Cui
2026-06-25 1:58 ` [PATCH 2/4] target/loongarch/kvm: pass device attr by reference to kvm_vcpu_ioctl Tao Cui
2026-06-25 2:32 ` Bibo Mao
2026-06-25 1:58 ` [PATCH 3/4] target/loongarch/kvm: remove redundant cpucfg failure traces Tao Cui
2026-06-25 2:38 ` Bibo Mao
2026-06-25 3:33 ` Tao Cui
2026-06-25 3:58 ` Bibo Mao
2026-06-25 1:58 ` [PATCH 4/4] target/loongarch/kvm: fix cpucfg sync error handling Tao Cui
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.