xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Luca Fancellu <luca.fancellu@arm.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>,
	wei.chen@arm.com, Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Ian Jackson <iwj@xenproject.org>, Julien Grall <julien@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v2 3/3] docs/doxygen: doxygen documentation for grant_table.h
Date: Thu, 22 Apr 2021 08:39:20 +0100	[thread overview]
Message-ID: <91B2399E-2E91-44A9-A5F2-C55F42F52F52@arm.com> (raw)
In-Reply-To: <dbcf10ef-0d40-a687-d242-f01a01bca342@suse.com>



> On 20 Apr 2021, at 11:27, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 20.04.2021 11:42, Luca Fancellu wrote:
>>> On 20 Apr 2021, at 10:14, Jan Beulich <jbeulich@suse.com> wrote:
>>> On 20.04.2021 10:46, Luca Fancellu wrote:
>>>>> On 19 Apr 2021, at 12:05, Jan Beulich <jbeulich@suse.com> wrote:
>>>>> On 19.04.2021 11:12, Luca Fancellu wrote:
>>>>>> - */
>>>>>> -
>>>>>> -/*
>>>>>> - * Reference to a grant entry in a specified domain's grant table.
>>>>>> - */
>>>>>> -typedef uint32_t grant_ref_t;
>>>>> 
>>>>> Why does this get moved ...
>>>>> 
>>>>>> -
>>>>>> -/*
>>>>>> + *
>>>>>> * 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.
>>>>>> + *
>>>>>> + * @addtogroup grant_table Grant Tables
>>>>>> + * @{
>>>>>> */
>>>>>> 
>>>>>> -/*
>>>>>> - * Version 1 of the grant table entry structure is maintained purely
>>>>>> - * for backwards compatibility.  New guests should use version 2.
>>>>>> +/**
>>>>>> + * Reference to a grant entry in a specified domain's grant table.
>>>>>> */
>>>>>> +typedef uint32_t grant_ref_t;
>>>>> 
>>>>> ... here, past a comment unrelated to it?
>>>> 
>>>> The comment “Version 1 of the grant table entry […]” was moved on top of the struct grant_entry_v1, in this way
>>>> Doxygen associate the comment to that structure.
>>>> 
>>>> The comment “Reference to a grant entry in a specified domain's grant table.” Is moved on top of typedef uint32_t grant_ref_t
>>>> for the same reason above
>>> 
>>> But it's the other comment ("A grant table comprises ...") that I
>>> was asking about.
>> 
>> I thought it was part of the brief about grant tables, am I wrong? For that reason I moved it.
> 
> Well, the comment talks about grant table entries (the layout of which
> gets defined immediately below, as visible in the original patch), not
> grant references.

Hi Jan,

Of course this can be reverted back if it is wrong. 

I’ve prepared a page with the output of all my commits (some of them are not yet in the ML),
so anyone can have a look on what is the output and how can sphinx+doxygen improve
the quality of the documentation.

Here: 

https://luca.fancellu.gitlab.io/xen-docs/hypercall-interfaces/arm64.html

the document Is trying to reproduce this other one from the current docs:

https://xenbits.xen.org/docs/unstable/hypercall/arm/index.html

You will receive a warning from the browser because the gitlab certificate has problems when
the username has a dot in it.

> 
>>>>>> @@ -243,23 +258,27 @@ union grant_entry_v2 {
>>>>>>    * In that case, the frame field has the same semantics as the
>>>>>>    * field of the same name in the V1 entry structure.
>>>>>>    */
>>>>>> +    /** @cond skip anonymous struct/union for doxygen */
>>>>>>   struct {
>>>>>>       grant_entry_header_t hdr;
>>>>>>       uint32_t pad0;
>>>>>>       uint64_t frame;
>>>>>>   } full_page;
>>>>>> +    /** @endcond */
>>>>>> 
>>>>>>   /*
>>>>>>    * 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.
>>>>>>    */
>>>>>> +    /** @cond skip anonymous struct/union for doxygen */
>>>>>>   struct {
>>>>>>       grant_entry_header_t hdr;
>>>>>>       uint16_t page_off;
>>>>>>       uint16_t length;
>>>>>>       uint64_t frame;
>>>>>>   } sub_page;
>>>>>> +    /** @endcond */
>>>>>> 
>>>>>>   /*
>>>>>>    * If the grant is GTF_transitive, @domid is allowed to use the
>>>>>> @@ -270,12 +289,14 @@ union grant_entry_v2 {
>>>>>>    * The current version of Xen does not allow transitive grants
>>>>>>    * to be mapped.
>>>>>>    */
>>>>>> +    /** @cond skip anonymous struct/union for doxygen */
>>>>>>   struct {
>>>>>>       grant_entry_header_t hdr;
>>>>>>       domid_t trans_domid;
>>>>>>       uint16_t pad0;
>>>>>>       grant_ref_t gref;
>>>>>>   } transitive;
>>>>>> +    /** @endcond */
>>>>> 
>>>>> While already better than the introduction of strange struct tags,
>>>>> I'm still not convinced we want this extra clutter (sorry). Plus -
>>>>> don't these additions mean the sub-structures then won't be
>>>>> represented in the generated doc, rendering it (partly) useless?
>>>> 
>>>> Are you suggesting to remove the structure from docs?
>>> 
>>> Just yet I'm not suggesting anything here - I was merely guessing at
>>> the effect of "@cond" and the associated "skip ..." to mean that this
>>> construct makes doxygen skip the enclosed section. If that's not the
>>> effect, then maybe the comment wants rewording? (And then - what does
>>> @cond mean?)
>> 
>> Yes you were right, in the documentation the structure grant_entry_v2 won’t display the
>> anonymous union.
> 
> In which case I'm now completely unclear about your prior question.

We have to choose just if the struct is useful even if it’s partially documented or if
it’s better to hide it completely from the docs since it can’t be fully documented.

Cheers,
Luca

> 
> Jan



  reply	other threads:[~2021-04-22  7:40 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-19  9:12 [PATCH v2 0/3] Use Doxygen and sphinx for html documentation Luca Fancellu
2021-04-19  9:12 ` [PATCH v2 1/3] docs: add doxygen support " Luca Fancellu
2021-04-19  9:12 ` [PATCH v2 2/3] docs: hypercalls sphinx skeleton for generated html Luca Fancellu
2021-04-19  9:12 ` [PATCH v2 3/3] docs/doxygen: doxygen documentation for grant_table.h Luca Fancellu
2021-04-19 11:05   ` Jan Beulich
2021-04-20  8:46     ` Luca Fancellu
2021-04-20  9:14       ` Jan Beulich
2021-04-20  9:42         ` Luca Fancellu
2021-04-20 10:27           ` Jan Beulich
2021-04-22  7:39             ` Luca Fancellu [this message]
2021-04-22  8:06               ` Jan Beulich
2021-04-26 15:40                 ` Luca Fancellu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=91B2399E-2E91-44A9-A5F2-C55F42F52F52@arm.com \
    --to=luca.fancellu@arm.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=bertrand.marquis@arm.com \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=sstabellini@kernel.org \
    --cc=wei.chen@arm.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH v2 3/3] docs/doxygen: doxygen documentation for grant_table.h' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).