Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ

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

On Tue, May 16, 2023 at 07:29:44PM +0000, Parav Pandit wrote:
> 
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Tuesday, May 16, 2023 12:08 AM
> 
> > > For guest and by guest are two different things.
> > > I am proposing that a cfgq/cmdq should be used directly by the guest driver as
> > base line like any other VQ.
> > 
> > I'm fine with this, but if I understand correctly, but I don't see any proposal like
> > this that is posted to the list?
> > 
> Because it is orthogonal to the work here, it's hard to make progress by mixing everything.
> 
> > > And if there is a need for mediation, it should be possible.
> > >
> > > > I think we've discussed many times that legacy is tricky for hardware.
> > > And we want to proceed for the described use case and requirements.
> > >
> > > > And your proposal proves this further by introducing modern alike
> > > > notification areas.
> > > Not really. A modern device is offering the notification area like legacy to the
> > legacy interface.
> > > So this is just fine.
> > 
> > I don't say it's not fine, it's pretty fine and transport virtqueue have similar
> > commands. And this also answer the question that you think tunnel PCI over
> > adminq is not scalable. We can keep this command in that case as well, or
> > invent new capabilities.
> > 
> Ok. so virtio_admin_q_cmd_lq_notify_result is fine.
> 
> > >
> > > > We probably need to deal with others like endianess.
> > > >
> > > No point in discussing this again. One size does not fit all.
> > > It solves large part of the use cases so it is valuable and will be supported.
> > 
> > This is doable, we need to leave some space for future development. Or is it
> > just complicated to have an endian command?
> > 
> Possibly one can set. I donât know if any actual device really supported endianness.
> No users have asked for it, even asking explicitly to those non_little_endian users.

For sure, ARM BE does exist and Red Hat supports it because yes,
it was requested. You can not just go by what marketing tells you
today we either try to build future proof interfaces or we don't
bother at all - by the time these things are in the field everything
shifted.

> So may be when there is/if a real user, it can be done in future.

The concern is if you assume LE is default, no way to say
"I do not support LE". Something like an explicit
"SET LE" and "SET BE" commands will automatically allow
discovering which endian-ness is supported.

> > >
> > > > For any case, having a simple and deterministic device design is
> > > > always better assuming we've agreed that mediation is a must for
> > > > legacy drivers. Using dedicated commands can make sure the
> > > > implementation won't need to go with corner cases of legacy.
> > > >
> > > Corner case handling just moved from the device to hypervisor.
> > 
> > That's not safe, the device should be ready for malicious inputs.
> > 
> With current offset, register, device will be just fine as most implementations have to program control path etc on these registers write/read etc.
> So, device will be able to handle them with plain 2 commands.

Except legacy is broken broken broken.  It just does not work for
physical devices in a crazy number of ways. How about the weird 48 bit
limitation on PAs? Because no one ever will need any more.

I have 0 sympathy to this idea of reviving all these bugs then circling
back and coming up with weird work arounds.  Please, just build a sane
transport and have software deal with bugs as best it can.



> > > For SIOV there is no backward compatibility requirement.
> > 
> > It has several ways to support current virtio-pci drivers in the guest, and such
> > compatibility is almost a must for the public cloud.
> > We can't enforce the user to reinstall/recompile their kernels/application in
> > order to work for SIOV.
> Sure, an SIOV device emulating PCI device for guest is one requirement.
> 
> And SIOV device working natively spawned from the parent PF and used without guest is another requirement.
> 
> And second can be done bit elegantly in self-contained way without depending on the parent PF.
> And requires more work unrelated to this one.
> 
> > I think we are discussing what it can be, so see the above reply for the
> > notification areas, we can stick virtio_admin_cmd_lq_notify_query_result
> > command for PCI over adminq, or invent new capabilities to make it fully self
> > contained.
> > 
> As we both understand well that a queue is not going to transport a doorbell and actual IMS interrupts, we will mostly have commands to service some needs for backward compat.
> It is a separate work when there are no PCI VF devices.
> 
> > >
> > > >
> > > > >
> > > > > You only want to exchange currently defined config registers,
> > > > > interrupts and
> > > > notification configuration using AQ.
> > > > >
> > > > > Transport VQ is NOT transporting actual data, it is not
> > > > > transporting
> > > > notifications etc.
> > > > > It is just a config channel.
> > > > > Hence, they are just commands not a "full transport".
> > > >
> > > >
> > > > For any case, a virtqueue could not be a full transport. It needs
> > > > bus facilities to do bootstrap/DMA/notifications at least. The idea
> > > > is to save for transport specific resources and use a more scalable
> > > > way to config devices.
> > > >
> > > Hence it needs only needs a cfgq, not the full transport. :)
> > >
> > 
> > Well, not a native speaker but I'm fine if you want to call it configq.
> Michael called it admin vq, so will stick to it for now. :)