Am 05.11.2014 18:37, schrieb Alexander Graf: > > > On 05.11.14 17:48, Christian Borntraeger wrote: >> 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@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? >>> >> >> 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? > > On PPC we have core granularity, so while the interface is on one vcpu, > it really only affects every 8th vcpu (or whatever you configure the > number of threads as). > > So there is precedence for an interface that modifies other vcpus while > the ONE_REG is only targeting a single vcpu. > > Whether you want to follow that approach or do it as VM attribute > straight away, I don't mind much :). given that top programmable field and epoch are available as ONEREG, lets do the same for TOD. > >> - Just one 64bit value today and the other one later or both now (64+8) > > Make it both today with a check that the second one has to be 0 maybe? > Then we only need to modify the kernel itself, not the API later. Makes sense. -- 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
