From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1VuQSb-0003rG-Si for mharc-grub-devel@gnu.org; Sat, 21 Dec 2013 12:35:34 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58511) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VuQST-0003pv-Lm for grub-devel@gnu.org; Sat, 21 Dec 2013 12:35:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VuQSN-00042b-JL for grub-devel@gnu.org; Sat, 21 Dec 2013 12:35:25 -0500 Received: from mail-ea0-f169.google.com ([209.85.215.169]:47155) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VuQSM-00042P-Vg for grub-devel@gnu.org; Sat, 21 Dec 2013 12:35:19 -0500 Received: by mail-ea0-f169.google.com with SMTP id l9so1397925eaj.0 for ; Sat, 21 Dec 2013 09:34:42 -0800 (PST) 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:subject:references :in-reply-to:content-type; bh=wKJ4TI3EvrHFqST9Y+EBS+KVG+2bE/tzQ3MqfikoD8Q=; b=cPIVf5n0xUZyMA7yk6IdlJhAEZyLk6vqEPbZWLp9RuMmpuopKuFg16b7nsRumUFNXb 4MCiDL66EyZEHxwXMELglCIRLVCzujDpVqOTTMC81LAM54IcHwwjO9HguvD7+RHhD01n XS2ddCGtVJFb2ErfTI0yaxVtingl9G0RmQUZC1jDoovt6L2X1WI+eoliS4h5W1dmBHn7 BnHQJPE6rsAqd2bvL4oUZIBarTKuHaGqvMrQNgudMdiVIPaCo02cJcCJ2ZDhNy3N19Ja YcPUAe4Y3b5TtvHx2Ozyk8Lz26fBfpxQdTRyuY0Pk41XlzG6mEamdtZZmb80sahvek28 Navw== X-Received: by 10.14.182.199 with SMTP id o47mr12487572eem.7.1387647282554; Sat, 21 Dec 2013 09:34:42 -0800 (PST) Received: from [192.168.1.16] (85-188.196-178.cust.bluewin.ch. [178.196.188.85]) by mx.google.com with ESMTPSA id e3sm29326595eeg.11.2013.12.21.09.34.41 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sat, 21 Dec 2013 09:34:41 -0800 (PST) Message-ID: <52B5D130.8000404@gmail.com> Date: Sat, 21 Dec 2013 18:34:40 +0100 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20131103 Icedove/17.0.10 MIME-Version: 1.0 To: The development of GNU GRUB Subject: Re: [PATCH v3 2/2] arm64: add EFI Linux loader References: <1387557889-14365-1-git-send-email-leif.lindholm@linaro.org> In-Reply-To: <1387557889-14365-1-git-send-email-leif.lindholm@linaro.org> X-Enigmail-Version: 1.6 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Ip2CPPThP2AW9SqFkAh4jBSsPt2UhKb5U" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.215.169 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: Sat, 21 Dec 2013 17:35:32 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Ip2CPPThP2AW9SqFkAh4jBSsPt2UhKb5U Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 20.12.2013 17:44, Leif Lindholm wrote: > Bugfix of v2 loaded_fdt handling and removal of redundant typedefs. >=20 > --- > grub-core/Makefile.core.def | 3 +- > grub-core/loader/arm64/linux.c | 479 ++++++++++++++++++++++++++++++++= ++++++++ > include/grub/arm64/linux.h | 54 +++++ > include/grub/efi/api.h | 4 + > 4 files changed, 539 insertions(+), 1 deletion(-) > create mode 100644 grub-core/loader/arm64/linux.c > create mode 100644 include/grub/arm64/linux.h >=20 > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def > index e5e558c..c916246 100644 > --- a/grub-core/Makefile.core.def > +++ b/grub-core/Makefile.core.def > @@ -1672,7 +1672,8 @@ module =3D { > sparc64_ieee1275 =3D loader/sparc64/ieee1275/linux.c; > ia64_efi =3D loader/ia64/efi/linux.c; > arm =3D loader/arm/linux.c; > - arm =3D lib/fdt.c; > + arm64 =3D loader/arm64/linux.c; > + fdt =3D lib/fdt.c; > common =3D loader/linux.c; > common =3D lib/cmdline.c; > enable =3D noemu; > diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/li= nux.c > new file mode 100644 > index 0000000..52c6160 > --- /dev/null > +++ b/grub-core/loader/arm64/linux.c > @@ -0,0 +1,479 @@ > +/* > + * GRUB -- GRand Unified Bootloader > + * Copyright (C) 2013 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 > + > +GRUB_MOD_LICENSE ("GPLv3+"); > + > +#define GRUB_EFI_PAGE_SHIFT 12 > +#define BYTES_TO_PAGES(bytes) (((bytes) + 0xfff) >> GRUB_EFI_PAGE_SH= IFT) > +#define GRUB_EFI_PE_MAGIC 0x5A4D > + > +static grub_efi_guid_t fdt_guid =3D GRUB_EFI_DEVICE_TREE_GUID; > + > +static grub_dl_t my_mod; > +static int loaded; > + > +static void *kernel_addr; > +static grub_uint64_t kernel_size; > + > +static char *linux_args; > +static grub_uint32_t cmdline_size; > + > +static grub_addr_t initrd_start; > +static grub_addr_t initrd_end; > + > +static void *loaded_fdt; > +static void *fdt; > + > +static void * > +get_firmware_fdt (void) > +{ > + grub_efi_configuration_table_t *tables; > + void *firmware_fdt =3D NULL; > + unsigned int i; > + > + /* Look for FDT in UEFI config tables. */ > + tables =3D grub_efi_system_table->configuration_table; > + > + for (i =3D 0; i < grub_efi_system_table->num_table_entries; i++) > + if (grub_memcmp (&tables[i].vendor_guid, &fdt_guid, sizeof (fdt_gu= id)) =3D=3D 0) > + { > + firmware_fdt =3D tables[i].vendor_table; > + grub_dprintf ("linux", "found registered FDT @ 0x%p\n", firmware_fdt)= ; > + break; > + } > + > + return firmware_fdt; > +} > + > +static void > +get_fdt (void) > +{ > + void *raw_fdt; > + int size; > + int doesn't sound like right type here. Did you mean grub_size_t ? > + if (fdt) > + { > + size =3D BYTES_TO_PAGES (grub_fdt_get_totalsize (fdt)); > + grub_efi_free_pages ((grub_efi_physical_address_t) fdt, size); > + } > + > + if (loaded_fdt) > + raw_fdt =3D loaded_fdt; > + else > + raw_fdt =3D get_firmware_fdt(); > + > + size =3D > + raw_fdt ? grub_fdt_get_totalsize (raw_fdt) : GRUB_FDT_EMPTY_TREE_S= Z; > + size +=3D 0x400; > + > + grub_dprintf ("linux", "allocating %d bytes for fdt\n", size); > + fdt =3D grub_efi_allocate_pages (0, BYTES_TO_PAGES (size)); > + if (!fdt) > + return; > + > + if (raw_fdt) > + { > + grub_memmove (fdt, raw_fdt, size); > + grub_fdt_set_totalsize (fdt, size); > + } > + else > + { > + grub_fdt_create_empty_tree (fdt, size); > + } > +} > + > +static grub_err_t > +check_kernel (struct linux_kernel_header *lh) > +{ > + if (lh->magic !=3D GRUB_LINUX_MAGIC) > + return grub_error(GRUB_ERR_BAD_OS, N_("Invalid kernel image")); > + Please reuse the strings from other platforms. "invalid magic number" in this case > + if ((lh->code0 & 0xffff) !=3D GRUB_EFI_PE_MAGIC) > + return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, > + N_("Plain Image kernel not supported - rebuild with CONFIG_UE= FI_STUB enabled.\n")); > + Error messgaes startwith lowercase and have neither newline nor dot at the end. They're used in error: %s.\n template. > + b =3D grub_efi_system_table->boot_services; > + status =3D b->install_configuration_table (&fdt_guid, fdt); > + if (status !=3D GRUB_EFI_SUCCESS) > + goto failure; > + > + grub_dprintf ("linux", "Installed/updated FDT configuration table @ = %p\n", > + fdt); > + > + return GRUB_ERR_NONE; > + > +failure: > + grub_efi_free_pages ((grub_efi_physical_address_t) fdt, > + BYTES_TO_PAGES (grub_fdt_get_totalsize (fdt))); > + fdt =3D NULL; > + return GRUB_ERR_BAD_OS; grub_error is missing. > + size =3D grub_file_size (dtb); > + blob =3D grub_malloc (size); > + if (!blob) > + goto out; > + > + if (grub_file_read (dtb, blob, size) < size) > + { > + if (!grub_errno) > + grub_error (GRUB_ERR_BAD_OS, N_("premature end of file %s"), argv[0])= ; > + goto out; > + } > + > + if (grub_fdt_check_header (blob, size) !=3D 0) > + { > + grub_error (GRUB_ERR_BAD_OS, N_("Invalid FDT")); Please reuse the strings: N_("invalid device tree") Every translatable string is work for translators. > + > + mempath =3D grub_malloc (2 * sizeof (grub_efi_memory_mapped_device_p= ath_t)); > + if (!mempath) > + return GRUB_ERR_OUT_OF_MEMORY; > + return grub_errno; > + mempath[0].header.type =3D GRUB_EFI_HARDWARE_DEVICE_PATH_TYPE; > + mempath[0].header.subtype =3D GRUB_EFI_MEMORY_MAPPED_DEVICE_PATH_SUB= TYPE; > + mempath[0].header.length =3D grub_cpu_to_le16_compile_time (sizeof (= *mempath)); > + mempath[0].memory_type =3D GRUB_EFI_LOADER_DATA; > + mempath[0].start_address =3D (grub_addr_t) kernel_addr; > + mempath[0].end_address =3D (grub_addr_t) kernel_addr + kernel_size; > + > + mempath[1].header.type =3D GRUB_EFI_END_DEVICE_PATH_TYPE; > + mempath[1].header.subtype =3D GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYP= E; > + mempath[1].header.length =3D 0; > + > + b =3D grub_efi_system_table->boot_services; > + status =3D b->load_image (0, grub_efi_image_handle, > + (grub_efi_device_path_t *) mempath, > + kernel_addr, kernel_size, &image_handle); > + if (status !=3D GRUB_EFI_SUCCESS) > + return GRUB_ERR_BAD_OS; > + grub_error is missing > + grub_dprintf ("linux", "linux command line: '%s'\n", linux_args); > + > + /* Convert command line to UCS-2 */ > + loaded_image =3D grub_efi_get_loaded_image (image_handle); > + loaded_image->load_options_size =3D > + grub_strlen (linux_args) * sizeof (grub_efi_char16_t); > + loaded_image->load_options =3D p16 =3D > + grub_efi_allocate_pages (0, > + BYTES_TO_PAGES (loaded_image->load_options_size)); > + if (!loaded_image->load_options) > + return GRUB_ERR_OUT_OF_MEMORY; > + grub_error > + p8 =3D linux_args; > + do > + *p16++ =3D *p8++; > + while (*p8); > + Please use utf8_to_utf16. MAke sure that you allocate enough memory. > + > + if (grub_file_read (file, &lh, sizeof (lh)) < (long) sizeof (lh)) > + return GRUB_ERR_FILE_READ_ERROR; > + return grub_errno; > + grub_error (GRUB_ERR_OUT_OF_MEMORY, N_("can't allocate kernel"))= ; N_("out of memory") > + linux_args =3D grub_malloc (cmdline_size); > + if (!linux_args) > + grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate command line"= ); goto fail. Otherwise you try to copy to NULL. Also I'd recommend calling grub_loader_unset after checking file format but before starting all the loading. It's not strictily necessarry but avoids possible problems if loading fails midway with half of variables referring to old and half to new load attempt. --Ip2CPPThP2AW9SqFkAh4jBSsPt2UhKb5U Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.15 (GNU/Linux) Comment: Using GnuPG with Icedove - http://www.enigmail.net/ iF4EAREKAAYFAlK10TAACgkQmBXlbbo5nOtAkAEAp9RTr0JTZq74wzKR/qIb541w MBB/YEXI7a6fU3TOEzgBAIlbh8BU/NtEoPelPvqnHTrRqaNR9PTPaQncKZAAkjCb =YVTD -----END PGP SIGNATURE----- --Ip2CPPThP2AW9SqFkAh4jBSsPt2UhKb5U--