lkml.org 
[lkml]   [2007]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 06/55] KVM: Per-architecture hypercall definitions
[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



\
 
 \ /
  Last update: 2007-12-27 08:05    [W:0.099 / U:1.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site