From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 1/5] vring: Replace mmap() interface with ioctl() Date: Fri, 13 Jun 2008 17:09:54 +0300 Message-ID: <48527FB2.1060409@qumranet.com> References: <1213365481-23460-1-git-send-email-markmc@redhat.com> <1213365481-23460-2-git-send-email-markmc@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Anthony Liguori , Rusty Russell , kvm@vger.kernel.org To: Mark McLoughlin Return-path: Received: from il.qumranet.com ([212.179.150.194]:18069 "EHLO il.qumranet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751267AbYFMOJ7 (ORCPT ); Fri, 13 Jun 2008 10:09:59 -0400 In-Reply-To: <1213365481-23460-2-git-send-email-markmc@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Mark McLoughlin wrote: > /dev/vring's mmap() interface is a strange creature. It > serves as a way for userland to supply the address of the > already allocated ring descriptors, but causes those pages > to be re-maped as a natural side effect of the mmap() > > This is not an issue for lguest because it does the mmap() > before even starting the guest. However, in the case of kvm, > the guest allocates the ring and informs the host of its > addresss. If we then mmap() it, we cause it to be remapped > to new pages which the vring driver will then use. > > Now, KVM guests don't actually use the ring pages before > informing the host of its address, so we could probably just > invalidate the guest's shadow page table and have the new > pfns picked up. That would be an odd requirement to impose > on the guest ABI, though. > > Since the mmap() semantics are so strange, switch to using a > single ioctl() for setting up the ring. > > Definitely an improvement; iterfaces which seem familiar but aren't are not user friendly. In any case the guest can't use the ring directly since physical memory is discontiguous. > - .ioctl = vring_ioctl, > + .unlocked_ioctl = vring_ioctl, > + .compat_ioctl = vring_compat_ioctl, > I think you can set compat_ioctl = vring_ioctl (that's what kvm does). > diff --git a/include/linux/vring.h b/include/linux/vring.h > index 47c8848..de4125d 100644 > --- a/include/linux/vring.h > +++ b/include/linux/vring.h > @@ -21,8 +21,14 @@ > #include > > /* Ioctl defines. */ > -#define VRINGSETBASE _IO(0xAD, 0) > -#define VRINGSETLIMIT _IO(0xAD, 1) > +#define VRINGSETINFO _IO(0xAD, 0) > + > +struct vring_ioctl_info { > + __u16 num_descs; > Padding for 64-bits here, otherwise compat_ioctl breaks. > + __u64 descs; > + __u64 base; > + __u64 limit; > +}; -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.