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

[Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>]

* Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote:

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

definitely needs to be a property of the vcpu. For example the cr3 cache 
is attached to the physical CPU. SMP scalability necessiates this too - 
we want to use the para_state to pass information between the guest and 
the host without any hypercall.

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

ok, done. I have also added a para_state_gpa to the vcpu, to save that 
gpa value as well.

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

done. I have also created a patch_hypercall CPU arch method, which is 
called from the generic code - and this enabled me to move the 
registration code to kvm_main.c as well.

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

ok, done.

> >+	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).

done.

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

the gpa of the cache, and its guest-side size, right?

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

done.

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

it should already be padded - i layed it out that way. (if it's not then 
let me know where it's not padded)

updated patch below. (This doesnt yet have the cr3 cache size change.)

	Ingo


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/kvm_main.c   |   74 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/kvm/mmu.c        |    1 
 drivers/kvm/vmx.c        |   16 ++++++++--
 include/linux/kvm_para.h |   72 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 166 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,9 @@ struct kvm_vcpu {
 	unsigned long cr0;
 	unsigned long cr2;
 	unsigned long cr3;
+	struct kvm_vcpu_para_state *para_state;
+	gpa_t para_state_gpa;
+	gpa_t hypercall_gpa;
 	unsigned long cr4;
 	unsigned long cr8;
 	u64 pdptrs[4]; /* pae */
@@ -379,6 +383,7 @@ struct kvm_arch_ops {
 	int (*run)(struct kvm_vcpu *vcpu, struct kvm_run *run);
 	int (*vcpu_setup)(struct kvm_vcpu *vcpu);
 	void (*skip_emulated_instruction)(struct kvm_vcpu *vcpu);
+	void (*patch_hypercall)(unsigned char *hypercall_addr);
 };
 
 extern struct kvm_stat kvm_stat;
Index: linux/drivers/kvm/kvm_main.c
===================================================================
--- linux.orig/drivers/kvm/kvm_main.c
+++ linux/drivers/kvm/kvm_main.c
@@ -1204,6 +1204,74 @@ void realmode_set_cr(struct kvm_vcpu *vc
 	}
 }
 
+
+/*
+ * Register the para guest with the host:
+ */
+static 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, hypercall_hpa;
+	gpa_t hypercall_gpa;
+
+	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 = -KVM_EINVAL;
+		goto err_skip;
+	}
+
+	hypercall_gpa = para_state->hypercall_addr;
+	hypercall_hpa = gpa_to_hpa(vcpu, hypercall_gpa);
+	printk(".... hypercall_hpa: %08Lx\n", hypercall_hpa);
+	if (is_error_hpa(hypercall_hpa)) {
+		para_state->ret = -KVM_EINVAL;
+		goto err_skip;
+	}
+
+	printk("KVM: para guest successfully registered.\n");
+	vcpu->para_state = para_state;
+	vcpu->para_state_gpa = para_state_gpa;
+	vcpu->hypercall_gpa = hypercall_gpa;
+
+	if (kvm_arch_ops->patch_hypercall) {
+		unsigned char *hypercall;
+		hypercall = kmap_atomic(pfn_to_page(hypercall_hpa>>PAGE_SHIFT),
+					KM_USER0);
+		kvm_arch_ops->patch_hypercall(hypercall);
+		kunmap_atomic(hypercall, KM_USER0);
+	}
+
+	para_state->ret = 0;
+err_skip:
+	return 0;
+err_gp:
+	return 1;
+}
+
 int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 {
 	u64 data;
@@ -1240,6 +1308,12 @@ int kvm_get_msr_common(struct kvm_vcpu *
 		data = vcpu->shadow_efer;
 		break;
 #endif
+	/*
+	 * This is the 'probe whether the host is KVM' logic:
+	 */
+	case MSR_KVM_API_MAGIC:
+		return vcpu_register_para(vcpu, *pdata);
+
 	default:
 		printk(KERN_ERR "kvm: unhandled rdmsr: 0x%x\n", msr);
 		return 1;
Index: linux/drivers/kvm/mmu.c
===================================================================
--- linux.orig/drivers/kvm/mmu.c
+++ linux/drivers/kvm/mmu.c
@@ -719,6 +719,7 @@ hpa_t gpa_to_hpa(struct kvm_vcpu *vcpu, 
 	return ((hpa_t)page_to_pfn(page) << PAGE_SHIFT)
 		| (gpa & (PAGE_SIZE-1));
 }
+EXPORT_SYMBOL_GPL(gpa_to_hpa);
 
 hpa_t gva_to_hpa(struct kvm_vcpu *vcpu, gva_t gva)
 {
Index: linux/drivers/kvm/vmx.c
===================================================================
--- linux.orig/drivers/kvm/vmx.c
+++ linux/drivers/kvm/vmx.c
@@ -406,10 +406,10 @@ 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;
-	}
+
 	default:
 		msr = find_msr_entry(vcpu, msr_index);
 		if (msr) {
@@ -1448,6 +1448,17 @@ static int handle_io(struct kvm_vcpu *vc
 	return 0;
 }
 
+static void
+vmx_patch_hypercall(struct kvm_vcpu *vcpu, unsigned char *hypercall)
+{
+	/*
+	 * Patch in the VMCALL instruction:
+	 */
+	hypercall[0] = 0x0f;
+	hypercall[1] = 0x01;
+	hypercall[2] = 0xc1;
+}
+
 static int handle_cr(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	u64 exit_qualification;
@@ -2042,6 +2053,7 @@ static struct kvm_arch_ops vmx_arch_ops 
 	.run = vmx_vcpu_run,
 	.skip_emulated_instruction = skip_emulated_instruction,
 	.vcpu_setup = vmx_vcpu_setup,
+	.patch_hypercall = vmx_patch_hypercall,
 };
 
 static int __init vmx_init(void)
Index: linux/include/linux/kvm_para.h
===================================================================
--- /dev/null
+++ linux/include/linux/kvm_para.h
@@ -0,0 +1,72 @@
+#ifndef __LINUX_KVM_PARA_H
+#define __LINUX_KVM_PARA_H
+
+#include <linux/errno.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 {
+	u32 max_idx;
+	u32 __pad;
+	struct kvm_cr3_cache_entry entry[KVM_CR3_CACHE_SIZE];
+};
+
+/*
+ * 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.
+ *
+ * NOTE: all addresses are guest-physical addresses (gpa), to make it
+ * easier for the hypervisor to map between the various addresses.
+ */
+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 hypercall_addr;
+
+	struct kvm_cr3_cache cr3_cache;
+
+} __attribute__ ((aligned(PAGE_SIZE)));
+
+#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
+
+#define KVM_EINVAL EINVAL
+
+#endif

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