On 18.12.2013 17:54, Leif Lindholm wrote:
> On Mon, Dec 16, 2013 at 10:34:51PM +0100, Vladimir 'φ-coder/phcoder'
> Serbinenko wrote:
>>> +static void
>>> +get_fdt (void)
>>> +{
>>> + grub_efi_configuration_table_t *tables;
>>> + unsigned int i;
>>> + int fdt_loaded;
>>> + int size;
>>> +
>>> + if (!orig_fdt)
>>> + {
>>> + fdt_loaded = 0;
>>> + /* Look for FDT in UEFI config tables. */
>>> + tables = grub_efi_system_table->configuration_table;
>>> +
>>> + for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
>>> + if (grub_memcmp (&tables[i].vendor_guid, &fdt_guid, sizeof (fdt_guid))
>>> + == 0)
>>> + {
>>> + orig_fdt = tables[i].vendor_table;
>>> + grub_dprintf ("linux", "found registered FDT @ 0x%p\n", orig_fdt);
>>> + break;
>>> + }
>>> + }
>>> + else
>>> + fdt_loaded = 1;
>>> +
>>> + size =
>>> + orig_fdt ? grub_fdt_get_totalsize (orig_fdt) : GRUB_FDT_EMPTY_TREE_SZ;
>>> + size += grub_strlen (linux_args) + 0x400;
>>> +
>>> + grub_dprintf ("linux", "allocating %d bytes for fdt\n", size);
>>> + fdt = grub_efi_allocate_pages (0, BYTES_TO_PAGES (size));
>>> + if (!fdt)
>>> + return;
>>> +
>>> + if (orig_fdt)
>>> + {
>>> + grub_memmove (fdt, orig_fdt, size);
>>> + grub_fdt_set_totalsize (fdt, size);
>>> + if (fdt_loaded)
>>> + grub_free (orig_fdt);
>>
>> There is a problem with this: in case of failure orig_fdt isn't
>> available for next try anymore.
>
> Right. I need to also NULL orig_fdt, will do.
>
I think that you have to keep orig_fdt as otherwise only first attempt
to boot will use FDT supplied by system. Second one won't, likely
resulting in another failure
>>> + if (!loaded)
>>> + {
>>> + grub_error (GRUB_ERR_BAD_ARGUMENT,
>>> + N_("you need to load the kernel first"));
>>> + goto fail;
>>> + }
>>> +
>>> + files = grub_zalloc (argc * sizeof (files[0]));
>>> + if (!files)
>>> + goto fail;
>>> +
>>> + for (i = 0; i < argc; i++)
>>> + {
>>> + grub_file_filter_disable_compression ();
>>> + files[i] = grub_file_open (argv[i]);
>>> + if (!files[i])
>>> + goto fail;
>>> + nfiles++;
>>> + size += ALIGN_UP (grub_file_size (files[i]), 4);
>>> + }
>>> +
>> Why don't you use methods from loader/linux.c ?
>
> Umm, this entire function is quite embarassing - it is left around from
> when I included Matthew Garrett's "linuxefi" code for understanding the
> process better..
> Updated set contains the much simpler one which I meant to include.
> ARM* do not even support multiple initrds.
They're concatenated in memory if you use common functions and results
in valid cpio which will be decompressed/parsed by Linux.
>
>>> + if (grub_file_read (file, kernel_mem, kernel_size)
>>> + != (grub_int64_t) kernel_size)
>>> + {
>>> + grub_error (GRUB_ERR_FILE_READ_ERROR, N_("Can't read kernel %s"),
>>> + argv[0]);
>> Please look at similar place in other architectures.
>
> i386 looks near-identical, apart from the fact that their bzImage has
> a size field which the ARM64 image does not. If you want me to change
> something here, I'm afraid you will have to rub my nose in it.
>
if grub_errno is set you shouldn't override it. And please use the same
message verbatim (unexpected end of file): it decreases work for translators
> /
> Leif
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
