Linux-MIPS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs()
@ 2020-02-07 22:35 Peter Xu
  2020-02-07 22:35 ` [PATCH RFC 1/4] KVM: Provide kvm_flush_remote_tlbs_common() Peter Xu
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Peter Xu @ 2020-02-07 22:35 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Sean Christopherson, Paolo Bonzini, linux-mips, peterx, Vitaly Kuznetsov

[This series is RFC because I don't have MIPS to compile and test]

kvm_flush_remote_tlbs() can be arch-specific, by either:

- Completely replace kvm_flush_remote_tlbs(), like ARM, who is the
  only user of CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL so far

- Doing something extra before kvm_flush_remote_tlbs(), like MIPS VZ
  support, however still wants to have the common tlb flush to be part
  of the process.  Could refer to kvm_vz_flush_shadow_all().  Then in
  MIPS it's awkward to flush remote TLBs: we'll need to call the mips
  hooks.

It's awkward to have different ways to specialize this procedure,
especially MIPS cannot use the genenal interface which is quite a
pity.  It's good to make it a common interface.

This patch series removes the 2nd MIPS usage above, and let it also
use the common kvm_flush_remote_tlbs() interface.  It should be
suggested that we always keep kvm_flush_remote_tlbs() be a common
entrance for tlb flushing on all archs.

This idea comes from the reading of Sean's patchset on dynamic memslot
allocation, where a new dirty log specific hook is added for flushing
TLBs only for the MIPS code [1].  With this patchset, logically the
new hook in that patch can be dropped so we can directly use
kvm_flush_remote_tlbs().

TODO: We can even extend another common interface for ranged TLB, but
let's see how we think about this series first.

Any comment is welcomed, thanks.

Peter Xu (4):
  KVM: Provide kvm_flush_remote_tlbs_common()
  KVM: MIPS: Drop flush_shadow_memslot() callback
  KVM: MIPS: Replace all the kvm_flush_remote_tlbs() references
  KVM: MIPS: Define arch-specific kvm_flush_remote_tlbs()

 arch/mips/include/asm/kvm_host.h |  7 -------
 arch/mips/kvm/Kconfig            |  1 +
 arch/mips/kvm/mips.c             | 22 ++++++++++------------
 arch/mips/kvm/trap_emul.c        | 15 +--------------
 arch/mips/kvm/vz.c               | 14 ++------------
 include/linux/kvm_host.h         |  1 +
 virt/kvm/kvm_main.c              | 10 ++++++++--
 7 files changed, 23 insertions(+), 47 deletions(-)

-- 
2.24.1


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

* [PATCH RFC 1/4] KVM: Provide kvm_flush_remote_tlbs_common()
  2020-02-07 22:35 [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs() Peter Xu
@ 2020-02-07 22:35 ` Peter Xu
  2020-02-12 13:33   ` Paolo Bonzini
  2020-02-07 22:35 ` [PATCH RFC 2/4] KVM: MIPS: Drop flush_shadow_memslot() callback Peter Xu
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Peter Xu @ 2020-02-07 22:35 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Sean Christopherson, Paolo Bonzini, linux-mips, peterx, Vitaly Kuznetsov

It's exactly kvm_flush_remote_tlbs() now but a internal wrapper of the
common code path.  With this, an arch can then optionally select
CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL=y and will be able to use the
common flushing code.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/linux/kvm_host.h |  1 +
 virt/kvm/kvm_main.c      | 10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 6d5331b0d937..915df64125f9 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -798,6 +798,7 @@ int kvm_vcpu_yield_to(struct kvm_vcpu *target);
 void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu, bool usermode_vcpu_not_eligible);
 
 void kvm_flush_remote_tlbs(struct kvm *kvm);
+void kvm_flush_remote_tlbs_common(struct kvm *kvm);
 void kvm_reload_remote_mmus(struct kvm *kvm);
 
 bool kvm_make_vcpus_request_mask(struct kvm *kvm, unsigned int req,
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index eb3709d55139..9c7b39b7bb21 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -302,8 +302,7 @@ bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req)
 	return called;
 }
 
-#ifndef CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL
-void kvm_flush_remote_tlbs(struct kvm *kvm)
+void kvm_flush_remote_tlbs_common(struct kvm *kvm)
 {
 	/*
 	 * Read tlbs_dirty before setting KVM_REQ_TLB_FLUSH in
@@ -327,6 +326,13 @@ void kvm_flush_remote_tlbs(struct kvm *kvm)
 		++kvm->stat.remote_tlb_flush;
 	cmpxchg(&kvm->tlbs_dirty, dirty_count, 0);
 }
+EXPORT_SYMBOL_GPL(kvm_flush_remote_tlbs_common);
+
+#ifndef CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL
+void kvm_flush_remote_tlbs(struct kvm *kvm)
+{
+	kvm_flush_remote_tlbs_common(kvm);
+}
 EXPORT_SYMBOL_GPL(kvm_flush_remote_tlbs);
 #endif
 
-- 
2.24.1


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

* [PATCH RFC 2/4] KVM: MIPS: Drop flush_shadow_memslot() callback
  2020-02-07 22:35 [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs() Peter Xu
  2020-02-07 22:35 ` [PATCH RFC 1/4] KVM: Provide kvm_flush_remote_tlbs_common() Peter Xu
@ 2020-02-07 22:35 ` Peter Xu
  2020-02-07 22:35 ` [PATCH RFC 3/4] KVM: MIPS: Replace all the kvm_flush_remote_tlbs() references Peter Xu
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Peter Xu @ 2020-02-07 22:35 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Sean Christopherson, Paolo Bonzini, linux-mips, peterx, Vitaly Kuznetsov

The MIPS flush_shadow_memslot() callback is always calling the
flush_shadow_all() implementation no matter for trap-emul or VZ.
Delete it and call flush_shadow_all() instead.

This patch prepares for a further replacement of letting MIPS to use
the common kvm_flush_remote_tlbs() call in all places.

No functional change expected.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 arch/mips/include/asm/kvm_host.h | 7 -------
 arch/mips/kvm/mips.c             | 8 ++++----
 arch/mips/kvm/trap_emul.c        | 7 -------
 arch/mips/kvm/vz.c               | 7 -------
 4 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 41204a49cf95..e95faffb23d8 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -786,13 +786,6 @@ struct kvm_mips_callbacks {
 	void (*vcpu_uninit)(struct kvm_vcpu *vcpu);
 	int (*vcpu_setup)(struct kvm_vcpu *vcpu);
 	void (*flush_shadow_all)(struct kvm *kvm);
-	/*
-	 * Must take care of flushing any cached GPA PTEs (e.g. guest entries in
-	 * VZ root TLB, or T&E GVA page tables and corresponding root TLB
-	 * mappings).
-	 */
-	void (*flush_shadow_memslot)(struct kvm *kvm,
-				     const struct kvm_memory_slot *slot);
 	gpa_t (*gva_to_gpa)(gva_t gva);
 	void (*queue_timer_int)(struct kvm_vcpu *vcpu);
 	void (*dequeue_timer_int)(struct kvm_vcpu *vcpu);
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 2606f3f02b54..1d5e7ffda746 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -216,7 +216,7 @@ void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
 	kvm_mips_flush_gpa_pt(kvm, slot->base_gfn,
 			      slot->base_gfn + slot->npages - 1);
 	/* Let implementation do the rest */
-	kvm_mips_callbacks->flush_shadow_memslot(kvm, slot);
+	kvm_mips_callbacks->flush_shadow_all(kvm);
 	spin_unlock(&kvm->mmu_lock);
 }
 
@@ -258,7 +258,7 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
 					new->base_gfn + new->npages - 1);
 		/* Let implementation do the rest */
 		if (needs_flush)
-			kvm_mips_callbacks->flush_shadow_memslot(kvm, new);
+			kvm_mips_callbacks->flush_shadow_all(kvm);
 		spin_unlock(&kvm->mmu_lock);
 	}
 }
@@ -1003,7 +1003,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log)
 		memslot = id_to_memslot(slots, log->slot);
 
 		/* Let implementation handle TLB/GVA invalidation */
-		kvm_mips_callbacks->flush_shadow_memslot(kvm, memslot);
+		kvm_mips_callbacks->flush_shadow_all(kvm);
 	}
 
 	mutex_unlock(&kvm->slots_lock);
@@ -1026,7 +1026,7 @@ int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm, struct kvm_clear_dirty_log *lo
 		memslot = id_to_memslot(slots, log->slot);
 
 		/* Let implementation handle TLB/GVA invalidation */
-		kvm_mips_callbacks->flush_shadow_memslot(kvm, memslot);
+		kvm_mips_callbacks->flush_shadow_all(kvm);
 	}
 
 	mutex_unlock(&kvm->slots_lock);
diff --git a/arch/mips/kvm/trap_emul.c b/arch/mips/kvm/trap_emul.c
index 5a11e83dffe6..2ecb430ea0f1 100644
--- a/arch/mips/kvm/trap_emul.c
+++ b/arch/mips/kvm/trap_emul.c
@@ -703,12 +703,6 @@ static void kvm_trap_emul_flush_shadow_all(struct kvm *kvm)
 	kvm_flush_remote_tlbs(kvm);
 }
 
-static void kvm_trap_emul_flush_shadow_memslot(struct kvm *kvm,
-					const struct kvm_memory_slot *slot)
-{
-	kvm_trap_emul_flush_shadow_all(kvm);
-}
-
 static u64 kvm_trap_emul_get_one_regs[] = {
 	KVM_REG_MIPS_CP0_INDEX,
 	KVM_REG_MIPS_CP0_ENTRYLO0,
@@ -1292,7 +1286,6 @@ static struct kvm_mips_callbacks kvm_trap_emul_callbacks = {
 	.vcpu_uninit = kvm_trap_emul_vcpu_uninit,
 	.vcpu_setup = kvm_trap_emul_vcpu_setup,
 	.flush_shadow_all = kvm_trap_emul_flush_shadow_all,
-	.flush_shadow_memslot = kvm_trap_emul_flush_shadow_memslot,
 	.gva_to_gpa = kvm_trap_emul_gva_to_gpa_cb,
 	.queue_timer_int = kvm_mips_queue_timer_int_cb,
 	.dequeue_timer_int = kvm_mips_dequeue_timer_int_cb,
diff --git a/arch/mips/kvm/vz.c b/arch/mips/kvm/vz.c
index dde20887a70d..814bd1564a79 100644
--- a/arch/mips/kvm/vz.c
+++ b/arch/mips/kvm/vz.c
@@ -3123,12 +3123,6 @@ static void kvm_vz_flush_shadow_all(struct kvm *kvm)
 	}
 }
 
-static void kvm_vz_flush_shadow_memslot(struct kvm *kvm,
-					const struct kvm_memory_slot *slot)
-{
-	kvm_vz_flush_shadow_all(kvm);
-}
-
 static void kvm_vz_vcpu_reenter(struct kvm_run *run, struct kvm_vcpu *vcpu)
 {
 	int cpu = smp_processor_id();
@@ -3185,7 +3179,6 @@ static struct kvm_mips_callbacks kvm_vz_callbacks = {
 	.vcpu_uninit = kvm_vz_vcpu_uninit,
 	.vcpu_setup = kvm_vz_vcpu_setup,
 	.flush_shadow_all = kvm_vz_flush_shadow_all,
-	.flush_shadow_memslot = kvm_vz_flush_shadow_memslot,
 	.gva_to_gpa = kvm_vz_gva_to_gpa_cb,
 	.queue_timer_int = kvm_vz_queue_timer_int_cb,
 	.dequeue_timer_int = kvm_vz_dequeue_timer_int_cb,
-- 
2.24.1


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

* [PATCH RFC 3/4] KVM: MIPS: Replace all the kvm_flush_remote_tlbs() references
  2020-02-07 22:35 [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs() Peter Xu
  2020-02-07 22:35 ` [PATCH RFC 1/4] KVM: Provide kvm_flush_remote_tlbs_common() Peter Xu
  2020-02-07 22:35 ` [PATCH RFC 2/4] KVM: MIPS: Drop flush_shadow_memslot() callback Peter Xu
@ 2020-02-07 22:35 ` Peter Xu
  2020-02-07 22:35 ` [PATCH RFC 4/4] KVM: MIPS: Define arch-specific kvm_flush_remote_tlbs() Peter Xu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Peter Xu @ 2020-02-07 22:35 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Sean Christopherson, Paolo Bonzini, linux-mips, peterx, Vitaly Kuznetsov

Replace kvm_flush_remote_tlbs() calls in MIPS code into
kvm_flush_remote_tlbs_common().  This is to prepare that MIPS will
define its own kvm_flush_remote_tlbs() soon.

The only three references are all in the flush_shadow_all() hooks.
One of them can be directly dropped because it's exactly the
kvm_flush_remote_tlbs_common().  Since at it, refactors the other one
a bit.

No functional change expected.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 arch/mips/kvm/trap_emul.c | 8 +-------
 arch/mips/kvm/vz.c        | 7 ++-----
 2 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/arch/mips/kvm/trap_emul.c b/arch/mips/kvm/trap_emul.c
index 2ecb430ea0f1..ced481c963be 100644
--- a/arch/mips/kvm/trap_emul.c
+++ b/arch/mips/kvm/trap_emul.c
@@ -697,12 +697,6 @@ static int kvm_trap_emul_vcpu_setup(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static void kvm_trap_emul_flush_shadow_all(struct kvm *kvm)
-{
-	/* Flush GVA page tables and invalidate GVA ASIDs on all VCPUs */
-	kvm_flush_remote_tlbs(kvm);
-}
-
 static u64 kvm_trap_emul_get_one_regs[] = {
 	KVM_REG_MIPS_CP0_INDEX,
 	KVM_REG_MIPS_CP0_ENTRYLO0,
@@ -1285,7 +1279,7 @@ static struct kvm_mips_callbacks kvm_trap_emul_callbacks = {
 	.vcpu_init = kvm_trap_emul_vcpu_init,
 	.vcpu_uninit = kvm_trap_emul_vcpu_uninit,
 	.vcpu_setup = kvm_trap_emul_vcpu_setup,
-	.flush_shadow_all = kvm_trap_emul_flush_shadow_all,
+	.flush_shadow_all = kvm_flush_remote_tlbs_common,
 	.gva_to_gpa = kvm_trap_emul_gva_to_gpa_cb,
 	.queue_timer_int = kvm_mips_queue_timer_int_cb,
 	.dequeue_timer_int = kvm_mips_dequeue_timer_int_cb,
diff --git a/arch/mips/kvm/vz.c b/arch/mips/kvm/vz.c
index 814bd1564a79..91fbf6710da4 100644
--- a/arch/mips/kvm/vz.c
+++ b/arch/mips/kvm/vz.c
@@ -3105,10 +3105,7 @@ static int kvm_vz_vcpu_setup(struct kvm_vcpu *vcpu)
 
 static void kvm_vz_flush_shadow_all(struct kvm *kvm)
 {
-	if (cpu_has_guestid) {
-		/* Flush GuestID for each VCPU individually */
-		kvm_flush_remote_tlbs(kvm);
-	} else {
+	if (!cpu_has_guestid) {
 		/*
 		 * For each CPU there is a single GPA ASID used by all VCPUs in
 		 * the VM, so it doesn't make sense for the VCPUs to handle
@@ -3119,8 +3116,8 @@ static void kvm_vz_flush_shadow_all(struct kvm *kvm)
 		 * kick any running VCPUs so they check asid_flush_mask.
 		 */
 		cpumask_setall(&kvm->arch.asid_flush_mask);
-		kvm_flush_remote_tlbs(kvm);
 	}
+	kvm_flush_remote_tlbs_common(kvm);
 }
 
 static void kvm_vz_vcpu_reenter(struct kvm_run *run, struct kvm_vcpu *vcpu)
-- 
2.24.1


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

* [PATCH RFC 4/4] KVM: MIPS: Define arch-specific kvm_flush_remote_tlbs()
  2020-02-07 22:35 [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs() Peter Xu
                   ` (2 preceding siblings ...)
  2020-02-07 22:35 ` [PATCH RFC 3/4] KVM: MIPS: Replace all the kvm_flush_remote_tlbs() references Peter Xu
@ 2020-02-07 22:35 ` Peter Xu
  2020-02-07 23:00 ` [PATCH RFC 0/4] KVM: MIPS: Provide " Peter Xu
  2020-02-12 12:25 ` Paolo Bonzini
  5 siblings, 0 replies; 11+ messages in thread
From: Peter Xu @ 2020-02-07 22:35 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Sean Christopherson, Paolo Bonzini, linux-mips, peterx, Vitaly Kuznetsov

Select HAVE_KVM_ARCH_TLB_FLUSH_ALL for MIPS to define its own
kvm_flush_remote_tlbs().  It's as simple as calling the
flush_shadow_all(kvm) hook.  Then replace all the flush_shadow_all()
calls to use this KVM generic API to do TLB flush.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 arch/mips/kvm/Kconfig |  1 +
 arch/mips/kvm/mips.c  | 22 ++++++++++------------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/mips/kvm/Kconfig b/arch/mips/kvm/Kconfig
index eac25aef21e0..8a06f660d39e 100644
--- a/arch/mips/kvm/Kconfig
+++ b/arch/mips/kvm/Kconfig
@@ -26,6 +26,7 @@ config KVM
 	select KVM_MMIO
 	select MMU_NOTIFIER
 	select SRCU
+	select HAVE_KVM_ARCH_TLB_FLUSH_ALL
 	---help---
 	  Support for hosting Guest kernels.
 
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 1d5e7ffda746..518020b466bf 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -194,13 +194,16 @@ int kvm_arch_create_memslot(struct kvm *kvm, struct kvm_memory_slot *slot,
 	return 0;
 }
 
+void kvm_flush_remote_tlbs(struct kvm *kvm)
+{
+	kvm_mips_callbacks->flush_shadow_all(kvm);
+}
+
 void kvm_arch_flush_shadow_all(struct kvm *kvm)
 {
 	/* Flush whole GPA */
 	kvm_mips_flush_gpa_pt(kvm, 0, ~0);
-
-	/* Let implementation do the rest */
-	kvm_mips_callbacks->flush_shadow_all(kvm);
+	kvm_flush_remote_tlbs(kvm);
 }
 
 void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
@@ -215,8 +218,7 @@ void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
 	/* Flush slot from GPA */
 	kvm_mips_flush_gpa_pt(kvm, slot->base_gfn,
 			      slot->base_gfn + slot->npages - 1);
-	/* Let implementation do the rest */
-	kvm_mips_callbacks->flush_shadow_all(kvm);
+	kvm_flush_remote_tlbs(kvm);
 	spin_unlock(&kvm->mmu_lock);
 }
 
@@ -258,7 +260,7 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
 					new->base_gfn + new->npages - 1);
 		/* Let implementation do the rest */
 		if (needs_flush)
-			kvm_mips_callbacks->flush_shadow_all(kvm);
+			kvm_flush_remote_tlbs(kvm);
 		spin_unlock(&kvm->mmu_lock);
 	}
 }
@@ -1001,9 +1003,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log)
 	if (flush) {
 		slots = kvm_memslots(kvm);
 		memslot = id_to_memslot(slots, log->slot);
-
-		/* Let implementation handle TLB/GVA invalidation */
-		kvm_mips_callbacks->flush_shadow_all(kvm);
+		kvm_flush_remote_tlbs(kvm);
 	}
 
 	mutex_unlock(&kvm->slots_lock);
@@ -1024,9 +1024,7 @@ int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm, struct kvm_clear_dirty_log *lo
 	if (flush) {
 		slots = kvm_memslots(kvm);
 		memslot = id_to_memslot(slots, log->slot);
-
-		/* Let implementation handle TLB/GVA invalidation */
-		kvm_mips_callbacks->flush_shadow_all(kvm);
+		kvm_flush_remote_tlbs(kvm);
 	}
 
 	mutex_unlock(&kvm->slots_lock);
-- 
2.24.1


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

* Re: [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs()
  2020-02-07 22:35 [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs() Peter Xu
                   ` (3 preceding siblings ...)
  2020-02-07 22:35 ` [PATCH RFC 4/4] KVM: MIPS: Define arch-specific kvm_flush_remote_tlbs() Peter Xu
@ 2020-02-07 23:00 ` " Peter Xu
  2020-02-12 12:25 ` Paolo Bonzini
  5 siblings, 0 replies; 11+ messages in thread
From: Peter Xu @ 2020-02-07 23:00 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Sean Christopherson, Paolo Bonzini, linux-mips, Vitaly Kuznetsov

On Fri, Feb 07, 2020 at 05:35:16PM -0500, Peter Xu wrote:
> [This series is RFC because I don't have MIPS to compile and test]
> 
> kvm_flush_remote_tlbs() can be arch-specific, by either:
> 
> - Completely replace kvm_flush_remote_tlbs(), like ARM, who is the
>   only user of CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL so far
> 
> - Doing something extra before kvm_flush_remote_tlbs(), like MIPS VZ
>   support, however still wants to have the common tlb flush to be part
>   of the process.  Could refer to kvm_vz_flush_shadow_all().  Then in
>   MIPS it's awkward to flush remote TLBs: we'll need to call the mips
>   hooks.
> 
> It's awkward to have different ways to specialize this procedure,
> especially MIPS cannot use the genenal interface which is quite a
> pity.  It's good to make it a common interface.
> 
> This patch series removes the 2nd MIPS usage above, and let it also
> use the common kvm_flush_remote_tlbs() interface.  It should be
> suggested that we always keep kvm_flush_remote_tlbs() be a common
> entrance for tlb flushing on all archs.
> 
> This idea comes from the reading of Sean's patchset on dynamic memslot
> allocation, where a new dirty log specific hook is added for flushing
> TLBs only for the MIPS code [1].  With this patchset, logically the

[1] https://lore.kernel.org/kvm/20200207194532.GK2401@linux.intel.com/T/#m2da733d75dab5e54e2ae68de94fe8411166d6274

> new hook in that patch can be dropped so we can directly use
> kvm_flush_remote_tlbs().
> 
> TODO: We can even extend another common interface for ranged TLB, but
> let's see how we think about this series first.
> 
> Any comment is welcomed, thanks.
> 
> Peter Xu (4):
>   KVM: Provide kvm_flush_remote_tlbs_common()
>   KVM: MIPS: Drop flush_shadow_memslot() callback
>   KVM: MIPS: Replace all the kvm_flush_remote_tlbs() references
>   KVM: MIPS: Define arch-specific kvm_flush_remote_tlbs()
> 
>  arch/mips/include/asm/kvm_host.h |  7 -------
>  arch/mips/kvm/Kconfig            |  1 +
>  arch/mips/kvm/mips.c             | 22 ++++++++++------------
>  arch/mips/kvm/trap_emul.c        | 15 +--------------
>  arch/mips/kvm/vz.c               | 14 ++------------
>  include/linux/kvm_host.h         |  1 +
>  virt/kvm/kvm_main.c              | 10 ++++++++--
>  7 files changed, 23 insertions(+), 47 deletions(-)
> 
> -- 
> 2.24.1
> 

-- 
Peter Xu


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

* Re: [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs()
  2020-02-07 22:35 [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs() Peter Xu
                   ` (4 preceding siblings ...)
  2020-02-07 23:00 ` [PATCH RFC 0/4] KVM: MIPS: Provide " Peter Xu
@ 2020-02-12 12:25 ` Paolo Bonzini
  2020-02-12 16:30   ` Paul Burton
  5 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2020-02-12 12:25 UTC (permalink / raw)
  To: Peter Xu, linux-kernel, kvm
  Cc: Sean Christopherson, linux-mips, Vitaly Kuznetsov

On 07/02/20 23:35, Peter Xu wrote:
> [This series is RFC because I don't have MIPS to compile and test]
> 
> kvm_flush_remote_tlbs() can be arch-specific, by either:
> 
> - Completely replace kvm_flush_remote_tlbs(), like ARM, who is the
>   only user of CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL so far
> 
> - Doing something extra before kvm_flush_remote_tlbs(), like MIPS VZ
>   support, however still wants to have the common tlb flush to be part
>   of the process.  Could refer to kvm_vz_flush_shadow_all().  Then in
>   MIPS it's awkward to flush remote TLBs: we'll need to call the mips
>   hooks.
> 
> It's awkward to have different ways to specialize this procedure,
> especially MIPS cannot use the genenal interface which is quite a
> pity.  It's good to make it a common interface.
> 
> This patch series removes the 2nd MIPS usage above, and let it also
> use the common kvm_flush_remote_tlbs() interface.  It should be
> suggested that we always keep kvm_flush_remote_tlbs() be a common
> entrance for tlb flushing on all archs.
> 
> This idea comes from the reading of Sean's patchset on dynamic memslot
> allocation, where a new dirty log specific hook is added for flushing
> TLBs only for the MIPS code [1].  With this patchset, logically the
> new hook in that patch can be dropped so we can directly use
> kvm_flush_remote_tlbs().
> 
> TODO: We can even extend another common interface for ranged TLB, but
> let's see how we think about this series first.
> 
> Any comment is welcomed, thanks.
> 
> Peter Xu (4):
>   KVM: Provide kvm_flush_remote_tlbs_common()
>   KVM: MIPS: Drop flush_shadow_memslot() callback
>   KVM: MIPS: Replace all the kvm_flush_remote_tlbs() references
>   KVM: MIPS: Define arch-specific kvm_flush_remote_tlbs()
> 
>  arch/mips/include/asm/kvm_host.h |  7 -------
>  arch/mips/kvm/Kconfig            |  1 +
>  arch/mips/kvm/mips.c             | 22 ++++++++++------------
>  arch/mips/kvm/trap_emul.c        | 15 +--------------
>  arch/mips/kvm/vz.c               | 14 ++------------
>  include/linux/kvm_host.h         |  1 +
>  virt/kvm/kvm_main.c              | 10 ++++++++--
>  7 files changed, 23 insertions(+), 47 deletions(-)
> 

Compile-tested and queued.

MIPS folks, I see that arch/mips/kvm/mmu.c uses pud_index, so it's not
clear to me if it's meant to only work if CONFIG_PGTABLE_LEVELS=4 or
it's just bit rot.  Should I add a "depends on PGTABLE_LEVEL=4" to
arch/mips/Kconfig?

Paolo


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

* Re: [PATCH RFC 1/4] KVM: Provide kvm_flush_remote_tlbs_common()
  2020-02-07 22:35 ` [PATCH RFC 1/4] KVM: Provide kvm_flush_remote_tlbs_common() Peter Xu
@ 2020-02-12 13:33   ` Paolo Bonzini
  0 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2020-02-12 13:33 UTC (permalink / raw)
  To: Peter Xu, linux-kernel, kvm
  Cc: Sean Christopherson, linux-mips, Vitaly Kuznetsov

On 07/02/20 23:35, Peter Xu wrote:
> It's exactly kvm_flush_remote_tlbs() now but a internal wrapper of the
> common code path.  With this, an arch can then optionally select
> CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL=y and will be able to use the
> common flushing code.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

Slightly more efficient, making it an inline function:

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index e89eb67356cb..f92180eeffc6 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -802,9 +802,18 @@ int kvm_vcpu_write_guest(struct kvm_vcpu *vcpu, gpa_t gpa, const void *data,
 int kvm_vcpu_yield_to(struct kvm_vcpu *target);
 void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu, bool usermode_vcpu_not_eligible);
 
-void kvm_flush_remote_tlbs(struct kvm *kvm);
+void kvm_flush_remote_tlbs_common(struct kvm *kvm);
 void kvm_reload_remote_mmus(struct kvm *kvm);
 
+#ifdef CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL
+void kvm_flush_remote_tlbs(struct kvm *kvm);
+#else
+static inline void kvm_flush_remote_tlbs(struct kvm *kvm)
+{
+	kvm_flush_remote_tlbs_common(kvm);
+}
+#endif
+
 bool kvm_make_vcpus_request_mask(struct kvm *kvm, unsigned int req,
 				 unsigned long *vcpu_bitmap, cpumask_var_t tmp);
 bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 70f03ce0e5c1..027259af883e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -303,8 +303,7 @@ bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req)
 	return called;
 }
 
-#ifndef CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL
-void kvm_flush_remote_tlbs(struct kvm *kvm)
+void kvm_flush_remote_tlbs_common(struct kvm *kvm)
 {
 	/*
 	 * Read tlbs_dirty before setting KVM_REQ_TLB_FLUSH in
@@ -328,8 +327,7 @@ void kvm_flush_remote_tlbs(struct kvm *kvm)
 		++kvm->stat.remote_tlb_flush;
 	cmpxchg(&kvm->tlbs_dirty, dirty_count, 0);
 }
-EXPORT_SYMBOL_GPL(kvm_flush_remote_tlbs);
-#endif
+EXPORT_SYMBOL_GPL(kvm_flush_remote_tlbs_common);
 
 void kvm_reload_remote_mmus(struct kvm *kvm)
 {


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

* Re: [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs()
  2020-02-12 12:25 ` Paolo Bonzini
@ 2020-02-12 16:30   ` Paul Burton
  2020-02-12 16:40     ` Paolo Bonzini
  0 siblings, 1 reply; 11+ messages in thread
From: Paul Burton @ 2020-02-12 16:30 UTC (permalink / raw)
  To: Paolo Bonzini, Mike Rapoport
  Cc: Peter Xu, linux-kernel, kvm, Sean Christopherson, linux-mips,
	Vitaly Kuznetsov

Hi Paolo,

On Wed, Feb 12, 2020 at 01:25:30PM +0100, Paolo Bonzini wrote:
> MIPS folks, I see that arch/mips/kvm/mmu.c uses pud_index, so it's not
> clear to me if it's meant to only work if CONFIG_PGTABLE_LEVELS=4 or
> it's just bit rot.  Should I add a "depends on PGTABLE_LEVEL=4" to
> arch/mips/Kconfig?

I'm no expert on this bit of code, but I'm pretty sure the systems
KVM/VZ has been used on the most internally had PGTABLE_LEVEL=3.

I suspect this is actually a regression from commit 31168f033e37 ("mips:
drop __pXd_offset() macros that duplicate pXd_index() ones"). Whilst
that commit is correct that pud_index() & __pud_offset() are the same
when pud_index() is actually provided, it doesn't take into account the
__PAGETABLE_PUD_FOLDED case. There __pud_offset() was available but
would always evaluate to zero, whereas pud_index() isn't defined...

Thanks,
    Paul

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

* Re: [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs()
  2020-02-12 16:30   ` Paul Burton
@ 2020-02-12 16:40     ` Paolo Bonzini
  2020-02-12 19:02       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2020-02-12 16:40 UTC (permalink / raw)
  To: Paul Burton, Mike Rapoport
  Cc: Peter Xu, linux-kernel, kvm, Sean Christopherson, linux-mips,
	Vitaly Kuznetsov

On 12/02/20 17:30, Paul Burton wrote:
> Hi Paolo,
> 
> On Wed, Feb 12, 2020 at 01:25:30PM +0100, Paolo Bonzini wrote:
>> MIPS folks, I see that arch/mips/kvm/mmu.c uses pud_index, so it's not
>> clear to me if it's meant to only work if CONFIG_PGTABLE_LEVELS=4 or
>> it's just bit rot.  Should I add a "depends on PGTABLE_LEVEL=4" to
>> arch/mips/Kconfig?
> 
> I'm no expert on this bit of code, but I'm pretty sure the systems
> KVM/VZ has been used on the most internally had PGTABLE_LEVEL=3.
> 
> I suspect this is actually a regression from commit 31168f033e37 ("mips:
> drop __pXd_offset() macros that duplicate pXd_index() ones"). Whilst
> that commit is correct that pud_index() & __pud_offset() are the same
> when pud_index() is actually provided, it doesn't take into account the
> __PAGETABLE_PUD_FOLDED case. There __pud_offset() was available but
> would always evaluate to zero, whereas pud_index() isn't defined...

Ok, I'll try to whip out a patch that handles __PAGETABLE_PUD_FOLDED.
On the other hand this makes me worry about how much KVM is being tested
by people that care about MIPS (even just compile-tested).

Paolo


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

* Re: [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs()
  2020-02-12 16:40     ` Paolo Bonzini
@ 2020-02-12 19:02       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-12 19:02 UTC (permalink / raw)
  To: Paolo Bonzini, Paul Burton, Mike Rapoport
  Cc: Peter Xu, linux-kernel, kvm, Sean Christopherson, linux-mips,
	Vitaly Kuznetsov

On 2/12/20 5:40 PM, Paolo Bonzini wrote:
> On 12/02/20 17:30, Paul Burton wrote:
>> Hi Paolo,
>>
>> On Wed, Feb 12, 2020 at 01:25:30PM +0100, Paolo Bonzini wrote:
>>> MIPS folks, I see that arch/mips/kvm/mmu.c uses pud_index, so it's not
>>> clear to me if it's meant to only work if CONFIG_PGTABLE_LEVELS=4 or
>>> it's just bit rot.  Should I add a "depends on PGTABLE_LEVEL=4" to
>>> arch/mips/Kconfig?
>>
>> I'm no expert on this bit of code, but I'm pretty sure the systems
>> KVM/VZ has been used on the most internally had PGTABLE_LEVEL=3.
>>
>> I suspect this is actually a regression from commit 31168f033e37 ("mips:
>> drop __pXd_offset() macros that duplicate pXd_index() ones"). Whilst
>> that commit is correct that pud_index() & __pud_offset() are the same
>> when pud_index() is actually provided, it doesn't take into account the
>> __PAGETABLE_PUD_FOLDED case. There __pud_offset() was available but
>> would always evaluate to zero, whereas pud_index() isn't defined...
> 
> Ok, I'll try to whip out a patch that handles __PAGETABLE_PUD_FOLDED.
> On the other hand this makes me worry about how much KVM is being tested
> by people that care about MIPS (even just compile-tested).

FYI last time James confirmed he tested QEMU was in 2017:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg477133.html

At the end of 2019 he orphaned the QEMU part:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg667240.html
and dropped the kernel maintainance:
https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?id=9c48c48cd499

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

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-07 22:35 [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs() Peter Xu
2020-02-07 22:35 ` [PATCH RFC 1/4] KVM: Provide kvm_flush_remote_tlbs_common() Peter Xu
2020-02-12 13:33   ` Paolo Bonzini
2020-02-07 22:35 ` [PATCH RFC 2/4] KVM: MIPS: Drop flush_shadow_memslot() callback Peter Xu
2020-02-07 22:35 ` [PATCH RFC 3/4] KVM: MIPS: Replace all the kvm_flush_remote_tlbs() references Peter Xu
2020-02-07 22:35 ` [PATCH RFC 4/4] KVM: MIPS: Define arch-specific kvm_flush_remote_tlbs() Peter Xu
2020-02-07 23:00 ` [PATCH RFC 0/4] KVM: MIPS: Provide " Peter Xu
2020-02-12 12:25 ` Paolo Bonzini
2020-02-12 16:30   ` Paul Burton
2020-02-12 16:40     ` Paolo Bonzini
2020-02-12 19:02       ` Philippe Mathieu-Daudé

Linux-MIPS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mips/0 linux-mips/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mips linux-mips/ https://lore.kernel.org/linux-mips \
		linux-mips@vger.kernel.org
	public-inbox-index linux-mips

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mips


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git