diff mbox series

[v7,9/9] docs/doxygen: doxygen documentation for grant_table.h

Message ID 20210705105103.14509-10-luca.fancellu@arm.com (mailing list archive)
State New, archived
Headers show
Series Use Doxygen and sphinx for html documentation | expand

Commit Message

Luca Fancellu July 5, 2021, 10:51 a.m. UTC
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

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
v7 changes:
- commit message changed
- Add comment about grant table queries and uses
to the documentation
v6 changes:
- Fix misaligned comment
- Moved comments to make them display in the docs
- Included more documentation in the docs
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              | 459 +++++++++++-------
 4 files changed, 288 insertions(+), 182 deletions(-)
 create mode 100644 docs/hypercall-interfaces/common/grant_tables.rst

Comments

Jan Beulich July 5, 2021, 1:03 p.m. UTC | #1
On 05.07.2021 12:51, 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
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> ---
> v7 changes:
> - commit message changed
> - Add comment about grant table queries and uses
> to the documentation
> v6 changes:
> - Fix misaligned comment
> - Moved comments to make them display in the docs
> - Included more documentation in the docs
> 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 +

In patch 8 you now add arm32.rst and x86.rst as well, so it's at
least odd that here you alter only one of the three. However, ...

> --- a/docs/hypercall-interfaces/arm64.rst
> +++ b/docs/hypercall-interfaces/arm64.rst
> @@ -8,6 +8,7 @@ Starting points
>  .. toctree::
>     :maxdepth: 2
>  
> +   common/grant_tables

... to me this seems the wrong way round anyway: I'd rather see
common stuff not be linked from per-arch locations, but per-arch
docs to be down the hierarchy from common ones.

Jan
Luca Fancellu July 5, 2021, 1:23 p.m. UTC | #2
Hi Jan,

> On 5 Jul 2021, at 14:03, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 05.07.2021 12:51, 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
>> 
>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>> ---
>> v7 changes:
>> - commit message changed
>> - Add comment about grant table queries and uses
>> to the documentation
>> v6 changes:
>> - Fix misaligned comment
>> - Moved comments to make them display in the docs
>> - Included more documentation in the docs
>> 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 +
> 
> In patch 8 you now add arm32.rst and x86.rst as well, so it's at
> least odd that here you alter only one of the three. However, ...
> 
>> --- a/docs/hypercall-interfaces/arm64.rst
>> +++ b/docs/hypercall-interfaces/arm64.rst
>> @@ -8,6 +8,7 @@ Starting points
>> .. toctree::
>>    :maxdepth: 2
>> 
>> +   common/grant_tables
> 
> ... to me this seems the wrong way round anyway: I'd rather see
> common stuff not be linked from per-arch locations, but per-arch
> docs to be down the hierarchy from common ones.

The things is that common stuff is not really common, if you go and solve every
define and so on, you end up with a different content for x86, aarch64, arm.

So you can’t have a common.rst containing the common things because depending on each
platform the content will change.

Thanks anyway for your feedback, I think it’s good to make the community know these changing
and discuss them all together.

Cheers,

Luca

> 
> Jan
Jan Beulich July 5, 2021, 1:27 p.m. UTC | #3
On 05.07.2021 15:23, Luca Fancellu wrote:
> Hi Jan,
> 
>> On 5 Jul 2021, at 14:03, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 05.07.2021 12:51, 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
>>>
>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>>> ---
>>> v7 changes:
>>> - commit message changed
>>> - Add comment about grant table queries and uses
>>> to the documentation
>>> v6 changes:
>>> - Fix misaligned comment
>>> - Moved comments to make them display in the docs
>>> - Included more documentation in the docs
>>> 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 +
>>
>> In patch 8 you now add arm32.rst and x86.rst as well, so it's at
>> least odd that here you alter only one of the three. However, ...
>>
>>> --- a/docs/hypercall-interfaces/arm64.rst
>>> +++ b/docs/hypercall-interfaces/arm64.rst
>>> @@ -8,6 +8,7 @@ Starting points
>>> .. toctree::
>>>    :maxdepth: 2
>>>
>>> +   common/grant_tables
>>
>> ... to me this seems the wrong way round anyway: I'd rather see
>> common stuff not be linked from per-arch locations, but per-arch
>> docs to be down the hierarchy from common ones.
> 
> The things is that common stuff is not really common, if you go and solve every
> define and so on, you end up with a different content for x86, aarch64, arm.
> 
> So you can’t have a common.rst containing the common things because depending on each
> platform the content will change.

Wait - the documentation should be uniform for all architectures.
If there are architecture specific aspects, then these should imo
still be mentioned in the common section of the doc, just pointing
out what those specifics are for which architectures. Architecture
specific doc pieces ought to cover architecture specific hypercalls
or sub-functions of common ones.

Jan
Julien Grall July 5, 2021, 2:20 p.m. UTC | #4
Hi Luca,

On 05/07/2021 11:51, 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
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> ---
> v7 changes:
> - commit message changed
> - Add comment about grant table queries and uses
> to the documentation
> v6 changes:
> - Fix misaligned comment
> - Moved comments to make them display in the docs
> - Included more documentation in the docs
> 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              | 459 +++++++++++-------
>   4 files changed, 288 insertions(+), 182 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..2f826c952d 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

`incontents is used by the script xen-headers to generate I believe [1].

Looking through the commit messages, I can't find any suggestion that 
the existing documentation has been retired or else. So can you clarify 
what's the intention?

If the plan to move to doxygen, then I think that
   1) the commit message or cover letter ought to explain why this is 
better than the current documentation
   2) you should remove xen-headers or outline the plan to do that. Note 
that after this series, I believe the bits for the grant table would end 
up to be broken.

Cheers,

[1] https://xenbits.xen.org/docs/unstable/hypercall/index.html
Luca Fancellu July 6, 2021, 8:38 a.m. UTC | #5
> On 5 Jul 2021, at 14:27, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 05.07.2021 15:23, Luca Fancellu wrote:
>> Hi Jan,
>> 
>>> On 5 Jul 2021, at 14:03, Jan Beulich <jbeulich@suse.com> wrote:
>>> 
>>> On 05.07.2021 12:51, 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
>>>> 
>>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>>>> ---
>>>> v7 changes:
>>>> - commit message changed
>>>> - Add comment about grant table queries and uses
>>>> to the documentation
>>>> v6 changes:
>>>> - Fix misaligned comment
>>>> - Moved comments to make them display in the docs
>>>> - Included more documentation in the docs
>>>> 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 +
>>> 
>>> In patch 8 you now add arm32.rst and x86.rst as well, so it's at
>>> least odd that here you alter only one of the three. However, ...
>>> 
>>>> --- a/docs/hypercall-interfaces/arm64.rst
>>>> +++ b/docs/hypercall-interfaces/arm64.rst
>>>> @@ -8,6 +8,7 @@ Starting points
>>>> .. toctree::
>>>>   :maxdepth: 2
>>>> 
>>>> +   common/grant_tables
>>> 
>>> ... to me this seems the wrong way round anyway: I'd rather see
>>> common stuff not be linked from per-arch locations, but per-arch
>>> docs to be down the hierarchy from common ones.
>> 
>> The things is that common stuff is not really common, if you go and solve every
>> define and so on, you end up with a different content for x86, aarch64, arm.
>> 
>> So you can’t have a common.rst containing the common things because depending on each
>> platform the content will change.
> 

Hi Jan,

> Wait - the documentation should be uniform for all architectures.
> If there are architecture specific aspects, then these should imo
> still be mentioned in the common section of the doc, just pointing
> out what those specifics are for which architectures. Architecture
> specific doc pieces ought to cover architecture specific hypercalls
> or sub-functions of common ones.
> 

Yes that could be a point, to be honest this patch was mainly used to collect as many feedbacks as possible
and it worked, maybe we can talk about that in the Xen call to see if we agree on a documentation structure.

For now this work was aimed to reproduce the actual documentation:

https://xenbits.xenproject.org/docs/unstable/hypercall/index.html

Cheers,

Luca

> Jan
>
Luca Fancellu July 6, 2021, 8:44 a.m. UTC | #6
> On 5 Jul 2021, at 15:20, Julien Grall <julien@xen.org> wrote:
> 
> Hi Luca,
> 
> On 05/07/2021 11:51, 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
>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>> ---
>> v7 changes:
>> - commit message changed
>> - Add comment about grant table queries and uses
>> to the documentation
>> v6 changes:
>> - Fix misaligned comment
>> - Moved comments to make them display in the docs
>> - Included more documentation in the docs
>> 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              | 459 +++++++++++-------
>>  4 files changed, 288 insertions(+), 182 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..2f826c952d 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
> 

Hi Julien,

> `incontents is used by the script xen-headers to generate I believe [1].
> 
> Looking through the commit messages, I can't find any suggestion that the existing documentation has been retired or else. So can you clarify what's the intention?
> 
> If the plan to move to doxygen, then I think that
>  1) the commit message or cover letter ought to explain why this is better than the current documentation
>  2) you should remove xen-headers or outline the plan to do that. Note that after this series, I believe the bits for the grant table would end up to be broken.
> 

Yes you are right, maybe it’s better to have a future serie that can reproduce and substitute the actual documentation.

All the patches below this one introduces the sphinx-doxygen system, so we are enabled to create the docs,
now we can agree on the structure that we want for the new documentation.

Cheers,

Luca 


> Cheers,
> 
> [1] https://xenbits.xen.org/docs/unstable/hypercall/index.html
> 
> -- 
> Julien Grall
Julien Grall July 7, 2021, 1:24 p.m. UTC | #7
Hi Luca,

On 06/07/2021 09:44, Luca Fancellu wrote:
> 
> 
>> On 5 Jul 2021, at 15:20, Julien Grall <julien@xen.org> wrote:
>>
>> Hi Luca,
>>
>> On 05/07/2021 11:51, 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
>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>>> ---
>>> v7 changes:
>>> - commit message changed
>>> - Add comment about grant table queries and uses
>>> to the documentation
>>> v6 changes:
>>> - Fix misaligned comment
>>> - Moved comments to make them display in the docs
>>> - Included more documentation in the docs
>>> 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              | 459 +++++++++++-------
>>>   4 files changed, 288 insertions(+), 182 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..2f826c952d 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
>>
> 
> Hi Julien,
> 
>> `incontents is used by the script xen-headers to generate I believe [1].
>>
>> Looking through the commit messages, I can't find any suggestion that the existing documentation has been retired or else. So can you clarify what's the intention?
>>
>> If the plan to move to doxygen, then I think that
>>   1) the commit message or cover letter ought to explain why this is better than the current documentation
>>   2) you should remove xen-headers or outline the plan to do that. Note that after this series, I believe the bits for the grant table would end up to be broken.
>>
> 
> Yes you are right, maybe it’s better to have a future serie that can reproduce and substitute the actual documentation.

I am actually not in favor of keeping two way to output the documentation.

What I am after is some clarification and plan so we don't end up to 
release 4.16 with half of the hypercalls described in doxygen and the 
other in the existing documentation.

Cheers,
diff mbox series

Patch

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..2f826c952d 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,62 @@ 
  *      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.
+ *
+ * GRANT TABLE QUERIES AND USES
+ *
+ * @code
+ * enum grant_table_op { // GNTTABOP_* => struct gnttab_*
+ *     GNTTABOP_map_grant_ref      = 0,
+ *     GNTTABOP_unmap_grant_ref    = 1,
+ *     GNTTABOP_setup_table        = 2,
+ *     GNTTABOP_dump_table         = 3,
+ *     GNTTABOP_transfer           = 4,
+ *     GNTTABOP_copy               = 5,
+ *     GNTTABOP_query_size         = 6,
+ *     GNTTABOP_unmap_and_replace  = 7,
+ * #if __XEN_INTERFACE_VERSION__ >= 0x0003020a
+ *     GNTTABOP_set_version        = 8,
+ *     GNTTABOP_get_status_frames  = 9,
+ *     GNTTABOP_get_version        = 10,
+ *     GNTTABOP_swap_grant_ref     = 11,
+ *     GNTTABOP_cache_flush        = 12
+ * #endif
+ * };
+ *
+ * enum neg_errnoval
+ * HYPERVISOR_grant_table_op(enum grant_table_op cmd,
+ *                           void *args,
+ *                           unsigned int count)
+ * @endcode
+ *
+ * \@args points to an array of a per-command data structure. The array
+ * has \@count members
+ *
+ * @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 +172,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 +195,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 +312,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 +333,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 +345,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 +361,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;
 
@@ -285,56 +369,41 @@  typedef uint16_t grant_status_t;
 
 #endif /* __XEN_INTERFACE_VERSION__ */
 
-/***********************************
- * GRANT TABLE QUERIES AND USES
- */
-
-/* ` enum neg_errnoval
- * ` HYPERVISOR_grant_table_op(enum grant_table_op cmd,
- * `                           void *args,
- * `                           unsigned int count)
- * `
- *
- * @args points to an array of a per-command data structure. The array
- * has @count members
- */
-
-/* ` enum grant_table_op { // GNTTABOP_* => struct gnttab_* */
-#define GNTTABOP_map_grant_ref        0
-#define GNTTABOP_unmap_grant_ref      1
-#define GNTTABOP_setup_table          2
-#define GNTTABOP_dump_table           3
-#define GNTTABOP_transfer             4
-#define GNTTABOP_copy                 5
-#define GNTTABOP_query_size           6
-#define GNTTABOP_unmap_and_replace    7
+#define GNTTABOP_map_grant_ref        0  /**< struct gnttab_map_grant_ref */
+#define GNTTABOP_unmap_grant_ref      1  /**< struct gnttab_unmap_grant_ref */
+#define GNTTABOP_setup_table          2  /**< struct gnttab_setup_table */
+#define GNTTABOP_dump_table           3  /**< struct gnttab_dump_table */
+#define GNTTABOP_transfer             4  /**< struct gnttab_transfer */
+#define GNTTABOP_copy                 5  /**< struct gnttab_copy */
+#define GNTTABOP_query_size           6  /**< struct gnttab_query_size */
+#define GNTTABOP_unmap_and_replace    7  /**< struct gnttab_unmap_and_replace */
 #if __XEN_INTERFACE_VERSION__ >= 0x0003020a
-#define GNTTABOP_set_version          8
-#define GNTTABOP_get_status_frames    9
-#define GNTTABOP_get_version          10
-#define GNTTABOP_swap_grant_ref	      11
-#define GNTTABOP_cache_flush	      12
+#define GNTTABOP_set_version          8  /**< struct gnttab_set_version */
+#define GNTTABOP_get_status_frames    9  /**< struct gnttab_get_status_frames */
+#define GNTTABOP_get_version          10 /**< struct gnttab_get_version */
+#define GNTTABOP_swap_grant_ref	      11 /**< struct gnttab_swap_grant_ref */
+#define GNTTABOP_cache_flush	      12 /**< struct gnttab_cache_flush */
 #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 +411,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 +440,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 +460,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 +470,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 +478,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 +498,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 +522,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 +533,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 +558,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 +581,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 +601,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 +639,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 +647,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 +661,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 +673,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 +734,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 +764,10 @@  DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t);
     "operation not done; try again"             \
 }
 
+/**
+ * @}
+ */
+
 #endif /* __XEN_PUBLIC_GRANT_TABLE_H__ */
 
 /*