Re: [virtio-comment] [PATCH 5/5] virtio-pci: implement VIRTIO_F_QUEUE_STATE

[Jason Wang <jasowang@redhat.com>]

On Sun, Sep 17, 2023 at 1:29âPM Parav Pandit <parav@nvidia.com> wrote:
>
>
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Thursday, September 14, 2023 8:42 AM
> >
> > On Wed, Sep 13, 2023 at 12:46âPM Parav Pandit <parav@nvidia.com> wrote:
> > >
> > > > From: Jason Wang <jasowang@redhat.com>
> > > > Sent: Wednesday, September 13, 2023 10:14 AM
> > >
> > > > It's not about how many states in a single state machine, it's about
> > > > how many state machines that exist for device status. Having more
> > > > than one creates big obstacles and complexity in the device. You
> > > > need to define the interaction of each state otherwise you leave undefined
> > behaviours.
> > > The device mode has zero relation to the device status.
> >
> > You will soon get this issue when you want to do nesting.
> >
> I donât think so. One needs to intercept it when one wants to do trap+emulation which seems to fullfil the nesting use case.

Well, how can you trap it? You have admin vq in L0, it means the
suspending is never exposed to L1 unless you assign the owner to L1.
Is this what you want?

>
> > > It does not mess with it at all.
> > > In fact the new bits in device status is making it more complex for the device
> > to handle.
> >
> > Are you challenging the design of the device status? It's definitely too late to do
> > this.
> >
> No. I am saying the extending device_status with yet another state is equally complex and its core of the device.

You never explain why.

>
> > This proposal increases just one bit and that worries you? Or you think one
> > more state is much more complicated than a new state machine with two
> > states?
>
> It is mode and not state. And two modes are needed for supporting P2P device.

You keep saying you are migrating the core virtio devices but then you
are saying it is required for PCI. And you never explain why it can't
be done by reusing the device status bit.

> When one wants to do with mediation, there also two states are needed.
>
> The key is modes are not interacting

You need to explain why they are not interacting. It touches the
virtio facility which (partially) overlaps the function of the device
status for sure. You invent a new state machine, and leave the vendors
to guess how or why they are not interacting with the existing one.
There are just too many corner cases that need to be figured out.

For example:

How do you define stop? Is it a virtio level stop, transport level or
a mixing of them both? Is the device allowed to stop in the middle or
reset, feature negotiation or even transport specific things like FLR?
If yes, how about other operations and who defines and maintains those
transitional states? If not, why and how long would a stop wait for an
operation? Can a stop fail? What happens if the driver wants to reset
but the device is stopped by the admin commands? Who suppresses who
and why?

This demonstrates the complexity of your proposal and I don't see any
of the above were clearly stated in your series. Reusing the existing
device status machine, everything would be simplified.

> with the device_status because device_status is just another register of the virtio.

Let's don't do layer violation, device status is the basic facility of
the virtio device which is not coupled with any transport so it is not
necessarily implemented via registers.

Thanks