[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 08/14] kernel-doc: public/memory.h
On 18.08.2020 00:56, Stefano Stabellini wrote: > On Mon, 17 Aug 2020, Jan Beulich wrote: >> On 07.08.2020 23:51, Stefano Stabellini wrote: >>> On Fri, 7 Aug 2020, Jan Beulich wrote: >>>> On 07.08.2020 01:49, Stefano Stabellini wrote: >>>>> @@ -200,90 +236,115 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mfn_list_t); >>>>> */ >>>>> #define XENMEM_machphys_compat_mfn_list 25 >>>>> >>>>> -/* >>>>> +#define XENMEM_machphys_mapping 12 >>>>> +/** >>>>> + * struct xen_machphys_mapping - XENMEM_machphys_mapping >>>>> + * >>>>> * Returns the location in virtual address space of the machine_to_phys >>>>> * mapping table. Architectures which do not have a m2p table, or which >>>>> do not >>>>> * map it by default into guest address space, do not implement this >>>>> command. >>>>> * arg == addr of xen_machphys_mapping_t. >>>>> */ >>>>> -#define XENMEM_machphys_mapping 12 >>>>> struct xen_machphys_mapping { >>>>> + /** @v_start: Start virtual address */ >>>>> xen_ulong_t v_start, v_end; /* Start and end virtual addresses. */ >>>>> - xen_ulong_t max_mfn; /* Maximum MFN that can be looked up. */ >>>>> + /** @v_end: End virtual addresses */ >>>>> + xen_ulong_t v_end; >>>>> + /** @max_mfn: Maximum MFN that can be looked up */ >>>>> + xen_ulong_t max_mfn; >>>>> }; >>>>> typedef struct xen_machphys_mapping xen_machphys_mapping_t; >>>>> DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t); >>>>> >>>>> -/* Source mapping space. */ >>>>> +/** >>>>> + * DOC: Source mapping space. >>>>> + * >>>>> + * - XENMAPSPACE_shared_info: shared info page >>>>> + * - XENMAPSPACE_grant_table: grant table page >>>>> + * - XENMAPSPACE_gmfn: GMFN >>>>> + * - XENMAPSPACE_gmfn_range: GMFN range, XENMEM_add_to_physmap only. >>>>> + * - XENMAPSPACE_gmfn_foreign: GMFN from another dom, >>>>> + * XENMEM_add_to_physmap_batch only. >>>>> + * - XENMAPSPACE_dev_mmio: device mmio region ARM only; the region >>>>> is mapped >>>>> + * in Stage-2 using the Normal >>>>> MemoryInner/Outer >>>>> + * Write-Back Cacheable memory attribute. >>>>> + */ >>>>> /* ` enum phys_map_space { */ >>>> >>>> Isn't this and ... >>>> >>>>> -#define XENMAPSPACE_shared_info 0 /* shared info page */ >>>>> -#define XENMAPSPACE_grant_table 1 /* grant table page */ >>>>> -#define XENMAPSPACE_gmfn 2 /* GMFN */ >>>>> -#define XENMAPSPACE_gmfn_range 3 /* GMFN range, XENMEM_add_to_physmap >>>>> only. */ >>>>> -#define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom, >>>>> - * XENMEM_add_to_physmap_batch only. >>>>> */ >>>>> -#define XENMAPSPACE_dev_mmio 5 /* device mmio region >>>>> - ARM only; the region is mapped in >>>>> - Stage-2 using the Normal Memory >>>>> - Inner/Outer Write-Back Cacheable >>>>> - memory attribute. */ >>>>> +#define XENMAPSPACE_shared_info 0 >>>>> +#define XENMAPSPACE_grant_table 1 >>>>> +#define XENMAPSPACE_gmfn 2 >>>>> +#define XENMAPSPACE_gmfn_range 3 >>>>> +#define XENMAPSPACE_gmfn_foreign 4 >>>>> +#define XENMAPSPACE_dev_mmio 5 >>>>> /* ` } */ >>>> >>>> ... this also something that wants converting? >>> >>> For clarity, I take you are talking about these two enum-related >>> comments: >>> >>> /* ` enum phys_map_space { */ >>> [... various #defines ... ] >>> /* ` } */ >>> >>> Is this something we want to convert to kernel-doc? I don't know. I >>> couldn't see an obvious value in doing it, in the sense that it doesn't >>> necessarely make things clearer. >>> >>> I took a second look at the header and the following would work: >>> >>> /** >>> * DOC: Source mapping space. >>> * >>> * enum phys_map_space { >>> * >>> * - XENMAPSPACE_shared_info: shared info page >>> * - XENMAPSPACE_grant_table: grant table page >>> * - XENMAPSPACE_gmfn: GMFN >>> * - XENMAPSPACE_gmfn_range: GMFN range, XENMEM_add_to_physmap only. >>> * - XENMAPSPACE_gmfn_foreign: GMFN from another dom, >>> * XENMEM_add_to_physmap_batch only. >>> * - XENMAPSPACE_dev_mmio: device mmio region ARM only; the region is >>> mapped >>> * in Stage-2 using the Normal MemoryInner/Outer >>> * Write-Back Cacheable memory attribute. >>> * } >>> */ >>> >>> Note the blank line after "enum phys_map_space {" is required. >>> >>> >>> All in all I am in favor of *not* converting the enum comment to >>> kernel-doc, but I'd be OK with it anyway. >> >> Iirc the enum comments were added for documentation purposes. This to >> me means there are two options at this point: >> - retain them in a way that the new doc model consumes them, >> - drop them at the same time as adding the new doc comments. >> >> Their (presumed) value is that they identify #define-s which supposed >> to be enum-like without actually being able to use enums in the public >> headers (with some exceptions). > > I understand. Then, it doesn't look like we want to keep them in the code > without converting them to kernel-doc. We could either: > > 1) remove them as part of this series > 2) convert them to kernel-doc in the top comment as shown above > > I could do either, but my preference is 1) because I think it leads to > clearer docs. While I'd slightly prefer 2, I'll be okay with your choice. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |