[PATCH 0/3] Cleanup for PCI transitional common cfg

[PATCH 0/3] Cleanup for PCI transitional common cfg

[Parav Pandit]

Legacy interface PCI Device layout description has following issues.

1. repeated 'structure' word
2. virtio header was defined the 0.9.5 spec. It is referred with
   different keywards in this section with multiple different words
   as (a) virtio header, (b) general headers, (c) legacy configuration
   structure, (d) virtio common configuration structure and
   (e) other fields.
3. Driver and device requirements listing is intermixed.
4. spelling error of structure
5. Legacy interface common configuration requirements are not adjacent
   to 1.x comm

Hence, this short series overcomes above issues.

Patch summary:
patch-1 overcomes above 1 to 4 issues
patch-2 splits feature bit operations from config layout
patch-3 relocate requirements adjacent to 1.x requirements

This series is in top of [1], [2] and [3].

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
Please review.

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00578.html
[2] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00585.html
[3] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00597.html

Parav Pandit (3):
  transport-pci: Improve PCI legacy device layout description
  transport-pci: Split notes of PCI Device Layout
  transport-pci: Relocate common config legacy interface

 conformance.tex   |   3 +-
 transport-pci.tex | 182 +++++++++++++++++++++++++---------------------
 2 files changed, 102 insertions(+), 83 deletions(-)

-- 
2.26.2

[virtio-dev] [PATCH 0/3] Cleanup for PCI transitional common cfg

[Parav Pandit]

Legacy interface PCI Device layout description has following issues.

1. repeated 'structure' word
2. virtio header was defined the 0.9.5 spec. It is referred with
   different keywards in this section with multiple different words
   as (a) virtio header, (b) general headers, (c) legacy configuration
   structure, (d) virtio common configuration structure and
   (e) other fields.
3. Driver and device requirements listing is intermixed.
4. spelling error of structure
5. Legacy interface common configuration requirements are not adjacent
   to 1.x comm

Hence, this short series overcomes above issues.

Patch summary:
patch-1 overcomes above 1 to 4 issues
patch-2 splits feature bit operations from config layout
patch-3 relocate requirements adjacent to 1.x requirements

This series is in top of [1], [2] and [3].

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
Please review.

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00578.html
[2] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00585.html
[3] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00597.html

Parav Pandit (3):
  transport-pci: Improve PCI legacy device layout description
  transport-pci: Split notes of PCI Device Layout
  transport-pci: Relocate common config legacy interface

 conformance.tex   |   3 +-
 transport-pci.tex | 182 +++++++++++++++++++++++++---------------------
 2 files changed, 102 insertions(+), 83 deletions(-)

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

[PATCH 1/3] transport-pci: Improve PCI legacy device layout description

[Parav Pandit]

Legacy interface PCI Device layout description has following issues.

1. repeated 'structure' word
2. virtio header was defined the 0.9.5 spec. In a legacy interface
   section it is referred with different keywards
   as (a) virtio header, (b) general headers, (c) legacy configuration
   structure, (d) virtio common configuration structure and
   (e) other fields.
3. Driver and device requirements listing is intermixed.
4. spelling error of structure

Hence, rewrite the description to eliminate above issues as below.

1. Removed repeated structure word
2. Fix spelling of structure
3. Place all device requirements toegether
3. Define legacy configuration structure that consist of
   a. legacy common configuration structure and
   b. device specific configuration structure
4. Rewrite section around above changes

This is only an editorial change.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 transport-pci.tex | 93 ++++++++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 41 deletions(-)

diff --git a/transport-pci.tex b/transport-pci.tex
index 5d22e6f..9ee37ba 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -769,25 +769,19 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
 
 \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
 
-Transitional devices MUST present part of configuration
-registers in a legacy configuration structure in BAR0 in the first I/O
-region of the PCI device, as documented below.
-When using the legacy interface, transitional drivers
-MUST use the legacy configuration structure in BAR0 in the first
-I/O region of the PCI device, as documented below.
-
-When using the legacy interface the driver MAY access
-the device-specific configuration region using any width accesses, and
-a transitional device MUST present driver with the same results as
-when accessed using the ``natural'' access method (i.e.
-32-bit accesses for 32-bit fields, etc).
-
-Note that this is possible because while the virtio common configuration structure is PCI
-(i.e. little) endian, when using the legacy interface the device-specific
-configuration region is encoded in the native endian of the guest (where such distinction is
-applicable).
-
-When used through the legacy interface, the virtio common configuration structure looks as follows:
+The transitional device MUST present part of the configuration
+registers in a legacy configuration structure in BAR0 in the
+first I/O region of the PCI Device.
+
+The legacy configuration structure is described below.
+It consists of two parts.
+\begin{enumerate}
+  \item Legacy common configuration structure
+  \item Device configuration structure (optional)
+\end{enumerate}
+
+When used through the legacy interface, the legacy common
+configuration structure has the following layout:
 
 \begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
 \hline
@@ -801,8 +795,8 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
 \hline
 \end{tabularx}
 
-If MSI-X is enabled for the device, two additional fields
-immediately follow this header:
+When MSI-X capability is enabled on the device, the device MUST
+present two additional fields immediately following the above fields:
 
 \begin{tabular}{ |l||l|l| }
 \hline
@@ -814,14 +808,12 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
 \hline
 \end{tabular}
 
-Note: When MSI-X capability is enabled, device-specific configuration starts at
-byte offset 24 in virtio common configuration structure structure. When MSI-X capability is not
-enabled, device-specific configuration starts at byte offset 20 in virtio
-header.  ie. once you enable MSI-X on the device, the other fields move.
-If you turn it off again, they move back!
+The device configuration structure is optional. Its existence
+is decided by each device type. The transitional device MUST
+present the device-specific configuration structure if any at an
+offset immediately following the legacy common configuration structure.
 
-Any device-specific configuration space immediately follows
-these general headers:
+The device configuration structure:
 
 \begin{tabular}{|l||l|l|}
 \hline
@@ -833,25 +825,44 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
 \hline
 \end{tabular}
 
-When accessing the device-specific configuration space
-using the legacy interface, transitional
-drivers MUST access the device-specific configuration space
-at an offset immediately following the general headers.
-
-When using the legacy interface, transitional
-devices MUST present the device-specific configuration space
-if any at an offset immediately following the general headers.
-
-Note that only Feature Bits 0 to 31 are accessible through the
-Legacy Interface. When used through the Legacy Interface,
-Transitional Devices MUST assume that Feature Bits 32 to 63
-are not acknowledged by Driver.
+Note: The device configuration structure byte offset is
+calculated dynamically; when MSI-X capability is enabled, the
+device configuration structure is located at byte offset 24,
+when MSI-X capability is disabled, the device configuration
+structure is located at byte offset 20.
 
 As legacy devices had no \field{config_generation} field,
 see \ref{sec:Basic Facilities of a Virtio Device / Device
 Configuration Space / Legacy Interface: Device Configuration
 Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
 
+When using the legacy interface, the transitional driver MUST
+use the legacy configuration structure in BAR0 in the first
+I/O region of the PCI device.
+
+When using the legacy interface, the driver MAY access
+the device-specific configuration structure using any width
+accesses and the transitional device MUST present the driver with
+the same results as when accessed using the ``natural'' access
+method (i.e. 32-bit accesses for 32-bit fields, etc).
+
+Note that this is possible because while the legacy common
+configuration structure is PCI (i.e. little) endian, when using
+the legacy interface the device-specific configuration structure
+is encoded in the native endian of the guest (where such
+distinction is applicable).
+
+When accessing the device-specific configuration structure
+using the legacy interface, transitional drivers MUST access
+the device-specific configuration structure
+at an offset immediately following the legacy common
+configuration structure.
+
+Note that only Feature Bits 0 to 31 are accessible through the
+Legacy Interface. When used through the Legacy Interface,
+the transitional device MUST assume that Feature Bits 32 to 63
+are not acknowledged by the driver.
+
 \subsubsection{Non-transitional Device With Legacy Driver: A Note
 on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio
 Over PCI Bus / PCI Device Layout / Non-transitional Device With
-- 
2.26.2

[virtio-dev] [PATCH 1/3] transport-pci: Improve PCI legacy device layout description

[Parav Pandit]

Legacy interface PCI Device layout description has following issues.

1. repeated 'structure' word
2. virtio header was defined the 0.9.5 spec. In a legacy interface
   section it is referred with different keywards
   as (a) virtio header, (b) general headers, (c) legacy configuration
   structure, (d) virtio common configuration structure and
   (e) other fields.
3. Driver and device requirements listing is intermixed.
4. spelling error of structure

Hence, rewrite the description to eliminate above issues as below.

1. Removed repeated structure word
2. Fix spelling of structure
3. Place all device requirements toegether
3. Define legacy configuration structure that consist of
   a. legacy common configuration structure and
   b. device specific configuration structure
4. Rewrite section around above changes

This is only an editorial change.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 transport-pci.tex | 93 ++++++++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 41 deletions(-)

diff --git a/transport-pci.tex b/transport-pci.tex
index 5d22e6f..9ee37ba 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -769,25 +769,19 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
 
 \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
 
-Transitional devices MUST present part of configuration
-registers in a legacy configuration structure in BAR0 in the first I/O
-region of the PCI device, as documented below.
-When using the legacy interface, transitional drivers
-MUST use the legacy configuration structure in BAR0 in the first
-I/O region of the PCI device, as documented below.
-
-When using the legacy interface the driver MAY access
-the device-specific configuration region using any width accesses, and
-a transitional device MUST present driver with the same results as
-when accessed using the ``natural'' access method (i.e.
-32-bit accesses for 32-bit fields, etc).
-
-Note that this is possible because while the virtio common configuration structure is PCI
-(i.e. little) endian, when using the legacy interface the device-specific
-configuration region is encoded in the native endian of the guest (where such distinction is
-applicable).
-
-When used through the legacy interface, the virtio common configuration structure looks as follows:
+The transitional device MUST present part of the configuration
+registers in a legacy configuration structure in BAR0 in the
+first I/O region of the PCI Device.
+
+The legacy configuration structure is described below.
+It consists of two parts.
+\begin{enumerate}
+  \item Legacy common configuration structure
+  \item Device configuration structure (optional)
+\end{enumerate}
+
+When used through the legacy interface, the legacy common
+configuration structure has the following layout:
 
 \begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
 \hline
@@ -801,8 +795,8 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
 \hline
 \end{tabularx}
 
-If MSI-X is enabled for the device, two additional fields
-immediately follow this header:
+When MSI-X capability is enabled on the device, the device MUST
+present two additional fields immediately following the above fields:
 
 \begin{tabular}{ |l||l|l| }
 \hline
@@ -814,14 +808,12 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
 \hline
 \end{tabular}
 
-Note: When MSI-X capability is enabled, device-specific configuration starts at
-byte offset 24 in virtio common configuration structure structure. When MSI-X capability is not
-enabled, device-specific configuration starts at byte offset 20 in virtio
-header.  ie. once you enable MSI-X on the device, the other fields move.
-If you turn it off again, they move back!
+The device configuration structure is optional. Its existence
+is decided by each device type. The transitional device MUST
+present the device-specific configuration structure if any at an
+offset immediately following the legacy common configuration structure.
 
-Any device-specific configuration space immediately follows
-these general headers:
+The device configuration structure:
 
 \begin{tabular}{|l||l|l|}
 \hline
@@ -833,25 +825,44 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
 \hline
 \end{tabular}
 
-When accessing the device-specific configuration space
-using the legacy interface, transitional
-drivers MUST access the device-specific configuration space
-at an offset immediately following the general headers.
-
-When using the legacy interface, transitional
-devices MUST present the device-specific configuration space
-if any at an offset immediately following the general headers.
-
-Note that only Feature Bits 0 to 31 are accessible through the
-Legacy Interface. When used through the Legacy Interface,
-Transitional Devices MUST assume that Feature Bits 32 to 63
-are not acknowledged by Driver.
+Note: The device configuration structure byte offset is
+calculated dynamically; when MSI-X capability is enabled, the
+device configuration structure is located at byte offset 24,
+when MSI-X capability is disabled, the device configuration
+structure is located at byte offset 20.
 
 As legacy devices had no \field{config_generation} field,
 see \ref{sec:Basic Facilities of a Virtio Device / Device
 Configuration Space / Legacy Interface: Device Configuration
 Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
 
+When using the legacy interface, the transitional driver MUST
+use the legacy configuration structure in BAR0 in the first
+I/O region of the PCI device.
+
+When using the legacy interface, the driver MAY access
+the device-specific configuration structure using any width
+accesses and the transitional device MUST present the driver with
+the same results as when accessed using the ``natural'' access
+method (i.e. 32-bit accesses for 32-bit fields, etc).
+
+Note that this is possible because while the legacy common
+configuration structure is PCI (i.e. little) endian, when using
+the legacy interface the device-specific configuration structure
+is encoded in the native endian of the guest (where such
+distinction is applicable).
+
+When accessing the device-specific configuration structure
+using the legacy interface, transitional drivers MUST access
+the device-specific configuration structure
+at an offset immediately following the legacy common
+configuration structure.
+
+Note that only Feature Bits 0 to 31 are accessible through the
+Legacy Interface. When used through the Legacy Interface,
+the transitional device MUST assume that Feature Bits 32 to 63
+are not acknowledged by the driver.
+
 \subsubsection{Non-transitional Device With Legacy Driver: A Note
 on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio
 Over PCI Bus / PCI Device Layout / Non-transitional Device With
-- 
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

[PATCH 2/3] transport-pci: Split notes of PCI Device Layout

[Parav Pandit]

Currently single legacy interface section describes PCI common
configuration layout and feature bits operation for the
legacy interface.
Secondly common configuration structure description of legacy interface
is not adjacent to the the respective normal device requirements for
same.

Hence, split PCI Device Layout legacy interface section into two
parts. First subsection for common configuration and second
subsection for feature bits.

Subsequent patch relocates common configuration legacy interface to
appropriate matching location.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 conformance.tex   |  4 +++-
 transport-pci.tex | 19 ++++++++++++-------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index 01ccd69..0d3616f 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -262,7 +262,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message Framing / Legacy Interface: Message Framing}
 \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}
+\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
+configuration Layout}
+\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
 \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{sec:Virtio Transport Options / Virtio Over MMIO / Legacy interface}
diff --git a/transport-pci.tex b/transport-pci.tex
index 9ee37ba..9d4c713 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -767,7 +767,10 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
 specified by some other Virtio Structure PCI Capability
 of type other than \field{VIRTIO_PCI_CAP_PCI_CFG}.
 
-\subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
+\subsubsection{Legacy Interfaces: A Note on Common configuration
+Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
+/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
+configuration Layout}
 
 The transitional device MUST present part of the configuration
 registers in a legacy configuration structure in BAR0 in the
@@ -852,13 +855,15 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
 is encoded in the native endian of the guest (where such
 distinction is applicable).
 
-When accessing the device-specific configuration structure
-using the legacy interface, transitional drivers MUST access
-the device-specific configuration structure
-at an offset immediately following the legacy common
-configuration structure.
+The transitional driver when using the legacy interface MUST
+the device-specific configuration structure at an offset
+immediately following the legacy common configuration structure.
 
-Note that only Feature Bits 0 to 31 are accessible through the
+\subsubsection{Legacy Interface: A Note on feature
+bits}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
+Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
+
+Only Feature Bits 0 to 31 are accessible through the
 Legacy Interface. When used through the Legacy Interface,
 the transitional device MUST assume that Feature Bits 32 to 63
 are not acknowledged by the driver.
-- 
2.26.2

[virtio-dev] [PATCH 2/3] transport-pci: Split notes of PCI Device Layout

[Parav Pandit]

Currently single legacy interface section describes PCI common
configuration layout and feature bits operation for the
legacy interface.
Secondly common configuration structure description of legacy interface
is not adjacent to the the respective normal device requirements for
same.

Hence, split PCI Device Layout legacy interface section into two
parts. First subsection for common configuration and second
subsection for feature bits.

Subsequent patch relocates common configuration legacy interface to
appropriate matching location.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 conformance.tex   |  4 +++-
 transport-pci.tex | 19 ++++++++++++-------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index 01ccd69..0d3616f 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -262,7 +262,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message Framing / Legacy Interface: Message Framing}
 \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}
+\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
+configuration Layout}
+\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
 \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{sec:Virtio Transport Options / Virtio Over MMIO / Legacy interface}
diff --git a/transport-pci.tex b/transport-pci.tex
index 9ee37ba..9d4c713 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -767,7 +767,10 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
 specified by some other Virtio Structure PCI Capability
 of type other than \field{VIRTIO_PCI_CAP_PCI_CFG}.
 
-\subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
+\subsubsection{Legacy Interfaces: A Note on Common configuration
+Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
+/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
+configuration Layout}
 
 The transitional device MUST present part of the configuration
 registers in a legacy configuration structure in BAR0 in the
@@ -852,13 +855,15 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
 is encoded in the native endian of the guest (where such
 distinction is applicable).
 
-When accessing the device-specific configuration structure
-using the legacy interface, transitional drivers MUST access
-the device-specific configuration structure
-at an offset immediately following the legacy common
-configuration structure.
+The transitional driver when using the legacy interface MUST
+the device-specific configuration structure at an offset
+immediately following the legacy common configuration structure.
 
-Note that only Feature Bits 0 to 31 are accessible through the
+\subsubsection{Legacy Interface: A Note on feature
+bits}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
+Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
+
+Only Feature Bits 0 to 31 are accessible through the
 Legacy Interface. When used through the Legacy Interface,
 the transitional device MUST assume that Feature Bits 32 to 63
 are not acknowledged by the driver.
-- 
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

[PATCH 3/3] transport-pci: Relocate common config legacy interface

[Parav Pandit]

Relocate legacy interface section of common configuration structure near
where 1.x based common configuration structure is defined.

This aligns the spec to follow rest of the other Legacy interfaces
section.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 conformance.tex   |   3 +-
 transport-pci.tex | 186 +++++++++++++++++++++++-----------------------
 2 files changed, 95 insertions(+), 94 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index 0d3616f..b654fe0 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -262,8 +262,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message Framing / Legacy Interface: Message Framing}
 \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/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
-configuration Layout}
+\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Common configuration structure layout / Legacy Interfaces: A Note on Common configuration Layout}
 \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
 \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}
diff --git a/transport-pci.tex b/transport-pci.tex
index 9d4c713..19375d0 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -494,6 +494,100 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 were used before the queue reset.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
+\paragraph{Legacy Interfaces: A Note on Common configuration
+Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
+/ Virtio Structure PCI Capabilities / Common configuration
+structure layout / Legacy Interfaces: A Note on Common
+configuration Layout}
+
+The transitional device MUST present part of the configuration
+registers in a legacy configuration structure in BAR0 in the
+first I/O region of the PCI Device.
+
+The legacy configuration structure is described below.
+It consists of two parts.
+\begin{enumerate}
+  \item Legacy common configuration structure
+  \item Device configuration structure (optional)
+\end{enumerate}
+
+When used through the legacy interface, the legacy common
+configuration structure has the following layout:
+
+\begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
+\hline
+ Bits & 32 & 32 & 32 & 16 & 16 & 16 & 8 & 8 \\
+\hline
+ Read / Write & R & R+W & R+W & R & R+W & R+W & R+W & R \\
+\hline
+ Purpose & Device Features bits 0:31 & Driver Features bits 0:31 &
+  Queue Address & \field{queue_size} & \field{queue_select} & Queue Notify &
+  Device Status & ISR \newline Status \\
+\hline
+\end{tabularx}
+
+When MSI-X capability is enabled on the device, the device MUST
+present two additional fields immediately following the above fields:
+
+\begin{tabular}{ |l||l|l| }
+\hline
+Bits       & 16             & 16     \\
+\hline
+Read/Write & R+W            & R+W    \\
+\hline
+Purpose (MSI-X) & \field{config_msix_vector}  & \field{queue_msix_vector} \\
+\hline
+\end{tabular}
+
+The device configuration structure is optional. Its existence
+is decided by each device type. The transitional device MUST
+present the device-specific configuration structure if any at an
+offset immediately following the legacy common configuration structure.
+
+The device configuration structure:
+
+\begin{tabular}{|l||l|l|}
+\hline
+Bits & Device Specific & \multirow{3}{*}{\ldots} \\
+\cline{1-2}
+Read / Write & Device Specific & \\
+\cline{1-2}
+Purpose & Device Specific & \\
+\hline
+\end{tabular}
+
+Note: The device configuration structure byte offset is
+calculated dynamically; when MSI-X capability is enabled, the
+device configuration structure is located at byte offset 24,
+when MSI-X capability is disabled, the device configuration
+structure is located at byte offset 20.
+
+As legacy devices had no \field{config_generation} field,
+see \ref{sec:Basic Facilities of a Virtio Device / Device
+Configuration Space / Legacy Interface: Device Configuration
+Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
+
+When using the legacy interface, the transitional driver MUST
+use the legacy configuration structure in BAR0 in the first
+I/O region of the PCI device.
+
+When using the legacy interface, the driver MAY access
+the device-specific configuration structure using any width
+accesses and the transitional device MUST present the driver with
+the same results as when accessed using the ``natural'' access
+method (i.e. 32-bit accesses for 32-bit fields, etc).
+
+Note that this is possible because while the legacy common
+configuration structure is PCI (i.e. little) endian, when using
+the legacy interface the device-specific configuration structure
+is encoded in the native endian of the guest (where such
+distinction is applicable).
+
+The transitional driver when using the legacy interface MUST
+the device-specific configuration structure at an offset
+immediately following the legacy common configuration structure.
+
+
 \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
 
 The notification location is found using the VIRTIO_PCI_CAP_NOTIFY_CFG
@@ -767,98 +861,6 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
 specified by some other Virtio Structure PCI Capability
 of type other than \field{VIRTIO_PCI_CAP_PCI_CFG}.
 
-\subsubsection{Legacy Interfaces: A Note on Common configuration
-Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
-/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
-configuration Layout}
-
-The transitional device MUST present part of the configuration
-registers in a legacy configuration structure in BAR0 in the
-first I/O region of the PCI Device.
-
-The legacy configuration structure is described below.
-It consists of two parts.
-\begin{enumerate}
-  \item Legacy common configuration structure
-  \item Device configuration structure (optional)
-\end{enumerate}
-
-When used through the legacy interface, the legacy common
-configuration structure has the following layout:
-
-\begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
-\hline
- Bits & 32 & 32 & 32 & 16 & 16 & 16 & 8 & 8 \\
-\hline
- Read / Write & R & R+W & R+W & R & R+W & R+W & R+W & R \\
-\hline
- Purpose & Device Features bits 0:31 & Driver Features bits 0:31 &
-  Queue Address & \field{queue_size} & \field{queue_select} & Queue Notify &
-  Device Status & ISR \newline Status \\
-\hline
-\end{tabularx}
-
-When MSI-X capability is enabled on the device, the device MUST
-present two additional fields immediately following the above fields:
-
-\begin{tabular}{ |l||l|l| }
-\hline
-Bits       & 16             & 16     \\
-\hline
-Read/Write & R+W            & R+W    \\
-\hline
-Purpose (MSI-X) & \field{config_msix_vector}  & \field{queue_msix_vector} \\
-\hline
-\end{tabular}
-
-The device configuration structure is optional. Its existence
-is decided by each device type. The transitional device MUST
-present the device-specific configuration structure if any at an
-offset immediately following the legacy common configuration structure.
-
-The device configuration structure:
-
-\begin{tabular}{|l||l|l|}
-\hline
-Bits & Device Specific & \multirow{3}{*}{\ldots} \\
-\cline{1-2}
-Read / Write & Device Specific & \\
-\cline{1-2}
-Purpose & Device Specific & \\
-\hline
-\end{tabular}
-
-Note: The device configuration structure byte offset is
-calculated dynamically; when MSI-X capability is enabled, the
-device configuration structure is located at byte offset 24,
-when MSI-X capability is disabled, the device configuration
-structure is located at byte offset 20.
-
-As legacy devices had no \field{config_generation} field,
-see \ref{sec:Basic Facilities of a Virtio Device / Device
-Configuration Space / Legacy Interface: Device Configuration
-Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
-
-When using the legacy interface, the transitional driver MUST
-use the legacy configuration structure in BAR0 in the first
-I/O region of the PCI device.
-
-When using the legacy interface, the driver MAY access
-the device-specific configuration structure using any width
-accesses and the transitional device MUST present the driver with
-the same results as when accessed using the ``natural'' access
-method (i.e. 32-bit accesses for 32-bit fields, etc).
-
-Note that this is possible because while the legacy common
-configuration structure is PCI (i.e. little) endian, when using
-the legacy interface the device-specific configuration structure
-is encoded in the native endian of the guest (where such
-distinction is applicable).
-
-The transitional driver when using the legacy interface MUST
-the device-specific configuration structure at an offset
-immediately following the legacy common configuration structure.
-
 \subsubsection{Legacy Interface: A Note on feature
 bits}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
 Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
-- 
2.26.2

[virtio-dev] [PATCH 3/3] transport-pci: Relocate common config legacy interface

[Parav Pandit]

Relocate legacy interface section of common configuration structure near
where 1.x based common configuration structure is defined.

This aligns the spec to follow rest of the other Legacy interfaces
section.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 conformance.tex   |   3 +-
 transport-pci.tex | 186 +++++++++++++++++++++++-----------------------
 2 files changed, 95 insertions(+), 94 deletions(-)

diff --git a/conformance.tex b/conformance.tex
index 0d3616f..b654fe0 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -262,8 +262,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message Framing / Legacy Interface: Message Framing}
 \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/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
-configuration Layout}
+\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Common configuration structure layout / Legacy Interfaces: A Note on Common configuration Layout}
 \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
 \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}
diff --git a/transport-pci.tex b/transport-pci.tex
index 9d4c713..19375d0 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -494,6 +494,100 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 were used before the queue reset.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
+\paragraph{Legacy Interfaces: A Note on Common configuration
+Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
+/ Virtio Structure PCI Capabilities / Common configuration
+structure layout / Legacy Interfaces: A Note on Common
+configuration Layout}
+
+The transitional device MUST present part of the configuration
+registers in a legacy configuration structure in BAR0 in the
+first I/O region of the PCI Device.
+
+The legacy configuration structure is described below.
+It consists of two parts.
+\begin{enumerate}
+  \item Legacy common configuration structure
+  \item Device configuration structure (optional)
+\end{enumerate}
+
+When used through the legacy interface, the legacy common
+configuration structure has the following layout:
+
+\begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
+\hline
+ Bits & 32 & 32 & 32 & 16 & 16 & 16 & 8 & 8 \\
+\hline
+ Read / Write & R & R+W & R+W & R & R+W & R+W & R+W & R \\
+\hline
+ Purpose & Device Features bits 0:31 & Driver Features bits 0:31 &
+  Queue Address & \field{queue_size} & \field{queue_select} & Queue Notify &
+  Device Status & ISR \newline Status \\
+\hline
+\end{tabularx}
+
+When MSI-X capability is enabled on the device, the device MUST
+present two additional fields immediately following the above fields:
+
+\begin{tabular}{ |l||l|l| }
+\hline
+Bits       & 16             & 16     \\
+\hline
+Read/Write & R+W            & R+W    \\
+\hline
+Purpose (MSI-X) & \field{config_msix_vector}  & \field{queue_msix_vector} \\
+\hline
+\end{tabular}
+
+The device configuration structure is optional. Its existence
+is decided by each device type. The transitional device MUST
+present the device-specific configuration structure if any at an
+offset immediately following the legacy common configuration structure.
+
+The device configuration structure:
+
+\begin{tabular}{|l||l|l|}
+\hline
+Bits & Device Specific & \multirow{3}{*}{\ldots} \\
+\cline{1-2}
+Read / Write & Device Specific & \\
+\cline{1-2}
+Purpose & Device Specific & \\
+\hline
+\end{tabular}
+
+Note: The device configuration structure byte offset is
+calculated dynamically; when MSI-X capability is enabled, the
+device configuration structure is located at byte offset 24,
+when MSI-X capability is disabled, the device configuration
+structure is located at byte offset 20.
+
+As legacy devices had no \field{config_generation} field,
+see \ref{sec:Basic Facilities of a Virtio Device / Device
+Configuration Space / Legacy Interface: Device Configuration
+Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
+
+When using the legacy interface, the transitional driver MUST
+use the legacy configuration structure in BAR0 in the first
+I/O region of the PCI device.
+
+When using the legacy interface, the driver MAY access
+the device-specific configuration structure using any width
+accesses and the transitional device MUST present the driver with
+the same results as when accessed using the ``natural'' access
+method (i.e. 32-bit accesses for 32-bit fields, etc).
+
+Note that this is possible because while the legacy common
+configuration structure is PCI (i.e. little) endian, when using
+the legacy interface the device-specific configuration structure
+is encoded in the native endian of the guest (where such
+distinction is applicable).
+
+The transitional driver when using the legacy interface MUST
+the device-specific configuration structure at an offset
+immediately following the legacy common configuration structure.
+
+
 \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
 
 The notification location is found using the VIRTIO_PCI_CAP_NOTIFY_CFG
@@ -767,98 +861,6 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
 specified by some other Virtio Structure PCI Capability
 of type other than \field{VIRTIO_PCI_CAP_PCI_CFG}.
 
-\subsubsection{Legacy Interfaces: A Note on Common configuration
-Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
-/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
-configuration Layout}
-
-The transitional device MUST present part of the configuration
-registers in a legacy configuration structure in BAR0 in the
-first I/O region of the PCI Device.
-
-The legacy configuration structure is described below.
-It consists of two parts.
-\begin{enumerate}
-  \item Legacy common configuration structure
-  \item Device configuration structure (optional)
-\end{enumerate}
-
-When used through the legacy interface, the legacy common
-configuration structure has the following layout:
-
-\begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
-\hline
- Bits & 32 & 32 & 32 & 16 & 16 & 16 & 8 & 8 \\
-\hline
- Read / Write & R & R+W & R+W & R & R+W & R+W & R+W & R \\
-\hline
- Purpose & Device Features bits 0:31 & Driver Features bits 0:31 &
-  Queue Address & \field{queue_size} & \field{queue_select} & Queue Notify &
-  Device Status & ISR \newline Status \\
-\hline
-\end{tabularx}
-
-When MSI-X capability is enabled on the device, the device MUST
-present two additional fields immediately following the above fields:
-
-\begin{tabular}{ |l||l|l| }
-\hline
-Bits       & 16             & 16     \\
-\hline
-Read/Write & R+W            & R+W    \\
-\hline
-Purpose (MSI-X) & \field{config_msix_vector}  & \field{queue_msix_vector} \\
-\hline
-\end{tabular}
-
-The device configuration structure is optional. Its existence
-is decided by each device type. The transitional device MUST
-present the device-specific configuration structure if any at an
-offset immediately following the legacy common configuration structure.
-
-The device configuration structure:
-
-\begin{tabular}{|l||l|l|}
-\hline
-Bits & Device Specific & \multirow{3}{*}{\ldots} \\
-\cline{1-2}
-Read / Write & Device Specific & \\
-\cline{1-2}
-Purpose & Device Specific & \\
-\hline
-\end{tabular}
-
-Note: The device configuration structure byte offset is
-calculated dynamically; when MSI-X capability is enabled, the
-device configuration structure is located at byte offset 24,
-when MSI-X capability is disabled, the device configuration
-structure is located at byte offset 20.
-
-As legacy devices had no \field{config_generation} field,
-see \ref{sec:Basic Facilities of a Virtio Device / Device
-Configuration Space / Legacy Interface: Device Configuration
-Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
-
-When using the legacy interface, the transitional driver MUST
-use the legacy configuration structure in BAR0 in the first
-I/O region of the PCI device.
-
-When using the legacy interface, the driver MAY access
-the device-specific configuration structure using any width
-accesses and the transitional device MUST present the driver with
-the same results as when accessed using the ``natural'' access
-method (i.e. 32-bit accesses for 32-bit fields, etc).
-
-Note that this is possible because while the legacy common
-configuration structure is PCI (i.e. little) endian, when using
-the legacy interface the device-specific configuration structure
-is encoded in the native endian of the guest (where such
-distinction is applicable).
-
-The transitional driver when using the legacy interface MUST
-the device-specific configuration structure at an offset
-immediately following the legacy common configuration structure.
-
 \subsubsection{Legacy Interface: A Note on feature
 bits}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
 Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
-- 
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

Re: [PATCH 1/3] transport-pci: Improve PCI legacy device layout description

[Michael S. Tsirkin]

On Sun, Feb 26, 2023 at 12:29:59AM +0200, Parav Pandit wrote:
> Legacy interface PCI Device layout description has following issues.
> 
> 1. repeated 'structure' word
> 2. virtio header was defined the 0.9.5 spec. In a legacy interface
>    section it is referred with different keywards
>    as (a) virtio header, (b) general headers, (c) legacy configuration
>    structure, (d) virtio common configuration structure and
>    (e) other fields.
> 3. Driver and device requirements listing is intermixed.
> 4. spelling error of structure
> 
> Hence, rewrite the description to eliminate above issues as below.
> 
> 1. Removed repeated structure word
> 2. Fix spelling of structure
> 3. Place all device requirements toegether
> 3. Define legacy configuration structure that consist of
>    a. legacy common configuration structure and
>    b. device specific configuration structure
> 4. Rewrite section around above changes
> 
> This is only an editorial change.

No, editorial changes are things like spelling corrections.

I have trouble reviewing
because I have no idea why you are making each change.

E.g. you just rewrote a bunch of text and I frankly don't know
why. For example:

	-When using the legacy interface, transitional drivers
	-MUST use the legacy configuration structure in BAR0 in the first
	-I/O region of the PCI device, as documented below.

is now apparently:

	+When used through the legacy interface, the legacy common
	+configuration structure has the following layout:

and this is better? why? we are replacing a clear requirement
which applied to drivers with a vague statement which
I can't say what it applies to.

Any chance of splitting these things up? Maybe that will help.

Apropos I don't know that we want to spend much time on legacy sections.
Really with legacy code is the main source of documentation -
if drivers work then you are golden. If they don't appealing
to spec will not help.



> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
>  transport-pci.tex | 93 ++++++++++++++++++++++++++---------------------
>  1 file changed, 52 insertions(+), 41 deletions(-)
> 
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 5d22e6f..9ee37ba 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -769,25 +769,19 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
>  
>  \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
>  
> -Transitional devices MUST present part of configuration
> -registers in a legacy configuration structure in BAR0 in the first I/O
> -region of the PCI device, as documented below.
> -When using the legacy interface, transitional drivers
> -MUST use the legacy configuration structure in BAR0 in the first
> -I/O region of the PCI device, as documented below.
> -
> -When using the legacy interface the driver MAY access
> -the device-specific configuration region using any width accesses, and
> -a transitional device MUST present driver with the same results as
> -when accessed using the ``natural'' access method (i.e.
> -32-bit accesses for 32-bit fields, etc).
> -
> -Note that this is possible because while the virtio common configuration structure is PCI
> -(i.e. little) endian, when using the legacy interface the device-specific
> -configuration region is encoded in the native endian of the guest (where such distinction is
> -applicable).
> -
> -When used through the legacy interface, the virtio common configuration structure looks as follows:
> +The transitional device MUST present part of the configuration
> +registers in a legacy configuration structure in BAR0 in the
> +first I/O region of the PCI Device.
> +
> +The legacy configuration structure is described below.
> +It consists of two parts.
> +\begin{enumerate}
> +  \item Legacy common configuration structure
> +  \item Device configuration structure (optional)
> +\end{enumerate}
> +
> +When used through the legacy interface, the legacy common
> +configuration structure has the following layout:
>  
>  \begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
>  \hline
> @@ -801,8 +795,8 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
>  \hline
>  \end{tabularx}
>  
> -If MSI-X is enabled for the device, two additional fields
> -immediately follow this header:
> +When MSI-X capability is enabled on the device, the device MUST
> +present two additional fields immediately following the above fields:
>  
>  \begin{tabular}{ |l||l|l| }
>  \hline
> @@ -814,14 +808,12 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
>  \hline
>  \end{tabular}
>  
> -Note: When MSI-X capability is enabled, device-specific configuration starts at
> -byte offset 24 in virtio common configuration structure structure. When MSI-X capability is not
> -enabled, device-specific configuration starts at byte offset 20 in virtio
> -header.  ie. once you enable MSI-X on the device, the other fields move.
> -If you turn it off again, they move back!
> +The device configuration structure is optional. Its existence
> +is decided by each device type. The transitional device MUST
> +present the device-specific configuration structure if any at an
> +offset immediately following the legacy common configuration structure.
>  
> -Any device-specific configuration space immediately follows
> -these general headers:
> +The device configuration structure:
>  
>  \begin{tabular}{|l||l|l|}
>  \hline
> @@ -833,25 +825,44 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
>  \hline
>  \end{tabular}
>  
> -When accessing the device-specific configuration space
> -using the legacy interface, transitional
> -drivers MUST access the device-specific configuration space
> -at an offset immediately following the general headers.
> -
> -When using the legacy interface, transitional
> -devices MUST present the device-specific configuration space
> -if any at an offset immediately following the general headers.
> -
> -Note that only Feature Bits 0 to 31 are accessible through the
> -Legacy Interface. When used through the Legacy Interface,
> -Transitional Devices MUST assume that Feature Bits 32 to 63
> -are not acknowledged by Driver.
> +Note: The device configuration structure byte offset is
> +calculated dynamically; when MSI-X capability is enabled, the
> +device configuration structure is located at byte offset 24,
> +when MSI-X capability is disabled, the device configuration
> +structure is located at byte offset 20.
>  
>  As legacy devices had no \field{config_generation} field,
>  see \ref{sec:Basic Facilities of a Virtio Device / Device
>  Configuration Space / Legacy Interface: Device Configuration
>  Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
>  
> +When using the legacy interface, the transitional driver MUST
> +use the legacy configuration structure in BAR0 in the first
> +I/O region of the PCI device.
> +
> +When using the legacy interface, the driver MAY access
> +the device-specific configuration structure using any width
> +accesses and the transitional device MUST present the driver with
> +the same results as when accessed using the ``natural'' access
> +method (i.e. 32-bit accesses for 32-bit fields, etc).
> +
> +Note that this is possible because while the legacy common
> +configuration structure is PCI (i.e. little) endian, when using
> +the legacy interface the device-specific configuration structure
> +is encoded in the native endian of the guest (where such
> +distinction is applicable).
> +
> +When accessing the device-specific configuration structure
> +using the legacy interface, transitional drivers MUST access
> +the device-specific configuration structure
> +at an offset immediately following the legacy common
> +configuration structure.
> +
> +Note that only Feature Bits 0 to 31 are accessible through the
> +Legacy Interface. When used through the Legacy Interface,
> +the transitional device MUST assume that Feature Bits 32 to 63
> +are not acknowledged by the driver.
> +
>  \subsubsection{Non-transitional Device With Legacy Driver: A Note
>  on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio
>  Over PCI Bus / PCI Device Layout / Non-transitional Device With
> -- 
> 2.26.2

[virtio-dev] Re: [PATCH 1/3] transport-pci: Improve PCI legacy device layout description

[Michael S. Tsirkin]

On Sun, Feb 26, 2023 at 12:29:59AM +0200, Parav Pandit wrote:
> Legacy interface PCI Device layout description has following issues.
> 
> 1. repeated 'structure' word
> 2. virtio header was defined the 0.9.5 spec. In a legacy interface
>    section it is referred with different keywards
>    as (a) virtio header, (b) general headers, (c) legacy configuration
>    structure, (d) virtio common configuration structure and
>    (e) other fields.
> 3. Driver and device requirements listing is intermixed.
> 4. spelling error of structure
> 
> Hence, rewrite the description to eliminate above issues as below.
> 
> 1. Removed repeated structure word
> 2. Fix spelling of structure
> 3. Place all device requirements toegether
> 3. Define legacy configuration structure that consist of
>    a. legacy common configuration structure and
>    b. device specific configuration structure
> 4. Rewrite section around above changes
> 
> This is only an editorial change.

No, editorial changes are things like spelling corrections.

I have trouble reviewing
because I have no idea why you are making each change.

E.g. you just rewrote a bunch of text and I frankly don't know
why. For example:

	-When using the legacy interface, transitional drivers
	-MUST use the legacy configuration structure in BAR0 in the first
	-I/O region of the PCI device, as documented below.

is now apparently:

	+When used through the legacy interface, the legacy common
	+configuration structure has the following layout:

and this is better? why? we are replacing a clear requirement
which applied to drivers with a vague statement which
I can't say what it applies to.

Any chance of splitting these things up? Maybe that will help.

Apropos I don't know that we want to spend much time on legacy sections.
Really with legacy code is the main source of documentation -
if drivers work then you are golden. If they don't appealing
to spec will not help.



> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
>  transport-pci.tex | 93 ++++++++++++++++++++++++++---------------------
>  1 file changed, 52 insertions(+), 41 deletions(-)
> 
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 5d22e6f..9ee37ba 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -769,25 +769,19 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
>  
>  \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
>  
> -Transitional devices MUST present part of configuration
> -registers in a legacy configuration structure in BAR0 in the first I/O
> -region of the PCI device, as documented below.
> -When using the legacy interface, transitional drivers
> -MUST use the legacy configuration structure in BAR0 in the first
> -I/O region of the PCI device, as documented below.
> -
> -When using the legacy interface the driver MAY access
> -the device-specific configuration region using any width accesses, and
> -a transitional device MUST present driver with the same results as
> -when accessed using the ``natural'' access method (i.e.
> -32-bit accesses for 32-bit fields, etc).
> -
> -Note that this is possible because while the virtio common configuration structure is PCI
> -(i.e. little) endian, when using the legacy interface the device-specific
> -configuration region is encoded in the native endian of the guest (where such distinction is
> -applicable).
> -
> -When used through the legacy interface, the virtio common configuration structure looks as follows:
> +The transitional device MUST present part of the configuration
> +registers in a legacy configuration structure in BAR0 in the
> +first I/O region of the PCI Device.
> +
> +The legacy configuration structure is described below.
> +It consists of two parts.
> +\begin{enumerate}
> +  \item Legacy common configuration structure
> +  \item Device configuration structure (optional)
> +\end{enumerate}
> +
> +When used through the legacy interface, the legacy common
> +configuration structure has the following layout:
>  
>  \begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
>  \hline
> @@ -801,8 +795,8 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
>  \hline
>  \end{tabularx}
>  
> -If MSI-X is enabled for the device, two additional fields
> -immediately follow this header:
> +When MSI-X capability is enabled on the device, the device MUST
> +present two additional fields immediately following the above fields:
>  
>  \begin{tabular}{ |l||l|l| }
>  \hline
> @@ -814,14 +808,12 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
>  \hline
>  \end{tabular}
>  
> -Note: When MSI-X capability is enabled, device-specific configuration starts at
> -byte offset 24 in virtio common configuration structure structure. When MSI-X capability is not
> -enabled, device-specific configuration starts at byte offset 20 in virtio
> -header.  ie. once you enable MSI-X on the device, the other fields move.
> -If you turn it off again, they move back!
> +The device configuration structure is optional. Its existence
> +is decided by each device type. The transitional device MUST
> +present the device-specific configuration structure if any at an
> +offset immediately following the legacy common configuration structure.
>  
> -Any device-specific configuration space immediately follows
> -these general headers:
> +The device configuration structure:
>  
>  \begin{tabular}{|l||l|l|}
>  \hline
> @@ -833,25 +825,44 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
>  \hline
>  \end{tabular}
>  
> -When accessing the device-specific configuration space
> -using the legacy interface, transitional
> -drivers MUST access the device-specific configuration space
> -at an offset immediately following the general headers.
> -
> -When using the legacy interface, transitional
> -devices MUST present the device-specific configuration space
> -if any at an offset immediately following the general headers.
> -
> -Note that only Feature Bits 0 to 31 are accessible through the
> -Legacy Interface. When used through the Legacy Interface,
> -Transitional Devices MUST assume that Feature Bits 32 to 63
> -are not acknowledged by Driver.
> +Note: The device configuration structure byte offset is
> +calculated dynamically; when MSI-X capability is enabled, the
> +device configuration structure is located at byte offset 24,
> +when MSI-X capability is disabled, the device configuration
> +structure is located at byte offset 20.
>  
>  As legacy devices had no \field{config_generation} field,
>  see \ref{sec:Basic Facilities of a Virtio Device / Device
>  Configuration Space / Legacy Interface: Device Configuration
>  Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
>  
> +When using the legacy interface, the transitional driver MUST
> +use the legacy configuration structure in BAR0 in the first
> +I/O region of the PCI device.
> +
> +When using the legacy interface, the driver MAY access
> +the device-specific configuration structure using any width
> +accesses and the transitional device MUST present the driver with
> +the same results as when accessed using the ``natural'' access
> +method (i.e. 32-bit accesses for 32-bit fields, etc).
> +
> +Note that this is possible because while the legacy common
> +configuration structure is PCI (i.e. little) endian, when using
> +the legacy interface the device-specific configuration structure
> +is encoded in the native endian of the guest (where such
> +distinction is applicable).
> +
> +When accessing the device-specific configuration structure
> +using the legacy interface, transitional drivers MUST access
> +the device-specific configuration structure
> +at an offset immediately following the legacy common
> +configuration structure.
> +
> +Note that only Feature Bits 0 to 31 are accessible through the
> +Legacy Interface. When used through the Legacy Interface,
> +the transitional device MUST assume that Feature Bits 32 to 63
> +are not acknowledged by the driver.
> +
>  \subsubsection{Non-transitional Device With Legacy Driver: A Note
>  on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio
>  Over PCI Bus / PCI Device Layout / Non-transitional Device With
> -- 
> 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

Re: [PATCH 2/3] transport-pci: Split notes of PCI Device Layout

[Michael S. Tsirkin]

On Sun, Feb 26, 2023 at 12:30:00AM +0200, Parav Pandit wrote:
> Currently single legacy interface section describes PCI common
> configuration layout and feature bits operation for the
> legacy interface.
> Secondly common configuration structure description of legacy interface
> is not adjacent to the the respective normal device requirements for
> same.
> 
> Hence, split PCI Device Layout legacy interface section into two
> parts. First subsection for common configuration and second
> subsection for feature bits.
> 
> Subsequent patch relocates common configuration legacy interface to
> appropriate matching location.
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> Signed-off-by: Parav Pandit <parav@nvidia.com>

I don't care really. However this does much more than move text around
as it claim to do:


> ---
>  conformance.tex   |  4 +++-
>  transport-pci.tex | 19 ++++++++++++-------
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/conformance.tex b/conformance.tex
> index 01ccd69..0d3616f 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -262,7 +262,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message Framing / Legacy Interface: Message Framing}
>  \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}
> +\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
> +configuration Layout}
> +\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
>  \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{sec:Virtio Transport Options / Virtio Over MMIO / Legacy interface}
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 9ee37ba..9d4c713 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -767,7 +767,10 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
>  specified by some other Virtio Structure PCI Capability
>  of type other than \field{VIRTIO_PCI_CAP_PCI_CFG}.
>  
> -\subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
> +\subsubsection{Legacy Interfaces: A Note on Common configuration
> +Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
> +/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
> +configuration Layout}
>  
>  The transitional device MUST present part of the configuration
>  registers in a legacy configuration structure in BAR0 in the


Please do not split up labels to multiple lines like this, it is hard to
find and fix them if you do.

> @@ -852,13 +855,15 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
>  is encoded in the native endian of the guest (where such
>  distinction is applicable).
>  
> -When accessing the device-specific configuration structure
> -using the legacy interface, transitional drivers MUST access
> -the device-specific configuration structure
> -at an offset immediately following the legacy common
> -configuration structure.
> +The transitional driver when using the legacy interface MUST
> +the device-specific configuration structure at an offset
> +immediately following the legacy common configuration structure.

Oh great and in the process of presumably just moving stuff around you
are also losing text - the result is agrammatical, and much less clear
than the original.


>  
> -Note that only Feature Bits 0 to 31 are accessible through the
> +\subsubsection{Legacy Interface: A Note on feature
> +bits}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
> +Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
> +
> +Only Feature Bits 0 to 31 are accessible through the
>  Legacy Interface. When used through the Legacy Interface,
>  the transitional device MUST assume that Feature Bits 32 to 63
>  are not acknowledged by the driver.

And you are dropping "Note" here because why? Seems notable
in that there are more feature bits, someone might miss this
fact.




> -- 
> 2.26.2

[virtio-dev] Re: [PATCH 2/3] transport-pci: Split notes of PCI Device Layout

[Michael S. Tsirkin]

On Sun, Feb 26, 2023 at 12:30:00AM +0200, Parav Pandit wrote:
> Currently single legacy interface section describes PCI common
> configuration layout and feature bits operation for the
> legacy interface.
> Secondly common configuration structure description of legacy interface
> is not adjacent to the the respective normal device requirements for
> same.
> 
> Hence, split PCI Device Layout legacy interface section into two
> parts. First subsection for common configuration and second
> subsection for feature bits.
> 
> Subsequent patch relocates common configuration legacy interface to
> appropriate matching location.
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> Signed-off-by: Parav Pandit <parav@nvidia.com>

I don't care really. However this does much more than move text around
as it claim to do:


> ---
>  conformance.tex   |  4 +++-
>  transport-pci.tex | 19 ++++++++++++-------
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/conformance.tex b/conformance.tex
> index 01ccd69..0d3616f 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -262,7 +262,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message Framing / Legacy Interface: Message Framing}
>  \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}
> +\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
> +configuration Layout}
> +\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
>  \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{sec:Virtio Transport Options / Virtio Over MMIO / Legacy interface}
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 9ee37ba..9d4c713 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -767,7 +767,10 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
>  specified by some other Virtio Structure PCI Capability
>  of type other than \field{VIRTIO_PCI_CAP_PCI_CFG}.
>  
> -\subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
> +\subsubsection{Legacy Interfaces: A Note on Common configuration
> +Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
> +/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
> +configuration Layout}
>  
>  The transitional device MUST present part of the configuration
>  registers in a legacy configuration structure in BAR0 in the


Please do not split up labels to multiple lines like this, it is hard to
find and fix them if you do.

> @@ -852,13 +855,15 @@ \subsubsection{Legacy Interfaces: A Note on PCI Device Layout}\label{sec:Virtio
>  is encoded in the native endian of the guest (where such
>  distinction is applicable).
>  
> -When accessing the device-specific configuration structure
> -using the legacy interface, transitional drivers MUST access
> -the device-specific configuration structure
> -at an offset immediately following the legacy common
> -configuration structure.
> +The transitional driver when using the legacy interface MUST
> +the device-specific configuration structure at an offset
> +immediately following the legacy common configuration structure.

Oh great and in the process of presumably just moving stuff around you
are also losing text - the result is agrammatical, and much less clear
than the original.


>  
> -Note that only Feature Bits 0 to 31 are accessible through the
> +\subsubsection{Legacy Interface: A Note on feature
> +bits}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
> +Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
> +
> +Only Feature Bits 0 to 31 are accessible through the
>  Legacy Interface. When used through the Legacy Interface,
>  the transitional device MUST assume that Feature Bits 32 to 63
>  are not acknowledged by the driver.

And you are dropping "Note" here because why? Seems notable
in that there are more feature bits, someone might miss this
fact.




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

Re: [PATCH 3/3] transport-pci: Relocate common config legacy interface

[Michael S. Tsirkin]

On Sun, Feb 26, 2023 at 12:30:01AM +0200, Parav Pandit wrote:
> Relocate legacy interface section of common configuration structure near
> where 1.x based common configuration structure is defined.
> 
> This aligns the spec to follow rest of the other Legacy interfaces
> section.
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
>  conformance.tex   |   3 +-
>  transport-pci.tex | 186 +++++++++++++++++++++++-----------------------
>  2 files changed, 95 insertions(+), 94 deletions(-)
> 

I can't be bothered to check whether this actually just moves text
around or also changes a bunch of stuff on the way.

> diff --git a/conformance.tex b/conformance.tex
> index 0d3616f..b654fe0 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -262,8 +262,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message Framing / Legacy Interface: Message Framing}
>  \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/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
> -configuration Layout}
> +\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Common configuration structure layout / Legacy Interfaces: A Note on Common configuration Layout}
>  \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
>  \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}
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 9d4c713..19375d0 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -494,6 +494,100 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>  were used before the queue reset.
>  (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
>  
> +\paragraph{Legacy Interfaces: A Note on Common configuration
> +Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
> +/ Virtio Structure PCI Capabilities / Common configuration
> +structure layout / Legacy Interfaces: A Note on Common
> +configuration Layout}
> +
> +The transitional device MUST present part of the configuration
> +registers in a legacy configuration structure in BAR0 in the
> +first I/O region of the PCI Device.
> +
> +The legacy configuration structure is described below.
> +It consists of two parts.
> +\begin{enumerate}
> +  \item Legacy common configuration structure
> +  \item Device configuration structure (optional)
> +\end{enumerate}
> +
> +When used through the legacy interface, the legacy common
> +configuration structure has the following layout:
> +
> +\begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
> +\hline
> + Bits & 32 & 32 & 32 & 16 & 16 & 16 & 8 & 8 \\
> +\hline
> + Read / Write & R & R+W & R+W & R & R+W & R+W & R+W & R \\
> +\hline
> + Purpose & Device Features bits 0:31 & Driver Features bits 0:31 &
> +  Queue Address & \field{queue_size} & \field{queue_select} & Queue Notify &
> +  Device Status & ISR \newline Status \\
> +\hline
> +\end{tabularx}
> +
> +When MSI-X capability is enabled on the device, the device MUST
> +present two additional fields immediately following the above fields:
> +
> +\begin{tabular}{ |l||l|l| }
> +\hline
> +Bits       & 16             & 16     \\
> +\hline
> +Read/Write & R+W            & R+W    \\
> +\hline
> +Purpose (MSI-X) & \field{config_msix_vector}  & \field{queue_msix_vector} \\
> +\hline
> +\end{tabular}
> +
> +The device configuration structure is optional. Its existence
> +is decided by each device type. The transitional device MUST
> +present the device-specific configuration structure if any at an
> +offset immediately following the legacy common configuration structure.
> +
> +The device configuration structure:
> +
> +\begin{tabular}{|l||l|l|}
> +\hline
> +Bits & Device Specific & \multirow{3}{*}{\ldots} \\
> +\cline{1-2}
> +Read / Write & Device Specific & \\
> +\cline{1-2}
> +Purpose & Device Specific & \\
> +\hline
> +\end{tabular}
> +
> +Note: The device configuration structure byte offset is
> +calculated dynamically; when MSI-X capability is enabled, the
> +device configuration structure is located at byte offset 24,
> +when MSI-X capability is disabled, the device configuration
> +structure is located at byte offset 20.
> +
> +As legacy devices had no \field{config_generation} field,
> +see \ref{sec:Basic Facilities of a Virtio Device / Device
> +Configuration Space / Legacy Interface: Device Configuration
> +Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
> +
> +When using the legacy interface, the transitional driver MUST
> +use the legacy configuration structure in BAR0 in the first
> +I/O region of the PCI device.
> +
> +When using the legacy interface, the driver MAY access
> +the device-specific configuration structure using any width
> +accesses and the transitional device MUST present the driver with
> +the same results as when accessed using the ``natural'' access
> +method (i.e. 32-bit accesses for 32-bit fields, etc).
> +
> +Note that this is possible because while the legacy common
> +configuration structure is PCI (i.e. little) endian, when using
> +the legacy interface the device-specific configuration structure
> +is encoded in the native endian of the guest (where such
> +distinction is applicable).
> +
> +The transitional driver when using the legacy interface MUST
> +the device-specific configuration structure at an offset
> +immediately following the legacy common configuration structure.
> +
> +
>  \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
>  
>  The notification location is found using the VIRTIO_PCI_CAP_NOTIFY_CFG
> @@ -767,98 +861,6 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
>  specified by some other Virtio Structure PCI Capability
>  of type other than \field{VIRTIO_PCI_CAP_PCI_CFG}.
>  
> -\subsubsection{Legacy Interfaces: A Note on Common configuration
> -Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
> -/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
> -configuration Layout}
> -
> -The transitional device MUST present part of the configuration
> -registers in a legacy configuration structure in BAR0 in the
> -first I/O region of the PCI Device.
> -
> -The legacy configuration structure is described below.
> -It consists of two parts.
> -\begin{enumerate}
> -  \item Legacy common configuration structure
> -  \item Device configuration structure (optional)
> -\end{enumerate}
> -
> -When used through the legacy interface, the legacy common
> -configuration structure has the following layout:
> -
> -\begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
> -\hline
> - Bits & 32 & 32 & 32 & 16 & 16 & 16 & 8 & 8 \\
> -\hline
> - Read / Write & R & R+W & R+W & R & R+W & R+W & R+W & R \\
> -\hline
> - Purpose & Device Features bits 0:31 & Driver Features bits 0:31 &
> -  Queue Address & \field{queue_size} & \field{queue_select} & Queue Notify &
> -  Device Status & ISR \newline Status \\
> -\hline
> -\end{tabularx}
> -
> -When MSI-X capability is enabled on the device, the device MUST
> -present two additional fields immediately following the above fields:
> -
> -\begin{tabular}{ |l||l|l| }
> -\hline
> -Bits       & 16             & 16     \\
> -\hline
> -Read/Write & R+W            & R+W    \\
> -\hline
> -Purpose (MSI-X) & \field{config_msix_vector}  & \field{queue_msix_vector} \\
> -\hline
> -\end{tabular}
> -
> -The device configuration structure is optional. Its existence
> -is decided by each device type. The transitional device MUST
> -present the device-specific configuration structure if any at an
> -offset immediately following the legacy common configuration structure.
> -
> -The device configuration structure:
> -
> -\begin{tabular}{|l||l|l|}
> -\hline
> -Bits & Device Specific & \multirow{3}{*}{\ldots} \\
> -\cline{1-2}
> -Read / Write & Device Specific & \\
> -\cline{1-2}
> -Purpose & Device Specific & \\
> -\hline
> -\end{tabular}
> -
> -Note: The device configuration structure byte offset is
> -calculated dynamically; when MSI-X capability is enabled, the
> -device configuration structure is located at byte offset 24,
> -when MSI-X capability is disabled, the device configuration
> -structure is located at byte offset 20.
> -
> -As legacy devices had no \field{config_generation} field,
> -see \ref{sec:Basic Facilities of a Virtio Device / Device
> -Configuration Space / Legacy Interface: Device Configuration
> -Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
> -
> -When using the legacy interface, the transitional driver MUST
> -use the legacy configuration structure in BAR0 in the first
> -I/O region of the PCI device.
> -
> -When using the legacy interface, the driver MAY access
> -the device-specific configuration structure using any width
> -accesses and the transitional device MUST present the driver with
> -the same results as when accessed using the ``natural'' access
> -method (i.e. 32-bit accesses for 32-bit fields, etc).
> -
> -Note that this is possible because while the legacy common
> -configuration structure is PCI (i.e. little) endian, when using
> -the legacy interface the device-specific configuration structure
> -is encoded in the native endian of the guest (where such
> -distinction is applicable).
> -
> -The transitional driver when using the legacy interface MUST
> -the device-specific configuration structure at an offset
> -immediately following the legacy common configuration structure.
> -
>  \subsubsection{Legacy Interface: A Note on feature
>  bits}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
>  Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
> -- 
> 2.26.2

[virtio-dev] Re: [PATCH 3/3] transport-pci: Relocate common config legacy interface

[Michael S. Tsirkin]

On Sun, Feb 26, 2023 at 12:30:01AM +0200, Parav Pandit wrote:
> Relocate legacy interface section of common configuration structure near
> where 1.x based common configuration structure is defined.
> 
> This aligns the spec to follow rest of the other Legacy interfaces
> section.
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
>  conformance.tex   |   3 +-
>  transport-pci.tex | 186 +++++++++++++++++++++++-----------------------
>  2 files changed, 95 insertions(+), 94 deletions(-)
> 

I can't be bothered to check whether this actually just moves text
around or also changes a bunch of stuff on the way.

> diff --git a/conformance.tex b/conformance.tex
> index 0d3616f..b654fe0 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -262,8 +262,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item Section \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message Framing / Legacy Interface: Message Framing}
>  \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/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
> -configuration Layout}
> +\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Common configuration structure layout / Legacy Interfaces: A Note on Common configuration Layout}
>  \item Section \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
>  \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}
> diff --git a/transport-pci.tex b/transport-pci.tex
> index 9d4c713..19375d0 100644
> --- a/transport-pci.tex
> +++ b/transport-pci.tex
> @@ -494,6 +494,100 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>  were used before the queue reset.
>  (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
>  
> +\paragraph{Legacy Interfaces: A Note on Common configuration
> +Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
> +/ Virtio Structure PCI Capabilities / Common configuration
> +structure layout / Legacy Interfaces: A Note on Common
> +configuration Layout}
> +
> +The transitional device MUST present part of the configuration
> +registers in a legacy configuration structure in BAR0 in the
> +first I/O region of the PCI Device.
> +
> +The legacy configuration structure is described below.
> +It consists of two parts.
> +\begin{enumerate}
> +  \item Legacy common configuration structure
> +  \item Device configuration structure (optional)
> +\end{enumerate}
> +
> +When used through the legacy interface, the legacy common
> +configuration structure has the following layout:
> +
> +\begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
> +\hline
> + Bits & 32 & 32 & 32 & 16 & 16 & 16 & 8 & 8 \\
> +\hline
> + Read / Write & R & R+W & R+W & R & R+W & R+W & R+W & R \\
> +\hline
> + Purpose & Device Features bits 0:31 & Driver Features bits 0:31 &
> +  Queue Address & \field{queue_size} & \field{queue_select} & Queue Notify &
> +  Device Status & ISR \newline Status \\
> +\hline
> +\end{tabularx}
> +
> +When MSI-X capability is enabled on the device, the device MUST
> +present two additional fields immediately following the above fields:
> +
> +\begin{tabular}{ |l||l|l| }
> +\hline
> +Bits       & 16             & 16     \\
> +\hline
> +Read/Write & R+W            & R+W    \\
> +\hline
> +Purpose (MSI-X) & \field{config_msix_vector}  & \field{queue_msix_vector} \\
> +\hline
> +\end{tabular}
> +
> +The device configuration structure is optional. Its existence
> +is decided by each device type. The transitional device MUST
> +present the device-specific configuration structure if any at an
> +offset immediately following the legacy common configuration structure.
> +
> +The device configuration structure:
> +
> +\begin{tabular}{|l||l|l|}
> +\hline
> +Bits & Device Specific & \multirow{3}{*}{\ldots} \\
> +\cline{1-2}
> +Read / Write & Device Specific & \\
> +\cline{1-2}
> +Purpose & Device Specific & \\
> +\hline
> +\end{tabular}
> +
> +Note: The device configuration structure byte offset is
> +calculated dynamically; when MSI-X capability is enabled, the
> +device configuration structure is located at byte offset 24,
> +when MSI-X capability is disabled, the device configuration
> +structure is located at byte offset 20.
> +
> +As legacy devices had no \field{config_generation} field,
> +see \ref{sec:Basic Facilities of a Virtio Device / Device
> +Configuration Space / Legacy Interface: Device Configuration
> +Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
> +
> +When using the legacy interface, the transitional driver MUST
> +use the legacy configuration structure in BAR0 in the first
> +I/O region of the PCI device.
> +
> +When using the legacy interface, the driver MAY access
> +the device-specific configuration structure using any width
> +accesses and the transitional device MUST present the driver with
> +the same results as when accessed using the ``natural'' access
> +method (i.e. 32-bit accesses for 32-bit fields, etc).
> +
> +Note that this is possible because while the legacy common
> +configuration structure is PCI (i.e. little) endian, when using
> +the legacy interface the device-specific configuration structure
> +is encoded in the native endian of the guest (where such
> +distinction is applicable).
> +
> +The transitional driver when using the legacy interface MUST
> +the device-specific configuration structure at an offset
> +immediately following the legacy common configuration structure.
> +
> +
>  \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
>  
>  The notification location is found using the VIRTIO_PCI_CAP_NOTIFY_CFG
> @@ -767,98 +861,6 @@ \subsubsection{PCI configuration access capability}\label{sec:Virtio Transport O
>  specified by some other Virtio Structure PCI Capability
>  of type other than \field{VIRTIO_PCI_CAP_PCI_CFG}.
>  
> -\subsubsection{Legacy Interfaces: A Note on Common configuration
> -Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus
> -/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on Common
> -configuration Layout}
> -
> -The transitional device MUST present part of the configuration
> -registers in a legacy configuration structure in BAR0 in the
> -first I/O region of the PCI Device.
> -
> -The legacy configuration structure is described below.
> -It consists of two parts.
> -\begin{enumerate}
> -  \item Legacy common configuration structure
> -  \item Device configuration structure (optional)
> -\end{enumerate}
> -
> -When used through the legacy interface, the legacy common
> -configuration structure has the following layout:
> -
> -\begin{tabularx}{\textwidth}{ |X||X|X|X|X|X|X|X|X| }
> -\hline
> - Bits & 32 & 32 & 32 & 16 & 16 & 16 & 8 & 8 \\
> -\hline
> - Read / Write & R & R+W & R+W & R & R+W & R+W & R+W & R \\
> -\hline
> - Purpose & Device Features bits 0:31 & Driver Features bits 0:31 &
> -  Queue Address & \field{queue_size} & \field{queue_select} & Queue Notify &
> -  Device Status & ISR \newline Status \\
> -\hline
> -\end{tabularx}
> -
> -When MSI-X capability is enabled on the device, the device MUST
> -present two additional fields immediately following the above fields:
> -
> -\begin{tabular}{ |l||l|l| }
> -\hline
> -Bits       & 16             & 16     \\
> -\hline
> -Read/Write & R+W            & R+W    \\
> -\hline
> -Purpose (MSI-X) & \field{config_msix_vector}  & \field{queue_msix_vector} \\
> -\hline
> -\end{tabular}
> -
> -The device configuration structure is optional. Its existence
> -is decided by each device type. The transitional device MUST
> -present the device-specific configuration structure if any at an
> -offset immediately following the legacy common configuration structure.
> -
> -The device configuration structure:
> -
> -\begin{tabular}{|l||l|l|}
> -\hline
> -Bits & Device Specific & \multirow{3}{*}{\ldots} \\
> -\cline{1-2}
> -Read / Write & Device Specific & \\
> -\cline{1-2}
> -Purpose & Device Specific & \\
> -\hline
> -\end{tabular}
> -
> -Note: The device configuration structure byte offset is
> -calculated dynamically; when MSI-X capability is enabled, the
> -device configuration structure is located at byte offset 24,
> -when MSI-X capability is disabled, the device configuration
> -structure is located at byte offset 20.
> -
> -As legacy devices had no \field{config_generation} field,
> -see \ref{sec:Basic Facilities of a Virtio Device / Device
> -Configuration Space / Legacy Interface: Device Configuration
> -Space}~\nameref{sec:Basic Facilities of a Virtio Device / Device Configuration Space / Legacy Interface: Device Configuration Space} for workarounds.
> -
> -When using the legacy interface, the transitional driver MUST
> -use the legacy configuration structure in BAR0 in the first
> -I/O region of the PCI device.
> -
> -When using the legacy interface, the driver MAY access
> -the device-specific configuration structure using any width
> -accesses and the transitional device MUST present the driver with
> -the same results as when accessed using the ``natural'' access
> -method (i.e. 32-bit accesses for 32-bit fields, etc).
> -
> -Note that this is possible because while the legacy common
> -configuration structure is PCI (i.e. little) endian, when using
> -the legacy interface the device-specific configuration structure
> -is encoded in the native endian of the guest (where such
> -distinction is applicable).
> -
> -The transitional driver when using the legacy interface MUST
> -the device-specific configuration structure at an offset
> -immediately following the legacy common configuration structure.
> -
>  \subsubsection{Legacy Interface: A Note on feature
>  bits}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
>  Virtio Structure PCI Capabilities / Legacy Interface: A Note on feature bits}
> -- 
> 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

Re: [PATCH 0/3] Cleanup for PCI transitional common cfg

[Michael S. Tsirkin]

On Sun, Feb 26, 2023 at 12:29:58AM +0200, Parav Pandit wrote:
> Legacy interface PCI Device layout description has following issues.
> 
> 1. repeated 'structure' word
> 2. virtio header was defined the 0.9.5 spec. It is referred with
>    different keywards in this section with multiple different words
>    as (a) virtio header, (b) general headers, (c) legacy configuration
>    structure, (d) virtio common configuration structure and
>    (e) other fields.
> 3. Driver and device requirements listing is intermixed.
> 4. spelling error of structure
> 5. Legacy interface common configuration requirements are not adjacent
>    to 1.x comm
> 
> Hence, this short series overcomes above issues.

Looking at the patchset so far I'm inclined to say - leave
legacy well alone. This is not an improvement.

Gratituis changes for trivial benefit have a cost - people have to
re-read spec this to see what changed.  Making things significantly
easier for new readers would make it worth it.  As it stands - this is
not worth it.


> Patch summary:
> patch-1 overcomes above 1 to 4 issues
> patch-2 splits feature bit operations from config layout
> patch-3 relocate requirements adjacent to 1.x requirements
> 
> This series is in top of [1], [2] and [3].
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> Please review.
> 
> [1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00578.html
> [2] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00585.html
> [3] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00597.html
> 
> Parav Pandit (3):
>   transport-pci: Improve PCI legacy device layout description
>   transport-pci: Split notes of PCI Device Layout
>   transport-pci: Relocate common config legacy interface
> 
>  conformance.tex   |   3 +-
>  transport-pci.tex | 182 +++++++++++++++++++++++++---------------------
>  2 files changed, 102 insertions(+), 83 deletions(-)
> 
> -- 
> 2.26.2

[virtio-dev] Re: [PATCH 0/3] Cleanup for PCI transitional common cfg

[Michael S. Tsirkin]

On Sun, Feb 26, 2023 at 12:29:58AM +0200, Parav Pandit wrote:
> Legacy interface PCI Device layout description has following issues.
> 
> 1. repeated 'structure' word
> 2. virtio header was defined the 0.9.5 spec. It is referred with
>    different keywards in this section with multiple different words
>    as (a) virtio header, (b) general headers, (c) legacy configuration
>    structure, (d) virtio common configuration structure and
>    (e) other fields.
> 3. Driver and device requirements listing is intermixed.
> 4. spelling error of structure
> 5. Legacy interface common configuration requirements are not adjacent
>    to 1.x comm
> 
> Hence, this short series overcomes above issues.

Looking at the patchset so far I'm inclined to say - leave
legacy well alone. This is not an improvement.

Gratituis changes for trivial benefit have a cost - people have to
re-read spec this to see what changed.  Making things significantly
easier for new readers would make it worth it.  As it stands - this is
not worth it.


> Patch summary:
> patch-1 overcomes above 1 to 4 issues
> patch-2 splits feature bit operations from config layout
> patch-3 relocate requirements adjacent to 1.x requirements
> 
> This series is in top of [1], [2] and [3].
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> Please review.
> 
> [1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00578.html
> [2] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00585.html
> [3] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00597.html
> 
> Parav Pandit (3):
>   transport-pci: Improve PCI legacy device layout description
>   transport-pci: Split notes of PCI Device Layout
>   transport-pci: Relocate common config legacy interface
> 
>  conformance.tex   |   3 +-
>  transport-pci.tex | 182 +++++++++++++++++++++++++---------------------
>  2 files changed, 102 insertions(+), 83 deletions(-)
> 
> -- 
> 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

RE: [PATCH 1/3] transport-pci: Improve PCI legacy device layout description

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Saturday, February 25, 2023 6:09 PM
> On Sun, Feb 26, 2023 at 12:29:59AM +0200, Parav Pandit wrote:
> > Legacy interface PCI Device layout description has following issues.
> >
> > 1. repeated 'structure' word
> > 2. virtio header was defined the 0.9.5 spec. In a legacy interface
> >    section it is referred with different keywards
> >    as (a) virtio header, (b) general headers, (c) legacy configuration
> >    structure, (d) virtio common configuration structure and
> >    (e) other fields.
> > 3. Driver and device requirements listing is intermixed.
> > 4. spelling error of structure
> >
> > Hence, rewrite the description to eliminate above issues as below.
> >
> > 1. Removed repeated structure word
> > 2. Fix spelling of structure
> > 3. Place all device requirements toegether 3. Define legacy
> > configuration structure that consist of
> >    a. legacy common configuration structure and
> >    b. device specific configuration structure 4. Rewrite section
> > around above changes
> >
> > This is only an editorial change.
> 
> No, editorial changes are things like spelling corrections.
> 
Got it. Will drop this remark.

> I have trouble reviewing
> because I have no idea why you are making each change.
> 
After documenting legacy interface in the spec, we cannot claim that driver is the source = specification.
Currently transitional device and pci device do not scale well (or doesn't scale at all).
We are working on supporting it and having better defined transitional device seems important part of it.

> E.g. you just rewrote a bunch of text and I frankly don't know why. For example:
> 
> 	-When using the legacy interface, transitional drivers
> 	-MUST use the legacy configuration structure in BAR0 in the first
> 	-I/O region of the PCI device, as documented below.
> 
> is now apparently:
> 
> 	+When used through the legacy interface, the legacy common
> 	+configuration structure has the following layout:
> 
> and this is better? why? we are replacing a clear requirement which applied to

Because as I explained in the commit log, that one structure is named using 5 different names.

> drivers with a vague statement which I can't say what it applies to.
> 
> Any chance of splitting these things up? Maybe that will help.
> 
That's a good idea. Yes. I will split these patches to smaller one.

> Apropos I don't know that we want to spend much time on legacy sections.
Transitional devices are very much in use and documenting them well is important to build scalable transitional devices.

> Really with legacy code is the main source of documentation - if drivers work
> then you are golden. If they don't appealing to spec will not help.

Yes, so don't want to add additional text. Just correcting the current one.

[virtio-dev] RE: [PATCH 1/3] transport-pci: Improve PCI legacy device layout description

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Saturday, February 25, 2023 6:09 PM
> On Sun, Feb 26, 2023 at 12:29:59AM +0200, Parav Pandit wrote:
> > Legacy interface PCI Device layout description has following issues.
> >
> > 1. repeated 'structure' word
> > 2. virtio header was defined the 0.9.5 spec. In a legacy interface
> >    section it is referred with different keywards
> >    as (a) virtio header, (b) general headers, (c) legacy configuration
> >    structure, (d) virtio common configuration structure and
> >    (e) other fields.
> > 3. Driver and device requirements listing is intermixed.
> > 4. spelling error of structure
> >
> > Hence, rewrite the description to eliminate above issues as below.
> >
> > 1. Removed repeated structure word
> > 2. Fix spelling of structure
> > 3. Place all device requirements toegether 3. Define legacy
> > configuration structure that consist of
> >    a. legacy common configuration structure and
> >    b. device specific configuration structure 4. Rewrite section
> > around above changes
> >
> > This is only an editorial change.
> 
> No, editorial changes are things like spelling corrections.
> 
Got it. Will drop this remark.

> I have trouble reviewing
> because I have no idea why you are making each change.
> 
After documenting legacy interface in the spec, we cannot claim that driver is the source = specification.
Currently transitional device and pci device do not scale well (or doesn't scale at all).
We are working on supporting it and having better defined transitional device seems important part of it.

> E.g. you just rewrote a bunch of text and I frankly don't know why. For example:
> 
> 	-When using the legacy interface, transitional drivers
> 	-MUST use the legacy configuration structure in BAR0 in the first
> 	-I/O region of the PCI device, as documented below.
> 
> is now apparently:
> 
> 	+When used through the legacy interface, the legacy common
> 	+configuration structure has the following layout:
> 
> and this is better? why? we are replacing a clear requirement which applied to

Because as I explained in the commit log, that one structure is named using 5 different names.

> drivers with a vague statement which I can't say what it applies to.
> 
> Any chance of splitting these things up? Maybe that will help.
> 
That's a good idea. Yes. I will split these patches to smaller one.

> Apropos I don't know that we want to spend much time on legacy sections.
Transitional devices are very much in use and documenting them well is important to build scalable transitional devices.

> Really with legacy code is the main source of documentation - if drivers work
> then you are golden. If they don't appealing to spec will not help.

Yes, so don't want to add additional text. Just correcting the current one.

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

RE: [PATCH 2/3] transport-pci: Split notes of PCI Device Layout

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Saturday, February 25, 2023 6:15 PM
> 
> On Sun, Feb 26, 2023 at 12:30:00AM +0200, Parav Pandit wrote:
> > Currently single legacy interface section describes PCI common
> > configuration layout and feature bits operation for the legacy
> > interface.
> > Secondly common configuration structure description of legacy
> > interface is not adjacent to the the respective normal device
> > requirements for same.
> >
> > Hence, split PCI Device Layout legacy interface section into two
> > parts. First subsection for common configuration and second subsection
> > for feature bits.
> >
> > Subsequent patch relocates common configuration legacy interface to
> > appropriate matching location.
> >
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> 
> I don't care really. However this does much more than move text around as it
> claim to do:
> 
> 
> > ---
> >  conformance.tex   |  4 +++-
> >  transport-pci.tex | 19 ++++++++++++-------
> >  2 files changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/conformance.tex b/conformance.tex index 01ccd69..0d3616f
> > 100644
> > --- a/conformance.tex
> > +++ b/conformance.tex
> > @@ -262,7 +262,9 @@ \section{Conformance
> > Targets}\label{sec:Conformance / Conformance Targets}  \item Section
> > \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message
> > Framing / Legacy Interface: Message Framing}  \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}
> > +\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI
> > +Bus/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on
> > +Common configuration Layout} \item Section \ref{sec:Virtio Transport
> > +Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities /
> > +Legacy Interface: A Note on feature bits}
> >  \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{sec:Virtio Transport Options /
> > Virtio Over MMIO / Legacy interface} diff --git a/transport-pci.tex
> > b/transport-pci.tex index 9ee37ba..9d4c713 100644
> > --- a/transport-pci.tex
> > +++ b/transport-pci.tex
> > @@ -767,7 +767,10 @@ \subsubsection{PCI configuration access
> > capability}\label{sec:Virtio Transport O  specified by some other
> > Virtio Structure PCI Capability  of type other than
> \field{VIRTIO_PCI_CAP_PCI_CFG}.
> >
> > -\subsubsection{Legacy Interfaces: A Note on PCI Device
> > Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI
> > Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
> > +\subsubsection{Legacy Interfaces: A Note on Common configuration
> > +Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
> > +Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on
> > +Common configuration Layout}
> >
> >  The transitional device MUST present part of the configuration
> > registers in a legacy configuration structure in BAR0 in the
> 
> 
> Please do not split up labels to multiple lines like this, it is hard to find and fix
> them if you do.
> 
Will fix it.

> > @@ -852,13 +855,15 @@ \subsubsection{Legacy Interfaces: A Note on PCI
> > Device Layout}\label{sec:Virtio  is encoded in the native endian of
> > the guest (where such  distinction is applicable).
> >
> > -When accessing the device-specific configuration structure -using the
> > legacy interface, transitional drivers MUST access -the
> > device-specific configuration structure -at an offset immediately
> > following the legacy common -configuration structure.
> > +The transitional driver when using the legacy interface MUST the
> > +device-specific configuration structure at an offset immediately
> > +following the legacy common configuration structure.
> 
> Oh great and in the process of presumably just moving stuff around you are
> also losing text - the result is agrammatical, and much less clear than the
> original.
> 
I missed "access".
While splitting the patch, a hunk got in the wrong patch.
Will fix it.
Thank for the catch.

> 
> >
> > -Note that only Feature Bits 0 to 31 are accessible through the
> > +\subsubsection{Legacy Interface: A Note on feature
> > +bits}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
> > +Virtio Structure PCI Capabilities / Legacy Interface: A Note on
> > +feature bits}
> > +
> > +Only Feature Bits 0 to 31 are accessible through the
> >  Legacy Interface. When used through the Legacy Interface,  the
> > transitional device MUST assume that Feature Bits 32 to 63  are not
> > acknowledged by the driver.
> 
> And you are dropping "Note" here because why? Seems notable in that there
> are more feature bits, someone might miss this fact.
The heading is "A Note ..."
Hence, I dropped the Note from the sentence.
But if you really care, can add it back...

[virtio-dev] RE: [PATCH 2/3] transport-pci: Split notes of PCI Device Layout

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Saturday, February 25, 2023 6:15 PM
> 
> On Sun, Feb 26, 2023 at 12:30:00AM +0200, Parav Pandit wrote:
> > Currently single legacy interface section describes PCI common
> > configuration layout and feature bits operation for the legacy
> > interface.
> > Secondly common configuration structure description of legacy
> > interface is not adjacent to the the respective normal device
> > requirements for same.
> >
> > Hence, split PCI Device Layout legacy interface section into two
> > parts. First subsection for common configuration and second subsection
> > for feature bits.
> >
> > Subsequent patch relocates common configuration legacy interface to
> > appropriate matching location.
> >
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> 
> I don't care really. However this does much more than move text around as it
> claim to do:
> 
> 
> > ---
> >  conformance.tex   |  4 +++-
> >  transport-pci.tex | 19 ++++++++++++-------
> >  2 files changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/conformance.tex b/conformance.tex index 01ccd69..0d3616f
> > 100644
> > --- a/conformance.tex
> > +++ b/conformance.tex
> > @@ -262,7 +262,9 @@ \section{Conformance
> > Targets}\label{sec:Conformance / Conformance Targets}  \item Section
> > \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Message
> > Framing / Legacy Interface: Message Framing}  \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}
> > +\item Section \ref{sec:Virtio Transport Options / Virtio Over PCI
> > +Bus/ Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on
> > +Common configuration Layout} \item Section \ref{sec:Virtio Transport
> > +Options / Virtio Over PCI Bus / Virtio Structure PCI Capabilities /
> > +Legacy Interface: A Note on feature bits}
> >  \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{sec:Virtio Transport Options /
> > Virtio Over MMIO / Legacy interface} diff --git a/transport-pci.tex
> > b/transport-pci.tex index 9ee37ba..9d4c713 100644
> > --- a/transport-pci.tex
> > +++ b/transport-pci.tex
> > @@ -767,7 +767,10 @@ \subsubsection{PCI configuration access
> > capability}\label{sec:Virtio Transport O  specified by some other
> > Virtio Structure PCI Capability  of type other than
> \field{VIRTIO_PCI_CAP_PCI_CFG}.
> >
> > -\subsubsection{Legacy Interfaces: A Note on PCI Device
> > Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI
> > Device Layout / Legacy Interfaces: A Note on PCI Device Layout}
> > +\subsubsection{Legacy Interfaces: A Note on Common configuration
> > +Layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
> > +Virtio Structure PCI Capabilities / Legacy Interfaces: A Note on
> > +Common configuration Layout}
> >
> >  The transitional device MUST present part of the configuration
> > registers in a legacy configuration structure in BAR0 in the
> 
> 
> Please do not split up labels to multiple lines like this, it is hard to find and fix
> them if you do.
> 
Will fix it.

> > @@ -852,13 +855,15 @@ \subsubsection{Legacy Interfaces: A Note on PCI
> > Device Layout}\label{sec:Virtio  is encoded in the native endian of
> > the guest (where such  distinction is applicable).
> >
> > -When accessing the device-specific configuration structure -using the
> > legacy interface, transitional drivers MUST access -the
> > device-specific configuration structure -at an offset immediately
> > following the legacy common -configuration structure.
> > +The transitional driver when using the legacy interface MUST the
> > +device-specific configuration structure at an offset immediately
> > +following the legacy common configuration structure.
> 
> Oh great and in the process of presumably just moving stuff around you are
> also losing text - the result is agrammatical, and much less clear than the
> original.
> 
I missed "access".
While splitting the patch, a hunk got in the wrong patch.
Will fix it.
Thank for the catch.

> 
> >
> > -Note that only Feature Bits 0 to 31 are accessible through the
> > +\subsubsection{Legacy Interface: A Note on feature
> > +bits}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
> > +Virtio Structure PCI Capabilities / Legacy Interface: A Note on
> > +feature bits}
> > +
> > +Only Feature Bits 0 to 31 are accessible through the
> >  Legacy Interface. When used through the Legacy Interface,  the
> > transitional device MUST assume that Feature Bits 32 to 63  are not
> > acknowledged by the driver.
> 
> And you are dropping "Note" here because why? Seems notable in that there
> are more feature bits, someone might miss this fact.
The heading is "A Note ..."
Hence, I dropped the Note from the sentence.
But if you really care, can add it back...

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

RE: [PATCH 3/3] transport-pci: Relocate common config legacy interface

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Saturday, February 25, 2023 6:16 PM
> 
> On Sun, Feb 26, 2023 at 12:30:01AM +0200, Parav Pandit wrote:
> > Relocate legacy interface section of common configuration structure
> > near where 1.x based common configuration structure is defined.
> >
> > This aligns the spec to follow rest of the other Legacy interfaces
> > section.
> >
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > ---
> >  conformance.tex   |   3 +-
> >  transport-pci.tex | 186
> > +++++++++++++++++++++++-----------------------
> >  2 files changed, 95 insertions(+), 94 deletions(-)
> >
> 
> I can't be bothered to check whether this actually just moves text around or
> also changes a bunch of stuff on the way.
> 
I will generate pdf after these each patch in this series to make sure that no text is changed.

[virtio-dev] RE: [PATCH 3/3] transport-pci: Relocate common config legacy interface

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Saturday, February 25, 2023 6:16 PM
> 
> On Sun, Feb 26, 2023 at 12:30:01AM +0200, Parav Pandit wrote:
> > Relocate legacy interface section of common configuration structure
> > near where 1.x based common configuration structure is defined.
> >
> > This aligns the spec to follow rest of the other Legacy interfaces
> > section.
> >
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/164
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > ---
> >  conformance.tex   |   3 +-
> >  transport-pci.tex | 186
> > +++++++++++++++++++++++-----------------------
> >  2 files changed, 95 insertions(+), 94 deletions(-)
> >
> 
> I can't be bothered to check whether this actually just moves text around or
> also changes a bunch of stuff on the way.
> 
I will generate pdf after these each patch in this series to make sure that no text is changed.

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

Re: [PATCH 1/3] transport-pci: Improve PCI legacy device layout description

[Michael S. Tsirkin]

On Mon, Feb 27, 2023 at 03:02:10AM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Saturday, February 25, 2023 6:09 PM
> > On Sun, Feb 26, 2023 at 12:29:59AM +0200, Parav Pandit wrote:
> > > Legacy interface PCI Device layout description has following issues.
> > >
> > > 1. repeated 'structure' word
> > > 2. virtio header was defined the 0.9.5 spec. In a legacy interface
> > >    section it is referred with different keywards
> > >    as (a) virtio header, (b) general headers, (c) legacy configuration
> > >    structure, (d) virtio common configuration structure and
> > >    (e) other fields.
> > > 3. Driver and device requirements listing is intermixed.
> > > 4. spelling error of structure
> > >
> > > Hence, rewrite the description to eliminate above issues as below.
> > >
> > > 1. Removed repeated structure word
> > > 2. Fix spelling of structure
> > > 3. Place all device requirements toegether 3. Define legacy
> > > configuration structure that consist of
> > >    a. legacy common configuration structure and
> > >    b. device specific configuration structure 4. Rewrite section
> > > around above changes
> > >
> > > This is only an editorial change.
> > 
> > No, editorial changes are things like spelling corrections.
> > 
> Got it. Will drop this remark.
> 
> > I have trouble reviewing
> > because I have no idea why you are making each change.
> > 
> After documenting legacy interface in the spec, we cannot claim that driver is the source = specification.

We document the *legacy interface*. What we don't is we don't document
legacy devices and drivers:

    Legacy devices and legacy drivers are not compliant with this
    specification.



> Currently transitional device and pci device do not scale well (or doesn't scale at all).
> We are working on supporting it and having better defined transitional device seems important part of it.
> 
> > E.g. you just rewrote a bunch of text and I frankly don't know why. For example:
> > 
> > 	-When using the legacy interface, transitional drivers
> > 	-MUST use the legacy configuration structure in BAR0 in the first
> > 	-I/O region of the PCI device, as documented below.
> > 
> > is now apparently:
> > 
> > 	+When used through the legacy interface, the legacy common
> > 	+configuration structure has the following layout:
> > 
> > and this is better? why? we are replacing a clear requirement which applied to
> 
> Because as I explained in the commit log, that one structure is named using 5 different names.

So maybe try just renaming. What you did is also convert a normative
statement to a non-normative one.


> > drivers with a vague statement which I can't say what it applies to.
> > 
> > Any chance of splitting these things up? Maybe that will help.
> > 
> That's a good idea. Yes. I will split these patches to smaller one.
> 
> > Apropos I don't know that we want to spend much time on legacy sections.
> Transitional devices are very much in use and documenting them well is important to build scalable transitional devices.
> 
> > Really with legacy code is the main source of documentation - if drivers work
> > then you are golden. If they don't appealing to spec will not help.
> 
> Yes, so don't want to add additional text. Just correcting the current one.

[virtio-dev] Re: [PATCH 1/3] transport-pci: Improve PCI legacy device layout description

[Michael S. Tsirkin]

On Mon, Feb 27, 2023 at 03:02:10AM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Saturday, February 25, 2023 6:09 PM
> > On Sun, Feb 26, 2023 at 12:29:59AM +0200, Parav Pandit wrote:
> > > Legacy interface PCI Device layout description has following issues.
> > >
> > > 1. repeated 'structure' word
> > > 2. virtio header was defined the 0.9.5 spec. In a legacy interface
> > >    section it is referred with different keywards
> > >    as (a) virtio header, (b) general headers, (c) legacy configuration
> > >    structure, (d) virtio common configuration structure and
> > >    (e) other fields.
> > > 3. Driver and device requirements listing is intermixed.
> > > 4. spelling error of structure
> > >
> > > Hence, rewrite the description to eliminate above issues as below.
> > >
> > > 1. Removed repeated structure word
> > > 2. Fix spelling of structure
> > > 3. Place all device requirements toegether 3. Define legacy
> > > configuration structure that consist of
> > >    a. legacy common configuration structure and
> > >    b. device specific configuration structure 4. Rewrite section
> > > around above changes
> > >
> > > This is only an editorial change.
> > 
> > No, editorial changes are things like spelling corrections.
> > 
> Got it. Will drop this remark.
> 
> > I have trouble reviewing
> > because I have no idea why you are making each change.
> > 
> After documenting legacy interface in the spec, we cannot claim that driver is the source = specification.

We document the *legacy interface*. What we don't is we don't document
legacy devices and drivers:

    Legacy devices and legacy drivers are not compliant with this
    specification.



> Currently transitional device and pci device do not scale well (or doesn't scale at all).
> We are working on supporting it and having better defined transitional device seems important part of it.
> 
> > E.g. you just rewrote a bunch of text and I frankly don't know why. For example:
> > 
> > 	-When using the legacy interface, transitional drivers
> > 	-MUST use the legacy configuration structure in BAR0 in the first
> > 	-I/O region of the PCI device, as documented below.
> > 
> > is now apparently:
> > 
> > 	+When used through the legacy interface, the legacy common
> > 	+configuration structure has the following layout:
> > 
> > and this is better? why? we are replacing a clear requirement which applied to
> 
> Because as I explained in the commit log, that one structure is named using 5 different names.

So maybe try just renaming. What you did is also convert a normative
statement to a non-normative one.


> > drivers with a vague statement which I can't say what it applies to.
> > 
> > Any chance of splitting these things up? Maybe that will help.
> > 
> That's a good idea. Yes. I will split these patches to smaller one.
> 
> > Apropos I don't know that we want to spend much time on legacy sections.
> Transitional devices are very much in use and documenting them well is important to build scalable transitional devices.
> 
> > Really with legacy code is the main source of documentation - if drivers work
> > then you are golden. If they don't appealing to spec will not help.
> 
> Yes, so don't want to add additional text. Just correcting the current one.


---------------------------------------------------------------------
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 2/3] transport-pci: Split notes of PCI Device Layout

[Michael S. Tsirkin]

On Mon, Feb 27, 2023 at 03:05:46AM +0000, Parav Pandit wrote:
> > >
> > > -Note that only Feature Bits 0 to 31 are accessible through the
> > > +\subsubsection{Legacy Interface: A Note on feature
> > > +bits}\label{sec:Virtio Transport Options / Virtio Over PCI Bus /
> > > +Virtio Structure PCI Capabilities / Legacy Interface: A Note on
> > > +feature bits}
> > > +
> > > +Only Feature Bits 0 to 31 are accessible through the
> > >  Legacy Interface. When used through the Legacy Interface,  the
> > > transitional device MUST assume that Feature Bits 32 to 63  are not
> > > acknowledged by the driver.
> > 
> > And you are dropping "Note" here because why? Seems notable in that there
> > are more feature bits, someone might miss this fact.
> The heading is "A Note ..."
> Hence, I dropped the Note from the sentence.
> But if you really care, can add it back...

That's a note as a noun versus note as a verb. Not the same thing.

-- 
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 0/3] Cleanup for PCI transitional common cfg

[Cornelia Huck]

On Sat, Feb 25 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Sun, Feb 26, 2023 at 12:29:58AM +0200, Parav Pandit wrote:
>> Legacy interface PCI Device layout description has following issues.
>> 
>> 1. repeated 'structure' word
>> 2. virtio header was defined the 0.9.5 spec. It is referred with
>>    different keywards in this section with multiple different words
>>    as (a) virtio header, (b) general headers, (c) legacy configuration
>>    structure, (d) virtio common configuration structure and
>>    (e) other fields.
>> 3. Driver and device requirements listing is intermixed.
>> 4. spelling error of structure
>> 5. Legacy interface common configuration requirements are not adjacent
>>    to 1.x comm
>> 
>> Hence, this short series overcomes above issues.
>
> Looking at the patchset so far I'm inclined to say - leave
> legacy well alone. This is not an improvement.
>
> Gratituis changes for trivial benefit have a cost - people have to
> re-read spec this to see what changed.  Making things significantly
> easier for new readers would make it worth it.  As it stands - this is
> not worth it.

I think I agree.


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