[PATCH v6 9/9] docs/doxygen: doxygen documentation for grant_table.h

Luca Fancellu posted 9 patches 3 years, 6 months ago
There is a newer version of this series
[PATCH v6 9/9] docs/doxygen: doxygen documentation for grant_table.h
Posted by Luca Fancellu 3 years, 6 months ago
Modification to include/public/grant_table.h:

1) Add doxygen tags to:
 - Create Grant tables section
 - include variables in the generated documentation
 - Used @keepindent/@endkeepindent to enclose comment
   section that are indented using spaces, to keep
   the indentation.
2) Add .rst file for grant table for Arm64

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
v6 changes:
- Fix misaligned comment
- Moved comments to make them display in the docs
- Included more documentation in the docs
  (see output here: https://luca.fancellu.gitlab.io/xen-docs/hypercall-interfaces/common/grant_tables.html)
v5 changes:
- Move GNTCOPY_* define next to the flags field
v4 changes:
- Used @keepindent/@endkeepindent doxygen commands
  to keep text with spaces indentation.
- drop changes to grant_entry_v1 comment, it will
  be changed and included in the docs in a future patch
- Move docs .rst to "common" folder
v3 changes:
- removed tags to skip anonymous union/struct
- moved back comment pointed out by Jan
- moved down defines related to struct gnttab_copy
  as pointed out by Jan
v2 changes:
- Revert back to anonymous union/struct
- add doxygen tags to skip anonymous union/struct
---
 docs/hypercall-interfaces/arm64.rst           |   1 +
 .../common/grant_tables.rst                   |   9 +
 docs/xen-doxygen/doxy_input.list              |   1 +
 xen/include/public/grant_table.h              | 387 +++++++++++-------
 4 files changed, 245 insertions(+), 153 deletions(-)
 create mode 100644 docs/hypercall-interfaces/common/grant_tables.rst

diff --git a/docs/hypercall-interfaces/arm64.rst b/docs/hypercall-interfaces/arm64.rst
index 5e701a2adc..cb4c0d13de 100644
--- a/docs/hypercall-interfaces/arm64.rst
+++ b/docs/hypercall-interfaces/arm64.rst
@@ -8,6 +8,7 @@ Starting points
 .. toctree::
    :maxdepth: 2
 
+   common/grant_tables
 
 
 Functions
diff --git a/docs/hypercall-interfaces/common/grant_tables.rst b/docs/hypercall-interfaces/common/grant_tables.rst
new file mode 100644
index 0000000000..b8a1ef8759
--- /dev/null
+++ b/docs/hypercall-interfaces/common/grant_tables.rst
@@ -0,0 +1,9 @@
+.. SPDX-License-Identifier: CC-BY-4.0
+
+Grant Tables
+============
+
+.. doxygengroup:: grant_table
+   :project: Xen
+   :members:
+   :undoc-members:
diff --git a/docs/xen-doxygen/doxy_input.list b/docs/xen-doxygen/doxy_input.list
index e69de29bb2..233d692fa7 100644
--- a/docs/xen-doxygen/doxy_input.list
+++ b/docs/xen-doxygen/doxy_input.list
@@ -0,0 +1 @@
+xen/include/public/grant_table.h
diff --git a/xen/include/public/grant_table.h b/xen/include/public/grant_table.h
index 84b1d26b36..dfa5155927 100644
--- a/xen/include/public/grant_table.h
+++ b/xen/include/public/grant_table.h
@@ -25,15 +25,19 @@
  * Copyright (c) 2004, K A Fraser
  */
 
+/**
+ * @file
+ * @brief Interface for granting foreign access to page frames, and receiving
+ * page-ownership transfers.
+ */
+
 #ifndef __XEN_PUBLIC_GRANT_TABLE_H__
 #define __XEN_PUBLIC_GRANT_TABLE_H__
 
 #include "xen.h"
 
-/*
- * `incontents 150 gnttab Grant Tables
- *
- * Xen's grant tables provide a generic mechanism to memory sharing
+/**
+ * @brief Xen's grant tables provide a generic mechanism to memory sharing
  * between domains. This shared memory interface underpins the split
  * device drivers for block and network IO.
  *
@@ -51,13 +55,13 @@
  * know the real machine address of a page it is sharing. This makes
  * it possible to share memory correctly with domains running in
  * fully virtualised memory.
- */
-
-/***********************************
+ *
  * GRANT TABLE REPRESENTATION
- */
-
-/* Some rough guidelines on accessing and updating grant-table entries
+ *
+ * A grant table comprises a packed array of grant entries in one or more
+ * page frames shared between Xen and a guest.
+ *
+ * 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
@@ -66,6 +70,7 @@
  *     compiler barrier will still be required.
  *
  * Introducing a valid entry into the grant table:
+ * @keepindent
  *  1. Write ent->domid.
  *  2. Write ent->frame:
  *      GTF_permit_access:   Frame to which access is permitted.
@@ -73,20 +78,25 @@
  *                           frame, or zero if none.
  *  3. Write memory barrier (WMB).
  *  4. Write ent->flags, inc. valid type.
+ * @endkeepindent
  *
  * Invalidating an unused GTF_permit_access entry:
+ * @keepindent
  *  1. flags = ent->flags.
  *  2. Observe that !(flags & (GTF_reading|GTF_writing)).
  *  3. Check result of SMP-safe CMPXCHG(&ent->flags, flags, 0).
  *  NB. No need for WMB as reuse of entry is control-dependent on success of
  *      step 3, and all architectures guarantee ordering of ctrl-dep writes.
+ * @endkeepindent
  *
  * Invalidating an in-use GTF_permit_access entry:
+ *
  *  This cannot be done directly. Request assistance from the domain controller
  *  which can set a timeout on the use of a grant entry and take necessary
  *  action. (NB. This is not yet implemented!).
  *
  * Invalidating an unused GTF_accept_transfer entry:
+ * @keepindent
  *  1. flags = ent->flags.
  *  2. Observe that !(flags & GTF_transfer_committed). [*]
  *  3. Check result of SMP-safe CMPXCHG(&ent->flags, flags, 0).
@@ -97,29 +107,32 @@
  *      transferred frame is written. It is safe for the guest to spin waiting
  *      for this to occur (detect by observing GTF_transfer_completed in
  *      ent->flags).
+ * @endkeepindent
  *
  * Invalidating a committed GTF_accept_transfer entry:
  *  1. Wait for (ent->flags & GTF_transfer_completed).
  *
  * Changing a GTF_permit_access from writable to read-only:
+ *
  *  Use SMP-safe CMPXCHG to set GTF_readonly, while checking !GTF_writing.
  *
  * Changing a GTF_permit_access from read-only to writable:
+ *
  *  Use SMP-safe bit-setting instruction.
+ *
+ * Data structure fields or defines described below have the following tags:
+ * * [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.
+ *
+ * @addtogroup grant_table Grant Tables
+ * @{
  */
 
-/*
+/**
  * Reference to a grant entry in a specified domain's grant table.
  */
 typedef uint32_t grant_ref_t;
 
-/*
- * 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.
@@ -129,15 +142,17 @@ typedef uint32_t grant_ref_t;
 #define grant_entry_v1_t grant_entry_t
 #endif
 struct grant_entry_v1 {
-    /* GTF_xxx: various type and flag information.  [XEN,GST] */
+    /** GTF_xxx: various type and flag information.  [XEN,GST] */
     uint16_t flags;
-    /* The domain being granted foreign privileges. [GST] */
+    /** The domain being granted foreign privileges. [GST] */
     domid_t  domid;
-    /*
+    /**
+     * @keepindent
      * 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]
+     * @endkeepindent
      */
     uint32_t frame;
 };
@@ -150,60 +165,99 @@ typedef struct grant_entry_v1 grant_entry_v1_t;
 #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.
- */
+/** This type of grant entry grants no privileges. */
 #define GTF_invalid         (0U<<0)
+
+/** This type of grant entry allow \@domid to map/access \@frame. */
 #define GTF_permit_access   (1U<<0)
+
+/**
+ * This type of grant entry allow \@domid to transfer ownership of one pageframe
+ * to this guest. Xen writes the page number to \@frame.
+ */
 #define GTF_accept_transfer (2U<<0)
+
+/**
+ * This type of grant entry allow \@domid to transitively access a subrange of
+ * \@trans_grant in \@trans_domid.  No mappings are allowed.
+ */
 #define GTF_transitive      (3U<<0)
+
 #define GTF_type_mask       (3U<<0)
 
-/*
- * Subflags for GTF_permit_access and GTF_transitive.
- *  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]
- * Further subflags for GTF_permit_access only.
- *  GTF_PAT, GTF_PWT, GTF_PCD: (x86) cache attribute flags to be used for
- *                             mappings of 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]
+/**
+ * @def GTF_readonly
+ * Subflag for GTF_permit_access and GTF_transitive: Restrict \@domid to
+ * read-only mappings and accesses. [GST]
  */
 #define _GTF_readonly       (2)
 #define GTF_readonly        (1U<<_GTF_readonly)
+
+/**
+ * @def GTF_reading
+ * Subflag for GTF_permit_access and GTF_transitive: Grant entry is currently
+ * mapped for reading by \@domid. [XEN]
+ */
 #define _GTF_reading        (3)
 #define GTF_reading         (1U<<_GTF_reading)
+
+/**
+ * @def GTF_writing
+ * Subflag for GTF_permit_access and GTF_transitive: Grant entry is currently
+ * mapped for writing by \@domid. [XEN]
+ */
 #define _GTF_writing        (4)
 #define GTF_writing         (1U<<_GTF_writing)
+
+/**
+ * @def GTF_PWT
+ * Subflag for GTF_permit_access only: (x86) cache attribute flags to be used
+ * for mappings of the grant [GST]
+ */
 #define _GTF_PWT            (5)
 #define GTF_PWT             (1U<<_GTF_PWT)
+
+/**
+ * @def GTF_PCD
+ * Subflag for GTF_permit_access only: (x86) cache attribute flags to be used
+ * for mappings of the grant [GST]
+ */
 #define _GTF_PCD            (6)
 #define GTF_PCD             (1U<<_GTF_PCD)
+
+/**
+ * @def GTF_PAT
+ * Subflag for GTF_permit_access only: (x86) cache attribute flags to be used
+ * for mappings of the grant [GST]
+ */
 #define _GTF_PAT            (7)
 #define GTF_PAT             (1U<<_GTF_PAT)
+
+/**
+ * @def GTF_sub_page
+ * Subflag for GTF_permit_access only: 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_sub_page       (8)
 #define GTF_sub_page        (1U<<_GTF_sub_page)
 
-/*
- * 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
- *      set by Xen.
- *  GTF_transfer_completed: It is safe for the guest to spin-wait on this flag
- *      after reading GTF_transfer_committed. Xen will always write the frame
- *      address, followed by ORing this flag, in a timely manner.
+/**
+ * @def GTF_transfer_committed
+ * Subflag for GTF_accept_transfer: 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
+ * set by Xen.
  */
 #define _GTF_transfer_committed (2)
 #define GTF_transfer_committed  (1U<<_GTF_transfer_committed)
+
+/**
+ * @def GTF_transfer_completed
+ * Subflag for GTF_accept_transfer: It is safe for the guest to spin-wait on
+ * this flag after reading GTF_transfer_committed. Xen will always write the
+ * frame address, followed by ORing this flag, in a timely manner.
+ */
 #define _GTF_transfer_completed (3)
 #define GTF_transfer_completed  (1U<<_GTF_transfer_completed)
 
@@ -228,17 +282,17 @@ struct grant_entry_header {
 };
 typedef struct grant_entry_header grant_entry_header_t;
 
-/*
+/**
  * Version 2 of the grant entry structure.
  */
 union grant_entry_v2 {
     grant_entry_header_t hdr;
 
-    /*
+    /**
      * 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.
@@ -249,10 +303,10 @@ union grant_entry_v2 {
         uint64_t frame;
     } full_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.
+     * \@domid is allowed to access bytes [\@page_off,\@page_off+\@length)
+     * in frame \@frame.
      */
     struct {
         grant_entry_header_t hdr;
@@ -261,9 +315,9 @@ union grant_entry_v2 {
         uint64_t frame;
     } sub_page;
 
-    /*
-     * If the grant is GTF_transitive, @domid is allowed to use the
-     * grant @gref in domain @trans_domid, as if it was the local
+    /**
+     * 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
      * with the original grant.
      *
@@ -277,7 +331,7 @@ union grant_entry_v2 {
         grant_ref_t gref;
     } transitive;
 
-    uint32_t __spacer[4]; /* Pad to a power of two */
+    uint32_t __spacer[4]; /**< Pad to a power of two */
 };
 typedef union grant_entry_v2 grant_entry_v2_t;
 
@@ -317,24 +371,25 @@ typedef uint16_t grant_status_t;
 #endif /* __XEN_INTERFACE_VERSION__ */
 /* ` } */
 
-/*
+/**
  * 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>
+/**
+ * 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
+ *  1. If GNTMAP_device_map is specified then \@dev_bus_addr is the address
  *     via which I/O devices may access the granted frame.
  *  2. If GNTMAP_host_map is specified then a mapping will be added at
  *     either a host virtual address in the current address space, or at
  *     a PTE at the specified machine address.  The type of mapping to
  *     perform is selected through the GNTMAP_contains_pte flag, and the
- *     address is specified in <host_addr>.
+ *     address is specified in \@host_addr.
  *  3. Mappings should only be destroyed via GNTTABOP_unmap_grant_ref. If a
  *     host mapping is destroyed by other means then it is *NOT* guaranteed
  *     to be accounted to the correct grant reference!
@@ -342,25 +397,26 @@ typedef uint32_t grant_handle_t;
 struct gnttab_map_grant_ref {
     /* IN parameters. */
     uint64_t host_addr;
-    uint32_t flags;               /* GNTMAP_* */
+    uint32_t flags;               /**< GNTMAP_* */
     grant_ref_t ref;
     domid_t  dom;
     /* OUT parameters. */
-    int16_t  status;              /* => enum grant_status */
+    int16_t  status;              /**< GNTST_* status code */
     grant_handle_t handle;
     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
+ * 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>
+ * that is tracked by \@handle
+ *
  * NOTES:
  *  1. The call may fail in an undefined manner if either mapping is not
- *     tracked by <handle>.
+ *     tracked by \@handle.
  *  3. After executing a batch of unmaps, it is guaranteed that no stale
  *     mappings will remain in the device or host TLBs.
  */
@@ -370,18 +426,19 @@ struct gnttab_unmap_grant_ref {
     uint64_t dev_bus_addr;
     grant_handle_t handle;
     /* OUT parameters. */
-    int16_t  status;              /* => enum grant_status */
+    int16_t  status;              /**< GNTST_* status code */
 };
 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.
+/**
+ * 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.
+ *  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 {
@@ -389,7 +446,7 @@ struct gnttab_setup_table {
     domid_t  dom;
     uint32_t nr_frames;
     /* OUT parameters. */
-    int16_t  status;              /* => enum grant_status */
+    int16_t  status;              /**< GNTST_* status code */
 #if __XEN_INTERFACE_VERSION__ < 0x00040300
     XEN_GUEST_HANDLE(ulong) frame_list;
 #else
@@ -399,7 +456,7 @@ 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.
  */
@@ -407,14 +464,14 @@ struct gnttab_dump_table {
     /* IN parameters. */
     domid_t dom;
     /* OUT parameters. */
-    int16_t status;               /* => enum grant_status */
+    int16_t status;               /**< GNTST_* status code */
 };
 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
- * has previously registered its interest in the transfer via <domid, ref>.
+/**
+ * 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
  * to the calling domain *unless* the error is GNTST_bad_page.
@@ -427,13 +484,13 @@ struct gnttab_transfer {
     domid_t       domid;
     grant_ref_t   ref;
     /* OUT parameters. */
-    int16_t       status;
+    int16_t       status;               /**< GNTST_* status code */
 };
 typedef struct gnttab_transfer gnttab_transfer_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_transfer_t);
 
 
-/*
+/**
  * 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
@@ -451,11 +508,6 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_transfer_t);
  * 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. */
     struct gnttab_copy_ptr {
@@ -467,19 +519,24 @@ struct gnttab_copy {
         uint16_t offset;
     } source, dest;
     uint16_t      len;
-    uint16_t      flags;          /* GNTCOPY_* */
+    uint16_t      flags;          /**< GNTCOPY_* */
+#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)
     /* 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.
+ *
  * NOTES:
- *  1. <dom> may be specified as DOMID_SELF.
- *  2. Only a sufficiently-privileged domain may specify <dom> != DOMID_SELF.
+ *  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. */
@@ -487,19 +544,20 @@ struct gnttab_query_size {
     /* OUT parameters. */
     uint32_t nr_frames;
     uint32_t max_nr_frames;
-    int16_t  status;              /* => enum grant_status */
+    int16_t  status;              /**< GNTST_* status code */
 };
 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
+ * 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>.
+ *     tracked by \@handle.
  *  2. After executing a batch of unmaps, it is guaranteed that no stale
  *     mappings will remain in the device or host TLBs.
  */
@@ -509,13 +567,13 @@ struct gnttab_unmap_and_replace {
     uint64_t new_addr;
     grant_handle_t handle;
     /* OUT parameters. */
-    int16_t  status;              /* => enum grant_status */
+    int16_t  status;              /**< GNTST_* status code */
 };
 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
@@ -529,32 +587,33 @@ 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
+ * 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.
+ * \@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.
+ *
  * NOTES:
- *  1. <dom> may be specified as DOMID_SELF.
- *  2. Only a sufficiently-privileged domain may specify <dom> != DOMID_SELF.
+ *  1. \@dom may be specified as DOMID_SELF.
+ *  2. Only a sufficiently-privileged domain may specify \@dom != DOMID_SELF.
  */
 struct gnttab_get_status_frames {
     /* IN parameters. */
     uint32_t nr_frames;
     domid_t  dom;
     /* OUT parameters. */
-    int16_t  status;              /* => enum grant_status */
+    int16_t  status;              /**< GNTST_* status code */
     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>.
+ * effect for domain \@dom.
  */
 struct gnttab_get_version {
     /* IN parameters */
@@ -566,7 +625,7 @@ struct gnttab_get_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 {
@@ -574,12 +633,12 @@ struct gnttab_swap_grant_ref {
     grant_ref_t ref_a;
     grant_ref_t ref_b;
     /* OUT parameters */
-    int16_t status;             /* => enum grant_status */
+    int16_t status;             /**< GNTST_* status code */
 };
 typedef struct gnttab_swap_grant_ref gnttab_swap_grant_ref_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t);
 
-/*
+/**
  * Issue one or more cache maintenance operations on a portion of a
  * page granted to the calling domain by a foreign domain.
  */
@@ -588,8 +647,8 @@ struct gnttab_cache_flush {
         uint64_t dev_bus_addr;
         grant_ref_t ref;
     } a;
-    uint16_t offset; /* offset from start of grant */
-    uint16_t length; /* size within the grant */
+    uint16_t offset; /**< offset from start of grant */
+    uint16_t length; /**< size within the grant */
 #define GNTTAB_CACHE_CLEAN          (1u<<0)
 #define GNTTAB_CACHE_INVAL          (1u<<1)
 #define GNTTAB_CACHE_SOURCE_GREF    (1u<<31)
@@ -600,40 +659,60 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
 
 #endif /* __XEN_INTERFACE_VERSION__ */
 
-/*
- * Bitfield values for gnttab_map_grant_ref.flags.
+/**
+ * @def GNTMAP_device_map
+ * Bitfield value for gnttab_map_grant_ref.flags: Map the grant entry for
+ * access by I/O devices.
  */
- /* 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. */
+
+/**
+ * @def GNTMAP_host_map
+ * Bitfield value for gnttab_map_grant_ref.flags: 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. */
+
+/**
+ * @def GNTMAP_readonly
+ * Bitfield value for gnttab_map_grant_ref.flags: 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.
-  */
+
+/**
+ * @def GNTMAP_application_map
+ * Bitfield value for gnttab_map_grant_ref.flags.
+ *
+ * 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.
-  */
+/**
+ * @def GNTMAP_contains_pte
+ * Bitfield value for gnttab_map_grant_ref.flags.
+ *
+ * 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).
+/**
+ * @def GNTMAP_guest_avail_mask
+ * Bitfield value for gnttab_map_grant_ref.flags: 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)
@@ -641,21 +720,19 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
 /*
  * Values for error status returns. All errors are -ve.
  */
-/* ` 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)  /**< 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 GNTTABOP_error_msgs {                   \
     "okay",                                     \
@@ -673,6 +750,10 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
     "operation not done; try again"             \
 }
 
+/**
+ * @}
+ */
+
 #endif /* __XEN_PUBLIC_GRANT_TABLE_H__ */
 
 /*
-- 
2.17.1


Re: [PATCH v6 9/9] docs/doxygen: doxygen documentation for grant_table.h
Posted by Stefano Stabellini 3 years, 5 months ago
On Mon, 10 May 2021, Luca Fancellu wrote:
> Modification to include/public/grant_table.h:
> 
> 1) Add doxygen tags to:
>  - Create Grant tables section
>  - include variables in the generated documentation
>  - Used @keepindent/@endkeepindent to enclose comment
>    section that are indented using spaces, to keep
>    the indentation.
> 2) Add .rst file for grant table for Arm64

Why only arm64?


> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> ---
> v6 changes:
> - Fix misaligned comment
> - Moved comments to make them display in the docs
> - Included more documentation in the docs
>   (see output here: https://luca.fancellu.gitlab.io/xen-docs/hypercall-interfaces/common/grant_tables.html)

It looks much much better. All the info we care about seems to be there.
The only things that I noticed missing and might be good to keep is the
small comment about HYPERVISOR_grant_table_op:

/* ` 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

All the changes look good to me.



> v5 changes:
> - Move GNTCOPY_* define next to the flags field
> v4 changes:
> - Used @keepindent/@endkeepindent doxygen commands
>   to keep text with spaces indentation.
> - drop changes to grant_entry_v1 comment, it will
>   be changed and included in the docs in a future patch
> - Move docs .rst to "common" folder
> v3 changes:
> - removed tags to skip anonymous union/struct
> - moved back comment pointed out by Jan
> - moved down defines related to struct gnttab_copy
>   as pointed out by Jan
> v2 changes:
> - Revert back to anonymous union/struct
> - add doxygen tags to skip anonymous union/struct
> ---
>  docs/hypercall-interfaces/arm64.rst           |   1 +
>  .../common/grant_tables.rst                   |   9 +
>  docs/xen-doxygen/doxy_input.list              |   1 +
>  xen/include/public/grant_table.h              | 387 +++++++++++-------
>  4 files changed, 245 insertions(+), 153 deletions(-)
>  create mode 100644 docs/hypercall-interfaces/common/grant_tables.rst
> 
> diff --git a/docs/hypercall-interfaces/arm64.rst b/docs/hypercall-interfaces/arm64.rst
> index 5e701a2adc..cb4c0d13de 100644
> --- a/docs/hypercall-interfaces/arm64.rst
> +++ b/docs/hypercall-interfaces/arm64.rst
> @@ -8,6 +8,7 @@ Starting points
>  .. toctree::
>     :maxdepth: 2
>  
> +   common/grant_tables
>  
>  
>  Functions
> diff --git a/docs/hypercall-interfaces/common/grant_tables.rst b/docs/hypercall-interfaces/common/grant_tables.rst
> new file mode 100644
> index 0000000000..b8a1ef8759
> --- /dev/null
> +++ b/docs/hypercall-interfaces/common/grant_tables.rst
> @@ -0,0 +1,9 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Grant Tables
> +============
> +
> +.. doxygengroup:: grant_table
> +   :project: Xen
> +   :members:
> +   :undoc-members:
> diff --git a/docs/xen-doxygen/doxy_input.list b/docs/xen-doxygen/doxy_input.list
> index e69de29bb2..233d692fa7 100644
> --- a/docs/xen-doxygen/doxy_input.list
> +++ b/docs/xen-doxygen/doxy_input.list
> @@ -0,0 +1 @@
> +xen/include/public/grant_table.h
> diff --git a/xen/include/public/grant_table.h b/xen/include/public/grant_table.h
> index 84b1d26b36..dfa5155927 100644
> --- a/xen/include/public/grant_table.h
> +++ b/xen/include/public/grant_table.h
> @@ -25,15 +25,19 @@
>   * Copyright (c) 2004, K A Fraser
>   */
>  
> +/**
> + * @file
> + * @brief Interface for granting foreign access to page frames, and receiving
> + * page-ownership transfers.
> + */
> +
>  #ifndef __XEN_PUBLIC_GRANT_TABLE_H__
>  #define __XEN_PUBLIC_GRANT_TABLE_H__
>  
>  #include "xen.h"
>  
> -/*
> - * `incontents 150 gnttab Grant Tables
> - *
> - * Xen's grant tables provide a generic mechanism to memory sharing
> +/**
> + * @brief Xen's grant tables provide a generic mechanism to memory sharing
>   * between domains. This shared memory interface underpins the split
>   * device drivers for block and network IO.
>   *
> @@ -51,13 +55,13 @@
>   * know the real machine address of a page it is sharing. This makes
>   * it possible to share memory correctly with domains running in
>   * fully virtualised memory.
> - */
> -
> -/***********************************
> + *
>   * GRANT TABLE REPRESENTATION
> - */
> -
> -/* Some rough guidelines on accessing and updating grant-table entries
> + *
> + * A grant table comprises a packed array of grant entries in one or more
> + * page frames shared between Xen and a guest.
> + *
> + * 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
> @@ -66,6 +70,7 @@
>   *     compiler barrier will still be required.
>   *
>   * Introducing a valid entry into the grant table:
> + * @keepindent
>   *  1. Write ent->domid.
>   *  2. Write ent->frame:
>   *      GTF_permit_access:   Frame to which access is permitted.
> @@ -73,20 +78,25 @@
>   *                           frame, or zero if none.
>   *  3. Write memory barrier (WMB).
>   *  4. Write ent->flags, inc. valid type.
> + * @endkeepindent
>   *
>   * Invalidating an unused GTF_permit_access entry:
> + * @keepindent
>   *  1. flags = ent->flags.
>   *  2. Observe that !(flags & (GTF_reading|GTF_writing)).
>   *  3. Check result of SMP-safe CMPXCHG(&ent->flags, flags, 0).
>   *  NB. No need for WMB as reuse of entry is control-dependent on success of
>   *      step 3, and all architectures guarantee ordering of ctrl-dep writes.
> + * @endkeepindent
>   *
>   * Invalidating an in-use GTF_permit_access entry:
> + *
>   *  This cannot be done directly. Request assistance from the domain controller
>   *  which can set a timeout on the use of a grant entry and take necessary
>   *  action. (NB. This is not yet implemented!).
>   *
>   * Invalidating an unused GTF_accept_transfer entry:
> + * @keepindent
>   *  1. flags = ent->flags.
>   *  2. Observe that !(flags & GTF_transfer_committed). [*]
>   *  3. Check result of SMP-safe CMPXCHG(&ent->flags, flags, 0).
> @@ -97,29 +107,32 @@
>   *      transferred frame is written. It is safe for the guest to spin waiting
>   *      for this to occur (detect by observing GTF_transfer_completed in
>   *      ent->flags).
> + * @endkeepindent
>   *
>   * Invalidating a committed GTF_accept_transfer entry:
>   *  1. Wait for (ent->flags & GTF_transfer_completed).
>   *
>   * Changing a GTF_permit_access from writable to read-only:
> + *
>   *  Use SMP-safe CMPXCHG to set GTF_readonly, while checking !GTF_writing.
>   *
>   * Changing a GTF_permit_access from read-only to writable:
> + *
>   *  Use SMP-safe bit-setting instruction.
> + *
> + * Data structure fields or defines described below have the following tags:
> + * * [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.
> + *
> + * @addtogroup grant_table Grant Tables
> + * @{
>   */
>  
> -/*
> +/**
>   * Reference to a grant entry in a specified domain's grant table.
>   */
>  typedef uint32_t grant_ref_t;
>  
> -/*
> - * 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.
> @@ -129,15 +142,17 @@ typedef uint32_t grant_ref_t;
>  #define grant_entry_v1_t grant_entry_t
>  #endif
>  struct grant_entry_v1 {
> -    /* GTF_xxx: various type and flag information.  [XEN,GST] */
> +    /** GTF_xxx: various type and flag information.  [XEN,GST] */
>      uint16_t flags;
> -    /* The domain being granted foreign privileges. [GST] */
> +    /** The domain being granted foreign privileges. [GST] */
>      domid_t  domid;
> -    /*
> +    /**
> +     * @keepindent
>       * 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]
> +     * @endkeepindent
>       */
>      uint32_t frame;
>  };
> @@ -150,60 +165,99 @@ typedef struct grant_entry_v1 grant_entry_v1_t;
>  #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.
> - */
> +/** This type of grant entry grants no privileges. */
>  #define GTF_invalid         (0U<<0)
> +
> +/** This type of grant entry allow \@domid to map/access \@frame. */
>  #define GTF_permit_access   (1U<<0)
> +
> +/**
> + * This type of grant entry allow \@domid to transfer ownership of one pageframe
> + * to this guest. Xen writes the page number to \@frame.
> + */
>  #define GTF_accept_transfer (2U<<0)
> +
> +/**
> + * This type of grant entry allow \@domid to transitively access a subrange of
> + * \@trans_grant in \@trans_domid.  No mappings are allowed.
> + */
>  #define GTF_transitive      (3U<<0)
> +
>  #define GTF_type_mask       (3U<<0)
>  
> -/*
> - * Subflags for GTF_permit_access and GTF_transitive.
> - *  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]
> - * Further subflags for GTF_permit_access only.
> - *  GTF_PAT, GTF_PWT, GTF_PCD: (x86) cache attribute flags to be used for
> - *                             mappings of 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]
> +/**
> + * @def GTF_readonly
> + * Subflag for GTF_permit_access and GTF_transitive: Restrict \@domid to
> + * read-only mappings and accesses. [GST]
>   */
>  #define _GTF_readonly       (2)
>  #define GTF_readonly        (1U<<_GTF_readonly)
> +
> +/**
> + * @def GTF_reading
> + * Subflag for GTF_permit_access and GTF_transitive: Grant entry is currently
> + * mapped for reading by \@domid. [XEN]
> + */
>  #define _GTF_reading        (3)
>  #define GTF_reading         (1U<<_GTF_reading)
> +
> +/**
> + * @def GTF_writing
> + * Subflag for GTF_permit_access and GTF_transitive: Grant entry is currently
> + * mapped for writing by \@domid. [XEN]
> + */
>  #define _GTF_writing        (4)
>  #define GTF_writing         (1U<<_GTF_writing)
> +
> +/**
> + * @def GTF_PWT
> + * Subflag for GTF_permit_access only: (x86) cache attribute flags to be used
> + * for mappings of the grant [GST]
> + */
>  #define _GTF_PWT            (5)
>  #define GTF_PWT             (1U<<_GTF_PWT)
> +
> +/**
> + * @def GTF_PCD
> + * Subflag for GTF_permit_access only: (x86) cache attribute flags to be used
> + * for mappings of the grant [GST]
> + */
>  #define _GTF_PCD            (6)
>  #define GTF_PCD             (1U<<_GTF_PCD)
> +
> +/**
> + * @def GTF_PAT
> + * Subflag for GTF_permit_access only: (x86) cache attribute flags to be used
> + * for mappings of the grant [GST]
> + */
>  #define _GTF_PAT            (7)
>  #define GTF_PAT             (1U<<_GTF_PAT)
> +
> +/**
> + * @def GTF_sub_page
> + * Subflag for GTF_permit_access only: 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_sub_page       (8)
>  #define GTF_sub_page        (1U<<_GTF_sub_page)
>  
> -/*
> - * 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
> - *      set by Xen.
> - *  GTF_transfer_completed: It is safe for the guest to spin-wait on this flag
> - *      after reading GTF_transfer_committed. Xen will always write the frame
> - *      address, followed by ORing this flag, in a timely manner.
> +/**
> + * @def GTF_transfer_committed
> + * Subflag for GTF_accept_transfer: 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
> + * set by Xen.
>   */
>  #define _GTF_transfer_committed (2)
>  #define GTF_transfer_committed  (1U<<_GTF_transfer_committed)
> +
> +/**
> + * @def GTF_transfer_completed
> + * Subflag for GTF_accept_transfer: It is safe for the guest to spin-wait on
> + * this flag after reading GTF_transfer_committed. Xen will always write the
> + * frame address, followed by ORing this flag, in a timely manner.
> + */
>  #define _GTF_transfer_completed (3)
>  #define GTF_transfer_completed  (1U<<_GTF_transfer_completed)
>  
> @@ -228,17 +282,17 @@ struct grant_entry_header {
>  };
>  typedef struct grant_entry_header grant_entry_header_t;
>  
> -/*
> +/**
>   * Version 2 of the grant entry structure.
>   */
>  union grant_entry_v2 {
>      grant_entry_header_t hdr;
>  
> -    /*
> +    /**
>       * 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.
> @@ -249,10 +303,10 @@ union grant_entry_v2 {
>          uint64_t frame;
>      } full_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.
> +     * \@domid is allowed to access bytes [\@page_off,\@page_off+\@length)
> +     * in frame \@frame.
>       */
>      struct {
>          grant_entry_header_t hdr;
> @@ -261,9 +315,9 @@ union grant_entry_v2 {
>          uint64_t frame;
>      } sub_page;
>  
> -    /*
> -     * If the grant is GTF_transitive, @domid is allowed to use the
> -     * grant @gref in domain @trans_domid, as if it was the local
> +    /**
> +     * 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
>       * with the original grant.
>       *
> @@ -277,7 +331,7 @@ union grant_entry_v2 {
>          grant_ref_t gref;
>      } transitive;
>  
> -    uint32_t __spacer[4]; /* Pad to a power of two */
> +    uint32_t __spacer[4]; /**< Pad to a power of two */
>  };
>  typedef union grant_entry_v2 grant_entry_v2_t;
>  
> @@ -317,24 +371,25 @@ typedef uint16_t grant_status_t;
>  #endif /* __XEN_INTERFACE_VERSION__ */
>  /* ` } */
>  
> -/*
> +/**
>   * 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>
> +/**
> + * 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
> + *  1. If GNTMAP_device_map is specified then \@dev_bus_addr is the address
>   *     via which I/O devices may access the granted frame.
>   *  2. If GNTMAP_host_map is specified then a mapping will be added at
>   *     either a host virtual address in the current address space, or at
>   *     a PTE at the specified machine address.  The type of mapping to
>   *     perform is selected through the GNTMAP_contains_pte flag, and the
> - *     address is specified in <host_addr>.
> + *     address is specified in \@host_addr.
>   *  3. Mappings should only be destroyed via GNTTABOP_unmap_grant_ref. If a
>   *     host mapping is destroyed by other means then it is *NOT* guaranteed
>   *     to be accounted to the correct grant reference!
> @@ -342,25 +397,26 @@ typedef uint32_t grant_handle_t;
>  struct gnttab_map_grant_ref {
>      /* IN parameters. */
>      uint64_t host_addr;
> -    uint32_t flags;               /* GNTMAP_* */
> +    uint32_t flags;               /**< GNTMAP_* */
>      grant_ref_t ref;
>      domid_t  dom;
>      /* OUT parameters. */
> -    int16_t  status;              /* => enum grant_status */
> +    int16_t  status;              /**< GNTST_* status code */
>      grant_handle_t handle;
>      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
> + * 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>
> + * that is tracked by \@handle
> + *
>   * NOTES:
>   *  1. The call may fail in an undefined manner if either mapping is not
> - *     tracked by <handle>.
> + *     tracked by \@handle.
>   *  3. After executing a batch of unmaps, it is guaranteed that no stale
>   *     mappings will remain in the device or host TLBs.
>   */
> @@ -370,18 +426,19 @@ struct gnttab_unmap_grant_ref {
>      uint64_t dev_bus_addr;
>      grant_handle_t handle;
>      /* OUT parameters. */
> -    int16_t  status;              /* => enum grant_status */
> +    int16_t  status;              /**< GNTST_* status code */
>  };
>  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.
> +/**
> + * 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.
> + *  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 {
> @@ -389,7 +446,7 @@ struct gnttab_setup_table {
>      domid_t  dom;
>      uint32_t nr_frames;
>      /* OUT parameters. */
> -    int16_t  status;              /* => enum grant_status */
> +    int16_t  status;              /**< GNTST_* status code */
>  #if __XEN_INTERFACE_VERSION__ < 0x00040300
>      XEN_GUEST_HANDLE(ulong) frame_list;
>  #else
> @@ -399,7 +456,7 @@ 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.
>   */
> @@ -407,14 +464,14 @@ struct gnttab_dump_table {
>      /* IN parameters. */
>      domid_t dom;
>      /* OUT parameters. */
> -    int16_t status;               /* => enum grant_status */
> +    int16_t status;               /**< GNTST_* status code */
>  };
>  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
> - * has previously registered its interest in the transfer via <domid, ref>.
> +/**
> + * 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
>   * to the calling domain *unless* the error is GNTST_bad_page.
> @@ -427,13 +484,13 @@ struct gnttab_transfer {
>      domid_t       domid;
>      grant_ref_t   ref;
>      /* OUT parameters. */
> -    int16_t       status;
> +    int16_t       status;               /**< GNTST_* status code */
>  };
>  typedef struct gnttab_transfer gnttab_transfer_t;
>  DEFINE_XEN_GUEST_HANDLE(gnttab_transfer_t);
>  
>  
> -/*
> +/**
>   * 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
> @@ -451,11 +508,6 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_transfer_t);
>   * 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. */
>      struct gnttab_copy_ptr {
> @@ -467,19 +519,24 @@ struct gnttab_copy {
>          uint16_t offset;
>      } source, dest;
>      uint16_t      len;
> -    uint16_t      flags;          /* GNTCOPY_* */
> +    uint16_t      flags;          /**< GNTCOPY_* */
> +#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)
>      /* 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.
> + *
>   * NOTES:
> - *  1. <dom> may be specified as DOMID_SELF.
> - *  2. Only a sufficiently-privileged domain may specify <dom> != DOMID_SELF.
> + *  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. */
> @@ -487,19 +544,20 @@ struct gnttab_query_size {
>      /* OUT parameters. */
>      uint32_t nr_frames;
>      uint32_t max_nr_frames;
> -    int16_t  status;              /* => enum grant_status */
> +    int16_t  status;              /**< GNTST_* status code */
>  };
>  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
> + * 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>.
> + *     tracked by \@handle.
>   *  2. After executing a batch of unmaps, it is guaranteed that no stale
>   *     mappings will remain in the device or host TLBs.
>   */
> @@ -509,13 +567,13 @@ struct gnttab_unmap_and_replace {
>      uint64_t new_addr;
>      grant_handle_t handle;
>      /* OUT parameters. */
> -    int16_t  status;              /* => enum grant_status */
> +    int16_t  status;              /**< GNTST_* status code */
>  };
>  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
> @@ -529,32 +587,33 @@ 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
> + * 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.
> + * \@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.
> + *
>   * NOTES:
> - *  1. <dom> may be specified as DOMID_SELF.
> - *  2. Only a sufficiently-privileged domain may specify <dom> != DOMID_SELF.
> + *  1. \@dom may be specified as DOMID_SELF.
> + *  2. Only a sufficiently-privileged domain may specify \@dom != DOMID_SELF.
>   */
>  struct gnttab_get_status_frames {
>      /* IN parameters. */
>      uint32_t nr_frames;
>      domid_t  dom;
>      /* OUT parameters. */
> -    int16_t  status;              /* => enum grant_status */
> +    int16_t  status;              /**< GNTST_* status code */
>      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>.
> + * effect for domain \@dom.
>   */
>  struct gnttab_get_version {
>      /* IN parameters */
> @@ -566,7 +625,7 @@ struct gnttab_get_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 {
> @@ -574,12 +633,12 @@ struct gnttab_swap_grant_ref {
>      grant_ref_t ref_a;
>      grant_ref_t ref_b;
>      /* OUT parameters */
> -    int16_t status;             /* => enum grant_status */
> +    int16_t status;             /**< GNTST_* status code */
>  };
>  typedef struct gnttab_swap_grant_ref gnttab_swap_grant_ref_t;
>  DEFINE_XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t);
>  
> -/*
> +/**
>   * Issue one or more cache maintenance operations on a portion of a
>   * page granted to the calling domain by a foreign domain.
>   */
> @@ -588,8 +647,8 @@ struct gnttab_cache_flush {
>          uint64_t dev_bus_addr;
>          grant_ref_t ref;
>      } a;
> -    uint16_t offset; /* offset from start of grant */
> -    uint16_t length; /* size within the grant */
> +    uint16_t offset; /**< offset from start of grant */
> +    uint16_t length; /**< size within the grant */
>  #define GNTTAB_CACHE_CLEAN          (1u<<0)
>  #define GNTTAB_CACHE_INVAL          (1u<<1)
>  #define GNTTAB_CACHE_SOURCE_GREF    (1u<<31)
> @@ -600,40 +659,60 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
>  
>  #endif /* __XEN_INTERFACE_VERSION__ */
>  
> -/*
> - * Bitfield values for gnttab_map_grant_ref.flags.
> +/**
> + * @def GNTMAP_device_map
> + * Bitfield value for gnttab_map_grant_ref.flags: Map the grant entry for
> + * access by I/O devices.
>   */
> - /* 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. */
> +
> +/**
> + * @def GNTMAP_host_map
> + * Bitfield value for gnttab_map_grant_ref.flags: 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. */
> +
> +/**
> + * @def GNTMAP_readonly
> + * Bitfield value for gnttab_map_grant_ref.flags: 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.
> -  */
> +
> +/**
> + * @def GNTMAP_application_map
> + * Bitfield value for gnttab_map_grant_ref.flags.
> + *
> + * 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.
> -  */
> +/**
> + * @def GNTMAP_contains_pte
> + * Bitfield value for gnttab_map_grant_ref.flags.
> + *
> + * 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).
> +/**
> + * @def GNTMAP_guest_avail_mask
> + * Bitfield value for gnttab_map_grant_ref.flags: 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)
> @@ -641,21 +720,19 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
>  /*
>   * Values for error status returns. All errors are -ve.
>   */
> -/* ` 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)  /**< 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 GNTTABOP_error_msgs {                   \
>      "okay",                                     \
> @@ -673,6 +750,10 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
>      "operation not done; try again"             \
>  }
>  
> +/**
> + * @}
> + */
> +
>  #endif /* __XEN_PUBLIC_GRANT_TABLE_H__ */
>  
>  /*
> -- 
> 2.17.1
> 

Re: [PATCH v6 9/9] docs/doxygen: doxygen documentation for grant_table.h
Posted by Luca Fancellu 3 years, 5 months ago
Hi Stefano,

> On 24 Jun 2021, at 00:34, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Mon, 10 May 2021, Luca Fancellu wrote:
>> Modification to include/public/grant_table.h:
>> 
>> 1) Add doxygen tags to:
>> - Create Grant tables section
>> - include variables in the generated documentation
>> - Used @keepindent/@endkeepindent to enclose comment
>>   section that are indented using spaces, to keep
>>   the indentation.
>> 2) Add .rst file for grant table for Arm64
> 
> Why only arm64?

This is a mistake, it should be just “Add .rst file for grant table"

> 
> 
>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>> ---
>> v6 changes:
>> - Fix misaligned comment
>> - Moved comments to make them display in the docs
>> - Included more documentation in the docs
>>  (see output here: https://luca.fancellu.gitlab.io/xen-docs/hypercall-interfaces/common/grant_tables.html)
> 
> It looks much much better. All the info we care about seems to be there.
> The only things that I noticed missing and might be good to keep is the
> small comment about HYPERVISOR_grant_table_op:
> 
> /* ` 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

Where do you want me to put this comment in the html page? In the end of the description in the top of the page?

Cheers,

Luca

> 
> All the changes look good to me.
> 
> 
> 
>> v5 changes:
>> - Move GNTCOPY_* define next to the flags field
>> v4 changes:
>> - Used @keepindent/@endkeepindent doxygen commands
>>  to keep text with spaces indentation.
>> - drop changes to grant_entry_v1 comment, it will
>>  be changed and included in the docs in a future patch
>> - Move docs .rst to "common" folder
>> v3 changes:
>> - removed tags to skip anonymous union/struct
>> - moved back comment pointed out by Jan
>> - moved down defines related to struct gnttab_copy
>>  as pointed out by Jan
>> v2 changes:
>> - Revert back to anonymous union/struct
>> - add doxygen tags to skip anonymous union/struct
>> ---
>> docs/hypercall-interfaces/arm64.rst           |   1 +
>> .../common/grant_tables.rst                   |   9 +
>> docs/xen-doxygen/doxy_input.list              |   1 +
>> xen/include/public/grant_table.h              | 387 +++++++++++-------
>> 4 files changed, 245 insertions(+), 153 deletions(-)
>> create mode 100644 docs/hypercall-interfaces/common/grant_tables.rst
>> 
>> diff --git a/docs/hypercall-interfaces/arm64.rst b/docs/hypercall-interfaces/arm64.rst
>> index 5e701a2adc..cb4c0d13de 100644
>> --- a/docs/hypercall-interfaces/arm64.rst
>> +++ b/docs/hypercall-interfaces/arm64.rst
>> @@ -8,6 +8,7 @@ Starting points
>> .. toctree::
>>    :maxdepth: 2
>> 
>> +   common/grant_tables
>> 
>> 
>> Functions
>> diff --git a/docs/hypercall-interfaces/common/grant_tables.rst b/docs/hypercall-interfaces/common/grant_tables.rst
>> new file mode 100644
>> index 0000000000..b8a1ef8759
>> --- /dev/null
>> +++ b/docs/hypercall-interfaces/common/grant_tables.rst
>> @@ -0,0 +1,9 @@
>> +.. SPDX-License-Identifier: CC-BY-4.0
>> +
>> +Grant Tables
>> +============
>> +
>> +.. doxygengroup:: grant_table
>> +   :project: Xen
>> +   :members:
>> +   :undoc-members:
>> diff --git a/docs/xen-doxygen/doxy_input.list b/docs/xen-doxygen/doxy_input.list
>> index e69de29bb2..233d692fa7 100644
>> --- a/docs/xen-doxygen/doxy_input.list
>> +++ b/docs/xen-doxygen/doxy_input.list
>> @@ -0,0 +1 @@
>> +xen/include/public/grant_table.h
>> diff --git a/xen/include/public/grant_table.h b/xen/include/public/grant_table.h
>> index 84b1d26b36..dfa5155927 100644
>> --- a/xen/include/public/grant_table.h
>> +++ b/xen/include/public/grant_table.h
>> @@ -25,15 +25,19 @@
>>  * Copyright (c) 2004, K A Fraser
>>  */
>> 
>> +/**
>> + * @file
>> + * @brief Interface for granting foreign access to page frames, and receiving
>> + * page-ownership transfers.
>> + */
>> +
>> #ifndef __XEN_PUBLIC_GRANT_TABLE_H__
>> #define __XEN_PUBLIC_GRANT_TABLE_H__
>> 
>> #include "xen.h"
>> 
>> -/*
>> - * `incontents 150 gnttab Grant Tables
>> - *
>> - * Xen's grant tables provide a generic mechanism to memory sharing
>> +/**
>> + * @brief Xen's grant tables provide a generic mechanism to memory sharing
>>  * between domains. This shared memory interface underpins the split
>>  * device drivers for block and network IO.
>>  *
>> @@ -51,13 +55,13 @@
>>  * know the real machine address of a page it is sharing. This makes
>>  * it possible to share memory correctly with domains running in
>>  * fully virtualised memory.
>> - */
>> -
>> -/***********************************
>> + *
>>  * GRANT TABLE REPRESENTATION
>> - */
>> -
>> -/* Some rough guidelines on accessing and updating grant-table entries
>> + *
>> + * A grant table comprises a packed array of grant entries in one or more
>> + * page frames shared between Xen and a guest.
>> + *
>> + * 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
>> @@ -66,6 +70,7 @@
>>  *     compiler barrier will still be required.
>>  *
>>  * Introducing a valid entry into the grant table:
>> + * @keepindent
>>  *  1. Write ent->domid.
>>  *  2. Write ent->frame:
>>  *      GTF_permit_access:   Frame to which access is permitted.
>> @@ -73,20 +78,25 @@
>>  *                           frame, or zero if none.
>>  *  3. Write memory barrier (WMB).
>>  *  4. Write ent->flags, inc. valid type.
>> + * @endkeepindent
>>  *
>>  * Invalidating an unused GTF_permit_access entry:
>> + * @keepindent
>>  *  1. flags = ent->flags.
>>  *  2. Observe that !(flags & (GTF_reading|GTF_writing)).
>>  *  3. Check result of SMP-safe CMPXCHG(&ent->flags, flags, 0).
>>  *  NB. No need for WMB as reuse of entry is control-dependent on success of
>>  *      step 3, and all architectures guarantee ordering of ctrl-dep writes.
>> + * @endkeepindent
>>  *
>>  * Invalidating an in-use GTF_permit_access entry:
>> + *
>>  *  This cannot be done directly. Request assistance from the domain controller
>>  *  which can set a timeout on the use of a grant entry and take necessary
>>  *  action. (NB. This is not yet implemented!).
>>  *
>>  * Invalidating an unused GTF_accept_transfer entry:
>> + * @keepindent
>>  *  1. flags = ent->flags.
>>  *  2. Observe that !(flags & GTF_transfer_committed). [*]
>>  *  3. Check result of SMP-safe CMPXCHG(&ent->flags, flags, 0).
>> @@ -97,29 +107,32 @@
>>  *      transferred frame is written. It is safe for the guest to spin waiting
>>  *      for this to occur (detect by observing GTF_transfer_completed in
>>  *      ent->flags).
>> + * @endkeepindent
>>  *
>>  * Invalidating a committed GTF_accept_transfer entry:
>>  *  1. Wait for (ent->flags & GTF_transfer_completed).
>>  *
>>  * Changing a GTF_permit_access from writable to read-only:
>> + *
>>  *  Use SMP-safe CMPXCHG to set GTF_readonly, while checking !GTF_writing.
>>  *
>>  * Changing a GTF_permit_access from read-only to writable:
>> + *
>>  *  Use SMP-safe bit-setting instruction.
>> + *
>> + * Data structure fields or defines described below have the following tags:
>> + * * [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.
>> + *
>> + * @addtogroup grant_table Grant Tables
>> + * @{
>>  */
>> 
>> -/*
>> +/**
>>  * Reference to a grant entry in a specified domain's grant table.
>>  */
>> typedef uint32_t grant_ref_t;
>> 
>> -/*
>> - * 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.
>> @@ -129,15 +142,17 @@ typedef uint32_t grant_ref_t;
>> #define grant_entry_v1_t grant_entry_t
>> #endif
>> struct grant_entry_v1 {
>> -    /* GTF_xxx: various type and flag information.  [XEN,GST] */
>> +    /** GTF_xxx: various type and flag information.  [XEN,GST] */
>>     uint16_t flags;
>> -    /* The domain being granted foreign privileges. [GST] */
>> +    /** The domain being granted foreign privileges. [GST] */
>>     domid_t  domid;
>> -    /*
>> +    /**
>> +     * @keepindent
>>      * 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]
>> +     * @endkeepindent
>>      */
>>     uint32_t frame;
>> };
>> @@ -150,60 +165,99 @@ typedef struct grant_entry_v1 grant_entry_v1_t;
>> #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.
>> - */
>> +/** This type of grant entry grants no privileges. */
>> #define GTF_invalid         (0U<<0)
>> +
>> +/** This type of grant entry allow \@domid to map/access \@frame. */
>> #define GTF_permit_access   (1U<<0)
>> +
>> +/**
>> + * This type of grant entry allow \@domid to transfer ownership of one pageframe
>> + * to this guest. Xen writes the page number to \@frame.
>> + */
>> #define GTF_accept_transfer (2U<<0)
>> +
>> +/**
>> + * This type of grant entry allow \@domid to transitively access a subrange of
>> + * \@trans_grant in \@trans_domid.  No mappings are allowed.
>> + */
>> #define GTF_transitive      (3U<<0)
>> +
>> #define GTF_type_mask       (3U<<0)
>> 
>> -/*
>> - * Subflags for GTF_permit_access and GTF_transitive.
>> - *  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]
>> - * Further subflags for GTF_permit_access only.
>> - *  GTF_PAT, GTF_PWT, GTF_PCD: (x86) cache attribute flags to be used for
>> - *                             mappings of 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]
>> +/**
>> + * @def GTF_readonly
>> + * Subflag for GTF_permit_access and GTF_transitive: Restrict \@domid to
>> + * read-only mappings and accesses. [GST]
>>  */
>> #define _GTF_readonly       (2)
>> #define GTF_readonly        (1U<<_GTF_readonly)
>> +
>> +/**
>> + * @def GTF_reading
>> + * Subflag for GTF_permit_access and GTF_transitive: Grant entry is currently
>> + * mapped for reading by \@domid. [XEN]
>> + */
>> #define _GTF_reading        (3)
>> #define GTF_reading         (1U<<_GTF_reading)
>> +
>> +/**
>> + * @def GTF_writing
>> + * Subflag for GTF_permit_access and GTF_transitive: Grant entry is currently
>> + * mapped for writing by \@domid. [XEN]
>> + */
>> #define _GTF_writing        (4)
>> #define GTF_writing         (1U<<_GTF_writing)
>> +
>> +/**
>> + * @def GTF_PWT
>> + * Subflag for GTF_permit_access only: (x86) cache attribute flags to be used
>> + * for mappings of the grant [GST]
>> + */
>> #define _GTF_PWT            (5)
>> #define GTF_PWT             (1U<<_GTF_PWT)
>> +
>> +/**
>> + * @def GTF_PCD
>> + * Subflag for GTF_permit_access only: (x86) cache attribute flags to be used
>> + * for mappings of the grant [GST]
>> + */
>> #define _GTF_PCD            (6)
>> #define GTF_PCD             (1U<<_GTF_PCD)
>> +
>> +/**
>> + * @def GTF_PAT
>> + * Subflag for GTF_permit_access only: (x86) cache attribute flags to be used
>> + * for mappings of the grant [GST]
>> + */
>> #define _GTF_PAT            (7)
>> #define GTF_PAT             (1U<<_GTF_PAT)
>> +
>> +/**
>> + * @def GTF_sub_page
>> + * Subflag for GTF_permit_access only: 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_sub_page       (8)
>> #define GTF_sub_page        (1U<<_GTF_sub_page)
>> 
>> -/*
>> - * 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
>> - *      set by Xen.
>> - *  GTF_transfer_completed: It is safe for the guest to spin-wait on this flag
>> - *      after reading GTF_transfer_committed. Xen will always write the frame
>> - *      address, followed by ORing this flag, in a timely manner.
>> +/**
>> + * @def GTF_transfer_committed
>> + * Subflag for GTF_accept_transfer: 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
>> + * set by Xen.
>>  */
>> #define _GTF_transfer_committed (2)
>> #define GTF_transfer_committed  (1U<<_GTF_transfer_committed)
>> +
>> +/**
>> + * @def GTF_transfer_completed
>> + * Subflag for GTF_accept_transfer: It is safe for the guest to spin-wait on
>> + * this flag after reading GTF_transfer_committed. Xen will always write the
>> + * frame address, followed by ORing this flag, in a timely manner.
>> + */
>> #define _GTF_transfer_completed (3)
>> #define GTF_transfer_completed  (1U<<_GTF_transfer_completed)
>> 
>> @@ -228,17 +282,17 @@ struct grant_entry_header {
>> };
>> typedef struct grant_entry_header grant_entry_header_t;
>> 
>> -/*
>> +/**
>>  * Version 2 of the grant entry structure.
>>  */
>> union grant_entry_v2 {
>>     grant_entry_header_t hdr;
>> 
>> -    /*
>> +    /**
>>      * 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.
>> @@ -249,10 +303,10 @@ union grant_entry_v2 {
>>         uint64_t frame;
>>     } full_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.
>> +     * \@domid is allowed to access bytes [\@page_off,\@page_off+\@length)
>> +     * in frame \@frame.
>>      */
>>     struct {
>>         grant_entry_header_t hdr;
>> @@ -261,9 +315,9 @@ union grant_entry_v2 {
>>         uint64_t frame;
>>     } sub_page;
>> 
>> -    /*
>> -     * If the grant is GTF_transitive, @domid is allowed to use the
>> -     * grant @gref in domain @trans_domid, as if it was the local
>> +    /**
>> +     * 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
>>      * with the original grant.
>>      *
>> @@ -277,7 +331,7 @@ union grant_entry_v2 {
>>         grant_ref_t gref;
>>     } transitive;
>> 
>> -    uint32_t __spacer[4]; /* Pad to a power of two */
>> +    uint32_t __spacer[4]; /**< Pad to a power of two */
>> };
>> typedef union grant_entry_v2 grant_entry_v2_t;
>> 
>> @@ -317,24 +371,25 @@ typedef uint16_t grant_status_t;
>> #endif /* __XEN_INTERFACE_VERSION__ */
>> /* ` } */
>> 
>> -/*
>> +/**
>>  * 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>
>> +/**
>> + * 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
>> + *  1. If GNTMAP_device_map is specified then \@dev_bus_addr is the address
>>  *     via which I/O devices may access the granted frame.
>>  *  2. If GNTMAP_host_map is specified then a mapping will be added at
>>  *     either a host virtual address in the current address space, or at
>>  *     a PTE at the specified machine address.  The type of mapping to
>>  *     perform is selected through the GNTMAP_contains_pte flag, and the
>> - *     address is specified in <host_addr>.
>> + *     address is specified in \@host_addr.
>>  *  3. Mappings should only be destroyed via GNTTABOP_unmap_grant_ref. If a
>>  *     host mapping is destroyed by other means then it is *NOT* guaranteed
>>  *     to be accounted to the correct grant reference!
>> @@ -342,25 +397,26 @@ typedef uint32_t grant_handle_t;
>> struct gnttab_map_grant_ref {
>>     /* IN parameters. */
>>     uint64_t host_addr;
>> -    uint32_t flags;               /* GNTMAP_* */
>> +    uint32_t flags;               /**< GNTMAP_* */
>>     grant_ref_t ref;
>>     domid_t  dom;
>>     /* OUT parameters. */
>> -    int16_t  status;              /* => enum grant_status */
>> +    int16_t  status;              /**< GNTST_* status code */
>>     grant_handle_t handle;
>>     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
>> + * 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>
>> + * that is tracked by \@handle
>> + *
>>  * NOTES:
>>  *  1. The call may fail in an undefined manner if either mapping is not
>> - *     tracked by <handle>.
>> + *     tracked by \@handle.
>>  *  3. After executing a batch of unmaps, it is guaranteed that no stale
>>  *     mappings will remain in the device or host TLBs.
>>  */
>> @@ -370,18 +426,19 @@ struct gnttab_unmap_grant_ref {
>>     uint64_t dev_bus_addr;
>>     grant_handle_t handle;
>>     /* OUT parameters. */
>> -    int16_t  status;              /* => enum grant_status */
>> +    int16_t  status;              /**< GNTST_* status code */
>> };
>> 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.
>> +/**
>> + * 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.
>> + *  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 {
>> @@ -389,7 +446,7 @@ struct gnttab_setup_table {
>>     domid_t  dom;
>>     uint32_t nr_frames;
>>     /* OUT parameters. */
>> -    int16_t  status;              /* => enum grant_status */
>> +    int16_t  status;              /**< GNTST_* status code */
>> #if __XEN_INTERFACE_VERSION__ < 0x00040300
>>     XEN_GUEST_HANDLE(ulong) frame_list;
>> #else
>> @@ -399,7 +456,7 @@ 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.
>>  */
>> @@ -407,14 +464,14 @@ struct gnttab_dump_table {
>>     /* IN parameters. */
>>     domid_t dom;
>>     /* OUT parameters. */
>> -    int16_t status;               /* => enum grant_status */
>> +    int16_t status;               /**< GNTST_* status code */
>> };
>> 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
>> - * has previously registered its interest in the transfer via <domid, ref>.
>> +/**
>> + * 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
>>  * to the calling domain *unless* the error is GNTST_bad_page.
>> @@ -427,13 +484,13 @@ struct gnttab_transfer {
>>     domid_t       domid;
>>     grant_ref_t   ref;
>>     /* OUT parameters. */
>> -    int16_t       status;
>> +    int16_t       status;               /**< GNTST_* status code */
>> };
>> typedef struct gnttab_transfer gnttab_transfer_t;
>> DEFINE_XEN_GUEST_HANDLE(gnttab_transfer_t);
>> 
>> 
>> -/*
>> +/**
>>  * 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
>> @@ -451,11 +508,6 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_transfer_t);
>>  * 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. */
>>     struct gnttab_copy_ptr {
>> @@ -467,19 +519,24 @@ struct gnttab_copy {
>>         uint16_t offset;
>>     } source, dest;
>>     uint16_t      len;
>> -    uint16_t      flags;          /* GNTCOPY_* */
>> +    uint16_t      flags;          /**< GNTCOPY_* */
>> +#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)
>>     /* 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.
>> + *
>>  * NOTES:
>> - *  1. <dom> may be specified as DOMID_SELF.
>> - *  2. Only a sufficiently-privileged domain may specify <dom> != DOMID_SELF.
>> + *  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. */
>> @@ -487,19 +544,20 @@ struct gnttab_query_size {
>>     /* OUT parameters. */
>>     uint32_t nr_frames;
>>     uint32_t max_nr_frames;
>> -    int16_t  status;              /* => enum grant_status */
>> +    int16_t  status;              /**< GNTST_* status code */
>> };
>> 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
>> + * 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>.
>> + *     tracked by \@handle.
>>  *  2. After executing a batch of unmaps, it is guaranteed that no stale
>>  *     mappings will remain in the device or host TLBs.
>>  */
>> @@ -509,13 +567,13 @@ struct gnttab_unmap_and_replace {
>>     uint64_t new_addr;
>>     grant_handle_t handle;
>>     /* OUT parameters. */
>> -    int16_t  status;              /* => enum grant_status */
>> +    int16_t  status;              /**< GNTST_* status code */
>> };
>> 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
>> @@ -529,32 +587,33 @@ 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
>> + * 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.
>> + * \@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.
>> + *
>>  * NOTES:
>> - *  1. <dom> may be specified as DOMID_SELF.
>> - *  2. Only a sufficiently-privileged domain may specify <dom> != DOMID_SELF.
>> + *  1. \@dom may be specified as DOMID_SELF.
>> + *  2. Only a sufficiently-privileged domain may specify \@dom != DOMID_SELF.
>>  */
>> struct gnttab_get_status_frames {
>>     /* IN parameters. */
>>     uint32_t nr_frames;
>>     domid_t  dom;
>>     /* OUT parameters. */
>> -    int16_t  status;              /* => enum grant_status */
>> +    int16_t  status;              /**< GNTST_* status code */
>>     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>.
>> + * effect for domain \@dom.
>>  */
>> struct gnttab_get_version {
>>     /* IN parameters */
>> @@ -566,7 +625,7 @@ struct gnttab_get_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 {
>> @@ -574,12 +633,12 @@ struct gnttab_swap_grant_ref {
>>     grant_ref_t ref_a;
>>     grant_ref_t ref_b;
>>     /* OUT parameters */
>> -    int16_t status;             /* => enum grant_status */
>> +    int16_t status;             /**< GNTST_* status code */
>> };
>> typedef struct gnttab_swap_grant_ref gnttab_swap_grant_ref_t;
>> DEFINE_XEN_GUEST_HANDLE(gnttab_swap_grant_ref_t);
>> 
>> -/*
>> +/**
>>  * Issue one or more cache maintenance operations on a portion of a
>>  * page granted to the calling domain by a foreign domain.
>>  */
>> @@ -588,8 +647,8 @@ struct gnttab_cache_flush {
>>         uint64_t dev_bus_addr;
>>         grant_ref_t ref;
>>     } a;
>> -    uint16_t offset; /* offset from start of grant */
>> -    uint16_t length; /* size within the grant */
>> +    uint16_t offset; /**< offset from start of grant */
>> +    uint16_t length; /**< size within the grant */
>> #define GNTTAB_CACHE_CLEAN          (1u<<0)
>> #define GNTTAB_CACHE_INVAL          (1u<<1)
>> #define GNTTAB_CACHE_SOURCE_GREF    (1u<<31)
>> @@ -600,40 +659,60 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
>> 
>> #endif /* __XEN_INTERFACE_VERSION__ */
>> 
>> -/*
>> - * Bitfield values for gnttab_map_grant_ref.flags.
>> +/**
>> + * @def GNTMAP_device_map
>> + * Bitfield value for gnttab_map_grant_ref.flags: Map the grant entry for
>> + * access by I/O devices.
>>  */
>> - /* 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. */
>> +
>> +/**
>> + * @def GNTMAP_host_map
>> + * Bitfield value for gnttab_map_grant_ref.flags: 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. */
>> +
>> +/**
>> + * @def GNTMAP_readonly
>> + * Bitfield value for gnttab_map_grant_ref.flags: 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.
>> -  */
>> +
>> +/**
>> + * @def GNTMAP_application_map
>> + * Bitfield value for gnttab_map_grant_ref.flags.
>> + *
>> + * 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.
>> -  */
>> +/**
>> + * @def GNTMAP_contains_pte
>> + * Bitfield value for gnttab_map_grant_ref.flags.
>> + *
>> + * 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).
>> +/**
>> + * @def GNTMAP_guest_avail_mask
>> + * Bitfield value for gnttab_map_grant_ref.flags: 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)
>> @@ -641,21 +720,19 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
>> /*
>>  * Values for error status returns. All errors are -ve.
>>  */
>> -/* ` 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)  /**< 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 GNTTABOP_error_msgs {                   \
>>     "okay",                                     \
>> @@ -673,6 +750,10 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
>>     "operation not done; try again"             \
>> }
>> 
>> +/**
>> + * @}
>> + */
>> +
>> #endif /* __XEN_PUBLIC_GRANT_TABLE_H__ */
>> 
>> /*
>> -- 
>> 2.17.1
>> 


Re: [PATCH v6 9/9] docs/doxygen: doxygen documentation for grant_table.h
Posted by Stefano Stabellini 3 years, 5 months ago
On Thu, 1 Jul 2021, Luca Fancellu wrote:
> Hi Stefano,
> 
> > On 24 Jun 2021, at 00:34, Stefano Stabellini <sstabellini@kernel.org> wrote:
> > 
> > On Mon, 10 May 2021, Luca Fancellu wrote:
> >> Modification to include/public/grant_table.h:
> >> 
> >> 1) Add doxygen tags to:
> >> - Create Grant tables section
> >> - include variables in the generated documentation
> >> - Used @keepindent/@endkeepindent to enclose comment
> >>   section that are indented using spaces, to keep
> >>   the indentation.
> >> 2) Add .rst file for grant table for Arm64
> > 
> > Why only arm64?
> 
> This is a mistake, it should be just “Add .rst file for grant table"
> 
> > 
> > 
> >> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> >> ---
> >> v6 changes:
> >> - Fix misaligned comment
> >> - Moved comments to make them display in the docs
> >> - Included more documentation in the docs
> >>  (see output here: https://luca.fancellu.gitlab.io/xen-docs/hypercall-interfaces/common/grant_tables.html)
> > 
> > It looks much much better. All the info we care about seems to be there.
> > The only things that I noticed missing and might be good to keep is the
> > small comment about HYPERVISOR_grant_table_op:
> > 
> > /* ` 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
> 
> Where do you want me to put this comment in the html page? In the end of the description in the top of the page?

Yeah, that looks like a good place
Re: [PATCH v6 9/9] docs/doxygen: doxygen documentation for grant_table.h
Posted by Luca Fancellu 3 years, 5 months ago

> On 1 Jul 2021, at 18:44, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Thu, 1 Jul 2021, Luca Fancellu wrote:
>> Hi Stefano,
>> 
>>> On 24 Jun 2021, at 00:34, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>> 
>>> On Mon, 10 May 2021, Luca Fancellu wrote:
>>>> Modification to include/public/grant_table.h:
>>>> 
>>>> 1) Add doxygen tags to:
>>>> - Create Grant tables section
>>>> - include variables in the generated documentation
>>>> - Used @keepindent/@endkeepindent to enclose comment
>>>>  section that are indented using spaces, to keep
>>>>  the indentation.
>>>> 2) Add .rst file for grant table for Arm64
>>> 
>>> Why only arm64?
>> 
>> This is a mistake, it should be just “Add .rst file for grant table"
>> 
>>> 
>>> 
>>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>>>> ---
>>>> v6 changes:
>>>> - Fix misaligned comment
>>>> - Moved comments to make them display in the docs
>>>> - Included more documentation in the docs
>>>> (see output here: https://luca.fancellu.gitlab.io/xen-docs/hypercall-interfaces/common/grant_tables.html)
>>> 
>>> It looks much much better. All the info we care about seems to be there.
>>> The only things that I noticed missing and might be good to keep is the
>>> small comment about HYPERVISOR_grant_table_op:
>>> 
>>> /* ` 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
>> 
>> Where do you want me to put this comment in the html page? In the end of the description in the top of the page?
> 
> Yeah, that looks like a good place

Great, for a preview, have a look on this: https://luca.fancellu.gitlab.io/xen-docs/hypercall-interfaces/common/grant_tables.html


Re: [PATCH v6 9/9] docs/doxygen: doxygen documentation for grant_table.h
Posted by Stefano Stabellini 3 years, 5 months ago
On Fri, 2 Jul 2021, Luca Fancellu wrote:
> > On 1 Jul 2021, at 18:44, Stefano Stabellini <sstabellini@kernel.org> wrote:
> > 
> > On Thu, 1 Jul 2021, Luca Fancellu wrote:
> >> Hi Stefano,
> >> 
> >>> On 24 Jun 2021, at 00:34, Stefano Stabellini <sstabellini@kernel.org> wrote:
> >>> 
> >>> On Mon, 10 May 2021, Luca Fancellu wrote:
> >>>> Modification to include/public/grant_table.h:
> >>>> 
> >>>> 1) Add doxygen tags to:
> >>>> - Create Grant tables section
> >>>> - include variables in the generated documentation
> >>>> - Used @keepindent/@endkeepindent to enclose comment
> >>>>  section that are indented using spaces, to keep
> >>>>  the indentation.
> >>>> 2) Add .rst file for grant table for Arm64
> >>> 
> >>> Why only arm64?
> >> 
> >> This is a mistake, it should be just “Add .rst file for grant table"
> >> 
> >>> 
> >>> 
> >>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> >>>> ---
> >>>> v6 changes:
> >>>> - Fix misaligned comment
> >>>> - Moved comments to make them display in the docs
> >>>> - Included more documentation in the docs
> >>>> (see output here: https://luca.fancellu.gitlab.io/xen-docs/hypercall-interfaces/common/grant_tables.html)
> >>> 
> >>> It looks much much better. All the info we care about seems to be there.
> >>> The only things that I noticed missing and might be good to keep is the
> >>> small comment about HYPERVISOR_grant_table_op:
> >>> 
> >>> /* ` 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
> >> 
> >> Where do you want me to put this comment in the html page? In the end of the description in the top of the page?
> > 
> > Yeah, that looks like a good place
> 
> Great, for a preview, have a look on this: https://luca.fancellu.gitlab.io/xen-docs/hypercall-interfaces/common/grant_tables.html

Looks good!