| Message ID | 20210402155807.49976-3-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: MIPS: cleanup TLB flushing callbacks | expand |
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 <pbonzini@redhat.com> 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 <pbonzini@redhat.com> > --- > 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 > >
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
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
Hi, Paolo, On Sat, Apr 3, 2021 at 6:43 PM Paolo Bonzini <pbonzini@redhat.com> 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 >
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
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.
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
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,
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 <pbonzini@redhat.com> --- 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(-)