Re: [virtio-dev] Re: [virtio-comment] [RFC PATCH v3] virtio-can: Device specification.
On Wed, Feb 21 2024, Marc Kleine-Budde wrote:
> On 21.02.2024 14:16:54, Matias Ezequiel Vara Larsen wrote:
>> On Wed, Feb 21, 2024 at 01:49:31PM +0100, Marc Kleine-Budde wrote:
>> > On 21.02.2024 11:37:58, Matias Ezequiel Vara Larsen wrote:
>> > > > > +The length of the \field{sdu} is determined by the \field{length}.
>> > > > > +
>> > > > > +The type of a CAN message identifier is determined by
>> > > > > \field{flags}. The
>> > > > > +3 most significant bits of \field{can_id} do not bear the
>> > > > > information
>> > > > > +about the type of the CAN message identifier and are 0.
>> > > > > +
>> > > > > +The device MUST reject any CAN frame type for which support has not
>> > > > > been
>> > > > > +negotiated with VIRTIO_CAN_RESULT_NOT_OK in \field{result} and MUST
>> > > > > NOT
>> > > > > +schedule the message for transmission. A CAN frame with an
>> > > > > undefined bit
>> > > > > +set in \field{flags} is treated like a CAN frame for which support
>> > > > > has
>> > > > > +not been negotiated.
>> > > > > +
>> > > > > +The device MUST reject any CAN frame for which \field{can_id} or
>> > > > > +\field{sdu} length are out of range or the CAN controller is in an
>> > > > > +invalid state with VIRTIO_CAN_RESULT_NOT_OK in \field{result} and
>> > > > > MUST
>> > > > > +NOT schedule the message for transmission.
>> > > > > +
>> > > I am not very familiar with CAN but how does the device figure out that
>> > > the can_id is out of range?
>> >
>> > In classical CAN we have the standard CAN frames, which have an 11 bit
>> > ID, and there are extended CAN frames, which have 29 bits ID. Extended
>> > frames are signaled with VIRTIO_CAN_FLAGS_EXTENDED set.
>> >
>> > So if a standard frame uses more than 11 Bits of CAN-ID, it's considered
>> > out of range.
>
> Another option would be an extended frame (VIRTIO_CAN_FLAGS_EXTENDED
> set) and using more than 29 bits.
>
>> Thanks Marc for the explanation. Do you think that it would be
>> worthwhile to add that to the spec at some point?
>
> Yes that makes sense as it clarifies what's meant by out of range for
> CAN-IDs, for the valid length a reference to
> \item[VIRTIO_CAN_F_CAN_CLASSIC (0)] and \item[VIRTIO_CAN_F_CAN_FD (1)]
> might be added.
[virtio mailing lists are supposedly down for migration right now, I
hope there's some kind of backfill happening later...]
If the question comes up, it does make sense to add a
clarification... as the virtio-can spec is already voted upon and
merged, we'd need a patch on top. Not sure if it would qualify as an
editorial update or a vote would be needed, best to see it first :)
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC PATCH v3] virtio-can: Device specification.
On Fri, Jun 09 2023, Mikhail Golubev-Ciuchea
wrote:
> From: Harald Mommer
>
> virtio-can is a virtual CAN device. It provides a way to give access to
> a CAN controller from a driver guest. The device is aimed to be used by
> driver guests running a HLOS as well as by driver guests running a
> typical RTOS as used in controller environments.
>
> Signed-off-by: Harald Mommer
> Signed-off-by: Mikhail Golubev-Ciuchea
>
(...)
> diff --git a/device-types/can/device-conformance.tex
> b/device-types/can/device-conformance.tex
> new file mode 100644
> index 000..f944ffd
> --- /dev/null
> +++ b/device-types/can/device-conformance.tex
> @@ -0,0 +1,8 @@
> +\conformance{\subsection}{CAN Device Conformance}\label{sec:Conformance /
> Device Conformance / CAN Device Conformance}
> +
> +A CAN device MUST conform to the following normative statements:
> +
> +\begin{itemize}
> +\item \ref{devicenormative:Device Types / CAN Device / Feature bits}
I just noticed that this introduces[1] an undefined reference (there's no
device normative section for feature bits.) I'd suggest to just drop
this reference (I can do that as a simple editorial update); if we want
to have an actual normative reference for feature bits, it should be
done via an extra change on top.
> +\item \ref{devicenormative:Device Types / CAN Device / Device Operation /
> CAN Message Transmission}
> +\end{itemize}
[1] Easy to miss, as the current 1.4 branch has a pre-existing undefined
reference, which will go away once we finally can merge into the main
branch again and have all that craziness go away after we manage to do
the actual 1.3 release :/
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC PATCH v3 0/3] Virtio SPI Linux driver compliant to draft spec V10
On Tue, Feb 13 2024, Harald Mommer wrote: > This is the 3rd RFC version of a virtio SPI Linux driver which is > intended to be compliant with the proposed virtio SPI draft > specification V10. FWIW: this version of the SPI spec has been voted in for virtio 1.4 (and is consequently available on the virtio-1.4 branch of the virtio spec.) For all intents and purposes, this makes this spec final (modulo possible future extensions). - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [PATCH] sound: add sampling rates 12000Hz and 24000Hz
On Wed, Feb 14 2024, Anton Yakovlev wrote: > Thanks Pape! > > > On 05.02.2024 23:41, Pape, Andreas (ADITG/ESS3) wrote: >> 24kHz is used for 'super wideband' voice transmission 12kHz is added 'for >> completeness' >> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/184 >> >> Signed-off-by: Andreas Pape > > Reviewed-by: Anton Yakovlev Thanks for re-adding your R-b! > >> --- >> device-types/sound/description.tex | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [RFC PATCH v3] virtio-can: Device specification.
On Fri, Jun 09 2023, Mikhail Golubev-Ciuchea
wrote:
> diff --git a/introduction.tex b/introduction.tex
> index b7155bf..d560c63 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -101,6 +101,8 @@ \section{Normative References}\label{sec:Normative
> References}
> \phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
> Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic
> Cureve Cryptography'', Version 1.0, September 2000.
> \newline\url{https://www.secg.org/sec1-v2.pdf}\\
> + \phantomsection\label{intro:CAN}\textbf{[CAN]} &
> +ISO 11898-1:2015 Road vehicles -- Controller area network (CAN) -- Part
> 1: Data link layer and physical signalling\\
Just noticed while massaging this patch to apply on top of the 1.4
branch: This reference does not include an url; I guess
https://www.iso.org/standard/63648.html would be the place to point to?
(This can easily be added via an editorial update on top.)
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH V2] virtio-transport: Clarify requirements
On Tue, Jan 30 2024, Viresh Kumar wrote:
> The virtio documentation currently doesn't define any generic
> requirements that are applicable to all transports. They can be useful
> while adding support for a new transport.
>
> This commit tries to define the same.
>
> Signed-off-by: Viresh Kumar
> ---
> V1->V2:
> - Lot of changes after discussions with Alex and Cornelia.
> - Almost a rewrite of the first commit.
> - Add Transport normative sections.
>
> commands.tex| 1 +
> conformance.tex | 14 +
> content.tex | 78 +++--
> 3 files changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/commands.tex b/commands.tex
> index 25ea8ee3bc78..692ef0833a88 100644
> --- a/commands.tex
> +++ b/commands.tex
> @@ -8,6 +8,7 @@
> \newcommand{\field}[1]{\emph{#1}}
>
> % Mark a normative section (driver or device)
Nit: driver, device, or transport
> +\newcommand{\transportnormative}[3]{#1{Transport Requirements:
> #2}\label{transportnormative:#3}}
> \newcommand{\drivernormative}[3]{#1{Driver Requirements:
> #2}\label{drivernormative:#3}}
> \newcommand{\devicenormative}[3]{#1{Device Requirements:
> #2}\label{devicenormative:#3}}
> \newcounter{clausecounter}
> diff --git a/conformance.tex b/conformance.tex
> index dc00e84e75ae..9bb1c9e2f6ec 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -11,6 +11,10 @@ \section{Conformance Targets}\label{sec:Conformance /
> Conformance Targets}
>
> Conformance targets:
> \begin{description}
> +\item[Transport] A transport MUST conform to following conformance clauses:
Maybe "MUST conform to one conformance clause"?
> + \begin{itemize}
> +\item Clause \ref{sec:Conformance / Transport Conformance}.
> + \end{itemize}
> \item[Driver] A driver MUST conform to four conformance clauses:
>\begin{itemize}
> \item Clause \ref{sec:Conformance / Driver Conformance}.
(...)
Apart from the nits above, this looks good to me.
I'd be in favour of initiating a vote for this, after the OASIS
infrastructure migragion has completed and we have dealt with any
fallout from that... that would probably mean March :(
(Oh, and this should probably go to virtio-comment as well.)
[Apologies for not reviewing earlier, but I'm struggling to keep afloat,
and the OASIS infrastructure saga did not exactly help...]
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [PATCH] sound: add sampling rates 12000Hz and 24000Hz
On Mon, Feb 05 2024, "Pape, Andreas (ADITG/ESS3)" wrote:
> 24kHz is used for 'super wideband' voice transmission 12kHz is added 'for
> completeness'
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/184
>
> Signed-off-by: Andreas Pape
> ---
> device-types/sound/description.tex | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/device-types/sound/description.tex
> b/device-types/sound/description.tex
> index 54c9c8e..bad4415 100644
> --- a/device-types/sound/description.tex
> +++ b/device-types/sound/description.tex
> @@ -504,7 +504,9 @@ \subsubsection{PCM Control Messages}\label{sec:Device
> Types / Sound Device / Dev
> VIRTIO_SND_PCM_RATE_96000,
> VIRTIO_SND_PCM_RATE_176400,
> VIRTIO_SND_PCM_RATE_192000,
> -VIRTIO_SND_PCM_RATE_384000
> +VIRTIO_SND_PCM_RATE_384000,
> +VIRTIO_SND_PCM_RATE_12000,
> +VIRTIO_SND_PCM_RATE_24000
> };
>
> struct virtio_snd_pcm_info {
I think there had been a Reviewed-by: for a previous iteration of this
patch? We can add it while applying (after voting, which I'm going to
initiate); but I'd appreciate it if the previous reviewer could reply
here again.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [RFC PATCH v3] virtio-can: Device specification.
On Mon, Jan 15 2024, Cornelia Huck wrote: > On Mon, Jan 08 2024, Mikhail Golubev-Ciuchea > wrote: > >> Hi all! >> >> I kindly request a vote. >> >> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/186 > > Oh, that has been a while... I believe that this still fits on top, > although it has conflicts with virtio-spi, but nothing that a bit of > fiddling can't sort out. > > The main problem is that we're currently in a bit of an unfortunate > situation regarding voting: There's still a twice-postponed migration of > the OASIS infrastructure coming up, and I don't want to risk the voting > period being affected by an outage... hopefully we'll get a new schedule > soon. > > In the meanwhile, if anyone could spare a few cycles looking at this, > it'd be appreciated. Nothing jumped out at me, but I'm not a CAN expert; > I'd be happy to open voting but for the looming migration issue. OK, I'll just go ahead and open a vote, no reason to drag this out further, and hopefully we're done by the time the migration _really_ happens... - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH] Define the DEVICE ID of Virtio Cpu balloon device as 47.
On Mon, Jan 29 2024, Shujun Xue wrote:
> Is there ETA for the migration?
Well, I wish I knew...
>
> Bests,
> Shujun
>
>
> Cornelia Huck 于2024年1月29日 周一上午6:55写道：
>
>> On Tue, Jan 23 2024, Shujun Xue wrote:
>>
>> > The Virtio CPU balloon device is a primitive device for managing guest
>> > CPU capacity: the device asks for certain CPU cores to be online,
>> > offline or throttled, and the driver performs the requested operation.
>> > This allows the guest to adapt to changes in allowance of underlying
>> > CPU capacity.
>>
>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/187
>>
>> >
>> > Signed-off-by: Shujun Xue
>> > ---
>> > content.tex | 2 ++
>> > 1 file changed, 2 insertions(+)
>> >
>> > diff --git a/content.tex b/content.tex
>> > index 0a62dce..87f0238 100644
>> > --- a/content.tex
>> > +++ b/content.tex
>> > @@ -739,6 +739,8 @@ \chapter{Device Types}\label{sec:Device Types}
>> > \hline
>> > 45 & SPI master \\
>> > \hline
>> > +47 & CPU balloon device \\
>> > +\hline
>> > \end{tabular}
>> >
>> > Some of the devices above are unspecified by this document,
>>
>> I'm holding off voting for now until the status of the OASIS
>> infrastructure migration is clear (sigh).
>>
>>
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio] [PATCH] virtio-net: clarify error handling for coalescing
On Mon, Jan 29 2024, "Michael S. Tsirkin" wrote:
> On Mon, Jan 29, 2024 at 04:06:01PM +0100, Cornelia Huck wrote:
>> On Wed, Jan 24 2024, "Michael S. Tsirkin" wrote:
>>
>> > This is not a huge deal since it's a SHOULD anyway,
>> > so make the new requirement SHOULD too.
>> >
>> > Signed-off-by: Michael S. Tsirkin
>> > ---
>> > device-types/net/description.tex | 6 +-
>> > 1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/device-types/net/description.tex
>> > b/device-types/net/description.tex
>> > index aff5e08..d1d25fe 100644
>> > --- a/device-types/net/description.tex
>> > +++ b/device-types/net/description.tex
>> > @@ -1792,7 +1792,11 @@ \subsubsection{Control Virtqueue}\label{sec:Device
>> > Types / Network Device / Devi
>> >
>> > The device MUST ignore \field{reserved}.
>> >
>> > -The device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
>> > VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if it was
>> > not able to change the parameters.
>> > +The device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
>> > +VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if
>> > +it was not able to change coalescing parameters. In this case,
>>
>> Hm, if we explicitly specify "coalescing parameters" here, should we add
>> it below as well? (We probably should keep the "the".)
>
> maybe "some of the" and in the next sentence "all of the".
>
> so it's atomic: all or nothing.
Makes sense.
>
>> > +the parameters SHOULD remain unchanged, for all VQs.
>> > +
>> >
>> > The device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command
>> > with VIRTIO_NET_ERR if it was not able to change the parameters.
Also here ^ ?
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements
On Mon, Jan 29 2024, Viresh Kumar wrote:
> On 18-12-23, 15:02, Cornelia Huck wrote:
>> Other things that probably should be mandatory:
>>
>> - A way for the driver to change the device status. Reading it would
>> probably be a strong SHOULD.
>> - A way to implement config space. (For example, channel devices don't
>> have a config space or anything similar enough to repurpose, so I had
>> to invent a mechanism for ccw... maybe future transports will be in
>> the same boat.)
>
> How about these changes now, before I post them again:
>
> diff --git a/content.tex b/content.tex
> index 0a62dce5f65f..2a86b1041747 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -631,8 +631,81 @@ \section{Device Cleanup}\label{sec:General
> Initialization And Device Operation /
>
> \chapter{Virtio Transport Options}\label{sec:Virtio Transport Options}
>
> -Virtio can use various different buses, thus the standard is split
> -into virtio general and bus-specific sections.
> +Devices and drivers can use different transport methods to enable
> +interaction, for example PCI, MMIO, or Channel I/O. The transport
> +methods define various aspects of the communication between the device
> +and the driver, like device discovery, exchanging capabilities,
> +interrupt handling, data transfer, etc. For example, in a host/guest
> +architecture, the host might expose a device to the guest on a PCI bus,
> +and the guest will use a PCI-specific driver to interact with it.
> +
> +The standard is split into sections describing general virtio
> +implementation and transport-specific sections.
> +
> +\section{Virtio Transport Requirements}\label{sec:Virtio Transport Options /
> Virtio Transport Requirements}
> +
> +The transport MUST provide a mechanism for the driver to discover the
> +device.
> +
> +The transport must provide a mechanism for the driver to identify the
s/must/MUST/
> +device type.
> +
> +The transport MUST provide a mechanism for communicating virtqueue
> +configurations between the device and the driver.
> +
> +The transport MUST allow multiple virtqueues per device. The number of
> +virtqueues for a pair of device-driver are governed by the individual
> +device protocol.
> +
> +The transport MUST provide a mechanism that the device and the driver
> +use to access memory for implementing virtqueues.
> +
> +The transport MUST provide a mechanism for the device to notify the
> +driver and a mechanism for the driver to notify the device, for example
> +regarding availability of a buffer on the virtqueue.
> +
> +The transport MAY provide a mechanism for the driver to initiate a
> +reset of the virtqueues and device.
> +
> +The transport MUST provide a mechanism for the driver to read the
> +device status. The transport SHOULD provide a mechanism for the driver to
> +change the device status.
I think the other way around? CCW only gained support for reading the
status with revision 2.
> +
> +The transport MUST provide a mechanism to implement config space between
> +the device and the driver.
> +
> +\devicenormative{\subsection}{Virtio Transport Requirements}{Virtio
> Transport Options}
> +
> +Any data associated with a device-initiated transaction MUST remain
> +accessible to the driver until the driver acknowledges the transaction
> +to be complete.
Maybe better "The device MUST keep any data (...) accessible to the
driver..." ?
> +
> +The device MUST NOT access the contents of a virtqueue before the
> +driver notifies, in a transport defined way, the device that the
> +virtqueue is ready to be accessed.
> +
> +The device MUST NOT access or modify buffers on a virtqueue after it has
> +notified the driver about their availability.
> +
> +The device MUST reset the virtqueues if requested, in a transport
> +defined way, by the driver.
"in a transport defined way, if the transport provides such a method" ?
> +
> +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio
> Transport Options}
> +
> +The driver MUST acknowledge device notifications, as mandated by the
> +transport.
> +
> +The driver MUST NOT access virtqueue contents before the device notifies
> +about the readiness of the same.
> +
> +The driver MUST NOT access buffers, after it has added them to the
> +virtqueue and notified the device about their availability. The driver
> +MAY access them after the device has processed them and notified the
> +driver of their availability, in a transport defined way.
> +
> +The driver MAY ask the device to reset the virtqueues if, for example,
> +the driver times out waiting for a notification from the device for a
> +previously queued request.
>
&
[virtio-dev] Re: [virtio] [PATCH] virtio-net: clarify error handling for coalescing
On Wed, Jan 24 2024, "Michael S. Tsirkin" wrote:
> This is not a huge deal since it's a SHOULD anyway,
> so make the new requirement SHOULD too.
>
> Signed-off-by: Michael S. Tsirkin
> ---
> device-types/net/description.tex | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/device-types/net/description.tex
> b/device-types/net/description.tex
> index aff5e08..d1d25fe 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1792,7 +1792,11 @@ \subsubsection{Control Virtqueue}\label{sec:Device
> Types / Network Device / Devi
>
> The device MUST ignore \field{reserved}.
>
> -The device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if it was not
> able to change the parameters.
> +The device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> +VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if
> +it was not able to change coalescing parameters. In this case,
Hm, if we explicitly specify "coalescing parameters" here, should we add
it below as well? (We probably should keep the "the".)
> +the parameters SHOULD remain unchanged, for all VQs.
> +
>
> The device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command with
> VIRTIO_NET_ERR if it was not able to change the parameters.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH] Define the DEVICE ID of Virtio Cpu balloon device as 47.
On Tue, Jan 23 2024, Shujun Xue wrote:
> The Virtio CPU balloon device is a primitive device for managing guest
> CPU capacity: the device asks for certain CPU cores to be online,
> offline or throttled, and the driver performs the requested operation.
> This allows the guest to adapt to changes in allowance of underlying
> CPU capacity.
Fixes: https://github.com/oasis-tcs/virtio-spec/issues/187
>
> Signed-off-by: Shujun Xue
> ---
> content.tex | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index 0a62dce..87f0238 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -739,6 +739,8 @@ \chapter{Device Types}\label{sec:Device Types}
> \hline
> 45 & SPI master \\
> \hline
> +47 & CPU balloon device \\
> +\hline
> \end{tabular}
>
> Some of the devices above are unspecified by this document,
I'm holding off voting for now until the status of the OASIS
infrastructure migration is clear (sigh).
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [RFC PATCH v3] virtio-can: Device specification.
On Mon, Jan 08 2024, Mikhail Golubev-Ciuchea wrote: > Hi all! > > I kindly request a vote. > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/186 Oh, that has been a while... I believe that this still fits on top, although it has conflicts with virtio-spi, but nothing that a bit of fiddling can't sort out. The main problem is that we're currently in a bit of an unfortunate situation regarding voting: There's still a twice-postponed migration of the OASIS infrastructure coming up, and I don't want to risk the voting period being affected by an outage... hopefully we'll get a new schedule soon. In the meanwhile, if anyone could spare a few cycles looking at this, it'd be appreciated. Nothing jumped out at me, but I'm not a CAN expert; I'd be happy to open voting but for the looming migration issue. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [PATCH v2 1/1] RFC: virtio-bt: add virtio BT device specification
On Thu, Dec 07 2023, Igor Skalkin wrote:
> diff --git a/device-types/bt/description.tex b/device-types/bt/description.tex
> new file mode 100644
> index 000..98311f4
> --- /dev/null
> +++ b/device-types/bt/description.tex
> @@ -0,0 +1,158 @@
> +\section{BT Device}\label{sec:Device Types / BT Device}
> +
> +The virtio-bt device provides an HCI (Host Control Interface) over VirtIO
> +link between the guest HCI device and the host HCI backend.
> +Also, the device can inform the guest driver which operational system /
> vendor
> + specific HCI extensions it supports.
Is there any good way to describe this without using guest/host
terminology while using the typical guest/host setup as an example
instead?
> +Host Control Interface is described in Bluetooth Core Specification (see
> +\hyperref[intro:bluetooth-core]{Bluetooth Core Specification}).
> +
> +\subsection{Device ID}\label{sec:Device Types / BT Device / Device ID}
> +
> +40
> +
> +\subsection{Virtqueues}\label{sec:Device Types / BT Device / Virtqueues}
> +
> +\begin{description}
> +\item[0] transmitq
> +\item[1] receiveq
> +\end{description}
> +
> +\subsection{Feature bits}\label{sec:Device Types / BT Device / Feature bits}
> +
> +\begin{description}
> +\item[VIRTIO_BT_F_VND_HCI (0)] The device supports vendor-specific HCI
> +extensions.
> +\item[VIRTIO_BT_F_MSFT_EXT (1)] The device supports Microsoft HCI Extensions.
> +\item[VIRTIO_BT_F_AOSP_EXT (2)] The device supports Android HCI Extensions.
> +\item[VIRTIO_BT_F_CONFIG_V2 (3)] The device uses the second version of the
> +configuration space structure.
> +\end{description}
> +
> +\devicenormative{\subsubsection}{Feature bits}{Device Types / BT Device /
> Feature bits}
> +
> +The device MAY require the driver to accept the \field{VIRTIO_BT_F_CONFIG_V2}
> +feature bit and use the second version of the configuration layout, because
> the
> +first one is unaligned, which violates the specification.
ISTR that it was "violates the requirements for configuration space
accesses for some transports"? That might be a bit clearer.
And if that's the case, can we maybe require that the device MUST
require the driver to accept the feature bit if the transport the device
uses cannot deal with v1?
> +
> +The device should offer \field{VIRTIO_BT_F_MSFT_EXT} or
s/should offer/SHOULD offer the/
> +\field{VIRTIO_BT_F_AOSP_EXT} feature bit if it supports correspondingly MSFT
> or
> +AOSP extension command sets (see \hyperref[intro:bluetooth-aosp]{Bluetooth
> AOSP
> +HCI Extensions}, \hyperref[intro:bluetooth-msft]{Bluetooth MSFT HCI
> Extensions}
> +). In case of \field{VIRTIO_BT_F_MSFT_EXT}, device should also set
> configuration
"the device SHOULD also set the configuration field"
> +field \field{msft_opcode}.
> +
> +The device should offer \field{VIRTIO_BT_F_VND_HCI} feature bit and set
s/should offer/SHOULD offer the/
s/set/set the/
> +configuration field \field{vendor}, if it supports corresponding vendor
s/corresponding/the corresponding/
> +extensions. Also, in case of second configuration version, fields
s/in case of second configuration version/if VIRTIO_BT_F_CONFIG_V2 is
negotiated/
> +\field{set_bdaddr_opcode} and \field{quirks} can be set according to this
MAY be set? Or SHOULD be set?
> +\field{vendor}.
> +
> +\drivernormative{\subsubsection}{Feature bits}{Device Types / BT Device /
> Feature bits}
> +
> +The driver MUST accept \field{VIRTIO_BT_F_CONFIG_V2} feature bit if offered
> by
> +the device.
> +
> +The driver SHOULD accept any of the \field{VIRTIO_BT_F_VND_HCI},
> +\field{VIRTIO_BT_F_MSFT_EXT} or \field{VIRTIO_BT_F_AOSP_EXT} feature bits if
> +offered by the device.
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / BT Device
> / Device configuration layout}
> +BT device supports two versions of the configuration structure layout.
> +\subsubsection{Device configuration layout: Version 1}\label{sec:Device
> Types / BT Device / Device configuration layout / Verion 1}
> +
> +All fields of this configuration are always available and read-only for the
> +driver.
> +\begin{lstlisting}
> +struct virtio_bt_config {
> +u8 type;
> +le16 vendor;
> +le16 msft_opcode;
> +} __attribute__((packed));
> +\end{lstlisting}
> +
> +\begin{description}
> +
> +\item[\field{type}] indicates the type of the device, is it "Primary BR/EDR
> +controller" or "Alternate MAC/PHYs (AMP) secondary controller" and can have
> the
> +following values:
> +\begin{lstlisting}
> +#define VIRTIO_BT_CONFIG_TYPE_PRIMARY 0
> +#define VIRTIO_BT_CONFIG_TYPE_AMP 1
> +\end{lstlisting}
> +
> +\item[\field{vendor}]
> +Indicates the vendor of the BT device and can have the following mutually
> +exclusive values:
> +\begin{lstlisting}
> +#define VIRTIO_BT_CONFIG_VENDOR_NONE 0
> +#define VIRTIO_BT_CONFIG_VENDOR_ZEPHYR 1
> +#define VIRTIO_BT_CONFIG_VENDOR_INTEL2
> +#define VIRTIO_BT_CONFIG_VENDOR_REALTEK 3
> +\end{lstlisting}
> +The device MUST offer the
Re: [virtio-dev] [PATCH v8] virtio-net: correct conditions for devices to validate packet checksum
On Thu, Jan 11 2024, Heng Qi wrote:
> 在 2024/1/4 下午12:47, Heng Qi 写道:
>>
>>
>> 在 2024/1/3 下午2:35, Heng Qi 写道:
>>> There is a historical error in virtio spec:
>>> "If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device *MUST*
>>> set flags
>>> to zero and SHOULD supply a fully checksummed packet to the driver."
>>>
>>> Currently in Linux and virtio-related implementations, the device
>>> validates
>>> the packet checksum and sets DATA_VALID regardless of whether
>>> VIRTIO_NET_F_GUEST_CSUM is negotiated.
>>>
>>> Please refer to the following summary and thread[1] for details and
>>> reasons:
>>>
>>> Summary
>>> =
>>> 1. GUEST_CSUM at virtio spec 0.95 is intended to be compatible with
>>> partially
>>> checksummed packets (NEEDS_CSUM <-> CHECKSUM_PARTIAL). So GUEST_CSUM
>>> is mapped
>>> to NETIF_F_RXCSUM.
>>> GUEST_CSUM only indicates whether the driver handles partially
>>> checksummed packets.
>>> When XDP is loaded, the GUEST_CSUM's offload will be disabled, which
>>> means that
>>> packets have NEEDS_CSUM set will be dropped, and packets have
>>> DATA_VALID set will
>>> still be received.
>>>
>>> 2. When DATA_VALID was added to Linux in 2011[2] and virtio1.0, it
>>> was actually
>>> expected that rx checksum offload (the driver can set
>>> CHECKSUM_UNNECESSARY) had
>>> nothing to do with whether GUEST_CSUM was negotiated. But due to an
>>> error, below
>>> desctiption was added incorrectly:
>>> "If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device *MUST*
>>> set flags
>>> to zero and SHOULD supply a fully checksummed packet to the driver."
>>>
>>> 3. We now hope to correct this error. Let the setting of DATA_VALID
>>> not be
>>> controlled by whether GUEST_CSUM is negotiated, but only controlled
>>> by whether
>>> rx checksum offload is enabled on the OS side. The state of this rx
>>> checksum
>>> offload is not aware of the device.
>>>
>>> 4. [Optional] NETIF_F_RXCSUM corresponding to rx checksum offload
>>> should be
>>> added to dev->hw_features. When the user turns off rx checksum
>>> offload through
>>> ethtool -K, neither NEEDS_CSUM nor DATA_VALID should be taken care
>>> of, that is,
>>> all packets will be marked as CHECKSUM_NONE.
>>>
>>> Related Link
>>> =
>>> [1]
>>> https://lists.oasis-open.org/archives/virtio-comment/202312/msg00135.html
>>> [2] 10a8d94a9574 ("virtio_net: introduce VIRTIO_NET_HDR_F_DATA_VALID")
>>>
>>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/185
>>
>> Hi Michael, Cornelia.
>>
>> Could you please open a voting for this fix?
>
> Hi Michael and Cornelia.
>
> Sincerely asking if you missed this message?
I've been back from end-of-year PTO only this week...
I'm currently waiting for a confirmed updated schedule for the
infrastructure migration, as I don't want to risk having the platform
out of order in the middle of the voting period.
>
> In addition, Jason recently responded with his reviewed-by tag.
> Do I need to publish a new version carrying his tag (thanks Jason)?
No, we can add his R-b when pushing to git.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev][PATCH V9 2/2] virtio-spi: add the device specification
On Tue, Dec 12 2023, Haixu Cui wrote:
> The Virtio SPI (Serial Peripheral Interface) device is a virtual
> SPI controller that allows the driver to operate and use the SPI
> controller under the control of the host.
>
> This patch adds the specification for virtio-spi.
>
> Signed-off-by: Haixu Cui
> Reviewed-by: Viresh Kumar
> ---
> device-types/spi/description.tex| 281
> device-types/spi/device-conformance.tex | 7 +
> device-types/spi/driver-conformance.tex | 7 +
> 3 files changed, 295 insertions(+)
> create mode 100644 device-types/spi/description.tex
> create mode 100644 device-types/spi/device-conformance.tex
> create mode 100644 device-types/spi/driver-conformance.tex
Oh, and I just noticed that you also need to include
{device,driver}-conformance.tex in conformance.tex.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev][PATCH V9 2/2] virtio-spi: add the device specification
On Thu, Dec 21 2023, Haixu Cui wrote:
> Hi Cornelia,
>
> On 12/21/2023 4:44 PM, Cornelia Huck wrote:
>> On Thu, Dec 21 2023, Haixu Cui wrote:
>>>> Please move inclusion of this new file into content.tex here, instead of
>>>> including a not-yet-existing file in the previous patch.
>>>>
>>>> (...)
>>>
>>>
>>> Sorry, I don't quite understand here. Maybe I should not add the
>>> following line in content.tex, is that so?
>>>
>>> \input{device-types/spi/description.tex}
>>
>> No, please add this line; but do so in this patch instead of the
>> previous one that only changes the wording.
>
> I am not clear where to put this line? In the commit message or in the
> main text, e.g. at the start of the description.tex? As follows:
>
> diff --git a/device-types/spi/description.tex
> b/device-types/spi/description.tex
> new file mode 100644
> index 000..b76258c
> --- /dev/null
> +++ b/device-types/spi/description.tex
> @@ -0,0 +1,281 @@
> +\input{device-types/spi/description.tex}
> +\section{SPI Controller Device}\label{sec:Device Types / SPI Controller
> Device}
> +
> ...
Ah no, the inclusion is of course in the right place where you put it in
context.tex; the *patch hunk* should be moved from patch 1 to patch 2,
that's all. Hope that's more clear now :)
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev][PATCH V9 2/2] virtio-spi: add the device specification
On Thu, Dec 21 2023, Haixu Cui wrote:
> Hi Cornelia,
> Much appreciated for your comments and please refer to my response.
>
> On 12/18/2023 7:04 PM, Cornelia Huck wrote:
>> On Tue, Dec 12 2023, Haixu Cui wrote:
>>
>> [BTW: A changelog would be useful, either in the patch or in the cover
>> letter.]
>
> Sure, I summarize the major changes between these versions and will add
> in next patch:
>
> changelog:
> v8->v9:
> - add explanation of bits_per_word_mask in config space
> v7->v8:
> - change device to host
> v6->v7:
> - fix the format problem, syntax problem
> v5->v6:
> - use driver/device instead guest/host
> - add the definition of some terminologies
> - use controller instead of master throughout the spec
> - add buffer length validation for full-duplex transfer
> v4->v5:
> - use controller instead of master
> - fix indentation issue
> - extend the config space to expose the backend supported features
> - add another result value to indicate parameter error
> - add device and driver requirement about parameter checkig
> v3->v4:
> - fix the spell error
> - bus_num is not SOC-specific, remove it
> - add driver requirement to deal with the situation that the cs delay
> parameters are not 0 but the backend doesn't support cs timing setting
> v2->v3
> - remove unnecessary statements and driver implimentation details
> - add the parameters about cs timing delay and transfer delay
> - use "le32" instead of "u32"
> - swap the rx_buf and tx_buf in the request format
> - add the parameters about transfer bit width
> v1->v2:
> - explain SPI when it is firstly used
> - update the ambiguous expression of virtqueue
> v0->v1:
> - add definition of abbreviation SPI
> - remove the ID
Thanks. Might be best to add this in the cover letter.
> ---
>
>>
>>> The Virtio SPI (Serial Peripheral Interface) device is a virtual
>>> SPI controller that allows the driver to operate and use the SPI
>>> controller under the control of the host.
>>>
>>> This patch adds the specification for virtio-spi.
>>>
>>> Signed-off-by: Haixu Cui
>>> Reviewed-by: Viresh Kumar
>>> ---
>>> device-types/spi/description.tex| 281
>>> device-types/spi/device-conformance.tex | 7 +
>>> device-types/spi/driver-conformance.tex | 7 +
>>> 3 files changed, 295 insertions(+)
>>> create mode 100644 device-types/spi/description.tex
>>> create mode 100644 device-types/spi/device-conformance.tex
>>> create mode 100644 device-types/spi/driver-conformance.tex
>>>
>>> diff --git a/device-types/spi/description.tex
>>> b/device-types/spi/description.tex
>>
>> Please move inclusion of this new file into content.tex here, instead of
>> including a not-yet-existing file in the previous patch.
>>
>> (...)
>
>
> Sorry, I don't quite understand here. Maybe I should not add the
> following line in content.tex, is that so?
>
> \input{device-types/spi/description.tex}
No, please add this line; but do so in this patch instead of the
previous one that only changes the wording.
>
>>
>>> +\field{mode_func_supported} indicates whether the following features are
>>> supported or not:
>>> +bit 0-1: CPHA feature,
>>> +0b00: invalid, must support as least one CPHA setting.
>>> +0b01: supports CPHA=0 only;
>>> +0b10: supports CPHA=1 only;
>>> +0b11: supports CPHA=0 and CPHA=1;
>>> +
>>> +bit 2-3: CPOL feature,
>>> +0b00: invalid, must support as least one CPOL setting.
>>> +0b01: supports CPOL=0 only;
>>> +0b10: supports CPOL=1 only;
>>> +0b11: supports CPOL=0 and CPOL=1;
>>> +
>>> +bit 4: chipselect active high feature, 0 for unsupported and 1 for
>>> supported,
>>> +chipselect active low must always be supported.
>>> +
>>> +bit 5: LSB first feature, 0 for unsupported and 1 for supported,
>>> MSB first must always be
>>> +supported.
>>> +
>>> +bit 6: loopback mode feature, 0 for unsupported and 1 for
>>> supported, normal mode
>>> +must always be supported.
>>> +
>>> +Note: CPOL is clock polarity and CPHA is clock phase. If CPOL is 0, the
>>> clock idles at the logical
>>> +low voltage, otherwise it idles at the logical high voltage. CPHA
>>>
[virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements
On Mon, Dec 18 2023, Bill Mills wrote: > On 12/18/23 9:19 AM, Alex Bennée wrote: >> Cornelia Huck writes: >> >>> On Wed, Dec 06 2023, Viresh Kumar wrote: >>> >>>> On 05-12-23, 14:18, Cornelia Huck wrote: >>>>> On Tue, Dec 05 2023, Viresh Kumar wrote: >> >>>> >>>> - The transport MUST provide a mechanism for the device and the driver >>>>to notify each other, for example about availability of a buffer on >>>>the virtqueue. >>> >>> Probably a mechanism for the device to notify the driver, and a >>> mechanism for the driver to notify the device? They can be different, as >>> long both of them are present. >>> >>>> >>>> - The transport SHOULD provide a mechanism for the driver to initiate >>>>a reset of the virtqueues and device. >>> >>> Can we mandate a mechanism for a device reset? Reset of virtqueues needs >>> to be optional, I'm not sure whether a SHOULD would be appropriate for >>> that (or maybe we should finally come up with something for ccw ;) >>> >>> Other things that probably should be mandatory: >>> >>> - A way for the driver to change the device status. Reading it would >>>probably be a strong SHOULD. >>> - A way to implement config space. (For example, channel devices don't >>>have a config space or anything similar enough to repurpose, so I had >>>to invent a mechanism for ccw... maybe future transports will be in >>>the same boat.) >> >> I think Bill has run into problems with config space on OpenAMP setups >> where there can be no specific trapping between domains making it hard >> to manage a config space where both sides might want to make updates. I >> guess the original MMIO transport gets away with it because config space >> is always in the trap-and-emulate address space in a normal VM/emulation >> situation. >> >> Bill, >> >> Is the plan to introduce a new transport that manages config in a >> different way? >> > > I/We had a couple of ideas. This is a thorny issue if you want > something that 100% matches the implications of the virtio spec. If you > want something that works with existing virtio protocols it is not as > bad but still tricky. > > If you read the virtio spec it implies you could have a device with zero > virtqueues and just does things with the config space. To me this > should be a SHOULD NOT. Something like "The config space SHOULD NOT be > used as a mechanism for frequent changes." > > To me the config space is best used as a device to driver info space > with infrequent updates from both sides. I don't know how that should > be expressed in the spec. We do have some text suggesting that this should work like that, i.e. in "Config Space" we say "Device configuration space is generally used for rarely-changing or initialization-time parameters." and in Appendix B we say "Device configuration space should only be used for initialization-time parameters. It is a limited resource with no synchronization between field written by the driver, so for most uses it is better to use a virtqueue to update configuration information". I'm not sure where to add a normative statement, given that this is something that device types need to take care of. > > It is true that virtio-pci and virtio-mmio can signal changes in config > space from the driver to the device. However the device driver can read > the config space asynchronously whenever it wants. The generation > counter was added to fix non-atomic updates from the device. > > The generation counter in virtio-mmio and virtio-pci transport config > spaces only work because the hypervisor can see each read attempt to the > config space. The virtio-pci suggests not incrementing the generation > counter unless the guest is doing a read. (Because the generation > counter on virtio-pci is small.) > > In addition, config updates from the driver at not formally signaled and > it again relies on the hypervisor intersecting writes to the config space. The configuration space for ccw is really different conceptually: both reading from and writing to it are initiated by the driver sending commands, and the architecture makes sure that reads and writes are processed by the device strictly in the order that the driver sent them. IOW, unless the device messes up, the driver will get consistent information. > > Anyway I have thought of two solutions for the config space that do not > rely on hypervisor magic. > > 1) Config generation counter odd == up
Re: [virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements
On Mon, Dec 18 2023, Alex Bennée wrote:
> Cornelia Huck writes:
>
>> On Tue, Dec 05 2023, Alex Bennée wrote:
>>
>>> Cornelia Huck writes:
>>>
>>>> On Tue, Dec 05 2023, Viresh Kumar wrote:
>>>>> +
>>>>> +The device MUST present each event, in a transport defined way, from the
>>>>> +moment it takes place until the driver acknowledges the event.
>>>>
>
>>>>> +
>>>>> +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio
>>>>> Transport Options}
>>>>> +
>>>>> +The driver MUST NOT access guest memory locations outside what's made
>>>>> +available by the device to the driver.
>>>>
>>>> I don't think that makes sense -- I'd assume most guest memory locations
>>>> do not have anything to do with virtio, and we should try to avoid
>>>> host/guest terminology.
>>>
>>> I agree guest memory isn't the right terminology here. However there are
>>> discussions about how to implement secure buffers for VirtIO - so for
>>> example a buffer mediated by some sort of secure layer. In those cases
>>> the driver may not have access to it outside of the transactions.
>>
>> Yes, I think we need to limit the scope of "guest memory" here. I think
>> we are basically wanting to deal with any memory used by virtio (device
>> type including memory access controlled by it, transport, and the
>> protocol itself). We would be talking about memory made available to the
>> device by the driver for explicit usage to implement the virtio spec. I
>> think this would cover mediation by a secure layer as well (with the
>> driver calling into that secure layer?) Or does the (host) device end up
>> donating memory to the (guest) driver, and we need to make sure it
>> doesn't scribble over it?
>
> I'm not sure if we have example of the host donating memory apart from
> the sort of static partitioning we see with guests on start-up where a
> region is defined as shared. The Xen grant model leaves the guest to
> grant access to its own pages to the backend.
>
> I guess for firmware mediated sharing this would still be driven by the
> guest rather than the host?
Yes, I think the guest telling the firmware which parts of its memory
the host may access is the usual pattern, or at least what I've seen so
far.
>>
>>>>> +
>>>>> +The driver MUST NOT access virtqueue contents before the device notifies
>>>>> +about the readiness of the same.
>>>>> +
>>>>> +The driver MUST NOT access buffers, after it has added them to the
>>>>> +virtqueue and notified the device about their availability. The driver
>>>>> +MAY access them after the device has processed them and notified the
>>>>> +driver of their availability, in a transport defined way.
>>>>> +
>>>>> +The driver MAY ask the device to reset the virtqueues if, for example,
>>>>> +the driver times out waiting for a notification from the device for a
>>>>> +previously queued request.
>>>>
>>>> Again, I believe this has already been covered in the generic
>>>> sections -- do we instead need to specify that a transport MUST provide
>>>> a method to do xy? (or SHOULD, MAY, as applicable -- it would be good to
>>>> list explicitly what is mandatory for a transport to implement, and what
>>>> is optional.)
>>>
>>> Yes I think so. The s390x channel transport gets referenced because it
>>> has a nice enumerated list of operations. It would be good to codify
>>> which operations are mandatory for all transports and which are
>>> optional.
>>
>> The problem with the ccw transport is that while it has a nice list of
>> operations, (a) it only covers guest-initiated actions,
>
> What examples of host initiated actions are there (aside from an IPI
> indicating a receive VirtQueue has buffers waiting)?
Also notifications for configuration changes; but I think we can put all
of this under the device->driver notification header. (Hmm, we probably
should change all those host/guest references for ccw some day...)
>
>> (b) probably not
>> all of them shold be mandatory (and some of them are more of an artifact
>> of how channel I/O works),
>
> These ones?
>
> #define CCW_CMD_SET_IND 0x43
> #define CCW_CMD_SET_CONF_IND 0x53
> #define CCW_CMD_SET_IND_ADAPTER 0x73
Yes, and also CCW_CMD_SET_VIRTIO_REV (I think there used to be some
interest to implement something like that for mmio, but I don't think
anything ever ended up being specified?)
>
>> and (c) it only implements a subset of the
>> defined operations (which makes the not-implemented ones de facto
>> optional, of course :) But yes, we could use it as a starting point.
>
> Got to start somewhere :-)
Indeed :)
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements
On Wed, Dec 06 2023, Viresh Kumar wrote:
> On 05-12-23, 14:18, Cornelia Huck wrote:
>> On Tue, Dec 05 2023, Viresh Kumar wrote:
>> > +\section{Virtio Transport Requirements}\label{sec:Virtio Transport
>> > Options / Virtio Transport Requirements}
>> > +
>> > +\devicenormative{\subsection}{Virtio Transport Requirements}{Virtio
>> > Transport Options}
>>
>> I'm not sure we can introduce MUST (NOT) requirements for basic
>> functionality after the spec has been published for quite a time already
>> (although I'd assume every implementation is fulfilling the requirements
>> anyway)... thoughts?
>
> Fair point. I think it would be okay to use MUST (NOT) if we are sure
> that the existing implementations are fulfilling the requirements.
> Else maybe we can use SHOULD (NOT) ?
Yes, I think if we are just spelling out things explicitly in one place
that have been required anyway (by existing transports etc.), we are
fine using MUST; everything where someone could have chosen a different
implementation needs to be SHOULD.
(...)
>> > +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio
>> > Transport Options}
>> > +
>> > +The driver MUST NOT access guest memory locations outside what's made
>> > +available by the device to the driver.
>>
>> I don't think that makes sense -- I'd assume most guest memory locations
>> do not have anything to do with virtio, and we should try to avoid
>> host/guest terminology.
>
> Hmm, you are right about the guest/host terminology. This comes mostly
> from the MMIO transport section, where a guest presents 0x100 bytes of
> memory, followed by configuration space (whose length is device/driver
> dependent). The MMIO section has this:
>
> "The driver MUST NOT access memory locations not described in the
> table \ref{tab:Virtio Transport Options / Virtio Over MMIO / MMIO Device
> Register Layout}
> (or, in case of the configuration space, described in the device
> specification),
> MUST NOT write to the read-only registers (direction R) and
> MUST NOT read from the write-only registers (direction W)."
Thanks for clarifying where this came from! See my suggestions in the
other fork of this thread.
>
>> > +The driver MUST NOT write to the read-only memory area and MUST NOT read
>> > +from the write-only memory area.
>>
>> Which memory areas does that refer to? Parts of the transport-specific
>> data structures?
>
> Yes, based again on the above MMIO section, but perhaps apply to other
> transports as well..
I think we can't assume transports to use r/o and w/o memory areas in
all cases -- for example, ccw is largely based on passing buffers
around. Of course, if you look at the actual structures, there are
always fields that are to be used for reading or writing only.
>
>> > +The driver MUST acknowledge events presented by the device, as mandated
>> > +by the transport.
>>
>> I don't think this is quite correct in the absolute -- for example, it
>> should be fine to not acknowledge events if some overriding event comes
>> along, or if the driver initiates a reset.
>
> What about:
>
> The driver SHOULD acknowledge events presented by the device, as
> mandated by the transport, unless a new event from the device
> overrides the previous one or the driver initiates a reset.
Should we do s/events/notifications/, or do we also cover transactions
initiated by the device explicitly (I'd assume that the device would
notify the driver im- or explicitly if it has to do something?)
>
>> > +The driver MUST NOT access virtqueue contents before the device notifies
>> > +about the readiness of the same.
>> > +
>> > +The driver MUST NOT access buffers, after it has added them to the
>> > +virtqueue and notified the device about their availability. The driver
>> > +MAY access them after the device has processed them and notified the
>> > +driver of their availability, in a transport defined way.
>> > +
>> > +The driver MAY ask the device to reset the virtqueues if, for example,
>> > +the driver times out waiting for a notification from the device for a
>> > +previously queued request.
>>
>> Again, I believe this has already been covered in the generic
>> sections
>
> Right, but having it all at a single place makes it more convenient,
> instead of looking at implementations.
In that case, I'm fine for the first two; however, I'm not sure what the
third one adds here -- I don't think we ever said anything about when
the driver can initiate a reset, it can basically be at any time, and it
overrides whatever the de
[virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements
On Tue, Dec 05 2023, Alex Bennée wrote:
> Cornelia Huck writes:
>
>> On Tue, Dec 05 2023, Viresh Kumar wrote:
>>> +
>>> +The device MUST present each event, in a transport defined way, from the
>>> +moment it takes place until the driver acknowledges the event.
>>
>> I don't believe "event" is well-defined here.
>
> Maybe:
>
> "A device initiated transaction can isn't considered complete until
> acknowledged by the driver. As such data MUST remain visible to the
> driver until the transaction is complete"?
Transaction is good, what about:
"Any data associated with a device-initiated transaction MUST remain
accessible to the driver until the driver acknowledges the transaction
to be complete."
>
>>
>>> +
>>> +The device MUST NOT access virtqueue's contents before the driver
>>> +notifies that the queue is ready for access, in a transport defined way.
>>> +
>>> +The device MUST NOT access buffers on the virtqueue, after it has
>>> +modified them and notified the driver about their availability.
>>> +
>>> +The device MUST reset the virtqueues if requested by the driver, in a
>>> +transport defined way.
>>
>> Isn't all of this already defined in one place of the spec or another?
>
> I think the recent example is the virtio-sound driver continuing to feed
> data into buffers after those buffers where submitted into the
> virtqueue. We should be explicit that the only time both sides of a
> VirtIO implementation can access things at the same time is with
> explicitly shared memory (and you need some sort of mechanism to mediate
> that to avoid chaos).
Fair enough, let's make it explicit if people already stumbled
here. Some rewording suggestions:
"The device MUST NOT access the contents of a virtqueue before the
driver notifies, in a transport defined way, the device that the
virtqueue is ready to be accessed.
The device MUST NOT access or modify buffers on a virtqueue after it has
notified the driver about their availability.
The device MUST reset the virtqueues if requested, in a transport
defined way, by the driver."
>
>>> +
>>> +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio
>>> Transport Options}
>>> +
>>> +The driver MUST NOT access guest memory locations outside what's made
>>> +available by the device to the driver.
>>
>> I don't think that makes sense -- I'd assume most guest memory locations
>> do not have anything to do with virtio, and we should try to avoid
>> host/guest terminology.
>
> I agree guest memory isn't the right terminology here. However there are
> discussions about how to implement secure buffers for VirtIO - so for
> example a buffer mediated by some sort of secure layer. In those cases
> the driver may not have access to it outside of the transactions.
Yes, I think we need to limit the scope of "guest memory" here. I think
we are basically wanting to deal with any memory used by virtio (device
type including memory access controlled by it, transport, and the
protocol itself). We would be talking about memory made available to the
device by the driver for explicit usage to implement the virtio spec. I
think this would cover mediation by a secure layer as well (with the
driver calling into that secure layer?) Or does the (host) device end up
donating memory to the (guest) driver, and we need to make sure it
doesn't scribble over it?
>>> +
>>> +The driver MUST NOT access virtqueue contents before the device notifies
>>> +about the readiness of the same.
>>> +
>>> +The driver MUST NOT access buffers, after it has added them to the
>>> +virtqueue and notified the device about their availability. The driver
>>> +MAY access them after the device has processed them and notified the
>>> +driver of their availability, in a transport defined way.
>>> +
>>> +The driver MAY ask the device to reset the virtqueues if, for example,
>>> +the driver times out waiting for a notification from the device for a
>>> +previously queued request.
>>
>> Again, I believe this has already been covered in the generic
>> sections -- do we instead need to specify that a transport MUST provide
>> a method to do xy? (or SHOULD, MAY, as applicable -- it would be good to
>> list explicitly what is mandatory for a transport to implement, and what
>> is optional.)
>
> Yes I think so. The s390x channel transport gets referenced because it
> has a nice enumerated list of operations. It would be good to codify
> which operations are mandatory for all transports and which are
> optional.
The problem
Re: [virtio-dev][PATCH V9 2/2] virtio-spi: add the device specification
On Tue, Dec 12 2023, Haixu Cui wrote:
[BTW: A changelog would be useful, either in the patch or in the cover
letter.]
> The Virtio SPI (Serial Peripheral Interface) device is a virtual
> SPI controller that allows the driver to operate and use the SPI
> controller under the control of the host.
>
> This patch adds the specification for virtio-spi.
>
> Signed-off-by: Haixu Cui
> Reviewed-by: Viresh Kumar
> ---
> device-types/spi/description.tex| 281
> device-types/spi/device-conformance.tex | 7 +
> device-types/spi/driver-conformance.tex | 7 +
> 3 files changed, 295 insertions(+)
> create mode 100644 device-types/spi/description.tex
> create mode 100644 device-types/spi/device-conformance.tex
> create mode 100644 device-types/spi/driver-conformance.tex
>
> diff --git a/device-types/spi/description.tex
> b/device-types/spi/description.tex
Please move inclusion of this new file into content.tex here, instead of
including a not-yet-existing file in the previous patch.
(...)
> +\field{mode_func_supported} indicates whether the following features are
> supported or not:
> +bit 0-1: CPHA feature,
> +0b00: invalid, must support as least one CPHA setting.
> +0b01: supports CPHA=0 only;
> +0b10: supports CPHA=1 only;
> +0b11: supports CPHA=0 and CPHA=1;
> +
> +bit 2-3: CPOL feature,
> +0b00: invalid, must support as least one CPOL setting.
> +0b01: supports CPOL=0 only;
> +0b10: supports CPOL=1 only;
> +0b11: supports CPOL=0 and CPOL=1;
> +
> +bit 4: chipselect active high feature, 0 for unsupported and 1 for
> supported,
> +chipselect active low must always be supported.
> +
> +bit 5: LSB first feature, 0 for unsupported and 1 for supported, MSB
> first must always be
> +supported.
> +
> +bit 6: loopback mode feature, 0 for unsupported and 1 for supported,
> normal mode
> +must always be supported.
> +
> +Note: CPOL is clock polarity and CPHA is clock phase. If CPOL is 0, the
> clock idles at the logical
> +low voltage, otherwise it idles at the logical high voltage. CPHA determines
> how data is outputted
> +and sampled.
Shouldn't you also specify what CPHA==0 and CPHA==1 mean?
(...)
> +VIRTIO_SPI_TRANS_ERR indicates a transfer error, which means that the
> parameters are all
> +valid but the trasnfer process failed.
s/trasnfer/transfer/
LGTM otherwise. Does anybody else have comments?
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] Changes coming to this mail list system
On Tue, Dec 05 2023, Scott McGrath wrote: [dropped non-virtio lists, added the other virtio lists] > The Transition Schedule > > Starting 14 December, there may be as much as several days of system > downtime. During the downtime you will not receive email messages sent to > this list or have access to the archive. > > We will announce the timing specific as we get closer to the transition. I've received off-list notification that this change is now postponed to January -- can you please confirm? It is important for us to plan how we are going to proceed with the 1.3 version of the virtio spec. (And it is of course also important for people reading those lists, who may not have an account in the system.) - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] virtio 1.3: update
You may have been wondering what the current state of virtio 1.3 is; the tl;dr is: we have to deal with some unfortunate delays, but it is really nearing release now. The current state - virtio-1.3-csd01 (committee specification draft) has been out for public review, and that 30 day period has just finished, with no additional comments being received. This means we're ready to start the process to get the committee specification draft released as an actual committee specification. The current issue - Moving from CSD to CS requires that the TC votes on it. Unfortunately, this will not be possible to fit into the remaining time before the OASIS infrastructure downtime due to the upcoming migration. While we hope that the new infrastructure will be in place quickly, it is unlikely that we will be able to do a vote before many/most of the voting members will be away for end-of-year holidays. [We actually need *two* votes in the end, with each ballot open for seven days.] The current plan We'll do any preparation work (which should be really minor) now, and start the voting process early in January. I'm reasonably confident that, once triggered, we should have finished the process in early February the latest. Apologies again for the numerous delays the spec encountered along the way, it's been pretty frustrating. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH V6 2/2] virtio-spi: add the device specification
On Mon, Dec 04 2023, Haixu Cui wrote: > Hi Huck, > > On 12/1/2023 7:01 PM, Cornelia Huck wrote: >> On Fri, Dec 01 2023, Haixu Cui wrote: >> >>>>> +Note: chipselect is an electrical signal controlled by the SPI >>>>> controller, used to select the >>>>> +SPI slaves connected to the controller. >>>> >>>> I wonder whether another term is now more commonly used... the Wikipedia >>>> article uses main/sub, is there a term that is usually used together >>>> with "controller"? >>> >>> In Wikipedia, >>> >>> "Historically, this arrangement has been called master/slave. But to >>> avoid referencing slavery, alternative names discussed in § Alternative >>> namings have been used." >>> >>> I am not quite sure if main/sub is widely used, but in latest Linux, >>> still use master/slave but master is gradually replaced by "controller". >>> >>> So I think "controller" here does make sense. What is your suggestion? >> >> "controller" is fine, I'm just wondering if "slave" has started to be >> replaced with something else as well... if it hasn't, we probably need >> to stick with it to avoid confusing readers. >> > > slave is still being used, but it's better to be used along with "master". > > Here I prefer using "SPI peripherals" instead of "SPI slaves". Looking > forward to your advice. Thank you so much. "SPI peripherals" sounds good to me. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements
On Tue, Dec 05 2023, Viresh Kumar wrote:
> The virtio documentation currently doesn't define any generic
> requirements that are applicable to all transports. They can be useful
> while adding support for a new transport.
>
> This commit tries to define the same.
Thank you for tackling this, albeit the devil's in the details :)
>
> Signed-off-by: Viresh Kumar
> ---
> content.tex | 48 ++--
> 1 file changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/content.tex b/content.tex
> index 0a62dce5f65f..d4d5e7d7045b 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -631,8 +631,52 @@ \section{Device Cleanup}\label{sec:General
> Initialization And Device Operation /
>
> \chapter{Virtio Transport Options}\label{sec:Virtio Transport Options}
>
> -Virtio can use various different buses, thus the standard is split
> -into virtio general and bus-specific sections.
> +The virtio devices are exposed to the guest as if they are physical
> +devices using a specific transport method, like PCI, MMIO or Channel
> +I/O.
I'm not sure we can talk about "exposed to the guest" here, except as an
example... maybe if we reword the whole paragraph (see my suggestion
below.)
> The transport methods define various aspects of the communication
> +between the device and the driver, like device discovery, exchanging
> +capabilities, interrupt handling, data transfer, etc.. Virtio can use
> +various different buses, thus the standard is split into virtio general
> +and bus-specific sections.
I think we should concentrate on the transport being what links device
and driver together... what about (reusing parts of your writeup):
"Devices and drivers can use different transport methods to enable
interaction, for example PCI, MMIO, or Channel I/O. The transport
methods define various aspects of the communication between the device
and the driver, like device discovery, exchanging capabilities,
interrupt handling, data transfer, etc. For example, in a host/guest
architecture, the host might expose a device to the guest on a PCI bus,
and the guest will use a PCI-specific driver to interact with it.
The standard is split into sections describing general virtio
implementation and transport-specific sections."
> +
> +\section{Virtio Transport Requirements}\label{sec:Virtio Transport Options /
> Virtio Transport Requirements}
> +
> +\devicenormative{\subsection}{Virtio Transport Requirements}{Virtio
> Transport Options}
I'm not sure we can introduce MUST (NOT) requirements for basic
functionality after the spec has been published for quite a time already
(although I'd assume every implementation is fulfilling the requirements
anyway)... thoughts?
> +
> +The device MUST present each event, in a transport defined way, from the
> +moment it takes place until the driver acknowledges the event.
I don't believe "event" is well-defined here.
> +
> +The device MUST NOT access virtqueue's contents before the driver
> +notifies that the queue is ready for access, in a transport defined way.
> +
> +The device MUST NOT access buffers on the virtqueue, after it has
> +modified them and notified the driver about their availability.
> +
> +The device MUST reset the virtqueues if requested by the driver, in a
> +transport defined way.
Isn't all of this already defined in one place of the spec or another?
> +
> +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio
> Transport Options}
> +
> +The driver MUST NOT access guest memory locations outside what's made
> +available by the device to the driver.
I don't think that makes sense -- I'd assume most guest memory locations
do not have anything to do with virtio, and we should try to avoid
host/guest terminology.
> +
> +The driver MUST NOT write to the read-only memory area and MUST NOT read
> +from the write-only memory area.
Which memory areas does that refer to? Parts of the transport-specific
data structures?
> +
> +The driver MUST acknowledge events presented by the device, as mandated
> +by the transport.
I don't think this is quite correct in the absolute -- for example, it
should be fine to not acknowledge events if some overriding event comes
along, or if the driver initiates a reset.
> +
> +The driver MUST NOT access virtqueue contents before the device notifies
> +about the readiness of the same.
> +
> +The driver MUST NOT access buffers, after it has added them to the
> +virtqueue and notified the device about their availability. The driver
> +MAY access them after the device has processed them and notified the
> +driver of their availability, in a transport defined way.
> +
> +The driver MAY ask the device to reset the virtqueues if, for example,
> +the driver times out waiting for a notification from the device for a
> +previously queued request.
Again, I believe this has already been covered in the generic
sections -- do we instead need to specify that a transport MUST provide
a method to do xy? (or
Re: [virtio-dev] [PATCH V6 2/2] virtio-spi: add the device specification
On Fri, Dec 01 2023, Haixu Cui wrote: >>> +Note: chipselect is an electrical signal controlled by the SPI controller, >>> used to select the >>> +SPI slaves connected to the controller. >> >> I wonder whether another term is now more commonly used... the Wikipedia >> article uses main/sub, is there a term that is usually used together >> with "controller"? > > In Wikipedia, > > "Historically, this arrangement has been called master/slave. But to > avoid referencing slavery, alternative names discussed in § Alternative > namings have been used." > > I am not quite sure if main/sub is widely used, but in latest Linux, > still use master/slave but master is gradually replaced by "controller". > > So I think "controller" here does make sense. What is your suggestion? "controller" is fine, I'm just wondering if "slave" has started to be replaced with something else as well... if it hasn't, we probably need to stick with it to avoid confusing readers. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH V6 2/2] virtio-spi: add the device specification
On Thu, Nov 30 2023, Haixu Cui wrote:
> The Virtio SPI (Serial Peripheral Interface) device is a virtual
> SPI controller that allows the driver to operate and use the SPI
> controller under the control of the device.
>
> This patch adds the specification for virtio-spi.
>
> Signed-off-by: Haixu Cui
> ---
> device-types/spi/description.tex| 288
> device-types/spi/device-conformance.tex | 7 +
> device-types/spi/driver-conformance.tex | 7 +
> 3 files changed, 302 insertions(+)
> create mode 100644 device-types/spi/description.tex
> create mode 100644 device-types/spi/device-conformance.tex
> create mode 100644 device-types/spi/driver-conformance.tex
>
> diff --git a/device-types/spi/description.tex
> b/device-types/spi/description.tex
> new file mode 100644
> index 000..c4816a6
> --- /dev/null
> +++ b/device-types/spi/description.tex
> @@ -0,0 +1,288 @@
> +\section{SPI Controller Device}\label{sec:Device Types / SPI Controller
> Device}
> +
> +The Virtio SPI (Serial Peripheral Interface) device is a virtual SPI
> controller that
> +allows the driver to operate and use the SPI controller under the control of
> the device,
> +either a physical SPI controller, or an emulated one.
> +
> +The Virtio SPI device has a single virtqueue. SPI transfer requests are
> placed into
> +the virtqueue, and serviced by the device.
I think it would still make sense to keep the host/guest example you
included in the last version; while we have to talk about device/driver
in the specification, giving host/guest as an example is completely
fine.
> +
> +\subsection{Device ID}\label{sec:Device Types / SPI Controller Device /
> Device ID}
> +45
> +
> +\subsection{Virtqueues}\label{sec:Device Types / SPI Controller Device /
> Virtqueues}
> +
> +\begin{description}
> +\item[0] requestq
> +\end{description}
> +
> +\subsection{Feature bits}\label{sec:Device Types / SPI Controller Device /
> Feature bits}
> +
> +None
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / SPI
> Controller Device / Device configuration layout}
> +
> +All fields of this configuration are always available and read-only for
> Virtio SPI driver.
s/for Virtio SPI driver/for the driver/
(matches what is said for other device types)
> +The config space shows the features and settings supported by the device.
> +
> +\begin{lstlisting}
> +struct virtio_spi_config {
> + u8 cs_max_number;
> + u8 cs_change_supported;
> + u8 tx_nbits_supported;
> + u8 rx_nbits_supported;
> + le32 bits_per_word_mask;
> + le32 mode_func_supported;
> + le32 max_freq_hz;
> + le32 max_word_delay_ns;
> + le32 max_cs_setup_ns;
> + le32 max_cs_hold_ns;
> + le32 max_cs_inactive_ns;
> +};
> +\end{lstlisting}
> +
> +\field{cs_max_number} is the maximum number of chipselect the device
> supports.
> +
> +Note: chipselect is an electrical signal controlled by the SPI controller,
> used to select the
> +SPI slaves connected to the controller.
I wonder whether another term is now more commonly used... the Wikipedia
article uses main/sub, is there a term that is usually used together
with "controller"?
> +
> +\field{cs_change_supported} indicates if the device supports to toggle
> chipselect
> +after each transfer in one message:
> +0: unsupported, means chipselect will keep active when executing the
> message transaction;
> +1: supported.
> +
> +Note: Message here contains a sequence of SPI transfers.
> +
> +\field{tx_nbits_supported} indicates the supported number of bit for
> writing, SINGLE is always
> +supported. In the bit definition below, a set bit means the transfer is
> supported, otherwise not:
> +bit 0: DUAL;
> +bit 1: QUAD;
> +bit 2: OCTAL;
> +other bits are reserved and must be set to 0 by the device.
> +
> +Note: Transfer bit options are commonly used in SPI:
> +\begin{itemize}
> +\item SINGLE: 1-bit transfer
> +\item DUAL: 2-bit transfer
> +\item QUAD: 4-bit transfer
> +\item OCTAL: 8-bit transfer
> +\end{itemize}
> +
> +\field{rx_nbits_supported} indicates the supported number of bit for
> reading, SINGLE is always
> +supported. In the bit definition below, a set bit means the transfer is
> supported, otherwise not:
> +bit 0: DUAL;
> +bit 1: QUAD;
> +bit 2: OCTAL;
> +other bits are reserved and must be set to 0 by the device.
> +
> +\field{bits_per_word_mask} is a mask indicating which values of
> bits_per_word are supported.
> +If not set, no limitation for bits_per_word.
s/no limitation/there is no limitation/
> +
> +Note: bits_per_word corresponds to \field{bits_per_word} in \field{struct
> virtio_spi_transfer_head}.
> +
> +\field{mode_func_supported} indicates whether the following features are
> supported or not:
> +bit 0-1: CPHA feature,
> +0b00: supports CPHA=0 and CPHA=1;
> +0b01: supports CPHA=0 only;
> +0b10:
[virtio-dev] Re: [virtio-comment] [PATCH] [PATCH v5] virtio-spi: add the device specification
On Wed, Nov 29 2023, Viresh Kumar wrote:
> On 29-11-23, 18:31, Haixu Cui wrote:
>> Hi Viresh,
>>
>> Thank you for your helpful comments. In next patch, I will clearly point
>> this out:
>
> Great, finally we are on the same page. Thanks Haixu.
>
>> "For full-duplex read and write transfer, both \field{tx_buf} and
>> \field{rx_buf} are used and the buffer size of \field{tx_buf} must be same
>> as \field{rx_buf}."
>
> Suggest rewriting as:
>
> In full-duplex transfer mode, both \field{tx_buf} and \field{rx_buf} are sent
> by
> the driver, \field{tx_buf} followed by \field{rx_buf}. The length of both the
> buffers MUST be same.
Is that in a non-normative section? (Sorry, I've lost track here...) If
so, I would say:
"The length of both buffers has to be the same."
>
>> And in drivernormative section, I will add a requirement:
>> "For full-duplex transfer, Virtio SPI driver MUST guarantee the write
>> transfer size is equal to the read transfer size"
>
> Maybe:
>
> drivernormative:
>
> In full-duplex transfer mode, the Virtio SPI driver MUST guarantee that the
> length of both \field{tx_buf} and \field{rx_buf} are same.
s/are same/is the same/
>
> devicenormative:
>
> In full-duplex transfer mode, the Virtio SPI device MUST verify that the
> length
> of both \field{tx_buf} and \field{rx_buf} are same. In case of any mismatch,
> the
s/are same/is the same/
> device MUST fail the transfer and notify the driver.
"notify the driver" == "set an appropriate error value"?
Can this be covered by one of the existing error values?
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [PATCH] [PATCH v5] virtio-spi: add the device specification
On Wed, Nov 29 2023, Haixu Cui wrote:
> Hi Viresh,
>
> On 11/29/2023 3:30 PM, Viresh Kumar wrote:
>> On 28-11-23, 20:58, Haixu Cui wrote:
>>> On 11/27/2023 6:17 PM, Viresh Kumar wrote:
On 24-11-23, 15:20, Haixu Cui wrote:
> +For half-duplex read transfer, \field{rx_buf} is filled by Virtio SPI
> device and consumed
> +by Virtio SPI driver. For half-duplex write transfer, \field{tx_buf} is
> filled by Virtio
> +SPI driver and consumed by Virtio SPI device. And for full-duplex read
> and write transfer,
> +both \field{tx_buf} and \field{rx_buf} are used.
Should the length of both the buffers in full-duplex mode be same ? If
yes, then
this should be mentioned (in case it is not).
>>>
>>> No, there is no such limitation. Write and read buffers may be different is
>>> size.
>>
>> Hmm, I worked with a SPI controller over a decade ago, and I must be
>> forgetting
>> something here I guess. But from whatever little I remember, with full-duplex
>> transfer, data flows on both MOSI and MISO lines as soon as clock signal is
>> applied. And so amount of data sent is always be equal to amount of data
>> received by both sides.
>>
>> Also if I see Linux's implementation of the `struct spi_transfer` [1], I see
>> `tx_buf`, `rx_buf` and a single `len` field, which applies to both the
>> buffers.
>> Which I guess is indicating that both buffers are supposed to be of same
>> length.
>>
>> What am I missing ?
>
> Oh so sorry for that. And I don't make it clear. Yes, tx_buf and rx_buf
> have the same size, Linux has such restriction. Just as you mention,
> kernel level spi_transfer has single "len", the same for
> spi_ioc_transfer passed from the userland.
>
> But I am not sure if this is in the scope of the spec. Because this is
> ensured by Linux, but Virtio SPI driver won't also can't verify this.
> This is a prerequisite for virtio spi processing requests.
>
> What is your suggestion? How about adding some descriptions here, like
> "for full-duplex, tx_buf and rx_buf are same in size, this is guaranteed
> by the kernel"?
We must not really make any assumptions in the spec about concrete
implementations (here, the Linux kernel), as someone implementing it in
a different environment will need to make explicit choices.
So, if tx_buf and rx_buf are required to be of the same size, it needs
to be explicitly stated in the spec, or an implementation might choose
to do it differently.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] Re: [PATCH] [PATCH v5] virtio-spi: add the device specification
On Mon, Nov 27 2023, Cornelia Huck wrote:
> On Mon, Nov 27 2023, Haixu Cui wrote:
>
>> On 11/24/2023 11:46 PM, Cornelia Huck wrote:
>>> On Fri, Nov 24 2023, Haixu Cui wrote:
>>>> +The \field{chip_select_max_number} is the maximum number of chipselect
>>>> the host SPI controller supports.
>>>
>>> "chipselect" is probably a known term for people familiar with SPI -- is
>>> there any definition of those terms that the spec can point to?
>>
>> Just as Mark said, there is no formal spec for SPI, so no standard spec
>> for such terms referring to. The same for CPHA/CPOL/LSB/MSB, please see
>> below.
>
> If we have nothing to point to, it is probably best to simply
> expand/explain the terms on their first usage.
>
>>> Can we point to some documentation that explains CPHA and CPOL?
>>
>> Here. No standard SPI spec to point to. CPOL/CPHA have definitions in
>> wikipedia(Clock polarity and phase chapter):
>>
>> https://en.wikipedia.org/wiki/Serial_Peripheral_Interface
>>
>> How about copying some concise information from wikipedia as Note? Or is
>> referring to such webpage acceptable in this spec.
>
> Not sure if we can do an outright copy (licence compatibility), but
> paraphrasing should be fine. (I'd rather not directly reference the
> site, because the content is not guaranteed to be stable, but we could
> maybe add it as "further reading".)
Looking again, the kernel doc referenced by Mark
(https://www.kernel.org/doc/html/v6.6/driver-api/spi.html) might also be
a good candidate, especially as we can refer to a specific version.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH] [PATCH v5] virtio-spi: add the device specification
On Mon, Nov 27 2023, Haixu Cui wrote:
> On 11/24/2023 11:46 PM, Cornelia Huck wrote:
>> On Fri, Nov 24 2023, Haixu Cui wrote:
>>> +The \field{chip_select_max_number} is the maximum number of chipselect the
>>> host SPI controller supports.
>>
>> "chipselect" is probably a known term for people familiar with SPI -- is
>> there any definition of those terms that the spec can point to?
>
> Just as Mark said, there is no formal spec for SPI, so no standard spec
> for such terms referring to. The same for CPHA/CPOL/LSB/MSB, please see
> below.
If we have nothing to point to, it is probably best to simply
expand/explain the terms on their first usage.
>> Can we point to some documentation that explains CPHA and CPOL?
>
> Here. No standard SPI spec to point to. CPOL/CPHA have definitions in
> wikipedia(Clock polarity and phase chapter):
>
> https://en.wikipedia.org/wiki/Serial_Peripheral_Interface
>
> How about copying some concise information from wikipedia as Note? Or is
> referring to such webpage acceptable in this spec.
Not sure if we can do an outright copy (licence compatibility), but
paraphrasing should be fine. (I'd rather not directly reference the
site, because the content is not guaranteed to be stable, but we could
maybe add it as "further reading".)
>>> +For each transfer request, Virtio SPI driver MUST check the fields in
>>> structure \field{virtio_spi_transfer_head}
>>> +and MUST reject the request if any filed is invalid or enabling the
>>> features not supported by host.
>>
>> s/filed/field/
>> s/host/device/
>>
>> Also, isn't the rejecting supposed to be done by the device, as the
>> driver is the party enqueueing the requests? Or do I have some kind of
>> fundamental misunderstanding?
>
> It may be better to filter some invalid requests by driver, as in the
> request header there are many parameters, and some of them are not
> supported by device, so it's quite possible that many requests invalid
> for the device. So if driver can do the first filter, such invalid
> requests will not be sent at all, this will conserve virtqueue and
> system overhead.
>
> And this is why exposing device supported features in the config space,
> it ensures that almost all requests in virtqueue are nice to the backend.
>
> device also will verify the requests again, as the following requirement:
> Virtio SPI device MUST verify the parameters in
> \field{virtio_spi_transfer_head} after receiving the request,
> and MUST set \field{virtio_spi_transfer_result} as VIRTIO_SPI_PARAM_ERR
> if not all parameters are valid or some device unsupported features are set.
>
> Although checking the requests twice seems a little redundant, it is
> more efficient comparing with sending some invalid requests to the device.
>
> What is your opinion? Do you think it is acceptable?
Thanks for your explanation, I think we simply have some terminology
issues. In the virtio spec, "driver" refers to one side of the
driver/device pair, and is used to describe how to communicate with the
device. In this case, "driver" would be the entity interacting with the
device, regardless of how it is implemented, and would be responsible
for sending the requests. Any filtering that would be done in a concrete
implementation (i.e. if you have one component generating the requests,
and then another component filtering them before actually putting them
into the queue) is out of scope for this spec -- we can maybe specify
that the driver should not send invalid requests, but I'm not sure that
this is actually needed.
> Once again, thanks a lot for your support.
You're welcome!
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH] [PATCH v5] virtio-spi: add the device specification
On Fri, Nov 24 2023, Haixu Cui wrote:
> virtio-spi is a virtual SPI (Serial Peripheral Interface) controller and it
> allows
> a guest to operate and use the host SPI controller.
>
> This patch adds the specification for virtio-spi.
As Mark has already reviewed it from the SPI POV (thanks!), I'm now
looking at the virtio side.
>
> Signed-off-by: Haixu Cui
> ---
> device-types/spi/description.tex| 281
> device-types/spi/device-conformance.tex | 7 +
> device-types/spi/driver-conformance.tex | 7 +
> 3 files changed, 295 insertions(+)
> create mode 100644 device-types/spi/description.tex
> create mode 100644 device-types/spi/device-conformance.tex
> create mode 100644 device-types/spi/driver-conformance.tex
>
> diff --git a/device-types/spi/description.tex
> b/device-types/spi/description.tex
> new file mode 100644
> index 000..8ca8c0d
> --- /dev/null
> +++ b/device-types/spi/description.tex
> @@ -0,0 +1,281 @@
> +\section{SPI Master Device}\label{sec:Device Types / SPI Master Device}
> +
> +virtio-spi is a virtual SPI (Serial Peripheral Interface) controller and it
> allows
> +a guest to operate and use the host SPI controller. Host SPI controller may
> be a
> +physical controller, or a virtual one(e.g. controller emulated by the
> software
Missing space before the opening bracket.
> +running in the host).
> +
> +virtio-spi has a single virtqueue. SPI transfer requests are placed into
> +the virtqueue, and serviced by the host SPI controller.
Is there any way to express all of this without referring to "host" and
"guest" here? (The paragraph below giving it as an example is fine.)
Something like "The virtio-spi device is a virtual SPI controller. It
allows the driver to operate and use an SPI controller under the control
of the device, either a physical controller, or an emulated one."
> +
> +In a typical host and guest architecture with virtio-spi, Virtio SPI driver
> is
> +the front-end running in the guest kernel, and Virtio SPI device acts as the
> +back-end in the host.
> +
> +\subsection{Device ID}\label{sec:Device Types / SPI Master Device / Device
> ID}
> +45
> +
> +\subsection{Virtqueues}\label{sec:Device Types / SPI Master Device /
> Virtqueues}
> +
> +\begin{description}
> +\item[0] requestq
> +\end{description}
> +
> +\subsection{Feature bits}\label{sec:Device Types / SPI Master Device /
> Feature bits}
> +
> +None
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / SPI Master
> Device / Device configuration layout}
> +
> +All fields of this configuration are always available and read-only for
> Virtio SPI driver.
> +The config space shows the features and settings supported by the host SPI
> controller.
> +
> +\begin{lstlisting}
> +struct virtio_spi_config {
> + u8 chip_select_max_number;
> + u8 cs_change_supported;
> + u8 tx_nbits_supported;
> + u8 rx_nbits_supported;
> + le32 bits_per_word_mask;
> + le32 mode_func_supported;
> + le32 max_freq_hz;
> + le32 max_word_delay_ns;
> + le32 max_cs_setup_ns;
> + le32 max_cs_hold_ns;
> + le32 max_cs_inactive_ns;
> +};
> +\end{lstlisting}
> +
> +The \field{chip_select_max_number} is the maximum number of chipselect the
> host SPI controller supports.
"chipselect" is probably a known term for people familiar with SPI -- is
there any definition of those terms that the spec can point to?
Also, it should be either "The field \field{whatever}" or
"\field{whatever}"; "The \field{whatever}" looks good in the LaTeX
source, but not in the end result.
> +
> +The \field{cs_change_supported} indicates if the host SPI controller
> supports to toggle chipselect
> +after each transfer in one message:
> +0: supported;
> +1: unsupported, means chipselect will keep active when executing the
> message transaction.
> +Note: Message here contains a sequence of SPI transfer.
> +
> +The \field{tx_nbits_supported} indicates the supported number of bit for
> writing:
> +bit 0: 2-bit transfer, 0 for unsupported and 1 for supported;
> +bit 1: 4-bit transfer, 0 for unsupported and 1 for supported;
> +bit 2: 8-bit transfer, 0 for unsupported and 1 for supported;
> +other bits are reserved as 0, 1-bit transfer is always supported.
> +
> +The \field{rx_nbits_supported} indicates the supported number of bit for
> reading:
> +bit 0: 2-bit transfer, 0 for unsupported and 1 for supported;
> +bit 1: 4-bit transfer, 0 for unsupported and 1 for supported;
> +bit 2: 8-bit transfer, 0 for unsupported and 1 for supported;
> +other bits are reserved as 0, 1-bit transfer is always supported.
> +
> +The \field{bits_per_word_mask} is a mask indicating which values of
> bits_per_word
> +are supported. If not set, no limitation for bits_per_word.
> +Note: bits_per_word corresponds to \field{bits_per_word} field in
> +structure \field{virtio_spi_transfer_head}, see below.
> +
>
Re: [virtio-dev] [PATCH] html: add missing enumitem package
On Wed, Sep 27 2023, "Michael S. Tsirkin" wrote:
> makediffhtml.sh currently fails with:
>
> ! Missing number, treated as zero.
>
>\c@*
> l.25850 \begin{enumerate}[label=\alph*
> .]
> ?
> ! Emergency stop.
>
>\c@*
> l.25850 \begin{enumerate}[label=\alph*
> .]
>
> Some web searches turned up suggestions to use enumitem and in fact,
> virtio.tex already does this - but virtio-html.tex doesn't.
>
> Adding \usepackage{enumitem} in virtio-html.tex too fixes the issue.
>
> Signed-off-by: Michael S. Tsirkin
> ---
> virtio-html.tex | 1 +
> 1 file changed, 1 insertion(+)
>
Oops, should read my mail before pushing. I'll use this one instead.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [PATCH virtio-1.3] update acknowledgements for 1.3
Move some names to the section for previous versions, add names of new
contributors, etc.
Signed-off-by: Cornelia Huck
---
acknowledgements.tex | 124 ++-
1 file changed, 75 insertions(+), 49 deletions(-)
diff --git a/acknowledgements.tex b/acknowledgements.tex
index 1c0a155143bf..39e80d4391d7 100644
--- a/acknowledgements.tex
+++ b/acknowledgements.tex
@@ -6,82 +6,65 @@ \chapter{Acknowledgements}\label{chap:Acknowledgements}
The following individuals have participated in the creation of this
specification and are gratefully acknowledged:
\subsection*{Participants}
-Alexander Duyck, Intel \newline
-Alex Bennée, Linaro \newline
+Alvaro Karsz, SolidRun \newline
Anton Yakovlev, OpenSynergy \newline
-Arseny Krasnov, Kaspersky Lab \newline
Cornelia Huck, Red Hat \newline
+David Edmondson, Oracle \newline
David Hildenbrand, Red Hat \newline
-David Stevens, Chromium \newline
-Dr. David Alan Gilbert, Red Hat \newline
+Dmitry Fomichev, Western Digital \newline
+Dust Li, Alibaba \newline
Enrico Granata, Google \newline
-Eugenio Pérez, Red Hat \newline
-Felipe Franciosi, Nutanix \newline
-Gaetan Harter, OpenSynergy \newline
-Gerd Hoffmann, Red Hat \newline
-Gurchetan Singh, Chromium \newline
+Haixu Cui, Quic Inc \newline
Halil Pasic, IBM \newline
-Hao Chen, Google \newline
-Huang Yang, Intel \newline
+Heng Qi, Alibaba \newline
+Hrishivarya Bhageeradhan, OpenSynergy \newline
Jan Kiszka, Siemens \newline
-Jean-Philippe Brucker, Linaro \newline
-Jiang Wang, Bytedance \newline
-Jie Deng, Intel \newline
-Joel Nider, Individual \newline
-Johannes Berg, Intel \newline
-Junji Wei, Bytedance \newline
-Keiichi Watanabe, Chromium \newline
-Marcel Holtmann, Individual \newline
-Max Gurtovoy, Nvidia \newline
+Jiri Pirko, Nvidia \newline
+Laura Loghin, Amazon \newline
+Lei He, Bytedance \newline
+Lingshan Zhu, Intel \newline
+Matti Moell, OpenSynergy \newline
Michael S. Tsirkin, Red Hat \newline
-Nikos Dragazis, Arrikto \newline
-Pankaj Gupta, Red Hat \newline
-Paolo Bonzini, Red Hat \newline
+Mihai Carabas, Oracle \newline
Parav Pandit, Nvidia \newline
-Peter Hilber, OpenSynergy \newline
-Petre Eftime, Amazon \newline
-Philipp Hahn, Univention \newline
-Rob Bradford, Intel \newline
-Stefan Fritsch, Individual \newline
+Ran Koren, Nvidia \newline
+Satananda Burla, Marvell \newline
+Shahaf Shuler, Nvidia \newline
+Si-Wei Liu, Oracle \newline
Stefan Hajnoczi, Red Hat \newline
Stefano Garzarella, Red Hat \newline
-Taylor Stark, Microsoft \newline
-Tiwei Bie, Intel \newline
-Viresh Kumar, Linaro \newline
-Vitaly Mireyno, Marvell \newline
Xuan Zhuo, Alibaba \newline
-Yadong Qi, Intel \newline
-Yoni Bettan, Red Hat \newline
Yuri Benditovich, Red Hat / Daynix \newline
+Zhenwei Pi, Bytedance \newline
The following non-members have provided valuable feedback on this
specification and are gratefully acknowledged:
\subsection*{Reviewers}
-Christophe de Dinechin, Red Hat \newline
-Gil Savir, Intel \newline
-Ruchika Gupta, Linaro \newline
-Arnd Bergmann, Individual \newline
-Bing Zhu, Intel \newline
-Eduardo Habkost, Red Hat \newline
-Eric Auger, Red Hat \newline
-Jason Wang, Red Hat \newline
-Jens Freimann, Red Hat \newline
-Kevin Tian, Intel \newline
-Linus Walleij, Linaro \newline
-Matti Möll, OpenSynergy \newline
-Tomas Winkler, Intel \newline
-Yang Huang, Intel \newline
+Damien Le Moal, Western Digital \newline
+Hans Holmberg, Western Digital \newline
+Hans Zhang, Alibaba \newline
+He Rongguang, Alibaba \newline
+Helin Guo, Alibaba \newline
+Jiang Liu, Alibaba \newline
+Matias Bjørling, Western Digital \newline
+Max Gurtovoy, Nvidia \newline
+Niklas Cassel, Western Digital \newline
+Tony Lu, Alibaba \newline
The following individuals have participated in the creation of
previous versions of this specification and are gratefully acknowledged:
\subsection*{Participants}
+Alexander Duyck, Intel \newline
+Alex Bennée, Linaro \newline
Allen Chia, Oracle \newline
Amit Shah, Red Hat \newline
Amos Kong, Red Hat \newline
Anthony Liguori, IBM \newline
+Anton Yakovlev, OpenSynergy \newline
+Arseny Krasnov, Kaspersky Lab \newline
Bruce Rogers, SUSE \newline
Bryan Venteicher, NetApp \newline
Chandra Thyamagondlu, Xilinx \newline
@@ -90,46 +73,78 @@ \subsection*{Participants}
Cunming, Liang, Intel \newline
Damjan, Marion, Cisco \newline
Daniel Kiper, Oracle \newline
+David Hildenbrand, Red Hat \newline
+David Stevens, Chromium \newline
+Dr. David Alan Gilbert, Red Hat \newline
+Enrico Granata, Google \newline
+Eugenio Pérez, Red Hat \newline
Fang Chen, Huawei \newline
Fang You, Huawei \newline
+Felipe Franciosi, Nutanix \newline
+Gaetan Harter, OpenSynergy \newline
Geoff Brown, M2Mi\newline
Gerd Hoffmann, Red Hat \newline
Gershon Janssen, Individual Member \newline
Grant Likely, ARM \newline
+Gurchetan Singh, Chromium \newline
Haggai Eran, Mellanox\newline
Halil Pasic
[virtio-dev] [virtio 1.3] Change freeze has started
As outlined in https://lists.oasis-open.org/archives/virtio/202304/msg00036.html ff., as part of the release process for the 1.3 version of the virtio spec, we have now entered change freeze. This means that the only changes to the main virtio branch will now be those needed for the 1.3 release (administrative and editorial.) We hope to have something ready for vote in August. For any changes for the next virtio release, please target the virtio-1.4 branch, which will eventually merge back into the main development branch once 1.3 has been released. (This next release might not be called 1.4, but that should be inconsequential here.) virtio 1.3 timeline: Development July 3rd: feature freeze (github issue and change on list) August 2nd: change freeze (all voted upon) virtio-next development branch opens < we are here August: prepare draft August 31st (latest): draft voted upon by TC September: public review period October: addtl public review period, if needed October (November): 1.3 released merge virtio-next development branch - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [virtio 1.3] Feature freeze has started
On Mon, Jul 03 2023, Cornelia Huck wrote:
> As outlined in
> https://lists.oasis-open.org/archives/virtio/202304/msg00036.html ff.,
> as part of the release process for the 1.3 version of the virtio spec,
> we have now entered feature freeze. This means:
>
> ***
> By July 1st[3rd], all not yet integrated features that are targeting 1.3 need
> to have at least a github issue open that is pointing to a patch (set)
> on the list.
> ***
>
> AFAICS, the list of features matching these criteria is the following:
>
> https://github.com/oasis-tcs/virtio-spec/issues/173 ("virtio-net:
> support inner header hash")
> * this looks very close, with voting likely to start in the next
> couple of days
>
> https://github.com/oasis-tcs/virtio-spec/issues/167 ("Support
> transitional device for PCIe VF")
> * I have not followed that one in detail, but it has been stated that
> this one should also be ready soon -- can people please confirm that
> this will be ready for inclusion in July?
>
> https://github.com/oasis-tcs/virtio-spec/issues/170 ("The size of
> virtio_net_hdr struct are wrong when VIRTIO_NET_HASH_REPORT feature is
> negotiated.")
> * Can someone please check the status of this? Looks like a patch is
>available, but no progress for a long time? Is this maybe interacting
>with the inner header hash feature?
>
> Any others that I have missed?
Last call for 1.3. The first two are done now, the third one has not
seen any action, so if anything, it's going to be post-1.3.
I think we're good (and can start preparing for the next steps).
>
> I'd like to wrap this up in July; not a real problem if we spill over
> a couple of days into August, but I'd really like to avoid delaying the
> process for too much (it will take long enough already.)
>
> virtio 1.3 timeline:
>
> Development
>
> July 3rd: feature freeze (github issue <-- we are here
> and change on list)
>
> August 1st: change freeze (all voted
> upon)
>
> virtio-next development branch opens
>
> August: prepare draft
>
> August 31st (latest): draft voted upon by TC
>
> September: public review period
>
> October: addtl public review period, if needed
>
> October (November): 1.3 released
>
> merge virtio-next development branch
>
>
> -
> To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: 1.3 and branching
On Fri, Jul 21 2023, "Michael S. Tsirkin" wrote: > On Fri, Jul 21, 2023 at 02:24:57PM +0200, Cornelia Huck wrote: >> On Wed, Jul 12 2023, "Michael S. Tsirkin" wrote: >> >> > On Wed, Jul 12, 2023 at 02:24:32PM +0200, Cornelia Huck wrote: >> >> On Wed, Jul 12 2023, "Michael S. Tsirkin" wrote: >> >> > Yes we were supposed to freeze for 1.3. This change can be merged on a >> >> > main branch after 1.3 forks and is under review. >> >> >> >> Nod, that's what I would prefer to do. Being merged on virtio-next >> >> should be enough for including device/driver implementations. >> > >> > Except I'd prefer a v1.3 branch instead of a next branch - adding things >> > on the branch should be harder, not easier. >> >> Just to clarify: What I had planned for 1.3 (and what we already did for >> 1.2) is to fork a -next branch, finish 1.3 on the master branch, and >> then merge -next back into master after we'd be done with 1.3. >> >> I'm not sure what you mean by "adding things on the branch should be >> harder, not easier" -- I think it is already a bit harder because it is >> a branch :) >> >> We can of course do a v1.3 branch instead and continue developing on >> master, but shouldn't we then create branches (glorified tags) for the >> older releases as well? > > Yea, makes sense. > I think we are all set WRT what we planned to be in 1.3 - right? I'll send out a "last call" JFTR, but I think we're good. > Next step is preparing the changelog and packaging it > all as WD, then voting to approve it as a CSD/CSPRD and start > public review. > > Have time this week? if not I will get to it next week. Certainly not this week, but one of us can get started with the changelog next week. Fortunately, it should be shorter than the one for 1.2 :) - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] 1.3 and branching (was: [virtio-dev] Re: [PATCH v12] virtio-net: support device stats)
On Wed, Jul 12 2023, "Michael S. Tsirkin" wrote: > On Wed, Jul 12, 2023 at 02:24:32PM +0200, Cornelia Huck wrote: >> On Wed, Jul 12 2023, "Michael S. Tsirkin" wrote: >> > Yes we were supposed to freeze for 1.3. This change can be merged on a >> > main branch after 1.3 forks and is under review. >> >> Nod, that's what I would prefer to do. Being merged on virtio-next >> should be enough for including device/driver implementations. > > Except I'd prefer a v1.3 branch instead of a next branch - adding things > on the branch should be harder, not easier. Just to clarify: What I had planned for 1.3 (and what we already did for 1.2) is to fork a -next branch, finish 1.3 on the master branch, and then merge -next back into master after we'd be done with 1.3. I'm not sure what you mean by "adding things on the branch should be harder, not easier" -- I think it is already a bit harder because it is a branch :) We can of course do a v1.3 branch instead and continue developing on master, but shouldn't we then create branches (glorified tags) for the older releases as well? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] Re: [PATCH 0/4] Short document fixes to inner hash feature
On Thu, Jul 13 2023, Cornelia Huck wrote:
> On Thu, Jul 13 2023, Parav Pandit wrote:
>
>> This short patches fixes the editing errors that stops the pdf and html
>> generation.
>>
>> These 3 fixes are for the patch [1] for the github issue [2].
>>
>> [1] https://lists.oasis-open.org/archives/virtio-comment/202307/msg00024.html
>> [2] https://github.com/oasis-tcs/virtio-spec/issues/173
>>
>> Patch summary:
>> patch-1 place C code under listing
>> patch-2 avoid hyphen and extra braces
>> patch-3 use table as hyperlink do not work well in C code listing
>> patch-4 refer 'advice' as 'note'
>>
>> Patch 1 to 3 appears to be must in the testing.
>> Patch 4 is not a fix and can be done later if it requires discussion.
>>
>> Parav Pandit (4):
>> virtio-net: Place C code under listing
>> virtio-net: Avoid hyphen and extra braces
>> virtio-net: Use table to describe inner hash to rfc mapping
>> virtio-net: Use note instead of advice
>>
>> device-types/net/description.tex | 45 ++--
>> introduction.tex | 15 +--
>> 2 files changed, 38 insertions(+), 22 deletions(-)
>>
>
> FTR, this is the diff I have locally (I had missed one underscore in the
> references yesterday...); maybe we can make the intra-reference links in
> introdcution.tex a bit nicer, but otherwise, this should be the minimal
> change to make this build:
For the "make it look nicer" part, I think this one would get the job
done; opinions?
diff --git a/introduction.tex b/introduction.tex
index 6f10a94b6fde..1e39a4b2c529 100644
--- a/introduction.tex
+++ b/introduction.tex
@@ -106,8 +106,8 @@ \section{Normative References}\label{sec:Normative
References}
Generic Routing Encapsulation. This protocol is only specified for IPv4
and used as either the payload or delivery protocol.
\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
\phantomsection\label{intro:rfc2890}\textbf{[RFC2890]} &
-Key and Sequence Number Extensions to GRE \ref{intro:rfc2784}. This
protocol describes extensions by which two fields, Key and
-Sequence Number, can be optionally carried in the GRE Header
\ref{intro:rfc2784}.
+Key and Sequence Number Extensions to \hyperref[intro:rfc2784]{GRE}. This
protocol describes extensions by which two fields, Key and
+Sequence Number, can be optionally carried in the
\hyperref[intro:rfc2784]{GRE} Header.
\newline\url{https://www.rfc-editor.org/rfc/rfc2890}\\
\phantomsection\label{intro:rfc7676}\textbf{[RFC7676]} &
IPv6 Support for Generic Routing Encapsulation (GRE). This protocol is
specified for IPv6 and used as either the payload or
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 2/4] virtio-net: Avoid hyphen and extra braces
On Thu, Jul 13 2023, Parav Pandit wrote:
> Avoid hyphen and replace it with white space like rest of the entries.
> Also avoid unnecessary braces.
> Name RFC as just RFC without special prefix about it.
>
> This likely resolves the html generation errors.
>
> Signed-off-by: Parav Pandit
> ---
> device-types/net/description.tex | 2 +-
> introduction.tex | 15 +++
> 2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/device-types/net/description.tex
> b/device-types/net/description.tex
> index 6fd4a20..53c811f 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -904,7 +904,7 @@ \subsubsection{Processing of Incoming
> Packets}\label{sec:Device Types / Network
> \end{itemize}
>
> The per-packet hash calculation can depend on the IP packet type. See
> -\hyperref[intro:IP]{[IP]}, \hyperref[intro:UDP]{[UDP]} and
> \hyperref[intro:TCP]{[TCP]}.
> +\hyperref[intro:IP]{IP}, \hyperref[intro:UDP]{UDP} and
> \hyperref[intro:TCP]{TCP}.
I'd keep that as-is (I think it looks nicer with the brackets).
>
> \subparagraph{Supported/enabled hash types}
> \label{sec:Device Types / Network Device / Device Operation / Processing of
> Incoming Packets / Hash calculation for incoming packets / Supported/enabled
> hash types}
> diff --git a/introduction.tex b/introduction.tex
> index 81f07a4..028ec17 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -101,26 +101,25 @@ \section{Normative References}\label{sec:Normative
> References}
> \phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
> Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic
> Cureve Cryptography'', Version 1.0, September 2000.
> \newline\url{https://www.secg.org/sec1-v2.pdf}\\
> -
> - \phantomsection\label{intro:gre_rfc2784}\textbf{[GRE_rfc2784]} &
> + \phantomsection\label{intro:rfc2784}\textbf{[RFC2784]} &
> Generic Routing Encapsulation. This protocol is only specified for IPv4
> and used as either the payload or delivery protocol.
> \newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
> - \phantomsection\label{intro:gre_rfc2890}\textbf{[GRE_rfc2890]} &
> -Key and Sequence Number Extensions to GRE \ref{intro:gre_rfc2784}. This
> protocol describes extensions by which two fields, Key and
> -Sequence Number, can be optionally carried in the GRE Header
> \ref{intro:gre_rfc2784}.
> + \phantomsection\label{intro:rfc2890}\textbf{[RFC2890]} &
> +Key and Sequence Number Extensions to GRE \ref{intro:rfc2784}. This
> protocol describes extensions by which two fields, Key and
> +Sequence Number, can be optionally carried in the GRE Header
> \ref{intro:rfc2784}.
> \newline\url{https://www.rfc-editor.org/rfc/rfc2890}\\
> - \phantomsection\label{intro:gre_rfc7676}\textbf{[GRE_rfc7676]} &
> + \phantomsection\label{intro:rfc7676}\textbf{[RFC7676]} &
> IPv6 Support for Generic Routing Encapsulation (GRE). This protocol is
> specified for IPv6 and used as either the payload or
> delivery protocol. Note that this does not change the GRE header format
> or any behaviors specified by RFC 2784 or RFC 2890.
> \newline\url{https://datatracker.ietf.org/doc/rfc7676/}\\
> - \phantomsection\label{intro:gre_in_udp_rfc8086}\textbf{[GRE-in-UDP]} &
> + \phantomsection\label{intro:rfc8086}\textbf{[GRE in UDP]} &
Hyphens are ok, I'd keep it.
> GRE-in-UDP Encapsulation. This specifies a method of encapsulating
> network protocol packets within GRE and UDP headers.
> This protocol is specified for IPv4 and IPv6, and used as either the
> payload or delivery protocol.
> \newline\url{https://www.rfc-editor.org/rfc/rfc8086}\\
> \phantomsection\label{intro:vxlan}\textbf{[VXLAN]} &
> Virtual eXtensible Local Area Network.
> \newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
> - \phantomsection\label{intro:vxlan-gpe}\textbf{[VXLAN-GPE]} &
> + \phantomsection\label{intro:vxlan gpe}\textbf{[VXLAN GPE]} &
> Generic Protocol Extension for VXLAN. This protocol describes extending
> Virtual eXtensible Local Area Network (VXLAN) via changes to the VXLAN header.
>
> \newline\url{https://www.ietf.org/archive/id/draft-ietf-nvo3-vxlan-gpe-12.txt}\\
> \phantomsection\label{intro:geneve}\textbf{[GENEVE]} &
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [PATCH 1/4] virtio-net: Place C code under listing
On Thu, Jul 13 2023, Parav Pandit wrote:
> With extra white space for the define, pdf generation fails.
> Also place the C code under listing.
>
> Signed-off-by: Parav Pandit
> ---
> device-types/net/description.tex | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/device-types/net/description.tex
> b/device-types/net/description.tex
> index 206020d..6fd4a20 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1024,12 +1024,14 @@ \subsubsection{Processing of Incoming
> Packets}\label{sec:Device Types / Network
> If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the
> command
> VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the inner
> header hash.
>
> +\begin{lstlisting}
> struct virtnet_hash_tunnel {
> le32 enabled_tunnel_types;
> };
>
> #define VIRTIO_NET_CTRL_HASH_TUNNEL 7
> - #define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
> +#define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
I think the indentation here is intentional (as in other, similare
sections.)
> +\end{lstlisting}
>
> Field \field{enabled_tunnel_types} contains the bitmask of encapsulation
> types enabled for inner header hash.
> See \ref{sec:Device Types / Network Device / Device Operation / Processing
> of Incoming Packets /
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 0/4] Short document fixes to inner hash feature
On Thu, Jul 13 2023, Parav Pandit wrote:
> This short patches fixes the editing errors that stops the pdf and html
> generation.
>
> These 3 fixes are for the patch [1] for the github issue [2].
>
> [1] https://lists.oasis-open.org/archives/virtio-comment/202307/msg00024.html
> [2] https://github.com/oasis-tcs/virtio-spec/issues/173
>
> Patch summary:
> patch-1 place C code under listing
> patch-2 avoid hyphen and extra braces
> patch-3 use table as hyperlink do not work well in C code listing
> patch-4 refer 'advice' as 'note'
>
> Patch 1 to 3 appears to be must in the testing.
> Patch 4 is not a fix and can be done later if it requires discussion.
>
> Parav Pandit (4):
> virtio-net: Place C code under listing
> virtio-net: Avoid hyphen and extra braces
> virtio-net: Use table to describe inner hash to rfc mapping
> virtio-net: Use note instead of advice
>
> device-types/net/description.tex | 45 ++--
> introduction.tex | 15 +--
> 2 files changed, 38 insertions(+), 22 deletions(-)
>
FTR, this is the diff I have locally (I had missed one underscore in the
references yesterday...); maybe we can make the intra-reference links in
introdcution.tex a bit nicer, but otherwise, this should be the minimal
change to make this build:
diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 206020de567d..76585b0dd6d3 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1024,12 +1024,14 @@ \subsubsection{Processing of Incoming
Packets}\label{sec:Device Types / Network
If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the
command
VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the inner
header hash.
+\begin{lstlisting}
struct virtnet_hash_tunnel {
le32 enabled_tunnel_types;
};
#define VIRTIO_NET_CTRL_HASH_TUNNEL 7
#define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
+\end{lstlisting}
Field \field{enabled_tunnel_types} contains the bitmask of encapsulation types
enabled for inner header hash.
See \ref{sec:Device Types / Network Device / Device Operation / Processing of
Incoming Packets /
@@ -1063,16 +1065,16 @@ \subsubsection{Processing of Incoming
Packets}\label{sec:Device Types / Network
Hash calculation for incoming packets / Encapsulation types supported/enabled
for inner header hash}
Encapsulation types applicable for inner header hash:
-\begin{lstlisting}
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /*
\hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /*
\hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /*
\hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /*
\hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN (1 << 4) /*
\hyperref[intro:vxlan]{[VXLAN]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE (1 << 5) /*
\hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE (1 << 6) /*
\hyperref[intro:geneve]{[GENEVE]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /*
\hyperref[intro:ipip]{[IPIP]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE (1 << 8) /*
\hyperref[intro:nvgre]{[NVGRE]} */
+\begin{lstlisting}[escapechar=|]
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /*
|\hyperref[intro:rfc2784]{[RFC2784]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /*
|\hyperref[intro:rfc2890]{[RFC2890]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /*
|\hyperref[intro:rfc7676]{[RFC7676]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /*
|\hyperref[intro:rfc8086]{[GRE-in-UDP]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN (1 << 4) /*
|\hyperref[intro:vxlan]{[VXLAN]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE (1 << 5) /*
|\hyperref[intro:vxlan-gpe]{[VXLAN-GPE]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE (1 << 6) /*
|\hyperref[intro:geneve]{[GENEVE]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /*
|\hyperref[intro:ipip]{[IPIP]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE (1 << 8) /*
|\hyperref[intro:nvgre]{[NVGRE]}| */
\end{lstlisting}
\subparagraph{Advice}
diff --git a/introduction.tex b/introduction.tex
index 81f07a4fee19..6f10a94b6fde 100644
--- a/introduction.tex
+++ b/introduction.tex
@@ -102,18 +102,18 @@ \section{Normative References}\label{sec:Normative
References}
Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve
Cryptography'', Version 1.0, September 2000.
\newline\url{https://www.secg.org/sec1-v2.pdf}\\
- \phantomsection\label{intro:gre_rfc2784}\textbf{[GRE_rfc2784]} &
+ \phantomsection\label{intro:rfc2784}\textbf{[RFC2784]} &
Generic Routing Encapsulation. This protocol
[virtio-dev] RE: [PATCH 3/4] virtio-net: Use table to describe inner hash to rfc mapping
On Wed, Jul 12 2023, Parav Pandit wrote: >> From: Michael S. Tsirkin >> Sent: Wednesday, July 12, 2023 6:41 PM >> Hmm. escapechar did not work then? > > Forgot to mention, I tried it, it didn't work. It seemed to work for me (at least in the pdf) -- I'd prefer that one as a minimal change. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v21] virtio-net: support inner header hash
On Wed, Jul 12 2023, Heng Qi wrote:
> 在 2023/7/12 下午9:25, Cornelia Huck 写道:
>> ...while the changes above give me a fine pdf, the html generated is
>> broken. The problem seems to originate in the normative references that
>> are added in introduction.tex, but I don't see where it goes astray.
>>
>> [401] [402] [403] [404] (./virtio-v1.2-cs01.aux
>> ! Missing \endcsname inserted.
>>
>> \unhbox
>> l.49 ...ction}Normative References}}{section.1}{}}
>>
>> ?
>> ! Emergency stop.
>>
>> \unhbox
>> l.49 ...ction}Normative References}}{section.1}{}}
>>
>> (makehtml.sh on master seems to work ok, so it's something in this
>> patch...)
>>
>> Does anyone else manage to spot the problem?
>
> The problem is that underline cannot be added directly in \textbf, we
> need a little modification:
>
> \textbf{[GRE_rfc2784]} --> \textbf{[GRE\_rfc2784]}
> \textbf{[GRE_rfc2890]} --> \textbf{[GRE\_rfc2890]}
> \textbf{[GRE_rfc7676]} --> \textbf{[GRE\_rfc7676]}
>
> After this modification, I can successfully compile pdf on overleaf,
> please try this.
I'm afraid that's not the problem (I had tried this and similar changes
without success); the pdf generation is already fine, but html still
fails with this change (or similar).
[Trying on a standard Fedora 37 with a full texlive install...]
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] Re: [PATCH v21] virtio-net: support inner header hash
On Wed, Jul 12 2023, Heng Qi wrote:
> 在 2023/7/12 下午8:42, Cornelia Huck 写道:
>> On Wed, Jul 12 2023, "Michael S. Tsirkin" wrote:
>>
>>> On Wed, Jul 12, 2023 at 02:22:26PM +0200, Cornelia Huck wrote:
>>>> On Mon, Jul 03 2023, Heng Qi wrote:
>>>>
>>>> (...)
>>>>
>>>>> +\paragraph{Inner Header Hash}
>>>>> +\label{sec:Device Types / Network Device / Device Operation / Processing
>>>>> of Incoming Packets / Inner Header Hash}
>>>>> +
>>>>> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the
>>>>> command
>>>>> +VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the
>>>>> inner header hash.
>>>>> +
>>>>> +struct virtnet_hash_tunnel {
>>>>> +le32 enabled_tunnel_types;
>>>>> +};
>>>>> +
>>>>> +#define VIRTIO_NET_CTRL_HASH_TUNNEL 7
>>>>> + #define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
>>>> This needs to be wrapped in \begin{lstlisting}..\end{lstlisting}, can do
>>>> so when applying.
>>>>
>>>> (...)
>>>>
>>>>> +Encapsulation types applicable for inner header hash:
>>>>> +\begin{lstlisting}
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /*
>>>>> \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /*
>>>>> \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /*
>>>>> \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /*
>>>>> \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN (1 << 4) /*
>>>>> \hyperref[intro:vxlan]{[VXLAN]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE (1 << 5) /*
>>>>> \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE (1 << 6) /*
>>>>> \hyperref[intro:geneve]{[GENEVE]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /*
>>>>> \hyperref[intro:ipip]{[IPIP]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE (1 << 8) /*
>>>>> \hyperref[intro:nvgre]{[NVGRE]} */
>>>>> +\end{lstlisting}
>>>> I'm afraid this one doesn't come out quite as intended, we'll end up
>>>> with verbatim "\hyperref" text instead of a link. Anyone have a good
>>>> idea on how to fix that?
>>>>
>>>> I'd prefer to push this now with the first issue addressed and to do an
>>>> (editorial) patch on top to deal with the second issue (unless someone
>>>> can come up with a really trivial fix for it, then I can apply that
>>>> straightaway.)
>>> Someone suggested using escapechar:
>>> https://tex.stackexchange.com/questions/314903/inline-links-in-code-listings
>>>
>>> Didn't try.
>> Looks reasonable (and also revealed a typo for VXLAN-GPE). I think I'll
>> go ahead with this one.
>
> Yes. "intro:vxlan_gpe" -> "intro:vxlan-gpe".
>
> May I ask if the fix to these two problems is for me to make fix patches
> or for you to solve it when editing?
I'd fix it myself while applying, but...
...while the changes above give me a fine pdf, the html generated is
broken. The problem seems to originate in the normative references that
are added in introduction.tex, but I don't see where it goes astray.
[401] [402] [403] [404] (./virtio-v1.2-cs01.aux
! Missing \endcsname inserted.
\unhbox
l.49 ...ction}Normative References}}{section.1}{}}
?
! Emergency stop.
\unhbox
l.49 ...ction}Normative References}}{section.1}{}}
(makehtml.sh on master seems to work ok, so it's something in this
patch...)
Does anyone else manage to spot the problem?
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v21] virtio-net: support inner header hash
On Wed, Jul 12 2023, "Michael S. Tsirkin" wrote:
> On Wed, Jul 12, 2023 at 02:22:26PM +0200, Cornelia Huck wrote:
>> On Mon, Jul 03 2023, Heng Qi wrote:
>>
>> (...)
>>
>> > +\paragraph{Inner Header Hash}
>> > +\label{sec:Device Types / Network Device / Device Operation / Processing
>> > of Incoming Packets / Inner Header Hash}
>> > +
>> > +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the
>> > command
>> > +VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the inner
>> > header hash.
>> > +
>> > +struct virtnet_hash_tunnel {
>> > +le32 enabled_tunnel_types;
>> > +};
>> > +
>> > +#define VIRTIO_NET_CTRL_HASH_TUNNEL 7
>> > + #define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
>>
>> This needs to be wrapped in \begin{lstlisting}..\end{lstlisting}, can do
>> so when applying.
>>
>> (...)
>>
>> > +Encapsulation types applicable for inner header hash:
>> > +\begin{lstlisting}
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /*
>> > \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /*
>> > \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /*
>> > \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /*
>> > \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN (1 << 4) /*
>> > \hyperref[intro:vxlan]{[VXLAN]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE (1 << 5) /*
>> > \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE (1 << 6) /*
>> > \hyperref[intro:geneve]{[GENEVE]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /*
>> > \hyperref[intro:ipip]{[IPIP]} */
>> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE (1 << 8) /*
>> > \hyperref[intro:nvgre]{[NVGRE]} */
>> > +\end{lstlisting}
>>
>> I'm afraid this one doesn't come out quite as intended, we'll end up
>> with verbatim "\hyperref" text instead of a link. Anyone have a good
>> idea on how to fix that?
>>
>> I'd prefer to push this now with the first issue addressed and to do an
>> (editorial) patch on top to deal with the second issue (unless someone
>> can come up with a really trivial fix for it, then I can apply that
>> straightaway.)
>
> Someone suggested using escapechar:
> https://tex.stackexchange.com/questions/314903/inline-links-in-code-listings
>
> Didn't try.
Looks reasonable (and also revealed a typo for VXLAN-GPE). I think I'll
go ahead with this one.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [PATCH v12] virtio-net: support device stats
On Wed, Jul 12 2023, "Michael S. Tsirkin" wrote: > On Wed, Jul 12, 2023 at 07:44:01PM +0800, Xuan Zhuo wrote: >> On Wed, 12 Jul 2023 07:34:38 -0400, "Michael S. Tsirkin" >> wrote: >> > On Tue, Mar 15, 2022 at 11:24:02AM +0800, Xuan Zhuo wrote: >> > > This patch allows the driver to obtain some statistics from the device. >> > > >> > > In the back-end implementation, we can count a lot of such information, >> > > which can be used for debugging and judging the running status of the >> > > back-end. We hope to directly display it to the user through ethtool. >> > > >> > > To get stats atomically, try to get stats for all queue pairs in one >> > > command. >> > > >> > > Signed-off-by: Xuan Zhuo >> > > Suggested-by: Michael S. Tsirkin >> > >> > Functionally, I think this is close to ok now. But the text needs >> > of work. Are you trying to target 1.3 with this? >> >> I personally worry about that 1.3 may be too late, if possible, I would like >> to >> respond as soon as possible. >> >> I really don't really care if it can get into 1.3 or not but I do hope that >> this >> can be done as soon as possible. >> >> Thanks. > > Yes we were supposed to freeze for 1.3. This change can be merged on a > main branch after 1.3 forks and is under review. Nod, that's what I would prefer to do. Being merged on virtio-next should be enough for including device/driver implementations. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v21] virtio-net: support inner header hash
On Mon, Jul 03 2023, Heng Qi wrote:
(...)
> +\paragraph{Inner Header Hash}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of
> Incoming Packets / Inner Header Hash}
> +
> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the
> command
> +VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the inner
> header hash.
> +
> +struct virtnet_hash_tunnel {
> +le32 enabled_tunnel_types;
> +};
> +
> +#define VIRTIO_NET_CTRL_HASH_TUNNEL 7
> + #define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
This needs to be wrapped in \begin{lstlisting}..\end{lstlisting}, can do
so when applying.
(...)
> +Encapsulation types applicable for inner header hash:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /*
> \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /*
> \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /*
> \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /*
> \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN (1 << 4) /*
> \hyperref[intro:vxlan]{[VXLAN]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE (1 << 5) /*
> \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE (1 << 6) /*
> \hyperref[intro:geneve]{[GENEVE]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /*
> \hyperref[intro:ipip]{[IPIP]} */
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE (1 << 8) /*
> \hyperref[intro:nvgre]{[NVGRE]} */
> +\end{lstlisting}
I'm afraid this one doesn't come out quite as intended, we'll end up
with verbatim "\hyperref" text instead of a link. Anyone have a good
idea on how to fix that?
I'd prefer to push this now with the first issue addressed and to do an
(editorial) patch on top to deal with the second issue (unless someone
can come up with a really trivial fix for it, then I can apply that
straightaway.)
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v14] admin: Add group member legacy register access commands
On Mon, Jul 10 2023, "Michael S. Tsirkin" wrote:
> On Mon, Jul 10, 2023 at 07:17:52PM +0300, Parav Pandit wrote:
>> +The \field{flags} value of 0x1 indicates that the notification address is of
>> +the owner device, value of 0x2 indicates that the notification address is of
>> +the member device, the value of 0 indicates that all the entries starting
>> from
>
> , the -> and value of 0x0
Looking at this sentence again, maybe make this
"A \field{flags} value of 0x1 indicates that the notification address is
that of the owner device, a value of 0x2 indicates that the notification
address is that of the member device, and a value of 0 indicates..."
if you're tweaking it anyway?
>
>
>> +that entry are invalid entries in \field{entries}. All other values in
>> +\field{flags} are reserved.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v13] admin: Add group member legacy register access commands
On Fri, Jul 07 2023, Parav Pandit wrote:
(...)
> +When the device supports VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO command, the
> group
s/supports/supports the/
> +owner device hardwires VF BAR0 to zero in the SR-IOV Extended capability.
(...)
> +\devicenormative{\paragraph}{Legacy Interface}{Basic Facilities of a Virtio
> Device / Device groups / Group administration commands / Legacy Interface}
> +
> +A device MUST either support all of, or none of
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands.
> +
> +For VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
s/For/For the/
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands,
> +the device MUST decode and encode (respectively) the value of the
> +\field{data} using the little-endian format.
> +
> +For VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE and
s/For/For the/
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ commands,
> +the device MUST fail the command when the value of the
> +\field{offset} and the length of the \field{data} does not refer to a
"do not", I think?
> +single field or is not completely within the virtio common configuration
> +excluding the device-specific configuration.
> +
> +For VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
s/For/For the/
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands,
> +the device MUST fail the command when the value of the
> +\field{offset} and the length of the \field{data} does not refer to a
"do not"?
> +single field or is not completely within the virtio device-specific
> +configuration.
> +
> +The command VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE MUST have the same
> effect
> +as writing into the virtio common configuration structure through the legacy
> +interface.
> +
> +The command VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ MUST have the same
> effect as
> +reading from the virtio common configuration structure through the legacy
> +interface.
> +
> +The command VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE MUST have the same effect
> as
> +writing into the virtio device-specific configuration through the legacy
> +interface.
> +
> +The command VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ MUST have the same effect as
> +reading from the virtio device-specific configuration through the legacy
> +interface.
> +
> +For the SR-IOV group type, when the owner device supports
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO
> +commands,
> +\begin{itemize}
> +\item the owner device and the group member device SHOULD follow the rules
> +for the PCI Revision ID and Subsystem Device ID of the non-transitional
> devices
> +documented in section \ref{sec:Virtio Transport Options / Virtio Over PCI
> Bus / PCI Device Discovery}.
> +
> +\item the owner device SHOULD follow the rules for the PCI Device ID of the
> non-transitional
> +devices documented in section
> +\ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device
> Discovery}.
> +
> +\item any driver notification received by the device at any of the
> notification
> +address supplied in the command result of
> +VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO MUST function as if the device received
> +the notification through VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE
> +command at an offset \field{offset} matching \field{Queue Notify}.
> +\end{itemize}
> +
> +If the device supports VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO command,
s/support/supports the/
> +\begin{itemize}
> +\item the device MUST also support all of
> VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands.
> +
> +\item in the command result of VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO, the last
> +\field{struct virtio_pci_legacy_notify_info} entry MUST have \field{flags} of
> +zero.
> +
> +\item in the command result of VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO, valid
> +entries MUST have a \field{bar} which is not hardwired to zero.
> +
> +\item in the command result of VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO, valid
> +entries MUST have an \field{offset} aligned to 2-byte.
> +
> +\item the device MAY support VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO with entries
> +of the owner device or the member device or both of them.
> +
> +\item for the SR-IOV group type, the group owner device MUST hardwire VF BAR0
> +to zero in the SR-IOV Extended capability.
> +\end{itemize}
> +
> +\drivernormative{\paragraph}{Legacy Interface}{Basic Facilities of a Virtio
> Device / Device groups / Group administration commands / Legacy Interface}
> +
> +For VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
s/For/For the/
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
>
Re: [virtio-dev][PATCH 1/2] virtio-spi: define the DEVICE ID for virtio SPI master
On Fri, Jul 07 2023, Haixu Cui wrote: > Hi Michael S. Tsirkin, Cornelia Huck, > I have created an issue > https://github.com/oasis-tcs/virtio-spec/issues/174 to request Device ID > 45 for SPI master. Please help to deal with it, thank you very much. Technically, we're in feature freeze already, but practically, I don't think we should block adding a simple device id reservation... Michael, any objections to me creating a ballot later today? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v12 0/3] admin: Access legacy registers using admin commands
On Fri, Jul 07 2023, Parav Pandit wrote: > This short series introduces legacy registers access commands for the owner > group member access the legacy registers of the member VFs. > This short series introduces legacy region access commands by the group owner > device for its member devices. > Currently it is applicable to the PCI PF and VF devices. If in future any > SIOV devices to support legacy registers, they can be easily supported using > same commands by using the group member identifiers of the future SIOV > devices. > > More details as overview, motivation, use case are further described > below. > > Patch summary: > -- > patch-1 fix split rows of admin opcode tables by a line > patch-2 fix section numbering Pushed 1+2 as editorial updates. > patch-3 add legacy region access commands - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v11 0/3] admin: Access legacy registers using admin commands
On Thu, Jul 06 2023, "Michael S. Tsirkin" wrote: > On Fri, Jul 07, 2023 at 12:27:19AM +0300, Parav Pandit wrote: >> This short series introduces legacy registers access commands for the owner >> group member access the legacy registers of the member VFs. >> This short series introduces legacy region access commands by the group owner >> device for its member devices. >> Currently it is applicable to the PCI PF and VF devices. If in future any >> SIOV devices to support legacy registers, they can be easily supported using >> same commands by using the group member identifiers of the future SIOV >> devices. >> >> More details as overview, motivation, use case are further described >> below. > > corneli want to apply 1,2 as editorial? I guess that makes sense to get them out of the way. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [PATCH v10 4/4] transport-pci: Introduce group legacy group member config region access
On Thu, Jul 06 2023, Parav Pandit wrote: >> From: Michael S. Tsirkin >> Sent: Thursday, July 6, 2023 4:42 PM > > [..] >> > Can we please avoid this over engineering? >> > Interface has the doors open for driver to make wise decision depending on >> its environment. >> >> what if driver can access both with the same ease? this is the case that >> bothers >> me and I think it's practical since it will be common on linux. > > Then Linux can say my preference is order, so it picks member. Let's step back and consider what our goal is here: to provide a spec that someone wanting to implement a driver can follow and end up with a driver that works with devices that adhere to this spec. I don't think expecting the driver to make "wise descisions" is helpful for the person wanting to implement a driver. "Do this, and in case this does not work in your environment, do that" seems much more helpful, and still gives enough flexibility to cover different environments. In short, make things simple for the consumer of the spec. [Also, I notice that the discussion seems to get bogged down in tiny details, fundamental statements, or a mixture of both. It might sometimes be helpful to just step back, re-read the original proposal, and only then answer. Otherwise, this leads to frustration for everyone involved.] - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [PATCH v10 4/4] transport-pci: Introduce group legacy group member config region access
On Thu, Jul 06 2023, Parav Pandit wrote:
>> From: Cornelia Huck
>> Sent: Thursday, July 6, 2023 12:28 PM
>>
>> On Thu, Jul 06 2023, Parav Pandit wrote:
>>
>> > diff --git a/transport-pci-legacy-regs.tex
>> > b/transport-pci-legacy-regs.tex new file mode 100644 index
>> > 000..ceea28c
>> > --- /dev/null
>> > +++ b/transport-pci-legacy-regs.tex
>> > @@ -0,0 +1,42 @@
>> > +\subsection{Legacy Interface: Group member device Configuration
>> > +Region Access}\label{sec:Virtio Transport Options / Virtio Over PCI
>> > +Bus / Legacy Interface: Group Member Device Configuration Region
>> > +Access}
>> > +
>> > +The PCI owner device or the member device or both support driver
>> > +notifications using
>>
>> What about
>>
>> "The PCI owner device, the member device, or both can choose to support..." ?
>>
> Fine too.
> Any thing wrong in having or as above, so I don't write it next time?
> Or that in current form reads better to me.
Just a bare "support" does not really tell the reader if this is
something that is required or optional. Dropping the first "or" makes it
read better for me.
>
>> > +a notification region defined in \field{struct
>> virtio_pci_legacy_notify_region}.
(...)
>> > +The group owner device or the group member device or both MAY support
>> > +driver notifications region.
>>
>> Make this "a driver notification region"?
>>
> Notifications are generally more than one and spec has the section "driver
> notifications", so...
I'd parse this as "a region for the purpose of notification" (and
"notification region" is used above)... but in any case, we need the
article here, I think.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v10 4/4] transport-pci: Introduce group legacy group member config region access
On Thu, Jul 06 2023, Parav Pandit wrote:
> diff --git a/transport-pci-legacy-regs.tex b/transport-pci-legacy-regs.tex
> new file mode 100644
> index 000..ceea28c
> --- /dev/null
> +++ b/transport-pci-legacy-regs.tex
> @@ -0,0 +1,42 @@
> +\subsection{Legacy Interface: Group member device Configuration Region
> Access}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / Legacy
> Interface: Group Member Device Configuration Region Access}
> +
> +The PCI owner device or the member device or both support driver
> notifications using
What about
"The PCI owner device, the member device, or both can choose to
support..." ?
> +a notification region defined in \field{struct
> virtio_pci_legacy_notify_region}.
> +
> +In \field{struct virtio_virtio_admin_cmd_legacy_notify_query_entry},
> +\field{region_data} is defined as following:
> +
> +\begin{lstlisting}
> +struct virtio_pci_legacy_notify_region {
> +u8 owner; /* When set to 1, notification region is of the owner
> device */
> +u8 bar;/* BAR of the member or owner device */
> +u8 padding[6];
> +le64 offset; /* Offset within bar. */
> +};
> +\end{lstlisting}
> +
> +The group owner device hardwires VF BAR0 to zero in the SR-IOV Extended
> +capability.
> +
> +The group member device does not use PCI BAR0 in all the Virtio PCI
> capabilities
"in [or for?] any of the...", I guess?
> +listed in section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus /
> Virtio Structure PCI Capabilities}.
> +
> +\devicenormative{\subsubsection}{Legacy Interface: Group Member Device
> Legacy Configuration Region Access}{Virtio Transport Options / Virtio Over
> PCI Bus / Legacy Interface: Group Member Device Configuration Region Access}
> +
> +When a PCI SR-IOV group owner device supports
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE commands, the group owner device MUST
> +hardwire VF BAR0 to zero in the SR-IOV Extended capability and the group
> memberi
s/memberi/member/
> +device MUST NOT use BAR0 in any of the Virtio Structure PCI Capabilities.
> +
> +The group owner device or the group member device or both MAY support driver
> +notifications region.
Make this "a driver notification region"?
> +
> +For the SR-IOV group type, the owner device supporting
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY
> +commands and its member device SHOULD follow the rules for the PCI Device ID,
> +Revision ID and Subsystem Device ID of the non-transitional devices
> documented in
> +section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device
> Discovery}.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v10 3/4] admin: Add group member legacy register access commands
On Thu, Jul 06 2023, Parav Pandit wrote:
> diff --git a/admin-cmds-legacy-interface.tex b/admin-cmds-legacy-interface.tex
> new file mode 100644
> index 000..dd01c0a
> --- /dev/null
> +++ b/admin-cmds-legacy-interface.tex
> @@ -0,0 +1,261 @@
> +\subsubsection{Legacy Interfaces}\label{sec:Basic Facilities of a Virtio
> Device / Device groups / Group
> +administration commands / Legacy Interface}
> +
> +In some systems, there is a need to support utilizing the legacy driver with
> +the device that do not directly support the legacy interface. In such
> scenarios,
Please use either Michael's or my suggestion from v9 here, using
definite articles here isn't a good idea.
> +a group owner device can provide the legacy interface functionality for the
> +group member devices. The driver of an owner device can then access the
> legacy
> +interface of a member device on behalf of the legacy member device driver.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v10 2/4] admin: Fix section numbering
On Thu, Jul 06 2023, Parav Pandit wrote: > Requirements are put one additional level down. Fix it. > > Signed-off-by: Parav Pandit > --- > changelog: > v9->v10: > - addressed comments from Cornelia > - fixed requirements for administration virtqueue section > v4->v5: > - new patch > --- > admin.tex | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Cornelia Huck - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [virtio-comment] [PATCH v9 0/4] admin: Access legacy registers using admin commands
On Wed, Jul 05 2023, Parav Pandit wrote: >> From: Michael S. Tsirkin >> Sent: Wednesday, July 5, 2023 7:09 AM >> >> OK I sent a bunch of comments. Pls try to address, unfortunately I didn't go >> in >> depth over the last patch, but I feel it should be squashed into the generic >> legacy command sections avoiding need for indirection, so rewriting it now >> wouldn't be efficient. > > I relooked at well yday. > Transport has very small section. > This aligns to how the existing spec is written, for example vq notify data > is explained in generic way and defined in pci way in PCI chapter. > The indirection is fine and also self-contained similarly here. Would it make sense to do this the other way around (i.e. use something that is PCI-only from the start?) IIUC, the "legacy" problem as it stands is something specific to PCI, with MMIO using two distinct approaches for pre-1.0 and 1.0+, and CCW simply using different commands (and no need for any kind of cross access.) Not 100% sure about MMIO, but I don't think we need a version of this for CCW. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v9 4/4] transport-pci: Introduce group legacy group member config region access
On Wed, Jul 05 2023, Parav Pandit wrote:
> diff --git a/admin-cmds-legacy-interface.tex b/admin-cmds-legacy-interface.tex
> index 09001d5..571b256 100644
> --- a/admin-cmds-legacy-interface.tex
> +++ b/admin-cmds-legacy-interface.tex
> @@ -150,7 +150,7 @@ \subsubsection{Legacy Interfaces}\label{sec:Basic
> Facilities of a Virtio Device
> by the device.
>
> Refer to the specific transport section for the definition of the
> -\field{region_data}.
> +\field{region_data}. For PCI transport refer to section \ref{sec:Virtio
> Transport Options / Virtio Over PCI Bus / Legacy Interface: Group Member
> Device Configuration Region Access}.
"For the PCI transport, refer to..."
>
> This command is currently only defined for the PCI SR-IOV group type.
>
(...)
> diff --git a/transport-pci-legacy-regs.tex b/transport-pci-legacy-regs.tex
> new file mode 100644
> index 000..e4f70c9
> --- /dev/null
> +++ b/transport-pci-legacy-regs.tex
> @@ -0,0 +1,41 @@
> +\subsection{Legacy Interface: Group member device Configuration Region
> Access}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / Legacy
> Interface: Group Member Device Configuration Region Access}
> +
> +The PCI owner device or the member device or both supports driver
> notifications using
s/supports/support/
> +a notification region defined in the \field{struct virtio_pci_notify_region}.
s/in the/in/
> +
> +In \field{struct virtio_virtio_admin_cmd_legacy_notify_query_entry},
> +\field{region_data} is defined as following:
> +
> +\begin{lstlisting}
> +struct virtio_pci_legacy_notify_region {
> +u8 owner; /* When set to 1, notification region is of the owner
> device */
> +u8 bar;/* BAR of the member or owner device */
> +u8 padding[6];
> +le64 offset; /* Offset within bar. */
> +};
> +\end{lstlisting}
> +
> +The group owner device hardwire VF BAR0 in the SR-IOV Extended capability.
s/hardwire/hardwires/
> +
> +The group member device does not use PCI BAR0 in various Virtio PCI
> capabilities
"in the various" caps? Or "in some of the" caps?
> +listed in section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus /
> Virtio Structure PCI Capabilities}.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v9 3/4] admin: Add group member legacy register access commands
On Wed, Jul 05 2023, Parav Pandit wrote:
> diff --git a/admin-cmds-legacy-interface.tex b/admin-cmds-legacy-interface.tex
> new file mode 100644
> index 000..09001d5
> --- /dev/null
> +++ b/admin-cmds-legacy-interface.tex
> @@ -0,0 +1,205 @@
> +\subsubsection{Legacy Interfaces}\label{sec:Basic Facilities of a Virtio
> Device / Device groups / Group
> +administration commands / Legacy Interface}
> +
> +In some systems, there is a need to support utilizing the legacy driver with
> +the device that do not directly support the legacy interface. In such
> scenarios,
Maybe better:
"In some systems, there is a need to support utilizing legacy drivers with
devices that do not directly support the legacy interface."
?
> +a group owner device can provide the legacy interface functionality for the
> +group member devices. The driver of an owner device can then access the
> legacy
> +interface of a member device on behalf of the legacy member device driver.
(...)
> +\paragraph{Legacy Driver Notification Query}\label{par:Basic Facilities of a
> Virtio Device / Device groups / Group administration commands / Legacy
> Interface / Legacy Driver Notifications Query}
> +
> +Even though the driver notifications can be communicated through the
> +administration command, if the group owner device or group member device
> +supports such notifications using a memory-mapped operation or I/O operation,
> +they are sent to the device by accessing such a notification region using
> +a memory or an I/O operation.
I find this sentence confusing, probably because it is too
complex... what about
"If the group owner device or the group member device support driver
notifications via a memory-mapped operation or I/O operation, these
notifications are sent to the device via accessing such a notification
region using a memory or I/O operation {in addition to|instead of}
communicating the notifications through the admistration command."
(I'm not sure whether it is either or both.)
> +
> +A group owner device optionally support querying driver notifications region
"can optionally support" ?
> +using VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY command.
> +
> +The driver sets command \field{opcode} to
> VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY.
> +This command does not use \field{command_specific_data}.
> +
> +When command completes successfully, \field{command_specific_result}
"the command"
> +uses following structure:
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v9 2/4] admin: Fix section numbering
On Wed, Jul 05 2023, Parav Pandit wrote: > Requirements are put one additional level down. Fix it. > > Signed-off-by: Parav Pandit > --- > changelog: > v4->v5: > - new patch > --- > admin.tex | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Same thing applies to the normative statements for the admin vq (in the same file) -- do you want to fix that as well? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v9 1/4] admin: Split opcode table rows with a line
On Wed, Jul 05 2023, Parav Pandit wrote: > Currently all opcode appears to be in a single row. > Separate them with a line similar to other tables. > > Signed-off-by: Parav Pandit > --- > changelog: > v2->v3: > - new patch > --- > admin.tex | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Cornelia Huck - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v21] virtio-net: support inner header hash
On Mon, Jul 03 2023, "Michael S. Tsirkin" wrote: > On Mon, Jul 03, 2023 at 11:27:11PM +0800, Heng Qi wrote: >> 1. Currently, a received encapsulated packet has an outer and an inner >> header, but >> the virtio device is unable to calculate the hash for the inner header. The >> same >> flow can traverse through different tunnels, resulting in the encapsulated >> packets being spread across multiple receive queues (refer to the figure >> below). >> However, in certain scenarios, we may need to direct these encapsulated >> packets of >> the same flow to a single receive queue. This facilitates the processing >> of the flow by the same CPU to improve performance (warm caches, less >> locking, etc.). >> >>client1client2 >> |+---+ | >> +--->|tunnels|<+ >>+---+ >> | | >> v v >> +-+ >> | monitoring host | >> +-+ >> >> To achieve this, the device can calculate a symmetric hash based on the >> inner headers >> of the same flow. >> >> 2. For legacy systems, they may lack entropy fields which modern protocols >> have in >> the outer header, resulting in multiple flows with the same outer header but >> different inner headers being directed to the same receive queue. This >> results in >> poor receive performance. >> >> To address this limitation, inner header hash can be used to enable the >> device to advertise >> the capability to calculate the hash for the inner packet, regaining better >> receive performance. >> >> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/173 >> Signed-off-by: Heng Qi >> Reviewed-by: Xuan Zhuo >> Reviewed-by: Parav Pandit > > Cornelia last minute comments before I start a vote? Please just go ahead. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [virtio 1.3] Feature freeze has started
As outlined in
https://lists.oasis-open.org/archives/virtio/202304/msg00036.html ff.,
as part of the release process for the 1.3 version of the virtio spec,
we have now entered feature freeze. This means:
***
By July 1st[3rd], all not yet integrated features that are targeting 1.3 need
to have at least a github issue open that is pointing to a patch (set)
on the list.
***
AFAICS, the list of features matching these criteria is the following:
https://github.com/oasis-tcs/virtio-spec/issues/173 ("virtio-net:
support inner header hash")
* this looks very close, with voting likely to start in the next
couple of days
https://github.com/oasis-tcs/virtio-spec/issues/167 ("Support
transitional device for PCIe VF")
* I have not followed that one in detail, but it has been stated that
this one should also be ready soon -- can people please confirm that
this will be ready for inclusion in July?
https://github.com/oasis-tcs/virtio-spec/issues/170 ("The size of
virtio_net_hdr struct are wrong when VIRTIO_NET_HASH_REPORT feature is
negotiated.")
* Can someone please check the status of this? Looks like a patch is
available, but no progress for a long time? Is this maybe interacting
with the inner header hash feature?
Any others that I have missed?
I'd like to wrap this up in July; not a real problem if we spill over
a couple of days into August, but I'd really like to avoid delaying the
process for too much (it will take long enough already.)
virtio 1.3 timeline:
Development
July 3rd: feature freeze (github issue <-- we are here
and change on list)
August 1st: change freeze (all voted
upon)
virtio-next development branch opens
August: prepare draft
August 31st (latest): draft voted upon by TC
September: public review period
October: addtl public review period, if needed
October (November): 1.3 released
merge virtio-next development branch
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [RFC] virtio 1.3 schedule
On Thu, Jun 29 2023, "Michael S. Tsirkin" wrote: > On Thu, Jun 29, 2023 at 05:41:34PM +, Parav Pandit wrote: >> Hi Cornelia, Michael, >> >> > From: virtio-comm...@lists.oasis-open.org > > open.org> On Behalf Of Cornelia Huck >> > Sent: Wednesday, April 12, 2023 12:17 PM >> >> > >> - July 1st 2023: enter feature freeze; all proposed changes must at >> > >> least have an open github issue that refers to a proposal on-list >> > >> - August 1st 2023: enter change freeze; everything needs to have been >> > >> voted upon (i.e. we can start with preparations for a release like >> > >> compiling the change log) >> > >> - at the same time, fork virtio-next, which can be used for development >> > >> while we're working on the release >> > >> - August/September 2023: prepare draft, initiate initial voting etc. >> > > >> > > Hmm looks like you forgot a 30 day the public review period (or is >> > > that included in the "etc"? ). During this period we will likely >> > > receive review comments which we have to address. Should the changes >> > > turn out to be material, another public review period will be required. >> > > Let's spell it out please. >> > >> > Yeah, that's what I meant with "etc"... let's make this: >> > >> > - August 2023: prepare draft, aim to have it ready and voted upon by >> > August 31 the latest (preferrably earlier) >> > - September 2023: public review period (30 days) >> > - October 2023: another public review period, if needed; otherwise, get >> > comittee specification ready and voted upon >> > - October (maybe November, if we're running late) 2023: virtio 1.3 >> > released >> > >> > The most important dates are feature freeze on Jul 1 and change freeze on >> > Aug >> > 1, I guess; for the rest, we depend on whether people are out on PTO, which >> > kind of feedback we get, etc. -- but as long as we release >> > 1.3 in 2023, I'll be happy enough. >> >> We have two extension features that has gone through several reviews and are >> in advance stage (and also late) for 1.3. :) >> >> 1. inner hash v19, rolling to v20 anytime this week. 1 Patch. >> [1] https://lists.oasis-open.org/archives/virtio-comment/202306/msg00490.html >> >> 2. first user of aq for legacy access at v7. >> 2 patches. First 2 are Fixes. >> [2] https://lists.oasis-open.org/archives/virtio-comment/202306/msg00491.html >> >> Both need few more days of review and vote. >> Shall we please have few days in July for them so that it can be part of >> 1.3? >> or whole July is needed to prepare the draft spec? > > I think it's not too bad, projects that do releases every couple of > months can afford to be strict, we do them ones a year and a half > it makes sense to be flexible. Yes, my only concern is to avoid slipping too badly -- and the "feature freeze" period was actually intended to give us some time to wrap up outstanding features. For the inner header hash, I see there's already an issue (#173), so it's good. I have not followed the aq legacy access feature -- if everyone agrees that we should be able to vote on it in July, we should have an issue for it as well (I cannot find one just by looking at titles.) Any other features that people are aware of? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [virtio 1.3] Reminder: Feature freeze coming up on July 1st
As outlined in https://lists.oasis-open.org/archives/virtio/202304/msg00036.html ff., we'll soon start the process of getting version 1.3 of the virtio spec released. As a first step, we'll enter feature freeze on July 1st. This means: *** By July 1st, all not yet integrated features that are targeting 1.3 need to have at least a github issue open that is pointing to a patch (set) on the list. *** This should hopefully give us enough time to conclude voting for those features before we enter change freeze on August 1st. virtio 1.3 timeline: Development <-- we are here July 1st: feature freeze (github issue and change on list) August 1st: change freeze (all voted upon) virtio-next development branch opens August: prepare draft August 31st (latest): draft voted upon by TC September: public review period October: addtl public review period, if needed October (November): 1.3 released merge virtio-next development branch - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification
>>> I also hope to be able to update virtio-video at a faster pace. Please >>> let me try. >> >> Please hold on a bit - there are two things here. >> >> 1) I'd like to settle the virtio-v4l2/virtio-video argument first to >> make sure we don't get two things clashing head-on. As far as codecs >> are concerned we certainly don't need both. Cornelia, I think we'll >> need you to make a call on this, or at least tell us what you need to >> make the call. If it helps I can send a draft of what the virtio-v4l2 >> spec would look like, it should be relatively short. >> >> 2) I (and other ChromeOS contributors) have been driving this spec so >> far and while I think virtio-v4l2 is a better solution, I have not >> said I would give up on virtio-video if virtio-v4l2 was not adopted >> and will keep iterating on it in that case. > > It actually very much looks like you gave up. I mean, you developed it > for years, and now you'd like to throw it away. Well, I meant something > like "please have some faith in my ability to update virtio-video at a > faster pace". I think I already have the permission to continue the > development. You know, OpenSynergy also spent some time on the > virtio-video. This includes the draft v1 version and the V4L2 driver. I > think this was kind of an informal agreement between us, that you do the > spec and we do the driver. It didn't work well enough since draft v4, I > think. Now as our interests are not aligned anymore, it is fine to > continue separately. I'm not going to wait until you change your mind on > virtio-v4l2. Obviously you're very busy with it right now. If I stop and > wait now, we won't have the video device in the 1.3 release for sure. I > think when we are ready to develop virtio-video together again, we'll > need a better agreement. Please, can we just calm down here? This thread is painful to read already, and pointing at each other is not going to help us to come up with something that is generally agreeable. In the end, I want _a_ spec, and I don't really have a horse in V4L2 vs. video race, but having to dig through all of this in the hope of moderating things is just impossible if you're arguing in circles, and much of it seemingly being a rehashing about who said/did something or not. > That said, I very much appreciate your efforts with the spec. It > definitely received a lot of improvements. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH] content: Replace guest OS with driver
On Tue, May 16 2023, "Michael S. Tsirkin" wrote:
> On Tue, May 16, 2023 at 10:24:12AM +0200, Cornelia Huck wrote:
>> On Tue, May 16 2023, "Michael S. Tsirkin" wrote:
>>
>> > On Tue, May 16, 2023 at 06:01:39AM +0300, Parav Pandit wrote:
>> >> diff --git a/content.tex b/content.tex
>> >> index 9df81b8..417d476 100644
>> >> --- a/content.tex
>> >> +++ b/content.tex
>> >> @@ -26,10 +26,10 @@ \section{\field{Device Status} Field}\label{sec:Basic
>> >> Facilities of a Virtio Dev
>> >> following bits are defined (listed below in the order in which
>> >> they would be typically set):
>> >> \begin{description}
>> >> -\item[ACKNOWLEDGE (1)] Indicates that the guest OS has found the
>> >> +\item[ACKNOWLEDGE (1)] Indicates that the driver has found the
>> >>device and recognized it as a valid virtio device.
>> >>
>> >> -\item[DRIVER (2)] Indicates that the guest OS knows how to drive the
>> >> +\item[DRIVER (2)] Indicates that the driver knows how to drive the
>> >>device.
>> >>\begin{note}
>> >> There could be a significant (or infinite) delay before setting
>> >
>> > Actually, there is a subtle difference here that this is losing.
>> > "guest OS" really refers to e.g. Linux virtio core code here.
>> >
>> >
>> > ACKNOWLEDGE and DRIVER are used by virtio core.
>> >
>> > ACKNOWLEDGE tells you virtio core attached to device, and DRIVER
>> > tells you core found a device specific driver.
>> >
>> >
>> >
>> > If you really want to make things better, let's find a way to explain
>> > all this.
>>
>> Agreed, this is a really old part of the spec, and likely had been
>> written with the Linux device probing sequence in mind.
>>
>> Basically, we want to distinguish between "something on the driver side
>> has discovered the device" and "something on the driver side knows how
>> to drive this specific device". If we consider "driver" as a catch-all
>> of the whole thing talking to a device, we need to be extra descriptive
>> (and we can add examples, as this is a non-normative section.)
>>
>> For ACKNOWLEDGE, maybe "indicates that the driver has discovered the
>> device and recognized it as a valid virtio device" (i.e. mostly what we
>> have now), but also add "For example, this can indicate that non-device
>> specific virtio driver code has attached to the device."
>>
>> For DRIVER, maybe "indicates that the driver has discovered that it
>> knows how to drive this device specifically. For example, this can
>> indicate that device-specific driver code has attached to the device."
>
>
> I feel examples are a weak way to document it - if we can not say
> what this is specifically, what purpose does it serve?
Not really documenting, but rather illustrating it.
> Actually, we do have a distinction, between transport and device type.
> Can't we use that? It seems more consistent than "non-device
> specific" and "device specific".
What do we consider to be the "transport"? {pci,mmio,ccw} + virtio ring,
or only {pci,mmio,ccw}? This might be confusiong, since at least in the
Linux case, it is the virtio ring/generic code that sets ACKNOWLEDGE,
while the pci/mmio/ccw code is not fiddling with the status at that
stage.
> SO I propose:
>
> \item[ACKNOWLEDGE (1)] Indicates that a transport driver has found the
> device and recognized it as a valid virtio device transport.
What is a "virtio device transport"?
>
> \item[DRIVER (2)] Indicates that a device type specific driver was found
> and will attempt to attach to the device.
>
>
>
> BTW somewhat related, I would maybe fix
> device-types/mem/description.tex:change
> not to say "device driver", just "driver" for brevity.
That one makes sense to me.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH] content: Replace guest OS with driver
On Tue, May 16 2023, "Michael S. Tsirkin" wrote:
> On Tue, May 16, 2023 at 06:01:39AM +0300, Parav Pandit wrote:
>> diff --git a/content.tex b/content.tex
>> index 9df81b8..417d476 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -26,10 +26,10 @@ \section{\field{Device Status} Field}\label{sec:Basic
>> Facilities of a Virtio Dev
>> following bits are defined (listed below in the order in which
>> they would be typically set):
>> \begin{description}
>> -\item[ACKNOWLEDGE (1)] Indicates that the guest OS has found the
>> +\item[ACKNOWLEDGE (1)] Indicates that the driver has found the
>>device and recognized it as a valid virtio device.
>>
>> -\item[DRIVER (2)] Indicates that the guest OS knows how to drive the
>> +\item[DRIVER (2)] Indicates that the driver knows how to drive the
>>device.
>>\begin{note}
>> There could be a significant (or infinite) delay before setting
>
> Actually, there is a subtle difference here that this is losing.
> "guest OS" really refers to e.g. Linux virtio core code here.
>
>
> ACKNOWLEDGE and DRIVER are used by virtio core.
>
> ACKNOWLEDGE tells you virtio core attached to device, and DRIVER
> tells you core found a device specific driver.
>
>
>
> If you really want to make things better, let's find a way to explain
> all this.
Agreed, this is a really old part of the spec, and likely had been
written with the Linux device probing sequence in mind.
Basically, we want to distinguish between "something on the driver side
has discovered the device" and "something on the driver side knows how
to drive this specific device". If we consider "driver" as a catch-all
of the whole thing talking to a device, we need to be extra descriptive
(and we can add examples, as this is a non-normative section.)
For ACKNOWLEDGE, maybe "indicates that the driver has discovered the
device and recognized it as a valid virtio device" (i.e. mostly what we
have now), but also add "For example, this can indicate that non-device
specific virtio driver code has attached to the device."
For DRIVER, maybe "indicates that the driver has discovered that it
knows how to drive this device specifically. For example, this can
indicate that device-specific driver code has attached to the device."
Probably need some more overhaul :) Not an editorial change in any case.
>> @@ -473,13 +473,13 @@ \section{Device Initialization}\label{sec:General
>> Initialization And Device Oper
>> \begin{enumerate}
>> \item Reset the device.
>>
>> -\item Set the ACKNOWLEDGE status bit: the guest OS has noticed the device.
>> +\item Set the ACKNOWLEDGE status bit: the driver has noticed the device.
>>
>> -\item Set the DRIVER status bit: the guest OS knows how to drive the device.
>> +\item Set the DRIVER status bit: the driver knows how to drive the device.
>
> besides the above, "drivers knows how to drive" sounds bad.
>
>> \item\label{itm:General Initialization And Device Operation /
>> Device Initialization / Read feature bits} Read device feature bits, and
>> write the subset of feature bits
>> - understood by the OS and driver to the device. During this step the
>> + understood by the driver to the device. During this step the
>
> Again the "the OS" here referred to core virtio (e.g. ring features).
> Less of a problem to remove but if we come up with
> a better terminology for ACKNOWLEDGE/DRIVER then I guess we can use it
> here, too.
Hm, I'm not sure how far we need to distinguish between generic and
device-specific features in this case. The "driver" as the whole entity
driving the device needs to decide on the subset; at this stage, it does
not really matter which parts of the driver code accepted which
feature. We probably want to be explicit that features are ring,
transport, and device features, though.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v17 03/11] content: Rename confusing queue_notify_data and vqn names
On Fri, May 05 2023, Parav Pandit wrote:
> diff --git a/content.tex b/content.tex
> index 9cf938c..327926f 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -405,8 +405,12 @@ \section{Driver Notifications} \label{sec:Basic
> Facilities of a Virtio Device /
> notification to the device.
>
> When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
> -this notification involves sending the
> -virtqueue index to the device (method depending on the transport).
> +this notification contains either a virtqueue index if
> +VIRTIO_F_NOTIF_CONFIG_DATA is not negotiated or device supplied virtqueue
> +notification config data if VIRTIO_F_NOTIF_CONFIG_DATA is negotiated.
> +
> +The notification method and suppling any such virtqueue notification config
> data
> +is transport specific.
Typo: s/suppling/supplying/ (trivial to fix on top)
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [PATCH] admin: Fix reference and table formation
On Fri, May 05 2023, Parav Pandit wrote: > This patch brings three fixes. > > 1. Opcode table has 3 columns, only two were enumerated. Due to this > pdf generation script stops. Fix it and also have resizeable description > column as it needs wrap. > > 2. Status description column content needs to wrap. Without it pdf >does not read good. Fix it by having resizeable description column. > > 3. Fix the broken link to the Device groups. > > This fix is for [1]. > > [1] > https://lore.kernel.org/virtio-comment/cover.1683301091.git@redhat.com/T/#t > > Signed-off-by: Parav Pandit > --- > admin.tex | 11 ++- > 1 file changed, 6 insertions(+), 5 deletions(-) Reviewed-by: Cornelia Huck Let's push this as an editorial change once the main series is merged. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification
On Wed, May 03 2023, Alex Bennée wrote: > Cornelia Huck writes: > >> On Fri, Apr 28 2023, Alexander Gordeev >> wrote: >> >>> On 27.04.23 15:16, Alexandre Courbot wrote: >>>> But in any case, that's irrelevant to the guest-host interface, and I >>>> think a big part of the disagreement stems from the misconception that >>>> V4L2 absolutely needs to be used on either the guest or the host, >>>> which is absolutely not the case. >>> >>> I understand this, of course. I'm arguing, that it is harder to >>> implement it, get it straight and then maintain it over years. Also it >>> brings limitations, that sometimes can be workarounded in the virtio >>> spec, but this always comes at a cost of decreased readability and >>> increased complexity. Overall it looks clearly as a downgrade compared >>> to virtio-video for our use-case. And I believe it would be the same for >>> every developer, that has to actually implement the spec, not just do >>> the pass through. So if we think of V4L2 UAPI pass through as a >>> compatibility device (which I believe it is), then it is fine to have >>> both and keep improving the virtio-video, including taking the best >>> ideas from the V4L2 and overall using it as a reference to make writing >>> the driver simpler. >> >> Let me jump in here and ask another question: >> >> Imagine that, some years in the future, somebody wants to add a virtio >> device for handling video encoding/decoding to their hypervisor. >> >> Option 1: There are different devices to chose from. How is the person >> implementing this supposed to pick a device? They might have a narrow >> use case, where it is clear which of the devices is the one that needs to >> be supported; but they also might have multiple, diverse use cases, and >> end up needing to implement all of the devices. >> >> Option 2: There is one device with various optional features. The person >> implementing this can start off with a certain subset of features >> depending on their expected use cases, and add to it later, if needed; >> but the upfront complexity might be too high for specialized use cases. >> >> Leaving concrete references to V4L2 out of the picture, we're currently >> trying to decide whether our future will be more like Option 1 or Option >> 2, with their respective trade-offs. >> >> I'm slightly biased towards Option 2; does it look feasible at all, or >> am I missing something essential here? (I had the impression that some >> previous confusion had been cleared up; apologies in advance if I'm >> misrepresenting things.) >> >> I'd really love to see some kind of consensus for 1.3, if at all >> possible :) > > I think feature discovery and extensibility is a key part of the VirtIO > paradigm which is why I find the virtio-v4l approach limiting. By > pegging the device to a Linux API we effectively limit the growth of the > device specification to as fast as the Linux API changes. I'm not fully > immersed in v4l but I don't think it is seeing any additional features > developed for it and its limitations for camera are one of the reasons > stuff is being pushed to userspace in solutions like libcamera: > > How is libcamera different from V4L2? > > We see libcamera as a continuation of V4L2. One that can more easily > handle the recent advances in hardware design. As embedded cameras have > developed, all of the complexity has been pushed on to the developers. > With libcamera, all of that complexity is simplified and a single model > is presented to application developers. Ok, that is interesting; thanks for the information. > > That said its not totally our experience to have virtio devices act as > simple pipes for some higher level protocol. The virtio-gpu spec says > very little about the details of how 3D devices work and simply offers > an opaque pipe to push a (potentially propriety) command stream to the > back end. As far as I'm aware the proposals for Vulkan and Wayland > device support doesn't even offer a feature bit but simply changes the > graphics stream type in the command packets. > > We could just offer a VIRTIO_VIDEO_F_V4L feature bit, document it as > incompatible with other feature bits and make that the baseline > implementation but it's not really in the spirit of what VirtIO is > trying to achieve. I'd not be in favour of an incompatible feature flag, either... extensions are good, but conflicting features is something that I'd like to avoid. So, given that I'd still prefer to have a single device: How well does the proposed virt
Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification
On Fri, Apr 28 2023, Alexander Gordeev wrote: > On 27.04.23 15:16, Alexandre Courbot wrote: >> But in any case, that's irrelevant to the guest-host interface, and I >> think a big part of the disagreement stems from the misconception that >> V4L2 absolutely needs to be used on either the guest or the host, >> which is absolutely not the case. > > I understand this, of course. I'm arguing, that it is harder to > implement it, get it straight and then maintain it over years. Also it > brings limitations, that sometimes can be workarounded in the virtio > spec, but this always comes at a cost of decreased readability and > increased complexity. Overall it looks clearly as a downgrade compared > to virtio-video for our use-case. And I believe it would be the same for > every developer, that has to actually implement the spec, not just do > the pass through. So if we think of V4L2 UAPI pass through as a > compatibility device (which I believe it is), then it is fine to have > both and keep improving the virtio-video, including taking the best > ideas from the V4L2 and overall using it as a reference to make writing > the driver simpler. Let me jump in here and ask another question: Imagine that, some years in the future, somebody wants to add a virtio device for handling video encoding/decoding to their hypervisor. Option 1: There are different devices to chose from. How is the person implementing this supposed to pick a device? They might have a narrow use case, where it is clear which of the devices is the one that needs to be supported; but they also might have multiple, diverse use cases, and end up needing to implement all of the devices. Option 2: There is one device with various optional features. The person implementing this can start off with a certain subset of features depending on their expected use cases, and add to it later, if needed; but the upfront complexity might be too high for specialized use cases. Leaving concrete references to V4L2 out of the picture, we're currently trying to decide whether our future will be more like Option 1 or Option 2, with their respective trade-offs. I'm slightly biased towards Option 2; does it look feasible at all, or am I missing something essential here? (I had the impression that some previous confusion had been cleared up; apologies in advance if I'm misrepresenting things.) I'd really love to see some kind of consensus for 1.3, if at all possible :) - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification
[I'm replying here, as that seems to be the last message in the thread, and my reply hopefully catches everyone interested here.] To do a very high level summary, we have (at least) two use cases for virtio-video, that unfortunately have quite different requirements. Both want to encode/decode video, but in different environments. - The "restricted" case: Priority is on security, and the attack surface should be kept as small as possible, for example, by avoiding unneded complexity in the interface. Fancy allocations and management should be avoided. The required functionality is also quite clearly defined. - The "feature-rich" case: Priority is on enabling features, and being able to re-use existing V4L2 support is considered a big plus. Both device and driver implementations will be implemented in a full OS environment, so all kind of helpers are already available. (This is not to say that one case does not care about functionality or security; it's mostly a case of different priorities and environments.) I had been hoping that it would be possible to find kind of a common ground between the two cases, but reading the thread, I'm not quite as hopeful anymore... if we really don't manage to find an approach to make the different requirements co-exist, a separate virtio-v4l2 device might be the way to go -- but I've not totally given up hope yet. Some remarks from my side: - I'm not totally convinced that counting words is always a good proxy for complexity -- an interface might be simple on paper, but if the actual implementation would need to be quite involved to get it right, we'd again have a lot of opportunity for mistakes. - How much of v4l2 does actually need to be in the device specification for a driver to make potentially good use of it? Sure, being able to directly map to v4l2 really gives a huge benefit, but is there a way to extract a subset that's not too complex, but can be easily wrapped for interfacing with v4l2? (Both interface and functionality wise.) Even if that means that a driver would need to implement some kind of shim, a layer that easily maps to v4l2 concepts would still be much easier to implement than one that needs to map two quite different interfaces. [I'm really relying on the good judgement of people familiar with the interfaces here :)] - To which extent does security need to be baked into the device specification? We should avoid footguns, and avoiding needless complication is also a good idea, but while every new functionality means more attack surface, it also enables more use cases. That tension is hard to resolve; how much of it can we alleviate by making things optional? I hope I have not muddied the waters here, but I'd really like to see an agreement on how to continue (with two different devices, if there is really no other way.) - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v1] device-types/multiple: replace queues with enqueues
On Thu, Apr 20 2023, Parav Pandit wrote: > Queue is a verb and noun both. Replacing it with enqueue avoids > ambiguity around plural queues noun vs verb; similar to virtio fs device > description. > > Acked-by: Michael S. Tsirkin > Signed-off-by: Parav Pandit > --- > changelog: > v0->v1: > - replace 'on' to 'to' > --- > device-types/blk/description.tex | 2 +- > device-types/gpio/description.tex | 4 ++-- > device-types/i2c/description.tex | 2 +- > device-types/scsi/description.tex | 2 +- > device-types/vsock/description.tex | 2 +- > 5 files changed, 6 insertions(+), 6 deletions(-) Thanks, pushed as editorial update. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH] device-types/multiple: replace queues with enqueues
On Tue, Apr 11 2023, Parav Pandit wrote:
> Queue is a verb and noun both. Replacing it with enqueue avoids
> ambiguity around plural queues noun vs verb; similar to virtio fs device
> description.
Personally, I don't think this patch adds much, but I don't really mind
it, either.
>
> Suggested-by: Michael S. Tsirkin
> Signed-off-by: Parav Pandit
> ---
> device-types/blk/description.tex | 2 +-
> device-types/gpio/description.tex | 4 ++--
> device-types/i2c/description.tex | 2 +-
> device-types/scsi/description.tex | 2 +-
> device-types/vsock/description.tex | 2 +-
> 5 files changed, 6 insertions(+), 6 deletions(-)
>
(...)
> diff --git a/device-types/vsock/description.tex
> b/device-types/vsock/description.tex
> index 105bb30..07909d6 100644
> --- a/device-types/vsock/description.tex
> +++ b/device-types/vsock/description.tex
> @@ -218,7 +218,7 @@ \subsubsection{Buffer Space Management}\label{sec:Device
> Types / Socket Device /
> \field{buf_alloc} and \field{fwd_cnt} fields.
>
> \subsubsection{Receive and Transmit}\label{sec:Device Types / Socket Device
> / Device Operation / Receive and Transmit}
> -The driver queues outgoing packets on the tx virtqueue and incoming packet
> +The driver enqueues outgoing packets on the tx virtqueue and incoming packet
"queues (...) on" reads natural to me, but I'd use "to" with
"enqueues"... can any native speakers comment?
> receive buffers on the rx virtqueue. Packets are of the following form:
>
> \begin{lstlisting}
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v13 03/11] content: Rename confusing queue_notify_data and vqn names
On Tue, Apr 18 2023, Halil Pasic wrote:
> On Mon, 17 Apr 2023 20:53:32 +
> Parav Pandit wrote:
>
>> > From: Halil Pasic
>> > Sent: Sunday, April 16, 2023 11:42 PM
> [..]
>> >
>> > > @@ -1053,9 +1059,9 @@ \subsubsection{Available Buffer
>> > > Notifications}\label{sec:Virtio Transport Option If
>> > VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated:
>> > > \begin{itemize}
>> > > \item If VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the
>> > > driver MUST use the -\field{queue_notify_data} value instead of the
>> > > virtqueue
>> > index.
>> > > +\field{queue_notify_id} value instead of the virtqueue index.
>> > > \item If VIRTIO_F_NOTIFICATION_DATA has been negotiated, the driver
>> > > MUST set the -\field{vqn} field to the \field{queue_notify_data} value.
>> > > +\field{vq_notify_id} field to the \field{queue_notify_id} value.
>> >
>> > Seems you did not catch these with the s/_id/_config_data/. By doing the
>> > replace one would get queue_notify_config_data, vq_notify_config_data and
>> > queue_notify_config_data respectively.
>> >
>> > But that still does not seem right, and the right answer depends on to
>> > what do
>> > we rename \field{vqn}.
>> >
>> vqn -> vq_notif_config_data in the notification structure.
>
> I think I got what you mean. I intend to double check it
> in v14.
>
>>
>> virtio_pci_common_cfg.queue_notify_data -> queue_notify_config_data.
>>
>
> I have no strong opinion about this one.
>
>> Better to rename
>>
>> VIRTIO_F_NOTIF_CONFIG_DATA to VIRTIO_F_NOTIFY_CONFIG_DATA.
>> Adding "Y" is not going to make it that long. Field names will have some
>> consistency.
>> WDYT?
>
> No strong opinion on this either. I agree with Michael that "NOTIF"
> likely stands for "notification", but I also see your point: having
> "notify" in the field names and "NOTIF" in the feature name isn't great
> either.
>
> IMHO VIRTIO_F_NOTIFY_CONFIG_DATA should remain aligned with
> VIRTIO_F_NOTIFICATION_DATA in one way or another. Thus way out would be
> to do something like:
> virtio_pci_common_cfg.queue_notify_data -> queue_notif_config_data
>
> I think as editors Connie and Michael should have the most say in this
> though.
Using "notif" throughout would probably be most consistent.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [virtio-dev] [PATCH v13 03/11] content: Rename confusing queue_notify_data and vqn names
On Tue, Apr 18 2023, Parav Pandit wrote: >> From: Halil Pasic >> Sent: Tuesday, April 18, 2023 7:40 AM >> >> IMHO VIRTIO_F_NOTIFY_CONFIG_DATA should remain aligned with >> VIRTIO_F_NOTIFICATION_DATA in one way or another. Thus way out would be >> to do something like: >> virtio_pci_common_cfg.queue_notify_data -> queue_notif_config_data >> >> I think as editors Connie and Michael should have the most say in this >> though. > > Are you ok with above name? I still need to read through the whole thing. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev][PATCH 2/2] virtio-spi: add the device specification
On Mon, Apr 17 2023, Haixu Cui wrote:
Meta comment: I think you want to send to virtio-comment (feel free to
keep virtio-dev on cc:).
> virtio-spi is a virtual SPI master and it allows a guset to operate and
> use the physical SPI master controlled by the host.
> ---
> device-types/spi/description.tex| 155
> device-types/spi/device-conformance.tex | 7 ++
> device-types/spi/driver-conformance.tex | 7 ++
> 3 files changed, 169 insertions(+)
> create mode 100644 device-types/spi/description.tex
> create mode 100644 device-types/spi/device-conformance.tex
> create mode 100644 device-types/spi/driver-conformance.tex
>
> diff --git a/device-types/spi/description.tex
> b/device-types/spi/description.tex
> new file mode 100644
> index 000..a68e5f4
> --- /dev/null
> +++ b/device-types/spi/description.tex
> @@ -0,0 +1,155 @@
> +\section{SPI Master Device}\label{sec:Device Types / SPI Master Device}
> +
> +virtio-spi is a virtual SPI(Serial Peripheral Interface) master and it
> allows a
Nit: missing blank after "SPI"
s/it//
> +guest to operate and use the physical SPI master devices controlled by the
> host.
> +By attaching the host SPI controlled nodes to the virtual SPI master device,
> the
> +guest can communicate with them without changing or adding extra drivers for
> these
> +controlled SPI devices.
Can we rewrite this paragraph without relying on the host/guest
terminology? Does
"allows a driver to operate and use the physical SPI master devices
controlled by the device"
capture the essence of what this is doing?
I don't quite understand the second sentence, maybe someone familiar
with SPI can come up with something.
> +
> +In a typical host and guest architecture with Virtio SPI, Virtio SPI driver
> +is the front-end and exists in the guest kernel, Virtio SPI device acts as
s/guest kernel/guest/ ?
> +the back-end and exists in the host. And VirtQueue is used for communication
> +between the front-end and the back-end.
"A virtqueue is used for communication between the front-end and the
back-end." ?
[I *think* "virtqueue" is the term we've agreed on?]
> +
> +\subsection{Device ID}\label{sec:Device Types / SPI Master Device / Device
> ID}
> +45
> +
> +\subsection{Virtqueues}\label{sec:Device Types / SPI Master Device /
> Virtqueues}
> +
> +\begin{description}
> +\item[0] requestq
> +\end{description}
> +
> +\subsection{Feature bits}\label{sec:Device Types / SPI Master Device /
> Feature bits}
> +
> +None.
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / SPI Master
> Device / Device configuration layout}
> +
> +All fields of this configuration are always available and read-only for the
> Virtio SPI driver.
> +
> +\begin{lstlisting}
> +struct virtio_spi_config {
> +u32 bus_num;
> +u32 chip_select_max_number;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{bus_num}] is the physical spi master assigned to guset, and
> this is SOC-specific.
Maybe better "the physical SPI master controled by the device"?
> +
> +\item[\field{chip_select_max_number}] is the number of chipselect the
> physical spi master supported.
> +\end{description}
> +
> +\subsection{Device Initialization}\label{sec:Device Types / SPI Master
> Device / Device Initialization}
> +
> +\begin{itemize}
> +\item The Virtio SPI driver configures and initializes the virtqueue.
> +
> +\item The Virtio SPI driver allocates and registers the virtual SPI master.
What does this mean? Shouldn't we rather only require the driver to set
up the virtqueue and leave details on how to operate beyond the device
<-> driver interface to the implementation?
> +\end{itemize}
> +
> +\subsection{Device Operation}\label{sec:Device Types / SPI Master Device /
> Device Operation}
> +
> +\subsubsection{Device Operation: Request Queue}\label{sec:Device Types / SPI
> Master Device / Device Operation: Request Queue}
> +
> +The Virtio SPI driver queues requests to the virtqueue, and they are used by
> the
> +Virtio SPI device. Each request represents one SPI transfer and it is of the
> form:
> +
> +\begin{lstlisting}
> +struct virtio_spi_transfer_head {
> +u32 mode;
> +u32 freq;
> +u32 word_delay;
> +u8 slave_id;
> +u8 bits_per_word;
> +u8 cs_change;
> +u8 reserved;
> +};
> +\end{lstlisting}
[Side note: it seems the master/slave terminology is baked into SPI and
I assume we cannot avoid it?]
> +
> +\begin{lstlisting}
> +struct virtio_spi_transfer_end {
> +u8 status;
> +};
> +\end{lstlisting}
> +
> +\begin{lstlisting}
> +struct virtio_spi_req {
> +struct virtio_spi_transfer_head head;
> +u8 *rx_buf;
> +u8 *tx_buf;
> +struct virtio_spi_transfer_end end;
> +};
> +\end{lstlisting}
> +
> +The \field{mode} defines the SPI transfer mode.
> +
> +The \field{freq} defines the SPI transfer speed in Hz.
> +
> +The \field{word_delay} defines how long to wait between words within one SPI
> transfer,
> +in ns
Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification
On Mon, Apr 17 2023, Alexander Gordeev wrote: > Hi Alexandre, > > Thanks for you letter! Sorry, it took me some time to write an answer. > > First of all I'd like to describe my perspective a little bit because it > seems, that in many cases we (and other people writing their feedbacks) > simply have very different priorities and background. Thank you for describing the environment you want to use this in, this helps to understand the different use cases. > > OpenSynergy, the company that I work for, develops a proprietary > hypervisor called COQOS mainly for automotive and aerospace domains. We > have our proprietary device implementations, but overall our goal is to > bring open standards into these quite closed domains and we're betting > big on virtio. The idea is to run safety-critical functions like cockpit > controller alongside with multimedia stuff in different VMs on the same > physical board. Right now they have it on separate physical devices. So > they already have maximum isolation. And we're trying to make this > equally safe on a single board. The benefit is the reduced costs and > some additional features. Of course, we also need features here, but at > the same time security and ease of certification are among the top of > our priorities. Nobody wants cars or planes to have security problems, > right? Also nobody really needs DVB and even more exotic devices in cars > and planes AFAIK. > > For the above mentioned reasons our COQOS hypervisor is running on bare > metal. Also memory management for the guests is mostly static. It is > possible to make a shared memory region between a device and a driver > managed by device in advance. But definitely no mapping of random host > pages on the fly is supported. > > AFAIU crosvm is about making Chrome OS more secure by putting every app > in its own virtualized environment, right? Both the host and guest are > linux. In this case I totally understand why V4L2 UAPI pass-through > feels like a right move. I guess, you'd like to make the switch to > virtualized apps as seemless as possible for your users. If they can't > use their DVBs anymore, they complain. And adding the virtualization > makes the whole thing more secure anyway. So I understand the desire to > have the range of supported devices as broad as possible. It is also > understandable that priorities are different with desktop > virtualization. Also I'm not trying to diminish the great work, that you > have done. It is just that from my perspective this looks like a step in > the wrong direction because of the mentioned concerns. So I'm going to > continue being a skeptic here, sorry. > > Of course, I don't expect that you continue working on the old approach > now as you have put that many efforts into the V4L2 UAPI pass-through. > So I think it is best to do the evolutionary changes in scope of virtio > video device specification, and create a new device specification > (virtio-v4l2 ?) for the revolutionary changes. Then I'd be glad to > continue the virtio-video development. In fact I already started making > draft v7 of the spec according to the comments. I hope it will be ready > for review soon. > > I hope this approach will also help fix issues with virtio-video spec > and driver development misalignment as well as V4L2 compliance issues > with the driver. I believe the problems were caused partly by poor > communication between us and by misalignment of our development cycles, > not by the driver complexity. > > So in my opinion it is OK to have different specs with overlapping > functionality for some time. My only concern is if this would be > accepted by the community and the committee. How the things usually go > here: preferring features and tolerating possible security issues or the > other way around? Also how acceptable is having linux-specific protocols > at all? My main question is: What would be something that we can merge as a spec, that would either cover the different use cases already, or that could be easily extended to cover the use cases it does not handle initially? For example, can some of the features that would be useful in crosvm be tucked behind some feature bit(s), so that the more restricted COQOS hypervisor would simply not offer them? (Two feature bits covering two different mechanisms, like the current approach and the v4l2 approach, would also be good, as long as there's enough common ground between the two.) If a staged approach (adding features controled by feature bits) would be possible, that would be my preferred way to do it. Regarding the protocol: I think Linux-originating protocols (that can be implemented on non-Linux setups) are fine, Linux-only protocols probably not so much. > > Also I still have concerns about memory management with V4L2 UAPI > pass-through. Please see below. > > On 17.03.23 08:24, Alexandre Courbot wrote: >> Hi Alexander, >> >> On Thu, Mar 16, 2023 at 7:13 PM Alexander Gordeev >> wrote: >>> Hi Alexandre,
Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification
On Mon, Apr 17 2023, Alexander Gordeev wrote: > Hello Cornelia, > > On 17.04.23 14:56, Cornelia Huck wrote: >> On Sat, Apr 15 2023, Alexandre Courbot wrote: >> >>> If nobody strongly objects, I think this can be pushed a bit more >>> officially. Cornelia, would you consider it for inclusion if I >>> switched the next version of the specification to use V4L2 as the >>> host/guest protocol? This may take some more time as I want to confirm >>> the last details with code, but it should definitely be faster to >>> merge and to test with a real implementation than our previous >>> virtio-video attempts. >> Yes, please do post a new version of this spec; I agree that an existing >> implementation is really helpful here. >> >> [I have proposed July 1st as a "freeze" date for new features for 1.3, >> with August 1st as an "everything must be in" date; I'd really like >> virtio-video to be a part of 1.3, if possible :)] > > I sent an email minutes ago with an alternative plan. I'd like to > volunteer to continue the evolutionary changes to virtio-video. I'm > already working on the v7 draft. I think it will be available next week. > Then I'll focus on making the driver V4L2 compliant. I think all of this > is achievable by July 1st. At least the spec part. I think the > revolutionary changes should be in a separate namespace. The rationale > is in the longer email. WDYT? Seems our mails crossed mid-air... I'll go ahead and read your mail (and probably answer there.) My ultimate goal is to have a spec everyone is happy with, regardless on how we arrive there. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification
On Sat, Apr 15 2023, Alexandre Courbot wrote: > If nobody strongly objects, I think this can be pushed a bit more > officially. Cornelia, would you consider it for inclusion if I > switched the next version of the specification to use V4L2 as the > host/guest protocol? This may take some more time as I want to confirm > the last details with code, but it should definitely be faster to > merge and to test with a real implementation than our previous > virtio-video attempts. Yes, please do post a new version of this spec; I agree that an existing implementation is really helpful here. [I have proposed July 1st as a "freeze" date for new features for 1.3, with August 1st as an "everything must be in" date; I'd really like virtio-video to be a part of 1.3, if possible :)] - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Participation (was: Re: [virtio-comment] RE: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names)
On Mon, Apr 17 2023, "Michael S. Tsirkin" wrote: > I am not saying don't give feedback but I'm saying please help us > all be more organized, feedback time really should be within a > day or two, in rare cases up to a week. Given that there are things like weekends, public holidays, etc. one day does not look reasonable; while it certainly makes sense to continue if no feedback is forthcoming for a few days, not accounting for the fact that this is not the exclusive job for most/any of us is just a fast track to either burnout or people dropping out of virtio standardization altogether. > And I'd like to remind everyone if you are going away you are supposed > to report a leave of absence. Well... "Each request shall be made by sending an email to the TC mailing list at least 7 days before the Leave is to start." is probably not going to work for many cases. (Also, in any other community I'm participating in it is expected that you just might not be there or have time to work on it every week -- I've always seen that leave of absence thingy as something for a really long vacation or for something like parental leave, for which the max 45 days is really too short...) Not to mention that it applies to voting, not to participation on the lists. > TC's that have meetings just take away voting rights from someone who > does not attend two meetings in a row. We do it by ballot so this does > not apply, but I think we should set some limits in group's bylaws, > anyway. Ideas on formalizing this? If not we can just have informal > guidelines. There's of course a flip side to this. Some patches > seemingly go through two versions a day. Keeping up becomes a full time > job. We'd need a guideline for that, too. What do we actually expect from TC members? "Reply to emails" is not part of any formal requirement AFAICS (and not all TC members do participate on the lists on a regular basis anyway). The requirement is only to vote on the ballots, and there you're completely free to vote "abstain", so you can always squeeze in voting even if you're not able to participate otherwise. I think that's fine. "If I don't get a reply after $NUMBER_OF_WORKING_DAYS, I'll assume I can proceed as I think fit" is a reasonable assumption to make, e.g. to request a vote. Not sure if/how to formalize that. Also, how is this supposed to work if the original author doesn't reply to comments? Should the proposal be considered abandoned? I agree that patch reposting is happening much too fast in some cases. Not sure how to formalize that, either. Can we please just be more mindful of that? Reviewing time is not free. If I'm trying to do a timely review of something and constantly see new versions while I'm not finished yet, I do not feel like my feedback is actually valued. > I also feel high latency is one of the reasons people are beginning to > ask to split into subcommitees where they won't have to deal with this > kind of thing. Let's try to keep the latency low, please. I think there's multiple things to unpack here. - The very common strain of limited reviewer time. This seems to be an issue nearly everywhere. Encouraging more review helps; but if review and ensuing discussing turns into a time sink, it just cannot be handled at a reasonable activity level anymore. - Latency due to missing feedback. Can be solved by just requesting a vote if no feedback is forthcoming in a reasonable time frame. - Latency due to missing reaction to feedback. This means the proposal just doesn't make any progress. The onus is on the submitter here. - Conflicting approaches favoured by different people. This cannot be resolved in a formal way; either people need to be convinced that a certain approach will work, a middle ground found, or a way worked out that the different approaches can co-exist. In any case, this usually means long discussions which can be very frustrating, but unless we want to bulldoze over some people this is something we'll have to live with. [Personally, I think this is the worst contributor to frustration, and not something that can be solved by subcommittees.] - [I'm also not happy with the tone of some emails I've been seeing. I won't point to them in order not to stir up things that have already calmed down again.] - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [RFC] virtio 1.3 schedule
On Fri, Apr 07 2023, "Michael S. Tsirkin" wrote: > On Wed, Apr 05, 2023 at 03:57:09PM +0200, Cornelia Huck wrote: >> That said, let me propose the following timeline: >> >> - July 1st 2023: enter feature freeze; all proposed changes must at >> least have an open github issue that refers to a proposal on-list >> - August 1st 2023: enter change freeze; everything needs to have been >> voted upon (i.e. we can start with preparations for a release like >> compiling the change log) >> - at the same time, fork virtio-next, which can be used for development >> while we're working on the release >> - August/September 2023: prepare draft, initiate initial voting etc. > > Hmm looks like you forgot a 30 day the public review period (or is that > included in the "etc"? ). During this period we will likely receive > review comments which we have to address. Should the changes turn out to > be material, another public review period will be required. > Let's spell it out please. Yeah, that's what I meant with "etc"... let's make this: - August 2023: prepare draft, aim to have it ready and voted upon by August 31 the latest (preferrably earlier) - September 2023: public review period (30 days) - October 2023: another public review period, if needed; otherwise, get comittee specification ready and voted upon - October (maybe November, if we're running late) 2023: virtio 1.3 released The most important dates are feature freeze on Jul 1 and change freeze on Aug 1, I guess; for the rest, we depend on whether people are out on PTO, which kind of feedback we get, etc. -- but as long as we release 1.3 in 2023, I'll be happy enough. > > > >> - by September/October 2023: have the final 1.3 spec ready, voted upon, >> and released >> - virtio-next can be merged into mainline, and we continue with business >> as usual >> >> Thoughts? We might want to move things a tad earlier, but I don't think >> we should push the schedule out much further. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] [PATCH] introduction: Add missing helping verb
On Tue, Apr 11 2023, Parav Pandit wrote:
> The terminology of Transitional device and driver is missing the helping
> verb 'is' similar to other terminologies.
>
> Hence, add them to complete the sentence.
>
> Signed-off-by: Parav Pandit
> ---
> introduction.tex | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/introduction.tex b/introduction.tex
> index 287c5fc..e8b34e3 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -145,14 +145,14 @@ \subsection{Legacy Interface:
> Terminology}\label{intro:Legacy
>
> \begin{description}
> \item[Transitional Device]
> -a device supporting both drivers conforming to this
> +is a device supporting both drivers conforming to this
> specification, and allowing legacy drivers.
> \end{description}
>
> Similarly, a driver MAY implement:
> \begin{description}
> \item[Transitional Driver]
> -a driver supporting both devices conforming to this
> +is a driver supporting both devices conforming to this
> specification, and legacy devices.
> \end{description}
I'd rather not add the "is" (and drop the trailing ".", as this is not
supposed to be a sentence), as this would read awkwardly. I think it is
intended to be:
MAY implement:
which does not really ask for a sentence.
Same thing applies to the section right above, where it talks about
legacy support; I'd drop the "is" there instead.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v14] virtio-net: support the virtqueue coalescing moderation
On Tue, Apr 11 2023, "Michael S. Tsirkin" wrote: > On Tue, Apr 11, 2023 at 10:05:11AM +0200, Cornelia Huck wrote: >> On Thu, Mar 23 2023, Heng Qi wrote: >> >> > Hi all! If after reviewing it you feel it meets the necessary >> > requirements, I kindly request a vote. >> > >> > Thanks a lot! >> > >> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/166 >> >> So this has been voted upon and accepted, but we have further discussion >> downthread. >> >> Should we merge the voted-upon version now and do further changes on >> top, or wait for the discussion to conclude first so that we can merge >> all of it in one go? > > I think we should merge as is. vqn/idx/id are not new discussions > and have been going on before voting took place, so the TC > was informed. Parav is working on making tree-wide changes > around vq index applying this will not make his work > measureably harder. I had not looked at the discussion in detail and was unsure whether it was covering something new... I went ahead and applied it. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [virtio-dev] RE: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names
On Fri, Apr 07 2023, "Michael S. Tsirkin" wrote: > On Wed, Apr 05, 2023 at 03:58:55PM +, Parav Pandit wrote: >> For sure "cookie" is better than "config_data" and I don't have objection to >> "cookie". >> >> But I disagree to the claim that "identifier" is less good than "cookie". >> >> It is pointless debate of "identifier" vs "cookie". >> >> The union format is very useful to describe this crisply, I will use it. > > I guess I'm fine with "cookie" in that in CS it is by now widely > understood to mean "some opaque data". > identifier comes from "identical", so implies a 1:1 mapping IMO. Agreed, a "cookie" is not the same as an "identifier", although it may contain one. > > > The logic behind using a cookie is that it's a bit similar > to host cookie from ccw. > However, with ccw host cookie is used unconditionally, as > opposed to depending on the flag. > > > >> Do you prefer to rename F_CONFIG_DATA To F_CONFIG_COOKIE? > > It should all be consistent, but I worry about ccw which uses cookies > unconditionally. I am fine with leaving it as F_CONFIG_DATA for now > unless we see a good way to avoid confusion for ccw. Yes, please leave it as F_CONFIG_DATA, as we're just putting some "data" there in the end (and F_CONFIG_COOKIE might indeed be confusing for the ccw case.) - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 08/11] transport-pci: Introduce virtio extended capability
On Mon, Apr 10 2023, "Michael S. Tsirkin" wrote: > On Mon, Apr 10, 2023 at 07:57:08PM +, Parav Pandit wrote: >> >> > From: Michael S. Tsirkin >> > Sent: Monday, April 10, 2023 3:49 PM >> >> > Attribution is nice but Signed-off-by is not that. >> >> Then what is Signed-off-by for virtio spec? > > we never defined it. using it kind of by > >> Can it be same definition as what Linux kernel and many other projects use >> like [1]? >> >> [1] >> https://www.kernel.org/doc/html/latest/process/submitting-patches.html?highlight=signed%20off > > That is DCO. useful for linux pointless for us as is since > we want people to agree to our IPR. > Unless you want to make DCO refer to our IPR. > That might be possible. > We will need to float this past OASIS stuff. > I prefer explicit agreement to license personally. In most projects, s-o-b either means "I abide by the DCO", or "I'm adding this because it seems to be the usual thing to do". "Agreeing to the IPR" would be a somewhat surprising meaning, so I'd prefer to keep IPR agreement separate as well. Not sure if we want to actually specify what we mean by s-o-b; when I'm attaching it when merging I'm using it mostly in the "let's record the chain" meaning. (Specifying that we indeed use it in that sense, and not the full DCO sense, might make sense -- or it might be overkill.) > >> > And fundamentally people go read the PDF where the Signed-off-by does not >> > appear at all no one pokes at git history. >> When people read PDF, they do not care about the sign-off. Signed-off-by is >> not for that. >> >> > Let's just do: >> > >> > Thanks-to: name >> > >> Why to now learn a new term? >> Why terminology of [1] is not enough like AQ status codes? :) > > I have no idea what problem you are trying to address. > If it is attribution Signed-off-by is not that. > If it is IPR Signed-off-by is not that either but might be > made to imply that. Yes, s-o-b is distinct from attribution (and that's why the Linux kernel requires it on top of a Co-developed-by: to satisfy the DCO, not instead of it.) If we think that Co-developed-by: without a s-o-b might be confusing, a Thanks-to: could be a better term (and also a broader one meaning "I discussed this with this person, and I want to acknowledge them".) - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v14] virtio-net: support the virtqueue coalescing moderation
On Thu, Mar 23 2023, Heng Qi wrote: > Hi all! If after reviewing it you feel it meets the necessary > requirements, I kindly request a vote. > > Thanks a lot! > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/166 So this has been voted upon and accepted, but we have further discussion downthread. Should we merge the voted-upon version now and do further changes on top, or wait for the discussion to conclude first so that we can merge all of it in one go? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] [RFC] virtio 1.3 schedule
The virtio 1.2 spec has been released on July 1st, 2022, so I think we should come up with a plan for the 1.3 release. Given that it took us several months last time, we need to think about declaring freeze during (northern hemisphere) summer if we want to have a release ready before the end-of-year holiday season starts. We have some things currently under discussion/review that we should still consider for 1.3: - vq index etc. naming cleanup - admin vq - virtio-video - some things we deferred for 1.2 (see https://github.com/oasis-tcs/virtio-spec/issues?q=is%3Aissue+is%3Aopen+label%3Adeferred-in-v1.2-csprd01) - other things that are not on my personal list (please mention whatever you think is important) Please also keep in mind that 1.3 will not be the last virtio release, and many projects like Linux or QEMU consider features stable and ready for inclusion once the spec has hit the git tree, but not neccessarily a virtio spec release. That said, let me propose the following timeline: - July 1st 2023: enter feature freeze; all proposed changes must at least have an open github issue that refers to a proposal on-list - August 1st 2023: enter change freeze; everything needs to have been voted upon (i.e. we can start with preparations for a release like compiling the change log) - at the same time, fork virtio-next, which can be used for development while we're working on the release - August/September 2023: prepare draft, initiate initial voting etc. - by September/October 2023: have the final 1.3 spec ready, voted upon, and released - virtio-next can be merged into mainline, and we continue with business as usual Thoughts? We might want to move things a tad earlier, but I don't think we should push the schedule out much further. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v12 03/10] content: Rename confusing queue_notify_data and vqn names
On Wed, Apr 05 2023, "Michael S. Tsirkin" wrote:
> On Wed, Apr 05, 2023 at 04:06:50AM +0300, Parav Pandit wrote:
>> diff --git a/content.tex b/content.tex
>> index cd93db2..d5f8026 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -405,8 +405,18 @@ \section{Driver Notifications} \label{sec:Basic
>> Facilities of a Virtio Device /
>> notification to the device.
>>
>> When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
>> -this notification involves sending the
>> -virtqueue index to the device (method depending on the transport).
>> +this notification involves sending only the 16-bit virtqueue notification
>> +identifier (notification method depends on the transport).
>> +
>> +\begin{itemize}
>> +\item When VIRTIO_F_NOTIF_CONFIG_DATA is not negotiated, virtqueue
>> +notification identifier is a 16-bit vq index.
>> +
>> +\item When VIRTIO_F_NOTIF_CONFIG_DATA is negotiated, virtqueue
>> +notification identifier is a device supplied virtqueue identifier. A method
>> +to supply such virtqueue notification identifier is transport
>> +specific.
>> +\end{itemize}
>>
>> However, some devices benefit from the ability to find out the
>> amount of available data in the queue without accessing the virtqueue in
>> memory:
>
>
> VIRTIO_F_NOTIF_CONFIG_DATA is such a narrow usecase, I don't like
> burning "vq identifier" on this. How about we just say something
> along the lines of:
>
>
> When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, this
> notification involves sending either the virtqueue index or the
> virtqueue config data to the device (method depending on the
> transport).
>
> And then "the data sent is a device supplied virtqueue config data".
Agreed, referring to "virtqueue configuration data" or somesuch is
better than using up the "vq identifier" name for it.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v12 02/10] content.tex Replace virtqueue number with index
On Wed, Apr 05 2023, Parav Pandit wrote: > Replace virtqueue number with index to align to rest of the > specification. > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163 > Signed-off-by: Parav Pandit > > --- > changelog: > v11->v12: > - new patch > --- > content.tex | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Cornelia Huck - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] [PATCH v10 0/8] Rename queue index to queue number
On Tue, Apr 04 2023, Halil Pasic wrote: > On Tue, 4 Apr 2023 03:07:26 -0400 > "Michael S. Tsirkin" wrote: > >> > 3. Once interrupt moderation series is merged, rename vqn to vq_index. >> > (really vqn reads better even though history say vq index). >> > Because votes is completed and voting period ended for the important >> > feature. >> >> interrupt moderation voting is still ongoing, isn't it? > > Yes it is. But not for very long. Close date is today. I > won't change my vote because of the index vs number discussion. I believe > we can include that patch in the cleanup if push comes to shove. But > I wouldn't have anything against that ballot getting withdrawn either. > I'm Cc-ing Heng Qi. ...it has actually closed right now. But I don't really see including it as a show stopper. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 08/11] transport-pci: Introduce virtio extended capability
On Tue, Apr 04 2023, "Michael S. Tsirkin" wrote: > On Tue, Apr 04, 2023 at 09:54:55AM +0200, Cornelia Huck wrote: >> On Tue, Apr 04 2023, "Michael S. Tsirkin" wrote: >> >> > On Fri, Mar 31, 2023 at 01:58:31AM +0300, Parav Pandit wrote: >> >> PCI device configuration space for capabilities is limited to only 192 >> >> bytes shared by many PCI capabilities of generic PCI device and virtio >> >> specific. >> >> >> >> Hence, introduce virtio extended capability that uses PCI Express >> >> extended capability. >> >> Subsequent patch uses this virtio extended capability. >> >> >> >> Co-developed-by: Satananda Burla >> > >> > What does it "Co-developed-by" mean exactly that Signed-off-by >> > does not? >> >> AIUI, "Co-developed-by:" is more akin to a second "Author:", i.e. to >> give attribution. (Linux kernel rules actually state that any >> "Co-developed-by:" must be followed by a "Signed-off-by:" for that >> author, but we don't really do DCO here, the S-o-b is more of a >> convention.) > > > Actually, we might want to generally, to signify agreement to the IPR. > How about adding this to our rules? But in this case Satananda Burla is > a TC member so yes, no problem. Adding a s-o-b requirement is not a bad idea... do you want to propose an update? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [virtio-comment] [PATCH 0/3] transport-pci: msix summary update
On Mon, Apr 03 2023, Parav Pandit wrote: > Hi Cornelia, > >> From: Xuan Zhuo >> Sent: Monday, March 27, 2023 2:09 AM >> Series: >> >> Reviewed-by: Xuan Zhuo >> > > It is editorial change rewording the text. > Can you please take this short series forward? Patch 1 (which Michael has already pushed) is editorial; for the other two patches, I'm not that sure, especially as there still seems to be some discussion? - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [PATCH v10 8/8] virtio-net: Describe RSS using rss rq id
On Mon, Apr 03 2023, Parav Pandit wrote:
>> From: Cornelia Huck
>> Sent: Thursday, March 30, 2023 5:17 AM
>> > +Field \field{indirection_table} is an array of receive virtqueues.
>>
>> "an array of receive virtqueues identified via their rss_rq_id" ?
> No need to make it this verbose. It is evident from the definition itself.
>
>>
>> >
>> > A driver sets \field{max_tx_vq} to inform a device how many transmit
>> virtqueues it may use (transmitq1\ldots transmitq \field{max_tx_vq}).
>> >
>> > @@ -1455,7 +1465,8 @@ \subsubsection{Control
>> > Virtqueue}\label{sec:Device Types / Network Device / Devi
>> >
>> > A driver MUST NOT send the VIRTIO_NET_CTRL_MQ_RSS_CONFIG command
>> if the feature VIRTIO_NET_F_RSS has not been negotiated.
>> >
>> > -A driver MUST fill the \field{indirection_table} array only with indices
>> > of
>> enabled queues. Index 0 corresponds to receiveq1.
>> > +A driver MUST fill the \field{indirection_table} array only with
>> > +enabled receive virtqueues.
>>
>> "only with rss_rq_id references to enabled receive virtqueues" ?
>>
> In this field and other fields, we just refer to the receive virtqueues as
> rss_rq_id parent structure itself is describing what it is.
> Hence, we omit re-iterating rss_rq_id at multiple places.
Personally, I'd prefer to spell it out, as it is a specific way to refer
to a virtqueue.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
