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

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

[Stefan Eichenberger]

From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>
---
 common/memsize.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/common/memsize.c b/common/memsize.c
index 86109579c95..8fa1b0e2a00 100644
--- a/common/memsize.c
+++ b/common/memsize.c
@@ -82,6 +82,8 @@ long get_ram_size(long *base, long maxsize)
 			addr  = base + cnt;
 			sync();
 			*addr = save[--i];
+			if (dcache_en)
+				dcache_flush_invalidate(addr);
 		}
 		return (0);
 	}
@@ -90,6 +92,8 @@ long get_ram_size(long *base, long maxsize)
 		addr = base + cnt;	/* pointer arith! */
 		val = *addr;
 		*addr = save[--i];
+		if (dcache_en)
+			dcache_flush_invalidate(addr);
 		if (val != ~cnt) {
 			size = cnt * sizeof(long);
 			/*
@@ -101,6 +105,8 @@ long get_ram_size(long *base, long maxsize)
 			     cnt <<= 1) {
 				addr  = base + cnt;
 				*addr = save[--i];
+				if (dcache_en)
+					dcache_flush_invalidate(addr);
 			}
 			/* warning: don't restore save_base in this case,
 			 * it is already done in the loop because
@@ -112,6 +118,8 @@ long get_ram_size(long *base, long maxsize)
 		}
 	}
 	*base = save_base;
+	if (dcache_en)
+		dcache_flush_invalidate(base);
 
 	return (maxsize);
 }
-- 
2.45.2

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

[Francesco Dolcini]

Hello Tom,

On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>

This patch is for master, and it is hopefully fixing the last remaining
issue that is preventing our board to boot with current U-Boot.

Thanks,
Francesco

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1763 bytes --]

On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:

> From: Stefan Eichenberger <stefan.eichenberger@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@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?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [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@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@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

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

[Francesco Dolcini]

Hello Tom,

On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>

Tom, can we merge this?
This is the last bit to solve the regression reported here,
https://lore.kernel.org/all/20260224152405.GD340942@francesco-nb/

Thanks,
Francesco

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1858 bytes --]

On Thu, Feb 26, 2026 at 08:05:02AM +0100, Francesco Dolcini wrote:
> Hello Tom,
> 
> On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
> > From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>
> 
> Tom, can we merge this?
> This is the last bit to solve the regression reported here,
> https://lore.kernel.org/all/20260224152405.GD340942@francesco-nb/

I wasn't happy with this at the time, and Stefan's last email in the
thread left me with the impression more investigation was needed and
likely something else was the root cause.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Francesco Dolcini]

+Emanuele

Hello Tom,

On Thu, Feb 26, 2026 at 08:23:45AM -0600, Tom Rini wrote:
> On Thu, Feb 26, 2026 at 08:05:02AM +0100, Francesco Dolcini wrote:
> > Hello Tom,
> > 
> > On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
> > > From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>
> > 
> > Tom, can we merge this?
> > This is the last bit to solve the regression reported here,
> > https://lore.kernel.org/all/20260224152405.GD340942@francesco-nb/
> 
> I wasn't happy with this at the time, and Stefan's last email in the
> thread left me with the impression more investigation was needed and
> likely something else was the root cause.

I believe that this patch is needed.

On AM62 what is happening is the following.

We have a cortex-R5 that is the first core booting (there is also a
cortex-m4, but it's not relevant for this discussion).

It runs from internal memory and it configures the DDR ram

We load to DDR memory various pieces of firmware (TFA, U-Boot for the
cortex A53, ...)

We do execute get_ram_size(), that read/write the memory, and it is
supposed to restore it back the original content

However when we have the cache enabled, we might miss to write back the
original memory content, where the other pieces of firmware are.

And after that we start the cortex A53, running in DDR, and there the
memory content might not be correct, because there is no cache coherency
between the cortex-A and the cortex-R. And because of that we have
crashes.

Stefan: any comment here? Can you help?

Francesco

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

[Stefan Eichenberger]

Hi Francesco and Tom,

On Thu, Feb 26, 2026 at 05:11:49PM +0100, Francesco Dolcini wrote:
> +Emanuele
> 
> Hello Tom,
> 
> On Thu, Feb 26, 2026 at 08:23:45AM -0600, Tom Rini wrote:
> > On Thu, Feb 26, 2026 at 08:05:02AM +0100, Francesco Dolcini wrote:
> > > Hello Tom,
> > > 
> > > On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
> > > > From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>
> > > 
> > > Tom, can we merge this?
> > > This is the last bit to solve the regression reported here,
> > > https://lore.kernel.org/all/20260224152405.GD340942@francesco-nb/
> > 
> > I wasn't happy with this at the time, and Stefan's last email in the
> > thread left me with the impression more investigation was needed and
> > likely something else was the root cause.
> 
> I believe that this patch is needed.
> 
> On AM62 what is happening is the following.
> 
> We have a cortex-R5 that is the first core booting (there is also a
> cortex-m4, but it's not relevant for this discussion).
> 
> It runs from internal memory and it configures the DDR ram
> 
> We load to DDR memory various pieces of firmware (TFA, U-Boot for the
> cortex A53, ...)
> 
> We do execute get_ram_size(), that read/write the memory, and it is
> supposed to restore it back the original content
> 
> However when we have the cache enabled, we might miss to write back the
> original memory content, where the other pieces of firmware are.
> 
> And after that we start the cortex A53, running in DDR, and there the
> memory content might not be correct, because there is no cache coherency
> between the cortex-A and the cortex-R. And because of that we have
> crashes.
> 
> Stefan: any comment here? Can you help?

I think what you wrote summarises the issue well. If I recall correctly,
I "fixed" the issue last time by simply calling get_ram_size() once
before enabling the cache. This was in commit 4164289db882e. The SPL
then informs U-Boot of the memory size via fdt fixup. However, something
has probably changed now (possibly in the R5 SPL), meaning the cache is
enabled earlier, so the cache is enabled again when get_ram_size() is
called.

For the AMP use case, either "get_ram_size" should not be called once
the cache is enabled, or a similar patch to the one I proposed is
required.

Regards,
Stefan

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4018 bytes --]

On Thu, Feb 26, 2026 at 05:30:06PM +0100, Stefan Eichenberger wrote:
> Hi Francesco and Tom,
> 
> On Thu, Feb 26, 2026 at 05:11:49PM +0100, Francesco Dolcini wrote:
> > +Emanuele
> > 
> > Hello Tom,
> > 
> > On Thu, Feb 26, 2026 at 08:23:45AM -0600, Tom Rini wrote:
> > > On Thu, Feb 26, 2026 at 08:05:02AM +0100, Francesco Dolcini wrote:
> > > > Hello Tom,
> > > > 
> > > > On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
> > > > > From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>
> > > > 
> > > > Tom, can we merge this?
> > > > This is the last bit to solve the regression reported here,
> > > > https://lore.kernel.org/all/20260224152405.GD340942@francesco-nb/
> > > 
> > > I wasn't happy with this at the time, and Stefan's last email in the
> > > thread left me with the impression more investigation was needed and
> > > likely something else was the root cause.
> > 
> > I believe that this patch is needed.
> > 
> > On AM62 what is happening is the following.
> > 
> > We have a cortex-R5 that is the first core booting (there is also a
> > cortex-m4, but it's not relevant for this discussion).
> > 
> > It runs from internal memory and it configures the DDR ram
> > 
> > We load to DDR memory various pieces of firmware (TFA, U-Boot for the
> > cortex A53, ...)
> > 
> > We do execute get_ram_size(), that read/write the memory, and it is
> > supposed to restore it back the original content
> > 
> > However when we have the cache enabled, we might miss to write back the
> > original memory content, where the other pieces of firmware are.
> > 
> > And after that we start the cortex A53, running in DDR, and there the
> > memory content might not be correct, because there is no cache coherency
> > between the cortex-A and the cortex-R. And because of that we have
> > crashes.
> > 
> > Stefan: any comment here? Can you help?
> 
> I think what you wrote summarises the issue well. If I recall correctly,
> I "fixed" the issue last time by simply calling get_ram_size() once
> before enabling the cache. This was in commit 4164289db882e. The SPL
> then informs U-Boot of the memory size via fdt fixup. However, something
> has probably changed now (possibly in the R5 SPL), meaning the cache is
> enabled earlier, so the cache is enabled again when get_ram_size() is
> called.
> 
> For the AMP use case, either "get_ram_size" should not be called once
> the cache is enabled, or a similar patch to the one I proposed is
> required.

I would lean towards the former if at all possible.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Francis, Neha]

On 2/26/2026 10:01 PM, Tom Rini wrote:
> On Thu, Feb 26, 2026 at 05:30:06PM +0100, Stefan Eichenberger wrote:
>> Hi Francesco and Tom,
>>
>> On Thu, Feb 26, 2026 at 05:11:49PM +0100, Francesco Dolcini wrote:
>>> +Emanuele
>>>
>>> Hello Tom,
>>>
>>> On Thu, Feb 26, 2026 at 08:23:45AM -0600, Tom Rini wrote:
>>>> On Thu, Feb 26, 2026 at 08:05:02AM +0100, Francesco Dolcini wrote:
>>>>> Hello Tom,
>>>>>
>>>>> On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
>>>>>> From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>
>>>>>
>>>>> Tom, can we merge this?
>>>>> This is the last bit to solve the regression reported here,
>>>>> https://lore.kernel.org/all/20260224152405.GD340942@francesco-nb/
>>>>
>>>> I wasn't happy with this at the time, and Stefan's last email in the
>>>> thread left me with the impression more investigation was needed and
>>>> likely something else was the root cause.
>>>
>>> I believe that this patch is needed.
>>>
>>> On AM62 what is happening is the following.
>>>
>>> We have a cortex-R5 that is the first core booting (there is also a
>>> cortex-m4, but it's not relevant for this discussion).
>>>
>>> It runs from internal memory and it configures the DDR ram
>>>
>>> We load to DDR memory various pieces of firmware (TFA, U-Boot for the
>>> cortex A53, ...)
>>>
>>> We do execute get_ram_size(), that read/write the memory, and it is
>>> supposed to restore it back the original content
>>>
>>> However when we have the cache enabled, we might miss to write back the
>>> original memory content, where the other pieces of firmware are.
>>>
>>> And after that we start the cortex A53, running in DDR, and there the
>>> memory content might not be correct, because there is no cache coherency
>>> between the cortex-A and the cortex-R. And because of that we have
>>> crashes.
>>>
>>> Stefan: any comment here? Can you help?
>>
>> I think what you wrote summarises the issue well. If I recall correctly,
>> I "fixed" the issue last time by simply calling get_ram_size() once
>> before enabling the cache. This was in commit 4164289db882e. The SPL
>> then informs U-Boot of the memory size via fdt fixup. However, something
>> has probably changed now (possibly in the R5 SPL), meaning the cache is
>> enabled earlier, so the cache is enabled again when get_ram_size() is
>> called.
>>
>> For the AMP use case, either "get_ram_size" should not be called once
>> the cache is enabled, or a similar patch to the one I proposed is
>> required.
> 
> I would lean towards the former if at all possible.
> 

Just trying to understand, what is the reasoning behind ensuring get_ram_size is
not called if cache is not enabled? Wasn't get_ram_size written with the
possibility of cache being enabled (existence of dcache_en logic); then this
patch is a valid fix right?

In parallel, I do agree we need to have a code analysis w.r.t dram_init, we are
making certain cache and dram calls spuriously making this confusing.

-- 
Thanking You
Neha Malcom Francis

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

[Emanuele Ghidoli]

On 2/27/26 11:13, Francis, Neha wrote:
> 
> 
> On 2/26/2026 10:01 PM, Tom Rini wrote:
>> On Thu, Feb 26, 2026 at 05:30:06PM +0100, Stefan Eichenberger wrote:
>>> Hi Francesco and Tom,
>>>
>>> On Thu, Feb 26, 2026 at 05:11:49PM +0100, Francesco Dolcini wrote:
>>>> +Emanuele
>>>>
>>>> Hello Tom,
>>>>
>>>> On Thu, Feb 26, 2026 at 08:23:45AM -0600, Tom Rini wrote:
>>>>> On Thu, Feb 26, 2026 at 08:05:02AM +0100, Francesco Dolcini wrote:
>>>>>> Hello Tom,
>>>>>>
>>>>>> On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
>>>>>>> From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>
>>>>>>
>>>>>> Tom, can we merge this?
>>>>>> This is the last bit to solve the regression reported here,
>>>>>> https://lore.kernel.org/all/20260224152405.GD340942@francesco-nb/
>>>>>
>>>>> I wasn't happy with this at the time, and Stefan's last email in the
>>>>> thread left me with the impression more investigation was needed and
>>>>> likely something else was the root cause.
>>>>
>>>> I believe that this patch is needed.
>>>>
>>>> On AM62 what is happening is the following.
>>>>
>>>> We have a cortex-R5 that is the first core booting (there is also a
>>>> cortex-m4, but it's not relevant for this discussion).
>>>>
>>>> It runs from internal memory and it configures the DDR ram
>>>>
>>>> We load to DDR memory various pieces of firmware (TFA, U-Boot for the
>>>> cortex A53, ...)
>>>>
>>>> We do execute get_ram_size(), that read/write the memory, and it is
>>>> supposed to restore it back the original content
>>>>
>>>> However when we have the cache enabled, we might miss to write back the
>>>> original memory content, where the other pieces of firmware are.
>>>>
>>>> And after that we start the cortex A53, running in DDR, and there the
>>>> memory content might not be correct, because there is no cache coherency
>>>> between the cortex-A and the cortex-R. And because of that we have
>>>> crashes.
>>>>
>>>> Stefan: any comment here? Can you help?
>>>
>>> I think what you wrote summarises the issue well. If I recall correctly,
>>> I "fixed" the issue last time by simply calling get_ram_size() once
>>> before enabling the cache. This was in commit 4164289db882e. The SPL
>>> then informs U-Boot of the memory size via fdt fixup. However, something
>>> has probably changed now (possibly in the R5 SPL), meaning the cache is
>>> enabled earlier, so the cache is enabled again when get_ram_size() is
>>> called.
>>>
>>> For the AMP use case, either "get_ram_size" should not be called once
>>> the cache is enabled, or a similar patch to the one I proposed is
>>> required.
>>
>> I would lean towards the former if at all possible.
>>
> 
> Just trying to understand, what is the reasoning behind ensuring get_ram_size is
> not called if cache is not enabled? Wasn't get_ram_size written with the
> possibility of cache being enabled (existence of dcache_en logic); then this
> patch is a valid fix right?
> 
> In parallel, I do agree we need to have a code analysis w.r.t dram_init, we are
> making certain cache and dram calls spuriously making this confusing.
> 

Hello Tom,
I agree with Francis.

When I proposed commit 1c64b98c1ec4 ("common/memsize.c: Fix get_ram_size()
when cache is enabled"), I was not considering the presence of other actors
(other cores, DMA engines, etc.).

That patch fixes what I had overlooked at the time. We need to restore the
actual RAM contents, not only what is perceived by the core executing
get_ram_size().

To me this patch sounds intrinsically correct.

Thank you and kind regards,
Emanuele

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

[Francesco Dolcini]

On Fri, Feb 27, 2026 at 03:43:07PM +0530, Francis, Neha wrote:
> On 2/26/2026 10:01 PM, Tom Rini wrote:
> > On Thu, Feb 26, 2026 at 05:30:06PM +0100, Stefan Eichenberger wrote:
> >> On Thu, Feb 26, 2026 at 05:11:49PM +0100, Francesco Dolcini wrote:
> >>> On Thu, Feb 26, 2026 at 08:23:45AM -0600, Tom Rini wrote:
> >>>> On Thu, Feb 26, 2026 at 08:05:02AM +0100, Francesco Dolcini wrote:
> >>>>> On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
> >>>>>> From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>
> >>>>>
> >>>>> Tom, can we merge this?
> >>>>> This is the last bit to solve the regression reported here,
> >>>>> https://lore.kernel.org/all/20260224152405.GD340942@francesco-nb/
> >>>>
> >>>> I wasn't happy with this at the time, and Stefan's last email in the
> >>>> thread left me with the impression more investigation was needed and
> >>>> likely something else was the root cause.
> >>>
> >>> I believe that this patch is needed.
> >>>
> >>> On AM62 what is happening is the following.
> >>>
> >>> We have a cortex-R5 that is the first core booting (there is also a
> >>> cortex-m4, but it's not relevant for this discussion).
> >>>
> >>> It runs from internal memory and it configures the DDR ram
> >>>
> >>> We load to DDR memory various pieces of firmware (TFA, U-Boot for the
> >>> cortex A53, ...)
> >>>
> >>> We do execute get_ram_size(), that read/write the memory, and it is
> >>> supposed to restore it back the original content
> >>>
> >>> However when we have the cache enabled, we might miss to write back the
> >>> original memory content, where the other pieces of firmware are.
> >>>
> >>> And after that we start the cortex A53, running in DDR, and there the
> >>> memory content might not be correct, because there is no cache coherency
> >>> between the cortex-A and the cortex-R. And because of that we have
> >>> crashes.
> >>>
> >>> Stefan: any comment here? Can you help?
> >>
> >> I think what you wrote summarises the issue well. If I recall correctly,
> >> I "fixed" the issue last time by simply calling get_ram_size() once
> >> before enabling the cache. This was in commit 4164289db882e. The SPL
> >> then informs U-Boot of the memory size via fdt fixup. However, something
> >> has probably changed now (possibly in the R5 SPL), meaning the cache is
> >> enabled earlier, so the cache is enabled again when get_ram_size() is
> >> called.
> >>
> >> For the AMP use case, either "get_ram_size" should not be called once
> >> the cache is enabled, or a similar patch to the one I proposed is
> >> required.
> > 
> > I would lean towards the former if at all possible.
> > 
> Just trying to understand, what is the reasoning behind ensuring get_ram_size is
> not called if cache is not enabled? Wasn't get_ram_size written with the
> possibility of cache being enabled (existence of dcache_en logic); then this
> patch is a valid fix right?

Yes, I am also confused about this.

Right now get_ram_size works well if cache is enabled ... apart this issue
in which the memory state is correct only from the CPU core that called the
function (or that we have some kind of HW cache coherency in place, but this is
not the case for AM62 A53/R5 cores).

If I have to solve the issue outside of get_ram_size(), I can either disable
the cache before calling it, or flush the whole cache to the DDR just afterward.

IOW, to me the patch is conceptually correct and I would apply it.

Francesco

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 5351 bytes --]

On Fri, Feb 27, 2026 at 11:39:44AM +0100, Emanuele Ghidoli wrote:
> 
> 
> On 2/27/26 11:13, Francis, Neha wrote:
> > 
> > 
> > On 2/26/2026 10:01 PM, Tom Rini wrote:
> >> On Thu, Feb 26, 2026 at 05:30:06PM +0100, Stefan Eichenberger wrote:
> >>> Hi Francesco and Tom,
> >>>
> >>> On Thu, Feb 26, 2026 at 05:11:49PM +0100, Francesco Dolcini wrote:
> >>>> +Emanuele
> >>>>
> >>>> Hello Tom,
> >>>>
> >>>> On Thu, Feb 26, 2026 at 08:23:45AM -0600, Tom Rini wrote:
> >>>>> On Thu, Feb 26, 2026 at 08:05:02AM +0100, Francesco Dolcini wrote:
> >>>>>> Hello Tom,
> >>>>>>
> >>>>>> On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
> >>>>>>> From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>
> >>>>>>
> >>>>>> Tom, can we merge this?
> >>>>>> This is the last bit to solve the regression reported here,
> >>>>>> https://lore.kernel.org/all/20260224152405.GD340942@francesco-nb/
> >>>>>
> >>>>> I wasn't happy with this at the time, and Stefan's last email in the
> >>>>> thread left me with the impression more investigation was needed and
> >>>>> likely something else was the root cause.
> >>>>
> >>>> I believe that this patch is needed.
> >>>>
> >>>> On AM62 what is happening is the following.
> >>>>
> >>>> We have a cortex-R5 that is the first core booting (there is also a
> >>>> cortex-m4, but it's not relevant for this discussion).
> >>>>
> >>>> It runs from internal memory and it configures the DDR ram
> >>>>
> >>>> We load to DDR memory various pieces of firmware (TFA, U-Boot for the
> >>>> cortex A53, ...)
> >>>>
> >>>> We do execute get_ram_size(), that read/write the memory, and it is
> >>>> supposed to restore it back the original content
> >>>>
> >>>> However when we have the cache enabled, we might miss to write back the
> >>>> original memory content, where the other pieces of firmware are.
> >>>>
> >>>> And after that we start the cortex A53, running in DDR, and there the
> >>>> memory content might not be correct, because there is no cache coherency
> >>>> between the cortex-A and the cortex-R. And because of that we have
> >>>> crashes.
> >>>>
> >>>> Stefan: any comment here? Can you help?
> >>>
> >>> I think what you wrote summarises the issue well. If I recall correctly,
> >>> I "fixed" the issue last time by simply calling get_ram_size() once
> >>> before enabling the cache. This was in commit 4164289db882e. The SPL
> >>> then informs U-Boot of the memory size via fdt fixup. However, something
> >>> has probably changed now (possibly in the R5 SPL), meaning the cache is
> >>> enabled earlier, so the cache is enabled again when get_ram_size() is
> >>> called.
> >>>
> >>> For the AMP use case, either "get_ram_size" should not be called once
> >>> the cache is enabled, or a similar patch to the one I proposed is
> >>> required.
> >>
> >> I would lean towards the former if at all possible.
> >>
> > 
> > Just trying to understand, what is the reasoning behind ensuring get_ram_size is
> > not called if cache is not enabled? Wasn't get_ram_size written with the
> > possibility of cache being enabled (existence of dcache_en logic); then this
> > patch is a valid fix right?
> > 
> > In parallel, I do agree we need to have a code analysis w.r.t dram_init, we are
> > making certain cache and dram calls spuriously making this confusing.
> > 
> 
> Hello Tom,
> I agree with Francis.
> 
> When I proposed commit 1c64b98c1ec4 ("common/memsize.c: Fix get_ram_size()
> when cache is enabled"), I was not considering the presence of other actors
> (other cores, DMA engines, etc.).
> 
> That patch fixes what I had overlooked at the time. We need to restore the
> actual RAM contents, not only what is perceived by the core executing
> get_ram_size().
> 
> To me this patch sounds intrinsically correct.

Alright. Can I please get some Reviewed / Tested by tags? Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Francesco Dolcini]

On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>

Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com> # Toradex Verdin AM62

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

[Emanuele Ghidoli]

On 2/27/26 18:39, Tom Rini wrote:
> On Fri, Feb 27, 2026 at 11:39:44AM +0100, Emanuele Ghidoli wrote:
>>
>>
>> On 2/27/26 11:13, Francis, Neha wrote:
>>>
>>>
>>> On 2/26/2026 10:01 PM, Tom Rini wrote:
>>>> On Thu, Feb 26, 2026 at 05:30:06PM +0100, Stefan Eichenberger wrote:
>>>>> Hi Francesco and Tom,
>>>>>
>>>>> On Thu, Feb 26, 2026 at 05:11:49PM +0100, Francesco Dolcini wrote:
>>>>>> +Emanuele
>>>>>>
>>>>>> Hello Tom,
>>>>>>
>>>>>> On Thu, Feb 26, 2026 at 08:23:45AM -0600, Tom Rini wrote:
>>>>>>> On Thu, Feb 26, 2026 at 08:05:02AM +0100, Francesco Dolcini wrote:
>>>>>>>> Hello Tom,
>>>>>>>>
>>>>>>>> On Fri, Mar 14, 2025 at 11:06:49AM +0100, Stefan Eichenberger wrote:
>>>>>>>>> From: Stefan Eichenberger <stefan.eichenberger@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@toradex.com>
>>>>>>>>
>>>>>>>> Tom, can we merge this?
>>>>>>>> This is the last bit to solve the regression reported here,
>>>>>>>> https://lore.kernel.org/all/20260224152405.GD340942@francesco-nb/
>>>>>>>
>>>>>>> I wasn't happy with this at the time, and Stefan's last email in the
>>>>>>> thread left me with the impression more investigation was needed and
>>>>>>> likely something else was the root cause.
>>>>>>
>>>>>> I believe that this patch is needed.
>>>>>>
>>>>>> On AM62 what is happening is the following.
>>>>>>
>>>>>> We have a cortex-R5 that is the first core booting (there is also a
>>>>>> cortex-m4, but it's not relevant for this discussion).
>>>>>>
>>>>>> It runs from internal memory and it configures the DDR ram
>>>>>>
>>>>>> We load to DDR memory various pieces of firmware (TFA, U-Boot for the
>>>>>> cortex A53, ...)
>>>>>>
>>>>>> We do execute get_ram_size(), that read/write the memory, and it is
>>>>>> supposed to restore it back the original content
>>>>>>
>>>>>> However when we have the cache enabled, we might miss to write back the
>>>>>> original memory content, where the other pieces of firmware are.
>>>>>>
>>>>>> And after that we start the cortex A53, running in DDR, and there the
>>>>>> memory content might not be correct, because there is no cache coherency
>>>>>> between the cortex-A and the cortex-R. And because of that we have
>>>>>> crashes.
>>>>>>
>>>>>> Stefan: any comment here? Can you help?
>>>>>
>>>>> I think what you wrote summarises the issue well. If I recall correctly,
>>>>> I "fixed" the issue last time by simply calling get_ram_size() once
>>>>> before enabling the cache. This was in commit 4164289db882e. The SPL
>>>>> then informs U-Boot of the memory size via fdt fixup. However, something
>>>>> has probably changed now (possibly in the R5 SPL), meaning the cache is
>>>>> enabled earlier, so the cache is enabled again when get_ram_size() is
>>>>> called.
>>>>>
>>>>> For the AMP use case, either "get_ram_size" should not be called once
>>>>> the cache is enabled, or a similar patch to the one I proposed is
>>>>> required.
>>>>
>>>> I would lean towards the former if at all possible.
>>>>
>>>
>>> Just trying to understand, what is the reasoning behind ensuring get_ram_size is
>>> not called if cache is not enabled? Wasn't get_ram_size written with the
>>> possibility of cache being enabled (existence of dcache_en logic); then this
>>> patch is a valid fix right?
>>>
>>> In parallel, I do agree we need to have a code analysis w.r.t dram_init, we are
>>> making certain cache and dram calls spuriously making this confusing.
>>>
>>
>> Hello Tom,
>> I agree with Francis.
>>
>> When I proposed commit 1c64b98c1ec4 ("common/memsize.c: Fix get_ram_size()
>> when cache is enabled"), I was not considering the presence of other actors
>> (other cores, DMA engines, etc.).
>>
>> That patch fixes what I had overlooked at the time. We need to restore the
>> actual RAM contents, not only what is perceived by the core executing
>> get_ram_size().
>>
>> To me this patch sounds intrinsically correct.
> 
> Alright. Can I please get some Reviewed / Tested by tags? Thanks.
> 

Reviewed-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>

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

[Tom Rini]

On Fri, 14 Mar 2025 11:06:49 +0100, Stefan Eichenberger wrote:

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

Applied to u-boot/master, thanks!

[1/1] common/memsize.c: Fix get_ram_size() original data restore
      commit: 8d24789abed0822fbe41a2f9d72cf19650159dc6
-- 
Tom