[PATCH 0/2] KVM: x86: abstract locking around pvclock_update_vm_gtod

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/2] KVM: x86: abstract locking around pvclock_update_vm_gtod_copy
@ 2021-08-11 10:23 Paolo Bonzini
  2021-08-11 10:23 ` [PATCH 1/2] KVM: KVM-on-hyperv: shorten no-entry section on reenlightenment Paolo Bonzini
  2021-08-11 10:23 ` [PATCH 2/2] kvm: x86: abstract locking around pvclock_update_vm_gtod_copy Paolo Bonzini
  0 siblings, 2 replies; 5+ messages in thread
From: Paolo Bonzini @ 2021-08-11 10:23 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: vkuznets, mtosatti

Two cleanup patches that factor out the handling of
KVM_REQ_MASTERCLOCK_UPDATE and KVM_REQ_MCLOCK_INPROGRESS.  I have another
patch actually to remove KVM_REQ_MCLOCK_INPROGRESS, but I don't have time
to finish testing it right now; so here are only the cleanups leading
to it.

Paolo Bonzini (2):
  KVM: KVM-on-hyperv: shorten no-entry section on reenlightenment
  kvm: x86: abstract locking around pvclock_update_vm_gtod_copy

 arch/x86/include/asm/kvm_host.h |  1 -
 arch/x86/kvm/x86.c              | 67 +++++++++++++--------------------
 2 files changed, 27 insertions(+), 41 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] KVM: KVM-on-hyperv: shorten no-entry section on reenlightenment
  2021-08-11 10:23 [PATCH 0/2] KVM: x86: abstract locking around pvclock_update_vm_gtod_copy Paolo Bonzini
@ 2021-08-11 10:23 ` Paolo Bonzini
  2021-08-11 18:03   ` Marcelo Tosatti
  2021-08-11 10:23 ` [PATCH 2/2] kvm: x86: abstract locking around pvclock_update_vm_gtod_copy Paolo Bonzini
  1 sibling, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2021-08-11 10:23 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: vkuznets, mtosatti

During re-enlightenment, update kvmclock a VM at a time instead of
raising KVM_REQ_MASTERCLOCK_UPDATE for all VMs.  Because the guests
can now run after TSC emulation has been disabled, invalidate
their TSC page so that they refer to the reference time counter
MSR while the update is in progress.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/x86.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index bab8eb3e0a47..284afaa1db86 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8111,7 +8111,7 @@ static void kvm_hyperv_tsc_notifier(void)
 
 	mutex_lock(&kvm_lock);
 	list_for_each_entry(kvm, &vm_list, vm_list)
-		kvm_make_mclock_inprogress_request(kvm);
+		kvm_hv_invalidate_tsc_page(kvm);
 
 	hyperv_stop_tsc_emulation();
 
@@ -8123,6 +8123,7 @@ static void kvm_hyperv_tsc_notifier(void)
 	list_for_each_entry(kvm, &vm_list, vm_list) {
 		struct kvm_arch *ka = &kvm->arch;
 
+		kvm_make_mclock_inprogress_request(kvm);
 		spin_lock_irqsave(&ka->pvclock_gtod_sync_lock, flags);
 		pvclock_update_vm_gtod_copy(kvm);
 		spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] KVM: KVM-on-hyperv: shorten no-entry section on reenlightenment
  2021-08-11 10:23 ` [PATCH 1/2] KVM: KVM-on-hyperv: shorten no-entry section on reenlightenment Paolo Bonzini
@ 2021-08-11 18:03   ` Marcelo Tosatti
  2021-08-12 15:53     ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Marcelo Tosatti @ 2021-08-11 18:03 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, vkuznets

On Wed, Aug 11, 2021 at 06:23:55AM -0400, Paolo Bonzini wrote:
> During re-enlightenment, update kvmclock a VM at a time instead of
> raising KVM_REQ_MASTERCLOCK_UPDATE for all VMs.  Because the guests
> can now run after TSC emulation has been disabled, invalidate
> their TSC page so that they refer to the reference time counter
> MSR while the update is in progress.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/x86.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index bab8eb3e0a47..284afaa1db86 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8111,7 +8111,7 @@ static void kvm_hyperv_tsc_notifier(void)
>  
>  	mutex_lock(&kvm_lock);
>  	list_for_each_entry(kvm, &vm_list, vm_list)
> -		kvm_make_mclock_inprogress_request(kvm);
> +		kvm_hv_invalidate_tsc_page(kvm);
>  
>  	hyperv_stop_tsc_emulation();

        hyperv_stop_tsc_emulation();

        /* TSC frequency always matches when on Hyper-V */
        for_each_present_cpu(cpu)
                per_cpu(cpu_tsc_khz, cpu) = tsc_khz;
        kvm_max_guest_tsc_khz = tsc_khz; 

Is this safe with a running guest? Why?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] KVM: KVM-on-hyperv: shorten no-entry section on reenlightenment
  2021-08-11 18:03   ` Marcelo Tosatti
@ 2021-08-12 15:53     ` Paolo Bonzini
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2021-08-12 15:53 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: linux-kernel, kvm, vkuznets

On 11/08/21 20:03, Marcelo Tosatti wrote:
>          hyperv_stop_tsc_emulation();
> 
>          /* TSC frequency always matches when on Hyper-V */
>          for_each_present_cpu(cpu)
>                  per_cpu(cpu_tsc_khz, cpu) = tsc_khz;
>          kvm_max_guest_tsc_khz = tsc_khz;

Yeah, it's more complicated than this.  The right sequence is:

- update the master clock

- update the TSC page parameters

- stop TSC emulation

There is no need to invalidate the TSC page.

Related to this, after kvm_hv_invalidate_tsc_page the sequence value in 
the Hyper-V TSC page will always be 1, which is wrong.  I'll take a look 
at that too.

Paolo


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] kvm: x86: abstract locking around pvclock_update_vm_gtod_copy
  2021-08-11 10:23 [PATCH 0/2] KVM: x86: abstract locking around pvclock_update_vm_gtod_copy Paolo Bonzini
  2021-08-11 10:23 ` [PATCH 1/2] KVM: KVM-on-hyperv: shorten no-entry section on reenlightenment Paolo Bonzini
@ 2021-08-11 10:23 ` Paolo Bonzini
  1 sibling, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2021-08-11 10:23 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: vkuznets, mtosatti

Updates to the kvmclock parameters needs to do a complicated dance of
KVM_REQ_MCLOCK_INPROGRESS and KVM_REQ_CLOCK_UPDATE in addition to taking
pvclock_gtod_sync_lock.  Place that in two functions that can be called
on all of master clock update, KVM_SET_CLOCK, and Hyper-V reenlightenment.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  1 -
 arch/x86/kvm/x86.c              | 66 +++++++++++++--------------------
 2 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index bc6157677753..c54f9da91f99 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1839,7 +1839,6 @@ u64 kvm_calc_nested_tsc_multiplier(u64 l1_multiplier, u64 l2_multiplier);
 unsigned long kvm_get_linear_rip(struct kvm_vcpu *vcpu);
 bool kvm_is_linear_rip(struct kvm_vcpu *vcpu, unsigned long linear_rip);
 
-void kvm_make_mclock_inprogress_request(struct kvm *kvm);
 void kvm_make_scan_ioapic_request(struct kvm *kvm);
 void kvm_make_scan_ioapic_request_mask(struct kvm *kvm,
 				       unsigned long *vcpu_bitmap);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 284afaa1db86..8f96cdfcf364 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2757,35 +2757,36 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
 #endif
 }
 
-void kvm_make_mclock_inprogress_request(struct kvm *kvm)
+static void kvm_start_pvclock_update(struct kvm *kvm)
 {
+	struct kvm_arch *ka = &kvm->arch;
 	kvm_make_all_cpus_request(kvm, KVM_REQ_MCLOCK_INPROGRESS);
+
+	/* no guest entries from this point */
+	spin_lock_irq(&ka->pvclock_gtod_sync_lock);
 }
 
-static void kvm_gen_update_masterclock(struct kvm *kvm)
+static void kvm_end_pvclock_update(struct kvm *kvm)
 {
-#ifdef CONFIG_X86_64
-	int i;
-	struct kvm_vcpu *vcpu;
 	struct kvm_arch *ka = &kvm->arch;
-	unsigned long flags;
-
-	kvm_hv_invalidate_tsc_page(kvm);
-
-	kvm_make_mclock_inprogress_request(kvm);
-
-	/* no guest entries from this point */
-	spin_lock_irqsave(&ka->pvclock_gtod_sync_lock, flags);
-	pvclock_update_vm_gtod_copy(kvm);
-	spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
+	struct kvm_vcpu *vcpu;
+	int i;
 
+	spin_unlock_irq(&ka->pvclock_gtod_sync_lock);
 	kvm_for_each_vcpu(i, vcpu, kvm)
 		kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
 
 	/* guest entries allowed */
 	kvm_for_each_vcpu(i, vcpu, kvm)
 		kvm_clear_request(KVM_REQ_MCLOCK_INPROGRESS, vcpu);
-#endif
+}
+
+static void kvm_update_masterclock(struct kvm *kvm)
+{
+	kvm_hv_invalidate_tsc_page(kvm);
+	kvm_start_pvclock_update(kvm);
+	pvclock_update_vm_gtod_copy(kvm);
+	kvm_end_pvclock_update(kvm);
 }
 
 u64 get_kvmclock_ns(struct kvm *kvm)
@@ -6077,12 +6078,10 @@ long kvm_arch_vm_ioctl(struct file *filp,
 			goto out;
 
 		r = 0;
-		/*
-		 * TODO: userspace has to take care of races with VCPU_RUN, so
-		 * kvm_gen_update_masterclock() can be cut down to locked
-		 * pvclock_update_vm_gtod_copy().
-		 */
-		kvm_gen_update_masterclock(kvm);
+
+		kvm_hv_invalidate_tsc_page(kvm);
+		kvm_start_pvclock_update(kvm);
+		pvclock_update_vm_gtod_copy(kvm);
 
 		/*
 		 * This pairs with kvm_guest_time_update(): when masterclock is
@@ -6091,15 +6090,12 @@ long kvm_arch_vm_ioctl(struct file *filp,
 		 * is slightly ahead) here we risk going negative on unsigned
 		 * 'system_time' when 'user_ns.clock' is very small.
 		 */
-		spin_lock_irq(&ka->pvclock_gtod_sync_lock);
 		if (kvm->arch.use_master_clock)
 			now_ns = ka->master_kernel_ns;
 		else
 			now_ns = get_kvmclock_base_ns();
 		ka->kvmclock_offset = user_ns.clock - now_ns;
-		spin_unlock_irq(&ka->pvclock_gtod_sync_lock);
-
-		kvm_make_all_cpus_request(kvm, KVM_REQ_CLOCK_UPDATE);
+		kvm_end_pvclock_update(kvm);
 		break;
 	}
 	case KVM_GET_CLOCK: {
@@ -8105,9 +8101,7 @@ static void tsc_khz_changed(void *data)
 static void kvm_hyperv_tsc_notifier(void)
 {
 	struct kvm *kvm;
-	struct kvm_vcpu *vcpu;
 	int cpu;
-	unsigned long flags;
 
 	mutex_lock(&kvm_lock);
 	list_for_each_entry(kvm, &vm_list, vm_list)
@@ -8121,19 +8115,11 @@ static void kvm_hyperv_tsc_notifier(void)
 	kvm_max_guest_tsc_khz = tsc_khz;
 
 	list_for_each_entry(kvm, &vm_list, vm_list) {
-		struct kvm_arch *ka = &kvm->arch;
-
-		kvm_make_mclock_inprogress_request(kvm);
-		spin_lock_irqsave(&ka->pvclock_gtod_sync_lock, flags);
+		kvm_start_pvclock_update(kvm);
 		pvclock_update_vm_gtod_copy(kvm);
-		spin_unlock_irqrestore(&ka->pvclock_gtod_sync_lock, flags);
-
-		kvm_for_each_vcpu(cpu, vcpu, kvm)
-			kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
-
-		kvm_for_each_vcpu(cpu, vcpu, kvm)
-			kvm_clear_request(KVM_REQ_MCLOCK_INPROGRESS, vcpu);
+		kvm_end_pvclock_update(kvm);
 	}
+
 	mutex_unlock(&kvm_lock);
 }
 #endif
@@ -9413,7 +9399,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 		if (kvm_check_request(KVM_REQ_MIGRATE_TIMER, vcpu))
 			__kvm_migrate_timers(vcpu);
 		if (kvm_check_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu))
-			kvm_gen_update_masterclock(vcpu->kvm);
+			kvm_update_masterclock(vcpu->kvm);
 		if (kvm_check_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu))
 			kvm_gen_kvmclock_update(vcpu);
 		if (kvm_check_request(KVM_REQ_CLOCK_UPDATE, vcpu)) {
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-08-12 15:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-11 10:23 [PATCH 0/2] KVM: x86: abstract locking around pvclock_update_vm_gtod_copy Paolo Bonzini
2021-08-11 10:23 ` [PATCH 1/2] KVM: KVM-on-hyperv: shorten no-entry section on reenlightenment Paolo Bonzini
2021-08-11 18:03   ` Marcelo Tosatti
2021-08-12 15:53     ` Paolo Bonzini
2021-08-11 10:23 ` [PATCH 2/2] kvm: x86: abstract locking around pvclock_update_vm_gtod_copy Paolo Bonzini

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.