From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= Subject: Re: [PATCH v2 1/3] arm64: Add Xen boot support file Date: Wed, 15 Jul 2015 18:18:45 +0200 Message-ID: <55A687E5.4070106@gmail.com> References: <=fu.wei@linaro.org> <1436777640-31871-1-git-send-email-fu.wei@linaro.org> <1436777640-31871-2-git-send-email-fu.wei@linaro.org> Reply-To: The development of GNU GRUB Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0099605781157508723==" Return-path: In-Reply-To: <1436777640-31871-2-git-send-email-fu.wei@linaro.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: grub-devel-bounces+gcbgd-grub-devel=m.gmane.org@gnu.org Sender: grub-devel-bounces+gcbgd-grub-devel=m.gmane.org@gnu.org To: fu.wei@linaro.org, grub-devel@gnu.org, arvidjaar@gmail.com Cc: xen-devel@lists.xensource.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, jcm@redhat.com, leif.lindholm@linaro.org, ryan.harkin@linaro.org, linaro-uefi@lists.linaro.org List-Id: xen-devel@lists.xenproject.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --===============0099605781157508723== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="DRBsWEM9gsvbjUR6a80bQufI8u8BME2SH" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --DRBsWEM9gsvbjUR6a80bQufI8u8BME2SH Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 13.07.2015 10:53, fu.wei@linaro.org wrote: > From: Fu Wei >=20 > This patch adds Xen boot support file: > grub-core/loader/arm64/xen_boot.c > include/grub/arm64/xen_boot.h >=20 > This patch also adds commands register code and hearder file into > grub-core/loader/arm64/linux.c >=20 > - This adds support for the Xen boot on ARM specification for arm64. > - The implementation for Xen is following : > http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/M= ultiboot Please don't refer to this protocol as multiboot anywhere in grub or around because it's NOT multiboot and we don't want to confuse those 2 protocols. > and xen/docs/misc/arm/device-tree/booting.txt in Xen source code. > - The multiboot/module commands have existed, > so we use xen_hypervisor/xen_module instead. > - This Xen boot support is built into linux module for aarch64. > - Adding this functionality to the existing "linux" module is for > reusing the existing code of devicetree. >=20 Please create separate module. Modules are dynamically linked. > Signed-off-by: Fu Wei > --- > grub-core/Makefile.core.def | 1 + > grub-core/loader/arm64/linux.c | 6 + > grub-core/loader/arm64/xen_boot.c | 615 ++++++++++++++++++++++++++++++= ++++++++ > include/grub/arm64/xen_boot.h | 115 +++++++ > 4 files changed, 737 insertions(+) > create mode 100644 grub-core/loader/arm64/xen_boot.c > create mode 100644 include/grub/arm64/xen_boot.h >=20 > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def > index a6101de..01f8261 100644 > --- a/grub-core/Makefile.core.def > +++ b/grub-core/Makefile.core.def > @@ -1659,6 +1659,7 @@ module =3D { > ia64_efi =3D loader/ia64/efi/linux.c; > arm =3D loader/arm/linux.c; > arm64 =3D loader/arm64/linux.c; > + arm64 =3D loader/arm64/xen_boot.c; > fdt =3D lib/fdt.c; > common =3D loader/linux.c; > common =3D lib/cmdline.c; > diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/li= nux.c > index 987f5b9..7ae9bde 100644 > --- a/grub-core/loader/arm64/linux.c > +++ b/grub-core/loader/arm64/linux.c > @@ -26,6 +26,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -477,6 +478,9 @@ GRUB_MOD_INIT (linux) > cmd_devicetree =3D > grub_register_command ("devicetree", grub_cmd_devicetree, 0, > N_("Load DTB file.")); > + > + grub_arm64_linux_register_xen_boot_command (mod, &loaded); > + > my_mod =3D mod; > } > =20 > @@ -485,4 +489,6 @@ GRUB_MOD_FINI (linux) > grub_unregister_command (cmd_linux); > grub_unregister_command (cmd_initrd); > grub_unregister_command (cmd_devicetree); > + > + grub_arm64_linux_unregister_xen_boot_command (); > } Not needed with separate module. > diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64= /xen_boot.c > new file mode 100644 > index 0000000..23bd00e > --- /dev/null > +++ b/grub-core/loader/arm64/xen_boot.c > @@ -0,0 +1,615 @@ > +/* > + * GRUB -- GRand Unified Bootloader > + * Copyright (C) 2014 Free Software Foundation, Inc. > + * > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published = by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see . > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +static grub_dl_t linux_mod; > +static int *loaded; > + > +static struct xen_boot_binary *xen_hypervisor; > +static struct xen_boot_binary *module_head; > +static const grub_size_t module_default_align[] =3D { > + MODULE_IMAGE_MIN_ALIGN, > + MODULE_INITRD_MIN_ALIGN, > + MODULE_OTHER_MIN_ALIGN, > + MODULE_CUSTOM_MIN_ALIGN > +}; > + > +static void *xen_boot_fdt; > +static const compat_string_struct_t default_compat_string[] =3D { > + FDT_COMPATIBLE (MODULE_IMAGE_COMPATIBLE), > + FDT_COMPATIBLE (MODULE_INITRD_COMPATIBLE), > + FDT_COMPATIBLE (MODULE_OTHER_COMPATIBLE) > +}; > + > + > +/* Parse all the options of xen_module command. For now, we support > + (1) --type > + (2) --nounzip > + We also set up the type of module in this function. > + If there are some "--type" options in the command line, > + we make a custom compatible stream in this function. */ > +static grub_err_t > +set_module_type (struct xen_boot_binary *module, int argc, char *argv[= ], > + int *file_name_index) > +{ > + char **compat_string_temp_array =3D > + (char **) grub_zalloc (sizeof (char *) * argc); > + static module_type_t default_type =3D MODULE_IMAGE; > + grub_size_t total_size =3D 0; > + int num_types =3D 0, i; > + char *temp =3D NULL; > + > + *file_name_index =3D 0; > + > + /* if there are some options we need to process. */ > + while (argc > 1 && !grub_strncmp (argv[0], "--", 2)) > + { > + if (!grub_strcmp (argv[0], "--type")) > + { > + module->node_info.type =3D MODULE_CUSTOM; > + ARG_SHIFT (argc, argv); > + total_size +=3D grub_strlen (argv[0]) + 1; > + compat_string_temp_array[num_types++] =3D argv[0]; > + ARG_SHIFT (argc, argv); > + (*file_name_index) +=3D 2; This (and subsequent) parsing is unecessarily complicated. Please create separate commands for different types > + } > + else if (!grub_strcmp (argv[0], "--nounzip")) > + { > + grub_file_filter_disable_compression (); > + ARG_SHIFT (argc, argv); > + (*file_name_index) +=3D 1; > + } > + else /* we can add more options process code here. */ > + { > + grub_dprintf ("xen_boot_loader", > + "Unknown option %s, skip.\n", argv[0]); > + ARG_SHIFT (argc, argv); > + (*file_name_index) +=3D 1; > + } > + } > + > + /* To prevent some wrong command lines using "--type" option */ > + if (!argc) > + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"))= ; > + > + /* For the default module type : > + The implementation is following := > + Each module will be given a default compatibility property > + based on the order in which the modules are added. > + The 1st module: compatible =3D "multiboot,kernel", "multiboot,mod= ule" > + The 2nd module: compatible =3D "multiboot,ramdisk", "multiboot,mo= dule" > + All subsequent modules: compatible =3D "multiboot,module" > + But this order will NOT be interfered with "--type"(MODULE_CUSTOM= ) > + For more detail, please refer to: > + http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/M= ultiboot */ > + if (module->node_info.type !=3D MODULE_CUSTOM) > + { > + /* the module type is set by the load order */ > + module->node_info.type =3D default_type; Please don't make it order-dependent more than necessarry. > + switch (default_type) > + { > + case MODULE_IMAGE: > + default_type =3D MODULE_INITRD; > + break; > + > + case MODULE_INITRD: > + default_type =3D MODULE_OTHER; > + break; > + > + case MODULE_OTHER: > + break; > + > + default: > + default_type =3D MODULE_IMAGE; /* error, reset the type */ > + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument")); > + } > + } > + else > + { > + /* the module type is set by "--type"(MODULE_CUSTOM) */ > + module->node_info.compat_string =3D temp =3D > + (char *) grub_zalloc (total_size); > + module->node_info.compat_string_size =3D total_size; > + for (i =3D 0; num_types > 0; num_types--, i++, temp++) > + { > + grub_strcpy (temp, compat_string_temp_array[i]); > + temp +=3D grub_strlen (compat_string_temp_array[i]); > + } > + } > + > + grub_free (compat_string_temp_array); > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +prepare_xen_hypervisor_params (void) > +{ > + int chosen_node =3D 0; > + int retval; > + > + xen_boot_fdt =3D grub_linux_get_fdt (); > + if (!xen_boot_fdt) > + return grub_error (GRUB_ERR_BAD_OS, "failed to get FDT"); > + > + chosen_node =3D grub_fdt_find_subnode (xen_boot_fdt, 0, "chosen"); > + if (chosen_node < 0) > + chosen_node =3D grub_fdt_add_subnode (xen_boot_fdt, 0, "chosen"); > + if (chosen_node < 1) > + return grub_error (GRUB_ERR_BAD_OS, "failed to get chosen node in = FDT"); > + BAD_OS means that OS images are invalid. ERR_IO is a generic error for such cases. > + grub_dprintf ("xen_boot_loader", > + "Xen Hypervisor cmdline : %s @ %p size:%d\n", > + xen_hypervisor->cmdline, xen_hypervisor->cmdline, > + xen_hypervisor->cmdline_size); > + No need for "boot_". xen_loader is fine and mre consistent. > + retval =3D grub_fdt_set_prop (xen_boot_fdt, chosen_node, "bootargs",= > + xen_hypervisor->cmdline, > + xen_hypervisor->cmdline_size); > + if (retval) > + return grub_error (GRUB_ERR_BAD_OS, "failed to install/update FDT"= ); > + ditto > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +prepare_xen_module_params (struct xen_boot_binary *module) > +{ > + int retval, chosen_node =3D 0, module_node =3D 0; > + char module_name[FDT_NODE_NAME_MAX_SIZE]; > + > + retval =3D grub_snprintf (module_name, FDT_NODE_NAME_MAX_SIZE, "modu= le@%lx", > + xen_boot_address_align (module->start, > + module->align)); > + grub_dprintf ("xen_boot_loader", "Module node name %s \n", module_na= me); > + > + if (retval < (int) sizeof ("module@")) > + return grub_error (GRUB_ERR_BAD_OS, N_("failed to get FDT")); > + > + chosen_node =3D grub_fdt_find_subnode (xen_boot_fdt, 0, "chosen"); > + if (chosen_node < 0) > + chosen_node =3D grub_fdt_add_subnode (xen_boot_fdt, 0, "chosen"); > + if (chosen_node < 1) > + return grub_error (GRUB_ERR_BAD_OS, "failed to get chosen node in = FDT"); > + > + module_node =3D > + grub_fdt_find_subnode (xen_boot_fdt, chosen_node, module_name); > + if (module_node < 0) > + module_node =3D > + grub_fdt_add_subnode (xen_boot_fdt, chosen_node, module_name); > + > + retval =3D grub_fdt_set_prop (xen_boot_fdt, module_node, "compatible= ", > + module->node_info.compat_string, > + (grub_uint32_t) module->node_info. > + compat_string_size); > + if (retval) > + return grub_error (GRUB_ERR_BAD_OS, N_("failed to update FDT")); > + > + grub_dprintf ("xen_boot_loader", "Module %s compatible =3D %s size =3D= 0x%lx\n", > + module->name, module->node_info.compat_string, > + module->node_info.compat_string_size); > + > + retval =3D grub_fdt_set_reg64 (xen_boot_fdt, module_node, > + xen_boot_address_align (module->start, > + module->align), > + module->size); > + if (retval) > + return grub_error (GRUB_ERR_BAD_OS, N_("failed to update FDT")); > + > + if (module->cmdline && module->cmdline_size > 0) > + { > + grub_dprintf ("xen_boot_loader", > + "Module %s cmdline : %s @ %p size:%d\n", module->name, > + module->cmdline, module->cmdline, module->cmdline_size); > + > + retval =3D grub_fdt_set_prop (xen_boot_fdt, module_node, "bootar= gs", > + module->cmdline, module->cmdline_size + 1); > + if (retval) > + return grub_error (GRUB_ERR_BAD_OS, "failed to update FDT"); > + } > + else > + { > + grub_dprintf ("xen_boot_loader", "Module %s has not bootargs!\n"= , > + module->name); > + } > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +install_all_params (void) > +{ > + grub_efi_guid_t fdt_guid =3D GRUB_EFI_DEVICE_TREE_GUID; > + grub_efi_boot_services_t *b; > + grub_efi_status_t status; > + > + b =3D grub_efi_system_table->boot_services; > + status =3D b->install_configuration_table (&fdt_guid, xen_boot_fdt);= > + if (status !=3D GRUB_EFI_SUCCESS) > + return grub_error (GRUB_ERR_BAD_OS, "failed to install FDT"); > + > + grub_dprintf ("xen_boot_loader", > + "Installed/updated FDT configuration table @ %p\n", > + xen_boot_fdt); > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +clean_all_params (void) > +{ > + if (xen_boot_fdt) > + { > + grub_efi_free_pages ((grub_efi_physical_address_t) xen_boot_fdt,= > + BYTES_TO_PAGES (grub_fdt_get_totalsize > + (xen_boot_fdt))); > + xen_boot_fdt =3D NULL; > + } > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +finalize_params_xen_boot (void) > +{ > + struct xen_boot_binary *module; > + > + if (xen_hypervisor) > + { > + if (prepare_xen_hypervisor_params () !=3D GRUB_ERR_NONE) > + goto fail; > + } > + else > + { > + grub_dprintf ("xen_boot_loader", > + "Failed to get Xen Hypervisor info!\n"); > + goto fail; > + } > + > + /* Set module params info */ > + FOR_LIST_ELEMENTS (module, module_head) > + { > + if (module->start && module->size > 0) > + { > + grub_dprintf ("xen_boot_loader", "Module %s @ 0x%lx size:0x%lx\n", > + module->name, > + xen_boot_address_align (module->start, module->align), > + module->size); > + if (prepare_xen_module_params (module) !=3D GRUB_ERR_NONE) > + goto fail; > + } > + else > + { > + grub_dprintf ("xen_boot_loader", "Module info error: %s!\n", > + module->name); > + goto fail; > + } > + } > + > + if (install_all_params () =3D=3D GRUB_ERR_NONE) > + return GRUB_ERR_NONE; > + > +fail: > + clean_all_params (); > + > + return grub_error (GRUB_ERR_BAD_OS, "failed to install/update FDT");= > +} > + > + > +static grub_err_t > +xen_boot (void) > +{ > + if (finalize_params_xen_boot () !=3D GRUB_ERR_NONE) > + return grub_errno; > + Better use err =3D finalize_params_xen_boot (); if (err) return err; > + return grub_arm64_uefi_boot_image (xen_hypervisor->start, > + xen_hypervisor->size, > + xen_hypervisor->cmdline); > +} > + > +static void > +single_binary_unload (struct xen_boot_binary *binary) > +{ Just put if (!binary) return; It will save a lot of if's. > + if (binary && binary->start && binary->size > 0) > + { > + grub_efi_free_pages ((grub_efi_physical_address_t) binary->start= , > + BYTES_TO_PAGES (binary->size + binary->align)); > + } > + > + if (binary && binary->cmdline && binary->cmdline_size > 0) > + { > + grub_free (binary->cmdline); > + grub_dprintf ("xen_boot_loader", > + "Module %s cmdline memory free @ %p size: %d\n", > + binary->name, binary->cmdline, binary->cmdline_size); > + } > + > + if (binary) > + { > + if (binary->node_info.type =3D=3D MODULE_CUSTOM) > + grub_free ((void *) binary->node_info.compat_string); > + if (grub_strcmp (binary->name, XEN_HYPERVISOR_NAME)) > + grub_list_remove (GRUB_AS_LIST (binary)); > + grub_dprintf ("xen_boot_loader", > + "Module %s struct memory free @ %p size: 0x%lx\n", > + binary->name, binary, sizeof (binary)); > + grub_free (binary); > + } > + > + return; > +} > + > +static void > +all_binaries_unload (void) > +{ > + struct xen_boot_binary *binary; > + > + FOR_LIST_ELEMENTS (binary, module_head) > + { > + single_binary_unload (binary); > + } > + > + if (xen_hypervisor) > + single_binary_unload (xen_hypervisor); > + > + return; > +} > + > +static grub_err_t > +xen_unload (void) > +{ > + *loaded =3D 0; > + all_binaries_unload (); > + clean_all_params (); > + grub_dl_unref (linux_mod); > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +xen_boot_binary_load (struct xen_boot_binary *binary, grub_file_t file= , > + int argc, char *argv[]) > +{ > + binary->size =3D grub_file_size (file); > + grub_dprintf ("xen_boot_loader", "Xen_boot %s file size: 0x%lx\n", > + binary->name, binary->size); > + > + binary->start =3D (grub_addr_t) grub_efi_allocate_pages (0, > + (BYTES_TO_PAGES > + (binary->size + > + binary->align))); > + if (!binary->start) > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory")); > + > + grub_dprintf ("xen_boot_loader", "Xen_boot %s numpages: 0x%lx\n", > + binary->name, BYTES_TO_PAGES (binary->size + binary->align)); > + > + if (grub_file_read (file, (void *) xen_boot_address_align (binary->s= tart, > + binary->align), > + binary->size) < (grub_ssize_t) binary->size) > + { We use !=3D throughout. It's safer. > + single_binary_unload (binary); > + return grub_error (GRUB_ERR_BAD_OS, N_("premature end of file %s= "), > + argv[0]); > + } > + > + /* Skip the xen_boot binary file name */ > + ARG_SHIFT (argc, argv); > + There shouldn't be any need for shifting. Just use argc - 1 and argv + 1 > + if (argc > 0) > + { > + binary->cmdline_size =3D grub_loader_cmdline_size (argc, argv); > + binary->cmdline =3D grub_zalloc (binary->cmdline_size); > + if (!binary->cmdline) > + { > + single_binary_unload (binary); > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory")); > + } > + grub_create_loader_cmdline (argc, argv, binary->cmdline, > + binary->cmdline_size); > + grub_dprintf ("xen_boot_loader", > + "Xen_boot %s cmdline @ %p %s, size: %d\n", binary->name, > + binary->cmdline, binary->cmdline, binary->cmdline_size); > + } > + else > + { > + binary->cmdline_size =3D 0; > + binary->cmdline =3D NULL; > + } > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +grub_cmd_xen_module (grub_command_t cmd __attribute__ ((unused)), > + int argc, char *argv[]) > +{ > + > + struct xen_boot_binary *module =3D NULL; > + int file_name_index =3D 0; > + grub_file_t file =3D 0; > + > + if (!argc) > + { > + grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected")); > + goto fail; > + } > + > + if (!*loaded) > + { > + grub_error (GRUB_ERR_BAD_ARGUMENT, > + N_("you need to load the Xen Hypervisor first")); > + goto fail; > + } > + > + module =3D > + (struct xen_boot_binary *) grub_zalloc (sizeof (struct xen_boot_bi= nary)); > + if (!module) > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory")); > + Just return grub_errno; zalloc already calls grub_error > + /* process all the options and get module type */ > + if (set_module_type (module, argc, argv, &file_name_index) !=3D GRUB= _ERR_NONE) > + goto fail; > + switch (module->node_info.type) > + { > + case MODULE_IMAGE: > + case MODULE_INITRD: > + case MODULE_OTHER: > + module->node_info.compat_string =3D > + default_compat_string[module->node_info.type].compat_string; > + module->node_info.compat_string_size =3D > + default_compat_string[module->node_info.type].size; > + break; > + > + case MODULE_CUSTOM: > + /* we have set the node_info in set_module_type */ > + break; > + > + default: > + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument")= ); > + } > + module->name =3D module->node_info.compat_string; > + module->align =3D module_default_align[module->node_info.type]; > + > + grub_dprintf ("xen_boot_loader", "Init %s module and node info:\n" > + "compatible %s\ncompat_string_size 0x%lx\n", > + module->name, module->node_info.compat_string, > + module->node_info.compat_string_size); > + > + file =3D grub_file_open (argv[file_name_index]); > + if (!file) > + goto fail; > + > + grub_errno =3D xen_boot_binary_load (module, file, argc - file_name_= index, > + argv + file_name_index); > + When you call grub_error. grub_errno is already set > + if (grub_errno =3D=3D GRUB_ERR_NONE) > + grub_list_push (GRUB_AS_LIST_P (&module_head), GRUB_AS_LIST (modul= e)); > + > +fail: > + if (file) > + grub_file_close (file); > + if (grub_errno !=3D GRUB_ERR_NONE) > + single_binary_unload (module); > + > + return grub_errno; > +} > + > +static grub_err_t > +grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)), > + int argc, char *argv[]) > +{ > + struct xen_hypervisor_header sh; > + grub_file_t file =3D NULL; > + > + grub_dl_ref (linux_mod); > + > + if (!argc) > + { > + grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected")); > + goto fail; > + } > + > + /* For now, we don't support any option in xen_hypervisor command. > + If there are some options, we skip them. */ > + while (argc > 1 && !grub_strncmp (argv[0], "--", 2)) > + { > + grub_dprintf ("xen_boot_loader", "Unknown option %s, skip.\n", a= rgv[0]); > + ARG_SHIFT (argc, argv); > + } > + Why do you need this? Just delete it. > + file =3D grub_file_open (argv[0]); > + if (!file) > + goto fail; > + > + if (grub_file_read (file, &sh, sizeof (sh)) < (long) sizeof (sh)) > + goto fail; > + if (grub_arm64_uefi_check_image > + ((struct grub_arm64_linux_kernel_header *) &sh) !=3D GRUB_ERR_NO= NE) > + goto fail; > + grub_file_seek (file, 0); > + > + grub_loader_unset (); > + This is implicit in loader_set. Please add a comment why it needs to be explicit. I suppose it's to avoid unloading oneself. > + xen_hypervisor =3D > + (struct xen_boot_binary *) grub_zalloc (sizeof (struct xen_boot_bi= nary)); > + if (!xen_hypervisor) > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory")); > + Ditto > + xen_hypervisor->name =3D XEN_HYPERVISOR_NAME; > + xen_hypervisor->align =3D (grub_size_t) sh.optional_header.section_a= lignment; > + > + grub_errno =3D xen_boot_binary_load (xen_hypervisor, file, argc, arg= v); > + if (grub_errno =3D=3D GRUB_ERR_NONE) > + { > + grub_loader_set (xen_boot, xen_unload, 0); > + *loaded =3D 1; > + } > + > +fail: > + if (file) > + grub_file_close (file); > + if (grub_errno !=3D GRUB_ERR_NONE) > + { > + *loaded =3D 0; > + all_binaries_unload (); > + grub_dl_unref (linux_mod); > + } > + > + return grub_errno; > +} > + > +static grub_command_t cmd_xen_hypervisor, cmd_xen_module; > + > +void > +grub_arm64_linux_register_xen_boot_command (grub_dl_t mod, int *linux_= loaded) > +{ > + cmd_xen_hypervisor =3D > + grub_register_command ("xen_hypervisor", grub_cmd_xen_hypervisor, = 0, > + N_("Load a xen hypervisor.")); > + cmd_xen_module =3D > + grub_register_command ("xen_module", grub_cmd_xen_module, 0, > + N_("Load a xen module.")); > + linux_mod =3D mod; > + loaded =3D linux_loaded; > +} > + > +void > +grub_arm64_linux_unregister_xen_boot_command (void) > +{ > + grub_unregister_command (cmd_xen_hypervisor); > + grub_unregister_command (cmd_xen_module); > +} > diff --git a/include/grub/arm64/xen_boot.h b/include/grub/arm64/xen_boo= t.h > new file mode 100644 > index 0000000..8e8f6cb > --- /dev/null > +++ b/include/grub/arm64/xen_boot.h > @@ -0,0 +1,115 @@ > +/* > + * xen_boot.h - Xen boot header file for Xen boot via FDT > + * on AArch64 architecture. > + * Copyright (C) 2014 Free Software Foundation, Inc. > + * > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published = by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see . > + */ > + > +#ifndef XEN_BOOT_HEADER > +#define XEN_BOOT_HEADER 1 > + > +#include > +#include > +#include /* required by struct xen_hypervisor_header= */ > + This file doesn't really look like having anything reusable. Please put it directly into .c > +#define XEN_HYPERVISOR_NAME "xen_hypervisor" > + > +#define MODULE_DEFAULT_ALIGN (0x0) > +#define MODULE_IMAGE_MIN_ALIGN MODULE_DEFAULT_ALIGN > +#define MODULE_INITRD_MIN_ALIGN MODULE_DEFAULT_ALIGN > +#define MODULE_OTHER_MIN_ALIGN MODULE_DEFAULT_ALIGN > +#define MODULE_CUSTOM_MIN_ALIGN MODULE_DEFAULT_ALIGN > + > +#define MODULE_IMAGE_COMPATIBLE "multiboot,kernel\0multiboot,module" > +#define MODULE_INITRD_COMPATIBLE "multiboot,ramdisk\0multiboot,module= " > +#define MODULE_OTHER_COMPATIBLE "multiboot,module" > + > +/* This maximum size is defined in Power.org ePAPR V1.1 > + * https://www.power.org/documentation/epapr-version-1-1/ > + * 2.2.1.1 Node Name Requirements > + * node-name@unit-address > + * 31 + 1(@) + 16(64bit address in hex format) + 1(\0) =3D 49 > + */ > +#define FDT_NODE_NAME_MAX_SIZE (49) > + > +#define ARG_SHIFT(argc, argv) \ > + do { \ > + (argc)--; \ > + (argv)++; \ > + } while (0) > + > +struct compat_string_struct > +{ > + grub_size_t size; > + const char *compat_string; > +}; > +typedef struct compat_string_struct compat_string_struct_t; > +#define FDT_COMPATIBLE(x) {.size =3D sizeof(x), .compat_string =3D (x)= } > + > +enum module_type > +{ > + MODULE_IMAGE, > + MODULE_INITRD, > + MODULE_OTHER, > + MODULE_CUSTOM > +}; > +typedef enum module_type module_type_t; > + > +struct fdt_node_info > +{ > + module_type_t type; > + > + const char *compat_string; > + grub_size_t compat_string_size; > +}; > + > +struct xen_hypervisor_header > +{ > + struct grub_arm64_linux_kernel_header efi_head; > + > + /* This is always PE\0\0. */ > + grub_uint8_t signature[GRUB_PE32_SIGNATURE_SIZE]; > + /* The COFF file header. */ > + struct grub_pe32_coff_header coff_header; > + /* The Optional header. */ > + struct grub_pe64_optional_header optional_header; > +}; > + > +struct xen_boot_binary > +{ > + struct xen_boot_binary *next; > + struct xen_boot_binary **prev; > + const char *name; > + > + grub_addr_t start; > + grub_size_t size; > + grub_size_t align; > + > + char *cmdline; > + int cmdline_size; > + > + struct fdt_node_info node_info; > +}; > + > +void grub_arm64_linux_register_xen_boot_command (grub_dl_t mod, int *l= oaded); > +void grub_arm64_linux_unregister_xen_boot_command (void); > + > +static __inline grub_addr_t > +xen_boot_address_align (grub_addr_t start, grub_size_t align) > +{ > + return (align ? (ALIGN_UP (start, align)) : start); > +} > + > +#endif /* ! XEN_BOOT_HEADER */ >=20 --DRBsWEM9gsvbjUR6a80bQufI8u8BME2SH Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iF4EAREKAAYFAlWmh+UACgkQmBXlbbo5nOtrVAD7B1kQOgd1JXTg2VvLwIrjYceZ mbey/wVkKxGWTkR/JYAA/igBiAamNM0d7HQ+RPaXOkM+DDWMCBnchlLyrfXnMM+q =6FHH -----END PGP SIGNATURE----- --DRBsWEM9gsvbjUR6a80bQufI8u8BME2SH-- --===============0099605781157508723== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel --===============0099605781157508723==-- From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ZFPOc-0004th-6c for mharc-grub-devel@gnu.org; Wed, 15 Jul 2015 12:18:58 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40243) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFPOX-0004sh-EJ for grub-devel@gnu.org; Wed, 15 Jul 2015 12:18:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFPOS-0007pL-35 for grub-devel@gnu.org; Wed, 15 Jul 2015 12:18:53 -0400 Received: from mail-wi0-x236.google.com ([2a00:1450:400c:c05::236]:35581) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFPOR-0007pD-Kc for grub-devel@gnu.org; Wed, 15 Jul 2015 12:18:48 -0400 Received: by wiga1 with SMTP id a1so4612226wig.0 for ; Wed, 15 Jul 2015 09:18:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type; bh=74v7YJ5uGmahifVX80I6aq09tWSlK+cRF+ogUyeXv4Q=; b=PrMvD+vlLS6jPZPx/2WW3FxNUjVEcIzfHZn43K6VhxzBNPKieTlHqhTbipZHeajIDj 0rNf9PNEJeOwAMl7eSOwbX56ouhFqPwTzOzZLlVLrrvyLHbn1aZbZSzXQpWFjcBpI+Ib EQwOv2Z0rCq1znb72YMKJoCjCQV/RnNHAEaWshmpUEy3j/amybt89CiDSbNfAFoKVvrS gUT0n42/S2wI64UOF+gncjV+kdNuelBZuWe4/D76iPyP8X4dYtbtkv/n2lN8JLQnIN3A ssJzHjk8XVI4swWt25eTO5673zzyDlbBkzn0KgqgOIuRrLEli9QEYzOcPXoMGQJ11fjN 9XVA== X-Received: by 10.194.235.66 with SMTP id uk2mr9837855wjc.62.1436977126819; Wed, 15 Jul 2015 09:18:46 -0700 (PDT) Received: from ?IPv6:2620:0:105f:fd00:863a:4bff:fe50:abc4? ([2620:0:105f:fd00:863a:4bff:fe50:abc4]) by smtp.gmail.com with ESMTPSA id s10sm697318wik.6.2015.07.15.09.18.45 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 15 Jul 2015 09:18:46 -0700 (PDT) Message-ID: <55A687E5.4070106@gmail.com> Date: Wed, 15 Jul 2015 18:18:45 +0200 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.7.0 MIME-Version: 1.0 To: fu.wei@linaro.org, grub-devel@gnu.org, arvidjaar@gmail.com Subject: Re: [PATCH v2 1/3] arm64: Add Xen boot support file References: <=fu.wei@linaro.org> <1436777640-31871-1-git-send-email-fu.wei@linaro.org> <1436777640-31871-2-git-send-email-fu.wei@linaro.org> In-Reply-To: <1436777640-31871-2-git-send-email-fu.wei@linaro.org> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="DRBsWEM9gsvbjUR6a80bQufI8u8BME2SH" X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:400c:c05::236 Cc: xen-devel@lists.xensource.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, jcm@redhat.com, leif.lindholm@linaro.org, ryan.harkin@linaro.org, linaro-uefi@lists.linaro.org X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Jul 2015 16:18:56 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --DRBsWEM9gsvbjUR6a80bQufI8u8BME2SH Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 13.07.2015 10:53, fu.wei@linaro.org wrote: > From: Fu Wei >=20 > This patch adds Xen boot support file: > grub-core/loader/arm64/xen_boot.c > include/grub/arm64/xen_boot.h >=20 > This patch also adds commands register code and hearder file into > grub-core/loader/arm64/linux.c >=20 > - This adds support for the Xen boot on ARM specification for arm64. > - The implementation for Xen is following : > http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/M= ultiboot Please don't refer to this protocol as multiboot anywhere in grub or around because it's NOT multiboot and we don't want to confuse those 2 protocols. > and xen/docs/misc/arm/device-tree/booting.txt in Xen source code. > - The multiboot/module commands have existed, > so we use xen_hypervisor/xen_module instead. > - This Xen boot support is built into linux module for aarch64. > - Adding this functionality to the existing "linux" module is for > reusing the existing code of devicetree. >=20 Please create separate module. Modules are dynamically linked. > Signed-off-by: Fu Wei > --- > grub-core/Makefile.core.def | 1 + > grub-core/loader/arm64/linux.c | 6 + > grub-core/loader/arm64/xen_boot.c | 615 ++++++++++++++++++++++++++++++= ++++++++ > include/grub/arm64/xen_boot.h | 115 +++++++ > 4 files changed, 737 insertions(+) > create mode 100644 grub-core/loader/arm64/xen_boot.c > create mode 100644 include/grub/arm64/xen_boot.h >=20 > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def > index a6101de..01f8261 100644 > --- a/grub-core/Makefile.core.def > +++ b/grub-core/Makefile.core.def > @@ -1659,6 +1659,7 @@ module =3D { > ia64_efi =3D loader/ia64/efi/linux.c; > arm =3D loader/arm/linux.c; > arm64 =3D loader/arm64/linux.c; > + arm64 =3D loader/arm64/xen_boot.c; > fdt =3D lib/fdt.c; > common =3D loader/linux.c; > common =3D lib/cmdline.c; > diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/li= nux.c > index 987f5b9..7ae9bde 100644 > --- a/grub-core/loader/arm64/linux.c > +++ b/grub-core/loader/arm64/linux.c > @@ -26,6 +26,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -477,6 +478,9 @@ GRUB_MOD_INIT (linux) > cmd_devicetree =3D > grub_register_command ("devicetree", grub_cmd_devicetree, 0, > N_("Load DTB file.")); > + > + grub_arm64_linux_register_xen_boot_command (mod, &loaded); > + > my_mod =3D mod; > } > =20 > @@ -485,4 +489,6 @@ GRUB_MOD_FINI (linux) > grub_unregister_command (cmd_linux); > grub_unregister_command (cmd_initrd); > grub_unregister_command (cmd_devicetree); > + > + grub_arm64_linux_unregister_xen_boot_command (); > } Not needed with separate module. > diff --git a/grub-core/loader/arm64/xen_boot.c b/grub-core/loader/arm64= /xen_boot.c > new file mode 100644 > index 0000000..23bd00e > --- /dev/null > +++ b/grub-core/loader/arm64/xen_boot.c > @@ -0,0 +1,615 @@ > +/* > + * GRUB -- GRand Unified Bootloader > + * Copyright (C) 2014 Free Software Foundation, Inc. > + * > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published = by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see . > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +static grub_dl_t linux_mod; > +static int *loaded; > + > +static struct xen_boot_binary *xen_hypervisor; > +static struct xen_boot_binary *module_head; > +static const grub_size_t module_default_align[] =3D { > + MODULE_IMAGE_MIN_ALIGN, > + MODULE_INITRD_MIN_ALIGN, > + MODULE_OTHER_MIN_ALIGN, > + MODULE_CUSTOM_MIN_ALIGN > +}; > + > +static void *xen_boot_fdt; > +static const compat_string_struct_t default_compat_string[] =3D { > + FDT_COMPATIBLE (MODULE_IMAGE_COMPATIBLE), > + FDT_COMPATIBLE (MODULE_INITRD_COMPATIBLE), > + FDT_COMPATIBLE (MODULE_OTHER_COMPATIBLE) > +}; > + > + > +/* Parse all the options of xen_module command. For now, we support > + (1) --type > + (2) --nounzip > + We also set up the type of module in this function. > + If there are some "--type" options in the command line, > + we make a custom compatible stream in this function. */ > +static grub_err_t > +set_module_type (struct xen_boot_binary *module, int argc, char *argv[= ], > + int *file_name_index) > +{ > + char **compat_string_temp_array =3D > + (char **) grub_zalloc (sizeof (char *) * argc); > + static module_type_t default_type =3D MODULE_IMAGE; > + grub_size_t total_size =3D 0; > + int num_types =3D 0, i; > + char *temp =3D NULL; > + > + *file_name_index =3D 0; > + > + /* if there are some options we need to process. */ > + while (argc > 1 && !grub_strncmp (argv[0], "--", 2)) > + { > + if (!grub_strcmp (argv[0], "--type")) > + { > + module->node_info.type =3D MODULE_CUSTOM; > + ARG_SHIFT (argc, argv); > + total_size +=3D grub_strlen (argv[0]) + 1; > + compat_string_temp_array[num_types++] =3D argv[0]; > + ARG_SHIFT (argc, argv); > + (*file_name_index) +=3D 2; This (and subsequent) parsing is unecessarily complicated. Please create separate commands for different types > + } > + else if (!grub_strcmp (argv[0], "--nounzip")) > + { > + grub_file_filter_disable_compression (); > + ARG_SHIFT (argc, argv); > + (*file_name_index) +=3D 1; > + } > + else /* we can add more options process code here. */ > + { > + grub_dprintf ("xen_boot_loader", > + "Unknown option %s, skip.\n", argv[0]); > + ARG_SHIFT (argc, argv); > + (*file_name_index) +=3D 1; > + } > + } > + > + /* To prevent some wrong command lines using "--type" option */ > + if (!argc) > + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"))= ; > + > + /* For the default module type : > + The implementation is following := > + Each module will be given a default compatibility property > + based on the order in which the modules are added. > + The 1st module: compatible =3D "multiboot,kernel", "multiboot,mod= ule" > + The 2nd module: compatible =3D "multiboot,ramdisk", "multiboot,mo= dule" > + All subsequent modules: compatible =3D "multiboot,module" > + But this order will NOT be interfered with "--type"(MODULE_CUSTOM= ) > + For more detail, please refer to: > + http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/M= ultiboot */ > + if (module->node_info.type !=3D MODULE_CUSTOM) > + { > + /* the module type is set by the load order */ > + module->node_info.type =3D default_type; Please don't make it order-dependent more than necessarry. > + switch (default_type) > + { > + case MODULE_IMAGE: > + default_type =3D MODULE_INITRD; > + break; > + > + case MODULE_INITRD: > + default_type =3D MODULE_OTHER; > + break; > + > + case MODULE_OTHER: > + break; > + > + default: > + default_type =3D MODULE_IMAGE; /* error, reset the type */ > + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument")); > + } > + } > + else > + { > + /* the module type is set by "--type"(MODULE_CUSTOM) */ > + module->node_info.compat_string =3D temp =3D > + (char *) grub_zalloc (total_size); > + module->node_info.compat_string_size =3D total_size; > + for (i =3D 0; num_types > 0; num_types--, i++, temp++) > + { > + grub_strcpy (temp, compat_string_temp_array[i]); > + temp +=3D grub_strlen (compat_string_temp_array[i]); > + } > + } > + > + grub_free (compat_string_temp_array); > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +prepare_xen_hypervisor_params (void) > +{ > + int chosen_node =3D 0; > + int retval; > + > + xen_boot_fdt =3D grub_linux_get_fdt (); > + if (!xen_boot_fdt) > + return grub_error (GRUB_ERR_BAD_OS, "failed to get FDT"); > + > + chosen_node =3D grub_fdt_find_subnode (xen_boot_fdt, 0, "chosen"); > + if (chosen_node < 0) > + chosen_node =3D grub_fdt_add_subnode (xen_boot_fdt, 0, "chosen"); > + if (chosen_node < 1) > + return grub_error (GRUB_ERR_BAD_OS, "failed to get chosen node in = FDT"); > + BAD_OS means that OS images are invalid. ERR_IO is a generic error for such cases. > + grub_dprintf ("xen_boot_loader", > + "Xen Hypervisor cmdline : %s @ %p size:%d\n", > + xen_hypervisor->cmdline, xen_hypervisor->cmdline, > + xen_hypervisor->cmdline_size); > + No need for "boot_". xen_loader is fine and mre consistent. > + retval =3D grub_fdt_set_prop (xen_boot_fdt, chosen_node, "bootargs",= > + xen_hypervisor->cmdline, > + xen_hypervisor->cmdline_size); > + if (retval) > + return grub_error (GRUB_ERR_BAD_OS, "failed to install/update FDT"= ); > + ditto > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +prepare_xen_module_params (struct xen_boot_binary *module) > +{ > + int retval, chosen_node =3D 0, module_node =3D 0; > + char module_name[FDT_NODE_NAME_MAX_SIZE]; > + > + retval =3D grub_snprintf (module_name, FDT_NODE_NAME_MAX_SIZE, "modu= le@%lx", > + xen_boot_address_align (module->start, > + module->align)); > + grub_dprintf ("xen_boot_loader", "Module node name %s \n", module_na= me); > + > + if (retval < (int) sizeof ("module@")) > + return grub_error (GRUB_ERR_BAD_OS, N_("failed to get FDT")); > + > + chosen_node =3D grub_fdt_find_subnode (xen_boot_fdt, 0, "chosen"); > + if (chosen_node < 0) > + chosen_node =3D grub_fdt_add_subnode (xen_boot_fdt, 0, "chosen"); > + if (chosen_node < 1) > + return grub_error (GRUB_ERR_BAD_OS, "failed to get chosen node in = FDT"); > + > + module_node =3D > + grub_fdt_find_subnode (xen_boot_fdt, chosen_node, module_name); > + if (module_node < 0) > + module_node =3D > + grub_fdt_add_subnode (xen_boot_fdt, chosen_node, module_name); > + > + retval =3D grub_fdt_set_prop (xen_boot_fdt, module_node, "compatible= ", > + module->node_info.compat_string, > + (grub_uint32_t) module->node_info. > + compat_string_size); > + if (retval) > + return grub_error (GRUB_ERR_BAD_OS, N_("failed to update FDT")); > + > + grub_dprintf ("xen_boot_loader", "Module %s compatible =3D %s size =3D= 0x%lx\n", > + module->name, module->node_info.compat_string, > + module->node_info.compat_string_size); > + > + retval =3D grub_fdt_set_reg64 (xen_boot_fdt, module_node, > + xen_boot_address_align (module->start, > + module->align), > + module->size); > + if (retval) > + return grub_error (GRUB_ERR_BAD_OS, N_("failed to update FDT")); > + > + if (module->cmdline && module->cmdline_size > 0) > + { > + grub_dprintf ("xen_boot_loader", > + "Module %s cmdline : %s @ %p size:%d\n", module->name, > + module->cmdline, module->cmdline, module->cmdline_size); > + > + retval =3D grub_fdt_set_prop (xen_boot_fdt, module_node, "bootar= gs", > + module->cmdline, module->cmdline_size + 1); > + if (retval) > + return grub_error (GRUB_ERR_BAD_OS, "failed to update FDT"); > + } > + else > + { > + grub_dprintf ("xen_boot_loader", "Module %s has not bootargs!\n"= , > + module->name); > + } > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +install_all_params (void) > +{ > + grub_efi_guid_t fdt_guid =3D GRUB_EFI_DEVICE_TREE_GUID; > + grub_efi_boot_services_t *b; > + grub_efi_status_t status; > + > + b =3D grub_efi_system_table->boot_services; > + status =3D b->install_configuration_table (&fdt_guid, xen_boot_fdt);= > + if (status !=3D GRUB_EFI_SUCCESS) > + return grub_error (GRUB_ERR_BAD_OS, "failed to install FDT"); > + > + grub_dprintf ("xen_boot_loader", > + "Installed/updated FDT configuration table @ %p\n", > + xen_boot_fdt); > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +clean_all_params (void) > +{ > + if (xen_boot_fdt) > + { > + grub_efi_free_pages ((grub_efi_physical_address_t) xen_boot_fdt,= > + BYTES_TO_PAGES (grub_fdt_get_totalsize > + (xen_boot_fdt))); > + xen_boot_fdt =3D NULL; > + } > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +finalize_params_xen_boot (void) > +{ > + struct xen_boot_binary *module; > + > + if (xen_hypervisor) > + { > + if (prepare_xen_hypervisor_params () !=3D GRUB_ERR_NONE) > + goto fail; > + } > + else > + { > + grub_dprintf ("xen_boot_loader", > + "Failed to get Xen Hypervisor info!\n"); > + goto fail; > + } > + > + /* Set module params info */ > + FOR_LIST_ELEMENTS (module, module_head) > + { > + if (module->start && module->size > 0) > + { > + grub_dprintf ("xen_boot_loader", "Module %s @ 0x%lx size:0x%lx\n", > + module->name, > + xen_boot_address_align (module->start, module->align), > + module->size); > + if (prepare_xen_module_params (module) !=3D GRUB_ERR_NONE) > + goto fail; > + } > + else > + { > + grub_dprintf ("xen_boot_loader", "Module info error: %s!\n", > + module->name); > + goto fail; > + } > + } > + > + if (install_all_params () =3D=3D GRUB_ERR_NONE) > + return GRUB_ERR_NONE; > + > +fail: > + clean_all_params (); > + > + return grub_error (GRUB_ERR_BAD_OS, "failed to install/update FDT");= > +} > + > + > +static grub_err_t > +xen_boot (void) > +{ > + if (finalize_params_xen_boot () !=3D GRUB_ERR_NONE) > + return grub_errno; > + Better use err =3D finalize_params_xen_boot (); if (err) return err; > + return grub_arm64_uefi_boot_image (xen_hypervisor->start, > + xen_hypervisor->size, > + xen_hypervisor->cmdline); > +} > + > +static void > +single_binary_unload (struct xen_boot_binary *binary) > +{ Just put if (!binary) return; It will save a lot of if's. > + if (binary && binary->start && binary->size > 0) > + { > + grub_efi_free_pages ((grub_efi_physical_address_t) binary->start= , > + BYTES_TO_PAGES (binary->size + binary->align)); > + } > + > + if (binary && binary->cmdline && binary->cmdline_size > 0) > + { > + grub_free (binary->cmdline); > + grub_dprintf ("xen_boot_loader", > + "Module %s cmdline memory free @ %p size: %d\n", > + binary->name, binary->cmdline, binary->cmdline_size); > + } > + > + if (binary) > + { > + if (binary->node_info.type =3D=3D MODULE_CUSTOM) > + grub_free ((void *) binary->node_info.compat_string); > + if (grub_strcmp (binary->name, XEN_HYPERVISOR_NAME)) > + grub_list_remove (GRUB_AS_LIST (binary)); > + grub_dprintf ("xen_boot_loader", > + "Module %s struct memory free @ %p size: 0x%lx\n", > + binary->name, binary, sizeof (binary)); > + grub_free (binary); > + } > + > + return; > +} > + > +static void > +all_binaries_unload (void) > +{ > + struct xen_boot_binary *binary; > + > + FOR_LIST_ELEMENTS (binary, module_head) > + { > + single_binary_unload (binary); > + } > + > + if (xen_hypervisor) > + single_binary_unload (xen_hypervisor); > + > + return; > +} > + > +static grub_err_t > +xen_unload (void) > +{ > + *loaded =3D 0; > + all_binaries_unload (); > + clean_all_params (); > + grub_dl_unref (linux_mod); > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +xen_boot_binary_load (struct xen_boot_binary *binary, grub_file_t file= , > + int argc, char *argv[]) > +{ > + binary->size =3D grub_file_size (file); > + grub_dprintf ("xen_boot_loader", "Xen_boot %s file size: 0x%lx\n", > + binary->name, binary->size); > + > + binary->start =3D (grub_addr_t) grub_efi_allocate_pages (0, > + (BYTES_TO_PAGES > + (binary->size + > + binary->align))); > + if (!binary->start) > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory")); > + > + grub_dprintf ("xen_boot_loader", "Xen_boot %s numpages: 0x%lx\n", > + binary->name, BYTES_TO_PAGES (binary->size + binary->align)); > + > + if (grub_file_read (file, (void *) xen_boot_address_align (binary->s= tart, > + binary->align), > + binary->size) < (grub_ssize_t) binary->size) > + { We use !=3D throughout. It's safer. > + single_binary_unload (binary); > + return grub_error (GRUB_ERR_BAD_OS, N_("premature end of file %s= "), > + argv[0]); > + } > + > + /* Skip the xen_boot binary file name */ > + ARG_SHIFT (argc, argv); > + There shouldn't be any need for shifting. Just use argc - 1 and argv + 1 > + if (argc > 0) > + { > + binary->cmdline_size =3D grub_loader_cmdline_size (argc, argv); > + binary->cmdline =3D grub_zalloc (binary->cmdline_size); > + if (!binary->cmdline) > + { > + single_binary_unload (binary); > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory")); > + } > + grub_create_loader_cmdline (argc, argv, binary->cmdline, > + binary->cmdline_size); > + grub_dprintf ("xen_boot_loader", > + "Xen_boot %s cmdline @ %p %s, size: %d\n", binary->name, > + binary->cmdline, binary->cmdline, binary->cmdline_size); > + } > + else > + { > + binary->cmdline_size =3D 0; > + binary->cmdline =3D NULL; > + } > + > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +grub_cmd_xen_module (grub_command_t cmd __attribute__ ((unused)), > + int argc, char *argv[]) > +{ > + > + struct xen_boot_binary *module =3D NULL; > + int file_name_index =3D 0; > + grub_file_t file =3D 0; > + > + if (!argc) > + { > + grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected")); > + goto fail; > + } > + > + if (!*loaded) > + { > + grub_error (GRUB_ERR_BAD_ARGUMENT, > + N_("you need to load the Xen Hypervisor first")); > + goto fail; > + } > + > + module =3D > + (struct xen_boot_binary *) grub_zalloc (sizeof (struct xen_boot_bi= nary)); > + if (!module) > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory")); > + Just return grub_errno; zalloc already calls grub_error > + /* process all the options and get module type */ > + if (set_module_type (module, argc, argv, &file_name_index) !=3D GRUB= _ERR_NONE) > + goto fail; > + switch (module->node_info.type) > + { > + case MODULE_IMAGE: > + case MODULE_INITRD: > + case MODULE_OTHER: > + module->node_info.compat_string =3D > + default_compat_string[module->node_info.type].compat_string; > + module->node_info.compat_string_size =3D > + default_compat_string[module->node_info.type].size; > + break; > + > + case MODULE_CUSTOM: > + /* we have set the node_info in set_module_type */ > + break; > + > + default: > + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument")= ); > + } > + module->name =3D module->node_info.compat_string; > + module->align =3D module_default_align[module->node_info.type]; > + > + grub_dprintf ("xen_boot_loader", "Init %s module and node info:\n" > + "compatible %s\ncompat_string_size 0x%lx\n", > + module->name, module->node_info.compat_string, > + module->node_info.compat_string_size); > + > + file =3D grub_file_open (argv[file_name_index]); > + if (!file) > + goto fail; > + > + grub_errno =3D xen_boot_binary_load (module, file, argc - file_name_= index, > + argv + file_name_index); > + When you call grub_error. grub_errno is already set > + if (grub_errno =3D=3D GRUB_ERR_NONE) > + grub_list_push (GRUB_AS_LIST_P (&module_head), GRUB_AS_LIST (modul= e)); > + > +fail: > + if (file) > + grub_file_close (file); > + if (grub_errno !=3D GRUB_ERR_NONE) > + single_binary_unload (module); > + > + return grub_errno; > +} > + > +static grub_err_t > +grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)), > + int argc, char *argv[]) > +{ > + struct xen_hypervisor_header sh; > + grub_file_t file =3D NULL; > + > + grub_dl_ref (linux_mod); > + > + if (!argc) > + { > + grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected")); > + goto fail; > + } > + > + /* For now, we don't support any option in xen_hypervisor command. > + If there are some options, we skip them. */ > + while (argc > 1 && !grub_strncmp (argv[0], "--", 2)) > + { > + grub_dprintf ("xen_boot_loader", "Unknown option %s, skip.\n", a= rgv[0]); > + ARG_SHIFT (argc, argv); > + } > + Why do you need this? Just delete it. > + file =3D grub_file_open (argv[0]); > + if (!file) > + goto fail; > + > + if (grub_file_read (file, &sh, sizeof (sh)) < (long) sizeof (sh)) > + goto fail; > + if (grub_arm64_uefi_check_image > + ((struct grub_arm64_linux_kernel_header *) &sh) !=3D GRUB_ERR_NO= NE) > + goto fail; > + grub_file_seek (file, 0); > + > + grub_loader_unset (); > + This is implicit in loader_set. Please add a comment why it needs to be explicit. I suppose it's to avoid unloading oneself. > + xen_hypervisor =3D > + (struct xen_boot_binary *) grub_zalloc (sizeof (struct xen_boot_bi= nary)); > + if (!xen_hypervisor) > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("out of memory")); > + Ditto > + xen_hypervisor->name =3D XEN_HYPERVISOR_NAME; > + xen_hypervisor->align =3D (grub_size_t) sh.optional_header.section_a= lignment; > + > + grub_errno =3D xen_boot_binary_load (xen_hypervisor, file, argc, arg= v); > + if (grub_errno =3D=3D GRUB_ERR_NONE) > + { > + grub_loader_set (xen_boot, xen_unload, 0); > + *loaded =3D 1; > + } > + > +fail: > + if (file) > + grub_file_close (file); > + if (grub_errno !=3D GRUB_ERR_NONE) > + { > + *loaded =3D 0; > + all_binaries_unload (); > + grub_dl_unref (linux_mod); > + } > + > + return grub_errno; > +} > + > +static grub_command_t cmd_xen_hypervisor, cmd_xen_module; > + > +void > +grub_arm64_linux_register_xen_boot_command (grub_dl_t mod, int *linux_= loaded) > +{ > + cmd_xen_hypervisor =3D > + grub_register_command ("xen_hypervisor", grub_cmd_xen_hypervisor, = 0, > + N_("Load a xen hypervisor.")); > + cmd_xen_module =3D > + grub_register_command ("xen_module", grub_cmd_xen_module, 0, > + N_("Load a xen module.")); > + linux_mod =3D mod; > + loaded =3D linux_loaded; > +} > + > +void > +grub_arm64_linux_unregister_xen_boot_command (void) > +{ > + grub_unregister_command (cmd_xen_hypervisor); > + grub_unregister_command (cmd_xen_module); > +} > diff --git a/include/grub/arm64/xen_boot.h b/include/grub/arm64/xen_boo= t.h > new file mode 100644 > index 0000000..8e8f6cb > --- /dev/null > +++ b/include/grub/arm64/xen_boot.h > @@ -0,0 +1,115 @@ > +/* > + * xen_boot.h - Xen boot header file for Xen boot via FDT > + * on AArch64 architecture. > + * Copyright (C) 2014 Free Software Foundation, Inc. > + * > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published = by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see . > + */ > + > +#ifndef XEN_BOOT_HEADER > +#define XEN_BOOT_HEADER 1 > + > +#include > +#include > +#include /* required by struct xen_hypervisor_header= */ > + This file doesn't really look like having anything reusable. Please put it directly into .c > +#define XEN_HYPERVISOR_NAME "xen_hypervisor" > + > +#define MODULE_DEFAULT_ALIGN (0x0) > +#define MODULE_IMAGE_MIN_ALIGN MODULE_DEFAULT_ALIGN > +#define MODULE_INITRD_MIN_ALIGN MODULE_DEFAULT_ALIGN > +#define MODULE_OTHER_MIN_ALIGN MODULE_DEFAULT_ALIGN > +#define MODULE_CUSTOM_MIN_ALIGN MODULE_DEFAULT_ALIGN > + > +#define MODULE_IMAGE_COMPATIBLE "multiboot,kernel\0multiboot,module" > +#define MODULE_INITRD_COMPATIBLE "multiboot,ramdisk\0multiboot,module= " > +#define MODULE_OTHER_COMPATIBLE "multiboot,module" > + > +/* This maximum size is defined in Power.org ePAPR V1.1 > + * https://www.power.org/documentation/epapr-version-1-1/ > + * 2.2.1.1 Node Name Requirements > + * node-name@unit-address > + * 31 + 1(@) + 16(64bit address in hex format) + 1(\0) =3D 49 > + */ > +#define FDT_NODE_NAME_MAX_SIZE (49) > + > +#define ARG_SHIFT(argc, argv) \ > + do { \ > + (argc)--; \ > + (argv)++; \ > + } while (0) > + > +struct compat_string_struct > +{ > + grub_size_t size; > + const char *compat_string; > +}; > +typedef struct compat_string_struct compat_string_struct_t; > +#define FDT_COMPATIBLE(x) {.size =3D sizeof(x), .compat_string =3D (x)= } > + > +enum module_type > +{ > + MODULE_IMAGE, > + MODULE_INITRD, > + MODULE_OTHER, > + MODULE_CUSTOM > +}; > +typedef enum module_type module_type_t; > + > +struct fdt_node_info > +{ > + module_type_t type; > + > + const char *compat_string; > + grub_size_t compat_string_size; > +}; > + > +struct xen_hypervisor_header > +{ > + struct grub_arm64_linux_kernel_header efi_head; > + > + /* This is always PE\0\0. */ > + grub_uint8_t signature[GRUB_PE32_SIGNATURE_SIZE]; > + /* The COFF file header. */ > + struct grub_pe32_coff_header coff_header; > + /* The Optional header. */ > + struct grub_pe64_optional_header optional_header; > +}; > + > +struct xen_boot_binary > +{ > + struct xen_boot_binary *next; > + struct xen_boot_binary **prev; > + const char *name; > + > + grub_addr_t start; > + grub_size_t size; > + grub_size_t align; > + > + char *cmdline; > + int cmdline_size; > + > + struct fdt_node_info node_info; > +}; > + > +void grub_arm64_linux_register_xen_boot_command (grub_dl_t mod, int *l= oaded); > +void grub_arm64_linux_unregister_xen_boot_command (void); > + > +static __inline grub_addr_t > +xen_boot_address_align (grub_addr_t start, grub_size_t align) > +{ > + return (align ? (ALIGN_UP (start, align)) : start); > +} > + > +#endif /* ! XEN_BOOT_HEADER */ >=20 --DRBsWEM9gsvbjUR6a80bQufI8u8BME2SH Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iF4EAREKAAYFAlWmh+UACgkQmBXlbbo5nOtrVAD7B1kQOgd1JXTg2VvLwIrjYceZ mbey/wVkKxGWTkR/JYAA/igBiAamNM0d7HQ+RPaXOkM+DDWMCBnchlLyrfXnMM+q =6FHH -----END PGP SIGNATURE----- --DRBsWEM9gsvbjUR6a80bQufI8u8BME2SH--