i386: miscellaneous cleanup

[PATCH v5 04/11] target/i386/kvm: Only save/load kvmclock MSRs when kvmclock enabled

Posted by Zhao Liu 1 year, 3 months ago

MSR_KVM_SYSTEM_TIME and MSR_KVM_WALL_CLOCK are attached with the (old)
kvmclock feature (KVM_FEATURE_CLOCKSOURCE).

So, just save/load them only when kvmclock (KVM_FEATURE_CLOCKSOURCE) is
enabled.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Reviewed-by: Zide Chen <zide.chen@intel.com>
---
 target/i386/kvm/kvm.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 54520a77d6af..4aba034865bb 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -3958,8 +3958,10 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
      */
     if (level >= KVM_PUT_RESET_STATE) {
         kvm_msr_entry_add(cpu, MSR_IA32_TSC, env->tsc);
-        kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
-        kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
+        if (env->features[FEAT_KVM] & CPUID_KVM_CLOCK) {
+            kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
+            kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
+        }
         if (env->features[FEAT_KVM] & CPUID_KVM_ASYNCPF_INT) {
             kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, env->async_pf_int_msr);
         }
@@ -4436,8 +4438,10 @@ static int kvm_get_msrs(X86CPU *cpu)
         }
     }
 #endif
-    kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, 0);
-    kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, 0);
+    if (env->features[FEAT_KVM] & CPUID_KVM_CLOCK) {
+        kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, 0);
+        kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, 0);
+    }
     if (env->features[FEAT_KVM] & CPUID_KVM_ASYNCPF_INT) {
         kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_INT, 0);
     }
-- 
2.34.1

Re: [PATCH v5 04/11] target/i386/kvm: Only save/load kvmclock MSRs when kvmclock enabled

Posted by Paolo Bonzini 1 year, 1 month ago

On 11/6/24 04:07, Zhao Liu wrote:
> MSR_KVM_SYSTEM_TIME and MSR_KVM_WALL_CLOCK are attached with the (old)
> kvmclock feature (KVM_FEATURE_CLOCKSOURCE).
> 
> So, just save/load them only when kvmclock (KVM_FEATURE_CLOCKSOURCE) is
> enabled.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> 

The MSRs contains the same values as the "new" pair; QEMU only has to 
save/restore one of them but the code should be active for both feature 
bits and thus use

+        if (env->env.features[FEAT_KVM] & (CPUID_KVM_CLOCK |
+                                           CPUID_KVM_CLOCK2)) {

Paolo

Re: [PATCH v5 04/11] target/i386/kvm: Only save/load kvmclock MSRs when kvmclock enabled

Posted by Zhao Liu 1 year, 1 month ago

On Tue, Dec 24, 2024 at 04:31:28PM +0100, Paolo Bonzini wrote:
> Date: Tue, 24 Dec 2024 16:31:28 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH v5 04/11] target/i386/kvm: Only save/load kvmclock MSRs
>  when kvmclock enabled
> 
> On 11/6/24 04:07, Zhao Liu wrote:
> > MSR_KVM_SYSTEM_TIME and MSR_KVM_WALL_CLOCK are attached with the (old)
> > kvmclock feature (KVM_FEATURE_CLOCKSOURCE).
> > 
> > So, just save/load them only when kvmclock (KVM_FEATURE_CLOCKSOURCE) is
> > enabled.
> > 
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > 
> 
> The MSRs contains the same values as the "new" pair; QEMU only has to
> save/restore one of them but the code should be active for both feature bits
> and thus use
> 
> +        if (env->env.features[FEAT_KVM] & (CPUID_KVM_CLOCK |
> +                                           CPUID_KVM_CLOCK2)) {
> 

This is the correct way, thanks.

Regards,
Zhao