From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=56761 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OLBxN-00061D-Gt for qemu-devel@nongnu.org; Sun, 06 Jun 2010 05:15:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OLBxM-00061k-7x for qemu-devel@nongnu.org; Sun, 06 Jun 2010 05:15:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:61146) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OLBxM-00061O-1G for qemu-devel@nongnu.org; Sun, 06 Jun 2010 05:15:48 -0400 Date: Sun, 6 Jun 2010 12:11:22 +0300 From: "Michael S. Tsirkin" Message-ID: <20100606091122.GC22599@redhat.com> References: <201006042046.49872.rusty@rustcorp.com.au> <20100604114205.GA22599@redhat.com> <201006051340.27194.rusty@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201006051340.27194.rusty@rustcorp.com.au> Subject: [Qemu-devel] Re: [PATCHv3 1/2] virtio: support layout with avail ring before idx List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Rusty Russell Cc: qemu-devel@nongnu.org, Andrew Morton , linux-kernel@vger.kernel.org, kvm@vger.kernel.org, virtualization@lists.linux-foundation.org On Sat, Jun 05, 2010 at 01:40:26PM +0930, Rusty Russell wrote: > On Fri, 4 Jun 2010 09:12:05 pm Michael S. Tsirkin wrote: > > On Fri, Jun 04, 2010 at 08:46:49PM +0930, Rusty Russell wrote: > > > I'm uncomfortable with moving a field. > > > > > > We haven't done that before and I wonder what will break with old code. > > > > With e.g. my patch, We only do this conditionally when bit is negotitated. > > Of course, but see this change: > > commit ef688e151c00e5d529703be9a04fd506df8bc54e > Author: Rusty Russell > Date: Fri Jun 12 22:16:35 2009 -0600 > > virtio: meet virtio spec by finalizing features before using device > > Virtio devices are supposed to negotiate features before they start using > the device, but the current code doesn't do this. This is because the > driver's probe() function invariably has to add buffers to a virtqueue, > or probe the disk (virtio_blk). > > This currently doesn't matter since no existing backend is strict about > the feature negotiation. But it's possible to imagine a future feature > which completely changes how a device operates: in this case, we'd need > to acknowledge it before using the device. > > Signed-off-by: Rusty Russell > > Now, this isn't impossible to overcome: we know that if they use the ring > before completing feature negotiation then they don't understand the new > format. > > But we have to be aware of that on the qemu side. Are we? I think we are ok. virtqueue_init which sets the avail/ysed pointers is called when we write the base address. So we only need to be careful and not change this feature bit after creating the rings. > > > Should we instead just abandon the flags field and use last_used only? > > > Or, more radically, put flags == last_used when the feature is on? > > > > > > Thoughts? > > > Rusty. > > > > Hmm, e.g. with TX and virtio net, we almost never want interrupts, > > whatever the index value. > > Good point. OK, I give in, I'll take your patch which moves the fields > to the end. Is that your preference? Yes, I think so. You mean PATCHv3 unchanged with 254 byte padding? > Please be careful with the qemu side though... > > It's not inconceivable that I'll write that virtio cacheline simulator this > (coming) week, too... > > Thanks. > Rusty.