[PATCH 1/2] arm: mm: Ignore memory banks which are in front of the kernel when HIGHMEM is ON

[PATCH 1/2] arm: mm: Ignore memory banks which are in front of the kernel when HIGHMEM is ON

[Michal Simek]

Arm kernel with HIGHMEM ON will fail when dts memory node is described
by memory banks and the starting address is not inside the first 16MB
of the first memory bank. If HIGHMEM is OFF and the configuration is the same
then the first memory bank is ignored and the kernel boots.

Here is the example of behavior:
dts memory reg = <0x0 0x10000000 0x10000000 0x30000000>
kernel load address = 0x10000000 (respectively 0x1000800)

Current:
"Machine: Xilinx Zynq Platform, model: Zynq ZC702 Development Board
bootconsole [earlycon0] enabled
Memory policy: ECC disabled, Data cache writeback
Kernel panic - not syncing: ERROR: Failed to allocate 0x1000 bytes below 0x0."

After:
The kernel ignore ram 0x0-0x0fffffff because is lower than the kernel starting
address and the kernel bootlog contains.
"Ignoring RAM at 00000000-0fffffff (CONFIG_HIGHMEM)."

Also using mem=768M on the command line will overwrite dts memory
map and kernel will boot with HIGHMEM ON too.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 arch/arm/mm/mmu.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index ce328c7..e60f370 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -937,6 +937,15 @@ void __init sanity_check_meminfo(void)
 			highmem = 1;
 
 #ifdef CONFIG_HIGHMEM
+		if (__va(bank->start + bank->size - 1) < (void *)PAGE_OFFSET) {
+			pr_notice("Ignoring RAM@%.8llx-%.8llx ",
+				(unsigned long long)bank->start,
+				(unsigned long long)bank->start
+							+ bank->size - 1);
+			pr_cont("(CONFIG_HIGHMEM).\n");
+			continue;
+		}
+
 		if (__va(bank->start) >= vmalloc_min ||
 		    __va(bank->start) < (void *)PAGE_OFFSET)
 			highmem = 1;
-- 
1.7.0.4

[PATCH 2/2] arm: mm: Add support for booting the kernel out of the first 16MB of memory

[Michal Simek]

The kernel should be loaded to the first 16MB memory and DTS
memory node should reflect this as well.

In case when the kernel is loaded to addresses higher than 16MB, the kernel
doesn't boot and fail.
This patch fix this case by changing the current memory bank start address
to the kernel loading address. The memory which is in front of the kernel
loading address will be ignored (which is also the current behavior
if memory is described with several memory banks).

Here is the example of behavior:
dts memory reg = <0x0 0x40000000>;
kernel load address = 0x10000000 (respectively 0x1000800)

Current:
The kernel doesn't boot and fails with this error:
"Ignoring RAM at 00000000-3fffffff (vmalloc region overlap).
Memory policy: ECC disabled, Data cache writeback
Kernel panic - not syncing: ERROR: Failed to allocate 0x1000 bytes below 0x0."

When this patch is applied:
The kernel works only with 0x10000000-0x3fffffff. (0x0-0x0fffffff is ignored)
And the kernel bootlog contains "Change memory bank to 10000000-3fffffff"
message.

The patch just add the same behavior as is the case
when you setup mem=768M on the command line and load the kernel
to the 0x10000000(0x10008000) address. When mem is specified
dts memory description is simply ignored.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 arch/arm/mm/mmu.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index e60f370..78fd18b 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -936,6 +936,17 @@ void __init sanity_check_meminfo(void)
 		if (bank->start > ULONG_MAX)
 			highmem = 1;
 
+		if (bank->start < __pa(PAGE_OFFSET) &&
+			__pa(PAGE_OFFSET) <= (bank->start + bank->size - 1)) {
+				int offset = __pa(PAGE_OFFSET) - bank->start;
+				bank->start += offset;
+				bank->size -= offset;
+				pr_crit("Change memory bank to %.8llx-%.8llx\n",
+					(unsigned long long)bank->start,
+					(unsigned long long)bank->start +
+								bank->size - 1);
+		}
+
 #ifdef CONFIG_HIGHMEM
 		if (__va(bank->start + bank->size - 1) < (void *)PAGE_OFFSET) {
 			pr_notice("Ignoring RAM@%.8llx-%.8llx ",
-- 
1.7.0.4

[PATCH 1/2] arm: mm: Ignore memory banks which are in front of the kernel when HIGHMEM is ON

[Russell King - ARM Linux]

On Wed, Feb 13, 2013 at 09:58:44AM +0100, Michal Simek wrote:
> Arm kernel with HIGHMEM ON will fail when dts memory node is described
> by memory banks and the starting address is not inside the first 16MB
> of the first memory bank. If HIGHMEM is OFF and the configuration is the same
> then the first memory bank is ignored and the kernel boots.
> 
> Here is the example of behavior:
> dts memory reg = <0x0 0x10000000 0x10000000 0x30000000>
> kernel load address = 0x10000000 (respectively 0x1000800)
> 
> Current:
> "Machine: Xilinx Zynq Platform, model: Zynq ZC702 Development Board
> bootconsole [earlycon0] enabled
> Memory policy: ECC disabled, Data cache writeback
> Kernel panic - not syncing: ERROR: Failed to allocate 0x1000 bytes below 0x0."
> 
> After:
> The kernel ignore ram 0x0-0x0fffffff because is lower than the kernel starting
> address and the kernel bootlog contains.
> "Ignoring RAM at 00000000-0fffffff (CONFIG_HIGHMEM)."
> 
> Also using mem=768M on the command line will overwrite dts memory
> map and kernel will boot with HIGHMEM ON too.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>  arch/arm/mm/mmu.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index ce328c7..e60f370 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -937,6 +937,15 @@ void __init sanity_check_meminfo(void)
>  			highmem = 1;
>  
>  #ifdef CONFIG_HIGHMEM
> +		if (__va(bank->start + bank->size - 1) < (void *)PAGE_OFFSET) {
> +			pr_notice("Ignoring RAM at %.8llx-%.8llx ",
> +				(unsigned long long)bank->start,
> +				(unsigned long long)bank->start
> +							+ bank->size - 1);
> +			pr_cont("(CONFIG_HIGHMEM).\n");
> +			continue;
> +		}

NAK.  __va() is only valid for memory within the kernel's direct mapping
(iow, lowmem).  Physical addresses outside of that range are free to
return any other value.

So, it's entirely possible that a valid highmem setup will have memory
in highmem where __va(start + size - 1) _is_ below PAGE_OFFSET, and this
memory should _not_ be discarded.

[PATCH 1/2] arm: mm: Ignore memory banks which are in front of the kernel when HIGHMEM is ON

[Michal Simek]

2013/2/13 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Wed, Feb 13, 2013 at 09:58:44AM +0100, Michal Simek wrote:
>> Arm kernel with HIGHMEM ON will fail when dts memory node is described
>> by memory banks and the starting address is not inside the first 16MB
>> of the first memory bank. If HIGHMEM is OFF and the configuration is the same
>> then the first memory bank is ignored and the kernel boots.
>>
>> Here is the example of behavior:
>> dts memory reg = <0x0 0x10000000 0x10000000 0x30000000>
>> kernel load address = 0x10000000 (respectively 0x1000800)
>>
>> Current:
>> "Machine: Xilinx Zynq Platform, model: Zynq ZC702 Development Board
>> bootconsole [earlycon0] enabled
>> Memory policy: ECC disabled, Data cache writeback
>> Kernel panic - not syncing: ERROR: Failed to allocate 0x1000 bytes below 0x0."
>>
>> After:
>> The kernel ignore ram 0x0-0x0fffffff because is lower than the kernel starting
>> address and the kernel bootlog contains.
>> "Ignoring RAM at 00000000-0fffffff (CONFIG_HIGHMEM)."
>>
>> Also using mem=768M on the command line will overwrite dts memory
>> map and kernel will boot with HIGHMEM ON too.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>  arch/arm/mm/mmu.c |    9 +++++++++
>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
>> index ce328c7..e60f370 100644
>> --- a/arch/arm/mm/mmu.c
>> +++ b/arch/arm/mm/mmu.c
>> @@ -937,6 +937,15 @@ void __init sanity_check_meminfo(void)
>>                       highmem = 1;
>>
>>  #ifdef CONFIG_HIGHMEM
>> +             if (__va(bank->start + bank->size - 1) < (void *)PAGE_OFFSET) {
>> +                     pr_notice("Ignoring RAM at %.8llx-%.8llx ",
>> +                             (unsigned long long)bank->start,
>> +                             (unsigned long long)bank->start
>> +                                                     + bank->size - 1);
>> +                     pr_cont("(CONFIG_HIGHMEM).\n");
>> +                     continue;
>> +             }
>
> NAK.  __va() is only valid for memory within the kernel's direct mapping
> (iow, lowmem).  Physical addresses outside of that range are free to
> return any other value.
>
> So, it's entirely possible that a valid highmem setup will have memory
> in highmem where __va(start + size - 1) _is_ below PAGE_OFFSET, and this
> memory should _not_ be discarded.

ok. Can you describe me this configuration? Enough to tell me dts memory
fragment and kernel load addr which match this case.
My configuration is above and it doesn't work that's why I believe you
have tested this case
and it will be easy for me to retest it.
Maybe we have any problem with zynq that's why I would like to be aware of it.

What about
"arm: mm: Add support for booting the kernel out of the first 16MB of memory"?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

[PATCH 1/2] arm: mm: Ignore memory banks which are in front of the kernel when HIGHMEM is ON

[Russell King - ARM Linux]

On Wed, Feb 13, 2013 at 03:39:21PM +0100, Michal Simek wrote:
> ok. Can you describe me this configuration? Enough to tell me dts memory
> fragment and kernel load addr which match this case.

Anything which uses more than 1GB of memory and has PAGE_OFFSET set at
0xc0000000 (3GB).  Simple maths will tell you that, and why it fails.

Look:
- if __va(bank->start) = PAGE_OFFSET, and PAGE_OFFSET is at 3GB.
- if bank->size = 1GB (which we _do_ have), then
     __va(bank->start + bank->size) = 4GB.  4GB represented as a 32-bit
  pointer is NULL.  NULL < (void *)PAGE_OFFSET.

Therefore, your patch will cause systems with 1GB or more of memory in
one bank to ignore _all_ the memory passed in.

And if you look@the code:

                if (__va(bank->start) >= vmalloc_min ||
                    __va(bank->start) < (void *)PAGE_OFFSET)
                        highmem = 1;

notice the facy that we're marking all memory starting with an apparant
virtual address outside of PAGE_OFFSET...vmalloc_min as highmem.  This is
to catch cases exactly like this.

Memory for which __va(bank->start) < (void *)PAGE_OFFSET will also have
__va(bank->start + bank->size - 1) (in your patch) also below PAGE_OFFSET,
and your modification will cause the kernel to ignore this memory -
which is not acceptable.

I don't think there's much option for solutions to this; not with a common
kernel designed to run on multiple platforms.  If a platform doesn't
conform to the Linux requirements for a common kernel, then it doesn't
conform and it can't use it.

In much the same way that we ended up saying "no" to people who wanted
to place two physical banks of memory in reverse order in the virtual
mapping, I think this is another case of "no, we can't permit this with
common cross-subarch kernels".

[PATCH 1/2] arm: mm: Ignore memory banks which are in front of the kernel when HIGHMEM is ON

[Michal Simek]

2013/2/13 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Wed, Feb 13, 2013 at 03:39:21PM +0100, Michal Simek wrote:
>> ok. Can you describe me this configuration? Enough to tell me dts memory
>> fragment and kernel load addr which match this case.
>
> Anything which uses more than 1GB of memory and has PAGE_OFFSET set at
> 0xc0000000 (3GB).  Simple maths will tell you that, and why it fails.
>
> Look:
> - if __va(bank->start) = PAGE_OFFSET, and PAGE_OFFSET is at 3GB.
> - if bank->size = 1GB (which we _do_ have), then
>      __va(bank->start + bank->size) = 4GB.  4GB represented as a 32-bit
>   pointer is NULL.  NULL < (void *)PAGE_OFFSET.
>
> Therefore, your patch will cause systems with 1GB or more of memory in
> one bank to ignore _all_ the memory passed in.
>
> And if you look at the code:
>
>                 if (__va(bank->start) >= vmalloc_min ||
>                     __va(bank->start) < (void *)PAGE_OFFSET)
>                         highmem = 1;
>
> notice the facy that we're marking all memory starting with an apparant
> virtual address outside of PAGE_OFFSET...vmalloc_min as highmem.  This is
> to catch cases exactly like this.
>
> Memory for which __va(bank->start) < (void *)PAGE_OFFSET will also have
> __va(bank->start + bank->size - 1) (in your patch) also below PAGE_OFFSET,
> and your modification will cause the kernel to ignore this memory -
> which is not acceptable.

ok - that's fair.


> I don't think there's much option for solutions to this; not with a common
> kernel designed to run on multiple platforms.  If a platform doesn't
> conform to the Linux requirements for a common kernel, then it doesn't
> conform and it can't use it.

I have no problem to admit that this patch is wrong in this implementation
but I tend to disagree with this part.
The Linux kernel has some requirements, limitations, etc,  that's all truth.

Placing the kernel to the main memory to any location you want which end
up with kernel panic is from my point of view fault.
Saying that the kernel is always loaded withing the 16MB of memory is limitation
but it doesn't mean that we shouldn't remove it.

Ignoring the memory before the kernel can be one solution which I
would like to discuss.
What about the second patch used for !HIGHMEM case?

> In much the same way that we ended up saying "no" to people who wanted
> to place two physical banks of memory in reverse order in the virtual
> mapping, I think this is another case of "no, we can't permit this with
> common cross-subarch kernels".

This case should probably end up on device-tree rule (+dtc fault).
Not sure if memory banks should be ordered or not. Rob and Grant any thought?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

[PATCH 1/2] arm: mm: Ignore memory banks which are in front of the kernel when HIGHMEM is ON

[Russell King - ARM Linux]

On Wed, Feb 13, 2013 at 04:52:50PM +0100, Michal Simek wrote:
> This case should probably end up on device-tree rule (+dtc fault).
> Not sure if memory banks should be ordered or not. Rob and Grant any thought?

They _are_ ordered by memblock, because memblock always arranges the
banks in ascending physical address order - and it will merge
contiguous banks together irrespective of what order they appear as.

So you can't do something like bank 0: 0x60000000-0x70000000, bank 1:
0x50000000-0x60000000.  You'll end up with a single memblock entry for
0x50000000-0x70000000.

[PATCH 1/2] arm: mm: Ignore memory banks which are in front of the kernel when HIGHMEM is ON

[Nicolas Pitre]

On Wed, 13 Feb 2013, Michal Simek wrote:

> The Linux kernel has some requirements, limitations, etc,  that's all truth.
> 
> Placing the kernel to the main memory to any location you want which end
> up with kernel panic is from my point of view fault.

I may agree with that, up to a point.

> Saying that the kernel is always loaded withing the 16MB of memory is limitation
> but it doesn't mean that we shouldn't remove it.

The actual limitation should be 128MB for zImage.  It will relocate the 
final kernel binary within the first 16MB by itself.

> Ignoring the memory before the kernel can be one solution which I
> would like to discuss.
> What about the second patch used for !HIGHMEM case?

Highmem is irrelevant.

If you want to ignore memory that way, you must look at the _physical_ 
addresses for each bank, and simply truncate anything that is below 
PHYS_OFFSET.  The comparison should be trivial with physical addresses 
as they're not limited to 32 bits.


Nicolas