[virtio-dev] Re: [virtio-comment] RE: [PATCH v19] virtio-net: support inner header hash
On Thu, Jun 29, 2023 at 01:56:34AM +, Parav Pandit wrote: > > > > From: Michael S. Tsirkin > > Sent: Wednesday, June 28, 2023 3:45 PM > > > > Maybe I get it. You want to use the new features as a carrot to > > > > force drivers to implement DMA? You suspect they will ignore the > > > > spec requirement just because things seem to work? > > > > > > > Right because it is not a must normative. > > > > Well SHOULD also does not mean "ok to just ignore". > > > > This word, or the adjective "RECOMMENDED", mean that there > >may exist valid reasons in particular circumstances to ignore a > >particular item, but the full implications must be understood and > >carefully weighed before choosing a different course. > > > RECOMMENDED and SHOULD forces the device to support MMIO, which is not good. You said this already. Let's not repeat ourselves please. It's a single word. We need to work on a patch, we can argue about small detail once it's posted. It's not in your list of plans so I am guessing we need someone on Red Hat side to work on this? -- MST - 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 v19] virtio-net: support inner header hash
> From: Heng Qi
> Sent: Thursday, June 29, 2023 2:41 AM
> I would like to make sure if we're aligned. The new version should contain the
> following:
> 1. The supported_tunnel_types are placed in the device config space; 2.
> Reserve the following structure:
>
> struct virtnet_hash_tunnel {
> le32 enabled_tunnel_types;
> };
>
> 3. Reserve the SET command for enabled_tunnel_types and remove the GET
> command for enabled_tunnel_types.
>
> If there is no problem, I will modify it accordingly.
Ack.
-
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 v19] virtio-net: support inner header hash
On Thu, Jun 29, 2023 at 02:40:42PM +0800, Heng Qi wrote:
> On Thu, Jun 29, 2023 at 01:56:34AM +, Parav Pandit wrote:
> >
> >
> > > From: Michael S. Tsirkin
> > > Sent: Wednesday, June 28, 2023 3:45 PM
> > > > > Maybe I get it. You want to use the new features as a carrot to
> > > > > force drivers to implement DMA? You suspect they will ignore the
> > > > > spec requirement just because things seem to work?
> > > > >
> > > > Right because it is not a must normative.
> > >
> > > Well SHOULD also does not mean "ok to just ignore".
> > >
> > > This word, or the adjective "RECOMMENDED", mean that there
> > > may exist valid reasons in particular circumstances to ignore a
> > > particular item, but the full implications must be understood and
> > > carefully weighed before choosing a different course.
> > >
> > RECOMMENDED and SHOULD forces the device to support MMIO, which is not good.
> > So rather a good design is device tells the starting offset for the
> > extended config space.
> > And extended config space MUST be accessed using a DMA.
> > With this sw can have infinite size MMIO and hw device forces DMA based on
> > its implementation of where to start DMA from.
> > This also gives the ability to maintain current config as MMIO for backward
> > compatibility.
> >
> > >
> > >
> > > > > There's some logic here, for sure. you just might be right.
> > > > >
> > > > > However, surely we can discuss this small tweak in 1.4 timeframe?
> > > >
> > > > Sure, if we prefer the DMA approach I don't have a problem in adding
> > > temporary one field to config space.
> > > >
> > > > I propose to add a line to the spec " Device Configuration Space"
> > > > section, something like,
> > > >
> > > > Note: Any new device configuration space fields additional MUST consider
> > > accessing such fields via a DMA interface.
> > > >
> > > > And this will guide the new patches of what to do instead of last moment
> > > rush.
> > >
> > > Yea, except again I'd probably make it a SHOULD: e.g. I can see how
> > > switching to
> > > MMIO might be an option for qemu helping us debug DMA issues.
> > >
> > There are too many queues whose debugging is needed and MMIO likely not the
> > way to debug.
> >
> > > The time to discuss this detail would be around when proposal for the DMA
> > > access to config space is on list though: I feel this SHOULD vs MUST is a
> > > small
> > > enough detail.
> > >
> > From implementation POV it is certainly critical and good step forward to
> > optimize virtio interface.
> >
> > > Going back to inner hash. If we move supported_tunnels back to config
> > > space,
> > > do you feel we still need GET or just drop it? I note we do not have GET
> > > for
> > > either hash or rss config.
> > >
> > For hash and rss config, debugging is missing. :)
> > Yes, we can drop the GET after switching supported_tunnels to struct
> > virtio_net_hash_config.
> >
>
> I would like to make sure if we're aligned. The new version should contain
> the following:
> 1. The supported_tunnel_types are placed in the device config space;
> 2. Reserve the following structure:
>
> struct virtnet_hash_tunnel {
> le32 enabled_tunnel_types;
> };
>
> 3. Reserve the SET command for enabled_tunnel_types and remove the GET
> command for enabled_tunnel_types.
>
> If there is no problem, I will modify it accordingly.
>
> Thanks!
> > > And if we no longer have GET is there still a reason for a separate
> > > command as
> > > opposed to a field in virtio_net_hash_config?
> > > I know this was done in v11 but there it was misaligned.
> > > We went with a command because we needed it for supported_tunnels but
> > > now that is no longer the case and there are reserved words in
> > > virtio_net_hash_config ...
> > >
> > > Let me know how you feel it about that, not critical for me.
> >
> > struct virtio_net_hash_config reserved is fine.
Just a second, when I talked about virtio_net_hash_config I meant this:
struct virtio_net_hash_config {
le32 hash_types;
- le16 reserved[4];
+ le32 enabled_tunnel_types;
+ le16 reserved[2];
u8 hash_key_length;
u8 hash_key_data[hash_key_length];
};
and then we do not add a new command we modify the hash config command.
Parav still ok with you?
--
MST
-
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 v19] virtio-net: support inner header hash
On Thu, Jun 29, 2023 at 10:05:09AM +0800, Heng Qi wrote: > > > 在 2023/6/29 上午9:56, Parav Pandit 写道: > > > > > From: Michael S. Tsirkin > > > Sent: Wednesday, June 28, 2023 3:45 PM > > > > > Maybe I get it. You want to use the new features as a carrot to > > > > > force drivers to implement DMA? You suspect they will ignore the > > > > > spec requirement just because things seem to work? > > > > > > > > > Right because it is not a must normative. > > > Well SHOULD also does not mean "ok to just ignore". > > > > > > This word, or the adjective "RECOMMENDED", mean that there > > > may exist valid reasons in particular circumstances to ignore a > > > particular item, but the full implications must be understood and > > > carefully weighed before choosing a different course. > > > > > RECOMMENDED and SHOULD forces the device to support MMIO, which is not good. > > So rather a good design is device tells the starting offset for the > > extended config space. > > And extended config space MUST be accessed using a DMA. > > With this sw can have infinite size MMIO and hw device forces DMA based on > > its implementation of where to start DMA from. > > This also gives the ability to maintain current config as MMIO for backward > > compatibility. > > > > > > > > There's some logic here, for sure. you just might be right. > > > > > > > > > > However, surely we can discuss this small tweak in 1.4 timeframe? > > > > Sure, if we prefer the DMA approach I don't have a problem in adding > > > temporary one field to config space. > > > > I propose to add a line to the spec " Device Configuration Space" > > > > section, something like, > > > > > > > > Note: Any new device configuration space fields additional MUST consider > > > accessing such fields via a DMA interface. > > > > And this will guide the new patches of what to do instead of last moment > > > rush. > > > > > > Yea, except again I'd probably make it a SHOULD: e.g. I can see how > > > switching to > > > MMIO might be an option for qemu helping us debug DMA issues. > > > > > There are too many queues whose debugging is needed and MMIO likely not the > > way to debug. > > > The time to discuss this detail would be around when proposal for the DMA > > > access to config space is on list though: I feel this SHOULD vs MUST is a > > > small > > > enough detail. > > > > > From implementation POV it is certainly critical and good step forward to > > optimize virtio interface. > > > Going back to inner hash. If we move supported_tunnels back to config > > > space, > > > do you feel we still need GET or just drop it? I note we do not have GET > > > for > > > either hash or rss config. > > > > > For hash and rss config, debugging is missing. :) > > Yes, we can drop the GET after switching supported_tunnels to struct > > virtio_net_hash_config. > > Great! Glad to hear this! > > > > And if we no longer have GET is there still a reason for a separate > > > command as > > > opposed to a field in virtio_net_hash_config? > > > I know this was done in v11 but there it was misaligned. > > > We went with a command because we needed it for supported_tunnels but > > > now that is no longer the case and there are reserved words in > > > virtio_net_hash_config ... > > > > > > Let me know how you feel it about that, not critical for me. > > struct virtio_net_hash_config reserved is fine. > > +1. > > Inner header hash is orthogonal to RSS, and it's fine to have its own > structure and commands. > There is no need to send additional RSS fields when we configure inner > header hash. > > Thanks. Not RSS, hash calculations. It's not critical, but I note that practically you said you will enable this with symmetric hash so it makes sense to me to send this in the same command with the key. Not critical though if there's opposition. -- MST - 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 v19] virtio-net: support inner header hash
On Thu, Jun 29, 2023 at 11:31:58AM +0800, Jason Wang wrote:
>
> 在 2023/6/28 18:10, Michael S. Tsirkin 写道:
> > On Wed, Jun 28, 2023 at 11:46:22AM +0800, Jason Wang wrote:
> > > On Wed, Jun 28, 2023 at 12:35 AM 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
> > > > ---
> > > > v18->v19:
> > > > 1. Have a single structure instead of two. @Michael S . Tsirkin
> > > > 2. Some small rewrites. @Michael S . Tsirkin
> > > > 3. Rebase to master.
> > > >
> > > > v17->v18:
> > > > 1. Some rewording suggestions from Michael (Thanks!).
> > > > 2. Use 0 to disable inner header hash and remove
> > > > VIRTIO_NET_HASH_TUNNEL_TYPE_NONE.
> > > > v16->v17:
> > > > 1. Some small rewrites. @Parav Pandit
> > > > 2. Add Parav's Reviewed-by tag (Thanks!).
> > > >
> > > > v15->v16:
> > > > 1. Remove the hash_option. In order to delimit the inner
> > > > header hash and RSS
> > > > configuration, the ability to configure the outer src udp
> > > > port hash is given
> > > > to RSS. This is orthogonal to inner header hash, which will
> > > > be done in the
> > > > RSS capability extension topic (considered as an RSS
> > > > extension together
> > > > with the symmetric toeplitz hash algorithm, etc.). @Parav
> > > > Pandit @Michael S . Tsirkin
> > > > 2. Fix a 'field' typo. @Parav Pandit
> > > >
> > > > v14->v15:
> > > > 1. Add tunnel hash option suggested by @Michael S . Tsirkin
> > > > 2. Adjust some descriptions.
> > > >
> > > > v13->v14:
> > > > 1. Move supported_hash_tunnel_types from config space into cvq
> > > > command. @Parav Pandit
> > > I may miss some discussions, but this complicates the provisioning a lot.
> > >
> > > Having it in the config space, then a type agnostic provisioning
> > > through config space + feature bits just works fine.
> > >
> > > If we move it only via cvq, we need device specific provisioning
> > > interface.
> > >
> > > Thanks
> > Yea that's what I said too. Debugging too. I think we should build a
> > consistent solution that allows accessing config space through DMA,
> > separately from this effort. Parav do you think you can live with this
> > approach so this specific proposal can move forward?
>
>
> We can probably go another way, invent a new device configuration space
> capability which fixed size like PCI configuration access capability?
>
> struct virtio_pci_cfg_cap {
> struct virtio_pci_cap cap;
> u8 dev_cfg_data[4]; /* Data for device configuration space access.
> */
> };
>
> So it won't grow as the size of device configuration space grows.
>
> Thanks
It is true, it does not have to be DMA strictly speaking.
The basic issue is with synchronous access.
We can change the capability in some way to allow asynch then
that works. E.g. make device change length to 0 and driver
must poll before considering the operation done.
Having said t
[virtio-dev] Re: [virtio-comment] Re: [PATCH v19] virtio-net: support inner header hash
On Thu, Jun 29, 2023 at 07:54:29AM -0400, Michael S. Tsirkin wrote:
> On Thu, Jun 29, 2023 at 11:31:58AM +0800, Jason Wang wrote:
> >
> > 在 2023/6/28 18:10, Michael S. Tsirkin 写道:
> > > On Wed, Jun 28, 2023 at 11:46:22AM +0800, Jason Wang wrote:
> > > > On Wed, Jun 28, 2023 at 12:35 AM 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
> > > > > ---
> > > > > v18->v19:
> > > > > 1. Have a single structure instead of two. @Michael S .
> > > > > Tsirkin
> > > > > 2. Some small rewrites. @Michael S . Tsirkin
> > > > > 3. Rebase to master.
> > > > >
> > > > > v17->v18:
> > > > > 1. Some rewording suggestions from Michael (Thanks!).
> > > > > 2. Use 0 to disable inner header hash and remove
> > > > > VIRTIO_NET_HASH_TUNNEL_TYPE_NONE.
> > > > > v16->v17:
> > > > > 1. Some small rewrites. @Parav Pandit
> > > > > 2. Add Parav's Reviewed-by tag (Thanks!).
> > > > >
> > > > > v15->v16:
> > > > > 1. Remove the hash_option. In order to delimit the inner
> > > > > header hash and RSS
> > > > > configuration, the ability to configure the outer src udp
> > > > > port hash is given
> > > > > to RSS. This is orthogonal to inner header hash, which
> > > > > will be done in the
> > > > > RSS capability extension topic (considered as an RSS
> > > > > extension together
> > > > > with the symmetric toeplitz hash algorithm, etc.). @Parav
> > > > > Pandit @Michael S . Tsirkin
> > > > > 2. Fix a 'field' typo. @Parav Pandit
> > > > >
> > > > > v14->v15:
> > > > > 1. Add tunnel hash option suggested by @Michael S . Tsirkin
> > > > > 2. Adjust some descriptions.
> > > > >
> > > > > v13->v14:
> > > > > 1. Move supported_hash_tunnel_types from config space into
> > > > > cvq command. @Parav Pandit
> > > > I may miss some discussions, but this complicates the provisioning a
> > > > lot.
> > > >
> > > > Having it in the config space, then a type agnostic provisioning
> > > > through config space + feature bits just works fine.
> > > >
> > > > If we move it only via cvq, we need device specific provisioning
> > > > interface.
> > > >
> > > > Thanks
> > > Yea that's what I said too. Debugging too. I think we should build a
> > > consistent solution that allows accessing config space through DMA,
> > > separately from this effort. Parav do you think you can live with this
> > > approach so this specific proposal can move forward?
> >
> >
> > We can probably go another way, invent a new device configuration space
> > capability which fixed size like PCI configuration access capability?
> >
> > struct virtio_pci_cfg_cap {
> > struct virtio_pci_cap cap;
> > u8 dev_cfg_data[4]; /* Data for device configuration space access.
> > */
> > };
> >
> > So it won't grow as the size of device co
[virtio-dev] RE: [virtio-comment] RE: [PATCH v19] virtio-net: support inner header hash
> From: Michael S. Tsirkin
> Sent: Thursday, June 29, 2023 7:46 AM
> Just a second, when I talked about virtio_net_hash_config I meant this:
>
>
> struct virtio_net_hash_config {
> le32 hash_types;
> - le16 reserved[4];
> + le32 enabled_tunnel_types;
> + le16 reserved[2];
> u8 hash_key_length;
> u8 hash_key_data[hash_key_length];
> };
>
> and then we do not add a new command we modify the hash config command.
>
> Parav still ok with you?
yes, looks good.
-
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: [virtio-comment] RE: [PATCH v19] virtio-net: support inner header hash
在 2023/6/29 下午7:48, Michael S. Tsirkin 写道: On Thu, Jun 29, 2023 at 10:05:09AM +0800, Heng Qi wrote: 在 2023/6/29 上午9:56, Parav Pandit 写道: From: Michael S. Tsirkin Sent: Wednesday, June 28, 2023 3:45 PM Maybe I get it. You want to use the new features as a carrot to force drivers to implement DMA? You suspect they will ignore the spec requirement just because things seem to work? Right because it is not a must normative. Well SHOULD also does not mean "ok to just ignore". This word, or the adjective "RECOMMENDED", mean that there may exist valid reasons in particular circumstances to ignore a particular item, but the full implications must be understood and carefully weighed before choosing a different course. RECOMMENDED and SHOULD forces the device to support MMIO, which is not good. So rather a good design is device tells the starting offset for the extended config space. And extended config space MUST be accessed using a DMA. With this sw can have infinite size MMIO and hw device forces DMA based on its implementation of where to start DMA from. This also gives the ability to maintain current config as MMIO for backward compatibility. There's some logic here, for sure. you just might be right. However, surely we can discuss this small tweak in 1.4 timeframe? Sure, if we prefer the DMA approach I don't have a problem in adding temporary one field to config space. I propose to add a line to the spec " Device Configuration Space" section, something like, Note: Any new device configuration space fields additional MUST consider accessing such fields via a DMA interface. And this will guide the new patches of what to do instead of last moment rush. Yea, except again I'd probably make it a SHOULD: e.g. I can see how switching to MMIO might be an option for qemu helping us debug DMA issues. There are too many queues whose debugging is needed and MMIO likely not the way to debug. The time to discuss this detail would be around when proposal for the DMA access to config space is on list though: I feel this SHOULD vs MUST is a small enough detail. From implementation POV it is certainly critical and good step forward to optimize virtio interface. Going back to inner hash. If we move supported_tunnels back to config space, do you feel we still need GET or just drop it? I note we do not have GET for either hash or rss config. For hash and rss config, debugging is missing. :) Yes, we can drop the GET after switching supported_tunnels to struct virtio_net_hash_config. Great! Glad to hear this! And if we no longer have GET is there still a reason for a separate command as opposed to a field in virtio_net_hash_config? I know this was done in v11 but there it was misaligned. We went with a command because we needed it for supported_tunnels but now that is no longer the case and there are reserved words in virtio_net_hash_config ... Let me know how you feel it about that, not critical for me. struct virtio_net_hash_config reserved is fine. +1. Inner header hash is orthogonal to RSS, and it's fine to have its own structure and commands. There is no need to send additional RSS fields when we configure inner header hash. Thanks. Not RSS, hash calculations. It's not critical, but I note that practically you said you will enable this with symmetric hash so it makes sense to me to send this in the same command This works for me. Thanks. with the key. Not critical though if there's opposition. - 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 PATCH v8 1/1] virtio-video: Add virtio video device specification
Add the specification of the video decoder and encoder devices, which
can be used to provide host-accelerated video operations to the guest.
Signed-off-by: Alexander Gordeev
Co-authored-by: Keiichi Watanabe
Co-authored-by: Alexandre Courbot
---
conformance.tex |4 +
content.tex |1 +
device-types/video/description.tex| 2040 +
device-types/video/device-conformance.tex | 22 +
device-types/video/driver-conformance.tex | 16 +
introduction.tex | 21 +
6 files changed, 2104 insertions(+)
create mode 100644 device-types/video/description.tex
create mode 100644 device-types/video/device-conformance.tex
create mode 100644 device-types/video/driver-conformance.tex
diff --git a/conformance.tex b/conformance.tex
index 01ccd69..d719eda 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -34,6 +34,7 @@ \section{Conformance Targets}\label{sec:Conformance /
Conformance Targets}
\ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
+\ref{sec:Conformance / Driver Conformance / Video Driver Conformance},
\item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device
and Transitional Driver Conformance}.
\end{itemize}
@@ -61,6 +62,7 @@ \section{Conformance Targets}\label{sec:Conformance /
Conformance Targets}
\ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
+\ref{sec:Conformance / Device Conformance / Video Device Conformance},
\item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device
and Transitional Driver Conformance}.
\end{itemize}
@@ -152,6 +154,7 @@ \section{Conformance Targets}\label{sec:Conformance /
Conformance Targets}
\input{device-types/scmi/driver-conformance.tex}
\input{device-types/gpio/driver-conformance.tex}
\input{device-types/pmem/driver-conformance.tex}
+\input{device-types/video/driver-conformance.tex}
\conformance{\section}{Device Conformance}\label{sec:Conformance / Device
Conformance}
@@ -238,6 +241,7 @@ \section{Conformance Targets}\label{sec:Conformance /
Conformance Targets}
\input{device-types/scmi/device-conformance.tex}
\input{device-types/gpio/device-conformance.tex}
\input{device-types/pmem/device-conformance.tex}
+\input{device-types/video/device-conformance.tex}
\conformance{\section}{Legacy Interface: Transitional Device and Transitional
Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional
Device and Transitional Driver Conformance}
A conformant implementation MUST be either transitional or
diff --git a/content.tex b/content.tex
index d2ab9eb..90708d7 100644
--- a/content.tex
+++ b/content.tex
@@ -765,6 +765,7 @@ \chapter{Device Types}\label{sec:Device Types}
\input{device-types/scmi/description.tex}
\input{device-types/gpio/description.tex}
\input{device-types/pmem/description.tex}
+\input{device-types/video/description.tex}
\chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
diff --git a/device-types/video/description.tex
b/device-types/video/description.tex
new file mode 100644
index 000..760df7f
--- /dev/null
+++ b/device-types/video/description.tex
@@ -0,0 +1,2040 @@
+\section{Video Device}
+\label{sec:Device Types / Video Device}
+
+The virtio video encoder and decoder devices provide support for
+host-accelerated video encoding and decoding. Despite being different
+device types, they use the same protocol and general flow.
+
+\subsection{Device ID}
+\label{sec:Device Types / Video Device / Device ID}
+
+\begin{description}
+\item[30]
+ encoder device
+\item[31]
+ decoder device
+\end{description}
+
+\subsection{Virtqueues}
+\label{sec:Device Types / Video Device / Virtqueues}
+
+\begin{description}
+\item[0]
+ commandq - queue for driver commands and device responses to these commands
+\item[1]
+ eventq - queue for device delayed responses to commands and standalone
+ device events
+\end{description}
+
+\subsection{Feature bits}
+\label{sec:Device Types / Video Device / Feature bits}
+
+\begin{description}
+\item[VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES (0)]
+ Guest pages can be used as the backing memory of resources.
+\item[VIRTIO_VIDEO_F_RESOURCE_NON_CONTIG (1)]
+ The device can use non-contiguous guest memory as the backing memory of
+ resources. Only meaningful if VIRTIO_VIDEO_F_RESOURCE_GUEST_PAGES is also
+ set.
+\item[VIRTIO_VIDEO_F_RESOURCE_VIRTIO_OBJECT (2)]
+ Objects exported by another virtio device can be used as the backing memory
+ of resources.
+\item[VIRTIO_VIDEO_F_RESOURCE_DYNAMIC (3)]
+ The device supports re-attaching memory to resources while streaming.
+% TODO this flag is not used a
[virtio-dev] [RFC PATCH v8 0/1] Virtio video device specification
Hi, This is the 8th version of virtio-video device patch. Feedback would be very appreciated. The main change coming with this draft is the introduction of background operations and delayed responses to commands (the term is taken from the SCMI spec) over eventq. Now most of the commands work like this. The only exceptions are STREAM_CREATE and RESOURCE_ATTACH_BACKING commands. This should help avoid commandq descriptors exhaustion. Also I introduced "in band" and "out of band" handling of SET_PARAMS command. Out of band is the way how it was done before i.e. the command is handled immediately after dequeuing from commandq. In band means putting it into device's INPUT queue so that it can be executed precisely after a certain input resource and before the next one. These two changes allowed me to represent the events as a custom case of the delayed responses. Also this enables processing resolution changes embedded in the stream and coming from outside in the same way. I think this makes it possible to implement V4L2 Requests API. Questions for discussion/roadmap: 1. It looks like the current way of describing the device capabilities is not very well extendable. I'd like to change it again. The goal is that after adding new generic parameters/new codecs/new codec parameters and covering them with the feature flags the format of the device response doesn't depend that much on the enabled flags. My current idea is to convert this into a serie of descriptors with offsets and sizes, so that the device can then simply adjust offsets and sizes, but keep that layout the same. Any ideas here would be appreciated. 2. Since draft v7 the device is expected to provide all and every one of its capabilities as a response to the DEVICE_QUERY_CAPS command. Maybe the device shouldn't adjust the parameters itself now? 3. I think the last piece, that is missing completely, is QoS. How can the device indicate to the driver, that it is saturated? How could the device be shared between multiple guests in a fair way? Full PDF: https://drive.google.com/file/d/1uPg4kxThlNPBSiC4b5veyFz4OFGytU7v/view?usp=sharing PDF with the video section only: https://drive.google.com/file/d/1iW3MbseRZLovlxpc4XAF8VTvhQ_Tww6q/view?usp=sharing Changelog v7 -> v8: * Added the delayed responses to commands sent on the eventq. * Added the in band STREAM_SET_PARAMS command handling. * Implemented device events using the newly added delayed responses. Alexander Gordeev (1): virtio-video: Add virtio video device specification conformance.tex |4 + content.tex |1 + device-types/video/description.tex| 2040 + device-types/video/device-conformance.tex | 22 + device-types/video/driver-conformance.tex | 16 + introduction.tex | 21 + 6 files changed, 2104 insertions(+) create mode 100644 device-types/video/description.tex create mode 100644 device-types/video/device-conformance.tex create mode 100644 device-types/video/driver-conformance.tex -- 2.34.1 - 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 v19] virtio-net: support inner header hash
> From: Michael S. Tsirkin > Sent: Thursday, June 29, 2023 7:48 AM > > > struct virtio_net_hash_config reserved is fine. > > > > +1. > > > > Inner header hash is orthogonal to RSS, and it's fine to have its own > > structure and commands. > > There is no need to send additional RSS fields when we configure inner > > header hash. > > > > Thanks. > > Not RSS, hash calculations. It's not critical, but I note that practically > you said > you will enable this with symmetric hash so it makes sense to me to send this > in > the same command with the key. > In the v19, we have, +\item[VIRTIO_NET_F_HASH_TUNNEL] Requires VIRTIO_NET_F_CTRL_VQ along with VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT. So it is done along with rss, so in same struct as rss config is fine.
[virtio-dev] RE: [virtio-comment] [RFC] virtio 1.3 schedule
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? - 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 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. -- MST - 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 v7 0/4] admin: Introduce legacy registers access using AQ
On Wed, Jun 28, 2023 at 12:10:46AM +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. Went over this quickly and I think this is ok from functionality perspective. Unfortunately needs a bit more work on language clarity, grammar etc :( My estimate would be another week to finalize. > Patch summary: > -- > patch-1 split rows of admin opcode tables by a line > patch-2 fix section numbering > patch-3 add generic legacy region access commands > patch-4 add pci specific definition > > Iadministation t uses the newly introduced administration command facility > with 4 new > commands which uses the existing virtio_admin_cmd and a new command to > query the legacy notification region. > > Usecase: > > 1. A hypervisor/system needs to provide transitional >virtio devices to the guest VM at scale of thousands, >typically, one to eight devices per VM. > > 2. A hypervisor/system needs to provide such devices using a >vendor agnostic driver in the hypervisor system. > > 3. A hypervisor system prefers to have single stack regardless of >virtio device type (net/blk) and be future compatible with a >single vfio stack using SR-IOV or other scalable device >virtualization technology to map PCI devices to the guest VM. >(as transitional or otherwise) > > Motivation/Background: > -- > The existing virtio transitional PCI device is missing support for > PCI SR-IOV based devices. Currently it does not work beyond > PCI PF, or as software emulated device in reality. Currently it > has below cited system level limitations: > > [a] PCIe spec citation: > VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. > > [b] cpu arch citiation: > Intel 64 and IA-32 Architectures Software Developer’s Manual: > The processor’s I/O address space is separate and distinct from > the physical-memory address space. The I/O address space consists > of 64K individually addressable 8-bit I/O ports, numbered 0 through H. > > [c] PCIe spec citation: > If a bridge implements an I/O address range,...I/O address range will be > aligned to a 4 KB boundary. > > Overview: > - > Above usecase requirements is solved by PCI PF group owner accessing > its group member PCI VFs legacy registers using an admin virtqueue of > the group owner PCI PF. > > Two new admin virtqueue commands are added which read/write PCI VF > registers. > > Software usage example: > --- > One way to use and map to the guest VM is by using vfio driver > framework in Linux kernel. > > +--+ > |pci_dev_id = 0x100X | > +---|pci_rev_id = 0x0 |-+ > |vfio device|BAR0 = I/O region | | > | |Other attributes | | > | +--+ | > || > + +--+ +-+ | > | |I/O BAR to AQ | | Other vfio | | > | |rd/wr mapper | | functionalities | | > | +--+ +-+ | > || > +--+-+---+ >| | >Legacy regionDriver notification > access | >| | > +++ +++ > | +-+ | | PCI VF device A | > | | AQ |-+>+-+ | > | +-+ | | | | legacy regs | | > | PCI PF device | | | +-+ | > +-+ | +-+ > | > | +++ > | | PCI VF device N | > +>+-+ | > | | legacy regs | | > | +-+ | > +-+ > > 2. Virtio pci driver to bind to the listed device id and >use it in the host. > > 3. Use it in a light weight hypervisor to run bare-metal OS. > > Please review. > > Alternatives considered: > > 1. Exposing BAR0 as MMIO BAR that follows legacy registers template > Pros: > a. Kind of works with legacy drivers as some of them have used API >which is agnostic to MMIO vs IOBAR. > b. Does not require hypervisor in
[virtio-dev] Re: [PATCH v7 4/4] transport-pci: Introduce group legacy group member config region access
On Wed, Jun 28, 2023 at 12:10:50AM +0300, Parav Pandit wrote:
> This patch links how in a PCI transport a group owner can access group
> member (PCI VFs) legacy registers using a legacy registers access
> commands using administration virtqueue infrastructure.
>
> Additionally it extend the PCI notification capability through which a
> PCI VF device indicates to the driver which PCI BAR region to be used
> for driver notifications.
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
> Signed-off-by: Parav Pandit
> ---
> changelog:
> v6->v7:
> - addressed comments from Michael
> - removed driver normative about I/O BAR emulation as it does not
> make much sense for the spec
> - removed references to administration virtqueue
> - rewrote driver legacy region access without guest and hypervisor
> wording
> - added normative for notification region
> - added normative for PCI IDs for device which support legacy commands
> v5->v6:
> - aligned pci capability to 4B as required by PCI spec
> - added text for the PCI capability for the group member device
> v4->v5:
> - split pci transport and generic command section to new patch
> - removed multiple references to the VF
> - written the description of the command as generic with member
> and group device terminology
> - reflected many section names to remove VF
> v3->v4:
> - moved noted to the conformance section details in next patch
> - removed queue notify address query AQ command on Michael's suggestion,
> though it is fine. Instead replaced with extending virtio_pci_notify_cap
> to indicate that legacy queue notifications can be done on the
> notification location.
> - fixed spelling errors.
> - replaced administrative virtqueue to administration virtqueue
> - added queue notification capability register to indicate legacy q
> notification supported
> v2->v3:
> - adddressed Jason and Michael's comment to split single register
> access command to common config and device specific commands.
> - dropped the suggetion to introduce enable/disable command as
> admin command cap bit already covers it.
> v1->v2:
> - addressed comments from Michael
> - added theory of operation
> - grammar corrections
> - removed group fields description from individual commands as
> it is already present in generic section
> - added endianness normative for legacy device registers region
> - renamed the file to drop vf and add legacy prefix
>
> - added overview in commit log
> - renamed subsection to reflect command
> ---
> conformance.tex | 1 +
> transport-pci-legacy-regs.tex | 43 +++
> transport-pci.tex | 2 ++
> 3 files changed, 46 insertions(+)
> create mode 100644 transport-pci-legacy-regs.tex
>
> diff --git a/conformance.tex b/conformance.tex
> index dc00e84..b3f2c92 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -267,6 +267,7 @@ \section{Conformance Targets}\label{sec:Conformance /
> Conformance Targets}
> \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI
> Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
> \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus /
> PCI-specific Initialization And Device Operation / Device Initialization /
> Virtio Device Configuration Layout Detection / Legacy Interface: A Note on
> Device Layout Detection}
> \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus /
> PCI-specific Initialization And Device Operation / Device Initialization /
> Virtqueue Configuration / Legacy Interface: A Note on Virtqueue Configuration}
> +\item Section \ref{devicenormative:Virtio Transport Options / Virtio Over
> PCI Bus / Legacy Interface: Group Member Device Configuration Region Access}
> \item Section \ref{sec:Virtio Transport Options / Virtio Over MMIO / Legacy
> interface}
> \item Section \ref{sec:Virtio Transport Options / Virtio over channel I/O /
> Device Initialization / Setting the Virtio Revision / Legacy Interfaces: A
> Note on Setting the Virtio Revision}
> \item Section \ref{sec:Virtio Transport Options / Virtio over channel I/O /
> Device Initialization / Configuring a Virtqueue / Legacy Interface: A Note on
> Configuring a Virtqueue}
> diff --git a/transport-pci-legacy-regs.tex b/transport-pci-legacy-regs.tex
> new file mode 100644
> index 000..9559768
> --- /dev/null
> +++ b/transport-pci-legacy-regs.tex
> @@ -0,0 +1,43 @@
> +\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
> +a notification region defined in the \field{struct virtio_pci_notify_region}.
> +
> +In \field{struct virtio_virtio_admin_cmd_legacy_notify_query_entry},
> +\field{region} is defined as following:
> +
> +\begin{lstli
[virtio-dev] Re: [PATCH v7 3/4] admin: Add group member legacy register access commands
On Wed, Jun 28, 2023 at 12:10:49AM +0300, Parav Pandit wrote: > Introduce group member legacy common configuration and legacy device > configuration access read/write commands. > > Group member legacy registers access commands enable group owner driver > software to access legacy registers on behalf of the guest virtual > machine. > > Usecase: > > 1. A hypervisor/system needs to provide transitional >virtio devices to the guest VM at scale of thousands, >typically, one to eight devices per VM. > > 2. A hypervisor/system needs to provide such devices using a >vendor agnostic driver in the hypervisor system. > > 3. A hypervisor system prefers to have single stack regardless of >virtio device type (net/blk) and be future compatible with a >single vfio stack using SR-IOV or other scalable device >virtualization technology to map PCI devices to the guest VM. >(as transitional or otherwise) > > Motivation/Background: > = > The existing virtio transitional PCI device is missing support for > PCI SR-IOV based devices. Currently it does not work beyond > PCI PF, or as software emulated device in reality. Currently it > has below cited system level limitations: > > [a] PCIe spec citation: > VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. > > [b] cpu arch citiation: > Intel 64 and IA-32 Architectures Software Developer’s Manual: > The processor’s I/O address space is separate and distinct from > the physical-memory address space. The I/O address space consists > of 64K individually addressable 8-bit I/O ports, numbered 0 through H. > > [c] PCIe spec citation: > If a bridge implements an I/O address range,...I/O address range will be > aligned to a 4 KB boundary. > > Overview: > = > Above usecase requirements is solved by PCI PF group owner accessing > its group member PCI VFs legacy registers using the administration > commands of the group owner PCI PF. > > Two types of administration commands are added which read/write PCI VF > registers. > > Software usage example: > === > > 1. One way to use and map to the guest VM is by using vfio driver > framework in Linux kernel. > > +--+ > |pci_dev_id = 0x100X | > +---|pci_rev_id = 0x0 |-+ > |vfio device|BAR0 = I/O region | | > | |Other attributes | | > | +--+ | > || > + +--+ +-+ | > | |I/O BAR to AQ | | Other vfio | | > | |rd/wr mapper& | | functionalities | | > | | forwarder| | | | > | +--+ +-+ | > || > +--+-+---+ >| | >Config region | > accessDriver notifications >| | > +++ +++ > | +-+ | | PCI VF device A | > | | AQ |-+>+-+ | > | +-+ | | | | legacy regs | | > | PCI PF device | | | +-+ | > +-+ | +-+ > | > | +++ > | | PCI VF device N | > +>+-+ | > | | legacy regs | | > | +-+ | > +-+ > > 2. Continue to use the virtio pci driver to bind to the >listed device id and use it as in the host. > > 3. Use it in a light weight hypervisor to run bare-metal OS. > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167 > Signed-off-by: Parav Pandit > --- > changelog: > v6->v7: > - changed administrative to administration > - renamed admin-access.tex to admin-interface.tex > - large rewrite ad generic admin commands instead of pci > - added theory of operation section > - added driver notification region query command > v5->v6: > - fixed previous missed abbreviation of LCC and LD > v4->v5: > - split from pci transport specific patch > - split conformance to transport and generic sections > - written the description of the command as generic with member > and group device terminology > - reflected many section names to remove VF > - rename fields from register to region > - avoided abbreviation for legacy, device and config > --- > admin-cmds-legacy-interface.tex | 204 > admin.tex | 14 ++- > conformance.tex | 2 + > 3 files changed, 219 insertions(+), 1 deletion(-) > create mode 100644 admin-cmds-legacy-interface.tex > > diff --git a/admin-cmds-legacy-interface.tex b/admin-cmds-legacy-in
[virtio-dev] Re: [PATCH v7 4/4] transport-pci: Introduce group legacy group member config region access
On Wed, Jun 28, 2023 at 12:10:50AM +0300, Parav Pandit wrote:
> This patch links how in a PCI transport a group owner can access group
> member (PCI VFs) legacy registers using a legacy registers access
> commands using administration virtqueue infrastructure.
>
> Additionally it extend the PCI notification capability through which a
> PCI VF device indicates to the driver which PCI BAR region to be used
> for driver notifications.
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
> Signed-off-by: Parav Pandit
> ---
> changelog:
> v6->v7:
> - addressed comments from Michael
> - removed driver normative about I/O BAR emulation as it does not
> make much sense for the spec
> - removed references to administration virtqueue
> - rewrote driver legacy region access without guest and hypervisor
> wording
> - added normative for notification region
> - added normative for PCI IDs for device which support legacy commands
> v5->v6:
> - aligned pci capability to 4B as required by PCI spec
> - added text for the PCI capability for the group member device
> v4->v5:
> - split pci transport and generic command section to new patch
> - removed multiple references to the VF
> - written the description of the command as generic with member
> and group device terminology
> - reflected many section names to remove VF
> v3->v4:
> - moved noted to the conformance section details in next patch
> - removed queue notify address query AQ command on Michael's suggestion,
> though it is fine. Instead replaced with extending virtio_pci_notify_cap
> to indicate that legacy queue notifications can be done on the
> notification location.
> - fixed spelling errors.
> - replaced administrative virtqueue to administration virtqueue
> - added queue notification capability register to indicate legacy q
> notification supported
> v2->v3:
> - adddressed Jason and Michael's comment to split single register
> access command to common config and device specific commands.
> - dropped the suggetion to introduce enable/disable command as
> admin command cap bit already covers it.
> v1->v2:
> - addressed comments from Michael
> - added theory of operation
> - grammar corrections
> - removed group fields description from individual commands as
> it is already present in generic section
> - added endianness normative for legacy device registers region
> - renamed the file to drop vf and add legacy prefix
>
> - added overview in commit log
> - renamed subsection to reflect command
> ---
> conformance.tex | 1 +
> transport-pci-legacy-regs.tex | 43 +++
> transport-pci.tex | 2 ++
> 3 files changed, 46 insertions(+)
> create mode 100644 transport-pci-legacy-regs.tex
>
> diff --git a/conformance.tex b/conformance.tex
> index dc00e84..b3f2c92 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -267,6 +267,7 @@ \section{Conformance Targets}\label{sec:Conformance /
> Conformance Targets}
> \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI
> Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
> \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus /
> PCI-specific Initialization And Device Operation / Device Initialization /
> Virtio Device Configuration Layout Detection / Legacy Interface: A Note on
> Device Layout Detection}
> \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus /
> PCI-specific Initialization And Device Operation / Device Initialization /
> Virtqueue Configuration / Legacy Interface: A Note on Virtqueue Configuration}
> +\item Section \ref{devicenormative:Virtio Transport Options / Virtio Over
> PCI Bus / Legacy Interface: Group Member Device Configuration Region Access}
> \item Section \ref{sec:Virtio Transport Options / Virtio Over MMIO / Legacy
> interface}
> \item Section \ref{sec:Virtio Transport Options / Virtio over channel I/O /
> Device Initialization / Setting the Virtio Revision / Legacy Interfaces: A
> Note on Setting the Virtio Revision}
> \item Section \ref{sec:Virtio Transport Options / Virtio over channel I/O /
> Device Initialization / Configuring a Virtqueue / Legacy Interface: A Note on
> Configuring a Virtqueue}
> diff --git a/transport-pci-legacy-regs.tex b/transport-pci-legacy-regs.tex
> new file mode 100644
> index 000..9559768
> --- /dev/null
> +++ b/transport-pci-legacy-regs.tex
> @@ -0,0 +1,43 @@
> +\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
> +a notification region defined in the \field{struct virtio_pci_notify_region}.
> +
> +In \field{struct virtio_virtio_admin_cmd_legacy_notify_query_entry},
> +\field{region} is defined as following:
> +
> +\begin{lstli
[virtio-dev] Re: [PATCH v7 0/4] admin: Introduce legacy registers access using AQ
On Wed, Jun 28, 2023 at 12:10:46AM +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. > > Patch summary: > -- > patch-1 split rows of admin opcode tables by a line > patch-2 fix section numbering > patch-3 add generic legacy region access commands > patch-4 add pci specific definition > > Iadministation t uses the newly introduced administration command facility > with 4 new > commands which uses the existing virtio_admin_cmd and a new command to > query the legacy notification region. > > Usecase: > > 1. A hypervisor/system needs to provide transitional >virtio devices to the guest VM at scale of thousands, >typically, one to eight devices per VM. > > 2. A hypervisor/system needs to provide such devices using a >vendor agnostic driver in the hypervisor system. > > 3. A hypervisor system prefers to have single stack regardless of >virtio device type (net/blk) and be future compatible with a >single vfio stack using SR-IOV or other scalable device >virtualization technology to map PCI devices to the guest VM. >(as transitional or otherwise) OK I went over it. Still some comments on the ABI. If you can address that by next week, I will try to find the way to fix up the wording next week. Problem is, when things are finally ready is when people start review in earnest and while I hope at least ABI wise we will not need to make changes it's likely that there will be comments on wording. If we are super lucky we can finish until end of next week but my problem is I can't work on this 100% of my time. > Motivation/Background: > -- > The existing virtio transitional PCI device is missing support for > PCI SR-IOV based devices. Currently it does not work beyond > PCI PF, or as software emulated device in reality. Currently it > has below cited system level limitations: > > [a] PCIe spec citation: > VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. > > [b] cpu arch citiation: > Intel 64 and IA-32 Architectures Software Developer’s Manual: > The processor’s I/O address space is separate and distinct from > the physical-memory address space. The I/O address space consists > of 64K individually addressable 8-bit I/O ports, numbered 0 through H. > > [c] PCIe spec citation: > If a bridge implements an I/O address range,...I/O address range will be > aligned to a 4 KB boundary. > > Overview: > - > Above usecase requirements is solved by PCI PF group owner accessing > its group member PCI VFs legacy registers using an admin virtqueue of > the group owner PCI PF. > > Two new admin virtqueue commands are added which read/write PCI VF > registers. > > Software usage example: > --- > One way to use and map to the guest VM is by using vfio driver > framework in Linux kernel. > > +--+ > |pci_dev_id = 0x100X | > +---|pci_rev_id = 0x0 |-+ > |vfio device|BAR0 = I/O region | | > | |Other attributes | | > | +--+ | > || > + +--+ +-+ | > | |I/O BAR to AQ | | Other vfio | | > | |rd/wr mapper | | functionalities | | > | +--+ +-+ | > || > +--+-+---+ >| | >Legacy regionDriver notification > access | >| | > +++ +++ > | +-+ | | PCI VF device A | > | | AQ |-+>+-+ | > | +-+ | | | | legacy regs | | > | PCI PF device | | | +-+ | > +-+ | +-+ > | > | +++ > | | PCI VF device N | > +>+-+ | > | | legacy regs | | > | +-+ | > +-+ > > 2. Virtio pci driver to bind to the listed device id and >use it in the host. > > 3. Use it in a light weight hypervisor to run bare-metal OS. > > Please review. > > Altern
[virtio-dev] RE: [PATCH v7 4/4] transport-pci: Introduce group legacy group member config region access
> From: Michael S. Tsirkin
> Sent: Thursday, June 29, 2023 3:54 PM
> > +In \field{struct virtio_virtio_admin_cmd_legacy_notify_query_entry},
> > +\field{region} is defined as following:
> > +
> > +\begin{lstlisting}
> > +struct virtio_pci_notify_region {
> > +u8 owner;/* When set to 1, notification region of the owner
> > device */
> > +u8 bar;/* BAR of the member or owner device */
> > +u8 padding[2];
> > +le32 offset; /* Offset within bar. */ }; \end{lstlisting}
>
> Is this just a legacy thing or somehow generic? If the former please stick
> _legacy_ in the struct name.
>
Yes, let me prefix legacy to it.
>
> > +
> > +The group owner device hardwire VF BAR0 in the SR-IOV Extended capability.
> > +
> > +The group member device does not use PCI BAR0 in various PCI capabilities.
>
> what does this mean? what various capabilities? VFs don't have PCI BAR0 at all
> ...
>
The PCI capabilities listed in "section 4.1.4 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 in the SR-IOV Extended capability and
> > +the group member device MUST NOT use BAR0 in any of the Virtio Structure
> PCI Capabilities.
> > +
> > +The group member driver SHOULD use the notification region supplied
> > +by the group owner driver.
> > +
> > +The group owner device or the group member device or both MAY support
> > +driver notifications region.
>
> why is this text here as opposed to the other file?
> But really I think for now it's easier to just have everything in admin-cmds-
> legacy-interface.tex
>
Because the owner/member flag is in this structure.
It was too much to split a byte out of a structure for the spirit of keeping it
generic.
Most things are in the admin-cmds-legacy-interface.tex.
Here only PCI specific structs are located.
Do you propose to move above
-
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 v7 0/4] admin: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Thursday, June 29, 2023 3:41 PM > Went over this quickly and I think this is ok from functionality perspective. > Unfortunately needs a bit more work on language clarity, grammar etc :( My > estimate would be another week to finalize. Ok. thanks a lot. I will fix current already posted comments today so that rest can be refined in v9.
[virtio-dev] RE: [PATCH v7 4/4] transport-pci: Introduce group legacy group member config region access
> From: Michael S. Tsirkin
> Sent: Thursday, June 29, 2023 3:43 PM
> > +\begin{lstlisting}
> > +struct virtio_pci_notify_region {
> > +u8 owner;/* When set to 1, notification region of the owner
> > device */
> > +u8 bar;/* BAR of the member or owner device */
> > +u8 padding[2];
> > +le32 offset; /* Offset within bar. */
>
> Let's go for a 64 bit offset straight away.
>
Ack.
> > +};
> > +\end{lstlisting}
-
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 v7 3/4] admin: Add group member legacy register access commands
> From: Michael S. Tsirkin
> Sent: Thursday, June 29, 2023 3:50 PM
> > +When command completes successfully, \field{command_specific_result}
> > +uses following structure:
> > +
> > +\begin{lstlisting}
> > +struct virtio_admin_cmd_legacy_notify_query_entry {
> > +u8 region[8];
> > +};
>
> This confuses more than it clarifies. Do this:
>
I rename region to region_data and link for the transport.
Mostly implementer will directly jump after learning this theory of operation
so, it should be ok to list in pci.
> struct virtio_admin_cmd_legacy_notify_query_entry {
> union {
> virtio_pci_notify_region region;
> };
> };
>
Yes, I thought about it, but it was pci transport listing so kept it generic.
More below.
>
> > +
> > +struct virtio_admin_cmd_legacy_notify_query_result {
> > + struct virtio_virtio_admin_cmd_legacy_notify_query_entry entries[];
> > +}; \end{lstlisting}
> > +
> > +The driver should pick the suitable entry when multiple entries are
> > +supplied by the device.
> > +
> > +Refer to the specific transport section for the definition of the
> > +\field{region}.
>
> Where? How does user know where to look? Add a link.
>
Will add the link.
>
> Or preferably I would just include that tex right here to avoid the need to
> jump
> back and forth.
>
We have vq notify config data as generic and transport specific listing,
So will improve this part of text with link.
[virtio-dev] RE: [PATCH v7 0/4] admin: Introduce legacy registers access using AQ
> From: Michael S. Tsirkin > Sent: Thursday, June 29, 2023 3:58 PM > > OK I went over it. Still some comments on the ABI. > If you can address that by next week, I will try to find the way to fix up the > wording next week. Sure, will fix it. > Problem is, when things are finally ready is when people start review in > earnest > and while I hope at least ABI wise we will not need to make changes it's > likely > that there will be comments on wording. > > If we are super lucky we can finish until end of next week but my problem is I > can't work on this 100% of my time. > Lets try, I should be able to address comments and roll next version.
[virtio-dev] Re: [PATCH v7 3/4] admin: Add group member legacy register access commands
On Thu, Jun 29, 2023 at 09:32:21PM +, Parav Pandit wrote:
>
>
> > From: Michael S. Tsirkin
> > Sent: Thursday, June 29, 2023 3:50 PM
>
> > > +When command completes successfully, \field{command_specific_result}
> > > +uses following structure:
> > > +
> > > +\begin{lstlisting}
> > > +struct virtio_admin_cmd_legacy_notify_query_entry {
> > > +u8 region[8];
> > > +};
> >
> > This confuses more than it clarifies. Do this:
> >
> I rename region to region_data and link for the transport.
> Mostly implementer will directly jump after learning this theory of operation
> so, it should be ok to list in pci.
>
> > struct virtio_admin_cmd_legacy_notify_query_entry {
> > union {
> > virtio_pci_notify_region region;
> > };
> > };
> >
> Yes, I thought about it, but it was pci transport listing so kept it generic.
> More below.
>
> >
> > > +
> > > +struct virtio_admin_cmd_legacy_notify_query_result {
> > > + struct virtio_virtio_admin_cmd_legacy_notify_query_entry entries[];
> > > +}; \end{lstlisting}
> > > +
> > > +The driver should pick the suitable entry when multiple entries are
> > > +supplied by the device.
> > > +
> > > +Refer to the specific transport section for the definition of the
> > > +\field{region}.
> >
> > Where? How does user know where to look? Add a link.
> >
> Will add the link.
>
> >
> > Or preferably I would just include that tex right here to avoid the need to
> > jump
> > back and forth.
> >
> We have vq notify config data as generic and transport specific listing,
But note how that is included directly in multiple places -
not a link. In the resulting PDF it appears inline.
> So will improve this part of text with link.
anyway, that's not part of ABI.
-
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 v7 3/4] admin: Add group member legacy register access commands
> From: Michael S. Tsirkin > Sent: Thursday, June 29, 2023 5:46 PM > > > Or preferably I would just include that tex right here to avoid the > > > need to jump back and forth. > > > > > We have vq notify config data as generic and transport specific > > listing, > > But note how that is included directly in multiple places - not a link. In the > resulting PDF it appears inline. For notify data structure is not defined, so its little simpler. Here for AQ command structure is defined in the generic section as just bytes. I have mixed feelings; I think definition in transport and link in generic section is fine. Are you ok with that? > > > So will improve this part of text with link. > > anyway, that's not part of ABI. - 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 v7 3/4] admin: Add group member legacy register access commands
On Thu, Jun 29, 2023 at 09:50:45PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Thursday, June 29, 2023 5:46 PM > > > > > Or preferably I would just include that tex right here to avoid the > > > > need to jump back and forth. > > > > > > > We have vq notify config data as generic and transport specific > > > listing, > > > > But note how that is included directly in multiple places - not a link. In > > the > > resulting PDF it appears inline. > > For notify data structure is not defined, so its little simpler. > Here for AQ command structure is defined in the generic section as just bytes. > > I have mixed feelings; I think definition in transport and link in generic > section is fine. > Are you ok with that? I am yet to focus on wording, can't tell you for sure. My gut feeling is that keeping everything in one place would be more readable, will help us converge more quickly, and the next user of admin commands is expected to be SIOV which would need the same structure anyway (it's also PCI). Also look at virtio_pci_notify_cap and check whether any normative statements there should apply here. Alignment I guess? > > > > > So will improve this part of text with link. > > > > anyway, that's not part of ABI. - 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 v7 3/4] admin: Add group member legacy register access commands
> From: Michael S. Tsirkin > Sent: Thursday, June 29, 2023 5:57 PM > > I have mixed feelings; I think definition in transport and link in generic > > section > is fine. > > Are you ok with that? > > I am yet to focus on wording, can't tell you for sure. My gut feeling is that > keeping everything in one place would be more readable, will help us converge > more quickly, and the next user of admin commands is expected to be SIOV > which would need the same structure anyway (it's also PCI). > Ok. For v8 I will have link and keep in pci transport section. Later I can move as union if needed. SIOV for PCI and CXL will look very different, so SIOV legacy I am not expecting any change for these commands. > Also look at virtio_pci_notify_cap and check whether any normative statements > there should apply here. Alignment I guess? I will move below normative from pci to generic. The group member driver SHOULD use the notification region supplied by the group owner 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 v7 3/4] admin: Add group member legacy register access commands
On Thu, Jun 29, 2023 at 10:02:53PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Thursday, June 29, 2023 5:57 PM > > > > I have mixed feelings; I think definition in transport and link in > > > generic section > > is fine. > > > Are you ok with that? > > > > I am yet to focus on wording, can't tell you for sure. My gut feeling is > > that > > keeping everything in one place would be more readable, will help us > > converge > > more quickly, and the next user of admin commands is expected to be SIOV > > which would need the same structure anyway (it's also PCI). > > > Ok. For v8 I will have link and keep in pci transport section. Later I can > move as union if needed. I can do that too. > SIOV for PCI and CXL will look very different, so SIOV legacy I am not > expecting any change for these commands. SIOV will need a notification structure within BAR, and it could reuse the same one. > > Also look at virtio_pci_notify_cap and check whether any normative > > statements > > there should apply here. Alignment I guess? > > I will move below normative from pci to generic. > > The group member driver SHOULD use the notification region supplied by the > group owner driver. By owner device. -- MST - 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 v7 3/4] admin: Add group member legacy register access commands
> From: Michael S. Tsirkin > Sent: Thursday, June 29, 2023 6:08 PM [..] > > Ok. For v8 I will have link and keep in pci transport section. Later I can > > move as > union if needed. > > I can do that too. > Ok. If you can merge the first two editorial fixes, I can drop them from v8. I didn't attach github issue in those first two patches. > > SIOV for PCI and CXL will look very different, so SIOV legacy I am not > > expecting > any change for these commands. > > SIOV will need a notification structure within BAR, and it could reuse the > same > one. > > > > Also look at virtio_pci_notify_cap and check whether any normative > > > statements there should apply here. Alignment I guess? > > > > I will move below normative from pci to generic. > > > > The group member driver SHOULD use the notification region supplied by the > group owner driver. > > By owner device. > Ok. > -- > MST - 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: [virtio-comment] RE: [PATCH v19] virtio-net: support inner header hash
On Thu, Jun 29, 2023 at 04:59:28PM +, Parav Pandit wrote:
>
>
> > From: Michael S. Tsirkin
> > Sent: Thursday, June 29, 2023 7:48 AM
>
>
> > > > struct virtio_net_hash_config reserved is fine.
> > >
> > > +1.
> > >
> > > Inner header hash is orthogonal to RSS, and it's fine to have its own
> > > structure and commands.
> > > There is no need to send additional RSS fields when we configure inner
> > > header hash.
> > >
> > > Thanks.
> >
> > Not RSS, hash calculations. It's not critical, but I note that practically
> > you said
> > you will enable this with symmetric hash so it makes sense to me to send
> > this in
> > the same command with the key.
> >
>
> In the v19, we have,
>
> +\item[VIRTIO_NET_F_HASH_TUNNEL] Requires VIRTIO_NET_F_CTRL_VQ along with
> VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
>
> So it is done along with rss, so in same struct as rss config is fine.
Do you mean having both virtio_net_rss_config and virtio_net_hash_config
have enabled_hash_types?
Like this:
struct virtio_net_rss_config {
le32 hash_types;
le16 indirection_table_mask;
struct rss_rq_id unclassified_queue;
struct rss_rq_id indirection_table[indirection_table_length];
le16 max_tx_vq;
u8 hash_key_length;
u8 hash_key_data[hash_key_length];
+le32 enabled_tunnel_types;
};
struct virtio_net_hash_config {
le32 hash_types;
-le16 reserved[4];
+le32 enabled_tunnel_types;
+le16 reserved[2];
u8 hash_key_length;
u8 hash_key_data[hash_key_length];
};
If yes, this should have been discussed in v10 [1] before, enabled_tunnel_types
in virtio_net_rss_config
will follow the variable length field and cause misalignment.
If we let the inner header hash reuse the virtio_net_hash_config structure, it
can work, but the only disadvantage
is that the configuration of the inner header hash and *RSS*(not hash
calculations) becomes somewhat coupled.
Just imagine:
If the driver and the device negotiated VIRTIO_NET_F_HASH_TUNNEL and
VIRTIO_NET_F_RSS, but did not negotiate VIRTIO_NET_F_HASH_REPORT,
1. then if we only want to configure the inner header hash (such as
enabled_tunnel_types), it is good for us to send
virtio_net_hash_config alone;
2. but then if we want to configure the inner header hash and RSS (such as
indirection table), we need to send all
virtio_net_rss_config and virtio_net_hash_config once, because
virtio_net_rss_config now does not carry enabled_tunnel_types
due to misalignment.
So, I think the following structure will make it clearer to configure inner
header hash and RSS/hash calculation.
But in any case, if we still propose to reuse virtio_net_hash_config proposal,
I am ok, no objection:
1. The supported_tunnel_types are placed in the device config space;
2.
Reserve the following structure:
struct virtnet_hash_tunnel {
le32 enabled_tunnel_types;
};
3. Reserve the SET command for enabled_tunnel_types and remove the GET
command for enabled_tunnel_types.
[1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00317.html
Thanks a lot!
-
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: [virtio-comment] RE: [PATCH v19] virtio-net: support inner header hash
> From: Heng Qi
> Sent: Thursday, June 29, 2023 8:54 PM
>
> On Thu, Jun 29, 2023 at 04:59:28PM +, Parav Pandit wrote:
> >
> >
> > > From: Michael S. Tsirkin
> > > Sent: Thursday, June 29, 2023 7:48 AM
> >
> >
> > > > > struct virtio_net_hash_config reserved is fine.
> > > >
> > > > +1.
> > > >
> > > > Inner header hash is orthogonal to RSS, and it's fine to have its
> > > > own structure and commands.
> > > > There is no need to send additional RSS fields when we configure
> > > > inner header hash.
> > > >
> > > > Thanks.
> > >
> > > Not RSS, hash calculations. It's not critical, but I note that
> > > practically you said you will enable this with symmetric hash so it
> > > makes sense to me to send this in the same command with the key.
> > >
> >
> > In the v19, we have,
> >
> > +\item[VIRTIO_NET_F_HASH_TUNNEL] Requires VIRTIO_NET_F_CTRL_VQ
> along with VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
> >
> > So it is done along with rss, so in same struct as rss config is fine.
>
> Do you mean having both virtio_net_rss_config and virtio_net_hash_config
> have enabled_hash_types?
> Like this:
>
> struct virtio_net_rss_config {
> le32 hash_types;
> le16 indirection_table_mask;
> struct rss_rq_id unclassified_queue;
> struct rss_rq_id indirection_table[indirection_table_length];
> le16 max_tx_vq;
> u8 hash_key_length;
> u8 hash_key_data[hash_key_length];
> +le32 enabled_tunnel_types;
> };
>
> struct virtio_net_hash_config {
> le32 hash_types;
> -le16 reserved[4];
> +le32 enabled_tunnel_types;
> +le16 reserved[2];
> u8 hash_key_length;
> u8 hash_key_data[hash_key_length];
> };
>
>
> If yes, this should have been discussed in v10 [1] before,
> enabled_tunnel_types
> in virtio_net_rss_config will follow the variable length field and cause
> misalignment.
>
> If we let the inner header hash reuse the virtio_net_hash_config structure, it
> can work, but the only disadvantage is that the configuration of the inner
> header hash and *RSS*(not hash calculations) becomes somewhat coupled.
> Just imagine:
> If the driver and the device negotiated VIRTIO_NET_F_HASH_TUNNEL and
> VIRTIO_NET_F_RSS, but did not negotiate VIRTIO_NET_F_HASH_REPORT, 1.
> then if we only want to configure the inner header hash (such as
> enabled_tunnel_types), it is good for us to send virtio_net_hash_config alone;
> 2. but then if we want to configure the inner header hash and RSS (such as
> indirection table), we need to send all virtio_net_rss_config and
> virtio_net_hash_config once, because virtio_net_rss_config now does not carry
> enabled_tunnel_types due to misalignment.
>
> So, I think the following structure will make it clearer to configure inner
> header
> hash and RSS/hash calculation.
> But in any case, if we still propose to reuse virtio_net_hash_config
> proposal, I
> am ok, no objection:
>
> 1. The supported_tunnel_types are placed in the device config space;
>
Yes. I forgot the variable length part.
The second disadvantage I remember now is one need to resupply all the rss hash
config parameter and device needs to compare and modify for this one field.
Given these two disadvantages, I also prefer independent SET command the way
you have it.
> 2.
> Reserve the following structure:
>
> struct virtnet_hash_tunnel {
> le32 enabled_tunnel_types;
> };
>
> 3. Reserve the SET command for enabled_tunnel_types and remove the GET
> command for enabled_tunnel_types.
>
> [1] https://lists.oasis-open.org/archives/virtio-dev/202303/msg00317.html
>
> Thanks a lot!
-
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 v8 2/4] admin: Fix section numbering
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(-)
diff --git a/admin.tex b/admin.tex
index e51f9e6..fd3b97d 100644
--- a/admin.tex
+++ b/admin.tex
@@ -286,7 +286,7 @@ \subsection{Group administration commands}\label{sec:Basic
Facilities of a Virti
supporting multiple group types, the list of supported commands
might differ between different group types.
-\devicenormative{\paragraph}{Group administration commands}{Basic Facilities
of a Virtio Device / Device groups / Group administration commands}
+\devicenormative{\subsubsection}{Group administration commands}{Basic
Facilities of a Virtio Device / Device groups / Group administration commands}
The device MUST validate \field{opcode}, \field{group_type} and
\field{group_member_id}, and if any of these has an invalid or
@@ -378,7 +378,7 @@ \subsection{Group administration commands}\label{sec:Basic
Facilities of a Virti
\field{VF Enable} refer to registers within the SR-IOV Extended
Capability as specified by \hyperref[intro:PCIe]{[PCIe]}.
-\drivernormative{\paragraph}{Group administration commands}{Basic Facilities
of a Virtio Device / Device groups / Group administration commands}
+\drivernormative{\subsubsection}{Group administration commands}{Basic
Facilities of a Virtio Device / Device groups / Group administration commands}
The driver MAY discover whether device supports a specific group type
by issuing VIRTIO_ADMIN_CMD_LIST_QUERY with the matching
--
2.26.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] [PATCH v8 0/4] admin: Access legacy registers using admin commands
MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 split rows of admin opcode tables by a line patch-2 fix section numbering patch-3 add generic legacy region access commands patch-4 add pci specific definition Iadministation t uses the newly introduced administration command facility with 4 new commands which uses the existing virtio_admin_cmd and a new command to query the legacy notification region. Usecase: 1. A hypervisor/system needs to provide transitional virtio devices to the guest VM at scale of thousands, typically, one to eight devices per VM. 2. A hypervisor/system needs to provide such devices using a vendor agnostic driver in the hypervisor system. 3. A hypervisor system prefers to have single stack regardless of virtio device type (net/blk) and be future compatible with a single vfio stack using SR-IOV or other scalable device virtualization technology to map PCI devices to the guest VM. (as transitional or otherwise) Motivation/Background: -- The existing virtio transitional PCI device is missing support for PCI SR-IOV based devices. Currently it does not work beyond PCI PF, or as software emulated device in reality. Currently it has below cited system level limitations: [a] PCIe spec citation: VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. [b] cpu arch citiation: Intel 64 and IA-32 Architectures Software Developer’s Manual: The processor’s I/O address space is separate and distinct from the physical-memory address space. The I/O address space consists of 64K individually addressable 8-bit I/O ports, numbered 0 through H. [c] PCIe spec citation: If a bridge implements an I/O address range,...I/O address range will be aligned to a 4 KB boundary. Overview: - Above usecase requirements is solved by PCI PF group owner accessing its group member PCI VFs legacy registers using an admin virtqueue of the group owner PCI PF. Two new admin virtqueue commands are added which read/write PCI VF registers. Software usage example: --- One way to use and map to the guest VM is by using vfio driver framework in Linux kernel. +--+ |pci_dev_id = 0x100X | +---|pci_rev_id = 0x0 |-+ |vfio device|BAR0 = I/O region | | | |Other attributes | | | +--+ | || + +--+ +-+ | | |I/O BAR to AQ | | Other vfio | | | |rd/wr mapper | | functionalities | | | +--+ +-+ | || +--+-+---+ | | Legacy regionDriver notification access | | | +++ +++ | +-+ | | PCI VF device A | | | AQ |-+>+-+ | | +-+ | | | | legacy regs | | | PCI PF device | | | +-+ | +-+ | +-+ | | +++ | | PCI VF device N | +>+-+ | | | legacy regs | | | +-+ | +-+ 2. Virtio pci driver to bind to the listed device id and use it in the host. 3. Use it in a light weight hypervisor to run bare-metal OS. Please review. Alternatives considered: 1. Exposing BAR0 as MMIO BAR that follows legacy registers template Pros: a. Kind of works with legacy drivers as some of them have used API which is agnostic to MMIO vs IOBAR. b. Does not require hypervisor intervantion Cons: a. Device reset is extremely hard to implement in device at scale as driver does not wait for device reset completion b. Device register width related problems persist that hypervisor if wishes, it cannot be fixed. 2. Accessing VF registers by tunneling it through new legacy PCI capability Pros: a. Self contained, but cannot work with future PCI SIOV devices Cons: a. Equally slo
[virtio-dev] [PATCH v8 1/4] admin: Split opcode table rows with a line
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(+)
diff --git a/admin.tex b/admin.tex
index 2efd4d7..e51f9e6 100644
--- a/admin.tex
+++ b/admin.tex
@@ -114,7 +114,9 @@ \subsection{Group administration commands}\label{sec:Basic
Facilities of a Virti
opcode & Name & Command Description \\
\hline \hline
0x & VIRTIO_ADMIN_CMD_LIST_QUERY & Provides to driver list of commands
supported for this group type\\
+\hline
0x0001 & VIRTIO_ADMIN_CMD_LIST_USE & Provides to device list of commands used
for this group type \\
+\hline
0x0002 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}\\
\hline
0x8000 - 0x & - & Reserved for future commands (possibly using a different
structure)\\
--
2.26.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] [PATCH v8 4/4] transport-pci: Introduce group legacy group member config region access
This patch links how in a PCI transport a group owner can access group
member (PCI VFs) legacy registers using a legacy registers access
commands using administration virtqueue infrastructure.
Additionally it extend the PCI notification capability through which a
PCI VF device indicates to the driver which PCI BAR region to be used
for driver notifications.
Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
Signed-off-by: Parav Pandit
---
changelog:
v7->v8:
- addressed comments from Michael
- made bar offset 64-bit
- prefix legacy specific structure with _legacy
- moved generic normative from pci to generic section
- added link to virtio pci capabilities when referring to bar 0
- remove 'should' from generic description
v6->v7:
- addressed comments from Michael
- removed driver normative about I/O BAR emulation as it does not
make much sense for the spec
- removed references to administration virtqueue
- rewrote driver legacy region access without guest and hypervisor
wording
- added normative for notification region
- added normative for PCI IDs for device which support legacy commands
v5->v6:
- aligned pci capability to 4B as required by PCI spec
- added text for the PCI capability for the group member device
v4->v5:
- split pci transport and generic command section to new patch
- removed multiple references to the VF
- written the description of the command as generic with member
and group device terminology
- reflected many section names to remove VF
v3->v4:
- moved noted to the conformance section details in next patch
- removed queue notify address query AQ command on Michael's suggestion,
though it is fine. Instead replaced with extending virtio_pci_notify_cap
to indicate that legacy queue notifications can be done on the
notification location.
- fixed spelling errors.
- replaced administrative virtqueue to administration virtqueue
- added queue notification capability register to indicate legacy q
notification supported
v2->v3:
- adddressed Jason and Michael's comment to split single register
access command to common config and device specific commands.
- dropped the suggetion to introduce enable/disable command as
admin command cap bit already covers it.
v1->v2:
- addressed comments from Michael
- added theory of operation
- grammar corrections
- removed group fields description from individual commands as
it is already present in generic section
- added endianness normative for legacy device registers region
- renamed the file to drop vf and add legacy prefix
- added overview in commit log
- renamed subsection to reflect command
---
admin-cmds-legacy-interface.tex | 4 ++--
conformance.tex | 1 +
transport-pci-legacy-regs.tex | 41 +
transport-pci.tex | 2 ++
4 files changed, 46 insertions(+), 2 deletions(-)
create mode 100644 transport-pci-legacy-regs.tex
diff --git a/admin-cmds-legacy-interface.tex b/admin-cmds-legacy-interface.tex
index 8036a0b..f5d7d08 100644
--- a/admin-cmds-legacy-interface.tex
+++ b/admin-cmds-legacy-interface.tex
@@ -149,11 +149,11 @@ \subsubsection{Legacy Interfaces}\label{sec:Basic
Facilities of a Virtio Device
};
\end{lstlisting}
-The driver should pick the suitable entry when multiple entries are supplied
+The driver picks the suitable entry when multiple entries are supplied
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}.
\devicenormative{\paragraph}{Legacy Interface}{Basic Facilities of a Virtio
Device / Device groups / Group administration commands / Legacy Interface}
diff --git a/conformance.tex b/conformance.tex
index dc00e84..b3f2c92 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -267,6 +267,7 @@ \section{Conformance Targets}\label{sec:Conformance /
Conformance Targets}
\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI
Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus /
PCI-specific Initialization And Device Operation / Device Initialization /
Virtio Device Configuration Layout Detection / Legacy Interface: A Note on
Device Layout Detection}
\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus /
PCI-specific Initialization And Device Operation / Device Initialization /
Virtqueue Configuration / Legacy Interface: A Note on Virtqueue Configuration}
+\item Section \ref{devicenormative:Virtio Transport Options / Virtio Over PCI
Bus / Legacy Interface: Group Member Device Configuration Region Access}
\item Section \ref{sec:Virtio Transport Options / Virtio Over MMIO / Legacy
interface}
\item Section \ref{sec:Virtio Transport Options / Virtio over channel I/O /
Device Ini
[virtio-dev] [PATCH v8 3/4] admin: Add group member legacy register access commands
Introduce group member legacy common configuration and legacy device configuration access read/write commands. Group member legacy registers access commands enable group owner driver software to access legacy registers on behalf of the guest virtual machine. Usecase: 1. A hypervisor/system needs to provide transitional virtio devices to the guest VM at scale of thousands, typically, one to eight devices per VM. 2. A hypervisor/system needs to provide such devices using a vendor agnostic driver in the hypervisor system. 3. A hypervisor system prefers to have single stack regardless of virtio device type (net/blk) and be future compatible with a single vfio stack using SR-IOV or other scalable device virtualization technology to map PCI devices to the guest VM. (as transitional or otherwise) Motivation/Background: = The existing virtio transitional PCI device is missing support for PCI SR-IOV based devices. Currently it does not work beyond PCI PF, or as software emulated device in reality. Currently it has below cited system level limitations: [a] PCIe spec citation: VFs do not support I/O Space and thus VF BARs shall not indicate I/O Space. [b] cpu arch citiation: Intel 64 and IA-32 Architectures Software Developer’s Manual: The processor’s I/O address space is separate and distinct from the physical-memory address space. The I/O address space consists of 64K individually addressable 8-bit I/O ports, numbered 0 through H. [c] PCIe spec citation: If a bridge implements an I/O address range,...I/O address range will be aligned to a 4 KB boundary. Overview: = Above usecase requirements is solved by PCI PF group owner accessing its group member PCI VFs legacy registers using the administration commands of the group owner PCI PF. Two types of administration commands are added which read/write PCI VF registers. Software usage example: === 1. One way to use and map to the guest VM is by using vfio driver framework in Linux kernel. +--+ |pci_dev_id = 0x100X | +---|pci_rev_id = 0x0 |-+ |vfio device|BAR0 = I/O region | | | |Other attributes | | | +--+ | || + +--+ +-+ | | |I/O BAR to AQ | | Other vfio | | | |rd/wr mapper& | | functionalities | | | | forwarder| | | | | +--+ +-+ | || +--+-+---+ | | Config region | accessDriver notifications | | +++ +++ | +-+ | | PCI VF device A | | | AQ |-+>+-+ | | +-+ | | | | legacy regs | | | PCI PF device | | | +-+ | +-+ | +-+ | | +++ | | PCI VF device N | +>+-+ | | | legacy regs | | | +-+ | +-+ 2. Continue to use the virtio pci driver to bind to the listed device id and use it as in the host. 3. Use it in a light weight hypervisor to run bare-metal OS. Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167 Signed-off-by: Parav Pandit --- changelog: v7->v8: - remove empty line at the end of file - removed white space at the end - addressed comments from Michael add link to pci - renamed region to region_data - made region_data width to be 16 bytes to cover for 8 bytes offset - moved generic notification region related normative from pci to generic section v6->v7: - changed administrative to administration - renamed admin-access.tex to admin-interface.tex - large rewrite ad generic admin commands instead of pci - added theory of operation section - added driver notification region query command v5->v6: - fixed previous missed abbreviation of LCC and LD v4->v5: - split from pci transport specific patch - split conformance to transport and generic sections - written the description of the command as generic with member and group device terminology - reflected many section names to remove VF - rename fields from register to region - avoided abbreviation for legacy, device and config --- admin-cmds-legacy-interface.tex | 206 admin.tex | 14 ++- conformance.tex | 2 + 3 files changed, 221 insertions(+), 1 deletion(-) create mode 100644 admin-cmds-legacy-interface.tex diff --git a/admin-cmds-legacy-interface.tex b/admin-cmds-l
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] RE: [virtio-comment] RE: [PATCH v19] virtio-net: support inner header hash
On Fri, Jun 30, 2023 at 09:55:41AM +0800, Heng Qi wrote:
>
>
> 在 2023/6/30 上午9:36, Parav Pandit 写道:
> >
> > > From: Heng Qi
> > > Sent: Thursday, June 29, 2023 8:54 PM
> > >
> > > On Thu, Jun 29, 2023 at 04:59:28PM +, Parav Pandit wrote:
> > > >
> > > > > From: Michael S. Tsirkin
> > > > > Sent: Thursday, June 29, 2023 7:48 AM
> > > >
> > > > > > > struct virtio_net_hash_config reserved is fine.
> > > > > > +1.
> > > > > >
> > > > > > Inner header hash is orthogonal to RSS, and it's fine to have its
> > > > > > own structure and commands.
> > > > > > There is no need to send additional RSS fields when we configure
> > > > > > inner header hash.
> > > > > >
> > > > > > Thanks.
> > > > > Not RSS, hash calculations. It's not critical, but I note that
> > > > > practically you said you will enable this with symmetric hash so it
> > > > > makes sense to me to send this in the same command with the key.
> > > > >
> > > > In the v19, we have,
> > > >
> > > > +\item[VIRTIO_NET_F_HASH_TUNNEL] Requires VIRTIO_NET_F_CTRL_VQ
> > > along with VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
> > > > So it is done along with rss, so in same struct as rss config is fine.
> > > Do you mean having both virtio_net_rss_config and virtio_net_hash_config
> > > have enabled_hash_types?
> > > Like this:
> > >
> > > struct virtio_net_rss_config {
> > > le32 hash_types;
> > > le16 indirection_table_mask;
> > > struct rss_rq_id unclassified_queue;
> > > struct rss_rq_id indirection_table[indirection_table_length];
> > > le16 max_tx_vq;
> > > u8 hash_key_length;
> > > u8 hash_key_data[hash_key_length];
> > > +le32 enabled_tunnel_types;
> > > };
> > >
> > > struct virtio_net_hash_config {
> > > le32 hash_types;
> > > -le16 reserved[4];
> > > +le32 enabled_tunnel_types;
> > > +le16 reserved[2];
> > > u8 hash_key_length;
> > > u8 hash_key_data[hash_key_length];
> > > };
Oh, I forgot that rss and hash had identical structures.
And we want to keep that I think.
But now it's not clear to me: does the same enabled_tunnel_types
apply to both hash calculation and rss?
I note we normally have separate configs for hash and rss.
So to keep it consistent what should we do? two set commands?
Or does enabled_tunnel_types apply to both rss and hash?
We should have reserved more space. We can still do it if you like:
struct virtio_net_rss_tunnel_config {
le32 enabled_tunnel_types;
le16 reserved[6];
struct virtio_net_rss_config hash;
};
struct virtio_net_hash_tunnel_config {
le32 enabled_tunnel_types;
le16 reserved[6];
struct virtio_net_hash_config hash;
};
?
> > >
> > >
> > > If yes, this should have been discussed in v10 [1] before,
> > > enabled_tunnel_types
> > > in virtio_net_rss_config will follow the variable length field and cause
> > > misalignment.
> > >
> > > If we let the inner header hash reuse the virtio_net_hash_config
> > > structure, it
> > > can work, but the only disadvantage is that the configuration of the inner
> > > header hash and *RSS*(not hash calculations) becomes somewhat coupled.
> > > Just imagine:
> > > If the driver and the device negotiated VIRTIO_NET_F_HASH_TUNNEL and
> > > VIRTIO_NET_F_RSS, but did not negotiate VIRTIO_NET_F_HASH_REPORT, 1.
> > > then if we only want to configure the inner header hash (such as
> > > enabled_tunnel_types), it is good for us to send virtio_net_hash_config
> > > alone;
> > > 2. but then if we want to configure the inner header hash and RSS (such as
> > > indirection table), we need to send all virtio_net_rss_config and
> > > virtio_net_hash_config once, because virtio_net_rss_config now does not
> > > carry
> > > enabled_tunnel_types due to misalignment.
> > >
> > > So, I think the following structure will make it clearer to configure
> > > inner header
> > > hash and RSS/hash calculation.
> > > But in any case, if we still propose to reuse virtio_net_hash_config
> > > proposal, I
> > > am ok, no objection:
> > >
> > > 1. The supported_tunnel_types are placed in the device config space;
> > >
> > Yes. I forgot the variable length part.
> > The second disadvantage I remember now is one need to resupply all the rss
> > hash config parameter and device needs to compare and modify for this one
> > field.
Or it could be an advantage since one normally wants to configure a
symmetric key with this. Further device can just use the new config
with no need to check what the old one was. I'd call it a wash.
> > Given these two disadvantages, I also prefer independent SET command the
> > way you have it.
>
> OK, let's wait for Michael's input again.
>
> Thanks.
This part is not critical to me, but now I understand we need two sets of SET
commands.
> > > 2.
> > > Reserve the following structure:
> > >
> > >struct virtnet_hash_tunnel {
> > > le32 enabled_tunnel_types;
> > >};
> > >
> > > 3. Reserve the SET command for enabl
