Re: [patch] KVM: add MSR based hypercall API

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Ingo Molnar wrote:
> * Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote:
>
>   
>>> Does this look good to you? I'd like the basic API to be as light as 
>>> possible.
>>>       
>> Won't 32-bit and 64-bit pick different registers?
>>
>> We can work around it (call is_long_mode() when decoding the 
>> hypercall), but it kind of defeats the purpose of the optimization, 
>> no?
>>     
>
> well, we can standardize on the 32-bit calling convention: eax, ecx, 
> edx, ebp, etc. We can do that via the 64-bit asm. So it should be the 
> same i think - just that a 32-bit guest on a 64-bit host wont be able to 
> set the high bits of those registers.
>   

That uglifies 64-bit at the expense of 32-bit.  I'd prefer it to be the 
other way round, but it's not really an issue either way.

In any case, it needs to be documented, as other guests may not use gcc 
or regparm.

> I've attached my current patch (ontop of tarball) - that's one idea 
> about how it could look like. Note that i didnt introduce a syscall 
> function table in kvm_handle_hypercall() - this will be the more optimal 
> solution until the # of hypercalls is relatively low. That way we only 
> prepare the parameters that are truly needed.
>
> you are right in that we cannot call the syscall functions directly and 
> that in practice we'll shuffle things around - but we have to check the 
> first parameter anyway, and the shuffling isnt /that/ big of a problem. 
> I wanted to keep the guest-side as low-impact as possible, so that a 
> native kernel's instruction sequence is not disturbed too much by the 
> presence of a NOP hypercall.
>   

Yes, that makes sense.

> in fact it would probably be more logical to use the standard syscall 
> order: eax, ebx, ecx, edx, esi, edi, ebp?
>   

Even better.  It allows more registers and avoids a random gcc dependency.

> +#define hypercall1(nr)				\
> +({						\
> +	int __ret;				\
> +						\
> +	asm (" call hypercall_addr\n"		\
> +		: "=g" (__ret)			\
> +		: "eax" (nr)			\
> +	);					\
> +	__ret;					\
> +})
>   
shouldn't that be

  asm ("call hypercall_addr" : "=a"(__ret) : "a"(nr))

?

I don't think "eax" is a valid asm constraint, and we need to specify 
eax as the return register.


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


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV