Re: [RFC PATCH 1/1] virtio: write back features before verify
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2021-10-03 07:26:18
Also in:
linux-s390, lkml
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Sun, Oct 03, 2021 at 02:42:30AM -0400, Michael S. Tsirkin wrote:
On Sun, Oct 03, 2021 at 07:00:30AM +0200, Halil Pasic wrote:quoted
On Sat, 2 Oct 2021 14:20:47 -0400 "Michael S. Tsirkin" [off-list ref] wrote:quoted
quoted
quoted
From my perspective the problem is that the version of the deviceremains in limbo as long as the features have not yet been finalized, which means that the endianness of the config space remains in limbo as well. Both device and driver might come to different conclusions.Version === legacy versus modern? It is true that feature negotiation can not be used by device to decide that question simply because it happens too late. So let's not use it for that then ;) Yes we have VERSION_1 which looks like it should allow this, but unfortunately it only helps with that for the driver, not the device. In practice legacy versus modern has to be determined by transport specific versioning, luckily we have that for all specified transports (can't say what happens with rproc).So if we look at ccw, you say that the revision negotiation already determines whether VERSION_1 is negotiated or not, and the feature bit VERSION_1 is superfluous? That would also imply, that 1) if revision > 0 was negotiated then the device must offer VERSION_1 2) if revision > 0 was negotiated and the driver cleared VERSION_1 the device must refuse to operate. 3) if revision > 0 was negotiated then the driver should reject to drive a device if it does not offer VERSION_1 4) if revision > 0 was negotiated the driver must accept VERSION_1 5) if revision > 0 was *not* negotiated then the device should not offer VERSION_1 because at this point it is already certain that the device can not act in accordance to the virtio 1.0 or higher interface. Does that sound about right?To me, it does.quoted
IMHO we should also change https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-160003 and the definition of VERSION_1 because both sides have to know what is going on before features are fully negotiated. Or? Regards, HalilI guess so. And I guess we need transport-specific sections describing this behaviour for each transport. So something like this, for starters?
Sent too early. So here's what I propose. Could you pls take a look and if you like this, post a ccw section? There's also an attempt to prevent fallback from modern to legacy here since if driver does fallback then failing FEATURES_OK can't work properly. That's a separate issue, will be a separate patch when I post this for consideration by the TC.
diff --git a/content.tex b/content.tex
index 1398390..06271f4 100644
--- a/content.tex
+++ b/content.tex@@ -140,10 +140,13 @@ \subsection{Legacy Interface: A Note on Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature Bits / Legacy Interface: A Note on Feature Bits} -Transitional Drivers MUST detect Legacy Devices by detecting that -the feature bit VIRTIO_F_VERSION_1 is not offered. -Transitional devices MUST detect Legacy drivers by detecting that -VIRTIO_F_VERSION_1 has not been acknowledged by the driver. +Transitional drivers MAY support operating legacy devices. +Transitional devices MAY support operation by legacy drivers. + +Transitional drivers MUST detect legacy devices in a way that is +transport specific. +Transitional devices MUST detect legacy drivers in a way that +is transport specific. In this case device is used through the legacy interface.
@@ -160,6 +163,33 @@ \subsection{Legacy Interface: A Note on Feature Specification text within these sections generally does not apply to non-transitional devices. +\begin{note} +The device offers different features when used through +the legacy interface and when operated in accordance with this +specification. +\end{note} + +Transitional drivers MUST use Devices only through the legacy interface +if the feature bit VIRTIO_F_VERSION_1 is not offered. +Transitional devices MUST NOT offer VIRTIO_F_VERSION_1 when used through +the legacy interface. + +When the driver uses a device through the legacy interface, then it +MUST only accept the features the device offered through the +legacy interface. + +When used through the legacy interface, the device SHOULD +validate that the driver only accepted the features it +offered through the legacy interface. + +When operating a transitional device, a transitional driver +SHOULD NOT use the device through the legacy interface if +operation through the modern interface has failed. +In particular, a transitional driver +SHOULD NOT fall back to using the device through the +legacy interface if feature negotiation failed +(since that would defeat the purpose of the FEATURES_OK bit). + \section{Notifications}\label{sec:Basic Facilities of a Virtio Device / Notifications}
@@ -1003,6 +1033,12 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport The driver MUST NOT write a 0 to \field{queue_enable}. +\paragraph}{Legacy Interface: Common configuration structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interface: Common configuration structure layout} +Transitional drivers SHOULD detect legacy devices by detecting +that the device has the Transitional PCI Device ID in +the range 0x1000 to 0x103f and lacks a VIRTIO_PCI_CAP_COMMON_CFG +capability specifying the location of a common configuration structure. + \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability} The notification location is found using the VIRTIO_PCI_CAP_NOTIFY_CFG
@@ -1288,6 +1324,10 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio Transitional devices MUST present part of configuration registers in a legacy configuration structure in BAR0 in the first I/O region of the PCI device, as documented below. + +Transitional devices SHOULD detect legacy drivers by detecting +access to the legacy configuration structure. + When using the legacy interface, transitional drivers MUST use the legacy configuration structure in BAR0 in the first I/O region of the PCI device, as documented below.
_______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization