All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Matlack <dmatlack@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>,
	David Matlack <dmatlack@google.com>, Peter Xu <peterx@redhat.com>,
	Jim Mattson <jmattson@google.com>,
	Yang Zhong <yang.zhong@intel.com>,
	Wei Wang <wei.w.wang@intel.com>,
	Colton Lewis <coltonlewis@google.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Vipin Sharma <vipinsh@google.com>,
	Aaron Lewis <aaronlewis@google.com>,
	kvm@vger.kernel.org
Subject: [PATCH v3 07/10] KVM: selftests: Avoid JMP in non-faulting path of KVM_ASM_SAFE()
Date: Mon, 31 Oct 2022 11:00:42 -0700	[thread overview]
Message-ID: <20221031180045.3581757-8-dmatlack@google.com> (raw)
In-Reply-To: <20221031180045.3581757-1-dmatlack@google.com>

From: Sean Christopherson <seanjc@google.com>

Clear R9 in the non-faulting path of KVM_ASM_SAFE() and fall through to
to a common load of "vector" to effectively load "vector" with '0' to
reduce the code footprint of the asm blob, to reduce the runtime overhead
of the non-faulting path (when "vector" is stored in a register), and so
that additional output constraints that are valid if and only if a fault
occur are loaded even in the non-faulting case.

A future patch will add a 64-bit output for the error code, and if its
output is not explicitly loaded with _something_, the user of the asm
blob can end up technically consuming uninitialized data.  Using a
common path to load the output constraints will allow using an existing
scratch register, e.g. r10, to hold the error code in the faulting path,
while also guaranteeing the error code is initialized with deterministic
data in the non-faulting patch (r10 is loaded with the RIP of
to-be-executed instruction).

Consuming the error code when a fault doesn't occur would obviously be a
test bug, but there's no guarantee the compiler will detect uninitialized
consumption.  And conversely, it's theoretically possible that the
compiler might throw a false positive on uninitialized data, e.g. if the
compiler can't determine that the non-faulting path won't touch the error
code.

Alternatively, the error code could be explicitly loaded in the
non-faulting path, but loading a 64-bit memory|register output operand
with an explicitl value requires a sign-extended "MOV imm32, r/m64",
which isn't exactly straightforward and has a largish code footprint.
And loading the error code with what is effectively garbage (from a
scratch register) avoids having to choose an arbitrary value for the
non-faulting case.

Opportunistically remove a rogue asterisk in the block comment.

Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 tools/testing/selftests/kvm/include/x86_64/processor.h | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index f7249cb27e0d..9efe80d52389 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
@@ -764,7 +764,7 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
  * for recursive faults when accessing memory in the handler.  The downside to
  * using registers is that it restricts what registers can be used by the actual
  * instruction.  But, selftests are 64-bit only, making register* pressure a
- * minor concern.  Use r9-r11 as they are volatile, i.e. don't need* to be saved
+ * minor concern.  Use r9-r11 as they are volatile, i.e. don't need to be saved
  * by the callee, and except for r11 are not implicit parameters to any
  * instructions.  Ideally, fixup would use r8-r10 and thus avoid implicit
  * parameters entirely, but Hyper-V's hypercall ABI uses r8 and testing Hyper-V
@@ -786,11 +786,9 @@ void vm_install_exception_handler(struct kvm_vm *vm, int vector,
 	"lea 1f(%%rip), %%r10\n\t"				\
 	"lea 2f(%%rip), %%r11\n\t"				\
 	"1: " insn "\n\t"					\
-	"movb $0, %[vector]\n\t"				\
-	"jmp 3f\n\t"						\
+	"xor %%r9, %%r9\n\t"					\
 	"2:\n\t"						\
-	"mov  %%r9b, %[vector]\n\t"				\
-	"3:\n\t"
+	"mov  %%r9b, %[vector]\n\t"
 
 #define KVM_ASM_SAFE_OUTPUTS(v)	[vector] "=qm"(v)
 #define KVM_ASM_SAFE_CLOBBERS	"r9", "r10", "r11"
-- 
2.38.1.273.g43a17bfeac-goog


  parent reply	other threads:[~2022-10-31 18:01 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-31 18:00 [PATCH v3 00/10] KVM: selftests: Fix and clean up emulator_error_test David Matlack
2022-10-31 18:00 ` [PATCH v3 01/10] KVM: selftests: Rename emulator_error_test to smaller_maxphyaddr_emulation_test David Matlack
2022-10-31 18:00 ` [PATCH v3 02/10] KVM: selftests: Explicitly require instructions bytes David Matlack
2022-10-31 18:19   ` Sean Christopherson
2022-10-31 18:00 ` [PATCH v3 03/10] KVM: selftests: Delete dead ucall code David Matlack
2022-10-31 18:19   ` Sean Christopherson
2022-10-31 18:00 ` [PATCH v3 04/10] KVM: selftests: Move flds instruction emulation failure handling to header David Matlack
2022-10-31 18:28   ` Sean Christopherson
2022-11-02 18:17     ` David Matlack
2022-11-02 19:03       ` Sean Christopherson
2022-11-02 22:02         ` David Matlack
2022-10-31 18:00 ` [PATCH v3 05/10] KVM: x86/mmu: Use BIT{,_ULL}() for PFERR masks David Matlack
2022-10-31 18:00 ` [PATCH v3 06/10] KVM: selftests: Copy KVM PFERR masks into selftests David Matlack
2022-10-31 18:28   ` Sean Christopherson
2022-10-31 18:00 ` David Matlack [this message]
2022-10-31 18:00 ` [PATCH v3 08/10] KVM: selftests: Provide error code as a KVM_ASM_SAFE() output David Matlack
2022-10-31 18:00 ` [PATCH v3 09/10] KVM: selftests: Expect #PF(RSVD) when TDP is disabled David Matlack
2022-10-31 18:07   ` Sean Christopherson
2022-10-31 18:00 ` [PATCH v3 10/10] KVM: selftests: Add a test for KVM_CAP_EXIT_ON_EMULATION_FAILURE David Matlack
2022-10-31 18:37   ` 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=20221031180045.3581757-8-dmatlack@google.com \
    --to=dmatlack@google.com \
    --cc=aaronlewis@google.com \
    --cc=coltonlewis@google.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=seanjc@google.com \
    --cc=vipinsh@google.com \
    --cc=vkuznets@redhat.com \
    --cc=wei.w.wang@intel.com \
    --cc=yang.zhong@intel.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.