RE: [virtio-comment] Re: [PATCH 07/11] transport-pci: Introduce transitional MMR device id

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Friday, April 7, 2023 11:51 AM

> > 1. A non-transitional device will expose a capability (not a feature bit, but a
> capability at transport level).
> 
> Note that we can allow this capability in transitional devices too.
> This is useful since IO bar might not be enabled even if present.
>
This capability exposure makes a device transitional in some sense.
 
> > This capability indicates that, it supports legacy interface.
> > Lets name it legacy_if_emulation for sake of this discussion.
> > It is a two-way pci capability.
> > Device reports it.
> > And driver enables it. (Why two way and why driver needs to enable it,
> described later in point #d below).
> >
> > Hence, such non transitional device does not need to comply to below listed
> requirements #a and #b.
> >
> > a. A driver MUST accept VIRTIO_F_VERSION_1 if it is offered.
> > (Because hypervisor driver is a passthrough driver; and legacy driver will not
> accept this feature bit).
> 
> This is not a device requirement at all.
> 
Those this is written as driver requirement; a device expects this feature bit to be negotiated.
What should device implementor do? It should allow driver to not negotiate bit, right?

Which means, below line to be change:

from:
device MAY fail to operate further if VIRTIO_F_VERSION_1 is not accepted.

to:
Non transitional device that does not have legacy interface capability MAY fail to operate further if V_1 is not accepted.
Non transitional device that has legacy interface capability SHOULD operate further even if V_1 is not accepted.

> > b. device MAY fail to operate further if VIRTIO_F_VERSION_1 is not accepted.
> 
> This is optional not a requirement.
> 
Please see above wording, if its acceptable.

> > c. A non-transitional device with above legacy_if_supported
> > capability, will allow device reset sequence, described in [1] Driver
> > Requirements: Device Initialization (3.1.1) [2] Legacy Interface:
> > Device Initialization (3.1.2)
> >
> > > > device reset sequence.
> > >
> > > what is this one?
> >
> > I listed above in #c.
> > And
> >
> > d. When legacy_if_emulation capability is offered and hypervisor driver
> enabled it, when driver perform device reset, driver will not wait for device
> reset to go zero.
> > When legacy_if_emulation capability is not enabled by (hypervisor or other
> say existing) driver, driver will wait for device reset to turn 0. (Following the
> driver requirement 2.4.2).
> 
> It might not be a bad idea to enable it, but I observe that it is possible for
> hypervisor to expose a standard transitional device on top of this MMR
> capability. Thus it will not be known whether guest driver accesses legacy or
> modern BAR until guest runs.
> I propose, instead, that device exposes same registers at two addresses and
> executes reset correctly depending on which address it was accessed through.
> WDYT?
Yep, this the exact proposal here.
Legacy registers exposes via AQ (aka TVQ) or MMR location, behaves like legacy.
And regular registers at their location as-is.

With that feature bit negotiation is the only thing to relax like worded above.