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 17:48:58 +0100 Message-ID: <545A54FA.5040802@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> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit To: Paolo Bonzini , Alexander Graf , "Jason J. Herne" , kvm@vger.kernel.org, cornelia.huck@de.ibm.com, aik@ozlabs.ru Return-path: Received: from e06smtp11.uk.ibm.com ([195.75.94.107]:56397 "EHLO e06smtp11.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751805AbaKEQtF (ORCPT ); Wed, 5 Nov 2014 11:49:05 -0500 Received: from /spool/local by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 5 Nov 2014 16:49:03 -0000 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id A8B4817D8045 for ; Wed, 5 Nov 2014 16:49:08 +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 sA5Gn24T10617334 for ; Wed, 5 Nov 2014 16:49:02 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 sA5GmxXh018552 for ; Wed, 5 Nov 2014 09:49:01 -0700 In-Reply-To: <545A2209.9010603@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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? - Just one 64bit value today and the other one later or both now (64+8) Christian