On Fri, 2022-08-26 at 16:12 -0700, David Matlack wrote:
> Change tdp_mmu to a read-only parameter and drop the per-vm
> tdp_mmu_enabled. For 32-bit KVM, make tdp_mmu_enabled a const bool so
> that the compiler can continue omitting calls to the TDP MMU.
>
> The TDP MMU was introduced in 5.10 and has been enabled by default since
> 5.15. At this point there are no known functionality gaps between the
> TDP MMU and the shadow MMU, and the TDP MMU uses less memory and scales
> better with the number of vCPUs. In other words, there is no good reason
> to disable the TDP MMU on a live system.
>
> Do not drop tdp_mmu=N support (i.e. do not force 64-bit KVM to always
> use the TDP MMU) since tdp_mmu=N is still used to get test coverage of
> KVM's shadow MMU TDP support, which is used in 32-bit KVM.
>
> Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
> ---
> arch/x86/include/asm/kvm_host.h | 9 ------
> arch/x86/kvm/mmu.h | 11 +++----
> arch/x86/kvm/mmu/mmu.c | 54 ++++++++++++++++++++++-----------
> arch/x86/kvm/mmu/tdp_mmu.c | 9 ++----
> 4 files changed, 44 insertions(+), 39 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 2c96c43c313a..d76059270a43 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1262,15 +1262,6 @@ struct kvm_arch {
> struct task_struct *nx_lpage_recovery_thread;
>
> #ifdef CONFIG_X86_64
> - /*
> - * Whether the TDP MMU is enabled for this VM. This contains a
> - * snapshot of the TDP MMU module parameter from when the VM was
> - * created and remains unchanged for the life of the VM. If this is
> - * true, TDP MMU handler functions will run for various MMU
> - * operations.
> - */
> - bool tdp_mmu_enabled;
> -
> /*
> * List of struct kvm_mmu_pages being used as roots.
> * All struct kvm_mmu_pages in the list should have
> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> index 6bdaacb6faa0..dd014bece7f0 100644
> --- a/arch/x86/kvm/mmu.h
> +++ b/arch/x86/kvm/mmu.h
> @@ -230,15 +230,14 @@ static inline bool kvm_shadow_root_allocated(struct kvm *kvm)
> }
>
> #ifdef CONFIG_X86_64
> -static inline bool is_tdp_mmu_enabled(struct kvm *kvm) { return kvm->arch.tdp_mmu_enabled; }
> -#else
> -static inline bool is_tdp_mmu_enabled(struct kvm *kvm) { return false; }
> -#endif
> -
> +extern bool tdp_mmu_enabled;
> static inline bool kvm_memslots_have_rmaps(struct kvm *kvm)
> {
> - return !is_tdp_mmu_enabled(kvm) || kvm_shadow_root_allocated(kvm);
> + return !tdp_mmu_enabled || kvm_shadow_root_allocated(kvm);
> }
> +#else
> +static inline bool kvm_memslots_have_rmaps(struct kvm *kvm) { return true; }
> +#endif
>
> static inline gfn_t gfn_to_index(gfn_t gfn, gfn_t base_gfn, int level)
> {
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index e418ef3ecfcb..7caf51023d47 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -98,6 +98,16 @@ module_param_named(flush_on_reuse, force_flush_and_sync_on_reuse, bool, 0644);
> */
> bool tdp_enabled = false;
>
> +bool __read_mostly tdp_mmu_allowed;
This can be __ro_after_init since it is only set in kvm_mmu_x86_module_init()
which is tagged with __init.
> +
> +#ifdef CONFIG_X86_64
> +bool __read_mostly tdp_mmu_enabled = true;
> +module_param_named(tdp_mmu, tdp_mmu_enabled, bool, 0444);
> +#else
> +/* TDP MMU is not supported on 32-bit KVM. */
> +const bool tdp_mmu_enabled;
> +#endif
> +
I am not sure by using 'const bool' the compile will always omit the function
call? I did some experiment on my 64-bit system and it seems if we don't use
any -O option then the generated code still does function call.
How about just (if it works):
#define tdp_mmu_enabled false
?
--
Thanks,
-Kai
