diff mbox series

[v2,1/1] KVM: selftests: Adjust VM's initial stack address to align with SysV ABI spec

Message ID 20230227180601.104318-1-ackerleytng@google.com
State New
Headers
Series [v2,1/1] KVM: selftests: Adjust VM's initial stack address to align with SysV ABI spec |

Commit Message

Ackerley Tng Feb. 27, 2023, 6:06 p.m. UTC 
  Align the guest stack to match calling sequence requirements in
section "The Stack Frame" of the System V ABI AMD64 Architecture
Processor Supplement, which requires the value (%rsp + 8), NOT %rsp,
to be a multiple of 16 when control is transferred to the function
entry point. I.e. in a normal function call, %rsp needs to be 16-byte
aligned _before_ CALL, not after.

This fixes unexpected #GPs in guest code when the compiler uses SSE
instructions, e.g. to initialize memory, as many SSE instructions
require memory operands (including those on the stack) to be
16-byte-aligned.

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---

This patch is a follow-up from discussions at
https://lore.kernel.org/lkml/20230121001542.2472357-9-ackerleytng@google.com/

v1 -> v2: Cleaned the patch up after getting comments from Sean in

v1: https://lore.kernel.org/lkml/Y%2FfHLdvKHlK6D%2F1v@google.com/

Please also see
https://lore.kernel.org/lkml/20230227174654.94641-1-ackerleytng@google.com/
regarding providing alignment macros for selftests.

---
 .../selftests/kvm/lib/x86_64/processor.c       | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

--
2.39.2.722.g9855ee24e9-goog

Comments

Sean Christopherson March 24, 2023, 8:58 p.m. UTC | #1 
On Mon, 27 Feb 2023 18:06:01 +0000, Ackerley Tng wrote:
> Align the guest stack to match calling sequence requirements in
> section "The Stack Frame" of the System V ABI AMD64 Architecture
> Processor Supplement, which requires the value (%rsp + 8), NOT %rsp,
> to be a multiple of 16 when control is transferred to the function
> entry point. I.e. in a normal function call, %rsp needs to be 16-byte
> aligned _before_ CALL, not after.
> 
> [...]

Applied to kvm-x86 selftests, thanks!

[1/1] KVM: selftests: Adjust VM's initial stack address to align with SysV ABI spec
      https://github.com/kvm-x86/linux/commit/1982754bd2a7

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes
Sean Christopherson March 24, 2023, 9:27 p.m. UTC | #2 
On Fri, Mar 24, 2023, Sean Christopherson wrote:
> On Mon, 27 Feb 2023 18:06:01 +0000, Ackerley Tng wrote:
> > Align the guest stack to match calling sequence requirements in
> > section "The Stack Frame" of the System V ABI AMD64 Architecture
> > Processor Supplement, which requires the value (%rsp + 8), NOT %rsp,
> > to be a multiple of 16 when control is transferred to the function
> > entry point. I.e. in a normal function call, %rsp needs to be 16-byte
> > aligned _before_ CALL, not after.
> > 
> > [...]
> 
> Applied to kvm-x86 selftests, thanks!
> 
> [1/1] KVM: selftests: Adjust VM's initial stack address to align with SysV ABI spec
>       https://github.com/kvm-x86/linux/commit/1982754bd2a7

Force pushed to selftests 'cause I had a goof, this now:

        KVM: selftests: Adjust VM's initial stack address to align with SysV ABI spec
        https://github.com/kvm-x86/linux/commit/8264e85560e5
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index ae1e573d94ce..a0669d31bb85 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -5,6 +5,7 @@ 
  * Copyright (C) 2018, Google LLC.
  */

+#include "linux/bitmap.h"
 #include "test_util.h"
 #include "kvm_util.h"
 #include "processor.h"
@@ -573,6 +574,21 @@  struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id,
 				       DEFAULT_GUEST_STACK_VADDR_MIN,
 				       MEM_REGION_DATA);

+	stack_vaddr += DEFAULT_STACK_PGS * getpagesize();
+
+	/*
+	 * Align stack to match calling sequence requirements in section "The
+	 * Stack Frame" of the System V ABI AMD64 Architecture Processor
+	 * Supplement, which requires the value (%rsp + 8) to be a multiple of
+	 * 16 when control is transferred to the function entry point.
+	 *
+	 * If this code is ever used to launch a vCPU with 32-bit entry point it
+	 * may need to subtract 4 bytes instead of 8 bytes.
+	 */
+	TEST_ASSERT(IS_ALIGNED(stack_vaddr, PAGE_SIZE),
+		    "__vm_vaddr_alloc() did not provide a page-aligned address");
+	stack_vaddr -= 8;
+
 	vcpu = __vm_vcpu_add(vm, vcpu_id);
 	vcpu_init_cpuid(vcpu, kvm_get_supported_cpuid());
 	vcpu_setup(vm, vcpu);
@@ -580,7 +596,7 @@  struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id,
 	/* Setup guest general purpose registers */
 	vcpu_regs_get(vcpu, &regs);
 	regs.rflags = regs.rflags | 0x2;
-	regs.rsp = stack_vaddr + (DEFAULT_STACK_PGS * getpagesize());
+	regs.rsp = stack_vaddr;
 	regs.rip = (unsigned long) guest_code;
 	vcpu_regs_set(vcpu, &regs);