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

[Julien Grall <julien@xxxxxxx>]

Hi Luca,

On 06/07/2021 09:44, Luca Fancellu wrote:
> > On 5 Jul 2021, at 15:20, Julien Grall <julien@xxxxxxx> 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@xxxxxxx>
> > > ---
> > > 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