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

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

On Sun, Apr 09, 2023 at 03:15:01AM +0000, Parav Pandit wrote:
> 
> > 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.

not in the sense spec uses it at the moment: transitional devices
are those that legacy drivers can bind to. transitional drivers
btw are those that can bind to legacy devices.

perhaps suprisingly, a transitional driver using a transitional
device does not rely on any legacy spec at all, they will
use the standard interfaces.


> > > 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.



Look nothing changes with MMR capability at all.
We currently have:

	A device MUST offer VIRTIO_F_VERSION_1.  A device MAY fail to operate further
	if VIRTIO_F_VERSION_1 is not accepted.

it's implied that this does not refer to legacy interface.


You want to clarify this adding to legacy interface section text
explaining that of course VIRTIO_F_VERSION_1 must not
be offered through that? Sure but it's a separate issue
from MMR capability. don't try to drink the ocean.





> > > 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.


you don't need any of that for this effort, generally
VIRTIO_F_VERSION_1 thing needs a lot of work, if you
want to invest the time just ask I'll try to list the issues.

But nothing to do with memory mapped legacy interface
directly.



> > > 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.

It's not really different from IO port legacy then.

-- 
MST