[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
> >>>>>>>>
> >>>>>
> >>>
> >
