Re: [PATCH 1/9] kern/ieee1275/init: ppc64: Introduce a request for regions_claim

[Daniel Kiper]

On Tue, Oct 31, 2023 at 01:50:23PM -0400, Stefan Berger wrote:
> The regions_claim() function limits the allocation of memory regions
> by excluding certain memory areas from being used by grub. This for

Nit, s/grub/GRUB/... Please fix this in all commit messages and comments.

> example includes a gap between 640MB and 768MB as well as an upper
> limit beyond which no memory may be used when an fadump is present.
> However, the ieee1275 loader for kernel and initrd currently does not
> current use regions_claim() for memory allocation on PowerVM and
> PowerKVM and therefore may allocate memory in those areas that it
> should not use.
>
> To make the regions_claim() function more flexible and ulimately usable
> for the ieee1275 loader, introduce a request structure to pass various
> parameters to the regions_claim() function that describe the properties
> of requested memory chunks. In a first step, move the total and flags
> variables into this structure.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Pavithra Prakash <pavrampu@in.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Carolyn Scherrer <cpscherr@us.ibm.com>
> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  grub-core/Makefile.am                 |  1 +
>  grub-core/kern/ieee1275/init.c        | 43 ++++++++++++++++++++-------
>  include/grub/powerpc/ieee1275/alloc.h | 11 +++++++
>  3 files changed, 45 insertions(+), 10 deletions(-)
>  create mode 100644 include/grub/powerpc/ieee1275/alloc.h
>
> diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
> index f0cb2f2cc..4f9f3f963 100644
> --- a/grub-core/Makefile.am
> +++ b/grub-core/Makefile.am
> @@ -240,6 +240,7 @@ endif
>
>  if COND_powerpc_ieee1275
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/ieee1275/ieee1275.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/powerpc/ieee1275/alloc.h
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/terminfo.h
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/extcmd.h
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/lib/arg.h
> diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
> index d6c9c9049..061c28c98 100644
> --- a/grub-core/kern/ieee1275/init.c
> +++ b/grub-core/kern/ieee1275/init.c
> @@ -46,6 +46,9 @@
>  #ifdef __sparc__
>  #include <grub/machine/kernel.h>
>  #endif
> +#ifdef __powerpc__
> +#include <grub/powerpc/ieee1275/alloc.h>
> +#endif
>
>  /* The maximum heap size we're going to claim at boot. Not used by sparc. */
>  #ifdef __i386__
> @@ -317,9 +320,9 @@ count_free (grub_uint64_t addr, grub_uint64_t len, 
> grub_memory_type_t type,
>
>  static int
>  regions_claim (grub_uint64_t addr, grub_uint64_t len, grub_memory_type_t 
> type,
> -           unsigned int flags, void *data)
> +            void *data)

Why not "struct regions_claim_request *rcr" instead of "void *data"?
Then you do not need extra assignment below...

Daniel