Re: [PATCH v1] s390/virtio_ccw: don't allocate/assign airqs for non-existing queues

From: Michael S. Tsirkin
Date: Fri Apr 04 2025 - 01:33:53 EST


On Fri, Apr 04, 2025 at 06:02:04AM +0200, Halil Pasic wrote:
> On Thu, 3 Apr 2025 10:35:33 -0400
> "Michael S. Tsirkin" <mst@xxxxxxxxxx> wrote:
>
> > On Thu, Apr 03, 2025 at 04:18:36PM +0200, Halil Pasic wrote:
> > > On Wed, 2 Apr 2025 22:36:21 +0200
> [..]
> > >
> > > 5.5.2 Virtqueues
> > >
> > > 0
> > > inflateq
> > > 1
> > > deflateq
> > > 2
> > > statsq
> > > 3
> > > free_page_vq
> > > 4
> > > reporting_vq
> >
> > Indeed. Unfortunately, no one at all implemented this properly as
> > per spec :(.
> >
> > Balloon is the worst offender here but other devices are broken
> > too in some configurations.
> >
> >
> > Given it has been like this for many years I'm inclined in this
> > instance to fix the spec not the implementations.
> >
>
> I understand! For me at least knowing if we are going to change the
> spec or the implementations is pretty important. It would probably
> make sense to spin a patch for the spec, just for the unlikely case that
> somebody did end up implementing this by the spec and wants to protest.
>
> I think, a consequence of this design is that all queues need to be
> created and allocated at initialization time.

Why? after feature negotiation.

> I mean in the spec we have something like
> """
> 5.1.6.5.6.1 Driver Requirements: Automatic receive steering in multiqueue mode
> The driver MUST configure the virtqueues before enabling them with the VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command.
> """
>
> For example one could want to hotplug 2 more vCPUs and still have a
> queue-pair per cpu (and a controlq). Those 2 extra queue-pairs could
> in theory be allocated on-demand instead of having to allocate memory
> for the rings for all the queues corresponding to the maxed out setup.
> I've had a look at the Linux virtio-net and it does allocate all the
> queues up front.
>
> Also with this design, I believe we would effectively prohibit "holes".

For existing devices at least.

> Now if holes are prohibited, IMHO it does not make a whole lot of
> sense for the virtio_find_vqs() to support holes. Because the holes
> and a fair amount of complexity. Of course that would make sense as a
> cleanup on top.
>
> Regards,
> Halil