Add Bootloader#42
Conversation
GoscZnickiem
force-pushed
the
bootloader
branch
3 times, most recently
from
13044f7 to
f991767
Compare
This reverts commit 754aa77.
GoscZnickiem
force-pushed
the
bootloader
branch
from
06fd20a to
7c493af
Compare
qbojj
left a comment
qbojj
left a comment
There was a problem hiding this comment.
The toolchain change should be in separate PR.
There are still some style guide inconsistencies.
In multiple places the cleanup code is incomplete or non-existent.
From what I remember only u-boot.bin is required so you can try to remove everything else from external/u-boot
There was a problem hiding this comment.
Why is this file even here?
There was a problem hiding this comment.
Great question. No idea. Love git sneaking in files that should never appear.
There was a problem hiding this comment.
Sneaky file in the pocket
There was a problem hiding this comment.
This should be in a separate PR.
| #include "log.h" | ||
| #include "partition.h" | ||
|
|
||
| #define META_CONFIG_PATH L"EFI\\BOOT\\conf.meta" |
There was a problem hiding this comment.
add a TODO to change this to be runtime configurable or to support (so multiple installations could be run with only one bootloader binary instance)
| // EFI_FILE_PROTOCOL** required; | ||
| // UINTN required_count; |
There was a problem hiding this comment.
Comments are meant to comment on something and not remove unwanted code. We use git for a reason.
There was a problem hiding this comment.
Or it's something that can only be implemented with further development from other groups, either way you're right about this needing to be adressed in some way, maybe an explanation?
There was a problem hiding this comment.
If this is something to be implemented, just add a TODO: ...
|
|
||
| loader_t g_loader; | ||
|
|
||
| status_t initialize_loader(void) { |
There was a problem hiding this comment.
g_loader.* are not closed on failure
| EFI_FILE_SYSTEM_INFO* file_system_info = nullptr; | ||
| UINTN file_system_info_size = 0; | ||
| EFI_GUID file_system_info_guid = EFI_FILE_SYSTEM_INFO_ID; | ||
| status = root->GetInfo(root, &file_system_info_guid, &file_system_info_size, file_system_info); |
There was a problem hiding this comment.
would be clearer if the last arg here would be explicitly nullptr and just later defined and assigned
| #include "error.h" | ||
|
|
||
| typedef struct { | ||
| UINTN flags; |
There was a problem hiding this comment.
Either document flags or you can use bitfields with normal names
There was a problem hiding this comment.
Yeah bit more info ✔️
| status = g_system_table->BootServices->GetMemoryMap(&map_size, nullptr, &map_key, &desc_size, &desc_version); | ||
| if (status != EFI_BUFFER_TOO_SMALL) { | ||
| err(L"Failed to get memory map size. BootServices.GetMemoryMap() return code: %u", status); | ||
| exit(); | ||
| } | ||
|
|
||
| map_size += 8 * desc_size; | ||
|
|
||
| mem_map = AllocatePool(map_size); | ||
| if (mem_map == nullptr) { | ||
| err(L"Failed to allocate memory map"); | ||
| exit(); | ||
| } | ||
|
|
||
| status = g_system_table->BootServices->GetMemoryMap(&map_size, mem_map, &map_key, &desc_size, &desc_version); | ||
| if (EFI_ERROR(status)) { | ||
| err(L"Failed to create memory map. BootServices.GetMemoryMap() return code: %u", status); | ||
| exit(); | ||
| } |
There was a problem hiding this comment.
this part should be in a loop untill success, as allocating memory may change the mem map
GoscZnickiem
force-pushed
the
bootloader
branch
from
e75fd52 to
58eb493
Compare
GoscZnickiem
force-pushed
the
bootloader
branch
from
58eb493 to
41bdd86
Compare
There was a problem hiding this comment.
Maybe expanding the types of errors would be useful.
| #include "error.h" | ||
| #include "log.h" | ||
|
|
||
| #define EXT2_DRIVER_PATH L"EFI\\BOOT\\ext2.efi" |
There was a problem hiding this comment.
Is this always going to be the same?
| exit(); | ||
| } | ||
|
|
||
| map_size += 8 * desc_size; |
There was a problem hiding this comment.
Wouldn't using sizeof(...) be better?
There was a problem hiding this comment.
There's no ending of logging when an error occurs, or before getting to the final loop.
|
|
||
| log(L"Starting kernel..."); | ||
| kernel_start(); | ||
| err(L"Kernel returned"); |
There was a problem hiding this comment.
Are we sure we can't do anything more with a returning kernel?
| #include "error.h" | ||
|
|
||
| typedef struct { | ||
| UINTN flags; |
There was a problem hiding this comment.
Yeah bit more info ✔️
…nted expanding fdt for writing and adding subnode
5 participants
A simple bootloader with basic kernel loading capabilities.