diff mbox series

[v2,1/6] resource: Extend the PPC32 reserved memory hack

Message ID 20180122050411.32460-2-j.neuschaefer@gmx.net
State New
Headers show
Series Nintendo Wii GPIO driver | expand

Commit Message

J. Neuschäfer Jan. 22, 2018, 5:04 a.m. UTC
On the Nintendo Wii, there are two ranges of physical memory, and MMIO
in between, but Linux on ppc32 doesn't support discontiguous memory.
Therefore a hack was introduced in commit c5df7f775148 ("powerpc: allow
ioremap within reserved memory regions") and commit de32400dd26e ("wii:
use both mem1 and mem2 as ram"):

 - Treat the area from the start of the first memory area (MEM1) to the
   end of the second (MEM2) as one big memory area, but mark the part
   that doesn't belong to MEM1 or MEM2 as reserved.
 - Only on the Wii, allow ioremap to be used on reserved memory.

This hack, however, doesn't account for the "resource"-based API in
kernel/resource.c, because __request_region performs its own checks.

Extend the hack to kernel/resource.c, to allow more drivers to allocate
their MMIO regions on the Wii.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Cc: Albert Herranz <albert_herranz@yahoo.es>
---

v2:
- CC Albert Herranz, who introduced this hack in 2009.
---
 kernel/resource.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

Michael Ellerman Jan. 23, 2018, 12:58 p.m. UTC | #1
Jonathan Neuschäfer <j.neuschaefer@gmx.net> writes:

> On the Nintendo Wii, there are two ranges of physical memory, and MMIO
> in between, but Linux on ppc32 doesn't support discontiguous memory.
> Therefore a hack was introduced in commit c5df7f775148 ("powerpc: allow
> ioremap within reserved memory regions") and commit de32400dd26e ("wii:
> use both mem1 and mem2 as ram"):
>
>  - Treat the area from the start of the first memory area (MEM1) to the
>    end of the second (MEM2) as one big memory area, but mark the part
>    that doesn't belong to MEM1 or MEM2 as reserved.
>  - Only on the Wii, allow ioremap to be used on reserved memory.
>
> This hack, however, doesn't account for the "resource"-based API in
> kernel/resource.c, because __request_region performs its own checks.
>
> Extend the hack to kernel/resource.c, to allow more drivers to allocate
> their MMIO regions on the Wii.

Hi Jonathan,

Sorry but I can't merge a hack like this in generic code.

Has anyone looked at adding proper discontig mem support to PPC32?

Or can we punch a hole in the resource in the right place? Maybe from
add_system_ram_resources() ?

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Neuschäfer Jan. 23, 2018, 4:37 p.m. UTC | #2
On Tue, Jan 23, 2018 at 11:58:06PM +1100, Michael Ellerman wrote:
> Jonathan Neuschäfer <j.neuschaefer@gmx.net> writes:
> 
> > On the Nintendo Wii, there are two ranges of physical memory, and MMIO
> > in between, but Linux on ppc32 doesn't support discontiguous memory.
> > Therefore a hack was introduced in commit c5df7f775148 ("powerpc: allow
> > ioremap within reserved memory regions") and commit de32400dd26e ("wii:
> > use both mem1 and mem2 as ram"):
> >
> >  - Treat the area from the start of the first memory area (MEM1) to the
> >    end of the second (MEM2) as one big memory area, but mark the part
> >    that doesn't belong to MEM1 or MEM2 as reserved.
> >  - Only on the Wii, allow ioremap to be used on reserved memory.
> >
> > This hack, however, doesn't account for the "resource"-based API in
> > kernel/resource.c, because __request_region performs its own checks.
> >
> > Extend the hack to kernel/resource.c, to allow more drivers to allocate
> > their MMIO regions on the Wii.
> 
> Hi Jonathan,
> 
> Sorry but I can't merge a hack like this in generic code.

Makes sense.

> Has anyone looked at adding proper discontig mem support to PPC32?

I'm not aware of any such effort.

Do you have any pointer on how to implement discontiguous memory
support? CONFIG_ARCH_SPARSEMEM_ENABLE seems relevant.

> Or can we punch a hole in the resource in the right place? Maybe from
> add_system_ram_resources() ?

Not sure. add_system_ram_resources would need the original memblock
table, which is overwritten in wii_memory_fixups, if I read the code
correctly.

If a proper solution doesn't take an overwhelming amount of work, I'd
prefer a proper solution.


Thanks,
Jonathan Neuschäfer
Michael Ellerman Jan. 24, 2018, 1:23 a.m. UTC | #3
Jonathan Neuschäfer <j.neuschaefer@gmx.net> writes:

> On Tue, Jan 23, 2018 at 11:58:06PM +1100, Michael Ellerman wrote:
>> Jonathan Neuschäfer <j.neuschaefer@gmx.net> writes:
>> 
>> > On the Nintendo Wii, there are two ranges of physical memory, and MMIO
>> > in between, but Linux on ppc32 doesn't support discontiguous memory.
>> > Therefore a hack was introduced in commit c5df7f775148 ("powerpc: allow
>> > ioremap within reserved memory regions") and commit de32400dd26e ("wii:
>> > use both mem1 and mem2 as ram"):
>> >
>> >  - Treat the area from the start of the first memory area (MEM1) to the
>> >    end of the second (MEM2) as one big memory area, but mark the part
>> >    that doesn't belong to MEM1 or MEM2 as reserved.
>> >  - Only on the Wii, allow ioremap to be used on reserved memory.
>> >
>> > This hack, however, doesn't account for the "resource"-based API in
>> > kernel/resource.c, because __request_region performs its own checks.
>> >
>> > Extend the hack to kernel/resource.c, to allow more drivers to allocate
>> > their MMIO regions on the Wii.
>> 
>> Hi Jonathan,
>> 
>> Sorry but I can't merge a hack like this in generic code.
>
> Makes sense.
>
>> Has anyone looked at adding proper discontig mem support to PPC32?
>
> I'm not aware of any such effort.
>
> Do you have any pointer on how to implement discontiguous memory
> support? CONFIG_ARCH_SPARSEMEM_ENABLE seems relevant.

I'm not really sure what the key impediment to it working is.

You don't need to go all the way to SPARSEMEM, there is DISCONTIGMEM
which IIUI is quite a bit simpler.

I'd actually be interested to know what happens (ie. breaks) if you just
add the two memblocks and leave the hole in between. Is it the generic
code that breaks or is it something in the powerpc code? If it's the
later maybe we can do a small fix/hack to work around that.

>> Or can we punch a hole in the resource in the right place? Maybe from
>> add_system_ram_resources() ?
>
> Not sure. add_system_ram_resources would need the original memblock
> table, which is overwritten in wii_memory_fixups, if I read the code
> correctly.

Or it just needs to know where the "wii hole" is, and it can skip that
region, that should be doable, but whether it actually works I'm not
100% sure.

> If a proper solution doesn't take an overwhelming amount of work, I'd
> prefer a proper solution.

Thanks.

cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Neuschäfer Jan. 27, 2018, 8 a.m. UTC | #4
On Wed, Jan 24, 2018 at 12:23:05PM +1100, Michael Ellerman wrote:
> Jonathan Neuschäfer <j.neuschaefer@gmx.net> writes:
[...]
> > Do you have any pointer on how to implement discontiguous memory
> > support? CONFIG_ARCH_SPARSEMEM_ENABLE seems relevant.
> 
> I'm not really sure what the key impediment to it working is.
> 
> You don't need to go all the way to SPARSEMEM, there is DISCONTIGMEM
> which IIUI is quite a bit simpler.
> 
> I'd actually be interested to know what happens (ie. breaks) if you just
> add the two memblocks and leave the hole in between. Is it the generic
> code that breaks or is it something in the powerpc code? If it's the
> later maybe we can do a small fix/hack to work around that.

Ok, I did some experimentation.

First, I made wii_memory_fixups return early, before actually doing
anything[1].

[    0.000000] __ioremap(): phys addr 0xc003000 is RAM lr flipper_pic_init
[    0.000000] flipper-pic: controller at 0x0c003000 mapped to 0x  (null)
[    0.000000] Unable to handle kernel paging request for data at address 0x00000004

* __ioremap_caller detects overlap with RAM like this: p < virt_to_phys(high_memory)
* flipper_pic_init gets NULL from ioremap, but doesn't check for NULL


Then I hacked up __ioremap_caller to use memblock_is_map_memory[2],
because it considers memblocks correctly. The result was that the system
boots further, but then enters the sleep mode where the power LED shines
yellow. In this mode the ARM runs but the PPC doesn't. The same thing
would happen if GPIO 3 ("DC_DC"[3]) was pulled low. These are the last few
lines:

[    0.770324] io scheduler mq-deadline registered
[    0.772472] io scheduler kyber registered

I don't know what exactly is triggering this effect.


Thanks for your help,
Jonathan Neuschäfer


[1]: diff --git a/arch/powerpc/platforms/embedded6xx/wii.c b/arch/powerpc/platforms/embedded6xx/wii.c
index 6e6db1e16d71..cddd5606a63d 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -81,6 +81,9 @@ void __init wii_memory_fixups(void)
 	BUG_ON(memblock.memory.cnt != 2);
 	BUG_ON(!page_aligned(p[0].base) || !page_aligned(p[1].base));
 
+	/* don't fix the memory map */
+	return;
+
 	/* trim unaligned tail */
 	memblock_remove(ALIGN(p[1].base + p[1].size, PAGE_SIZE),
 			(phys_addr_t)ULLONG_MAX);
[2]: diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index f6c7f54c0515..bff581003c50 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -154,8 +154,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, unsigned long flags,
 	 * Don't allow anybody to remap normal RAM that we're using.
 	 * mem_init() sets high_memory so only do the check after that.
 	 */
-	if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
-	    !(__allow_ioremap_reserved && memblock_is_region_reserved(p, size))) {
+	if (slab_is_available() && memblock_is_map_memory(p)) {
 		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
 		       (unsigned long long)p, __builtin_return_address(0));
 		return NULL;
[3]: http://wiibrew.org/wiki/Hardware/Hollywood_GPIOs
diff mbox series

Patch

diff --git a/kernel/resource.c b/kernel/resource.c
index 54ba6de3757c..bb3d329329da 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1134,6 +1134,24 @@  resource_size_t resource_alignment(struct resource *res)
 
 static DECLARE_WAIT_QUEUE_HEAD(muxed_resource_wait);
 
+/*
+ * On some ppc32 platforms (Nintendo Wii), reserved memory is used to work
+ * around the fact that Linux doesn't support discontiguous memory (all memory
+ * is treated as one large area with holes punched in it), and reserved memory
+ * is allowed to be allocated.
+ */
+#ifdef CONFIG_PPC32
+static bool conflict_ignored(struct resource *conflict)
+{
+	extern int __allow_ioremap_reserved;
+
+	return __allow_ioremap_reserved &&
+		(conflict->flags & IORESOURCE_SYSRAM);
+}
+#else
+static bool conflict_ignored(struct resource *conflict) { return false; }
+#endif
+
 /**
  * __request_region - create a new busy resource region
  * @parent: parent resource descriptor
@@ -1166,8 +1184,9 @@  struct resource * __request_region(struct resource *parent,
 		res->desc = parent->desc;
 
 		conflict = __request_resource(parent, res);
-		if (!conflict)
+		if (!conflict || conflict_ignored(conflict))
 			break;
+
 		if (conflict != parent) {
 			if (!(conflict->flags & IORESOURCE_BUSY)) {
 				parent = conflict;