[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 0000000..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 0x40000000
>>
>
> 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/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
