[PATCH 0/2] MIPS: Memory setup tweaks

[PATCH 0/2] MIPS: Memory setup tweaks

[James Hogan]

Here are a couple of tweaks for MIPS memory setup, primarily in order to
handle memory which extends right up to the end of physical memory on
32-bit systems with 32-bit phys_addr_t. More specifically we omit the
final page of physical memory to avoid the overflow (see patch 1 for
details).

Patch 2 improves the rounding in the MAAR setup, so as to include the
first full page of an already aligned region, and to avoid a BUG_ON for
regions with non 64-KByte aligned end addresses (which I happened to hit
while working on a different version of patch 1 which wasn't correctly
merging the kernel data section into the main RAM region).

Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org

James Hogan (2):
  MIPS: Allow memory reaching top of physical
  MIPS: MAAR: Fix address alignment

 arch/mips/kernel/setup.c |  7 +++++++
 arch/mips/mm/init.c      | 13 ++++++-------
 2 files changed, 13 insertions(+), 7 deletions(-)

-- 
git-series 0.8.7

[PATCH 0/2] MIPS: Memory setup tweaks

[James Hogan]

Here are a couple of tweaks for MIPS memory setup, primarily in order to
handle memory which extends right up to the end of physical memory on
32-bit systems with 32-bit phys_addr_t. More specifically we omit the
final page of physical memory to avoid the overflow (see patch 1 for
details).

Patch 2 improves the rounding in the MAAR setup, so as to include the
first full page of an already aligned region, and to avoid a BUG_ON for
regions with non 64-KByte aligned end addresses (which I happened to hit
while working on a different version of patch 1 which wasn't correctly
merging the kernel data section into the main RAM region).

Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org

James Hogan (2):
  MIPS: Allow memory reaching top of physical
  MIPS: MAAR: Fix address alignment

 arch/mips/kernel/setup.c |  7 +++++++
 arch/mips/mm/init.c      | 13 ++++++-------
 2 files changed, 13 insertions(+), 7 deletions(-)

-- 
git-series 0.8.7

[PATCH 1/2] MIPS: Allow memory reaching top of physical

[James Hogan]

Memory regions added with add_memory_region() at the top of the physical
address space will have their end address overflow to 0. This causes
them to be rejected as invalid, and would cause various other issues
later on.

This causes issues on Malta and Boston platforms when wanting to use all
2GB of RAM on a 32-bit kernel, either via highmem (using physical
addresses 0x90000000..0xFFFFFFFF), or with the Malta Enhanced Virtual
Addressing (EVA) layout which exposes the whole 0x80000000..0xFFFFFFFF
physical address range to kernel mode at 0x00000000..0x7FFFFFFF.

Due to the abundance of these non-overflow assumptions and the fact that
memblock already avoids the arithmetic overflow by limiting the size of
new memory regions without the arch code knowing it (in particular
mem_init_free_highmem() will trigger a page dump due to nonzero mapcount
on the last page), it is simpler and safer to just limit the size of the
region in a similar way to memblock but at the arch level to allow most
of the RAM to be used without arithmetic overflows.

Therefore we detect this case specifically and reduce the size of the
region slightly to avoid the arithmetic overflows and cause the last
page to be ignored.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/kernel/setup.c | 7 +++++++
 1 file changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 36cf8d65c47d..3be0e6ba2797 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -87,6 +87,13 @@ void __init add_memory_region(phys_addr_t start, phys_addr_t size, long type)
 	int x = boot_mem_map.nr_map;
 	int i;
 
+	/*
+	 * If the region reaches the top of the physical address space, adjust
+	 * the size slightly so that (start + size) doesn't overflow
+	 */
+	if (start + size - 1 == (phys_addr_t)ULLONG_MAX)
+		--size;
+
 	/* Sanity check */
 	if (start + size < start) {
 		pr_warn("Trying to add an invalid memory region, skipped\n");
-- 
git-series 0.8.7

[PATCH 1/2] MIPS: Allow memory reaching top of physical

[James Hogan]

Memory regions added with add_memory_region() at the top of the physical
address space will have their end address overflow to 0. This causes
them to be rejected as invalid, and would cause various other issues
later on.

This causes issues on Malta and Boston platforms when wanting to use all
2GB of RAM on a 32-bit kernel, either via highmem (using physical
addresses 0x90000000..0xFFFFFFFF), or with the Malta Enhanced Virtual
Addressing (EVA) layout which exposes the whole 0x80000000..0xFFFFFFFF
physical address range to kernel mode at 0x00000000..0x7FFFFFFF.

Due to the abundance of these non-overflow assumptions and the fact that
memblock already avoids the arithmetic overflow by limiting the size of
new memory regions without the arch code knowing it (in particular
mem_init_free_highmem() will trigger a page dump due to nonzero mapcount
on the last page), it is simpler and safer to just limit the size of the
region in a similar way to memblock but at the arch level to allow most
of the RAM to be used without arithmetic overflows.

Therefore we detect this case specifically and reduce the size of the
region slightly to avoid the arithmetic overflows and cause the last
page to be ignored.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/kernel/setup.c | 7 +++++++
 1 file changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 36cf8d65c47d..3be0e6ba2797 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -87,6 +87,13 @@ void __init add_memory_region(phys_addr_t start, phys_addr_t size, long type)
 	int x = boot_mem_map.nr_map;
 	int i;
 
+	/*
+	 * If the region reaches the top of the physical address space, adjust
+	 * the size slightly so that (start + size) doesn't overflow
+	 */
+	if (start + size - 1 == (phys_addr_t)ULLONG_MAX)
+		--size;
+
 	/* Sanity check */
 	if (start + size < start) {
 		pr_warn("Trying to add an invalid memory region, skipped\n");
-- 
git-series 0.8.7

[PATCH 2/2] MIPS: MAAR: Fix address alignment

[James Hogan]

The alignment of MIPS MAAR region addresses isn't quite right.

- It rounds an already 64 KiB aligned start address up to the next
  64 KiB boundary, e.g. 0x80000000 is rounded up to 0x80010000.

- It assumes the end address is already on a 64 KiB boundary and doesn't
  round it down. Should that not be the case it will hit the second
  BUG_ON() in write_maar_pair().

Both cases are addressed by rounding up and down to 64 KiB boundaries in
the more traditional way of adding 0xffff (for rounding up) and masking
off the low 16 bits.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org
---
 arch/mips/mm/init.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index a5509e7dcad2..2c3749d98f04 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -261,7 +261,6 @@ unsigned __weak platform_maar_init(unsigned num_pairs)
 {
 	struct maar_config cfg[BOOT_MEM_MAP_MAX];
 	unsigned i, num_configured, num_cfg = 0;
-	phys_addr_t skip;
 
 	for (i = 0; i < boot_mem_map.nr_map; i++) {
 		switch (boot_mem_map.map[i].type) {
@@ -272,14 +271,14 @@ unsigned __weak platform_maar_init(unsigned num_pairs)
 			continue;
 		}
 
-		skip = 0x10000 - (boot_mem_map.map[i].addr & 0xffff);
-
+		/* Round lower up */
 		cfg[num_cfg].lower = boot_mem_map.map[i].addr;
-		cfg[num_cfg].lower += skip;
+		cfg[num_cfg].lower = (cfg[num_cfg].lower + 0xffff) & ~0xffff;
 
-		cfg[num_cfg].upper = cfg[num_cfg].lower;
-		cfg[num_cfg].upper += boot_mem_map.map[i].size - 1;
-		cfg[num_cfg].upper -= skip;
+		/* Round upper down */
+		cfg[num_cfg].upper = boot_mem_map.map[i].addr +
+					boot_mem_map.map[i].size;
+		cfg[num_cfg].upper = (cfg[num_cfg].upper & ~0xffff) - 1;
 
 		cfg[num_cfg].attrs = MIPS_MAAR_S;
 		num_cfg++;
-- 
git-series 0.8.7

[PATCH 2/2] MIPS: MAAR: Fix address alignment

[James Hogan]

The alignment of MIPS MAAR region addresses isn't quite right.

- It rounds an already 64 KiB aligned start address up to the next
  64 KiB boundary, e.g. 0x80000000 is rounded up to 0x80010000.

- It assumes the end address is already on a 64 KiB boundary and doesn't
  round it down. Should that not be the case it will hit the second
  BUG_ON() in write_maar_pair().

Both cases are addressed by rounding up and down to 64 KiB boundaries in
the more traditional way of adding 0xffff (for rounding up) and masking
off the low 16 bits.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mips@linux-mips.org
---
 arch/mips/mm/init.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index a5509e7dcad2..2c3749d98f04 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -261,7 +261,6 @@ unsigned __weak platform_maar_init(unsigned num_pairs)
 {
 	struct maar_config cfg[BOOT_MEM_MAP_MAX];
 	unsigned i, num_configured, num_cfg = 0;
-	phys_addr_t skip;
 
 	for (i = 0; i < boot_mem_map.nr_map; i++) {
 		switch (boot_mem_map.map[i].type) {
@@ -272,14 +271,14 @@ unsigned __weak platform_maar_init(unsigned num_pairs)
 			continue;
 		}
 
-		skip = 0x10000 - (boot_mem_map.map[i].addr & 0xffff);
-
+		/* Round lower up */
 		cfg[num_cfg].lower = boot_mem_map.map[i].addr;
-		cfg[num_cfg].lower += skip;
+		cfg[num_cfg].lower = (cfg[num_cfg].lower + 0xffff) & ~0xffff;
 
-		cfg[num_cfg].upper = cfg[num_cfg].lower;
-		cfg[num_cfg].upper += boot_mem_map.map[i].size - 1;
-		cfg[num_cfg].upper -= skip;
+		/* Round upper down */
+		cfg[num_cfg].upper = boot_mem_map.map[i].addr +
+					boot_mem_map.map[i].size;
+		cfg[num_cfg].upper = (cfg[num_cfg].upper & ~0xffff) - 1;
 
 		cfg[num_cfg].attrs = MIPS_MAAR_S;
 		num_cfg++;
-- 
git-series 0.8.7

Re: [PATCH 0/2] MIPS: Memory setup tweaks

[Ralf Baechle]

On Tue, Aug 09, 2016 at 01:21:47PM +0100, James Hogan wrote:

> Here are a couple of tweaks for MIPS memory setup, primarily in order to
> handle memory which extends right up to the end of physical memory on
> 32-bit systems with 32-bit phys_addr_t. More specifically we omit the
> final page of physical memory to avoid the overflow (see patch 1 for
> details).
> 
> Patch 2 improves the rounding in the MAAR setup, so as to include the
> first full page of an already aligned region, and to avoid a BUG_ON for
> regions with non 64-KByte aligned end addresses (which I happened to hit
> while working on a different version of patch 1 which wasn't correctly
> merging the kernel data section into the main RAM region).

There's a DMA issue with one of the system controllers on Malta which
afair only affects one endianess and can be worked around by not using
the last bit of memory. That isn't the only platform having such issues
I've seen and debugging has always been very painful so I'm wondering
if as a general precaution we should just leave the last page of memory
unused.

  Ralf

Re: [PATCH 0/2] MIPS: Memory setup tweaks

[James Hogan]

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

On Tue, Aug 16, 2016 at 05:36:07PM +0200, Ralf Baechle wrote:
> On Tue, Aug 09, 2016 at 01:21:47PM +0100, James Hogan wrote:
> 
> > Here are a couple of tweaks for MIPS memory setup, primarily in order to
> > handle memory which extends right up to the end of physical memory on
> > 32-bit systems with 32-bit phys_addr_t. More specifically we omit the
> > final page of physical memory to avoid the overflow (see patch 1 for
> > details).
> > 
> > Patch 2 improves the rounding in the MAAR setup, so as to include the
> > first full page of an already aligned region, and to avoid a BUG_ON for
> > regions with non 64-KByte aligned end addresses (which I happened to hit
> > while working on a different version of patch 1 which wasn't correctly
> > merging the kernel data section into the main RAM region).
> 
> There's a DMA issue with one of the system controllers on Malta which
> afair only affects one endianess and can be worked around by not using
> the last bit of memory. That isn't the only platform having such issues
> I've seen and debugging has always been very painful so I'm wondering
> if as a general precaution we should just leave the last page of memory
> unused.

If I understand you right, I think thats pretty much what patch 1 does,
or rather it allows such a region to be created but reduces the length
so it doesn't quite reach the end, I suppose assuming it will get
rounded down to a page boundary.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/2] MIPS: Memory setup tweaks

[James Hogan]

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

On Tue, Aug 16, 2016 at 05:36:07PM +0200, Ralf Baechle wrote:
> On Tue, Aug 09, 2016 at 01:21:47PM +0100, James Hogan wrote:
> 
> > Here are a couple of tweaks for MIPS memory setup, primarily in order to
> > handle memory which extends right up to the end of physical memory on
> > 32-bit systems with 32-bit phys_addr_t. More specifically we omit the
> > final page of physical memory to avoid the overflow (see patch 1 for
> > details).
> > 
> > Patch 2 improves the rounding in the MAAR setup, so as to include the
> > first full page of an already aligned region, and to avoid a BUG_ON for
> > regions with non 64-KByte aligned end addresses (which I happened to hit
> > while working on a different version of patch 1 which wasn't correctly
> > merging the kernel data section into the main RAM region).
> 
> There's a DMA issue with one of the system controllers on Malta which
> afair only affects one endianess and can be worked around by not using
> the last bit of memory. That isn't the only platform having such issues
> I've seen and debugging has always been very painful so I'm wondering
> if as a general precaution we should just leave the last page of memory
> unused.

If I understand you right, I think thats pretty much what patch 1 does,
or rather it allows such a region to be created but reduces the length
so it doesn't quite reach the end, I suppose assuming it will get
rounded down to a page boundary.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]