On Thu, 26 Jan 2023 18:40:20 +0000,
David Matlack <dmatlack@xxxxxxxxxx> wrote:
>
> Use kvm_arch_flush_remote_tlbs() instead of
> CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL. The two mechanisms solve the same
> problem, allowing architecture-specific code to provide a non-IPI
> implementation of remote TLB flushing.
>
> Dropping CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL allows KVM to standardize
> all architectures on kvm_arch_flush_remote_tlbs() instead of maintaining
> two mechanisms.
>
> Opt to standardize on kvm_arch_flush_remote_tlbs() since it avoids
> duplicating the generic TLB stats across architectures that implement
> their own remote TLB flush.
>
> This adds an extra function call to the ARM64 kvm_flush_remote_tlbs()
> path, but that is a small cost in comparison to flushing remote TLBs.
>
> No functional change intended.
>
> Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
> ---
> arch/arm64/include/asm/kvm_host.h | 3 +++
> arch/arm64/kvm/Kconfig | 1 -
> arch/arm64/kvm/mmu.c | 6 +++---
> virt/kvm/Kconfig | 3 ---
> virt/kvm/kvm_main.c | 2 --
> 5 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 113e20fdbb56..062800f1dc54 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -998,6 +998,9 @@ int __init kvm_set_ipa_limit(void);
> #define __KVM_HAVE_ARCH_VM_ALLOC
> struct kvm *kvm_arch_alloc_vm(void);
>
> +#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS
> +int kvm_arch_flush_remote_tlbs(struct kvm *kvm);
> +
> static inline bool kvm_vm_is_protected(struct kvm *kvm)
> {
> return false;
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index ca6eadeb7d1a..e9ac57098a0b 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -25,7 +25,6 @@ menuconfig KVM
> select MMU_NOTIFIER
> select PREEMPT_NOTIFIERS
> select HAVE_KVM_CPU_RELAX_INTERCEPT
> - select HAVE_KVM_ARCH_TLB_FLUSH_ALL
> select KVM_MMIO
> select KVM_GENERIC_DIRTYLOG_READ_PROTECT
> select KVM_XFER_TO_GUEST_WORK
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 01352f5838a0..8840f65e0e40 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -80,15 +80,15 @@ static bool memslot_is_logging(struct kvm_memory_slot *memslot)
> }
>
> /**
> - * kvm_flush_remote_tlbs() - flush all VM TLB entries for v7/8
> + * kvm_arch_flush_remote_tlbs() - flush all VM TLB entries for v7/8
> * @kvm: pointer to kvm structure.
> *
> * Interface to HYP function to flush all VM TLB entries
> */
> -void kvm_flush_remote_tlbs(struct kvm *kvm)
> +int kvm_arch_flush_remote_tlbs(struct kvm *kvm)
> {
> - ++kvm->stat.generic.remote_tlb_flush_requests;
> kvm_call_hyp(__kvm_tlb_flush_vmid, &kvm->arch.mmu);
> + return 0;
> }
>
> static bool kvm_is_device_pfn(unsigned long pfn)
> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> index b74916de5183..484d0873061c 100644
> --- a/virt/kvm/Kconfig
> +++ b/virt/kvm/Kconfig
> @@ -62,9 +62,6 @@ config HAVE_KVM_CPU_RELAX_INTERCEPT
> config KVM_VFIO
> bool
>
> -config HAVE_KVM_ARCH_TLB_FLUSH_ALL
> - bool
> -
> config HAVE_KVM_INVALID_WAKEUPS
> bool
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 277507463678..fefd3e3c8fe1 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -347,7 +347,6 @@ bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req)
> }
> EXPORT_SYMBOL_GPL(kvm_make_all_cpus_request);
>
> -#ifndef CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL
> void kvm_flush_remote_tlbs(struct kvm *kvm)
> {
> ++kvm->stat.generic.remote_tlb_flush_requests;
> @@ -368,7 +367,6 @@ void kvm_flush_remote_tlbs(struct kvm *kvm)
> ++kvm->stat.generic.remote_tlb_flush;
> }
For context, we currently have this:
if (!kvm_arch_flush_remote_tlb(kvm)
|| kvm_make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH))
++kvm->stat.generic.remote_tlb_flush;
Is there any reason why we shouldn't move the KVM_REQ_TLB_FLUSH call
into the arch-specific helpers? This is architecture specific, even if
the majority of the supported architecture cannot do broadcast
invalidation like arm64 does.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
