[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: RE: [PATCH v6 1/1] content: Add new feature VIRTIO_F_PRESERVE_RESOURCES
Hi Jiqian,
> From: Jiqian Chen <Jiqian.Chen@amd.com>
> Sent: Saturday, October 21, 2023 9:21 AM
> To: Michael S . Tsirkin <mst@redhat.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Parav Pandit <[email protected]>; Jason Wang
> <jasowang@redhat.com>; Xuan Zhuo <[email protected]>; David
> Airlie <airlied@redhat.com>; Gurchetan Singh
> <gurchetansingh@chromium.org>; Chia-I Wu <[email protected]>; Marc-
> André Lureau <marcandre.lureau@gmail.com>; Robert Beckett
> <bob.beckett@collabora.com>; Mikhail Golubev-Ciuchea <Mikhail.Golubev-
> Ciuchea@opensynergy.com>; [email protected]; virtio-
> dev@lists.oasis-open.org
> Cc: Honglei Huang <Honglei1.Huang@amd.com>; Julia Zhang
> <Julia.Zhang@amd.com>; Huang Rui <[email protected]>; Jiqian Chen
> <Jiqian.Chen@amd.com>
> Subject: [PATCH v6 1/1] content: Add new feature
> VIRTIO_F_PRESERVE_RESOURCES
>
> In some scenes, Qemu may reset or destroy resources of virtio device, but some
> of them can't be re-created, so that causes some problems.
>
It can be re-created. It is just that the guest driver lost the previous resource values.
So a better wording for the motivation is combining with below line to me is,
Currently guest drivers destroy and re-create virtio resources during power management suspend/resume sequence respectively.
For example, for a PCI transport, even if the device offers D3 to d0 state transition, a virtio guest driver has no way to know if
the device can store the state during D0->D3 transition and same state can be restored on D3->D0 transition.
> For example, when we do S3 for guest, guest will set device_status to 0, it
> causes Qemu to reset virtioi-gpu device, and then all render resources of virtio-
> gpu will be destroyed. As a result, after guest resuming, the display can't come
> back, and we only see a black screen.
>
To make guest drivers aware of above device capability, introduce a new feature bit to indicate such device capability.
This patch adds...
> In order to deal with the above scene, we need a mechanism that allows guest
> and Qemu to negotiate their behaviors for resources. So, this patch adds a new
> feature named VIRTIO_F_PRESERVE_RESOURCES. It allows guest to tell Qemu
> when there is a need to preserve resources, guest must preserve resources until
> 0 is set.
>
I think this can be done without introducing the new register.
Can you please check if the PM register itself can serve the purpose instead of new virtio level register?
> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
> ---
> conformance.tex | 2 ++
> content.tex | 25 +++++++++++++++++++++++++
> transport-pci.tex | 6 ++++++
> 3 files changed, 33 insertions(+)
>
> diff --git a/conformance.tex b/conformance.tex index dc00e84..60cc0b1
> 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -91,6 +91,7 @@ \section{Conformance Targets}\label{sec:Conformance /
> Conformance Targets} \item \ref{drivernormative:Basic Facilities of a Virtio
> Device / Packed Virtqueues / The Virtqueue Descriptor Table / Indirect
> Descriptors} \item \ref{drivernormative:Basic Facilities of a Virtio Device /
> Packed Virtqueues / Supplying Buffers to The Device / Updating flags} \item
> \ref{drivernormative:Basic Facilities of a Virtio Device / Packed Virtqueues /
> Supplying Buffers to The Device / Sending Available Buffer Notifications}
> +\item \ref{drivernormative:Basic Facilities of a Virtio Device /
> +Preserve Resources}
> \item \ref{drivernormative:General Initialization And Device Operation /
> Device Initialization} \item \ref{drivernormative:General Initialization And
> Device Operation / Device Cleanup} \item \ref{drivernormative:Reserved
> Feature Bits} @@ -172,6 +173,7 @@ \section{Conformance
> Targets}\label{sec:Conformance / Conformance Targets} \item
> \ref{devicenormative:Basic Facilities of a Virtio Device / Packed Virtqueues /
> The Virtqueue Descriptor Table} \item \ref{devicenormative:Basic Facilities of
> a Virtio Device / Packed Virtqueues / Scatter-Gather Support} \item
> \ref{devicenormative:Basic Facilities of a Virtio Device / Shared Memory
> Regions}
> +\item \ref{devicenormative:Basic Facilities of a Virtio Device /
> +Preserve Resources}
> \item \ref{devicenormative:Reserved Feature Bits} \end{itemize}
>
> diff --git a/content.tex b/content.tex
> index 0a62dce..b6b1859 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -502,6 +502,27 @@ \section{Exporting Objects}\label{sec:Basic Facilities
> of a Virtio Device / Expo types. It is RECOMMENDED that devices generate
> version 4 UUIDs as specified by \hyperref[intro:rfc4122]{[RFC4122]}.
>
> +\section{Preserve Resources}\label{sec:Basic Facilities of a Virtio
> +Device / Preserve Resources}
> +
> +As virtio devices are paravirtualization devices by design.
This is not true and not relevant for the spec. Please remove this line.
> +There are various devices resources created by sending commands from
> +frontend and stored in backend.
> +
> +In some scenes, resources may be destroyed or reset, some of them can
> +be re-created since frontend has enough information , but some can't.
> +At this case, we can set \field{Preserve Resources} to 1 by specific
> +transport, to prevent resources being destroyed.
> +
> +Which kind of resources need to be preserved and how to preserve
> +resources depend on specific devices.
s/on specific devices/on specific device type/
> +
> +\drivernormative{\subsection}{Preserve Resources}{Basic Facilities of a
> +Virtio Device / Preserve resources} A driver SHOULD set \field{Preserve
> +Resources} to 1 when there is a need to preserve resources.
> +
> +\devicenormative{\subsection}{Preserve Resources}{Basic Facilities of a
> +Virtio Device / Preserve resources} A device MUST NOT destroy resources until
> \field{Preserve Resources} is 0.
> +
> \input{admin.tex}
>
> \chapter{General Initialization And Device Operation}\label{sec:General
> Initialization And Device Operation} @@ -872,6 +893,10 @@
> \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> \ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits}
> for
> handling features reserved for future use.
>
> + \item[VIRTIO_F_PRESERVE_RESOURCES(42)] This feature indicates that
> + the device need to preserve resources.
> + See \ref{sec:Basic Facilities of a Virtio Device / Preserve Resources}.
> +
> \end{description}
>
> \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits} diff --
> git a/transport-pci.tex b/transport-pci.tex index a5c6719..f6eea65 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -325,6 +325,7 @@ \subsubsection{Common configuration structure
> layout}\label{sec:Virtio Transport
> /* About the administration virtqueue. */
> le16 admin_queue_index; /* read-only for driver */
> le16 admin_queue_num; /* read-only for driver */
> + le16 preserve_resources; /* read-write */
Preserving these resources in the device implementation takes finite amount of time.
Possibly more than 40nsec (time of PCIe write TLP).
Hence this register must be a polling register to indicate that preservation_done.
This will tell the guest when the preservation is done, and when restoration is done, so that it can resume upper layers.
Please refer to queue_reset definition to learn more about such register definition.
Lets please make sure that PCIe PM level registers are sufficient/not-sufficient to decide the addition of this register.
> };
> \end{lstlisting}
>
> @@ -428,6 +429,11 @@ \subsubsection{Common configuration structure
> layout}\label{sec:Virtio Transport
> The value 0 indicates no supported administration virtqueues.
> This field is valid only if VIRTIO_F_ADMIN_VQ has been
> negotiated.
> +
> +\item[\field{preserve_resources}]
> + The driver writes this to let device preserve resources whenever driver
> has demands.
> + 1 - device need to preserve resources which can't be re-created, until 0 is
> set.
> + 0 - all resources can be destroyed.
> \end{description}
>
> \devicenormative{\paragraph}{Common configuration structure layout}{Virtio
> Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common
> configuration structure layout}
> --
> 2.34.1
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]