From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH 1/1] kvm-s390: Provide guest TOD Clock Get/Set Controls Date: Wed, 05 Nov 2014 14:11:37 +0100 Message-ID: <545A2209.9010603@redhat.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> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit To: Christian Borntraeger , Alexander Graf , "Jason J. Herne" , kvm@vger.kernel.org, cornelia.huck@de.ibm.com, aik@ozlabs.ru Return-path: Received: from mail-lb0-f176.google.com ([209.85.217.176]:56696 "EHLO mail-lb0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753988AbaKENLn (ORCPT ); Wed, 5 Nov 2014 08:11:43 -0500 Received: by mail-lb0-f176.google.com with SMTP id 10so612996lbg.35 for ; Wed, 05 Nov 2014 05:11:41 -0800 (PST) In-Reply-To: <545A17EF.4090004@de.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: 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? Paolo