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

[Julien Grall <julien@xen.org>]

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,

-- 
Julien Grall