Re: [PATCH] KVM: Suppress warning in __kvm_gfn_to_hva_cache_init

[Oliver Upton <oupton@google.com>]

Hey Sean,

On Tue, Feb 18, 2020 at 11:07:29AM -0800, Sean Christopherson wrote:
> On Tue, Feb 18, 2020 at 10:47:56AM -0800, Oliver Upton wrote:
> > Particularly draconian compilers warn of a possible uninitialized use of
> > the nr_pages_avail variable. Silence this warning by initializing it to
> > zero.
> 
> Can you check if the warning still exists with commit 6ad1e29fe0ab ("KVM:
> Clean up __kvm_gfn_to_hva_cache_init() and its callers")?  I'm guessing
> (hoping?) the suppression is no longer necessary.

Hmm. I rebased this patch right before sending out + it seems that it is
required (at least for me) to silence the compiler warning. For good
measure, I ran git branch --contains to ensure I had your change. Looks
like my topic branch did in fact have your fix.

--
Oliver

> commit 6ad1e29fe0aba843dfffc714fced0ef6a2e19502
> Author: Sean Christopherson <sean.j.christopherson@intel.com>
> Date:   Thu Jan 9 14:58:55 2020 -0500
> 
>     KVM: Clean up __kvm_gfn_to_hva_cache_init() and its callers
> 
>     Barret reported a (technically benign) bug where nr_pages_avail can be
>     accessed without being initialized if gfn_to_hva_many() fails.
> 
>       virt/kvm/kvm_main.c:2193:13: warning: 'nr_pages_avail' may be
>       used uninitialized in this function [-Wmaybe-uninitialized]
> 
>     Rather than simply squashing the warning by initializing nr_pages_avail,
>     fix the underlying issues by reworking __kvm_gfn_to_hva_cache_init() to
>     return immediately instead of continuing on.  Now that all callers check
>     the result and/or bail immediately on a bad hva, there's no need to
>     explicitly nullify the memslot on error.
> 
>     Reported-by: Barret Rhoden <brho@google.com>
>     Fixes: f1b9dd5eb86c ("kvm: Disallow wraparound in kvm_gfn_to_hva_cache_init")
>     Cc: Jim Mattson <jmattson@google.com>
>     Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
>     Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> 
> > Signed-off-by: Oliver Upton <oupton@google.com>
> > ---
> >  virt/kvm/kvm_main.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 70f03ce0e5c1..dc8a67ad082d 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -2219,7 +2219,7 @@ static int __kvm_gfn_to_hva_cache_init(struct kvm_memslots *slots,
> >  	gfn_t start_gfn = gpa >> PAGE_SHIFT;
> >  	gfn_t end_gfn = (gpa + len - 1) >> PAGE_SHIFT;
> >  	gfn_t nr_pages_needed = end_gfn - start_gfn + 1;
> > -	gfn_t nr_pages_avail;
> > +	gfn_t nr_pages_avail = 0;
> >  
> >  	/* Update ghc->generation before performing any error checks. */
> >  	ghc->generation = slots->generation;
> > -- 
> > 2.25.0.265.gbab2e86ba0-goog
> >