virtio-comment

 View Only

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

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

    Posted 02-13-2024 04:23


    > 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.