Re: [RFC] Paravirt timer for KVM

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

Glauber de Oliveira Costa wrote:
>>>       
>> Here you're passing a virtual address across the guest/host interface,
>> which is something we've previously agreed is bad.
>>     
> And that's the reason for the "nothing". It was just the easier way,
> and I'm not aware of any discussion in which it was agreed bad. I'll
> dig into the archives, but it would be a lot easier if you give me
> some pointers ;-)
>   

On x86, translating a virtual address in the hypervisor is merely
annoying.  On architectures which don't have hardware-walked page tables
(merely software-managed tlbs), translation is impossible.  So when you
talk to the hypervisor, use physical addresses exclusively.

>   
>> Are you intending this shared page to contain other data in the future,
>> or is it just poorly named?
>>     
>
> Well, this is still a work in progress, so the accurate answer should
> be: Something in the middle, between "more data", and "poorly named".
>
> I think more data can be in, but right now, everything I intend to
> (maybe) put in is timer related, so a rename could go well anyway.
>
>   

Having granular interfaces (one cpuid bit, one shared data structure per
feature is the best way of insulating ourselves against ABI
"accidents".  If we find out later the interface is bad, we throw it
away and implement a new one, without affecting others.

>>> +struct kvm_hv_clock {
>>> +       int stable_tsc; /* use raw tsc for clock_read */
>>> +       unsigned long tsc_mult;
>>> +       struct timespec now;
>>> +       u64 last_tsc;
>>> +       /* That's the wall clock, not the water closet */
>>> +       struct timespec wc;
>>> +};
>>>       
>> This isn't even good for x86: you're using "long" in a binary interface!
>> You're also using timespec, and what sort of binary compatibility
>> guarantees would you like to make about that?
>>
>>     
>
> Binary guarantees goes in version two ;-)
> But c'mon, now that I'm starting to be able to make some merely
> decents puns in english, I put one very well placed there, and you
> come telling me about binary compatiblity?
>
> Now seriously, the long is just my bad, but I'm not aware of any
> complications regarding the timespec. You are welcome to enlighten me.
> (Meanwhile, I'll go look at it)
>   

We want this to be usable in non-Linux and cross-bitness.  So:
- all primitive data types are __u32, __u64 and friends (not u32 or u64)
- all structures and constants are prefixed with kvm_
- all fields are naturally aligned
- add padding on structures larger than 8 bytes to be a multiple of 8 bytes

Following these simple rules is the key to health and happiness.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/