Re: [PATCH 3/4] x86/vmx: cleanup vmx.c

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.02.2023 12:35, Xenia Ragiadakou wrote:
> On 2/21/23 13:26, Jan Beulich wrote:
>> On 17.02.2023 19:48, Xenia Ragiadakou wrote:
>>> Do not include the headers:
>>>    asm/hvm/vpic.h
>>>    asm/hvm/vpt.h
>>>    asm/io.h
>>>    asm/mce.h
>>>    asm/mem_sharing.h
>>>    asm/regs.h
>>>    public/arch-x86/cpuid.h
>>>    public/hvm/save.h
>>> because none of the declarations and macro definitions in them is used.
>>> Sort alphabetically the rest of the headers.
>>>
>>> Rearrange the code to replace all forward declarations with the function
>>> definitions.
>>>
>>> Replace double new lines with one.
>>>
>>> Reduce scope of nvmx_enqueue_n2_exceptions() to static because it is used
>>> only in this file.
>>>
>>> Move vmx_update_debug_state() in vmcs.c because it is used only in this file
>>> and limit its scope to this file by declaring it static and removing its
>>> declaration from vmx.h.
>>>
>>> Take the opportunity to remove all trailing spaces in vmx.c.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
>>> ---
>>>   xen/arch/x86/hvm/vmx/vmcs.c            |   12 +
>>>   xen/arch/x86/hvm/vmx/vmx.c             | 5844 ++++++++++++------------
>>>   xen/arch/x86/include/asm/hvm/vmx/vmx.h |    1 -
>>>   3 files changed, 2913 insertions(+), 2944 deletions(-)
>>
>> I'm afraid this is close to unreviewable and hence absolutely needs 
>> splitting.
>> With this massive amount of re-arrangement (it's half of vmx.c, after all) 
>> I'm
>> also concerned of losing "git blame"-ability for fair parts of the code 
>> there.
> 
> I understand. Let me split the changes apart from the one that 
> rearranges the code. Do you agree in principle? or do you want me to 
> except and sth else?

Well, the large amount of code movement wants at least one other party
(e.g. Kevin, Andrew, or Roger) agreeing with your approach. As said, I
for one don't like this interruption in half-way easy history
determination (which can be particularly helpful e.g. when you want to
find a commit to put in a Fixes: tag).

Jan