On Fri, Oct 12, 2018 at 04:27:55PM +0100, Sergey Dyasli wrote:
> Calling vmfail_valid() is correct only if vvmcx is valid. Modify
> functions to use vmfail() instead which performs the necessary check.
>
> Signed-off-by: Sergey Dyasli <sergey.dya...@citrix.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
I think the code is a bit convoluted because what fields get access is
hidden behind layers of macros and functions. In order to catch future
issues, may I suggest you add some assertions to vmfail_{in,}valid?
Like
ASSERT(!cpu_has_vmx_vmcs_shadowing && vvmcx_invalid(v));
in vmfail_invalid. Something similar can be added in vmfail_valid as
well.
> ---
> xen/arch/x86/hvm/vmx/vvmx.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 8eee6d0ea8..26b7d72660 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1744,7 +1744,7 @@ int nvmx_handle_vmptrld(struct cpu_user_regs *regs)
> !map_io_bitmap_all(v) ||
> !_map_msr_bitmap(v) )
> {
> - vmfail_valid(regs, VMX_INSN_VMPTRLD_INVALID_PHYADDR);
> + vmfail(regs, VMX_INSN_VMPTRLD_INVALID_PHYADDR);
> goto out;
> }
> }
> @@ -1828,7 +1828,7 @@ int nvmx_handle_vmclear(struct cpu_user_regs *regs)
> if ( rc == VMSUCCEED )
> vmsucceed(regs);
> else if ( rc == VMFAIL_VALID )
> - vmfail_valid(regs, VMX_INSN_VMCLEAR_INVALID_PHYADDR);
> + vmfail(regs, VMX_INSN_VMCLEAR_INVALID_PHYADDR);
> else
> vmfail_invalid(regs);
>
> --
> 2.17.1
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
