linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ackerley Tng <ackerleytng@google.com>
To: Sean Christopherson <seanjc@google.com>
Cc: erdemaktas@google.com, mail@maciej.szmigiero.name,
	linux-kselftest@vger.kernel.org, pbonzini@redhat.com,
	isaku.yamahata@intel.com, sagis@google.com, afranji@google.com,
	runanwang@google.com, shuah@kernel.org, drjones@redhat.com,
	maz@kernel.org, bgardon@google.com, jmattson@google.com,
	dmatlack@google.com, peterx@redhat.com, oupton@google.com,
	ricarkol@google.com, yang.zhong@intel.com, wei.w.wang@intel.com,
	xiaoyao.li@intel.com, pgonda@google.com, marcorr@google.com,
	eesposit@redhat.com, borntraeger@de.ibm.com,
	eric.auger@redhat.com, wangyanan55@huawei.com,
	aaronlewis@google.com, vkuznets@redhat.com, pshier@google.com,
	axelrasmussen@google.com, zhenzhong.duan@intel.com,
	like.xu@linux.intel.com, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org
Subject: Re: [RFC PATCH v3 08/31] KVM: selftests: Require GCC to realign stacks on function entry
Date: Fri, 17 Feb 2023 18:57:54 +0000	[thread overview]
Message-ID: <diqzy1owjf0t.fsf@ackerleytng-cloudtop.c.googlers.com> (raw)
In-Reply-To: <Y+1bom6CMFeNGWmm@google.com> (message from Sean Christopherson on Wed, 15 Feb 2023 14:24:34 -0800)


> > I figured it out!
> >
> > GCC assumes that the stack is 16-byte aligned **before** the call
> > instruction. Since call pushes rip to the stack, GCC will compile code
> > assuming that on entrance to the function, the stack is -8 from a
> > 16-byte aligned address.
> >
> > Since for TDs we do a ljmp to guest code, providing a function's
> > address, the stack was not modified by a call instruction pushing rip to
> > the stack, so the stack is 16-byte aligned when the guest code starts
> > running, instead of 16-byte aligned -8 that GCC expects.
> >
> > For VMs, we set rip to a function pointer, and the VM starts running
> > with a 16-byte algined stack too.
> >
> > To fix this, I propose that in vm_arch_vcpu_add(), we align the
> > allocated stack address and then subtract 8 from that:
> >
> > @@ -573,10 +573,13 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm  
> *vm,
> > uint32_t vcpu_id,
> >         vcpu_init_cpuid(vcpu, kvm_get_supported_cpuid());
> >         vcpu_setup(vm, vcpu);
> >
> > +       stack_vaddr += (DEFAULT_STACK_PGS * getpagesize());
> > +       stack_vaddr = ALIGN_DOWN(stack_vaddr, 16) - 8;

> The ALIGN_DOWN should be unnecessary, we've got larger issues if  
> getpagesize() isn't
> 16-byte aligned and/or if __vm_vaddr_alloc() returns anything but a  
> page-aligned
> address.  Maybe add a TEST_ASSERT() sanity check that stack_vaddr is  
> page-aligned
> at this point?

> And in addition to the comment suggested by Maciej, can you also add a  
> comment
> explaining the -8 adjust?  Yeah, someone can go read the changelog, but I  
> think
> this is worth explicitly documenting in code.

> Lastly, can you post it as a standalone patch?

> Many thanks!

Thanks Maciej and Sean, I've made the changes you requested and posted
it as a standalone patch at
https://lore.kernel.org/lkml/32866e5d00174697730d6231d2fb81f6b8d98c8a.1676659352.git.ackerleytng@google.com/

  reply	other threads:[~2023-02-17 18:58 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-21  0:15 [RFC PATCH v3 00/31] TDX KVM selftests Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 01/31] KVM: selftests: Add function to allow one-to-one GVA to GPA mappings Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 02/31] KVM: selftests: Add support for creating non-default type VMs Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 03/31] KVM: selftests: Expose function that sets up sregs based on VM's mode Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 04/31] KVM: selftests: Store initial stack address in struct kvm_vcpu Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 05/31] KVM: selftests: Refactor steps in vCPU descriptor table initialization Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 06/31] KVM: selftests: Add helper functions to create TDX VMs Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 07/31] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 08/31] KVM: selftests: Require GCC to realign stacks on function entry Ackerley Tng
2023-01-21  0:27   ` Sean Christopherson
2023-01-23 18:30     ` Erdem Aktas
2023-01-23 18:50       ` Maciej S. Szmigiero
2023-01-23 18:53         ` Sean Christopherson
2023-01-24  0:04           ` Erdem Aktas
2023-01-24  1:21             ` Sean Christopherson
2023-02-15  0:50               ` Ackerley Tng
2023-02-15 18:44                 ` Maciej S. Szmigiero
2023-02-15 22:19                   ` Sean Christopherson
2023-02-15 22:24                 ` Sean Christopherson
2023-02-17 18:57                   ` Ackerley Tng [this message]
2023-01-21  0:15 ` [RFC PATCH v3 09/31] KVM: selftests: TDX: Add TDX lifecycle test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 10/31] KVM: selftests: TDX: Add report_fatal_error test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 11/31] KVM: selftests: TDX: Adding test case for TDX port IO Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 12/31] KVM: selftests: TDX: Add basic TDX CPUID test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 13/31] KVM: selftests: TDX: Add basic get_td_vmcall_info test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 14/31] KVM: selftests: TDX: Add TDX IO writes test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 15/31] KVM: selftests: TDX: Add TDX IO reads test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 16/31] KVM: selftests: TDX: Add TDX MSR read/write tests Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 17/31] KVM: selftests: TDX: Add TDX HLT exit test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 18/31] KVM: selftests: TDX: Add TDX MMIO reads test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 19/31] KVM: selftests: TDX: Add TDX MMIO writes test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 20/31] KVM: selftests: TDX: Add TDX CPUID TDVMCALL test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 21/31] KVM: selftests: TDX: Verify the behavior when host consumes a TD private memory Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 22/31] KVM: selftests: TDX: Add TDG.VP.INFO test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 23/31] KVM: selftests: Add functions to allow mapping as shared Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 24/31] KVM: selftests: TDX: Add shared memory test Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 25/31] KVM: selftests: Add support for restricted memory Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 26/31] KVM: selftests: TDX: Update load_td_memory_region for VM memory backed by restricted memfd Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 27/31] KVM: selftests: Expose _vm_vaddr_alloc Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 28/31] KVM: selftests: TDX: Add support for TDG.MEM.PAGE.ACCEPT Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 29/31] KVM: selftests: TDX: Add support for TDG.VP.VEINFO.GET Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 30/31] KVM: selftests: TDX: Add TDX UPM selftest Ackerley Tng
2023-01-21  0:15 ` [RFC PATCH v3 31/31] KVM: selftests: TDX: Add TDX UPM selftests for implicit conversion Ackerley Tng

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=diqzy1owjf0t.fsf@ackerleytng-cloudtop.c.googlers.com \
    --to=ackerleytng@google.com \
    --cc=aaronlewis@google.com \
    --cc=afranji@google.com \
    --cc=axelrasmussen@google.com \
    --cc=bgardon@google.com \
    --cc=borntraeger@de.ibm.com \
    --cc=dmatlack@google.com \
    --cc=drjones@redhat.com \
    --cc=eesposit@redhat.com \
    --cc=erdemaktas@google.com \
    --cc=eric.auger@redhat.com \
    --cc=isaku.yamahata@intel.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=like.xu@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mail@maciej.szmigiero.name \
    --cc=marcorr@google.com \
    --cc=maz@kernel.org \
    --cc=oupton@google.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=pgonda@google.com \
    --cc=pshier@google.com \
    --cc=ricarkol@google.com \
    --cc=runanwang@google.com \
    --cc=sagis@google.com \
    --cc=seanjc@google.com \
    --cc=shuah@kernel.org \
    --cc=vkuznets@redhat.com \
    --cc=wangyanan55@huawei.com \
    --cc=wei.w.wang@intel.com \
    --cc=xiaoyao.li@intel.com \
    --cc=yang.zhong@intel.com \
    --cc=zhenzhong.duan@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 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).