KVM - add hypercall nr to kvm_run

PATCH Applied Jeff Dike 3 participants 8 messages Jul 16, 2007 – Jul 18, 2007 KVM

lore

Share & tools

lore

Jeff Dike

author

2007-07-16 19:24 UTC #0 lore raw

PATCH 3/3 KVM - add hypercall nr to kvm_run

Add the hypercall number to kvm_run and initialize it. This might be considered API-changing, so I kept it separate.

Signed-off-by Jeff Dike <jdike-VuQAYsv1563Yd54FQh9/[email protected]>

 drivers/kvm/kvm_main.c |    1 +
 include/linux/kvm.h    |    1 +
 2 files changed, 2 insertions(+)

drivers/kvm/kvm_main.c +1 -0

Index: kvm/drivers/kvm/kvm_main.c
===================================================================
@@ -1361,6 +1361,7 @@ int kvm_hypercall(struct kvm_vcpu *vcpu,
 	}
 	switch (nr) {
 	default:
+		run->hypercall.nr = nr;
 		run->hypercall.args[0] = a0;
 		run->hypercall.args[1] = a1;
 		run->hypercall.args[2] = a2;

include/linux/kvm.h +1 -0

Index: kvm/include/linux/kvm.h
===================================================================
@@ -106,6 +106,7 @@ struct kvm_run {
 		} mmio;
 		/* KVM_EXIT_HYPERCALL */
 		struct {
+			__u64 nr;
 			__u64 args[6];
 			__u64 ret;
 			__u32 longmode;

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Avi Kivity

2007-07-17 08:23 UTC #1 lore raw

Quoted 27 lines · Jeff Dike

Add the hypercall number to kvm_run and initialize it. This might be considered API-changing, so I kept it separate.
Signed-off-by Jeff Dike <jdike-VuQAYsv1563Yd54FQh9/[email protected]>
--
 drivers/kvm/kvm_main.c |    1 +
 include/linux/kvm.h    |    1 +
 2 files changed, 2 insertions(+)
Index: kvm/drivers/kvm/kvm_main.c
===================================================================
drivers/kvm/kvm_main.c +1 -0
@@ -1361,6 +1361,7 @@ int kvm_hypercall(struct kvm_vcpu *vcpu,
 	}
 	switch (nr) {
 	default:
+		run->hypercall.nr = nr;
 		run->hypercall.args[0] = a0;
 		run->hypercall.args[1] = a1;
 		run->hypercall.args[2] = a2;
Index: kvm/include/linux/kvm.h
===================================================================

include/linux/kvm.h struct kvm_run { · line 112

Quoted 7 lines

include/linux/kvm.h +1 -0

@@ -106,6 +106,7 @@ struct kvm_run {
 		} mmio;
 		/* KVM_EXIT_HYPERCALL */
 		struct {
+			__u64 nr;
 			__u64 args[6];
 			__u64 ret;
 			__u32 longmode;

It isn't API changing as the API could not be (and has not been) used. We do need to add a padding element to the union to make sure additional union members don't cause its size to change.

Quite sad that we got this far with such brokenness.

Signature · 8 lines

error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Avi Kivity

Applied

2007-07-17 08:40 UTC #2 lore raw

Quoted 40 lines · Avi Kivity

Quoted 27 lines · Jeff Dike
Add the hypercall number to kvm_run and initialize it. This might be considered API-changing, so I kept it separate.
Signed-off-by Jeff Dike <jdike-VuQAYsv1563Yd54FQh9/[email protected]>
Signature · 6 lines
 drivers/kvm/kvm_main.c |    1 +
 include/linux/kvm.h    |    1 +
 2 files changed, 2 insertions(+)

Index: kvm/drivers/kvm/kvm_main.c
===================================================================
drivers/kvm/kvm_main.c +1 -0
@@ -1361,6 +1361,7 @@ int kvm_hypercall(struct kvm_vcpu *vcpu,
     }
     switch (nr) {
     default:
+        run->hypercall.nr = nr;
         run->hypercall.args[0] = a0;
         run->hypercall.args[1] = a1;
         run->hypercall.args[2] = a2;
Index: kvm/include/linux/kvm.h
===================================================================
include/linux/kvm.h struct kvm_run { · line 112
Quoted 7 lines
include/linux/kvm.h +1 -0
@@ -106,6 +106,7 @@ struct kvm_run {
         } mmio;
         /* KVM_EXIT_HYPERCALL */
         struct {
+            __u64 nr;
             __u64 args[6];
             __u64 ret;
             __u32 longmode;
It isn't API changing as the API could not be (and has not been) used. We do need to add a padding element to the union to make sure additional union members don't cause its size to change.

Quoted 2 lines

Quite sad that we got this far with such brokenness.

Applied this, btw. Thanks.

Signature · 8 lines

error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Anthony Liguori

2007-07-17 14:15 UTC #3 lore raw

In reply to Avi Kivity

Quoted 42 lines · Avi Kivity

Quoted 28 lines · Jeff Dike
Add the hypercall number to kvm_run and initialize it. This might be considered API-changing, so I kept it separate.
Signed-off-by Jeff Dike <jdike-VuQAYsv1563Yd54FQh9/[email protected]>
--
 drivers/kvm/kvm_main.c |    1 +
 include/linux/kvm.h    |    1 +
 2 files changed, 2 insertions(+)
Index: kvm/drivers/kvm/kvm_main.c
===================================================================
drivers/kvm/kvm_main.c +1 -0
@@ -1361,6 +1361,7 @@ int kvm_hypercall(struct kvm_vcpu *vcpu,
 	}
 	switch (nr) {
 	default:
+		run->hypercall.nr = nr;
 		run->hypercall.args[0] = a0;
 		run->hypercall.args[1] = a1;
 		run->hypercall.args[2] = a2;
Index: kvm/include/linux/kvm.h
===================================================================
include/linux/kvm.h struct kvm_run { · line 112
Quoted 7 lines
include/linux/kvm.h +1 -0
@@ -106,6 +106,7 @@ struct kvm_run {
 		} mmio;
 		/* KVM_EXIT_HYPERCALL */
 		struct {
+			__u64 nr;
 			__u64 args[6];
 			__u64 ret;
 			__u32 longmode;
It isn't API changing as the API could not be (and has not been) used. We do need to add a padding element to the union to make sure additional union members don't cause its size to change.

Quoted 2 lines

Quite sad that we got this far with such brokenness.

I'm planning on breaking this interface again since the new hypercall API only takes 4 arguments instead of 6.

Regards,

Anthony Liguori

Jeff Dike

author

2007-07-17 16:07 UTC #4 lore raw

Quoted 2 lines · Anthony Liguori

I'm planning on breaking this interface again since the new hypercall API only takes 4 arguments instead of 6.

Is anything written anywhere about this hypercall interface?

The thing which would make me happy (which the current code comes close to) is for the guest to be able to pass out Linux system calls with VMCALL.

Jeff

Signature · 7 lines

Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Anthony Liguori

2007-07-18 16:28 UTC #5 lore raw

Quoted 8 lines · Jeff Dike

Quoted 3 lines · Anthony Liguori
I'm planning on breaking this interface again since the new hypercall API only takes 4 arguments instead of 6.
Is anything written anywhere about this hypercall interface?

I've posted patches. I'll post more in the near future since we decided to make some rather drastic changes. With the new patchset, guest's will use vmcall directly with a couple bytes of padding. The hypervisor may or may not patch the calling site to use something different (like vmmcall).

Quoted 4 lines

The thing which would make me happy (which the current code comes close to) is for the guest to be able to pass out Linux system calls with VMCALL.

Within the kernel right (VMCALL is only usable in ring 1). Is it terribly important to be able to pass through the syscall arguments in registers verses packing them in a data structure and passing a pointer to that structure?

Regards,

Anthony Liguori

Quoted 3 lines

Jeff

Jeff Dike

author

2007-07-18 17:41 UTC #6 lore raw

Quoted 1 line · Anthony Liguori

Within the kernel right (VMCALL is only usable in ring 1).

Yup.

Quoted 4 lines

Is it
terribly important to be able to pass through the syscall arguments in registers verses packing them in a data structure and passing a pointer to that structure?

An arg block would be fine, too.

Jeff

Signature · 7 lines

Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Anthony Liguori

2007-07-18 19:25 UTC #7 lore raw

Quoted 8 lines · Jeff Dike

Quoted 2 lines · Anthony Liguori
Within the kernel right (VMCALL is only usable in ring 1).
Yup.

Quoted 8 lines

Quoted 5 lines
Is it
terribly important to be able to pass through the syscall arguments in registers verses packing them in a data structure and passing a pointer to that structure?
An arg block would be fine, too.

Okay, then you should be pretty well insulated against the changing hypercall stuff since you only need one hypercall NR and can use a one argument hypercall.

Regards,

Anthony Liguori

Quoted 3 lines

Jeff