From 6cbd82688cfbd6d568dc2f3cfceabc5b9d86ccbf Mon Sep 17 00:00:00 2001 From: Travis Geiselbrecht Date: Thu, 9 Jan 2025 20:33:30 -0800 Subject: [PATCH] [lib][uefi] fix a few warnings and a little code tidying 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. --- lib/uefi/boot_service_provider.cpp | 62 +++++++++++++-------------- lib/uefi/runtime_service_provider.cpp | 4 ++ lib/uefi/switch_stack.S | 8 ++-- lib/uefi/switch_stack.h | 28 ++++++------ lib/uefi/uefi.cpp | 13 +++--- 5 files changed, 62 insertions(+), 53 deletions(-) diff --git a/lib/uefi/boot_service_provider.cpp b/lib/uefi/boot_service_provider.cpp index ca619ba39..fc72c46de 100644 --- a/lib/uefi/boot_service_provider.cpp +++ b/lib/uefi/boot_service_provider.cpp @@ -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(buffer_size)) { + if (bytes_read != buffer_size) { printf("Failed to read %ld bytes from %s\n", buffer_size, dev->name); return DEVICE_ERROR; } @@ -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( mspace_malloc(get_mspace(), sizeof(EfiBlockIoInterface))); memset(interface, 0, sizeof(EfiBlockIoInterface)); @@ -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; } @@ -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"; @@ -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; } @@ -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; @@ -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), @@ -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; } @@ -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; @@ -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; @@ -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; diff --git a/lib/uefi/runtime_service_provider.cpp b/lib/uefi/runtime_service_provider.cpp index c3e8d9899..97b22b526 100644 --- a/lib/uefi/runtime_service_provider.cpp +++ b/lib/uefi/runtime_service_provider.cpp @@ -21,6 +21,8 @@ #include #include +namespace { + constexpr auto &&kSecureBoot = L"SecureBoot"; EFI_STATUS GetVariable(char16_t *VariableName, EfiGuid *VendorGuid, @@ -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; diff --git a/lib/uefi/switch_stack.S b/lib/uefi/switch_stack.S index e4ffd8bba..15f301ac4 100644 --- a/lib/uefi/switch_stack.S +++ b/lib/uefi/switch_stack.S @@ -14,12 +14,11 @@ * limitations under the License. * */ - -#include +#include -// 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 @@ -38,3 +37,4 @@ mov sp,x7 ldp fp, lr, [sp], 16 ret lr +END_FUNCTION(call_with_stack_asm) diff --git a/lib/uefi/switch_stack.h b/lib/uefi/switch_stack.h index 45948aadd..18df3f743 100644 --- a/lib/uefi/switch_stack.h +++ b/lib/uefi/switch_stack.h @@ -15,28 +15,30 @@ * */ +#include #include -#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 size_t call_with_stack(void *stack, Function &&fp, P1 &¶m1, P2 &¶m2, - P3 param3, P4 &¶m4) { - return call_with_stack( - stack, reinterpret_cast(fp), + P3 &¶m3, P4 &¶m4) { + return call_with_stack_asm( + stack, reinterpret_cast(fp), reinterpret_cast(param1), reinterpret_cast(param2), reinterpret_cast(param3), reinterpret_cast(param4)); } + +template +size_t call_with_stack(void *stack, Function &&fp, P1 &¶m1, P2 &¶m2) { + return call_with_stack_asm( + stack, reinterpret_cast(fp), + reinterpret_cast(param1), reinterpret_cast(param2), 0, 0); +} #endif \ No newline at end of file diff --git a/lib/uefi/uefi.cpp b/lib/uefi/uefi.cpp index 604a3d1f2..fe0c66671 100644 --- a/lib/uefi/uefi.cpp +++ b/lib/uefi/uefi.cpp @@ -18,7 +18,6 @@ #include "boot_service.h" #include "boot_service_provider.h" #include "defer.h" -#include "kernel/thread.h" #include "pe.h" #include @@ -40,6 +39,8 @@ #include "system_table.h" #include "text_protocol.h" +namespace { + constexpr auto EFI_SYSTEM_TABLE_SIGNATURE = static_cast(0x5453595320494249ULL); @@ -57,9 +58,9 @@ template 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 @@ -299,7 +300,7 @@ int load_sections_and_execute(bdev_t *dev, constexpr size_t kStackSize = 8 * 1024ul * 1024; auto stack = reinterpret_cast(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); } @@ -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