Re: [virtio-comment] [RFC PATCH v2 2/2] virtio-gpu: add context init support

[Chia-I Wu <olvaffe@gmail.com>]

Thanks.  v1 looks good to me.

On Wed, Sep 8, 2021 at 6:33 PM Gurchetan Singh
<gurchetansingh@chromium.org> wrote:
>
>
>
> On Thu, Sep 2, 2021 at 9:59 AM Chia-I Wu <olvaffe@gmail.com> wrote:
>>
>> On Mon, Aug 30, 2021 at 12:42 PM Gurchetan Singh
>> <gurchetansingh@chromium.org> wrote:
>> > +\item[\field{info}] Information associated with the command.  If
>> > +  VIRTIO_GPU_F_CONTEXT_INIT is supported, then the driver MAY set
>> > +  VIRTIO_GPU_FLAG_INFO_RING_IDX bit in the request \field{flags}.  In
>> > +  that case:
>> > +  \begin{itemize*}
>> > +  \item VIRTIO_GPU_FLAG_FENCE MUST also be set
>> VIRTIO_GPU_FLAG_FENCE is optional currently and has a cost.  There
>> might be use cases where fencing is not needed even with rings.  Why
>> is it required when VIRTIO_GPU_FLAG_INFO_RING_IDX is set?
>
>
> The latest non-RFC spec changes don't require VIRTIO_GPU_FLAG_INFO_RING_IDX + VIRTIO_GPU_FLAG_FENCE to go in unison.  Implementation-wise, command buffer submission always sets VIRTIO_GPU_FLAG_FENCE at the moment (prior behavior unchanged with multiple-rings).  More guest-side changes (say VIRTGPU_EXECBUF_NO_WAIT) would be required if one wants to submit a command without waiting for a response (or use the ASG technique).
>
>>
>>
>> > +  \item the lower 5 bits of \field{info} indicates the value of a
>> > +   context-specific ring index.  The minimum value is 0 and maximum
>> > +   value is 31.
>> For Venus at least, we plan to assign each VkQueue of each logical
>> VkDevice a ring index.  While 32 for a logical VkDevice seems enough
>> (TM), there are apps that require more than one logical device.
>>
>> Should we reserve 8 bits and allow up to 63?  This way the last two
>> bits can be flexible if we run out of bit space in the future.
>
>
> Done.
>
>
>
>>
>>
>> > +  \item \field{fence_id} acts as a sequence number on the synchronization
>> > +   timeline defined by \field{ctx_idx} and the ring index.
>> > +  \item when the command associated with \field{fence_id} is complete, the
>> > +  device MUST send a response for all outstanding commands with a sequence
>> > +  number less than or equal to \field{fence_id} on the same
>> > +  synchronization timeline.
>> > +  \end{itemize*}
>> >  \end{description}
>> >
>> >  On success the device will return VIRTIO_GPU_RESP_OK_NODATA in
>> > @@ -516,6 +533,12 @@ \subsubsection{Device Operation: controlq}\label{sec:Device Types / GPU Device /
>> >         the first edition of Virgl (Gallium OpenGL) protocol.
>> >    \item \href{https://gitlab.freedesktop.org/virgl/virglrenderer/-/blob/master/src/virgl_hw.h#L550}{VIRTIO_GPU_CAPSET_VIRGL2} --
>> >         the second edition of Virgl (Gallium OpenGL) protocol after the capset fix.
>> > +  \item \href{https://android.googlesource.com/device/generic/vulkan-cereal/+/refs/heads/master/protocols/}{VIRTIO_GPU_CAPSET_GFXSTREAM} --
>> > +       gfxtream's (mostly) autogenerated GLES and Vulkan streaming protocols.
>> > +  \item \href{https://gitlab.freedesktop.org/olv/venus-protocol}{VIRTIO_GPU_CAPSET_VENUS} --
>> > +       Mesa's (mostly) autogenerated Vulkan protocol.
>> > +  \item \href{https://chromium.googlesource.com/chromiumos/platform/crosvm/+/refs/heads/main/rutabaga_gfx/src/cross_domain/cross_domain_protocol.rs}{VIRTIO_GPU_CAPSET_CROSS_DOMAIN} --
>> > +       protocol for display virtualization via Wayland proxying.
>> >    \end{itemize*}
>> >
>> >  \begin{lstlisting}
>> > @@ -527,6 +550,9 @@ \subsubsection{Device Operation: controlq}\label{sec:Device Types / GPU Device /
>> >
>> >  #define VIRTIO_GPU_CAPSET_VIRGL 1
>> >  #define VIRTIO_GPU_CAPSET_VIRGL2 2
>> > +#define VIRTIO_GPU_CAPSET_GFXSTREAM 3
>> > +#define VIRTIO_GPU_CAPSET_VENUS 4
>> > +#define VIRTIO_GPU_CAPSET_CROSS_DOMAIN 5
>> >  struct virtio_gpu_resp_capset_info {
>> >          struct virtio_gpu_ctrl_hdr hdr;
>> >          le32 capset_id;
>> > @@ -684,21 +710,46 @@ \subsubsection{Device Operation: controlq (3d)}\label{sec:Device Types / GPU Dev
>> >
>> >  \begin{description}
>> >
>> > -\item[VIRTIO_GPU_CMD_CTX_CREATE]
>> > +\item[VIRTIO_GPU_CMD_CTX_CREATE] creates a context for submitting an opaque
>> > +  command stream.  Request data is \field{struct virtio_gpu_ctx_create}.
>> > +  Response type is VIRTIO_GPU_RESP_OK_NODATA.
>> > +
>> > +\begin{lstlisting}
>> > +#define VIRTIO_GPU_CONTEXT_INIT_CAPSET_ID_MASK 0x0000003f;
>> > +struct virtio_gpu_ctx_create {
>> > +       struct virtio_gpu_ctrl_hdr hdr;
>> > +       le32 nlen;
>> > +       le32 context_init;
>> > +       char debug_name[64];
>> > +};
>> > +\end{lstlisting}
>> > +
>> > +The implementation MUST create a context for the given \field{ctx_id} in
>> > +the \field{hdr}.  For debugging purposes, a \field{debug_name} and it's
>> > +length \field{nlen} is provided by the driver.  If
>> > +VIRTIO_GPU_F_CONTEXT_INIT is supported, then lower 6 bits of
>> > +\field{context_init} MAY contain the \field{capset_id} associated with
>> > +context.  In that case, then the device MUST create a context that can
>> > +handle the specified command stream.
>> > +
>> > +If the lower 6-bits of the \field{context_init} are zero, then the type of
>> > +the context is determined by the device.
>> > +
>> >  \item[VIRTIO_GPU_CMD_CTX_DESTROY]
>> >  \item[VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE]
>> >  \item[VIRTIO_GPU_CMD_CTX_DETACH_RESOURCE]
>> > -  Manage OpenGL contexts.
>> > +  Manage virtio-gpu 3d contexts.
>> >
>> >  \item[VIRTIO_GPU_CMD_RESOURCE_CREATE_3D]
>> > -  Create OpenGL resources.
>> > +  Create virtio-gpu 3d resources.
>> >
>> >  \item[VIRTIO_GPU_CMD_TRANSFER_TO_HOST_3D]
>> >  \item[VIRTIO_GPU_CMD_TRANSFER_FROM_HOST_3D]
>> > -  Transfer data from and to OpenGL resources.
>> > +  Transfer data from and to virtio-gpu 3d resources.
>> >
>> >  \item[VIRTIO_GPU_CMD_SUBMIT_3D]
>> > -  Submit rendering commands (mesa gallium command stream).
>> > +  Submit an opaque command stream.  The type of the command stream is
>> > +  determined when creating a context.
>> >
>> >  \item[VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB] maps a host-only
>> >    blob resource into an offset in the host visible memory region. Request
>> > --
>> > 2.31.0
>> >
>> >
>> > 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/
>> >