[AMD Official Use Only - General] Hi Heinrich,
> -----Original Message----- > From: Heinrich Schuchardt <xypron.g...@gmx.de> > Sent: Monday, January 29, 2024 4:09 AM > To: Bhumkar, Tejas Arvind <tejas.arvind.bhum...@amd.com> > Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org>; U-Boot Mailing List <u- > b...@lists.denx.de>; Tom Rini <tr...@konsulko.com> > Subject: Re: [PATCH] efi_loader : Suppress error print message > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > On 1/28/24 18:38, Bhumkar, Tejas Arvind wrote: > > [AMD Official Use Only - General] > > > > Hi Heinrich, > > > >> -----Original Message----- > >> From: Heinrich Schuchardt <xypron.g...@gmx.de> > >> Sent: Sunday, January 28, 2024 2:51 PM > >> To: Bhumkar, Tejas Arvind <tejas.arvind.bhum...@amd.com> > >> Cc: u-boot@lists.denx.de; tr...@konsulko.com; s...@chromium.org; > >> Simek, Michal <michal.si...@amd.com>; Abbarapu, Venkatesh > >> <venkatesh.abbar...@amd.com>; g...@xilinx.com; Ilias Apalodimas > >> <ilias.apalodi...@linaro.org> > >> Subject: Re: [PATCH] efi_loader : Suppress error print message > >> > >> Caution: This message originated from an External Source. Use proper > >> caution when opening attachments, clicking links, or responding. > >> > >> > >> On 1/28/24 06:20, Bhumkar, Tejas Arvind wrote: > >>> [AMD Official Use Only - General] > >>> > >>> Hi Heinrich, > >>> > >>>> -----Original Message----- > >>>> From: Heinrich Schuchardt <xypron.g...@gmx.de> > >>>> Sent: Wednesday, January 24, 2024 2:09 AM > >>>> To: Bhumkar, Tejas Arvind <tejas.arvind.bhum...@amd.com> > >>>> Cc: u-boot@lists.denx.de; tr...@konsulko.com; s...@chromium.org; > >>>> Simek, Michal <michal.si...@amd.com>; Abbarapu, Venkatesh > >>>> <venkatesh.abbar...@amd.com>; g...@xilinx.com; Ilias Apalodimas > >>>> <ilias.apalodi...@linaro.org> > >>>> Subject: Re: [PATCH] efi_loader : Suppress error print message > >>>> > >>>> Caution: This message originated from an External Source. Use > >>>> proper caution when opening attachments, clicking links, or responding. > >>>> > >>>> > >>>> On 1/23/24 19:53, Bhumkar, Tejas Arvind wrote: > >>>>> [AMD Official Use Only - General] > >>>>> > >>>>> Hi Ilias & Heinrich, > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Heinrich Schuchardt <xypron.g...@gmx.de> > >>>>>> Sent: Tuesday, January 23, 2024 3:30 PM > >>>>>> To: Ilias Apalodimas <ilias.apalodi...@linaro.org>; Bhumkar, > >>>>>> Tejas Arvind <tejas.arvind.bhum...@amd.com> > >>>>>> Cc: u-boot@lists.denx.de; xypron.g...@gmx.de; tr...@konsulko.com; > >>>>>> s...@chromium.org; Simek, Michal <michal.si...@amd.com>; Abbarapu, > >>>>>> Venkatesh <venkatesh.abbar...@amd.com>; g...@xilinx.com > >>>>>> Subject: Re: [PATCH] efi_loader : Suppress error print message > >>>>>> > >>>>>> Caution: This message originated from an External Source. Use > >>>>>> proper caution when opening attachments, clicking links, or responding. > >>>>>> > >>>>>> > >>>>>> On 1/23/24 09:33, Ilias Apalodimas wrote: > >>>>>>> Hi Tejas, > >>>>>>> > >>>>>>> On Mon, 22 Jan 2024 at 21:12, Tejas Bhumkar > >>>>>>> <tejas.arvind.bhum...@amd.com> wrote: > >>>>>>>> > >>>>>>>> Currently, on certain Xilinx platforms, an issue has been > >>>>>>>> identified, manifesting as follows: > >>>>>> > >>>>>> Hello Tejas, > >>>>>> > >>>>>> thank you for bringing forward this issue. > >>>>>> > >>>>>> Which defconfig are you relating to? > >>>>> > >>>>> [Tejas]: > >>>>> Checking with xilinx_zynqmp_virt_defconfig > >>>>> > >>>>>> > >>>>>>>> > >>>>>>>> Starting kernel ... > >>>>>>>> > >>>>>>>> efi_free_pool: illegal free 0x0000000077830040 > >>>>>>>> efi_free_pool: illegal free 0x000000007782d040 > >>>>>>>> efi_free_pool: illegal free 0x000000007782c040 > >>>>>>>> > >>>>>>>> The issue arises when the ramdisk image is relocated, placing > >>>>>>>> it within the previously allocated EFI memory region( as EFI is > >>>>>>>> established quite early in U-Boot). > >>>>>> > >>>>>> Which version of U-Boot are you on? Commit 06d514d77c37 ("lmb: > >>>>>> consider EFI memory map") was meant to avoid such a situation. > >>>>> > >>>>> [Tejas] : > >>>>> I'm verifying against the latest changes in the master branch. The > >>>>> introduction of commit 06d514d77c37 ("lmb: consider EFI memory > >>>>> map") has resolved the occurrence of the "efi_free_pool: illegal free" > >>>>> error. but, it leads to a new error, as detailed in the following > >>>>> patch: > >>>>> https://lore.kernel.org/all/20230212150706.2967007-1-sjoerd@collabora. > >>>>> com/ > >>>> > >>>> Could you, please, try to reproduce your issues with origin/master > >>>> as of today and provide the full boot log. Please, also provide the > >>>> output of the bdinfo and the printenv command as well as the sizes > >>>> of the > >> kernel and the RAM disk. > >>> > >>> [Tejas]: I've provided two logs: one obtained from the latest u-boot > >> origin/master, resulting in the "ERROR: reserving fdt memory region > >> failed" error, and the other from reverting commit 06d514d77c37 > >> ("lmb: consider EFI memory map"), which leads to the "efi_free_pool: > >> illegal > free" error. > >>> > >>> Thank You, > >>> Tejas. > >> > >> Where can I find these logs? > > [Tejas ] : I've sent the attachment in a previous email. I'm not sure why > > you > didn't receive it. > > I've attached it again now. Please confirm if you still haven't received it. > > > > Thank You, > > Tejas. > >> > > If I understand your FDT_memory_region_failed.log correctly: > > Except for some added debug messages this log matches booting with an > unmodified upstream U-Boot as of Jan 22nd. > > Your memory is split into two regions: > > 0x0 - 0x7ffffff and > 0x800000000 - 0x87ffffff. > > Your device-tree has a reserved region 0x7ff00000-0x7fffffff. > > U-Boot relocates itself into the lower memory block due to current design. > > efi_lmb_reserve() marks memory up to 0x7fffffff and 0x800000000-0x87fffffff as > reserved. > > boot_fdt_reserve_region() writes an error when it tries to reserve the > overlapping > region 0x7ff00000-0x7fffffff because LMB recognizes this region is already > reserved. > > This should not interfere with booting as boot_fdt_reserve_region() does not > have > a return value but is not very clean and should be fixed. [Tejas]: Thank you for the explanation. Could you please confirm if the maintainer will be looking into this matter? Thank You, Tejas > > Best regards > > Heinrich > >>>> > >>>>> > >>>>>> > >>>>>>> > >>>>>>> I don't mind suppressing the print for some time, but out of > >>>>>>> curiosity, how is the ramdisk relocated? LMB should be aware of > >>>>>>> the EFI regions by then, so I assume the relocation code doesn't > >>>>>>> check those? > >>>>> > >>>>> > >>>>> [Tejas] : I observe that the relocation of the RAM disk is taking > >>>>> place in the line > >>>> below. > >>>>> https://github.com/u-boot/u-boot/blob/master/lib/lmb.c#L480-L491 > >>>>> Yes, the relocated code specifically examines the LMB region and > >>>>> does not > >>>> consider the EFI region. > >>>>> > >>>>>> > >>>>>> The indicated situation is serious. If the EFI sub-system is > >>>>>> using the same memory area as the ramdisk, this may lead to > >>>>>> corruption of the ramdisk. Suppressing the messages is by no > >>>>>> means adequate to solve the > >>>> problem. > >>>>> > >>>>> [Tejas] : > >>>>> The challenge arises when relocating the ramdisk image, inserting > >>>>> it into the previously assigned EFI memory region, established > >>>>> early in U-Boot. Consequently, when attempting to release memory > >>>>> in the EFI region > >>>> during the handover process to the kernel in the efi_free_pool > >>>> function, we first verify if the memory was allocated by > >>>> efi_allocate_pool(). > >>>>> The issue originates from a checksum mismatch because, during the > >>>>> ramdisk > >>>> relocation, we overwrite memory allocated by EFI. > >>>>> This leads to the appearance of the error message: efi_free_pool: > >>>>> illegal free > >>>> 0x0000000077830040. > >>>>> Crucially, there is no corruption of the ramdisk seen since we do > >>>>> not actually > >>>> releasing memory due to the checksum mismatch. > >>>>> In our testing, this issue was observed only when the ramdisk size > >>>>> exceeded > >>>> approximately 24 MB. > >>>>> > >>>>>> > >>>>>> Best regards > >>>>>> > >>>>>> Heinrich > >>>>>> > >>>>>> > >>>>>>> > >>>>>>> Thanks > >>>>>>> /Ilias > >>>>>>> > >>>>>>>> Consequently, when > >>>>>>>> attempting to release memory in the EFI memory region during > >>>>>>>> the handover process to the kernel,we encounter memory violations. > >>>>>>>> > >>>>>>>> Highlighting that EFI remains active primarily during the > >>>>>>>> booting of an EFI application, and the lmb persists while > >>>>>>>> configuring images for the boot process. Since we aren't > >>>>>>>> utilizing the EFI memory region during the boot process, there > >>>>>>>> is no adverse impact even in the event of a violation. > >>>>>>>> > >>>>>>>> Currently, there is an ongoing discussion regarding the > >>>>>>>> handling strategies of three memory allocators: malloc, lmb, > >>>>>>>> and EFI. This discussion is documented in the email chain titled > "Proposal: > >>>>>>>> U-Boot memory management." > >>>>>>>> > >>>>>>>> Therefore, it is advisable to suppress the print message during > >>>>>>>> the boot process for now. > >>>>>>>> > >>>>>>>> Signed-off-by: Tejas Bhumkar <tejas.arvind.bhum...@amd.com> > >>>>>>>> --- > >>>>>>>> lib/efi_loader/efi_memory.c | 2 +- > >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>>>>> > >>>>>>>> diff --git a/lib/efi_loader/efi_memory.c > >>>>>>>> b/lib/efi_loader/efi_memory.c index edfad2d95a..821fe7616e > >>>>>>>> 100644 > >>>>>>>> --- a/lib/efi_loader/efi_memory.c > >>>>>>>> +++ b/lib/efi_loader/efi_memory.c > >>>>>>>> @@ -713,7 +713,7 @@ efi_status_t efi_free_pool(void *buffer) > >>>>>>>> /* Check that this memory was allocated by > >>>>>>>> efi_allocate_pool() > */ > >>>>>>>> if (((uintptr_t)alloc & EFI_PAGE_MASK) || > >>>>>>>> alloc->checksum != checksum(alloc)) { > >>>>>>>> - printf("%s: illegal free 0x%p\n", __func__, buffer); > >>>>>>>> + debug("%s: illegal free 0x%p\n", __func__, > >>>>>>>> + buffer); > >>>>>>>> return EFI_INVALID_PARAMETER; > >>>>>>>> } > >>>>>>>> /* Avoid double free */ > >>>>>>>> -- > >>>>>>>> 2.27.0 > >>>>>>>> > >>>>> > >>> > >