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

[Christian Borntraeger <borntraeger@de.ibm.com>]

Am 05.11.2014 14:11, schrieb Paolo Bonzini:
> 
> 
> 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?
> 

I think the change to a ONEREG should be trivial. (it will be synced between all guest CPUs, so we could also use a VM attribute but a ONEREG should be ok as well.

I think two registers (one 64bit and another 8bit register (which must be 0 all the time as of today) is preferred.

I think we could even defer the 2nd register until we know what the hardware folks will come up with before 2042. (stcke in the POP indicates an 8bit counter).

So Paolo, Alex two simple questions:

- ONEREG or VM attribute?
- Just one 64bit value today and the other one later or both now (64+8)

Christian