From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rusty Russell Subject: Re: [PATCH 3/4] s390/kvm: Add a channel I/O based virtio transport driver. Date: Wed, 08 Aug 2012 13:52:57 +0930 Message-ID: <87pq72t3ku.fsf@rustcorp.com.au> References: <1344351168-2568-1-git-send-email-cornelia.huck@de.ibm.com> <1344351168-2568-4-git-send-email-cornelia.huck@de.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: In-Reply-To: <1344351168-2568-4-git-send-email-cornelia.huck@de.ibm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org List-Archive: List-Post: To: Cornelia Huck , KVM , linux-s390 , qemu-devel Cc: Carsten Otte , Anthony Liguori , Sebastian Ott , Marcelo Tosatti , Heiko Carstens , Alexander Graf , Christian Borntraeger , Avi Kivity , Martin Schwidefsky List-ID: On Tue, 7 Aug 2012 16:52:47 +0200, Cornelia Huck wrote: > Add a driver for kvm guests that matches virtual ccw devices provided > by the host as virtio bridge devices. Hi Cornelia, OK, this is a good opportunity to fix some limitations, just as we did for virtio_mmio (drivers/virtio/virtio_mmio.c). 1) Please don't limit yourself to 32 feature bits! If you look at how virtio_mmio does it, they use a selector to index into a theoretically-infinite array of feature bits: * 0x010 R HostFeatures Features supported by the host * 0x014 W HostFeaturesSel Set of host features to access via HostFeatures * * 0x020 W GuestFeatures Features activated by the guest * 0x024 W GuestFeaturesSel Set of activated features to set via GuestFeatures 2) Please also allow the guest to set the alignment for virtio ring layout (it controls the spacing between the rings), eg: * 0x03c W QueueAlign Used Ring alignment for the current queue 3) Finally, make sure the guest can set the size of the queue, in case it can't allocate the size the host suggests, eg: * 0x034 R QueueNumMax Maximum size of the currently selected queue * 0x038 W QueueNum Queue size for the currently selected queue This means the host can suggest huge queues, knowing the guest won't simply fail if it does so. Note that we're also speculating a move to a new vring format, which will probably be little-endian. But you probably want a completely new ccw code for that anyway. Cheers, Rusty. From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41047) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0g9W-0007vs-Oh for qemu-devel@nongnu.org; Sun, 12 Aug 2012 17:56:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T0g9V-0007xD-SL for qemu-devel@nongnu.org; Sun, 12 Aug 2012 17:56:54 -0400 Received: from ozlabs.org ([203.10.76.45]:46762) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0g9V-0007wI-HS for qemu-devel@nongnu.org; Sun, 12 Aug 2012 17:56:53 -0400 From: Rusty Russell In-Reply-To: <1344351168-2568-4-git-send-email-cornelia.huck@de.ibm.com> References: <1344351168-2568-1-git-send-email-cornelia.huck@de.ibm.com> <1344351168-2568-4-git-send-email-cornelia.huck@de.ibm.com> Date: Wed, 08 Aug 2012 13:52:57 +0930 Message-ID: <87pq72t3ku.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH 3/4] s390/kvm: Add a channel I/O based virtio transport driver. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck , KVM , linux-s390 , qemu-devel Cc: Carsten Otte , Anthony Liguori , Sebastian Ott , Marcelo Tosatti , Heiko Carstens , Alexander Graf , Christian Borntraeger , Avi Kivity , Martin Schwidefsky On Tue, 7 Aug 2012 16:52:47 +0200, Cornelia Huck wrote: > Add a driver for kvm guests that matches virtual ccw devices provided > by the host as virtio bridge devices. Hi Cornelia, OK, this is a good opportunity to fix some limitations, just as we did for virtio_mmio (drivers/virtio/virtio_mmio.c). 1) Please don't limit yourself to 32 feature bits! If you look at how virtio_mmio does it, they use a selector to index into a theoretically-infinite array of feature bits: * 0x010 R HostFeatures Features supported by the host * 0x014 W HostFeaturesSel Set of host features to access via HostFeatures * * 0x020 W GuestFeatures Features activated by the guest * 0x024 W GuestFeaturesSel Set of activated features to set via GuestFeatures 2) Please also allow the guest to set the alignment for virtio ring layout (it controls the spacing between the rings), eg: * 0x03c W QueueAlign Used Ring alignment for the current queue 3) Finally, make sure the guest can set the size of the queue, in case it can't allocate the size the host suggests, eg: * 0x034 R QueueNumMax Maximum size of the currently selected queue * 0x038 W QueueNum Queue size for the currently selected queue This means the host can suggest huge queues, knowing the guest won't simply fail if it does so. Note that we're also speculating a move to a new vring format, which will probably be little-endian. But you probably want a completely new ccw code for that anyway. Cheers, Rusty.