[PATCH v4 3/3] docs/doxygen: doxygen documentation for grant_table.h

Luca Fancellu posted 3 patches 4 years, 9 months ago
There is a newer version of this series
[PATCH v4 3/3] docs/doxygen: doxygen documentation for grant_table.h
Posted by Luca Fancellu 4 years, 9 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>
---
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                   |  8 +++
 docs/xen-doxygen/doxy_input.list              |  1 +
 xen/include/public/grant_table.h              | 68 ++++++++++++-------
 4 files changed, 54 insertions(+), 24 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..8955ec5812
--- /dev/null
+++ b/docs/hypercall-interfaces/common/grant_tables.rst
@@ -0,0 +1,8 @@
+.. SPDX-License-Identifier: CC-BY-4.0
+
+Grant Tables
+============
+
+.. doxygengroup:: grant_table
+   :project: Xen
+   :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..d879c3e23e 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,10 @@
  * 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
+ *
+ * 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 +67,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 +75,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,18 +104,24 @@
  *      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.
+ *
+ * @addtogroup grant_table Grant Tables
+ * @{
  */
 
-/*
+/**
  * Reference to a grant entry in a specified domain's grant table.
  */
 typedef uint32_t grant_ref_t;
@@ -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;
 };
@@ -228,7 +243,7 @@ struct grant_entry_header {
 };
 typedef struct grant_entry_header grant_entry_header_t;
 
-/*
+/**
  * Version 2 of the grant entry structure.
  */
 union grant_entry_v2 {
@@ -433,7 +448,7 @@ 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 +466,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 {
@@ -471,6 +481,12 @@ struct gnttab_copy {
     /* OUT parameters. */
     int16_t       status;
 };
+
+#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)
+
 typedef struct gnttab_copy  gnttab_copy_t;
 DEFINE_XEN_GUEST_HANDLE(gnttab_copy_t);
 
@@ -579,7 +595,7 @@ struct gnttab_swap_grant_ref {
 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 +604,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)
@@ -673,6 +689,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 v4 3/3] docs/doxygen: doxygen documentation for grant_table.h
Posted by Jan Beulich 4 years, 9 months ago
On 04.05.2021 11:46, Luca Fancellu wrote:
> @@ -451,11 +466,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 {
> @@ -471,6 +481,12 @@ struct gnttab_copy {
>      /* OUT parameters. */
>      int16_t       status;
>  };
> +
> +#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)

Didn't you say you agree with moving this back up some, next to the
field using these?

Jan

Re: [PATCH v4 3/3] docs/doxygen: doxygen documentation for grant_table.h
Posted by Luca Fancellu 4 years, 9 months ago

> On 4 May 2021, at 12:48, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 04.05.2021 11:46, Luca Fancellu wrote:
>> @@ -451,11 +466,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 {
>> @@ -471,6 +481,12 @@ struct gnttab_copy {
>>     /* OUT parameters. */
>>     int16_t       status;
>> };
>> +
>> +#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)
> 
> Didn't you say you agree with moving this back up some, next to the
> field using these?

Hi Jan,

My mistake! I’ll move it in the next patch, did you spot anything else I might have forgot of what we agreed?

Cheers,
Luca

> 
> Jan


Re: [PATCH v4 3/3] docs/doxygen: doxygen documentation for grant_table.h
Posted by Jan Beulich 4 years, 9 months ago
On 04.05.2021 15:09, Luca Fancellu wrote:
>> On 4 May 2021, at 12:48, Jan Beulich <jbeulich@suse.com> wrote:
>> On 04.05.2021 11:46, Luca Fancellu wrote:
>>> @@ -451,11 +466,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 {
>>> @@ -471,6 +481,12 @@ struct gnttab_copy {
>>>     /* OUT parameters. */
>>>     int16_t       status;
>>> };
>>> +
>>> +#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)
>>
>> Didn't you say you agree with moving this back up some, next to the
>> field using these?
> 
> My mistake! I’ll move it in the next patch, did you spot anything else I might have forgot of what we agreed?

No, thanks. I don't think I have any more comments to make on this
series (once this last aspect got addressed, and assuming no new
issues get introduced). But to be clear on that side as well - I
don't think I'm up to actually ack-ing the patch (let alone the
entire series).

Jan

Re: [PATCH v4 3/3] docs/doxygen: doxygen documentation for grant_table.h
Posted by Luca Fancellu 4 years, 9 months ago

> On 4 May 2021, at 14:28, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 04.05.2021 15:09, Luca Fancellu wrote:
>>> On 4 May 2021, at 12:48, Jan Beulich <jbeulich@suse.com> wrote:
>>> On 04.05.2021 11:46, Luca Fancellu wrote:
>>>> @@ -451,11 +466,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 {
>>>> @@ -471,6 +481,12 @@ struct gnttab_copy {
>>>>    /* OUT parameters. */
>>>>    int16_t       status;
>>>> };
>>>> +
>>>> +#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)
>>> 
>>> Didn't you say you agree with moving this back up some, next to the
>>> field using these?
>> 
>> My mistake! I’ll move it in the next patch, did you spot anything else I might have forgot of what we agreed?
> 
> No, thanks. I don't think I have any more comments to make on this
> series (once this last aspect got addressed, and assuming no new
> issues get introduced). But to be clear on that side as well - I
> don't think I'm up to actually ack-ing the patch (let alone the
> entire series).

Ok, at least would you mind to do a review by of the patches we discussed together?

Cheers,
Luca

> 
> Jan


Re: [PATCH v4 3/3] docs/doxygen: doxygen documentation for grant_table.h
Posted by Jan Beulich 4 years, 9 months ago
On 04.05.2021 15:33, Luca Fancellu wrote:
> 
> 
>> On 4 May 2021, at 14:28, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 04.05.2021 15:09, Luca Fancellu wrote:
>>>> On 4 May 2021, at 12:48, Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 04.05.2021 11:46, Luca Fancellu wrote:
>>>>> @@ -451,11 +466,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 {
>>>>> @@ -471,6 +481,12 @@ struct gnttab_copy {
>>>>>    /* OUT parameters. */
>>>>>    int16_t       status;
>>>>> };
>>>>> +
>>>>> +#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)
>>>>
>>>> Didn't you say you agree with moving this back up some, next to the
>>>> field using these?
>>>
>>> My mistake! I’ll move it in the next patch, did you spot anything else I might have forgot of what we agreed?
>>
>> No, thanks. I don't think I have any more comments to make on this
>> series (once this last aspect got addressed, and assuming no new
>> issues get introduced). But to be clear on that side as well - I
>> don't think I'm up to actually ack-ing the patch (let alone the
>> entire series).
> 
> Ok, at least would you mind to do a review by of the patches we discussed together?

I'm afraid I don't understand: I did look over this one.

Jan