[virtio-dev] [PATCH v7 0/4] admin: Introduce legacy registers access using AQ

[virtio-dev] [PATCH v7 0/4] admin: Introduce legacy registers access using AQ

[Parav Pandit]

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

[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 region            Driver 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 slow as AQ access
b. Still requires new capability for notification access
c. Requires hardware to build low level registers access which is not worth
   for long term future

3. Accessing VF notification region using new PF BAR
Cons:
a. Requires hardware to build new PCI steering logic per PF to forward
   notification from the PF to VF, requires double the amount of logic
   compared to today
b. Requires very large additional PF BAR whose size must be max_Vfs * BAR size.

4. Trapping CVQ, configuration region, LEGACY_HDR
Cons:
a. This does not fullfil the very basic requirement to not trap the
   1.x objects (configuration registers, vqs)
b. Requires feature negotiations mediation in hypervisor software
c. Requires constant device type specific knowledge in hypervisor driver
   (Does not scale for 30+ device types)

4. F_LEACY_HDR, F_WRITE_MAC
Cons:
a. Requires device support to have read/write mac address which is
   hard to implement on every member device.
b. such functionality is duplicate of existing cvq per device.
c. config space is only for the initialization specific purpose.
d. Requires mediation of 1.x objects, which is not good design.
e. Solves only for the net device.
Pros:
a. May work for nested env

conclusion for picking AQ approach:
==================================
1. Overall AQ based access is simpler to implement with combination of
   best from software and device so that legacy registers do not get baked
   in the device hardware
2. AQ allows hypervisor software to intercept legacy registers and make
   corrections if needed
3. Provides trade-off between performance, device complexity vs spec,
   while still maintaining passthrough mode for the VFs with minimal
   hypervisor intercepts only for legacy registers access
4. AQ mechanism is designed for accessing other member devices registers
   as noted in AQ submission, it utilizes the existing infrastructure over
   other alternatives.
5. Uses existing driver notification region similar to legacy notification
   saves hardware resources

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
Signed-off-by: Parav Pandit <parav@nvidia.com>

---
changelog:
v6->v7:
- addressed several comments from Michael
- use AQ command to query legacy notify region, dropped pci capability
  modifications
- moved most part of the text to the generic admin command section
- replace administrative to administration
- replace admin vq citation to admin commands
- added normatives for device and driver side
- made BAR0 to be not used at all when supporting legacy interface
- added normative around BAR0 and SR-IOV extended capability
- grammar corrections
v5->v6:
- fixed previous missed abbreviation of LCC and LD
- 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
- 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
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
- moved legacy interface normative references to legacy conformance
  section
v2->v3:
- added new patch to split raws of admin vq opcode table
- 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.
- added other alternative design considered and discussed in detail in v0, v1 and v2
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
v0->v1:
- addressed comments, suggesetions and ideas from Michael Tsirkin and Jason Wang
- far more simpler design than MMR access
- removed complexities of MMR device ids
- removed complexities of MMR registers and extended capabilities
- dropped adding new extended capabilities because if if they are
  added, a pci device still needs to have existing capabilities
  in the legacy configuration space and hypervisor driver do not
  need to access them


Parav Pandit (4):
  admin: Split opcode table rows with a line
  admin: Fix section numbering
  admin: Add group member legacy register access commands
  transport-pci: Introduce group legacy group member config region
    access

 admin-cmds-legacy-interface.tex | 204 ++++++++++++++++++++++++++++++++
 admin.tex                       |  20 +++-
 conformance.tex                 |   3 +
 transport-pci-legacy-regs.tex   |  43 +++++++
 transport-pci.tex               |   2 +
 5 files changed, 269 insertions(+), 3 deletions(-)
 create mode 100644 admin-cmds-legacy-interface.tex
 create mode 100644 transport-pci-legacy-regs.tex

-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v7 1/4] admin: Split opcode table rows with a line

[Parav Pandit]

Currently all opcode appears to be in a single row.
Separate them with a line similar to other tables.

Signed-off-by: Parav Pandit <parav@nvidia.com>
---
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
 0x0000 & 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 - 0xFFFF & - & Reserved for future commands (possibly using a different structure)    \\
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v7 2/4] admin: Fix section numbering

[Parav Pandit]

Requirements are put one additional level down. Fix it.

Signed-off-by: Parav Pandit <parav@nvidia.com>
---
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-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v7 3/4] admin: Add group member legacy register access commands

[Parav Pandit]

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

[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                 |
     access                Driver 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 <parav@nvidia.com>
---
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-interface.tex
new file mode 100644
index 0000000..48d40ee
--- /dev/null
+++ b/admin-cmds-legacy-interface.tex
@@ -0,0 +1,204 @@
+\subsubsection{Legacy Interfaces}\label{sec:Basic Facilities of a Virtio Device / Device groups / Group
+administration commands / Legacy Interface}
+
+When the member device cannot support certain transport specific
+functionality of a legacy interface, it is difficult to compose a
+transitional group member device. In such scenario, a owner device
+can provide the legacy interface functionality. With such support,
+the member device driver can utilize the group owner device driver legacy
+interface functionality, thereby it can supply either a transitional device
+or a legacy device for the guest virtual machine. For example a PCI VF
+group member device do not support I/O BAR region functionality. For such
+limitation, a PCI PF group owner device support accessing member device's legacy
+configuration region using the administration commands. These administration
+commands are executed by the driver of the owner device when requested by the
+driver of the member device. For example, the member device driver
+intercepts configuration accesses done by the legacy driver and forwards
+such access request to the owner device driver; the owner device driver fulfil
+such requests by executing administration commands on the group owner device.
+
+The group owner device support following administration commands for legacy
+commong configuration and device configuration region access:
+
+\begin{enumerate}
+\item Legacy Common Configuration Region Write Command
+\item Legacy Common Configuration Region Read Command
+\item Legacy Device Configuration Region Write Command
+\item Legacy Device Configuration Region Read Command
+\end{enumerate}
+
+Accessing the member device legacy regions using these commands has the same
+effect as accessing it using the the legacy interface defined by the transport
+of the member device.
+
+\paragraph{Legacy Common Configuration Region Write Command}\label{par:Basic Facilities of a Virtio Device / Device groups / Group
+administration commands / Legacy Interface / Common Configuration Region Write Command}
+
+This command writes in a legacy common configuration region of a group member device.
+This command follows \field{struct virtio_admin_cmd}.
+This command uses following structure for \field{command_specific_data}:
+
+\begin{lstlisting}
+struct virtio_admin_cmd_legacy_common_cfg_wr_data {
+        u8 offset; /* Starting byte offset of the common configuration region to write */
+        u8 region_data[];
+};
+\end{lstlisting}
+
+The driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE.
+The driver sets valid \field{offset} and associated \field{region_data} bytes to
+write to the common configuration region.
+
+This command does not have any command specific result.
+
+\paragraph{Legacy Common Configuration Region Read Command}\label{par:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface / Common Configuration Region Read Command}
+
+This command reads from a legacy common configuration region of a group member device.
+This command follows \field{struct virtio_admin_cmd}.
+This command uses following structure for \field{command_specific_data}:
+
+\begin{lstlisting}
+struct virtio_admin_cmd_legacy_common_cfg_rd_data {
+	u8 offset; /* Starting byte offset of the common configuration region to read */
+};
+\end{lstlisting}
+
+The driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ.
+
+The driver sets valid \field{offset} of the region from where to read
+\field{region_data}.
+
+When command completes successfully, \field{command_specific_result}
+uses following structure:
+
+\begin{lstlisting}
+struct virtio_admin_cmd_legacy_common_cfg_rd_result {
+        u8 region_data[];
+};
+\end{lstlisting}
+
+\paragraph{Legacy Device Configuration Region Write Command}\label{par:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface / Device Configuration Region Write Command}
+
+This command writes in a legacy device configuration region of a group member device.
+This command follows \field{struct virtio_admin_cmd}.
+This command uses following structure for \field{command_specific_data}:
+
+\begin{lstlisting}
+struct virtio_admin_cmd_legacy_dev_reg_wr_data {
+        u8 offset; /* Starting byte offset of the device configuration region to write */
+        u8 region_data[];
+};
+\end{lstlisting}
+
+The driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LEGACY_DEV_REG_WRITE.
+The driver sets valid \field{offset} and associated \field{region_data} bytes to
+the device configuration region.
+
+This command does not have any command specific result.
+
+\paragraph{Legacy Device Configuration Region Write Command}\label{par:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface / Device Configuration Region Read Command}
+
+This command reads from a legacy device configuration region of a group member device.
+This command follows \field{struct virtio_admin_cmd}.
+
+This command uses following structure for \field{command_specific_data}:
+
+\begin{lstlisting}
+struct virtio_admin_cmd_legacy_dev_cfg_rd_data {
+        u8 offset; /* Starting byte offset of the device configuration region to read */
+};
+\end{lstlisting}
+
+The driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LEGACY_DEV_REG_READ.
+The driver sets valid \field{offset} of the device configurationi region from
+where to read \field{region_data}.
+
+When command completes successfully, \field{command_specific_result}
+uses following structure:
+
+\begin{lstlisting}
+struct virtio_admin_cmd_legacy_dev_reg_rd_result {
+        u8 region_data[];
+};
+\end{lstlisting}
+
+\paragraph{Legacy Driver Notification Region Query}\label{par:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface / Legacy Driver Notifications Region Query}
+
+Even though the driver notifications can be communicated through the
+administration command, if the group owner device or group member device
+supports such notifications using a memory-mapped operation or I/O operation,
+they are sent to the device by accessing this notification region using memory
+or I/O operation.
+
+A group owner device optionally support querying driver notifications region
+using VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY command.
+
+The driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY.
+This command does not have any command specific data.
+
+When command completes successfully, \field{command_specific_result}
+uses following structure:
+
+\begin{lstlisting}
+struct virtio_admin_cmd_legacy_notify_query_entry {
+        u8 region[8];
+};
+
+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}.
+
+\devicenormative{\paragraph}{Legacy Interface}{Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface}
+
+If the group owner device supports legacy region access for its group member
+devices, the device MUST set all corresponding bits for commands
+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 in
+the command result of VIRTIO_ADMIN_CMD_LIST_QUERY in
+\field{device_admin_cmd_opcodes}.
+
+The device MUST encode and decode legacy device specific registers using
+little-endian format.
+
+The device MUST fail VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE and
+VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ commands for the invalid offset
+which is outside the legacy common configuration region's address range.
+
+The device MUST fail VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE,
+VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands for the invalid offset
+which is outside the legacy device specific region's address range.
+
+The device SHOULD support VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY command for
+driver notifications. If the group owner device supports driver
+notifications region for its group member devices, the device MUST set
+VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY in the command result of
+VIRTIO_ADMIN_CMD_LIST_QUERY in \field{device_admin_cmd_opcodes}.
+
+When the driver accesses the legacy region of the member device using 
+VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
+VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
+VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE, VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ
+commands, device MUST function as if they are accessed by the legacy interface
+defined by the transport of the member device.
+
+\drivernormative{\paragraph}{Legacy Interface}{Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface}
+
+The driver MUST encode and decode legacy device specific registers using
+little-endian format.
+
+The driver SHOULD send VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE and
+VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ commands with a valid offset which
+is in the legacy common configuration region address range.
+
+The driver SHOULD send commands VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
+VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ with a valid offset which is in the legacy
+device specific configuration region address range.
+
diff --git a/admin.tex b/admin.tex
index fd3b97d..0de26a9 100644
--- a/admin.tex
+++ b/admin.tex
@@ -117,7 +117,17 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
 \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}    \\
+0x0002 & VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE & Write legacy common configuration region of a member device \\
+\hline
+0x0003 & VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ & Read legacy common configuration region of a member device \\
+\hline
+0x0004 & VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE & Write legacy device configuration region of a member device \\
+\hline
+0x0005 & VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ & Read legacy device configuration region of a member device \\
+\hline
+0x0006 & VIRTIO_ADMIN_CMD_LEGACY_DEV_NOTIFY_QUERY & Query notification region for a member device \\
+\hline
+0x0007 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
 \hline
 0x8000 - 0xFFFF & - & Reserved for future commands (possibly using a different structure)    \\
 \hline
@@ -286,6 +296,8 @@ \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.
 
+\input{admin-cmds-legacy-interface.tex}
+
 \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
diff --git a/conformance.tex b/conformance.tex
index 01ccd69..dc00e84 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -260,6 +260,8 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Layout}
 \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Endianness}
 \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message Framing / Legacy Interface: Message Framing}
+\item Section \ref{devicenormative:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface}
+\item Section \ref{drivernormative:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface}
 \item Section \ref{sec:General Initialization And Device Operation / Device Initialization / Legacy Interface: Device Initialization}
 \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery / Legacy Interfaces: A Note on PCI Device Discovery}
 \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v7 4/4] transport-pci: Introduce group legacy group member config region access

[Parav Pandit]

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 <parav@nvidia.com>
---
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 0000000..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{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}
+
+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.
+
+\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.
+
+For the SR-IOV group type, the owner device supporting
+VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
+VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE, VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ,
+VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY
+commands and its member device SHOULD follow the rules for the PCI Device ID,
+Revision ID and Subsystem Device ID of the non-transitional devices documented in
+section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery}.
diff --git a/transport-pci.tex b/transport-pci.tex
index a5c6719..72c78f6 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -1212,3 +1212,5 @@ \subsubsection{Driver Handling Interrupts}\label{sec:Virtio Transport Options /
         re-examine the configuration space to see what changed.
     \end{itemize}
 \end{itemize}
+
+\input{transport-pci-legacy-regs.tex}
-- 
2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 0/4] admin: Introduce legacy registers access using AQ

[Michael S. Tsirkin]

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 FFFFH.
> 
> [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 region            Driver 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 slow as AQ access
> b. Still requires new capability for notification access
> c. Requires hardware to build low level registers access which is not worth
>    for long term future
> 
> 3. Accessing VF notification region using new PF BAR
> Cons:
> a. Requires hardware to build new PCI steering logic per PF to forward
>    notification from the PF to VF, requires double the amount of logic
>    compared to today
> b. Requires very large additional PF BAR whose size must be max_Vfs * BAR size.
> 
> 4. Trapping CVQ, configuration region, LEGACY_HDR
> Cons:
> a. This does not fullfil the very basic requirement to not trap the
>    1.x objects (configuration registers, vqs)
> b. Requires feature negotiations mediation in hypervisor software
> c. Requires constant device type specific knowledge in hypervisor driver
>    (Does not scale for 30+ device types)
> 
> 4. F_LEACY_HDR, F_WRITE_MAC
> Cons:
> a. Requires device support to have read/write mac address which is
>    hard to implement on every member device.
> b. such functionality is duplicate of existing cvq per device.
> c. config space is only for the initialization specific purpose.
> d. Requires mediation of 1.x objects, which is not good design.
> e. Solves only for the net device.
> Pros:
> a. May work for nested env
> 
> conclusion for picking AQ approach:
> ==================================
> 1. Overall AQ based access is simpler to implement with combination of
>    best from software and device so that legacy registers do not get baked
>    in the device hardware
> 2. AQ allows hypervisor software to intercept legacy registers and make
>    corrections if needed
> 3. Provides trade-off between performance, device complexity vs spec,
>    while still maintaining passthrough mode for the VFs with minimal
>    hypervisor intercepts only for legacy registers access
> 4. AQ mechanism is designed for accessing other member devices registers
>    as noted in AQ submission, it utilizes the existing infrastructure over
>    other alternatives.
> 5. Uses existing driver notification region similar to legacy notification
>    saves hardware resources
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> 
> ---
> changelog:
> v6->v7:
> - addressed several comments from Michael
> - use AQ command to query legacy notify region, dropped pci capability
>   modifications
> - moved most part of the text to the generic admin command section
> - replace administrative to administration
> - replace admin vq citation to admin commands
> - added normatives for device and driver side
> - made BAR0 to be not used at all when supporting legacy interface
> - added normative around BAR0 and SR-IOV extended capability
> - grammar corrections
> v5->v6:
> - fixed previous missed abbreviation of LCC and LD
> - 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
> - 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
> 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
> - moved legacy interface normative references to legacy conformance
>   section
> v2->v3:
> - added new patch to split raws of admin vq opcode table
> - 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.
> - added other alternative design considered and discussed in detail in v0, v1 and v2
> 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
> v0->v1:
> - addressed comments, suggesetions and ideas from Michael Tsirkin and Jason Wang
> - far more simpler design than MMR access
> - removed complexities of MMR device ids
> - removed complexities of MMR registers and extended capabilities
> - dropped adding new extended capabilities because if if they are
>   added, a pci device still needs to have existing capabilities
>   in the legacy configuration space and hypervisor driver do not
>   need to access them
> 
> 
> Parav Pandit (4):
>   admin: Split opcode table rows with a line
>   admin: Fix section numbering
>   admin: Add group member legacy register access commands
>   transport-pci: Introduce group legacy group member config region
>     access
> 
>  admin-cmds-legacy-interface.tex | 204 ++++++++++++++++++++++++++++++++
>  admin.tex                       |  20 +++-
>  conformance.tex                 |   3 +
>  transport-pci-legacy-regs.tex   |  43 +++++++
>  transport-pci.tex               |   2 +
>  5 files changed, 269 insertions(+), 3 deletions(-)
>  create mode 100644 admin-cmds-legacy-interface.tex
>  create mode 100644 transport-pci-legacy-regs.tex
> 
> -- 
> 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 4/4] transport-pci: Introduce group legacy group member config region access

[Michael S. Tsirkin]

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 <parav@nvidia.com>
> ---
> 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 0000000..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{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.

> +};
> +\end{lstlisting}
> +
> +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.
> +
> +\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.
> +
> +For the SR-IOV group type, the owner device supporting
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE, VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY
> +commands and its member device SHOULD follow the rules for the PCI Device ID,
> +Revision ID and Subsystem Device ID of the non-transitional devices documented in
> +section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery}.
> diff --git a/transport-pci.tex b/transport-pci.tex
> index a5c6719..72c78f6 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -1212,3 +1212,5 @@ \subsubsection{Driver Handling Interrupts}\label{sec:Virtio Transport Options /
>          re-examine the configuration space to see what changed.
>      \end{itemize}
>  \end{itemize}
> +
> +\input{transport-pci-legacy-regs.tex}
> -- 
> 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 3/4] admin: Add group member legacy register access commands

[Michael S. Tsirkin]

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 FFFFH.
> 
> [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                 |
>      access                Driver 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 <parav@nvidia.com>
> ---
> 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-interface.tex
> new file mode 100644
> index 0000000..48d40ee
> --- /dev/null
> +++ b/admin-cmds-legacy-interface.tex
> @@ -0,0 +1,204 @@
> +\subsubsection{Legacy Interfaces}\label{sec:Basic Facilities of a Virtio Device / Device groups / Group
> +administration commands / Legacy Interface}
> +
> +When the member device cannot support certain transport specific
> +functionality of a legacy interface, it is difficult to compose a
> +transitional group member device. In such scenario, a owner device
> +can provide the legacy interface functionality. With such support,
> +the member device driver can utilize the group owner device driver legacy
> +interface functionality, thereby it can supply either a transitional device
> +or a legacy device for the guest virtual machine. For example a PCI VF
> +group member device do not support I/O BAR region functionality. For such
> +limitation, a PCI PF group owner device support accessing member device's legacy
> +configuration region using the administration commands. These administration
> +commands are executed by the driver of the owner device when requested by the
> +driver of the member device. For example, the member device driver
> +intercepts configuration accesses done by the legacy driver and forwards
> +such access request to the owner device driver; the owner device driver fulfil
> +such requests by executing administration commands on the group owner device.
> +
> +The group owner device support following administration commands for legacy
> +commong configuration and device configuration region access:
> +
> +\begin{enumerate}
> +\item Legacy Common Configuration Region Write Command
> +\item Legacy Common Configuration Region Read Command
> +\item Legacy Device Configuration Region Write Command
> +\item Legacy Device Configuration Region Read Command
> +\end{enumerate}
> +
> +Accessing the member device legacy regions using these commands has the same
> +effect as accessing it using the the legacy interface defined by the transport
> +of the member device.
> +
> +\paragraph{Legacy Common Configuration Region Write Command}\label{par:Basic Facilities of a Virtio Device / Device groups / Group
> +administration commands / Legacy Interface / Common Configuration Region Write Command}
> +
> +This command writes in a legacy common configuration region of a group member device.
> +This command follows \field{struct virtio_admin_cmd}.
> +This command uses following structure for \field{command_specific_data}:
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd_legacy_common_cfg_wr_data {
> +        u8 offset; /* Starting byte offset of the common configuration region to write */
> +        u8 region_data[];
> +};
> +\end{lstlisting}
> +
> +The driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE.
> +The driver sets valid \field{offset} and associated \field{region_data} bytes to
> +write to the common configuration region.
> +
> +This command does not have any command specific result.
> +
> +\paragraph{Legacy Common Configuration Region Read Command}\label{par:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface / Common Configuration Region Read Command}
> +
> +This command reads from a legacy common configuration region of a group member device.
> +This command follows \field{struct virtio_admin_cmd}.
> +This command uses following structure for \field{command_specific_data}:
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd_legacy_common_cfg_rd_data {
> +	u8 offset; /* Starting byte offset of the common configuration region to read */
> +};
> +\end{lstlisting}
> +
> +The driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ.
> +
> +The driver sets valid \field{offset} of the region from where to read
> +\field{region_data}.
> +
> +When command completes successfully, \field{command_specific_result}
> +uses following structure:
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd_legacy_common_cfg_rd_result {
> +        u8 region_data[];
> +};
> +\end{lstlisting}
> +
> +\paragraph{Legacy Device Configuration Region Write Command}\label{par:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface / Device Configuration Region Write Command}
> +
> +This command writes in a legacy device configuration region of a group member device.
> +This command follows \field{struct virtio_admin_cmd}.
> +This command uses following structure for \field{command_specific_data}:
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd_legacy_dev_reg_wr_data {
> +        u8 offset; /* Starting byte offset of the device configuration region to write */
> +        u8 region_data[];
> +};
> +\end{lstlisting}
> +
> +The driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LEGACY_DEV_REG_WRITE.
> +The driver sets valid \field{offset} and associated \field{region_data} bytes to
> +the device configuration region.
> +
> +This command does not have any command specific result.
> +
> +\paragraph{Legacy Device Configuration Region Write Command}\label{par:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface / Device Configuration Region Read Command}
> +
> +This command reads from a legacy device configuration region of a group member device.
> +This command follows \field{struct virtio_admin_cmd}.
> +
> +This command uses following structure for \field{command_specific_data}:
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd_legacy_dev_cfg_rd_data {
> +        u8 offset; /* Starting byte offset of the device configuration region to read */
> +};
> +\end{lstlisting}
> +
> +The driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LEGACY_DEV_REG_READ.
> +The driver sets valid \field{offset} of the device configurationi region from
> +where to read \field{region_data}.
> +
> +When command completes successfully, \field{command_specific_result}
> +uses following structure:
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd_legacy_dev_reg_rd_result {
> +        u8 region_data[];
> +};
> +\end{lstlisting}
> +
> +\paragraph{Legacy Driver Notification Region Query}\label{par:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface / Legacy Driver Notifications Region Query}
> +
> +Even though the driver notifications can be communicated through the
> +administration command, if the group owner device or group member device
> +supports such notifications using a memory-mapped operation or I/O operation,
> +they are sent to the device by accessing this notification region using memory
> +or I/O operation.
> +
> +A group owner device optionally support querying driver notifications region
> +using VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY command.
> +
> +The driver sets command \field{opcode} to VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY.
> +This command does not have any command specific data.
> +
> +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:

struct virtio_admin_cmd_legacy_notify_query_entry {
	union {
		virtio_pci_notify_region region;
	};
};


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


Or preferably I would just include that tex right here
to avoid the need to jump back and forth.


> +
> +\devicenormative{\paragraph}{Legacy Interface}{Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface}
> +
> +If the group owner device supports legacy region access for its group member
> +devices, the device MUST set all corresponding bits for commands
> +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 in
> +the command result of VIRTIO_ADMIN_CMD_LIST_QUERY in
> +\field{device_admin_cmd_opcodes}.
> +
> +The device MUST encode and decode legacy device specific registers using
> +little-endian format.
> +
> +The device MUST fail VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE and
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ commands for the invalid offset
> +which is outside the legacy common configuration region's address range.
> +
> +The device MUST fail VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ commands for the invalid offset
> +which is outside the legacy device specific region's address range.
> +
> +The device SHOULD support VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY command for
> +driver notifications. If the group owner device supports driver
> +notifications region for its group member devices, the device MUST set
> +VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY in the command result of
> +VIRTIO_ADMIN_CMD_LIST_QUERY in \field{device_admin_cmd_opcodes}.
> +
> +When the driver accesses the legacy region of the member device using 
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE,
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE, VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ
> +commands, device MUST function as if they are accessed by the legacy interface
> +defined by the transport of the member device.
> +
> +\drivernormative{\paragraph}{Legacy Interface}{Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface}
> +
> +The driver MUST encode and decode legacy device specific registers using
> +little-endian format.
> +
> +The driver SHOULD send VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE and
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ commands with a valid offset which
> +is in the legacy common configuration region address range.
> +
> +The driver SHOULD send commands VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ with a valid offset which is in the legacy
> +device specific configuration region address range.
> +
> diff --git a/admin.tex b/admin.tex
> index fd3b97d..0de26a9 100644
> --- a/admin.tex
> +++ b/admin.tex
> @@ -117,7 +117,17 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
>  \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}    \\
> +0x0002 & VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE & Write legacy common configuration region of a member device \\
> +\hline
> +0x0003 & VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ & Read legacy common configuration region of a member device \\
> +\hline
> +0x0004 & VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE & Write legacy device configuration region of a member device \\
> +\hline
> +0x0005 & VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ & Read legacy device configuration region of a member device \\
> +\hline
> +0x0006 & VIRTIO_ADMIN_CMD_LEGACY_DEV_NOTIFY_QUERY & Query notification region for a member device \\
> +\hline
> +0x0007 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
>  \hline
>  0x8000 - 0xFFFF & - & Reserved for future commands (possibly using a different structure)    \\
>  \hline
> @@ -286,6 +296,8 @@ \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.
>  
> +\input{admin-cmds-legacy-interface.tex}
> +
>  \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
> diff --git a/conformance.tex b/conformance.tex
> index 01ccd69..dc00e84 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -260,6 +260,8 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Layout}
>  \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Legacy Interfaces: A Note on Virtqueue Endianness}
>  \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message Framing / Legacy Interface: Message Framing}
> +\item Section \ref{devicenormative:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface}
> +\item Section \ref{drivernormative:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Legacy Interface}
>  \item Section \ref{sec:General Initialization And Device Operation / Device Initialization / Legacy Interface: Device Initialization}
>  \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery / Legacy Interfaces: A Note on PCI Device Discovery}
>  \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
> -- 
> 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 4/4] transport-pci: Introduce group legacy group member config region access

[Michael S. Tsirkin]

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 <parav@nvidia.com>
> ---
> 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 0000000..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{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.


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

> +
> +\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

> +
> +For the SR-IOV group type, the owner device supporting
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_COMMON_CFG_WRITE, VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_READ,
> +VIRTIO_ADMIN_CMD_LEGACY_DEV_CFG_WRITE and VIRTIO_ADMIN_CMD_LEGACY_NOTIFY_QUERY
> +commands and its member device SHOULD follow the rules for the PCI Device ID,
> +Revision ID and Subsystem Device ID of the non-transitional devices documented in
> +section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Discovery}.
> diff --git a/transport-pci.tex b/transport-pci.tex
> index a5c6719..72c78f6 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -1212,3 +1212,5 @@ \subsubsection{Driver Handling Interrupts}\label{sec:Virtio Transport Options /
>          re-examine the configuration space to see what changed.
>      \end{itemize}
>  \end{itemize}
> +
> +\input{transport-pci-legacy-regs.tex}
> -- 
> 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 0/4] admin: Introduce legacy registers access using AQ

[Michael S. Tsirkin]

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 FFFFH.
> 
> [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 region            Driver 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 slow as AQ access
> b. Still requires new capability for notification access
> c. Requires hardware to build low level registers access which is not worth
>    for long term future
> 
> 3. Accessing VF notification region using new PF BAR
> Cons:
> a. Requires hardware to build new PCI steering logic per PF to forward
>    notification from the PF to VF, requires double the amount of logic
>    compared to today
> b. Requires very large additional PF BAR whose size must be max_Vfs * BAR size.
> 
> 4. Trapping CVQ, configuration region, LEGACY_HDR
> Cons:
> a. This does not fullfil the very basic requirement to not trap the
>    1.x objects (configuration registers, vqs)
> b. Requires feature negotiations mediation in hypervisor software
> c. Requires constant device type specific knowledge in hypervisor driver
>    (Does not scale for 30+ device types)
> 
> 4. F_LEACY_HDR, F_WRITE_MAC
> Cons:
> a. Requires device support to have read/write mac address which is
>    hard to implement on every member device.
> b. such functionality is duplicate of existing cvq per device.
> c. config space is only for the initialization specific purpose.
> d. Requires mediation of 1.x objects, which is not good design.
> e. Solves only for the net device.
> Pros:
> a. May work for nested env
> 
> conclusion for picking AQ approach:
> ==================================
> 1. Overall AQ based access is simpler to implement with combination of
>    best from software and device so that legacy registers do not get baked
>    in the device hardware
> 2. AQ allows hypervisor software to intercept legacy registers and make
>    corrections if needed
> 3. Provides trade-off between performance, device complexity vs spec,
>    while still maintaining passthrough mode for the VFs with minimal
>    hypervisor intercepts only for legacy registers access
> 4. AQ mechanism is designed for accessing other member devices registers
>    as noted in AQ submission, it utilizes the existing infrastructure over
>    other alternatives.
> 5. Uses existing driver notification region similar to legacy notification
>    saves hardware resources
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/167
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> 
> ---
> changelog:
> v6->v7:
> - addressed several comments from Michael
> - use AQ command to query legacy notify region, dropped pci capability
>   modifications
> - moved most part of the text to the generic admin command section
> - replace administrative to administration
> - replace admin vq citation to admin commands
> - added normatives for device and driver side
> - made BAR0 to be not used at all when supporting legacy interface
> - added normative around BAR0 and SR-IOV extended capability
> - grammar corrections
> v5->v6:
> - fixed previous missed abbreviation of LCC and LD
> - 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
> - 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
> 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
> - moved legacy interface normative references to legacy conformance
>   section
> v2->v3:
> - added new patch to split raws of admin vq opcode table
> - 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.
> - added other alternative design considered and discussed in detail in v0, v1 and v2
> 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
> v0->v1:
> - addressed comments, suggesetions and ideas from Michael Tsirkin and Jason Wang
> - far more simpler design than MMR access
> - removed complexities of MMR device ids
> - removed complexities of MMR registers and extended capabilities
> - dropped adding new extended capabilities because if if they are
>   added, a pci device still needs to have existing capabilities
>   in the legacy configuration space and hypervisor driver do not
>   need to access them
> 
> 
> Parav Pandit (4):
>   admin: Split opcode table rows with a line
>   admin: Fix section numbering
>   admin: Add group member legacy register access commands
>   transport-pci: Introduce group legacy group member config region
>     access
> 
>  admin-cmds-legacy-interface.tex | 204 ++++++++++++++++++++++++++++++++
>  admin.tex                       |  20 +++-
>  conformance.tex                 |   3 +
>  transport-pci-legacy-regs.tex   |  43 +++++++
>  transport-pci.tex               |   2 +
>  5 files changed, 269 insertions(+), 3 deletions(-)
>  create mode 100644 admin-cmds-legacy-interface.tex
>  create mode 100644 transport-pci-legacy-regs.tex
> 
> -- 
> 2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v7 4/4] transport-pci: Introduce group legacy group member config region access

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> 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-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v7 0/4] admin: Introduce legacy registers access using AQ

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> 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

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> 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-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v7 3/4] admin: Add group member legacy register access commands

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> 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

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> 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

[Michael S. Tsirkin]

On Thu, Jun 29, 2023 at 09:32:21PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > 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-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v7 3/4] admin: Add group member legacy register access commands

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> 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-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 3/4] admin: Add group member legacy register access commands

[Michael S. Tsirkin]

On Thu, Jun 29, 2023 at 09:50:45PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > 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-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v7 3/4] admin: Add group member legacy register access commands

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> 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-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 3/4] admin: Add group member legacy register access commands

[Michael S. Tsirkin]

On Thu, Jun 29, 2023 at 10:02:53PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > 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-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v7 3/4] admin: Add group member legacy register access commands

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> 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-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org