| Message ID | 20210504133145.767-4-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Use Doxygen and sphinx for html documentation | expand |
On Tue, 4 May 2021, 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 for Arm64 Thanks Luca for your hard work on this. It is going to make things a lot better! I reviewed this patch while looking at https://luca.fancellu.gitlab.io/xen-docs/hypercall-interfaces/arm64/grant_tables.html In short, I think this changes look fine except for a trivial code style issue on the very last comment at the bottom of the patch. All my questions below are basically about some other in-code comments, currently existing in the code, but not outputted in the html file. Is there an easy way to add them? > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > --- > 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 | 8 +++ > docs/xen-doxygen/doxy_input.list | 1 + > xen/include/public/grant_table.h | 66 ++++++++++++------- > 4 files changed, 52 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..e1fb91dfc6 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; Just below this typedef there is the following comment: /* * 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. */ I noticed it doesn't appear in the output html. Any way we can retain it somewhere? Maybe we have to move it together with the larger comment above? > @@ -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; Also this comment is missing from the output: /* * 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. */ Is there a way to keep it? Similarly for the other subflags descriptions. > -/* > +/** > * 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); What about the comments for each members of the union? Basically I am trying to ask if we can output almost all comments currently living in this header. > -/* > +/** > * 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 { > @@ -468,6 +478,10 @@ struct gnttab_copy { > } source, dest; > uint16_t len; > 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) I think this is OK > /* OUT parameters. */ > int16_t status; > }; > @@ -579,7 +593,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 +602,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 +687,10 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t); > "operation not done; try again" \ > } > > +/** > + * @} > +*/ Alignment of the * > #endif /* __XEN_PUBLIC_GRANT_TABLE_H__ */ > > /* > -- > 2.17.1 >
> On 4 May 2021, at 23:27, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Tue, 4 May 2021, 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 for Arm64 > > Thanks Luca for your hard work on this. It is going to make things a lot > better! > > I reviewed this patch while looking at > https://luca.fancellu.gitlab.io/xen-docs/hypercall-interfaces/arm64/grant_tables.html > > In short, I think this changes look fine except for a trivial code style > issue on the very last comment at the bottom of the patch. > > All my questions below are basically about some other in-code comments, > currently existing in the code, but not outputted in the html file. > Is there an easy way to add them? > > > >> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> >> --- >> 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 | 8 +++ >> docs/xen-doxygen/doxy_input.list | 1 + >> xen/include/public/grant_table.h | 66 ++++++++++++------- >> 4 files changed, 52 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..e1fb91dfc6 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; > > Just below this typedef there is the following comment: > > /* > * 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. > */ > > I noticed it doesn't appear in the output html. Any way we can retain it > somewhere? Maybe we have to move it together with the larger comment > above? I agree with you, this comment should appear in the html docs, but to do so It has to be moved together with the larger comment above. In the original patch it was like that but I had to revert it back due to objection, my proposal is to put it together with the larger comment and write something like this to maintain a good readability: * 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. + * + * A grant table comprises a packed array of grant entries in one or more + * page frames shared between Xen and a guest. + * + * 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. * * @addtogroup grant_table Grant Tables * @{ If we all agree I will put it in the v6 > > >> @@ -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; > > > Also this comment is missing from the output: > > /* > * 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. > */ > > Is there a way to keep it? > > > Similarly for the other subflags descriptions. Here we could split the comment and do something like this: /**< GTF_invalid: This grant entry grants no privileges. */ #define GTF_invalid (0U<<0) /**< GTF_permit_access: Allow \@domid to map/access \@frame. */ #define GTF_permit_access (1U<<0) /** * GTF_accept_transfer: Allow \@domid to transfer * ownership of one page frame to this guest. Xen * writes the page number to \@frame. */ #define GTF_accept_transfer (2U<<0) /** * GTF_transitive: Allow @domid to transitively access a subrange of * \@trans_grant in \@trans_domid. No mappings are allowed. */ #define GTF_transitive (3U<<0) In this way each define will have its own description and I think both source code and html docs would be improved. > > >> -/* >> +/** >> * 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); > > What about the comments for each members of the union? Basically I am > trying to ask if we can output almost all comments currently living in > this header. Yes we can, every comment that is in place near the commented data structure needs just to have the /* changed to /** to be part of the html documentation. There are other cases as above where the comment could be split, I can do that in the v6, I need just to know if we agree on a solution like that so I don’t have to do work that I will trash later. > > >> -/* >> +/** >> * 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 { >> @@ -468,6 +478,10 @@ struct gnttab_copy { >> } source, dest; >> uint16_t len; >> 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) > > I think this is OK > > >> /* OUT parameters. */ >> int16_t status; >> }; >> @@ -579,7 +593,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 +602,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 +687,10 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t); >> "operation not done; try again" \ >> } >> >> +/** >> + * @} >> +*/ > > Alignment of the * Sure I’ll fix it in the v6 I’ll wait a couple of days for the reply, then I’ll assume everything above is agreed and I’ll push the v6. Cheers, Luca > > >> #endif /* __XEN_PUBLIC_GRANT_TABLE_H__ */ >> >> /* >> -- >> 2.17.1
On 06.05.2021 10:48, Luca Fancellu wrote: >> On 4 May 2021, at 23:27, Stefano Stabellini <sstabellini@kernel.org> wrote: >> On Tue, 4 May 2021, Luca Fancellu wrote: >>> @@ -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; >> >> Just below this typedef there is the following comment: >> >> /* >> * 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. >> */ >> >> I noticed it doesn't appear in the output html. Any way we can retain it >> somewhere? Maybe we have to move it together with the larger comment >> above? > > I agree with you, this comment should appear in the html docs, but to do so > It has to be moved together with the larger comment above. > > In the original patch it was like that but I had to revert it back due to objection, my proposal is > to put it together with the larger comment and write something like this to > maintain a good readability: > > * 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. > + * > + * A grant table comprises a packed array of grant entries in one or more > + * page frames shared between Xen and a guest. I think if this part was moved to the top of this big comment while ... > + * 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. ... this part was, as suggested by you, left near its bottom, I could agree. However, you making this suggestion caused me to look more closely at what the comments actually describe. If there's effort to make the documentation easier accessible by extracting it from the header, I wonder whether - like with the v1 vs v2 comment pointed out previously as misleading - we shouldn't, as a prereq step, make an attempt to actually have the documentation be correct. For example I found this: /* * Version 1 and version 2 grant entries share a common prefix. The * fields of the prefix are documented as part of struct * grant_entry_v1. */ struct grant_entry_header { uint16_t flags; domid_t domid; }; The comment is wrong. "flags" here is only holding what's tagged [GST] for v1. The [XEN] tagged bits actually live in grant_status_t. This can perhaps best be seen in gnttab_set_version()'s code dealing with the first 8 entries. However, contrary to v2's intentions, GTF_transfer_committed and GTF_transfer_completed (which aren't properly tagged either way) get set by Xen in shared entries, not status ones. Maybe this was considered "okay" because the frame field also gets written in this case (i.e. the cache line will get dirtied in any event). Similarly I'd like to refer to my still pending "gnttab: GTF_sub_page is a v2-only flag", which also corrects documentation in this regard. And perhaps there's more. An alternative to correcting the (as it seems) v2 related issues, it may be worth considering to extract only v1 documentation in this initial phase. Jan
> On 6 May 2021, at 10:58, Jan Beulich <jbeulich@suse.com> wrote: > > On 06.05.2021 10:48, Luca Fancellu wrote: >>> On 4 May 2021, at 23:27, Stefano Stabellini <sstabellini@kernel.org> wrote: >>> On Tue, 4 May 2021, Luca Fancellu wrote: >>>> @@ -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; >>> >>> Just below this typedef there is the following comment: >>> >>> /* >>> * 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. >>> */ >>> >>> I noticed it doesn't appear in the output html. Any way we can retain it >>> somewhere? Maybe we have to move it together with the larger comment >>> above? >> >> I agree with you, this comment should appear in the html docs, but to do so >> It has to be moved together with the larger comment above. >> >> In the original patch it was like that but I had to revert it back due to objection, my proposal is >> to put it together with the larger comment and write something like this to >> maintain a good readability: >> >> * 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. >> + * >> + * A grant table comprises a packed array of grant entries in one or more >> + * page frames shared between Xen and a guest. > > I think if this part was moved to the top of this big comment while ... > >> + * 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. > > ... this part was, as suggested by you, left near its bottom, I could > agree. Hi Jan, Just to be sure that we are on the same page, something like this could be ok? * fully virtualised memory. * * GRANT TABLE REPRESENTATION * + * 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 […] * 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. * * @addtogroup grant_table Grant Tables * @{ > > However, you making this suggestion caused me to look more closely at > what the comments actually describe. If there's effort to make the > documentation easier accessible by extracting it from the header, I > wonder whether - like with the v1 vs v2 comment pointed out previously > as misleading - we shouldn't, as a prereq step, make an attempt to > actually have the documentation be correct. For example I found this: > > /* > * Version 1 and version 2 grant entries share a common prefix. The > * fields of the prefix are documented as part of struct > * grant_entry_v1. > */ > struct grant_entry_header { > uint16_t flags; > domid_t domid; > }; > > The comment is wrong. "flags" here is only holding what's tagged > [GST] for v1. The [XEN] tagged bits actually live in grant_status_t. > This can perhaps best be seen in gnttab_set_version()'s code > dealing with the first 8 entries. However, contrary to v2's > intentions, GTF_transfer_committed and GTF_transfer_completed (which > aren't properly tagged either way) get set by Xen in shared entries, > not status ones. Maybe this was considered "okay" because the frame > field also gets written in this case (i.e. the cache line will get > dirtied in any event). > > Similarly I'd like to refer to my still pending "gnttab: GTF_sub_page > is a v2-only flag", which also corrects documentation in this regard. > And perhaps there's more. > > An alternative to correcting the (as it seems) v2 related issues, it > may be worth considering to extract only v1 documentation in this > initial phase. > > Jan >
On 06.05.2021 16:43, Luca Fancellu wrote: >> On 6 May 2021, at 10:58, Jan Beulich <jbeulich@suse.com> wrote: >> On 06.05.2021 10:48, Luca Fancellu wrote: >>>> On 4 May 2021, at 23:27, Stefano Stabellini <sstabellini@kernel.org> wrote: >>>> On Tue, 4 May 2021, Luca Fancellu wrote: >>>>> @@ -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; >>>> >>>> Just below this typedef there is the following comment: >>>> >>>> /* >>>> * 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. >>>> */ >>>> >>>> I noticed it doesn't appear in the output html. Any way we can retain it >>>> somewhere? Maybe we have to move it together with the larger comment >>>> above? >>> >>> I agree with you, this comment should appear in the html docs, but to do so >>> It has to be moved together with the larger comment above. >>> >>> In the original patch it was like that but I had to revert it back due to objection, my proposal is >>> to put it together with the larger comment and write something like this to >>> maintain a good readability: >>> >>> * 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. >>> + * >>> + * A grant table comprises a packed array of grant entries in one or more >>> + * page frames shared between Xen and a guest. >> >> I think if this part was moved to the top of this big comment while ... >> >>> + * 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. >> >> ... this part was, as suggested by you, left near its bottom, I could >> agree. > > Hi Jan, > > Just to be sure that we are on the same page, something like this could be ok? > > * fully virtualised memory. > * > * GRANT TABLE REPRESENTATION > * > + * 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 > […] > * 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. > * > * @addtogroup grant_table Grant Tables > * @{ I think so, yes. Thanks. Jan
On Thu, 6 May 2021, Jan Beulich wrote: > An alternative to correcting the (as it seems) v2 related issues, it > may be worth considering to extract only v1 documentation in this > initial phase. FWIW I agree with Jan that "grant table v1" documentation only is a good idea.
> On 11 May 2021, at 22:58, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Thu, 6 May 2021, Jan Beulich wrote: >> An alternative to correcting the (as it seems) v2 related issues, it >> may be worth considering to extract only v1 documentation in this >> initial phase. > > FWIW I agree with Jan that "grant table v1" documentation only is a good idea. Ok, I already pushed the v6: https://patchwork.kernel.org/project/xen-devel/cover/20210510084105.17108-1-luca.fancellu@arm.com/
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..e1fb91dfc6 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 { @@ -468,6 +478,10 @@ struct gnttab_copy { } source, dest; uint16_t len; 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; }; @@ -579,7 +593,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 +602,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 +687,10 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_cache_flush_t); "operation not done; try again" \ } +/** + * @} +*/ + #endif /* __XEN_PUBLIC_GRANT_TABLE_H__ */ /*
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> --- 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 | 8 +++ docs/xen-doxygen/doxy_input.list | 1 + xen/include/public/grant_table.h | 66 ++++++++++++------- 4 files changed, 52 insertions(+), 24 deletions(-) create mode 100644 docs/hypercall-interfaces/common/grant_tables.rst