All of
 help / color / mirror / Atom feed
From: Paolo Bonzini <>
To: Sean Christopherson <>
Subject: Re: [PATCH v2 4/8] KVM: SVM: retrieve VMCB from assembly
Date: Wed, 9 Nov 2022 10:09:50 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 11/9/22 01:53, Sean Christopherson wrote:
> On Tue, Nov 08, 2022, Paolo Bonzini wrote:
>> This is needed in order to keep the number of arguments to 3 or less,
>> after adding hsave_pa and spec_ctrl_intercepted.  32-bit builds only
>> support passing three arguments in registers, fortunately all other
>> data is reachable from the vcpu_svm struct.
> Is it actually a problem if parameters are passed on the stack?  The assembly
> code mostly creates a stack frame, i.e. %ebp can be used to pull values off the
> stack.

It's not, but given how little love 32-bit KVM receives, I prefer to 
stick to the subset of the ABI that is "equivalent" to 64-bit.

> no one cares about 32-bit and I highly doubt a few extra PUSH+POP
> instructions will  be noticeable.

Same reasoning (no one cares about 32-bits), different conclusions...

>> What fields are actually used is (like with any other function)
>> "potentially all, you'll have to read the source code and in fact you
>> can just read asm-offsets.c instead".  What I mean is, I cannot offhand
>> see or remember what fields are touched by svm_prepare_switch_to_guest,
>> why would __svm_vcpu_run be any different?
> It's different because if it were a normal C function, it would simply take
> @vcpu, and maybe @spec_ctrl_intercepted to shave cycles after CLGI.

Not just for that, but especially to avoid making 
msr_write_intercepted() noinstr.

> But because
> it's assembly and doesn't have to_svm() readily available (among other restrictions),
> __svm_vcpu_run() ends up taking a mishmash of parameters, which for me makes it
> rather difficult to understand what to expect.

Yeah, there could be three reasons to have parameters in assembly:

* you just need them (@svm)

* it's too much of a pain to compute it in assembly 
(@spec_ctrl_intercepted, @hsave_pa)

* it needs to be computed outside the clgi/stgi region (not happening 
here, only mentioned for completeness)

As this patch shows, @vmcb is not much of a pain to compute in assembly: 
it is just two instructions, and not passing it in simplifies register 
allocation (the weird push/pop goes away) because all the arguments 
except @svm/_ASM_ARG1 are needed only after vmexit.

> Oooh, and after much staring I realized that the address of the host save area
> is passed in because grabbing it after VM-Exit can't work.  That's subtle, and
> passing it in isn't strictly necessary; there's no reason the assembly code can't
> grab it and stash it on the stack.

Right, in fact that's not the reason why it's passed in---it's just to 
avoid coding page_to_pfn() in assembly, and to limit the differences 
between the regular and SEV-ES cases.  But using a per-CPU variable is 
fine (either in addition to the struct page, which "wastes" 8 bytes per 
CPU, or as a replacement).

> What about killing a few birds with one stone?  Move the host save area PA to
> its own per-CPU variable, and then grab that from assembly as well.

I would still place it in struct svm_cpu_data itself, I'll see how it 
looks and possibly post v3.


  reply	other threads:[~2022-11-09  9:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-08 15:15 [PATCH v2 0/8] KVM: SVM: fixes for vmentry code Paolo Bonzini
2022-11-08 15:15 ` [PATCH v2 1/8] KVM: x86: use a separate asm-offsets.c file Paolo Bonzini
2022-11-08 15:15 ` [PATCH v2 2/8] KVM: SVM: replace regs argument of __svm_vcpu_run with vcpu_svm Paolo Bonzini
2022-11-08 20:54   ` Sean Christopherson
2022-11-09 10:35     ` Paolo Bonzini
2022-11-08 15:15 ` [PATCH v2 3/8] KVM: SVM: adjust register allocation for __svm_vcpu_run Paolo Bonzini
2022-11-08 20:55   ` Sean Christopherson
2022-11-08 15:15 ` [PATCH v2 4/8] KVM: SVM: retrieve VMCB from assembly Paolo Bonzini
2022-11-09  0:53   ` Sean Christopherson
2022-11-09  9:09     ` Paolo Bonzini [this message]
2022-11-08 15:15 ` [PATCH v2 5/8] KVM: SVM: move guest vmsave/vmload to assembly Paolo Bonzini
2022-11-08 15:15 ` [PATCH v2 6/8] KVM: SVM: restore host save area from assembly Paolo Bonzini
2022-11-08 15:15 ` [PATCH v2 7/8] KVM: SVM: move MSR_IA32_SPEC_CTRL save/restore to assembly Paolo Bonzini
2022-11-09  1:14   ` Sean Christopherson
2022-11-09  9:29     ` Paolo Bonzini
2022-11-08 15:15 ` [PATCH v2 8/8] x86, KVM: remove unnecessary argument to x86_virt_spec_ctrl and callers Paolo Bonzini
2022-11-08 19:43 ` [PATCH v2 0/8] KVM: SVM: fixes for vmentry code Nathan Chancellor

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \

* 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.