Re: [RESEND PATCH v5 1/1] docs: driver-api: virtio: virtio on Linux

[Bagas Sanjaya <bagasdotme@gmail.com>]

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

On Mon, Oct 10, 2022 at 08:43:59AM +0200, Ricardo Cañuelo wrote:
> diff --git a/Documentation/driver-api/virtio/virtio.rst b/Documentation/driver-api/virtio/virtio.rst
> new file mode 100644
> index 000000000000..921758fc3b23
> --- /dev/null
> +++ b/Documentation/driver-api/virtio/virtio.rst
> @@ -0,0 +1,144 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _virtio:
> +
> +===============
> +Virtio on Linux
> +===============
> +
> +Introduction
> +============
> +
> +Virtio is an open standard that defines a protocol for communication
> +between 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.
> <snipped>
> +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.
> +

What link is for [2]? I think you'll need to spell out the link title.
Also, the external reference pointers should be parenthesized to indicate
that these are references:

---- >8 ----
diff --git a/Documentation/driver-api/virtio/virtio.rst b/Documentation/driver-api/virtio/virtio.rst
index 4f3abbec4e8b2c..07fd2d7c51e689 100644
--- a/Documentation/driver-api/virtio/virtio.rst
+++ b/Documentation/driver-api/virtio/virtio.rst
@@ -11,7 +11,7 @@ Introduction
 
 Virtio is an open standard that defines a protocol for communication
 between drivers and devices of different types, see Chapter 5 ("Device
-Types") of the virtio spec `[1]`_. Originally developed as a standard
+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.
 
@@ -43,9 +43,10 @@ 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
-overview of how the host device and the guest driver communicate.
+Refer to Chapter 2.5 ("Virtqueues") of the virtio spec (`[1]`_) for the
+reference definitions of virtqueues and "Virtqueues and virtio ring: How
+the data travels" blog post (`[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


Personally speaking, ReST citations should do the job better (these
links are external references, right?).

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

Looks like the wording at the beginning confuses me, so better say:

---- >8 ----
diff --git a/Documentation/driver-api/virtio/virtio.rst b/Documentation/driver-api/virtio/virtio.rst
index 07fd2d7c51e689..7947b4ca690efd 100644
--- a/Documentation/driver-api/virtio/virtio.rst
+++ b/Documentation/driver-api/virtio/virtio.rst
@@ -123,10 +123,10 @@ 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.
+At this point, 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

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

The version checking should be made comment (not visible on the output):

---- >8 ----
diff --git a/Documentation/driver-api/virtio/virtio.rst b/Documentation/driver-api/virtio/virtio.rst
index 70b3aa6bcf5518..4f3abbec4e8b2c 100644
--- a/Documentation/driver-api/virtio/virtio.rst
+++ b/Documentation/driver-api/virtio/virtio.rst
@@ -134,7 +134,7 @@ 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.
+.. 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

> 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..e14c58796d25
> --- /dev/null
> +++ b/Documentation/driver-api/virtio/writing_virtio_drivers.rst
> <snipped>...
> +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.

Same reply.

Thanks.

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

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