Re: [RFC] selftests: kvm: add a selftest for SMM

[Paolo Bonzini <pbonzini@redhat.com>]

On 10/04/19 11:38, Vitaly Kuznetsov wrote:
> Early RFC, based on state_test.
> 
> Add a simplistic test for SMM. Currently it fails with
> "Unexpected result from KVM_SET_NESTED_STATE" even when a patch fixing
> rsm after VMXON is added. There's likey some other issue in nested
> save/restore when SMM os on.
> 
> The test implements its own sync between the guest and the host as using
> our ucall library seems to be too cumbersome: SMI handler is happening in
> real-address mode.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  tools/testing/selftests/kvm/Makefile          |   1 +
>  .../selftests/kvm/include/x86_64/processor.h  |  27 +++
>  tools/testing/selftests/kvm/x86_64/smm_test.c | 159 ++++++++++++++++++
>  3 files changed, 187 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/x86_64/smm_test.c
> 
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 3c1f4bdf9000..257955593cdf 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -17,6 +17,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/state_test
>  TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
>  TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid
>  TEST_GEN_PROGS_x86_64 += x86_64/vmx_close_while_nested_test
> +TEST_GEN_PROGS_x86_64 += x86_64/smm_test
>  TEST_GEN_PROGS_x86_64 += dirty_log_test
>  TEST_GEN_PROGS_x86_64 += clear_dirty_log_test
>  
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index e2884c2b81ff..6063d5b2f356 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -778,6 +778,33 @@ void vcpu_set_msr(struct kvm_vm *vm, uint32_t vcpuid, uint64_t msr_index,
>  #define MSR_IA32_APICBASE_ENABLE	(1<<11)
>  #define MSR_IA32_APICBASE_BASE		(0xfffff<<12)
>  
> +#define APIC_BASE_MSR	0x800
> +#define X2APIC_ENABLE	(1UL << 10)
> +#define	APIC_ICR	0x300
> +#define		APIC_DEST_SELF		0x40000
> +#define		APIC_DEST_ALLINC	0x80000
> +#define		APIC_DEST_ALLBUT	0xC0000
> +#define		APIC_ICR_RR_MASK	0x30000
> +#define		APIC_ICR_RR_INVALID	0x00000
> +#define		APIC_ICR_RR_INPROG	0x10000
> +#define		APIC_ICR_RR_VALID	0x20000
> +#define		APIC_INT_LEVELTRIG	0x08000
> +#define		APIC_INT_ASSERT		0x04000
> +#define		APIC_ICR_BUSY		0x01000
> +#define		APIC_DEST_LOGICAL	0x00800
> +#define		APIC_DEST_PHYSICAL	0x00000
> +#define		APIC_DM_FIXED		0x00000
> +#define		APIC_DM_FIXED_MASK	0x00700
> +#define		APIC_DM_LOWEST		0x00100
> +#define		APIC_DM_SMI		0x00200
> +#define		APIC_DM_REMRD		0x00300
> +#define		APIC_DM_NMI		0x00400
> +#define		APIC_DM_INIT		0x00500
> +#define		APIC_DM_STARTUP		0x00600
> +#define		APIC_DM_EXTINT		0x00700
> +#define		APIC_VECTOR_MASK	0x000FF
> +#define	APIC_ICR2	0x310
> +
>  #define MSR_IA32_TSCDEADLINE		0x000006e0
>  
>  #define MSR_IA32_UCODE_WRITE		0x00000079
> diff --git a/tools/testing/selftests/kvm/x86_64/smm_test.c b/tools/testing/selftests/kvm/x86_64/smm_test.c
> new file mode 100644
> index 000000000000..3452b592588a
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/smm_test.c
> @@ -0,0 +1,159 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018, Red Hat, Inc.
> + *
> + * Tests for SMM.
> + */
> +#define _GNU_SOURCE /* for program_invocation_short_name */
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +
> +#include "test_util.h"
> +
> +#include "kvm_util.h"
> +
> +#include "vmx.h"
> +
> +#define VCPU_ID	      1
> +
> +#define PAGE_SIZE  4096
> +
> +#define SMRAM_SIZE (16 * PAGE_SIZE)
> +#define SMRAM_GPA 0x1000000
> +#define SMRAM_STAGE 0xfe
> +
> +#define STR(x) #x
> +#define XSTR(s) STR(s)
> +
> +#define SYNC_PORT 0xe
> +#define DONE 0xff
> +
> +/*
> + * This is compiled as normal 64-bit code, however, SMI handler is executed
> + * in real-address mode. To stay simple we're limiting ourselves to a mode
> + * independent subset of asm here.
> + * SMI handler always report back fixed stage SMRAM_STAGE.
> + */
> +void smi_handler(void)
> +{
> +	asm volatile("mov $" XSTR(SMRAM_STAGE) ", %al \n"
> +		     "in $" XSTR(SYNC_PORT) ", %al \n"
> +		     "rsm \n");
> +}
> +/* This should change if the code above changes in length */
> +#define SMI_HANDLER_SIZE 6

Here I had similar code but I just wrote hex instead. :)

> +
> +void sync_with_host(uint64_t phase)
> +{
> +	asm volatile("in $" XSTR(SYNC_PORT)", %%al \n"
> +		     : : "a" (phase));
> +}
> +
> +void self_smi(void)
> +{
> +	wrmsr(APIC_BASE_MSR + (APIC_ICR >> 4),
> +	      APIC_DEST_SELF | APIC_INT_ASSERT | APIC_DM_SMI);
> +}
> +
> +void guest_code(struct vmx_pages *vmx_pages)
> +{
> +	uint64_t apicbase = rdmsr(MSR_IA32_APICBASE);
> +
> +	sync_with_host(1);
> +
> +	wrmsr(MSR_IA32_APICBASE, apicbase | X2APIC_ENABLE);
> +
> +	sync_with_host(2);
> +
> +	self_smi();
> +
> +	sync_with_host(4);
> +
> +	if (vmx_pages) {
> +		GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
> +
> +		sync_with_host(5);
> +
> +		self_smi();
> +
> +		sync_with_host(7);
> +	}
> +
> +	sync_with_host(DONE);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	struct vmx_pages *vmx_pages = NULL;
> +	vm_vaddr_t vmx_pages_gva = 0;
> +
> +	struct kvm_regs regs;
> +	struct kvm_vm *vm;
> +	struct kvm_run *run;
> +	struct kvm_x86_state *state;
> +	int stage, stage_reported;
> +
> +	/* Create VM */
> +	vm = vm_create_default(VCPU_ID, 0, guest_code);
> +
> +	vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
> +
> +	run = vcpu_state(vm, VCPU_ID);
> +
> +	vcpu_regs_get(vm, VCPU_ID, &regs);

Unused?

> +
> +	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, SMRAM_GPA,
> +				    1 << 16 | 1, SMRAM_SIZE/PAGE_SIZE, 0);

Better add a vm_phy_pages_alloc here.

I'll debug the remaining failure and post it again.

Paolo

> +	memset(addr_gpa2hva(vm, SMRAM_GPA), 0x0, SMRAM_SIZE);
> +	memcpy(addr_gpa2hva(vm, SMRAM_GPA) + 0x8000, smi_handler,
> +	       SMI_HANDLER_SIZE);
> +
> +	vcpu_set_msr(vm, VCPU_ID, MSR_IA32_SMBASE, SMRAM_GPA);
> +
> +	if (kvm_check_cap(KVM_CAP_NESTED_STATE)) {
> +		vmx_pages = vcpu_alloc_vmx(vm, &vmx_pages_gva);
> +		vcpu_args_set(vm, VCPU_ID, 1, vmx_pages_gva);
> +	} else {
> +		printf("will skip SMM test with VMX enabled\n");
> +		vcpu_args_set(vm, VCPU_ID, 1, 0);
> +	}
> +
> +	for (stage = 1;; stage++) {
> +		_vcpu_run(vm, VCPU_ID);
> +		TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
> +			    "Stage %d: unexpected exit reason: %u (%s),\n",
> +			    stage, run->exit_reason,
> +			    exit_reason_str(run->exit_reason));
> +
> +		memset(&regs, 0, sizeof(regs));
> +		vcpu_regs_get(vm, VCPU_ID, &regs);
> +
> +		stage_reported = regs.rax & 0xff;
> +
> +		if (stage_reported == DONE)
> +			goto done;
> +
> +		TEST_ASSERT(stage_reported == stage ||
> +			    stage_reported == SMRAM_STAGE,
> +			    "Unexpected stage: #%x, got %x",
> +			    stage, stage_reported);
> +
> +		/* This piece of code needs sharing with state_test/evmcs_test */
> +		state = vcpu_save_state(vm, VCPU_ID);
> +		kvm_vm_release(vm);
> +		kvm_vm_restart(vm, O_RDWR);
> +		vm_vcpu_add(vm, VCPU_ID, 0, 0);
> +		vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
> +		vcpu_load_state(vm, VCPU_ID, state);
> +		run = vcpu_state(vm, VCPU_ID);
> +		free(state);
> +	}
> +
> +done:
> +	kvm_vm_free(vm);
> +}
>