Re: [PATCH v6 29/51] x86/kvm: Don't disable kvmclock on BSP in syscore_suspend()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the KVM migration control MSR (MSR_KVM_MIGRATION_CONTROL) is disabled on suspend or CPU offline and never re-enabled on resume or CPU online, permanently breaking SEV live migration.
--

--- Patch [29]: [PATCH v6 29/51] x86/kvm: Don't disable kvmclock on BSP in syscore_suspend() ---
commit bb5c379a7a7675001e9cbd3ac5ed082d68cc68ae
Author: Sean Christopherson <[email protected]>

x86/kvm: Don't disable kvmclock on BSP in syscore_suspend()

This commit introduces a kvm_guest_cpu_action enum to avoid disabling
kvmclock on the BSP during syscore_suspend. The BSP's clock is restored
earlier via sched_clock, and disabling it can cause time drift over time.

> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c

[ ... ]

> @@ -460,7 +460,7 @@ static void __init sev_map_percpu_data(void)
>  	}
>  }
>  
> -static void kvm_guest_cpu_offline(bool shutdown)
> +static void kvm_guest_cpu_offline(enum kvm_guest_cpu_action action)
>  {
>  	kvm_disable_steal_time();
>  	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
> @@ -468,9 +468,10 @@ static void kvm_guest_cpu_offline(bool shutdown)
>  	if (kvm_para_has_feature(KVM_FEATURE_MIGRATION_CONTROL))
>  		wrmsrq(MSR_KVM_MIGRATION_CONTROL, 0);

[Severity: High]
This is a pre-existing issue, but does this permanently break SEV live
migration after a suspend/resume cycle?

When a CPU goes offline or the system suspends, kvm_guest_cpu_offline()
clears MSR_KVM_MIGRATION_CONTROL. However, it appears this MSR is never
re-enabled when the CPU comes back online.

For example, when the system resumes, kvm_guest_cpu_init() is called
to restore other PV state like PV EOI and async PF, but it does not
appear to restore MSR_KVM_MIGRATION_CONTROL.

Could this lack of restoration cause a SEV guest to permanently lose the
ability to be live-migrated after an AP offline/online event or a
suspend/resume?

>  	kvm_pv_disable_apf();
> -	if (!shutdown)
> +	if (action != KVM_GUEST_SHUTDOWN)
>  		apf_task_wake_all();
> -	kvmclock_disable();
> +
> +	kvmclock_cpu_action(action);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=29