Re: [PATCH v6 00/10] ppc64: Restrict memory allocations for kernel and initrd

[Daniel Kiper]

On Fri, Nov 24, 2023 at 02:10:05PM -0500, Stefan Berger wrote:
> On 11/23/23 11:38, Daniel Kiper wrote:
> > On Mon, Nov 20, 2023 at 09:49:57AM -0500, Stefan Berger wrote:
> > > This series of patches converts the PowerPC ieee1275 memory allocator
> > > for kernel and initrd to use the (PowerPC) regions_claim memory allocator
> > > that takes into account memory regions that are not allowed to be used,
> > > such as the gap between 640MB and 768MB as well as memory regions beyond
> > > an address in case an fadump is present. Otherwise those two could be
> > > loaded into restricted memory regions and either cause a crash or
> > > corruption of the fadump.
> > >
> > > I adjusted the kernel and initrd load in loader/powerpc/ieee1275 to use 
> > > the
> > > new memory allocator only on PowerVM and PowerKVM since this code is 
> > > shared
> > > with other platforms, such as old PowerMACs.
> >
> > Sadly your patch set breaks at least i386/ieee1275 build...
> >
> >    ./configure --target=i386 --with-platform=ieee1275 ...
> >    make
> >
> >    kern/ieee1275/cmain.c: In function ‘grub_ieee1275_find_options’:
> >    kern/ieee1275/cmain.c:199:31: error: ‘GRUB_IEEE1275_FLAG_POWER_KVM’ 
> > undeclared (first use in this function)
> >           grub_ieee1275_set_flag (GRUB_IEEE1275_FLAG_POWER_KVM);
> >                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > ... or...
> >
> >    kern/ieee1275/init.c:322:1: error: no previous prototype for 
> > ‘grub_regions_claim’ [-Werror=missing-prototypes]
> >      322 | grub_regions_claim (grub_uint64_t addr, grub_uint64_t len,
> >          | ^~~~~~~~~~~~~~~~~~
> >    kern/ieee1275/init.c: In function ‘grub_regions_claim’:
> >    kern/ieee1275/init.c:510:10: error: invalid use of undefined type 
> > ‘struct regions_claim_request’
> >      510 |   if (rcr->align)
> >          |          ^~
> >    In file included from ../include/grub/dl.h:29,
> >                     from kern/ieee1275/init.c:23:
> >    kern/ieee1275/init.c:512:53: error: invalid use of undefined type 
> > ‘struct regions_claim_request’
> >      512 |       grub_uint64_t align_addr = ALIGN_UP (addr, rcr->align);
> >          |                                                     ^~
> >    ../include/grub/misc.h:31:37: note: in definition of macro ‘ALIGN_UP’
> >       31 |         (((addr) + (typeof (addr)) (align) - 1) & ~((typeof 
> > (addr)) (align) - 1))
> >          |                                     ^~~~~
> >    ...
> >
> > Please fix these issues...
>
> This required two fixes:
> - move include/grub/powerpc/ieee1275/alloc.h to
> include/grub/ieee1275/alloc.h
> - move the 2 new #defines for POWER_VM and KVM out of #ifdef __powerpc__

I think we need some more #ifdef __powerpc__ instead, e.g. around
grub_ieee1275_set_flag(GRUB_IEEE1275_FLAG_POWER_KVM).

>  Can I keep your Reviewed-by tags?

If changes are not that big and you provide git range-diff then yes.

Daniel