[virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

[virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

[David Stevens]

Signed-off-by: David Stevens <stevensd@chromium.org>
---
 virtio-gpu.tex | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/virtio-gpu.tex b/virtio-gpu.tex
index af4ca61..522f478 100644
--- a/virtio-gpu.tex
+++ b/virtio-gpu.tex
@@ -186,12 +186,16 @@ \subsubsection{Device Operation: Request
header}\label{sec:Device Types / GPU De
         VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300,
         VIRTIO_GPU_CMD_MOVE_CURSOR,

+        /* misc commands */
+        VIRTIO_GPU_CMD_EXPORT_RESOURCE = 0x0400,
+
         /* success responses */
         VIRTIO_GPU_RESP_OK_NODATA = 0x1100,
         VIRTIO_GPU_RESP_OK_DISPLAY_INFO,
         VIRTIO_GPU_RESP_OK_CAPSET_INFO,
         VIRTIO_GPU_RESP_OK_CAPSET,
         VIRTIO_GPU_RESP_OK_EDID,
+        VIRTIO_GPU_RESP_OK_EXPORT_RESOURCE,

         /* error responses */
         VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200,
@@ -454,6 +458,31 @@ \subsubsection{Device Operation:
controlq}\label{sec:Device Types / GPU Device /
 This detaches any backing pages from a resource, to be used in case of
 guest swapping or object destruction.

+\item[VIRTIO_GPU_CMD_EXPORT_RESOURCE] Exports a resource for use by other
+  virtio devices. Request data is \field{struct
+    virtio_gpu_export_resource}.  Response type is
+  VIRTIO_GPU_RESP_OK_EXPORT_RESOURCE, response data is \field{struct
+    virtio_gpu_resp_export_resource}.
+
+\begin{lstlisting}
+struct virtio_gpu_export_resource {
+        struct virtio_gpu_ctrl_hdr hdr;
+        le32 resource_id;
+        le32 padding;
+};
+
+struct virtio_gpu_resp_export_resource {
+        struct virtio_gpu_ctrl_hdr hdr;
+        le64 uuid_low;
+        le64 uuid_high;
+};
+\end{lstlisting}
+
+The response contains a uuid which identifies the host private resource to
+other virtio devices. Note that if the resource has an attached backing,
+modifications made to an exported resource by other devices are not visible
+in the attached backing until they are transferred into the backing.
+
 \end{description}

 \subsubsection{Device Operation: cursorq}\label{sec:Device Types /
GPU Device / Device Operation / Device Operation: cursorq}
-- 
2.24.1.735.g03f4e72817-goog

Re: [virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

[Gerd Hoffmann]

> +\begin{lstlisting}
> +struct virtio_gpu_export_resource {
> +        struct virtio_gpu_ctrl_hdr hdr;
> +        le32 resource_id;
> +        le32 padding;
> +};
> +
> +struct virtio_gpu_resp_export_resource {
> +        struct virtio_gpu_ctrl_hdr hdr;
> +        le64 uuid_low;
> +        le64 uuid_high;
> +};
> +\end{lstlisting}

Is there a specific reason why you want the host pick the uuid?  I would
let the guest define the uuid, i.e. move the uuid fields to
virtio_gpu_export_resource and scratch virtio_gpu_resp_export_resource.

Also I'd siggest to name the command (and struct) RESOURCE_ASSIGN_UUID.

cheers,
  Gerd

Re: [virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

[David Stevens]

> Is there a specific reason why you want the host pick the uuid?  I would
> let the guest define the uuid, i.e. move the uuid fields to
> virtio_gpu_export_resource and scratch virtio_gpu_resp_export_resource.

Sending the uuid in the original request doesn't really buy us
anything, at least in terms of asynchronicity. The guest would still
need to wait for the response to arrive before it could safely pass
the uuid to any other virtio devices, to prevent a race where the
import fails because it is processed before virtio-gpu processes the
export. Perhaps this wouldn't be the case if we supported sharing
fences between virtio devices, but even then, fences are more of a
thing for the operation of a pipeline, not for the setup of a
pipeline.

At that point, I think it's just a matter of aesthetics. I lean
slightly towards returning the uuid from the host, since that rules
out any implementation with the aforementioned race. That being said,
if there are any specific reasons or preferences to assigning the uuid
from the guest, I can switch to that direction.

-David

Re: [virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

[Gerd Hoffmann]

  Hi,

> At that point, I think it's just a matter of aesthetics. I lean
> slightly towards returning the uuid from the host, since that rules
> out any implementation with the aforementioned race.

Ok, design the API in a way that you can't get it wrong.  Makes sense.
I'd still name it ressource_assign_uuid though.

cheers,
  Gerd

Re: [virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

[Gurchetan Singh]

I like the idea of having one central place in the kernel where
virtio-devices get their uuid from -- i.e, no separate VM specific,
device specific implementations calling into uuid_gen().

On Wed, Jan 8, 2020 at 3:20 AM David Stevens <stevensd@chromium.org> wrote:
>
> > Is there a specific reason why you want the host pick the uuid?  I would
> > let the guest define the uuid, i.e. move the uuid fields to
> > virtio_gpu_export_resource and scratch virtio_gpu_resp_export_resource.
>
> Sending the uuid in the original request doesn't really buy us
> anything, at least in terms of asynchronicity. The guest would still
> need to wait for the response to arrive before it could safely pass
> the uuid to any other virtio devices, to prevent a race where the
> import fails because it is processed before virtio-gpu processes the
> export. Perhaps this wouldn't be the case if we supported sharing
> fences between virtio devices, but even then, fences are more of a
> thing for the operation of a pipeline, not for the setup of a
> pipeline.
>
> At that point, I think it's just a matter of aesthetics. I lean
> slightly towards returning the uuid from the host, since that rules
> out any implementation with the aforementioned race. That being said,
> if there are any specific reasons or preferences to assigning the uuid
> from the guest, I can switch to that direction.
>
> -David

Re: [virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

[David Stevens]

> ok but how is this then used? will there be more commands to pass
> this uuid to another device?

This is intended to be used with the virtio video device being
discussed here https://markmail.org/thread/ingyqlps4rbcuazh. I don't
have a specific patch for how that will work, but it will likely be an
extension to VIRTIO_VIDEO_T_RESOURCE_CREATE.

> > +The response contains a uuid which identifies the exported object created from
> > +the host private resource.
>
> Are the uuids as specified in rfc-4122? I guess we need to link to that spec then

I don't think it's terribly important to specify how the uuids are
generated, as long as they're actually unique. That being said, I'm
not opposed to defining them as rfc-4122 version 4 uuids. Although if
we do that, it should go in the patch that defines what exported
objects and uuids are in the context of virtio, not in the virtio-gpu
section.

> > Note that if the resource has an attached backing,
> > +modifications made to the host private resource through the exported object by
> > +other devices are not visible in the attached backing until they are
> > transferred
> > +into the backing.
> > +
>
> s/host/device/?

The virtio-gpu is based around "resources private to the host", to
quote the existing specification. I think consistency with that
language is important.

-David

Re: [virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

[Michael S. Tsirkin]

On Wed, Jan 22, 2020 at 04:16:35PM +0900, David Stevens wrote:
> Signed-off-by: David Stevens <stevensd@chromium.org>
> ---
>  virtio-gpu.tex | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/virtio-gpu.tex b/virtio-gpu.tex
> index af4ca61..a1f0210 100644
> --- a/virtio-gpu.tex
> +++ b/virtio-gpu.tex
> @@ -186,12 +186,16 @@ \subsubsection{Device Operation: Request
> header}\label{sec:Device Types / GPU De
>          VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300,
>          VIRTIO_GPU_CMD_MOVE_CURSOR,
> 
> +        /* misc commands */
> +        VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID = 0x0400,
> +
>          /* success responses */
>          VIRTIO_GPU_RESP_OK_NODATA = 0x1100,
>          VIRTIO_GPU_RESP_OK_DISPLAY_INFO,
>          VIRTIO_GPU_RESP_OK_CAPSET_INFO,
>          VIRTIO_GPU_RESP_OK_CAPSET,
>          VIRTIO_GPU_RESP_OK_EDID,
> +        VIRTIO_GPU_RESP_OK_RESOURCE_ASSIGN_UUID,
> 
>          /* error responses */
>          VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200,
> @@ -454,6 +458,32 @@ \subsubsection{Device Operation:
> controlq}\label{sec:Device Types / GPU Device /
>  This detaches any backing pages from a resource, to be used in case of
>  guest swapping or object destruction.
> 
> +\item[VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID] Creates an exported object from
> +  a resource. Request data is \field{struct
> +    virtio_gpu_resource_assign_uuid}.  Response type is
> +  VIRTIO_GPU_RESP_OK_RESOURCE_ASSIGN_UUID, response data is \field{struct
> +    virtio_gpu_resp_resource_assign_uuid}.
> +
> +\begin{lstlisting}
> +struct virtio_gpu_resource_assign_uuid {
> +        struct virtio_gpu_ctrl_hdr hdr;
> +        le32 resource_id;
> +        le32 padding;
> +};
> +
> +struct virtio_gpu_resp_resource_assign_uuid {
> +        struct virtio_gpu_ctrl_hdr hdr;
> +        le64 uuid_low;
> +        le64 uuid_high;
> +};
> +\end{lstlisting}
> +

ok but how is this then used? will there be more commands to pass
this uuid to another device?

> +The response contains a uuid which identifies the exported object created from
> +the host private resource.

Are the uuids as specified in rfc-4122? I guess we need to link to that spec then
..

> Note that if the resource has an attached backing,
> +modifications made to the host private resource through the exported object by
> +other devices are not visible in the attached backing until they are
> transferred
> +into the backing.
> +

s/host/device/?

>  \end{description}
> 
>  \subsubsection{Device Operation: cursorq}\label{sec:Device Types /
> GPU Device / Device Operation / Device Operation: cursorq}
> -- 
> 2.25.0.341.g760bfbb309-goog
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev][RFC PATCH v1 2/2] virtio-gpu: add the ability to export resources

[David Stevens]

Signed-off-by: David Stevens <stevensd@chromium.org>
---
 virtio-gpu.tex | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/virtio-gpu.tex b/virtio-gpu.tex
index af4ca61..a1f0210 100644
--- a/virtio-gpu.tex
+++ b/virtio-gpu.tex
@@ -186,12 +186,16 @@ \subsubsection{Device Operation: Request
header}\label{sec:Device Types / GPU De
         VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300,
         VIRTIO_GPU_CMD_MOVE_CURSOR,

+        /* misc commands */
+        VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID = 0x0400,
+
         /* success responses */
         VIRTIO_GPU_RESP_OK_NODATA = 0x1100,
         VIRTIO_GPU_RESP_OK_DISPLAY_INFO,
         VIRTIO_GPU_RESP_OK_CAPSET_INFO,
         VIRTIO_GPU_RESP_OK_CAPSET,
         VIRTIO_GPU_RESP_OK_EDID,
+        VIRTIO_GPU_RESP_OK_RESOURCE_ASSIGN_UUID,

         /* error responses */
         VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200,
@@ -454,6 +458,32 @@ \subsubsection{Device Operation:
controlq}\label{sec:Device Types / GPU Device /
 This detaches any backing pages from a resource, to be used in case of
 guest swapping or object destruction.

+\item[VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID] Creates an exported object from
+  a resource. Request data is \field{struct
+    virtio_gpu_resource_assign_uuid}.  Response type is
+  VIRTIO_GPU_RESP_OK_RESOURCE_ASSIGN_UUID, response data is \field{struct
+    virtio_gpu_resp_resource_assign_uuid}.
+
+\begin{lstlisting}
+struct virtio_gpu_resource_assign_uuid {
+        struct virtio_gpu_ctrl_hdr hdr;
+        le32 resource_id;
+        le32 padding;
+};
+
+struct virtio_gpu_resp_resource_assign_uuid {
+        struct virtio_gpu_ctrl_hdr hdr;
+        le64 uuid_low;
+        le64 uuid_high;
+};
+\end{lstlisting}
+
+The response contains a uuid which identifies the exported object created from
+the host private resource. Note that if the resource has an attached backing,
+modifications made to the host private resource through the exported object by
+other devices are not visible in the attached backing until they are
transferred
+into the backing.
+
 \end{description}

 \subsubsection{Device Operation: cursorq}\label{sec:Device Types /
GPU Device / Device Operation / Device Operation: cursorq}
-- 
2.25.0.341.g760bfbb309-goog