Page MenuHomeFreeBSD

Print an error message in efi_main.c if we can't allocate memory for the heap.
ClosedPublic

Authored by bcran on Nov 12 2018, 5:04 AM.

Details

Summary

With the default Qemu parameters, only 128MB RAM gets given a VM. This causes
the loader to be unable to allocate the 64MB it needs for the heap. This change
makes the cause of the error more obvious.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bcran created this revision.Nov 12 2018, 5:04 AM
bcran added a comment.Nov 12 2018, 5:13 AM

On my build of Qemu and OVMF, the largest contiguous region left is 44MB. There's also a 40MB region, then smaller areas making up 97MB total free.

eadler accepted this revision.Nov 12 2018, 10:56 AM
eadler added inline comments.
stand/efi/loader/efi_main.c
98 ↗(On Diff #50301)

perhaps include the size in the error message. I've found that kind of thing useful before.

This revision is now accepted and ready to land.Nov 12 2018, 10:56 AM

+1 for including the size of the allocation. I wish we could also (cheaply) work out and print how much space we actually have to work with, but meh. I'm not proud of how many times qemu's bitten me because I didn't pay attention to the -m default and spent x amount of time working out again that I didn't give it enough memory to boot successfully.

imp added inline comments.Nov 12 2018, 5:34 PM
stand/efi/loader/efi_main.c
98 ↗(On Diff #50301)

I'd include the size here. I don't know if printf is up and running until before or after the setheap call, which is the only wrinkle I can see.

bcran added a comment.Nov 16 2018, 3:38 PM

Good point about including the size: I'll add it. I'll also double check, but printf didn't work in that location when I tried it, but it's possible I messed up newlines and it got overwritten.

bcran added a comment.Nov 18 2018, 7:54 PM

I've checked and printf isn't available in loader at that point, so there's really nothing we can do except display a static string like I have in my patch.

bcran marked 2 inline comments as done.Dec 13 2018, 11:21 PM
This revision was automatically updated to reflect the committed changes.