|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 08/14] kernel-doc: public/memory.h
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:
>>> From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
>>>
>>> Convert in-code comments to kernel-doc format wherever possible.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
>>> ---
>>> xen/include/public/memory.h | 232 ++++++++++++++++++++++++------------
>>> 1 file changed, 155 insertions(+), 77 deletions(-)
>>>
>>> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
>>> index 21057ed78e..4c57ed213c 100644
>>> --- a/xen/include/public/memory.h
>>> +++ b/xen/include/public/memory.h
>>> @@ -30,7 +30,9 @@
>>> #include "xen.h"
>>> #include "physdev.h"
>>>
>>> -/*
>>> +/**
>>> + * DOC: XENMEM_increase_reservation and XENMEM_decrease_reservation
>>> + *
>>> * Increase or decrease the specified domain's memory reservation. Returns
>>> the
>>> * number of extents successfully allocated or freed.
>>> * arg == addr of struct xen_memory_reservation.
>>> @@ -40,29 +42,37 @@
>>> #define XENMEM_populate_physmap 6
>>>
>>> #if __XEN_INTERFACE_VERSION__ >= 0x00030209
>>> -/*
>>> - * Maximum # bits addressable by the user of the allocated region (e.g.,
>>> I/O
>>> - * devices often have a 32-bit limitation even in 64-bit systems). If zero
>>> - * then the user has no addressing restriction. This field is not used by
>>> - * XENMEM_decrease_reservation.
>>> +/**
>>> + * DOC: XENMEMF_*
>>> + *
>>> + * - XENMEMF_address_bits, XENMEMF_get_address_bits:
>>> + * Maximum # bits addressable by the user of the allocated region
>>> + * (e.g., I/O devices often have a 32-bit limitation even in 64-bit
>>> + * systems). If zero then the user has no addressing restriction.
>>> This
>>> + * field is not used by XENMEM_decrease_reservation.
>>> + * - XENMEMF_node, XENMEMF_get_node: NUMA node to allocate from
>>> + * - XENMEMF_populate_on_demand: Flag to populate physmap with
>>> populate-on-demand entries
>>> + * - XENMEMF_exact_node_request, XENMEMF_exact_node: Flag to request
>>> allocation only from the node specified
>>
>> Nit: overly long line
>
> I'll fix
>
>
>>> + * - XENMEMF_vnode: Flag to indicate the node specified is virtual node
>>> */
>>> #define XENMEMF_address_bits(x) (x)
>>> #define XENMEMF_get_address_bits(x) ((x) & 0xffu)
>>> -/* NUMA node to allocate from. */
>>> #define XENMEMF_node(x) (((x) + 1) << 8)
>>> #define XENMEMF_get_node(x) ((((x) >> 8) - 1) & 0xffu)
>>> -/* Flag to populate physmap with populate-on-demand entries */
>>> #define XENMEMF_populate_on_demand (1<<16)
>>> -/* Flag to request allocation only from the node specified */
>>> #define XENMEMF_exact_node_request (1<<17)
>>> #define XENMEMF_exact_node(n) (XENMEMF_node(n) |
>>> XENMEMF_exact_node_request)
>>> -/* Flag to indicate the node specified is virtual node */
>>> #define XENMEMF_vnode (1<<18)
>>> #endif
>>>
>>> +/**
>>> + * struct xen_memory_reservation
>>> + */
>>> struct xen_memory_reservation {
>>>
>>> - /*
>>> + /**
>>> + * @extent_start:
>>> + *
>>
>> Take the opportunity and drop the stray blank line?
>
> Sure
>
>
>>> @@ -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).
Jan
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |