On 02.07.2024 15:03, Alejandro Vallejo wrote:
> --- a/xen/include/xen/vmap.h
> +++ b/xen/include/xen/vmap.h
> @@ -1,34 +1,131 @@
> +/*
> + * Interface to map physical memory onto contiguous virtual memory areas.
> + *
> + * Two ranges of linear address space are reserved for this purpose: A
> general
> + * purpose area (VMAP_DEFAULT) and a livepatch-specific area (VMAP_XEN). The
> + * latter is used when loading livepatches and the former for everything
> else.
> + */
> #if !defined(__XEN_VMAP_H__) && defined(VMAP_VIRT_START)
> #define __XEN_VMAP_H__
>
> #include <xen/mm-frame.h>
> #include <xen/page-size.h>
>
> +/** Identifiers for the linear ranges tracked by vmap */
Here and below: Why /** ? Seeing ...
> enum vmap_region {
> + /*
> + * Region used for general purpose RW mappings. Mapping/allocating memory
> + * here can induce extra allocations for the supporting page tables.
> + */
... this vs ...
> VMAP_DEFAULT,
> + /**
> + * Region used for loading livepatches. Can't use VMAP_DEFAULT because it
> + * must live close to the running Xen image. The caller also ensures all
> + * page tables are already in place with adequate PTE flags.
> + */
... this, it's not even looking to be consistent.
> VMAP_XEN,
> + /** Sentinel value for bounds checking */
> VMAP_REGION_NR,
> };
>
> +/**
> + * Runtime initialiser for each vmap region type
> + *
> + * Must only be called once per vmap region type.
> + *
> + * @param type Designation of the region to initialise.
> + * @param start Start address of the `type` region.
> + * @param end End address (not inclusive) of the `type` region
> + */
> void vm_init_type(enum vmap_region type, void *start, void *end);
>
> +/**
> + * Maps a range of physical ranges onto a single virtual range
Largely related to it not really being clear what "a range of physical ranges"
actually is, maybe "a set of physical ranges"?
> + * `mfn` is an array of `nr` physical ranges, each of which is `granularity`
> + * pages wide. `type` defines which vmap region to use for the mapping and
> + * `flags` is the PTE flags the page table leaves are meant to have.
> + *
> + * Typically used via the vmap() and vmap_contig() helpers.
> + *
> + * @param mfn Array of mfns
> + * @param granularity Number of contiguous pages each mfn represents
> + * @param nr Number of mfns in the `mfn` array
> + * @param align Alignment of the virtual area to map
> + * @param flags PTE flags for the leaves of the PT tree.
> + * @param type Which region to create the mappings on
> + */
> void *__vmap(const mfn_t *mfn, unsigned int granularity, unsigned int nr,
> unsigned int align, unsigned int flags, enum vmap_region type);
> +
> +/**
> + * Map an arrray of pages contiguously into the VMAP_DEFAULT vmap region
Nit: One r too many in "array".
> + * @param[in] mfn Pointer to the base of an array of mfns
> + * @param[in] nr Number of mfns in the array
> + */
> void *vmap(const mfn_t *mfn, unsigned int nr);
> +
> +/**
> + * Maps physically contiguous pages onto the VMAP_DEFAULT vmap region
> + *
> + * Used when the directmap is unavailable (i.e: due to secret hiding)
Please omit this. There's no reason to suggest it can't also be used for
other purposes.
> + * @param mfn Base mfn of the physical region
> + * @param nr Number of mfns in the physical region
> + */
> void *vmap_contig(mfn_t mfn, unsigned int nr);
> +
> +/**
> + * Unmaps a range of virtually contiguous memory from one of the vmap regions
> + *
> + * The system remembers internally how wide the mapping is and unmaps it all.
> + * It also can determine the vmap region type from the `va`.
> + *
> + * @param va Virtual base address of the range to unmap
> + */
> void vunmap(const void *va);
>
> +/**
> + * Allocate `size` octets of possibly non-contiguous physical memory and map
> + * them contiguously in the VMAP_DEFAULT vmap region
> + *
> + * The system remembers internally how wide the mapping is and unmaps it all.
This is a verbatim copy of what you say for vunmap(), yet it looks unrelated
here. Maybe you meant to edit it after copy-and-paste? Or maybe this was
meant to go ...
> + * @param size Pointer to the base of an array of mfns
> + * @return Pointer to the mapped area on success; NULL otherwise.
> + */
> void *vmalloc(size_t size);
> +
> +/** Same as vmalloc(), but for the VMAP_XEN vmap region. */
> void *vmalloc_xen(size_t size);
>
> +/** Same as vmalloc(), but set the contents to zero before returning */
> void *vzalloc(size_t size);
> +
> +/**
> + * Unmap and free memory from vmalloc(), vmalloc_xen() or vzalloc()
> + *
> + * @param va Virtual base address of the range to free and unmap
> + */
> void vfree(void *va);
... here?
Also, if you want this to be considered for 4.19, please don't forget to Cc
Oleksii.
Jan
