[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 5/7] add imported "FDT" module for flattened device tree oper
Re: [PATCH 5/7] add imported "FDT" module for flattened device tree operations
Sun, 19 May 2013 19:36:16 +0200
Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20110929 Thunderbird/7.0.1
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
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
- 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.
=== 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)
* Prepares a loaded FDT for being passed to Linux.
@@ -212,8 +203,6 @@
linux_args = NULL;
- initrd_start = initrd_end = 0;
@@ -278,8 +267,6 @@
if (size == 0)
- if (initrd_start)
- grub_free ((void *) initrd_start);
initrd_start = (grub_addr_t) grub_efi_allocate_loader_memory
@@ -337,7 +324,7 @@
dtb = grub_file_open (argv);
- goto out;
+ return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("failed to open file"));
size = grub_file_size (dtb);
if (size == 0)