RE: [virtio-dev] [PATCH v13] admin: Add group member legacy register access commands
> From: Michael S. Tsirkin
> Sent: Monday, July 10, 2023 11:17 AM
> > > From: Michael S. Tsirkin
> > > Sent: Sunday, July 9, 2023 9:47 AM
> >
> >
> > > > +If within \field{struct
> > > > +virtio_admin_cmd_legacy_notify_info_result}
> > > > +returned by VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO, the
> > > > +\field{flags} value for a specific \field{struct
> > > > +virtio_pci_legacy_notify_info} entry is 0x0, the driver MUST
> > > > +ignore this entry and all the following \field{entries}.
> >
> > Please see above, it appears to me that it covers what you asked for, no?
> > What is missing?
>
> Oh, my bad. I agree. following stresses this a bit more:
>
>
> is 0x0, then the driver MUST ignore this entry as well as all the following
> \field{entries} until the end of the structure.
>
>
> > > The driver MUST additionally validate, for each
> > > > +entry, that \begin{itemize} \item the \field{flags} is either
> > > > +0x0,
> > > > +0x1 or 0x2 \item the \field{bar} corresponds to a valid BAR of
> > > > +either the owner or the member device, depending on the
> > > > +\field{flags} \item the \field{offset} is 2-byte aligned and
> > > > +corresponds to an address within the BAR specified by the
> > > > +\field{bar} on \field{flags} \end{itemize}, any entry which does
> > > > +not meet these constraints MUST be ignored by the driver.
> > >
> > > Wait a second.
> > > - end of list not documented - we want conformance statements
> > > - for end of list I am guessing other fields do not matter
> > > - entries after end of list also do not matter?
> >
> > I reread the above driver conformance statement.
> > It already indicates what you asked.
>
> So the above two do not contradict. They kind of seem to since 1st one says
> ignore second says validate, until you look closely and see that the result of
> validation is to ignore.
> I suggest we clarify it's other entries not each entry:
>
> MUST ignore this entry and all the following \field{entries}.
>
> Additionally, for all other entries, the driver MUST validate
> that \begin{itemize} \item the \field{flags} is either
> 0x1 or 0x2
Sounds good. Revising it.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH v13] admin: Add group member legacy register access commands
On Mon, Jul 10, 2023 at 01:05:34PM +, Parav Pandit wrote:
>
> > From: Michael S. Tsirkin
> > Sent: Sunday, July 9, 2023 9:47 AM
>
>
> > > +If within \field{struct virtio_admin_cmd_legacy_notify_info_result}
> > > +returned by VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO, the \field{flags}
> > > +value for a specific \field{struct virtio_pci_legacy_notify_info}
> > > +entry is 0x0, the driver MUST ignore this entry and all the following
> > > +\field{entries}.
>
> Please see above, it appears to me that it covers what you asked for, no?
> What is missing?
Oh, my bad. I agree. following stresses this a bit more:
is 0x0, then the driver MUST ignore this entry as well as all
the following \field{entries} until the end of the structure.
> > The driver MUST additionally validate, for each
> > > +entry, that \begin{itemize} \item the \field{flags} is either 0x0,
> > > +0x1 or 0x2 \item the \field{bar} corresponds to a valid BAR of either
> > > +the owner or the member device, depending on the \field{flags} \item
> > > +the \field{offset} is 2-byte aligned and corresponds to an address
> > > +within the BAR specified by the \field{bar} on \field{flags}
> > > +\end{itemize}, any entry which does not meet these constraints MUST
> > > +be ignored by the driver.
> >
> > Wait a second.
> > - end of list not documented - we want conformance statements
> > - for end of list I am guessing other fields do not matter
> > - entries after end of list also do not matter?
>
> I reread the above driver conformance statement.
> It already indicates what you asked.
So the above two do not contradict. They kind of seem to since 1st
one says ignore second says validate, until you look closely
and see that the result of validation is to ignore.
I suggest we clarify it's other entries not each entry:
MUST ignore this entry and all the following \field{entries}.
Additionally, for all other entries, the driver MUST validate
that \begin{itemize} \item the \field{flags} is either
0x1 or 0x2
--
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] [PATCH v13] admin: Add group member legacy register access commands
> From: Michael S. Tsirkin
> Sent: Sunday, July 9, 2023 9:47 AM
> > +If within \field{struct virtio_admin_cmd_legacy_notify_info_result}
> > +returned by VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO, the \field{flags}
> > +value for a specific \field{struct virtio_pci_legacy_notify_info}
> > +entry is 0x0, the driver MUST ignore this entry and all the following
> > +\field{entries}.
Please see above, it appears to me that it covers what you asked for, no?
What is missing?
> The driver MUST additionally validate, for each
> > +entry, that \begin{itemize} \item the \field{flags} is either 0x0,
> > +0x1 or 0x2 \item the \field{bar} corresponds to a valid BAR of either
> > +the owner or the member device, depending on the \field{flags} \item
> > +the \field{offset} is 2-byte aligned and corresponds to an address
> > +within the BAR specified by the \field{bar} on \field{flags}
> > +\end{itemize}, any entry which does not meet these constraints MUST
> > +be ignored by the driver.
>
> Wait a second.
> - end of list not documented - we want conformance statements
> - for end of list I am guessing other fields do not matter
> - entries after end of list also do not matter?
I reread the above driver conformance statement.
It already indicates what you asked.
[virtio-dev] RE: [virtio-comment] Re: [virtio-dev] [PATCH v13] admin: Add group member legacy register access commands
> From: virtio-comm...@lists.oasis-open.org open.org> On Behalf Of Cornelia Huck
> Sent: Monday, July 10, 2023 5:38 AM
> On Fri, Jul 07 2023, Parav Pandit wrote:
>
> (...)
>
> > +When the device supports VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO
> command,
> > +the group
>
> s/supports/supports the/
>
> > +owner device hardwires VF BAR0 to zero in the SR-IOV Extended capability.
>
> (...)
>
> > +\devicenormative{\paragraph}{Legacy Interface}{Basic Facilities of a
> > +Virtio Device / Device groups / Group administration commands /
> > +Legacy Interface}
> > +
> > +A device MUST either support all of, or none of
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> > +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> > +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands.
> > +
> > +For VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
>
> s/For/For the/
>
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> > +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> > +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands, the device
> MUST decode
> > +and encode (respectively) the value of the \field{data} using the
> > +little-endian format.
> > +
> > +For VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE and
>
> s/For/For the/
>
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ commands, the
> device MUST
> > +fail the command when the value of the \field{offset} and the length
> > +of the \field{data} does not refer to a
>
> "do not", I think?
>
> > +single field or is not completely within the virtio common
> > +configuration excluding the device-specific configuration.
> > +
> > +For VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
>
> s/For/For the/
>
> > +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands, the device
> MUST fail
> > +the command when the value of the \field{offset} and the length of
> > +the \field{data} does not refer to a
>
> "do not"?
>
> > +single field or is not completely within the virtio device-specific
> > +configuration.
> > +
> > +The command VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE MUST
> have the
> > +same effect as writing into the virtio common configuration structure
> > +through the legacy interface.
> > +
> > +The command VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ MUST
> have the
> > +same effect as reading from the virtio common configuration structure
> > +through the legacy interface.
> > +
> > +The command VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE MUST have
> the same
> > +effect as writing into the virtio device-specific configuration
> > +through the legacy interface.
> > +
> > +The command VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ MUST have
> the same
> > +effect as reading from the virtio device-specific configuration
> > +through the legacy interface.
> > +
> > +For the SR-IOV group type, when the owner device supports
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> > +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ,
> > +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> > +VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO
> > +commands,
> > +\begin{itemize}
> > +\item the owner device and the group member device SHOULD follow the
> > +rules for the PCI Revision ID and Subsystem Device ID of the
> > +non-transitional devices documented in section \ref{sec:Virtio Transport
> Options / Virtio Over PCI Bus / PCI Device Discovery}.
> > +
> > +\item the owner device SHOULD follow the rules for the PCI Device ID
> > +of the non-transitional devices documented in section \ref{sec:Virtio
> > +Transport Options / Virtio Over PCI Bus / PCI Device Discovery}.
> > +
> > +\item any driver notification received by the device at any of the
> > +notification address supplied in the command result of
> > +VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO MUST function as if the device
> > +received the notification through
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE
> > +command at an offset \field{offset} matching \field{Queue Notify}.
> > +\end{itemize}
> > +
> > +If the device supports VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO
> command,
>
> s/support/supports the/
>
> > +\begin{itemize}
> > +\item the device MUST also support all of
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> > +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> > +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> > +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands.
> > +
> > +\item in the command result of
> VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO,
> > +the last \field{struct virtio_pci_legacy_notify_info} entry MUST have
> > +\field{flags} of zero.
> > +
> > +\item in the command result of
> VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO,
> > +valid entries MUST have a \field{bar} which is not hardwired to zero.
> > +
> > +\item in the command result of
> VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO,
> > +valid entries MUST have an \field{offset} aligned to 2-byte.
> > +
> > +\item the device MAY support VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO
> with
> > +entries of the owner device or the member device or both of them.
> > +
> > +\item for the SR-IOV group type,
Re: [virtio-dev] [PATCH v13] admin: Add group member legacy register access commands
On Fri, Jul 07 2023, Parav Pandit wrote:
(...)
> +When the device supports VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO command, the
> group
s/supports/supports the/
> +owner device hardwires VF BAR0 to zero in the SR-IOV Extended capability.
(...)
> +\devicenormative{\paragraph}{Legacy Interface}{Basic Facilities of a Virtio
> Device / Device groups / Group administration commands / Legacy Interface}
> +
> +A device MUST either support all of, or none of
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands.
> +
> +For VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
s/For/For the/
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands,
> +the device MUST decode and encode (respectively) the value of the
> +\field{data} using the little-endian format.
> +
> +For VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE and
s/For/For the/
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ commands,
> +the device MUST fail the command when the value of the
> +\field{offset} and the length of the \field{data} does not refer to a
"do not", I think?
> +single field or is not completely within the virtio common configuration
> +excluding the device-specific configuration.
> +
> +For VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
s/For/For the/
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands,
> +the device MUST fail the command when the value of the
> +\field{offset} and the length of the \field{data} does not refer to a
"do not"?
> +single field or is not completely within the virtio device-specific
> +configuration.
> +
> +The command VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE MUST have the same
> effect
> +as writing into the virtio common configuration structure through the legacy
> +interface.
> +
> +The command VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ MUST have the same
> effect as
> +reading from the virtio common configuration structure through the legacy
> +interface.
> +
> +The command VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE MUST have the same effect
> as
> +writing into the virtio device-specific configuration through the legacy
> +interface.
> +
> +The command VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ MUST have the same effect as
> +reading from the virtio device-specific configuration through the legacy
> +interface.
> +
> +For the SR-IOV group type, when the owner device supports
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO
> +commands,
> +\begin{itemize}
> +\item the owner device and the group member device SHOULD follow the rules
> +for the PCI Revision ID and Subsystem Device ID of the non-transitional
> devices
> +documented in section \ref{sec:Virtio Transport Options / Virtio Over PCI
> Bus / PCI Device Discovery}.
> +
> +\item the owner device SHOULD follow the rules for the PCI Device ID of the
> non-transitional
> +devices documented in section
> +\ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device
> Discovery}.
> +
> +\item any driver notification received by the device at any of the
> notification
> +address supplied in the command result of
> +VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO MUST function as if the device received
> +the notification through VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE
> +command at an offset \field{offset} matching \field{Queue Notify}.
> +\end{itemize}
> +
> +If the device supports VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO command,
s/support/supports the/
> +\begin{itemize}
> +\item the device MUST also support all of
> VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands.
> +
> +\item in the command result of VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO, the last
> +\field{struct virtio_pci_legacy_notify_info} entry MUST have \field{flags} of
> +zero.
> +
> +\item in the command result of VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO, valid
> +entries MUST have a \field{bar} which is not hardwired to zero.
> +
> +\item in the command result of VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO, valid
> +entries MUST have an \field{offset} aligned to 2-byte.
> +
> +\item the device MAY support VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO with entries
> +of the owner device or the member device or both of them.
> +
> +\item for the SR-IOV group type, the group owner device MUST hardwire VF BAR0
> +to zero in the SR-IOV Extended capability.
> +\end{itemize}
> +
> +\drivernormative{\paragraph}{Legacy Interface}{Basic Facilities of a Virtio
> Device / Device groups / Group administration commands / Legacy Interface}
> +
> +For VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
s/For/For the/
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
>
RE: [virtio-dev] [PATCH v13] admin: Add group member legacy register access commands
> From: Michael S. Tsirkin
> Sent: Sunday, July 9, 2023 9:47 AM
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
> > Signed-off-by: Parav Pandit
>
> Ugh. I have a feeling you are rewriting what I wrote so we keep doing more and
> more review rounds. I would prefer it if I could just comment instead of
> rewriting - that's really going way beyond normal review - but that seems to
> get
> us nowhere so here we are. But given this, if you don't want to use what I
> wrote
> then please comment.
>
Please see inline, why some parts are rewritten.
Only two lines to my knowledge are rewritten because you wrote,
" when the \field{flags} is 0x2 this is specified by the VF BARn"
Above "this is specified" was confusing to read.
It meant to me as that "this" refers to "bar".
So it interpreted to me as
"when flags is 0x2, bar is specified by the Base Address Registers in the PCI
...".
Here bar is not specified, bar specifies which register of which device to
refer to.
What we wanted to say that,
"bar" field here points to (specifies) the Base Address Registers in the PCI ...
So that part was rewritten.
>
> > ---
> > changelog:
> > v12->v13:
> > - added article
> > - add hyphen between little and endian
> > - mentioned vq index depth of 16-bit
> > - rewrote alternative approach line
> > - mention vq index, length and endianness in mmio description
> > - fixed padding bytes size from 7 to 6 bytes
> > - rewrote bar field description
> > - offset alignment text added
> > - added text to ignore reserved notification entries
> > - device and driver conformance lines added for notification info
> > command fields
> > - dropped group member prefix to the driver
> > - reworded text for flags requirements
> > - reworded to say all driver notifications in conformance
> > - itemize conformance entries under command to ease reading
Most of above comments were addressed as_is as you wrote of asked to write.
[..]
> > +struct virtio_admin_cmd_legacy_notify_info_result {
> > +struct virtio_pci_legacy_notify_info entries[4]; };
> > +\end{lstlisting}
> > +
> > +The \field{flags} value of 0x1 indicates that the notification
> > +address is of the owner device, value of 0x2 indicates that the
> > +notification address is of the member device, the value of 0
> > +indicates that all the entries starting from that entry are invalid
> > +entries in \field{entries}. All other values in \field{flags} are
> > +reserved. The driver skips the entries whose \field{flag} contains reserved
> value.
>
> contain reserved values.
>
Will fix this.
> > +
> > +The \field{bar} values 0x1 to 0x5 specifies a Base Address register
> > +(BAR),
>
> specify
>
Will fix this.
>
> > +when the \field{flags} is 0x1, \field{bar} specifies the Base Address
> > +Registers
>
> one of the Base Address Registers
>
> > +in the PCI header of the owner device, when the \field{flags} is 0x2,
> > +\field{bar} specifies the VF BARn registers in the SR-IOV Extended
> > +Capability of the device.
>
Will fix this.
> These attempts to clarify only confused.
> Problem with this is that VF BARn is not valled "Base Address register"
> you also did not capitalize "r" here. And also VF BARn in the capability
> actually
> refers to VF0 only. Others are calculated from that.
>
> This is why I wrote what I wrote in the previous review.
>
It is confusing, but I will take it as_is.
>
>
> > +
> > +The \field{offset} indicates the notification address relative to the
> > +base address associated with the BAR indicated in \field{bar}. This
> > +value
>
> let's not invent the new term base address. Just "relative to the BAR" should
> be
> enough.
Will fix it.
>
> > +is 2-byte aligned.
> > +
> > +When the command completes successfully,
> > +\field{command_specific_result} is in the format of \field{struct
> > +virtio_admin_cmd_legacy_notify_info_result}. The device can supply up
> > +to 4 entries each with a different notification address. In this
> > +case, any of the entries can be used by the driver. The order of the
> > +entries serves as a preference hint to the driver. The driver is
> > +expected to utilize the entries placed earlier in the array to the later
> > ones.
> The driver is also expected to ignore reserved entries.
>
>
> You need to document end of list too.
>
End of list is described where the flag values 0,1,2 are described.
Should we duplicate here?
> I wrote all of this in my previous review. Donnu why you decided to rewrite
> yet
> again but here we are.
>
I rechecked.
I didn’t find rewrite for above lines in [1].
You rewrote the conformance section.
[1] https://lists.oasis-open.org/archives/virtio-comment/202307/msg00125.html
> > +If within \field{struct virtio_admin_cmd_legacy_notify_info_result}
> > +returned by VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_INFO, the \field{flags}
> > +value for a specific \field{struct virtio_pci_legacy_notify_info}
> > +entry is 0x0, the driver MUST ignore this entry and all the
Re: [virtio-dev] [PATCH v13] admin: Add group member legacy register access commands
On Fri, Jul 07, 2023 at 06:30:19PM +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 > 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 Ugh. I have a feeling you are rewriting what I wrote so we keep doing more and more review rounds. I would prefer it if I could just comment instead of rewriting - that's really going way beyond normal review - but that seems to get us nowhere so here we are. But given this, if you don't want to use what I wrote then please comment. > --- > changelog: > v12->v13: > - added article > - add hyphen between little and endian > - mentioned vq index depth of 16-bit > - rewrote alternative approach line > - mention vq index, length and endianness in mmio description > - fixed padding bytes size from 7 to 6 bytes > - rewrote bar field description > - offset alignment text added > - added text to ignore reserved notification entries > - device and driver conformance lines added for notification info command > fields > - dropped group member prefix to the driver > - reworded text for flags requirements > - reworded to say all driver notifications in conformance > - itemize conformance entries under command to ease reading > v11->v12:
[virtio-dev] [PATCH v13] 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 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: v12->v13: - added article - add hyphen between little and endian - mentioned vq index depth of 16-bit - rewrote alternative approach line - mention vq index, length and endianness in mmio description - fixed padding bytes size from 7 to 6 bytes - rewrote bar field description - offset alignment text added - added text to ignore reserved notification entries - device and driver conformance lines added for notification info command fields - dropped group member prefix to the driver - reworded text for flags requirements - reworded to say all driver notifications in conformance - itemize conformance entries under command to ease reading v11->v12: - added missing article the at few places - rewrote group_member_id statements like other existing commands which is cleaner and shorter - added length and alignment lines to multiple commands - rewrote fast path to separate dedicated mechanism - rewrote example and description para for legacy notification command - made separate paragraph for the notify info command - dropped citation to virtio pci capabilities for member device - notification region changed to notification address throughout - added description to all the fields of the info struct - avoided union in spirit of keeping all for pci - used single listing -
