On Mon, Nov 26, 2018 at 03:42:15PM -0800, Jim Mattson wrote:
> Reads of unbacked addresses from the PCI bus should return all
> ones. Since kvm's VMCS revision identifier isn't 0xffffffff, this
> means that a VMPTRLD with an argument outside of backed memory should
> set the VM-instruction error number to 11 (VMPTRLD with incorrect VMCS
> revision identifier) if there is already a valid VMCS pointer loaded.
>
> make_vmcs_current() has been modified to return all of the arithmetic
> flags, so that the caller can distinguish VMfailInvalid from
> VMfailValid.
>
> Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> Reviewed-by: Peter Shier <pshier@xxxxxxxxxx>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Thank you!
> ---
> x86/vmx.c | 17 ++++++++++++++---
> x86/vmx.h | 10 ++++++----
> 2 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 79a21d8..1318548 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -31,6 +31,7 @@
> #include "libcflat.h"
> #include "processor.h"
> #include "alloc_page.h"
> +#include "fwcfg.h"
> #include "vm.h"
> #include "desc.h"
> #include "vmx.h"
> @@ -1374,23 +1375,33 @@ static void test_vmptrld(void)
> /* Unaligned page access */
> tmp_root = (struct vmcs *)((intptr_t)vmcs + 1);
> report("test vmptrld with unaligned vmcs",
> - make_vmcs_current(tmp_root) == 1);
> + make_vmcs_current(tmp_root) == VM_FAIL_INVALID);
>
> /* gpa bits beyond physical address width are set*/
> tmp_root = (struct vmcs *)((intptr_t)vmcs |
> ((u64)1 << (width+1)));
> report("test vmptrld with vmcs address bits set beyond physical address width",
> - make_vmcs_current(tmp_root) == 1);
> + make_vmcs_current(tmp_root) == VM_FAIL_INVALID);
>
> /* Pass VMXON region */
> make_vmcs_current(vmcs);
> tmp_root = (struct vmcs *)vmxon_region;
> report("test vmptrld with vmxon region",
> - make_vmcs_current(tmp_root) == 1);
> + make_vmcs_current(tmp_root) == VM_FAIL_VALID);
> report("test vmptrld with vmxon region vm-instruction error",
> vmcs_read(VMX_INST_ERROR) == VMXERR_VMPTRLD_VMXON_POINTER);
>
> + tmp_root = (struct vmcs *)(fwcfg_get_u64(FW_CFG_RAM_SIZE));
> + if ((uintptr_t)tmp_root < (1ul << width)) {
> + report("test vmptrld of unbacked physical address",
> + make_vmcs_current(tmp_root) == VM_FAIL_VALID);
> + report("test vmptrld of unbacked physical address vm-instruction error",
> + vmcs_read(VMX_INST_ERROR) ==
> + VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID);
> + }
> +
> report("test vmptrld with valid vmcs region", make_vmcs_current(vmcs) == 0);
> +
> }
>
> static void test_vmptrst(void)
> diff --git a/x86/vmx.h b/x86/vmx.h
> index ba47455..6e3620b 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -7,6 +7,9 @@
> #include "asm/page.h"
> #include "asm/io.h"
>
> +#define VM_FAIL_INVALID X86_EFLAGS_CF
> +#define VM_FAIL_VALID X86_EFLAGS_ZF
> +
> struct vmcs_hdr {
> u32 revision_id:31;
> u32 shadow_vmcs:1;
> @@ -680,12 +683,11 @@ static int vmx_off(void)
>
> static inline int make_vmcs_current(struct vmcs *vmcs)
> {
> - bool ret;
> u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
>
> - asm volatile ("push %1; popf; vmptrld %2; setbe %0"
> - : "=q" (ret) : "q" (rflags), "m" (vmcs) : "cc");
> - return ret;
> + asm volatile ("push %1; popf; vmptrld %2; pushf; pop %0"
> + : "=r" (rflags) : "0" (rflags), "m" (vmcs) : "cc");
> + return rflags & VMX_ENTRY_FLAGS;
> }
>
> static inline int vmcs_clear(struct vmcs *vmcs)
> --
> 2.20.0.rc0.387.gc7a69e6b6c-goog
>
