Skip to content

Commit

Permalink
[lib][uefi] fix a few warnings and a little code tidying
Browse files Browse the repository at this point in the history
GCC 14 is quite picky about warnings, probably more so than clang.

-Fix a bunch of printf warnings. Pointers should be printed with %p.
-Move some stuff into an anonymous namespace.
-Worked around GCC really not liking reinterpret_casting from one
function pointer type to another. Fiddled with it a bit and eventually
settled on casting the function pointer to const void * and passing it
through.
  • Loading branch information
travisg committed Jan 10, 2025
1 parent e7b7162 commit 6cbd826
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 53 deletions.
62 changes: 31 additions & 31 deletions lib/uefi/boot_service_provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ EfiStatus read_blocks(EfiBlockIoProtocol *self, uint32_t media_id, uint64_t lba,
const auto bytes_read =
call_with_stack(interface->io_stack, bio_read_block, dev, buffer, lba,
buffer_size / dev->block_size);
if (bytes_read != static_cast<ssize_t>(buffer_size)) {
if (bytes_read != buffer_size) {
printf("Failed to read %ld bytes from %s\n", buffer_size, dev->name);
return DEVICE_ERROR;
}
Expand All @@ -449,7 +449,7 @@ EfiStatus open_block_device(EfiHandle handle, void **intf) {
vmm_alloc(vmm_get_kernel_aspace(), "uefi_io_stack", kIoStackSize, &io_stack,
PAGE_SIZE_SHIFT, 0, 0);
}
printf("%s(%s)\n", __FUNCTION__, handle);
printf("%s(%p)\n", __FUNCTION__, handle);
const auto interface = reinterpret_cast<EfiBlockIoInterface *>(
mspace_malloc(get_mspace(), sizeof(EfiBlockIoInterface)));
memset(interface, 0, sizeof(EfiBlockIoInterface));
Expand Down Expand Up @@ -504,7 +504,7 @@ EFI_STATUS efi_dt_fixup(struct EfiDtFixupProtocol *self, void *fdt,
EfiStatus fixup_kernel_commandline(struct GblEfiOsConfigurationProtocol *self,
const char *command_line, char *fixup,
size_t *fixup_buffer_size) {
printf("%s(0x%lx, \"%s\")\n", __FUNCTION__, self, command_line);
printf("%s(%p, \"%s\")\n", __FUNCTION__, self, command_line);
*fixup_buffer_size = 0;
return SUCCESS;
}
Expand All @@ -513,7 +513,7 @@ EfiStatus fixup_kernel_commandline(struct GblEfiOsConfigurationProtocol *self,
EfiStatus fixup_bootconfig(struct GblEfiOsConfigurationProtocol *self,
const char *bootconfig, size_t size, char *fixup,
size_t *fixup_buffer_size) {
printf("%s(0x%lx, %s, %lu, %lu)\n", __FUNCTION__, self, bootconfig, size,
printf("%s(%p, %s, %lu, %lu)\n", __FUNCTION__, self, bootconfig, size,
*fixup_buffer_size);
constexpr auto &&to_add = "\nandroidboot.fstab_suffix=cf.f2fs."
"hctr2\nandroidboot.boot_devices=4010000000.pcie";
Expand All @@ -532,7 +532,7 @@ EfiStatus fixup_bootconfig(struct GblEfiOsConfigurationProtocol *self,
EfiStatus select_device_trees(struct GblEfiOsConfigurationProtocol *self,
GblEfiVerifiedDeviceTree *device_trees,
size_t num_device_trees) {
printf("%s(0x%lx, %lx, %lu)\n", __FUNCTION__, self, device_trees,
printf("%s(%p, %p %lu)\n", __FUNCTION__, self, device_trees,
num_device_trees);
return UNSUPPORTED;
}
Expand All @@ -547,29 +547,29 @@ EfiStatus open_protocol(EfiHandle handle, const EfiGuid *protocol, void **intf,
interface->parent_handle = handle;
interface->image_base = handle;
*intf = interface;
printf("%s(LOADED_IMAGE_PROTOCOL_GUID, handle=0x%lx, agent_handle=0x%lx, "
"controller_handle=0x%lx, attr=0x%x)\n",
printf("%s(LOADED_IMAGE_PROTOCOL_GUID, handle=%p, agent_handle=%p, "
"controller_handle=%p, attr=0x%x)\n",
__FUNCTION__, handle, agent_handle, controller_handle, attr);
return SUCCESS;
} else if (guid_eq(protocol, EFI_DEVICE_PATH_PROTOCOL_GUID)) {
printf(
"%s(EFI_DEVICE_PATH_PROTOCOL_GUID, handle=0x%lx, agent_handle=0x%lx, "
"controller_handle=0x%lx, attr=0x%x)\n",
"%s(EFI_DEVICE_PATH_PROTOCOL_GUID, handle=%p, agent_handle=%p, "
"controller_handle=%p, attr=0x%x)\n",
__FUNCTION__, handle, agent_handle, controller_handle, attr);
return UNSUPPORTED;
} else if (guid_eq(protocol, EFI_BLOCK_IO_PROTOCOL_GUID)) {
printf("%s(EFI_BLOCK_IO_PROTOCOL_GUID, handle=0x%lx, agent_handle=0x%lx, "
"controller_handle=0x%lx, attr=0x%x)\n",
printf("%s(EFI_BLOCK_IO_PROTOCOL_GUID, handle=%p, agent_handle=%p, "
"controller_handle=%p, attr=0x%x)\n",
__FUNCTION__, handle, agent_handle, controller_handle, attr);
return open_block_device(handle, intf);
} else if (guid_eq(protocol, EFI_BLOCK_IO2_PROTOCOL_GUID)) {
printf("%s(EFI_BLOCK_IO2_PROTOCOL_GUID, handle=0x%lx, agent_handle=0x%lx, "
"controller_handle=0x%lx, attr=0x%x)\n",
printf("%s(EFI_BLOCK_IO2_PROTOCOL_GUID, handle=%p, agent_handle=%p, "
"controller_handle=%p, attr=0x%x)\n",
__FUNCTION__, handle, agent_handle, controller_handle, attr);
return UNSUPPORTED;
} else if (guid_eq(protocol, EFI_DT_FIXUP_PROTOCOL_GUID)) {
printf("%s(EFI_DT_FIXUP_PROTOCOL_GUID, handle=0x%lx, agent_handle=0x%lx, "
"controller_handle=0x%lx, attr=0x%x)\n",
printf("%s(EFI_DT_FIXUP_PROTOCOL_GUID, handle=%p, agent_handle=%p, "
"controller_handle=%p, attr=0x%x)\n",
__FUNCTION__, handle, agent_handle, controller_handle, attr);
if (intf != nullptr) {
EfiDtFixupProtocol *fixup = nullptr;
Expand All @@ -584,9 +584,9 @@ EfiStatus open_protocol(EfiHandle handle, const EfiGuid *protocol, void **intf,
}
return SUCCESS;
} else if (guid_eq(protocol, EFI_GBL_OS_CONFIGURATION_PROTOCOL_GUID)) {
printf("%s(EFI_GBL_OS_CONFIGURATION_PROTOCOL_GUID, handle=0x%lx, "
"agent_handle=0x%lx, "
"controller_handle=0x%lx, attr=0x%x)\n",
printf("%s(EFI_GBL_OS_CONFIGURATION_PROTOCOL_GUID, handle=%p, "
"agent_handle=%p, "
"controller_handle=%p, attr=0x%x)\n",
__FUNCTION__, handle, agent_handle, controller_handle, attr);
GblEfiOsConfigurationProtocol *config = nullptr;
allocate_pool(BOOT_SERVICES_DATA, sizeof(*config),
Expand All @@ -610,24 +610,24 @@ EfiStatus open_protocol(EfiHandle handle, const EfiGuid *protocol, void **intf,
EfiStatus close_protocol(EfiHandle handle, const EfiGuid *protocol,
EfiHandle agent_handle, EfiHandle controller_handle) {
if (guid_eq(protocol, LOADED_IMAGE_PROTOCOL_GUID)) {
printf("%s(LOADED_IMAGE_PROTOCOL_GUID, handle=0x%lx, agent_handle=0x%lx, "
"controller_handle=0x%lx)\n",
printf("%s(LOADED_IMAGE_PROTOCOL_GUID, handle=%p, agent_handle=%p, "
"controller_handle=%p)\n",
__FUNCTION__, handle, agent_handle, controller_handle);
return SUCCESS;
} else if (guid_eq(protocol, EFI_DEVICE_PATH_PROTOCOL_GUID)) {
printf(
"%s(EFI_DEVICE_PATH_PROTOCOL_GUID, handle=0x%lx, agent_handle=0x%lx, "
"controller_handle=0x%lx)\n",
"%s(EFI_DEVICE_PATH_PROTOCOL_GUID, handle=%p, agent_handle=%p, "
"controller_handle=%p)\n",
__FUNCTION__, handle, agent_handle, controller_handle);
return SUCCESS;
} else if (guid_eq(protocol, EFI_BLOCK_IO_PROTOCOL_GUID)) {
printf("%s(EFI_BLOCK_IO_PROTOCOL_GUID, handle=0x%lx, agent_handle=0x%lx, "
"controller_handle=0x%lx)\n",
printf("%s(EFI_BLOCK_IO_PROTOCOL_GUID, handle=%p, agent_handle=%p, "
"controller_handle=%p)\n",
__FUNCTION__, handle, agent_handle, controller_handle);
return SUCCESS;
} else if (guid_eq(protocol, EFI_DT_FIXUP_PROTOCOL_GUID)) {
printf("%s(EFI_DT_FIXUP_PROTOCOL_GUID, handle=0x%lx, agent_handle=0x%lx, "
"controller_handle=0x%lx)\n",
printf("%s(EFI_DT_FIXUP_PROTOCOL_GUID, handle=%p, agent_handle=%p, "
"controller_handle=%p)\n",
__FUNCTION__, handle, agent_handle, controller_handle);
return SUCCESS;
}
Expand Down Expand Up @@ -661,16 +661,16 @@ EfiStatus locate_handle_buffer(EfiLocateHandleSearchType search_type,
if (search_type == BY_PROTOCOL) {
return list_block_devices(num_handles, buf);
}
printf("%s(0x%x, EFI_BLOCK_IO_PROTOCOL_GUID, search_key=0x%lx)\n",
printf("%s(0x%x, EFI_BLOCK_IO_PROTOCOL_GUID, search_key=%p)\n",
__FUNCTION__, search_type, search_key);
return UNSUPPORTED;
} else if (guid_eq(protocol, EFI_TEXT_INPUT_PROTOCOL_GUID)) {
printf("%s(0x%x, EFI_TEXT_INPUT_PROTOCOL_GUID, search_key=0x%lx)\n",
printf("%s(0x%x, EFI_TEXT_INPUT_PROTOCOL_GUID, search_key=%p)\n",
__FUNCTION__, search_type, search_key);
return NOT_FOUND;
} else if (guid_eq(protocol, EFI_GBL_OS_CONFIGURATION_PROTOCOL_GUID)) {
printf(
"%s(0x%x, EFI_GBL_OS_CONFIGURATION_PROTOCOL_GUID, search_key=0x%lx)\n",
"%s(0x%x, EFI_GBL_OS_CONFIGURATION_PROTOCOL_GUID, search_key=%p)\n",
__FUNCTION__, search_type, search_key);
if (num_handles != nullptr) {
*num_handles = 1;
Expand All @@ -681,7 +681,7 @@ EfiStatus locate_handle_buffer(EfiLocateHandleSearchType search_type,
}
return SUCCESS;
} else if (guid_eq(protocol, EFI_DT_FIXUP_PROTOCOL_GUID)) {
printf("%s(0x%x, EFI_DT_FIXUP_PROTOCOL_GUID, search_key=0x%lx)\n",
printf("%s(0x%x, EFI_DT_FIXUP_PROTOCOL_GUID, search_key=%p)\n",
__FUNCTION__, search_type, search_key);
if (num_handles != nullptr) {
*num_handles = 1;
Expand All @@ -692,7 +692,7 @@ EfiStatus locate_handle_buffer(EfiLocateHandleSearchType search_type,
}
return SUCCESS;
}
printf("%s(0x%x, (0x%x 0x%x 0x%x 0x%llx), search_key=0x%lx)\n", __FUNCTION__,
printf("%s(0x%x, (0x%x 0x%x 0x%x 0x%llx), search_key=%p)\n", __FUNCTION__,
search_type, protocol->data1, protocol->data2, protocol->data3,
*(uint64_t *)&protocol->data4, search_key);
return UNSUPPORTED;
Expand Down
4 changes: 4 additions & 0 deletions lib/uefi/runtime_service_provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include <stdio.h>
#include <string.h>

namespace {

constexpr auto &&kSecureBoot = L"SecureBoot";

EFI_STATUS GetVariable(char16_t *VariableName, EfiGuid *VendorGuid,
Expand Down Expand Up @@ -62,6 +64,8 @@ EFI_STATUS SetVariable(char16_t *VariableName, EfiGuid *VendorGuid,
return UNSUPPORTED;
}

} // namespace

void setup_runtime_service_table(EfiRuntimeService *service) {
service->GetVariable = GetVariable;
service->SetVariable = SetVariable;
Expand Down
8 changes: 4 additions & 4 deletions lib/uefi/switch_stack.S
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@
* limitations under the License.
*
*/

#include <lk/asm.h>

#include <lk/asm.h>

// int call_with_stack(void *stack, int (*fp)(), int arg1, int arg2, int arg3, int arg4);
FUNCTION(call_with_stack)
// int call_with_stack_asm(void *stack, int (*fp)(), int arg1, int arg2, int arg3, int arg4);
FUNCTION(call_with_stack_asm)
stp fp, lr, [sp, #-16]!
mov fp, sp

Expand All @@ -38,3 +37,4 @@ mov sp,x7

ldp fp, lr, [sp], 16
ret lr
END_FUNCTION(call_with_stack_asm)
28 changes: 15 additions & 13 deletions lib/uefi/switch_stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,30 @@
*
*/

#include <lk/compiler.h>
#include <stddef.h>

#ifdef __cplusplus
extern "C" {

#endif
__BEGIN_CDECLS

size_t call_with_stack(void *stack, int (*fp)(void *, void *), void *param1,
void *param2, void *param3 = nullptr,
void *param4 = nullptr);
size_t call_with_stack_asm(void *stack, const void *function, void *param1,
void *param2, void *param3, void *param4);

#ifdef __cplusplus
}
__END_CDECLS

#endif
#ifdef __cplusplus
template <typename Function, typename P1, typename P2, typename P3, typename P4>
size_t call_with_stack(void *stack, Function &&fp, P1 &&param1, P2 &&param2,
P3 param3, P4 &&param4) {
return call_with_stack(
stack, reinterpret_cast<int (*)(void *, void *)>(fp),
P3 &&param3, P4 &&param4) {
return call_with_stack_asm(
stack, reinterpret_cast<const void *>(fp),
reinterpret_cast<void *>(param1), reinterpret_cast<void *>(param2),
reinterpret_cast<void *>(param3), reinterpret_cast<void *>(param4));
}

template <typename Function, typename P1, typename P2>
size_t call_with_stack(void *stack, Function &&fp, P1 &&param1, P2 &&param2) {
return call_with_stack_asm(
stack, reinterpret_cast<const void *>(fp),
reinterpret_cast<void *>(param1), reinterpret_cast<void *>(param2), 0, 0);
}
#endif
13 changes: 8 additions & 5 deletions lib/uefi/uefi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "boot_service.h"
#include "boot_service_provider.h"
#include "defer.h"
#include "kernel/thread.h"
#include "pe.h"

#include <lib/bio.h>
Expand All @@ -40,6 +39,8 @@
#include "system_table.h"
#include "text_protocol.h"

namespace {

constexpr auto EFI_SYSTEM_TABLE_SIGNATURE =
static_cast<u64>(0x5453595320494249ULL);

Expand All @@ -57,9 +58,9 @@ template <typename T> void fill(T *data, size_t skip, uint8_t begin = 0) {
}
}

static constexpr size_t BIT26 = 1 << 26;
static constexpr size_t BIT11 = 1 << 11;
static constexpr size_t BIT10 = 1 << 10;
constexpr size_t BIT26 = 1 << 26;
constexpr size_t BIT11 = 1 << 11;
constexpr size_t BIT10 = 1 << 10;

/**
Pass in a pointer to an ARM MOVT or MOVW immediate instruciton and
Expand Down Expand Up @@ -299,7 +300,7 @@ int load_sections_and_execute(bdev_t *dev,
constexpr size_t kStackSize = 8 * 1024ul * 1024;
auto stack = reinterpret_cast<char *>(alloc_page(kStackSize, 23));
memset(stack, 0, kStackSize);
printf("Calling kernel with stack [0x%lx, 0x%lx]\n", stack,
printf("Calling kernel with stack [%p, %p]\n", stack,
stack + kStackSize - 1);
return call_with_stack(stack + kStackSize, entry, image_base, &table);
}
Expand Down Expand Up @@ -373,3 +374,5 @@ int cmd_uefi_load(int argc, const console_cmd_args *argv) {
STATIC_COMMAND_START
STATIC_COMMAND("uefi_load", "load UEFI application and run it", &cmd_uefi_load)
STATIC_COMMAND_END(uefi);

} // namespace

0 comments on commit 6cbd826

Please sign in to comment.