Re: [PATCH v2 3/6] ieee1275: support runtime memory claiming

[Stefan Berger]

On 12/13/22 11:14, Daniel Kiper wrote:
> On Thu, Dec 01, 2022 at 04:11:58PM -0500, Stefan Berger wrote:
> > From: Daniel Axtens <dja@axtens.net>
> >
> > On powerpc-ieee1275, we are running out of memory trying to verify
> > anything. This is because:
> >
> >   - we have to load an entire file into memory to verify it. This is
> >     difficult to change with appended signatures.
> >   - We only have 32MB of heap.
> >   - Distro kernels are now often around 30MB.

> > +
> > +  if (flags & GRUB_MM_ADD_REGION_CONSECUTIVE)
> > +    {
> > +      /* first try rounding up hard for the sake of speed */
> > +      total = grub_max (ALIGN_UP(size, 1024 * 1024) + 1024 * 1024, 32 * 1024 * 
> > 1024);
>
> s/ALIGN_UP(/ALIGN_UP (/
>
> > +      total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
> > +
> > +      grub_dprintf ("ieee1275", "looking for %x bytes of memory (%x 
> > requested)\n", total, size);
> > +
> > +      grub_machine_mmap_iterate (region_claim, &total);
> > +      grub_dprintf ("ieee1275", "get memory from fw %s\n", total == 0 ? "succeeded" : 
> > "failed");
> > +
> > +      /* fallback: requested size + padding for a grub_mm_header_t and region 
> > */
>
> This...

With 'this' you mean the comment above or which exact part?

> > +      if (total != 0)
> > +        {
> > +          total = size + 48;
>
> ... and this should be dropped. I think this patch set, [1], [2], solves

You mean adding 48 to it can be dropped?

> this kinds of problems in general. Please take a look...

I looked at them and I don't know how they are related.


> > +          total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
> > +
> > +          grub_dprintf ("ieee1275", "fallback for %x bytes of memory (%x 
> > requested)\n", total, size);
> > +
> > +          grub_machine_mmap_iterate (region_claim, &total);
> > +          grub_dprintf ("ieee1275", "fallback from fw %s\n", total == 0 ? "succeeded" : 
> > "failed");
> > +        }
> > +    }
> > +  else
> > +    {
> > +      /* provide padding for a grub_mm_header_t and region */
> > +      total = size + 48;
>
> Ditto... And then probably total can be dropped too...

Don't we need a parameter to grub_machine_mmap_iterate (region_claim, &total) ?


   Stefan
> > +      total = grub_min (free_memory - RUNTIME_MIN_SPACE, total);
> > +      grub_machine_mmap_iterate (heap_init, &total);
> > +      grub_dprintf ("ieee1275", "get noncontig memory from fw %s\n", total == 0 ? 
> > "succeeded" : "failed");
> > +    }
> > +
> > +  if (total == 0)
> > +    return GRUB_ERR_NONE;
> > +  else
> > +    return GRUB_ERR_OUT_OF_MEMORY;
> > +}
>
> Daniel
>
> [1] https://lists.gnu.org/archive/html/grub-devel/2022-11/msg00147.html
> [2] https://lists.gnu.org/archive/html/grub-devel/2022-12/msg00115.html
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel