[MODERATED] Re: [PATCH v5 8/8] NX 8

On Thu, Jul 11, 2019 at 10:46:37AM +0200, speck for Paolo Bonzini wrote:
> On 01/07/19 15:52, speck for Paolo Bonzini wrote:
> > From: Junaid Shahid <[email protected]>
> > Subject: [PATCH 8/8] kvm: x86: mmu: Recovery of shattered NX large pages
> > 
> > 
> > The page table pages corresponding to broken down large pages are
> > periodically zapped, so that the large page can potentially be
> > recovered, if it is no longer being used for execution.  This removes
> > the performance penalty for walking deeper EPT page tables.
> > 
> > Signed-off-by: Junaid Shahid <[email protected]>
> > Signed-off-by: Paolo Bonzini <[email protected]>
> 
> Hi all,
> 
> Andrea Arcangeli suggested that zapping everything could cause big
> slowdowns for large guests.  The follow up here changes the recovery
> to process a fraction of the NX pages every minute, in FIFO order.
> 
> Paolo
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 6454e5c79287..6749ebaa3ead 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2034,9 +2034,11 @@
>  			If the sw workaround is enabled for the host, guests
>  			need not enable it for nested guests.
>  
> -	kvm.nx_huge_pages_recovery_period_secs=
> -			[KVM] Time (s) interval to wait before zapping 4KiB
> -			pages back to huge pages.
> +	kvm.nx_huge_pages_recovery_ratio=
> +			[KVM] Controls how many 4KiB pages are periodically zapped
> +			back to huge pages.  0 disables the recovery, otherwise if
> +			the value is N KVM will zap 1/Nth of the 4KiB pages every
> +			minute.  The default is 60.
>  
>  	kvm-amd.nested=	[KVM,AMD] Allow nested virtualization in KVM/SVM.
>  			Default is 1 (enabled)
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index aea64339c57a..9c0497913ba3 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -54,26 +54,24 @@
>  extern bool itlb_multihit_kvm_mitigation;
>  
>  static int __read_mostly nx_huge_pages = -1;
> -static uint __read_mostly nx_huge_pages_recovery_period_secs = 3600;
> +static uint __read_mostly nx_huge_pages_recovery_ratio = 60;
>  
>  static int set_nx_huge_pages(const char *val, const struct kernel_param *kp);
> -static int set_nx_huge_pages_recovery_period(const char *val,
> -					     const struct kernel_param *kp);
> +static int set_nx_huge_pages_recovery_ratio(const char *val, const struct kernel_param *kp);
>  
>  static struct kernel_param_ops nx_huge_pages_ops = {
>  	.set = set_nx_huge_pages,
>  	.get = param_get_bool,
>  };
>  
> -static struct kernel_param_ops nx_huge_pages_recovery_period_ops = {
> -	.set = set_nx_huge_pages_recovery_period,
> -	.get = param_get_uint,
> +static struct kernel_param_ops nx_huge_pages_recovery_ratio_ops = {
> +	.set = set_nx_huge_pages_recovery_ratio,
> +	.get = param_get_bool,

Should 'param_get_bool' be 'param_get_uint' instead, as we would want to
output the ratio.

Do you want to rebase this series to a more recent version?

Thanks,
Pawan