From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Borntraeger Subject: Re: [PATCH 1/1] kvm-s390: Provide guest TOD Clock Get/Set Controls Date: Wed, 05 Nov 2014 18:56:43 +0100 Message-ID: <545A64DB.6000100@de.ibm.com> References: <1414424654-21946-1-git-send-email-jjherne@linux.vnet.ibm.com> <1414424654-21946-2-git-send-email-jjherne@linux.vnet.ibm.com> <5459F6E0.8080400@suse.de> <545A17EF.4090004@de.ibm.com> <545A2209.9010603@redhat.com> <545A54FA.5040802@de.ibm.com> <545A604C.3010202@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit To: Alexander Graf , Paolo Bonzini , "Jason J. Herne" , kvm@vger.kernel.org, cornelia.huck@de.ibm.com, aik@ozlabs.ru Return-path: Received: from e06smtp14.uk.ibm.com ([195.75.94.110]:51100 "EHLO e06smtp14.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751976AbaKER4t (ORCPT ); Wed, 5 Nov 2014 12:56:49 -0500 Received: from /spool/local by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 5 Nov 2014 17:56:47 -0000 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 6D30D1B08070 for ; Wed, 5 Nov 2014 17:56:52 +0000 (GMT) Received: from d06av12.portsmouth.uk.ibm.com (d06av12.portsmouth.uk.ibm.com [9.149.37.247]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id sA5HukDl16384312 for ; Wed, 5 Nov 2014 17:56:46 GMT Received: from d06av12.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av12.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id sA5HuijC004792 for ; Wed, 5 Nov 2014 10:56:45 -0700 In-Reply-To: <545A604C.3010202@suse.de> Sender: kvm-owner@vger.kernel.org List-ID: 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" >>>>>> >>>>>> 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 >>>>>> Reviewed-by: David Hildenbrand >>>>> >>>>> I like it. >>>>> >>>>> Reviewed-by: Alexander Graf >>>> >>>> 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.