Re: [PATCH v1] virtio-gpu: Document new fence-passing feature

[Dmitry Osipenko <dmitry.osipenko@collabora.com>]

On 1/30/24 04:34, Gurchetan Singh wrote:
> On Sun, Jan 28, 2024 at 11:12âAM Dmitry Osipenko <
> dmitry.osipenko@collabora.com> wrote:
> 
>> Document new virtio-gpu protocol fence-passing feature that extends 3d
>> submit command with support of in-fences which are passed from guest to
>> host for waiting. This feature allows to offload waiting for in-fences
>> from guest to host, where it can be done more efficiently.
>>
>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> ---
>>  device-types/gpu/description.tex | 22 +++++++++++++++++++++-
>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/device-types/gpu/description.tex
>> b/device-types/gpu/description.tex
>> index 443524851a05..e5e23b5c9072 100644
>> --- a/device-types/gpu/description.tex
>> +++ b/device-types/gpu/description.tex
>> @@ -37,6 +37,8 @@ \subsection{Feature bits}\label{sec:Device Types / GPU
>> Device / Feature bits}
>>    resources is supported.
>>  \item[VIRTIO_GPU_F_CONTEXT_INIT (4)] multiple context types and
>>    synchronization timelines supported.  Requires VIRTIO_GPU_F_VIRGL.
>> +\item[VIRTIO_GPU_F_FENCE_PASSING (5)] passing fence IDs from guest to host
>> +  for waiting supported.  Requires VIRTIO_GPU_F_VIRGL.
>>  \end{description}
>>
>>  \subsection{Device configuration layout}\label{sec:Device Types / GPU
>> Device / Device configuration layout}
>> @@ -746,7 +748,25 @@ \subsubsection{Device Operation: controlq
>> (3d)}\label{sec:Device Types / GPU Dev
>>
>>  \item[VIRTIO_GPU_CMD_SUBMIT_3D]
>>    Submit an opaque command stream.  The type of the command stream is
>> -  determined when creating a context.
>> +  determined when creating a context.  Request data is
>> +  \field{struct virtio_gpu_cmd_submit}.  The \field{size} field describes
>> +  the size of \field{cmd_data} array in bytes.  The \field{num_in_fences}
>> +  field is active only if VIRTIO_GPU_F_FENCE_PASSING is enabled, otherwise
>> +  \field{num_in_fences} is treated as zero.  The array of
>> \field{in_fences}
>> +  IDs is placed in between the \field{num_in_fences} field and the
>> +  \field{cmd_data} array.  The \field{in_fences} array contains virtio-gpu
>> +  \field{fence_id}'s corresponding to the out-fence IDs of a previous
>> +  3d submits.
>> +
>> +\begin{lstlisting}
>> +struct virtio_gpu_cmd_submit {
>> +        struct virtio_gpu_ctrl_hdr hdr;
>> +        le32 size;
>> +        le32 num_in_fences;
>> +        le64 in_fences[num_in_fences];
>> +        u32 cmd_data[size/4];
>> +};
>> +\end{lstlisting}
>>
> 
> Don't you need VIRTIO_GPU_FLAG_FENCE_SHAREABLE somewhere too?

We haven't made use of this flag in the experimental/prototype version
that was made for the native contexts. The flag is unused by both Qemu
and CrosVM.

We added Linux kernel UAPI flag VIRTGPU_EXECBUF_SHARED_FENCE and it's
deemed to be enough. If the UAPI flag isn't set for a job's out-fence,
then guest uses guest-side synchronization for this fence when it's
passed as in-fence for the further jobs.

The FENCE_SHAREABLE flag isn't needed by nctx/virgl/venus protocols. All
fences are shareable by default. If VMM fails to export fence, it is
treated like fence was already signaled. In case of Venus, it's up to
Venus to decide internally which fences can be made shareable, i.e. both
guest and virglrenderer Venus implementations are responsible for making
decision whether fence should be made exportable. Venus won't make
out-fence exportable for internal intermediate jobs and will encode this
export-info within the Venus protocol.

Whether FENCE_SHAREABLE virtio-gpu flag will be needed by gfxstream is
questionable. Gfxstream should be similar to Venus, i.e. you should be
able to add flag to the gfxstream. We'll need to be certain that the new
common flag is definitely needed by making use of it before specifying
it in the stable virtio-gpu protocol.

-- 
Best regards,
Dmitry