On 17/04/24 21:34, Tom Rini wrote:
On Wed, Apr 17, 2024 at 05:48:31PM +0530, Sughosh Ganu wrote:
hi Chintan,
On Wed, 17 Apr 2024 at 13:21, Chintan Vankar <c-van...@ti.com> wrote:
On 16/04/24 22:30, Tom Rini wrote:
On Tue, Apr 16, 2024 at 05:52:58PM +0530, Chintan Vankar wrote:
On 12/04/24 03:37, Tom Rini wrote:
On Wed, Apr 03, 2024 at 06:18:01PM +0530, Chintan Vankar wrote:
On 22/01/24 10:11, Siddharth Vadapalli wrote:
On 20/01/24 22:11, Tom Rini wrote:
On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote:
Hello Tom,
On 12/01/24 18:56, Tom Rini wrote:
...
The list of conditionals in common/spl/spl.c::board_init_r() should be
updated and probably use SPL_NET as the option to check for.
Thank you for reviewing the patch and pointing this out. I wasn't aware of it. I
assume that you are referring to the following change:
if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) ||
- IS_ENABLED(CONFIG_SPL_ATF))
+ IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET))
dram_init_banksize();
I shall replace the current patch with the above change in the v2 series. Since
this is in the common section, is there a generic reason I could provide in the
commit message rather than the existing commit message which seems to be board
specific? Also, I hope that the above change will not cause regressions for
other non-TI devices. Please let me know.
Yes, that's the area, and just note that networking also requires the
DDR to be initialized.
Thank you for confirming and providing your suggestion for the contents of the
commit message.
Following Tom's Suggestion of adding CONFIG_SPL_NET in common/spl/spl.c
"dram_init_banksize()", the issue of fetching a file at SPL stage seemed
to be fixed. However the commit "ba20b2443c29", which sets gd->ram_top
for the very first time in "spl_enable_cache()" results in
"arch_lmb_reserve()" function reserving memory region from Stack pointer
at "0x81FFB820" to gd->ram_top pointing to "0x100000000". Previously
when gd->ram_top was zero "arch_lmb_reserve()" was noop. Now using TFTP
to fetch U-Boot image at SPL stage results in "tftp_init_load_addr()"
function call that invokes "arch_lmb_reserve()" function, which reserves
entire memory starting from Stack Pointer to gd->ram_top leaving no
space to load U-Boot image via TFTP since TFTP loads files at pre
configured memory address at "0x82000000".
As a workaround for this issue, one solution we can propose is to
disable the checks "lmb_get_free_size()" at SPL and U-Boot stage. For
that we can define a new config option for LMB reserve checks as
"SPL_LMB". This config will be enable by default for the backword
compatibility and disable for our use case at SPL and U-Boot stage.
The problem here is that we need LMB for booting an OS, which is
something we'll want in SPL in non-cortex-R cases too, which means this
platform, so that's a no-go. I think you need to dig harder and see if
you can correct the logic somewhere so that we don't over reserve?
Since this issue is due to function call "lmb_init_and_reserve()"
function invoked from "tftp_init_load_addr()" function. This function
is defined by Simon in commit "a156c47e39ad", which fixes
"CVE-2018-18439" to prevent overwriting reserved memory. Simon, can you
explain why do we need to call "lmb_init_and_reserve()" function here ?
This is indeed a tricky area which is why Sughosh is looking in to
trying to re-work the LMB mechanic and we've had a few long threads
about it as well.
I've honestly forgotten the use case you have here, can you please
remind us?
We are trying to boot AM62x using Ethernet for which we need to load
binary files at SPL and U-Boot stage using TFTP. To store the file we
need a free memory in RAM, specifically we are storing these files at
0x82000000. But we are facing an issue while loading the file since
the memory area having an address 0x82000000 is reserved due to
"lmb_init_and_reserve()" function call. This function is called in
"tftp_init_load_addr()" function which is getting called exactly before
we are trying to get the free memory area by calling
"lmb_get_free_size()".
I have no idea about your platform but I was wondering if there is any
particular importance of the load address of 0x82000000? It looks as
though the current location of the SP when arch_lmb_reserve() gets
called means that the load address is getting reserved for the U-Boot
image. Do you not have the option of loading the image at a lower
address instead?
Or using a higher address for SPL stack? You might be able to solve this
just by re-examining which addresses (and RAM size limitations) need to
be considered here.
Tom,
We changed SPL_STACK_R_ADDR to higher address as you suggested here and
observe that the memory area which was getting reserved by
"lmb_init_and_reserve()" function, when SPL_STACK_R_ADDR was 0x82000000,
is from 0x81FFB820 to gd->ram_top, but when SPL_STACK_R_ADDR is changed
to 0x83000000, stack pointer is pointing to 0x82FFB810 and reserving a
memory area till gd->ram_top. Since memory address 0x82000000 is not
there in reserved memory area region U-Boot proper is successfully
getting fetched and we are able to boot.
Can it be considered of changing "SPL_STACK_R_ADDR" independently for
Ethernet Boot feature ?
