Adding Zhang...
On Tue, Dec 13, 2022 at 02:10:01PM -0500, Stefan Berger wrote:
> 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 <d...@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?
Yes, exactly...
> > > + 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?
Yep... Sorry for being imprecise...
> > this kinds of problems in general. Please take a look...
>
> I looked at them and I don't know how they are related.
If I do not miss anything your patch set does not take into account size
needed for *grub_mm_region_t and *grub_mm_header structs. They are
needed for mm mgmt. So, grub_mm_add_region_fn() can be called twice to
fulfill request or memory addition may fail. The "mm: Preallocate some
space when adding new regions" patch extends the region size further to
not call firmware too often to fulfill small allocations. Due to that
I think your series should be merged into master together with Zhang one.
I hope he will repost updated mm series soon.
> > > + 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)
> ?
I think you can use size variable instead of total then.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
