Re: [patch] KVM: add MSR based hypercall API

Avi Kivity
2007-01-09T09:58:48Z

Ingo Molnar wrote:
> Subject: [patch] KVM: add MSR based hypercall API
> From: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
>
> this adds a special MSR based hypercall API to KVM. This is to be used
> by paravirtual kernels and virtual drivers.
>
> VMX-only at the moment.
>
> Signed-off-by: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
> ---
> drivers/kvm/kvm.h | 5 +++
> drivers/kvm/mmu.c | 1
> drivers/kvm/vmx.c | 74 +++++++++++++++++++++++++++++++++++++++++++++--
> include/linux/kvm_para.h | 64 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 142 insertions(+), 2 deletions(-)
>
> Index: linux/drivers/kvm/kvm.h
> ===================================================================
> --- linux.orig/drivers/kvm/kvm.h
> +++ linux/drivers/kvm/kvm.h
> @@ -14,6 +14,7 @@
>
> #include "vmx.h"
> #include <linux/kvm.h>
> +#include <linux/kvm_para.h>
>
> #define CR0_PE_MASK (1ULL << 0)
> #define CR0_TS_MASK (1ULL << 3)
> @@ -237,6 +238,8 @@ struct kvm_vcpu {
> unsigned long cr0;
> unsigned long cr2;
> unsigned long cr3;
> + struct kvm_vcpu_para_state *para_state;
>
Do we want this as part of kvm_vcpu or kvm? I can see arguments for
both views.

> + hpa_t vm_syscall_hpa;
>

This should be a gpa so it can be migrated, and so we can support guest
paging. Should also be named hypercall to avoid confusion with the
syscall protocol.

> Index: linux/drivers/kvm/vmx.c
> ===================================================================
> --- linux.orig/drivers/kvm/vmx.c
> +++ linux/drivers/kvm/vmx.c
> @@ -406,10 +406,15 @@ static int vmx_set_msr(struct kvm_vcpu *
> case MSR_IA32_SYSENTER_ESP:
> vmcs_write32(GUEST_SYSENTER_ESP, data);
> break;
> - case MSR_IA32_TIME_STAMP_COUNTER: {
> + case MSR_IA32_TIME_STAMP_COUNTER:
> guest_write_tsc(data);
> break;
> - }
> + /*
> + * This is the 'probe whether the host is KVM' logic:
> + */
> + case MSR_KVM_API_MAGIC:
> + return vcpu_register_para(vcpu, data);
> +
>

Why not move this to kvm_set_msr_common()? That will get svm support
for free.

> default:
> msr = find_msr_entry(vcpu, msr_index);
> if (msr) {
> @@ -1448,6 +1453,71 @@ static int handle_io(struct kvm_vcpu *vc
> return 0;
> }
>
> +/*
> + * Register the para guest with the host:
> + */
> +int vcpu_register_para(struct kvm_vcpu *vcpu, gpa_t para_state_gpa)
> +{
> + struct kvm_vcpu_para_state *para_state;
> + hpa_t para_state_hpa, vm_syscall_hpa;
> + unsigned char *vm_syscall;
> +
> + printk("KVM: guest trying to enter paravirtual mode\n");
> + printk(".... para_state_gpa: %08Lx\n", para_state_gpa);
> +
> + /*
> + * Needs to be page aligned:
> + */
> + if (para_state_gpa != PAGE_ALIGN(para_state_gpa))
> + goto err_gp;
> +
> + para_state_hpa = gpa_to_hpa(vcpu, para_state_gpa);
> + printk(".... para_state_hpa: %08Lx\n", para_state_hpa);
> + if (is_error_hpa(para_state_hpa))
> + goto err_gp;
> +
> + para_state = (void *)__va(para_state_hpa);
> + printk(".... para_state_hva: %p\n", para_state);
> +
> + printk(".... guest version: %d\n", para_state->guest_version);
> + printk(".... size: %d\n", para_state->size);
> +
> + para_state->host_version = KVM_PARA_API_VERSION;
> + /*
> + * We cannot support guests that try to register themselves
> + * with a newer API version than the host supports:
> + */
> + if (para_state->guest_version > KVM_PARA_API_VERSION) {
> + para_state->ret = -EINVAL;
>

EINVAL may be different or nonexistent on the guest. We need to define
kvm-specific error codes.

> + goto err_skip;
> + }
> +
> + vm_syscall_hpa = gpa_to_hpa(vcpu, para_state->vm_syscall_addr);
> + printk(".... vm_syscall_hpa: %08Lx\n", vm_syscall_hpa);
> + if (is_error_hpa(vm_syscall_hpa)) {
> + para_state->ret = -EINVAL;
> + goto err_skip;
> + }
> +
> + printk("KVM: para guest successfully registered.\n");
> + vcpu->para_state = para_state;
> + vcpu->vm_syscall_hpa = vm_syscall_hpa;
> +
> + vm_syscall = __va(vm_syscall_hpa);
>

kmap() is needed here (guest pages are not GFP_KERNEL).

> + /*
> + * Patch in the VMCALL instruction:
> + */
> + vm_syscall[0] = 0x0f;
> + vm_syscall[1] = 0x01;
> + vm_syscall[2] = 0xc1;
>

This should be an arch op, and the rest of the function moved to kvm_main.c.

> +
> + para_state->ret = 0;
> +err_skip:
> + return 0;
> +err_gp:
> + return 1;
> +}
> +
> static int handle_cr(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> {
> u64 exit_qualification;
> Index: linux/include/linux/kvm_para.h
> ===================================================================
> --- /dev/null
> +++ linux/include/linux/kvm_para.h
> @@ -0,0 +1,64 @@
> +#ifndef __LINUX_KVM_PARA_H
> +#define __LINUX_KVM_PARA_H
> +
> +/*
> + * Guest OS interface for KVM paravirtualization
> + *
> + * Note: this interface is considered experimental and may change without
> + * notice.
> + */
> +
> +#define KVM_CR3_CACHE_SIZE 4
> +
> +struct kvm_cr3_cache_entry {
> + u64 guest_cr3;
> + u64 host_cr3;
> +};
> +
> +struct kvm_cr3_cache {
> + struct kvm_cr3_cache_entry entry[KVM_CR3_CACHE_SIZE];
> + u32 max_idx;
> +};
>

This will require an api version bump whenever KVM_CR3_CACHE_SIZE changes.

Better to advertise the gpa of the cache, so it can be unlimited.

> +
> +/*
> + * Per-VCPU descriptor area shared between guest and host. Writable to
> + * both guest and host. Registered with the host by the guest when
> + * a guest acknowledges paravirtual mode.
> + */
> +struct kvm_vcpu_para_state {
> + /*
> + * API version information for compatibility. If there's any support
> + * mismatch (too old host trying to execute too new guest) then
> + * the host will deny entry into paravirtual mode. Any other
> + * combination (new host + old guest and new host + new guest)
> + * is supposed to work - new host versions will support all old
> + * guest API versions.
> + */
> + u32 guest_version;
> + u32 host_version;
> + u32 size;
> + u32 ret;
> +
> + /*
> + * The address of the vm exit instruction (VMCALL or VMMCALL),
> + * which the host will patch according to the CPU model the
> + * VM runs on:
> + */
> + u64 vm_syscall_addr;
>

Please rename to hypercall, and make it explicit that it is not a
virtual address.

> +
> + struct kvm_cr3_cache cr3_cache;
> +
> +} __attribute__ ((aligned(PAGE_SIZE)));
>

Perhaps packed too, to avoid 32/64 ambiguity. Or even better, pad it
explicitly to avoid unaligned fields.

> +
> +#define KVM_PARA_API_VERSION 1
> +
> +/*
> + * This is used for an RDMSR's ECX parameter to probe for a KVM host.
> + * Hopefully no CPU vendor will use up this number. This is placed well
> + * out of way of the typical space occupied by CPU vendors' MSR indices,
> + * and we think (or at least hope) it wont be occupied in the future
> + * either.
> + */
> +#define MSR_KVM_API_MAGIC 0x87655678
> +
> +#endif
>

--
error compiling committee.c: too many arguments to function