On Fri, Aug 19, 2022, Vishal Annapurve wrote:
> Add support for executing vmmcall/vmcall instruction on amd/intel cpus.
> In general kvm patches the instruction according to the cpu
> implementation at runtime. While executing selftest vms from private
> memory KVM will not be able to update the private memory of the guest.
>
> Hypercall parameters are fixed to explicitly populate hypercall number
> in eax. Otherwise inlined function calls to kvm_hypercall would call
> vmmcall/vmcall instruction without updating eax with hypercall number.
Can you send a seperate non-RFC series to clean up the selftests mess? kvm_hypercall()
isn't the only culprit.
git grep \"vmcall | wc -l
16
I'm pretty sure things work only because of KVM's dubious behavior of patching
VMCALL/VMMCALL by default.
Note, svm_vmcall_test.c intentionally uses the wrong instructions and shouldn't
be converted. Actually, we can and should just delete that test, it's superseded
by the wonderfully named fix_hypercall_test.
> Signed-off-by: Vishal Annapurve <vannapurve@xxxxxxxxxx>
> ---
> .../testing/selftests/kvm/lib/x86_64/processor.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> index 53b115876417..09d757a0b148 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> @@ -1254,10 +1254,21 @@ uint64_t kvm_hypercall(uint64_t nr, uint64_t a0, uint64_t a1, uint64_t a2,
> uint64_t a3)
> {
> uint64_t r;
> + static bool is_cpu_checked;
> + static bool is_cpu_amd;
>
> - asm volatile("vmcall"
> + if (!is_cpu_checked)
> + is_cpu_amd = is_amd_cpu();
This can be done using a single int, e.g.
static bool is_cpu_amd = -1;
if (is_cpu_amd < 0)
is_cpu_amd = is_amd_cpu();
Although... what if we declare main() in lib/kvm_util.c (or maybe a dedicated
file?) and rename all tests to use __main()? Then add an arch hook to do global
initialization and avoid the "did we check CPUID?!?!?" altogether.
That would allow us to dedup all of the hilarious copy paste:
/* Tell stdout not to buffer its content */
setbuf(stdout, NULL);
and we could turn is_amd_cpu() and is_intel_cpu() into bools.
E.g.
int main(int argc, char *argv[])
{
/* Tell stdout not to buffer its content */
setbuf(stdout, NULL);
kvm_arch_main();
return __main(argc, argv);
}
void kvm_arch_main(void)
{
is_cpu_amd = cpu_vendor_string_is("AuthenticAMD");
is_cpu_intel = cpu_vendor_string_is("AuthenticAMD");
}
And then we just need macro magic to emit the right VMCALL/VMMCALL instruction.
