Re: [kvm-unit-tests PATCH v3 7/8] x86: VMX: Make guest_state_test_main() check state from nested VM

[Krish Sadhukhan <krish.sadhukhan@oracle.com>]

On 09/03/2019 02:58 PM, Oliver Upton wrote:
> The current tests for guest state do not yet check the validity of
> loaded state from within the nested VM. Introduce the
> load_state_test_data struct to share data with the nested VM.
>
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---
>   x86/vmx_tests.c | 23 ++++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index f035f24a771a..b72a27583793 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -5017,13 +5017,28 @@ static void test_entry_msr_load(void)
>   	test_vmx_valid_controls(false);
>   }
>   
> +static struct load_state_test_data {
> +	u32 msr;
> +	u64 exp;
> +	bool enabled;
> +} load_state_test_data;

A better name is probably 'loaded_state_test_data'  as you are checking 
the validity of the loaded MSR in the guest.

> +
>   static void guest_state_test_main(void)
>   {
> +	u64 obs;
> +	struct load_state_test_data *data = &load_state_test_data;
> +
>   	while (1) {
> -		if (vmx_get_test_stage() != 2)
> -			vmcall();
> -		else
> +		if (vmx_get_test_stage() == 2)
>   			break;
> +
> +		if (data->enabled) {
> +			obs = rdmsr(obs);

Although you fixed it in the next patch, why not use  'data->msr' in 
place of 'obs' as the parameter to rdmsr() in this patch only ?

> +			report("Guest state is 0x%lx (expected 0x%lx)",
> +			       data->exp == obs, obs, data->exp);
> +		}
> +
> +		vmcall();
>   	}
>   
>   	asm volatile("fnop");
> @@ -6854,7 +6869,9 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
>   	u64 i, val;
>   	u32 j;
>   	int error;
> +	struct load_state_test_data *data = &load_state_test_data;
>   
> +	data->enabled = false;
>   	vmcs_clear_bits(ctrl_field, ctrl_bit);
>   	if (field == GUEST_PAT) {
>   		vmx_set_test_stage(1);