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

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

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


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV