Re: [PATCH 2/4] x86/svm: cleanup svm.h

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/02/2023 6:48 pm, Xenia Ragiadakou wrote:
> Remove the forward declaration of struct vcpu because it is not used.

Huh, turns out that was my fault in c/s b158e72abe, shortly after I
introduced them in the first place.

Also, looking into that, there's one legitimate use of svm.h from
outside, which is svm_load_segs*() which means we can't make all the
headers be local.

But still, most of svm.h shouldn't be includable in the rest of Xen. 
Perhaps we can make a separate dedicated header for just this.

[edit]  And svm_{doman,vcpu}.  Perhaps we want a brand new
include/asm/hvm/svm.h with only the things needed elsewhere.

> Move the forward declaration of struct cpu_user_regs just above the
> function that needs it (to remind that it will need to be removed
> along with the function).

I'm not sure.  This feels like churn.

> Move the definitions of NPT_PFEC_with_gla and NPT_PFEC_in_gpt in svm.c
> because they are used only in this file.

IMO, these would better live in vmcb.h because that's where all the
other decode information lives, not that there is much.  I previously
started trying to convert all the exit_into fields into a typed union,
but I didn't get as far the NPT info.

> Move the definitions of SVM_PAUSE{FILTER,THRESH}_INIT in vmcb.c because
> they are used only in this file.

Honestly, at this point you might as well just delete the defines, and
opencode at their single usage site.  They're pure obfuscation like
this, given no statement of units / behaviour, etc.

That said, we do need to stea^W borrow adaptive PLE, and make the
settings hvm-common because right now we've got two different ways of
configuring the same thing for VT-x and SVM.  (This is definitely not
cleanup work.  Just an observation for anyone feeling at a loose end.)

~Andrew