Re: [PATCH 03/13] kvm: monolithic: fixup x86-32 build

From: Paolo Bonzini <pbonzini@redhat.com>
To: Andrea Arcangeli <aarcange@redhat.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Sean Christopherson <sean.j.christopherson@intel.com>,
	Masahiro Yamada <yamada.masahiro@socionext.com>,
	Jessica Yu <jeyu@kernel.org>,
	Matthias Maennich <maennich@google.com>
Subject: Re: [PATCH 03/13] kvm: monolithic: fixup x86-32 build
Date: Tue, 5 Nov 2019 16:10:47 +0100	[thread overview]
Message-ID: <ab18744b-afc7-75d4-b5f3-e77e9aae41a6@redhat.com> (raw)
In-Reply-To: <20191105145651.GD30717@redhat.com>

On 05/11/19 15:56, Andrea Arcangeli wrote:
>>> I think we should:
>>>
>>> 1) whitelist to shut off the warnings on demand
>>
>> Do you mean adding a whitelist to modpost?  That would work, though I am
>> not sure if the module maintainer (Jessica Yu) would accept that.
> 
> Yes that's exactly what I meant.

Ok, thanks.  Jessica, the issue here is that we have two (mutually
exclusive) modules providing the same interface to a third module.

Andrea will check that, when the same symbol is exported by two modules,
the second-loaded module correctly fails insmod.  If that is okay, we
will also need modpost not to warn for these symbols in sym_add_exported.

>> The answer is maintainability.  My suggestion is that we start looking
>> into removing all assignments and tests of kvm_x86_ops, one step at a
>> time.  Until this is done, unfortunately we won't be able to reap the
>> performance benefit.  But the advantage is that this can be done in many
> 
> There's not much performance benefit left from the removal
> kvm_x86_ops.

Indeed; what I mean is that until then we will have to keep the
retpolines.  Not removing kvm_x86_ops leaves an unsustainable mess in
terms of maintainability, therefore we will need to first refactor the
code.  Once the refactoring is over, kvm_x86_ops can be dropped easily,
just like kvm_pmu_ops in this version of the series.

The good thing is that the modpost discussion can proceed in parallel.

> The removal of kvm_x86_ops is just a badly needed code cleanup and of
> course I agree it must happen sooner than later. I'm just trying to
> avoid running into rejects on those further commit cleanups too.

>> That is good enough to prove the feasibility of the idea, so I agree
>> that was a good plan.
> 
> All right, so I'm not exactly sure what's the plan and if it's ok to
> do it over time or if I should go ahead doing all logic changes while
> the big patch remains out of tree.

Yes, the changes to remove tests and assignments to kvm_x86_ops must
happen first.  I understand that the big patch is a conflict magnet, but
once all the refactoring is done it will be very easy to review and it
will get in quickly.

Paolo