Re: [RFC PATCH] admin-queue: bind the group member to the device

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jun 26, 2023 at 02:22:10PM +0800, Xuan Zhuo wrote:
> The VFs of the SR-IOV are created by the user inside the guest OS, so the virtio
> devices don't know about these VFs. Because each VF may be assigned a different role
> by the user, the virtio device can not choose one VF to bind random.
> So only the user knows how to bind the virtio devices to the VFs.
> On the other hand, generally the virtio devices are not created by the user
> inside the guest OS. This requires some management platform to participate.
> 
> So the usage of this command:
> 1. The user purchases a virtio network card on the management platform,
>    and sets the ip, queue number, etc. The user obtains the identity of
>    the network card.
> 2. The user creates a VF with echo 8 > sriov_numvfs

this one is unlikely, as this requires PF access and affects all VFs.

> 3. The user binds the net crad to a VF with identity through the command
>    of the patch
> 
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>




> ---
>  admin.tex | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/admin.tex b/admin.tex
> index 2efd4d7..64d0667 100644
> --- a/admin.tex
> +++ b/admin.tex
> @@ -115,7 +115,8 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
>  \hline \hline
>  0x0000 & VIRTIO_ADMIN_CMD_LIST_QUERY & Provides to driver list of commands supported for this group type    \\
>  0x0001 & VIRTIO_ADMIN_CMD_LIST_USE & Provides to device list of commands used for this group type \\
> -0x0002 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
> +0x0002 & VIRTIO_ADMIN_CMD_BIND_DEVICE & Bind the device to one group member \\
> +0x0003 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
>  \hline
>  0x8000 - 0xFFFF & - & Reserved for future commands (possibly using a different structure)    \\
>  \hline
> @@ -429,6 +430,44 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
>  \field{VF Enable} refer to registers within the SR-IOV Extended
>  Capability as specified by \hyperref[intro:PCIe]{[PCIe]}.
>  
> +\subsubsection{Bind the device for member}
> +
> +The VFs of the SR-IOV are created by the user inside the guest OS, so the virtio
> +devices don't know about these VFs.

I don't really get how.

> Because each VF may be assigned a different role
> +by the user, the virtio device can not choose one VF to bind random.
> +So only the user knows how to bind the virtio devices to the VFs.
> +On the other hand, generally the virtio devices are not created by the user
> +inside the guest OS. This requires some management platform to participate.
> +
> +So we introduce a new admin queue command to bind the VFs and the virtio
> +devices.

This description needs a complete rewrite, but to try to help you I'd
need to figure out what is being said. My guess is that this is an
attempt to introduce some kind of identity to VFs.  OK but what is this
identity and how is it better than using VF#?  You want it to persist
when NumVFs in the SR-IOV capability changes?

> +\begin{lstlisting}
> +struct virtio_admin_cmd_bind {
> +    u64 identity;
> +};
> +\end{lstlisting}
> +
> +The user got the \field{identity} from the management platform, that is not
> +included by this spec.

Without even understanding the above, what I don't like about this
proposal, is that it assumes some out of band interface to either
specify or discover these IDs. With admin commands we now have
ability to fully specify a management interface, and I'd rather
we had a complete proposal covering all aspects of it.
If some platform does not want to use the management aspects,
that's ok.

> +
> +\drivernormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Device groups / Group administration commands / Bind the device for member}
> +
> +VIRTIO_ADMIN_CMD_BIND_DEVICE requires that the \field{group_member_id} MUST be set.
> +
> +The \field{identity} is passed by the user. It is the identity of the virtio
> +device.
> +
> +\devicenormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Device groups / Group administration commands / Bind the device for member}
> +
> +Every device MUST have one unique \field{identity} in the host.
> +
> +If the PF device can not find the device by the \field{identity},
> +the \field{status} MUST be set to VIRTIO_ADMIN_STATUS_EINVAL.
> +
> +If the device is found by the \field{identity}, the device MUST work as the
> +device of this group member specified by the \field{group_member_id}.
> +
>  \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
>  
>  An administration virtqueue of an owner device is used to submit

> -- 
> 2.32.0.3.g01195cf9f