From: David Stevens <stevensd@chromium.org>
To: "Zhu, Lingshan" <lingshan.zhu@intel.com>
Cc: "Michael S . Tsirkin" <mst@redhat.com>,
Jason Wang <jasowang@redhat.com>,
virtio-comment@lists.oasis-open.org,
virtio-dev@lists.oasis-open.org, parav@nvidia.com
Subject: [virtio-comment] Re: [PATCH v5 1/1] Add SUSPEND bit to device status
Date: Wed, 28 Feb 2024 10:18:16 +0900 [thread overview]
Message-ID: <CAD=HUj66_gBBbf5EZ2dDrc51hKQ83zJotgS8vRanrxbD0oy_vw@mail.gmail.com> (raw)
In-Reply-To: <e4caa5e3-2cd3-465e-aa9c-275b4426d31d@intel.com>
On Tue, Feb 27, 2024 at 5:52 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> On 2/27/2024 3:59 PM, David Stevens wrote:
> > On Tue, Feb 27, 2024 at 11:22 AM Zhu, Lingshan <lingshan.zhu@intel.com> wrote:
> >> On 2/27/2024 9:53 AM, David Stevens wrote:
> >>> Add a SUSPEND bit to the device status field to allow drivers to suspend
> >>> virtio devices. On systems where drivers don't directly manage interrupt
> >>> routing (e.g. Linux), this allows the drivers to suspend their devices
> >>> and prevent interrupts from being sent when the interrupt routing system
> >>> does not expect interrupts.
> >>> ---
> >>> content.tex | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++---
> >>> 1 file changed, 84 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/content.tex b/content.tex
> >>> index 0a62dce5f65f..2183c63c45ea 100644
> >>> --- a/content.tex
> >>> +++ b/content.tex
> >>> @@ -49,6 +49,9 @@ \section{\field{Device Status} Field}\label{sec:Basic Facilities of a Virtio Dev
> >>>
> >>> \item[DEVICE_NEEDS_RESET (64)] Indicates that the device has experienced
> >>> an error from which it can't recover.
> >>> +
> >>> +\item[SUSPEND (16)] Indicates that the device has been suspended by the
> >>> + driver. Only valid when the VIRTIO_F_SUSPEND feature bit is negotiated.
> >>> \end{description}
> >>>
> >>> The \field{device status} field starts out as 0, and is reinitialized to 0 by
> >>> @@ -60,9 +63,11 @@ \section{\field{Device Status} Field}\label{sec:Basic Facilities of a Virtio Dev
> >>> initialization sequence specified in
> >>> \ref{sec:General Initialization And Device Operation / Device
> >>> Initialization}.
> >>> -The driver MUST NOT clear a
> >>> -\field{device status} bit. If the driver sets the FAILED bit,
> >>> -the driver MUST later reset the device before attempting to re-initialize.
> >>> +
> >>> +The driver MUST NOT clear a \field{device status} bit, except for the
> >>> +SUSPEND bit as described in \ref{sec:General Initialization And Device
> >>> +Operation / Device Suspend}. If the driver sets the FAILED bit, the
> >>> +driver MUST later reset the device before attempting to re-initialize.
> >> Comparing to add a new exception, why not just re-setting DRIVER_OK to
> >> let the device
> >> clearing SUSPEND? This issue has been addressed when Eugenio working on
> >> a similar STOP_BIT
> >>
> >> https://lists.oasis-open.org/archives/virtio-dev/202111/msg00020.html
> > I don't think the device status bit is bit-addressable, so it's not
> > possible to re-set DRIVER_OK without also either re-setting or
> > clearing SUSPEND.
> Please correct me if I misunderstand your point,
> I think it can be addressed, for example, in PCI it is an u8,
> the device can clear a bit in it.
>
> It may be easier to let the device to clear the SUSPEND bit.
If it's a u8, then the driver will be writing all 8 bits at the same
time. So I don't see how it's possible for the driver to set one bit
without also setting/clearing all the other bits at the same time.
When you say the driver can re-set DRIVER_OK, it can either write 0x1F
or 0xF. If we don't allow clearing the suspend bit, then it has to
write 0x1F. But that's exactly what the driver wrote to suspend the
device in the first place - how is the device supposed to tell the
difference? I guess we could add something to the spec saying that's
how the device is supposed to interpret it. But at that point, that's
really the same as allowing the driver to clear the suspend bit, just
with more complexity.
-David
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/
next prev parent reply other threads:[~2024-02-28 1:18 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-27 1:53 [virtio-comment] [PATCH v5 0/1] Define a low power mode for devices David Stevens
2024-02-27 1:53 ` [virtio-comment] [PATCH v5 1/1] Add SUSPEND bit to device status David Stevens
2024-02-27 2:22 ` [virtio-comment] " Zhu, Lingshan
2024-02-27 7:59 ` David Stevens
2024-02-27 8:51 ` Zhu, Lingshan
2024-02-28 1:18 ` David Stevens [this message]
2024-02-28 6:45 ` Zhu, Lingshan
2024-02-29 3:20 ` David Stevens
2024-02-29 8:55 ` [virtio-comment] Re: [virtio-dev] " Zhu, Lingshan
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='CAD=HUj66_gBBbf5EZ2dDrc51hKQ83zJotgS8vRanrxbD0oy_vw@mail.gmail.com' \
--to=stevensd@chromium.org \
--cc=jasowang@redhat.com \
--cc=lingshan.zhu@intel.com \
--cc=mst@redhat.com \
--cc=parav@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).