All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Marc Zyngier <maz@kernel.org>,
	 Oliver Upton <oliver.upton@linux.dev>,
	Christian Borntraeger <borntraeger@linux.ibm.com>,
	 Janosch Frank <frankja@linux.ibm.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org,
	"Sean Christopherson" <seanjc@google.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Aaron Lewis" <aaronlewis@google.com>
Subject: [PATCH v4 03/34] KVM: selftests: Add a shameful hack to preserve/clobber GPRs across ucall
Date: Fri, 28 Jul 2023 17:36:12 -0700	[thread overview]
Message-ID: <20230729003643.1053367-4-seanjc@google.com> (raw)
In-Reply-To: <20230729003643.1053367-1-seanjc@google.com>

Preserve or clobber all GPRs (except RIP and RSP, as they're saved and
restored via the VMCS) when performing a ucall on x86 to fudge around a
horrific long-standing bug in selftests' nested VMX support where L2's
GPRs are not preserved across a nested VM-Exit.  I.e. if a test triggers a
nested VM-Exit to L1 in response to a ucall, e.g. GUEST_SYNC(), then L2's
GPR state can be corrupted.

The issues manifests as an unexpected #GP in clear_bit() when running the
hyperv_evmcs test due to RBX being used to track the ucall object, and RBX
being clobbered by the nested VM-Exit.  The problematic hyperv_evmcs
testcase is where L0 (test's host userspace) injects an NMI in response to
GUEST_SYNC(8) from L2, but the bug could "randomly" manifest in any test
that induces a nested VM-Exit from L0.  The bug hasn't caused failures in
the past due to sheer dumb luck.

The obvious fix is to rework the nVMX helpers to save/restore L2 GPRs
across VM-Exit and VM-Enter, but that is a much bigger task and carries
its own risks, e.g. nSVM does save/restore GPRs, but not in a thread-safe
manner, and there is a _lot_ of cleanup that can be done to unify code
for doing VM-Enter on nVMX, nSVM, and eVMCS.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../testing/selftests/kvm/lib/x86_64/ucall.c  | 32 +++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/x86_64/ucall.c b/tools/testing/selftests/kvm/lib/x86_64/ucall.c
index 4d41dc63cc9e..a53df3ece2f8 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/ucall.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/ucall.c
@@ -14,8 +14,36 @@ void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
 
 void ucall_arch_do_ucall(vm_vaddr_t uc)
 {
-	asm volatile("in %[port], %%al"
-		: : [port] "d" (UCALL_PIO_PORT), "D" (uc) : "rax", "memory");
+	/*
+	 * FIXME: Revert this hack (the entire commit that added it) once nVMX
+	 * preserves L2 GPRs across a nested VM-Exit.  If a ucall from L2, e.g.
+	 * to do a GUEST_SYNC(), lands the vCPU in L1, any and all GPRs can be
+	 * clobbered by L1.  Save and restore non-volatile GPRs (clobbering RBP
+	 * in particular is problematic) along with RDX and RDI (which are
+	 * inputs), and clobber volatile GPRs. *sigh*
+	 */
+#define HORRIFIC_L2_UCALL_CLOBBER_HACK	\
+	"rcx", "rsi", "r8", "r9", "r10", "r11"
+
+	asm volatile("push %%rbp\n\t"
+		     "push %%r15\n\t"
+		     "push %%r14\n\t"
+		     "push %%r13\n\t"
+		     "push %%r12\n\t"
+		     "push %%rbx\n\t"
+		     "push %%rdx\n\t"
+		     "push %%rdi\n\t"
+		     "in %[port], %%al\n\t"
+		     "pop %%rdi\n\t"
+		     "pop %%rdx\n\t"
+		     "pop %%rbx\n\t"
+		     "pop %%r12\n\t"
+		     "pop %%r13\n\t"
+		     "pop %%r14\n\t"
+		     "pop %%r15\n\t"
+		     "pop %%rbp\n\t"
+		: : [port] "d" (UCALL_PIO_PORT), "D" (uc) : "rax", "memory",
+		     HORRIFIC_L2_UCALL_CLOBBER_HACK);
 }
 
 void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu)
-- 
2.41.0.487.g6d72f3e995-goog


WARNING: multiple messages have this Message-ID (diff)
From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Marc Zyngier <maz@kernel.org>,
	 Oliver Upton <oliver.upton@linux.dev>,
	Christian Borntraeger <borntraeger@linux.ibm.com>,
	 Janosch Frank <frankja@linux.ibm.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org,
	"Sean Christopherson" <seanjc@google.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Aaron Lewis" <aaronlewis@google.com>
Subject: [PATCH v4 03/34] KVM: selftests: Add a shameful hack to preserve/clobber GPRs across ucall
Date: Fri, 28 Jul 2023 17:36:12 -0700	[thread overview]
Message-ID: <20230729003643.1053367-4-seanjc@google.com> (raw)
In-Reply-To: <20230729003643.1053367-1-seanjc@google.com>

Preserve or clobber all GPRs (except RIP and RSP, as they're saved and
restored via the VMCS) when performing a ucall on x86 to fudge around a
horrific long-standing bug in selftests' nested VMX support where L2's
GPRs are not preserved across a nested VM-Exit.  I.e. if a test triggers a
nested VM-Exit to L1 in response to a ucall, e.g. GUEST_SYNC(), then L2's
GPR state can be corrupted.

The issues manifests as an unexpected #GP in clear_bit() when running the
hyperv_evmcs test due to RBX being used to track the ucall object, and RBX
being clobbered by the nested VM-Exit.  The problematic hyperv_evmcs
testcase is where L0 (test's host userspace) injects an NMI in response to
GUEST_SYNC(8) from L2, but the bug could "randomly" manifest in any test
that induces a nested VM-Exit from L0.  The bug hasn't caused failures in
the past due to sheer dumb luck.

The obvious fix is to rework the nVMX helpers to save/restore L2 GPRs
across VM-Exit and VM-Enter, but that is a much bigger task and carries
its own risks, e.g. nSVM does save/restore GPRs, but not in a thread-safe
manner, and there is a _lot_ of cleanup that can be done to unify code
for doing VM-Enter on nVMX, nSVM, and eVMCS.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../testing/selftests/kvm/lib/x86_64/ucall.c  | 32 +++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/x86_64/ucall.c b/tools/testing/selftests/kvm/lib/x86_64/ucall.c
index 4d41dc63cc9e..a53df3ece2f8 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/ucall.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/ucall.c
@@ -14,8 +14,36 @@ void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
 
 void ucall_arch_do_ucall(vm_vaddr_t uc)
 {
-	asm volatile("in %[port], %%al"
-		: : [port] "d" (UCALL_PIO_PORT), "D" (uc) : "rax", "memory");
+	/*
+	 * FIXME: Revert this hack (the entire commit that added it) once nVMX
+	 * preserves L2 GPRs across a nested VM-Exit.  If a ucall from L2, e.g.
+	 * to do a GUEST_SYNC(), lands the vCPU in L1, any and all GPRs can be
+	 * clobbered by L1.  Save and restore non-volatile GPRs (clobbering RBP
+	 * in particular is problematic) along with RDX and RDI (which are
+	 * inputs), and clobber volatile GPRs. *sigh*
+	 */
+#define HORRIFIC_L2_UCALL_CLOBBER_HACK	\
+	"rcx", "rsi", "r8", "r9", "r10", "r11"
+
+	asm volatile("push %%rbp\n\t"
+		     "push %%r15\n\t"
+		     "push %%r14\n\t"
+		     "push %%r13\n\t"
+		     "push %%r12\n\t"
+		     "push %%rbx\n\t"
+		     "push %%rdx\n\t"
+		     "push %%rdi\n\t"
+		     "in %[port], %%al\n\t"
+		     "pop %%rdi\n\t"
+		     "pop %%rdx\n\t"
+		     "pop %%rbx\n\t"
+		     "pop %%r12\n\t"
+		     "pop %%r13\n\t"
+		     "pop %%r14\n\t"
+		     "pop %%r15\n\t"
+		     "pop %%rbp\n\t"
+		: : [port] "d" (UCALL_PIO_PORT), "D" (uc) : "rax", "memory",
+		     HORRIFIC_L2_UCALL_CLOBBER_HACK);
 }
 
 void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu)
-- 
2.41.0.487.g6d72f3e995-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2023-07-29  0:36 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-29  0:36 [PATCH v4 00/34] KVM: selftests: Guest printf and asserts overhaul Sean Christopherson
2023-07-29  0:36 ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 01/34] KVM: selftests: Rename the ASSERT_EQ macro Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 02/34] KVM: selftests: Make TEST_ASSERT_EQ() output look like normal TEST_ASSERT() Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` Sean Christopherson [this message]
2023-07-29  0:36   ` [PATCH v4 03/34] KVM: selftests: Add a shameful hack to preserve/clobber GPRs across ucall Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 04/34] KVM: selftests: Add strnlen() to the string overrides Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 05/34] KVM: selftests: Add guest_snprintf() to KVM selftests Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 06/34] KVM: selftests: Add additional pages to the guest to accommodate ucall Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 07/34] KVM: selftests: Add string formatting options to ucall Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 08/34] KVM: selftests: Add formatted guest assert support in ucall framework Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 09/34] KVM: selftests: Add a selftest for guest prints and formatted asserts Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-31 16:32   ` Andrew Jones
2023-07-31 16:32     ` Andrew Jones
2023-07-31 17:04     ` Sean Christopherson
2023-07-31 17:04       ` Sean Christopherson
2023-07-31 17:19       ` Sean Christopherson
2023-07-31 17:19         ` Sean Christopherson
2023-07-31 18:22         ` Andrew Jones
2023-07-31 18:22           ` Andrew Jones
2023-07-29  0:36 ` [PATCH v4 10/34] KVM: selftests: Convert aarch_timer to printf style GUEST_ASSERT Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 11/34] KVM: selftests: Convert debug-exceptions " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 12/34] KVM: selftests: Convert ARM's hypercalls test " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 13/34] KVM: selftests: Convert ARM's page fault " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 14/34] KVM: selftests: Convert ARM's vGIC IRQ " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 15/34] KVM: selftests: Convert the memslot performance test to printf guest asserts Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 16/34] KVM: selftests: Convert s390's memop test to printf style GUEST_ASSERT Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 17/34] KVM: selftests: Convert s390's tprot " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 18/34] KVM: selftests: Convert set_memory_region_test to printf-based GUEST_ASSERT Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 19/34] KVM: selftests: Convert steal_time test to printf style GUEST_ASSERT Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 20/34] KVM: selftests: Convert x86's CPUID " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 21/34] KVM: selftests: Convert the Hyper-V extended hypercalls test to printf asserts Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 22/34] KVM: selftests: Convert the Hyper-V feature test to printf style GUEST_ASSERT Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 23/34] KVM: selftests: Convert x86's KVM paravirt " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 24/34] KVM: selftests: Convert the MONITOR/MWAIT test to use printf guest asserts Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 25/34] KVM: selftests: Convert x86's nested exceptions test to " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 26/34] KVM: selftests: Convert x86's set BSP ID test to printf style " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 27/34] KVM: selftests: Convert the nSVM software interrupt test to printf " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 28/34] KVM: selftests: Convert x86's TSC MSRs test to use " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 29/34] KVM: selftests: Convert the x86 userspace I/O test to printf guest assert Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 30/34] KVM: selftests: Convert VMX's PMU capabilities test to printf guest asserts Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 31/34] KVM: selftests: Convert x86's XCR0 test to use printf-based " Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 32/34] KVM: selftests: Rip out old, param-based guest assert macros Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 33/34] KVM: selftests: Print out guest RIP on unhandled exception Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-07-29  0:36 ` [PATCH v4 34/34] KVM: selftests: Use GUEST_FAIL() in ARM's arch timer helpers Sean Christopherson
2023-07-29  0:36   ` Sean Christopherson
2023-08-02 22:01 ` [PATCH v4 00/34] KVM: selftests: Guest printf and asserts overhaul Sean Christopherson
2023-08-02 22:01   ` Sean Christopherson

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=20230729003643.1053367-4-seanjc@google.com \
    --to=seanjc@google.com \
    --cc=aaronlewis@google.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=frankja@linux.ibm.com \
    --cc=imbrenda@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=oliver.upton@linux.dev \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=thuth@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.