lkml.org 
[lkml]   [2021]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [RFC PATCH 1/1] virtio: write back features before verify
    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:
    > > On Sat, 2 Oct 2021 14:20:47 -0400
    > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
    > >
    > > > > >From my perspective the problem is that the version of the device
    > > > > remains 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.
    >
    > > 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,
    > > Halil
    > >
    >
    > I 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.
    \
     
     \ /
      Last update: 2021-10-03 09:27    [W:2.136 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site