[PATCH 00/14] kernel-doc: public/arch-arm.h

[PATCH 00/14] kernel-doc: public/arch-arm.h

[Stefano Stabellini]

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

Hi all,

This patch series convert Xen in-code comments to the kernel-doc format:

https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html


# WHAT WAS CONVERTED

I started from the public/ header files as I thought they are the most
important to generated documentation for.

I didn't cover all files under xen/include/public/, but we don't have to
boil the ocean in one go.

For the header files I addressed, I did cover all in-code comments
except for very few exceptions where the conversion to kernel-doc format
wasn't easily doable without major changes to the comments/code.

The conversion was done by hand (sigh!) but was mechanical, and only
stylistic: I didn't change the content of the comments (only in a couple
of places to make the English work right, e.g. when a comment has been
split into two comments.)


# THE KERNEL-DOC KEYWORDS USED

I used the "struct" keyword for structures, i.e.:

/**
 * struct foobar
 */

"struct" makes kernel-doc go and look at the following struct in the
code, parses struct members comments, and generate a doc optimized to
describe a struct. Note that in these cases the struct needs to follow
immediately the comment. Thus, I had to move an #define between the
comment and the struct in a few places.

Also note that kernel-doc supports nested structs but due to a quirk,
comments for nested struct members cannot be on a single line. They have
to be:

  struct foo {
      struct {
          /**
           * @u.bar: foobar
           */
          bar;
      } u;
  }

Otherwise for normal struct the single line comment works fine:

  struct foo {
      /** @bar: foobar */
      bar;
  }


I used the "DOC" keyword otherwise. "DOC" is freeform, not particularly
tied to anything following (functions, enums, etc.) I kept a black line
between "DOC" and the following comment if multiline and no blank line
if it is single line.

  /**
   * DOC: doc1
   * single line comment
   */

   /**
    * DOC: doc2
    *
    * this is
    * multiline
    */

DOC doesn't generate any cross-documents links but it is still a great
place to start as it makes the in-code comments immediately available as
documents. Linking and references can be added later.


# HOW TO TEST IT

Simply run kernel-doc on a header file, for instance:

  ../linux/scripts/kernel-doc xen/include/public/event_channel.h > /tmp/doc.rst

You can inspect the rst file and also generate a html file out of it with
sphinx:

  sphinx-quickstart
  sphinx-build . /path/to/out

I am attaching two example output html files together with the static CSS
and images to render them correctly. Note that of course I haven't
worked on the CSS at all, clearly the style can be vastly improved, but
I wanted to give you an idea of how readable they actually are even like
this.


Cheers,

Stefano


The following changes since commit 81fd0d3ca4b2cd309403c6e8da662c325dd35750:

  x86/hvm: simplify 'mmio_direct' check in epte_get_entry_emt() (2020-07-31 17:43:31 +0200)

are available in the Git repository at:

  http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git hyp-docs-1 

for you to fetch changes up to abbd21dfa0ff14a7eb5faa57aaf3db24f83a149f:

  kernel-doc: public/hvm/params.h (2020-08-06 16:27:22 -0700)

----------------------------------------------------------------
Stefano Stabellini (14):
      kernel-doc: public/arch-arm.h
      kernel-doc: public/hvm/hvm_op.h
      kernel-doc: public/device_tree_defs.h
      kernel-doc: public/event_channel.h
      kernel-doc: public/features.h
      kernel-doc: public/grant_table.h
      kernel-doc: public/hypfs.h
      kernel-doc: public/memory.h
      kernel-doc: public/sched.h
      kernel-doc: public/vcpu.h
      kernel-doc: public/version.h
      kernel-doc: public/xen.h
      kernel-doc: public/elfnote.h
      kernel-doc: public/hvm/params.h

 xen/include/public/arch-arm.h         |  43 ++-
 xen/include/public/device_tree_defs.h |  24 +-
 xen/include/public/elfnote.h          | 109 +++++--
 xen/include/public/event_channel.h    | 188 +++++++----
 xen/include/public/features.h         |  78 +++--
 xen/include/public/grant_table.h      | 443 +++++++++++++++-----------
 xen/include/public/hvm/hvm_op.h       |  20 +-
 xen/include/public/hvm/params.h       | 158 ++++++++--
 xen/include/public/hypfs.h            |  72 +++--
 xen/include/public/memory.h           | 232 +++++++++-----
 xen/include/public/sched.h            | 129 +++++---
 xen/include/public/vcpu.h             | 180 ++++++++---
 xen/include/public/version.h          |  74 ++++-
 xen/include/public/xen.h              | 567 ++++++++++++++++++++++------------
 14 files changed, 1564 insertions(+), 753 deletions(-)

[-- Attachment #2: Type: application/gzip, Size: 128296 bytes --]

[PATCH 01/14] kernel-doc: public/arch-arm.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/arch-arm.h | 43 ++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index c365b1b39e..409697dede 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -27,8 +27,8 @@
 #ifndef __XEN_PUBLIC_ARCH_ARM_H__
 #define __XEN_PUBLIC_ARCH_ARM_H__
 
-/*
- * `incontents 50 arm_abi Hypercall Calling Convention
+/**
+ * DOC: Hypercall Calling Convention
  *
  * A hypercall is issued using the ARM HVC instruction.
  *
@@ -72,8 +72,8 @@
  * Any cache allocation hints are acceptable.
  */
 
-/*
- * `incontents 55 arm_hcall Supported Hypercalls
+/**
+ * DOC: Supported Hypercalls
  *
  * Xen on ARM makes extensive use of hardware facilities and therefore
  * only a subset of the potential hypercalls are required.
@@ -175,10 +175,17 @@
     typedef union { type *p; uint64_aligned_t q; }              \
         __guest_handle_64_ ## name
 
-/*
+/**
+ * DOC: XEN_GUEST_HANDLE - a guest pointer in a struct
+ *
  * XEN_GUEST_HANDLE represents a guest pointer, when passed as a field
  * in a struct in memory. On ARM is always 8 bytes sizes and 8 bytes
  * aligned.
+ */
+
+/**
+ * DOC: XEN_GUEST_HANDLE_PARAM - a guest pointer as a hypercall arg
+ *
  * XEN_GUEST_HANDLE_PARAM represents a guest pointer, when passed as an
  * hypercall argument. It is 4 bytes on aarch32 and 8 bytes on aarch64.
  */
@@ -201,7 +208,9 @@ typedef uint64_t xen_pfn_t;
 #define PRI_xen_pfn PRIx64
 #define PRIu_xen_pfn PRIu64
 
-/*
+/**
+ * DOC: XEN_LEGACY_MAX_VCPUS
+ *
  * Maximum number of virtual CPUs in legacy multi-processor guests.
  * Only one. All other VCPUS must use VCPUOP_register_vcpu_info.
  */
@@ -299,26 +308,28 @@ struct vcpu_guest_context {
 typedef struct vcpu_guest_context vcpu_guest_context_t;
 DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
 
-/*
+
+/**
+ * struct xen_arch_domainconfig - arch-specific domain creation params
+ *
  * struct xen_arch_domainconfig's ABI is covered by
  * XEN_DOMCTL_INTERFACE_VERSION.
  */
+struct xen_arch_domainconfig {
+    /** @gic_version: IN/OUT parameter */
 #define XEN_DOMCTL_CONFIG_GIC_NATIVE    0
 #define XEN_DOMCTL_CONFIG_GIC_V2        1
 #define XEN_DOMCTL_CONFIG_GIC_V3        2
-
+    uint8_t gic_version;
+    /** @tee_type: IN parameter */
 #define XEN_DOMCTL_CONFIG_TEE_NONE      0
 #define XEN_DOMCTL_CONFIG_TEE_OPTEE     1
-
-struct xen_arch_domainconfig {
-    /* IN/OUT */
-    uint8_t gic_version;
-    /* IN */
     uint16_t tee_type;
-    /* IN */
+    /** @nr_spis: IN parameter */
     uint32_t nr_spis;
-    /*
-     * OUT
+    /**
+     * @clock_frequency: OUT parameter
+     *
      * Based on the property clock-frequency in the DT timer node.
      * The property may be present when the bootloader/firmware doesn't
      * set correctly CNTFRQ which hold the timer frequency.
-- 
2.17.1

Re: [PATCH 01/14] kernel-doc: public/arch-arm.h

[Ian Jackson]

Stefano Stabellini writes ("[PATCH 01/14] kernel-doc: public/arch-arm.h"):
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> Convert in-code comments to kernel-doc format wherever possible.

Thanks.  But, err, I think there is not yet any in-tree machinery for
actually building and publishing these kernel-doc comments ?

As I said I think replacing our ad-hoc in-tree system with kernel-doc
is a good idea, but...

> -/*
> - * `incontents 50 arm_abi Hypercall Calling Convention
> +/**
> + * DOC: Hypercall Calling Convention

... let us not replace the in-tree markup for that system until we
have its replacement.

Ian.

Re: [PATCH 01/14] kernel-doc: public/arch-arm.h

[Stefano Stabellini]

On Tue, 18 Aug 2020, Ian Jackson wrote:
> Stefano Stabellini writes ("[PATCH 01/14] kernel-doc: public/arch-arm.h"):
> > From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > 
> > Convert in-code comments to kernel-doc format wherever possible.
> 
> Thanks.  But, err, I think there is not yet any in-tree machinery for
> actually building and publishing these kernel-doc comments ?

No, there isn't. But you can call kernel-doc on the headers manually and
it will produce fully readable docs in RST format. (Then you can covert
RST docs to HTML with Sphinx.) Like:

  kernel-doc xen/include/public/features.h > readme-features.rst

I also gave a few more details on the plan I had in my other email
reply.


> As I said I think replacing our ad-hoc in-tree system with kernel-doc
> is a good idea, but...
> 
> > -/*
> > - * `incontents 50 arm_abi Hypercall Calling Convention
> > +/**
> > + * DOC: Hypercall Calling Convention
> 
> ... let us not replace the in-tree markup for that system until we
> have its replacement.

Ah! I didn't know what 

  `incontents 50 arm_abi

was for. I assumed it was a relic of another era and removed it.

Is it actually used (and the other markups like that)? Is there
a script somewhere that parses it in xen.git or on xenbits already?

If they are in-use, then I can try to retain them for now until we have
the kernel-doc infrastructure on xenbits -- they should be compatible
with the kernel-doc syntax.

[PATCH 02/14] kernel-doc: public/hvm/hvm_op.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/hvm/hvm_op.h | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h
index 870ec52060..d62d3a96f8 100644
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -27,14 +27,26 @@
 #include "../trace.h"
 #include "../event_channel.h"
 
-/* Get/set subcommands: extra argument == pointer to xen_hvm_param struct. */
+/**
+ * DOC: HVMOP_set_param and HVMOP_get_param
+ *
+ * Get/set subcommands: extra argument == pointer to xen_hvm_param struct.
+ */
 #define HVMOP_set_param           0
 #define HVMOP_get_param           1
+
+/**
+ * struct xen_hvm_param
+ */
 struct xen_hvm_param {
-    domid_t  domid;    /* IN */
+    /** @domid: IN parameter */
+    domid_t  domid;
+    /** @pad: padding */
     uint16_t pad;
-    uint32_t index;    /* IN */
-    uint64_t value;    /* IN/OUT */
+    /** @index: IN parameter */
+    uint32_t index;
+    /** @value: IN/OUT parameter */
+    uint64_t value;
 };
 typedef struct xen_hvm_param xen_hvm_param_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_param_t);
-- 
2.17.1

[PATCH 03/14] kernel-doc: public/device_tree_defs.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/device_tree_defs.h | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/xen/include/public/device_tree_defs.h b/xen/include/public/device_tree_defs.h
index 209d43de3f..be35598b53 100644
--- a/xen/include/public/device_tree_defs.h
+++ b/xen/include/public/device_tree_defs.h
@@ -2,7 +2,9 @@
 #define __XEN_DEVICE_TREE_DEFS_H__
 
 #if defined(__XEN__) || defined(__XEN_TOOLS__)
-/*
+/**
+ * DOC: GUEST_PHANDLE_GIC
+ *
  * The device tree compiler (DTC) is allocating the phandle from 1 to
  * onwards. Reserve a high value for the GIC phandle.
  */
@@ -12,17 +14,17 @@
 #define GUEST_ROOT_SIZE_CELLS 2
 
 /**
- * IRQ line type.
+ * DOC: IRQ line type.
  *
- * DT_IRQ_TYPE_NONE            - default, unspecified type
- * DT_IRQ_TYPE_EDGE_RISING     - rising edge triggered
- * DT_IRQ_TYPE_EDGE_FALLING    - falling edge triggered
- * DT_IRQ_TYPE_EDGE_BOTH       - rising and falling edge triggered
- * DT_IRQ_TYPE_LEVEL_HIGH      - high level triggered
- * DT_IRQ_TYPE_LEVEL_LOW       - low level triggered
- * DT_IRQ_TYPE_LEVEL_MASK      - Mask to filter out the level bits
- * DT_IRQ_TYPE_SENSE_MASK      - Mask for all the above bits
- * DT_IRQ_TYPE_INVALID         - Use to initialize the type
+ * - DT_IRQ_TYPE_NONE            - default, unspecified type
+ * - DT_IRQ_TYPE_EDGE_RISING     - rising edge triggered
+ * - DT_IRQ_TYPE_EDGE_FALLING    - falling edge triggered
+ * - DT_IRQ_TYPE_EDGE_BOTH       - rising and falling edge triggered
+ * - DT_IRQ_TYPE_LEVEL_HIGH      - high level triggered
+ * - DT_IRQ_TYPE_LEVEL_LOW       - low level triggered
+ * - DT_IRQ_TYPE_LEVEL_MASK      - Mask to filter out the level bits
+ * - DT_IRQ_TYPE_SENSE_MASK      - Mask for all the above bits
+ * - DT_IRQ_TYPE_INVALID         - Use to initialize the type
  */
 #define DT_IRQ_TYPE_NONE           0x00000000
 #define DT_IRQ_TYPE_EDGE_RISING    0x00000001
-- 
2.17.1

[PATCH 04/14] kernel-doc: public/event_channel.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/event_channel.h | 188 +++++++++++++++++++----------
 1 file changed, 121 insertions(+), 67 deletions(-)

diff --git a/xen/include/public/event_channel.h b/xen/include/public/event_channel.h
index cfb7929fef..893ea744d4 100644
--- a/xen/include/public/event_channel.h
+++ b/xen/include/public/event_channel.h
@@ -29,8 +29,8 @@
 
 #include "xen.h"
 
-/*
- * `incontents 150 evtchn Event Channels
+/**
+ * DOC: Event Channels
  *
  * Event channels are the basic primitive provided by Xen for event
  * notifications. An event is the Xen equivalent of a hardware
@@ -79,27 +79,34 @@
 typedef uint32_t evtchn_port_t;
 DEFINE_XEN_GUEST_HANDLE(evtchn_port_t);
 
-/*
- * EVTCHNOP_alloc_unbound: Allocate a port in domain <dom> and mark as
- * accepting interdomain bindings from domain <remote_dom>. A fresh port
- * is allocated in <dom> and returned as <port>.
+/**
+ * struct evtchn_alloc_unbound - EVTCHNOP_alloc_unbound
+ *
+ * Allocate a port in domain <dom> and mark as accepting interdomain
+ * bindings from domain <remote_dom>. A fresh port is allocated in <dom>
+ * and returned as <port>.
+ *
  * NOTES:
  *  1. If the caller is unprivileged then <dom> must be DOMID_SELF.
  *  2. <remote_dom> may be DOMID_SELF, allowing loopback connections.
  */
 struct evtchn_alloc_unbound {
-    /* IN parameters */
-    domid_t dom, remote_dom;
-    /* OUT parameters */
+    /** @dom: IN parameter */
+    domid_t dom;
+    /** @remote_dom: IN parameter */
+    domid_t remote_dom;
+    /** @port: OUT parameter */
     evtchn_port_t port;
 };
 typedef struct evtchn_alloc_unbound evtchn_alloc_unbound_t;
 
-/*
- * EVTCHNOP_bind_interdomain: Construct an interdomain event channel between
- * the calling domain and <remote_dom>. <remote_dom,remote_port> must identify
- * a port that is unbound and marked as accepting bindings from the calling
- * domain. A fresh port is allocated in the calling domain and returned as
+/**
+ * struct evtchn_bind_interdomain - EVTCHNOP_bind_interdomain
+ *
+ * Construct an interdomain event channel between the calling domain and
+ * <remote_dom>. <remote_dom,remote_port> must identify a port that is
+ * unbound and marked as accepting bindings from the calling domain. A
+ * fresh port is allocated in the calling domain and returned as
  * <local_port>.
  *
  * In case the peer domain has already tried to set our event channel
@@ -116,17 +123,20 @@ typedef struct evtchn_alloc_unbound evtchn_alloc_unbound_t;
  *  1. <remote_dom> may be DOMID_SELF, allowing loopback connections.
  */
 struct evtchn_bind_interdomain {
-    /* IN parameters. */
+    /** @remote_dom: IN parameter */
     domid_t remote_dom;
+    /** @remote_port: IN parameter */
     evtchn_port_t remote_port;
-    /* OUT parameters. */
+    /** @local_port:OUT parameter */
     evtchn_port_t local_port;
 };
 typedef struct evtchn_bind_interdomain evtchn_bind_interdomain_t;
 
-/*
- * EVTCHNOP_bind_virq: Bind a local event channel to VIRQ <irq> on specified
- * vcpu.
+/**
+ * struct evtchn_bind_virq - EVTCHNOP_bind_virq
+ *
+ * Bind a local event channel to VIRQ <irq> on specified vcpu.
+ *
  * NOTES:
  *  1. Virtual IRQs are classified as per-vcpu or global. See the VIRQ list
  *     in xen.h for the classification of each VIRQ.
@@ -137,65 +147,78 @@ typedef struct evtchn_bind_interdomain evtchn_bind_interdomain_t;
  *     binding cannot be changed.
  */
 struct evtchn_bind_virq {
-    /* IN parameters. */
-    uint32_t virq; /* enum virq */
+    /** @virq: IN parameter, enum virq */
+    uint32_t virq;
+    /** @vcpu: IN parameter */
     uint32_t vcpu;
-    /* OUT parameters. */
+    /** @port: OUT parameter */
     evtchn_port_t port;
 };
 typedef struct evtchn_bind_virq evtchn_bind_virq_t;
 
-/*
- * EVTCHNOP_bind_pirq: Bind a local event channel to a real IRQ (PIRQ <irq>).
+/**
+ * struct evtchn_bind_pirq - EVTCHNOP_bind_pirq
+ *
+ * Bind a local event channel to a real IRQ (PIRQ <irq>).
  * NOTES:
  *  1. A physical IRQ may be bound to at most one event channel per domain.
  *  2. Only a sufficiently-privileged domain may bind to a physical IRQ.
  */
 struct evtchn_bind_pirq {
-    /* IN parameters. */
+    /** @pirq: IN parameter */
     uint32_t pirq;
+    /** @flags: IN parameter,  BIND_PIRQ__* */
 #define BIND_PIRQ__WILL_SHARE 1
-    uint32_t flags; /* BIND_PIRQ__* */
-    /* OUT parameters. */
+    uint32_t flags;
+    /** @port: OUT parameter */
     evtchn_port_t port;
 };
 typedef struct evtchn_bind_pirq evtchn_bind_pirq_t;
 
-/*
- * EVTCHNOP_bind_ipi: Bind a local event channel to receive events.
+/**
+ * struct struct evtchn_bind_ipi - EVTCHNOP_bind_ipi
+ *
+ * Bind a local event channel to receive events.
  * NOTES:
  *  1. The allocated event channel is bound to the specified vcpu. The binding
  *     may not be changed.
  */
 struct evtchn_bind_ipi {
+    /** @vcpu: IN parameter */
     uint32_t vcpu;
-    /* OUT parameters. */
+    /** @port: OUT parameter */
     evtchn_port_t port;
 };
 typedef struct evtchn_bind_ipi evtchn_bind_ipi_t;
 
-/*
- * EVTCHNOP_close: Close a local event channel <port>. If the channel is
- * interdomain then the remote end is placed in the unbound state
+/**
+ * struct evtchn_close - EVTCHNOP_close
+ *
+ * Close a local event channel <port>. If the channel is interdomain
+ * then the remote end is placed in the unbound state
  * (EVTCHNSTAT_unbound), awaiting a new connection.
  */
 struct evtchn_close {
-    /* IN parameters. */
+    /** @port: IN parameter */
     evtchn_port_t port;
 };
 typedef struct evtchn_close evtchn_close_t;
 
-/*
- * EVTCHNOP_send: Send an event to the remote end of the channel whose local
- * endpoint is <port>.
+/**
+ * struct evtchn_send - EVTCHNOP_send
+ *
+ * Send an event to the remote end of the channel whose local endpoint
+ * is <port>.
  */
 struct evtchn_send {
-    /* IN parameters. */
+    /** @port: IN parameter */
     evtchn_port_t port;
 };
 typedef struct evtchn_send evtchn_send_t;
 
-/*
+/**
+ * struct evtchn_status - EVTCHNOP_status
+ *
  * EVTCHNOP_status: Get the current status of the communication channel which
  * has an endpoint at <dom, port>.
  * NOTES:
@@ -204,10 +227,11 @@ typedef struct evtchn_send evtchn_send_t;
  *     channel for which <dom> is not DOMID_SELF.
  */
 struct evtchn_status {
-    /* IN parameters */
+    /** @dom: IN parameter */
     domid_t  dom;
+    /** @port: IN parameter */
     evtchn_port_t port;
-    /* OUT parameters */
+    /** @status: OUT parameter */
 #define EVTCHNSTAT_closed       0  /* Channel is not in use.                 */
 #define EVTCHNSTAT_unbound      1  /* Channel is waiting interdom connection.*/
 #define EVTCHNSTAT_interdomain  2  /* Channel is connected to remote domain. */
@@ -215,24 +239,39 @@ struct evtchn_status {
 #define EVTCHNSTAT_virq         4  /* Channel is bound to a virtual IRQ line */
 #define EVTCHNSTAT_ipi          5  /* Channel is bound to a virtual IPI line */
     uint32_t status;
-    uint32_t vcpu;                 /* VCPU to which this channel is bound.   */
+    /** @vcpu: OUT parameter, VCPU to which this channel is bound */
+    uint32_t vcpu;
+    /** @u: OUT parameter */
     union {
+        /**
+         * @u.unbound: EVTCHNSTAT_unbound
+         */
         struct {
             domid_t dom;
-        } unbound;                 /* EVTCHNSTAT_unbound */
+        } unbound;
+        /**
+         * @u.interdomain: EVTCHNSTAT_interdomain
+         */
         struct {
             domid_t dom;
             evtchn_port_t port;
-        } interdomain;             /* EVTCHNSTAT_interdomain */
-        uint32_t pirq;             /* EVTCHNSTAT_pirq        */
-        uint32_t virq;             /* EVTCHNSTAT_virq        */
+        } interdomain;
+        /**
+         * @u.pirq: EVTCHNSTAT_pirq
+         */
+        uint32_t pirq;
+        /**
+         * @u.virq: EVTCHNSTAT_virq
+         */
+        uint32_t virq;
     } u;
 };
 typedef struct evtchn_status evtchn_status_t;
 
-/*
- * EVTCHNOP_bind_vcpu: Specify which vcpu a channel should notify when an
- * event is pending.
+/**
+ * struct evtchn_bind_vcpu - EVTCHNOP_bind_vcpu
+ *
+ * Specify which vcpu a channel should notify when an event is pending.
  * NOTES:
  *  1. IPI-bound channels always notify the vcpu specified at bind time.
  *     This binding cannot be changed.
@@ -243,24 +282,29 @@ typedef struct evtchn_status evtchn_status_t;
  *     has its binding reset to vcpu0).
  */
 struct evtchn_bind_vcpu {
-    /* IN parameters. */
+    /** @port: IN parameter */
     evtchn_port_t port;
+    /** @vcpu: IN parameter */
     uint32_t vcpu;
 };
 typedef struct evtchn_bind_vcpu evtchn_bind_vcpu_t;
 
-/*
- * EVTCHNOP_unmask: Unmask the specified local event-channel port and deliver
- * a notification to the appropriate VCPU if an event is pending.
+/**
+ * struct evtchn_unmask - EVTCHNOP_unmask
+ *
+ * Unmask the specified local event-channel port and deliver a
+ * notification to the appropriate VCPU if an event is pending.
  */
 struct evtchn_unmask {
-    /* IN parameters. */
+    /** @port: IN parameter */
     evtchn_port_t port;
 };
 typedef struct evtchn_unmask evtchn_unmask_t;
 
-/*
- * EVTCHNOP_reset: Close all event channels associated with specified domain.
+/**
+ * struct evtchn_reset - EVTCHNOP_reset
+ *
+ * Close all event channels associated with specified domain.
  * NOTES:
  *  1. <dom> may be specified as DOMID_SELF.
  *  2. Only a sufficiently-privileged domain may specify other than DOMID_SELF.
@@ -270,44 +314,54 @@ typedef struct evtchn_unmask evtchn_unmask_t;
  *     as these events are likely to be lost.
  */
 struct evtchn_reset {
-    /* IN parameters. */
+    /** @dom: IN parameter */
     domid_t dom;
 };
 typedef struct evtchn_reset evtchn_reset_t;
 
-/*
- * EVTCHNOP_init_control: initialize the control block for the FIFO ABI.
+/**
+ * struct evtchn_init_control - EVTCHNOP_init_control
+ *
+ * Initialize the control block for the FIFO ABI.
  *
  * Note: any events that are currently pending will not be resent and
  * will be lost.  Guests should call this before binding any event to
  * avoid losing any events.
  */
 struct evtchn_init_control {
-    /* IN parameters. */
+    /** @control_gfn: IN parameter */
     uint64_t control_gfn;
+    /** @offset: IN parameter */
     uint32_t offset;
+    /** @vcpu: IN parameter */
     uint32_t vcpu;
-    /* OUT parameters. */
+    /** @link_bits: OUT parameter */
     uint8_t link_bits;
+    /** @_pad: padding */
     uint8_t _pad[7];
 };
 typedef struct evtchn_init_control evtchn_init_control_t;
 
-/*
- * EVTCHNOP_expand_array: add an additional page to the event array.
+/**
+ * struct evtchn_expand_array - EVTCHNOP_expand_array
+ *
+ * Add an additional page to the event array.
  */
 struct evtchn_expand_array {
-    /* IN parameters. */
+    /** @array_gfn: IN parameter */
     uint64_t array_gfn;
 };
 typedef struct evtchn_expand_array evtchn_expand_array_t;
 
-/*
- * EVTCHNOP_set_priority: set the priority for an event channel.
+/**
+ * struct evtchn_set_priority - EVTCHNOP_set_priority
+ *
+ * Set the priority for an event channel.
  */
 struct evtchn_set_priority {
-    /* IN parameters. */
+    /** @port: IN parameter */
     evtchn_port_t port;
+    /** @priority: IN parameter */
     uint32_t priority;
 };
 typedef struct evtchn_set_priority evtchn_set_priority_t;
-- 
2.17.1

Re: [PATCH 04/14] kernel-doc: public/event_channel.h

[Jan Beulich]

On 07.08.2020 01:49, Stefano Stabellini wrote:
> @@ -137,65 +147,78 @@ typedef struct evtchn_bind_interdomain evtchn_bind_interdomain_t;
>   *     binding cannot be changed.
>   */
>  struct evtchn_bind_virq {
> -    /* IN parameters. */
> -    uint32_t virq; /* enum virq */
> +    /** @virq: IN parameter, enum virq */
> +    uint32_t virq;
> +    /** @vcpu: IN parameter */
>      uint32_t vcpu;
> -    /* OUT parameters. */
> +    /** @port: OUT parameter */
>      evtchn_port_t port;
>  };
>  typedef struct evtchn_bind_virq evtchn_bind_virq_t;
>  
> -/*
> - * EVTCHNOP_bind_pirq: Bind a local event channel to a real IRQ (PIRQ <irq>).
> +/**
> + * struct evtchn_bind_pirq - EVTCHNOP_bind_pirq
> + *
> + * Bind a local event channel to a real IRQ (PIRQ <irq>).
>   * NOTES:
>   *  1. A physical IRQ may be bound to at most one event channel per domain.
>   *  2. Only a sufficiently-privileged domain may bind to a physical IRQ.
>   */
>  struct evtchn_bind_pirq {
> -    /* IN parameters. */
> +    /** @pirq: IN parameter */
>      uint32_t pirq;
> +    /** @flags: IN parameter,  BIND_PIRQ__* */
>  #define BIND_PIRQ__WILL_SHARE 1
> -    uint32_t flags; /* BIND_PIRQ__* */
> -    /* OUT parameters. */
> +    uint32_t flags;
> +    /** @port: OUT parameter */
>      evtchn_port_t port;
>  };
>  typedef struct evtchn_bind_pirq evtchn_bind_pirq_t;
>  
> -/*
> - * EVTCHNOP_bind_ipi: Bind a local event channel to receive events.
> +/**
> + * struct struct evtchn_bind_ipi - EVTCHNOP_bind_ipi
> + *
> + * Bind a local event channel to receive events.
>   * NOTES:
>   *  1. The allocated event channel is bound to the specified vcpu. The binding
>   *     may not be changed.
>   */
>  struct evtchn_bind_ipi {
> +    /** @vcpu: IN parameter */
>      uint32_t vcpu;
> -    /* OUT parameters. */
> +    /** @port: OUT parameter */
>      evtchn_port_t port;
>  };
>  typedef struct evtchn_bind_ipi evtchn_bind_ipi_t;
>  
> -/*
> - * EVTCHNOP_close: Close a local event channel <port>. If the channel is
> - * interdomain then the remote end is placed in the unbound state
> +/**
> + * struct evtchn_close - EVTCHNOP_close
> + *
> + * Close a local event channel <port>. If the channel is interdomain
> + * then the remote end is placed in the unbound state
>   * (EVTCHNSTAT_unbound), awaiting a new connection.
>   */
>  struct evtchn_close {
> -    /* IN parameters. */
> +    /** @port: IN parameter */
>      evtchn_port_t port;
>  };
>  typedef struct evtchn_close evtchn_close_t;
>  
> -/*
> - * EVTCHNOP_send: Send an event to the remote end of the channel whose local
> - * endpoint is <port>.
> +/**
> + * struct evtchn_send - EVTCHNOP_send
> + *
> + * Send an event to the remote end of the channel whose local endpoint
> + * is <port>.
>   */
>  struct evtchn_send {
> -    /* IN parameters. */
> +    /** @port: IN parameter */
>      evtchn_port_t port;
>  };
>  typedef struct evtchn_send evtchn_send_t;
>  
> -/*
> +/**
> + * struct evtchn_status - EVTCHNOP_status
> + *
>   * EVTCHNOP_status: Get the current status of the communication channel which
>   * has an endpoint at <dom, port>.
>   * NOTES:

Nit: I guess you meant to remove the "EVTCHNOP_status: " prefix from
the original comment, like is done elsewhere?

Jan

Re: [PATCH 04/14] kernel-doc: public/event_channel.h

[Stefano Stabellini]

On Fri, 7 Aug 2020, Jan Beulich wrote:
> On 07.08.2020 01:49, Stefano Stabellini wrote:
> > @@ -137,65 +147,78 @@ typedef struct evtchn_bind_interdomain evtchn_bind_interdomain_t;
> >   *     binding cannot be changed.
> >   */
> >  struct evtchn_bind_virq {
> > -    /* IN parameters. */
> > -    uint32_t virq; /* enum virq */
> > +    /** @virq: IN parameter, enum virq */
> > +    uint32_t virq;
> > +    /** @vcpu: IN parameter */
> >      uint32_t vcpu;
> > -    /* OUT parameters. */
> > +    /** @port: OUT parameter */
> >      evtchn_port_t port;
> >  };
> >  typedef struct evtchn_bind_virq evtchn_bind_virq_t;
> >  
> > -/*
> > - * EVTCHNOP_bind_pirq: Bind a local event channel to a real IRQ (PIRQ <irq>).
> > +/**
> > + * struct evtchn_bind_pirq - EVTCHNOP_bind_pirq
> > + *
> > + * Bind a local event channel to a real IRQ (PIRQ <irq>).
> >   * NOTES:
> >   *  1. A physical IRQ may be bound to at most one event channel per domain.
> >   *  2. Only a sufficiently-privileged domain may bind to a physical IRQ.
> >   */
> >  struct evtchn_bind_pirq {
> > -    /* IN parameters. */
> > +    /** @pirq: IN parameter */
> >      uint32_t pirq;
> > +    /** @flags: IN parameter,  BIND_PIRQ__* */
> >  #define BIND_PIRQ__WILL_SHARE 1
> > -    uint32_t flags; /* BIND_PIRQ__* */
> > -    /* OUT parameters. */
> > +    uint32_t flags;
> > +    /** @port: OUT parameter */
> >      evtchn_port_t port;
> >  };
> >  typedef struct evtchn_bind_pirq evtchn_bind_pirq_t;
> >  
> > -/*
> > - * EVTCHNOP_bind_ipi: Bind a local event channel to receive events.
> > +/**
> > + * struct struct evtchn_bind_ipi - EVTCHNOP_bind_ipi
> > + *
> > + * Bind a local event channel to receive events.
> >   * NOTES:
> >   *  1. The allocated event channel is bound to the specified vcpu. The binding
> >   *     may not be changed.
> >   */
> >  struct evtchn_bind_ipi {
> > +    /** @vcpu: IN parameter */
> >      uint32_t vcpu;
> > -    /* OUT parameters. */
> > +    /** @port: OUT parameter */
> >      evtchn_port_t port;
> >  };
> >  typedef struct evtchn_bind_ipi evtchn_bind_ipi_t;
> >  
> > -/*
> > - * EVTCHNOP_close: Close a local event channel <port>. If the channel is
> > - * interdomain then the remote end is placed in the unbound state
> > +/**
> > + * struct evtchn_close - EVTCHNOP_close
> > + *
> > + * Close a local event channel <port>. If the channel is interdomain
> > + * then the remote end is placed in the unbound state
> >   * (EVTCHNSTAT_unbound), awaiting a new connection.
> >   */
> >  struct evtchn_close {
> > -    /* IN parameters. */
> > +    /** @port: IN parameter */
> >      evtchn_port_t port;
> >  };
> >  typedef struct evtchn_close evtchn_close_t;
> >  
> > -/*
> > - * EVTCHNOP_send: Send an event to the remote end of the channel whose local
> > - * endpoint is <port>.
> > +/**
> > + * struct evtchn_send - EVTCHNOP_send
> > + *
> > + * Send an event to the remote end of the channel whose local endpoint
> > + * is <port>.
> >   */
> >  struct evtchn_send {
> > -    /* IN parameters. */
> > +    /** @port: IN parameter */
> >      evtchn_port_t port;
> >  };
> >  typedef struct evtchn_send evtchn_send_t;
> >  
> > -/*
> > +/**
> > + * struct evtchn_status - EVTCHNOP_status
> > + *
> >   * EVTCHNOP_status: Get the current status of the communication channel which
> >   * has an endpoint at <dom, port>.
> >   * NOTES:
> 
> Nit: I guess you meant to remove the "EVTCHNOP_status: " prefix from
> the original comment, like is done elsewhere?

Yes, good catch, I'll change it.

[PATCH 05/14] kernel-doc: public/features.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/features.h | 78 ++++++++++++++++++++++++++---------
 1 file changed, 59 insertions(+), 19 deletions(-)

diff --git a/xen/include/public/features.h b/xen/include/public/features.h
index 1613b2aab8..524d1758c4 100644
--- a/xen/include/public/features.h
+++ b/xen/include/public/features.h
@@ -27,8 +27,8 @@
 #ifndef __XEN_PUBLIC_FEATURES_H__
 #define __XEN_PUBLIC_FEATURES_H__
 
-/*
- * `incontents 200 elfnotes_features XEN_ELFNOTE_FEATURES
+/**
+ * DOC: XEN_ELFNOTE_FEATURES
  *
  * The list of all the features the guest supports. They are set by
  * parsing the XEN_ELFNOTE_FEATURES and XEN_ELFNOTE_SUPPORTED_FEATURES
@@ -41,19 +41,25 @@
  * XENFEAT_dom0 MUST be set if the guest is to be booted as dom0,
  */
 
-/*
- * If set, the guest does not need to write-protect its pagetables, and can
- * update them via direct writes.
+/**
+ * DOC: XENFEAT_writable_page_tables
+ *
+ * If set, the guest does not need to write-protect its pagetables, and
+ * can update them via direct writes.
  */
 #define XENFEAT_writable_page_tables       0
 
-/*
+/**
+ * DOC: XENFEAT_writable_descriptor_tables
+ *
  * If set, the guest does not need to write-protect its segment descriptor
  * tables, and can update them via direct writes.
  */
 #define XENFEAT_writable_descriptor_tables 1
 
-/*
+/**
+ * DOC: XENFEAT_auto_translated_physmap
+ *
  * If set, translation between the guest's 'pseudo-physical' address space
  * and the host's machine address space are handled by the hypervisor. In this
  * mode the guest does not need to perform phys-to/from-machine translations
@@ -61,37 +67,63 @@
  */
 #define XENFEAT_auto_translated_physmap    2
 
-/* If set, the guest is running in supervisor mode (e.g., x86 ring 0). */
+/**
+ * DOC: XENFEAT_supervisor_mode_kernel
+ *
+ * If set, the guest is running in supervisor mode (e.g., x86 ring 0).
+ */
 #define XENFEAT_supervisor_mode_kernel     3
 
-/*
+/**
+ * DOC: XENFEAT_pae_pgdir_above_4gb
+ *
  * If set, the guest does not need to allocate x86 PAE page directories
  * below 4GB. This flag is usually implied by auto_translated_physmap.
  */
 #define XENFEAT_pae_pgdir_above_4gb        4
 
-/* x86: Does this Xen host support the MMU_PT_UPDATE_PRESERVE_AD hypercall? */
+/**
+ * DOC: XENFEAT_mmu_pt_update_preserve_ad
+ * x86: Does this Xen host support the MMU_PT_UPDATE_PRESERVE_AD hypercall?
+ */
 #define XENFEAT_mmu_pt_update_preserve_ad  5
 
-/* x86: Does this Xen host support the MMU_{CLEAR,COPY}_PAGE hypercall? */
+/**
+ * DOC: XENFEAT_highmem_assist
+ * x86: Does this Xen host support the MMU_{CLEAR,COPY}_PAGE hypercall?
+ */
 #define XENFEAT_highmem_assist             6
 
-/*
+/**
+ * DOC: XENFEAT_gnttab_map_avail_bits
+ *
  * If set, GNTTABOP_map_grant_ref honors flags to be placed into guest kernel
  * available pte bits.
  */
 #define XENFEAT_gnttab_map_avail_bits      7
 
-/* x86: Does this Xen host support the HVM callback vector type? */
+/**
+ * DOC: XENFEAT_hvm_callback_vector
+ * x86: Does this Xen host support the HVM callback vector type?
+ */
 #define XENFEAT_hvm_callback_vector        8
 
-/* x86: pvclock algorithm is safe to use on HVM */
+/**
+ * DOC: XENFEAT_hvm_safe_pvclock
+ * x86: pvclock algorithm is safe to use on HVM
+ */
 #define XENFEAT_hvm_safe_pvclock           9
 
-/* x86: pirq can be used by HVM guests */
+/**
+ * DOC: XENFEAT_hvm_pirqs
+ * x86: pirq can be used by HVM guests
+ */
 #define XENFEAT_hvm_pirqs                 10
 
-/* operation as Dom0 is supported */
+/**
+ * DOC: XENFEAT_dom0
+ * operation as Dom0 is supported
+ */
 #define XENFEAT_dom0                      11
 
 /* Xen also maps grant references at pfn = mfn.
@@ -99,13 +131,21 @@
 #define XENFEAT_grant_map_identity        12
  */
 
-/* Guest can use XENMEMF_vnode to specify virtual node for memory op. */
+/**
+ * DOC: XENFEAT_memory_op_vnode_supported
+ * Guest can use XENMEMF_vnode to specify virtual node for memory op.
+ */
 #define XENFEAT_memory_op_vnode_supported 13
 
-/* arm: Hypervisor supports ARM SMC calling convention. */
+/**
+ * DOC: XENFEAT_ARM_SMCCC_supported
+ * arm: Hypervisor supports ARM SMC calling convention.
+ */
 #define XENFEAT_ARM_SMCCC_supported       14
 
-/*
+/**
+ * DOC: XENFEAT_linux_rsdp_unrestricted
+ *
  * x86/PVH: If set, ACPI RSDP can be placed at any address. Otherwise RSDP
  * must be located in lower 1MB, as required by ACPI Specification for IA-PC
  * systems.
-- 
2.17.1

Re: [PATCH 05/14] kernel-doc: public/features.h

[Jan Beulich]

On 07.08.2020 01:49, Stefano Stabellini wrote:
> @@ -41,19 +41,25 @@
>   * XENFEAT_dom0 MUST be set if the guest is to be booted as dom0,
>   */
>  
> -/*
> - * If set, the guest does not need to write-protect its pagetables, and can
> - * update them via direct writes.
> +/**
> + * DOC: XENFEAT_writable_page_tables
> + *
> + * If set, the guest does not need to write-protect its pagetables, and
> + * can update them via direct writes.
>   */
>  #define XENFEAT_writable_page_tables       0

I dislike such redundancy (and it's more noticable here than with
the struct-s). Is there really no way for the tool to find the
right item, the more that in the cover letter you say that you
even need to get the placement right, i.e. there can't be e.g.
intervening #define-s?

Jan

Re: [PATCH 05/14] kernel-doc: public/features.h

[Stefano Stabellini]

On Fri, 7 Aug 2020, Jan Beulich wrote:
> On 07.08.2020 01:49, Stefano Stabellini wrote:
> > @@ -41,19 +41,25 @@
> >   * XENFEAT_dom0 MUST be set if the guest is to be booted as dom0,
> >   */
> >  
> > -/*
> > - * If set, the guest does not need to write-protect its pagetables, and can
> > - * update them via direct writes.
> > +/**
> > + * DOC: XENFEAT_writable_page_tables
> > + *
> > + * If set, the guest does not need to write-protect its pagetables, and
> > + * can update them via direct writes.
> >   */
> >  #define XENFEAT_writable_page_tables       0
> 
> I dislike such redundancy (and it's more noticable here than with
> the struct-s). Is there really no way for the tool to find the
> right item, the more that in the cover letter you say that you
> even need to get the placement right, i.e. there can't be e.g.
> intervening #define-s?

Let me clarify that the right placement (nothing between the comment and
the following structure) is important for structs, typedefs, etc., but
not for "DOC". DOC is freeform and doesn't have to be followed by
anything specifically.


In regards to the redundancy, there is only another option, that I
didn't choose because it leads to worse documents being generated.
However, they are still readable, so if the agreement is to use the
other format, I would be OK with it.


The other format is the keyword "macro" (this one would have to have the
right placement, straight on top of the #define):

/**
 * macro XENFEAT_writable_page_tables
 *
 * If set, the guest does not need to write-protect its pagetables, and
 * can update them via direct writes.
 */


Which could be further simplified to:

/**
 * macro
 *
 * If set, the guest does not need to write-protect its pagetables, and
 * can update them via direct writes.
 */


In terms of redundancy, that's the best we can do.

The reason why I say it is not optimal is that with DOC the pleudo-html
generated via sphinx is:

---
* XENFEAT_writable_page_tables *

If set, the guest does not need to write-protect its pagetables, and
can update them via direct writes.
---

While with macro, two () parenthesis gets added to the title, and also an
empty "Parameters" section gets added, like this:

---
* XENFEAT_writable_page_tables() *

** Parameters **

** Description **

If set, the guest does not need to write-protect its pagetables, and
can update them via direct writes.
---


I think it could be confusing to the user: it looks like a macro with
parameters, which is not what we want.

For that reason, I think we should stick with "DOC" for now.

Re: [PATCH 05/14] kernel-doc: public/features.h

[Jan Beulich]

On 07.08.2020 23:52, Stefano Stabellini wrote:
> On Fri, 7 Aug 2020, Jan Beulich wrote:
>> On 07.08.2020 01:49, Stefano Stabellini wrote:
>>> @@ -41,19 +41,25 @@
>>>   * XENFEAT_dom0 MUST be set if the guest is to be booted as dom0,
>>>   */
>>>  
>>> -/*
>>> - * If set, the guest does not need to write-protect its pagetables, and can
>>> - * update them via direct writes.
>>> +/**
>>> + * DOC: XENFEAT_writable_page_tables
>>> + *
>>> + * If set, the guest does not need to write-protect its pagetables, and
>>> + * can update them via direct writes.
>>>   */
>>>  #define XENFEAT_writable_page_tables       0
>>
>> I dislike such redundancy (and it's more noticable here than with
>> the struct-s). Is there really no way for the tool to find the
>> right item, the more that in the cover letter you say that you
>> even need to get the placement right, i.e. there can't be e.g.
>> intervening #define-s?
> 
> Let me clarify that the right placement (nothing between the comment and
> the following structure) is important for structs, typedefs, etc., but
> not for "DOC". DOC is freeform and doesn't have to be followed by
> anything specifically.
> 
> 
> In regards to the redundancy, there is only another option, that I
> didn't choose because it leads to worse documents being generated.
> However, they are still readable, so if the agreement is to use the
> other format, I would be OK with it.
> 
> 
> The other format is the keyword "macro" (this one would have to have the
> right placement, straight on top of the #define):
> 
> /**
>  * macro XENFEAT_writable_page_tables
>  *
>  * If set, the guest does not need to write-protect its pagetables, and
>  * can update them via direct writes.
>  */
> 
> 
> Which could be further simplified to:
> 
> /**
>  * macro
>  *
>  * If set, the guest does not need to write-protect its pagetables, and
>  * can update them via direct writes.
>  */
> 
> 
> In terms of redundancy, that's the best we can do.
> 
> The reason why I say it is not optimal is that with DOC the pleudo-html
> generated via sphinx is:
> 
> ---
> * XENFEAT_writable_page_tables *
> 
> If set, the guest does not need to write-protect its pagetables, and
> can update them via direct writes.
> ---
> 
> While with macro, two () parenthesis gets added to the title, and also an
> empty "Parameters" section gets added, like this:
> 
> ---
> * XENFEAT_writable_page_tables() *
> 
> ** Parameters **
> 
> ** Description **
> 
> If set, the guest does not need to write-protect its pagetables, and
> can update them via direct writes.
> ---
> 
> 
> I think it could be confusing to the user: it looks like a macro with
> parameters, which is not what we want.

Agreed, so ...

> For that reason, I think we should stick with "DOC" for now.

... if there are no (better) alternatives we'll have to live with the
redundancy.

Jan

Re: [PATCH 05/14] kernel-doc: public/features.h

[Stefano Stabellini]

On Mon, 17 Aug 2020, Jan Beulich wrote:
> On 07.08.2020 23:52, Stefano Stabellini wrote:
> > On Fri, 7 Aug 2020, Jan Beulich wrote:
> >> On 07.08.2020 01:49, Stefano Stabellini wrote:
> >>> @@ -41,19 +41,25 @@
> >>>   * XENFEAT_dom0 MUST be set if the guest is to be booted as dom0,
> >>>   */
> >>>  
> >>> -/*
> >>> - * If set, the guest does not need to write-protect its pagetables, and can
> >>> - * update them via direct writes.
> >>> +/**
> >>> + * DOC: XENFEAT_writable_page_tables
> >>> + *
> >>> + * If set, the guest does not need to write-protect its pagetables, and
> >>> + * can update them via direct writes.
> >>>   */
> >>>  #define XENFEAT_writable_page_tables       0
> >>
> >> I dislike such redundancy (and it's more noticable here than with
> >> the struct-s). Is there really no way for the tool to find the
> >> right item, the more that in the cover letter you say that you
> >> even need to get the placement right, i.e. there can't be e.g.
> >> intervening #define-s?
> > 
> > Let me clarify that the right placement (nothing between the comment and
> > the following structure) is important for structs, typedefs, etc., but
> > not for "DOC". DOC is freeform and doesn't have to be followed by
> > anything specifically.
> > 
> > 
> > In regards to the redundancy, there is only another option, that I
> > didn't choose because it leads to worse documents being generated.
> > However, they are still readable, so if the agreement is to use the
> > other format, I would be OK with it.
> > 
> > 
> > The other format is the keyword "macro" (this one would have to have the
> > right placement, straight on top of the #define):
> > 
> > /**
> >  * macro XENFEAT_writable_page_tables
> >  *
> >  * If set, the guest does not need to write-protect its pagetables, and
> >  * can update them via direct writes.
> >  */
> > 
> > 
> > Which could be further simplified to:
> > 
> > /**
> >  * macro
> >  *
> >  * If set, the guest does not need to write-protect its pagetables, and
> >  * can update them via direct writes.
> >  */
> > 
> > 
> > In terms of redundancy, that's the best we can do.
> > 
> > The reason why I say it is not optimal is that with DOC the pleudo-html
> > generated via sphinx is:
> > 
> > ---
> > * XENFEAT_writable_page_tables *
> > 
> > If set, the guest does not need to write-protect its pagetables, and
> > can update them via direct writes.
> > ---
> > 
> > While with macro, two () parenthesis gets added to the title, and also an
> > empty "Parameters" section gets added, like this:
> > 
> > ---
> > * XENFEAT_writable_page_tables() *
> > 
> > ** Parameters **
> > 
> > ** Description **
> > 
> > If set, the guest does not need to write-protect its pagetables, and
> > can update them via direct writes.
> > ---
> > 
> > 
> > I think it could be confusing to the user: it looks like a macro with
> > parameters, which is not what we want.
> 
> Agreed, so ...
> 
> > For that reason, I think we should stick with "DOC" for now.
> 
> ... if there are no (better) alternatives we'll have to live with the
> redundancy.

Thanks Jan. I would prefer to get this series in as is (with the other
minor changes we discussed) as basic enablement for kernel-doc. I
volunteer to have a look into this issue and try to come up with a
better alternative afterward.

[PATCH 06/14] kernel-doc: public/grant_table.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/grant_table.h | 443 ++++++++++++++++++-------------
 1 file changed, 257 insertions(+), 186 deletions(-)

diff --git a/xen/include/public/grant_table.h b/xen/include/public/grant_table.h
index 3b7bf93d74..e461458225 100644
--- a/xen/include/public/grant_table.h
+++ b/xen/include/public/grant_table.h
@@ -30,8 +30,8 @@
 
 #include "xen.h"
 
-/*
- * `incontents 150 gnttab Grant Tables
+/**
+ * DOC: Grant Tables
  *
  * Xen's grant tables provide a generic mechanism to memory sharing
  * between domains. This shared memory interface underpins the split
@@ -53,11 +53,10 @@
  * fully virtualised memory.
  */
 
-/***********************************
- * GRANT TABLE REPRESENTATION
- */
-
-/* Some rough guidelines on accessing and updating grant-table entries
+/**
+ * DOC: GRANT TABLE REPRESENTATION
+ *
+ * Some rough guidelines on accessing and updating grant-table entries
  * in a concurrency-safe manner. For more information, Linux contains a
  * reference implementation for guest OSes (drivers/xen/grant_table.c, see
  * http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/xen/grant-table.c;hb=HEAD
@@ -108,56 +107,66 @@
  *  Use SMP-safe bit-setting instruction.
  */
 
-/*
+/**
+ * typedef
  * Reference to a grant entry in a specified domain's grant table.
  */
 typedef uint32_t grant_ref_t;
 
-/*
+/**
+ * DOC: grant table
+ *
  * A grant table comprises a packed array of grant entries in one or more
  * page frames shared between Xen and a guest.
  * [XEN]: This field is written by Xen and read by the sharing guest.
  * [GST]: This field is written by the guest and read by Xen.
  */
 
-/*
- * Version 1 of the grant table entry structure is maintained purely
- * for backwards compatibility.  New guests should use version 2.
- */
 #if __XEN_INTERFACE_VERSION__ < 0x0003020a
 #define grant_entry_v1 grant_entry
 #define grant_entry_v1_t grant_entry_t
 #endif
+/**
+ * struct grant_entry_v1
+ *
+ * Version 1 of the grant table entry structure is maintained purely
+ * for backwards compatibility.  New guests should use version 2.
+ */
 struct grant_entry_v1 {
-    /* GTF_xxx: various type and flag information.  [XEN,GST] */
+    /** @flags: GTF_xxx, various type and flag information.  [XEN,GST] */
     uint16_t flags;
-    /* The domain being granted foreign privileges. [GST] */
+    /** @domid: The domain being granted foreign privileges. [GST] */
     domid_t  domid;
-    /*
-     * GTF_permit_access: GFN that @domid is allowed to map and access. [GST]
-     * GTF_accept_transfer: GFN that @domid is allowed to transfer into. [GST]
-     * GTF_transfer_completed: MFN whose ownership transferred by @domid
-     *                         (non-translated guests only). [XEN]
+    /**
+     * @frame:
+     * - GTF_permit_access: GFN that @domid is allowed to map and access. [GST]
+     * - GTF_accept_transfer: GFN that @domid is allowed to transfer into. [GST]
+     * - GTF_transfer_completed: MFN whose ownership transferred by @domid
+     *                           (non-translated guests only). [XEN]
      */
     uint32_t frame;
 };
 typedef struct grant_entry_v1 grant_entry_v1_t;
 
-/* The first few grant table entries will be preserved across grant table
+/**
+ * DOC: GNTTAB_NR_RESERVED_ENTRIES
+ *
+ * The first few grant table entries will be preserved across grant table
  * version changes and may be pre-populated at domain creation by tools.
  */
 #define GNTTAB_NR_RESERVED_ENTRIES     8
 #define GNTTAB_RESERVED_CONSOLE        0
 #define GNTTAB_RESERVED_XENSTORE       1
 
-/*
- * Type of grant entry.
- *  GTF_invalid: This grant entry grants no privileges.
- *  GTF_permit_access: Allow @domid to map/access @frame.
- *  GTF_accept_transfer: Allow @domid to transfer ownership of one page frame
- *                       to this guest. Xen writes the page number to @frame.
- *  GTF_transitive: Allow @domid to transitively access a subrange of
- *                  @trans_grant in @trans_domid.  No mappings are allowed.
+/**
+ * DOC: Type of grant entry.
+ *
+ * - GTF_invalid: This grant entry grants no privileges.
+ * - GTF_permit_access: Allow @domid to map/access @frame.
+ * - GTF_accept_transfer: Allow @domid to transfer ownership of one page frame
+ *                        to this guest. Xen writes the page number to @frame.
+ * - GTF_transitive: Allow @domid to transitively access a subrange of
+ *                   @trans_grant in @trans_domid.  No mappings are allowed.
  */
 #define GTF_invalid         (0U<<0)
 #define GTF_permit_access   (1U<<0)
@@ -165,15 +174,16 @@ typedef struct grant_entry_v1 grant_entry_v1_t;
 #define GTF_transitive      (3U<<0)
 #define GTF_type_mask       (3U<<0)
 
-/*
- * Subflags for GTF_permit_access.
- *  GTF_readonly: Restrict @domid to read-only mappings and accesses. [GST]
- *  GTF_reading: Grant entry is currently mapped for reading by @domid. [XEN]
- *  GTF_writing: Grant entry is currently mapped for writing by @domid. [XEN]
- *  GTF_PAT, GTF_PWT, GTF_PCD: (x86) cache attribute flags for the grant [GST]
- *  GTF_sub_page: Grant access to only a subrange of the page.  @domid
- *                will only be allowed to copy from the grant, and not
- *                map it. [GST]
+/**
+ * DOC: Subflags for GTF_permit_access.
+ *
+ * - GTF_readonly: Restrict @domid to read-only mappings and accesses. [GST]
+ * - GTF_reading: Grant entry is currently mapped for reading by @domid. [XEN]
+ * - GTF_writing: Grant entry is currently mapped for writing by @domid. [XEN]
+ * - GTF_PAT, GTF_PWT, GTF_PCD: (x86) cache attribute flags for the grant [GST]
+ * - GTF_sub_page: Grant access to only a subrange of the page.  @domid
+ *                 will only be allowed to copy from the grant, and not
+ *                 map it. [GST]
  */
 #define _GTF_readonly       (2)
 #define GTF_readonly        (1U<<_GTF_readonly)
@@ -190,8 +200,9 @@ typedef struct grant_entry_v1 grant_entry_v1_t;
 #define _GTF_sub_page       (8)
 #define GTF_sub_page        (1U<<_GTF_sub_page)
 
-/*
- * Subflags for GTF_accept_transfer:
+/**
+ * DOC: Subflags for GTF_accept_transfer:
+ *
  *  GTF_transfer_committed: Xen sets this flag to indicate that it is committed
  *      to transferring ownership of a page frame. When a guest sees this flag
  *      it must /not/ modify the grant entry until GTF_transfer_completed is
@@ -205,38 +216,43 @@ typedef struct grant_entry_v1 grant_entry_v1_t;
 #define _GTF_transfer_completed (3)
 #define GTF_transfer_completed  (1U<<_GTF_transfer_completed)
 
-/*
- * Version 2 grant table entries.  These fulfil the same role as
- * version 1 entries, but can represent more complicated operations.
- * Any given domain will have either a version 1 or a version 2 table,
- * and every entry in the table will be the same version.
+#if __XEN_INTERFACE_VERSION__ >= 0x0003020a
+/**
+ * DOC: Version 2 grant table entries
+ *
+ * These fulfil the same role as version 1 entries, but can represent
+ * more complicated operations.  Any given domain will have either a
+ * version 1 or a version 2 table, and every entry in the table will be
+ * the same version.
  *
  * The interface by which domains use grant references does not depend
  * on the grant table version in use by the other domain.
- */
-#if __XEN_INTERFACE_VERSION__ >= 0x0003020a
-/*
+ *
  * Version 1 and version 2 grant entries share a common prefix.  The
  * fields of the prefix are documented as part of struct
  * grant_entry_v1.
  */
+/**
+ * struct grant_entry_header
+ */
 struct grant_entry_header {
     uint16_t flags;
     domid_t  domid;
 };
 typedef struct grant_entry_header grant_entry_header_t;
 
-/*
- * Version 2 of the grant entry structure.
+/**
+ * union grant_entry_v2 - Version 2 of the grant entry structure.
  */
 union grant_entry_v2 {
     grant_entry_header_t hdr;
 
-    /*
+    /**
+     * @full_page:
      * This member is used for V1-style full page grants, where either:
      *
-     * -- hdr.type is GTF_accept_transfer, or
-     * -- hdr.type is GTF_permit_access and GTF_sub_page is not set.
+     * - hdr.type is GTF_accept_transfer, or
+     * - hdr.type is GTF_permit_access and GTF_sub_page is not set.
      *
      * In that case, the frame field has the same semantics as the
      * field of the same name in the V1 entry structure.
@@ -247,7 +263,9 @@ union grant_entry_v2 {
         uint64_t frame;
     } full_page;
 
-    /*
+    /**
+     * @sub_page:
+     *
      * If the grant type is GTF_grant_access and GTF_sub_page is set,
      * @domid is allowed to access bytes [@page_off,@page_off+@length)
      * in frame @frame.
@@ -259,7 +277,9 @@ union grant_entry_v2 {
         uint64_t frame;
     } sub_page;
 
-    /*
+    /**
+     * @transitive:
+     *
      * If the grant is GTF_transitive, @domid is allowed to use the
      * grant @gref in domain @trans_domid, as if it was the local
      * domain.  Obviously, the transitive access must be compatible
@@ -275,7 +295,8 @@ union grant_entry_v2 {
         grant_ref_t gref;
     } transitive;
 
-    uint32_t __spacer[4]; /* Pad to a power of two */
+    /** @__spacer: Pad to a power of two */
+    uint32_t __spacer[4];
 };
 typedef union grant_entry_v2 grant_entry_v2_t;
 
@@ -283,16 +304,14 @@ typedef uint16_t grant_status_t;
 
 #endif /* __XEN_INTERFACE_VERSION__ */
 
-/***********************************
- * GRANT TABLE QUERIES AND USES
- */
-
-/* ` enum neg_errnoval
- * ` HYPERVISOR_grant_table_op(enum grant_table_op cmd,
- * `                           void *args,
- * `                           unsigned int count)
- * `
+/**
+ * DOC: GRANT TABLE QUERIES AND USES
  *
+ * enum neg_errnoval
+ * HYPERVISOR_grant_table_op(enum grant_table_op cmd,
+ *                           void *args,
+ *                           unsigned int count)
+ *  
  * @args points to an array of a per-command data structure. The array
  * has @count members
  */
@@ -315,16 +334,19 @@ typedef uint16_t grant_status_t;
 #endif /* __XEN_INTERFACE_VERSION__ */
 /* ` } */
 
-/*
+/**
+ * typedef
  * Handle to track a mapping created via a grant reference.
  */
 typedef uint32_t grant_handle_t;
 
-/*
- * GNTTABOP_map_grant_ref: Map the grant entry (<dom>,<ref>) for access
- * by devices and/or host CPUs. If successful, <handle> is a tracking number
- * that must be presented later to destroy the mapping(s). On error, <status>
- * is a negative status code.
+/**
+ * struct gnttab_map_grant_ref - GNTTABOP_map_grant_ref
+ *
+ * Map the grant entry (<dom>,<ref>) for access by devices and/or host
+ * CPUs. If successful, <handle> is a tracking number that must be
+ * presented later to destroy the mapping(s). On error, <status> is a
+ * negative status code.
  * NOTES:
  *  1. If GNTMAP_device_map is specified then <dev_bus_addr> is the address
  *     via which I/O devices may access the granted frame.
@@ -338,24 +360,31 @@ typedef uint32_t grant_handle_t;
  *     to be accounted to the correct grant reference!
  */
 struct gnttab_map_grant_ref {
-    /* IN parameters. */
+    /** @host_addr: IN parameter */
     uint64_t host_addr;
-    uint32_t flags;               /* GNTMAP_* */
+    /** @flags: IN parameter, GNTMAP_* */
+    uint32_t flags;
+    /** @ref: IN parameter */
     grant_ref_t ref;
+    /** @dom: IN parameter */
     domid_t  dom;
-    /* OUT parameters. */
-    int16_t  status;              /* => enum grant_status */
+    /** @status: OUT parameter, enum grant_status */
+    int16_t  status;
+    /** @handle: OUT parameter */
     grant_handle_t handle;
+    /** @dev_bus_addr: OUT parameter */
     uint64_t dev_bus_addr;
 };
 typedef struct gnttab_map_grant_ref gnttab_map_grant_ref_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_map_grant_ref_t);
 
-/*
- * GNTTABOP_unmap_grant_ref: Destroy one or more grant-reference mappings
- * tracked by <handle>. If <host_addr> or <dev_bus_addr> is zero, that
- * field is ignored. If non-zero, they must refer to a device/host mapping
- * that is tracked by <handle>
+/**
+ * struct gnttab_unmap_grant_ref - GNTTABOP_unmap_grant_ref
+ *
+ * Destroy one or more grant-reference mappings tracked by <handle>. If
+ * <host_addr> or <dev_bus_addr> is zero, that field is ignored. If
+ * non-zero, they must refer to a device/host mapping that is tracked by
+ * <handle>
  * NOTES:
  *  1. The call may fail in an undefined manner if either mapping is not
  *     tracked by <handle>.
@@ -363,31 +392,37 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_map_grant_ref_t);
  *     mappings will remain in the device or host TLBs.
  */
 struct gnttab_unmap_grant_ref {
-    /* IN parameters. */
+    /** @host_addr: IN parameter */
     uint64_t host_addr;
+    /** @dev_bus_addr: IN parameter */
     uint64_t dev_bus_addr;
+    /** @handle: IN parameter */
     grant_handle_t handle;
-    /* OUT parameters. */
-    int16_t  status;              /* => enum grant_status */
+    /** @status: OUT parameter, enum grant_status */
+    int16_t  status;
 };
 typedef struct gnttab_unmap_grant_ref gnttab_unmap_grant_ref_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_unmap_grant_ref_t);
 
-/*
- * GNTTABOP_setup_table: Set up a grant table for <dom> comprising at least
- * <nr_frames> pages. The frame addresses are written to the <frame_list>.
- * Only <nr_frames> addresses are written, even if the table is larger.
+/**
+ * struct gnttab_setup_table - GNTTABOP_setup_table
+ *
+ * Set up a grant table for <dom> comprising at least <nr_frames> pages.
+ * The frame addresses are written to the <frame_list>.  Only
+ * <nr_frames> addresses are written, even if the table is larger.
  * NOTES:
  *  1. <dom> may be specified as DOMID_SELF.
  *  2. Only a sufficiently-privileged domain may specify <dom> != DOMID_SELF.
  *  3. Xen may not support more than a single grant-table page per domain.
  */
 struct gnttab_setup_table {
-    /* IN parameters. */
+    /** @dom: IN parameter */
     domid_t  dom;
+    /** @nr_frames: IN parameter */
     uint32_t nr_frames;
-    /* OUT parameters. */
+    /** @status: OUT parameter */
     int16_t  status;              /* => enum grant_status */
+    /** @frame_list: OUT parameter */
 #if __XEN_INTERFACE_VERSION__ < 0x00040300
     XEN_GUEST_HANDLE(ulong) frame_list;
 #else
@@ -397,21 +432,25 @@ struct gnttab_setup_table {
 typedef struct gnttab_setup_table gnttab_setup_table_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_setup_table_t);
 
-/*
- * GNTTABOP_dump_table: Dump the contents of the grant table to the
- * xen console. Debugging use only.
+/**
+ * struct gnttab_dump_table - GNTTABOP_dump_table
+ * 
+ * Dump the contents of the grant table to the xen console. Debugging
+ * use only.
  */
 struct gnttab_dump_table {
-    /* IN parameters. */
+    /** @dom: IN parameter */
     domid_t dom;
-    /* OUT parameters. */
-    int16_t status;               /* => enum grant_status */
+    /** @status: OUT parameter, enum grant_status */
+    int16_t status;
 };
 typedef struct gnttab_dump_table gnttab_dump_table_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_dump_table_t);
 
-/*
- * GNTTABOP_transfer: Transfer <frame> to a foreign domain. The foreign domain
+/**
+ * struct gnttab_transfer - GNTTABOP_transfer
+ *
+ * Transfer <frame> to a foreign domain. The foreign domain
  * has previously registered its interest in the transfer via <domid, ref>.
  *
  * Note that, even if the transfer fails, the specified page no longer belongs
@@ -420,19 +459,26 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_dump_table_t);
  * Note further that only PV guests can use this operation.
  */
 struct gnttab_transfer {
-    /* IN parameters. */
+    /** @mfn: IN parameter */
     xen_pfn_t     mfn;
+    /** @domid: IN parameter */
     domid_t       domid;
+    /** @ref: IN parameter */
     grant_ref_t   ref;
-    /* OUT parameters. */
+    /** @status: OUT parameter */
     int16_t       status;
 };
 typedef struct gnttab_transfer gnttab_transfer_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_transfer_t);
 
 
-/*
- * GNTTABOP_copy: Hypervisor based copy
+#define _GNTCOPY_source_gref      (0)
+#define GNTCOPY_source_gref       (1<<_GNTCOPY_source_gref)
+#define _GNTCOPY_dest_gref        (1)
+#define GNTCOPY_dest_gref         (1<<_GNTCOPY_dest_gref)
+/**
+ * struct gnttab_copy - GNTTABOP_copy, Hypervisor based copy
+ *
  * source and destinations can be eithers MFNs or, for foreign domains,
  * grant references. the foreign domain has to grant read/write access
  * in its grant table.
@@ -448,14 +494,9 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_transfer_t);
  * the offset in the target frame and  len specifies the number of
  * bytes to be copied.
  */
-
-#define _GNTCOPY_source_gref      (0)
-#define GNTCOPY_source_gref       (1<<_GNTCOPY_source_gref)
-#define _GNTCOPY_dest_gref        (1)
-#define GNTCOPY_dest_gref         (1<<_GNTCOPY_dest_gref)
-
 struct gnttab_copy {
-    /* IN parameters. */
+    /** @source: IN parameter */
+    /** @dest: IN parameter */
     struct gnttab_copy_ptr {
         union {
             grant_ref_t ref;
@@ -464,37 +505,44 @@ struct gnttab_copy {
         domid_t  domid;
         uint16_t offset;
     } source, dest;
+    /** @len: IN parameter */
     uint16_t      len;
-    uint16_t      flags;          /* GNTCOPY_* */
-    /* OUT parameters. */
+    /** @flags: IN parameter, GNTCOPY_* */
+    uint16_t      flags;
+    /** @status: OUT parameters. */
     int16_t       status;
 };
 typedef struct gnttab_copy  gnttab_copy_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_copy_t);
 
-/*
- * GNTTABOP_query_size: Query the current and maximum sizes of the shared
- * grant table.
+/**
+ * struct gnttab_query_size - GNTTABOP_query_size
+ *
+ * Query the current and maximum sizes of the shared grant table.
  * NOTES:
  *  1. <dom> may be specified as DOMID_SELF.
  *  2. Only a sufficiently-privileged domain may specify <dom> != DOMID_SELF.
  */
 struct gnttab_query_size {
-    /* IN parameters. */
+    /** @dom: IN parameter */
     domid_t  dom;
-    /* OUT parameters. */
+    /** @nr_frames: OUT parameter */
     uint32_t nr_frames;
+    /** @max_nr_frames: OUT parameter */
     uint32_t max_nr_frames;
-    int16_t  status;              /* => enum grant_status */
+    /** @status: OUT parameter, enum grant_status */
+    int16_t  status;
 };
 typedef struct gnttab_query_size gnttab_query_size_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_query_size_t);
 
-/*
- * GNTTABOP_unmap_and_replace: Destroy one or more grant-reference mappings
- * tracked by <handle> but atomically replace the page table entry with one
- * pointing to the machine address under <new_addr>.  <new_addr> will be
- * redirected to the null entry.
+/**
+ * struct gnttab_unmap_and_replace - GNTTABOP_unmap_and_replace
+ *
+ * Destroy one or more grant-reference mappings tracked by <handle> but
+ * atomically replace the page table entry with one pointing to the
+ * machine address under <new_addr>.  <new_addr> will be redirected to
+ * the null entry.
  * NOTES:
  *  1. The call may fail in an undefined manner if either mapping is not
  *     tracked by <handle>.
@@ -502,36 +550,42 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_query_size_t);
  *     mappings will remain in the device or host TLBs.
  */
 struct gnttab_unmap_and_replace {
-    /* IN parameters. */
+    /** @host_addr: IN parameter */
     uint64_t host_addr;
+    /** @new_addr: IN parameter */
     uint64_t new_addr;
+    /** @handle: IN parameter */
     grant_handle_t handle;
-    /* OUT parameters. */
-    int16_t  status;              /* => enum grant_status */
+    /** @status: OUT parameter, enum grant_status */
+    int16_t  status;
 };
 typedef struct gnttab_unmap_and_replace gnttab_unmap_and_replace_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_unmap_and_replace_t);
 
 #if __XEN_INTERFACE_VERSION__ >= 0x0003020a
-/*
- * GNTTABOP_set_version: Request a particular version of the grant
- * table shared table structure.  This operation may be used to toggle
- * between different versions, but must be performed while no grants
- * are active.  The only defined versions are 1 and 2.
+/**
+ * struct gnttab_set_version - GNTTABOP_set_version
+ *
+ * Request a particular version of the grant table shared table
+ * structure.  This operation may be used to toggle between different
+ * versions, but must be performed while no grants are active.  The only
+ * defined versions are 1 and 2.
  */
 struct gnttab_set_version {
-    /* IN/OUT parameters */
+    /** @version: IN/OUT parameter */
     uint32_t version;
 };
 typedef struct gnttab_set_version gnttab_set_version_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_set_version_t);
 
 
-/*
- * GNTTABOP_get_status_frames: Get the list of frames used to store grant
- * status for <dom>. In grant format version 2, the status is separated
- * from the other shared grant fields to allow more efficient synchronization
- * using barriers instead of atomic cmpexch operations.
+/**
+ * struct gnttab_get_status_frames - GNTTABOP_get_status_frames
+ *
+ * Get the list of frames used to store grant status for <dom>. In grant
+ * format version 2, the status is separated from the other shared grant
+ * fields to allow more efficient synchronization using barriers instead
+ * of atomic cmpexch operations.
  * <nr_frames> specify the size of vector <frame_list>.
  * The frame addresses are returned in the <frame_list>.
  * Only <nr_frames> addresses are returned, even if the table is larger.
@@ -540,44 +594,53 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_set_version_t);
  *  2. Only a sufficiently-privileged domain may specify <dom> != DOMID_SELF.
  */
 struct gnttab_get_status_frames {
-    /* IN parameters. */
+    /** @nr_frames: IN parameter */
     uint32_t nr_frames;
+    /** @dom: IN parameter */
     domid_t  dom;
-    /* OUT parameters. */
-    int16_t  status;              /* => enum grant_status */
+    /** @status: OUT parameter, enum grant_status */
+    int16_t  status;
+    /** @frame_list: OUT parameter */
     XEN_GUEST_HANDLE(uint64_t) frame_list;
 };
 typedef struct gnttab_get_status_frames gnttab_get_status_frames_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_get_status_frames_t);
 
-/*
- * GNTTABOP_get_version: Get the grant table version which is in
- * effect for domain <dom>.
+/**
+ * struct gnttab_get_version - GNTTABOP_get_version
+ *
+ * Get the grant table version which is in effect for domain <dom>.
  */
 struct gnttab_get_version {
-    /* IN parameters */
+    /** @dom: IN parameters */
     domid_t dom;
+    /** @pad: padding */
     uint16_t pad;
-    /* OUT parameters */
+    /** @version: OUT parameter */
     uint32_t version;
 };
 typedef struct gnttab_get_version gnttab_get_version_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_get_version_t);
 
-/*
- * GNTTABOP_swap_grant_ref: Swap the contents of two grant entries.
+/**
+ * struct gnttab_swap_grant_ref - GNTTABOP_swap_grant_ref
+ *
+ * Swap the contents of two grant entries.
  */
 struct gnttab_swap_grant_ref {
-    /* IN parameters */
+    /** @ref_a: IN parameter */
     grant_ref_t ref_a;
+    /** @ref_b: IN parameter */
     grant_ref_t ref_b;
-    /* OUT parameters */
-    int16_t status;             /* => enum grant_status */
+    /** @status: OUT parameter, enum grant_status */
+    int16_t status;
 };
 typedef struct gnttab_swap_grant_ref gnttab_swap_grant_ref_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t);
 
-/*
+/**
+ * struct gnttab_cache_flush - GNTTABOP_cache_flush
+ *
  * Issue one or more cache maintenance operations on a portion of a
  * page granted to the calling domain by a foreign domain.
  */
@@ -598,61 +661,69 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
 
 #endif /* __XEN_INTERFACE_VERSION__ */
 
-/*
- * Bitfield values for gnttab_map_grant_ref.flags.
+/**
+ * DOC: Bitfield values for gnttab_map_grant_ref.flags.
+ *
+ * - GNTMAP_device_map: Map the grant entry for access by I/O devices.
+ * - GNTMAP_host_map: Map the grant entry for access by host CPUs.
+ * - GNTMAP_readonly: Accesses to the granted frame will be restricted to read-only access.
+ * - GNTMAP_host_map: 0 => The host mapping is usable only by the guest OS
+ *                    1 => The host mapping is usable by guest OS + current application.
+ * - GNTMAP_contains_pte: 0 => This map request contains a host virtual address.
+ *                        1 => This map request contains the machine addess of the PTE to update.
+ * - GNTMAP_can_fail
+ * - GNTMAP_guest_avail_mask: Bits to be placed in guest kernel available PTE bits
+ *                            (architecture dependent; only supported when
+ *                             XENFEAT_gnttab_map_avail_bits is set).
  */
- /* Map the grant entry for access by I/O devices. */
 #define _GNTMAP_device_map      (0)
 #define GNTMAP_device_map       (1<<_GNTMAP_device_map)
- /* Map the grant entry for access by host CPUs. */
 #define _GNTMAP_host_map        (1)
 #define GNTMAP_host_map         (1<<_GNTMAP_host_map)
- /* Accesses to the granted frame will be restricted to read-only access. */
 #define _GNTMAP_readonly        (2)
 #define GNTMAP_readonly         (1<<_GNTMAP_readonly)
- /*
-  * GNTMAP_host_map subflag:
-  *  0 => The host mapping is usable only by the guest OS.
-  *  1 => The host mapping is usable by guest OS + current application.
-  */
 #define _GNTMAP_application_map (3)
 #define GNTMAP_application_map  (1<<_GNTMAP_application_map)
-
- /*
-  * GNTMAP_contains_pte subflag:
-  *  0 => This map request contains a host virtual address.
-  *  1 => This map request contains the machine addess of the PTE to update.
-  */
 #define _GNTMAP_contains_pte    (4)
 #define GNTMAP_contains_pte     (1<<_GNTMAP_contains_pte)
-
 #define _GNTMAP_can_fail        (5)
 #define GNTMAP_can_fail         (1<<_GNTMAP_can_fail)
-
-/*
- * Bits to be placed in guest kernel available PTE bits (architecture
- * dependent; only supported when XENFEAT_gnttab_map_avail_bits is set).
- */
 #define _GNTMAP_guest_avail0    (16)
 #define GNTMAP_guest_avail_mask ((uint32_t)~0 << _GNTMAP_guest_avail0)
 
-/*
- * Values for error status returns. All errors are -ve.
+/**
+ * DOC: Values for error status returns. All errors are -ve.
+ *
+ *
+ * - GNTST_okay: Normal return.
+ * - GNTST_general_error: General undefined error.
+ * - GNTST_bad_domain: Unrecognsed domain id.
+ * - GNTST_bad_gntref: Unrecognised or inappropriate gntref.
+ * - GNTST_bad_handle: Unrecognised or inappropriate handle.
+ * - GNTST_bad_virt_addr: Inappropriate virtual address to map.
+ * - GNTST_bad_dev_addr: Inappropriate device address to unmap.
+ * - GNTST_no_device_space: Out of space in I/O MMU.
+ * - GNTST_permission_denied: Not enough privilege for operation.
+ * - GNTST_bad_page: Specified page was invalid for op.
+ * - GNTST_bad_copy_arg: copy arguments cross page boundary.
+ * - GNTST_address_too_big: transfer page address too large.
+ * - GNTST_eagain: Operation not done; try again.
+ *
  */
 /* ` enum grant_status { */
-#define GNTST_okay             (0)  /* Normal return.                        */
-#define GNTST_general_error    (-1) /* General undefined error.              */
-#define GNTST_bad_domain       (-2) /* Unrecognsed domain id.                */
-#define GNTST_bad_gntref       (-3) /* Unrecognised or inappropriate gntref. */
-#define GNTST_bad_handle       (-4) /* Unrecognised or inappropriate handle. */
-#define GNTST_bad_virt_addr    (-5) /* Inappropriate virtual address to map. */
-#define GNTST_bad_dev_addr     (-6) /* Inappropriate device address to unmap.*/
-#define GNTST_no_device_space  (-7) /* Out of space in I/O MMU.              */
-#define GNTST_permission_denied (-8) /* Not enough privilege for operation.  */
-#define GNTST_bad_page         (-9) /* Specified page was invalid for op.    */
-#define GNTST_bad_copy_arg    (-10) /* copy arguments cross page boundary.   */
-#define GNTST_address_too_big (-11) /* transfer page address too large.      */
-#define GNTST_eagain          (-12) /* Operation not done; try again.        */
+#define GNTST_okay             (0)
+#define GNTST_general_error    (-1)
+#define GNTST_bad_domain       (-2)
+#define GNTST_bad_gntref       (-3)
+#define GNTST_bad_handle       (-4)
+#define GNTST_bad_virt_addr    (-5)
+#define GNTST_bad_dev_addr     (-6)
+#define GNTST_no_device_space  (-7)
+#define GNTST_permission_denied (-8)
+#define GNTST_bad_page         (-9)
+#define GNTST_bad_copy_arg    (-10)
+#define GNTST_address_too_big (-11)
+#define GNTST_eagain          (-12)
 /* ` } */
 
 #define GNTTABOP_error_msgs {                   \
-- 
2.17.1

Re: [PATCH 06/14] kernel-doc: public/grant_table.h

[Jan Beulich]

On 07.08.2020 01:49, Stefano Stabellini wrote:
> @@ -108,56 +107,66 @@
>   *  Use SMP-safe bit-setting instruction.
>   */
>  
> -/*
> +/**
> + * typedef
>   * Reference to a grant entry in a specified domain's grant table.
>   */
>  typedef uint32_t grant_ref_t;

In the comment, did you mean grant_ref_t?

> -/*
> +/**
> + * DOC: grant table
> + *
>   * A grant table comprises a packed array of grant entries in one or more
>   * page frames shared between Xen and a guest.
>   * [XEN]: This field is written by Xen and read by the sharing guest.
>   * [GST]: This field is written by the guest and read by Xen.
>   */
>  
> -/*
> - * Version 1 of the grant table entry structure is maintained purely
> - * for backwards compatibility.  New guests should use version 2.
> - */
>  #if __XEN_INTERFACE_VERSION__ < 0x0003020a
>  #define grant_entry_v1 grant_entry
>  #define grant_entry_v1_t grant_entry_t
>  #endif
> +/**
> + * struct grant_entry_v1
> + *
> + * Version 1 of the grant table entry structure is maintained purely
> + * for backwards compatibility.  New guests should use version 2.
> + */

I realize content changes aren't intended, but here I can't resist
recommending to drop the second sentence at this occasion.

> +/**
> + * DOC: Values for error status returns. All errors are -ve.
> + *
> + *
> + * - GNTST_okay: Normal return.

Nit: Why two blank comment lines above?

Jan

Re: [PATCH 06/14] kernel-doc: public/grant_table.h

[Stefano Stabellini]

On Fri, 7 Aug 2020, Jan Beulich wrote:
> On 07.08.2020 01:49, Stefano Stabellini wrote:
> > @@ -108,56 +107,66 @@
> >   *  Use SMP-safe bit-setting instruction.
> >   */
> >  
> > -/*
> > +/**
> > + * typedef
> >   * Reference to a grant entry in a specified domain's grant table.
> >   */
> >  typedef uint32_t grant_ref_t;
> 
> In the comment, did you mean grant_ref_t?

Yes, I'll fix it.


> > -/*
> > +/**
> > + * DOC: grant table
> > + *
> >   * A grant table comprises a packed array of grant entries in one or more
> >   * page frames shared between Xen and a guest.
> >   * [XEN]: This field is written by Xen and read by the sharing guest.
> >   * [GST]: This field is written by the guest and read by Xen.
> >   */
> >  
> > -/*
> > - * Version 1 of the grant table entry structure is maintained purely
> > - * for backwards compatibility.  New guests should use version 2.
> > - */
> >  #if __XEN_INTERFACE_VERSION__ < 0x0003020a
> >  #define grant_entry_v1 grant_entry
> >  #define grant_entry_v1_t grant_entry_t
> >  #endif
> > +/**
> > + * struct grant_entry_v1
> > + *
> > + * Version 1 of the grant table entry structure is maintained purely
> > + * for backwards compatibility.  New guests should use version 2.
> > + */
> 
> I realize content changes aren't intended, but here I can't resist
> recommending to drop the second sentence at this occasion.

OK


> > +/**
> > + * DOC: Values for error status returns. All errors are -ve.
> > + *
> > + *
> > + * - GNTST_okay: Normal return.
> 
> Nit: Why two blank comment lines above?

A small mistake, I'll fix it.

[PATCH 07/14] kernel-doc: public/hypfs.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/hypfs.h | 72 ++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 27 deletions(-)

diff --git a/xen/include/public/hypfs.h b/xen/include/public/hypfs.h
index 63a5df1629..ef33aee4ce 100644
--- a/xen/include/public/hypfs.h
+++ b/xen/include/public/hypfs.h
@@ -32,12 +32,21 @@
  * Definitions for the __HYPERVISOR_hypfs_op hypercall.
  */
 
-/* Highest version number of the hypfs interface currently defined. */
+/**
+ * DOC: XEN_HYPFS_VERSION
+ * Highest version number of the hypfs interface currently defined.
+ */
 #define XEN_HYPFS_VERSION      1
 
-/* Maximum length of a path in the filesystem. */
+/**
+ * DOC: XEN_HYPFS_MAX_PATHLEN
+ * Maximum length of a path in the filesystem.
+ */
 #define XEN_HYPFS_MAX_PATHLEN  1024
 
+/**
+ * struct xen_hypfs_direntry
+ */
 struct xen_hypfs_direntry {
     uint8_t type;
 #define XEN_HYPFS_TYPE_DIR     0
@@ -49,16 +58,22 @@ struct xen_hypfs_direntry {
     uint8_t encoding;
 #define XEN_HYPFS_ENC_PLAIN    0
 #define XEN_HYPFS_ENC_GZIP     1
-    uint16_t pad;              /* Returned as 0. */
-    uint32_t content_len;      /* Current length of data. */
-    uint32_t max_write_len;    /* Max. length for writes (0 if read-only). */
+    /** @pad: Returned as 0. */
+    uint16_t pad;
+    /** @content_len: Current length of data. */
+    uint32_t content_len;
+    /** @max_write_len: Max. length for writes (0 if read-only). */
+    uint32_t max_write_len;
 };
 
+/**
+ * struct xen_hypfs_dirlistentry
+ */
 struct xen_hypfs_dirlistentry {
     struct xen_hypfs_direntry e;
-    /* Offset in bytes to next entry (0 == this is the last entry). */
+    /** @off_next: Offset in bytes to next entry (0 == this is the last entry). */
     uint16_t off_next;
-    /* Zero terminated entry name, possibly with some padding for alignment. */
+    /** @name: Zero terminated entry name, possibly with some padding for alignment. */
     char name[XEN_FLEX_ARRAY_DIM];
 };
 
@@ -66,21 +81,22 @@ struct xen_hypfs_dirlistentry {
  * Hypercall operations.
  */
 
-/*
- * XEN_HYPFS_OP_get_version
+/**
+ * DOC: XEN_HYPFS_OP_get_version
  *
  * Read highest interface version supported by the hypervisor.
  *
  * arg1 - arg4: all 0/NULL
  *
  * Possible return values:
- * >0: highest supported interface version
- * <0: negative Xen errno value
+ *
+ * - >0: highest supported interface version
+ * - <0: negative Xen errno value
  */
 #define XEN_HYPFS_OP_get_version     0
 
-/*
- * XEN_HYPFS_OP_read
+/**
+ * DOC: XEN_HYPFS_OP_read
  *
  * Read a filesystem entry.
  *
@@ -95,19 +111,20 @@ struct xen_hypfs_dirlistentry {
  * The contents of a directory are multiple struct xen_hypfs_dirlistentry
  * items.
  *
- * arg1: XEN_GUEST_HANDLE(path name)
- * arg2: length of path name (including trailing zero byte)
- * arg3: XEN_GUEST_HANDLE(data buffer written by hypervisor)
- * arg4: data buffer size
+ * - arg1: XEN_GUEST_HANDLE(path name)
+ * - arg2: length of path name (including trailing zero byte)
+ * - arg3: XEN_GUEST_HANDLE(data buffer written by hypervisor)
+ * - arg4: data buffer size
  *
  * Possible return values:
- * 0: success
- * <0 : negative Xen errno value
+ *
+ * - 0: success
+ * - <0 : negative Xen errno value
  */
 #define XEN_HYPFS_OP_read              1
 
-/*
- * XEN_HYPFS_OP_write_contents
+/**
+ * DOC: XEN_HYPFS_OP_write_contents
  *
  * Write contents of a filesystem entry.
  *
@@ -115,14 +132,15 @@ struct xen_hypfs_dirlistentry {
  * The data type and encoding can't be changed. The size can be changed only
  * for blobs and strings.
  *
- * arg1: XEN_GUEST_HANDLE(path name)
- * arg2: length of path name (including trailing zero byte)
- * arg3: XEN_GUEST_HANDLE(content buffer read by hypervisor)
- * arg4: content buffer size
+ * - arg1: XEN_GUEST_HANDLE(path name)
+ * - arg2: length of path name (including trailing zero byte)
+ * - arg3: XEN_GUEST_HANDLE(content buffer read by hypervisor)
+ * - arg4: content buffer size
  *
  * Possible return values:
- * 0: success
- * <0 : negative Xen errno value
+ *
+ * - 0: success
+ * - <0: negative Xen errno value
  */
 #define XEN_HYPFS_OP_write_contents    2
 
-- 
2.17.1

[PATCH 08/14] kernel-doc: public/memory.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/memory.h | 232 ++++++++++++++++++++++++------------
 1 file changed, 155 insertions(+), 77 deletions(-)

diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 21057ed78e..4c57ed213c 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -30,7 +30,9 @@
 #include "xen.h"
 #include "physdev.h"
 
-/*
+/**
+ * DOC: XENMEM_increase_reservation and XENMEM_decrease_reservation
+ *
  * Increase or decrease the specified domain's memory reservation. Returns the
  * number of extents successfully allocated or freed.
  * arg == addr of struct xen_memory_reservation.
@@ -40,29 +42,37 @@
 #define XENMEM_populate_physmap     6
 
 #if __XEN_INTERFACE_VERSION__ >= 0x00030209
-/*
- * Maximum # bits addressable by the user of the allocated region (e.g., I/O
- * devices often have a 32-bit limitation even in 64-bit systems). If zero
- * then the user has no addressing restriction. This field is not used by
- * XENMEM_decrease_reservation.
+/**
+ * DOC: XENMEMF_*
+ *
+ * - XENMEMF_address_bits, XENMEMF_get_address_bits:
+ *       Maximum # bits addressable by the user of the allocated region
+ *       (e.g., I/O devices often have a 32-bit limitation even in 64-bit
+ *       systems). If zero then the user has no addressing restriction. This
+ *       field is not used by XENMEM_decrease_reservation.
+ * - XENMEMF_node, XENMEMF_get_node: NUMA node to allocate from
+ * - XENMEMF_populate_on_demand: Flag to populate physmap with populate-on-demand entries
+ * - XENMEMF_exact_node_request, XENMEMF_exact_node: Flag to request allocation only from the node specified
+ * - XENMEMF_vnode: Flag to indicate the node specified is virtual node
  */
 #define XENMEMF_address_bits(x)     (x)
 #define XENMEMF_get_address_bits(x) ((x) & 0xffu)
-/* NUMA node to allocate from. */
 #define XENMEMF_node(x)     (((x) + 1) << 8)
 #define XENMEMF_get_node(x) ((((x) >> 8) - 1) & 0xffu)
-/* Flag to populate physmap with populate-on-demand entries */
 #define XENMEMF_populate_on_demand (1<<16)
-/* Flag to request allocation only from the node specified */
 #define XENMEMF_exact_node_request  (1<<17)
 #define XENMEMF_exact_node(n) (XENMEMF_node(n) | XENMEMF_exact_node_request)
-/* Flag to indicate the node specified is virtual node */
 #define XENMEMF_vnode  (1<<18)
 #endif
 
+/**
+ * struct xen_memory_reservation
+ */
 struct xen_memory_reservation {
 
-    /*
+    /**
+     * @extent_start:
+     *
      * XENMEM_increase_reservation:
      *   OUT: MFN (*not* GMFN) bases of extents that were allocated
      * XENMEM_decrease_reservation:
@@ -76,18 +86,20 @@ struct xen_memory_reservation {
      */
     XEN_GUEST_HANDLE(xen_pfn_t) extent_start;
 
-    /* Number of extents, and size/alignment of each (2^extent_order pages). */
+    /** @nr_extents: Number of extents, and size/alignment of each (2^extent_order pages). */
     xen_ulong_t    nr_extents;
     unsigned int   extent_order;
 
 #if __XEN_INTERFACE_VERSION__ >= 0x00030209
-    /* XENMEMF flags. */
+    /** @mem_flags: XENMEMF flags. */
     unsigned int   mem_flags;
 #else
     unsigned int   address_bits;
 #endif
 
-    /*
+    /**
+     * @domid:
+     *
      * Domain whose reservation is being changed.
      * Unprivileged domains can specify only DOMID_SELF.
      */
@@ -96,7 +108,10 @@ struct xen_memory_reservation {
 typedef struct xen_memory_reservation xen_memory_reservation_t;
 DEFINE_XEN_GUEST_HANDLE(xen_memory_reservation_t);
 
-/*
+#define XENMEM_exchange             11
+/**
+ * struct xen_memory_exchange - XENMEM_exchange
+ *
  * An atomic exchange of memory pages. If return code is zero then
  * @out.extent_list provides GMFNs of the newly-allocated memory.
  * Returns zero on complete success, otherwise a negative error code.
@@ -105,15 +120,18 @@ DEFINE_XEN_GUEST_HANDLE(xen_memory_reservation_t);
  *
  * Note that only PV guests can use this operation.
  */
-#define XENMEM_exchange             11
 struct xen_memory_exchange {
-    /*
+    /**
+     * @in:
+     *
      * [IN] Details of memory extents to be exchanged (GMFN bases).
      * Note that @in.address_bits is ignored and unused.
      */
     struct xen_memory_reservation in;
 
-    /*
+    /**
+     * @out:
+     *
      * [IN/OUT] Details of new memory extents.
      * We require that:
      *  1. @in.domid == @out.domid
@@ -125,7 +143,9 @@ struct xen_memory_exchange {
      */
     struct xen_memory_reservation out;
 
-    /*
+    /**
+     * @nr_exchanged:
+     *
      * [OUT] Number of input extents that were successfully exchanged:
      *  1. The first @nr_exchanged input extents were successfully
      *     deallocated.
@@ -141,14 +161,18 @@ struct xen_memory_exchange {
 typedef struct xen_memory_exchange xen_memory_exchange_t;
 DEFINE_XEN_GUEST_HANDLE(xen_memory_exchange_t);
 
-/*
+/**
+ * DOC: XENMEM_maximum_ram_page
+ *
  * Returns the maximum machine frame number of mapped RAM in this system.
  * This command always succeeds (it never returns an error code).
  * arg == NULL.
  */
 #define XENMEM_maximum_ram_page     2
 
-/*
+/**
+ * DOC: XENMEM_current_reservation and XENMEM_maximum_reservation
+ *
  * Returns the current or maximum memory reservation, in pages, of the
  * specified domain (may be DOMID_SELF). Returns -ve errcode on failure.
  * arg == addr of domid_t.
@@ -156,33 +180,43 @@ DEFINE_XEN_GUEST_HANDLE(xen_memory_exchange_t);
 #define XENMEM_current_reservation  3
 #define XENMEM_maximum_reservation  4
 
-/*
+/**
+ * DOC: XENMEM_maximum_gpfn
+ *
  * Returns the maximum GPFN in use by the guest, or -ve errcode on failure.
  */
 #define XENMEM_maximum_gpfn         14
 
-/*
+#define XENMEM_machphys_mfn_list    5
+/**
+ * struct xen_machphys_mfn_list - XENMEM_machphys_mfn_list
+ *
  * Returns a list of MFN bases of 2MB extents comprising the machine_to_phys
  * mapping table. Architectures which do not have a m2p table do not implement
  * this command.
  * arg == addr of xen_machphys_mfn_list_t.
  */
-#define XENMEM_machphys_mfn_list    5
 struct xen_machphys_mfn_list {
-    /*
+    /**
+     * @max_extents:
+     *
      * Size of the 'extent_start' array. Fewer entries will be filled if the
      * machphys table is smaller than max_extents * 2MB.
      */
     unsigned int max_extents;
 
-    /*
+    /**
+     * @extent_start:
+     *
      * Pointer to buffer to fill with list of extent starts. If there are
      * any large discontiguities in the machine address space, 2MB gaps in
      * the machphys table will be represented by an MFN base of zero.
      */
     XEN_GUEST_HANDLE(xen_pfn_t) extent_start;
 
-    /*
+    /**
+     * @nr_extents:
+     *
      * Number of extents written to the above array. This will be smaller
      * than 'max_extents' if the machphys table is smaller than max_e * 2MB.
      */
@@ -191,7 +225,9 @@ struct xen_machphys_mfn_list {
 typedef struct xen_machphys_mfn_list xen_machphys_mfn_list_t;
 DEFINE_XEN_GUEST_HANDLE(xen_machphys_mfn_list_t);
 
-/*
+/**
+ * DOC: XENMEM_machphys_compat_mfn_list
+ *
  * For a compat caller, this is identical to XENMEM_machphys_mfn_list.
  *
  * For a non compat caller, this functions similarly to
@@ -200,90 +236,115 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mfn_list_t);
  */
 #define XENMEM_machphys_compat_mfn_list     25
 
-/*
+#define XENMEM_machphys_mapping     12
+/**
+ * struct xen_machphys_mapping - XENMEM_machphys_mapping
+ *
  * Returns the location in virtual address space of the machine_to_phys
  * mapping table. Architectures which do not have a m2p table, or which do not
  * map it by default into guest address space, do not implement this command.
  * arg == addr of xen_machphys_mapping_t.
  */
-#define XENMEM_machphys_mapping     12
 struct xen_machphys_mapping {
+    /** @v_start: Start virtual address */
     xen_ulong_t v_start, v_end; /* Start and end virtual addresses.   */
-    xen_ulong_t max_mfn;        /* Maximum MFN that can be looked up. */
+    /** @v_end: End virtual addresses */
+    xen_ulong_t v_end;
+    /** @max_mfn: Maximum MFN that can be looked up */
+    xen_ulong_t max_mfn;
 };
 typedef struct xen_machphys_mapping xen_machphys_mapping_t;
 DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
 
-/* Source mapping space. */
+/**
+ * DOC: Source mapping space.
+ *
+ * - XENMAPSPACE_shared_info:  shared info page
+ * - XENMAPSPACE_grant_table:  grant table page
+ * - XENMAPSPACE_gmfn:         GMFN
+ * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
+ * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
+ *                             XENMEM_add_to_physmap_batch only.
+ * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
+ *                             in Stage-2 using the Normal MemoryInner/Outer
+ *                             Write-Back Cacheable memory attribute.
+ */
 /* ` enum phys_map_space { */
-#define XENMAPSPACE_shared_info  0 /* shared info page */
-#define XENMAPSPACE_grant_table  1 /* grant table page */
-#define XENMAPSPACE_gmfn         2 /* GMFN */
-#define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap only. */
-#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
-                                    * XENMEM_add_to_physmap_batch only. */
-#define XENMAPSPACE_dev_mmio     5 /* device mmio region
-                                      ARM only; the region is mapped in
-                                      Stage-2 using the Normal Memory
-                                      Inner/Outer Write-Back Cacheable
-                                      memory attribute. */
+#define XENMAPSPACE_shared_info  0
+#define XENMAPSPACE_grant_table  1
+#define XENMAPSPACE_gmfn         2
+#define XENMAPSPACE_gmfn_range   3
+#define XENMAPSPACE_gmfn_foreign 4
+#define XENMAPSPACE_dev_mmio     5
 /* ` } */
 
-/*
+#define XENMEM_add_to_physmap      7
+/**
+ * struct xen_add_to_physmap - XENMEM_add_to_physmap
+ *
  * Sets the GPFN at which a particular page appears in the specified guest's
  * physical address space (translated guests only).
  * arg == addr of xen_add_to_physmap_t.
  */
-#define XENMEM_add_to_physmap      7
 struct xen_add_to_physmap {
-    /* Which domain to change the mapping for. */
+    /** @domid: Which domain to change the mapping for. */
     domid_t domid;
 
-    /* Number of pages to go through for gmfn_range */
+    /** @size: Number of pages to go through for gmfn_range */
     uint16_t    size;
 
-    unsigned int space; /* => enum phys_map_space */
+    /** @space: enum phys_map_space */
+    unsigned int space;
 
 #define XENMAPIDX_grant_table_status 0x80000000
 
-    /* Index into space being mapped. */
+    /** @idx: Index into space being mapped. */
     xen_ulong_t idx;
 
-    /* GPFN in domid where the source mapping page should appear. */
+    /** @gpfn: GPFN in domid where the source mapping page should appear. */
     xen_pfn_t     gpfn;
 };
 typedef struct xen_add_to_physmap xen_add_to_physmap_t;
 DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_t);
 
-/* A batched version of add_to_physmap. */
 #define XENMEM_add_to_physmap_batch 23
+/**
+ * struct xen_add_to_physmap_batch - XENMEM_add_to_physmap_batch
+ *
+ * A batched version of add_to_physmap.
+ */
 struct xen_add_to_physmap_batch {
-    /* IN */
-    /* Which domain to change the mapping for. */
+    /** @domid: IN, which domain to change the mapping for */
     domid_t domid;
-    uint16_t space; /* => enum phys_map_space */
+    /** @space: IN, enum phys_map_space */
+    uint16_t space;
 
-    /* Number of pages to go through */
+    /** @size: IN, Number of pages to go through */
     uint16_t size;
 
 #if __XEN_INTERFACE_VERSION__ < 0x00040700
-    domid_t foreign_domid; /* IFF gmfn_foreign. Should be 0 for other spaces. */
+    /** @foreign_domid: IN, iff gmfn_foreign. Should be 0 for other spaces. */
+    domid_t foreign_domid;
 #else
+    /**
+     * @u: IN
+     *
+     * - u.foreign_domid: gmfn_foreign
+     * - u.res0: All the other spaces. Should be 0
+     */
     union xen_add_to_physmap_batch_extra {
-        domid_t foreign_domid; /* gmfn_foreign */
-        uint16_t res0;  /* All the other spaces. Should be 0 */
+        domid_t foreign_domid;
+        uint16_t res0;
     } u;
 #endif
 
-    /* Indexes into space being mapped. */
+    /** @idxs: IN, Indexes into space being mapped. */
     XEN_GUEST_HANDLE(xen_ulong_t) idxs;
 
-    /* GPFN in domid where the source mapping page should appear. */
+    /** @gpfns: IN, GPFN in domid where the source mapping page should appear. */
     XEN_GUEST_HANDLE(xen_pfn_t) gpfns;
 
-    /* OUT */
-
-    /* Per index error code. */
+    /** @errs: OUT, Per index error code. */
     XEN_GUEST_HANDLE(int) errs;
 };
 typedef struct xen_add_to_physmap_batch xen_add_to_physmap_batch_t;
@@ -296,17 +357,19 @@ typedef struct xen_add_to_physmap_batch xen_add_to_physmap_range_t;
 DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_range_t);
 #endif
 
-/*
+#define XENMEM_remove_from_physmap      15
+/**
+ * struct xen_remove_from_physmap - XENMEM_remove_from_physmap
+ *
  * Unmaps the page appearing at a particular GPFN from the specified guest's
  * physical address space (translated guests only).
  * arg == addr of xen_remove_from_physmap_t.
  */
-#define XENMEM_remove_from_physmap      15
 struct xen_remove_from_physmap {
-    /* Which domain to change the mapping for. */
+    /** @domid: Which domain to change the mapping for. */
     domid_t domid;
 
-    /* GPFN of the current mapping of the page. */
+    /** @gpfn: GPFN of the current mapping of the page. */
     xen_pfn_t     gpfn;
 };
 typedef struct xen_remove_from_physmap xen_remove_from_physmap_t;
@@ -315,21 +378,27 @@ DEFINE_XEN_GUEST_HANDLE(xen_remove_from_physmap_t);
 /*** REMOVED ***/
 /*#define XENMEM_translate_gpfn_list  8*/
 
-/*
+#define XENMEM_memory_map           9
+/**
+ * struct xen_memory_map - XENMEM_memory_map
+ *
  * Returns the pseudo-physical memory map as it was when the domain
  * was started (specified by XENMEM_set_memory_map).
  * arg == addr of xen_memory_map_t.
  */
-#define XENMEM_memory_map           9
 struct xen_memory_map {
-    /*
+    /**
+     * @nr_entries:
+     *
      * On call the number of entries which can be stored in buffer. On
      * return the number of entries which have been stored in
      * buffer.
      */
     unsigned int nr_entries;
 
-    /*
+    /**
+     * @buffer:
+     *
      * Entries in the buffer are in the same format as returned by the
      * BIOS INT 0x15 EAX=0xE820 call.
      */
@@ -338,7 +407,9 @@ struct xen_memory_map {
 typedef struct xen_memory_map xen_memory_map_t;
 DEFINE_XEN_GUEST_HANDLE(xen_memory_map_t);
 
-/*
+/**
+ * DOC: XENMEM_machine_memory_map
+ *
  * Returns the real physical memory map. Passes the same structure as
  * XENMEM_memory_map.
  * Specifying buffer as NULL will return the number of entries required
@@ -347,12 +418,14 @@ DEFINE_XEN_GUEST_HANDLE(xen_memory_map_t);
  */
 #define XENMEM_machine_memory_map   10
 
-/*
+#define XENMEM_set_memory_map       13
+/**
+ * struct xen_foreign_memory_map - XENMEM_set_memory_map
+ *
  * Set the pseudo-physical memory map of a domain, as returned by
  * XENMEM_memory_map.
  * arg == addr of xen_foreign_memory_map_t.
  */
-#define XENMEM_set_memory_map       13
 struct xen_foreign_memory_map {
     domid_t domid;
     struct xen_memory_map map;
@@ -362,14 +435,19 @@ DEFINE_XEN_GUEST_HANDLE(xen_foreign_memory_map_t);
 
 #define XENMEM_set_pod_target       16
 #define XENMEM_get_pod_target       17
+/**
+ * struct xen_pod_target - XENMEM_set_pod_target and XENMEM_get_pod_target
+ */
 struct xen_pod_target {
-    /* IN */
+    /** @target_pages: IN */
     uint64_t target_pages;
-    /* OUT */
+    /** @tot_pages: OUT */
     uint64_t tot_pages;
+    /** @pod_cache_pages: OUT */
     uint64_t pod_cache_pages;
+    /** @pod_entries: OUT */
     uint64_t pod_entries;
-    /* IN */
+    /** @domid: IN */
     domid_t domid;
 };
 typedef struct xen_pod_target xen_pod_target_t;
-- 
2.17.1

Re: [PATCH 08/14] kernel-doc: public/memory.h

[Jan Beulich]

On 07.08.2020 01:49, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> Convert in-code comments to kernel-doc format wherever possible.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> ---
>  xen/include/public/memory.h | 232 ++++++++++++++++++++++++------------
>  1 file changed, 155 insertions(+), 77 deletions(-)
> 
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index 21057ed78e..4c57ed213c 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -30,7 +30,9 @@
>  #include "xen.h"
>  #include "physdev.h"
>  
> -/*
> +/**
> + * DOC: XENMEM_increase_reservation and XENMEM_decrease_reservation
> + *
>   * Increase or decrease the specified domain's memory reservation. Returns the
>   * number of extents successfully allocated or freed.
>   * arg == addr of struct xen_memory_reservation.
> @@ -40,29 +42,37 @@
>  #define XENMEM_populate_physmap     6
>  
>  #if __XEN_INTERFACE_VERSION__ >= 0x00030209
> -/*
> - * Maximum # bits addressable by the user of the allocated region (e.g., I/O
> - * devices often have a 32-bit limitation even in 64-bit systems). If zero
> - * then the user has no addressing restriction. This field is not used by
> - * XENMEM_decrease_reservation.
> +/**
> + * DOC: XENMEMF_*
> + *
> + * - XENMEMF_address_bits, XENMEMF_get_address_bits:
> + *       Maximum # bits addressable by the user of the allocated region
> + *       (e.g., I/O devices often have a 32-bit limitation even in 64-bit
> + *       systems). If zero then the user has no addressing restriction. This
> + *       field is not used by XENMEM_decrease_reservation.
> + * - XENMEMF_node, XENMEMF_get_node: NUMA node to allocate from
> + * - XENMEMF_populate_on_demand: Flag to populate physmap with populate-on-demand entries
> + * - XENMEMF_exact_node_request, XENMEMF_exact_node: Flag to request allocation only from the node specified

Nit: overly long line

> + * - XENMEMF_vnode: Flag to indicate the node specified is virtual node
>   */
>  #define XENMEMF_address_bits(x)     (x)
>  #define XENMEMF_get_address_bits(x) ((x) & 0xffu)
> -/* NUMA node to allocate from. */
>  #define XENMEMF_node(x)     (((x) + 1) << 8)
>  #define XENMEMF_get_node(x) ((((x) >> 8) - 1) & 0xffu)
> -/* Flag to populate physmap with populate-on-demand entries */
>  #define XENMEMF_populate_on_demand (1<<16)
> -/* Flag to request allocation only from the node specified */
>  #define XENMEMF_exact_node_request  (1<<17)
>  #define XENMEMF_exact_node(n) (XENMEMF_node(n) | XENMEMF_exact_node_request)
> -/* Flag to indicate the node specified is virtual node */
>  #define XENMEMF_vnode  (1<<18)
>  #endif
>  
> +/**
> + * struct xen_memory_reservation
> + */
>  struct xen_memory_reservation {
>  
> -    /*
> +    /**
> +     * @extent_start:
> +     *

Take the opportunity and drop the stray blank line?

> @@ -200,90 +236,115 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mfn_list_t);
>   */
>  #define XENMEM_machphys_compat_mfn_list     25
>  
> -/*
> +#define XENMEM_machphys_mapping     12
> +/**
> + * struct xen_machphys_mapping - XENMEM_machphys_mapping
> + *
>   * Returns the location in virtual address space of the machine_to_phys
>   * mapping table. Architectures which do not have a m2p table, or which do not
>   * map it by default into guest address space, do not implement this command.
>   * arg == addr of xen_machphys_mapping_t.
>   */
> -#define XENMEM_machphys_mapping     12
>  struct xen_machphys_mapping {
> +    /** @v_start: Start virtual address */
>      xen_ulong_t v_start, v_end; /* Start and end virtual addresses.   */
> -    xen_ulong_t max_mfn;        /* Maximum MFN that can be looked up. */
> +    /** @v_end: End virtual addresses */
> +    xen_ulong_t v_end;
> +    /** @max_mfn: Maximum MFN that can be looked up */
> +    xen_ulong_t max_mfn;
>  };
>  typedef struct xen_machphys_mapping xen_machphys_mapping_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
>  
> -/* Source mapping space. */
> +/**
> + * DOC: Source mapping space.
> + *
> + * - XENMAPSPACE_shared_info:  shared info page
> + * - XENMAPSPACE_grant_table:  grant table page
> + * - XENMAPSPACE_gmfn:         GMFN
> + * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
> + * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
> + *                             XENMEM_add_to_physmap_batch only.
> + * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
> + *                             in Stage-2 using the Normal MemoryInner/Outer
> + *                             Write-Back Cacheable memory attribute.
> + */
>  /* ` enum phys_map_space { */

Isn't this and ...

> -#define XENMAPSPACE_shared_info  0 /* shared info page */
> -#define XENMAPSPACE_grant_table  1 /* grant table page */
> -#define XENMAPSPACE_gmfn         2 /* GMFN */
> -#define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap only. */
> -#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
> -                                    * XENMEM_add_to_physmap_batch only. */
> -#define XENMAPSPACE_dev_mmio     5 /* device mmio region
> -                                      ARM only; the region is mapped in
> -                                      Stage-2 using the Normal Memory
> -                                      Inner/Outer Write-Back Cacheable
> -                                      memory attribute. */
> +#define XENMAPSPACE_shared_info  0
> +#define XENMAPSPACE_grant_table  1
> +#define XENMAPSPACE_gmfn         2
> +#define XENMAPSPACE_gmfn_range   3
> +#define XENMAPSPACE_gmfn_foreign 4
> +#define XENMAPSPACE_dev_mmio     5
>  /* ` } */

... this also something that wants converting?

Jan

Re: [PATCH 08/14] kernel-doc: public/memory.h

[Stefano Stabellini]

On Fri, 7 Aug 2020, Jan Beulich wrote:
> On 07.08.2020 01:49, Stefano Stabellini wrote:
> > From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > 
> > Convert in-code comments to kernel-doc format wherever possible.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > ---
> >  xen/include/public/memory.h | 232 ++++++++++++++++++++++++------------
> >  1 file changed, 155 insertions(+), 77 deletions(-)
> > 
> > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> > index 21057ed78e..4c57ed213c 100644
> > --- a/xen/include/public/memory.h
> > +++ b/xen/include/public/memory.h
> > @@ -30,7 +30,9 @@
> >  #include "xen.h"
> >  #include "physdev.h"
> >  
> > -/*
> > +/**
> > + * DOC: XENMEM_increase_reservation and XENMEM_decrease_reservation
> > + *
> >   * Increase or decrease the specified domain's memory reservation. Returns the
> >   * number of extents successfully allocated or freed.
> >   * arg == addr of struct xen_memory_reservation.
> > @@ -40,29 +42,37 @@
> >  #define XENMEM_populate_physmap     6
> >  
> >  #if __XEN_INTERFACE_VERSION__ >= 0x00030209
> > -/*
> > - * Maximum # bits addressable by the user of the allocated region (e.g., I/O
> > - * devices often have a 32-bit limitation even in 64-bit systems). If zero
> > - * then the user has no addressing restriction. This field is not used by
> > - * XENMEM_decrease_reservation.
> > +/**
> > + * DOC: XENMEMF_*
> > + *
> > + * - XENMEMF_address_bits, XENMEMF_get_address_bits:
> > + *       Maximum # bits addressable by the user of the allocated region
> > + *       (e.g., I/O devices often have a 32-bit limitation even in 64-bit
> > + *       systems). If zero then the user has no addressing restriction. This
> > + *       field is not used by XENMEM_decrease_reservation.
> > + * - XENMEMF_node, XENMEMF_get_node: NUMA node to allocate from
> > + * - XENMEMF_populate_on_demand: Flag to populate physmap with populate-on-demand entries
> > + * - XENMEMF_exact_node_request, XENMEMF_exact_node: Flag to request allocation only from the node specified
> 
> Nit: overly long line

I'll fix


> > + * - XENMEMF_vnode: Flag to indicate the node specified is virtual node
> >   */
> >  #define XENMEMF_address_bits(x)     (x)
> >  #define XENMEMF_get_address_bits(x) ((x) & 0xffu)
> > -/* NUMA node to allocate from. */
> >  #define XENMEMF_node(x)     (((x) + 1) << 8)
> >  #define XENMEMF_get_node(x) ((((x) >> 8) - 1) & 0xffu)
> > -/* Flag to populate physmap with populate-on-demand entries */
> >  #define XENMEMF_populate_on_demand (1<<16)
> > -/* Flag to request allocation only from the node specified */
> >  #define XENMEMF_exact_node_request  (1<<17)
> >  #define XENMEMF_exact_node(n) (XENMEMF_node(n) | XENMEMF_exact_node_request)
> > -/* Flag to indicate the node specified is virtual node */
> >  #define XENMEMF_vnode  (1<<18)
> >  #endif
> >  
> > +/**
> > + * struct xen_memory_reservation
> > + */
> >  struct xen_memory_reservation {
> >  
> > -    /*
> > +    /**
> > +     * @extent_start:
> > +     *
> 
> Take the opportunity and drop the stray blank line?
 
Sure


> > @@ -200,90 +236,115 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mfn_list_t);
> >   */
> >  #define XENMEM_machphys_compat_mfn_list     25
> >  
> > -/*
> > +#define XENMEM_machphys_mapping     12
> > +/**
> > + * struct xen_machphys_mapping - XENMEM_machphys_mapping
> > + *
> >   * Returns the location in virtual address space of the machine_to_phys
> >   * mapping table. Architectures which do not have a m2p table, or which do not
> >   * map it by default into guest address space, do not implement this command.
> >   * arg == addr of xen_machphys_mapping_t.
> >   */
> > -#define XENMEM_machphys_mapping     12
> >  struct xen_machphys_mapping {
> > +    /** @v_start: Start virtual address */
> >      xen_ulong_t v_start, v_end; /* Start and end virtual addresses.   */
> > -    xen_ulong_t max_mfn;        /* Maximum MFN that can be looked up. */
> > +    /** @v_end: End virtual addresses */
> > +    xen_ulong_t v_end;
> > +    /** @max_mfn: Maximum MFN that can be looked up */
> > +    xen_ulong_t max_mfn;
> >  };
> >  typedef struct xen_machphys_mapping xen_machphys_mapping_t;
> >  DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
> >  
> > -/* Source mapping space. */
> > +/**
> > + * DOC: Source mapping space.
> > + *
> > + * - XENMAPSPACE_shared_info:  shared info page
> > + * - XENMAPSPACE_grant_table:  grant table page
> > + * - XENMAPSPACE_gmfn:         GMFN
> > + * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
> > + * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
> > + *                             XENMEM_add_to_physmap_batch only.
> > + * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
> > + *                             in Stage-2 using the Normal MemoryInner/Outer
> > + *                             Write-Back Cacheable memory attribute.
> > + */
> >  /* ` enum phys_map_space { */
> 
> Isn't this and ...
> 
> > -#define XENMAPSPACE_shared_info  0 /* shared info page */
> > -#define XENMAPSPACE_grant_table  1 /* grant table page */
> > -#define XENMAPSPACE_gmfn         2 /* GMFN */
> > -#define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap only. */
> > -#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
> > -                                    * XENMEM_add_to_physmap_batch only. */
> > -#define XENMAPSPACE_dev_mmio     5 /* device mmio region
> > -                                      ARM only; the region is mapped in
> > -                                      Stage-2 using the Normal Memory
> > -                                      Inner/Outer Write-Back Cacheable
> > -                                      memory attribute. */
> > +#define XENMAPSPACE_shared_info  0
> > +#define XENMAPSPACE_grant_table  1
> > +#define XENMAPSPACE_gmfn         2
> > +#define XENMAPSPACE_gmfn_range   3
> > +#define XENMAPSPACE_gmfn_foreign 4
> > +#define XENMAPSPACE_dev_mmio     5
> >  /* ` } */
> 
> ... this also something that wants converting?

For clarity, I take you are talking about these two enum-related
comments:

/* ` enum phys_map_space { */
[... various #defines ... ]
/* ` } */

Is this something we want to convert to kernel-doc? I don't know. I
couldn't see an obvious value in doing it, in the sense that it doesn't
necessarely make things clearer.

I took a second look at the header and the following would work:

/**
 * DOC: Source mapping space.
 *
 * enum phys_map_space {
 *
 * - XENMAPSPACE_shared_info:  shared info page
 * - XENMAPSPACE_grant_table:  grant table page
 * - XENMAPSPACE_gmfn:         GMFN
 * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
 * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
 *                             XENMEM_add_to_physmap_batch only.
 * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
 *                             in Stage-2 using the Normal MemoryInner/Outer
 *                             Write-Back Cacheable memory attribute.
 * }
 */

Note the blank line after "enum phys_map_space {" is required.


All in all I am in favor of *not* converting the enum comment to
kernel-doc, but I'd be OK with it anyway.

Re: [PATCH 08/14] kernel-doc: public/memory.h

[Jan Beulich]

On 07.08.2020 23:51, Stefano Stabellini wrote:
> On Fri, 7 Aug 2020, Jan Beulich wrote:
>> On 07.08.2020 01:49, Stefano Stabellini wrote:
>>> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>>
>>> Convert in-code comments to kernel-doc format wherever possible.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>> ---
>>>  xen/include/public/memory.h | 232 ++++++++++++++++++++++++------------
>>>  1 file changed, 155 insertions(+), 77 deletions(-)
>>>
>>> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
>>> index 21057ed78e..4c57ed213c 100644
>>> --- a/xen/include/public/memory.h
>>> +++ b/xen/include/public/memory.h
>>> @@ -30,7 +30,9 @@
>>>  #include "xen.h"
>>>  #include "physdev.h"
>>>  
>>> -/*
>>> +/**
>>> + * DOC: XENMEM_increase_reservation and XENMEM_decrease_reservation
>>> + *
>>>   * Increase or decrease the specified domain's memory reservation. Returns the
>>>   * number of extents successfully allocated or freed.
>>>   * arg == addr of struct xen_memory_reservation.
>>> @@ -40,29 +42,37 @@
>>>  #define XENMEM_populate_physmap     6
>>>  
>>>  #if __XEN_INTERFACE_VERSION__ >= 0x00030209
>>> -/*
>>> - * Maximum # bits addressable by the user of the allocated region (e.g., I/O
>>> - * devices often have a 32-bit limitation even in 64-bit systems). If zero
>>> - * then the user has no addressing restriction. This field is not used by
>>> - * XENMEM_decrease_reservation.
>>> +/**
>>> + * DOC: XENMEMF_*
>>> + *
>>> + * - XENMEMF_address_bits, XENMEMF_get_address_bits:
>>> + *       Maximum # bits addressable by the user of the allocated region
>>> + *       (e.g., I/O devices often have a 32-bit limitation even in 64-bit
>>> + *       systems). If zero then the user has no addressing restriction. This
>>> + *       field is not used by XENMEM_decrease_reservation.
>>> + * - XENMEMF_node, XENMEMF_get_node: NUMA node to allocate from
>>> + * - XENMEMF_populate_on_demand: Flag to populate physmap with populate-on-demand entries
>>> + * - XENMEMF_exact_node_request, XENMEMF_exact_node: Flag to request allocation only from the node specified
>>
>> Nit: overly long line
> 
> I'll fix
> 
> 
>>> + * - XENMEMF_vnode: Flag to indicate the node specified is virtual node
>>>   */
>>>  #define XENMEMF_address_bits(x)     (x)
>>>  #define XENMEMF_get_address_bits(x) ((x) & 0xffu)
>>> -/* NUMA node to allocate from. */
>>>  #define XENMEMF_node(x)     (((x) + 1) << 8)
>>>  #define XENMEMF_get_node(x) ((((x) >> 8) - 1) & 0xffu)
>>> -/* Flag to populate physmap with populate-on-demand entries */
>>>  #define XENMEMF_populate_on_demand (1<<16)
>>> -/* Flag to request allocation only from the node specified */
>>>  #define XENMEMF_exact_node_request  (1<<17)
>>>  #define XENMEMF_exact_node(n) (XENMEMF_node(n) | XENMEMF_exact_node_request)
>>> -/* Flag to indicate the node specified is virtual node */
>>>  #define XENMEMF_vnode  (1<<18)
>>>  #endif
>>>  
>>> +/**
>>> + * struct xen_memory_reservation
>>> + */
>>>  struct xen_memory_reservation {
>>>  
>>> -    /*
>>> +    /**
>>> +     * @extent_start:
>>> +     *
>>
>> Take the opportunity and drop the stray blank line?
>  
> Sure
> 
> 
>>> @@ -200,90 +236,115 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mfn_list_t);
>>>   */
>>>  #define XENMEM_machphys_compat_mfn_list     25
>>>  
>>> -/*
>>> +#define XENMEM_machphys_mapping     12
>>> +/**
>>> + * struct xen_machphys_mapping - XENMEM_machphys_mapping
>>> + *
>>>   * Returns the location in virtual address space of the machine_to_phys
>>>   * mapping table. Architectures which do not have a m2p table, or which do not
>>>   * map it by default into guest address space, do not implement this command.
>>>   * arg == addr of xen_machphys_mapping_t.
>>>   */
>>> -#define XENMEM_machphys_mapping     12
>>>  struct xen_machphys_mapping {
>>> +    /** @v_start: Start virtual address */
>>>      xen_ulong_t v_start, v_end; /* Start and end virtual addresses.   */
>>> -    xen_ulong_t max_mfn;        /* Maximum MFN that can be looked up. */
>>> +    /** @v_end: End virtual addresses */
>>> +    xen_ulong_t v_end;
>>> +    /** @max_mfn: Maximum MFN that can be looked up */
>>> +    xen_ulong_t max_mfn;
>>>  };
>>>  typedef struct xen_machphys_mapping xen_machphys_mapping_t;
>>>  DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
>>>  
>>> -/* Source mapping space. */
>>> +/**
>>> + * DOC: Source mapping space.
>>> + *
>>> + * - XENMAPSPACE_shared_info:  shared info page
>>> + * - XENMAPSPACE_grant_table:  grant table page
>>> + * - XENMAPSPACE_gmfn:         GMFN
>>> + * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
>>> + * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
>>> + *                             XENMEM_add_to_physmap_batch only.
>>> + * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
>>> + *                             in Stage-2 using the Normal MemoryInner/Outer
>>> + *                             Write-Back Cacheable memory attribute.
>>> + */
>>>  /* ` enum phys_map_space { */
>>
>> Isn't this and ...
>>
>>> -#define XENMAPSPACE_shared_info  0 /* shared info page */
>>> -#define XENMAPSPACE_grant_table  1 /* grant table page */
>>> -#define XENMAPSPACE_gmfn         2 /* GMFN */
>>> -#define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap only. */
>>> -#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
>>> -                                    * XENMEM_add_to_physmap_batch only. */
>>> -#define XENMAPSPACE_dev_mmio     5 /* device mmio region
>>> -                                      ARM only; the region is mapped in
>>> -                                      Stage-2 using the Normal Memory
>>> -                                      Inner/Outer Write-Back Cacheable
>>> -                                      memory attribute. */
>>> +#define XENMAPSPACE_shared_info  0
>>> +#define XENMAPSPACE_grant_table  1
>>> +#define XENMAPSPACE_gmfn         2
>>> +#define XENMAPSPACE_gmfn_range   3
>>> +#define XENMAPSPACE_gmfn_foreign 4
>>> +#define XENMAPSPACE_dev_mmio     5
>>>  /* ` } */
>>
>> ... this also something that wants converting?
> 
> For clarity, I take you are talking about these two enum-related
> comments:
> 
> /* ` enum phys_map_space { */
> [... various #defines ... ]
> /* ` } */
> 
> Is this something we want to convert to kernel-doc? I don't know. I
> couldn't see an obvious value in doing it, in the sense that it doesn't
> necessarely make things clearer.
> 
> I took a second look at the header and the following would work:
> 
> /**
>  * DOC: Source mapping space.
>  *
>  * enum phys_map_space {
>  *
>  * - XENMAPSPACE_shared_info:  shared info page
>  * - XENMAPSPACE_grant_table:  grant table page
>  * - XENMAPSPACE_gmfn:         GMFN
>  * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
>  * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
>  *                             XENMEM_add_to_physmap_batch only.
>  * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
>  *                             in Stage-2 using the Normal MemoryInner/Outer
>  *                             Write-Back Cacheable memory attribute.
>  * }
>  */
> 
> Note the blank line after "enum phys_map_space {" is required.
> 
> 
> All in all I am in favor of *not* converting the enum comment to
> kernel-doc, but I'd be OK with it anyway.

Iirc the enum comments were added for documentation purposes. This to
me means there are two options at this point:
- retain them in a way that the new doc model consumes them,
- drop them at the same time as adding the new doc comments.

Their (presumed) value is that they identify #define-s which supposed
to be enum-like without actually being able to use enums in the public
headers (with some exceptions).

Jan

Re: [PATCH 08/14] kernel-doc: public/memory.h

[Stefano Stabellini]

On Mon, 17 Aug 2020, Jan Beulich wrote:
> On 07.08.2020 23:51, Stefano Stabellini wrote:
> > On Fri, 7 Aug 2020, Jan Beulich wrote:
> >> On 07.08.2020 01:49, Stefano Stabellini wrote:
> >>> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> >>>
> >>> Convert in-code comments to kernel-doc format wherever possible.
> >>>
> >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> >>> ---
> >>>  xen/include/public/memory.h | 232 ++++++++++++++++++++++++------------
> >>>  1 file changed, 155 insertions(+), 77 deletions(-)
> >>>
> >>> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> >>> index 21057ed78e..4c57ed213c 100644
> >>> --- a/xen/include/public/memory.h
> >>> +++ b/xen/include/public/memory.h
> >>> @@ -30,7 +30,9 @@
> >>>  #include "xen.h"
> >>>  #include "physdev.h"
> >>>  
> >>> -/*
> >>> +/**
> >>> + * DOC: XENMEM_increase_reservation and XENMEM_decrease_reservation
> >>> + *
> >>>   * Increase or decrease the specified domain's memory reservation. Returns the
> >>>   * number of extents successfully allocated or freed.
> >>>   * arg == addr of struct xen_memory_reservation.
> >>> @@ -40,29 +42,37 @@
> >>>  #define XENMEM_populate_physmap     6
> >>>  
> >>>  #if __XEN_INTERFACE_VERSION__ >= 0x00030209
> >>> -/*
> >>> - * Maximum # bits addressable by the user of the allocated region (e.g., I/O
> >>> - * devices often have a 32-bit limitation even in 64-bit systems). If zero
> >>> - * then the user has no addressing restriction. This field is not used by
> >>> - * XENMEM_decrease_reservation.
> >>> +/**
> >>> + * DOC: XENMEMF_*
> >>> + *
> >>> + * - XENMEMF_address_bits, XENMEMF_get_address_bits:
> >>> + *       Maximum # bits addressable by the user of the allocated region
> >>> + *       (e.g., I/O devices often have a 32-bit limitation even in 64-bit
> >>> + *       systems). If zero then the user has no addressing restriction. This
> >>> + *       field is not used by XENMEM_decrease_reservation.
> >>> + * - XENMEMF_node, XENMEMF_get_node: NUMA node to allocate from
> >>> + * - XENMEMF_populate_on_demand: Flag to populate physmap with populate-on-demand entries
> >>> + * - XENMEMF_exact_node_request, XENMEMF_exact_node: Flag to request allocation only from the node specified
> >>
> >> Nit: overly long line
> > 
> > I'll fix
> > 
> > 
> >>> + * - XENMEMF_vnode: Flag to indicate the node specified is virtual node
> >>>   */
> >>>  #define XENMEMF_address_bits(x)     (x)
> >>>  #define XENMEMF_get_address_bits(x) ((x) & 0xffu)
> >>> -/* NUMA node to allocate from. */
> >>>  #define XENMEMF_node(x)     (((x) + 1) << 8)
> >>>  #define XENMEMF_get_node(x) ((((x) >> 8) - 1) & 0xffu)
> >>> -/* Flag to populate physmap with populate-on-demand entries */
> >>>  #define XENMEMF_populate_on_demand (1<<16)
> >>> -/* Flag to request allocation only from the node specified */
> >>>  #define XENMEMF_exact_node_request  (1<<17)
> >>>  #define XENMEMF_exact_node(n) (XENMEMF_node(n) | XENMEMF_exact_node_request)
> >>> -/* Flag to indicate the node specified is virtual node */
> >>>  #define XENMEMF_vnode  (1<<18)
> >>>  #endif
> >>>  
> >>> +/**
> >>> + * struct xen_memory_reservation
> >>> + */
> >>>  struct xen_memory_reservation {
> >>>  
> >>> -    /*
> >>> +    /**
> >>> +     * @extent_start:
> >>> +     *
> >>
> >> Take the opportunity and drop the stray blank line?
> >  
> > Sure
> > 
> > 
> >>> @@ -200,90 +236,115 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mfn_list_t);
> >>>   */
> >>>  #define XENMEM_machphys_compat_mfn_list     25
> >>>  
> >>> -/*
> >>> +#define XENMEM_machphys_mapping     12
> >>> +/**
> >>> + * struct xen_machphys_mapping - XENMEM_machphys_mapping
> >>> + *
> >>>   * Returns the location in virtual address space of the machine_to_phys
> >>>   * mapping table. Architectures which do not have a m2p table, or which do not
> >>>   * map it by default into guest address space, do not implement this command.
> >>>   * arg == addr of xen_machphys_mapping_t.
> >>>   */
> >>> -#define XENMEM_machphys_mapping     12
> >>>  struct xen_machphys_mapping {
> >>> +    /** @v_start: Start virtual address */
> >>>      xen_ulong_t v_start, v_end; /* Start and end virtual addresses.   */
> >>> -    xen_ulong_t max_mfn;        /* Maximum MFN that can be looked up. */
> >>> +    /** @v_end: End virtual addresses */
> >>> +    xen_ulong_t v_end;
> >>> +    /** @max_mfn: Maximum MFN that can be looked up */
> >>> +    xen_ulong_t max_mfn;
> >>>  };
> >>>  typedef struct xen_machphys_mapping xen_machphys_mapping_t;
> >>>  DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
> >>>  
> >>> -/* Source mapping space. */
> >>> +/**
> >>> + * DOC: Source mapping space.
> >>> + *
> >>> + * - XENMAPSPACE_shared_info:  shared info page
> >>> + * - XENMAPSPACE_grant_table:  grant table page
> >>> + * - XENMAPSPACE_gmfn:         GMFN
> >>> + * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
> >>> + * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
> >>> + *                             XENMEM_add_to_physmap_batch only.
> >>> + * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
> >>> + *                             in Stage-2 using the Normal MemoryInner/Outer
> >>> + *                             Write-Back Cacheable memory attribute.
> >>> + */
> >>>  /* ` enum phys_map_space { */
> >>
> >> Isn't this and ...
> >>
> >>> -#define XENMAPSPACE_shared_info  0 /* shared info page */
> >>> -#define XENMAPSPACE_grant_table  1 /* grant table page */
> >>> -#define XENMAPSPACE_gmfn         2 /* GMFN */
> >>> -#define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap only. */
> >>> -#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
> >>> -                                    * XENMEM_add_to_physmap_batch only. */
> >>> -#define XENMAPSPACE_dev_mmio     5 /* device mmio region
> >>> -                                      ARM only; the region is mapped in
> >>> -                                      Stage-2 using the Normal Memory
> >>> -                                      Inner/Outer Write-Back Cacheable
> >>> -                                      memory attribute. */
> >>> +#define XENMAPSPACE_shared_info  0
> >>> +#define XENMAPSPACE_grant_table  1
> >>> +#define XENMAPSPACE_gmfn         2
> >>> +#define XENMAPSPACE_gmfn_range   3
> >>> +#define XENMAPSPACE_gmfn_foreign 4
> >>> +#define XENMAPSPACE_dev_mmio     5
> >>>  /* ` } */
> >>
> >> ... this also something that wants converting?
> > 
> > For clarity, I take you are talking about these two enum-related
> > comments:
> > 
> > /* ` enum phys_map_space { */
> > [... various #defines ... ]
> > /* ` } */
> > 
> > Is this something we want to convert to kernel-doc? I don't know. I
> > couldn't see an obvious value in doing it, in the sense that it doesn't
> > necessarely make things clearer.
> > 
> > I took a second look at the header and the following would work:
> > 
> > /**
> >  * DOC: Source mapping space.
> >  *
> >  * enum phys_map_space {
> >  *
> >  * - XENMAPSPACE_shared_info:  shared info page
> >  * - XENMAPSPACE_grant_table:  grant table page
> >  * - XENMAPSPACE_gmfn:         GMFN
> >  * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
> >  * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
> >  *                             XENMEM_add_to_physmap_batch only.
> >  * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
> >  *                             in Stage-2 using the Normal MemoryInner/Outer
> >  *                             Write-Back Cacheable memory attribute.
> >  * }
> >  */
> > 
> > Note the blank line after "enum phys_map_space {" is required.
> > 
> > 
> > All in all I am in favor of *not* converting the enum comment to
> > kernel-doc, but I'd be OK with it anyway.
> 
> Iirc the enum comments were added for documentation purposes. This to
> me means there are two options at this point:
> - retain them in a way that the new doc model consumes them,
> - drop them at the same time as adding the new doc comments.
> 
> Their (presumed) value is that they identify #define-s which supposed
> to be enum-like without actually being able to use enums in the public
> headers (with some exceptions).

I understand. Then, it doesn't look like we want to keep them in the code
without converting them to kernel-doc. We could either:

1) remove them as part of this series
2) convert them to kernel-doc in the top comment as shown above

I could do either, but my preference is 1) because I think it leads to
clearer docs.

Re: [PATCH 08/14] kernel-doc: public/memory.h

[Jan Beulich]

On 18.08.2020 00:56, Stefano Stabellini wrote:
> On Mon, 17 Aug 2020, Jan Beulich wrote:
>> On 07.08.2020 23:51, Stefano Stabellini wrote:
>>> On Fri, 7 Aug 2020, Jan Beulich wrote:
>>>> On 07.08.2020 01:49, Stefano Stabellini wrote:
>>>>> @@ -200,90 +236,115 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mfn_list_t);
>>>>>   */
>>>>>  #define XENMEM_machphys_compat_mfn_list     25
>>>>>  
>>>>> -/*
>>>>> +#define XENMEM_machphys_mapping     12
>>>>> +/**
>>>>> + * struct xen_machphys_mapping - XENMEM_machphys_mapping
>>>>> + *
>>>>>   * Returns the location in virtual address space of the machine_to_phys
>>>>>   * mapping table. Architectures which do not have a m2p table, or which do not
>>>>>   * map it by default into guest address space, do not implement this command.
>>>>>   * arg == addr of xen_machphys_mapping_t.
>>>>>   */
>>>>> -#define XENMEM_machphys_mapping     12
>>>>>  struct xen_machphys_mapping {
>>>>> +    /** @v_start: Start virtual address */
>>>>>      xen_ulong_t v_start, v_end; /* Start and end virtual addresses.   */
>>>>> -    xen_ulong_t max_mfn;        /* Maximum MFN that can be looked up. */
>>>>> +    /** @v_end: End virtual addresses */
>>>>> +    xen_ulong_t v_end;
>>>>> +    /** @max_mfn: Maximum MFN that can be looked up */
>>>>> +    xen_ulong_t max_mfn;
>>>>>  };
>>>>>  typedef struct xen_machphys_mapping xen_machphys_mapping_t;
>>>>>  DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
>>>>>  
>>>>> -/* Source mapping space. */
>>>>> +/**
>>>>> + * DOC: Source mapping space.
>>>>> + *
>>>>> + * - XENMAPSPACE_shared_info:  shared info page
>>>>> + * - XENMAPSPACE_grant_table:  grant table page
>>>>> + * - XENMAPSPACE_gmfn:         GMFN
>>>>> + * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
>>>>> + * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
>>>>> + *                             XENMEM_add_to_physmap_batch only.
>>>>> + * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
>>>>> + *                             in Stage-2 using the Normal MemoryInner/Outer
>>>>> + *                             Write-Back Cacheable memory attribute.
>>>>> + */
>>>>>  /* ` enum phys_map_space { */
>>>>
>>>> Isn't this and ...
>>>>
>>>>> -#define XENMAPSPACE_shared_info  0 /* shared info page */
>>>>> -#define XENMAPSPACE_grant_table  1 /* grant table page */
>>>>> -#define XENMAPSPACE_gmfn         2 /* GMFN */
>>>>> -#define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap only. */
>>>>> -#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
>>>>> -                                    * XENMEM_add_to_physmap_batch only. */
>>>>> -#define XENMAPSPACE_dev_mmio     5 /* device mmio region
>>>>> -                                      ARM only; the region is mapped in
>>>>> -                                      Stage-2 using the Normal Memory
>>>>> -                                      Inner/Outer Write-Back Cacheable
>>>>> -                                      memory attribute. */
>>>>> +#define XENMAPSPACE_shared_info  0
>>>>> +#define XENMAPSPACE_grant_table  1
>>>>> +#define XENMAPSPACE_gmfn         2
>>>>> +#define XENMAPSPACE_gmfn_range   3
>>>>> +#define XENMAPSPACE_gmfn_foreign 4
>>>>> +#define XENMAPSPACE_dev_mmio     5
>>>>>  /* ` } */
>>>>
>>>> ... this also something that wants converting?
>>>
>>> For clarity, I take you are talking about these two enum-related
>>> comments:
>>>
>>> /* ` enum phys_map_space { */
>>> [... various #defines ... ]
>>> /* ` } */
>>>
>>> Is this something we want to convert to kernel-doc? I don't know. I
>>> couldn't see an obvious value in doing it, in the sense that it doesn't
>>> necessarely make things clearer.
>>>
>>> I took a second look at the header and the following would work:
>>>
>>> /**
>>>  * DOC: Source mapping space.
>>>  *
>>>  * enum phys_map_space {
>>>  *
>>>  * - XENMAPSPACE_shared_info:  shared info page
>>>  * - XENMAPSPACE_grant_table:  grant table page
>>>  * - XENMAPSPACE_gmfn:         GMFN
>>>  * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
>>>  * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
>>>  *                             XENMEM_add_to_physmap_batch only.
>>>  * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
>>>  *                             in Stage-2 using the Normal MemoryInner/Outer
>>>  *                             Write-Back Cacheable memory attribute.
>>>  * }
>>>  */
>>>
>>> Note the blank line after "enum phys_map_space {" is required.
>>>
>>>
>>> All in all I am in favor of *not* converting the enum comment to
>>> kernel-doc, but I'd be OK with it anyway.
>>
>> Iirc the enum comments were added for documentation purposes. This to
>> me means there are two options at this point:
>> - retain them in a way that the new doc model consumes them,
>> - drop them at the same time as adding the new doc comments.
>>
>> Their (presumed) value is that they identify #define-s which supposed
>> to be enum-like without actually being able to use enums in the public
>> headers (with some exceptions).
> 
> I understand. Then, it doesn't look like we want to keep them in the code
> without converting them to kernel-doc. We could either:
> 
> 1) remove them as part of this series
> 2) convert them to kernel-doc in the top comment as shown above
> 
> I could do either, but my preference is 1) because I think it leads to
> clearer docs.

While I'd slightly prefer 2, I'll be okay with your choice.

Jan

Re: [PATCH 08/14] kernel-doc: public/memory.h

[Stefano Stabellini]

On Tue, 18 Aug 2020, Jan Beulich wrote:
> On 18.08.2020 00:56, Stefano Stabellini wrote:
> > On Mon, 17 Aug 2020, Jan Beulich wrote:
> >> On 07.08.2020 23:51, Stefano Stabellini wrote:
> >>> On Fri, 7 Aug 2020, Jan Beulich wrote:
> >>>> On 07.08.2020 01:49, Stefano Stabellini wrote:
> >>>>> @@ -200,90 +236,115 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mfn_list_t);
> >>>>>   */
> >>>>>  #define XENMEM_machphys_compat_mfn_list     25
> >>>>>  
> >>>>> -/*
> >>>>> +#define XENMEM_machphys_mapping     12
> >>>>> +/**
> >>>>> + * struct xen_machphys_mapping - XENMEM_machphys_mapping
> >>>>> + *
> >>>>>   * Returns the location in virtual address space of the machine_to_phys
> >>>>>   * mapping table. Architectures which do not have a m2p table, or which do not
> >>>>>   * map it by default into guest address space, do not implement this command.
> >>>>>   * arg == addr of xen_machphys_mapping_t.
> >>>>>   */
> >>>>> -#define XENMEM_machphys_mapping     12
> >>>>>  struct xen_machphys_mapping {
> >>>>> +    /** @v_start: Start virtual address */
> >>>>>      xen_ulong_t v_start, v_end; /* Start and end virtual addresses.   */
> >>>>> -    xen_ulong_t max_mfn;        /* Maximum MFN that can be looked up. */
> >>>>> +    /** @v_end: End virtual addresses */
> >>>>> +    xen_ulong_t v_end;
> >>>>> +    /** @max_mfn: Maximum MFN that can be looked up */
> >>>>> +    xen_ulong_t max_mfn;
> >>>>>  };
> >>>>>  typedef struct xen_machphys_mapping xen_machphys_mapping_t;
> >>>>>  DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
> >>>>>  
> >>>>> -/* Source mapping space. */
> >>>>> +/**
> >>>>> + * DOC: Source mapping space.
> >>>>> + *
> >>>>> + * - XENMAPSPACE_shared_info:  shared info page
> >>>>> + * - XENMAPSPACE_grant_table:  grant table page
> >>>>> + * - XENMAPSPACE_gmfn:         GMFN
> >>>>> + * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
> >>>>> + * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
> >>>>> + *                             XENMEM_add_to_physmap_batch only.
> >>>>> + * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
> >>>>> + *                             in Stage-2 using the Normal MemoryInner/Outer
> >>>>> + *                             Write-Back Cacheable memory attribute.
> >>>>> + */
> >>>>>  /* ` enum phys_map_space { */
> >>>>
> >>>> Isn't this and ...
> >>>>
> >>>>> -#define XENMAPSPACE_shared_info  0 /* shared info page */
> >>>>> -#define XENMAPSPACE_grant_table  1 /* grant table page */
> >>>>> -#define XENMAPSPACE_gmfn         2 /* GMFN */
> >>>>> -#define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap only. */
> >>>>> -#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
> >>>>> -                                    * XENMEM_add_to_physmap_batch only. */
> >>>>> -#define XENMAPSPACE_dev_mmio     5 /* device mmio region
> >>>>> -                                      ARM only; the region is mapped in
> >>>>> -                                      Stage-2 using the Normal Memory
> >>>>> -                                      Inner/Outer Write-Back Cacheable
> >>>>> -                                      memory attribute. */
> >>>>> +#define XENMAPSPACE_shared_info  0
> >>>>> +#define XENMAPSPACE_grant_table  1
> >>>>> +#define XENMAPSPACE_gmfn         2
> >>>>> +#define XENMAPSPACE_gmfn_range   3
> >>>>> +#define XENMAPSPACE_gmfn_foreign 4
> >>>>> +#define XENMAPSPACE_dev_mmio     5
> >>>>>  /* ` } */
> >>>>
> >>>> ... this also something that wants converting?
> >>>
> >>> For clarity, I take you are talking about these two enum-related
> >>> comments:
> >>>
> >>> /* ` enum phys_map_space { */
> >>> [... various #defines ... ]
> >>> /* ` } */
> >>>
> >>> Is this something we want to convert to kernel-doc? I don't know. I
> >>> couldn't see an obvious value in doing it, in the sense that it doesn't
> >>> necessarely make things clearer.
> >>>
> >>> I took a second look at the header and the following would work:
> >>>
> >>> /**
> >>>  * DOC: Source mapping space.
> >>>  *
> >>>  * enum phys_map_space {
> >>>  *
> >>>  * - XENMAPSPACE_shared_info:  shared info page
> >>>  * - XENMAPSPACE_grant_table:  grant table page
> >>>  * - XENMAPSPACE_gmfn:         GMFN
> >>>  * - XENMAPSPACE_gmfn_range:   GMFN range, XENMEM_add_to_physmap only.
> >>>  * - XENMAPSPACE_gmfn_foreign: GMFN from another dom,
> >>>  *                             XENMEM_add_to_physmap_batch only.
> >>>  * - XENMAPSPACE_dev_mmio:     device mmio region ARM only; the region is mapped
> >>>  *                             in Stage-2 using the Normal MemoryInner/Outer
> >>>  *                             Write-Back Cacheable memory attribute.
> >>>  * }
> >>>  */
> >>>
> >>> Note the blank line after "enum phys_map_space {" is required.
> >>>
> >>>
> >>> All in all I am in favor of *not* converting the enum comment to
> >>> kernel-doc, but I'd be OK with it anyway.
> >>
> >> Iirc the enum comments were added for documentation purposes. This to
> >> me means there are two options at this point:
> >> - retain them in a way that the new doc model consumes them,
> >> - drop them at the same time as adding the new doc comments.
> >>
> >> Their (presumed) value is that they identify #define-s which supposed
> >> to be enum-like without actually being able to use enums in the public
> >> headers (with some exceptions).
> > 
> > I understand. Then, it doesn't look like we want to keep them in the code
> > without converting them to kernel-doc. We could either:
> > 
> > 1) remove them as part of this series
> > 2) convert them to kernel-doc in the top comment as shown above
> > 
> > I could do either, but my preference is 1) because I think it leads to
> > clearer docs.
> 
> While I'd slightly prefer 2, I'll be okay with your choice.

OK. I removed the enum comments.

[PATCH 09/14] kernel-doc: public/sched.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/sched.h | 129 ++++++++++++++++++++++++++-----------
 1 file changed, 92 insertions(+), 37 deletions(-)

diff --git a/xen/include/public/sched.h b/xen/include/public/sched.h
index 811bd87c82..6f36c3748a 100644
--- a/xen/include/public/sched.h
+++ b/xen/include/public/sched.h
@@ -29,41 +29,50 @@
 
 #include "event_channel.h"
 
-/*
- * `incontents 150 sched Guest Scheduler Operations
+/**
+ * DOC: Guest Scheduler Operations
  *
  * The SCHEDOP interface provides mechanisms for a guest to interact
  * with the scheduler, including yield, blocking and shutting itself
  * down.
  */
 
-/*
+/**
+ * DOC: HYPERVISOR_sched_op
+ *
  * The prototype for this hypercall is:
- * ` long HYPERVISOR_sched_op(enum sched_op cmd, void *arg, ...)
  *
- * @cmd == SCHEDOP_??? (scheduler operation).
- * @arg == Operation-specific extra argument(s), as described below.
- * ...  == Additional Operation-specific extra arguments, described below.
+ * long HYPERVISOR_sched_op(enum sched_op cmd, void *arg, ...)
+ *
+ * - @cmd == SCHEDOP_??? (scheduler operation).
+ * - @arg == Operation-specific extra argument(s), as described below.
+ * - ...  == Additional Operation-specific extra arguments, described below.
  *
  * Versions of Xen prior to 3.0.2 provided only the following legacy version
  * of this hypercall, supporting only the commands yield, block and shutdown:
+ *
  *  long sched_op(int cmd, unsigned long arg)
- * @cmd == SCHEDOP_??? (scheduler operation).
- * @arg == 0               (SCHEDOP_yield and SCHEDOP_block)
- *      == SHUTDOWN_* code (SCHEDOP_shutdown)
+ *
+ * - @cmd == SCHEDOP_??? (scheduler operation).
+ * - @arg == 0               (SCHEDOP_yield and SCHEDOP_block)
+ * -      == SHUTDOWN_* code (SCHEDOP_shutdown)
  *
  * This legacy version is available to new guests as:
- * ` long HYPERVISOR_sched_op_compat(enum sched_op cmd, unsigned long arg)
+ *
+ * long HYPERVISOR_sched_op_compat(enum sched_op cmd, unsigned long arg)
  */
 
 /* ` enum sched_op { // SCHEDOP_* => struct sched_* */
-/*
- * Voluntarily yield the CPU.
- * @arg == NULL.
+
+/**
+ * DOC: SCHEDOP_yield
+ * Voluntarily yield the CPU. @arg == NULL.
  */
 #define SCHEDOP_yield       0
 
-/*
+/**
+ * DOC: SCHEDOP_block
+ *
  * Block execution of this VCPU until an event is received for processing.
  * If called with event upcalls masked, this operation will atomically
  * reenable event delivery and check for pending events before blocking the
@@ -72,7 +81,9 @@
  */
 #define SCHEDOP_block       1
 
-/*
+/**
+ * DOC: SCHEDOP_shutdown
+ *
  * Halt execution of this domain (all VCPUs) and notify the system controller.
  * @arg == pointer to sched_shutdown_t structure.
  *
@@ -87,14 +98,18 @@
  */
 #define SCHEDOP_shutdown    2
 
-/*
+/**
+ * DOC: SCHEDOP_poll
+ *
  * Poll a set of event-channel ports. Return when one or more are pending. An
  * optional timeout may be specified.
  * @arg == pointer to sched_poll_t structure.
  */
 #define SCHEDOP_poll        3
 
-/*
+/**
+ * DOC: SCHEDOP_remote_shutdown
+ *
  * Declare a shutdown for another domain. The main use of this function is
  * in interpreting shutdown requests and reasons for fully-virtualized
  * domains.  A para-virtualized domain may use SCHEDOP_shutdown directly.
@@ -102,14 +117,18 @@
  */
 #define SCHEDOP_remote_shutdown        4
 
-/*
+/**
+ * DOC: SCHEDOP_shutdown_code
+ *
  * Latch a shutdown code, so that when the domain later shuts down it
  * reports this code to the control tools.
  * @arg == sched_shutdown_t, as for SCHEDOP_shutdown.
  */
 #define SCHEDOP_shutdown_code 5
 
-/*
+/**
+ * DOC: SCHEDOP_watchdog
+ *
  * Setup, poke and destroy a domain watchdog timer.
  * @arg == pointer to sched_watchdog_t structure.
  * With id == 0, setup a domain watchdog timer to cause domain shutdown
@@ -119,7 +138,9 @@
  */
 #define SCHEDOP_watchdog    6
 
-/*
+/**
+ * DOC: SCHEDOP_pin_override
+ *
  * Override the current vcpu affinity by pinning it to one physical cpu or
  * undo this override restoring the previous affinity.
  * @arg == pointer to sched_pin_override_t structure.
@@ -132,12 +153,19 @@
 #define SCHEDOP_pin_override 7
 /* ` } */
 
+/**
+ * struct sched_shutdown
+ */
 struct sched_shutdown {
-    unsigned int reason; /* SHUTDOWN_* => enum sched_shutdown_reason */
+    /** @reason: SHUTDOWN_* => enum sched_shutdown_reason */
+    unsigned int reason;
 };
 typedef struct sched_shutdown sched_shutdown_t;
 DEFINE_XEN_GUEST_HANDLE(sched_shutdown_t);
 
+/**
+ * struct sched_poll
+ */
 struct sched_poll {
     XEN_GUEST_HANDLE(evtchn_port_t) ports;
     unsigned int nr_ports;
@@ -146,39 +174,62 @@ struct sched_poll {
 typedef struct sched_poll sched_poll_t;
 DEFINE_XEN_GUEST_HANDLE(sched_poll_t);
 
+/**
+ * struct sched_remote_shutdown
+ */
 struct sched_remote_shutdown {
-    domid_t domain_id;         /* Remote domain ID */
-    unsigned int reason;       /* SHUTDOWN_* => enum sched_shutdown_reason */
+    /** @domain_id: Remote domain ID */
+    domid_t domain_id;
+    /** @reason: SHUTDOWN_* => enum sched_shutdown_reason */
+    unsigned int reason;
 };
 typedef struct sched_remote_shutdown sched_remote_shutdown_t;
 DEFINE_XEN_GUEST_HANDLE(sched_remote_shutdown_t);
 
+/**
+ * struct sched_watchdog
+ */
 struct sched_watchdog {
-    uint32_t id;                /* watchdog ID */
-    uint32_t timeout;           /* timeout */
+    /** @id: watchdog ID */
+    uint32_t id;
+    /** @timeout: timeout */
+    uint32_t timeout;
 };
 typedef struct sched_watchdog sched_watchdog_t;
 DEFINE_XEN_GUEST_HANDLE(sched_watchdog_t);
 
+/**
+ * struct sched_pin_override
+ */
 struct sched_pin_override {
     int32_t pcpu;
 };
 typedef struct sched_pin_override sched_pin_override_t;
 DEFINE_XEN_GUEST_HANDLE(sched_pin_override_t);
 
-/*
- * Reason codes for SCHEDOP_shutdown. These may be interpreted by control
- * software to determine the appropriate action. For the most part, Xen does
- * not care about the shutdown code.
+/**
+ * DOC: Reason codes for SCHEDOP_shutdown
+ *
+ * These may be interpreted by control software to determine the
+ * appropriate action. For the most part, Xen does not care about the
+ * shutdown code.
+ *
+ * - SHUTDOWN_poweroff: Domain exited normally. Clean up and kill.
+ * - SHUTDOWN_reboot:   Clean up, kill, and then restart.
+ * - SHUTDOWN_suspend:  Clean up, save suspend info, kill.
+ * - SHUTDOWN_crash:    Tell controller we've crashed.
+ * - SHUTDOWN_watchdog: Restart because watchdog time expired.
  */
 /* ` enum sched_shutdown_reason { */
-#define SHUTDOWN_poweroff   0  /* Domain exited normally. Clean up and kill. */
-#define SHUTDOWN_reboot     1  /* Clean up, kill, and then restart.          */
-#define SHUTDOWN_suspend    2  /* Clean up, save suspend info, kill.         */
-#define SHUTDOWN_crash      3  /* Tell controller we've crashed.             */
-#define SHUTDOWN_watchdog   4  /* Restart because watchdog time expired.     */
+#define SHUTDOWN_poweroff   0
+#define SHUTDOWN_reboot     1
+#define SHUTDOWN_suspend    2
+#define SHUTDOWN_crash      3
+#define SHUTDOWN_watchdog   4
 
-/*
+/**
+ * DOC: SHUTDOWN_soft_reset
+ *
  * Domain asked to perform 'soft reset' for it. The expected behavior is to
  * reset internal Xen state for the domain returning it to the point where it
  * was created but leaving the domain's memory contents and vCPU contexts
@@ -186,7 +237,11 @@ DEFINE_XEN_GUEST_HANDLE(sched_pin_override_t);
  * interfaces again.
  */
 #define SHUTDOWN_soft_reset 5
-#define SHUTDOWN_MAX        5  /* Maximum valid shutdown reason.             */
+/**
+ * DOC: SHUTDOWN_MAX
+ * Maximum valid shutdown reason
+ */
+#define SHUTDOWN_MAX        5
 /* ` } */
 
 #endif /* __XEN_PUBLIC_SCHED_H__ */
-- 
2.17.1

[PATCH 10/14] kernel-doc: public/vcpu.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/vcpu.h | 180 ++++++++++++++++++++++++++++----------
 1 file changed, 136 insertions(+), 44 deletions(-)

diff --git a/xen/include/public/vcpu.h b/xen/include/public/vcpu.h
index 3623af932f..e50471e2b2 100644
--- a/xen/include/public/vcpu.h
+++ b/xen/include/public/vcpu.h
@@ -29,15 +29,20 @@
 
 #include "xen.h"
 
-/*
+/**
+ * DOC: VCPUOP hypercall
+ *
  * Prototype for this hypercall is:
  *  long vcpu_op(int cmd, unsigned int vcpuid, void *extra_args)
- * @cmd        == VCPUOP_??? (VCPU operation).
- * @vcpuid     == VCPU to operate on.
- * @extra_args == Operation-specific extra arguments (NULL if none).
+ *
+ * - @cmd        == VCPUOP_??? (VCPU operation).
+ * - @vcpuid     == VCPU to operate on.
+ * - @extra_args == Operation-specific extra arguments (NULL if none).
  */
 
-/*
+/**
+ * DOC: VCPUOP_initialise
+ *
  * Initialise a VCPU. Each VCPU can be initialised only once. A
  * newly-initialised VCPU will not run until it is brought up by VCPUOP_up.
  *
@@ -48,13 +53,17 @@
  */
 #define VCPUOP_initialise            0
 
-/*
+/**
+ * DOC: VCPUOP_up
+ *
  * Bring up a VCPU. This makes the VCPU runnable. This operation will fail
  * if the VCPU has not been initialised (VCPUOP_initialise).
  */
 #define VCPUOP_up                    1
 
-/*
+/**
+ * DOC: VCPUOP_down
+ *
  * Bring down a VCPU (i.e., make it non-runnable).
  * There are a few caveats that callers should observe:
  *  1. This operation may return, and VCPU_is_up may return false, before the
@@ -70,26 +79,36 @@
  */
 #define VCPUOP_down                  2
 
-/* Returns 1 if the given VCPU is up. */
+/**
+ * DOC: VCPUOP_is_up
+ * Returns 1 if the given VCPU is up.
+ */
 #define VCPUOP_is_up                 3
 
-/*
+#define VCPUOP_get_runstate_info     4
+/**
+ * struct vcpu_runstate_info - VCPUOP_get_runstate_info
+ *
  * Return information about the state and running time of a VCPU.
  * @extra_arg == pointer to vcpu_runstate_info structure.
  */
-#define VCPUOP_get_runstate_info     4
 struct vcpu_runstate_info {
-    /* VCPU's current state (RUNSTATE_*). */
+    /** @state: VCPU's current state (RUNSTATE_*). */
     int      state;
-    /* When was current state entered (system time, ns)? */
-    uint64_t state_entry_time;
-    /*
-     * Update indicator set in state_entry_time:
+    /**
+     * @state_entry_time:
+     *
+     * When was current state entered (system time, ns)?
+     *
+     * XEN_RUNSTATE_UPDATE is the update indicator in state_entry_time:
      * When activated via VMASST_TYPE_runstate_update_flag, set during
      * updates in guest memory mapped copy of vcpu_runstate_info.
      */
 #define XEN_RUNSTATE_UPDATE          (xen_mk_ullong(1) << 63)
-    /*
+    uint64_t state_entry_time;
+    /**
+     * @time:
+     *
      * Time spent in each RUNSTATE_* (ns). The sum of these times is
      * guaranteed not to drift from system time.
      */
@@ -98,16 +117,27 @@ struct vcpu_runstate_info {
 typedef struct vcpu_runstate_info vcpu_runstate_info_t;
 DEFINE_XEN_GUEST_HANDLE(vcpu_runstate_info_t);
 
-/* VCPU is currently running on a physical CPU. */
+/**
+ * DOC: RUNSTATE_running
+ * VCPU is currently running on a physical CPU.
+ */
 #define RUNSTATE_running  0
 
-/* VCPU is runnable, but not currently scheduled on any physical CPU. */
+/**
+ * DOC: RUNSTATE_runnable
+ * VCPU is runnable, but not currently scheduled on any physical CPU.
+ */
 #define RUNSTATE_runnable 1
 
-/* VCPU is blocked (a.k.a. idle). It is therefore not runnable. */
+/**
+ * DOC: RUNSTATE_blocked
+ * VCPU is blocked (a.k.a. idle). It is therefore not runnable.
+ */
 #define RUNSTATE_blocked  2
 
-/*
+/**
+ * DOC: RUNSTATE_offline
+ *
  * VCPU is not runnable, but it is not blocked.
  * This is a 'catch all' state for things like hotplug and pauses by the
  * system administrator (or for critical sections in the hypervisor).
@@ -115,7 +145,10 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_runstate_info_t);
  */
 #define RUNSTATE_offline  3
 
-/*
+#define VCPUOP_register_runstate_memory_area 5
+/**
+ * struct vcpu_register_runstate_memory_area - VCPUOP_register_runstate_memory_area
+ *
  * Register a shared memory area from which the guest may obtain its own
  * runstate information without needing to execute a hypercall.
  * Notes:
@@ -127,9 +160,9 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_runstate_info_t);
  *     runstate.state will always be RUNSTATE_running and
  *     runstate.state_entry_time will indicate the system time at which the
  *     VCPU was last scheduled to run.
+ *
  * @extra_arg == pointer to vcpu_register_runstate_memory_area structure.
  */
-#define VCPUOP_register_runstate_memory_area 5
 struct vcpu_register_runstate_memory_area {
     union {
         XEN_GUEST_HANDLE(vcpu_runstate_info_t) h;
@@ -140,38 +173,74 @@ struct vcpu_register_runstate_memory_area {
 typedef struct vcpu_register_runstate_memory_area vcpu_register_runstate_memory_area_t;
 DEFINE_XEN_GUEST_HANDLE(vcpu_register_runstate_memory_area_t);
 
-/*
- * Set or stop a VCPU's periodic timer. Every VCPU has one periodic timer
- * which can be set via these commands. Periods smaller than one millisecond
- * may not be supported.
+/**
+ * DOC: VCPUOP_set_periodic_timer
+ *
+ * Set a VCPU's periodic timer. Every VCPU has one periodic timer which
+ * can be set via this command. Periods smaller than one millisecond may
+ * not be supported.
+ *
+ * @arg == vcpu_set_periodic_timer_t
+ */
+#define VCPUOP_set_periodic_timer    6
+/**
+ * DOC: VCPUOP_stop_periodic_timer
+ *
+ * Stop a VCPU's periodic timer.
+ *
+ * @arg == NULL
+ */
+#define VCPUOP_stop_periodic_timer   7
+/**
+ * struct vcpu_set_periodic_timer
  */
-#define VCPUOP_set_periodic_timer    6 /* arg == vcpu_set_periodic_timer_t */
-#define VCPUOP_stop_periodic_timer   7 /* arg == NULL */
 struct vcpu_set_periodic_timer {
     uint64_t period_ns;
 };
 typedef struct vcpu_set_periodic_timer vcpu_set_periodic_timer_t;
 DEFINE_XEN_GUEST_HANDLE(vcpu_set_periodic_timer_t);
 
-/*
- * Set or stop a VCPU's single-shot timer. Every VCPU has one single-shot
- * timer which can be set via these commands.
+/**
+ * DOC: VCPUOP_set_singleshot_timer
+ *
+ * Set a VCPU's single-shot timer. Every VCPU has one single-shot timer
+ * which can be set via this command.
+ *
+ * @arg == vcpu_set_singleshot_timer_t
+ */
+#define VCPUOP_set_singleshot_timer  8
+/**
+ * DOC: VCPUOP_stop_singleshot_timer
+ *
+ * Stop a VCPU's single-shot timer.
+ *
+ * arg == NULL
+ */
+#define VCPUOP_stop_singleshot_timer 9
+/**
+ * struct vcpu_set_singleshot_timer
  */
-#define VCPUOP_set_singleshot_timer  8 /* arg == vcpu_set_singleshot_timer_t */
-#define VCPUOP_stop_singleshot_timer 9 /* arg == NULL */
 struct vcpu_set_singleshot_timer {
-    uint64_t timeout_abs_ns;   /* Absolute system time value in nanoseconds. */
-    uint32_t flags;            /* VCPU_SSHOTTMR_??? */
+    /** @timeout_abs_ns: Absolute system time value in nanoseconds. */
+    uint64_t timeout_abs_ns;
+    /** @flags: VCPU_SSHOTTMR_??? */
+    uint32_t flags;
 };
 typedef struct vcpu_set_singleshot_timer vcpu_set_singleshot_timer_t;
 DEFINE_XEN_GUEST_HANDLE(vcpu_set_singleshot_timer_t);
 
-/* Flags to VCPUOP_set_singleshot_timer. */
- /* Require the timeout to be in the future (return -ETIME if it's passed). */
+/**
+ * DOC: flags to VCPUOP_set_singleshot_timer.
+ *
+ * VCPU_SSHOTTMR_future: Require the timeout to be in the future
+ *                       (return -ETIME if it's passed).
+ */
 #define _VCPU_SSHOTTMR_future (0)
 #define VCPU_SSHOTTMR_future  (1U << _VCPU_SSHOTTMR_future)
 
-/*
+/**
+ * DOC: VCPUOP_register_vcpu_info
+ *
  * Register a memory location in the guest address space for the
  * vcpu_info structure.  This allows the guest to place the vcpu_info
  * structure in a convenient place, such as in a per-cpu data area.
@@ -179,26 +248,44 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_set_singleshot_timer_t);
  * cross a page boundary.
  *
  * This may be called only once per vcpu.
+ *
+ * @arg == vcpu_register_vcpu_info_t
+ */
+#define VCPUOP_register_vcpu_info   10
+/**
+ * struct vcpu_register_vcpu_info - VCPUOP_register_vcpu_info
  */
-#define VCPUOP_register_vcpu_info   10  /* arg == vcpu_register_vcpu_info_t */
 struct vcpu_register_vcpu_info {
+    /** @mfn: mfn of page to place vcpu_info */
     uint64_t mfn;    /* mfn of page to place vcpu_info */
+    /** @offset: offset within page */
     uint32_t offset; /* offset within page */
-    uint32_t rsvd;   /* unused */
+    /** @rsvd: unused */
+    uint32_t rsvd;
 };
 typedef struct vcpu_register_vcpu_info vcpu_register_vcpu_info_t;
 DEFINE_XEN_GUEST_HANDLE(vcpu_register_vcpu_info_t);
 
-/* Send an NMI to the specified VCPU. @extra_arg == NULL. */
+/**
+ * DOC: VCPUOP_send_nmi
+ * Send an NMI to the specified VCPU. @extra_arg == NULL.
+ */
 #define VCPUOP_send_nmi             11
 
-/*
+/**
+ * DOC: VCPUOP_get_physid
+ *
  * Get the physical ID information for a pinned vcpu's underlying physical
  * processor.  The physical ID informmation is architecture-specific.
  * On x86: id[31:0]=apic_id, id[63:32]=acpi_id.
  * This command returns -EINVAL if it is not a valid operation for this VCPU.
+ *
+ * @arg == vcpu_get_physid_t
+ */
+#define VCPUOP_get_physid           12
+/**
+ * struct vcpu_get_physid
  */
-#define VCPUOP_get_physid           12 /* arg == vcpu_get_physid_t */
 struct vcpu_get_physid {
     uint64_t phys_id;
 };
@@ -207,7 +294,9 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_get_physid_t);
 #define xen_vcpu_physid_to_x86_apicid(physid) ((uint32_t)(physid))
 #define xen_vcpu_physid_to_x86_acpiid(physid) ((uint32_t)((physid) >> 32))
 
-/*
+/**
+ * DOC: VCPUOP_register_vcpu_time_memory_area
+ *
  * Register a memory location to get a secondary copy of the vcpu time
  * parameters.  The master copy still exists as part of the vcpu shared
  * memory area, and this secondary copy is updated whenever the master copy
@@ -225,6 +314,9 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_get_physid_t);
  */
 #define VCPUOP_register_vcpu_time_memory_area   13
 DEFINE_XEN_GUEST_HANDLE(vcpu_time_info_t);
+/**
+ * struct vcpu_register_time_memory_area
+ */
 struct vcpu_register_time_memory_area {
     union {
         XEN_GUEST_HANDLE(vcpu_time_info_t) h;
-- 
2.17.1

[PATCH 11/14] kernel-doc: public/version.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/version.h | 74 +++++++++++++++++++++++++++++-------
 1 file changed, 60 insertions(+), 14 deletions(-)

diff --git a/xen/include/public/version.h b/xen/include/public/version.h
index 17a81e23cd..9a7d8d26dd 100644
--- a/xen/include/public/version.h
+++ b/xen/include/public/version.h
@@ -30,19 +30,33 @@
 
 #include "xen.h"
 
-/* NB. All ops return zero on success, except XENVER_{version,pagesize}
- * XENVER_{version,pagesize,build_id} */
+/**
+ * DOC: XENVER_*
+ *
+ * NB. All ops return zero on success, except for:
+ *
+ * - XENVER_{version,pagesize}
+ * - XENVER_{version,pagesize,build_id}
+ */
 
-/* arg == NULL; returns major:minor (16:16). */
+/**
+ * DOC: XENVER_version
+ * @arg == NULL; returns major:minor (16:16).
+ */
 #define XENVER_version      0
 
-/* arg == xen_extraversion_t. */
+/**
+ * DOC: XENVER_extraversion
+ * @arg == xen_extraversion_t.
+ */
 #define XENVER_extraversion 1
 typedef char xen_extraversion_t[16];
 #define XEN_EXTRAVERSION_LEN (sizeof(xen_extraversion_t))
 
-/* arg == xen_compile_info_t. */
 #define XENVER_compile_info 2
+/**
+ * struct xen_compile_info - XENVER_compile_info
+ */
 struct xen_compile_info {
     char compiler[64];
     char compile_by[16];
@@ -51,52 +65,84 @@ struct xen_compile_info {
 };
 typedef struct xen_compile_info xen_compile_info_t;
 
+/**
+ * DOC: XENVER_capabilities
+ * @arg: xen_capabilities_info_t
+ */
 #define XENVER_capabilities 3
 typedef char xen_capabilities_info_t[1024];
 #define XEN_CAPABILITIES_INFO_LEN (sizeof(xen_capabilities_info_t))
 
+/**
+ * DOC: XENVER_changeset
+ * @arg: xen_changeset_info_t
+ */
 #define XENVER_changeset 4
 typedef char xen_changeset_info_t[64];
 #define XEN_CHANGESET_INFO_LEN (sizeof(xen_changeset_info_t))
 
 #define XENVER_platform_parameters 5
+/**
+ * struct xen_platform_parameters - XENVER_platform_parameters
+ */
 struct xen_platform_parameters {
     xen_ulong_t virt_start;
 };
 typedef struct xen_platform_parameters xen_platform_parameters_t;
 
 #define XENVER_get_features 6
+/**
+ * struct xen_feature_info - XENVER_get_features
+ */
 struct xen_feature_info {
-    unsigned int submap_idx;    /* IN: which 32-bit submap to return */
-    uint32_t     submap;        /* OUT: 32-bit submap */
+    /** @submap_idx: IN: which 32-bit submap to return */
+    unsigned int submap_idx;
+    /** @submap: OUT: 32-bit submap */
+    uint32_t     submap;
 };
 typedef struct xen_feature_info xen_feature_info_t;
 
-/* Declares the features reported by XENVER_get_features. */
+/**
+ * DOC: features.h
+ * Declares the features reported by XENVER_get_features.
+ */
 #include "features.h"
 
-/* arg == NULL; returns host memory page size. */
+/**
+ * DOC: XENVER_pagesize
+ * @arg == NULL; returns host memory page size.
+ */
 #define XENVER_pagesize 7
 
-/* arg == xen_domain_handle_t.
+/**
+ * DOC: XENVER_guest_handle
+ *
+ * @arg == xen_domain_handle_t.
  *
  * The toolstack fills it out for guest consumption. It is intended to hold
  * the UUID of the guest.
  */
 #define XENVER_guest_handle 8
 
+/**
+ * DOC: XENVER_commandline
+ * @arg: xen_commandline_t
+ */
 #define XENVER_commandline 9
 typedef char xen_commandline_t[1024];
 
-/*
+#define XENVER_build_id 10
+/**
+ * struct xen_build_id - XENVER_build_id
+ *
  * Return value is the number of bytes written, or XEN_Exx on error.
  * Calling with empty parameter returns the size of build_id.
  */
-#define XENVER_build_id 10
 struct xen_build_id {
-        uint32_t        len; /* IN: size of buf[]. */
+        /** @len: IN: size of buf[]. */
+        uint32_t        len;
+        /** @buf: OUT: Variable length buffer with build_id. */
         unsigned char   buf[XEN_FLEX_ARRAY_DIM];
-                             /* OUT: Variable length buffer with build_id. */
 };
 typedef struct xen_build_id xen_build_id_t;
 
-- 
2.17.1

Re: [PATCH 11/14] kernel-doc: public/version.h

[Jan Beulich]

On 07.08.2020 01:49, Stefano Stabellini wrote:
> --- a/xen/include/public/version.h
> +++ b/xen/include/public/version.h
> @@ -30,19 +30,33 @@
>  
>  #include "xen.h"
>  
> -/* NB. All ops return zero on success, except XENVER_{version,pagesize}
> - * XENVER_{version,pagesize,build_id} */
> +/**
> + * DOC: XENVER_*
> + *
> + * NB. All ops return zero on success, except for:
> + *
> + * - XENVER_{version,pagesize}
> + * - XENVER_{version,pagesize,build_id}

You make the mistake made by 460d24188d81 even more obvious, instead of
fixing it (by removing the first of these two lines).

Jan

Re: [PATCH 11/14] kernel-doc: public/version.h

[Stefano Stabellini]

On Fri, 7 Aug 2020, Jan Beulich wrote:
> On 07.08.2020 01:49, Stefano Stabellini wrote:
> > --- a/xen/include/public/version.h
> > +++ b/xen/include/public/version.h
> > @@ -30,19 +30,33 @@
> >  
> >  #include "xen.h"
> >  
> > -/* NB. All ops return zero on success, except XENVER_{version,pagesize}
> > - * XENVER_{version,pagesize,build_id} */
> > +/**
> > + * DOC: XENVER_*
> > + *
> > + * NB. All ops return zero on success, except for:
> > + *
> > + * - XENVER_{version,pagesize}
> > + * - XENVER_{version,pagesize,build_id}
> 
> You make the mistake made by 460d24188d81 even more obvious, instead of
> fixing it (by removing the first of these two lines).

Ops, I have been too mechanic in these transformations :-)
I can fix it.

[PATCH 12/14] kernel-doc: public/xen.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/xen.h | 567 ++++++++++++++++++++++++++-------------
 1 file changed, 378 insertions(+), 189 deletions(-)

diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 945ef30273..ff1a62209d 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -81,14 +81,63 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
 
 #endif
 
-/*
- * HYPERCALLS
- */
-
-/* `incontents 100 hcalls List of hypercalls
- * ` enum hypercall_num { // __HYPERVISOR_* => HYPERVISOR_*()
+/**
+ * DOC: HYPERCALLS
+ *
+ * List of hypercalls
+ *
+ * - __HYPERVISOR_set_trap_table
+ * - __HYPERVISOR_mmu_update
+ * - __HYPERVISOR_set_gdt
+ * - __HYPERVISOR_stack_switch
+ * - __HYPERVISOR_set_callbacks
+ * - __HYPERVISOR_fpu_taskswitch
+ * - __HYPERVISOR_sched_op_compat
+ * - __HYPERVISOR_platform_op
+ * - __HYPERVISOR_set_debugreg
+ * - __HYPERVISOR_get_debugreg
+ * - __HYPERVISOR_update_descriptor
+ * - __HYPERVISOR_memory_op
+ * - __HYPERVISOR_multicall
+ * - __HYPERVISOR_update_va_mapping
+ * - __HYPERVISOR_set_timer_op
+ * - __HYPERVISOR_event_channel_op_compat
+ * - __HYPERVISOR_xen_version
+ * - __HYPERVISOR_console_io
+ * - __HYPERVISOR_physdev_op_compat
+ * - __HYPERVISOR_grant_table_op
+ * - __HYPERVISOR_vm_assist
+ * - __HYPERVISOR_update_va_mapping_otherdomain
+ * - __HYPERVISOR_iret
+ * - __HYPERVISOR_vcpu_op
+ * - __HYPERVISOR_set_segment_base
+ * - __HYPERVISOR_mmuext_op
+ * - __HYPERVISOR_xsm_op
+ * - __HYPERVISOR_nmi_op
+ * - __HYPERVISOR_sched_op
+ * - __HYPERVISOR_callback_op
+ * - __HYPERVISOR_xenoprof_op
+ * - __HYPERVISOR_event_channel_op
+ * - __HYPERVISOR_physdev_op
+ * - __HYPERVISOR_hvm_op
+ * - __HYPERVISOR_sysctl
+ * - __HYPERVISOR_domctl
+ * - __HYPERVISOR_kexec_op
+ * - __HYPERVISOR_tmem_op
+ * - __HYPERVISOR_argo_op
+ * - __HYPERVISOR_xenpmu_op
+ * - __HYPERVISOR_dm_op
+ * - __HYPERVISOR_hypfs_op
+ * - __HYPERVISOR_arch_0
+ * - __HYPERVISOR_arch_1
+ * - __HYPERVISOR_arch_2
+ * - __HYPERVISOR_arch_3
+ * - __HYPERVISOR_arch_4
+ * - __HYPERVISOR_arch_5
+ * - __HYPERVISOR_arch_6
+ * - __HYPERVISOR_arch_7
  */
-
+ /* ` enum hypercall_num { // __HYPERVISOR_* => HYPERVISOR_*() */
 #define __HYPERVISOR_set_trap_table        0
 #define __HYPERVISOR_mmu_update            1
 #define __HYPERVISOR_set_gdt               2
@@ -167,8 +216,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
 #define __HYPERVISOR_dom0_op __HYPERVISOR_platform_op
 #endif
 
-/*
- * VIRTUAL INTERRUPTS
+/**
+ * DOC: VIRTUAL INTERRUPTS
  *
  * Virtual interrupts that a guest OS may receive from Xen.
  *
@@ -176,21 +225,43 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
  * global VIRQ. The former can be bound once per VCPU and cannot be re-bound.
  * The latter can be allocated only once per guest: they must initially be
  * allocated to VCPU0 but can subsequently be re-bound.
+ *
+ * - VIRQ_TIMER:      V. Timebase update, and/or requested timeout.
+ * - VIRQ_DEBUG:      V. Request guest to dump debug info.
+ * - VIRQ_CONSOLE:    G. (DOM0) Bytes received on emergency console.
+ * - VIRQ_DOM_EXC:    G. (DOM0) Exceptional event for some domain.
+ * - VIRQ_TBUF:       G. (DOM0) Trace buffer has records available.
+ * - VIRQ_DEBUGGER:   G. (DOM0) A domain has paused for debugging.
+ * - VIRQ_XENOPROF:   V. XenOprofile interrupt: new sample available
+ * - VIRQ_CON_RING:   G. (DOM0) Bytes received on console
+ * - VIRQ_PCPU_STATE: G. (DOM0) PCPU state changed
+ * - VIRQ_MEM_EVENT:  G. (DOM0) A memory event has occurred
+ * - VIRQ_ARGO:       G. Argo interdomain message notification
+ * - VIRQ_ENOMEM:     G. (DOM0) Low on heap memory
+ * - VIRQ_XENPMU:     V.  PMC interrupt
+ * - VIRQ_ARCH_0
+ * - VIRQ_ARCH_1
+ * - VIRQ_ARCH_2
+ * - VIRQ_ARCH_3
+ * - VIRQ_ARCH_4
+ * - VIRQ_ARCH_5
+ * - VIRQ_ARCH_6
+ * - VIRQ_ARCH_7
  */
 /* ` enum virq { */
-#define VIRQ_TIMER      0  /* V. Timebase update, and/or requested timeout.  */
-#define VIRQ_DEBUG      1  /* V. Request guest to dump debug info.           */
-#define VIRQ_CONSOLE    2  /* G. (DOM0) Bytes received on emergency console. */
-#define VIRQ_DOM_EXC    3  /* G. (DOM0) Exceptional event for some domain.   */
-#define VIRQ_TBUF       4  /* G. (DOM0) Trace buffer has records available.  */
-#define VIRQ_DEBUGGER   6  /* G. (DOM0) A domain has paused for debugging.   */
-#define VIRQ_XENOPROF   7  /* V. XenOprofile interrupt: new sample available */
-#define VIRQ_CON_RING   8  /* G. (DOM0) Bytes received on console            */
-#define VIRQ_PCPU_STATE 9  /* G. (DOM0) PCPU state changed                   */
-#define VIRQ_MEM_EVENT  10 /* G. (DOM0) A memory event has occurred          */
-#define VIRQ_ARGO       11 /* G. Argo interdomain message notification       */
-#define VIRQ_ENOMEM     12 /* G. (DOM0) Low on heap memory       */
-#define VIRQ_XENPMU     13 /* V.  PMC interrupt                              */
+#define VIRQ_TIMER      0
+#define VIRQ_DEBUG      1
+#define VIRQ_CONSOLE    2
+#define VIRQ_DOM_EXC    3
+#define VIRQ_TBUF       4
+#define VIRQ_DEBUGGER   6
+#define VIRQ_XENOPROF   7
+#define VIRQ_CON_RING   8
+#define VIRQ_PCPU_STATE 9
+#define VIRQ_MEM_EVENT  10
+#define VIRQ_ARGO       11
+#define VIRQ_ENOMEM     12
+#define VIRQ_XENPMU     13
 
 /* Architecture-specific VIRQ definitions. */
 #define VIRQ_ARCH_0    16
@@ -205,12 +276,14 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
 
 #define NR_VIRQS       24
 
-/*
- * ` enum neg_errnoval
- * ` HYPERVISOR_mmu_update(const struct mmu_update reqs[],
- * `                       unsigned count, unsigned *done_out,
- * `                       unsigned foreigndom)
- * `
+/**
+ * DOC: HYPERVISOR_mmu_update
+ *
+ * enum neg_errnoval
+ * HYPERVISOR_mmu_update(const struct mmu_update reqs[],
+ *                       unsigned count, unsigned *done_out,
+ *                       unsigned foreigndom)
+ *
  * @reqs is an array of mmu_update_t structures ((ptr, val) pairs).
  * @count is the length of the above array.
  * @pdone is an output parameter indicating number of completed operations
@@ -222,8 +295,9 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
  *                     x == 0 => PFD == DOMID_SELF
  *                     x != 0 => PFD == x - 1
  *
+ *
  * Sub-commands: ptr[1:0] specifies the appropriate MMU_* command.
- * -------------
+ *
  * ptr[1:0] == MMU_NORMAL_PT_UPDATE:
  * Updates an entry in a page table belonging to PFD. If updating an L1 table,
  * and the new table entry is valid/present, the mapped frame must belong to
@@ -354,16 +428,16 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
 #define MMU_PT_UPDATE_NO_TRANSLATE 3 /* checked '*ptr = val'. ptr is MA.      */
                                      /* val never translated.                 */
 
-/*
- * MMU EXTENDED OPERATIONS
+/**
+ * DOC: MMU EXTENDED OPERATIONS
  *
- * ` enum neg_errnoval
- * ` HYPERVISOR_mmuext_op(mmuext_op_t uops[],
- * `                      unsigned int count,
- * `                      unsigned int *pdone,
- * `                      unsigned int foreigndom)
- */
-/* HYPERVISOR_mmuext_op() accepts a list of mmuext_op structures.
+ * enum neg_errnoval
+ * HYPERVISOR_mmuext_op(mmuext_op_t uops[],
+ *                      unsigned int count,
+ *                      unsigned int *pdone,
+ *                      unsigned int foreigndom)
+ *
+ * HYPERVISOR_mmuext_op() accepts a list of mmuext_op structures.
  * A foreigndom (FD) can be specified (or DOMID_SELF for none).
  * Where the FD has some effect, it is described below.
  *
@@ -468,19 +542,21 @@ typedef struct mmuext_op mmuext_op_t;
 DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
 #endif
 
-/*
- * ` enum neg_errnoval
- * ` HYPERVISOR_update_va_mapping(unsigned long va, u64 val,
- * `                              enum uvm_flags flags)
- * `
- * ` enum neg_errnoval
- * ` HYPERVISOR_update_va_mapping_otherdomain(unsigned long va, u64 val,
- * `                                          enum uvm_flags flags,
- * `                                          domid_t domid)
- * `
- * ` @va: The virtual address whose mapping we want to change
- * ` @val: The new page table entry, must contain a machine address
- * ` @flags: Control TLB flushes
+/**
+ * DOC: HYPERVISOR_update_va_mapping
+ *
+ * enum neg_errnoval
+ * HYPERVISOR_update_va_mapping(unsigned long va, u64 val,
+ *                              enum uvm_flags flags)
+ *
+ * enum neg_errnoval
+ * HYPERVISOR_update_va_mapping_otherdomain(unsigned long va, u64 val,
+ *                                          enum uvm_flags flags,
+ *                                          domid_t domid)
+ *
+ * @va: The virtual address whose mapping we want to change
+ * @val: The new page table entry, must contain a machine address
+ * @flags: Control TLB flushes
  */
 /* These are passed as 'flags' to update_va_mapping. They can be ORed. */
 /* When specifying UVMF_MULTI, also OR in a pointer to a CPU bitmap.   */
@@ -495,11 +571,13 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
 #define UVMF_ALL            (xen_mk_ulong(1)<<2) /* Flush all TLBs.       */
 /* ` } */
 
-/*
- * ` int
- * ` HYPERVISOR_console_io(unsigned int cmd,
- * `                       unsigned int count,
- * `                       char buffer[]);
+/**
+ * DOC: HYPERVISOR_console_io
+ *
+ * int
+ * HYPERVISOR_console_io(unsigned int cmd,
+ *                       unsigned int count,
+ *                       char buffer[]);
  *
  * @cmd: Command (see below)
  * @count: Size of the buffer to read/write
@@ -522,29 +600,43 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
 #define CONSOLEIO_write         0
 #define CONSOLEIO_read          1
 
-/*
+/**
+ * DOC: VMASST_CMD_enable and VMASST_CMD_disable
  * Commands to HYPERVISOR_vm_assist().
  */
 #define VMASST_CMD_enable                0
 #define VMASST_CMD_disable               1
 
-/* x86/32 guests: simulate full 4GB segment limits. */
+/**
+ * DOC: VMASST_TYPE_4gb_segments
+ * x86/32 guests: simulate full 4GB segment limits.
+ */
 #define VMASST_TYPE_4gb_segments         0
 
-/* x86/32 guests: trap (vector 15) whenever above vmassist is used. */
+/**
+ * DOC: VMASST_TYPE_4gb_segments_notify
+ * x86/32 guests: trap (vector 15) whenever above vmassist is used.
+ */
 #define VMASST_TYPE_4gb_segments_notify  1
 
-/*
+/**
+ * DOC: VMASST_TYPE_writable_pagetables
+ *
  * x86 guests: support writes to bottom-level PTEs.
  * NB1. Page-directory entries cannot be written.
  * NB2. Guest must continue to remove all writable mappings of PTEs.
  */
 #define VMASST_TYPE_writable_pagetables  2
 
-/* x86/PAE guests: support PDPTs above 4GB. */
+/**
+ * DOC: VMASST_TYPE_pae_extended_cr3
+ * x86/PAE guests: support PDPTs above 4GB.
+ */
 #define VMASST_TYPE_pae_extended_cr3     3
 
-/*
+/**
+ * DOC: VMASST_TYPE_architectural_iopl
+ *
  * x86 guests: Sane behaviour for virtual iopl
  *  - virtual iopl updated from do_iret() hypercalls.
  *  - virtual iopl reported in bounce frames.
@@ -552,14 +644,18 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
  */
 #define VMASST_TYPE_architectural_iopl   4
 
-/*
+/**
+ * DOC: VMASST_TYPE_runstate_update_flag
+ *
  * All guests: activate update indicator in vcpu_runstate_info
  * Enable setting the XEN_RUNSTATE_UPDATE flag in guest memory mapped
  * vcpu_runstate_info during updates of the runstate information.
  */
 #define VMASST_TYPE_runstate_update_flag 5
 
-/*
+/**
+ * DOC: VMASST_TYPE_m2p_strict
+ *
  * x86/64 guests: strictly hide M2P from user mode.
  * This allows the guest to control respective hypervisor behavior:
  * - when not set, L4 tables get created with the respective slot blank,
@@ -578,10 +674,15 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
 /* Domain ids >= DOMID_FIRST_RESERVED cannot be used for ordinary domains. */
 #define DOMID_FIRST_RESERVED xen_mk_uint(0x7FF0)
 
-/* DOMID_SELF is used in certain contexts to refer to oneself. */
+/**
+ * DOC: DOMID_SELF
+ * DOMID_SELF is used in certain contexts to refer to oneself.
+ */
 #define DOMID_SELF           xen_mk_uint(0x7FF0)
 
-/*
+/**
+ * DOC: DOMID_IO
+ *
  * DOMID_IO is used to restrict page-table updates to mapping I/O memory.
  * Although no Foreign Domain need be specified to map I/O pages, DOMID_IO
  * is useful to ensure that no mappings to the OS's own heap are accidentally
@@ -594,7 +695,9 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
  */
 #define DOMID_IO             xen_mk_uint(0x7FF1)
 
-/*
+/**
+ * DOC: DOMID_XEN
+ *
  * DOMID_XEN is used to allow privileged domains to map restricted parts of
  * Xen's heap space (e.g., the machine_to_phys table).
  * This only makes sense as
@@ -605,38 +708,55 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
  */
 #define DOMID_XEN            xen_mk_uint(0x7FF2)
 
-/*
- * DOMID_COW is used as the owner of sharable pages */
+/**
+ * DOC: DOMID_COW
+ * DOMID_COW is used as the owner of sharable pages
+ */
 #define DOMID_COW            xen_mk_uint(0x7FF3)
 
-/* DOMID_INVALID is used to identify pages with unknown owner. */
+/**
+ * DOC: DOMID_INVALID
+ * DOMID_INVALID is used to identify pages with unknown owner.
+ */
 #define DOMID_INVALID        xen_mk_uint(0x7FF4)
 
-/* Idle domain. */
+/**
+ * DOC: DOMID_IDLE
+ * Idle domain.
+ */
 #define DOMID_IDLE           xen_mk_uint(0x7FFF)
 
-/* Mask for valid domain id values */
+/**
+ * DOC: DOMID_MASK
+ * Mask for valid domain id values
+ */
 #define DOMID_MASK           xen_mk_uint(0x7FFF)
 
 #ifndef __ASSEMBLY__
 
 typedef uint16_t domid_t;
 
-/*
+/**
+ * struct mmu_update - HYPERVISOR_mmu_update
+ *
  * Send an array of these to HYPERVISOR_mmu_update().
  * NB. The fields are natural pointer/address size for this architecture.
  */
 struct mmu_update {
-    uint64_t ptr;       /* Machine address of PTE. */
-    uint64_t val;       /* New contents of PTE.    */
+    /** @ptr: Machine address of PTE. */
+    uint64_t ptr;
+    /** @val: New contents of PTE. */
+    uint64_t val;
 };
 typedef struct mmu_update mmu_update_t;
 DEFINE_XEN_GUEST_HANDLE(mmu_update_t);
 
-/*
- * ` enum neg_errnoval
- * ` HYPERVISOR_multicall(multicall_entry_t call_list[],
- * `                      uint32_t nr_calls);
+/**
+ * struct multicall_entry - HYPERVISOR_multicall
+ *
+ * enum neg_errnoval
+ * HYPERVISOR_multicall(multicall_entry_t call_list[],
+ *                      uint32_t nr_calls);
  *
  * NB. The fields are logically the natural register size for this
  * architecture. In cases where xen_ulong_t is larger than this then
@@ -650,34 +770,40 @@ typedef struct multicall_entry multicall_entry_t;
 DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);
 
 #if __XEN_INTERFACE_VERSION__ < 0x00040400
-/*
+/**
+ * DOC: NR_EVENT_CHANNELS
+ *
  * Event channel endpoints per domain (when using the 2-level ABI):
  *  1024 if a long is 32 bits; 4096 if a long is 64 bits.
  */
 #define NR_EVENT_CHANNELS EVTCHN_2L_NR_CHANNELS
 #endif
 
+/**
+ * struct vcpu_time_info
+ *
+ * Updates to the following values are preceded and followed by an
+ * increment of 'version'. The guest can therefore detect updates by
+ * looking for changes to 'version'. If the least-significant bit of
+ * the version number is set then an update is in progress and the guest
+ * must wait to read a consistent set of values.
+ * The correct way to interact with the version number is similar to
+ * Linux's seqlock: see the implementations of read_seqbegin/read_seqretry.
+ *
+ * Current system time:
+ *   system_time +
+ *   ((((tsc - tsc_timestamp) << tsc_shift) * tsc_to_system_mul) >> 32)
+ * CPU frequency (Hz):
+ *   ((10^9 << 32) / tsc_to_system_mul) >> tsc_shift
+ */
 struct vcpu_time_info {
-    /*
-     * Updates to the following values are preceded and followed by an
-     * increment of 'version'. The guest can therefore detect updates by
-     * looking for changes to 'version'. If the least-significant bit of
-     * the version number is set then an update is in progress and the guest
-     * must wait to read a consistent set of values.
-     * The correct way to interact with the version number is similar to
-     * Linux's seqlock: see the implementations of read_seqbegin/read_seqretry.
-     */
     uint32_t version;
     uint32_t pad0;
-    uint64_t tsc_timestamp;   /* TSC at last update of time vals.  */
-    uint64_t system_time;     /* Time, in nanosecs, since boot.    */
-    /*
-     * Current system time:
-     *   system_time +
-     *   ((((tsc - tsc_timestamp) << tsc_shift) * tsc_to_system_mul) >> 32)
-     * CPU frequency (Hz):
-     *   ((10^9 << 32) / tsc_to_system_mul) >> tsc_shift
-     */
+    /** @tsc_timestamp: TSC at last update of time vals. */
+    uint64_t tsc_timestamp;
+    /** @system_time: Time, in nanosecs, since boot. */
+    uint64_t system_time;
+
     uint32_t tsc_to_system_mul;
     int8_t   tsc_shift;
 #if __XEN_INTERFACE_VERSION__ > 0x040600
@@ -692,18 +818,23 @@ typedef struct vcpu_time_info vcpu_time_info_t;
 #define XEN_PVCLOCK_TSC_STABLE_BIT     (1 << 0)
 #define XEN_PVCLOCK_GUEST_STOPPED      (1 << 1)
 
+/**
+ * struct vcpu_info
+ */
 struct vcpu_info {
-    /*
-     * 'evtchn_upcall_pending' is written non-zero by Xen to indicate
-     * a pending notification for a particular VCPU. It is then cleared
-     * by the guest OS /before/ checking for pending work, thus avoiding
-     * a set-and-check race. Note that the mask is only accessed by Xen
-     * on the CPU that is currently hosting the VCPU. This means that the
-     * pending and mask flags can be updated by the guest without special
-     * synchronisation (i.e., no need for the x86 LOCK prefix).
-     * This may seem suboptimal because if the pending flag is set by
-     * a different CPU then an IPI may be scheduled even when the mask
-     * is set. However, note:
+    /**
+     * @evtchn_upcall_pending:
+     *
+     * it is written non-zero by Xen to indicate a pending notification
+     * for a particular VCPU. It is then cleared by the guest OS
+     * /before/ checking for pending work, thus avoiding a set-and-check
+     * race. Note that the mask is only accessed by Xen on the CPU that
+     * is currently hosting the VCPU. This means that the pending and
+     * mask flags can be updated by the guest without special
+     * synchronisation (i.e., no need for the x86 LOCK prefix).  This
+     * may seem suboptimal because if the pending flag is set by a
+     * different CPU then an IPI may be scheduled even when the mask is
+     * set. However, note:
      *  1. The task of 'interrupt holdoff' is covered by the per-event-
      *     channel mask bits. A 'noisy' event that is continually being
      *     triggered can be masked at source at this very precise
@@ -732,61 +863,69 @@ struct vcpu_info {
 typedef struct vcpu_info vcpu_info_t;
 #endif
 
-/*
- * `incontents 200 startofday_shared Start-of-day shared data structure
+/**
+ * struct shared_info - Start-of-day shared data structure
+ *
  * Xen/kernel shared data -- pointer provided in start_info.
  *
  * This structure is defined to be both smaller than a page, and the
  * only data on the shared page, but may vary in actual size even within
  * compatible Xen versions; guests should not rely on the size
  * of this structure remaining constant.
+ *
+ * A domain can create "event channels" on which it can send and receive
+ * asynchronous event notifications. There are three classes of event that
+ * are delivered by this mechanism:
+ *  1. Bi-directional inter- and intra-domain connections. Domains must
+ *     arrange out-of-band to set up a connection (usually by allocating
+ *     an unbound 'listener' port and avertising that via a storage service
+ *     such as xenstore).
+ *  2. Physical interrupts. A domain with suitable hardware-access
+ *     privileges can bind an event-channel port to a physical interrupt
+ *     source.
+ *  3. Virtual interrupts ('events'). A domain can bind an event-channel
+ *     port to a virtual interrupt source, such as the virtual-timer
+ *     device or the emergency console.
+ *
+ *
+ * @evtchn_pending: pending notifications
+ * @evtchn_mask: mask/unmask notifications
+ *
+ * Event channels are addressed by a "port index". Each channel is
+ * associated with two bits of information:
+ *  1. PENDING -- notifies the domain that there is a pending notification
+ *     to be processed. This bit is cleared by the guest.
+ *  2. MASK -- if this bit is clear then a 0->1 transition of PENDING
+ *     will cause an asynchronous upcall to be scheduled. This bit is only
+ *     updated by the guest. It is read-only within Xen. If a channel
+ *     becomes pending while the channel is masked then the 'edge' is lost
+ *     (i.e., when the channel is unmasked, the guest must manually handle
+ *     pending notifications as no upcall will be scheduled by Xen).
+ *
+ * To expedite scanning of pending notifications, any 0->1 pending
+ * transition on an unmasked channel causes a corresponding bit in a
+ * per-vcpu selector word to be set. Each bit in the selector covers a
+ * 'C long' in the PENDING bitfield array.
+ *
+ *
+ * @wc_version: wallclock time version
+ * @wc_sec: secs offset from Unix epoc
+ * @wc_nsec: nsecs offset from Unix epoc
+ *
+ * Wallclock time: updated by control software or RTC emulation.
+ * Guests should base their gettimeofday() syscall on this
+ * wallclock-base value.
+ * The values of wc_sec and wc_nsec are offsets from the Unix epoch
+ * adjusted by the domain's 'time offset' (in seconds) as set either
+ * by XEN_DOMCTL_settimeoffset, or adjusted via a guest write to the
+ * emulated RTC.
  */
 struct shared_info {
     struct vcpu_info vcpu_info[XEN_LEGACY_MAX_VCPUS];
 
-    /*
-     * A domain can create "event channels" on which it can send and receive
-     * asynchronous event notifications. There are three classes of event that
-     * are delivered by this mechanism:
-     *  1. Bi-directional inter- and intra-domain connections. Domains must
-     *     arrange out-of-band to set up a connection (usually by allocating
-     *     an unbound 'listener' port and avertising that via a storage service
-     *     such as xenstore).
-     *  2. Physical interrupts. A domain with suitable hardware-access
-     *     privileges can bind an event-channel port to a physical interrupt
-     *     source.
-     *  3. Virtual interrupts ('events'). A domain can bind an event-channel
-     *     port to a virtual interrupt source, such as the virtual-timer
-     *     device or the emergency console.
-     *
-     * Event channels are addressed by a "port index". Each channel is
-     * associated with two bits of information:
-     *  1. PENDING -- notifies the domain that there is a pending notification
-     *     to be processed. This bit is cleared by the guest.
-     *  2. MASK -- if this bit is clear then a 0->1 transition of PENDING
-     *     will cause an asynchronous upcall to be scheduled. This bit is only
-     *     updated by the guest. It is read-only within Xen. If a channel
-     *     becomes pending while the channel is masked then the 'edge' is lost
-     *     (i.e., when the channel is unmasked, the guest must manually handle
-     *     pending notifications as no upcall will be scheduled by Xen).
-     *
-     * To expedite scanning of pending notifications, any 0->1 pending
-     * transition on an unmasked channel causes a corresponding bit in a
-     * per-vcpu selector word to be set. Each bit in the selector covers a
-     * 'C long' in the PENDING bitfield array.
-     */
     xen_ulong_t evtchn_pending[sizeof(xen_ulong_t) * 8];
     xen_ulong_t evtchn_mask[sizeof(xen_ulong_t) * 8];
 
-    /*
-     * Wallclock time: updated by control software or RTC emulation.
-     * Guests should base their gettimeofday() syscall on this
-     * wallclock-base value.
-     * The values of wc_sec and wc_nsec are offsets from the Unix epoch
-     * adjusted by the domain's 'time offset' (in seconds) as set either
-     * by XEN_DOMCTL_settimeoffset, or adjusted via a guest write to the
-     * emulated RTC.
-     */
     uint32_t wc_version;      /* Version counter: see vcpu_time_info_t. */
     uint32_t wc_sec;
     uint32_t wc_nsec;
@@ -804,8 +943,9 @@ struct shared_info {
 typedef struct shared_info shared_info_t;
 #endif
 
-/*
- * `incontents 200 startofday Start-of-day memory layout
+#ifdef XEN_HAVE_PV_GUEST_ENTRY
+/**
+ * struct start_info - Start-of-day memory layout
  *
  *  1. The domain is started within contiguous virtual-memory region.
  *  2. The contiguous region ends on an aligned 4MB boundary.
@@ -841,38 +981,72 @@ typedef struct shared_info shared_info_t;
  * 32-bit and runs under a 64-bit hypervisor should _NOT_ use two of the
  * pages preceding pt_base and mark them as reserved/unused.
  */
-#ifdef XEN_HAVE_PV_GUEST_ENTRY
 struct start_info {
     /* THE FOLLOWING ARE FILLED IN BOTH ON INITIAL BOOT AND ON RESUME.    */
-    char magic[32];             /* "xen-<version>-<platform>".            */
-    unsigned long nr_pages;     /* Total pages allocated to this domain.  */
-    unsigned long shared_info;  /* MACHINE address of shared info struct. */
-    uint32_t flags;             /* SIF_xxx flags.                         */
-    xen_pfn_t store_mfn;        /* MACHINE page number of shared page.    */
-    uint32_t store_evtchn;      /* Event channel for store communication. */
+    /** @magic: "xen-<version>-<platform>". */
+    char magic[32];
+    /** @nr_pages: Total pages allocated to this domain. */
+    unsigned long nr_pages;
+    /** @shared_info: MACHINE address of shared info struct. */
+    unsigned long shared_info;
+    /** flags: SIF_xxx flags. */
+    uint32_t flags;
+    /** @store_mfn: MACHINE page number of shared page. */
+    xen_pfn_t store_mfn;
+    /** @store_evtchn: Event channel for store communication. */
+    uint32_t store_evtchn;
     union {
         struct {
-            xen_pfn_t mfn;      /* MACHINE page number of console page.   */
-            uint32_t  evtchn;   /* Event channel for console page.        */
+            /**
+             * @console.domU.mfn: MACHINE page number of console page.
+             */
+            xen_pfn_t mfn;
+            /**
+             * @console.domU.evtchn: Event channel for console page.
+             */
+            uint32_t evtchn;
         } domU;
         struct {
-            uint32_t info_off;  /* Offset of console_info struct.         */
-            uint32_t info_size; /* Size of console_info struct from start.*/
+            /**
+             * @console.dom0.info_off: Offset of console_info struct.
+             */
+            uint32_t info_off;
+            /**
+             * @info_size: Size of console_info struct from start.
+             */
+            uint32_t info_size;
         } dom0;
     } console;
     /* THE FOLLOWING ARE ONLY FILLED IN ON INITIAL BOOT (NOT RESUME).     */
-    unsigned long pt_base;      /* VIRTUAL address of page directory.     */
-    unsigned long nr_pt_frames; /* Number of bootstrap p.t. frames.       */
-    unsigned long mfn_list;     /* VIRTUAL address of page-frame list.    */
-    unsigned long mod_start;    /* VIRTUAL address of pre-loaded module   */
-                                /* (PFN of pre-loaded module if           */
-                                /*  SIF_MOD_START_PFN set in flags).      */
-    unsigned long mod_len;      /* Size (bytes) of pre-loaded module.     */
+    /** @pt_base: VIRTUAL address of page directory. */
+    unsigned long pt_base;
+    /** @nr_pt_frames: Number of bootstrap p.t. frames. */
+    unsigned long nr_pt_frames;
+    /** @mfn_list: VIRTUAL address of page-frame list. */
+    unsigned long mfn_list;
+    /**
+     * @mod_start: VIRTUAL address of pre-loaded module.
+     * (PFN of pre-loaded module if SIF_MOD_START_PFN set in flags).
+     */
+    unsigned long mod_start;
+    /** @mod_len: Size (bytes) of pre-loaded module. */
+    unsigned long mod_len;
 #define MAX_GUEST_CMDLINE 1024
     int8_t cmd_line[MAX_GUEST_CMDLINE];
-    /* The pfn range here covers both page table and p->m table frames.   */
-    unsigned long first_p2m_pfn;/* 1st pfn forming initial P->M table.    */
-    unsigned long nr_p2m_frames;/* # of pfns forming initial P->M table.  */
+    /**
+     * @first_p2m_pfn:
+     *
+     * 1st pfn forming initial P->M table from the pfn range that covers
+     * both page table and p->m table frames.
+     */
+    unsigned long first_p2m_pfn;
+    /**
+     * @nr_p2m_frames:
+     *
+     * # of pfns forming initial P->M table from the pfn range that
+     * covers both page table and p->m table frames.
+     */
+    unsigned long nr_p2m_frames;
 };
 typedef struct start_info start_info_t;
 
@@ -883,16 +1057,29 @@ typedef struct start_info start_info_t;
 #endif
 #endif /* XEN_HAVE_PV_GUEST_ENTRY */
 
-/* These flags are passed in the 'flags' field of start_info_t. */
-#define SIF_PRIVILEGED    (1<<0)  /* Is the domain privileged? */
-#define SIF_INITDOMAIN    (1<<1)  /* Is this the initial control domain? */
-#define SIF_MULTIBOOT_MOD (1<<2)  /* Is mod_start a multiboot module? */
-#define SIF_MOD_START_PFN (1<<3)  /* Is mod_start a PFN? */
-#define SIF_VIRT_P2M_4TOOLS (1<<4) /* Do Xen tools understand a virt. mapped */
-                                   /* P->M making the 3 level tree obsolete? */
-#define SIF_PM_MASK       (0xFF<<8) /* reserve 1 byte for xen-pm options */
-
-/*
+/**
+ * DOC: SIF_*
+ *
+ * These flags are passed in the 'flags' field of start_info_t.
+ *
+ * - SIF_PRIVILEGED:       Is the domain privileged?
+ * - SIF_INITDOMAIN:       Is this the initial control domain?
+ * - SIF_MULTIBOOT_MOD:    Is mod_start a multiboot module?
+ * - SIF_MOD_START_PFN:    Is mod_start a PFN?
+ * - SIF_VIRT_P2M_4TOOLS:  Do Xen tools understand a virt. mapped
+ *                         P->M making the 3 level tree obsolete?
+ * - SIF_PM_MASK:          reserve 1 byte for xen-pm options
+ */
+#define SIF_PRIVILEGED    (1<<0)
+#define SIF_INITDOMAIN    (1<<1)
+#define SIF_MULTIBOOT_MOD (1<<2)
+#define SIF_MOD_START_PFN (1<<3)
+#define SIF_VIRT_P2M_4TOOLS (1<<4)
+#define SIF_PM_MASK       (0xFF<<8)
+
+/**
+ * struct xen_multiboot_mod_list
+ *
  * A multiboot module is a package containing modules very similar to a
  * multiboot module array. The only differences are:
  * - the array of module descriptors is by convention simply at the beginning
@@ -908,13 +1095,13 @@ typedef struct start_info start_info_t;
  */
 struct xen_multiboot_mod_list
 {
-    /* Address of first byte of the module */
+    /** @mod_start: Address of first byte of the module */
     uint32_t mod_start;
-    /* Address of last byte of the module (inclusive) */
+    /** @mod_end: Address of last byte of the module (inclusive) */
     uint32_t mod_end;
-    /* Address of zero-terminated command line */
+    /** @cmdline: Address of zero-terminated command line */
     uint32_t cmdline;
-    /* Unused, must be zero */
+    /** @pad: Unused, must be zero */
     uint32_t pad;
 };
 /*
@@ -984,7 +1171,9 @@ typedef struct {
     uint8_t a[16];
 } xen_uuid_t;
 
-/*
+/**
+ * DOC: XEN_DEFINE_UUID
+ *
  * XEN_DEFINE_UUID(0x00112233, 0x4455, 0x6677, 0x8899,
  *                 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff)
  * will construct UUID 00112233-4455-6677-8899-aabbccddeeff presented as
-- 
2.17.1

[PATCH 13/14] kernel-doc: public/elfnote.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/elfnote.h | 109 ++++++++++++++++++++++++++---------
 1 file changed, 81 insertions(+), 28 deletions(-)

diff --git a/xen/include/public/elfnote.h b/xen/include/public/elfnote.h
index 181cbc4ec7..1dd567a6f1 100644
--- a/xen/include/public/elfnote.h
+++ b/xen/include/public/elfnote.h
@@ -27,8 +27,8 @@
 #ifndef __XEN_PUBLIC_ELFNOTE_H__
 #define __XEN_PUBLIC_ELFNOTE_H__
 
-/*
- * `incontents 200 elfnotes ELF notes
+/**
+ * DOC: ELF notes
  *
  * The notes should live in a PT_NOTE segment and have "Xen" in the
  * name field.
@@ -43,26 +43,35 @@
  * as ASCIZ type.
  */
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_INFO
  * NAME=VALUE pair (string).
  */
 #define XEN_ELFNOTE_INFO           0
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_ENTRY
+ *
  * The virtual address of the entry point (numeric).
  *
  * LEGACY: VIRT_ENTRY
  */
 #define XEN_ELFNOTE_ENTRY          1
 
-/* The virtual address of the hypercall transfer page (numeric).
+/**
+ * DOC: XEN_ELFNOTE_HYPERCALL_PAGE
+ *
+ * The virtual address of the hypercall transfer page (numeric).
  *
  * LEGACY: HYPERCALL_PAGE. (n.b. legacy value is a physical page
  * number not a virtual address)
  */
 #define XEN_ELFNOTE_HYPERCALL_PAGE 2
 
-/* The virtual address where the kernel image should be mapped (numeric).
+/**
+ * DOC: XEN_ELFNOTE_VIRT_BASE
+ *
+ * The virtual address where the kernel image should be mapped (numeric).
  *
  * Defaults to 0.
  *
@@ -70,7 +79,9 @@
  */
 #define XEN_ELFNOTE_VIRT_BASE      3
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_PADDR_OFFSET
+ *
  * The offset of the ELF paddr field from the actual required
  * pseudo-physical address (numeric).
  *
@@ -82,35 +93,45 @@
  */
 #define XEN_ELFNOTE_PADDR_OFFSET   4
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_XEN_VERSION
+ *
  * The version of Xen that we work with (string).
  *
  * LEGACY: XEN_VER
  */
 #define XEN_ELFNOTE_XEN_VERSION    5
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_GUEST_OS
+ *
  * The name of the guest operating system (string).
  *
  * LEGACY: GUEST_OS
  */
 #define XEN_ELFNOTE_GUEST_OS       6
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_GUEST_VERSION
+ *
  * The version of the guest operating system (string).
  *
  * LEGACY: GUEST_VER
  */
 #define XEN_ELFNOTE_GUEST_VERSION  7
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_LOADER
+ *
  * The loader type (string).
  *
  * LEGACY: LOADER
  */
 #define XEN_ELFNOTE_LOADER         8
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_PAE_MODE
+ *
  * The kernel supports PAE (x86/32 only, string = "yes", "no" or
  * "bimodal").
  *
@@ -126,7 +147,9 @@
  */
 #define XEN_ELFNOTE_PAE_MODE       9
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_FEATURES
+ *
  * The features supported/required by this kernel (string).
  *
  * The string must consist of a list of feature names (as given in
@@ -138,7 +161,9 @@
  */
 #define XEN_ELFNOTE_FEATURES      10
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_BSD_SYMTAB
+ *
  * The kernel requires the symbol table to be loaded (string = "yes" or "no")
  * LEGACY: BSD_SYMTAB (n.b. The legacy treated the presence or absence
  * of this string as a boolean flag rather than requiring "yes" or
@@ -146,7 +171,9 @@
  */
 #define XEN_ELFNOTE_BSD_SYMTAB    11
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_HV_START_LOW
+ *
  * The lowest address the hypervisor hole can begin at (numeric).
  *
  * This must not be set higher than HYPERVISOR_VIRT_START. Its presence
@@ -155,13 +182,17 @@
  */
 #define XEN_ELFNOTE_HV_START_LOW  12
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_L1_MFN_VALID
+ *
  * List of maddr_t-sized mask/value pairs describing how to recognize
  * (non-present) L1 page table entries carrying valid MFNs (numeric).
  */
 #define XEN_ELFNOTE_L1_MFN_VALID  13
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_SUSPEND_CANCEL
+ *
  * Whether or not the guest supports cooperative suspend cancellation.
  * This is a numeric value.
  *
@@ -169,7 +200,9 @@
  */
 #define XEN_ELFNOTE_SUSPEND_CANCEL 14
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_INIT_P2M
+ *
  * The (non-default) location the initial phys-to-machine map should be
  * placed at by the hypervisor (Dom0) or the tools (DomU).
  * The kernel must be prepared for this mapping to be established using
@@ -182,13 +215,17 @@
  */
 #define XEN_ELFNOTE_INIT_P2M      15
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_MOD_START_PFN
+ *
  * Whether or not the guest can deal with being passed an initrd not
  * mapped through its initial page tables.
  */
 #define XEN_ELFNOTE_MOD_START_PFN 16
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_SUPPORTED_FEATURES
+ *
  * The features supported by this kernel (numeric).
  *
  * Other than XEN_ELFNOTE_FEATURES on pre-4.2 Xen, this note allows a
@@ -201,7 +238,9 @@
  */
 #define XEN_ELFNOTE_SUPPORTED_FEATURES 17
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_PHYS32_ENTRY
+ *
  * Physical entry point into the kernel.
  *
  * 32bit entry point into the kernel. When requested to launch the
@@ -211,12 +250,16 @@
  */
 #define XEN_ELFNOTE_PHYS32_ENTRY 18
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_MAX
+ *
  * The number of the highest elfnote defined.
  */
 #define XEN_ELFNOTE_MAX XEN_ELFNOTE_PHYS32_ENTRY
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_CRASH_INFO
+ *
  * System information exported through crash notes.
  *
  * The kexec / kdump code will create one XEN_ELFNOTE_CRASH_INFO
@@ -225,7 +268,9 @@
  */
 #define XEN_ELFNOTE_CRASH_INFO 0x1000001
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_CRASH_REGS
+ *
  * System registers exported through crash notes.
  *
  * The kexec / kdump code will create one XEN_ELFNOTE_CRASH_REGS
@@ -236,7 +281,9 @@
 #define XEN_ELFNOTE_CRASH_REGS 0x1000002
 
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_DUMPCORE_NONE
+ *
  * xen dump-core none note.
  * xm dump-core code will create one XEN_ELFNOTE_DUMPCORE_NONE
  * in its dump file to indicate that the file is xen dump-core
@@ -245,7 +292,9 @@
  */
 #define XEN_ELFNOTE_DUMPCORE_NONE               0x2000000
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_DUMPCORE_HEADER
+ *
  * xen dump-core header note.
  * xm dump-core code will create one XEN_ELFNOTE_DUMPCORE_HEADER
  * in its dump file.
@@ -253,7 +302,9 @@
  */
 #define XEN_ELFNOTE_DUMPCORE_HEADER             0x2000001
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_DUMPCORE_XEN_VERSION
+ *
  * xen dump-core xen version note.
  * xm dump-core code will create one XEN_ELFNOTE_DUMPCORE_XEN_VERSION
  * in its dump file. It contains the xen version obtained via the
@@ -262,7 +313,9 @@
  */
 #define XEN_ELFNOTE_DUMPCORE_XEN_VERSION        0x2000002
 
-/*
+/**
+ * DOC: XEN_ELFNOTE_DUMPCORE_FORMAT_VERSION
+ *
  * xen dump-core format version note.
  * xm dump-core code will create one XEN_ELFNOTE_DUMPCORE_FORMAT_VERSION
  * in its dump file. It contains a format version identifier.
-- 
2.17.1

[PATCH 14/14] kernel-doc: public/hvm/params.h

[Stefano Stabellini]

From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/hvm/params.h | 158 +++++++++++++++++++++++++-------
 1 file changed, 124 insertions(+), 34 deletions(-)

diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index 0a91bfa749..1f2a9fe4f9 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -40,13 +40,16 @@
 
 #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
 
-/*
+/**
+ * DOC: HVMOP_set_param and HVMOP_get_param
  * Parameter space for HVMOP_{set,get}_param.
  */
 
 #define HVM_PARAM_CALLBACK_IRQ 0
 #define HVM_PARAM_CALLBACK_IRQ_TYPE_MASK xen_mk_ullong(0xFF00000000000000)
-/*
+/**
+ * DOC: HVM_PARAM_CALLBACK_*
+ *
  * How should CPU0 event-channel notifications be delivered?
  *
  * If val == 0 then CPU0 event-channel notifications are not delivered.
@@ -54,26 +57,34 @@
  */
 
 #define HVM_PARAM_CALLBACK_TYPE_GSI      0
-/*
+/**
+ * DOC: HVM_PARAM_CALLBACK_TYPE_GSI
+ *
  * val[55:0] is a delivery GSI.  GSI 0 cannot be used, as it aliases val == 0,
  * and disables all notifications.
  */
 
 #define HVM_PARAM_CALLBACK_TYPE_PCI_INTX 1
-/*
+/**
+ * DOC: HVM_PARAM_CALLBACK_TYPE_PCI_INTX
+ *
  * val[55:0] is a delivery PCI INTx line:
  * Domain = val[47:32], Bus = val[31:16] DevFn = val[15:8], IntX = val[1:0]
  */
 
 #if defined(__i386__) || defined(__x86_64__)
 #define HVM_PARAM_CALLBACK_TYPE_VECTOR   2
-/*
+/**
+ * DOC: HVM_PARAM_CALLBACK_TYPE_VECTOR
+ *
  * val[7:0] is a vector number.  Check for XENFEAT_hvm_callback_vector to know
  * if this delivery method is available.
  */
 #elif defined(__arm__) || defined(__aarch64__)
 #define HVM_PARAM_CALLBACK_TYPE_PPI      2
-/*
+/**
+ * DOC: HVM_PARAM_CALLBACK_TYPE_PPI
+ *
  * val[55:16] needs to be zero.
  * val[15:8] is interrupt flag of the PPI used by event-channel:
  *  bit 8: the PPI is edge(1) or level(0) triggered
@@ -86,7 +97,9 @@
 #define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL 2
 #endif
 
-/*
+/**
+ * DOC: HVM_PARAM_STORE_*, HVM_PARAM_IOREQ_PFN, HVM_PARAM_BUFIOREQ_PFN
+ *
  * These are not used by Xen. They are here for convenience of HVM-guest
  * xenbus implementations.
  */
@@ -99,7 +112,9 @@
 
 #if defined(__i386__) || defined(__x86_64__)
 
-/*
+/**
+ * DOC: HVM_PARAM_VIRIDIAN
+ *
  * Viridian enlightenments
  *
  * (See http://download.microsoft.com/download/A/B/4/AB43A34E-BDD0-4FA6-BDEF-79EEF16E880B/Hypervisor%20Top%20Level%20Functional%20Specification%20v4.0.docx)
@@ -110,7 +125,10 @@
  */
 #define HVM_PARAM_VIRIDIAN     9
 
-/* Base+Freq viridian feature sets:
+/**
+ * DOC: HVMPV_base_freq
+ *
+ * Base+Freq viridian feature sets:
  *
  * - Hypercall MSRs (HV_X64_MSR_GUEST_OS_ID and HV_X64_MSR_HYPERCALL)
  * - APIC access MSRs (HV_X64_MSR_EOI, HV_X64_MSR_ICR and HV_X64_MSR_TPR)
@@ -123,7 +141,10 @@
 
 /* Feature set modifications */
 
-/* Disable timer frequency MSRs (HV_X64_MSR_TSC_FREQUENCY and
+/**
+ * DOC: HVMPV_no_freq
+ *
+ * Disable timer frequency MSRs (HV_X64_MSR_TSC_FREQUENCY and
  * HV_X64_MSR_APIC_FREQUENCY).
  * This modification restores the viridian feature set to the
  * original 'base' set exposed in releases prior to Xen 4.4.
@@ -131,35 +152,59 @@
 #define _HVMPV_no_freq 1
 #define HVMPV_no_freq  (1 << _HVMPV_no_freq)
 
-/* Enable Partition Time Reference Counter (HV_X64_MSR_TIME_REF_COUNT) */
+/**
+ * DOC: HVMPV_time_ref_count
+ * Enable Partition Time Reference Counter (HV_X64_MSR_TIME_REF_COUNT)
+ */
 #define _HVMPV_time_ref_count 2
 #define HVMPV_time_ref_count  (1 << _HVMPV_time_ref_count)
 
-/* Enable Reference TSC Page (HV_X64_MSR_REFERENCE_TSC) */
+/**
+ * DOC: HVMPV_reference_tsc
+ * Enable Reference TSC Page (HV_X64_MSR_REFERENCE_TSC)
+ */
 #define _HVMPV_reference_tsc 3
 #define HVMPV_reference_tsc  (1 << _HVMPV_reference_tsc)
 
-/* Use Hypercall for remote TLB flush */
+/**
+ * DOC: HVMPV_hcall_remote_tlb_flush
+ * Use Hypercall for remote TLB flush
+ */
 #define _HVMPV_hcall_remote_tlb_flush 4
 #define HVMPV_hcall_remote_tlb_flush (1 << _HVMPV_hcall_remote_tlb_flush)
 
-/* Use APIC assist */
+/**
+ * DOC: HVMPV_apic_assist
+ * Use APIC assist
+ */
 #define _HVMPV_apic_assist 5
 #define HVMPV_apic_assist (1 << _HVMPV_apic_assist)
 
-/* Enable crash MSRs */
+/**
+ * DOC: HVMPV_crash_ctl
+ * Enable crash MSRs
+ */
 #define _HVMPV_crash_ctl 6
 #define HVMPV_crash_ctl (1 << _HVMPV_crash_ctl)
 
-/* Enable SYNIC MSRs */
+/**
+ * DOC: HVMPV_synic
+ * Enable SYNIC MSRs
+ */
 #define _HVMPV_synic 7
 #define HVMPV_synic (1 << _HVMPV_synic)
 
-/* Enable STIMER MSRs */
+/**
+ * DOC: HVMPV_stimer
+ * Enable STIMER MSRs
+ */
 #define _HVMPV_stimer 8
 #define HVMPV_stimer (1 << _HVMPV_stimer)
 
-/* Use Synthetic Cluster IPI Hypercall */
+/**
+ * DOC: HVMPV_hcall_ipi
+ * Use Synthetic Cluster IPI Hypercall
+ */
 #define _HVMPV_hcall_ipi 9
 #define HVMPV_hcall_ipi (1 << _HVMPV_hcall_ipi)
 
@@ -177,7 +222,9 @@
 
 #endif
 
-/*
+/**
+ * DOC: HVM_PARAM_TIMER_MODE
+ *
  * Set mode for virtual timers (currently x86 only):
  *  delay_for_missed_ticks (default):
  *   Do not advance a vcpu's time beyond the correct delivery time for
@@ -202,26 +249,47 @@
 #define HVMPTM_no_missed_ticks_pending   2
 #define HVMPTM_one_missed_tick_pending   3
 
-/* Boolean: Enable virtual HPET (high-precision event timer)? (x86-only) */
+/**
+ * DOC: HVM_PARAM_HPET_ENABLED
+ * Boolean: Enable virtual HPET (high-precision event timer)? (x86-only)
+ */
 #define HVM_PARAM_HPET_ENABLED 11
 
-/* Identity-map page directory used by Intel EPT when CR0.PG=0. */
+/**
+ * DOC: HVM_PARAM_IDENT_PT
+ * Identity-map page directory used by Intel EPT when CR0.PG=0.
+ */
 #define HVM_PARAM_IDENT_PT     12
 
-/* ACPI S state: currently support S0 and S3 on x86. */
+/**
+ * DOC: HVM_PARAM_ACPI_S_STATE
+ * ACPI S state: currently support S0 and S3 on x86.
+ */
 #define HVM_PARAM_ACPI_S_STATE 14
 
-/* TSS used on Intel when CR0.PE=0. */
+/**
+ * DOC: HVM_PARAM_VM86_TSS
+ * TSS used on Intel when CR0.PE=0.
+ */
 #define HVM_PARAM_VM86_TSS     15
 
-/* Boolean: Enable aligning all periodic vpts to reduce interrupts */
+/**
+ * DOC: HVM_PARAM_VPT_ALIGN
+ * Boolean: Enable aligning all periodic vpts to reduce interrupts
+ */
 #define HVM_PARAM_VPT_ALIGN    16
 
-/* Console debug shared memory ring and event channel */
+/**
+ * DOC: HVM_PARAM_CONSOLE_PFN and HVM_PARAM_CONSOLE_EVTCHN
+ *
+ * Console debug shared memory ring and event channel
+ */
 #define HVM_PARAM_CONSOLE_PFN    17
 #define HVM_PARAM_CONSOLE_EVTCHN 18
 
-/*
+/**
+ * DOC: HVM_PARAM_ACPI_IOPORTS_LOCATION
+ *
  * Select location of ACPI PM1a and TMR control blocks. Currently two locations
  * are supported, specified by version 0 or 1 in this parameter:
  *   - 0: default, use the old addresses
@@ -232,24 +300,39 @@
  */
 #define HVM_PARAM_ACPI_IOPORTS_LOCATION 19
 
-/* Boolean: Enable nestedhvm (hvm only) */
+/**
+ * DOC: HVM_PARAM_NESTEDHVM
+ * Boolean: Enable nestedhvm (hvm only)
+ */
 #define HVM_PARAM_NESTEDHVM    24
 
-/* Params for the mem event rings */
+/**
+ * DOC: HVM_PARAM_*_RING_PFN
+ *
+ * Params for the mem event rings
+ */
 #define HVM_PARAM_PAGING_RING_PFN   27
 #define HVM_PARAM_MONITOR_RING_PFN  28
 #define HVM_PARAM_SHARING_RING_PFN  29
 
-/* SHUTDOWN_* action in case of a triple fault */
+/**
+ * DOC: HVM_PARAM_TRIPLE_FAULT_REASON
+ * SHUTDOWN_* action in case of a triple fault
+ */
 #define HVM_PARAM_TRIPLE_FAULT_REASON 31
 
 #define HVM_PARAM_IOREQ_SERVER_PFN 32
 #define HVM_PARAM_NR_IOREQ_SERVER_PAGES 33
 
-/* Location of the VM Generation ID in guest physical address space. */
+/**
+ * DOC: HVM_PARAM_VM_GENERATION_ID_ADDR
+ * Location of the VM Generation ID in guest physical address space.
+ */
 #define HVM_PARAM_VM_GENERATION_ID_ADDR 34
 
-/*
+/**
+ * DOC: HVM_PARAM_ALTP2M
+ *
  * Set mode for altp2m:
  *  disabled: don't activate altp2m (default)
  *  mixed: allow access to all altp2m ops for both in-guest and external tools
@@ -267,7 +350,9 @@
 #define XEN_ALTP2M_external      2
 #define XEN_ALTP2M_limited       3
 
-/*
+/**
+ * DOC: HVM_PARAM_X87_FIP_WIDTH
+ *
  * Size of the x87 FPU FIP/FDP registers that the hypervisor needs to
  * save/restore.  This is a workaround for a hardware limitation that
  * does not allow the full FIP/FDP and FCS/FDS to be restored.
@@ -289,13 +374,18 @@
  */
 #define HVM_PARAM_X87_FIP_WIDTH 36
 
-/*
+/**
+ * DOC: HVM_PARAM_VM86_TSS_SIZED
+ *
  * TSS (and its size) used on Intel when CR0.PE=0. The address occupies
  * the low 32 bits, while the size is in the high 32 ones.
  */
 #define HVM_PARAM_VM86_TSS_SIZED 37
 
-/* Enable MCA capabilities. */
+/**
+ * DOC: HVM_PARAM_MCA_CAP
+ * Enable MCA capabilities.
+ */
 #define HVM_PARAM_MCA_CAP 38
 #define XEN_HVM_MCA_CAP_LMCE   (xen_mk_ullong(1) << 0)
 #define XEN_HVM_MCA_CAP_MASK   XEN_HVM_MCA_CAP_LMCE
-- 
2.17.1

Re: [PATCH 00/14] kernel-doc: public/arch-arm.h

[Jan Beulich]

On 07.08.2020 01:49, Stefano Stabellini wrote:
> # THE KERNEL-DOC KEYWORDS USED
> 
> I used the "struct" keyword for structures, i.e.:
> 
> /**
>  * struct foobar
>  */
> 
> "struct" makes kernel-doc go and look at the following struct in the
> code, parses struct members comments, and generate a doc optimized to
> describe a struct. Note that in these cases the struct needs to follow
> immediately the comment. Thus, I had to move an #define between the
> comment and the struct in a few places.
> 
> Also note that kernel-doc supports nested structs but due to a quirk,
> comments for nested struct members cannot be on a single line. They have
> to be:
> 
>   struct foo {
>       struct {
>           /**
>            * @u.bar: foobar
>            */
>           bar;
>       } u;
>   }

Urgh.

> Otherwise for normal struct the single line comment works fine:
> 
>   struct foo {
>       /** @bar: foobar */
>       bar;
>   }
> 
> 
> I used the "DOC" keyword otherwise. "DOC" is freeform, not particularly
> tied to anything following (functions, enums, etc.) I kept a black line
> between "DOC" and the following comment if multiline and no blank line
> if it is single line.
> 
>   /**
>    * DOC: doc1
>    * single line comment
>    */
> 
>    /**
>     * DOC: doc2
>     *
>     * this is
>     * multiline
>     */

I think before the first piece of this goes in (or together with it
going in), ./CODING_STYLE wants to be updated to reflect this. It is
particularly relevant imo to mention the quirk above and hence the
necessary exception (even better of course would be to get the quirk
out of kernel-doc).

Jan

Re: [PATCH 00/14] kernel-doc: public/arch-arm.h

[Stefano Stabellini]

On Fri, 7 Aug 2020, Jan Beulich wrote:
> On 07.08.2020 01:49, Stefano Stabellini wrote:
> > # THE KERNEL-DOC KEYWORDS USED
> > 
> > I used the "struct" keyword for structures, i.e.:
> > 
> > /**
> >  * struct foobar
> >  */
> > 
> > "struct" makes kernel-doc go and look at the following struct in the
> > code, parses struct members comments, and generate a doc optimized to
> > describe a struct. Note that in these cases the struct needs to follow
> > immediately the comment. Thus, I had to move an #define between the
> > comment and the struct in a few places.
> > 
> > Also note that kernel-doc supports nested structs but due to a quirk,
> > comments for nested struct members cannot be on a single line. They have
> > to be:
> > 
> >   struct foo {
> >       struct {
> >           /**
> >            * @u.bar: foobar
> >            */
> >           bar;
> >       } u;
> >   }
> 
> Urgh.
> 
> > Otherwise for normal struct the single line comment works fine:
> > 
> >   struct foo {
> >       /** @bar: foobar */
> >       bar;
> >   }
> > 
> > 
> > I used the "DOC" keyword otherwise. "DOC" is freeform, not particularly
> > tied to anything following (functions, enums, etc.) I kept a black line
> > between "DOC" and the following comment if multiline and no blank line
> > if it is single line.
> > 
> >   /**
> >    * DOC: doc1
> >    * single line comment
> >    */
> > 
> >    /**
> >     * DOC: doc2
> >     *
> >     * this is
> >     * multiline
> >     */
> 
> I think before the first piece of this goes in (or together with it
> going in), ./CODING_STYLE wants to be updated to reflect this. It is
> particularly relevant imo to mention the quirk above and hence the
> necessary exception (even better of course would be to get the quirk
> out of kernel-doc).
 
Yes, I can add a patch to change ./CODING_STYLE. I think we should
encourage people to use the kernel-doc syntax at least for public
headers. I can document the use of the "struct" and "DOC" keywords, and
also the quirk with nested structs.

Note that instead the black line after DOC is completely optional from
a kernel-doc perspective, I did it for style, but I can add that to
./CODING_STYLE too.

Re: [PATCH 00/14] kernel-doc: public/arch-arm.h

[Stefano Stabellini]

I am replying to this email as I have been told that the original was
filtered as spam due to the tarball attachment. The tarball contains
some example html output document files from sphinx.

I have uploaded the tarball here for your convenience:

http://xenbits.xenproject.org/people/sstabellini/html.tar.gz

You can read the original email below.


On Thu, 6 Aug 2020, Stefano Stabellini wrote:
> Hi all,
> 
> This patch series convert Xen in-code comments to the kernel-doc format:
> 
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
> 
> 
> # WHAT WAS CONVERTED
> 
> I started from the public/ header files as I thought they are the most
> important to generated documentation for.
> 
> I didn't cover all files under xen/include/public/, but we don't have to
> boil the ocean in one go.
> 
> For the header files I addressed, I did cover all in-code comments
> except for very few exceptions where the conversion to kernel-doc format
> wasn't easily doable without major changes to the comments/code.
> 
> The conversion was done by hand (sigh!) but was mechanical, and only
> stylistic: I didn't change the content of the comments (only in a couple
> of places to make the English work right, e.g. when a comment has been
> split into two comments.)
> 
> 
> # THE KERNEL-DOC KEYWORDS USED
> 
> I used the "struct" keyword for structures, i.e.:
> 
> /**
>  * struct foobar
>  */
> 
> "struct" makes kernel-doc go and look at the following struct in the
> code, parses struct members comments, and generate a doc optimized to
> describe a struct. Note that in these cases the struct needs to follow
> immediately the comment. Thus, I had to move an #define between the
> comment and the struct in a few places.
> 
> Also note that kernel-doc supports nested structs but due to a quirk,
> comments for nested struct members cannot be on a single line. They have
> to be:
> 
>   struct foo {
>       struct {
>           /**
>            * @u.bar: foobar
>            */
>           bar;
>       } u;
>   }
> 
> Otherwise for normal struct the single line comment works fine:
> 
>   struct foo {
>       /** @bar: foobar */
>       bar;
>   }
> 
> 
> I used the "DOC" keyword otherwise. "DOC" is freeform, not particularly
> tied to anything following (functions, enums, etc.) I kept a black line
> between "DOC" and the following comment if multiline and no blank line
> if it is single line.
> 
>   /**
>    * DOC: doc1
>    * single line comment
>    */
> 
>    /**
>     * DOC: doc2
>     *
>     * this is
>     * multiline
>     */
> 
> DOC doesn't generate any cross-documents links but it is still a great
> place to start as it makes the in-code comments immediately available as
> documents. Linking and references can be added later.
> 
> 
> # HOW TO TEST IT
> 
> Simply run kernel-doc on a header file, for instance:
> 
>   ../linux/scripts/kernel-doc xen/include/public/event_channel.h > /tmp/doc.rst
> 
> You can inspect the rst file and also generate a html file out of it with
> sphinx:
> 
>   sphinx-quickstart
>   sphinx-build . /path/to/out
> 
> I am attaching two example output html files together with the static CSS
> and images to render them correctly. Note that of course I haven't
> worked on the CSS at all, clearly the style can be vastly improved, but
> I wanted to give you an idea of how readable they actually are even like
> this.
> 
> 
> Cheers,
> 
> Stefano
> 
> 
> The following changes since commit 81fd0d3ca4b2cd309403c6e8da662c325dd35750:
> 
>   x86/hvm: simplify 'mmio_direct' check in epte_get_entry_emt() (2020-07-31 17:43:31 +0200)
> 
> are available in the Git repository at:
> 
>   http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git hyp-docs-1 
> 
> for you to fetch changes up to abbd21dfa0ff14a7eb5faa57aaf3db24f83a149f:
> 
>   kernel-doc: public/hvm/params.h (2020-08-06 16:27:22 -0700)
> 
> ----------------------------------------------------------------
> Stefano Stabellini (14):
>       kernel-doc: public/arch-arm.h
>       kernel-doc: public/hvm/hvm_op.h
>       kernel-doc: public/device_tree_defs.h
>       kernel-doc: public/event_channel.h
>       kernel-doc: public/features.h
>       kernel-doc: public/grant_table.h
>       kernel-doc: public/hypfs.h
>       kernel-doc: public/memory.h
>       kernel-doc: public/sched.h
>       kernel-doc: public/vcpu.h
>       kernel-doc: public/version.h
>       kernel-doc: public/xen.h
>       kernel-doc: public/elfnote.h
>       kernel-doc: public/hvm/params.h
> 
>  xen/include/public/arch-arm.h         |  43 ++-
>  xen/include/public/device_tree_defs.h |  24 +-
>  xen/include/public/elfnote.h          | 109 +++++--
>  xen/include/public/event_channel.h    | 188 +++++++----
>  xen/include/public/features.h         |  78 +++--
>  xen/include/public/grant_table.h      | 443 +++++++++++++++-----------
>  xen/include/public/hvm/hvm_op.h       |  20 +-
>  xen/include/public/hvm/params.h       | 158 ++++++++--
>  xen/include/public/hypfs.h            |  72 +++--
>  xen/include/public/memory.h           | 232 +++++++++-----
>  xen/include/public/sched.h            | 129 +++++---
>  xen/include/public/vcpu.h             | 180 ++++++++---
>  xen/include/public/version.h          |  74 ++++-
>  xen/include/public/xen.h              | 567 ++++++++++++++++++++++------------
>  14 files changed, 1564 insertions(+), 753 deletions(-)

Re: [PATCH 00/14] kernel-doc: public/arch-arm.h

[Ian Jackson]

Stefano Stabellini writes ("Re: [PATCH 00/14] kernel-doc: public/arch-arm.h"):
> I am replying to this email as I have been told that the original was
> filtered as spam due to the tarball attachment. The tarball contains
> some example html output document files from sphinx.

Thanks.

Thanks for all your work.  This is definiteely going in the right
direection.  I skimread all the patches and have nothing further to
add to what others have said.

How soon can we arrange for this processing to be done automatically
(on xenbits, I guess) ?  Would you be prepared to set this up if I add
your ssh key to the "xendocs" account which builds the existing docs ?

Ian.

Re: [PATCH 00/14] kernel-doc: public/arch-arm.h

[Stefano Stabellini]

On Tue, 18 Aug 2020, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [PATCH 00/14] kernel-doc: public/arch-arm.h"):
> > I am replying to this email as I have been told that the original was
> > filtered as spam due to the tarball attachment. The tarball contains
> > some example html output document files from sphinx.
> 
> Thanks.
> 
> Thanks for all your work.  This is definiteely going in the right
> direection.  I skimread all the patches and have nothing further to
> add to what others have said.

Thanks for looking into it!


> How soon can we arrange for this processing to be done automatically
> (on xenbits, I guess) ?  Would you be prepared to set this up if I add
> your ssh key to the "xendocs" account which builds the existing docs ?

Yes, I can do that.

This series was only meant to provide the basic groundwork, I wasn't
thinking of adding the kernel-doc script to xen.git or the automatic
docs build as part of it. However, I do have work in the pipeline to do
that too: right now I am experimenting with some kernel-doc changes to
produce better output docs for Xen. I am planning on sending that out
soon after this series gets in, so maybe in few weeks or a month.

Since I am here, I'd like to give you a heads up that I'll need your
help reviewing or maybe making some changes to kernel-doc because my
perl is nonexistent so I am probably doing something awful :-)