From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1UMhXT-0004xj-HR for mharc-grub-devel@gnu.org; Mon, 01 Apr 2013 12:24:55 -0400 Received: from eggs.gnu.org ([208.118.235.92]:37105) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UMhXM-0004vu-B0 for grub-devel@gnu.org; Mon, 01 Apr 2013 12:24:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UMhXF-0002Pl-KF for grub-devel@gnu.org; Mon, 01 Apr 2013 12:24:48 -0400 Received: from mail-ee0-f51.google.com ([74.125.83.51]:62067) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UMhXF-0002PW-6d for grub-devel@gnu.org; Mon, 01 Apr 2013 12:24:41 -0400 Received: by mail-ee0-f51.google.com with SMTP id c4so1083098eek.24 for ; Mon, 01 Apr 2013 09:24:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:subject :references:in-reply-to:content-type; bh=t132IBWeZUVWp/NmSowH5rieNWxNRm46FEFa3qyBZu4=; b=t56AZXKr//lISudwMWUbDu9K3TJNdEoIE4L2OT8U5My2B/RNFfk9RcCwjVsGvxS8Us BCSa/1cud0U7ZIJKsLYf6khf69krBH1skhqY+0qdWPeDnzjZeyb5BVE3eh4/XlOlXICH Jn33/euKvaKg6dcCJO1Q2cljYfzWSC9GOoMvSurHTdyMqLRbdHC6yBSIFb8ZByDu0Vt5 jczUQM78HdsRjQtsOn0GwTZhIGXI9LG1+lWrBlqF2DLROgu3l82HSxgvgdBHwkVp7A0Z 5cnqilQyWEo8Nom/yaKBkz+GUkAMEM0xl/9gICkbTnY7VZ7sPKvV3WH0P4rFsyr3Kslf LwcQ== X-Received: by 10.15.36.135 with SMTP id i7mr39226283eev.34.1364833480235; Mon, 01 Apr 2013 09:24:40 -0700 (PDT) Received: from [192.168.56.2] ([151.36.245.148]) by mx.google.com with ESMTPS id q42sm21896196eem.14.2013.04.01.09.24.32 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 01 Apr 2013 09:24:38 -0700 (PDT) Message-ID: <5159B4CB.2040300@gmail.com> Date: Mon, 01 Apr 2013 18:24:43 +0200 From: Francesco Lavra User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20110929 Thunderbird/7.0.1 MIME-Version: 1.0 To: grub-devel@gnu.org Subject: Re: [PATCH 7/7] Add support for ARM UEFI ("EFI") platforms References: In-Reply-To: Content-Type: multipart/mixed; boundary="------------040301010500050003030503" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 74.125.83.51 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: Mon, 01 Apr 2013 16:24:53 -0000 This is a multi-part message in MIME format. --------------040301010500050003030503 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 03/24/2013 06:01 PM, Leif Lindholm wrote: > === added file 'grub-core/kern/arm/efi/misc.c' > --- grub-core/kern/arm/efi/misc.c 1970-01-01 00:00:00 +0000 > +++ grub-core/kern/arm/efi/misc.c 2013-03-24 15:43:19 +0000 [...] > +void * > +grub_efi_allocate_loader_memory (grub_uint32_t min_offset, grub_uint32_t size) > +{ [...] > + for (desc = mmap ; desc < mmap_end ; > + desc = NEXT_MEMORY_DESCRIPTOR(desc, desc_size)) > + { > + grub_uint64_t start, end; > + > + grub_dprintf("mm", "%s: 0x%08x bytes @ 0x%08x\n", > + __FUNCTION__, > + (grub_uint32_t) (desc->num_pages << PAGE_SHIFT), > + (grub_uint32_t) (desc->physical_start)); > + > + if (desc->type != GRUB_EFI_CONVENTIONAL_MEMORY) > + continue; > + > + start = desc->physical_start; > + end = start + (desc->num_pages << PAGE_SHIFT); > + grub_dprintf("mm", "%s: start=0x%016llx, end=0x%016llx\n", > + __FUNCTION__, start, end); > + start = start < min_start ? min_start : start; > + if (start + size > end) > + continue; > + grub_dprintf("mm", "%s: let's allocate some (0x%x) pages @ 0x%08x...\n", > + __FUNCTION__, (size >> PAGE_SHIFT), (grub_addr_t) start); > + mem = grub_efi_allocate_pages (start, (size >> PAGE_SHIFT) + 1); > + grub_dprintf("mm", "%s: retval=0x%08x\n", > + __FUNCTION__, (grub_addr_t) mem); > + if (! mem) > + { > + grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory"); > + goto fail; > + } This if block is redundant, since it is repeated just after the for() loop. The break statement below would do. > + break; > + } > + > + if (! mem) > + { > + grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot allocate memory"); > + goto fail; > + } > + > + grub_free (mmap); > + return mem; > + > + fail: > + grub_free (mmap); > + return NULL; > +} [...] > === added file 'grub-core/kern/arm/efi/startup.S' > --- grub-core/kern/arm/efi/startup.S 1970-01-01 00:00:00 +0000 > +++ grub-core/kern/arm/efi/startup.S 2013-03-24 15:32:46 +0000 > @@ -0,0 +1,38 @@ > +/* > + * (C) Copyright 2013 Free Software Foundation > + * > + * This program 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 2 of > + * the License, or (at your option) any later version. > + * > + * This program 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 this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, > + * MA 02111-1307 USA > + * > + */ This license header doesn't follow exactly GRUB's template. [...] > --- util/grub-mkimage.c 2013-03-24 14:49:04 +0000 > +++ util/grub-mkimage.c 2013-03-24 15:32:46 +0000 > @@ -472,6 +472,27 @@ > .mod_align = GRUB_KERNEL_ARM_UBOOT_MOD_ALIGN, > .link_align = 4 > }, > + { > + .dirname = "arm-efi", > + .names = { "arm-efi", NULL }, > + .voidp_sizeof = 4, > + .bigendian = 0, > + .id = IMAGE_EFI, Nitpick: the last two lines above have trailing whitespace. > + .flags = PLATFORM_FLAGS_NONE, > + .total_module_size = TARGET_NO_FIELD, > + .decompressor_compressed_size = TARGET_NO_FIELD, > + .decompressor_uncompressed_size = TARGET_NO_FIELD, > + .decompressor_uncompressed_addr = TARGET_NO_FIELD, > + .section_align = GRUB_PE32_SECTION_ALIGNMENT, > + .vaddr_offset = ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE > + + GRUB_PE32_SIGNATURE_SIZE > + + sizeof (struct grub_pe32_coff_header) > + + sizeof (struct grub_pe32_optional_header) > + + 4 * sizeof (struct grub_pe32_section_table), > + GRUB_PE32_SECTION_ALIGNMENT), This ALIGN_UP() thing is a bit duplicated in this file. How about adding a #define EFI32_HEADER_SIZE, analogous to EFI64_HEADER_SIZE, and using the new define? The attached patch does this simple refactoring and could be applied right away, independently from the ARM port. > + .pe_target = GRUB_PE32_MACHINE_ARMTHUMB_MIXED, > + .elf_target = EM_ARM, > + }, > }; > > #define grub_target_to_host32(x) (grub_target_to_host32_real (image_target, (x))) > > === modified file 'util/grub-mkimagexx.c' > --- util/grub-mkimagexx.c 2012-02-29 17:57:43 +0000 > +++ util/grub-mkimagexx.c 2013-03-24 15:32:46 +0000 > @@ -58,6 +58,11 @@ > #error "I'm confused" > #endif > > +static Elf_Addr SUFFIX (entry_point); > + > +grub_err_t reloc_thm_call (grub_uint16_t *addr, Elf32_Addr sym_addr); > +grub_err_t reloc_thm_jump19 (grub_uint16_t *addr, Elf32_Addr sym_addr); > + > /* Relocate symbols; note that this function overwrites the symbol table. > Return the address of a start symbol. */ > static Elf_Addr > @@ -528,6 +533,48 @@ > } > break; > #endif > +#if defined(MKIMAGE_ELF32) > + case EM_ARM: > + { > + sym_addr += addend; > + sym_addr -= SUFFIX (entry_point); > + switch (ELF_R_TYPE (info)) > + { > + case R_ARM_ABS32: > + { > + grub_util_info (" ABS32:\toffset=%d\t(0x%08x)", > + (int) sym_addr, (int) sym_addr); > + /* Data will be naturally aligned */ > + // sym_addr -= offset; Maybe this commented line should be removed altogether? > + sym_addr += 0x400; Where does this 0x400 value come from? > + *target = grub_host_to_target32 (grub_target_to_host32 (*target) + sym_addr); > + } > + break; > + case R_ARM_THM_CALL: > + case R_ARM_THM_JUMP24: > + { > + grub_util_info (" THM_JUMP24:\ttarget=0x%08x\toffset=(0x%08x)", (unsigned int) target, sym_addr); > + sym_addr -= offset; > + /* Thumb instructions can be 16-bit aligned */ > + reloc_thm_call ((grub_uint16_t *) target, sym_addr); reloc_thm_call() works with native endianness, so it cannot be used as is here, as explained in the other mail a few hours ago. > + } > + break; > + case R_ARM_THM_JUMP19: > + { > + grub_util_info (" THM_JUMP19:\toffset=%d\t(0x%08x)", > + sym_addr, sym_addr); > + sym_addr -= offset; > + /* Thumb instructions can be 16-bit aligned */ > + reloc_thm_jump19 ((grub_uint16_t *) target, sym_addr); Ditto for reloc_thm_jump19(), it works with native endianness and cannot be used as is. -- Francesco --------------040301010500050003030503 Content-Type: text/x-patch; name="grub-mkimage-refactoring.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="grub-mkimage-refactoring.patch" === modified file 'util/grub-mkimage.c' --- util/grub-mkimage.c 2013-03-07 07:17:24 +0000 +++ util/grub-mkimage.c 2013-03-31 13:50:50 +0000 @@ -91,6 +91,13 @@ grub_uint16_t pe_target; }; +#define EFI32_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE \ + + GRUB_PE32_SIGNATURE_SIZE \ + + sizeof (struct grub_pe32_coff_header) \ + + sizeof (struct grub_pe32_optional_header) \ + + 4 * sizeof (struct grub_pe32_section_table), \ + GRUB_PE32_SECTION_ALIGNMENT) + #define EFI64_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE \ + GRUB_PE32_SIGNATURE_SIZE \ + sizeof (struct grub_pe32_coff_header) \ @@ -182,12 +189,7 @@ .decompressor_uncompressed_size = TARGET_NO_FIELD, .decompressor_uncompressed_addr = TARGET_NO_FIELD, .section_align = GRUB_PE32_SECTION_ALIGNMENT, - .vaddr_offset = ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE - + GRUB_PE32_SIGNATURE_SIZE - + sizeof (struct grub_pe32_coff_header) - + sizeof (struct grub_pe32_optional_header) - + 4 * sizeof (struct grub_pe32_section_table), - GRUB_PE32_SECTION_ALIGNMENT), + .vaddr_offset = EFI32_HEADER_SIZE, .pe_target = GRUB_PE32_MACHINE_I386, .elf_target = EM_386, }, @@ -1101,12 +1103,7 @@ int reloc_addr; if (image_target->voidp_sizeof == 4) - header_size = ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE - + GRUB_PE32_SIGNATURE_SIZE - + sizeof (struct grub_pe32_coff_header) - + sizeof (struct grub_pe32_optional_header) - + 4 * sizeof (struct grub_pe32_section_table), - GRUB_PE32_SECTION_ALIGNMENT); + header_size = EFI32_HEADER_SIZE; else header_size = EFI64_HEADER_SIZE; --------------040301010500050003030503--