Re: [PATCH v3 4/4] KVM: VMX: Change ple_window type to unsigned int

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Thu, Sep 05, 2019 at 10:36:16AM +0800, Peter Xu wrote:
> The VMX ple_window is 32 bits wide, so logically it can overflow with
> an int.  The module parameter is declared as unsigned int which is
> good, however the dynamic variable is not.  Switching all the
> ple_window references to use unsigned int.
> 
> The tracepoint changes will also affect SVM, but SVM is using an even
> smaller width (16 bits) so it's always fine.
> 
> Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  arch/x86/kvm/trace.h   | 8 ++++----
>  arch/x86/kvm/vmx/vmx.c | 4 ++--
>  arch/x86/kvm/vmx/vmx.h | 2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
> index f1177e03768f..ae924566c401 100644
> --- a/arch/x86/kvm/trace.h
> +++ b/arch/x86/kvm/trace.h
> @@ -891,13 +891,13 @@ TRACE_EVENT(kvm_pml_full,
>  );
>  
>  TRACE_EVENT(kvm_ple_window_update,
> -	TP_PROTO(unsigned int vcpu_id, int new, int old),
> +	TP_PROTO(unsigned int vcpu_id, unsigned int new, unsigned int old),
>  	TP_ARGS(vcpu_id, new, old),
>  
>  	TP_STRUCT__entry(
>  		__field(        unsigned int,   vcpu_id         )
> -		__field(                 int,       new         )
> -		__field(                 int,       old         )
> +		__field(        unsigned int,       new         )
> +		__field(        unsigned int,       old         )

Changing the trace event storage needs to be done in patch 3/4, otherwise
we're knowingly introducing a bug (for one commit).  Alternatively, swap
the order of the patches.

>  	),
>  
>  	TP_fast_assign(
> @@ -906,7 +906,7 @@ TRACE_EVENT(kvm_ple_window_update,
>  		__entry->old            = old;
>  	),
>  
> -	TP_printk("vcpu %u old %d new %d (%s)",
> +	TP_printk("vcpu %u old %u new %u (%s)",
>  	          __entry->vcpu_id, __entry->old, __entry->new,
>  		  __entry->old < __entry->new ? "growed" : "shrinked")
>  );
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 469c4134a4a7..1dbb63ffdd6d 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -5227,7 +5227,7 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
>  static void grow_ple_window(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> -	int old = vmx->ple_window;
> +	unsigned int old = vmx->ple_window;
>  
>  	vmx->ple_window = __grow_ple_window(old, ple_window,
>  					    ple_window_grow,
> @@ -5243,7 +5243,7 @@ static void grow_ple_window(struct kvm_vcpu *vcpu)
>  static void shrink_ple_window(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> -	int old = vmx->ple_window;
> +	unsigned int old = vmx->ple_window;
>  
>  	vmx->ple_window = __shrink_ple_window(old, ple_window,
>  					      ple_window_shrink,
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index 82d0bc3a4d52..64d5a4890aa9 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -253,7 +253,7 @@ struct vcpu_vmx {
>  	struct nested_vmx nested;
>  
>  	/* Dynamic PLE window. */
> -	int ple_window;
> +	unsigned int ple_window;
>  	bool ple_window_dirty;
>  
>  	bool req_immediate_exit;
> -- 
> 2.21.0
>