[PATCH] ARM: dts: rainier,everest: Move reserved memory regions

[Andrew Jeffery]

Hi Adriana,

On Fri, 16 Sep 2022, at 06:38, Adriana Kobylak wrote:
> From: Adriana Kobylak <anoo at us.ibm.com>
>
> Move the reserved regions to account for a decrease in DRAM when ECC is
> enabled. ECC takes 1/9th of memory, bringing the size down from 1024MiB
> to 912MiB (minus 16MiB for VGA) for a total of 896MiB available DRAM.

The 912MiB mentioned here struck me, so I poked at the numbers:

ECC(1024MiB)
= ECC(1024 * 1024 * 1024)
= 1024 * 1024 * 1024 * 8 / 9
= 954437176.888888889
= 910.222222222MiB

>From there subtracting 16MiB gets us 894.222222222MiB, which isn't a 
number we see in practice.

On the other hand:

ECC(1024 - 16)MiB)
= ECC((1024 - 16) * 1024 * 1024)
= (1024 - 16) * 1024 * 1024 * 8 / 9
= 939524096
= 896MiB

We do see 896MiB in practice which implies that MCR54 is configured for 
ECC to be bounded at the bottom of a 16MiB VGA memory region. 912MiB 
doesn't fall out of the numbers, so I think we need to rework the words 
to avoid confusion?

>
> Move the regions by 128MiB since the flash_memory region needs to be
> aligned to 64MiB. This change doesn't affect the host if ECC is not
> enabled.

So I was curious about the 128MiB relocation as well. More on this 
below:

>
> Signed-off-by: Adriana Kobylak <anoo at us.ibm.com>
> ---
>  arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts | 11 ++++++-----
>  arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 12 ++++++++----
>  2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts 
> b/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
> index 1bba5ad7378e..9a09301dd79e 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-everest.dts
> @@ -163,15 +163,16 @@ reserved-memory {
>  		ranges;
> 
>  		/* LPC FW cycle bridge region requires natural alignment */
> -		flash_memory: region at b8000000 {
> +		flash_memory: region at b0000000 {
>  			no-map;
> -			reg = <0xb8000000 0x04000000>; /* 64M */
> +			reg = <0xb0000000 0x04000000>; /* 64M */
>  		};

0xb8000000 is an offset 2944MiB into the address space. The RAM base 
address is in the middle of the address space, so 2944MiB - 2048MiB, 
which means the buffer is offset 896MiB into RAM. This drives the need 
for the patch with ECC, as our RAM address space is now only 896MiB in 
size and basing the buffer at that offset is obviously invalid. So 
change is necessary.

We know that the flash_memory buffer needs to be naturally aligned, so
the fact that it was where it is we know that 896MiB is valid alignment
(as it already works). We also have the ramoops buffer below, which I
initially put after the flash_memory buffer to reduce the "hole" in
between vga_memory and flash_memory. However there's no other reason to
order it this way, and we can shift it as we see fit. Specifically,
we're free to order it before the flash_memory buffer.

Focusing back on the flash_memory buffer, if 896MiB is 64MiB aligned, 
then (896-64)MiB is also aligned. This means we can butt the 
flash_memory buffer up against the "end" of RAM, and with the 
observation that we can put the ramoops buffer before it, we can butt 
the ramoops buffer up against the start of flash_memory. This minimises 
the address-space fragmentation.

So maybe we can go with this arrangement?

ramoops at b3e00000 {
    compatible = "ramoops";
    reg = <0xb3e00000 0x200000>;
    ...
}

flash_memory: region at b4000000 {
    no-map;
    reg = <0xb4000000 0x04000000>;
    ...
};

Thoughts?

Andrew