virtio: Improve queue_reset polarity to match to default reset state

Currently when driver initiates a queue reset, device is expected to communicate reset status to the driver by changing the value of the queue_reset register twice. First to return value other than 1 when reset is ongoing, later to return 1 when queue reset is completed.

However initially during the device reset time the queue reset value is zero. queue_reset changes the value of the register to a different value on reset completion. Yet another time queue_reset value is expected to change when queue_select is reprogrammed.

For example in below flow, a created virtqueue, which is disabled by driver leaves the queue state as
queue_enable = 0, queue_reset = 1.

example flow:
a) 0,0 -> device init time value
b) 1,0 -> vq is enabled by driver and working
c) 1,1 -> vq is enabled, driver initiated reset
d) 0,0 -> queue reset is ongoing
conflicts with #a, because queue is still enabled in device whose reset is in progress.
External entity has no knowledge if VQ is undergoing reset or VQ is never enabled.
e) 0,1 -> queue reset is completed (conflicts with #a initial value)

On next iteration, when queue_select selects the same VQ again, without enablement, device is confused to return 1 or 0 because it was reset once before via queue_reset register.

This demands complex device implementation to understand what should be returned for a VQ that is reset using queue_reset register vs other means.

Additionally, external entity that may have to manage a virtio device cannot distinguish between #a and #d, whether a VQ is not yet enabled or it is in use and undergoing a reset.

Instead, it is better and efficient to maintain the same VQ state on the device when queue reset is completed.

new proposed flow:
q_enable, q_reset
A) 0, 0 -> default, device init time
B) 1, 0 -> driver has enabled vq
C) 1, 1 -> driver started q reset
D) 1, 1 -> q_reset stays 1 until device is busy resetting vq (device communicates that its working on resetting VQ, consistent with #C)
E) 0, 0 -> q_reset by device is completed, q got disabled (consistent with device init time #A)

Hence, this patch proposes a simple change to have reset register polarity to be same as that of initial reset value.

Fixes https://github.com/oasis-tcs/virtio-spec/issues/139

Fixes 12998e738621 ("virtio: pci support virtqueue reset")

Fixes a4ce81a83780 ("virtio: mmio support virtqueue reset")

Fixes 3b5378d70a42 ("virtio: introduce virtqueue reset as basic facility")

Signed-off-by Parav Pandit <[email protected]>

---
changelog:
v3->v4:
- removed incorrect requirement to read queue_enable on resetting the queue
v2->v3:
- Updated the commit log to capture the problem more accurately
- Fixed below comments from Michael and Cornelia
- Fixed adding article 'the' before driver
- Change wording in driver requirements section to reflect driver requirements reflecting device behavior
- Added device compliance statement on what to return for queue_reset register to indicate ongoing reset in progress
- Moved device specific description adjacent to other device related queue reset description
v1->v2:
- Fixed below comment from Xuan Zhuo
- Fixed the configuration and register layout text to reflect the polarity v0->v1:
- Fixed below comments from Michael
- Removed example device side pseudo code for old and new polarity
- Removed unwanted articles 'a' and 'the'
- Dropped extra empty line

---
  content.tex | 48 ++++++++++++++++++++----------------------------
  1 file changed, 20 insertions(+), 28 deletions(-)

content.tex +20 -28

index 060bdab..fdd255d 100644
@@ -1029,19 +1029,15 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
  The device MUST present a 0 in \field{queue_enable} on reset.
 
  If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
-\field{queue_enable} after the driver has reset the virtqueue via
-\field{queue_reset}.
-
-If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
  \field{queue_reset} on reset.
 
  If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
  \field{queue_reset} after the virtqueue is enabled with \field{queue_enable}.
 
-The device MUST reset the queue when 1 is written to \field{queue_reset}, and
-present a 1 in \field{queue_reset} after the queue has been reset, until the
-driver re-enables the queue via \field{queue_enable} or the device is reset.
-The device MUST present consistent default values after queue reset.
+The device MUST reset the queue when 1 is written to \field{queue_reset}. The
+device MUST continue to present 1 in \field{queue_reset} when the queue reset
+is ongoing. The device MUST present 0 in \field{queue_reset} and \field{queue_enable}
+when queue reset completes.
  (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
  The device MUST present a 0 in \field{queue_size} if the virtqueue

@@ -1069,12 +1065,12 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
  The driver MUST NOT write a 0 to \field{queue_enable}.
 
  If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to
-\field{queue_reset} to reset the queue, it MUST verify that the queue
-has been reset by reading back \field{queue_reset} and ensuring that it
-is 1. The driver MAY re-enable the queue by writing a 1 to
-\field{queue_enable} after ensuring that the other virtqueue fields have
-been set up correctly. The driver MAY set driver-writeable queue configuration
-values to different values than those that were used before the queue reset.
+\field{queue_reset} to reset the queue, the driver MUST NOT consider queue
+reset to be complete until it reads back 0 in \field{queue_reset}. The driver
+MAY re-enable the queue by writing 1 to \field{queue_enable} after ensuring
+that other virtqueue fields have been set up correctly. The driver MAY set
+driver-writeable queue configuration values to different values than those that
+were used before the queue reset.
  (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
  \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}

@@ -2005,19 +2001,15 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
  The device MUST NOT access virtual queue contents when \field{QueueReady} is zero (0x0).
 
  If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
-\field{QueueReady} after the driver has reset the virtqueue via
-\field{QueueReset}.
-
-If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
  \field{QueueReset} on reset.
 
  If VIRTIO_F_RING_RESET has been negotiated, The device MUST present a 0 in
  \field{QueueReset} after the virtqueue is enabled with \field{QueueReady}.
 
-The device MUST reset the queue when 1 is written to \field{QueueReset}, and
-present a 1 in \field{QueueReset} after the queue has been reset, until the
-driver re-enables the queue via \field{QueueReady} or until the device is reset.
-The device MUST present consistent default values after queue reset.
+The device MUST reset the queue when 1 is written to \field{QueueReset}. The
+device MUST continue to present 1 in \field{QueueReset} when the queue reset
+is ongoing. The device MUST present 0 in \field{QueueReset} and \field{QueueReady}
+when queue reset completes.
  (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
  \drivernormative{\subsubsection}{MMIO Device Register Layout}{Virtio Transport Options / Virtio Over MMIO / MMIO Device Register Layout}

@@ -2063,12 +2055,12 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
  it finishes handling an interrupt and MUST NOT set any of the undefined bits in the value.
 
  If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to
-\field{QueueReset} to reset the queue, it MUST verify that the queue
-has been reset by reading back \field{QueueReset} and ensuring that it
-is 1. The driver MAY re-enable the queue by writing a 1 to
-\field{QueueReady} after ensuring that the other virtqueue fields have
-been set up correctly. The driver MAY set driver-writeable queue configuration
-values to different values than those that were used before the queue reset.
+\field{QueueReset} to reset the queue, the driver MUST NOT consider queue
+reset to be complete until it reads back 0 in \field{QueueReset}. The driver
+MAY re-enable the queue by writing 1 to \field{QueueReady} after ensuring
+that other virtqueue fields have been set up correctly. The driver MAY set
+driver-writeable queue configuration values to different values than those
+that were used before the queue reset.
  (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
  \subsection{MMIO-specific Initialization And Device Operation}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation}

Currently when driver initiates a queue reset, device is expected to communicate reset status to the driver by changing the value of the queue_reset register twice. First to return value other than 1 when reset is ongoing, later to return 1 when queue reset is completed.

However initially during the device reset time the queue reset value is zero. queue_reset changes the value of the register to a different value on reset completion. Yet another time queue_reset value is expected to change when queue_select is reprogrammed.

For example in below flow, a created virtqueue, which is disabled by driver leaves the queue state as
queue_enable = 0, queue_reset = 1.

example flow:
a) 0,0 -> device init time value
b) 1,0 -> vq is enabled by driver and working
c) 1,1 -> vq is enabled, driver initiated reset
d) 0,0 -> queue reset is ongoing
conflicts with #a, because queue is still enabled in device whose reset is in progress.
External entity has no knowledge if VQ is undergoing reset or VQ is never enabled.
e) 0,1 -> queue reset is completed (conflicts with #a initial value)

On next iteration, when queue_select selects the same VQ again, without enablement, device is confused to return 1 or 0 because it was reset once before via queue_reset register.

This demands complex device implementation to understand what should be returned for a VQ that is reset using queue_reset register vs other means.

Additionally, external entity that may have to manage a virtio device cannot distinguish between #a and #d, whether a VQ is not yet enabled or it is in use and undergoing a reset.

Instead, it is better and efficient to maintain the same VQ state on the device when queue reset is completed.

new proposed flow:
q_enable, q_reset
A) 0, 0 -> default, device init time
B) 1, 0 -> driver has enabled vq
C) 1, 1 -> driver started q reset
D) 1, 1 -> q_reset stays 1 until device is busy resetting vq (device communicates that its working on resetting VQ, consistent with #C)
E) 0, 0 -> q_reset by device is completed, q got disabled (consistent with device init time #A)

Hence, this patch proposes a simple change to have reset register polarity to be same as that of initial reset value.

Fixes https://github.com/oasis-tcs/virtio-spec/issues/139

Fixes 12998e738621 ("virtio: pci support virtqueue reset")

Fixes a4ce81a83780 ("virtio: mmio support virtqueue reset")

Fixes 3b5378d70a42 ("virtio: introduce virtqueue reset as basic facility")

Signed-off-by Parav Pandit <[email protected]>

---
changelog:
v3->v4:
- removed incorrect requirement to read queue_enable on resetting the queue
v2->v3:
- Updated the commit log to capture the problem more accurately
- Fixed below comments from Michael and Cornelia
- Fixed adding article 'the' before driver
- Change wording in driver requirements section to reflect driver requirements reflecting device behavior
- Added device compliance statement on what to return for queue_reset register to indicate ongoing reset in progress
- Moved device specific description adjacent to other device related queue reset description
v1->v2:
- Fixed below comment from Xuan Zhuo
- Fixed the configuration and register layout text to reflect the polarity v0->v1:
- Fixed below comments from Michael
- Removed example device side pseudo code for old and new polarity
- Removed unwanted articles 'a' and 'the'
- Dropped extra empty line

---
 content.tex | 48 ++++++++++++++++++++----------------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

content.tex +20 -29

index 060bdab..fdd255d 100644
@@ -1029,19 +1029,15 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 The device MUST present a 0 in \field{queue_enable} on reset.
 
 If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
-\field{queue_enable} after the driver has reset the virtqueue via
-\field{queue_reset}.
-
-If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
 \field{queue_reset} on reset.
 
 If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
 \field{queue_reset} after the virtqueue is enabled with \field{queue_enable}.
 
-The device MUST reset the queue when 1 is written to \field{queue_reset}, and
-present a 1 in \field{queue_reset} after the queue has been reset, until the
-driver re-enables the queue via \field{queue_enable} or the device is reset.
-The device MUST present consistent default values after queue reset.
+The device MUST reset the queue when 1 is written to \field{queue_reset}. The
+device MUST continue to present 1 in \field{queue_reset} when the queue reset
+is ongoing. The device MUST present 0 in \field{queue_reset} and \field{queue_enable}
+when queue reset completes.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 The device MUST present a 0 in \field{queue_size} if the virtqueue

@@ -1069,12 +1065,12 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 The driver MUST NOT write a 0 to \field{queue_enable}.
 
 If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to
-\field{queue_reset} to reset the queue, it MUST verify that the queue
-has been reset by reading back \field{queue_reset} and ensuring that it
-is 1. The driver MAY re-enable the queue by writing a 1 to
-\field{queue_enable} after ensuring that the other virtqueue fields have
-been set up correctly. The driver MAY set driver-writeable queue configuration
-values to different values than those that were used before the queue reset.
+\field{queue_reset} to reset the queue, the driver MUST NOT consider queue
+reset to be complete until it reads back 0 in \field{queue_reset}. The driver
+MAY re-enable the queue by writing 1 to \field{queue_enable} after ensuring
+that other virtqueue fields have been set up correctly. The driver MAY set
+driver-writeable queue configuration values to different values than those that
+were used before the queue reset.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}

@@ -2005,19 +2001,15 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 The device MUST NOT access virtual queue contents when \field{QueueReady} is zero (0x0).
 
 If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
-\field{QueueReady} after the driver has reset the virtqueue via
-\field{QueueReset}.
-
-If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
 \field{QueueReset} on reset.
 
 If VIRTIO_F_RING_RESET has been negotiated, The device MUST present a 0 in
 \field{QueueReset} after the virtqueue is enabled with \field{QueueReady}.
 
-The device MUST reset the queue when 1 is written to \field{QueueReset}, and
-present a 1 in \field{QueueReset} after the queue has been reset, until the
-driver re-enables the queue via \field{QueueReady} or until the device is reset.
-The device MUST present consistent default values after queue reset.
+The device MUST reset the queue when 1 is written to \field{QueueReset}. The
+device MUST continue to present 1 in \field{QueueReset} when the queue reset
+is ongoing. The device MUST present 0 in \field{QueueReset} and \field{QueueReady}
+when queue reset completes.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 \drivernormative{\subsubsection}{MMIO Device Register Layout}{Virtio Transport Options / Virtio Over MMIO / MMIO Device Register Layout}

@@ -2063,12 +2055,12 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 it finishes handling an interrupt and MUST NOT set any of the undefined bits in the value.
 
 If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to
-\field{QueueReset} to reset the queue, it MUST verify that the queue
-has been reset by reading back \field{QueueReset} and ensuring that it
-is 1. The driver MAY re-enable the queue by writing a 1 to
-\field{QueueReady} after ensuring that the other virtqueue fields have
-been set up correctly. The driver MAY set driver-writeable queue configuration
-values to different values than those that were used before the queue reset.
+\field{QueueReset} to reset the queue, the driver MUST NOT consider queue
+reset to be complete until it reads back 0 in \field{QueueReset}. The driver
+MAY re-enable the queue by writing 1 to \field{QueueReady} after ensuring
+that other virtqueue fields have been set up correctly. The driver MAY set
+driver-writeable queue configuration values to different values than those
+that were used before the queue reset.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 \subsection{MMIO-specific Initialization And Device Operation}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation}
--

1.8.3.1

Currently when driver initiates a queue reset, device is expected to communicate reset status to the driver by changing the value of the queue_reset register twice. First to return value other than 1 when reset is ongoing, later to return 1 when queue reset is completed.

However initially during the device reset time the queue reset value is zero. queue_reset changes the value of the register to a different value on reset completion. Yet another time queue_reset value is expected to change when queue_select is reprogrammed.

For example in below flow, a created virtqueue, which is disabled by driver leaves the queue state as
queue_enable = 0, queue_reset = 1.

example flow:
a) 0,0 -> device init time value
b) 1,0 -> vq is enabled by driver and working
c) 1,1 -> vq is enabled, driver initiated reset
d) 0,0 -> queue reset is ongoing
conflicts with #a, because queue is still enabled in device whose reset is in progress.
External entity has no knowledge if VQ is undergoing reset or VQ is never enabled.
e) 0,1 -> queue reset is completed (conflicts with #a initial value)

On next iteration, when queue_select selects the same VQ again, without enablement, device is confused to return 1 or 0 because it was reset once before via queue_reset register.

This demands complex device implementation to understand what should be returned for a VQ that is reset using queue_reset register vs other means.

Additionally, external entity that may have to manage a virtio device cannot distinguish between #a and #d, whether a VQ is not yet enabled or it is in use and undergoing a reset.

Instead, it is better and efficient to maintain the same VQ state on the device when queue reset is completed.

new proposed flow:
q_enable, q_reset
A) 0, 0 -> default, device init time
B) 1, 0 -> driver has enabled vq
C) 1, 1 -> driver started q reset
D) 1, 1 -> q_reset stays 1 until device is busy resetting vq (device communicates that its working on resetting VQ, consistent with #C)
E) 0, 0 -> q_reset by device is completed, q got disabled (consistent with device init time #A)

Hence, this patch proposes a simple change to have reset register polarity to be same as that of initial reset value.

Fixes https://github.com/oasis-tcs/virtio-spec/issues/139

Fixes 12998e738621 ("virtio: pci support virtqueue reset")

Fixes a4ce81a83780 ("virtio: mmio support virtqueue reset")

Fixes 3b5378d70a42 ("virtio: introduce virtqueue reset as basic facility")

Signed-off-by Parav Pandit <[email protected]>

---
changelog:
v3->v4:
- removed incorrect requirement to read queue_enable on resetting the queue
v2->v3:
- Updated the commit log to capture the problem more accurately
- Fixed below comments from Michael and Cornelia
- Fixed adding article 'the' before driver
- Change wording in driver requirements section to reflect driver requirements reflecting device behavior
- Added device compliance statement on what to return for queue_reset register to indicate ongoing reset in progress
- Moved device specific description adjacent to other device related queue reset description
v1->v2:
- Fixed below comment from Xuan Zhuo
- Fixed the configuration and register layout text to reflect the polarity v0->v1:
- Fixed below comments from Michael
- Removed example device side pseudo code for old and new polarity
- Removed unwanted articles 'a' and 'the'
- Dropped extra empty line

---
 content.tex | 48 ++++++++++++++++++++----------------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

content.tex +0 -4

index 060bdab..fdd255d 100644
@@ -1029,19 +1029,15 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 The device MUST present a 0 in \field{queue_enable} on reset.
 
 If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
-\field{queue_enable} after the driver has reset the virtqueue via
-\field{queue_reset}.
-
-If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
 \field{queue_reset} on reset.

content.tex +6 -10

 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 The device MUST present a 0 in \field{queue_size} if the virtqueue

@@ -1069,12 +1065,12 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 The driver MUST NOT write a 0 to \field{queue_enable}.
 
 If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to
-\field{queue_reset} to reset the queue, it MUST verify that the queue
-has been reset by reading back \field{queue_reset} and ensuring that it
-is 1. The driver MAY re-enable the queue by writing a 1 to
-\field{queue_enable} after ensuring that the other virtqueue fields have
-been set up correctly. The driver MAY set driver-writeable queue configuration
-values to different values than those that were used before the queue reset.
+\field{queue_reset} to reset the queue, the driver MUST NOT consider queue
+reset to be complete until it reads back 0 in \field{queue_reset}. The driver
+MAY re-enable the queue by writing 1 to \field{queue_enable} after ensuring
+that other virtqueue fields have been set up correctly. The driver MAY set
+driver-writeable queue configuration values to different values than those that
+were used before the queue reset.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}

@@ -2005,19 +2001,15 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 The device MUST NOT access virtual queue contents when \field{QueueReady} is zero (0x0).
 
 If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
-\field{QueueReady} after the driver has reset the virtqueue via
-\field{QueueReset}.
-
-If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
 \field{QueueReset} on reset.

content.tex +0 -5

 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 \drivernormative{\subsubsection}{MMIO Device Register Layout}{Virtio Transport Options / Virtio Over MMIO / MMIO Device Register Layout}

@@ -2063,12 +2055,12 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 it finishes handling an interrupt and MUST NOT set any of the undefined bits in the value.
 
 If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to
-\field{QueueReset} to reset the queue, it MUST verify that the queue
-has been reset by reading back \field{QueueReset} and ensuring that it
-is 1. The driver MAY re-enable the queue by writing a 1 to
-\field{QueueReady} after ensuring that the other virtqueue fields have
-been set up correctly. The driver MAY set driver-writeable queue configuration

Quoted 103 lines · Parav Pandit

Currently when driver initiates a queue reset, device is expected to communicate reset status to the driver by changing the value of the queue_reset register twice. First to return value other than 1 when reset is ongoing, later to return 1 when queue reset is completed.

However initially during the device reset time the queue reset value is zero. queue_reset changes the value of the register to a different value on reset completion. Yet another time queue_reset value is expected to change when queue_select is reprogrammed.

For example in below flow, a created virtqueue, which is disabled by driver leaves the queue state as
queue_enable = 0, queue_reset = 1.

example flow:
a) 0,0 -> device init time value
b) 1,0 -> vq is enabled by driver and working
c) 1,1 -> vq is enabled, driver initiated reset
d) 0,0 -> queue reset is ongoing
conflicts with #a, because queue is still enabled in device whose reset is in progress.
External entity has no knowledge if VQ is undergoing reset or VQ is never enabled.
e) 0,1 -> queue reset is completed (conflicts with #a initial value)

On next iteration, when queue_select selects the same VQ again, without enablement, device is confused to return 1 or 0 because it was reset once before via queue_reset register.

This demands complex device implementation to understand what should be returned for a VQ that is reset using queue_reset register vs other means.

Additionally, external entity that may have to manage a virtio device cannot distinguish between #a and #d, whether a VQ is not yet enabled or it is in use and undergoing a reset.

Instead, it is better and efficient to maintain the same VQ state on the device when queue reset is completed.

new proposed flow:
q_enable, q_reset
A) 0, 0 -> default, device init time
B) 1, 0 -> driver has enabled vq
C) 1, 1 -> driver started q reset
D) 1, 1 -> q_reset stays 1 until device is busy resetting vq (device communicates that its working on resetting VQ, consistent with #C)
E) 0, 0 -> q_reset by device is completed, q got disabled (consistent with device init time #A)

Hence, this patch proposes a simple change to have reset register polarity to be same as that of initial reset value.

Fixes https://github.com/oasis-tcs/virtio-spec/issues/139

Fixes 12998e738621 ("virtio: pci support virtqueue reset")

Fixes a4ce81a83780 ("virtio: mmio support virtqueue reset")

Fixes 3b5378d70a42 ("virtio: introduce virtqueue reset as basic facility")

Signed-off-by Parav Pandit <[email protected]>

---
changelog:
v3->v4:
- removed incorrect requirement to read queue_enable on resetting the queue
v2->v3:
- Updated the commit log to capture the problem more accurately
- Fixed below comments from Michael and Cornelia
- Fixed adding article 'the' before driver
- Change wording in driver requirements section to reflect driver requirements reflecting device behavior
- Added device compliance statement on what to return for queue_reset register to indicate ongoing reset in progress
- Moved device specific description adjacent to other device related queue reset description
v1->v2:
- Fixed below comment from Xuan Zhuo
- Fixed the configuration and register layout text to reflect the polarity v0->v1:
- Fixed below comments from Michael
- Removed example device side pseudo code for old and new polarity
- Removed unwanted articles 'a' and 'the'
- Dropped extra empty line

---
 content.tex | 48 ++++++++++++++++++++----------------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

content.tex +0 -4

index 060bdab..fdd255d 100644
@@ -1029,19 +1029,15 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 The device MUST present a 0 in \field{queue_enable} on reset.
 
 If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
-\field{queue_enable} after the driver has reset the virtqueue via
-\field{queue_reset}.
-
-If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
 \field{queue_reset} on reset.
 
 If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
 \field{queue_reset} after the virtqueue is enabled with \field{queue_enable}.
 

content.tex \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport · line 1038

Quoted 6 lines

content.tex +2 -4

-The device MUST reset the queue when 1 is written to \field{queue_reset}, and
-present a 1 in \field{queue_reset} after the queue has been reset, until the
-driver re-enables the queue via \field{queue_enable} or the device is reset.
-The device MUST present consistent default values after queue reset.
+The device MUST reset the queue when 1 is written to \field{queue_reset}. The
+device MUST continue to present 1 in \field{queue_reset} when the queue reset

s/when/as long as/

content.tex \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport · line 1039

Quoted 1 line

content.tex +1 -0

+is ongoing. The device MUST present 0 in \field{queue_reset} and \field{queue_enable}

s/in/in both/

content.tex \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport · line 1040

Quoted 1 line

content.tex +1 -0

+when queue reset completes.

Maybe "when queue reset has completed"?

Quoted 31 lines

content.tex +6 -10

 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 The device MUST present a 0 in \field{queue_size} if the virtqueue

@@ -1069,12 +1065,12 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 The driver MUST NOT write a 0 to \field{queue_enable}.
 
 If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to
-\field{queue_reset} to reset the queue, it MUST verify that the queue
-has been reset by reading back \field{queue_reset} and ensuring that it
-is 1. The driver MAY re-enable the queue by writing a 1 to
-\field{queue_enable} after ensuring that the other virtqueue fields have
-been set up correctly. The driver MAY set driver-writeable queue configuration
-values to different values than those that were used before the queue reset.
+\field{queue_reset} to reset the queue, the driver MUST NOT consider queue
+reset to be complete until it reads back 0 in \field{queue_reset}. The driver
+MAY re-enable the queue by writing 1 to \field{queue_enable} after ensuring
+that other virtqueue fields have been set up correctly. The driver MAY set
+driver-writeable queue configuration values to different values than those that
+were used before the queue reset.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}

@@ -2005,19 +2001,15 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 The device MUST NOT access virtual queue contents when \field{QueueReady} is zero (0x0).
 
 If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
-\field{QueueReady} after the driver has reset the virtqueue via
-\field{QueueReset}.
-
-If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
 \field{QueueReset} on reset.

content.tex \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi · line 2010

Quoted 10 lines

content.tex +2 -4

 
 If VIRTIO_F_RING_RESET has been negotiated, The device MUST present a 0 in
 \field{QueueReset} after the virtqueue is enabled with \field{QueueReady}.
 
-The device MUST reset the queue when 1 is written to \field{QueueReset}, and
-present a 1 in \field{QueueReset} after the queue has been reset, until the
-driver re-enables the queue via \field{QueueReady} or until the device is reset.
-The device MUST present consistent default values after queue reset.
+The device MUST reset the queue when 1 is written to \field{QueueReset}. The
+device MUST continue to present 1 in \field{QueueReset} when the queue reset

s/when/as long as/

content.tex \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi · line 2011

Quoted 1 line

content.tex +1 -0

+is ongoing. The device MUST present 0 in \field{QueueReset} and \field{QueueReady}

s/in/in both/

content.tex \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi · line 2012

Quoted 1 line

content.tex +1 -0

+when queue reset completes.

Maybe "has completed" here as well.

Quoted 21 lines

content.tex +6 -6

 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 \drivernormative{\subsubsection}{MMIO Device Register Layout}{Virtio Transport Options / Virtio Over MMIO / MMIO Device Register Layout}

@@ -2063,12 +2055,12 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 it finishes handling an interrupt and MUST NOT set any of the undefined bits in the value.
 
 If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to
-\field{QueueReset} to reset the queue, it MUST verify that the queue
-has been reset by reading back \field{QueueReset} and ensuring that it
-is 1. The driver MAY re-enable the queue by writing a 1 to
-\field{QueueReady} after ensuring that the other virtqueue fields have
-been set up correctly. The driver MAY set driver-writeable queue configuration
-values to different values than those that were used before the queue reset.
+\field{QueueReset} to reset the queue, the driver MUST NOT consider queue
+reset to be complete until it reads back 0 in \field{QueueReset}. The driver
+MAY re-enable the queue by writing 1 to \field{QueueReady} after ensuring
+that other virtqueue fields have been set up correctly. The driver MAY set
+driver-writeable queue configuration values to different values than those
+that were used before the queue reset.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 

content.tex \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi · line 2066

Quoted 1 line

content.tex +0 -0

 \subsection{MMIO-specific Initialization And Device Operation}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation}

From: Cornelia Huck <[email protected]>
Sent: Friday, April 29, 2022 4:07 AM

s/when/as long as/

Quoted 2 lines

+is ongoing. The device MUST present 0 in \field{queue_reset} and +\field{queue_enable}

s/in/in both/

Quoted 1 line

+when queue reset completes.

Maybe "when queue reset has completed"?

s/when/as long as/

Quoted 2 lines

+is ongoing. The device MUST present 0 in \field{QueueReset} and +\field{QueueReady}

s/in/in both/

Quoted 1 line

+when queue reset completes.

Maybe "has completed" here as well.

Currently when driver initiates a queue reset, device is expected to communicate reset status to the driver by changing the value of the queue_reset register twice. First to return value other than 1 when reset is ongoing, later to return 1 when queue reset is completed.

However initially during the device reset time the queue reset value is zero. queue_reset changes the value of the register to a different value on reset completion. Yet another time queue_reset value is expected to change when queue_select is reprogrammed.

For example in below flow, a created virtqueue, which is disabled by driver leaves the queue state as
queue_enable = 0, queue_reset = 1.

example flow:
a) 0,0 -> device init time value
b) 1,0 -> vq is enabled by driver and working
c) 1,1 -> vq is enabled, driver initiated reset
d) 0,0 -> queue reset is ongoing
conflicts with #a, because queue is still enabled in device whose reset is in progress.
External entity has no knowledge if VQ is undergoing reset or VQ is never enabled.
e) 0,1 -> queue reset is completed (conflicts with #a initial value)

On next iteration, when queue_select selects the same VQ again, without enablement, device is confused to return 1 or 0 because it was reset once before via queue_reset register.

This demands complex device implementation to understand what should be returned for a VQ that is reset using queue_reset register vs other means.

Additionally, external entity that may have to manage a virtio device cannot distinguish between #a and #d, whether a VQ is not yet enabled or it is in use and undergoing a reset.

Instead, it is better and efficient to maintain the same VQ state on the device when queue reset is completed.

new proposed flow:
q_enable, q_reset
A) 0, 0 -> default, device init time
B) 1, 0 -> driver has enabled vq
C) 1, 1 -> driver started q reset
D) 1, 1 -> q_reset stays 1 until device is busy resetting vq (device communicates that its working on resetting VQ, consistent with #C)
E) 0, 0 -> q_reset by device is completed, q got disabled (consistent with device init time #A)

Hence, this patch proposes a simple change to have reset register polarity to be same as that of initial reset value.

Fixes https://github.com/oasis-tcs/virtio-spec/issues/139

Fixes 12998e738621 ("virtio: pci support virtqueue reset")

Fixes a4ce81a83780 ("virtio: mmio support virtqueue reset")

Fixes 3b5378d70a42 ("virtio: introduce virtqueue reset as basic facility")

Signed-off-by Parav Pandit <[email protected]>

---
changelog:
v3->v4:
- removed incorrect requirement to read queue_enable on resetting the queue
v2->v3:
- Updated the commit log to capture the problem more accurately
- Fixed below comments from Michael and Cornelia
- Fixed adding article 'the' before driver
- Change wording in driver requirements section to reflect driver requirements reflecting device behavior
- Added device compliance statement on what to return for queue_reset register to indicate ongoing reset in progress
- Moved device specific description adjacent to other device related queue reset description
v1->v2:
- Fixed below comment from Xuan Zhuo
- Fixed the configuration and register layout text to reflect the polarity v0->v1:
- Fixed below comments from Michael
- Removed example device side pseudo code for old and new polarity
- Removed unwanted articles 'a' and 'the'
- Dropped extra empty line

---
 content.tex | 48 ++++++++++++++++++++----------------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

content.tex +20 -28

index 060bdab..fdd255d 100644
@@ -1029,19 +1029,15 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 The device MUST present a 0 in \field{queue_enable} on reset.
 
 If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
-\field{queue_enable} after the driver has reset the virtqueue via
-\field{queue_reset}.
-
-If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
 \field{queue_reset} on reset.
 
 If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
 \field{queue_reset} after the virtqueue is enabled with \field{queue_enable}.
 
-The device MUST reset the queue when 1 is written to \field{queue_reset}, and
-present a 1 in \field{queue_reset} after the queue has been reset, until the
-driver re-enables the queue via \field{queue_enable} or the device is reset.
-The device MUST present consistent default values after queue reset.
+The device MUST reset the queue when 1 is written to \field{queue_reset}. The
+device MUST continue to present 1 in \field{queue_reset} when the queue reset
+is ongoing. The device MUST present 0 in \field{queue_reset} and \field{queue_enable}
+when queue reset completes.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 The device MUST present a 0 in \field{queue_size} if the virtqueue

@@ -1069,12 +1065,12 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 The driver MUST NOT write a 0 to \field{queue_enable}.
 
 If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to
-\field{queue_reset} to reset the queue, it MUST verify that the queue
-has been reset by reading back \field{queue_reset} and ensuring that it
-is 1. The driver MAY re-enable the queue by writing a 1 to
-\field{queue_enable} after ensuring that the other virtqueue fields have
-been set up correctly. The driver MAY set driver-writeable queue configuration
-values to different values than those that were used before the queue reset.
+\field{queue_reset} to reset the queue, the driver MUST NOT consider queue
+reset to be complete until it reads back 0 in \field{queue_reset}. The driver
+MAY re-enable the queue by writing 1 to \field{queue_enable} after ensuring
+that other virtqueue fields have been set up correctly. The driver MAY set
+driver-writeable queue configuration values to different values than those that
+were used before the queue reset.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}

@@ -2005,19 +2001,15 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 The device MUST NOT access virtual queue contents when \field{QueueReady} is zero (0x0).
 
 If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
-\field{QueueReady} after the driver has reset the virtqueue via
-\field{QueueReset}.
-
-If VIRTIO_F_RING_RESET has been negotiated, the device MUST present a 0 in
 \field{QueueReset} on reset.
 
 If VIRTIO_F_RING_RESET has been negotiated, The device MUST present a 0 in
 \field{QueueReset} after the virtqueue is enabled with \field{QueueReady}.
 
-The device MUST reset the queue when 1 is written to \field{QueueReset}, and
-present a 1 in \field{QueueReset} after the queue has been reset, until the
-driver re-enables the queue via \field{QueueReady} or until the device is reset.
-The device MUST present consistent default values after queue reset.
+The device MUST reset the queue when 1 is written to \field{QueueReset}. The
+device MUST continue to present 1 in \field{QueueReset} when the queue reset
+is ongoing. The device MUST present 0 in \field{QueueReset} and \field{QueueReady}
+when queue reset completes.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 \drivernormative{\subsubsection}{MMIO Device Register Layout}{Virtio Transport Options / Virtio Over MMIO / MMIO Device Register Layout}

@@ -2063,12 +2055,12 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 it finishes handling an interrupt and MUST NOT set any of the undefined bits in the value.
 
 If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to
-\field{QueueReset} to reset the queue, it MUST verify that the queue
-has been reset by reading back \field{QueueReset} and ensuring that it
-is 1. The driver MAY re-enable the queue by writing a 1 to
-\field{QueueReady} after ensuring that the other virtqueue fields have
-been set up correctly. The driver MAY set driver-writeable queue configuration
-values to different values than those that were used before the queue reset.
+\field{QueueReset} to reset the queue, the driver MUST NOT consider queue
+reset to be complete until it reads back 0 in \field{QueueReset}. The driver
+MAY re-enable the queue by writing 1 to \field{QueueReady} after ensuring
+that other virtqueue fields have been set up correctly. The driver MAY set
+driver-writeable queue configuration values to different values than those
+that were used before the queue reset.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
 \subsection{MMIO-specific Initialization And Device Operation}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation}

Signature · 18 lines

1.8.3.1


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: [email protected]
Unsubscribe: [email protected]
List help: [email protected]
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/