On 05/17/2013 01:49 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote: > I applied this to ARM branch after fixing several grave problems > including attempts to free statically allocated memory. The files > originally didn't even compile.
You seem to have missed the follow-up e-mail I sent on April 7th (see http://lists.gnu.org/archive/html/grub-devel/2013-04/msg00086.html), where I sent an updated version of the fdt driver; in fact, my first version of fdt.c had several issues which have been addressed in the second version. So please replace the current fdt.c file in the arm branch with the updated file. In the mail I mentioned above, I also acknowledged my mistake in using grub_free() to free memory allocated by grub_efi_allocate_loader_memory(). But the memory used for the Linux kernel, the initrd and the fdt is statically allocated only in the u-boot case, which as I explained when I first posted my code wasn't supported by my code. In the EFI case the memory is not statically allocated and IMHO should be freed when not needed anymore. I also proposed to add a function grub_efi_free_memory() to kern/arm/efi/misc.c and to use that function to free the memory. If you want to add this stuff, just say so and I will happily send a patch. Regarding the current Linux loader file, I noticed a few issues: - The defines added at the beginning of the file are not necessary in the current code, because they are already in include/grub/arm/linux.h. When I sent my loader code, I added those defines to the .c file because with support for EFI only it seemed overkill to have a new header file. - In linux_unload(), the initrd data is discarded: why? This doesn't seem right, and looking for example at the i386 linux loader, I see no such thing there either. - In grub_cmd_initrd() there is a leftover call to grub_free() on initrd_start, which should be removed (and IMHO replaced in the EFI case by the right function call to free the memory). - In grub_cmd_devicetree(), if the supplied file name doesn't correspond to an existing file, grub_file_close() is called with a NULL argument, which results in a fatal crash. The patch below addresses these issues. Thanks, Francesco === modified file 'grub-core/loader/arm/linux.c' --- grub-core/loader/arm/linux.c 2013-05-17 11:45:22 +0000 +++ grub-core/loader/arm/linux.c 2013-05-19 17:00:13 +0000 @@ -43,15 +43,6 @@ static grub_uint32_t machine_type; static void *fdt_addr; -#define LINUX_ZIMAGE_OFFSET 0x24 -#define LINUX_ZIMAGE_MAGIC 0x016f2818 - -#define ARM_FDT_MACHINE_TYPE 0xFFFFFFFF - -#define LINUX_PHYS_OFFSET (0x00008000) -#define LINUX_INITRD_PHYS_OFFSET (LINUX_PHYS_OFFSET + 0x02000000) -#define LINUX_FDT_PHYS_OFFSET (LINUX_INITRD_PHYS_OFFSET - 0x10000) - /* * linux_prepare_fdt(): * Prepares a loaded FDT for being passed to Linux. @@ -212,8 +203,6 @@ grub_free (linux_args); linux_args = NULL; - initrd_start = initrd_end = 0; - return GRUB_ERR_NONE; } @@ -278,8 +267,6 @@ if (size == 0) goto fail; - if (initrd_start) - grub_free ((void *) initrd_start); #ifdef GRUB_MACHINE_EFI initrd_start = (grub_addr_t) grub_efi_allocate_loader_memory (LINUX_INITRD_PHYS_OFFSET, size); @@ -337,7 +324,7 @@ dtb = grub_file_open (argv[0]); if (!dtb) - goto out; + return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("failed to open file")); size = grub_file_size (dtb); if (size == 0) _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
