On 2/13/2023 4:12 PM, Michael S. Tsirkin wrote:
On Mon, Feb 13, 2023 at 12:49:44AM +0200, Max Gurtovoy wrote:
On 12/02/2023 22:19, Michael S. Tsirkin wrote:
On Sun, Feb 12, 2023 at 04:34:09PM +0200, Max Gurtovoy wrote:
On 12/02/2023 15:15, Michael S. Tsirkin wrote:
On Sun, Feb 12, 2023 at 02:10:29PM +0200, Max Gurtovoy wrote:
On 09/02/2023 21:58, Michael S. Tsirkin wrote:
On Thu, Feb 09, 2023 at 07:47:56PM +0200, Max Gurtovoy wrote:
On 09/02/2023 17:22, David Edmondson wrote:
On Thursday, 2023-02-09 at 10:13:46 -05, Michael S. Tsirkin wrote:
On Thu, Feb 09, 2023 at 03:00:58PM +0000, David Edmondson wrote:
On Thursday, 2023-02-09 at 07:13:36 -05, Michael S. Tsirkin
wrote:
Each device group has a type. For now, define one initial
group:
SR-IOV type - PCI SR-IOV virtual functions (VFs) of a given
PCI SR-IOV physical function (PF). This group may contain
one or more
virtio devices.
The specification says zero or more generally and nothing to
refine that
for SR-IOV groups.
Well it says:
(SR-IOV) physical function (PF) device as the owner and
includes
all its SR-IOV virtual functions (VFs) as members (see
\hyperref[intro:PCIe]{[PCIe]}).
how can that be zero?
If I remove all of the VFs of a PF, does the group implicitly disappear?
Enable/Disable SR-IOV are dynamic operations controlled by NumVFs register.
Are you sure? Not by VF Enable?
Right. It's a combination of the VF_Enable and numVfs.
...
"The NumVFs field defines the number of VFs that are enabled when VF Enable
is Set in the associated PF."
We can mention that a group include all virtual functions indicated by TotalVFs
RO register.
TotalVFs indicates the maximum number of VFs that could be associated with the
PF
but not all of these exist. If we use this we get into issues of
discovering whether VFs can actually be used. I'd rather avoid that.
This group exist as long as the owner PF supports SR-IOV and has admin
capabilities.
SRIOV spec says:
NumVFs controls the number of VFs that are visible.
And one can not play with NumVFs dynamically:
NumVFs may only be written while VF Enable is Clear. If NumVFs is
written when VF Enable is
Set, the results are undefined.
one has to kill all VFs then enable them all again.
As long as we are doing that, we can just unload the driver.
what driver to unload ? Not sure I'm following..
Basically I am saying that if VFs are disabled this destroys all VFs
so it is not
clear that it makes sense to keep some of the configuration
for previous VFs around.
My idea is to say that reset is required to disable VFs.
which reset ?
VFs disabled/enabled using VF_enable PCI register.
Lets follow that please.
Lets define what is happening during VF_enable = 1 and VF_enable = 0.
On VF_enable = 0 --> 1 transition the corresponding device_group, that is
owned by the PF, grows and contains NumVFs members with initial features and
resources.
On VF_enable = 1 --> 0 transition the corresponding device_group, that is
owned by the PF, shrinks and contains 0 members and all the dynamic
resources are back to the owner.
Maybe, it's a possible approach. As current patch set has no
resources I feel we can leave this part to a patch on top
just so that this one can move forward while we have
a more specific example to work from.
we can differ the resources part but lets add the part that the group
becomes empty/non-empty according to the disable/enable of VF_enable
register as mentioned above.
Hmm. Another option is VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP when VF_enable
is clear - since this disables the whole group not a specific member.
This allows distinguishing between id > NumVFs and VF_enable == 0
so it seems like a nicer option to me.
How to detect whether VF_enable set to 0 or 1, pulling is not a good
idea, and do we want
SRIOV cap to notify the admin vq on changes?
Thanks
So I would say we should just ask that VF Enable is set
before poking at admin vq, and prohibit clearing VF Enable.
Sending admin commands shouldn't be restricted and should be allowed always.
Device should react to incoming command.
We can say that an SR-IOV group is empty while the VF enable is unset and is
not empty when it's set. The size of the non empty SR-IOV group is equal to
the numVFs and the VF index is equal to the member identifier in this group.
I had this definition in my patches.
Sending admin command to a non existing member will result in an ERROR
VIRTIO_ADMIN_STATUS_Q_INVALID_MEM.
just an example:
- assign resource X to VF 16
- set NumVFs to 8
Does VF 16 keep resource X? I note that
resource X can not be taken back because VF 16
is not accessible.
all dynamic resources should return to the owner when VF_enable moves 1 -->
0
It's a possible approach but my question would be whether we
need a transport independent way to do this through VQ.
And if we do it's annoying to have multiple ways.
We are defining the existence of the SR-IOV group.
Also, the approach of sending commands to a non-existing member is valid for
any group type and should return error VIRTIO_ADMIN_STATUS_Q_INVALID_MEM in
that case.
why do we need more ways ?
Sorry I mean a transport independent way to destroy a group as a whole.
For sure, all this is solvable but I'd rather just forbid
changes to VF enable for now.
how we can forbid it ? setting this register is not related to virtio.
We can ask driver to write 0 to device reset before changing VF
enable to 0. The advantage is that this kind of reset by definition
frees up all resources.
why do we need to reset the device ? It is fully functional and should be
able to run perfectly fine without any need to reset.
The PF device might have a very big role in the hypervisor, so restricting
it in this way doesn't sounds mandatory.
Controlling the SR-IOV group using SR-IOV register enable/disable and NumVFs
used as the number of the group size sounds simple from Spec and
implementation POV.
Simple, however
- only works for SRIOV
- also limited, for example it's impossible to report an error
I would rather not decide now until we have a use-case.
There's also the concern of VF migration under MRIOV where VFs might
not exit. I'm guessing
trying to include that right now is overthinking. So maybe
this should mention that VF Migration Capable should be clear.
I will add all this to the spec.
How about the converse?
Each device within a group has a unique identifier. This
identifier
is the group member identifier.
Note: one can argue both ways whether the new device group
handling
functionality (this and following patches) is closer
to a new device type or a new transport type.
However, I expect that we will add more features in the
near future. To
facilitate this as much as possible of the text is located
in the new
admin chapter.
I did my best to minimize transport-specific text.
Signed-off-by: Max Gurtovoy <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
---
admin.tex | 49
+++++++++++++++++++++++++++++++++++++++++++++++++
content.tex | 2 ++
2 files changed, 51 insertions(+)
create mode 100644 admin.tex
diff --git a/admin.tex b/admin.tex
new file mode 100644
index 0000000..2bc7322
--- /dev/null
+++ b/admin.tex
@@ -0,0 +1,49 @@
+\section{Device groups}\label{sec:Basic Facilities of a
Virtio Device / Device groups}
+
+It is occasionally useful to have a device control a group
of
+other devices. Terminology used in such cases:
+
+\begin{description}
+\item[Device group]
+ or just group, includes zero or more devices.
+\item[Owner device]
+ or owner, the device controlling the group.
+\item[Member device]
+ a device within a group. The owner device itself
is not
+ a member of the group.
+\item[Member identifier]
+ each member has this identifier, unique within the
group
+ and used to address it through the owner device.
+\item[Group type identifier]
+ specifies what kind of member devices there are in a
+ group, how is the member identifier is interpreted
+ and what kind of control the owner has.
+ A given owner can control a single group of a given
type,
+ thus the type and the owner together identify the
group.
+ \footnote{Even though some group types only support
+ specific transports, group type
identifiers
+ are global rather than
transport-specific -
+ we don't expect a flood of new
group types.}
+\end{description}
+
+The following group types, and their identifiers, are
currently specified):
+\begin{description}
+\item[SR-IOV group type (0x1)]
+This device group has a PCI Single Root I/O Virtualization
+(SR-IOV) physical function (PF) device as the owner and
includes
+all its SR-IOV virtual functions (VFs) as members (see
+\hyperref[intro:PCIe]{[PCIe]}).
+
+The PF device itself is not a member of the group.
+
+The group type identifier for this group is 0x1.
+
+A member identifier for this group can have a value 0x1 to
0xFFFF
+and equals the SR-IOV VF number of the member device (see
+\hyperref[intro:PCIe]{[PCIe]}).
+
+Both owner and member devices for this group type use the
Virtio
+PCI transport (see \ref{sec:Virtio Transport Options /
Virtio Over PCI Bus}).
+\end{description}
+
+
diff --git a/content.tex b/content.tex
index 0c2d917..ffe45c4 100644
--- a/content.tex
+++ b/content.tex
@@ -491,6 +491,8 @@ \section{Exporting
Objects}\label{sec:Basic Facilities of a Virtio Device / Expo
types. It is RECOMMENDED that devices generate version 4
UUIDs as specified by \hyperref[intro:rfc4122]{[RFC4122]}.
+\input{admin.tex}
+
\chapter{General Initialization And Device
Operation}\label{sec:General Initialization And Device Operation}
We start with an overview of device initialization, then
expand on the
--
Tell her I'll be waiting, in the usual place.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
