KVM: add MSR based hypercall API

PATCH In review Ingo Molnar 2 participants 19 messages Jan 9, 2007 17 messages not in the archive

Subsystems (3)

KVM KVM paravirt PARAVIRT_OPS interface

lore

Share & tools

lore

Ingo Molnar

author

2007-01-09 09:27 UTC #0 lore raw

PATCH 1/1 KVM: add MSR based hypercall API

Subject: [patch] KVM: add MSR based hypercall API
From: Ingo Molnar <[email protected]>

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 <[email protected]>

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

drivers/kvm/kvm.h +5 -0

Index: 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;
+	hpa_t vm_syscall_hpa;
 	unsigned long cr4;
 	unsigned long cr8;
 	u64 pdptrs[4]; /* pae */

@@ -626,4 +629,6 @@ static inline u32 get_rdx_init_val(void)
 #define TSS_REDIRECTION_SIZE (256 / 8)
 #define RMODE_TSS_SIZE (TSS_BASE_SIZE + TSS_REDIRECTION_SIZE + TSS_IOPB_SIZE + 1)
 
+extern int vcpu_register_para(struct kvm_vcpu *vcpu, gpa_t para_state_gpa);
+
 #endif

drivers/kvm/mmu.c +1 -0

Index: 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)
 {

drivers/kvm/vmx.c +72 -2

88 lines not on this page · raw message

include/linux/kvm_para.h new file +64 -0

64 lines not on this page · raw message

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Avi Kivity

2007-01-09 09:58 UTC #2 lore raw

Quoted 29 lines · Ingo Molnar

Subject: [patch] KVM: add MSR based hypercall API
From: Ingo Molnar <[email protected]>
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 <[email protected]>
---
 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
===================================================================
drivers/kvm/kvm.h +1 -0
@@ -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)

drivers/kvm/kvm.h struct kvm_vcpu { · line 242

Quoted 5 lines

drivers/kvm/kvm.h +1 -0

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

drivers/kvm/kvm.h struct kvm_vcpu { · line 244

Quoted 2 lines

drivers/kvm/kvm.h +1 -0
+	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.

Quoted 10 lines

Index: linux/drivers/kvm/vmx.c
===================================================================
drivers/kvm/vmx.c +1 -1
@@ -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:

drivers/kvm/vmx.c static int vmx_set_msr(struct kvm_vcpu * · line 418

Quoted 10 lines

drivers/kvm/vmx.c +6 -1

 		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.

Quoted 34 lines

drivers/kvm/vmx.c +0 -0

 	default:
 		msr = find_msr_entry(vcpu, msr_index);

drivers/kvm/vmx.c +27 -0

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

drivers/kvm/vmx.c static int handle_io(struct kvm_vcpu *vc · line 1492

Quoted 10 lines

drivers/kvm/vmx.c +9 -0

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

Quoted 6 lines

drivers/kvm/vmx.c +6 -0

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

drivers/kvm/vmx.c static int handle_io(struct kvm_vcpu *vc

Quoted 10 lines

drivers/kvm/vmx.c +9 -0

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

drivers/kvm/vmx.c static int handle_io(struct kvm_vcpu *vc

Quoted 7 lines

drivers/kvm/vmx.c +6 -0

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

Quoted 28 lines

drivers/kvm/vmx.c +8 -0
+
+	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
===================================================================
include/linux/kvm_para.h new file +11 -0
@@ -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

include/linux/kvm_para.h line 21

Quoted 10 lines

include/linux/kvm_para.h new file +10 -0

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

Quoted 17 lines

include/linux/kvm_para.h new file +16 -0

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

include/linux/kvm_para.h line 47

Quoted 10 lines

include/linux/kvm_para.h new file +10 -0

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

include/linux/kvm_para.h line 51

Quoted 5 lines

include/linux/kvm_para.h new file +4 -0

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

Quoted 14 lines

include/linux/kvm_para.h new file +13 -0

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

Signature · 8 lines

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

A message that is not in the archive (sent off-list or not yet received): 45A36758.1000808-atKUWr5tajBWk0Htik3J/[email protected]

Ingo Molnar

author

2007-01-09 10:38 UTC #4 lore raw

Quoted 8 lines · Avi Kivity

Review struct kvm_vcpu { · line 242
Quoted 6 lines
(quoted diff) +1 -0
@@ -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.

Quoted 6 lines

Review struct kvm_vcpu { · line 244
Quoted 2 lines
(quoted diff) +1 -0
+	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.

Quoted 8 lines

Review struct kvm_vcpu {
Quoted 5 lines
(quoted diff) +5 -0
+	/*
+	 * 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.

Quoted 5 lines

Review
Quoted 2 lines
(quoted diff) +2 -0
+	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.

Quoted 8 lines

Review
Quoted 6 lines
(quoted diff) +5 -0
+	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.

Quoted 10 lines

Review
Quoted 5 lines
(quoted diff) +4 -0
+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?

Quoted 22 lines

(quoted diff) +22 -0

+
+/*
+ * 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),

Quoted 8 lines

Review
Quoted 5 lines
(quoted diff) +4 -0
+	 * 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.

Quoted 8 lines

Review
Quoted 5 lines
(quoted diff) +4 -0
+
+	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 <[email protected]>

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 <[email protected]>

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

drivers/kvm/kvm.h +5 -0

Index: 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;

drivers/kvm/kvm_main.c +74 -0

86 lines not on this page · raw message

drivers/kvm/mmu.c +1 -0

Index: 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)
 {

drivers/kvm/vmx.c +14 -2

Index: 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)

include/linux/kvm_para.h new file +72 -0

72 lines not on this page · raw message

A message that is not in the archive (sent off-list or not yet received): [email protected]

Avi Kivity

2007-01-09 11:24 UTC #6 lore raw

Quoted 14 lines · Ingo Molnar

Quoted 10 lines · Avi Kivity
drivers/kvm/kvm.h struct kvm_vcpu { · line 243
Quoted 7 lines
drivers/kvm/kvm.h +1 -0
@@ -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.

Quoted 6 lines

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.

Ok, agreed.

It makes guest-side registration a bit more icky (need execute the code on all vcpus).

Quoted 9 lines

Quoted 9 lines
include/linux/kvm_para.h line 24
Quoted 6 lines
include/linux/kvm_para.h new file +4 -0
+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.

Quoted 6 lines

Quoted 2 lines
Better to advertise the gpa of the cache, so it can be unlimited.
the gpa of the cache, and its guest-side size, right?

Yes (can use max_idx, no?).

BTW, max_idx is ambiguous: is it the last valid entry or one past the end? entry_count is more explicit IMO.

Quoted 10 lines

Quoted 9 lines
include/linux/kvm_para.h line 57
Quoted 6 lines
include/linux/kvm_para.h new file +4 -0
+
+	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.

Quoted 4 lines

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

Right, I was confused by the cr3 cache, but it's the last field.

Review

Quoted 3 lines

(quoted diff) +2 -0

+
+	if (kvm_arch_ops->patch_hypercall) {

It's safe to assume that the arch op exists.

Quoted 2 lines

+EXPORT_SYMBOL_GPL(gpa_to_hpa);

Is this needed now? If so, it needs a kvm_ prefix.

Quoted 19 lines

drivers/kvm/vmx.c +11 -0

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

drivers/kvm/vmx.c static struct kvm_arch_ops vmx_arch_ops · line 2058

Quoted 6 lines

drivers/kvm/vmx.c +1 -0

@@ -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,
 };

Where is the vmcall exit handler?

Please add the svm code too. I can test it if you lack amd hardware.

Signature · 8 lines

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

A message that is not in the archive (sent off-list or not yet received): 45A37B7A.8020709-atKUWr5tajBWk0Htik3J/[email protected]

Ingo Molnar

author

2007-01-09 11:36 UTC #8 lore raw

Quoted 9 lines · Avi Kivity

Quoted 9 lines
Quoted 9 lines
Review
Quoted 6 lines
(quoted diff) +4 -0
+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.

Quoted 6 lines

Quoted 4 lines
Quoted 2 lines
Better to advertise the gpa of the cache, so it can be unlimited.
the gpa of the cache, and its guest-side size, right?
Yes (can use max_idx, no?).

agreed, done. the cr3 registration now passes in the guest-side max size of the cr3 cache, which the host-side updates (trims if necessary).

Quoted 2 lines

BTW, max_idx is ambiguous: is it the last valid entry or one past the end? entry_count is more explicit IMO.

agreed, done.

Quoted 4 lines

Quoted 2 lines
it should already be padded - i layed it out that way. (if it's not then let me know where it's not padded)
Right, I was confused by the cr3 cache, but it's the last field.

i moved it to the first place and added a padding word - that way all structure sizes are properly aligned and we cannot combine them in an unaligned way by accident.

Quoted 5 lines

Review
Quoted 3 lines
(quoted diff) +2 -0
+
+	if (kvm_arch_ops->patch_hypercall) {
It's safe to assume that the arch op exists.

ok, i changed this and i added the SVM patch function too. AMD's VMMCALL is 0x0f, 0x01, 0xd9, correct?

Quoted 3 lines

Quoted 1 line
+EXPORT_SYMBOL_GPL(gpa_to_hpa);
Is this needed now? If so, it needs a kvm_ prefix.

not needed anymore because usage is now in kvm_main.c - i removed the export.

Quoted 4 lines

Quoted 3 lines
(quoted diff) +1 -0
+	.patch_hypercall = vmx_patch_hypercall,
};
Where is the vmcall exit handler?

in my tree, have not sent the patch yet - first want to combine it with the cr3 feature to have it tested.

Quoted 1 line

Please add the svm code too. I can test it if you lack amd hardware.

done.

updated patches attached.

Ingo

Subject: [patch] KVM: add MSR based hypercall API
From: Ingo Molnar <[email protected]>

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 <[email protected]>

---
 drivers/kvm/kvm.h        |    6 +++
 drivers/kvm/kvm_main.c   |   74 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/kvm/svm.c        |   12 +++++++
 drivers/kvm/vmx.c        |   16 ++++++++--
 include/linux/kvm_para.h |   75 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 181 insertions(+), 2 deletions(-)

drivers/kvm/kvm.h +6 -0

Index: 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;
+	gpa_t para_state_gpa;
+	struct page *para_state_page;
+	gpa_t hypercall_gpa;
 	unsigned long cr4;
 	unsigned long cr8;
 	u64 pdptrs[4]; /* pae */

@@ -379,6 +383,8 @@ 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)(struct kvm_vcpu *vcpu,
+				unsigned char *hypercall_addr);
 };
 
 extern struct kvm_stat kvm_stat;

drivers/kvm/kvm_main.c +74 -0

86 lines not on this page · raw message

drivers/kvm/svm.c +12 -0

Index: linux/drivers/kvm/svm.c
===================================================================
@@ -1642,6 +1642,17 @@ static int is_disabled(void)
 	return 0;
 }
 
+static void
+svm_patch_hypercall(struct kvm_vcpu *vcpu, unsigned char *hypercall)
+{
+	/*
+	 * Patch in the VMMCALL instruction:
+	 */
+	hypercall[0] = 0x0f;
+	hypercall[1] = 0x01;
+	hypercall[2] = 0xd9;
+}
+
 static struct kvm_arch_ops svm_arch_ops = {
 	.cpu_has_kvm_support = has_svm,
 	.disabled_by_bios = is_disabled,

@@ -1689,6 +1700,7 @@ static struct kvm_arch_ops svm_arch_ops 
 	.run = svm_vcpu_run,
 	.skip_emulated_instruction = skip_emulated_instruction,
 	.vcpu_setup = svm_vcpu_setup,
+	.patch_hypercall = svm_patch_hypercall,
 };
 
 static int __init svm_init(void)

drivers/kvm/vmx.c +14 -2

Index: 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)

include/linux/kvm_para.h new file +75 -0

75 lines not on this page · raw message

Subject: [patch] KVM: paravirtual guest support
From: Ingo Molnar <[email protected]>

this enables a CONFIG_PARAVIRT Linux guest kernel to establish a hypercall API to a KVM host. If successfully registered, then the Linux guest will optimize a few things like its interrupt controller, io-delay and it also registers its cr3-cache structures with the host. (but the host will not touch those, just yet)

(this is fully backwards compatible - if the WRMSR fails then the Linux guest continues to execute as a native kernel.)

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/i386/kernel/paravirt.c  |  238 +++++++++++++++++++++++++++++++++++++++++++
 drivers/serial/8250.c        |    3 
 include/asm-i386/processor.h |    1 
 include/linux/paravirt.h     |   12 ++
 init/main.c                  |    6 -
 5 files changed, 257 insertions(+), 3 deletions(-)

arch/i386/kernel/paravirt.c +238 -0

253 lines not on this page · raw message

drivers/serial/8250.c +2 -1

Index: linux/drivers/serial/8250.c
===================================================================
@@ -27,6 +27,7 @@
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/ioport.h>
+#include <linux/paravirt.h>
 #include <linux/init.h>
 #include <linux/console.h>
 #include <linux/sysrq.h>

@@ -1371,7 +1372,7 @@ static irqreturn_t serial8250_interrupt(
 
 		l = l->next;
 
-		if (l == i->head && pass_counter++ > PASS_LIMIT) {
+		if (!paravirt_enabled() && l == i->head && pass_counter++ > PASS_LIMIT) {
 			/* If we hit this, we're dead. */
 			printk(KERN_ERR "serial8250: too much work for "
 				"irq%d\n", irq);

include/asm-i386/processor.h +0 -1

Index: linux/include/asm-i386/processor.h
===================================================================
@@ -547,7 +547,6 @@ static inline void rep_nop(void)
 #ifdef CONFIG_PARAVIRT
 #include <asm/paravirt.h>
 #else
-#define paravirt_enabled() 0
 #define __cpuid native_cpuid
 
 static inline void load_esp0(struct tss_struct *tss, struct thread_struct *thread)

include/linux/paravirt.h new file +12 -0

Index: linux/include/linux/paravirt.h
===================================================================
@@ -0,0 +1,12 @@
+#ifndef __LINUX_PARAVIRT_H
+#define __LINUX_PARAVIRT_H
+
+/*
+ * Paravirtualization support
+ */
+
+#ifndef CONFIG_PARAVIRT
+# define paravirt_enabled()	0
+#endif
+
+#endif

init/main.c +5 -1

Index: linux/init/main.c
===================================================================
@@ -374,7 +374,11 @@ static void __init setup_per_cpu_areas(v
 	if (size < PERCPU_ENOUGH_ROOM)
 		size = PERCPU_ENOUGH_ROOM;
 #endif
-	ptr = alloc_bootmem(size * nr_possible_cpus);
+	/*
+	 * Align them to page size - just in case someone aligns
+	 * the per-CPU data to page that alignment should be preserved:
+	 */
+	ptr = alloc_bootmem_pages(size * nr_possible_cpus);
 
 	for_each_possible_cpu(i) {
 		__per_cpu_offset[i] = ptr - __per_cpu_start;

_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

A message that is not in the archive (sent off-list or not yet received): [email protected]

Avi Kivity

2007-01-09 12:54 UTC #10 lore raw

Quoted 4 lines · Ingo Molnar

ok, i changed this and i added the SVM patch function too. AMD's VMMCALL is 0x0f, 0x01, 0xd9, correct?

yes.

Quoted 7 lines

Quoted 2 lines
Where is the vmcall exit handler?
in my tree, have not sent the patch yet - first want to combine it with the cr3 feature to have it tested.

I'd like it in the final patch.

btw, we need to define the hypercall protocol. We should aim for most things (esp. hypercall number and return code) to be passed in registers. Need to take care of 32/64 bit compatibility from day one.

Patches look good. Only thing missing is printk() log levels.

Signature · 8 lines

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

A message that is not in the archive (sent off-list or not yet received): 45A39095.80005-atKUWr5tajBWk0Htik3J/[email protected]

Ingo Molnar

author

2007-01-09 13:17 UTC #12 lore raw

Quoted 8 lines · Avi Kivity

Quoted 6 lines
Quoted 2 lines
Where is the vmcall exit handler?
in my tree, have not sent the patch yet - first want to combine it with the cr3 feature to have it tested.
I'd like it in the final patch.

yeah. Right now i have:

patches/kvm-paravirt-host.patch
patches/kvm-paravirt-guest.patch
patches/kvm-paravirt-hypercall-host.patch
patches/kvm-paravirt-hypercall-guest.patch
patches/kvm-paravirt-cr3-cache.patch

this builds and boots fine on VMX, and a test-hypercall is successfully issued. I've attached a tarball of them.

i've changed the details you mentioned (printk loglevels, etc.), and some other detail as well: the ->patch_hypercall() thing now patches in the 'ret' instruction as well. This allowed me to add a -ENOSYS default entry there, to have defined error behavior in case the hypervisor does not patch things.

NOTE: i have not updated the cr3 patch to the hypercall API yet (hence the aliasing bug is not fixed yet), i wanted to get this to you so that we can think about the hypercall API.

Right now what i have is only good to test that the VMCALL instruction works - but the API must look differently. I'd prefer a register-based thing so that i can embedd hypercalls within Linux without having to go to a wrapper function. Right now a "call hypercall_addr" is a regparm-based function entry. I'd like to keep that - but not have a fixed number of parameters but inlines/macros for all parameter
combinations: 1, 2, 3, 4, 5 param, picked up automatically via use. I.e. a 2-param call would be:

	hypercall(KVM_cr3_miss, cr3);

a 3-param call would be:

	hypercall(KVM_api_call1, param1, param2);

a 1-param call would be:

	hypercall(KVM_api_call2);

Does this look good to you? I'd like the basic API to be as light as possible.

Ingo

_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

A message that is not in the archive (sent off-list or not yet received): [email protected]

Ingo Molnar

author

2007-01-09 13:30 UTC #14 lore raw

Quoted 15 lines · Ingo Molnar

NOTE: i have not updated the cr3 patch to the hypercall API yet (hence the aliasing bug is not fixed yet), i wanted to get this to you so that we can think about the hypercall API.
Right now what i have is only good to test that the VMCALL instruction works - but the API must look differently. I'd prefer a register-based thing so that i can embedd hypercalls within Linux without having to go to a wrapper function. Right now a "call hypercall_addr" is a regparm-based function entry. I'd like to keep that - but not have a fixed number of parameters but inlines/macros for all parameter
combinations: 1, 2, 3, 4, 5 param, picked up automatically via use. I.e. a 2-param call would be:
	hypercall(KVM_cr3_miss, cr3);

Quoted 7 lines

a 3-param call would be:
	hypercall(KVM_api_call1, param1, param2);
a 1-param call would be:
	hypercall(KVM_api_call2);

i.e. something like the patch below (ontop of the previous tarball, still ad-hoc, these things need to go into a header, and i only have the 1 and 2 param macros done).

this is very tightly integrated into the natural instruction sequence of functions that call it:

c01209d4 <test_hypercall>:

 c01209d4:       55                      push   %ebp
 c01209d5:       89 e5                   mov    %esp,%ebp
 c01209d7:       50                      push   %eax
 c01209d8:       50                      push   %eax
 c01209d9:       e8 06 fa ff ff          call   c01203e4 <hypercall_addr>
 c01209de:       c7 04 24 48 41 50 c0    movl   $0xc0504148,(%esp)
 c01209e5:       e8 1f a9 00 00          call   c012b309 <printk>
 c01209ea:       e8 f5 f9 ff ff          call   c01203e4 <hypercall_addr>
 c01209ef:       c7 04 24 67 41 50 c0    movl   $0xc0504167,(%esp)
 c01209f6:       e8 0e a9 00 00          call   c012b309 <printk>
 c01209fb:       c9                      leave
 c01209fc:       c3                      ret

this is as cheap as it gets, and it only clobbers the registers that are needed. Basically "call hypercall_addr" is equivalent to a syscall trap instruction.

Ingo

arch/i386/kernel/paravirt.c +29 ~~-18~~

Index: linux/arch/i386/kernel/paravirt.c
===================================================================
@@ -888,29 +888,40 @@ asm (
 	"		ret				\n"
 );
 
-extern unsigned char hypercall_addr[4];
+extern unsigned char hypercall_addr[6];
 
-
-static inline int
-kvm_hypercall(void *param1, void *param2, void *param3, void *param4)
-{
-	int ret = -1;
-
-	asm (" call hypercall_addr\n"
-		: "=g" (ret)
-		: "eax" (param1),
-		  "ecx" (param2),
-		  "edx" (param3),
-		  "ebp" (param4));
-
-	return ret;
-}
+#define hypercall1(nr)				\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=g" (__ret)			\
+		: "eax" (nr)			\
+	);					\
+	__ret;					\
+})
+
+#define hypercall2(nr, p1)			\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=g" (__ret)			\
+		: "eax" (nr),			\
+		  "ecx" (p1)			\
+	);					\
+	__ret;					\
+})
 
 void test_hypercall(void)
 {
-	int ret = kvm_hypercall((void *)1, (void *)2, (void *)3, (void *)4);
+	int ret;
+
+	ret = hypercall1(1);
+	printk(KERN_DEBUG "hypercall test #1, ret: %d\n", ret);
 
-	printk(KERN_DEBUG "hypercall test, ret: %d\n", ret);
+	ret = hypercall2(1, 2);
+	printk(KERN_DEBUG "hypercall test #2, ret: %d\n", ret);
 }
 
 int kvm_guest_register_para(int cpu)

drivers/kvm/vmx.c +1 -1

Index: linux/drivers/kvm/vmx.c
===================================================================
@@ -1032,7 +1032,7 @@ static int vmcs_setup_cr3_cache(struct k
 	cr3_target_values = (msr_val >> 16) & ((1 << 10) - 1);
 	printk(KERN_DEBUG " cr3 target values: %d\n", cr3_target_values);
 	if (cr3_target_values > KVM_CR3_CACHE_SIZE) {
-		printk(KERN_WARN "KVM: limiting cr3 cache size from %d to %d\n",
+		printk(KERN_WARNING "KVM: limiting cr3 cache size from %d to %d\n",
 			cr3_target_values, KVM_CR3_CACHE_SIZE);
 		cr3_target_values = KVM_CR3_CACHE_SIZE;
 	}

Avi Kivity

2007-01-09 13:41 UTC #15 lore raw

In reply to a message not in the archive

Quoted 47 lines · Ingo Molnar

Quoted 9 lines · Avi Kivity
Quoted 7 lines
Quoted 3 lines
Where is the vmcall exit handler?
in my tree, have not sent the patch yet - first want to combine it with the cr3 feature to have it tested.
I'd like it in the final patch.
yeah. Right now i have:
patches/kvm-paravirt-host.patch
patches/kvm-paravirt-guest.patch
patches/kvm-paravirt-hypercall-host.patch
patches/kvm-paravirt-hypercall-guest.patch
patches/kvm-paravirt-cr3-cache.patch
this builds and boots fine on VMX, and a test-hypercall is successfully issued. I've attached a tarball of them.
i've changed the details you mentioned (printk loglevels, etc.), and some other detail as well: the ->patch_hypercall() thing now patches in the 'ret' instruction as well. This allowed me to add a -ENOSYS default entry there, to have defined error behavior in case the hypervisor does not patch things.
NOTE: i have not updated the cr3 patch to the hypercall API yet (hence the aliasing bug is not fixed yet), i wanted to get this to you so that we can think about the hypercall API.
Right now what i have is only good to test that the VMCALL instruction works - but the API must look differently. I'd prefer a register-based thing so that i can embedd hypercalls within Linux without having to go to a wrapper function. Right now a "call hypercall_addr" is a regparm-based function entry. I'd like to keep that - but not have a fixed number of parameters but inlines/macros for all parameter
combinations: 1, 2, 3, 4, 5 param, picked up automatically via use. I.e. a 2-param call would be:
	hypercall(KVM_cr3_miss, cr3);
a 3-param call would be:

Quoted 10 lines

	hypercall(KVM_api_call1, param1, param2);
a 1-param call would be:
	hypercall(KVM_api_call2);
Does this look good to you? I'd like the basic API to be as light as possible.

Won't 32-bit and 64-bit pick different registers?

We can work around it (call is_long_mode() when decoding the hypercall), but it kind of defeats the purpose of the optimization, no?

Signature · 8 lines

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

A message that is not in the archive (sent off-list or not yet received): 45A39B90.6070908-atKUWr5tajBWk0Htik3J/[email protected]

Ingo Molnar

author

2007-01-09 13:53 UTC #17 lore raw

Quoted 8 lines · Avi Kivity

Quoted 2 lines
Does this look good to you? I'd like the basic API to be as light as possible.
Won't 32-bit and 64-bit pick different registers?
We can work around it (call is_long_mode() when decoding the hypercall), but it kind of defeats the purpose of the optimization, no?

well, we can standardize on the 32-bit calling convention: eax, ecx, edx, ebp, etc. We can do that via the 64-bit asm. So it should be the same i think - just that a 32-bit guest on a 64-bit host wont be able to set the high bits of those registers.

I've attached my current patch (ontop of tarball) - that's one idea about how it could look like. Note that i didnt introduce a syscall function table in kvm_handle_hypercall() - this will be the more optimal solution until the # of hypercalls is relatively low. That way we only prepare the parameters that are truly needed.

you are right in that we cannot call the syscall functions directly and that in practice we'll shuffle things around - but we have to check the first parameter anyway, and the shuffling isnt /that/ big of a problem. I wanted to keep the guest-side as low-impact as possible, so that a native kernel's instruction sequence is not disturbed too much by the presence of a NOP hypercall.

in fact it would probably be more logical to use the standard syscall
order: eax, ebx, ecx, edx, esi, edi, ebp?

Ingo

arch/i386/kernel/paravirt.c +29 ~~-18~~

Index: linux/arch/i386/kernel/paravirt.c
===================================================================
@@ -888,29 +888,40 @@ asm (
 	"		ret				\n"
 );
 
-extern unsigned char hypercall_addr[4];
+extern unsigned char hypercall_addr[6];
 
-
-static inline int
-kvm_hypercall(void *param1, void *param2, void *param3, void *param4)
-{
-	int ret = -1;
-
-	asm (" call hypercall_addr\n"
-		: "=g" (ret)
-		: "eax" (param1),
-		  "ecx" (param2),
-		  "edx" (param3),
-		  "ebp" (param4));
-
-	return ret;
-}
+#define hypercall1(nr)				\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=g" (__ret)			\
+		: "eax" (nr)			\
+	);					\
+	__ret;					\
+})
+
+#define hypercall2(nr, p1)			\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=g" (__ret)			\
+		: "eax" (nr),			\
+		  "ecx" (p1)			\
+	);					\
+	__ret;					\
+})
 
 void test_hypercall(void)
 {
-	int ret = kvm_hypercall((void *)1, (void *)2, (void *)3, (void *)4);
+	int ret;
+
+	ret = hypercall1(__NR_hypercall_load_cr3);
+	printk(KERN_DEBUG "hypercall test #1, ret: %d\n", ret);
 
-	printk(KERN_DEBUG "hypercall test, ret: %d\n", ret);
+	ret = hypercall2(0xbad, 0xbad);
+	printk(KERN_DEBUG "hypercall test #2, ret: %d\n", ret);
 }
 
 int kvm_guest_register_para(int cpu)

drivers/kvm/kvm.h +2 -0

Index: linux/drivers/kvm/kvm.h
===================================================================
@@ -639,4 +639,6 @@ static inline u32 get_rdx_init_val(void)
 #define TSS_REDIRECTION_SIZE (256 / 8)
 #define RMODE_TSS_SIZE (TSS_BASE_SIZE + TSS_REDIRECTION_SIZE + TSS_IOPB_SIZE + 1)
 
+extern int kvm_handle_hypercall(struct kvm_vcpu *vcpu);
+
 #endif

drivers/kvm/kvm_main.c +26 -0

Index: linux/drivers/kvm/kvm_main.c
===================================================================
@@ -1138,6 +1138,32 @@ int emulate_instruction(struct kvm_vcpu 
 }
 EXPORT_SYMBOL_GPL(emulate_instruction);
 
+int hypercall_load_cr3(struct kvm_vcpu *vcpu, unsigned long new_cr3)
+{
+	printk("not yet\n");
+
+	return -ENOSYS;
+}
+
+int kvm_handle_hypercall(struct kvm_vcpu *vcpu)
+{
+	int nr = vcpu->regs[VCPU_REGS_RAX];
+	int ret = -EINVAL;
+
+	switch (nr) {
+		case __NR_hypercall_load_cr3:
+
+		ret = hypercall_load_cr3(vcpu, vcpu->regs[VCPU_REGS_RCX]);
+		break;
+	default:
+		printk(KERN_DEBUG "invalid hypercall %d\n", nr);
+	}
+	vcpu->regs[VCPU_REGS_RAX] = ret;
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(kvm_handle_hypercall);
+
 static u64 mk_cr_64(u64 curr_cr, u32 new_val)
 {
 	return (curr_cr & ~((1ULL << 32) - 1)) | new_val;

drivers/kvm/vmx.c +4 -8

Index: linux/drivers/kvm/vmx.c
===================================================================
@@ -1032,7 +1032,7 @@ static int vmcs_setup_cr3_cache(struct k
 	cr3_target_values = (msr_val >> 16) & ((1 << 10) - 1);
 	printk(KERN_DEBUG " cr3 target values: %d\n", cr3_target_values);
 	if (cr3_target_values > KVM_CR3_CACHE_SIZE) {
-		printk(KERN_WARN "KVM: limiting cr3 cache size from %d to %d\n",
+		printk(KERN_WARNING "KVM: limiting cr3 cache size from %d to %d\n",
 			cr3_target_values, KVM_CR3_CACHE_SIZE);
 		cr3_target_values = KVM_CR3_CACHE_SIZE;
 	}

@@ -1726,16 +1726,12 @@ static int handle_halt(struct kvm_vcpu *
 static int handle_vmcall(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	kvm_run->exit_reason = KVM_EXIT_DEBUG;
-	printk(KERN_DEBUG "got vmcall at RIP %08lx\n", vmcs_readl(GUEST_RIP));
-	printk(KERN_DEBUG "vmcall params: %08lx, %08lx, %08lx, %08lx\n",
-		vcpu->regs[VCPU_REGS_RAX],
-		vcpu->regs[VCPU_REGS_RCX],
-		vcpu->regs[VCPU_REGS_RDX],
-		vcpu->regs[VCPU_REGS_RBP]);
-	vcpu->regs[VCPU_REGS_RAX] = 0;
+	kvm_handle_hypercall(vcpu);
 	vmcs_writel(GUEST_RIP, vmcs_readl(GUEST_RIP)+3);
+
 	return 1;
 }
+
 /*
  * The exit handlers return 1 if the exit was handled fully and guest execution
  * may resume.  Otherwise they set the kvm_run parameter to indicate what needs

include/linux/kvm_para.h +3 -0

Index: linux/include/linux/kvm_para.h
===================================================================
@@ -72,4 +72,7 @@ struct kvm_vcpu_para_state {
 
 #define KVM_EINVAL EINVAL
 
+#define __NR_hypercall_load_cr3		0
+#define __NR_hypercalls			1
+
 #endif

A message that is not in the archive (sent off-list or not yet received): [email protected]

Avi Kivity

2007-01-09 14:08 UTC #19 lore raw

Quoted 13 lines · Ingo Molnar

Quoted 9 lines · Avi Kivity
Quoted 3 lines
Does this look good to you? I'd like the basic API to be as light as possible.
Won't 32-bit and 64-bit pick different registers?
We can work around it (call is_long_mode() when decoding the hypercall), but it kind of defeats the purpose of the optimization, no?

Quoted 5 lines

well, we can standardize on the 32-bit calling convention: eax, ecx, edx, ebp, etc. We can do that via the 64-bit asm. So it should be the same i think - just that a 32-bit guest on a 64-bit host wont be able to set the high bits of those registers.

That uglifies 64-bit at the expense of 32-bit. I'd prefer it to be the other way round, but it's not really an issue either way.

In any case, it needs to be documented, as other guests may not use gcc or regparm.

Quoted 6 lines

I've attached my current patch (ontop of tarball) - that's one idea about how it could look like. Note that i didnt introduce a syscall function table in kvm_handle_hypercall() - this will be the more optimal solution until the # of hypercalls is relatively low. That way we only prepare the parameters that are truly needed.

Quoted 7 lines

you are right in that we cannot call the syscall functions directly and that in practice we'll shuffle things around - but we have to check the first parameter anyway, and the shuffling isnt /that/ big of a problem. I wanted to keep the guest-side as low-impact as possible, so that a native kernel's instruction sequence is not disturbed too much by the presence of a NOP hypercall.

Yes, that makes sense.

Quoted 3 lines

in fact it would probably be more logical to use the standard syscall
order: eax, ebx, ecx, edx, esi, edi, ebp?

Even better. It allows more registers and avoids a random gcc dependency.

arch/i386/kernel/paravirt.c asm ( · line 902

Quoted 11 lines

arch/i386/kernel/paravirt.c +10 -0

+#define hypercall1(nr)				\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=g" (__ret)			\
+		: "eax" (nr)			\
+	);					\
+	__ret;					\
+})

shouldn't that be

asm ("call hypercall_addr" : "=a"(__ret) : "a"(nr))

I don't think "eax" is a valid asm constraint, and we need to specify eax as the return register.

Signature · 8 lines

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

A message that is not in the archive (sent off-list or not yet received): 45A3A1C6.201-atKUWr5tajBWk0Htik3J/[email protected]

Ingo Molnar

author

2007-01-09 14:22 UTC #21 lore raw

Quoted 8 lines · Avi Kivity

Quoted 5 lines
well, we can standardize on the 32-bit calling convention: eax, ecx, edx, ebp, etc. We can do that via the 64-bit asm. So it should be the same i think - just that a 32-bit guest on a 64-bit host wont be able to set the high bits of those registers.
That uglifies 64-bit at the expense of 32-bit. I'd prefer it to be the other way round, but it's not really an issue either way.

i can pick whichever is better. If we pick 64-bit's natural register order, we at least have the chance to do the entry in assembly and then to call the hyper-call table directly? (with a default-not-taken branch leading out of this logic to a reshuffle thing if the guest is 32-bit)

[ We also have the chance to let future hardware do the call for us from
a MSR-programmed hypercall table, straight from the VMCALL, after it has verified that RAX is within a pre-defined boundary. ]

so i'd vote for the 64-bit natural register order: return value in rax, parameters in: rdi, rsi, rdx, rcx, r8, r9. On 32-bit that would be edi, esi, edx, ecx, ebx, ebp - the last two shuffled into VCPU_REGS_R8/R9. That's 6 parameters already - should be enough - that's what Linux has itself. Whatever else must be passed in should come pointer-passed.

Quoted 2 lines

In any case, it needs to be documented, as other guests may not use gcc or regparm.

yeah. Once we pick one and declare it stable, it's cast into stone - wont change, ever.

Quoted 5 lines

Quoted 2 lines
in fact it would probably be more logical to use the standard syscall
order: eax, ebx, ecx, edx, esi, edi, ebp?
Even better. It allows more registers and avoids a random gcc dependency.

ok.

Quoted 14 lines

arch/i386/kernel/paravirt.c asm ( · line 902
Quoted 12 lines
arch/i386/kernel/paravirt.c +10 -0
+#define hypercall1(nr)				\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=g" (__ret)			\
+		: "eax" (nr)			\
+	);					\
+	__ret;					\
+})
shouldn't that be

Quoted 3 lines

asm ("call hypercall_addr" : "=a"(__ret) : "a"(nr))
?

oops, yes. I was already wondering about the bogus return value printout ;)

Ingo

A message that is not in the archive (sent off-list or not yet received): [email protected]

Avi Kivity

2007-01-09 14:35 UTC #23 lore raw

Quoted 22 lines · Ingo Molnar

Quoted 9 lines · Avi Kivity
Quoted 6 lines
well, we can standardize on the 32-bit calling convention: eax, ecx, edx, ebp, etc. We can do that via the 64-bit asm. So it should be the same i think - just that a 32-bit guest on a 64-bit host wont be able to set the high bits of those registers.
That uglifies 64-bit at the expense of 32-bit. I'd prefer it to be the other way round, but it's not really an issue either way.
i can pick whichever is better. If we pick 64-bit's natural register order, we at least have the chance to do the entry in assembly and then to call the hyper-call table directly? (with a default-not-taken branch leading out of this logic to a reshuffle thing if the guest is 32-bit)
[ We also have the chance to let future hardware do the call for us from
a MSR-programmed hypercall table, straight from the VMCALL, after it has verified that RAX is within a pre-defined boundary. ]

Quoted 7 lines

so i'd vote for the 64-bit natural register order: return value in rax, parameters in: rdi, rsi, rdx, rcx, r8, r9. On 32-bit that would be edi, esi, edx, ecx, ebx, ebp - the last two shuffled into VCPU_REGS_R8/R9. That's 6 parameters already - should be enough - that's what Linux has itself. Whatever else must be passed in should come pointer-passed.

Agreed, let's make it so. When you say "pointer-passed" you mean physical address passed, right? ;-)

Signature · 8 lines

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

A message that is not in the archive (sent off-list or not yet received): 45A3A816.6010308-atKUWr5tajBWk0Htik3J/[email protected]

Ingo Molnar

author

2007-01-09 14:38 UTC #25 lore raw

Quoted 9 lines · Avi Kivity

Quoted 6 lines
so i'd vote for the 64-bit natural register order: return value in rax, parameters in: rdi, rsi, rdx, rcx, r8, r9. On 32-bit that would be edi, esi, edx, ecx, ebx, ebp - the last two shuffled into VCPU_REGS_R8/R9. That's 6 parameters already - should be enough - that's what Linux has itself. Whatever else must be passed in should come pointer-passed.
Agreed, let's make it so. When you say "pointer-passed" you mean physical address passed, right? ;-)

yeah ;)

/me whistles

below is the current snapshot (ontop of tarball).

Ingo

arch/i386/kernel/paravirt.c +29 ~~-18~~

Index: linux/arch/i386/kernel/paravirt.c
===================================================================
@@ -888,29 +888,40 @@ asm (
 	"		ret				\n"
 );
 
-extern unsigned char hypercall_addr[4];
+extern unsigned char hypercall_addr[6];
 
-
-static inline int
-kvm_hypercall(void *param1, void *param2, void *param3, void *param4)
-{
-	int ret = -1;
-
-	asm (" call hypercall_addr\n"
-		: "=g" (ret)
-		: "eax" (param1),
-		  "ecx" (param2),
-		  "edx" (param3),
-		  "ebp" (param4));
-
-	return ret;
-}
+#define hypercall0(nr)				\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=a" (__ret)			\
+		: "a" (nr)			\
+	);					\
+	__ret;					\
+})
+
+#define hypercall1(nr, p1)			\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=a" (__ret)			\
+		: "a" (nr),			\
+		  "D" (p1)			\
+	);					\
+	__ret;					\
+})
 
 void test_hypercall(void)
 {
-	int ret = kvm_hypercall((void *)1, (void *)2, (void *)3, (void *)4);
+	int ret;
+
+	ret = hypercall0(__NR_hypercall_load_cr3);
+	printk(KERN_DEBUG "hypercall test #1, ret: %d\n", ret);
 
-	printk(KERN_DEBUG "hypercall test, ret: %d\n", ret);
+	ret = hypercall1(0xbad, 0xbad);
+	printk(KERN_DEBUG "hypercall test #2, ret: %d\n", ret);
 }
 
 int kvm_guest_register_para(int cpu)

drivers/kvm/kvm.h +2 -0

Index: linux/drivers/kvm/kvm.h
===================================================================
@@ -639,4 +639,6 @@ static inline u32 get_rdx_init_val(void)
 #define TSS_REDIRECTION_SIZE (256 / 8)
 #define RMODE_TSS_SIZE (TSS_BASE_SIZE + TSS_REDIRECTION_SIZE + TSS_IOPB_SIZE + 1)
 
+extern int kvm_handle_hypercall(struct kvm_vcpu *vcpu);
+
 #endif

drivers/kvm/kvm_main.c +26 -0

Index: linux/drivers/kvm/kvm_main.c
===================================================================
@@ -1138,6 +1138,32 @@ int emulate_instruction(struct kvm_vcpu 
 }
 EXPORT_SYMBOL_GPL(emulate_instruction);
 
+int hypercall_load_cr3(struct kvm_vcpu *vcpu, unsigned long new_cr3)
+{
+	printk("not yet\n");
+
+	return -ENOSYS;
+}
+
+int kvm_handle_hypercall(struct kvm_vcpu *vcpu)
+{
+	int nr = vcpu->regs[VCPU_REGS_RAX];
+	int ret = -EINVAL;
+
+	switch (nr) {
+		case __NR_hypercall_load_cr3:
+
+		ret = hypercall_load_cr3(vcpu, vcpu->regs[VCPU_REGS_RDI]);
+		break;
+	default:
+		printk(KERN_DEBUG "invalid hypercall %d\n", nr);
+	}
+	vcpu->regs[VCPU_REGS_RAX] = ret;
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(kvm_handle_hypercall);
+
 static u64 mk_cr_64(u64 curr_cr, u32 new_val)
 {
 	return (curr_cr & ~((1ULL << 32) - 1)) | new_val;

drivers/kvm/vmx.c +4 -8

Index: linux/drivers/kvm/vmx.c
===================================================================
@@ -1032,7 +1032,7 @@ static int vmcs_setup_cr3_cache(struct k
 	cr3_target_values = (msr_val >> 16) & ((1 << 10) - 1);
 	printk(KERN_DEBUG " cr3 target values: %d\n", cr3_target_values);
 	if (cr3_target_values > KVM_CR3_CACHE_SIZE) {
-		printk(KERN_WARN "KVM: limiting cr3 cache size from %d to %d\n",
+		printk(KERN_WARNING "KVM: limiting cr3 cache size from %d to %d\n",
 			cr3_target_values, KVM_CR3_CACHE_SIZE);
 		cr3_target_values = KVM_CR3_CACHE_SIZE;
 	}

@@ -1726,16 +1726,12 @@ static int handle_halt(struct kvm_vcpu *
 static int handle_vmcall(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	kvm_run->exit_reason = KVM_EXIT_DEBUG;
-	printk(KERN_DEBUG "got vmcall at RIP %08lx\n", vmcs_readl(GUEST_RIP));
-	printk(KERN_DEBUG "vmcall params: %08lx, %08lx, %08lx, %08lx\n",
-		vcpu->regs[VCPU_REGS_RAX],
-		vcpu->regs[VCPU_REGS_RCX],
-		vcpu->regs[VCPU_REGS_RDX],
-		vcpu->regs[VCPU_REGS_RBP]);
-	vcpu->regs[VCPU_REGS_RAX] = 0;
+	kvm_handle_hypercall(vcpu);
 	vmcs_writel(GUEST_RIP, vmcs_readl(GUEST_RIP)+3);
+
 	return 1;
 }
+
 /*
  * The exit handlers return 1 if the exit was handled fully and guest execution
  * may resume.  Otherwise they set the kvm_run parameter to indicate what needs

include/linux/kvm_para.h +15 -0

Index: linux/include/linux/kvm_para.h
===================================================================
@@ -72,4 +72,19 @@ struct kvm_vcpu_para_state {
 
 #define KVM_EINVAL EINVAL
 
+/*
+ * Hypercall calling convention:
+ *
+ * RAX is the hypercall index, goes from 0 to __NR_hypercalls-1
+ *
+ * Each hypercall may have 0-6 parameters.
+ *
+ * parameters 1-6 are in the standard gcc x86_64 calling convention
+ * order: RDI, RSI, RDX, RCX, R8, R9.
+ *
+ * 32-bit parameters are: EDI, ESI, EDX, ECX, EBX, EBP
+ */
+#define __NR_hypercall_load_cr3		0
+#define __NR_hypercalls			1
+
 #endif

A message that is not in the archive (sent off-list or not yet received): [email protected]

Ingo Molnar

author

2007-01-09 14:44 UTC #27 lore raw

Quoted 4 lines · Ingo Molnar

+/*
+ * Hypercall calling convention:
+ *
+ * RAX is the hypercall index, goes from 0 to __NR_hypercalls-1

Quoted 8 lines

+ *
+ * Each hypercall may have 0-6 parameters.
+ *
+ * parameters 1-6 are in the standard gcc x86_64 calling convention
+ * order: RDI, RSI, RDX, RCX, R8, R9.
+ *
+ * 32-bit parameters are: EDI, ESI, EDX, ECX, EBX, EBP
+ */

about the 32-bit side: maybe it gives us a bit more options for later if we pick the natural order for 32-bit too: EBX for index, EAX, EDX, ECX for the first 3 parameters, with the remaining 3 parameters in ESI, EDI, EBP? Most hypercalls will have 0, 1, 2 or 3 parameters.

Ingo

A message that is not in the archive (sent off-list or not yet received): [email protected]

Avi Kivity

2007-01-09 14:50 UTC #29 lore raw

Quoted 17 lines · Ingo Molnar

Quoted 4 lines · Ingo Molnar
+/*
+ * Hypercall calling convention:
+ *
+ * RAX is the hypercall index, goes from 0 to __NR_hypercalls-1
Quoted 9 lines
+ *
+ * Each hypercall may have 0-6 parameters.
+ *
+ * parameters 1-6 are in the standard gcc x86_64 calling convention
+ * order: RDI, RSI, RDX, RCX, R8, R9.
+ *
+ * 32-bit parameters are: EDI, ESI, EDX, ECX, EBX, EBP
+ */

Quoted 5 lines

about the 32-bit side: maybe it gives us a bit more options for later if we pick the natural order for 32-bit too: EBX for index, EAX, EDX, ECX for the first 3 parameters, with the remaining 3 parameters in ESI, EDI, EBP? Most hypercalls will have 0, 1, 2 or 3 parameters.

I don't mind, as long as it's clearly documented.

Signature · 8 lines

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

A message that is not in the archive (sent off-list or not yet received): 45A3ABAF.90208-atKUWr5tajBWk0Htik3J/[email protected]

Ingo Molnar

author

2007-01-09 15:04 UTC #31 lore raw

Quoted 7 lines · Avi Kivity

Quoted 5 lines
about the 32-bit side: maybe it gives us a bit more options for later if we pick the natural order for 32-bit too: EBX for index, EAX, EDX, ECX for the first 3 parameters, with the remaining 3 parameters in ESI, EDI, EBP? Most hypercalls will have 0, 1, 2 or 3 parameters.
I don't mind, as long as it's clearly documented.

ok. Below is the current snapshot - hypercall calling convention documented and implemented for both 64-bit and 32-bit on the KVM side, and a small demo call done on the 32-bit guest side.

Ingo

arch/i386/kernel/paravirt.c +29 ~~-18~~

Index: linux/arch/i386/kernel/paravirt.c
===================================================================
@@ -888,29 +888,40 @@ asm (
 	"		ret				\n"
 );
 
-extern unsigned char hypercall_addr[4];
+extern unsigned char hypercall_addr[6];
 
-
-static inline int
-kvm_hypercall(void *param1, void *param2, void *param3, void *param4)
-{
-	int ret = -1;
-
-	asm (" call hypercall_addr\n"
-		: "=g" (ret)
-		: "eax" (param1),
-		  "ecx" (param2),
-		  "edx" (param3),
-		  "ebp" (param4));
-
-	return ret;
-}
+#define hypercall0(nr)				\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=a" (__ret)			\
+		: "b" (nr)			\
+	);					\
+	__ret;					\
+})
+
+#define hypercall1(nr, p1)			\
+({						\
+	int __ret;				\
+						\
+	asm (" call hypercall_addr\n"		\
+		: "=a" (__ret)			\
+		: "b" (nr),			\
+		  "a" (p1)			\
+	);					\
+	__ret;					\
+})
 
 void test_hypercall(void)
 {
-	int ret = kvm_hypercall((void *)1, (void *)2, (void *)3, (void *)4);
+	int ret;
+
+	ret = hypercall0(__NR_hypercall_load_cr3);
+	printk(KERN_DEBUG "hypercall test #1, ret: %d\n", ret);
 
-	printk(KERN_DEBUG "hypercall test, ret: %d\n", ret);
+	ret = hypercall1(0xbad, 0xbad);
+	printk(KERN_DEBUG "hypercall test #2, ret: %d\n", ret);
 }
 
 int kvm_guest_register_para(int cpu)

drivers/kvm/kvm.h +2 -0

Index: linux/drivers/kvm/kvm.h
===================================================================
@@ -639,4 +639,6 @@ static inline u32 get_rdx_init_val(void)
 #define TSS_REDIRECTION_SIZE (256 / 8)
 #define RMODE_TSS_SIZE (TSS_BASE_SIZE + TSS_REDIRECTION_SIZE + TSS_IOPB_SIZE + 1)
 
+extern int kvm_handle_hypercall(struct kvm_vcpu *vcpu);
+
 #endif

drivers/kvm/kvm_main.c +56 -0

62 lines not on this page · raw message

drivers/kvm/vmx.c +4 -8

Index: linux/drivers/kvm/vmx.c
===================================================================
@@ -1032,7 +1032,7 @@ static int vmcs_setup_cr3_cache(struct k
 	cr3_target_values = (msr_val >> 16) & ((1 << 10) - 1);
 	printk(KERN_DEBUG " cr3 target values: %d\n", cr3_target_values);
 	if (cr3_target_values > KVM_CR3_CACHE_SIZE) {
-		printk(KERN_WARN "KVM: limiting cr3 cache size from %d to %d\n",
+		printk(KERN_WARNING "KVM: limiting cr3 cache size from %d to %d\n",
 			cr3_target_values, KVM_CR3_CACHE_SIZE);
 		cr3_target_values = KVM_CR3_CACHE_SIZE;
 	}

@@ -1726,16 +1726,12 @@ static int handle_halt(struct kvm_vcpu *
 static int handle_vmcall(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	kvm_run->exit_reason = KVM_EXIT_DEBUG;
-	printk(KERN_DEBUG "got vmcall at RIP %08lx\n", vmcs_readl(GUEST_RIP));
-	printk(KERN_DEBUG "vmcall params: %08lx, %08lx, %08lx, %08lx\n",
-		vcpu->regs[VCPU_REGS_RAX],
-		vcpu->regs[VCPU_REGS_RCX],
-		vcpu->regs[VCPU_REGS_RDX],
-		vcpu->regs[VCPU_REGS_RBP]);
-	vcpu->regs[VCPU_REGS_RAX] = 0;
+	kvm_handle_hypercall(vcpu);
 	vmcs_writel(GUEST_RIP, vmcs_readl(GUEST_RIP)+3);
+
 	return 1;
 }
+
 /*
  * The exit handlers return 1 if the exit was handled fully and guest execution
  * may resume.  Otherwise they set the kvm_run parameter to indicate what needs

include/linux/kvm_para.h +19 -0

Index: linux/include/linux/kvm_para.h
===================================================================
@@ -72,4 +72,23 @@ struct kvm_vcpu_para_state {
 
 #define KVM_EINVAL EINVAL
 
+/*
+ * Hypercall calling convention:
+ *
+ * Each hypercall may have 0-6 parameters.
+ *
+ * 64-bit hypercall index is in RAX, goes from 0 to __NR_hypercalls-1
+ *
+ * 64-bit parameters 1-6 are in the standard gcc x86_64 calling convention
+ * order: RDI, RSI, RDX, RCX, R8, R9.
+ *
+ * 32-bit index is EBX, parameters are: EAX, ECX, EDX, ESI, EDI, EBP.
+ * (the first 3 are according to the gcc regparm calling convention)
+ *
+ * No registers are clobbered by the hypercall, except that the
+ * return value is in RAX.
+ */
+#define __NR_hypercall_load_cr3		0
+#define __NR_hypercalls			1
+
 #endif

A message that is not in the archive (sent off-list or not yet received): [email protected]

Ingo Molnar

author

2007-01-09 16:20 UTC #33 lore raw

PATCH 1/1 KVM: paravirt/hypercall queue

Quoted 3 lines · Ingo Molnar

ok. Below is the current snapshot - hypercall calling convention documented and implemented for both 64-bit and 32-bit on the KVM side, and a small demo call done on the 32-bit guest side.

i've updated the cr3-cache code to use the new hypercall facility - this solves the hpa/gpa aliasing bug you found.

i have changed a few more details around hypercalls - the macros are now cleaner, the cr3 cache is aligned (is easier to grow both para_state and cr3_cache in a compatible way), and a few more details here and there. Current tarball attached. Build and boot tested, the cr3 cache still works fine.

Ingo

_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

A message that is not in the archive (sent off-list or not yet received): [email protected]

Ingo Molnar

author

2007-01-09 16:23 UTC #35 lore raw

Re: [patchset] KVM: paravirt/hypercall queue

Quoted 8 lines · Ingo Molnar

i've updated the cr3-cache code to use the new hypercall facility - this solves the hpa/gpa aliasing bug you found.
i have changed a few more details around hypercalls - the macros are now cleaner, the cr3 cache is aligned (is easier to grow both para_state and cr3_cache in a compatible way), and a few more details here and there. Current tarball attached. Build and boot tested, the cr3 cache still works fine.

sorry - new tarball attached - the previous one depended on Rusty's paravirt_ops unexport patch - i flipped them around so this queue should apply more or less fine to KVM-trunk.

Ingo

_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel