2022-09-08 23:51:40

by Isaku Yamahata

Subject: [PATCH v4 12/26] KVM: x86: Move TSC fixup logic to KVM arch resume callback

From: Isaku Yamahata <[email protected]>

commit 0dd6a6edb012 ("KVM: Dont mark TSC unstable due to S4 suspend") made
use of kvm_arch_hardware_enable() callback to detect that TSC goes backward
due to S4 suspend. It has to check it only when resuming from S4. Not
every time virtualization hardware ennoblement. Move the logic to
kvm_arch_resume() callback.

Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Isaku Yamahata <[email protected]>
---
arch/x86/kvm/x86.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0ab8db07fa0e..c733f65aaf3c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11835,18 +11835,27 @@ void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)
EXPORT_SYMBOL_GPL(kvm_vcpu_deliver_sipi_vector);

int kvm_arch_hardware_enable(void)
+{
+ return static_call(kvm_x86_hardware_enable)();
+}
+
+void kvm_arch_hardware_disable(void)
+{
+ static_call(kvm_x86_hardware_disable)();
+ drop_user_return_notifiers();
+}
+
+void kvm_arch_resume(int usage_count)
{
struct kvm *kvm;
struct kvm_vcpu *vcpu;
unsigned long i;
- int ret;
u64 local_tsc;
u64 max_tsc = 0;
bool stable, backwards_tsc = false;

- ret = static_call(kvm_x86_hardware_enable)();
- if (ret != 0)
- return ret;
+ if (!usage_count)
+ return;

local_tsc = rdtsc();
stable = !kvm_check_tsc_unstable();
@@ -11921,13 +11930,6 @@ int kvm_arch_hardware_enable(void)
}

}
- return 0;
-}
-
-void kvm_arch_hardware_disable(void)
-{
- static_call(kvm_x86_hardware_disable)();
- drop_user_return_notifiers();
}

static inline void kvm_ops_update(struct kvm_x86_init_ops *ops)
--
2.25.1

2022-09-09 06:47:19

by Chao Gao

Subject: Re: [PATCH v4 12/26] KVM: x86: Move TSC fixup logic to KVM arch resume callback

On Thu, Sep 08, 2022 at 04:25:28PM -0700, [email protected] wrote:
>From: Isaku Yamahata <[email protected]>
>
>commit 0dd6a6edb012 ("KVM: Dont mark TSC unstable due to S4 suspend") made
>use of kvm_arch_hardware_enable() callback to detect that TSC goes backward
>due to S4 suspend. It has to check it only when resuming from S4. Not
>every time virtualization hardware ennoblement. Move the logic to
>kvm_arch_resume() callback.

IIUC, kvm_arch_resume() is called on the first CPU waking up from suspension.
But the detection was done on every CPU. Is it a problem (i.e., we fail to
detect TSC goes backward on some CPUs)?

2022-09-11 03:23:22

by Isaku Yamahata

Subject: Re: [PATCH v4 12/26] KVM: x86: Move TSC fixup logic to KVM arch resume callback

On Fri, Sep 09, 2022 at 01:48:32PM +0800,
Chao Gao <[email protected]> wrote:

> On Thu, Sep 08, 2022 at 04:25:28PM -0700, [email protected] wrote:
> >From: Isaku Yamahata <[email protected]>
> >
> >commit 0dd6a6edb012 ("KVM: Dont mark TSC unstable due to S4 suspend") made
> >use of kvm_arch_hardware_enable() callback to detect that TSC goes backward
> >due to S4 suspend. It has to check it only when resuming from S4. Not
> >every time virtualization hardware ennoblement. Move the logic to
> >kvm_arch_resume() callback.
>
> IIUC, kvm_arch_resume() is called on the first CPU waking up from suspension.
> But the detection was done on every CPU. Is it a problem (i.e., we fail to
> detect TSC goes backward on some CPUs)?

The problem is, TSC fixup logic is only needed once on resuming.
The current code calls TSC fixup logic on each cpu onlining. It's quick
plumbing the logic to each cpu online. Although it won't harm to call the logic,
it's ugly and this time is good occasion to clean it up.

I will clarify the commit message.
--
Isaku Yamahata <[email protected]>