Re: [Qemu-devel] [PATCH V7 08/16] virtio: introduce bus specific queue limit

[Michael S. Tsirkin]

On Tue, Apr 28, 2015 at 10:04:15AM +0200, Cornelia Huck wrote:
> On Tue, 28 Apr 2015 09:14:07 +0200
> "Michael S. Tsirkin" <address@hidden> wrote:
> 
> > On Tue, Apr 28, 2015 at 02:13:20PM +0800, Jason Wang wrote:
> > > 
> > > 
> > > On Tue, Apr 28, 2015 at 1:13 PM, Michael S. Tsirkin <address@hidden> 
> > > wrote:
> > > >On Tue, Apr 28, 2015 at 11:14:04AM +0800, Jason Wang wrote:
> > > >>     On Mon, Apr 27, 2015 at 7:05 PM, Michael S. Tsirkin
> > > >><address@hidden> wrote:
> > > >> >On Thu, Apr 23, 2015 at 02:21:41PM +0800, Jason Wang wrote:
> > > >> >> This patch introduces a bus specific queue limitation. It will be
> > > >> >> useful for increasing the limit for one of the bus without
> > > >>disturbing
> > > >> >> other buses.
> > > >> >> Cc: Michael S. Tsirkin <address@hidden>
> > > >> >> Cc: Alexander Graf <address@hidden>
> > > >> >> Cc: Richard Henderson <address@hidden>
> > > >> >> Cc: Cornelia Huck <address@hidden>
> > > >> >> Cc: Christian Borntraeger <address@hidden>
> > > >> >> Cc: Paolo Bonzini <address@hidden>
> > > >> >> Signed-off-by: Jason Wang <address@hidden>
> > > >> >> Reviewed-by: Cornelia Huck <address@hidden>
> > > >> >
> > > >> >Is this still needed if you drop the attempt to
> > > >> >keep the limit around for old machine types?
> > > >> If we agree to drop, we probably need transport specific macro.
> > > >
> > > >You mean just rename VIRTIO_PCI_QUEUE_MAX to VIRTIO_QUEUE_MAX?
> > > >Fine, why not.
> > > 
> > > I mean keeping VIRTIO_PCI_QUEUE_MAX for pci only and just increase pci
> > > limit. And introduce e.g VIRTIO_PCI_QUEUE_CCW for ccw and keep it as 64.
> > > Since to my understanding, it's not safe to increase the limit for all 
> > > other
> > > transports which was pointed out by Cornelia in V1:
> > > http://permalink.gmane.org/gmane.comp.emulators.qemu/318245.
> > 
> > I think all you need is add a check to CCW_CMD_SET_IND:
> > limit to 64 for legacy interrupts only.
> 
> It isn't that easy.
> 
> What is easy is to add a check to the guest driver that fails setup for
> devices with more than 64 queues not using adapter interrupts.
> 
> On the host side, we're lacking information when interpreting
> CCW_CMD_SET_IND (the command does not contain a queue count, and the
> actual number of virtqueues is not readily available.)

Why isn't it available? All devices call virtio_add_queue
as appropriate. Just fail legacy adaptors.

> We also can't
> fence off when setting up the vqs, as this happens before we know which
> kind of indicators the guest wants to use.
> 
> More importantly, we haven't even speced what we want to do in this
> case. Do we want to reject SET_IND for devices with more than 64
> queues? (Probably yes.)
> 
> All this involves more work, and I'd prefer to do Jason's changes
> instead as this gives us some more time to figure this out properly.
> 
> And we haven't even considered s390-virtio yet, which I really want to
> touch as little as possible :)

Well this patch does touch it anyway :)
For s390 just check and fail at init if you like.

-- 
MST