On Tue, Oct 31, 2023 at 08:35:26PM +0100, Vladimir 'phcoder' Serbinenko wrote: > From 48c239ad9efc40563015052383d80830d410c2c8 Mon Sep 17 00:00:00 2001 > From: Vladimir Serbinenko <phco...@gmail.com> > Date: Sun, 13 Aug 2023 09:18:23 +0200 > Subject: [PATCH 3/4] types: Split aligned and packed guids > > On ia64 alignment requirements are strict. When we pass a pointer to > UUID it needs to be at least 4-byte aligned or EFI will crash. > On the other hand in device path there is no padding for UUID, so we > need 2 types in one formor another. Make 4-byte aligned and unaligned types > > Signed-off-by: Vladimir Serbinenko <phco...@gmail.com> > --- > grub-core/commands/efi/lsefi.c | 2 +- > grub-core/efiemu/runtime/efiemu.c | 33 +++++++++++++++++++++---------- > grub-core/kern/efi/efi.c | 2 +- > grub-core/kern/misc.c | 2 +- > grub-core/loader/i386/xnu.c | 2 +- > include/grub/efi/api.h | 12 +++++------ > include/grub/efiemu/efiemu.h | 4 ++-- > include/grub/efiemu/runtime.h | 2 +- > include/grub/types.h | 11 ++++++++++- > 9 files changed, 46 insertions(+), 24 deletions(-) > > diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/commands/efi/lsefi.c > index 95cba5baf..7b8316d41 100644 > --- a/grub-core/commands/efi/lsefi.c > +++ b/grub-core/commands/efi/lsefi.c > @@ -96,7 +96,7 @@ grub_cmd_lsefi (grub_command_t cmd __attribute__ ((unused)), > grub_efi_handle_t handle = handles[i]; > grub_efi_status_t status; > grub_efi_uintn_t num_protocols; > - grub_guid_t **protocols; > + grub_packed_guid_t **protocols; > grub_efi_device_path_t *dp; > > grub_printf ("Handle %p\n", handle); > diff --git a/grub-core/efiemu/runtime/efiemu.c > b/grub-core/efiemu/runtime/efiemu.c > index c84b30652..f6b6d19f7 100644 > --- a/grub-core/efiemu/runtime/efiemu.c > +++ b/grub-core/efiemu/runtime/efiemu.c > @@ -66,7 +66,7 @@ efiemu_convert_pointer (grub_efi_uintn_t debug_disposition, > > grub_efi_status_t __grub_efi_api > efiemu_get_variable (grub_efi_char16_t *variable_name, > - const grub_guid_t *vendor_guid, > + const grub_packed_guid_t *vendor_guid, > grub_efi_uint32_t *attributes, > grub_efi_uintn_t *data_size, > void *data); > @@ -74,11 +74,11 @@ efiemu_get_variable (grub_efi_char16_t *variable_name, > 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_guid_t *vendor_guid); > + grub_packed_guid_t *vendor_guid); > > grub_efi_status_t __grub_efi_api > efiemu_set_variable (grub_efi_char16_t *variable_name, > - const grub_guid_t *vendor_guid, > + const grub_packed_guid_t *vendor_guid, > grub_efi_uint32_t attributes, > grub_efi_uintn_t data_size, > void *data); > @@ -416,7 +416,7 @@ EFI_FUNC (efiemu_convert_pointer) (grub_efi_uintn_t > debug_disposition, > > /* Find variable by name and GUID. */ > static struct efi_variable * > -find_variable (const grub_guid_t *vendor_guid, > +find_variable (const grub_packed_guid_t *vendor_guid, > grub_efi_char16_t *variable_name) > { > grub_uint8_t *ptr; > @@ -438,7 +438,7 @@ find_variable (const grub_guid_t *vendor_guid, > > grub_efi_status_t __grub_efi_api > EFI_FUNC (efiemu_get_variable) (grub_efi_char16_t *variable_name, > - const grub_guid_t *vendor_guid, > + const grub_packed_guid_t *vendor_guid, > grub_efi_uint32_t *attributes, > grub_efi_uintn_t *data_size, > void *data) > @@ -464,7 +464,7 @@ EFI_FUNC (efiemu_get_variable) (grub_efi_char16_t > *variable_name, > 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_guid_t *vendor_guid) > + grub_packed_guid_t *vendor_guid) > { > struct efi_variable *efivar; > LOG ('l'); > @@ -503,7 +503,7 @@ grub_efi_status_t __grub_efi_api EFI_FUNC > > grub_efi_status_t __grub_efi_api > EFI_FUNC (efiemu_set_variable) (grub_efi_char16_t *variable_name, > - const grub_guid_t *vendor_guid, > + const grub_packed_guid_t *vendor_guid, > grub_efi_uint32_t attributes, > grub_efi_uintn_t data_size, > void *data) > @@ -597,9 +597,22 @@ struct grub_efi_runtime_services efiemu_runtime_services > = > .set_virtual_address_map = efiemu_set_virtual_address_map, > .convert_pointer = efiemu_convert_pointer, > > - .get_variable = efiemu_get_variable, > - .get_next_variable_name = efiemu_get_next_variable_name, > - .set_variable = efiemu_set_variable, > + .get_variable = (grub_efi_status_t > + (__grub_efi_api *) (grub_efi_char16_t *variable_name, > + const grub_guid_t *vendor_guid, > + grub_efi_uint32_t *attributes, > + grub_efi_uintn_t *data_size, > + void *data)) efiemu_get_variable, > + .get_next_variable_name = (grub_efi_status_t > + (__grub_efi_api *) (grub_efi_uintn_t > *variable_name_size, > + grub_efi_char16_t > *variable_name, > + grub_guid_t *vendor_guid)) > efiemu_get_next_variable_name, > + .set_variable = (grub_efi_status_t > + (__grub_efi_api *) (grub_efi_char16_t *variable_name, > + const grub_guid_t *vendor_guid, > + grub_efi_uint32_t attributes, > + grub_efi_uintn_t data_size, > + void *data)) efiemu_set_variable,
These casts looks strange for me. Could not we change the functions declarations in the grub_efi_runtime_services? If not I think this change should be explained in the commit message. Or leave efiemu_* functions as is... The other patches in these series LGTM. When we are done with the thing mentioned above I will ask folks for more testing. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel