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

[Tao Cui <cui.tao@linux.dev>]

在 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"
>>
>