Re: [PATCH v2 4/8] KVM: SVM: retrieve VMCB from assembly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Paolo




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux