[PATCH v2 0/2] docs: driver-api: virtio: virtio on Linux

[PATCH v2 0/2] docs: driver-api: virtio: virtio on Linux

[Ricardo Cañuelo]

Basic documentation about virtio in the kernel and a small tutorial for
virtio drivers.

Tested on linux-next (next-20220802)

Changes in v2:
  - virtio spec links updated to v1.2
  - simplify virtio.rst and remove most low level parts that are
    already covered by the spec
  - split the kerneldocs fixes to a separate patch
  - remove :c:func: rst formatting

Ricardo Cañuelo (2):
  virtio: kerneldocs fixes and enhancements
  docs: driver-api: virtio: virtio on Linux

 Documentation/driver-api/index.rst            |   1 +
 Documentation/driver-api/virtio/index.rst     |  11 +
 Documentation/driver-api/virtio/virtio.rst    | 151 ++++++++++++++
 .../virtio/writing_virtio_drivers.rst         | 189 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 drivers/virtio/virtio_ring.c                  |   8 +
 include/linux/virtio.h                        |   6 +-
 include/linux/virtio_config.h                 |   6 +-
 include/uapi/linux/virtio_ring.h              |  16 +-
 9 files changed, 378 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/driver-api/virtio/index.rst
 create mode 100644 Documentation/driver-api/virtio/virtio.rst
 create mode 100644 Documentation/driver-api/virtio/writing_virtio_drivers.rst

-- 
2.25.1

[PATCH v2 1/2] virtio: kerneldocs fixes and enhancements

[Ricardo Cañuelo]

Fix variable names in some htmldocs, naming in others.
Add htmldocs for struct vring_desc and vring_interrupt.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 drivers/virtio/virtio_ring.c     |  8 ++++++++
 include/linux/virtio.h           |  6 +++---
 include/linux/virtio_config.h    |  6 +++---
 include/uapi/linux/virtio_ring.h | 16 +++++++++++-----
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index a5ec724c01d8..e2091345c5c2 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2147,6 +2147,14 @@ static inline bool more_used(const struct vring_virtqueue *vq)
 	return vq->packed_ring ? more_used_packed(vq) : more_used_split(vq);
 }
 
+/**
+ * vring_interrupt - notify a virtqueue on an interrupt
+ * @irq: the IRQ number
+ * @_vq: the struct virtqueue to notify
+ *
+ * Calls the callback function of @_vq to process the virtqueue
+ * notification.
+ */
 irqreturn_t vring_interrupt(int irq, void *_vq)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index d8fdf170637c..fd8440e85933 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -11,7 +11,7 @@
 #include <linux/gfp.h>
 
 /**
- * virtqueue - a queue to register buffers for sending or receiving.
+ * struct virtqueue - a queue to register buffers for sending or receiving.
  * @list: the chain of virtqueues for this device
  * @callback: the function to call when buffers are consumed (can be NULL).
  * @name: the name of this virtqueue (mainly for debugging)
@@ -90,7 +90,7 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq);
 dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq);
 
 /**
- * virtio_device - representation of a device using virtio
+ * struct virtio_device - representation of a device using virtio
  * @index: unique position on the virtio bus
  * @failed: saved value for VIRTIO_CONFIG_S_FAILED bit (for restore)
  * @config_enabled: configuration change reporting enabled
@@ -146,7 +146,7 @@ size_t virtio_max_dma_size(struct virtio_device *vdev);
 	list_for_each_entry(vq, &vdev->vqs, list)
 
 /**
- * virtio_driver - operations for a virtio I/O driver
+ * struct virtio_driver - operations for a virtio I/O driver
  * @driver: underlying device driver (populate name and owner).
  * @id_table: the ids serviced by this driver.
  * @feature_table: an array of feature numbers supported by this driver.
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index b47c2e7ed0ee..997801018ae4 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -225,7 +225,7 @@ int virtio_find_vqs_ctx(struct virtio_device *vdev, unsigned nvqs,
 
 /**
  * virtio_synchronize_cbs - synchronize with virtqueue callbacks
- * @vdev: the device
+ * @dev: the device
  */
 static inline
 void virtio_synchronize_cbs(struct virtio_device *dev)
@@ -244,7 +244,7 @@ void virtio_synchronize_cbs(struct virtio_device *dev)
 
 /**
  * virtio_device_ready - enable vq use in probe function
- * @vdev: the device
+ * @dev: the device
  *
  * Driver must call this to use vqs in the probe function.
  *
@@ -292,7 +292,7 @@ const char *virtio_bus_name(struct virtio_device *vdev)
 /**
  * virtqueue_set_affinity - setting affinity for a virtqueue
  * @vq: the virtqueue
- * @cpu: the cpu no.
+ * @cpu_mask: the cpu mask
  *
  * Pay attention the function are best-effort: the affinity hint may not be set
  * due to config support, irq type and sharing.
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 476d3e5c0fe7..f8c20d3de8da 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -93,15 +93,21 @@
 #define VRING_USED_ALIGN_SIZE 4
 #define VRING_DESC_ALIGN_SIZE 16
 
-/* Virtio ring descriptors: 16 bytes.  These can chain together via "next". */
+/**
+ * struct vring_desc - Virtio ring descriptors,
+ * 16 bytes long. These can chain together via @next.
+ *
+ * @addr: buffer address (guest-physical)
+ * @len: buffer length
+ * @flags: descriptor flags
+ * @next: index of the next descriptor in the chain,
+ *        if the VRING_DESC_F_NEXT flag is set. We chain unused
+ *        descriptors via this, too.
+ */
 struct vring_desc {
-	/* Address (guest-physical). */
 	__virtio64 addr;
-	/* Length. */
 	__virtio32 len;
-	/* The flags as indicated above. */
 	__virtio16 flags;
-	/* We chain unused descriptors via this, too */
 	__virtio16 next;
 };
 
-- 
2.25.1

[PATCH v2 2/2] docs: driver-api: virtio: virtio on Linux

[Ricardo Cañuelo]

Basic doc about Virtio on Linux and a short tutorial on Virtio drivers.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
 Documentation/driver-api/index.rst            |   1 +
 Documentation/driver-api/virtio/index.rst     |  11 +
 Documentation/driver-api/virtio/virtio.rst    | 151 ++++++++++++++
 .../virtio/writing_virtio_drivers.rst         | 189 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 5 files changed, 353 insertions(+)
 create mode 100644 Documentation/driver-api/virtio/index.rst
 create mode 100644 Documentation/driver-api/virtio/virtio.rst
 create mode 100644 Documentation/driver-api/virtio/writing_virtio_drivers.rst

diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
index d3a58f77328e..30a3de452b1d 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -106,6 +106,7 @@ available subsections can be seen below.
    vfio-mediated-device
    vfio
    vfio-pci-device-specific-driver-acceptance
+   virtio/index
    xilinx/index
    xillybus
    zorro
diff --git a/Documentation/driver-api/virtio/index.rst b/Documentation/driver-api/virtio/index.rst
new file mode 100644
index 000000000000..528b14b291e3
--- /dev/null
+++ b/Documentation/driver-api/virtio/index.rst
@@ -0,0 +1,11 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+======
+Virtio
+======
+
+.. toctree::
+   :maxdepth: 1
+
+   virtio
+   writing_virtio_drivers
diff --git a/Documentation/driver-api/virtio/virtio.rst b/Documentation/driver-api/virtio/virtio.rst
new file mode 100644
index 000000000000..4b73c705c94c
--- /dev/null
+++ b/Documentation/driver-api/virtio/virtio.rst
@@ -0,0 +1,151 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _virtio:
+
+===============
+Virtio on Linux
+===============
+
+Introduction
+============
+
+Virtio is an open standard interface for virtual machines to access
+paravirtualized devices, ie. devices that aren't emulated by a
+hypervisor but rather real host devices that are exposed by the
+hypervisor to the guest to achieve native performance. In other words,
+it provides a communication mechanism for a guest OS to use devices on
+the host machine. Additionally, some devices also implement the virtio
+interface in hardware.
+
+For paravirtualized devices, the concrete hardware details of the real
+host devices are abstracted in the hypervisor, which provides a set of
+simplified virtual devices that implement the virtio protocol. These
+devices are defined in Chapter 5 ("Device Types") of the virtio spec [1]
+and they're the devices that the guest OS will ultimately handle. So, in
+that regard, the guest OS knows it's running in a virtual environment
+and that it needs to use the appropriate virtio drivers to handle the
+devices instead of the regular device drivers it'd use in a native or
+purely virtual environment (with emulated devices).
+
+
+Device - Driver communication: virtqueues
+=========================================
+
+Although the virtio devices are really an abstraction layer in the
+hypervisor, they're exposed to the guest as if they are physical devices
+using a specific transport method -- PCI, MMIO or CCW -- that is
+orthogonal to the device itself. The virtio spec defines these transport
+methods in detail, including device discovery, capabilities and
+interrupt handling.
+
+The communication between the driver in the guest OS and the device in
+the hypervisor is done through shared memory (that's what makes virtio
+devices so efficient) using specialized data structures called
+virtqueues, which are actually ring buffers [#f1]_ of buffer descriptors
+similar to the ones used in a network device:
+
+.. kernel-doc:: include/uapi/linux/virtio_ring.h
+    :identifiers: struct vring_desc
+
+All the buffers the descriptors point to are allocated by the guest and
+used by the host either for reading or for writing but not for both.
+
+Refer to Chapter 2.5 ("Virtqueues") of the virtio spec [1] for the
+reference definitions of virtqueues and to [2] for an illustrated
+overview of how the host device and the guest driver communicate.
+
+The :c:type:`vring_virtqueue` struct models a virtqueue, including the
+ring buffers and management data. Embedded in this struct is the
+:c:type:`virtqueue` struct, which is the data structure that's
+ultimately used by virtio drivers:
+
+.. kernel-doc:: include/linux/virtio.h
+    :identifiers: struct virtqueue
+
+The callback function pointed by this struct is triggered when the
+device has consumed the buffers provided by the driver. More
+specifically, the trigger will be an interrupt issued by the hypervisor
+(see vring_interrupt()). Interrupt request handlers are registered for
+a virtqueue during the virtqueue setup process (transport-specific).
+
+.. kernel-doc:: drivers/virtio/virtio_ring.c
+    :identifiers: vring_interrupt
+
+
+Device discovery and probing
+============================
+
+In the kernel, the virtio core contains the virtio bus driver and
+transport-specific drivers like `virtio-pci` and `virtio-mmio`. Then
+there are individual virtio drivers for specific device types that are
+registered to the virtio bus driver.
+
+How a virtio device is found and configured by the kernel depends on how
+the hypervisor defines it. Taking the `QEMU virtio-console
+<https://gitlab.com/qemu-project/qemu/-/blob/master/hw/char/virtio-console.c>`__
+device as an example. When using PCI as a transport method, the device
+will present itself in the PCI bus with vendor 0x1af4 (RedHat, Inc.) and
+device id 0x1003 (virtio console), as defined in the spec, so the kernel
+will detect it as it would do with any other PCI device.
+
+During the PCI enumeration process, if a device is found to match the
+virtio-pci driver (according to the virtio-pci device table, any PCI
+device with vendor id = 0x1af4)::
+
+	/* Qumranet donated their vendor ID for devices 0x1000 thru 0x10FF. */
+	static const struct pci_device_id virtio_pci_id_table[] = {
+		{ PCI_DEVICE(PCI_VENDOR_ID_REDHAT_QUMRANET, PCI_ANY_ID) },
+		{ 0 }
+	};
+
+then the virtio-pci driver is probed and, if the probing goes well, the
+device is registered to the virtio bus::
+
+	static int virtio_pci_probe(struct pci_dev *pci_dev,
+				    const struct pci_device_id *id)
+	{
+		...
+
+		if (force_legacy) {
+			rc = virtio_pci_legacy_probe(vp_dev);
+			/* Also try modern mode if we can't map BAR0 (no IO space). */
+			if (rc == -ENODEV || rc == -ENOMEM)
+				rc = virtio_pci_modern_probe(vp_dev);
+			if (rc)
+				goto err_probe;
+		} else {
+			rc = virtio_pci_modern_probe(vp_dev);
+			if (rc == -ENODEV)
+				rc = virtio_pci_legacy_probe(vp_dev);
+			if (rc)
+				goto err_probe;
+		}
+
+		...
+
+		rc = register_virtio_device(&vp_dev->vdev);
+
+When the device is registered to the virtio bus the kernel will look
+for a driver in the bus that can handle the device and call that
+driver's ``probe`` method.
+
+It's at this stage that the virtqueues will be allocated and configured
+by calling the appropriate ``virtio_find`` helper function, such as
+virtio_find_single_vq() or virtio_find_vqs(), which will end up
+calling a transport-specific ``find_vqs`` method.
+
+
+References
+==========
+
+[1] Virtio Spec v1.2:
+https://docs.oasis-open.org/virtio/virtio/v1.2/virtio-v1.2.html
+
+Check for later versions of the spec as well.
+
+[2] Virtqueues and virtio ring: How the data travels
+https://www.redhat.com/en/blog/virtqueues-and-virtio-ring-how-data-travels
+
+.. rubric:: Footnotes
+
+.. [#f1] that's why they may be also referred as virtrings.
diff --git a/Documentation/driver-api/virtio/writing_virtio_drivers.rst b/Documentation/driver-api/virtio/writing_virtio_drivers.rst
new file mode 100644
index 000000000000..139c785a38ef
--- /dev/null
+++ b/Documentation/driver-api/virtio/writing_virtio_drivers.rst
@@ -0,0 +1,189 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _writing_virtio_drivers:
+
+======================
+Writing Virtio Drivers
+======================
+
+Introduction
+============
+
+Chapter 5 ("Device Types") in the virtio specification [1] defines all
+the supported virtio device types. Since these devices are, by
+definition, meant as abstractions for a wide variety of real hardware,
+the addition of new virtio drivers is not expected to be very
+frequent. Still, this document serves as a basic guideline for driver
+programmers that need to hack a new virtio driver or understand the
+essentials of the existing ones. See :ref:`Virtio on Linux <virtio>` for
+a general overview of virtio.
+
+
+Driver boilerplate
+==================
+
+As a bare minimum, a virtio driver should register in the virtio bus and
+configure the virtqueues for the device according to its spec, the
+configuration of the virtqueues in the driver side must match the
+virtqueue definitions in the device. A basic driver skeleton could look
+like this::
+
+	#include <linux/virtio.h>
+	#include <linux/virtio_ids.h>
+	#include <linux/virtio_config.h>
+	#include <linux/module.h>
+
+	/* device private data (one per device) */
+	struct virtio_dummy_dev {
+		struct virtqueue *vq;
+	};
+
+	static void virtio_dummy_recv_cb(struct virtqueue *vq)
+	{
+		struct virtio_dummy_dev *dev = vq->vdev->priv;
+		char *buf;
+		unsigned int len;
+
+		buf = virtqueue_get_buf(dev->vq, &len);
+		/* spurious callback? */
+		if (!buf)
+			return;
+
+		/* Process the received data */
+	}
+
+	static int virtio_dummy_probe(struct virtio_device *vdev)
+	{
+		struct virtio_dummy_dev *dev = NULL;
+
+		/* initialize device data */
+		dev = kzalloc(sizeof(struct virtio_dummy_dev), GFP_KERNEL);
+		if (!dev)
+			return -ENOMEM;
+
+		/* the device has a single virtqueue */
+		dev->vq = virtio_find_single_vq(vdev, virtio_dummy_recv_cb, "input");
+		if (IS_ERR(dev->vq)) {
+			kfree(dev);
+			return PTR_ERR(dev->vq);
+
+		}
+		vdev->priv = dev;
+
+		return 0;
+	}
+
+	static void virtio_dummy_remove(struct virtio_device *vdev)
+	{
+		struct virtio_dummy_dev *dev = vdev->priv;
+
+		/*
+		 * Disable vq interrupts: equivalent to
+		 * vdev->config->reset(vdev)
+		 */
+		virtio_reset_device(vdev);
+
+		/* remove virtqueues */
+		vdev->config->del_vqs(vdev);
+
+		kfree(dev);
+	}
+
+	static const struct virtio_device_id id_table[] = {
+		{ VIRTIO_ID_DUMMY, VIRTIO_DEV_ANY_ID },
+		{ 0 },
+	};
+
+	static struct virtio_driver virtio_dummy_driver = {
+		.driver.name =	KBUILD_MODNAME,
+		.driver.owner =	THIS_MODULE,
+		.id_table =	id_table,
+		.probe =	virtio_dummy_probe,
+		.remove =	virtio_dummy_remove,
+	};
+
+	module_virtio_driver(virtio_dummy_driver);
+	MODULE_DEVICE_TABLE(virtio, id_table);
+	MODULE_DESCRIPTION("Dummy virtio driver");
+	MODULE_LICENSE("GPL");
+
+The device id ``VIRTIO_ID_DUMMY`` here is a placeholder, virtio
+drivers should be defined only for devices that are defined in the
+spec. See include/uapi/linux/virtio_ids.h.
+
+If your driver doesn't have to do anything special in its ``init`` and
+``exit`` methods, you can use the module_virtio_driver() helper to
+reduce the amount of boilerplate code.
+
+The ``probe`` method does the minimum driver setup in this case
+(memory allocation for the device data) and initializes the
+virtqueue. The virtqueues are automatically enabled after ``probe``
+returns, sending the appropriate "DRIVER_OK" status signal to the
+device. If the virtqueues need to be enabled before ``probe`` ends, they
+can be manually enabled by calling virtio_device_ready():
+
+.. kernel-doc:: include/linux/virtio_config.h
+    :identifiers: virtio_device_ready
+
+
+Sending and receiving data
+==========================
+
+The virtio_dummy_recv_cb() callback in the code above will be triggered
+when the device notifies the driver after it finishes processing a
+descriptor or descriptor chain, either for reading or writing. However,
+that's only the second half of the virtio device-driver communication
+process, as the communication is always started by the driver regardless
+of the direction of the data transfer.
+
+To configure a buffer transfer from the driver to the device, first you
+have to add the buffers -- packed as `scatterlists` -- to the
+appropriate virtqueue using any of the virtqueue_add_inbuf(),
+virtqueue_add_outbuf() or virtqueue_add_sgs(), depending on whether you
+need to add one input `scatterlist` (for the device to fill in), one
+output `scatterlist` (for the device to consume) or multiple
+`scatterlists`, respectively. Then, once the virtqueue is set up, a call
+to virtqueue_kick() sends a notification that will be serviced by the
+hypervisor that implements the device::
+
+	struct scatterlist sg[1];
+	sg_init_one(sg, buffer, BUFLEN);
+	virtqueue_add_inbuf(dev->vq, sg, 1, buffer, GFP_ATOMIC);
+	virtqueue_kick(dev->vq);
+
+.. kernel-doc:: drivers/virtio/virtio_ring.c
+    :identifiers: virtqueue_add_inbuf
+
+.. kernel-doc:: drivers/virtio/virtio_ring.c
+    :identifiers: virtqueue_add_outbuf
+
+.. kernel-doc:: drivers/virtio/virtio_ring.c
+    :identifiers: virtqueue_add_sgs
+
+Then, after the device has read or written the buffers prepared by the
+driver and notifies it back, the driver can call virtqueue_get_buf() to
+read the data produced by the device (if the virtqueue was set up with
+input buffers) or simply to reclaim the buffers if they were already
+consumed by the device:
+
+.. kernel-doc:: drivers/virtio/virtio_ring.c
+    :identifiers: virtqueue_get_buf_ctx
+
+The virtqueue callbacks can be disabled and re-enabled using the
+virtqueue_disable_cb() and the family of virtqueue_enable_cb() functions
+respectively. See drivers/virtio/virtio_ring.c for more details:
+
+.. kernel-doc:: drivers/virtio/virtio_ring.c
+    :identifiers: virtqueue_disable_cb
+
+.. kernel-doc:: drivers/virtio/virtio_ring.c
+    :identifiers: virtqueue_enable_cb
+
+
+References
+==========
+
+[1] Virtio Spec v1.2:
+https://docs.oasis-open.org/virtio/virtio/v1.2/virtio-v1.2.html
+
+Check for later versions of the spec as well.
diff --git a/MAINTAINERS b/MAINTAINERS
index 779f599f9abf..6ecdddb89da4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21488,6 +21488,7 @@ S:	Maintained
 F:	Documentation/ABI/testing/sysfs-bus-vdpa
 F:	Documentation/ABI/testing/sysfs-class-vduse
 F:	Documentation/devicetree/bindings/virtio/
+F:	Documentation/driver-api/virtio/
 F:	drivers/block/virtio_blk.c
 F:	drivers/crypto/virtio/
 F:	drivers/net/virtio_net.c
-- 
2.25.1

Re: [PATCH v2 2/2] docs: driver-api: virtio: virtio on Linux

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 2253 bytes --]

On Thu, Aug 04, 2022 at 12:59:14PM +0200, Ricardo Cañuelo wrote:
> +References
> +==========
> +
> +[1] Virtio Spec v1.2:
> +https://docs.oasis-open.org/virtio/virtio/v1.2/virtio-v1.2.html
> +
> +Check for later versions of the spec as well.
> +
> +[2] Virtqueues and virtio ring: How the data travels
> +https://www.redhat.com/en/blog/virtqueues-and-virtio-ring-how-data-travels
> +
> +.. rubric:: Footnotes
> +
> +.. [#f1] that's why they may be also referred as virtrings.

Sphinx citation syntax can be used for external references, like:

diff --git a/Documentation/driver-api/virtio/virtio.rst b/Documentation/driver-api/virtio/virtio.rst
index 4b73c705c94c61..abd682cfd41eda 100644
--- a/Documentation/driver-api/virtio/virtio.rst
+++ b/Documentation/driver-api/virtio/virtio.rst
@@ -50,8 +50,8 @@ similar to the ones used in a network device:
 All the buffers the descriptors point to are allocated by the guest and
 used by the host either for reading or for writing but not for both.
 
-Refer to Chapter 2.5 ("Virtqueues") of the virtio spec [1] for the
-reference definitions of virtqueues and to [2] for an illustrated
+Refer to Chapter 2.5 ("Virtqueues") of the virtio spec [1]_ for the
+reference definitions of virtqueues and to [2]_ for an illustrated
 overview of how the host device and the guest driver communicate.
 
 The :c:type:`vring_virtqueue` struct models a virtqueue, including the
@@ -138,13 +138,13 @@ calling a transport-specific ``find_vqs`` method.
 References
 ==========
 
-[1] Virtio Spec v1.2:
-https://docs.oasis-open.org/virtio/virtio/v1.2/virtio-v1.2.html
+.. [1] Virtio Spec v1.2:
+   https://docs.oasis-open.org/virtio/virtio/v1.2/virtio-v1.2.html
 
-Check for later versions of the spec as well.
+   Check for later versions of the spec as well.
 
-[2] Virtqueues and virtio ring: How the data travels
-https://www.redhat.com/en/blog/virtqueues-and-virtio-ring-how-data-travels
+.. [2] Virtqueues and virtio ring: How the data travels
+   https://www.redhat.com/en/blog/virtqueues-and-virtio-ring-how-data-travels
 
 .. rubric:: Footnotes
 

Otherwise the documentation LGTM (no new warnings).

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v2 2/2] docs: driver-api: virtio: virtio on Linux

[Ricardo Cañuelo]

Hi Bagas,

Thanks for reviewing the patch, comments below:

On sáb, ago 06 2022 at 14:58:08, Bagas Sanjaya <bagasdotme@gmail.com> wrote:
> Sphinx citation syntax can be used for external references, like:
>
> diff --git a/Documentation/driver-api/virtio/virtio.rst b/Documentation/driver-api/virtio/virtio.rst
> index 4b73c705c94c61..abd682cfd41eda 100644
> --- a/Documentation/driver-api/virtio/virtio.rst
> +++ b/Documentation/driver-api/virtio/virtio.rst
> @@ -50,8 +50,8 @@ similar to the ones used in a network device:
>  All the buffers the descriptors point to are allocated by the guest and
>  used by the host either for reading or for writing but not for both.
>  
> -Refer to Chapter 2.5 ("Virtqueues") of the virtio spec [1] for the
> -reference definitions of virtqueues and to [2] for an illustrated
> +Refer to Chapter 2.5 ("Virtqueues") of the virtio spec [1]_ for the
> +reference definitions of virtqueues and to [2]_ for an illustrated
>  overview of how the host device and the guest driver communicate.
>  
>  The :c:type:`vring_virtqueue` struct models a virtqueue, including the
> @@ -138,13 +138,13 @@ calling a transport-specific ``find_vqs`` method.
>  References
>  ==========
>  
> -[1] Virtio Spec v1.2:
> -https://docs.oasis-open.org/virtio/virtio/v1.2/virtio-v1.2.html
> +.. [1] Virtio Spec v1.2:
> +   https://docs.oasis-open.org/virtio/virtio/v1.2/virtio-v1.2.html
>  
> -Check for later versions of the spec as well.
> +   Check for later versions of the spec as well.
>  
> -[2] Virtqueues and virtio ring: How the data travels
> -https://www.redhat.com/en/blog/virtqueues-and-virtio-ring-how-data-travels
> +.. [2] Virtqueues and virtio ring: How the data travels
> +   https://www.redhat.com/en/blog/virtqueues-and-virtio-ring-how-data-travels
>  
>  .. rubric:: Footnotes

Is that the preferred way to do it? I didn't find any guidelines about
it and the existing docs don't seem to settle on any specific
style. Personally I prefer to keep it as it is in the patch because I
like the bibliography references to look different than footnote links
([] vs superscript).

Cheers,
Ricardo

Re: [PATCH v2 1/2] virtio: kerneldocs fixes and enhancements

[Cornelia Huck]

On Thu, Aug 04 2022, Ricardo Cañuelo <ricardo.canuelo@collabora.com> wrote:

> Fix variable names in some htmldocs, naming in others.
> Add htmldocs for struct vring_desc and vring_interrupt.

Isn't that "kerneldoc"? But maybe I'm a bit behind on the current
terminology.

>
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  drivers/virtio/virtio_ring.c     |  8 ++++++++
>  include/linux/virtio.h           |  6 +++---
>  include/linux/virtio_config.h    |  6 +++---
>  include/uapi/linux/virtio_ring.h | 16 +++++++++++-----
>  4 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index a5ec724c01d8..e2091345c5c2 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -2147,6 +2147,14 @@ static inline bool more_used(const struct vring_virtqueue *vq)
>  	return vq->packed_ring ? more_used_packed(vq) : more_used_split(vq);
>  }
>  
> +/**
> + * vring_interrupt - notify a virtqueue on an interrupt
> + * @irq: the IRQ number

This is being ignored, however. Append "(ignored)"?

(I think it is only there so that this function can be used as an
irq_handler_t.)

> + * @_vq: the struct virtqueue to notify
> + *
> + * Calls the callback function of @_vq to process the virtqueue
> + * notification.
> + */
>  irqreturn_t vring_interrupt(int irq, void *_vq)
>  {
>  	struct vring_virtqueue *vq = to_vvq(_vq);

(...)

> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index b47c2e7ed0ee..997801018ae4 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -225,7 +225,7 @@ int virtio_find_vqs_ctx(struct virtio_device *vdev, unsigned nvqs,
>  
>  /**
>   * virtio_synchronize_cbs - synchronize with virtqueue callbacks
> - * @vdev: the device
> + * @dev: the device

As you're touching this anyway: maybe s/device/virtio device/ ?

>   */
>  static inline
>  void virtio_synchronize_cbs(struct virtio_device *dev)
> @@ -244,7 +244,7 @@ void virtio_synchronize_cbs(struct virtio_device *dev)
>  
>  /**
>   * virtio_device_ready - enable vq use in probe function
> - * @vdev: the device
> + * @dev: the device

Same here.

>   *
>   * Driver must call this to use vqs in the probe function.
>   *

Re: [PATCH v2 2/2] docs: driver-api: virtio: virtio on Linux

[Bagas Sanjaya]

On 8/8/22 16:06, Ricardo Cañuelo wrote:
>> -[1] Virtio Spec v1.2:
>> -https://docs.oasis-open.org/virtio/virtio/v1.2/virtio-v1.2.html
>> +.. [1] Virtio Spec v1.2:
>> +   https://docs.oasis-open.org/virtio/virtio/v1.2/virtio-v1.2.html
>>  
>> -Check for later versions of the spec as well.
>> +   Check for later versions of the spec as well.
>>  
>> -[2] Virtqueues and virtio ring: How the data travels
>> -https://www.redhat.com/en/blog/virtqueues-and-virtio-ring-how-data-travels
>> +.. [2] Virtqueues and virtio ring: How the data travels
>> +   https://www.redhat.com/en/blog/virtqueues-and-virtio-ring-how-data-travels
>>  
>>  .. rubric:: Footnotes
> 
> Is that the preferred way to do it? I didn't find any guidelines about
> it and the existing docs don't seem to settle on any specific
> style. Personally I prefer to keep it as it is in the patch because I
> like the bibliography references to look different than footnote links
> ([] vs superscript).
> 

I think the citation syntax is better suited for this case of external
references mentioned in a documentation.

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH v2 1/2] virtio: kerneldocs fixes and enhancements

[Ricardo Cañuelo]

Hi Cornelia, thanks for the review:

On lun, ago 08 2022 at 14:14:07, Cornelia Huck <cohuck@redhat.com> wrote:
> Isn't that "kerneldoc"? But maybe I'm a bit behind on the current
> terminology.

Ugh what a silly slip. You're totally right, I got kerneldocs and
htmldocs mixed in my head after so many "make htmldocs" tests.

> This is being ignored, however. Append "(ignored)"?

Ack.

> As you're touching this anyway: maybe s/device/virtio device/ ?
> ...
> Same here.

Sure, no problem. I'll prepare v3 with the fixes.

Cheers,
Ricardo

Re: [PATCH v2 2/2] docs: driver-api: virtio: virtio on Linux

[Cornelia Huck]

On Thu, Aug 04 2022, Ricardo Cañuelo <ricardo.canuelo@collabora.com> wrote:

> Basic doc about Virtio on Linux and a short tutorial on Virtio drivers.
>
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  Documentation/driver-api/index.rst            |   1 +
>  Documentation/driver-api/virtio/index.rst     |  11 +
>  Documentation/driver-api/virtio/virtio.rst    | 151 ++++++++++++++
>  .../virtio/writing_virtio_drivers.rst         | 189 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  5 files changed, 353 insertions(+)
>  create mode 100644 Documentation/driver-api/virtio/index.rst
>  create mode 100644 Documentation/driver-api/virtio/virtio.rst
>  create mode 100644 Documentation/driver-api/virtio/writing_virtio_drivers.rst

(...)

> diff --git a/Documentation/driver-api/virtio/virtio.rst b/Documentation/driver-api/virtio/virtio.rst
> new file mode 100644
> index 000000000000..4b73c705c94c
> --- /dev/null
> +++ b/Documentation/driver-api/virtio/virtio.rst
> @@ -0,0 +1,151 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _virtio:
> +
> +===============
> +Virtio on Linux
> +===============
> +
> +Introduction
> +============
> +
> +Virtio is an open standard interface for virtual machines to access
> +paravirtualized devices, ie. devices that aren't emulated by a
> +hypervisor but rather real host devices that are exposed by the
> +hypervisor to the guest to achieve native performance. In other words,
> +it provides a communication mechanism for a guest OS to use devices on
> +the host machine. Additionally, some devices also implement the virtio
> +interface in hardware.
> +
> +For paravirtualized devices, the concrete hardware details of the real
> +host devices are abstracted in the hypervisor, which provides a set of
> +simplified virtual devices that implement the virtio protocol. These
> +devices are defined in Chapter 5 ("Device Types") of the virtio spec [1]
> +and they're the devices that the guest OS will ultimately handle. So, in
> +that regard, the guest OS knows it's running in a virtual environment
> +and that it needs to use the appropriate virtio drivers to handle the
> +devices instead of the regular device drivers it'd use in a native or
> +purely virtual environment (with emulated devices).

Hm... so I'm not quite happy with those two paragraphs, but I'm not sure
how to make it better.

- While the origins of virtio are hypervisor implementations, the
  standard is describing the mechanisms of device <-> driver
  communication, regardless whether the device is a hardware entity or
  something emulated by a hypervisor.
- I'm not quite sure what you are referring to with "real host
  devices". We can have e.g. a real disk that is handed to a guest via
  virtio-blk as a whole, or we can have some kind of file that is
  exposed via virtio-blk. Other device types can also be completely
  emulated.
- The OS picks the driver depending on what device is discovers; virtio
  device drivers are not any different from other device drivers in that
  regard.

So I think the key pieces of virtio are the following:

- it is an open standard
- it describes a protocol, which can be used to implement a lot of
  different device types
- those devices are exposed to the operating system via standard
  mechanisms such as PCI
- virtio devices can be implemented in various ways, such as in
  hypervisors (more common) or as a real hardware device

For the remainder of this document, it is probably fine to focus on the
hypervisor use case.

(...)

> +How a virtio device is found and configured by the kernel depends on how
> +the hypervisor defines it. Taking the `QEMU virtio-console
> +<https://gitlab.com/qemu-project/qemu/-/blob/master/hw/char/virtio-console.c>`__
> +device as an example. When using PCI as a transport method, the device
> +will present itself in the PCI bus with vendor 0x1af4 (RedHat, Inc.) and

s/in/on/ ?
s/RedHat/Red Hat/ :)

> +device id 0x1003 (virtio console), as defined in the spec, so the kernel
> +will detect it as it would do with any other PCI device.

(...)

> diff --git a/Documentation/driver-api/virtio/writing_virtio_drivers.rst b/Documentation/driver-api/virtio/writing_virtio_drivers.rst
> new file mode 100644
> index 000000000000..139c785a38ef
> --- /dev/null
> +++ b/Documentation/driver-api/virtio/writing_virtio_drivers.rst
> @@ -0,0 +1,189 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _writing_virtio_drivers:
> +
> +======================
> +Writing Virtio Drivers
> +======================
> +
> +Introduction
> +============
> +
> +Chapter 5 ("Device Types") in the virtio specification [1] defines all
> +the supported virtio device types. Since these devices are, by
> +definition, meant as abstractions for a wide variety of real hardware,

See my comments above, virtio devices can also be fully emulated or real
hardware devices.

> +the addition of new virtio drivers is not expected to be very
> +frequent. Still, this document serves as a basic guideline for driver

I think we should not make any statement regarding frequency of new
additions; sometimes, there's a flurry of activity, sometimes, there's
nothing for ages :)

> +programmers that need to hack a new virtio driver or understand the
> +essentials of the existing ones. See :ref:`Virtio on Linux <virtio>` for
> +a general overview of virtio.
> +
> +
> +Driver boilerplate
> +==================
> +
> +As a bare minimum, a virtio driver should register in the virtio bus and

s/should/needs to/ ?

> +configure the virtqueues for the device according to its spec, the
> +configuration of the virtqueues in the driver side must match the
> +virtqueue definitions in the device. A basic driver skeleton could look
> +like this::

(...)

> +The device id ``VIRTIO_ID_DUMMY`` here is a placeholder, virtio
> +drivers should be defined only for devices that are defined in the

s/defined/added/ ?

> +spec. See include/uapi/linux/virtio_ids.h.

Maybe "Device ids need to be at least reserved in the virtio spec before
being added to that file." ?

Re: [PATCH v2 2/2] docs: driver-api: virtio: virtio on Linux

[Ricardo Cañuelo]

Hi Cornelia,

Thanks for the insightful comments and ideas. Answers below:

On lun, ago 08 2022 at 15:47:06, Cornelia Huck <cohuck@redhat.com> wrote:
> Hm... so I'm not quite happy with those two paragraphs, but I'm not sure
> how to make it better.
>
> - While the origins of virtio are hypervisor implementations, the
>   standard is describing the mechanisms of device <-> driver
>   communication, regardless whether the device is a hardware entity or
>   something emulated by a hypervisor.
> - I'm not quite sure what you are referring to with "real host
>   devices". We can have e.g. a real disk that is handed to a guest via
>   virtio-blk as a whole, or we can have some kind of file that is
>   exposed via virtio-blk. Other device types can also be completely
>   emulated.
> - The OS picks the driver depending on what device is discovers; virtio
>   device drivers are not any different from other device drivers in that
>   regard.
>
> So I think the key pieces of virtio are the following:
>
> - it is an open standard
> - it describes a protocol, which can be used to implement a lot of
>   different device types
> - those devices are exposed to the operating system via standard
>   mechanisms such as PCI
> - virtio devices can be implemented in various ways, such as in
>   hypervisors (more common) or as a real hardware device
>
> For the remainder of this document, it is probably fine to focus on the
> hypervisor use case.

How about this as an introduction?

    Virtio is an open standard that defines a protocol to communicate
    drivers and devices of different types, see Chapter 5 ("Device
    Types") of the virtio spec `[1]`_. Originally developed as a
    standard for paravirtualized devices implemented by a hypervisor, it
    can be used to interface any compliant device (real or emulated)
    with a driver.

    For illustrative purposes, this document will focus on the common
    case of a Linux kernel running in a virtual machine and using
    paravirtualized devices provided by the hypervisor, which exposes
    them as virtio devices via standard mechanisms such as PCI.

> s/in/on/ ?
> s/RedHat/Red Hat/ :)

Got it.

> See my comments above, virtio devices can also be fully emulated or real
> hardware devices.
>
> [...]
>
> I think we should not make any statement regarding frequency of new
> additions; sometimes, there's a flurry of activity, sometimes, there's
> nothing for ages :)

Ok, I'll simplify the intro to this:

    This document serves as a basic guideline for driver programmers
    that need to hack a new virtio driver or understand the essentials
    of the existing ones. See :ref:`Virtio on Linux <virtio>` for a
    general overview of virtio.

> s/should/needs to/ ?

Ack.

> s/defined/added/ ?

Ack.

> Maybe "Device ids need to be at least reserved in the virtio spec before
> being added to that file." ?

Sure, I'll add all of these to the next version.

Cheers,
Ricardo

Re: [PATCH v2 2/2] docs: driver-api: virtio: virtio on Linux

[Cornelia Huck]

On Tue, Aug 09 2022, Ricardo Cañuelo <ricardo.canuelo@collabora.com> wrote:

> How about this as an introduction?
>
>     Virtio is an open standard that defines a protocol to communicate
>     drivers and devices of different types, see Chapter 5 ("Device

"...that defines a protocol for communication between drivers and
devices of different device types, see..."

>     Types") of the virtio spec `[1]`_. Originally developed as a
>     standard for paravirtualized devices implemented by a hypervisor, it
>     can be used to interface any compliant device (real or emulated)
>     with a driver.
>
>     For illustrative purposes, this document will focus on the common
>     case of a Linux kernel running in a virtual machine and using
>     paravirtualized devices provided by the hypervisor, which exposes
>     them as virtio devices via standard mechanisms such as PCI.

Sounds good to me.

(...)

> Ok, I'll simplify the intro to this:
>
>     This document serves as a basic guideline for driver programmers
>     that need to hack a new virtio driver or understand the essentials
>     of the existing ones. See :ref:`Virtio on Linux <virtio>` for a
>     general overview of virtio.

Ok.