Re: [kvm-unit-tests PATCH 13/16] svm: move vmcb_ident to svm_lib.c

[Sean Christopherson <seanjc@google.com>]

On Thu, Oct 20, 2022, Maxim Levitsky wrote:

Changelog please.  
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  lib/x86/svm_lib.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
>  lib/x86/svm_lib.h |  4 ++++

What about calling these simply svm.{c,h} and renaming x86/svm.{c,h} to something
like svm_common.{c,h}?  Long term, it would be wonderful to rid of x86/svm.{c,h}
by genericizing the test framework, e.g. there's a ton of duplicate code between
SVM and VMX.

>  x86/svm.c         | 54 -----------------------------------------------
>  x86/svm.h         |  1 -
>  4 files changed, 58 insertions(+), 55 deletions(-)
> 
> diff --git a/lib/x86/svm_lib.c b/lib/x86/svm_lib.c
> index 9e82e363..2b067c65 100644
> --- a/lib/x86/svm_lib.c
> +++ b/lib/x86/svm_lib.c
> @@ -103,3 +103,57 @@ void setup_svm(void)
>  
>  	setup_npt();
>  }
> +
> +void vmcb_set_seg(struct vmcb_seg *seg, u16 selector,
> +			 u64 base, u32 limit, u32 attr)

Funky indentation and wrap.

void vmcb_set_seg(struct vmcb_seg *seg, u16 selector, u64 base, u32 limit,
		  u32 attr)

> +{
> +	seg->selector = selector;
> +	seg->attrib = attr;
> +	seg->limit = limit;
> +	seg->base = base;
> +}
> +
> +void vmcb_ident(struct vmcb *vmcb)
> +{
> +	u64 vmcb_phys = virt_to_phys(vmcb);
> +	struct vmcb_save_area *save = &vmcb->save;
> +	struct vmcb_control_area *ctrl = &vmcb->control;
> +	u32 data_seg_attr = 3 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK

Ugh, a #define for '3' and '9' (in lib/x86/desc.h?) would be nice, but that can
be left for another day/patch.

> +		| SVM_SELECTOR_DB_MASK | SVM_SELECTOR_G_MASK;

Pre-existing mess, but can you move the '|' to the previous line?  And align the
code?

> +	u32 code_seg_attr = 9 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK
> +		| SVM_SELECTOR_L_MASK | SVM_SELECTOR_G_MASK;

| on the previous line.

	u32 data_seg_attr = 3 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK |
			    SVM_SELECTOR_DB_MASK | SVM_SELECTOR_G_MASK;
	u32 code_seg_attr = 9 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK |
			    SVM_SELECTOR_L_MASK | SVM_SELECTOR_G_MASK;