Hello Clement,
On 6/29/20 10:23 AM, Clément Leger wrote:
> Hi Ahmad
>
> ----- On 29 Jun, 2020, at 10:15, Ahmad Fatoum a.fatoum@xxxxxxxxxxxxxx wrote:
>
>> Hello,
>>
>> On 6/24/20 9:15 AM, Clément Leger wrote:
>>>> You already flushed out the dcache contents to the point of unification.
>>>> What do you gain by invalidating it?
>>>
>>> Indeed, this is not strictly necessary, it just allow to boot the elf without a
>>> dirty D-cache. But actually, the booted program should invalidate its D-cache
>>> before fetching any data to avoid using cached data. I can remove it since
>>> this is only for some buggy programs.
>>
>> Why would use of cached data hurt here? If you keep the MMU throughout, you
>> should only need to invalidate the data cache when doing DMA. Am I missing
>> something?
>
> You are actually right, since barebox loaded the program by itself, the D-cache
> contains the expected data even for the loaded program.
> BTW, MMU is not enabled yet in barebox.
Ah, I assumed the MMU would have been enabled, so that MMIO isn't cached/speculated
into. If the MMU is not enabled, you should only need to invalidate if the cache
contents are indeterminate on reset.
>
>>
>> Either way, I've no preference (except for naming it the same as on ARM,
>> see previous mail).
>
> Sorry, I missed the points you made on my previous patchset, my mailer marked
> all the mail has read :/. I will make another more through review.
No worries.
Cheers
Ahmad
>
> Regards,
>
> Clément
>
>>
>> Cheers
>> Ahmad
>>
>>>
>>>>
>>>>> +
>>>>> + /**
>>>>> + * Parameters passing
>>>>> + * r0: boot magic
>>>>> + * r1: device tree pointer
>>>>> + */
>>>>> + entry(LINUX_BOOT_PARAM_MAGIC, (void *) fdt_load_addr);
>>>>> +
>>>>> + /* should never return ! */
>>>>> + panic("Returned from boot program !\n");
>>>>> +
>>>>> + return -EINVAL;
>>>>> +}
>>>>> +
>>>>> +static int do_boot_elf(struct image_data *data, struct elf_image *elf)
>>>>> +{
>>>>> + int ret;
>>>>> + void *fdt;
>>>>> + boot_func_entry entry;
>>>>> + unsigned long load_addr, initrd_address;
>>>>> +
>>>>> + /* load initrd after the elf */
>>>>> + load_addr = PAGE_ALIGN((unsigned long) elf->high_addr);
>>>>> + if (bootm_has_initrd(data)) {
>>>>> + if (data->initrd_address != UIMAGE_INVALID_ADDRESS)
>>>>> + initrd_address = data->initrd_address;
>>>>> + else
>>>>> + initrd_address = load_addr;
>>>>> +
>>>>> + printf("Loading initrd at 0x%lx\n", initrd_address);
>>>>> + ret = bootm_load_initrd(data, initrd_address);
>>>>> + if (ret) {
>>>>> + printf("Failed to load initrd\n");
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + if (data->initrd_address == UIMAGE_INVALID_ADDRESS) {
>>>>> + load_addr += resource_size(data->initrd_res);
>>>>> + load_addr = PAGE_ALIGN(load_addr);
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + fdt = bootm_get_devicetree(data);
>>>>> + if (IS_ERR(fdt)) {
>>>>> + printf("Failed to load dtb\n");
>>>>> + return PTR_ERR(fdt);
>>>>> + }
>>>>> +
>>>>> + printf("Loading device tree at %lx\n", load_addr);
>>>>> + /* load device tree after the initrd if any */
>>>>> + ret = bootm_load_devicetree(data, fdt, load_addr);
>>>>> + if (ret) {
>>>>> + printf("Failed to load device tree: %d\n", ret);
>>>>> + goto err_free_fdt;
>>>>> + }
>>>>> +
>>>>> + entry = (boot_func_entry) data->os_address;
>>>>> +
>>>>> + ret = do_boot_entry(data, entry, fdt);
>>>>> +
>>>>> +err_free_fdt:
>>>>> + free(fdt);
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static int do_bootm_elf(struct image_data *data)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + ret = bootm_load_os(data, data->os_address);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + return do_boot_elf(data, data->elf);
>>>>> +}
>>>>> +
>>>>> +static struct image_handler elf_handler = {
>>>>> + .name = "ELF",
>>>>> + .bootm = do_bootm_elf,
>>>>> + .filetype = filetype_elf,
>>>>> +};
>>>>> +
>>>>> +static struct binfmt_hook binfmt_elf_hook = {
>>>>> + .type = filetype_elf,
>>>>> + .exec = "bootm",
>>>>> +};
>>>>> +
>>>>> +static int kvx_register_image_handler(void)
>>>>> +{
>>>>> + register_image_handler(&elf_handler);
>>>>> +
>>>>> + binfmt_register(&binfmt_elf_hook);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +late_initcall(kvx_register_image_handler);
>>>>>
>>>>
>>>> --
>>>> Pengutronix e.K. | |
>>>> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
>>>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
>>>> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>>>
>>
>> --
>> Pengutronix e.K. | |
>> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
>> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
