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
=== 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;
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel