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

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

On Fri, Apr 07, 2023 at 03:18:47PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Friday, April 7, 2023 8:03 AM
> > 
> > On Tue, Apr 04, 2023 at 12:08:54PM -0400, Parav Pandit wrote:
> 
> [..]
> > > > I took a fresh look at it, and I don't get it: what exactly is wrong
> > > > with just using modern ID? Why do we need new ones?
> > >
> > > A modern (non transitional device) do not support legacy functionality
> > > such as virtio net hdr,
> > 
> > It's all a question of terminology, but it is not worth sacrificing functionality for
> > cleaner terminology.
> > Basically most of the spec just talks about the legacy interface, and will work
> > fine with this.
> > 
> > Yes we do say:
> > Devices or drivers with no legacy compatibility are referred to as non-
> > transitional devices and drivers, respectively.
> > 
> > 
> > So we will want to refine this somewhat. Maybe:
> > 
> > 	Devices not compatible with legacy drivers and drivers not compatible
> > 	with legacy devices are referred to as non-transitional devices and
> > 	drivers, respectively.
> > 
> > 
> > This allows non-transitional devices to expose the legacy capability - having this
> > capability does not make them compatible with
> > 
> > Similarly in the conformance section:
> > 
> > An implementation MAY choose to implement OPTIONAL support for the
> > legacy interface, including support for legacy drivers or devices, by conforming
> > to all of the MUST or REQUIRED level requirements for the legacy interface for
> > the transitional devices and drivers.
> > 
> > we would just remove "for the transitional devices and drivers"
> > here as now non-transitional can have a legacy interface.
> > 
> > Similarly:
> > 
> > The requirements for the legacy interface for transitional implementations
> > 
> > would become:
> > 
> > "The requirements for the legacy interface"
> > 
> >
> I will hold to respond to other emails in this series, because the key part is here.
> 
> If I understand you correctly, will above wording translate to below behavior?
> 
> 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 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.

> b. device MAY fail to operate further if VIRTIO_F_VERSION_1 is not accepted.

This is optional not a requirement.

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



-- 
MST