linux-kernel - Re: [PATCH 9/9] KVM: VMX: automatic PLE window maximum

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <53F44B40.6060806@redhat.com>
Date:	Wed, 20 Aug 2014 09:16:16 +0200
From:	Paolo Bonzini <pbonzini@...hat.com>
To:	Radim Krčmář <rkrcmar@...hat.com>,
	kvm@...r.kernel.org
CC:	linux-kernel@...r.kernel.org, Gleb Natapov <gleb@...nel.org>,
	Raghavendra KT <raghavendra.kt@...ux.vnet.ibm.com>,
	Vinod Chegu <chegu_vinod@...com>, Hui-Zhi <hui-zhi.zhao@...com>
Subject: Re: [PATCH 9/9] KVM: VMX: automatic PLE window maximum

Il 19/08/2014 22:35, Radim Krčmář ha scritto:
> Every increase of ple_window_grow creates potential overflows.
> They are not serious, because we clamp ple_window and userspace is
> expected to fix ple_window_max within a second.
> ---
>  arch/x86/kvm/vmx.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index d7f58e8..6873a0b 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -138,7 +138,9 @@ module_param(ple_window, int, S_IRUGO | S_IWUSR);
>  
>  /* Default doubles per-vcpu window every exit. */
>  static int ple_window_grow = KVM_VMX_DEFAULT_PLE_WINDOW_GROW;
> -module_param(ple_window_grow, int, S_IRUGO | S_IWUSR);
> +static struct kernel_param_ops ple_window_grow_ops;
> +module_param_cb(ple_window_grow, &ple_window_grow_ops,
> +                &ple_window_grow, S_IRUGO | S_IWUSR);
>  
>  /* Default resets per-vcpu window every exit to ple_window. */
>  static int ple_window_shrink = KVM_VMX_DEFAULT_PLE_WINDOW_SHRINK;
> @@ -5717,6 +5719,36 @@ static void type##_ple_window(struct kvm_vcpu *vcpu) \
>  make_ple_window_modifier(grow,   *, +) /* grow_ple_window */
>  make_ple_window_modifier(shrink, /, -) /* shrink_ple_window */
>  
> +static void clamp_ple_window_max(void)
> +{
> +	int maximum;
> +
> +	if (ple_window_grow < 1)
> +		return;
> +
> +	if (ple_window_grow < ple_window)
> +		maximum = INT_MAX / ple_window_grow;
> +	else
> +		maximum = INT_MAX - ple_window_grow;
> +
> +	ple_window_max = clamp(ple_window_max, ple_window, maximum);
> +}

I think avoiding overflows is better.  In fact, I think you should call
this function for ple_window_max too.

You could keep the ple_window_max variable to the user-set value.
Whenever ple_window_grow or ple_window_max are changed, you can set an
internal variable (let's call it ple_window_actual_max, but I'm not wed
to this name) to the computed value, and then do:

	if (ple_window_grow < 1 || ple_window_actual_max < ple_window)
		new = ple_window;
	else if (ple_window_grow < ple_window)
		new = max(ple_window_actual_max, old) * ple_window_grow;
	else
		new = max(ple_window_actual_max, old) + ple_window_grow;

(I think the || in the first "if" can be eliminated with some creativity
in clamp_ple_window_max).

Paolo

> +static int set_ple_window_grow(const char *arg, const struct kernel_param *kp)
> +{
> +	int ret;
> +
> +	clamp_ple_window_max();
> +	ret = param_set_int(arg, kp);
> +
> +	return ret;
> +}
> +
> +static struct kernel_param_ops ple_window_grow_ops = {
> +	.set = set_ple_window_grow,
> +	.get = param_get_int,
> +};
> +
>  /*
>   * Indicate a busy-waiting vcpu in spinlock. We do not enable the PAUSE
>   * exiting, so only get here on cpu with PAUSE-Loop-Exiting.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/