2021-04-02 15:59:14

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH 0/4] KVM: MIPS: cleanup TLB flushing callbacks

This series cleans up the flush_shadow_all and flush_shadow_memslot
callbacks, turning them into a single function that can be called from
kvm_arch_flush_remote_tlb. This lets MIPS use the generic TLB flushing
code in the MMU notifiers. With the upcoming refactoring of generic
MMU notifier code, without this patch MIPS would not be able to
coalesce TLB flushes anymore across multiple memslots.

Patch 1 is an unrelated cleanup that is needed for the rest to compile
(due to a call to kvm_arch_flush_remote_tlbs_memslot with a const
argument).

Paolo

Paolo Bonzini (4):
KVM: constify kvm_arch_flush_remote_tlbs_memslot
KVM: MIPS: rework flush_shadow_* callbacks into one that prepares the
flush
KVM: MIPS: let generic code call prepare_flush_shadow
KVM: MIPS: defer flush to generic MMU notifier code

arch/arm64/kvm/arm.c | 2 +-
arch/mips/include/asm/kvm_host.h | 12 ++++--------
arch/mips/kvm/mips.c | 21 +++++++++++----------
arch/mips/kvm/mmu.c | 12 ++----------
arch/mips/kvm/trap_emul.c | 13 ++-----------
arch/mips/kvm/vz.c | 19 ++++---------------
arch/x86/kvm/mmu/mmu.c | 2 +-
include/linux/kvm_host.h | 2 +-
8 files changed, 26 insertions(+), 57 deletions(-)

--
2.30.1


2021-04-02 15:59:23

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH 1/4] KVM: constify kvm_arch_flush_remote_tlbs_memslot

memslots are stored in RCU and there should be no need to
change them.

Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/arm64/kvm/arm.c | 2 +-
arch/mips/kvm/mips.c | 2 +-
arch/x86/kvm/mmu/mmu.c | 2 +-
include/linux/kvm_host.h | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 7f06ba76698d..9141730b7963 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1268,7 +1268,7 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)
}

void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
- struct kvm_memory_slot *memslot)
+ const struct kvm_memory_slot *memslot)
{
kvm_flush_remote_tlbs(kvm);
}
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 58a8812e2fa5..7db8234a4407 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -997,7 +997,7 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)
}

void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
- struct kvm_memory_slot *memslot)
+ const struct kvm_memory_slot *memslot)
{
/* Let implementation handle TLB/GVA invalidation */
kvm_mips_callbacks->flush_shadow_memslot(kvm, memslot);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 0d92a269c5fa..f75c677910c9 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5616,7 +5616,7 @@ void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm,
}

void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
- struct kvm_memory_slot *memslot)
+ const struct kvm_memory_slot *memslot)
{
/*
* All current use cases for flushing the TLBs for a specific memslot
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index e6d77353025c..34a974ffc882 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -894,7 +894,7 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot);

#ifdef CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT
void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
- struct kvm_memory_slot *memslot);
+ const struct kvm_memory_slot *memslot);
#else /* !CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT */
int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log);
int kvm_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log,
--
2.30.1


2021-04-02 16:00:08

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH 3/4] KVM: MIPS: let generic code call prepare_flush_shadow

Since all calls to kvm_flush_remote_tlbs must be preceded by
kvm_mips_callbacks->prepare_flush_shadow, repurpose
kvm_arch_flush_remote_tlb to invoke it. This makes it possible
to use the TLB flushing mechanism provided by the generic MMU
notifier code.

Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/mips/include/asm/kvm_host.h | 3 +++
arch/mips/kvm/mips.c | 11 ++++++-----
arch/mips/kvm/mmu.c | 6 +-----
3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 6c8c0ab53be2..d0944a75fc8d 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -1142,4 +1142,7 @@ static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}

+#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB
+int kvm_arch_flush_remote_tlb(struct kvm *kvm);
+
#endif /* __MIPS_KVM_HOST_H__ */
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 867b8de0fc07..4a22ba70c943 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -204,9 +204,6 @@ 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->prepare_flush_shadow(kvm);
kvm_flush_remote_tlbs(kvm);
}

@@ -995,11 +992,15 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)

}

+int kvm_arch_flush_remote_tlb(struct kvm *kvm)
+{
+ kvm_mips_callbacks->prepare_flush_shadow(kvm);
+ return 1;
+}
+
void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
const struct kvm_memory_slot *memslot)
{
- /* Let implementation handle TLB/GVA invalidation */
- kvm_mips_callbacks->prepare_flush_shadow(kvm);
kvm_flush_remote_tlbs(kvm);
}

diff --git a/arch/mips/kvm/mmu.c b/arch/mips/kvm/mmu.c
index 7e055e5dfd3c..2cedf908d744 100644
--- a/arch/mips/kvm/mmu.c
+++ b/arch/mips/kvm/mmu.c
@@ -490,8 +490,6 @@ int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end,
unsigned flags)
{
handle_hva_to_gpa(kvm, start, end, &kvm_unmap_hva_handler, NULL);
-
- kvm_mips_callbacks->prepare_flush_shadow(kvm);
kvm_flush_remote_tlbs(kvm);
return 0;
}
@@ -533,10 +531,8 @@ int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
int ret;

ret = handle_hva_to_gpa(kvm, hva, end, &kvm_set_spte_handler, &pte);
- if (ret) {
- kvm_mips_callbacks->prepare_flush_shadow(kvm);
+ if (ret)
kvm_flush_remote_tlbs(kvm);
- }
return 0;
}

--
2.30.1


2021-04-02 16:01:31

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH 2/4] KVM: MIPS: rework flush_shadow_* callbacks into one that prepares the flush

Both trap-and-emulate and VZ have a single implementation that covers
both .flush_shadow_all and .flush_shadow_memslot, and both of them end
with a call to kvm_flush_remote_tlbs.

Unify the callbacks into one and extract the call to kvm_flush_remote_tlbs.
The next patches will pull it further out of the the architecture-specific
MMU notifier functions kvm_unmap_hva_range and kvm_set_spte_hva.

Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/mips/include/asm/kvm_host.h | 9 +--------
arch/mips/kvm/mips.c | 12 ++++++------
arch/mips/kvm/mmu.c | 9 ++++++---
arch/mips/kvm/trap_emul.c | 13 ++-----------
arch/mips/kvm/vz.c | 19 ++++---------------
5 files changed, 19 insertions(+), 43 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index feaa77036b67..6c8c0ab53be2 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -815,14 +815,7 @@ struct kvm_mips_callbacks {
int (*vcpu_init)(struct kvm_vcpu *vcpu);
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);
+ void (*prepare_flush_shadow)(struct kvm *kvm);
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 7db8234a4407..867b8de0fc07 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -206,7 +206,8 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm)
kvm_mips_flush_gpa_pt(kvm, 0, ~0);

/* Let implementation do the rest */
- kvm_mips_callbacks->flush_shadow_all(kvm);
+ kvm_mips_callbacks->prepare_flush_shadow(kvm);
+ kvm_flush_remote_tlbs(kvm);
}

void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
@@ -221,8 +222,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_memslot(kvm, slot);
+ kvm_arch_flush_remote_tlbs_memslot(kvm, slot);
spin_unlock(&kvm->mmu_lock);
}

@@ -262,9 +262,8 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
/* Write protect GPA page table entries */
needs_flush = kvm_mips_mkclean_gpa_pt(kvm, new->base_gfn,
new->base_gfn + new->npages - 1);
- /* Let implementation do the rest */
if (needs_flush)
- kvm_mips_callbacks->flush_shadow_memslot(kvm, new);
+ kvm_arch_flush_remote_tlbs_memslot(kvm, new);
spin_unlock(&kvm->mmu_lock);
}
}
@@ -1000,7 +999,8 @@ void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
const struct kvm_memory_slot *memslot)
{
/* Let implementation handle TLB/GVA invalidation */
- kvm_mips_callbacks->flush_shadow_memslot(kvm, memslot);
+ kvm_mips_callbacks->prepare_flush_shadow(kvm);
+ kvm_flush_remote_tlbs(kvm);
}

long kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
diff --git a/arch/mips/kvm/mmu.c b/arch/mips/kvm/mmu.c
index 3dabeda82458..7e055e5dfd3c 100644
--- a/arch/mips/kvm/mmu.c
+++ b/arch/mips/kvm/mmu.c
@@ -491,7 +491,8 @@ int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end,
{
handle_hva_to_gpa(kvm, start, end, &kvm_unmap_hva_handler, NULL);

- kvm_mips_callbacks->flush_shadow_all(kvm);
+ kvm_mips_callbacks->prepare_flush_shadow(kvm);
+ kvm_flush_remote_tlbs(kvm);
return 0;
}

@@ -532,8 +533,10 @@ int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
int ret;

ret = handle_hva_to_gpa(kvm, hva, end, &kvm_set_spte_handler, &pte);
- if (ret)
- kvm_mips_callbacks->flush_shadow_all(kvm);
+ if (ret) {
+ kvm_mips_callbacks->prepare_flush_shadow(kvm);
+ kvm_flush_remote_tlbs(kvm);
+ }
return 0;
}

diff --git a/arch/mips/kvm/trap_emul.c b/arch/mips/kvm/trap_emul.c
index 0788c00d7e94..5f2df497599c 100644
--- a/arch/mips/kvm/trap_emul.c
+++ b/arch/mips/kvm/trap_emul.c
@@ -687,16 +687,8 @@ static int kvm_trap_emul_vcpu_setup(struct kvm_vcpu *vcpu)
return 0;
}

-static void kvm_trap_emul_flush_shadow_all(struct kvm *kvm)
+static void kvm_trap_emul_prepare_flush_shadow(struct kvm *kvm)
{
- /* Flush GVA page tables and invalidate GVA ASIDs on all VCPUs */
- 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[] = {
@@ -1280,8 +1272,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_memslot = kvm_trap_emul_flush_shadow_memslot,
+ .prepare_flush_shadow = kvm_trap_emul_prepare_flush_shadow,
.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 2ffbe9264a31..2c75571dc4a2 100644
--- a/arch/mips/kvm/vz.c
+++ b/arch/mips/kvm/vz.c
@@ -3211,32 +3211,22 @@ static int kvm_vz_vcpu_setup(struct kvm_vcpu *vcpu)
return 0;
}

-static void kvm_vz_flush_shadow_all(struct kvm *kvm)
+static void kvm_vz_prepare_flush_shadow(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
* invalidation of these ASIDs individually.
*
* Instead mark all CPUs as needing ASID invalidation in
- * asid_flush_mask, and just use kvm_flush_remote_tlbs(kvm) to
+ * asid_flush_mask, and kvm_flush_remote_tlbs(kvm) will
* kick any running VCPUs so they check asid_flush_mask.
*/
cpumask_setall(&kvm->arch.asid_flush_mask);
- kvm_flush_remote_tlbs(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_vcpu *vcpu)
{
int cpu = smp_processor_id();
@@ -3292,8 +3282,7 @@ static struct kvm_mips_callbacks kvm_vz_callbacks = {
.vcpu_init = kvm_vz_vcpu_init,
.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,
+ .prepare_flush_shadow = kvm_vz_prepare_flush_shadow,
.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.30.1


2021-04-02 16:03:05

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH 4/4] KVM: MIPS: defer flush to generic MMU notifier code

Return 1 from kvm_unmap_hva_range and kvm_set_spte_hva if a flush is
needed, so that the generic code can coalesce the flushes.

Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/mips/kvm/mmu.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/arch/mips/kvm/mmu.c b/arch/mips/kvm/mmu.c
index 2cedf908d744..a3a49b009dc4 100644
--- a/arch/mips/kvm/mmu.c
+++ b/arch/mips/kvm/mmu.c
@@ -489,9 +489,7 @@ static int kvm_unmap_hva_handler(struct kvm *kvm, gfn_t gfn, gfn_t gfn_end,
int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end,
unsigned flags)
{
- handle_hva_to_gpa(kvm, start, end, &kvm_unmap_hva_handler, NULL);
- kvm_flush_remote_tlbs(kvm);
- return 0;
+ return handle_hva_to_gpa(kvm, start, end, &kvm_unmap_hva_handler, NULL);
}

static int kvm_set_spte_handler(struct kvm *kvm, gfn_t gfn, gfn_t gfn_end,
@@ -528,12 +526,7 @@ static int kvm_set_spte_handler(struct kvm *kvm, gfn_t gfn, gfn_t gfn_end,
int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
{
unsigned long end = hva + PAGE_SIZE;
- int ret;
-
- ret = handle_hva_to_gpa(kvm, hva, end, &kvm_set_spte_handler, &pte);
- if (ret)
- kvm_flush_remote_tlbs(kvm);
- return 0;
+ return handle_hva_to_gpa(kvm, hva, end, &kvm_set_spte_handler, &pte);
}

static int kvm_age_hva_handler(struct kvm *kvm, gfn_t gfn, gfn_t gfn_end,
--
2.30.1

2021-04-03 02:33:53

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH 1/4] KVM: constify kvm_arch_flush_remote_tlbs_memslot

Reviewed-by: Huacai Chen <[email protected]>

On Fri, Apr 2, 2021 at 11:58 PM Paolo Bonzini <[email protected]> wrote:
>
> memslots are stored in RCU and there should be no need to
> change them.
>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> arch/arm64/kvm/arm.c | 2 +-
> arch/mips/kvm/mips.c | 2 +-
> arch/x86/kvm/mmu/mmu.c | 2 +-
> include/linux/kvm_host.h | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 7f06ba76698d..9141730b7963 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1268,7 +1268,7 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)
> }
>
> void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
> - struct kvm_memory_slot *memslot)
> + const struct kvm_memory_slot *memslot)
> {
> kvm_flush_remote_tlbs(kvm);
> }
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index 58a8812e2fa5..7db8234a4407 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -997,7 +997,7 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)
> }
>
> void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
> - struct kvm_memory_slot *memslot)
> + const struct kvm_memory_slot *memslot)
> {
> /* Let implementation handle TLB/GVA invalidation */
> kvm_mips_callbacks->flush_shadow_memslot(kvm, memslot);
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 0d92a269c5fa..f75c677910c9 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -5616,7 +5616,7 @@ void kvm_mmu_zap_collapsible_sptes(struct kvm *kvm,
> }
>
> void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
> - struct kvm_memory_slot *memslot)
> + const struct kvm_memory_slot *memslot)
> {
> /*
> * All current use cases for flushing the TLBs for a specific memslot
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index e6d77353025c..34a974ffc882 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -894,7 +894,7 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot);
>
> #ifdef CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT
> void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
> - struct kvm_memory_slot *memslot);
> + const struct kvm_memory_slot *memslot);
> #else /* !CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT */
> int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log);
> int kvm_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log,
> --
> 2.30.1
>
>

2021-04-03 02:37:22

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH 2/4] KVM: MIPS: rework flush_shadow_* callbacks into one that prepares the flush

Hi, Paolo,

TE mode has been removed in the MIPS tree, can we also remove it in
KVM tree before this rework?

Huacai

On Fri, Apr 2, 2021 at 11:58 PM Paolo Bonzini <[email protected]> wrote:
>
> Both trap-and-emulate and VZ have a single implementation that covers
> both .flush_shadow_all and .flush_shadow_memslot, and both of them end
> with a call to kvm_flush_remote_tlbs.
>
> Unify the callbacks into one and extract the call to kvm_flush_remote_tlbs.
> The next patches will pull it further out of the the architecture-specific
> MMU notifier functions kvm_unmap_hva_range and kvm_set_spte_hva.
>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> arch/mips/include/asm/kvm_host.h | 9 +--------
> arch/mips/kvm/mips.c | 12 ++++++------
> arch/mips/kvm/mmu.c | 9 ++++++---
> arch/mips/kvm/trap_emul.c | 13 ++-----------
> arch/mips/kvm/vz.c | 19 ++++---------------
> 5 files changed, 19 insertions(+), 43 deletions(-)
>
> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> index feaa77036b67..6c8c0ab53be2 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -815,14 +815,7 @@ struct kvm_mips_callbacks {
> int (*vcpu_init)(struct kvm_vcpu *vcpu);
> 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);
> + void (*prepare_flush_shadow)(struct kvm *kvm);
> 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 7db8234a4407..867b8de0fc07 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -206,7 +206,8 @@ void kvm_arch_flush_shadow_all(struct kvm *kvm)
> kvm_mips_flush_gpa_pt(kvm, 0, ~0);
>
> /* Let implementation do the rest */
> - kvm_mips_callbacks->flush_shadow_all(kvm);
> + kvm_mips_callbacks->prepare_flush_shadow(kvm);
> + kvm_flush_remote_tlbs(kvm);
> }
>
> void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
> @@ -221,8 +222,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_memslot(kvm, slot);
> + kvm_arch_flush_remote_tlbs_memslot(kvm, slot);
> spin_unlock(&kvm->mmu_lock);
> }
>
> @@ -262,9 +262,8 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
> /* Write protect GPA page table entries */
> needs_flush = kvm_mips_mkclean_gpa_pt(kvm, new->base_gfn,
> new->base_gfn + new->npages - 1);
> - /* Let implementation do the rest */
> if (needs_flush)
> - kvm_mips_callbacks->flush_shadow_memslot(kvm, new);
> + kvm_arch_flush_remote_tlbs_memslot(kvm, new);
> spin_unlock(&kvm->mmu_lock);
> }
> }
> @@ -1000,7 +999,8 @@ void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
> const struct kvm_memory_slot *memslot)
> {
> /* Let implementation handle TLB/GVA invalidation */
> - kvm_mips_callbacks->flush_shadow_memslot(kvm, memslot);
> + kvm_mips_callbacks->prepare_flush_shadow(kvm);
> + kvm_flush_remote_tlbs(kvm);
> }
>
> long kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> diff --git a/arch/mips/kvm/mmu.c b/arch/mips/kvm/mmu.c
> index 3dabeda82458..7e055e5dfd3c 100644
> --- a/arch/mips/kvm/mmu.c
> +++ b/arch/mips/kvm/mmu.c
> @@ -491,7 +491,8 @@ int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end,
> {
> handle_hva_to_gpa(kvm, start, end, &kvm_unmap_hva_handler, NULL);
>
> - kvm_mips_callbacks->flush_shadow_all(kvm);
> + kvm_mips_callbacks->prepare_flush_shadow(kvm);
> + kvm_flush_remote_tlbs(kvm);
> return 0;
> }
>
> @@ -532,8 +533,10 @@ int kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
> int ret;
>
> ret = handle_hva_to_gpa(kvm, hva, end, &kvm_set_spte_handler, &pte);
> - if (ret)
> - kvm_mips_callbacks->flush_shadow_all(kvm);
> + if (ret) {
> + kvm_mips_callbacks->prepare_flush_shadow(kvm);
> + kvm_flush_remote_tlbs(kvm);
> + }
> return 0;
> }
>
> diff --git a/arch/mips/kvm/trap_emul.c b/arch/mips/kvm/trap_emul.c
> index 0788c00d7e94..5f2df497599c 100644
> --- a/arch/mips/kvm/trap_emul.c
> +++ b/arch/mips/kvm/trap_emul.c
> @@ -687,16 +687,8 @@ static int kvm_trap_emul_vcpu_setup(struct kvm_vcpu *vcpu)
> return 0;
> }
>
> -static void kvm_trap_emul_flush_shadow_all(struct kvm *kvm)
> +static void kvm_trap_emul_prepare_flush_shadow(struct kvm *kvm)
> {
> - /* Flush GVA page tables and invalidate GVA ASIDs on all VCPUs */
> - 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[] = {
> @@ -1280,8 +1272,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_memslot = kvm_trap_emul_flush_shadow_memslot,
> + .prepare_flush_shadow = kvm_trap_emul_prepare_flush_shadow,
> .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 2ffbe9264a31..2c75571dc4a2 100644
> --- a/arch/mips/kvm/vz.c
> +++ b/arch/mips/kvm/vz.c
> @@ -3211,32 +3211,22 @@ static int kvm_vz_vcpu_setup(struct kvm_vcpu *vcpu)
> return 0;
> }
>
> -static void kvm_vz_flush_shadow_all(struct kvm *kvm)
> +static void kvm_vz_prepare_flush_shadow(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
> * invalidation of these ASIDs individually.
> *
> * Instead mark all CPUs as needing ASID invalidation in
> - * asid_flush_mask, and just use kvm_flush_remote_tlbs(kvm) to
> + * asid_flush_mask, and kvm_flush_remote_tlbs(kvm) will
> * kick any running VCPUs so they check asid_flush_mask.
> */
> cpumask_setall(&kvm->arch.asid_flush_mask);
> - kvm_flush_remote_tlbs(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_vcpu *vcpu)
> {
> int cpu = smp_processor_id();
> @@ -3292,8 +3282,7 @@ static struct kvm_mips_callbacks kvm_vz_callbacks = {
> .vcpu_init = kvm_vz_vcpu_init,
> .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,
> + .prepare_flush_shadow = kvm_vz_prepare_flush_shadow,
> .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.30.1
>
>

2021-04-03 07:12:12

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 2/4] KVM: MIPS: rework flush_shadow_* callbacks into one that prepares the flush

On 03/04/21 04:31, Huacai Chen wrote:
> Hi, Paolo,
>
> TE mode has been removed in the MIPS tree, can we also remove it in
> KVM tree before this rework?

Fortunately I can pull the exact commit that was applied to the MIPS
tree, as it was the first patch that was applied to the tree, but next
time please send KVM changes through the KVM tree.

Paolo

2021-04-03 10:51:19

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 2/4] KVM: MIPS: rework flush_shadow_* callbacks into one that prepares the flush

On 03/04/21 04:31, Huacai Chen wrote:
> Hi, Paolo,
>
> TE mode has been removed in the MIPS tree, can we also remove it in
> KVM tree before this rework?

I tried the merge and it will be enough for Linus to remove
arch/mips/kvm/trap_emul.c. So I will leave it as is, but next time I'd
prefer KVM MIPS changes to go through either my tree or a common topic
branch.

Paolo

2021-04-06 12:23:08

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH 2/4] KVM: MIPS: rework flush_shadow_* callbacks into one that prepares the flush

Hi, Paolo,

On Sat, Apr 3, 2021 at 6:43 PM Paolo Bonzini <[email protected]> wrote:
>
> On 03/04/21 04:31, Huacai Chen wrote:
> > Hi, Paolo,
> >
> > TE mode has been removed in the MIPS tree, can we also remove it in
> > KVM tree before this rework?
>
> I tried the merge and it will be enough for Linus to remove
> arch/mips/kvm/trap_emul.c. So I will leave it as is, but next time I'd
> prefer KVM MIPS changes to go through either my tree or a common topic
> branch.
Emmm, the TE removal series is done by Thomas, not me. :)

Huacai
>
> Paolo
>

2021-04-06 15:50:56

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 2/4] KVM: MIPS: rework flush_shadow_* callbacks into one that prepares the flush

On 06/04/21 03:36, Huacai Chen wrote:
>> I tried the merge and it will be enough for Linus to remove
>> arch/mips/kvm/trap_emul.c. So I will leave it as is, but next time I'd
>> prefer KVM MIPS changes to go through either my tree or a common topic
>> branch.
> Emmm, the TE removal series is done by Thomas, not me.:)

Sure, sorry if the sentence sounded like it was directed to you. No
matter who wrote the code, synchronization between trees is only the
maintainers' task. :)

Paolo

2021-04-06 21:04:00

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 2/4] KVM: MIPS: rework flush_shadow_* callbacks into one that prepares the flush

On 06/04/21 13:39, Thomas Bogendoerfer wrote:
> On Tue, Apr 06, 2021 at 08:05:40AM +0200, Paolo Bonzini wrote:
>> On 06/04/21 03:36, Huacai Chen wrote:
>>>> I tried the merge and it will be enough for Linus to remove
>>>> arch/mips/kvm/trap_emul.c. So I will leave it as is, but next time I'd
>>>> prefer KVM MIPS changes to go through either my tree or a common topic
>>>> branch.
>>> Emmm, the TE removal series is done by Thomas, not me.:)
>>
>> Sure, sorry if the sentence sounded like it was directed to you. No matter
>> who wrote the code, synchronization between trees is only the maintainers'
>> task. :)
>
> Sorry about the mess. I'll leave arch/mips/kvm to go via your tree then.

No problem. In this specific case, at some point I'll pull from
mips-next, prior to sending the pull request to Linus. It will look
just like other KVM submaintainer pulls.

Paolo

2021-04-07 01:28:28

by Thomas Bogendoerfer

[permalink] [raw]
Subject: Re: [PATCH 2/4] KVM: MIPS: rework flush_shadow_* callbacks into one that prepares the flush

On Tue, Apr 06, 2021 at 08:05:40AM +0200, Paolo Bonzini wrote:
> On 06/04/21 03:36, Huacai Chen wrote:
> > > I tried the merge and it will be enough for Linus to remove
> > > arch/mips/kvm/trap_emul.c. So I will leave it as is, but next time I'd
> > > prefer KVM MIPS changes to go through either my tree or a common topic
> > > branch.
> > Emmm, the TE removal series is done by Thomas, not me.:)
>
> Sure, sorry if the sentence sounded like it was directed to you. No matter
> who wrote the code, synchronization between trees is only the maintainers'
> task. :)

Sorry about the mess. I'll leave arch/mips/kvm to go via your tree then.

Thomas.

--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]