On Tue, May 23, 2023 at 10:23:50AM +0200, Ard Biesheuvel wrote:
> Switch the i386-efi and x86_64-efi builds to the generic EFI loader,
> which enters the Linux kernel via the EFI stub and provides the initrd
> via the LoadFile2 protocol. This unifies x86 with other EFI
> architectures, and removes the dependency on the setup header and struct
> bootparams.
>
> Do some preparatory cleanup first, so we no longer need to rely on the
> MS to SysV calling convention translation code.
>
> Changes since v2:
> - rebase onto latest master, which has the Loongarch changes
> - retain Itanium support for now
> - simplify the fallback logic - attempt to load the image as usual, and
> fallback on failure or if the image does not implement LoadFile2
>
> Changes since v1:
> - drop Itanium support, which is a maintenance burden as it shares the
> EFI code with other architectures, but does not have a EFI stub in
> Linux, and there is no way to test whether our changes break the boot
> for it or not;
> - enable generic EFI for i386 as well
> - wire up the existing x86 code as a fallback for kernels that lack EFI
> stub or LoadFile2 support. This removes the need for additional
> changes to support v5.8 or older kernels.
>
> Cc: Daniel Kiper <daniel.ki...@oracle.com>
> Cc: Glenn Washburn <developm...@efficientek.com>
>
> Ard Biesheuvel (5):
> efi: Make EFI PXE protocol methods non-callable
> efi: Add calling convention annotation to all prototypes
> efi: Drop all uses of efi_call_XX wrappers
> efi: Remove x86_64 call wrappers
> efi: Use generic EFI loader for x86_64 and i386
I run test and Coverity builds and they reported some problems. You can
find in attachments a fix for the build problems and a Coverity report.
Please take a look...
Otherwise patches LGTM...
Thanks,
Daniel
diff --git a/grub-core/efiemu/runtime/efiemu.c b/grub-core/efiemu/runtime/efiemu.c
index 5db1f347b..53b3cce7b 100644
--- a/grub-core/efiemu/runtime/efiemu.c
+++ b/grub-core/efiemu/runtime/efiemu.c
@@ -32,17 +32,17 @@
#include <grub/efi/api.h>
#include <grub/efiemu/runtime.h>
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
efiemu_get_time (grub_efi_time_t *time,
grub_efi_time_capabilities_t *capabilities);
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
efiemu_set_time (grub_efi_time_t *time);
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
efiemu_get_wakeup_time (grub_efi_boolean_t *enabled,
grub_efi_boolean_t *pending,
grub_efi_time_t *time);
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
efiemu_set_wakeup_time (grub_efi_boolean_t enabled,
grub_efi_time_t *time);
@@ -52,51 +52,51 @@ efiemu_set_wakeup_time (grub_efi_boolean_t enabled,
#define PHYSICAL_ATTRIBUTE __attribute__ ((section(".text-physical")));
#endif
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
efiemu_set_virtual_address_map (grub_efi_uintn_t memory_map_size,
grub_efi_uintn_t descriptor_size,
grub_efi_uint32_t descriptor_version,
grub_efi_memory_descriptor_t *virtual_map)
PHYSICAL_ATTRIBUTE;
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
efiemu_convert_pointer (grub_efi_uintn_t debug_disposition,
void **address)
PHYSICAL_ATTRIBUTE;
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
efiemu_get_variable (grub_efi_char16_t *variable_name,
const grub_efi_guid_t *vendor_guid,
grub_efi_uint32_t *attributes,
grub_efi_uintn_t *data_size,
void *data);
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
efiemu_get_next_variable_name (grub_efi_uintn_t *variable_name_size,
grub_efi_char16_t *variable_name,
grub_efi_guid_t *vendor_guid);
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
efiemu_set_variable (grub_efi_char16_t *variable_name,
const grub_efi_guid_t *vendor_guid,
grub_efi_uint32_t attributes,
grub_efi_uintn_t data_size,
void *data);
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
efiemu_get_next_high_monotonic_count (grub_efi_uint32_t *high_count);
-void
+void __grub_efi_api
efiemu_reset_system (grub_efi_reset_type_t reset_type,
grub_efi_status_t reset_status,
grub_efi_uintn_t data_size,
grub_efi_char16_t *reset_data);
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
EFI_FUNC (efiemu_set_virtual_address_map) (grub_efi_uintn_t,
grub_efi_uintn_t,
grub_efi_uint32_t,
grub_efi_memory_descriptor_t *)
PHYSICAL_ATTRIBUTE;
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
EFI_FUNC (efiemu_convert_pointer) (grub_efi_uintn_t debug_disposition,
void **address)
PHYSICAL_ATTRIBUTE;
@@ -202,7 +202,7 @@ bcd_to_hex (grub_uint8_t in)
return 10 * ((in & 0xf0) >> 4) + (in & 0x0f);
}
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
EFI_FUNC (efiemu_get_time) (grub_efi_time_t *time,
grub_efi_time_capabilities_t *capabilities)
{
@@ -246,7 +246,7 @@ EFI_FUNC (efiemu_get_time) (grub_efi_time_t *time,
return GRUB_EFI_SUCCESS;
}
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
EFI_FUNC (efiemu_set_time) (grub_efi_time_t *time)
{
LOG ('b');
@@ -265,7 +265,7 @@ EFI_FUNC (efiemu_set_time) (grub_efi_time_t *time)
}
/* Following 2 functions are vendor specific. So announce it as unsupported */
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
EFI_FUNC (efiemu_get_wakeup_time) (grub_efi_boolean_t *enabled,
grub_efi_boolean_t *pending,
grub_efi_time_t *time)
@@ -274,7 +274,7 @@ EFI_FUNC (efiemu_get_wakeup_time) (grub_efi_boolean_t *enabled,
return GRUB_EFI_UNSUPPORTED;
}
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
EFI_FUNC (efiemu_set_wakeup_time) (grub_efi_boolean_t enabled,
grub_efi_time_t *time)
{
@@ -337,7 +337,7 @@ efiemu_getcrc32 (grub_uint32_t crc, void *buf, int size)
}
-grub_efi_status_t EFI_FUNC
+grub_efi_status_t __grub_efi_api EFI_FUNC
(efiemu_set_virtual_address_map) (grub_efi_uintn_t memory_map_size,
grub_efi_uintn_t descriptor_size,
grub_efi_uint32_t descriptor_version,
@@ -403,7 +403,7 @@ grub_efi_status_t EFI_FUNC
/* since efiemu_set_virtual_address_map corrects all the pointers
we don't need efiemu_convert_pointer */
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
EFI_FUNC (efiemu_convert_pointer) (grub_efi_uintn_t debug_disposition,
void **address)
{
@@ -436,7 +436,7 @@ find_variable (const grub_efi_guid_t *vendor_guid,
return 0;
}
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
EFI_FUNC (efiemu_get_variable) (grub_efi_char16_t *variable_name,
const grub_efi_guid_t *vendor_guid,
grub_efi_uint32_t *attributes,
@@ -461,7 +461,7 @@ EFI_FUNC (efiemu_get_variable) (grub_efi_char16_t *variable_name,
return GRUB_EFI_SUCCESS;
}
-grub_efi_status_t EFI_FUNC
+grub_efi_status_t __grub_efi_api EFI_FUNC
(efiemu_get_next_variable_name) (grub_efi_uintn_t *variable_name_size,
grub_efi_char16_t *variable_name,
grub_efi_guid_t *vendor_guid)
@@ -501,7 +501,7 @@ grub_efi_status_t EFI_FUNC
return GRUB_EFI_SUCCESS;
}
-grub_efi_status_t
+grub_efi_status_t __grub_efi_api
EFI_FUNC (efiemu_set_variable) (grub_efi_char16_t *variable_name,
const grub_efi_guid_t *vendor_guid,
grub_efi_uint32_t attributes,
@@ -556,7 +556,7 @@ EFI_FUNC (efiemu_set_variable) (grub_efi_char16_t *variable_name,
return GRUB_EFI_SUCCESS;
}
-grub_efi_status_t EFI_FUNC
+grub_efi_status_t __grub_efi_api EFI_FUNC
(efiemu_get_next_high_monotonic_count) (grub_efi_uint32_t *high_count)
{
LOG ('j');
@@ -570,7 +570,7 @@ grub_efi_status_t EFI_FUNC
Besides EFI specification says that this function shouldn't be used
on systems supporting ACPI
*/
-void
+void __grub_efi_api
EFI_FUNC (efiemu_reset_system) (grub_efi_reset_type_t reset_type,
grub_efi_status_t reset_status,
grub_efi_uintn_t data_size,
Hi,
Please find the latest report on new defect(s) introduced to GRUB found with
Coverity Scan.
2 new defect(s) introduced to GRUB found with Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 2 of 2 defect(s)
** CID 407976: Null pointer dereferences (NULL_RETURNS)
/grub-core/loader/efi/linux.c: 217 in grub_arch_efi_linux_boot_image()
________________________________________________________________________________________________________
*** CID 407976: Null pointer dereferences (NULL_RETURNS)
/grub-core/loader/efi/linux.c: 217 in grub_arch_efi_linux_boot_image()
211 return grub_error (GRUB_ERR_BAD_OS, "cannot load image");
212
213 grub_dprintf ("linux", "linux command line: '%s'\n", args);
214
215 /* Convert command line to UCS-2 */
216 loaded_image = grub_efi_get_loaded_image (image_handle);
>>> CID 407976: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing "loaded_image", which is known to be "NULL".
217 loaded_image->load_options_size = len =
218 (grub_strlen (args) + 1) * sizeof (grub_efi_char16_t);
219 loaded_image->load_options =
220 grub_efi_allocate_any_pages (GRUB_EFI_BYTES_TO_PAGES
(loaded_image->load_options_size));
221 if (!loaded_image->load_options)
222 return grub_errno;
** CID 407975: Possible Control flow issues (DEADCODE)
/grub-core/loader/efi/linux.c: 437 in grub_cmd_initrd()
________________________________________________________________________________________________________
*** CID 407975: Possible Control flow issues (DEADCODE)
/grub-core/loader/efi/linux.c: 437 in grub_cmd_initrd()
431 grub_dprintf ("linux", "[addr=%p, size=0x%x]\n",
432 (void *) initrd_start, initrd_size);
433 #endif
434
435 fail:
436 grub_initrd_close (&initrd_ctx);
>>> CID 407975: Possible Control flow issues (DEADCODE)
>>> Execution cannot reach the expression "initrd_start" inside this
>>> statement: "if (initrd_mem && !initrd_s...".
437 if (initrd_mem && !initrd_start)
438 grub_efi_free_pages ((grub_addr_t) initrd_mem, initrd_pages);
439
440 return grub_errno;
441 }
442
________________________________________________________________________________________________________
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
