Update the description of of efi_relocate_kernel() to match Sphinx style.
Update parameter references in the description of other memory functions
to use @param style.
Signed-off-by: Heinrich Schuchardt <[email protected]>
---
drivers/firmware/efi/libstub/mem.c | 38 +++++++++++++++++++-----------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
index 0d57078e5e62..7efe3ed2d5a6 100644
--- a/drivers/firmware/efi/libstub/mem.c
+++ b/drivers/firmware/efi/libstub/mem.c
@@ -86,7 +86,7 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
*
* Allocate pages as EFI_LOADER_DATA. The allocated pages are aligned according
* to EFI_ALLOC_ALIGN. The last allocated page will not exceed the address
- * given by 'max'.
+ * given by @max.
*
* Return: status code
*/
@@ -126,10 +126,10 @@ efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
* @addr: on exit the address of the allocated memory
* @min: minimum address to used for the memory allocation
*
- * Allocate at the lowest possible address that is not below 'min' as
- * EFI_LOADER_DATA. The allocated pages are aligned according to 'align' but at
+ * Allocate at the lowest possible address that is not below @min as
+ * EFI_LOADER_DATA. The allocated pages are aligned according to @align but at
* least EFI_ALLOC_ALIGN. The first allocated page will not below the address
- * given by 'min'.
+ * given by @min.
*
* Return: status code
*/
@@ -214,7 +214,7 @@ efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
* @addr: start of the memory area to free (must be EFI_PAGE_SIZE
* aligned)
*
- * 'size' is rounded up to a multiple of EFI_ALLOC_ALIGN which is an
+ * @size is rounded up to a multiple of EFI_ALLOC_ALIGN which is an
* architecture specific multiple of EFI_PAGE_SIZE. So this function should
* only be used to return pages allocated with efi_allocate_pages() or
* efi_low_alloc_above().
@@ -230,15 +230,25 @@ void efi_free(unsigned long size, unsigned long addr)
efi_bs_call(free_pages, addr, nr_pages);
}
-/*
- * Relocate a kernel image, either compressed or uncompressed.
- * In the ARM64 case, all kernel images are currently
- * uncompressed, and as such when we relocate it we need to
- * allocate additional space for the BSS segment. Any low
- * memory that this function should avoid needs to be
- * unavailable in the EFI memory map, as if the preferred
- * address is not available the lowest available address will
- * be used.
+/**
+ * efi_relocate_kernel() - copy memory area
+ * @image_addr: address of memory area to copy, on exit target address
+ * @image_size: size of memory area to copy
+ * @alloc_size: minimum size of memory to allocate, must be greater or
+ * equal to image_size
+ * @preferred_addr: preferred target address
+ * @alignment: minimum alignment of the allocated memory area. It
+ * should be a power of two.
+ * @min_addr: minimum target address
+ *
+ * Copy a memory area to a newly allocated memory area aligned according
+ * to @alignment but at least EFI_ALLOC_ALIGN. If the preferred address
+ * is not available, the allocated address will not be below @min_addr.
+ *
+ * This function is used to copy the Linux kernel verbatim. It does not apply
+ * any relocation changes.
+ *
+ * Return: status code
*/
efi_status_t efi_relocate_kernel(unsigned long *image_addr,
unsigned long image_size,
--
2.25.0
Hi,
Mostly looks good. One comment below:
On 2/19/20 10:53 PM, Heinrich Schuchardt wrote:
> Update the description of of efi_relocate_kernel() to match Sphinx style.
>
> Update parameter references in the description of other memory functions
> to use @param style.
>
> Signed-off-by: Heinrich Schuchardt <[email protected]>
> ---
> drivers/firmware/efi/libstub/mem.c | 38 +++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
> index 0d57078e5e62..7efe3ed2d5a6 100644
> --- a/drivers/firmware/efi/libstub/mem.c
> +++ b/drivers/firmware/efi/libstub/mem.c
> @@ -86,7 +86,7 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
> *
> * Allocate pages as EFI_LOADER_DATA. The allocated pages are aligned according
> * to EFI_ALLOC_ALIGN. The last allocated page will not exceed the address
> - * given by 'max'.
> + * given by @max.
> *
> * Return: status code
> */
> @@ -126,10 +126,10 @@ efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
> * @addr: on exit the address of the allocated memory
> * @min: minimum address to used for the memory allocation
> *
> - * Allocate at the lowest possible address that is not below 'min' as
> - * EFI_LOADER_DATA. The allocated pages are aligned according to 'align' but at
> + * Allocate at the lowest possible address that is not below @min as
> + * EFI_LOADER_DATA. The allocated pages are aligned according to @align but at
> * least EFI_ALLOC_ALIGN. The first allocated page will not below the address
> - * given by 'min'.
> + * given by @min.
> *
> * Return: status code
> */
> @@ -214,7 +214,7 @@ efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
> * @addr: start of the memory area to free (must be EFI_PAGE_SIZE
> * aligned)
> *
> - * 'size' is rounded up to a multiple of EFI_ALLOC_ALIGN which is an
> + * @size is rounded up to a multiple of EFI_ALLOC_ALIGN which is an
> * architecture specific multiple of EFI_PAGE_SIZE. So this function should
> * only be used to return pages allocated with efi_allocate_pages() or
> * efi_low_alloc_above().
> @@ -230,15 +230,25 @@ void efi_free(unsigned long size, unsigned long addr)
> efi_bs_call(free_pages, addr, nr_pages);
> }
>
> -/*
> - * Relocate a kernel image, either compressed or uncompressed.
> - * In the ARM64 case, all kernel images are currently
> - * uncompressed, and as such when we relocate it we need to
> - * allocate additional space for the BSS segment. Any low
> - * memory that this function should avoid needs to be
> - * unavailable in the EFI memory map, as if the preferred
> - * address is not available the lowest available address will
> - * be used.
> +/**
> + * efi_relocate_kernel() - copy memory area
> + * @image_addr: address of memory area to copy, on exit target address
The "on exit target address" is a little bit confusing IMO.
Is it like this?
On exit, @image_addr is updated to the target copy address that was used.
? or some other better description?
Thanks.
Acked-by: Randy Dunlap <[email protected]>
> + * @image_size: size of memory area to copy
> + * @alloc_size: minimum size of memory to allocate, must be greater or
> + * equal to image_size
> + * @preferred_addr: preferred target address
> + * @alignment: minimum alignment of the allocated memory area. It
> + * should be a power of two.
> + * @min_addr: minimum target address
> + *
> + * Copy a memory area to a newly allocated memory area aligned according
> + * to @alignment but at least EFI_ALLOC_ALIGN. If the preferred address
> + * is not available, the allocated address will not be below @min_addr.
> + *
> + * This function is used to copy the Linux kernel verbatim. It does not apply
> + * any relocation changes.
> + *
> + * Return: status code
> */
> efi_status_t efi_relocate_kernel(unsigned long *image_addr,
> unsigned long image_size,
> --
> 2.25.0
>
--
~Randy
On Thu, 20 Feb 2020 at 08:02, Randy Dunlap <[email protected]> wrote:
>
> Hi,
> Mostly looks good. One comment below:
>
> On 2/19/20 10:53 PM, Heinrich Schuchardt wrote:
> > Update the description of of efi_relocate_kernel() to match Sphinx style.
> >
> > Update parameter references in the description of other memory functions
> > to use @param style.
> >
> > Signed-off-by: Heinrich Schuchardt <[email protected]>
> > ---
> > drivers/firmware/efi/libstub/mem.c | 38 +++++++++++++++++++-----------
> > 1 file changed, 24 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
> > index 0d57078e5e62..7efe3ed2d5a6 100644
> > --- a/drivers/firmware/efi/libstub/mem.c
> > +++ b/drivers/firmware/efi/libstub/mem.c
> > @@ -86,7 +86,7 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
> > *
> > * Allocate pages as EFI_LOADER_DATA. The allocated pages are aligned according
> > * to EFI_ALLOC_ALIGN. The last allocated page will not exceed the address
> > - * given by 'max'.
> > + * given by @max.
> > *
> > * Return: status code
> > */
> > @@ -126,10 +126,10 @@ efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
> > * @addr: on exit the address of the allocated memory
> > * @min: minimum address to used for the memory allocation
> > *
> > - * Allocate at the lowest possible address that is not below 'min' as
> > - * EFI_LOADER_DATA. The allocated pages are aligned according to 'align' but at
> > + * Allocate at the lowest possible address that is not below @min as
> > + * EFI_LOADER_DATA. The allocated pages are aligned according to @align but at
> > * least EFI_ALLOC_ALIGN. The first allocated page will not below the address
> > - * given by 'min'.
> > + * given by @min.
> > *
> > * Return: status code
> > */
> > @@ -214,7 +214,7 @@ efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
> > * @addr: start of the memory area to free (must be EFI_PAGE_SIZE
> > * aligned)
> > *
> > - * 'size' is rounded up to a multiple of EFI_ALLOC_ALIGN which is an
> > + * @size is rounded up to a multiple of EFI_ALLOC_ALIGN which is an
> > * architecture specific multiple of EFI_PAGE_SIZE. So this function should
> > * only be used to return pages allocated with efi_allocate_pages() or
> > * efi_low_alloc_above().
> > @@ -230,15 +230,25 @@ void efi_free(unsigned long size, unsigned long addr)
> > efi_bs_call(free_pages, addr, nr_pages);
> > }
> >
> > -/*
> > - * Relocate a kernel image, either compressed or uncompressed.
> > - * In the ARM64 case, all kernel images are currently
> > - * uncompressed, and as such when we relocate it we need to
> > - * allocate additional space for the BSS segment. Any low
> > - * memory that this function should avoid needs to be
> > - * unavailable in the EFI memory map, as if the preferred
> > - * address is not available the lowest available address will
> > - * be used.
> > +/**
> > + * efi_relocate_kernel() - copy memory area
> > + * @image_addr: address of memory area to copy, on exit target address
>
> The "on exit target address" is a little bit confusing IMO.
> Is it like this?
>
> On exit, @image_addr is updated to the target copy address that was used.
>
Agreed.
> ? or some other better description?
>
> Thanks.
>
> Acked-by: Randy Dunlap <[email protected]>
>
Applied to efi/next with Randy's suggestion folded in
Heinrich: I folded the first few hunks into the respective changes of
yours that were already queued up
Thanks,
>
> > + * @image_size: size of memory area to copy
> > + * @alloc_size: minimum size of memory to allocate, must be greater or
> > + * equal to image_size
> > + * @preferred_addr: preferred target address
> > + * @alignment: minimum alignment of the allocated memory area. It
> > + * should be a power of two.
> > + * @min_addr: minimum target address
> > + *
> > + * Copy a memory area to a newly allocated memory area aligned according
> > + * to @alignment but at least EFI_ALLOC_ALIGN. If the preferred address
> > + * is not available, the allocated address will not be below @min_addr.
> > + *
> > + * This function is used to copy the Linux kernel verbatim. It does not apply
> > + * any relocation changes.
> > + *
> > + * Return: status code
> > */
> > efi_status_t efi_relocate_kernel(unsigned long *image_addr,
> > unsigned long image_size,
> > --
> > 2.25.0
> >
>
>
> --
> ~Randy
>