From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Cc: kvm@vger.kernel.org, rkrcmar@redhat.com, pbonzini@redhat.com,
jmattson@google.com
Subject: Re: [PATCH 3/4] kvm-unit-test: nVMX: __enter_guest() should not set "launched" state when VM-entry fails
Date: Wed, 4 Sep 2019 08:42:31 -0700 [thread overview]
Message-ID: <20190904154231.GB24079@linux.intel.com> (raw)
In-Reply-To: <20190829205635.20189-4-krish.sadhukhan@oracle.com>
On Thu, Aug 29, 2019 at 04:56:34PM -0400, Krish Sadhukhan wrote:
> Bit# 31 in VM-exit reason is set by hardware in both cases of early VM-entry
> failures and VM-entry failures due to invalid guest state.
This is incorrect, VMCS.EXIT_REASON is not written on a VM-Fail. If the
tests are passing, you're probably consuming a stale EXIT_REASON.
> Whenever VM-entry
> fails, the nested VMCS is not in "launched" state any more. Hence,
> __enter_guest() should not set the "launched" state when a VM-entry fails.
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
> ---
> x86/vmx.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 872ba11..183d11b 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -1805,6 +1805,8 @@ static void check_for_guest_termination(void)
> */
> static void __enter_guest(u8 abort_flag, struct vmentry_failure *failure)
> {
> + bool vm_entry_failure;
> +
> TEST_ASSERT_MSG(v2_guest_main,
> "Never called test_set_guest_func!");
>
> @@ -1812,15 +1814,14 @@ static void __enter_guest(u8 abort_flag, struct vmentry_failure *failure)
> "Called enter_guest() after guest returned.");
>
> vmx_enter_guest(failure);
> + vm_entry_failure = vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE;
Rather than duplicating the code in vmx_run(), what if we move this check
into vmx_enter_guest() and rework struct vmentry_failure? The code was
originally designed to handle only VM-Fail conditions, we should clean it
up instead of bolting more stuff on top. E.g.:
struct vmentry_status {
/* Did we attempt VMLAUNCH or VMRESUME */
bool vmlaunch;
/* Instruction mnemonic (for convenience). */
const char *instr;
/* VM-Enter passed all consistency checks, i.e. did not fail. */
bool succeeded;
/* VM-Enter failed before loading guest state, i.e. VM-Fail. */
bool vm_fail;
/* Contents of RFLAGS on VM-Fail, EXIT_REASON on VM-Exit. */
union {
unsigned long vm_fail_flags;
unsigned long vm_exit_reason;
};
};
static void vmx_enter_guest(struct vmentry_status *status)
{
status->vm_fail = 0;
in_guest = 1;
asm volatile (
"mov %[HOST_RSP], %%rdi\n\t"
"vmwrite %%rsp, %%rdi\n\t"
LOAD_GPR_C
"cmpb $0, %[launched]\n\t"
"jne 1f\n\t"
"vmlaunch\n\t"
"jmp 2f\n\t"
"1: "
"vmresume\n\t"
"2: "
SAVE_GPR_C
"pushf\n\t"
"pop %%rdi\n\t"
"mov %%rdi, %[vm_fail_flags]\n\t"
"movl $1, %[vm_fail]\n\t"
"jmp 3f\n\t"
"vmx_return:\n\t"
SAVE_GPR_C
"3: \n\t"
: [vm_fail]"+m"(status->vm_fail),
[vm_fail_flags]"=m"(status->vm_fail_flags)
: [launched]"m"(launched), [HOST_RSP]"i"(HOST_RSP)
: "rdi", "memory", "cc"
);
in_guest = 0;
if (!status->vm_fail)
status->vm_exit_reason = vmcs_read(EXI_REASON);
status->succeeded = !status->vm_fail &&
!(status->vm_exit_reason & VMX_ENTRY_FAILURE);
status->vmlaunch = !launched;
status->instr = launched ? "vmresume" : "vmlaunch";
if (status->succeeded)
launched = 1;
}
> if ((abort_flag & ABORT_ON_EARLY_VMENTRY_FAIL && failure->early) ||
> - (abort_flag & ABORT_ON_INVALID_GUEST_STATE &&
> - vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE)) {
> -
> + (abort_flag & ABORT_ON_INVALID_GUEST_STATE && vm_entry_failure)) {
> print_vmentry_failure_info(failure);
> abort();
> }
>
> - if (!failure->early) {
> + if (!vm_entry_failure) {
> launched = 1;
> check_for_guest_termination();
> }
> --
> 2.20.1
>
next prev parent reply other threads:[~2019-09-04 15:42 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-29 20:56 [PATCH 0/4] KVM: nVMX: Check GUEST_DEBUGCTL and GUEST_DR7 on vmentry of nested guests Krish Sadhukhan
2019-08-29 20:56 ` [PATCH 1/4] KVM: nVMX: Check GUEST_DEBUGCTL " Krish Sadhukhan
2019-08-29 22:12 ` Jim Mattson
2019-08-30 23:26 ` Krish Sadhukhan
2019-09-01 23:55 ` Jim Mattson
2019-08-29 20:56 ` [PATCH 2/4] KVM: nVMX: Check GUEST_DR7 " Krish Sadhukhan
2019-08-29 22:26 ` Jim Mattson
2019-08-30 23:07 ` Krish Sadhukhan
2019-08-30 23:15 ` Jim Mattson
2019-09-02 0:33 ` Jim Mattson
[not found] ` <e229bea2-acb2-e268-6281-d8e467c3282e@oracle.com>
2019-09-04 16:44 ` Jim Mattson
2019-09-04 16:58 ` Sean Christopherson
2019-09-04 18:05 ` Krish Sadhukhan
2019-09-04 18:20 ` Jim Mattson
2019-09-09 4:11 ` Krish Sadhukhan
2019-09-09 15:56 ` Jim Mattson
2019-09-04 17:14 ` Sean Christopherson
2019-12-20 23:45 ` Jim Mattson
2019-12-21 0:27 ` Jim Mattson
2019-08-29 20:56 ` [PATCH 3/4] kvm-unit-test: nVMX: __enter_guest() should not set "launched" state when VM-entry fails Krish Sadhukhan
2019-09-04 15:42 ` Sean Christopherson [this message]
2019-09-13 20:37 ` Krish Sadhukhan
2019-09-13 21:06 ` Sean Christopherson
2019-09-16 19:12 ` Krish Sadhukhan
2019-08-29 20:56 ` [PATCH 4/4] kvm-unit-test: nVMX: Check GUEST_DEBUGCTL and GUEST_DR7 on vmentry of nested guests Krish Sadhukhan
2019-08-29 23:17 ` Jim Mattson
2019-08-30 1:12 ` Nadav Amit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190904154231.GB24079@linux.intel.com \
--to=sean.j.christopherson@intel.com \
--cc=jmattson@google.com \
--cc=krish.sadhukhan@oracle.com \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=rkrcmar@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).