Re: [PATCH] tsc: use kvmclock for calibration

[Avi Kivity <avi@redhat.com>]

On 08/10/2012 11:10 AM, Gerd Hoffmann wrote:
>   Hi,
> 
>>>> >>>   (1) Use this patch (with alignment issue fixed of course).
>>>> >>>   (2) Do a full kvmclock implementation.  Feels a bit like overkill.
>>>> >>>   (3) SeaBIOS can fallback to the PIT for timing on machines which
>>>> >>>       have no TSC.  We could do that too in case we detect kvm ...
>>> >>
>>> >> What sort of timeouts are these?  If seconds, maybe the rtc would be best.
>> > 
>> > I vote for 3 so nobody has to maintain kvmclock code in SeaBIOS and Gerd
>> > can fix the in-kernel PIT issues with GRUB (see Michaels message) while testing.
> (2) turned out to be not too bad when taking a shortcut: Go through an
> enable/disable cycle each time we read the clock, then just grab
> system_time.  Not that efficient, but should be ok for seabios.  Usually
> it checks the clock when sitting around idle, waiting for something to
> happen.  And it simplifies the implementation alot as we can just skip
> all the tsc frequency & delta calculations.
> 
> Draft patch attached.  Comments?
> 
> +
> +static void kvmclock_fetch(struct pvclock_vcpu_time_info *time)
> +{
> +    u32 addr = (u32)MAKE_FLATPTR(GET_SEG(SS), time);
> +    u32 msr = GET_GLOBAL(kvm_systime_msr);
> +
> +    memset(time, 0, sizeof(*time));
> +    wrmsr(msr, addr | 1);

I'd put the time calculations in here.  We don't specify what happens to
the data area after disabling kvmclock; it could be in the middle of an
update.

> +    wrmsr(msr, 0);
> +}
> +
> +u64 kvmclock_get(void)
> +{
> +    struct pvclock_vcpu_time_info time;
> +
> +    kvmclock_fetch(&time);
> +    return time.system_time;

That's just a random number.  You have to do the full calculation.


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