From mboxrd@z Thu Jan  1 00:00:00 1970
From: Laurent Vivier <Laurent.Vivier@bull.net>
Subject: Re: [PATCH 1/7] libkvm: delayed MMIO support (common part)
Date: Mon, 26 May 2008 16:15:33 +0200
Message-ID: <1211811333.3908.26.camel@frecb07144>
References: <12115326462681-git-send-email-Laurent.Vivier@bull.net>
	 <12115326463363-git-send-email-Laurent.Vivier@bull.net>
	 <483AC161.2010804@qumranet.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: kvm@vger.kernel.org
To: Avi Kivity <avi@qumranet.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from ecfrec.frec.bull.fr ([129.183.4.8]:57395 "EHLO
	ecfrec.frec.bull.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752059AbYEZOPJ (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 26 May 2008 10:15:09 -0400
In-Reply-To: <483AC161.2010804@qumranet.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Le lundi 26 mai 2008 =C3=A0 16:55 +0300, Avi Kivity a =C3=A9crit :
> Laurent Vivier wrote:
> > This patch introduces in libkvm the common part of MMIO batching in=
terface.
> >
> > It checks the MMIO batching availability with ioctl(KVM_CHECK_EXTEN=
SION).
> > If KVM_CAP_DELAYED_MMIO is available, it processes the MMIO ring bu=
ffer
> > at the return of ioctl(KVM_RUN).
> > It defines kvm_register_delayed_mmio() to register a delayed MMIO z=
one, and=20
> > kvm_unregister_delayed_mmio() to unregister it.
> >
> > =20
> >  void init_slots(void);
> > diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
> > index d1e95a4..718f4c6 100644
> > --- a/libkvm/libkvm.c
> > +++ b/libkvm/libkvm.c
> > @@ -879,6 +879,20 @@ again:
> > =20
> >  	post_kvm_run(kvm, vcpu);
> > =20
> > +#if defined(KVM_CAP_DELAYED_MMIO)
> > +	if (kvm->delayed_mmio) {
> > +	        struct kvm_batch *batch =3D (void *)run +
> > +					  kvm->delayed_mmio * PAGE_SIZE;
> > +		while (batch->first !=3D batch->last) {
> > +			kvm->callbacks->mmio_write(kvm->opaque,
> > +					    batch->mmio[batch->first].phys_addr,
> > +					   &batch->mmio[batch->first].data[0],
> > +					    batch->mmio[batch->first].len);
> > +			batch->first =3D (batch->first + 1) % KVM_MAX_BATCH;
> >  =20
>=20
> There needs to be a write barrier after this (smp_wmb()), to avoid=20
> compiler and cpu reordering.  Especially important for non-x86.  Also=
=20
> need a read barrier before reading batch->first.
>=20
> Or maybe not, likely the pthread_mutex_lock() acts as the correct=20
> barrier.  But we do need the equivalent barriers in the kernel code=20
> (missed it on that review).

OK

> > +		}
> > +	}
> > +#endif
> > +
> >  	if (r =3D=3D -1) {
> >  		r =3D handle_io_window(kvm);
> >  		goto more;
> >  =20
>=20
> > +
> > +int kvm_register_delayed_mmio(kvm_context_t kvm, uint64_t addr, ui=
nt32_t size)
> > +{
> > +#ifdef KVM_CAP_DELAYED_MMIO
> > +	struct kvm_delayed_mmio_zone zone;
> > +	int r;
> > +
> > +	if (kvm->delayed_mmio) {
> > +
> > +		zone.addr =3D addr;
> > +		zone.size =3D size;
> > +
> > +		r =3D ioctl(kvm->vm_fd, KVM_REGISTER_DELAYED_MMIO, &zone);
> > +		if (r =3D=3D -1) {
> > +			perror("kvm_register_delayed_mmio_zone");
> > +			return -errno;
> > +		}
> > +		return 0;
> > +	}
> > +#endif
> > +	return -1;
> >  =20
>=20
> -ENOSYS, or something.  -1 is -ENOPERM.

OK

>=20
> > +}
> > +
> > +int kvm_unregister_delayed_mmio(kvm_context_t kvm, uint64_t addr, =
uint32_t size)
> > +{
> > +#ifdef KVM_CAP_DELAYED_MMIO
> > +	struct kvm_delayed_mmio_zone zone;
> > +	int r;
> > +
> > +	if (kvm->delayed_mmio) {
> > +
> > +		zone.addr =3D addr;
> > +		zone.size =3D size;
> > +
> > +		r =3D ioctl(kvm->vm_fd, KVM_UNREGISTER_DELAYED_MMIO, &zone);
> > +		if (r =3D=3D -1) {
> > +			perror("kvm_unregister_delayed_mmio_zone");
> > +			return -errno;
> > +		}
> > +		return 0;
> > +	}
> > +#endif
> > +	return -1;
> >  =20
>=20
> ditto.

OK
--=20
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay