RE: [EXT] [PATCH v9 04/13] admin-cmds-capabilities: Add capabilities admin commands

[Parav Pandit <parav@nvidia.com>]

> From: Satananda Burla <sburla@marvell.com>
> Sent: Tuesday, February 13, 2024 12:21 AM
> 
> Hi Parav
> 
> > -----Original Message-----
> > From: Parav Pandit <parav@nvidia.com>
> > Sent: Sunday, February 11, 2024 7:42 PM
> > To: Satananda Burla <sburla@marvell.com>; virtio-comment@lists.oasis-
> > open.org; mst@redhat.com; cohuck@redhat.com
> > Cc: hengqi@linux.alibaba.com; Shahaf Shuler <shahafs@nvidia.com>; si-
> > wei.liu@oracle.com; peter.hilber@opensynergy.com;
> jasowang@redhat.com;
> > xuanzhuo@linux.alibaba.com
> > Subject: RE: [EXT] [PATCH v9 04/13] admin-cmds-capabilities: Add
> > capabilities admin commands
> >
> > Hi Satananda,
> >
> > > From: Satananda Burla <sburla@marvell.com>
> > > Sent: Monday, February 12, 2024 12:51 AM
> > >
> > > Hi Parav
> > >
> > > > -----Original Message-----
> > > > From: Parav Pandit <parav@nvidia.com>
> > > > Sent: Tuesday, February 6, 2024 11:23 PM
> > > > To: virtio-comment@lists.oasis-open.org; mst@redhat.com;
> > > > cohuck@redhat.com
> > > > Cc: hengqi@linux.alibaba.com; Satananda Burla
> > > > <sburla@marvell.com>; shahafs@nvidia.com; si-wei.liu@oracle.com;
> > > > peter.hilber@opensynergy.com; jasowang@redhat.com;
> > > > xuanzhuo@linux.alibaba.com; Parav Pandit <parav@nvidia.com>
> > > > Subject: [EXT] [PATCH v9 04/13] admin-cmds-capabilities: Add
> > > > capabilities admin commands
> > > >
> > > > External Email
> > > >
> > > > ------------------------------------------------------------------
> > > > --
> > --
> > > > Add three capabilities related commands.
> > > > First to read the device and driver capabilities.
> > > > Second to write the driver capabilities.
> > > > Third for driver to discover which capabilities can be accessed.
> > > >
> > > > Write capabilities by the owner driver is not currently supported.
> > > > It will be supported in future by the owner driver to write for
> > > > the member device.
> > > >
> > > > Fixes: https://urldefense.proofpoint.com/v2/url?u=https-
> > > > 3A__github.com_oasis-2Dtcs_virtio-
> > > >
> > >
> 2Dspec_issues_179&d=DwIDAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=NHDPsfcAY
> > > lN2z-
> > > > NXHHG4WB09qqS0voo_nf6_kGS625A&m=KI-
> > > >
> > >
> QceHuScbIZDc_Kxteq_nU775BHpY5uMRJiwQuxhrgFOpnAx0SON8L2UXPJqty&
> > > s=jnFn-
> > > > DDDK_3FuIG5OPXgWud35KVh8KaeUDAro8N3jqU&e=
> > > > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > > > ---
> > > >  admin-cmds-capabilities.tex | 161
> > > ++++++++++++++++++++++++++++++++++++
> > > >  admin.tex                   |   9 +-
> > > >  capabilities.tex            |   3 +
> > > >  conformance.tex             |   2 +
> > > >  4 files changed, 174 insertions(+), 1 deletion(-)  create mode
> > 100644
> > > > admin-cmds-capabilities.tex
> > > >
> > > > diff --git a/admin-cmds-capabilities.tex b/admin-cmds-
> > capabilities.tex
> > > > new file mode 100644 index 0000000..47394c7
> > > > --- /dev/null
> > > > +++ b/admin-cmds-capabilities.tex
> > > > @@ -0,0 +1,161 @@
> > > > +\subsubsection{Device and Driver Capabilities}\label{sec:Basic
> > > > Facilities of a Virtio Device / Device groups / Group
> > > > administration commands / Device and Driver Capabilities}
> > > > +
> > > > +The device and driver capabilities commands are currently defined
> > for
> > > > self group type and for
> > > > +SR-IOV group type.
> > > > +
> > > > +\begin{enumerate}
> > > > +\item Device Capabilities Support Query Command \item
> > > > +Capabilities Read Command \item Capabilities Write Command
> > > > +\end{enumerate}
> > > > +
> > > > +\paragraph{Device Capabilities Support Query
> > Command}\label{par:Basic
> > > > Facilities of a Virtio Device / Device groups / Group
> > > > administration commands / Device and Driver Capabilities / Device
> > > > Capabilities Support Query Command}
> > > > +
> > > > +For the command VIRTIO_ADMIN_CMD_CAP_SUPPORT_QUERY,
> > > \field{opcode} is
> > > > set to 0x7.
> > > > +
> > > > +This command queries the supported device capabilities
> > > > +identifiers
> > > > bitmap in
> > > > +\field{supported_caps}. Each bit in \field{supported_caps}
> > > > +corresponds
> > > > to the
> > > > +capability identifier. Thus, supported_caps[0] refers to the
> > > > +first
> > > > +64-
> > > > bit value
> > > > +in this array corresponding to identifiers 0 to 63,
> > supported_caps[1]
> > > > +is the second 64-bit value corresponding to identifiers 64 to
> > > > +127,
> > etc.
> > > > +For example, with \field{num_caps = 2}, the array of size 2
> > including
> > > > the values
> > > > +0x3 in supported_caps[0], and 0x1 in supported_caps[1] indicates
> > that
> > > > only
> > > > +identifiers 0, 1 and 64 are supported.
> > > > +
> > > > +The length of the array \field{supported_caps} depends on the
> > > > +supported identifiers - it is large enough to include bits set
> > > > +for all supported identifiers, that is the length is set by
> > > > +calculating by starting with
> > > > the
> > > > +largest supported opcode adding one, dividing by 64 and rounding
> > up.
> > > > +
> > > > +The array \field{supported_caps} is also allowed to be larger and
> > to
> > > > +additionally include an arbitrary number of all-zero entries.
> > > > +
> > > > +For each identifier, the driver can read the device capabilities
> > and
> > > > write
> > > > +the driver capabilities. The driver can also read the written
> > > > capabilities.
> > > > +
> > > > +This command has no command specific data.
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_admin_cmd_query_supported_cap_result {
> > > > +        le16 num_caps;
> > > > +        u8 reserved[6];
> > > > +        le64 supported_caps[];
> > > > +};
> > > > +\end{lstlisting}
> > > Since this is not fixed size, what happens if the allocated write
> > buffer for the
> > > command cannot fit the result ?
> > The device returns only the capabilities worth of supplied buffer.
> >
> > > Does the device return error with specific  error code?
> > No. it truncates to the supplied length.
> > This command relies on the existing admin cmd spec.
> > Please see the snippet from the spec.
> >
> > "Drivers are allowed to submit buffers that are shorter than what the
> > device expects (that is, shorter than the length of status through
> > command_specific_result). This allows the device to maintain a single
> > format structure even if some structure fields are unused by the
> > driver.
> > The device compares the length of each part (device-readable and
> > device-
> > writeable) of the buffer as submitted
> > by driver to what it expects and then silently truncates the
> > structures to either the length submitted by the driver, or the length
> > described in this specification, whichever is shorter."
> But how would driver know if it has read in all the capabilities or not ?

Lets take example:
The driver interested in capability identifier=A, 
the driver's view of capability struct = X bytes.
The device view of the capability = Y bytes.

Example_1:
Typically, old driver, new device.
X < Y.
Result: Device copies X bytes. So driver will see only capabilities field worth of X bytes.

Example_2:
Typically, new driver, old device.
X > Y.
Result: Device copies Y bytes because device only understands Y bytes .
So driver will see only capabilities field worth of only Y bytes from the X bytes it allocated.