Re: [PATCH 1/1] kvm-s390: Provide guest TOD Clock Get/Set Controls

[Alexander Graf <agraf@suse.de>]

On 05.11.14 14:11, Paolo Bonzini wrote:
> 
> 
> On 05/11/2014 13:28, Christian Borntraeger wrote:
>> Am 05.11.2014 11:07, schrieb Alexander Graf:
>>>
>>>
>>> On 27.10.14 16:44, Jason J. Herne wrote:
>>>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>>>
>>>> Enable KVM_SET_CLOCK and KVM_GET_CLOCK ioctls on s390 for
>>>> managing guest Time Of Day clock value.
>>>>
>>>> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> 
>>>> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
>>>
>>> I like it.
>>>
>>> Reviewed-by: Alexander Graf <agraf@suse.de>
>>
>> Paolo, are you ok with that patch as well? If yes I will send it with
>> the next bunch of s390 patches.
>>
>> PS: I remember that you were considering some different take on the
>> interface: IIRC you suggest to have the same format in
>> kvm_clock_data->clock as x86, and that we might want to use a flag
>> and a new field in the padding area that then contains the TOD value.
>> Now looking again at Documentation/virtual/kvm/api.txt I actually
>> prefer Jasons implementation since the api does not mention the
>> value/format/offset. It seems to be ns since boot, correct?
>>
>> So if any changes, I would prefer a small change to the
>> documentation, that makes the meaning of clock explicit per
>> architecture?
> 
> After a quick refresh on IRC, I remembered our previous discussion.
> 
> I was a bit worried that the interface did not let us pass the extra
> byte for the stcke instruction's overflow counter.  The question then is
> whether to:
> 
> 1) keep an x86-consistent interface for KVM_GET/SET_CLOCK, and put the
> whole 16 byte stcke output in the padding
> 
> 2) put the 8-byte stck value (stcke bytes 1-8) in the value, and the
> overflow counter (stcke byte 0) in the padding (with the presence
> governed by a flag).  As you explained, bytes 9-13 are computed by the
> CPU and we do not care anyway of accuracy beyond 0.25 ns, while bytes
> 14-15 are accessed separately via ONEREG.
> 
> 3) use ONEREG instead of KVM_GET/SET_CLOCK.  You can decide whether to
> use a 72 (or 96) bit value, or two separate 8+64 values.
> 
> 1 or 3 seem the cleanest.  On the other hand s390 doesn't have a use for
> a bootbased counter, which makes 1 much less interesting/useful than I
> imagined.
> 
> PPC uses a combination of KVM_GET_SREGS and KVM_GET/SET_ONEREG for the
> closest equivalent (TBL/TBU), not KVM_GET/SET_CLOCK.  MIPS is also
> ONEREG-based.  This makes me lean towards 3.
> 
> Of course 2 has code written, but it should be a small change to use
> ONEREG instead.  What do you think?

How far does the existing nanosecond number get us until we hit the
64bit limit? And by the time we hit it, wouldn't we hit it on x86 as well?


Alex