On Tue, Apr 28, 2015 at 12:40:07PM +0200, Cornelia Huck wrote: > On Tue, 28 Apr 2015 10:16:04 +0200 > "Michael S. Tsirkin" <m...@redhat.com> wrote: > > > 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" <m...@redhat.com> 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 <m...@redhat.com> > > > > > 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 > > > > > >><m...@redhat.com> 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 <m...@redhat.com> > > > > > >> >> Cc: Alexander Graf <ag...@suse.de> > > > > > >> >> Cc: Richard Henderson <r...@twiddle.net> > > > > > >> >> Cc: Cornelia Huck <cornelia.h...@de.ibm.com> > > > > > >> >> Cc: Christian Borntraeger <borntrae...@de.ibm.com> > > > > > >> >> Cc: Paolo Bonzini <pbonz...@redhat.com> > > > > > >> >> Signed-off-by: Jason Wang <jasow...@redhat.com> > > > > > >> >> Reviewed-by: Cornelia Huck <cornelia.h...@de.ibm.com> > > > > > >> > > > > > > >> >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. > > Because we don't know what the guest is going to use? It is free to > use per-subchannel indicators, even if it is operating in virtio-1 mode. > > > > > 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 :) > > But only small, self-evident changes. >
Sorry, I don't see what you are trying to say. There's no chance legacy interrupts work with > 64 queues. Guests should have validated the # of queues, and not attempted to use >64 queues. Looks like there's no such validation in guest, right? Solution - don't specify this configuration with legacy guests. Modern guests work so there's value in supporting such configuration in QEMU, I don't see why we must deny it in QEMU. > > For s390 just check and fail at init if you like. > > What about devices that may change their number of queues? I'd really > prefer large queue numbers to be fenced off in the the individual > devices, and for that they need to be able to grab a transport-specific > queue limit. This is why I don't want bus specific limits in core, it just makes it too easy to sweep dirt under the carpet. s390 is legacy - fine, but don't perpetuate the issue in devices. -- MST
