Re: [PATCH 5/5] KVM: paravirt time source

[Anthony Liguori <anthony-rdkfGonbjUSkNkDKm+mE6A@public.gmane.org>]

Jeremy Fitzhardinge wrote:
> Anthony Liguori wrote:
>   
>> +static cycle_t read_hyper(void)
>> +{
>> +	struct timespec now;
>> +	int ret;
>> +
>> +	ret = kvm_hypercall(KVM_HYPERCALL_GET_KTIME, (u32)&now, 0, 0, 0);
>> +	WARN_ON(ret);
>> +
>> +	return now.tv_nsec + now.tv_sec * (cycles_t)1e9;
>>   
>>     
>
> Hm, use of FP looks pretty odd.  I guess its OK to assume the compiler
> will completely remove all the FP stuff at compile time.  Or you could
> use NSEC_PER_SEC.
>   

Agreed.

>> +}
>> +
>> +static struct clocksource clocksource_hyper = {
>> +	.name			= "hyper",
>> +	.rating			= 200,
>>   
>>     
>
> We should probably standardize on this.  I guess that if you're in a
> paravirt environment, and there's a paravirt clocksource, that would
> always be the best clocksource to use.
>
>   
>> +	.read			= read_hyper,
>> +	.mask			= CLOCKSOURCE_MASK(64),
>> +	.mult			= 1,
>> +	.shift			= 0,
>>   
>>     
>
> It would be better to use a scale and shift here, so that adjtime has
> something to work with when warping time.
>   

Okay.  I may remove this patch from the patch series and attempt to sit 
down next week and work out something more complete that also implements 
stolen time accounting.

Regards,

Anthony Liguori

>     J
>
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/