[PATCH] arm: Prevent memory aliasing on non-LPAE kernels

Hello Stepan,

On Fri, May 31, 2013 at 6:45 AM, Stepan Moskovchenko
[off-list ref] wrote:

Some LPAE-capable systems may use a Device Tree containing
memory nodes that describe memory extending beyond the 4GB
physical address boundary. Ignore or truncate these memory
nodes on kernels that have not been built with LPAE
support, to prevent the extended physical addresses from
being truncated and aliasing with physical addresses below
the 4GB boundary.

Signed-off-by: Stepan Moskovchenko <redacted>
---
 arch/arm/kernel/devtree.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

Thanks for your efforts on fixing this issue. Before I was aware of
this patch I wrote a different implementation to solve most likely the
same issue, please see the following patches for more information.
Thanks to Arnd for pointing me in the right direction.

[PATCH 00/03] ARM: 64-bit memory fixes, APE6EVM second memory bank
[PATCH 01/03] ARM: Let arm_add_memory() always use 64-bit arguments
[PATCH 02/03] ARM: Handle 64-bit memory in case of 32-bit phys_addr_t
[PATCH 03/03] ARM: shmobile: Add second memory bank to DTS for APE6EVM

Regarding this patch, I have now tested it on my APE6EVM board
together with this patch:

[PATCH 03/03] ARM: shmobile: Add second memory bank to DTS for APE6EVM

Without your patch the situation is as follows:

HIGHMEM=n, LPAE=n - OK (busted, second bank ignored with message [1])
HIGHMEM=y, LPAE=n - NG (busted, board hangs on boot)
HIGHMEM=n, LPAE=y - OK
HIGHMEM=y, LPAE=y - OK

[1] Ignoring RAM at 00000000-3fffffff (vmalloc region overlap).

With your patch applied I get the following:

HIGHMEM=n, LPAE=n - OK (with message [2])
HIGHMEM=y, LPAE=n - OK (with message [2])
HIGHMEM=n, LPAE=y - OK
HIGHMEM=y, LPAE=y - OK

[2] Ignoring memory at 0x200000000 due to lack of LPAE support


So your patch unbreaks the second memory on my board perfectly well, thank you!

Regarding implementation details, I wonder if we only need to cover
the DT memory banks by performing the check inside
early_init_dt_add_memory_arch()?

To me the root cause of this issue seems to be how phys_addr_t is
configured when LPAE=n. It is understandable that the kernel cannot
handle 64-bit addresses when phys_addr_t is 32-bit, but I believe we
need some sane way to omit those memory banks. Your patch handles the
non-LPAE case before phys_addr_t is involved which seems to work well.
Your approach is much better compared to as-is today with potentially
wrapping phys_addr_t parameters to arm_add_memory().

The only question in my mind is about the location for this kind of
test, shall it be done in early_init_dt_add_memory_arch() or
arm_add_memory()?

If we care about adding some bounds checking for the kernel command
line mem=xxx option then arm_add_memory() seems to be the best
location from my point of view.

Any ideas?

Please add me to CC if you respin your patch. I will give it a go on my board.

Thanks,

/ magnus