From owner-freebsd-virtualization@FreeBSD.ORG Wed Jan 15 13:45:37 2014 Return-Path: Delivered-To: freebsd-virtualization@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id B53AF5D4 for ; Wed, 15 Jan 2014 13:45:37 +0000 (UTC) Received: from os.inf.tu-dresden.de (os.inf.tu-dresden.de [IPv6:2002:8d4c:3001:48::99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 790CF14FE for ; Wed, 15 Jan 2014 13:45:37 +0000 (UTC) Received: from [178.5.117.7] (helo=[192.168.178.35]) by os.inf.tu-dresden.de with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82) id 1W3Qmk-0006xJ-Et; Wed, 15 Jan 2014 14:45:34 +0100 Message-ID: <1389793530.2668.13.camel@janus.xn--pl-wia.net> Subject: Re: KVM Clock From: Julian Stecklina To: Roger Pau =?ISO-8859-1?Q?Monn=E9?= Date: Wed, 15 Jan 2014 14:45:30 +0100 In-Reply-To: <52D68831.2000503@citrix.com> References: <52D5623C.7060401@freebsd.org> <52D68831.2000503@citrix.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.3 (3.10.3-1.fc20) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Cc: freebsd-virtualization@freebsd.org X-BeenThere: freebsd-virtualization@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Discussion of various virtualization techniques FreeBSD supports." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Jan 2014 13:45:37 -0000 On Mi, 2014-01-15 at 14:08 +0100, Roger Pau Monné wrote: > On 15/01/14 13:05, Julian Stecklina wrote: > > On 01/14/2014 05:13 PM, Peter Grehan wrote: > >> Hi Julian, > >> > >>> is anyone working on KVM clock support for FreeBSD? If not, I > >>> might take a shot at it. > >> > >> None I know of: go for it :) > > > > Works for me so far: > > https://github.com/blitz/freebsd/commit/cdc5f872b3e48cc0dda031fc7d6bdedc65c3148f > > Looking > > > at the code it seems some common routines could be shared > between the KVM PV clock and the Xen PV clock > (sys/dev/xen/timer/timer.c). The data passed from the hypervisor to > the guest has exactly the same format (see struct vcpu_time_info in > Xen public headers). Yes, I know. Didn't get around to making it pretty yesterday evening. ;) I'll post an updated patch, when I have some time. Any suggestions where to put the two common functions? > At a first sight the KVM clock can benefit from using scale_delta > (which is going to be faster than the C version implemented in > kvmclock_get_timecount), At least somewhat on amd64. 32-bit looks pretty identical. > and xen_fetch_vcpu_tinfo is exactly the same > as kvmclock_fetch. I think xen_fetch_vcpu_tinfo has a subtle bug: 217 do { 218 dst->version = src->version; 219 rmb(); 220 dst->tsc_timestamp = src->tsc_timestamp; 221 dst->system_time = src->system_time; 222 dst->tsc_to_system_mul = src->tsc_to_system_mul; 223 dst->tsc_shift = src->tsc_shift; 224 rmb(); 225 } while ((src->version & 1) | (dst->version ^ src->version)); In line 225 src->version is potentially read twice. If you consider the following schedule: host starts updating data, sets src->version to 1 guest reads src->version (1) and stores it into dst->version. guest copies inconsistent data guest reads src->version (1) and computes xor with dst->version. host finishes updating data and sets src->version to 2 guest reads src->version (2) and checks whether lower bit is not set. while loop exits with inconsistent data! I think the C standard (at least C11) permits this as it does not specify in which order the two reads in line 225 need to happen. Julian