[PATCH v1] common/memsize.c: Fix get_ram_size() original data restore

[Stefan Eichenberger]

Hi Tom,

On Fri, Mar 14, 2025 at 10:34:22AM -0600, Tom Rini wrote:
> On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
> 
> > From: Stefan Eichenberger <stefan.eichenberger at toradex.com>
> > 
> > The get_ram_size() function fails to restore the original RAM data when
> > the data cache is enabled. This issue was observed on an AM625 R5 SPL
> > with 512MB of RAM and is a regression that became visible with
> > commit bc07851897bd ("board: ti: Pull redundant DDR functions to a common
> > location and Fixup DDR size when ECC is enabled").
> > 
> > Observed boot failure messages:
> >   Warning: Did not detect image signing certificate. Skipping authentication to prevent boot failure. This will fail on Security Enforcing(HS-SE) devices
> >   Authentication passed
> >   Starting ATF on ARM64 core...
> > 
> > The system then hangs. This indicates that without a data cache flush,
> > data in the cache is not coherent with RAM, preventing the system from
> > booting. This was verified by printing the content of this address when
> > the issue occurs.
> > 
> > Add a data cache flush after each restore operation to resolve this
> > issue.
> > 
> > Fixes: bc07851897bd ("board: ti: Pull redundant DDR functions to a common location and Fixup DDR size when ECC is enabled")
> > Fixes: 1c64b98c1ec4 ("common/memsize.c: Fix get_ram_size() when cache is enabled")
> > Signed-off-by: Stefan Eichenberger <stefan.eichenberger at toradex.com>
> > ---
> >  common/memsize.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> 
> Ugh. Is there not a chance the problem being that we need a
> dcache_flush_all() in the K3 ddr code somewhere? I'm just trying to see
> how we got here and I notice now that yes, this does look like it
> finishes what 1c64b98c1ec4 started but in hindsight was that showing
> something else needing to be fixed?

I am also not entirely satisfied with the current fix. I rechecked with
U-Boot v2024.10, where we did not observe the issue. This difference is
due to the cache always being disabled when get_ram_size() is called.
This explains why it was working before commit bc07851897bd ("board: ti:
Pull redundant DDR functions to a common location and Fixup DDR size
when ECC is enabled").

I think my previous fix introduced this second call to get_ram_size()
when adding:
void spl_perform_fixups(struct spl_image_info *spl_image)
{
	fixup_memory_node(spl_image);
}

Doing some more tests now shows that probably only adding the following
would have been sufficient:
gd->bd->bi_dram[0].size = gd->ram_size;

If confirmed through further testing, this patch may be unnecessary.

Regards,
Stefan