linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [virtio-dev] [PATCH v2 0/5] virtio-mmio enhancement
@ 2020-01-21 13:54 Jing Liu
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 1/5] virtio-mmio: Add feature bit for MMIO notification Jing Liu
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Jing Liu @ 2020-01-21 13:54 UTC (permalink / raw)
  To: virtio-dev; +Cc: linux-kernel, kvm, qemu-devel, Jing Liu

The current virtio over MMIO has some limitations that impact the performance.
It only supports single legacy, dedicated interrupt and one virtqueue
notification register for all virtqueues which cause performance penalties.

To address such limitations, we proposed to update virtio-mmio spec with
two new feature bits to support MSI interrupt and enhancing notification mechanism.

For keeping virtio-mmio simple as it was, and considering practical usages, this
provides two kinds of mapping mode for device: MSI non-sharing mode and MSI sharing mode.
MSI non-sharng mode indicates a fixed static vector and event relationship specified
in spec, which can simplify the setup process and reduce vmexit, fitting for
a high interrupt rate request.
MSI sharing mode indicates a dynamic mapping, which is more like PCI does, fitting
for a non-high interrupt rate request.

Change Log:
v1->v2:
* Change version update to feature bit
* Add mask/unmask support
* Add two MSI sharing/non-sharing modes
* Change MSI registers layout and bits

Jing Liu (5):
  virtio-mmio: Add feature bit for MMIO notification
  virtio-mmio: Enhance queue notification support
  virtio-mmio: Add feature bit for MMIO MSI
  virtio-mmio: Introduce MSI details
  virtio-mmio: MSI vector and event mapping

 content.tex | 286 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 msi-state.c |   5 ++
 2 files changed, 262 insertions(+), 29 deletions(-)
 create mode 100644 msi-state.c

-- 
2.7.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [virtio-dev] [PATCH v2 1/5] virtio-mmio: Add feature bit for MMIO notification
  2020-01-21 13:54 [virtio-dev] [PATCH v2 0/5] virtio-mmio enhancement Jing Liu
@ 2020-01-21 13:54 ` Jing Liu
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 2/5] virtio-mmio: Enhance queue notification support Jing Liu
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Jing Liu @ 2020-01-21 13:54 UTC (permalink / raw)
  To: virtio-dev
  Cc: linux-kernel, kvm, qemu-devel, Jing Liu, Chao Peng, Liu Jiang, Zha Bin

All the queues notifications use the same register on MMIO transport
layer. Add a feature bit (39) for enhancing the notification capability.
The detailed mechanism would be in next patch.

Co-developed-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Co-developed-by: Liu Jiang <gerry@linux.alibaba.com>
Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
Co-developed-by: Zha Bin <zhabin@linux.alibaba.com>
Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>
Signed-off-by: Jing Liu <jing2.liu@linux.intel.com>
---
 content.tex | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/content.tex b/content.tex
index d68cfaf..826bc7d 100644
--- a/content.tex
+++ b/content.tex
@@ -5810,6 +5810,9 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
   in its device notifications.
   See \ref{sec:Virtqueues / Driver notifications}~\nameref{sec:Virtqueues / Driver notifications}.
 \end{description}
+  \item[VIRTIO_F_MMIO_NOTIFICATION(39)] This feature indicates
+  that the device supports enhanced notification mechanism on
+  MMIO transport layer.
 
 \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
 
@@ -5843,6 +5846,8 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
 or partially reset, and even without re-negotiating
 VIRTIO_F_SR_IOV after the reset.
 
+A driver SHOULD accept VIRTIO_F_MMIO_NOTIFICATION if it is offered.
+
 \devicenormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
 
 A device MUST offer VIRTIO_F_VERSION_1.  A device MAY fail to operate further
@@ -5872,6 +5877,10 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
 and presents a PCI SR-IOV capability structure, otherwise
 it MUST NOT offer VIRTIO_F_SR_IOV.
 
+If VIRTIO_F_MMIO_NOTIFICATION has been negotiated, a device
+MUST support handling the notification from driver at the
+calculated location.
+
 \section{Legacy Interface: Reserved Feature Bits}\label{sec:Reserved Feature Bits / Legacy Interface: Reserved Feature Bits}
 
 Transitional devices MAY offer the following:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [virtio-dev] [PATCH v2 2/5] virtio-mmio: Enhance queue notification support
  2020-01-21 13:54 [virtio-dev] [PATCH v2 0/5] virtio-mmio enhancement Jing Liu
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 1/5] virtio-mmio: Add feature bit for MMIO notification Jing Liu
@ 2020-01-21 13:54 ` Jing Liu
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 3/5] virtio-mmio: Add feature bit for MMIO MSI Jing Liu
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Jing Liu @ 2020-01-21 13:54 UTC (permalink / raw)
  To: virtio-dev
  Cc: linux-kernel, kvm, qemu-devel, Jing Liu, Chao Peng, Liu Jiang, Zha Bin

With VIRTIO_F_MMIO_NOTIFICATION feature bit offered, the notification
mechanism is enhanced. Driver reads QueueNotify register to get
notification structure and calculate notification addresses of
each virtqueue.

Co-developed-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Co-developed-by: Liu Jiang <gerry@linux.alibaba.com>
Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
Co-developed-by: Zha Bin <zhabin@linux.alibaba.com>
Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>
Signed-off-by: Jing Liu <jing2.liu@linux.intel.com>
---
 content.tex | 53 ++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 13 deletions(-)

diff --git a/content.tex b/content.tex
index 826bc7d..5881253 100644
--- a/content.tex
+++ b/content.tex
@@ -1671,20 +1671,18 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
     accesses apply to the queue selected by writing to \field{QueueSel}.
   }
   \hline 
-  \mmioreg{QueueNotify}{Queue notifier}{0x050}{W}{%
-    Writing a value to this register notifies the device that
-    there are new buffers to process in a queue.
+  \mmioreg{QueueNotify}{Queue notifier}{0x050}{RW}{%
+    When VIRTIO_F_MMIO_NOTIFICATION has not been negotiated, writing to this
+    register notifies the device that there are new buffers to process in a queue.
 
-    When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
-    the value written is the queue index.
+    When VIRTIO_F_MMIO_NOTIFICATION has been negotiated, reading this register
+    returns the virtqueue notification structure for calculating notification location.
 
-    When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
-    the \field{Notification data} value has the following format:
+    See \ref{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Notification Structure Layout}
+    for the notification structure format.
 
-    \lstinputlisting{notifications-le.c}
-
-    See \ref{sec:Virtqueues / Driver notifications}~\nameref{sec:Virtqueues / Driver notifications}
-    for the definition of the components.
+    See \ref{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Available Buffer Notifications}
+    for the notification data format.
   }
   \hline 
   \mmioreg{InterruptStatus}{Interrupt status}{0x60}{R}{%
@@ -1858,6 +1856,31 @@ \subsubsection{Device Initialization}\label{sec:Virtio Transport Options / Virti
 Further initialization MUST follow the procedure described in
 \ref{sec:General Initialization And Device Operation / Device Initialization}~\nameref{sec:General Initialization And Device Operation / Device Initialization}.
 
+\subsubsection{Notification Structure Layout}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Notification Structure Layout}
+
+When VIRTIO_F_MMIO_NOTIFICATION has been negotiated, the notification location is calculated
+by notification structure. Driver reads \field{QueueNotify} to get this structure formatted
+as follows.
+
+\begin{lstlisting}
+le32 {
+        notify_base : 16;
+        notify_multiplier : 16;
+};
+\end{lstlisting}
+
+\field{notify_multiplier} is combined with virtqueue index to derive the Queue Notify address
+within a memory mapped control registers for a virtqueue:
+
+\begin{lstlisting}
+        notify_base + queue_index * notify_multiplier
+\end{lstlisting}
+
+\begin{note}
+For example, if notify_multiplier is 0, the device uses the same Queue Notify address for all
+queues.
+\end{note}
+
 \subsubsection{Virtqueue Configuration}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Virtqueue Configuration}
 
 The driver will typically initialize the virtual queue in the following way:
@@ -1893,16 +1916,20 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Option
 When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
 the driver sends an available buffer notification to the device by writing
 the 16-bit virtqueue index
-of the queue to be notified to \field{QueueNotify}.
+of the queue to be notified to Queue Notify address.
 
 When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
 the driver sends an available buffer notification to the device by writing
-the following 32-bit value to \field{QueueNotify}:
+the following 32-bit value to Queue Notify address:
 \lstinputlisting{notifications-le.c}
 
 See \ref{sec:Virtqueues / Driver notifications}~\nameref{sec:Virtqueues / Driver notifications}
 for the definition of the components.
 
+For device not offering VIRTIO_F_MMIO_NOTIFICATION, the Queue Notify address is \field{QueueNotify}.
+For device offering VIRTIO_F_MMIO_NOTIFICATION, see \ref{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Notification Structure Layout}
+for how to calculate the Queue Notify address.
+
 \subsubsection{Notifications From The Device}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Notifications From The Device}
 
 The memory mapped virtio device is using a single, dedicated
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [virtio-dev] [PATCH v2 3/5] virtio-mmio: Add feature bit for MMIO MSI
  2020-01-21 13:54 [virtio-dev] [PATCH v2 0/5] virtio-mmio enhancement Jing Liu
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 1/5] virtio-mmio: Add feature bit for MMIO notification Jing Liu
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 2/5] virtio-mmio: Enhance queue notification support Jing Liu
@ 2020-01-21 13:54 ` Jing Liu
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 4/5] virtio-mmio: Introduce MSI details Jing Liu
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 5/5] virtio-mmio: MSI vector and event mapping Jing Liu
  4 siblings, 0 replies; 8+ messages in thread
From: Jing Liu @ 2020-01-21 13:54 UTC (permalink / raw)
  To: virtio-dev
  Cc: linux-kernel, kvm, qemu-devel, Jing Liu, Chao Peng, Liu Jiang, Zha Bin

The current MMIO transport layer uses a single, dedicated interrupt
signal, which brings performance penalty. Add a feature bit (40)
for introducing MSI capability.

Co-developed-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Co-developed-by: Liu Jiang <gerry@linux.alibaba.com>
Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
Co-developed-by: Zha Bin <zhabin@linux.alibaba.com>
Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>
Signed-off-by: Jing Liu <jing2.liu@linux.intel.com>
---
 content.tex | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/content.tex b/content.tex
index 5881253..ff151ba 100644
--- a/content.tex
+++ b/content.tex
@@ -5840,6 +5840,9 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
   \item[VIRTIO_F_MMIO_NOTIFICATION(39)] This feature indicates
   that the device supports enhanced notification mechanism on
   MMIO transport layer.
+  \item[VIRTIO_F_MMIO_MSI(40)] This feature indicates that the
+  device supports Message Signal Interrupts (MSI) mechanism on
+  MMIO transport layer.
 
 \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
 
@@ -5875,6 +5878,10 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
 
 A driver SHOULD accept VIRTIO_F_MMIO_NOTIFICATION if it is offered.
 
+A driver SHOULD accept VIRTIO_F_MMIO_MSI if it is offered.
+If VIRTIO_F_MMIO_MSI has been negotiated, a driver MUST try to
+set up MSI at first priority.
+
 \devicenormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
 
 A device MUST offer VIRTIO_F_VERSION_1.  A device MAY fail to operate further
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [virtio-dev] [PATCH v2 4/5] virtio-mmio: Introduce MSI details
  2020-01-21 13:54 [virtio-dev] [PATCH v2 0/5] virtio-mmio enhancement Jing Liu
                   ` (2 preceding siblings ...)
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 3/5] virtio-mmio: Add feature bit for MMIO MSI Jing Liu
@ 2020-01-21 13:54 ` Jing Liu
  2020-01-29 10:12   ` Michael S. Tsirkin
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 5/5] virtio-mmio: MSI vector and event mapping Jing Liu
  4 siblings, 1 reply; 8+ messages in thread
From: Jing Liu @ 2020-01-21 13:54 UTC (permalink / raw)
  To: virtio-dev
  Cc: linux-kernel, kvm, qemu-devel, Jing Liu, Chao Peng, Liu Jiang, Zha Bin

With VIRTIO_F_MMIO_MSI feature bit offered, the Message Signal
Interrupts (MSI) is supported as first priority. For any reason it
fails to use MSI, it need use the single dedicated interrupt as before.

For MSI vectors and events mapping relationship, introduce in next patch.

Co-developed-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Co-developed-by: Liu Jiang <gerry@linux.alibaba.com>
Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
Co-developed-by: Zha Bin <zhabin@linux.alibaba.com>
Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>
Signed-off-by: Jing Liu <jing2.liu@linux.intel.com>
---
 content.tex | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 msi-state.c |   4 ++
 2 files changed, 159 insertions(+), 16 deletions(-)
 create mode 100644 msi-state.c

diff --git a/content.tex b/content.tex
index ff151ba..dcf6c71 100644
--- a/content.tex
+++ b/content.tex
@@ -1687,7 +1687,8 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
   \hline 
   \mmioreg{InterruptStatus}{Interrupt status}{0x60}{R}{%
     Reading from this register returns a bit mask of events that
-    caused the device interrupt to be asserted.
+    caused the device interrupt to be asserted. This is only used
+    when MSI is not enabled.
     The following events are possible:
     \begin{description}
       \item[Used Buffer Notification] - bit 0 - the interrupt was asserted
@@ -1701,7 +1702,7 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
   \mmioreg{InterruptACK}{Interrupt acknowledge}{0x064}{W}{%
     Writing a value with bits set as defined in \field{InterruptStatus}
     to this register notifies the device that events causing
-    the interrupt have been handled.
+    the interrupt have been handled. This is only used when MSI is not enabled.
   }
   \hline 
   \mmioreg{Status}{Device status}{0x070}{RW}{%
@@ -1760,6 +1761,47 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
     \field{SHMSel} is unused) results in a base address of
     0xffffffffffffffff.
   }
+  \hline
+  \mmioreg{MsiVecNum}{MSI max vector number}{0x0c0}{R}{%
+    When VIRTIO_F_MMIO_MSI has been negotiated, reading
+    from this register returns the maximum MSI vector number
+    that device supports.
+  }
+  \hline
+  \mmioreg{MsiState}{MSI state}{0x0c4}{R}{%
+    When VIRTIO_F_MMIO_MSI has been negotiated, reading
+    from this register returns the global MSI enable/disable status.
+    \lstinputlisting{msi-state.c}
+  }
+  \hline
+  \mmioreg{MsiCmd}{MSI command}{0x0c8}{W}{%
+    When VIRTIO_F_MMIO_MSI has been negotiated, writing
+    to this register executes the corresponding command to device.
+    Part of this applies to the MSI vector selected by writing to \field{MsiVecSel}.
+    See \ref{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Device Initialization / MSI Vector Configuration}
+    for using details.
+  }
+  \hline
+  \mmioreg{MsiVecSel}{MSI vector index}{0x0d0}{W}{%
+    When VIRTIO_F_MMIO_MSI has been negotiated, writing
+    to this register selects the MSI vector index that the following operations
+    on \field{MsiAddrLow}, \field{MsiAddrHigh}, \field{MsiData} and part of
+    \field{MsiCmd} commands specified in \ref{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Device Initialization / MSI Vector Configuration}
+    apply to. The index number of the first vector is zero (0x0).
+  }
+  \hline
+  \mmiodreg{MsiAddrLow}{MsiAddrHigh}{MSI 64 bit address}{0x0d4}{0x0d8}{W}{%
+    When VIRTIO_F_MMIO_MSI has been negotiated, writing
+    to these two registers (lower 32 bits of the address to \field{MsiAddrLow},
+    higher 32 bits to \field{MsiAddrHigh}) notifies the device about the
+    MSI address. This applies to the MSI vector selected by writing to \field{MsiVecSel}.
+  }
+  \hline
+  \mmioreg{MsiData}{MSI 32 bit data}{0x0dc}{W}{%
+    When VIRTIO_F_MMIO_MSI has been negotiated, writing
+    to this register notifies the device about the MSI data.
+    This applies to the MSI vector selected by writing to \field{MsiVecSel}.
+  }
   \hline 
   \mmioreg{ConfigGeneration}{Configuration atomicity value}{0x0fc}{R}{
     Reading from this register returns a value describing a version of the device-specific configuration space (see \field{Config}).
@@ -1783,10 +1825,16 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 
 The device MUST return value 0x2 in \field{Version}.
 
-The device MUST present each event by setting the corresponding bit in \field{InterruptStatus} from the
+When MSI is disabled, the device MUST present each event by setting the
+corresponding bit in \field{InterruptStatus} from the
 moment it takes place, until the driver acknowledges the interrupt
-by writing a corresponding bit mask to the \field{InterruptACK} register.  Bits which
-do not represent events which took place MUST be zero.
+by writing a corresponding bit mask to the \field{InterruptACK} register.
+Bits which do not represent events which took place MUST be zero.
+
+When MSI is enabled, the device MUST NOT set \field{InterruptStatus} and MUST
+ignore \field{InterruptACK}.
+
+Upon reset, the device MUST clear \field{msi_enabled} bit in \field{MsiState}.
 
 Upon reset, the device MUST clear all bits in \field{InterruptStatus} and ready bits in the
 \field{QueueReady} register for all queues in the device.
@@ -1835,7 +1883,12 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
 
 The driver MUST ignore undefined bits in \field{InterruptStatus}.
 
-The driver MUST write a value with a bit mask describing events it handled into \field{InterruptACK} when
+The driver MUST ignore undefined bits in the return value of reading \field{MsiState}.
+
+When MSI is enabled, the driver MUST NOT access \field{InterruptStatus} and MUST NOT write to \field{InterruptACK}.
+
+When MSI is disabled, the driver MUST write a value with a bit mask
+describing events it handled into \field{InterruptACK} when
 it finishes handling an interrupt and MUST NOT set any of the undefined bits in the value.
 
 \subsection{MMIO-specific Initialization And Device Operation}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation}
@@ -1856,6 +1909,63 @@ \subsubsection{Device Initialization}\label{sec:Virtio Transport Options / Virti
 Further initialization MUST follow the procedure described in
 \ref{sec:General Initialization And Device Operation / Device Initialization}~\nameref{sec:General Initialization And Device Operation / Device Initialization}.
 
+\paragraph{MSI Vector Configuration}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Device Initialization / MSI Vector Configuration}
+The VIRTIO_F_MMIO_MSI feature bit offered by device shows the capability
+using MSI vectors for virtqueue and configuration events.
+
+When VIRTIO_F_MMIO_MSI has been negotiated,
+writing \field{MsiCmd} executes a corresponding command to the device:
+
+VIRTIO_MMIO_MSI_CMD_ENABLE and VIRTIO_MMIO_MSI_CMD_DISABLE commands set global
+MSI enable and disable status.
+
+VIRTIO_MMIO_MSI_CMD_CONFIGURE is used to configure the MSI vector
+applying to the one selected by writing to \field{MsiVecSel}.
+
+VIRTIO_MMIO_MSI_CMD_MASK and VIRTIO_MMIO_MSI_CMD_UNMASK commands are used to
+mask and unmask the MSI vector applying to the one selected by writing
+to \field{MsiVecSel}.
+
+\begin{lstlisting}
+#define  VIRTIO_MMIO_MSI_CMD_ENABLE           0x1
+#define  VIRTIO_MMIO_MSI_CMD_DISABLE          0x2
+#define  VIRTIO_MMIO_MSI_CMD_CONFIGURE        0x3
+#define  VIRTIO_MMIO_MSI_CMD_MASK             0x4
+#define  VIRTIO_MMIO_MSI_CMD_UNMASK           0x5
+\end{lstlisting}
+
+Setting a special NO_VECTOR value means disabling an interrupt for an event type.
+
+\begin{lstlisting}
+/* Vector value used to disable MSI for event */
+#define VIRTIO_MMIO_MSI_NO_VECTOR             0xffffffff
+\end{lstlisting}
+
+\drivernormative{\subparagraph}{MSI Vector Configuration}{Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / MSI Vector Configuration}
+When VIRTIO_F_MMIO_MSI has been negotiated, driver should try to configure
+and enable MSI.
+
+To configure MSI vector, driver SHOULD firstly specify the MSI vector index by
+writing to \field{MsiVecSel}.
+Then notify the MSI address and data by writing to \field{MsiAddrLow}, \field{MsiAddrHigh},
+and \field{MsiData}, and immediately follow a \field{MsiCmd} write operation
+using VIRTIO_MMIO_MSI_CMD_CONFIGURE to device for configuring an event to
+this MSI vector.
+
+After all MSI vectors are configured, driver SHOULD set global MSI enabled
+by writing to \field{MsiCmd} using VIRTIO_MMIO_MSI_CMD_ENABLE.
+
+Driver should use VIRTIO_MMIO_MSI_CMD_DISABLE when disabling MSI.
+
+Driver should use VIRTIO_MMIO_MSI_CMD_MASK with an MSI index \field{MsiVecSel}
+to prohibit the event from the corresponding interrupt source.
+
+Driver should use VIRTIO_MMIO_MSI_CMD_UNMASK with an MSI index \field{MsiVecSel}
+to recover the event from the corresponding interrupt source.
+
+If driver fails to setup any event with a vector,
+it MUST disable MSI by \field{MsiCmd} and use the single dedicated interrupt for device.
+
 \subsubsection{Notification Structure Layout}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Notification Structure Layout}
 
 When VIRTIO_F_MMIO_NOTIFICATION has been negotiated, the notification location is calculated
@@ -1908,6 +2018,12 @@ \subsubsection{Virtqueue Configuration}\label{sec:Virtio Transport Options / Vir
    \field{QueueDriverLow}/\field{QueueDriverHigh} and
    \field{QueueDeviceLow}/\field{QueueDeviceHigh} register pairs.
 
+\item Write MSI address \field{MsiAddrLow}/\field{MsiAddrHigh},
+MSI data \field{MsiData} and MSI update command \field{MsiCtrlStat} with corresponding
+virtqueue index to update
+MSI configuration for device requesting interrupts triggered by
+virtqueue events.
+
 \item Write 0x1 to \field{QueueReady}.
 \end{enumerate}
 
@@ -1932,20 +2048,43 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Option
 
 \subsubsection{Notifications From The Device}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Notifications From The Device}
 
-The memory mapped virtio device is using a single, dedicated
+If MSI is enabled, the memory mapped virtio
+device uses appropriate MSI interrupt message
+for configuration change notification and used buffer notification which are
+configured by \field{MsiAddrLow}, \field{MsoAddrHigh} and \field{MsiData}.
+
+If MSI is not enabled, the memory mapped virtio device
+uses a single, dedicated
 interrupt signal, which is asserted when at least one of the
 bits described in the description of \field{InterruptStatus}
-is set. This is how the device sends a used buffer notification
-or a configuration change notification to the device.
+is set.
 
 \drivernormative{\paragraph}{Notifications From The Device}{Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Notifications From The Device}
-After receiving an interrupt, the driver MUST read
-\field{InterruptStatus} to check what caused the interrupt (see the
-register description).  The used buffer notification bit being set
-SHOULD be interpreted as a used buffer notification for each active
-virtqueue.  After the interrupt is handled, the driver MUST acknowledge
-it by writing a bit mask corresponding to the handled events to the
-InterruptACK register.
+A driver MUST handle the case where MSI is disabled, which uses the same interrupt indicating both device configuration
+space change and one or more virtqueues being used.
+
+\subsubsection{Driver Handling Interrupts}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Driver Handling Interrupts}
+
+The driver interrupt handler would typically:
+
+\begin{itemize}
+  \item If MSI is enabled:
+    \begin{itemize}
+      \item
+        Figure out the virtqueue mapped to that MSI vector for the
+        device, to see if any progress has been made by the device
+        which requires servicing.
+      \item
+        If the interrupt belongs to configuration space changing signal,
+        re-examine the configuration space to see what changed.
+    \end{itemize}
+  \item If MSI is disabled:
+    \begin{itemize}
+      \item Read \field{InterruptStatus} to check what caused the interrupt.
+      \item Acknowledge the interrupt by writing a bit mask corresponding
+            to the handled events to the InterruptACK register.
+    \end{itemize}
+\end{itemize}
 
 \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over MMIO / Legacy interface}
 
diff --git a/msi-state.c b/msi-state.c
new file mode 100644
index 0000000..b1fa0c1
--- /dev/null
+++ b/msi-state.c
@@ -0,0 +1,4 @@
+le32 {
+    msi_enabled : 1;
+    reserved : 31;
+};
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [virtio-dev] [PATCH v2 5/5] virtio-mmio: MSI vector and event mapping
  2020-01-21 13:54 [virtio-dev] [PATCH v2 0/5] virtio-mmio enhancement Jing Liu
                   ` (3 preceding siblings ...)
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 4/5] virtio-mmio: Introduce MSI details Jing Liu
@ 2020-01-21 13:54 ` Jing Liu
  2020-01-29 10:14   ` Michael S. Tsirkin
  4 siblings, 1 reply; 8+ messages in thread
From: Jing Liu @ 2020-01-21 13:54 UTC (permalink / raw)
  To: virtio-dev
  Cc: linux-kernel, kvm, qemu-devel, Jing Liu, Chao Peng, Liu Jiang, Zha Bin

Bit 1 msi_sharing reported in the MsiState register indicates the mapping mode
device uses.

Bit 1 is 0 - device uses MSI non-sharing mode. This indicates vector per event and
fixed static vectors and events relationship. This fits for devices with a high interrupt
rate and best performance;
Bit 1 is 1 - device uses MSI sharing mode. This indicates vectors and events
dynamic mapping and fits for devices not requiring a high interrupt rate.

Co-developed-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Co-developed-by: Liu Jiang <gerry@linux.alibaba.com>
Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
Co-developed-by: Zha Bin <zhabin@linux.alibaba.com>
Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>
Signed-off-by: Jing Liu <jing2.liu@linux.intel.com>
---
 content.tex | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
 msi-state.c |  3 ++-
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/content.tex b/content.tex
index dcf6c71..2fd1686 100644
--- a/content.tex
+++ b/content.tex
@@ -1770,7 +1770,8 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
   \hline
   \mmioreg{MsiState}{MSI state}{0x0c4}{R}{%
     When VIRTIO_F_MMIO_MSI has been negotiated, reading
-    from this register returns the global MSI enable/disable status.
+    from this register returns the global MSI enable/disable status
+    and whether device uses MSI sharing mode.
     \lstinputlisting{msi-state.c}
   }
   \hline
@@ -1926,12 +1927,18 @@ \subsubsection{Device Initialization}\label{sec:Virtio Transport Options / Virti
 mask and unmask the MSI vector applying to the one selected by writing
 to \field{MsiVecSel}.
 
+VIRTIO_MMIO_MSI_CMD_MAP_CONFIG command is to set the configuration event and MSI vector
+mapping. VIRTIO_MMIO_MSI_CMD_MAP_QUEUE is to set the queue event and MSI vector
+mapping. They SHOULD only be used in MSI sharing mode.
+
 \begin{lstlisting}
 #define  VIRTIO_MMIO_MSI_CMD_ENABLE           0x1
 #define  VIRTIO_MMIO_MSI_CMD_DISABLE          0x2
 #define  VIRTIO_MMIO_MSI_CMD_CONFIGURE        0x3
 #define  VIRTIO_MMIO_MSI_CMD_MASK             0x4
 #define  VIRTIO_MMIO_MSI_CMD_UNMASK           0x5
+#define  VIRTIO_MMIO_MSI_CMD_MAP_CONFIG       0x6
+#define  VIRTIO_MMIO_MSI_CMD_MAP_QUEUE        0x7
 \end{lstlisting}
 
 Setting a special NO_VECTOR value means disabling an interrupt for an event type.
@@ -1941,10 +1948,49 @@ \subsubsection{Device Initialization}\label{sec:Virtio Transport Options / Virti
 #define VIRTIO_MMIO_MSI_NO_VECTOR             0xffffffff
 \end{lstlisting}
 
+\subparagraph{MSI Vector and Event Mapping}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Device Initialization / MSI Vector Configuration}
+The reported \field{msi_sharing} bit in the \field{MsiState} return value shows
+the MSI sharing mode that device uses.
+
+When \field{msi_sharing} bit is 0, it indicates the device uses non-sharing mode
+and vector per event fixed static relationship is used. The first vector is for device
+configuraiton change event, the second vector is for virtqueue 1, the third vector
+is for virtqueue 2 and so on.
+
+When \field{msi_sharing} bit is 1, it indicates the device uses MSI sharing mode,
+and the vector and event mapping is dynamic. Writing \field{MsiVecSel}
+followed by writing VIRTIO_MMIO_MSI_CMD_MAP_CONFIG/VIRTIO_MMIO_MSI_CMD_MAP_QUEUE command
+maps interrupts triggered by the configuration change/selected queue events respectively
+to the corresponding MSI vector.
+
+\devicenormative{\subparagraph}{MSI Vector Configuration}{Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / MSI Vector Configuration}
+
+When the device reports \field{msi_sharing} bit as 0, it SHOULD support a number of
+vectors that greater than the maximum number of virtqueues.
+Device MUST report the number of vectors supported in \field{MsiVecNum}.
+
+When the device reports \field{msi_sharing} bit as 1, it SHOULD support at least
+2 MSI vectors and MUST report in \field{MsiVecNum}. Device SHOULD support mapping any
+event type to any vector under \field{MsiVecNum}.
+
+Device MUST support unmapping any event type (NO_VECTOR).
+
+The device SHOULD restrict the reported \field{msi_sharing} and \field{MsiVecNum}
+to a value that might benefit system performance.
+
+\begin{note}
+For example, a device which does not expect to send interrupts at a high rate might
+return \field{msi_sharing} bit as 1.
+\end{note}
+
 \drivernormative{\subparagraph}{MSI Vector Configuration}{Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / MSI Vector Configuration}
 When VIRTIO_F_MMIO_MSI has been negotiated, driver should try to configure
 and enable MSI.
 
+To set up the event and vector mapping for MSI sharing mode, driver SHOULD
+write a valid \field{MsiVecSel} followed by VIRTIO_MMIO_MSI_CMD_MAP_CONFIG/VIRTIO_MMIO_MSI_CMD_MAP_QUEUE
+command to map the configuration change/selected queue events respectively.
+
 To configure MSI vector, driver SHOULD firstly specify the MSI vector index by
 writing to \field{MsiVecSel}.
 Then notify the MSI address and data by writing to \field{MsiAddrLow}, \field{MsiAddrHigh},
diff --git a/msi-state.c b/msi-state.c
index b1fa0c1..d470be4 100644
--- a/msi-state.c
+++ b/msi-state.c
@@ -1,4 +1,5 @@
 le32 {
     msi_enabled : 1;
-    reserved : 31;
+    msi_sharing: 1;
+    reserved : 30;
 };
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [virtio-dev] [PATCH v2 4/5] virtio-mmio: Introduce MSI details
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 4/5] virtio-mmio: Introduce MSI details Jing Liu
@ 2020-01-29 10:12   ` Michael S. Tsirkin
  0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2020-01-29 10:12 UTC (permalink / raw)
  To: Jing Liu
  Cc: virtio-dev, linux-kernel, kvm, qemu-devel, Chao Peng, Liu Jiang, Zha Bin

On Tue, Jan 21, 2020 at 09:54:32PM +0800, Jing Liu wrote:
> With VIRTIO_F_MMIO_MSI feature bit offered, the Message Signal
> Interrupts (MSI) is supported as first priority. For any reason it
> fails to use MSI, it need use the single dedicated interrupt as before.
> 
> For MSI vectors and events mapping relationship, introduce in next patch.
> 
> Co-developed-by: Chao Peng <chao.p.peng@linux.intel.com>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Co-developed-by: Liu Jiang <gerry@linux.alibaba.com>
> Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
> Co-developed-by: Zha Bin <zhabin@linux.alibaba.com>
> Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>
> Signed-off-by: Jing Liu <jing2.liu@linux.intel.com>


So we have a concept of "MSI vectors" here, which can be
selected and configured and which in the
following patch are mapped to VQs either 1:1 or dynamically.


My question is, do we need this indirection?

In fact an MSI vector is just an address/data pair.

So it seems that instead, we could just have commands specifying
MSI address/data pairs for each VQ, and separately for config changes.

It is useful to have hypervisor hint to guest how many different
pairs should be allocated, and that could be the RO max value.


> ---
>  content.tex | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  msi-state.c |   4 ++
>  2 files changed, 159 insertions(+), 16 deletions(-)
>  create mode 100644 msi-state.c
> 
> diff --git a/content.tex b/content.tex
> index ff151ba..dcf6c71 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -1687,7 +1687,8 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
>    \hline 
>    \mmioreg{InterruptStatus}{Interrupt status}{0x60}{R}{%
>      Reading from this register returns a bit mask of events that
> -    caused the device interrupt to be asserted.
> +    caused the device interrupt to be asserted. This is only used
> +    when MSI is not enabled.
>      The following events are possible:
>      \begin{description}
>        \item[Used Buffer Notification] - bit 0 - the interrupt was asserted
> @@ -1701,7 +1702,7 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
>    \mmioreg{InterruptACK}{Interrupt acknowledge}{0x064}{W}{%
>      Writing a value with bits set as defined in \field{InterruptStatus}
>      to this register notifies the device that events causing
> -    the interrupt have been handled.
> +    the interrupt have been handled. This is only used when MSI is not enabled.
>    }
>    \hline 
>    \mmioreg{Status}{Device status}{0x070}{RW}{%
> @@ -1760,6 +1761,47 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
>      \field{SHMSel} is unused) results in a base address of
>      0xffffffffffffffff.
>    }
> +  \hline
> +  \mmioreg{MsiVecNum}{MSI max vector number}{0x0c0}{R}{%
> +    When VIRTIO_F_MMIO_MSI has been negotiated, reading
> +    from this register returns the maximum MSI vector number
> +    that device supports.
> +  }
> +  \hline
> +  \mmioreg{MsiState}{MSI state}{0x0c4}{R}{%
> +    When VIRTIO_F_MMIO_MSI has been negotiated, reading
> +    from this register returns the global MSI enable/disable status.
> +    \lstinputlisting{msi-state.c}
> +  }
> +  \hline
> +  \mmioreg{MsiCmd}{MSI command}{0x0c8}{W}{%
> +    When VIRTIO_F_MMIO_MSI has been negotiated, writing
> +    to this register executes the corresponding command to device.
> +    Part of this applies to the MSI vector selected by writing to \field{MsiVecSel}.
> +    See \ref{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Device Initialization / MSI Vector Configuration}
> +    for using details.
> +  }
> +  \hline
> +  \mmioreg{MsiVecSel}{MSI vector index}{0x0d0}{W}{%
> +    When VIRTIO_F_MMIO_MSI has been negotiated, writing
> +    to this register selects the MSI vector index that the following operations
> +    on \field{MsiAddrLow}, \field{MsiAddrHigh}, \field{MsiData} and part of
> +    \field{MsiCmd} commands specified in \ref{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Device Initialization / MSI Vector Configuration}
> +    apply to. The index number of the first vector is zero (0x0).
> +  }
> +  \hline
> +  \mmiodreg{MsiAddrLow}{MsiAddrHigh}{MSI 64 bit address}{0x0d4}{0x0d8}{W}{%
> +    When VIRTIO_F_MMIO_MSI has been negotiated, writing
> +    to these two registers (lower 32 bits of the address to \field{MsiAddrLow},
> +    higher 32 bits to \field{MsiAddrHigh}) notifies the device about the
> +    MSI address. This applies to the MSI vector selected by writing to \field{MsiVecSel}.
> +  }
> +  \hline
> +  \mmioreg{MsiData}{MSI 32 bit data}{0x0dc}{W}{%
> +    When VIRTIO_F_MMIO_MSI has been negotiated, writing
> +    to this register notifies the device about the MSI data.
> +    This applies to the MSI vector selected by writing to \field{MsiVecSel}.
> +  }
>    \hline 
>    \mmioreg{ConfigGeneration}{Configuration atomicity value}{0x0fc}{R}{
>      Reading from this register returns a value describing a version of the device-specific configuration space (see \field{Config}).
> @@ -1783,10 +1825,16 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
>  
>  The device MUST return value 0x2 in \field{Version}.
>  
> -The device MUST present each event by setting the corresponding bit in \field{InterruptStatus} from the
> +When MSI is disabled, the device MUST present each event by setting the
> +corresponding bit in \field{InterruptStatus} from the
>  moment it takes place, until the driver acknowledges the interrupt
> -by writing a corresponding bit mask to the \field{InterruptACK} register.  Bits which
> -do not represent events which took place MUST be zero.
> +by writing a corresponding bit mask to the \field{InterruptACK} register.
> +Bits which do not represent events which took place MUST be zero.
> +
> +When MSI is enabled, the device MUST NOT set \field{InterruptStatus} and MUST
> +ignore \field{InterruptACK}.
> +
> +Upon reset, the device MUST clear \field{msi_enabled} bit in \field{MsiState}.
>  
>  Upon reset, the device MUST clear all bits in \field{InterruptStatus} and ready bits in the
>  \field{QueueReady} register for all queues in the device.
> @@ -1835,7 +1883,12 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
>  
>  The driver MUST ignore undefined bits in \field{InterruptStatus}.
>  
> -The driver MUST write a value with a bit mask describing events it handled into \field{InterruptACK} when
> +The driver MUST ignore undefined bits in the return value of reading \field{MsiState}.
> +
> +When MSI is enabled, the driver MUST NOT access \field{InterruptStatus} and MUST NOT write to \field{InterruptACK}.
> +
> +When MSI is disabled, the driver MUST write a value with a bit mask
> +describing events it handled into \field{InterruptACK} when
>  it finishes handling an interrupt and MUST NOT set any of the undefined bits in the value.
>  
>  \subsection{MMIO-specific Initialization And Device Operation}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation}
> @@ -1856,6 +1909,63 @@ \subsubsection{Device Initialization}\label{sec:Virtio Transport Options / Virti
>  Further initialization MUST follow the procedure described in
>  \ref{sec:General Initialization And Device Operation / Device Initialization}~\nameref{sec:General Initialization And Device Operation / Device Initialization}.
>  
> +\paragraph{MSI Vector Configuration}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Device Initialization / MSI Vector Configuration}
> +The VIRTIO_F_MMIO_MSI feature bit offered by device shows the capability
> +using MSI vectors for virtqueue and configuration events.
> +
> +When VIRTIO_F_MMIO_MSI has been negotiated,
> +writing \field{MsiCmd} executes a corresponding command to the device:
> +
> +VIRTIO_MMIO_MSI_CMD_ENABLE and VIRTIO_MMIO_MSI_CMD_DISABLE commands set global
> +MSI enable and disable status.
> +
> +VIRTIO_MMIO_MSI_CMD_CONFIGURE is used to configure the MSI vector
> +applying to the one selected by writing to \field{MsiVecSel}.
> +
> +VIRTIO_MMIO_MSI_CMD_MASK and VIRTIO_MMIO_MSI_CMD_UNMASK commands are used to
> +mask and unmask the MSI vector applying to the one selected by writing
> +to \field{MsiVecSel}.
> +
> +\begin{lstlisting}
> +#define  VIRTIO_MMIO_MSI_CMD_ENABLE           0x1
> +#define  VIRTIO_MMIO_MSI_CMD_DISABLE          0x2
> +#define  VIRTIO_MMIO_MSI_CMD_CONFIGURE        0x3
> +#define  VIRTIO_MMIO_MSI_CMD_MASK             0x4
> +#define  VIRTIO_MMIO_MSI_CMD_UNMASK           0x5
> +\end{lstlisting}
> +
> +Setting a special NO_VECTOR value means disabling an interrupt for an event type.
> +
> +\begin{lstlisting}
> +/* Vector value used to disable MSI for event */
> +#define VIRTIO_MMIO_MSI_NO_VECTOR             0xffffffff
> +\end{lstlisting}
> +
> +\drivernormative{\subparagraph}{MSI Vector Configuration}{Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / MSI Vector Configuration}
> +When VIRTIO_F_MMIO_MSI has been negotiated, driver should try to configure
> +and enable MSI.
> +
> +To configure MSI vector, driver SHOULD firstly specify the MSI vector index by
> +writing to \field{MsiVecSel}.
> +Then notify the MSI address and data by writing to \field{MsiAddrLow}, \field{MsiAddrHigh},
> +and \field{MsiData}, and immediately follow a \field{MsiCmd} write operation
> +using VIRTIO_MMIO_MSI_CMD_CONFIGURE to device for configuring an event to
> +this MSI vector.
> +
> +After all MSI vectors are configured, driver SHOULD set global MSI enabled
> +by writing to \field{MsiCmd} using VIRTIO_MMIO_MSI_CMD_ENABLE.
> +
> +Driver should use VIRTIO_MMIO_MSI_CMD_DISABLE when disabling MSI.
> +
> +Driver should use VIRTIO_MMIO_MSI_CMD_MASK with an MSI index \field{MsiVecSel}
> +to prohibit the event from the corresponding interrupt source.
> +
> +Driver should use VIRTIO_MMIO_MSI_CMD_UNMASK with an MSI index \field{MsiVecSel}
> +to recover the event from the corresponding interrupt source.
> +
> +If driver fails to setup any event with a vector,
> +it MUST disable MSI by \field{MsiCmd} and use the single dedicated interrupt for device.
> +
>  \subsubsection{Notification Structure Layout}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Notification Structure Layout}
>  
>  When VIRTIO_F_MMIO_NOTIFICATION has been negotiated, the notification location is calculated
> @@ -1908,6 +2018,12 @@ \subsubsection{Virtqueue Configuration}\label{sec:Virtio Transport Options / Vir
>     \field{QueueDriverLow}/\field{QueueDriverHigh} and
>     \field{QueueDeviceLow}/\field{QueueDeviceHigh} register pairs.
>  
> +\item Write MSI address \field{MsiAddrLow}/\field{MsiAddrHigh},
> +MSI data \field{MsiData} and MSI update command \field{MsiCtrlStat} with corresponding
> +virtqueue index to update
> +MSI configuration for device requesting interrupts triggered by
> +virtqueue events.
> +
>  \item Write 0x1 to \field{QueueReady}.
>  \end{enumerate}
>  
> @@ -1932,20 +2048,43 @@ \subsubsection{Available Buffer Notifications}\label{sec:Virtio Transport Option
>  
>  \subsubsection{Notifications From The Device}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Notifications From The Device}
>  
> -The memory mapped virtio device is using a single, dedicated
> +If MSI is enabled, the memory mapped virtio
> +device uses appropriate MSI interrupt message
> +for configuration change notification and used buffer notification which are
> +configured by \field{MsiAddrLow}, \field{MsoAddrHigh} and \field{MsiData}.
> +
> +If MSI is not enabled, the memory mapped virtio device
> +uses a single, dedicated
>  interrupt signal, which is asserted when at least one of the
>  bits described in the description of \field{InterruptStatus}
> -is set. This is how the device sends a used buffer notification
> -or a configuration change notification to the device.
> +is set.
>  
>  \drivernormative{\paragraph}{Notifications From The Device}{Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Notifications From The Device}
> -After receiving an interrupt, the driver MUST read
> -\field{InterruptStatus} to check what caused the interrupt (see the
> -register description).  The used buffer notification bit being set
> -SHOULD be interpreted as a used buffer notification for each active
> -virtqueue.  After the interrupt is handled, the driver MUST acknowledge
> -it by writing a bit mask corresponding to the handled events to the
> -InterruptACK register.
> +A driver MUST handle the case where MSI is disabled, which uses the same interrupt indicating both device configuration
> +space change and one or more virtqueues being used.
> +
> +\subsubsection{Driver Handling Interrupts}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Driver Handling Interrupts}
> +
> +The driver interrupt handler would typically:
> +
> +\begin{itemize}
> +  \item If MSI is enabled:
> +    \begin{itemize}
> +      \item
> +        Figure out the virtqueue mapped to that MSI vector for the
> +        device, to see if any progress has been made by the device
> +        which requires servicing.
> +      \item
> +        If the interrupt belongs to configuration space changing signal,
> +        re-examine the configuration space to see what changed.
> +    \end{itemize}
> +  \item If MSI is disabled:
> +    \begin{itemize}
> +      \item Read \field{InterruptStatus} to check what caused the interrupt.
> +      \item Acknowledge the interrupt by writing a bit mask corresponding
> +            to the handled events to the InterruptACK register.
> +    \end{itemize}
> +\end{itemize}
>  
>  \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over MMIO / Legacy interface}
>  
> diff --git a/msi-state.c b/msi-state.c
> new file mode 100644
> index 0000000..b1fa0c1
> --- /dev/null
> +++ b/msi-state.c
> @@ -0,0 +1,4 @@
> +le32 {
> +    msi_enabled : 1;
> +    reserved : 31;
> +};
> -- 
> 2.7.4
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [virtio-dev] [PATCH v2 5/5] virtio-mmio: MSI vector and event mapping
  2020-01-21 13:54 ` [virtio-dev] [PATCH v2 5/5] virtio-mmio: MSI vector and event mapping Jing Liu
@ 2020-01-29 10:14   ` Michael S. Tsirkin
  0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2020-01-29 10:14 UTC (permalink / raw)
  To: Jing Liu
  Cc: virtio-dev, linux-kernel, kvm, qemu-devel, Chao Peng, Liu Jiang, Zha Bin

On Tue, Jan 21, 2020 at 09:54:33PM +0800, Jing Liu wrote:
> Bit 1 msi_sharing reported in the MsiState register indicates the mapping mode
> device uses.
> 
> Bit 1 is 0 - device uses MSI non-sharing mode. This indicates vector per event and
> fixed static vectors and events relationship. This fits for devices with a high interrupt
> rate and best performance;
> Bit 1 is 1 - device uses MSI sharing mode. This indicates vectors and events
> dynamic mapping and fits for devices not requiring a high interrupt rate.

It seems that sharing mode is a superset of non-sharing mode.
Isn't that right? E.g. with sharing mode drivers
can still avoid sharing if they like.

Maybe it should just be a hint to drivers whether to share
interrupts, instead of a completely different layout?

> Co-developed-by: Chao Peng <chao.p.peng@linux.intel.com>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Co-developed-by: Liu Jiang <gerry@linux.alibaba.com>
> Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
> Co-developed-by: Zha Bin <zhabin@linux.alibaba.com>
> Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>
> Signed-off-by: Jing Liu <jing2.liu@linux.intel.com>
> ---
>  content.tex | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
>  msi-state.c |  3 ++-
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/content.tex b/content.tex
> index dcf6c71..2fd1686 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -1770,7 +1770,8 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi
>    \hline
>    \mmioreg{MsiState}{MSI state}{0x0c4}{R}{%
>      When VIRTIO_F_MMIO_MSI has been negotiated, reading
> -    from this register returns the global MSI enable/disable status.
> +    from this register returns the global MSI enable/disable status
> +    and whether device uses MSI sharing mode.
>      \lstinputlisting{msi-state.c}
>    }
>    \hline
> @@ -1926,12 +1927,18 @@ \subsubsection{Device Initialization}\label{sec:Virtio Transport Options / Virti
>  mask and unmask the MSI vector applying to the one selected by writing
>  to \field{MsiVecSel}.
>  
> +VIRTIO_MMIO_MSI_CMD_MAP_CONFIG command is to set the configuration event and MSI vector
> +mapping. VIRTIO_MMIO_MSI_CMD_MAP_QUEUE is to set the queue event and MSI vector
> +mapping. They SHOULD only be used in MSI sharing mode.
> +
>  \begin{lstlisting}
>  #define  VIRTIO_MMIO_MSI_CMD_ENABLE           0x1
>  #define  VIRTIO_MMIO_MSI_CMD_DISABLE          0x2
>  #define  VIRTIO_MMIO_MSI_CMD_CONFIGURE        0x3
>  #define  VIRTIO_MMIO_MSI_CMD_MASK             0x4
>  #define  VIRTIO_MMIO_MSI_CMD_UNMASK           0x5
> +#define  VIRTIO_MMIO_MSI_CMD_MAP_CONFIG       0x6
> +#define  VIRTIO_MMIO_MSI_CMD_MAP_QUEUE        0x7
>  \end{lstlisting}
>  
>  Setting a special NO_VECTOR value means disabling an interrupt for an event type.
> @@ -1941,10 +1948,49 @@ \subsubsection{Device Initialization}\label{sec:Virtio Transport Options / Virti
>  #define VIRTIO_MMIO_MSI_NO_VECTOR             0xffffffff
>  \end{lstlisting}
>  
> +\subparagraph{MSI Vector and Event Mapping}\label{sec:Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / Device Initialization / MSI Vector Configuration}
> +The reported \field{msi_sharing} bit in the \field{MsiState} return value shows
> +the MSI sharing mode that device uses.
> +
> +When \field{msi_sharing} bit is 0, it indicates the device uses non-sharing mode
> +and vector per event fixed static relationship is used. The first vector is for device
> +configuraiton change event, the second vector is for virtqueue 1, the third vector
> +is for virtqueue 2 and so on.
> +
> +When \field{msi_sharing} bit is 1, it indicates the device uses MSI sharing mode,
> +and the vector and event mapping is dynamic. Writing \field{MsiVecSel}
> +followed by writing VIRTIO_MMIO_MSI_CMD_MAP_CONFIG/VIRTIO_MMIO_MSI_CMD_MAP_QUEUE command
> +maps interrupts triggered by the configuration change/selected queue events respectively
> +to the corresponding MSI vector.
> +
> +\devicenormative{\subparagraph}{MSI Vector Configuration}{Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / MSI Vector Configuration}
> +
> +When the device reports \field{msi_sharing} bit as 0, it SHOULD support a number of
> +vectors that greater than the maximum number of virtqueues.
> +Device MUST report the number of vectors supported in \field{MsiVecNum}.
> +
> +When the device reports \field{msi_sharing} bit as 1, it SHOULD support at least
> +2 MSI vectors and MUST report in \field{MsiVecNum}. Device SHOULD support mapping any
> +event type to any vector under \field{MsiVecNum}.
> +
> +Device MUST support unmapping any event type (NO_VECTOR).
> +
> +The device SHOULD restrict the reported \field{msi_sharing} and \field{MsiVecNum}
> +to a value that might benefit system performance.
> +
> +\begin{note}
> +For example, a device which does not expect to send interrupts at a high rate might
> +return \field{msi_sharing} bit as 1.
> +\end{note}
> +
>  \drivernormative{\subparagraph}{MSI Vector Configuration}{Virtio Transport Options / Virtio Over MMIO / MMIO-specific Initialization And Device Operation / MSI Vector Configuration}
>  When VIRTIO_F_MMIO_MSI has been negotiated, driver should try to configure
>  and enable MSI.
>  
> +To set up the event and vector mapping for MSI sharing mode, driver SHOULD
> +write a valid \field{MsiVecSel} followed by VIRTIO_MMIO_MSI_CMD_MAP_CONFIG/VIRTIO_MMIO_MSI_CMD_MAP_QUEUE
> +command to map the configuration change/selected queue events respectively.
> +
>  To configure MSI vector, driver SHOULD firstly specify the MSI vector index by
>  writing to \field{MsiVecSel}.
>  Then notify the MSI address and data by writing to \field{MsiAddrLow}, \field{MsiAddrHigh},
> diff --git a/msi-state.c b/msi-state.c
> index b1fa0c1..d470be4 100644
> --- a/msi-state.c
> +++ b/msi-state.c
> @@ -1,4 +1,5 @@
>  le32 {
>      msi_enabled : 1;
> -    reserved : 31;
> +    msi_sharing: 1;
> +    reserved : 30;
>  };
> -- 
> 2.7.4
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-01-29 10:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-21 13:54 [virtio-dev] [PATCH v2 0/5] virtio-mmio enhancement Jing Liu
2020-01-21 13:54 ` [virtio-dev] [PATCH v2 1/5] virtio-mmio: Add feature bit for MMIO notification Jing Liu
2020-01-21 13:54 ` [virtio-dev] [PATCH v2 2/5] virtio-mmio: Enhance queue notification support Jing Liu
2020-01-21 13:54 ` [virtio-dev] [PATCH v2 3/5] virtio-mmio: Add feature bit for MMIO MSI Jing Liu
2020-01-21 13:54 ` [virtio-dev] [PATCH v2 4/5] virtio-mmio: Introduce MSI details Jing Liu
2020-01-29 10:12   ` Michael S. Tsirkin
2020-01-21 13:54 ` [virtio-dev] [PATCH v2 5/5] virtio-mmio: MSI vector and event mapping Jing Liu
2020-01-29 10:14   ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).