[PATCH 3/3] KVM - add hypercall nr to kvm_run

[PATCH 3/3] KVM - add hypercall nr to kvm_run

[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@linux.intel.com>
--
 drivers/kvm/kvm_main.c |    1 +
 include/linux/kvm.h    |    1 +
 2 files changed, 2 insertions(+)

Index: kvm/drivers/kvm/kvm_main.c
===================================================================
--- kvm.orig/drivers/kvm/kvm_main.c
+++ 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;
Index: kvm/include/linux/kvm.h
===================================================================
--- kvm.orig/include/linux/kvm.h
+++ 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;

Re: [PATCH 3/3] KVM - add hypercall nr to kvm_run

[Avi Kivity]

Jeff Dike wrote:
> 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@linux.intel.com>
> --
>  drivers/kvm/kvm_main.c |    1 +
>  include/linux/kvm.h    |    1 +
>  2 files changed, 2 insertions(+)
>
> Index: kvm/drivers/kvm/kvm_main.c
> ===================================================================
> --- kvm.orig/drivers/kvm/kvm_main.c
> +++ 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;
> Index: kvm/include/linux/kvm.h
> ===================================================================
> --- kvm.orig/include/linux/kvm.h
> +++ 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;
>   

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.

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

Re: [PATCH 3/3] KVM - add hypercall nr to kvm_run

[Avi Kivity]

Avi Kivity wrote:
> Jeff Dike wrote:
>> 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@linux.intel.com>
>> -- 
>>  drivers/kvm/kvm_main.c |    1 +
>>  include/linux/kvm.h    |    1 +
>>  2 files changed, 2 insertions(+)
>>
>> Index: kvm/drivers/kvm/kvm_main.c
>> ===================================================================
>> --- kvm.orig/drivers/kvm/kvm_main.c
>> +++ 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;
>> Index: kvm/include/linux/kvm.h
>> ===================================================================
>> --- kvm.orig/include/linux/kvm.h
>> +++ 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;
>>   
>
> 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.
>

Applied this, btw.  Thanks.


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

Re: [kvm-devel] [PATCH 3/3] KVM - add hypercall nr to kvm_run

[Anthony Liguori]

Avi Kivity wrote:
> Jeff Dike wrote:
>   
>> 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@linux.intel.com>
>> --
>>  drivers/kvm/kvm_main.c |    1 +
>>  include/linux/kvm.h    |    1 +
>>  2 files changed, 2 insertions(+)
>>
>> Index: kvm/drivers/kvm/kvm_main.c
>> ===================================================================
>> --- kvm.orig/drivers/kvm/kvm_main.c
>> +++ 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;
>> Index: kvm/include/linux/kvm.h
>> ===================================================================
>> --- kvm.orig/include/linux/kvm.h
>> +++ 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;
>>   
>>     
>
> 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.
>   

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

Regards,

Anthony Liguori

Re: [kvm-devel] [PATCH 3/3] KVM - add hypercall nr to kvm_run

[Jeff Dike]

On Tue, Jul 17, 2007 at 09:15:51AM -0500, Anthony Liguori wrote:
> 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

-- 
Work email - jdike at linux dot intel dot com

Re: [kvm-devel] [PATCH 3/3] KVM - add hypercall nr to kvm_run

[Anthony Liguori]

Jeff Dike wrote:
> On Tue, Jul 17, 2007 at 09:15:51AM -0500, Anthony Liguori wrote:
>   
>> 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).

> 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

> 				Jeff
>
>   

Re: [kvm-devel] [PATCH 3/3] KVM - add hypercall nr to kvm_run

[Jeff Dike]

On Wed, Jul 18, 2007 at 11:28:18AM -0500, Anthony Liguori wrote:
> Within the kernel right (VMCALL is only usable in ring 1).  

Yup.

> 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

-- 
Work email - jdike at linux dot intel dot com

Re: [kvm-devel] [PATCH 3/3] KVM - add hypercall nr to kvm_run

[Anthony Liguori]

Jeff Dike wrote:
> On Wed, Jul 18, 2007 at 11:28:18AM -0500, Anthony Liguori wrote:
>   
>> Within the kernel right (VMCALL is only usable in ring 1).  
>>     
>
> Yup.
>
>   
>> 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

> 				Jeff
>
>