Re: [PATCH 1/4] x86/svm: cleanup svm.c

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.02.2023 08:45, Xenia Ragiadakou wrote:
> Hi Andrew,
> 
> On 2/21/23 00:12, Andrew Cooper wrote:
>> On 17/02/2023 6:48 pm, Xenia Ragiadakou wrote:
>>> Do not include the headers:
>>>    xen/irq.h
>>>    asm/hvm/svm/intr.h
>>>    asm/io.h
>>>    asm/mem_sharing.h
>>>    asm/regs.h
>>
>> Out of interest, how are you calculating these?  They're accurate as far
>> as I can tell.
> 
> I do not use a script (at least not a decent one), if that 's the 
> question :) . I verify that none of the symbols defined or declared in 
> the header is used in the file including the header.
> 
>>
>> Looking at asm/hvm/svm/*, intr.h itself can be straight deleted,
>> svmdebug.h can be merged into vmcb.h, and all the others can move into
>> xen/arch/x86/hvm/svm/ as local headers.  None of them have any business
>> being included elsewhere in Xen.
> 
> I can send another patch for that.
> 
>>
>>> because none of the declarations and macro definitions in them is used.
>>> Sort alphabetically the rest of the headers.
>>
>> Minor grammar point. "Sort the rest of the headers alphabetically" would
>> be a more normal way of phrasing this.
> 
> I will fix it in v2.

I guess this can be adjusted while committing, seeing that ...

>>> Remove the forward declaration of svm_function_table and place start_svm()
>>> after the svm_function_table's definition.
>>>
>>> Replace double new lines with one.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
>>
>> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

... it's otherwise ready to be committed.

Jan