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@xxxxxxxxx>
> Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> ---
> 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
>
