public inbox for virtio-comment@lists.linux.dev
 help / color / mirror / Atom feed
From: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
To: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Cc: virtio-comment@lists.linux.dev
Subject: Re: [PATCH] gpu: add missing controlq (3d) struct listings
Date: Fri, 27 Mar 2026 17:10:57 +0100	[thread overview]
Message-ID: <acasEcGGGUpzQExl@fedora> (raw)
In-Reply-To: <20260304-virtio-gpu-missing-structs-v1-1-654218322e6a@linaro.org>


Hello Manos,

I used Claude to review the patch and I added some comments, most of them
minors.

On Wed, Mar 04, 2026 at 09:41:17AM +0200, Manos Pitsidianakis wrote:
> Lifted from include/uapi/linux/virtio_gpu.h with adjustments.
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/239
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>  device-types/gpu/description.tex | 75 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/device-types/gpu/description.tex b/device-types/gpu/description.tex
> index ac3b427725f6036ca069b398fdde712cab833225..5adf02c5ac9bd80fb0986bc3847cff0698f85801 100644
> --- a/device-types/gpu/description.tex
> +++ b/device-types/gpu/description.tex
> @@ -749,21 +749,94 @@ \subsubsection{Device Operation: controlq (3d)}\label{sec:Device Types / GPU Dev
>  If the lower 8-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_DESTROY] destroyes a context.

s/destroyes/destroys

> +  Request data is \field{struct virtio_gpu_ctx_destroy}.
> +  Response type is VIRTIO_GPU_RESP_OK_NODATA.
> +
> +\begin{lstlisting}
> +struct virtio_gpu_ctx_destroy {
> +        struct virtio_gpu_ctrl_hdr hdr;
> +};
> +\end{lstlisting}
> +
>  \item[VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE]
>  \item[VIRTIO_GPU_CMD_CTX_DETACH_RESOURCE]
>    Manage virtio-gpu 3d contexts.
> +  Request data is \field{struct virtio_gpu_ctx_resource}.
> +  Response type is VIRTIO_GPU_RESP_OK_NODATA.
> +
> +\begin{lstlisting}
> +struct virtio_gpu_ctx_resource {
> +        struct virtio_gpu_ctrl_hdr hdr;
> +        le32 resource_id;
> +        le32 padding;
> +};
> +\end{lstlisting}
>  
>  \item[VIRTIO_GPU_CMD_RESOURCE_CREATE_3D]
>    Create virtio-gpu 3d resources.
> +  Request data is \field{struct virtio_gpu_resource_create_3d}.
> +  Response type is VIRTIO_GPU_RESP_OK_NODATA.
> +
> +\begin{lstlisting}
> +#define VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP (1 << 0)

VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP is not explained anywhere, however,
most of the defines in this file are not well explained so I think it is
OK.

> +struct virtio_gpu_resource_create_3d {
> +        struct virtio_gpu_ctrl_hdr hdr;
> +        le32 resource_id;
> +        le32 target;
> +        le32 format;
> +        le32 bind;
> +        le32 width;
> +        le32 height;
> +        le32 depth;
> +        le32 array_size;
> +        le32 last_level;
> +        le32 nr_samples;
> +        le32 flags;
> +        le32 padding;
> +};
> +\end{lstlisting}
>  
Would it worth to explain some of these fields like for the field `size`
in `virtio_gpu_cmd_submit`?

>  \item[VIRTIO_GPU_CMD_TRANSFER_TO_HOST_3D]
>  \item[VIRTIO_GPU_CMD_TRANSFER_FROM_HOST_3D]
>    Transfer data from and to virtio-gpu 3d resources.
> +  Request data is \field{struct virtio_gpu_transfer_host_3d}.
> +  Response type is VIRTIO_GPU_RESP_OK_NODATA.
> +
> +\begin{lstlisting}
> +struct virtio_gpu_box {
> +        le32 x, y, z;
> +        le32 w, h, d;
> +};
> +
> +struct virtio_gpu_transfer_host_3d {
> +        struct virtio_gpu_ctrl_hdr hdr;
> +        struct virtio_gpu_box box;
> +        le64 offset;
> +        le32 resource_id;
> +        le32 level;
> +        le32 stride;
> +        le32 layer_stride;
> +};
> +\end{lstlisting}
>  
>  \item[VIRTIO_GPU_CMD_SUBMIT_3D]
>    Submit an opaque command stream.  The type of the command stream is
>    determined when creating a context.
> +  Request data is \field{struct virtio_gpu_cmd_submit}, followed by the stream bytes.
> +  Response type is VIRTIO_GPU_RESP_OK_NODATA.
> +
> +\begin{lstlisting}
> +struct virtio_gpu_cmd_submit {
> +        struct virtio_gpu_ctrl_hdr hdr;
> +        le32 size;
> +        le32 padding;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +  \item[\field{size}] the size of the opaque command stream in bytes.
> +\end{description}
>  
>  \item[VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB] maps a host-only
>    blob resource into an offset in the host visible memory region. Request
> 
> ---
> base-commit: 1ba6e8108644d598cf9c772bac7b856fb5c7df6b
> change-id: 20260304-virtio-gpu-missing-structs-dc8f75a7d370
> 
> Best regards,
> -- 
> Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> 
> 


      parent reply	other threads:[~2026-03-27 16:11 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-04  7:41 [PATCH] gpu: add missing controlq (3d) struct listings Manos Pitsidianakis
2026-03-04 13:21 ` Alex Bennée
2026-03-27 16:10 ` Matias Ezequiel Vara Larsen [this message]

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=acasEcGGGUpzQExl@fedora \
    --to=mvaralar@redhat.com \
    --cc=manos.pitsidianakis@linaro.org \
    --cc=virtio-comment@lists.linux.dev \
    /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