Re: [kvm-devel] [PATCH 06/55] KVM: Per-architecture hypercall definitions

2007-12-26 Thread Avi Kivity
[copying Anthony, the original author]

Pavel Machek wrote:
> Hi!
>
>   
>> Currently kvm provides hypercalls only for x86* architectures. To
>> provide hypercall infrastructure for other kvm architectures I split
>> kvm_para.h into a generic header file and architecture specific
>> definitions.
>> 

>> diff --git a/include/asm-x86/kvm_para.h b/include/asm-x86/kvm_para.h
>> new file mode 100644
>> index 000..c6f3fd8
>> --- /dev/null
>> +++ b/include/asm-x86/kvm_para.h
>> @@ -0,0 +1,105 @@
>> +#ifndef __X86_KVM_PARA_H
>> +#define __X86_KVM_PARA_H
>> +
>> +/* This CPUID returns the signature 'KVMKVMKVM' in ebx, ecx, and edx.  It
>> + * should be used to determine that a VM is running under KVM.
>> + */
>> +#define KVM_CPUID_SIGNATURE 0x4000
>> 
>
> so it returns 'KVMKVMKVM' in %rax, too? 
>
>   

No, as documented. 'KVMKVMKVM' is spread among all three registers (9 
bytes won't fit into one...)

>> +/* For KVM hypercalls, a three-byte sequence of either the vmrun or the 
>> vmmrun
>> + * instruction.  The hypervisor may replace it with something else but only 
>> the
>> + * instructions are guaranteed to be supported.
>> + *
>> + * Up to four arguments may be passed in rbx, rcx, rdx, and rsi 
>> respectively.
>> + * The hypercall number should be placed in rax and the return
>> 
>
> rax? First, this file is shared with i386, AFAICT.
>   

rax is used here in the sense of 'rax on x86-64, eax on i386'.  I guess 
this should be documented.

>   
>> + * placed in rax.  No other registers will be clobbered unless explicited
>> + * noted by the particular hypercall.
>> + */
>> +
>> +static inline long kvm_hypercall0(unsigned int nr)
>> +{
>> +long ret;
>> +asm volatile(KVM_HYPERCALL
>> + : "=a"(ret)
>> + : "a"(nr));
>> 
>
> Second, if it is to be placed in rax, nr should be unsigned long?
>
>
>   

Hm.  Since hypercall numbers are shared with i386, we can't have >4G of 
them.  So the hypercall number should be redefined to be in eax, 
regardless of arch (while the arguments still are unsigned longs).

>> +static inline int kvm_para_available(void)
>> +{
>> +unsigned int eax, ebx, ecx, edx;
>> +char signature[13];
>> +
>> +cpuid(KVM_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
>> +memcpy(signature + 0, &ebx, 4);
>> +memcpy(signature + 4, &ecx, 4);
>> +memcpy(signature + 8, &edx, 4);
>> 
>
>   
>> +signature[12] = 0;
>> +
>> 
>ebx|ecx|ed
>   
>> +if (strcmp(signature, "KVMKVMKVM") == 0)
>> +return 1;
>> 
>
> Should the comment say
>
>   
>> +/* This CPUID returns the signature 'KVMKVMKVM\0\0\0' in ebx, ecx, and edx. 
>>  It
>> + * should be used to determine that a VM is running under KVM.
>> + */
>> 
>
> Plus, I'd use memcmp, and actually test for those zeros, too.
>
> ...which probably can be done later, as this is pure move...
>   Pavel
>   

Or maybe direct 32-bit compares, and document the 32-bit values of the 
registers directly, to avoid any chance of confusion.

Thanks for the comments, I'll update the patches.


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


-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH 06/55] KVM: Per-architecture hypercall definitions

2007-12-26 Thread Pavel Machek
Hi!

> Currently kvm provides hypercalls only for x86* architectures. To
> provide hypercall infrastructure for other kvm architectures I split
> kvm_para.h into a generic header file and architecture specific
> definitions.
> 
> Signed-off-by: Christian Borntraeger <[EMAIL PROTECTED]>
> Signed-off-by: Avi Kivity <[EMAIL PROTECTED]>
> ---
>  include/asm-x86/kvm_para.h |  105 
> 
>  include/linux/kvm_para.h   |  105 
> +---
>  2 files changed, 117 insertions(+), 93 deletions(-)
>  create mode 100644 include/asm-x86/kvm_para.h
> 
> diff --git a/include/asm-x86/kvm_para.h b/include/asm-x86/kvm_para.h
> new file mode 100644
> index 000..c6f3fd8
> --- /dev/null
> +++ b/include/asm-x86/kvm_para.h
> @@ -0,0 +1,105 @@
> +#ifndef __X86_KVM_PARA_H
> +#define __X86_KVM_PARA_H
> +
> +/* This CPUID returns the signature 'KVMKVMKVM' in ebx, ecx, and edx.  It
> + * should be used to determine that a VM is running under KVM.
> + */
> +#define KVM_CPUID_SIGNATURE  0x4000

so it returns 'KVMKVMKVM' in %rax, too? 

> +/* For KVM hypercalls, a three-byte sequence of either the vmrun or the 
> vmmrun
> + * instruction.  The hypervisor may replace it with something else but only 
> the
> + * instructions are guaranteed to be supported.
> + *
> + * Up to four arguments may be passed in rbx, rcx, rdx, and rsi respectively.
> + * The hypercall number should be placed in rax and the return

rax? First, this file is shared with i386, AFAICT.

> + * placed in rax.  No other registers will be clobbered unless explicited
> + * noted by the particular hypercall.
> + */
> +
> +static inline long kvm_hypercall0(unsigned int nr)
> +{
> + long ret;
> + asm volatile(KVM_HYPERCALL
> +  : "=a"(ret)
> +  : "a"(nr));

Second, if it is to be placed in rax, nr should be unsigned long?


> +static inline int kvm_para_available(void)
> +{
> + unsigned int eax, ebx, ecx, edx;
> + char signature[13];
> +
> + cpuid(KVM_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
> + memcpy(signature + 0, &ebx, 4);
> + memcpy(signature + 4, &ecx, 4);
> + memcpy(signature + 8, &edx, 4);

> + signature[12] = 0;
> +
   ebx|ecx|ed
> + if (strcmp(signature, "KVMKVMKVM") == 0)
> + return 1;

Should the comment say

> +/* This CPUID returns the signature 'KVMKVMKVM\0\0\0' in ebx, ecx, and edx.  
> It
> + * should be used to determine that a VM is running under KVM.
> + */

Plus, I'd use memcmp, and actually test for those zeros, too.

...which probably can be done later, as this is pure move...
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


[kvm-devel] [PATCH 06/55] KVM: Per-architecture hypercall definitions

2007-12-26 Thread Avi Kivity
From: Christian Borntraeger <[EMAIL PROTECTED]>

Currently kvm provides hypercalls only for x86* architectures. To
provide hypercall infrastructure for other kvm architectures I split
kvm_para.h into a generic header file and architecture specific
definitions.

Signed-off-by: Christian Borntraeger <[EMAIL PROTECTED]>
Signed-off-by: Avi Kivity <[EMAIL PROTECTED]>
---
 include/asm-x86/kvm_para.h |  105 
 include/linux/kvm_para.h   |  105 +---
 2 files changed, 117 insertions(+), 93 deletions(-)
 create mode 100644 include/asm-x86/kvm_para.h

diff --git a/include/asm-x86/kvm_para.h b/include/asm-x86/kvm_para.h
new file mode 100644
index 000..c6f3fd8
--- /dev/null
+++ b/include/asm-x86/kvm_para.h
@@ -0,0 +1,105 @@
+#ifndef __X86_KVM_PARA_H
+#define __X86_KVM_PARA_H
+
+/* This CPUID returns the signature 'KVMKVMKVM' in ebx, ecx, and edx.  It
+ * should be used to determine that a VM is running under KVM.
+ */
+#define KVM_CPUID_SIGNATURE0x4000
+
+/* This CPUID returns a feature bitmap in eax.  Before enabling a particular
+ * paravirtualization, the appropriate feature bit should be checked.
+ */
+#define KVM_CPUID_FEATURES 0x4001
+
+#ifdef __KERNEL__
+#include 
+
+/* This instruction is vmcall.  On non-VT architectures, it will generate a
+ * trap that we will then rewrite to the appropriate instruction.
+ */
+#define KVM_HYPERCALL ".byte 0x0f,0x01,0xc1"
+
+/* For KVM hypercalls, a three-byte sequence of either the vmrun or the vmmrun
+ * instruction.  The hypervisor may replace it with something else but only the
+ * instructions are guaranteed to be supported.
+ *
+ * Up to four arguments may be passed in rbx, rcx, rdx, and rsi respectively.
+ * The hypercall number should be placed in rax and the return value will be
+ * placed in rax.  No other registers will be clobbered unless explicited
+ * noted by the particular hypercall.
+ */
+
+static inline long kvm_hypercall0(unsigned int nr)
+{
+   long ret;
+   asm volatile(KVM_HYPERCALL
+: "=a"(ret)
+: "a"(nr));
+   return ret;
+}
+
+static inline long kvm_hypercall1(unsigned int nr, unsigned long p1)
+{
+   long ret;
+   asm volatile(KVM_HYPERCALL
+: "=a"(ret)
+: "a"(nr), "b"(p1));
+   return ret;
+}
+
+static inline long kvm_hypercall2(unsigned int nr, unsigned long p1,
+ unsigned long p2)
+{
+   long ret;
+   asm volatile(KVM_HYPERCALL
+: "=a"(ret)
+: "a"(nr), "b"(p1), "c"(p2));
+   return ret;
+}
+
+static inline long kvm_hypercall3(unsigned int nr, unsigned long p1,
+ unsigned long p2, unsigned long p3)
+{
+   long ret;
+   asm volatile(KVM_HYPERCALL
+: "=a"(ret)
+: "a"(nr), "b"(p1), "c"(p2), "d"(p3));
+   return ret;
+}
+
+static inline long kvm_hypercall4(unsigned int nr, unsigned long p1,
+ unsigned long p2, unsigned long p3,
+ unsigned long p4)
+{
+   long ret;
+   asm volatile(KVM_HYPERCALL
+: "=a"(ret)
+: "a"(nr), "b"(p1), "c"(p2), "d"(p3), "S"(p4));
+   return ret;
+}
+
+static inline int kvm_para_available(void)
+{
+   unsigned int eax, ebx, ecx, edx;
+   char signature[13];
+
+   cpuid(KVM_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
+   memcpy(signature + 0, &ebx, 4);
+   memcpy(signature + 4, &ecx, 4);
+   memcpy(signature + 8, &edx, 4);
+   signature[12] = 0;
+
+   if (strcmp(signature, "KVMKVMKVM") == 0)
+   return 1;
+
+   return 0;
+}
+
+static inline unsigned int kvm_arch_para_features(void)
+{
+   return cpuid_eax(KVM_CPUID_FEATURES);
+}
+
+#endif
+
+#endif
diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
index cc5dfb4..e4db25f 100644
--- a/include/linux/kvm_para.h
+++ b/include/linux/kvm_para.h
@@ -1,110 +1,29 @@
 #ifndef __LINUX_KVM_PARA_H
 #define __LINUX_KVM_PARA_H
 
-/* This CPUID returns the signature 'KVMKVMKVM' in ebx, ecx, and edx.  It
- * should be used to determine that a VM is running under KVM.
+/*
+ * This header file provides a method for making a hypercall to the host
+ * Architectures should define:
+ * - kvm_hypercall0, kvm_hypercall1...
+ * - kvm_arch_para_features
+ * - kvm_para_available
  */
-#define KVM_CPUID_SIGNATURE0x4000
-
-/* This CPUID returns a feature bitmap in eax.  Before enabling a particular
- * paravirtualization, the appropriate feature bit should be checked.
- */
-#define KVM_CPUID_FEATURES 0x4001
 
 /* Return values for hypercalls */
 #define KVM_ENOSYS 1000
 
 #ifdef __KERNEL__
-#include 
-
-/* This instruction is vmcall.  On non-VT architectures, it will generate a
- * trap that we will then rewrite to the appropriate instru