On 21/02/20 05:32, Oliver Upton wrote:
> On Thu, Feb 20, 2020 at 3:23 AM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>>
>> On 18/02/20 20:07, 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.
>>
>> What if __gfn_to_hva_many and gfn_to_hva_many are marked __always_inline?
>>
>> Thanks,
>>
>> Paolo
>>
>
> Even with this suggestion the compiler is ill-convinced :/
>
> in re to Sean: what do I mean by "draconian compiler"
>
> Well, the public answer is that both Barret and I use the same
> compiler. Nothing particularly interesting about it, but idk what our
> toolchain folks' stance is on divulging details.
>
> I'll instead use Sean's suggested fix (which reads much better) and resend.
Can you instead look into fixing that compiler? After inlining, it is
trivial to realize that the first two returns imply
kvm_is_error_hva(ghc->hva). I'm asking this because even for GCC
-Wuninitialized *used to be* total crap, but these days it's quite
reliable and even basic data flow should be able to thread through the
jumps.
I'm more than willing to help the compiler with __always_inline hints,
but an "uninitialized_var()" adds load on the code and I'm not sure it
makes sense to do that for the sake of some proprietary, or at least
unnamed, software.
Paolo
