| Message ID | 20220915210852.858170-1-anoo@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | ARM: dts: rainier,everest: Move reserved memory regions | expand |
Hi Adriana, On Fri, 16 Sep 2022, at 06:38, Adriana Kobylak wrote: > From: Adriana Kobylak <anoo@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@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@b8000000 { > + flash_memory: region@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@b3e00000 { compatible = "ramoops"; reg = <0xb3e00000 0x200000>; ... } flash_memory: region@b4000000 { no-map; reg = <0xb4000000 0x04000000>; ... }; Thoughts? Andrew
> On Sep 16, 2022, at 12:35 AM, Andrew Jeffery <andrew@aj.id.au> wrote: > > Hi Adriana, > > On Fri, 16 Sep 2022, at 06:38, Adriana Kobylak wrote: >> From: Adriana Kobylak <anoo@us.ibm.com <mailto:anoo@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@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@b8000000 { >> + flash_memory: region@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@b3e00000 { > compatible = "ramoops"; > reg = <0xb3e00000 0x200000>; > ... > } > > flash_memory: region@b4000000 { > no-map; > reg = <0xb4000000 0x04000000>; > ... > }; > > Thoughts? Thanks Andrew for the background and explanation. This would be even better like you mentioned to minimize the fragmentation. I’ll send a v2 with those changes. > > Andrew
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@b8000000 { + flash_memory: region@b0000000 { no-map; - reg = <0xb8000000 0x04000000>; /* 64M */ + reg = <0xb0000000 0x04000000>; /* 64M */ }; - /* 48MB region from the end of flash to start of vga memory */ - ramoops@bc000000 { + /* 176MB region from the end of flash to start of vga memory */ + + ramoops@b4000000 { compatible = "ramoops"; - reg = <0xbc000000 0x200000>; /* 16 * (4 * 0x8000) */ + reg = <0xb4000000 0x200000>; /* 16 * (4 * 0x8000) */ record-size = <0x8000>; console-size = <0x8000>; ftrace-size = <0x8000>; diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts index 8bd2f441b159..01e7d8f46dc7 100644 --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts @@ -95,14 +95,17 @@ reserved-memory { #size-cells = <1>; ranges; - flash_memory: region@b8000000 { + /* LPC FW cycle bridge region requires natural alignment */ + flash_memory: region@b0000000 { no-map; - reg = <0xb8000000 0x04000000>; /* 64M */ + reg = <0xb0000000 0x04000000>; /* 64M */ }; - ramoops@bc000000 { + /* 176MB region from the end of flash to start of vga memory */ + + ramoops@b4000000 { compatible = "ramoops"; - reg = <0xbc000000 0x200000>; /* 16 * (4 * 0x8000) */ + reg = <0xb4000000 0x200000>; /* 16 * (4 * 0x8000) */ record-size = <0x8000>; console-size = <0x8000>; ftrace-size = <0x8000>; @@ -110,6 +113,7 @@ ramoops@bc000000 { max-reason = <3>; /* KMSG_DUMP_EMERG */ }; + /* VGA region is dictated by hardware strapping */ vga_memory: region@bf000000 { no-map; compatible = "shared-dma-pool";