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@xxxxxxxxxxxxxxxxxx> >>> >>> 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@xxxxxxxxxxxxxxxxxx> >>> Reviewed-by: David Hildenbrand <dahi@xxxxxxxxxxxxxxxxxx> >> >> I like it. >> >> Reviewed-by: Alexander Graf <agraf@xxxxxxx> > > 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? Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
