Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/02/2023 8:08 pm, Xenia Ragiadakou wrote:
>
> On 2/24/23 21:29, Andrew Cooper wrote:
>> On 24/02/2023 6:49 pm, Xenia Ragiadakou wrote:
>>> There are more detailed per-patch changesets.
>>>
>>> Xenia Ragiadakou (14):
>>>    x86/svm: move declarations used only by svm code from svm.h to
>>> private
>>>      header
>>>    x86/svm: make asid.h private
>>>    x86/svm: delete header asm/hvm/svm/intr.h
>>
>> Thankyou for this work.  I've acked and committed patches 1 and 3.
>>
>> Note that you had one hunk in patch 5 (whitespace correction in
>> svm_invlpga) that obviously should have been in patch 1, so I've
>> moved it.
>
> Thanks, I missed that  ...
>
>>
>> Looking at asid.h, I still can't bare to keep it even in that state, so
>> I've constructed an alternative which I'll email out in a moment.
>
> I 'm also in favor of less headers.

I've committed as far as the nestedhvm move.  At that point, I've sent
out a small patch to try and help decouple later changes.

But I think we want to change tact slightly at this point, so I'm not
going to do any further tweaking on commit.

Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h,
updating the non-SVM include paths as we go.  Probably best to
chain-include the other svm/hvm/svm/*.h headers temporarily, so we've
only got one tree-wide cleanup of the external include paths.

Quick tangent - I will be making all of that cpu_has_svm_*
infrastructure disappear by moving it into the normal CPUID handling,
but I've not had sufficient time to finish that yet.

Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and
disappear (after my decoupling patch has gone in).

After that, hvm/svm/vmcb.h can be cleanly split in half.  struct
svm_{domain,vcpu} can move sideways into hvm/svm.h (with a forward
declaration of vmcb_struct), as can the svm msr intercept declarations. 
Everything else can move to being a private vmcb.h

Finally your svmdebug.h can come at the end, pretty much in the same
shape it is now.  One thing to say is that right now, you've left enum
vmcb_sync_state public, but it's type is already decoupled by virtue of
struct svm_vcpu having a uint8_t field rather than an enum field.

And I think that cleanly gets rid of the entire asm/hvm/svm/* dir, which
is a great position to get to.


Beyond that, you will want to clean up the hvm msr intercept handling as
hvm_funcs, which I think will decouple the vpmu files from svm/vmx
specifically, but that will want to be a separate series.

How does this sound?

Thanks,

~Andrew