Re: [virtio-comment] Re: [PATCH 0/2] Selective queue enabling

["Michael S. Tsirkin" <mst@redhat.com>]

On Sat, Jun 17, 2023 at 02:53:46PM +0200, Eugenio Perez Martin wrote:
> On Fri, Jun 16, 2023 at 4:40âPM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, Jun 15, 2023 at 10:35:09AM +0200, Eugenio Perez Martin wrote:
> > > On Tue, Jun 13, 2023 at 2:29âPM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Tue, Jun 13, 2023 at 12:12:43PM +0200, Eugenio Perez Martin wrote:
> > > > > On Tue, Jun 13, 2023 at 9:53âAM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > > >
> > > > > > On Tue, Jun 13, 2023 at 03:46:35AM -0400, Michael S. Tsirkin wrote:
> > > > > > > On Mon, Jun 12, 2023 at 09:56:57AM +0200, Eugenio Perez Martin wrote:
> > > > > > > > > > Another drawback of that is the long time a device can spend to
> > > > > > > > > > reconfigure its memory maps.
> > > > > > > > >
> > > > > > > > > The map is already used for cvq though, I don't see why it would take
> > > > > > > > > much more with this.
> > > > > > > > >
> > > > > > > >
> > > > > > > > In the case of vDPA HW, CVQ maps may not need to reach the device's or
> > > > > > > > platform IOMMU, so the modification of that mapping is way cheaper.
> > > > > > >
> > > > > > > Oh, I get it. It's setup time, but you are right.
> > > > > > >
> > > > > > > > > > taking that into account, does it make
> > > > > > > > > > sense to send a new version with the bit 1 on enable?
> > > > > > > > > >
> > > > > > > > > > Thanks!
> > > > > > > > >
> > > > > > > > > Current hardware won't support the extra bit though.
> > > > > >
> > > > > >
> > > > > > Thinking more about this, I think the following works:
> > > > > >
> > > > > > - enable all queues before DRIVER_OK
> > > > > > - reset all data queues before DRIVER_OK
> > > > > > - DRIVER_OK
> > > > > > - CVQ commands
> > > > > > - re-enable data queues
> > > > > >
> > > > > >
> > > > > > No?
> > > > > >
> > > > >
> > > > > If it is valid to write to queue_reset before DRIVER_OK, then sure. I
> > > > > asked for that in the past at
> > > > > https://lists.oasis-open.org/archives/virtio-dev/202305/msg00329.html
> > > > > .
> > > >
> > > > Well then Jason said:
> > > >  we should make svq work without RING_RESET
> > > > but now apparently we went full circle and propose
> > > > a feature bit after all?
> > > >
> > >
> > > My understanding is that RING_RESET covers more than just delaying the
> > > enabling of net data vqs, like to change the address or properties of
> > > vqs after DRIVER_OK. While the late enabling may be easy to implement
> > > in current hw, vq property changes may be more challenging. There are
> > > current HW that only support late enabling but not reconfigure the
> > > queues. Ideally, just the subset of late enabling is needed.
> > >
> > > Does it make sense to change the proposal to something in the line of:
> > > RING_F_LATE_ENABLING: This feature flag is a subset of _F_RING_RESET.
> > > If this feature is negotiated, all vqs start as if the vqs has been
> > > reset before DRIVER_OK. This feature does not allow resetting a queue
> > > once it is live.
> > >
> > > Thanks!
> >
> > Maybe. No idea - I am not a hardware designer. Can some hardware
> > vendors chime in and confirm?
> > It seems like a very very narrow "make my hack work" use-case.
> > And I'm not even sure how well it works.
> > For example what if cvq command enables rss? If the queues in
> > question are not enabled packets will be dropped.
> > So - fix some setups, break others?
> >
> 
> I don't think it breaks, they are already dropped before setting
> DRIVER_OK anyway. If we restore the configuration before DRIVER_OK in
> the future, the device is supposed to drop packets meanwhile anyway,
> isn't it?


I just thought of an idea. Devices generally don't poke at rings
by themselves. How about at init time we cause kicks on data queues
to vmexit? Then on 1st kick we map hardware so following ones will
go straight to hardware. This will make sure cvq is processed
first. Hmm?

> >
> > > >
> > > > > >
> > > > > >
> > > > > > > --
> > > > > > > MST
> > > > > >
> > > > > >
> > > > > > This publicly archived list offers a means to provide input to the
> > > > > > OASIS Virtual I/O Device (VIRTIO) TC.
> > > > > >
> > > > > > In order to verify user consent to the Feedback License terms and
> > > > > > to minimize spam in the list archive, subscription is required
> > > > > > before posting.
> > > > > >
> > > > > > Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> > > > > > Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> > > > > > List help: virtio-comment-help@lists.oasis-open.org
> > > > > > List archive: https://lists.oasis-open.org/archives/virtio-comment/
> > > > > > Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> > > > > > List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> > > > > > Committee: https://www.oasis-open.org/committees/virtio/
> > > > > > Join OASIS: https://www.oasis-open.org/join/
> > > > > >
> > > >
> >