[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.

Author: travisg

lib/uefi/boot_service_provider.cpp

@@ -421,10 +421,10 @@ EfiStatus read_blocks(EfiBlockIoProtocol *self, uint32_t media_id, uint64_t lba,
     return END_OF_MEDIA;
   }
 
-  const auto bytes_read =
+  const size_t 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;
   }
@@ -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));
@@ -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;

lib/uefi/runtime_service_provider.cpp

@@ -21,6 +21,8 @@
 #include <stdio.h>
 #include <string.h>
 
+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;

lib/uefi/switch_stack.S

@@ -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
 
@@ -38,3 +37,4 @@ mov     sp,x7
 
 ldp     fp, lr, [sp], 16
 ret     lr
+END_FUNCTION(call_with_stack_asm)

lib/uefi/switch_stack.h

@@ -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

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 <lib/bio.h>
@@ -40,6 +39,8 @@
 #include "system_table.h"
 #include "text_protocol.h"
 
+namespace {
+
 constexpr auto EFI_SYSTEM_TABLE_SIGNATURE =
     static_cast<u64>(0x5453595320494249ULL);
 
@@ -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
@@ -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);
 }
@@ -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