On Tue, Jul 8, 2025 at 9:02 PM Frediano Ziglio via Grub-devel
<grub-devel@gnu.org> wrote:
>
> Allows to load modules using LoadFile2 protocol.
> Add and use a new GUID for kernel media device.
> This will allow Xen to pick up additional modules using
> EFI interface instead of using multiboot2 interface (not
> available on x86_64).
>
> Signed-off-by: Frediano Ziglio <frediano.zig...@cloud.com>
> ---
> grub-core/loader/arm64/xen_boot.c | 172 +++++++++++++++++++++++++++++-
> include/grub/efi/api.h | 5 +
> 2 files changed, 176 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/loader/arm64/xen_boot.c
> b/grub-core/loader/arm64/xen_boot.c
> index 2975a546e..64f685de6 100644
> --- a/grub-core/loader/arm64/xen_boot.c
> +++ b/grub-core/loader/arm64/xen_boot.c
> @@ -68,6 +68,56 @@ enum module_type
> };
> typedef enum module_type module_type_t;
>
> +static grub_guid_t load_file2_guid = GRUB_EFI_LOAD_FILE2_PROTOCOL_GUID;
> +static grub_guid_t device_path_guid = GRUB_EFI_DEVICE_PATH_GUID;
> +
> +static initrd_media_device_path_t initrd_lf2_device_path = {
> + {
> + {
> + GRUB_EFI_MEDIA_DEVICE_PATH_TYPE,
> + GRUB_EFI_VENDOR_MEDIA_DEVICE_PATH_SUBTYPE,
> + sizeof(grub_efi_vendor_media_device_path_t),
> + },
> + LINUX_EFI_INITRD_MEDIA_GUID
> + }, {
> + GRUB_EFI_END_DEVICE_PATH_TYPE,
> + GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE,
> + sizeof(grub_efi_device_path_t)
> + }
> +};
> +
> +static initrd_media_device_path_t kernel_lf2_device_path = {
> + {
> + {
> + GRUB_EFI_MEDIA_DEVICE_PATH_TYPE,
> + GRUB_EFI_VENDOR_MEDIA_DEVICE_PATH_SUBTYPE,
> + sizeof(grub_efi_vendor_media_device_path_t),
> + },
> + XEN_EFI_KERNEL_MEDIA_GUID
> + }, {
> + GRUB_EFI_END_DEVICE_PATH_TYPE,
> + GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE,
> + sizeof(grub_efi_device_path_t)
> + }
> +};
> +
> +typedef struct lf2
> +{
> + grub_efi_load_file2_t lf2;
> + grub_efi_handle_t handle;
> + initrd_media_device_path_t *media;
> +} lf2_t;
> +
> +static grub_efi_status_t __grub_efi_api
> +grub_efi_load_file2 (grub_efi_load_file2_t *this,
> + grub_efi_device_path_t *device_path,
> + grub_efi_boolean_t boot_policy,
> + grub_efi_uintn_t *buffer_size,
> + void *buffer);
> +static void lf2_init(lf2_t *lf2, initrd_media_device_path_t *media);
> +static grub_err_t lf2_start(lf2_t *lf2);
> +static void lf2_done(lf2_t *lf2);
> +
> struct xen_boot_binary
> {
> struct xen_boot_binary *next;
> @@ -80,6 +130,8 @@ struct xen_boot_binary
>
> char *cmdline;
> int cmdline_size;
> +
> + lf2_t lf2;
> };
>
> static grub_dl_t my_mod;
> @@ -289,6 +341,8 @@ single_binary_unload (struct xen_boot_binary *binary)
> binary->cmdline, binary->cmdline_size);
> }
>
> + lf2_done (&binary->lf2);
> +
> if (!binary->is_hypervisor)
> grub_list_remove (GRUB_AS_LIST (binary));
>
> @@ -393,6 +447,7 @@ grub_cmd_xen_module (grub_command_t cmd
> __attribute__((unused)),
> struct xen_boot_binary *module = NULL;
> grub_file_t file = 0;
> int nounzip = 0;
> + initrd_media_device_path_t *media;
>
> if (!argc)
> {
> @@ -427,6 +482,19 @@ grub_cmd_xen_module (grub_command_t cmd
> __attribute__((unused)),
>
> module->is_hypervisor = false;
> module->align = 4096;
> + switch (grub_list_length (GRUB_AS_LIST (module_head)))
> + {
> + case MODULE_IMAGE:
> + media = &kernel_lf2_device_path;
> + break;
> + case MODULE_INITRD:
> + media = &initrd_lf2_device_path;
> + break;
> + default:
> + media = NULL;
> + break;
Shouldn't this be a hard error, at least on x86_64, rather than silently
ignoring the extra modules?
In general, I'm not sure I like this interface since it is neither
scalable or flexible. i.e. adding a new module would require both Xen
and GRUB changes and it prevents certain combinations, e.g. booting a
combined Xen + kernel image while _only_ passing the initrd as a module.
Perhaps it is the best we can do without overcomplicating the code.
> + }
> + lf2_init(&module->lf2, media);
>
> grub_dprintf ("xen_loader", "Init module and node info\n");
>
> @@ -438,7 +506,10 @@ grub_cmd_xen_module (grub_command_t cmd
> __attribute__((unused)),
>
> xen_boot_binary_load (module, file, argc, argv);
> if (grub_errno == GRUB_ERR_NONE)
> - grub_list_push (GRUB_AS_LIST_P (&module_head), GRUB_AS_LIST (module));
> + {
> + grub_list_push (GRUB_AS_LIST_P (&module_head), GRUB_AS_LIST (module));
> + lf2_start (&module->lf2);
> + }
It's not directly related to this patch but isn't there a bug here where
xen_boot_binary_load() calls single_binary_unload() on error and then it
gets called again below causing a double free?
Ross
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
