All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Cornelia Huck <cohuck@redhat.com>
Cc: Parav Pandit <parav@nvidia.com>,
	virtio-comment@lists.oasis-open.org,
	virtio-dev@lists.oasis-open.org, shahafs@nvidia.com
Subject: [virtio-comment] Re: [PATCH] content: Replace guest OS with driver
Date: Tue, 16 May 2023 06:05:23 -0400	[thread overview]
Message-ID: <20230516054313-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <87lehohefn.fsf@redhat.com>

On Tue, May 16, 2023 at 10:24:12AM +0200, Cornelia Huck wrote:
> On Tue, May 16 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Tue, May 16, 2023 at 06:01:39AM +0300, Parav Pandit wrote:
> >> diff --git a/content.tex b/content.tex
> >> index 9df81b8..417d476 100644
> >> --- a/content.tex
> >> +++ b/content.tex
> >> @@ -26,10 +26,10 @@ \section{\field{Device Status} Field}\label{sec:Basic Facilities of a Virtio Dev
> >>  following bits are defined (listed below in the order in which
> >>  they would be typically set):
> >>  \begin{description}
> >> -\item[ACKNOWLEDGE (1)] Indicates that the guest OS has found the
> >> +\item[ACKNOWLEDGE (1)] Indicates that the driver has found the
> >>    device and recognized it as a valid virtio device.
> >>  
> >> -\item[DRIVER (2)] Indicates that the guest OS knows how to drive the
> >> +\item[DRIVER (2)] Indicates that the driver knows how to drive the
> >>    device.
> >>    \begin{note}
> >>      There could be a significant (or infinite) delay before setting
> >
> > Actually, there is a subtle difference here that this is losing.
> > "guest OS" really refers to e.g. Linux virtio core code here.
> >
> >
> > ACKNOWLEDGE and DRIVER are used by virtio core.
> >
> > ACKNOWLEDGE tells you virtio core attached to device, and DRIVER
> > tells you core found a device specific driver.
> >
> >
> >
> > If you really want to make things better, let's find a way to explain
> > all this.
> 
> Agreed, this is a really old part of the spec, and likely had been
> written with the Linux device probing sequence in mind.
> 
> Basically, we want to distinguish between "something on the driver side
> has discovered the device" and "something on the driver side knows how
> to drive this specific device". If we consider "driver" as a catch-all
> of the whole thing talking to a device, we need to be extra descriptive
> (and we can add examples, as this is a non-normative section.)
> 
> For ACKNOWLEDGE, maybe "indicates that the driver has discovered the
> device and recognized it as a valid virtio device" (i.e. mostly what we
> have now), but also add "For example, this can indicate that non-device
> specific virtio driver code has attached to the device."
> 
> For DRIVER, maybe "indicates that the driver has discovered that it
> knows how to drive this device specifically. For example, this can
> indicate that device-specific driver code has attached to the device."


I feel examples are a weak way to document it - if we can not say
what this is specifically, what purpose does it serve?
Actually, we do have a distinction, between transport and device type.
Can't we use that? It seems more consistent than "non-device
specific" and "device specific".


SO I propose:

\item[ACKNOWLEDGE (1)] Indicates that a transport driver has found the
    device and recognized it as a valid virtio device transport.
  
\item[DRIVER (2)] Indicates that a device type specific driver was found
    and will attempt to attach to the device.



BTW somewhat related, I would maybe fix
device-types/mem/description.tex:change
not to say "device driver", just "driver" for brevity.





> Probably need some more overhaul :) Not an editorial change in any case.
> 
> >> @@ -473,13 +473,13 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
> >>  \begin{enumerate}
> >>  \item Reset the device.
> >>  
> >> -\item Set the ACKNOWLEDGE status bit: the guest OS has noticed the device.
> >> +\item Set the ACKNOWLEDGE status bit: the driver has noticed the device.
> >>  
> >> -\item Set the DRIVER status bit: the guest OS knows how to drive the device.
> >> +\item Set the DRIVER status bit: the driver knows how to drive the device.
> >
> > besides the above, "drivers knows how to drive" sounds bad.
> >
> >>  \item\label{itm:General Initialization And Device Operation /
> >>  Device Initialization / Read feature bits} Read device feature bits, and write the subset of feature bits
> >> -   understood by the OS and driver to the device.  During this step the
> >> +   understood by the driver to the device.  During this step the
> >
> > Again the "the OS" here referred to core virtio (e.g. ring features).
> > Less of a problem to remove but if we come up with
> > a better terminology for ACKNOWLEDGE/DRIVER then I guess we can use it
> > here, too.
> 
> Hm, I'm not sure how far we need to distinguish between generic and
> device-specific features in this case. The "driver" as the whole entity
> driving the device needs to decide on the subset; at this stage, it does
> not really matter which parts of the driver code accepted which
> feature. We probably want to be explicit that features are ring,
> transport, and device features, though.

OK.

-- 
MST


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Cornelia Huck <cohuck@redhat.com>
Cc: Parav Pandit <parav@nvidia.com>,
	virtio-comment@lists.oasis-open.org,
	virtio-dev@lists.oasis-open.org, shahafs@nvidia.com
Subject: [virtio-dev] Re: [PATCH] content: Replace guest OS with driver
Date: Tue, 16 May 2023 06:05:23 -0400	[thread overview]
Message-ID: <20230516054313-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <87lehohefn.fsf@redhat.com>

On Tue, May 16, 2023 at 10:24:12AM +0200, Cornelia Huck wrote:
> On Tue, May 16 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Tue, May 16, 2023 at 06:01:39AM +0300, Parav Pandit wrote:
> >> diff --git a/content.tex b/content.tex
> >> index 9df81b8..417d476 100644
> >> --- a/content.tex
> >> +++ b/content.tex
> >> @@ -26,10 +26,10 @@ \section{\field{Device Status} Field}\label{sec:Basic Facilities of a Virtio Dev
> >>  following bits are defined (listed below in the order in which
> >>  they would be typically set):
> >>  \begin{description}
> >> -\item[ACKNOWLEDGE (1)] Indicates that the guest OS has found the
> >> +\item[ACKNOWLEDGE (1)] Indicates that the driver has found the
> >>    device and recognized it as a valid virtio device.
> >>  
> >> -\item[DRIVER (2)] Indicates that the guest OS knows how to drive the
> >> +\item[DRIVER (2)] Indicates that the driver knows how to drive the
> >>    device.
> >>    \begin{note}
> >>      There could be a significant (or infinite) delay before setting
> >
> > Actually, there is a subtle difference here that this is losing.
> > "guest OS" really refers to e.g. Linux virtio core code here.
> >
> >
> > ACKNOWLEDGE and DRIVER are used by virtio core.
> >
> > ACKNOWLEDGE tells you virtio core attached to device, and DRIVER
> > tells you core found a device specific driver.
> >
> >
> >
> > If you really want to make things better, let's find a way to explain
> > all this.
> 
> Agreed, this is a really old part of the spec, and likely had been
> written with the Linux device probing sequence in mind.
> 
> Basically, we want to distinguish between "something on the driver side
> has discovered the device" and "something on the driver side knows how
> to drive this specific device". If we consider "driver" as a catch-all
> of the whole thing talking to a device, we need to be extra descriptive
> (and we can add examples, as this is a non-normative section.)
> 
> For ACKNOWLEDGE, maybe "indicates that the driver has discovered the
> device and recognized it as a valid virtio device" (i.e. mostly what we
> have now), but also add "For example, this can indicate that non-device
> specific virtio driver code has attached to the device."
> 
> For DRIVER, maybe "indicates that the driver has discovered that it
> knows how to drive this device specifically. For example, this can
> indicate that device-specific driver code has attached to the device."


I feel examples are a weak way to document it - if we can not say
what this is specifically, what purpose does it serve?
Actually, we do have a distinction, between transport and device type.
Can't we use that? It seems more consistent than "non-device
specific" and "device specific".


SO I propose:

\item[ACKNOWLEDGE (1)] Indicates that a transport driver has found the
    device and recognized it as a valid virtio device transport.
  
\item[DRIVER (2)] Indicates that a device type specific driver was found
    and will attempt to attach to the device.



BTW somewhat related, I would maybe fix
device-types/mem/description.tex:change
not to say "device driver", just "driver" for brevity.





> Probably need some more overhaul :) Not an editorial change in any case.
> 
> >> @@ -473,13 +473,13 @@ \section{Device Initialization}\label{sec:General Initialization And Device Oper
> >>  \begin{enumerate}
> >>  \item Reset the device.
> >>  
> >> -\item Set the ACKNOWLEDGE status bit: the guest OS has noticed the device.
> >> +\item Set the ACKNOWLEDGE status bit: the driver has noticed the device.
> >>  
> >> -\item Set the DRIVER status bit: the guest OS knows how to drive the device.
> >> +\item Set the DRIVER status bit: the driver knows how to drive the device.
> >
> > besides the above, "drivers knows how to drive" sounds bad.
> >
> >>  \item\label{itm:General Initialization And Device Operation /
> >>  Device Initialization / Read feature bits} Read device feature bits, and write the subset of feature bits
> >> -   understood by the OS and driver to the device.  During this step the
> >> +   understood by the driver to the device.  During this step the
> >
> > Again the "the OS" here referred to core virtio (e.g. ring features).
> > Less of a problem to remove but if we come up with
> > a better terminology for ACKNOWLEDGE/DRIVER then I guess we can use it
> > here, too.
> 
> Hm, I'm not sure how far we need to distinguish between generic and
> device-specific features in this case. The "driver" as the whole entity
> driving the device needs to decide on the subset; at this stage, it does
> not really matter which parts of the driver code accepted which
> feature. We probably want to be explicit that features are ring,
> transport, and device features, though.

OK.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


  reply	other threads:[~2023-05-16 10:05 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-16  3:01 [virtio-comment] [PATCH] content: Replace guest OS with driver Parav Pandit
2023-05-16  3:01 ` [virtio-dev] " Parav Pandit
2023-05-16  4:12 ` [virtio-comment] " Jason Wang
2023-05-16  4:12   ` [virtio-dev] " Jason Wang
2023-05-16  5:54 ` [virtio-comment] " Michael S. Tsirkin
2023-05-16  5:54   ` [virtio-dev] " Michael S. Tsirkin
2023-05-16  8:24   ` [virtio-comment] " Cornelia Huck
2023-05-16  8:24     ` [virtio-dev] " Cornelia Huck
2023-05-16 10:05     ` Michael S. Tsirkin [this message]
2023-05-16 10:05       ` Michael S. Tsirkin
2023-05-16 11:47       ` [virtio-comment] " Cornelia Huck
2023-05-16 11:47         ` [virtio-dev] " Cornelia Huck
2023-05-16 19:50       ` [virtio-comment] " Parav Pandit
2023-05-16 19:50         ` [virtio-dev] " Parav Pandit
2023-05-16 20:47         ` [virtio-comment] " Michael S. Tsirkin
2023-05-16 20:47           ` [virtio-dev] " Michael S. Tsirkin
2023-05-16 20:59           ` [virtio-comment] " Parav Pandit
2023-05-16 20:59             ` [virtio-dev] " Parav Pandit
2023-05-16 21:25             ` [virtio-comment] " Michael S. Tsirkin
2023-05-16 21:25               ` [virtio-dev] " Michael S. Tsirkin
2023-05-16 21:31               ` [virtio-comment] " Parav Pandit
2023-05-16 21:31                 ` [virtio-dev] " Parav Pandit
2023-05-16 21:48                 ` [virtio-comment] " Michael S. Tsirkin
2023-05-16 21:48                   ` [virtio-dev] " Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230516054313-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=parav@nvidia.com \
    --cc=shahafs@nvidia.com \
    --cc=virtio-comment@lists.oasis-open.org \
    --cc=virtio-dev@lists.oasis-open.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.