linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL
@ 2018-12-11 22:40 Christophe Leroy
  2018-12-12  0:23 ` Michael Ellerman
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Christophe Leroy @ 2018-12-11 22:40 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev

This patch implements CONFIG_DEBUG_VIRTUAL to warn about
incorrect use of virt_to_phys() and page_to_phys()

Below is the result of test_debug_virtual:

[    1.438746] WARNING: CPU: 0 PID: 1 at ./arch/powerpc/include/asm/io.h:808 test_debug_virtual_init+0x3c/0xd4
[    1.448156] CPU: 0 PID: 1 Comm: swapper Not tainted 4.20.0-rc5-00560-g6bfb52e23a00-dirty #532
[    1.457259] NIP:  c066c550 LR: c0650ccc CTR: c066c514
[    1.462257] REGS: c900bdb0 TRAP: 0700   Not tainted  (4.20.0-rc5-00560-g6bfb52e23a00-dirty)
[    1.471184] MSR:  00029032 <EE,ME,IR,DR,RI>  CR: 48000422  XER: 20000000
[    1.477811]
[    1.477811] GPR00: c0650ccc c900be60 c60d0000 00000000 006000c0 c9000000 00009032 c7fa0020
[    1.477811] GPR08: 00002400 00000001 09000000 00000000 c07b5d04 00000000 c00037d8 00000000
[    1.477811] GPR16: 00000000 00000000 00000000 00000000 c0760000 c0740000 00000092 c0685bb0
[    1.477811] GPR24: c065042c c068a734 c0685b8c 00000006 00000000 c0760000 c075c3c0 ffffffff
[    1.512711] NIP [c066c550] test_debug_virtual_init+0x3c/0xd4
[    1.518315] LR [c0650ccc] do_one_initcall+0x8c/0x1cc
[    1.523163] Call Trace:
[    1.525595] [c900be60] [c0567340] 0xc0567340 (unreliable)
[    1.530954] [c900be90] [c0650ccc] do_one_initcall+0x8c/0x1cc
[    1.536551] [c900bef0] [c0651000] kernel_init_freeable+0x1f4/0x2cc
[    1.542658] [c900bf30] [c00037ec] kernel_init+0x14/0x110
[    1.547913] [c900bf40] [c000e1d0] ret_from_kernel_thread+0x14/0x1c
[    1.553971] Instruction dump:
[    1.556909] 3ca50100 bfa10024 54a5000e 3fa0c076 7c0802a6 3d454000 813dc204 554893be
[    1.564566] 7d294010 7d294910 90010034 39290001 <0f090000> 7c3e0b78 955e0008 3fe0c062
[    1.572425] ---[ end trace 6f6984225b280ad6 ]---
[    1.577467] PA: 0x09000000 for VA: 0xc9000000
[    1.581799] PA: 0x061e8f50 for VA: 0xc61e8f50

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 v3: Added missing linux/mm.h
     I realised that a driver may use DMA on stack after checking with virt_addr_valid(), so the new
     verification might induce false positives. I remove it for now, will add it again later in a more
     controled way.

 v2: Using asm/pgtable.h to avoid build failure on ppc64e.
     Added a verification that the object is not in stack to catch problems before activing VMAP_STACK.

 arch/powerpc/Kconfig          |  1 +
 arch/powerpc/include/asm/io.h | 13 ++++++++++++-
 arch/powerpc/mm/pgtable_32.c  |  2 +-
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index e312e92e3381..94b46624068d 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -128,6 +128,7 @@ config PPC
 	#
 	# Please keep this list sorted alphabetically.
 	#
+	select ARCH_HAS_DEBUG_VIRTUAL
 	select ARCH_HAS_DEVMEM_IS_ALLOWED
 	select ARCH_HAS_DMA_SET_COHERENT_MASK
 	select ARCH_HAS_ELF_RANDOMIZE
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index e746becd9d6f..7f19fbd3ba55 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -29,12 +29,14 @@ extern struct pci_dev *isa_bridge_pcidev;
 
 #include <linux/device.h>
 #include <linux/compiler.h>
+#include <linux/mm.h>
 #include <asm/page.h>
 #include <asm/byteorder.h>
 #include <asm/synch.h>
 #include <asm/delay.h>
 #include <asm/mmu.h>
 #include <asm/ppc_asm.h>
+#include <asm/pgtable.h>
 
 #ifdef CONFIG_PPC64
 #include <asm/paca.h>
@@ -804,6 +806,8 @@ extern void __iounmap_at(void *ea, unsigned long size);
  */
 static inline unsigned long virt_to_phys(volatile void * address)
 {
+	WARN_ON(IS_ENABLED(CONFIG_DEBUG_VIRTUAL) && !virt_addr_valid(address));
+
 	return __pa((unsigned long)address);
 }
 
@@ -827,7 +831,14 @@ static inline void * phys_to_virt(unsigned long address)
 /*
  * Change "struct page" to physical address.
  */
-#define page_to_phys(page)	((phys_addr_t)page_to_pfn(page) << PAGE_SHIFT)
+static inline phys_addr_t page_to_phys(struct page *page)
+{
+	unsigned long pfn = page_to_pfn(page);
+
+	WARN_ON(IS_ENABLED(CONFIG_DEBUG_VIRTUAL) && !pfn_valid(pfn));
+
+	return PFN_PHYS(pfn);
+}
 
 /*
  * 32 bits still uses virt_to_bus() for it's implementation of DMA
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 4fc77a99c9bf..68d204a45cd0 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -143,7 +143,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, pgprot_t prot, void *call
 	 * 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)) &&
+	if (slab_is_available() && virt_addr_valid(p) &&
 	    page_is_ram(__phys_to_pfn(p))) {
 		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
 		       (unsigned long long)p, __builtin_return_address(0));
-- 
2.13.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL
  2018-12-11 22:40 [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL Christophe Leroy
@ 2018-12-12  0:23 ` Michael Ellerman
  2018-12-12  7:19   ` Christophe Leroy
  2018-12-18 14:06 ` Michael Ellerman
  2018-12-22  9:54 ` [v3] " Michael Ellerman
  2 siblings, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2018-12-12  0:23 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev

Christophe Leroy <christophe.leroy@c-s.fr> writes:

> This patch implements CONFIG_DEBUG_VIRTUAL to warn about
> incorrect use of virt_to_phys() and page_to_phys()
>
> Below is the result of test_debug_virtual:
>
> [    1.438746] WARNING: CPU: 0 PID: 1 at ./arch/powerpc/include/asm/io.h:808 test_debug_virtual_init+0x3c/0xd4
> [    1.448156] CPU: 0 PID: 1 Comm: swapper Not tainted 4.20.0-rc5-00560-g6bfb52e23a00-dirty #532
> [    1.457259] NIP:  c066c550 LR: c0650ccc CTR: c066c514
> [    1.462257] REGS: c900bdb0 TRAP: 0700   Not tainted  (4.20.0-rc5-00560-g6bfb52e23a00-dirty)
> [    1.471184] MSR:  00029032 <EE,ME,IR,DR,RI>  CR: 48000422  XER: 20000000
> [    1.477811]
> [    1.477811] GPR00: c0650ccc c900be60 c60d0000 00000000 006000c0 c9000000 00009032 c7fa0020
> [    1.477811] GPR08: 00002400 00000001 09000000 00000000 c07b5d04 00000000 c00037d8 00000000
> [    1.477811] GPR16: 00000000 00000000 00000000 00000000 c0760000 c0740000 00000092 c0685bb0
> [    1.477811] GPR24: c065042c c068a734 c0685b8c 00000006 00000000 c0760000 c075c3c0 ffffffff
> [    1.512711] NIP [c066c550] test_debug_virtual_init+0x3c/0xd4
> [    1.518315] LR [c0650ccc] do_one_initcall+0x8c/0x1cc
> [    1.523163] Call Trace:
> [    1.525595] [c900be60] [c0567340] 0xc0567340 (unreliable)
> [    1.530954] [c900be90] [c0650ccc] do_one_initcall+0x8c/0x1cc
> [    1.536551] [c900bef0] [c0651000] kernel_init_freeable+0x1f4/0x2cc
> [    1.542658] [c900bf30] [c00037ec] kernel_init+0x14/0x110
> [    1.547913] [c900bf40] [c000e1d0] ret_from_kernel_thread+0x14/0x1c
> [    1.553971] Instruction dump:
> [    1.556909] 3ca50100 bfa10024 54a5000e 3fa0c076 7c0802a6 3d454000 813dc204 554893be
> [    1.564566] 7d294010 7d294910 90010034 39290001 <0f090000> 7c3e0b78 955e0008 3fe0c062
> [    1.572425] ---[ end trace 6f6984225b280ad6 ]---
> [    1.577467] PA: 0x09000000 for VA: 0xc9000000
> [    1.581799] PA: 0x061e8f50 for VA: 0xc61e8f50
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  v3: Added missing linux/mm.h
>      I realised that a driver may use DMA on stack after checking with virt_addr_valid(), so the new
>      verification might induce false positives. I remove it for now, will add it again later in a more
>      controled way.

What is this comment referring to?

I can't see any difference to v2 except the linux/mm.h include.

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL
  2018-12-12  0:23 ` Michael Ellerman
@ 2018-12-12  7:19   ` Christophe Leroy
  2018-12-12 10:35     ` Michael Ellerman
  0 siblings, 1 reply; 10+ messages in thread
From: Christophe Leroy @ 2018-12-12  7:19 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev



Le 12/12/2018 à 01:23, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
> 
>> This patch implements CONFIG_DEBUG_VIRTUAL to warn about
>> incorrect use of virt_to_phys() and page_to_phys()
>>
>> Below is the result of test_debug_virtual:
>>
>> [    1.438746] WARNING: CPU: 0 PID: 1 at ./arch/powerpc/include/asm/io.h:808 test_debug_virtual_init+0x3c/0xd4
>> [    1.448156] CPU: 0 PID: 1 Comm: swapper Not tainted 4.20.0-rc5-00560-g6bfb52e23a00-dirty #532
>> [    1.457259] NIP:  c066c550 LR: c0650ccc CTR: c066c514
>> [    1.462257] REGS: c900bdb0 TRAP: 0700   Not tainted  (4.20.0-rc5-00560-g6bfb52e23a00-dirty)
>> [    1.471184] MSR:  00029032 <EE,ME,IR,DR,RI>  CR: 48000422  XER: 20000000
>> [    1.477811]
>> [    1.477811] GPR00: c0650ccc c900be60 c60d0000 00000000 006000c0 c9000000 00009032 c7fa0020
>> [    1.477811] GPR08: 00002400 00000001 09000000 00000000 c07b5d04 00000000 c00037d8 00000000
>> [    1.477811] GPR16: 00000000 00000000 00000000 00000000 c0760000 c0740000 00000092 c0685bb0
>> [    1.477811] GPR24: c065042c c068a734 c0685b8c 00000006 00000000 c0760000 c075c3c0 ffffffff
>> [    1.512711] NIP [c066c550] test_debug_virtual_init+0x3c/0xd4
>> [    1.518315] LR [c0650ccc] do_one_initcall+0x8c/0x1cc
>> [    1.523163] Call Trace:
>> [    1.525595] [c900be60] [c0567340] 0xc0567340 (unreliable)
>> [    1.530954] [c900be90] [c0650ccc] do_one_initcall+0x8c/0x1cc
>> [    1.536551] [c900bef0] [c0651000] kernel_init_freeable+0x1f4/0x2cc
>> [    1.542658] [c900bf30] [c00037ec] kernel_init+0x14/0x110
>> [    1.547913] [c900bf40] [c000e1d0] ret_from_kernel_thread+0x14/0x1c
>> [    1.553971] Instruction dump:
>> [    1.556909] 3ca50100 bfa10024 54a5000e 3fa0c076 7c0802a6 3d454000 813dc204 554893be
>> [    1.564566] 7d294010 7d294910 90010034 39290001 <0f090000> 7c3e0b78 955e0008 3fe0c062
>> [    1.572425] ---[ end trace 6f6984225b280ad6 ]---
>> [    1.577467] PA: 0x09000000 for VA: 0xc9000000
>> [    1.581799] PA: 0x061e8f50 for VA: 0xc61e8f50
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>>   v3: Added missing linux/mm.h
>>       I realised that a driver may use DMA on stack after checking with virt_addr_valid(), so the new
>>       verification might induce false positives. I remove it for now, will add it again later in a more
>>       controled way.
> 
> What is this comment referring to?
> 
> I can't see any difference to v2 except the linux/mm.h include.

v2 was:


@@ -804,6 +806,11 @@ extern void __iounmap_at(void *ea, unsigned long size);
   */
  static inline unsigned long virt_to_phys(volatile void * address)
  {
+	if (IS_ENABLED(CONFIG_DEBUG_VIRTUAL) &&
+	    !WARN_ON(IS_ENABLED(CONFIG_HAVE_ARCH_VMAP_STACK) && current->pid &&
+		     object_is_on_stack((const void*)address)))
+			WARN_ON(!virt_addr_valid(address));
+
  	return __pa((unsigned long)address);
  }


v3 is: (same as v1)


@@ -804,6 +806,8 @@ extern void __iounmap_at(void *ea, unsigned long size);
   */
  static inline unsigned long virt_to_phys(volatile void * address)
  {
+	WARN_ON(IS_ENABLED(CONFIG_DEBUG_VIRTUAL) && !virt_addr_valid(address));
+
  	return __pa((unsigned long)address);
  }


The idea in v2 was to detect objects on stack used for DMA before 
activating CONFIG_VMAP_STACK, but if the driver uses virt_addr_valid() 
to decide if it can DMA map it, then we'll get false positives.
So I think this should be added with a dedicated DEBUG CONFIG option, 
not implicitely.

Christophe

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL
  2018-12-12  7:19   ` Christophe Leroy
@ 2018-12-12 10:35     ` Michael Ellerman
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2018-12-12 10:35 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev

Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 12/12/2018 à 01:23, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>> 
>>> This patch implements CONFIG_DEBUG_VIRTUAL to warn about
>>> incorrect use of virt_to_phys() and page_to_phys()
>>>
>>> Below is the result of test_debug_virtual:
>>>
>>> [    1.438746] WARNING: CPU: 0 PID: 1 at ./arch/powerpc/include/asm/io.h:808 test_debug_virtual_init+0x3c/0xd4
>>> [    1.448156] CPU: 0 PID: 1 Comm: swapper Not tainted 4.20.0-rc5-00560-g6bfb52e23a00-dirty #532
>>> [    1.457259] NIP:  c066c550 LR: c0650ccc CTR: c066c514
>>> [    1.462257] REGS: c900bdb0 TRAP: 0700   Not tainted  (4.20.0-rc5-00560-g6bfb52e23a00-dirty)
>>> [    1.471184] MSR:  00029032 <EE,ME,IR,DR,RI>  CR: 48000422  XER: 20000000
>>> [    1.477811]
>>> [    1.477811] GPR00: c0650ccc c900be60 c60d0000 00000000 006000c0 c9000000 00009032 c7fa0020
>>> [    1.477811] GPR08: 00002400 00000001 09000000 00000000 c07b5d04 00000000 c00037d8 00000000
>>> [    1.477811] GPR16: 00000000 00000000 00000000 00000000 c0760000 c0740000 00000092 c0685bb0
>>> [    1.477811] GPR24: c065042c c068a734 c0685b8c 00000006 00000000 c0760000 c075c3c0 ffffffff
>>> [    1.512711] NIP [c066c550] test_debug_virtual_init+0x3c/0xd4
>>> [    1.518315] LR [c0650ccc] do_one_initcall+0x8c/0x1cc
>>> [    1.523163] Call Trace:
>>> [    1.525595] [c900be60] [c0567340] 0xc0567340 (unreliable)
>>> [    1.530954] [c900be90] [c0650ccc] do_one_initcall+0x8c/0x1cc
>>> [    1.536551] [c900bef0] [c0651000] kernel_init_freeable+0x1f4/0x2cc
>>> [    1.542658] [c900bf30] [c00037ec] kernel_init+0x14/0x110
>>> [    1.547913] [c900bf40] [c000e1d0] ret_from_kernel_thread+0x14/0x1c
>>> [    1.553971] Instruction dump:
>>> [    1.556909] 3ca50100 bfa10024 54a5000e 3fa0c076 7c0802a6 3d454000 813dc204 554893be
>>> [    1.564566] 7d294010 7d294910 90010034 39290001 <0f090000> 7c3e0b78 955e0008 3fe0c062
>>> [    1.572425] ---[ end trace 6f6984225b280ad6 ]---
>>> [    1.577467] PA: 0x09000000 for VA: 0xc9000000
>>> [    1.581799] PA: 0x061e8f50 for VA: 0xc61e8f50
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>> ---
>>>   v3: Added missing linux/mm.h
>>>       I realised that a driver may use DMA on stack after checking with virt_addr_valid(), so the new
>>>       verification might induce false positives. I remove it for now, will add it again later in a more
>>>       controled way.
>> 
>> What is this comment referring to?
>> 
>> I can't see any difference to v2 except the linux/mm.h include.
>
> v2 was:
>
>
> @@ -804,6 +806,11 @@ extern void __iounmap_at(void *ea, unsigned long size);
>    */
>   static inline unsigned long virt_to_phys(volatile void * address)
>   {
> +	if (IS_ENABLED(CONFIG_DEBUG_VIRTUAL) &&
> +	    !WARN_ON(IS_ENABLED(CONFIG_HAVE_ARCH_VMAP_STACK) && current->pid &&
> +		     object_is_on_stack((const void*)address)))
> +			WARN_ON(!virt_addr_valid(address));
> +
>   	return __pa((unsigned long)address);
>   }
>
>
> v3 is: (same as v1)
>
>
> @@ -804,6 +806,8 @@ extern void __iounmap_at(void *ea, unsigned long size);
>    */
>   static inline unsigned long virt_to_phys(volatile void * address)
>   {
> +	WARN_ON(IS_ENABLED(CONFIG_DEBUG_VIRTUAL) && !virt_addr_valid(address));
> +
>   	return __pa((unsigned long)address);
>   }

Right, sorry I must have been looking at v1 (which was already applied
in my tree).

> The idea in v2 was to detect objects on stack used for DMA before 
> activating CONFIG_VMAP_STACK, but if the driver uses virt_addr_valid() 
> to decide if it can DMA map it, then we'll get false positives.
> So I think this should be added with a dedicated DEBUG CONFIG option, 
> not implicitely.

Sounds good. I'll take v3.

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL
  2018-12-11 22:40 [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL Christophe Leroy
  2018-12-12  0:23 ` Michael Ellerman
@ 2018-12-18 14:06 ` Michael Ellerman
  2018-12-19  0:26   ` Michael Ellerman
  2018-12-22  9:54 ` [v3] " Michael Ellerman
  2 siblings, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2018-12-18 14:06 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev

Christophe Leroy <christophe.leroy@c-s.fr> writes:

> This patch implements CONFIG_DEBUG_VIRTUAL to warn about
> incorrect use of virt_to_phys() and page_to_phys()

This commit is breaking my p5020ds booting a 32-bit kernel with:

  smp: Bringing up secondary CPUs ...
  __ioremap(): phys addr 0x7fef5000 is RAM lr ioremap_coherent
  Unable to handle kernel paging request for data at address 0x00000000
  Faulting instruction address: 0xc002e950
  Oops: Kernel access of bad area, sig: 11 [#1]
  BE SMP NR_CPUS=24 CoreNet Generic
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.20.0-rc2-gcc-7.0.1-00138-g9a0380d299e9 #148
  NIP:  c002e950 LR: c002eb20 CTR: 00000001
  REGS: e804bd20 TRAP: 0300   Not tainted  (4.20.0-rc2-gcc-7.0.1-00138-g9a0380d299e9)
  MSR:  00021002 <CE,ME>  CR: 28004222  XER: 00000000
  DEAR: 00000000 ESR: 00000000 
  GPR00: c002eb20 e804bdd0 e8050000 00000000 00021002 00000000 00000050 00021002 
  GPR08: 2d3f0000 00000001 00000000 00000004 24000842 00000000 c00026d0 00000000 
  GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000001 
  GPR24: 00029002 7fef5140 30000000 00000000 00000000 00000040 00000001 00000000 
  NIP [c002e950] smp_85xx_kick_cpu+0x120/0x410
  LR [c002eb20] smp_85xx_kick_cpu+0x2f0/0x410
  Call Trace:
  [e804bdd0] [c002eb20] smp_85xx_kick_cpu+0x2f0/0x410 (unreliable)
  [e804be20] [c0012e38] __cpu_up+0xc8/0x230
  [e804be50] [c0040b34] bringup_cpu+0x34/0x110
  [e804be70] [c00418a8] cpu_up+0x128/0x250
  [e804beb0] [c0b84b14] smp_init+0xc4/0x10c
  [e804bee0] [c0b75c1c] kernel_init_freeable+0xc8/0x250
  [e804bf20] [c00026e8] kernel_init+0x18/0x120
  [e804bf40] [c0011298] ret_from_kernel_thread+0x14/0x1c
  Instruction dump:
  7fb3e850 57bdd1be 2e1d0000 41d20250 57bd3032 393dffc0 7e6a9b78 5529d1be 
  39290001 7d2903a6 60000000 60000000 <7c0050ac> 394a0040 4200fff8 7c0004ac 
  ---[ end trace edcab2a1dfd5b38c ]---


Which is obviously this hunk:

> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index 4fc77a99c9bf..68d204a45cd0 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -143,7 +143,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, pgprot_t prot, void *call
>  	 * 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)) &&
> +	if (slab_is_available() && virt_addr_valid(p) &&
>  	    page_is_ram(__phys_to_pfn(p))) {
>  		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
>  		       (unsigned long long)p, __builtin_return_address(0));


I'll try and come up with a fix tomorrow.

cheers


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL
  2018-12-18 14:06 ` Michael Ellerman
@ 2018-12-19  0:26   ` Michael Ellerman
  2018-12-19  6:57     ` Christophe Leroy
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2018-12-19  0:26 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev

Michael Ellerman <mpe@ellerman.id.au> writes:
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>
>> This patch implements CONFIG_DEBUG_VIRTUAL to warn about
>> incorrect use of virt_to_phys() and page_to_phys()
>
> This commit is breaking my p5020ds booting a 32-bit kernel with:
>
>   smp: Bringing up secondary CPUs ...
>   __ioremap(): phys addr 0x7fef5000 is RAM lr ioremap_coherent
>   Unable to handle kernel paging request for data at address 0x00000000
>   Faulting instruction address: 0xc002e950
>   Oops: Kernel access of bad area, sig: 11 [#1]
>   BE SMP NR_CPUS=24 CoreNet Generic
>   Modules linked in:
>   CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.20.0-rc2-gcc-7.0.1-00138-g9a0380d299e9 #148
>   NIP:  c002e950 LR: c002eb20 CTR: 00000001
>   REGS: e804bd20 TRAP: 0300   Not tainted  (4.20.0-rc2-gcc-7.0.1-00138-g9a0380d299e9)
>   MSR:  00021002 <CE,ME>  CR: 28004222  XER: 00000000
>   DEAR: 00000000 ESR: 00000000 
>   GPR00: c002eb20 e804bdd0 e8050000 00000000 00021002 00000000 00000050 00021002 
>   GPR08: 2d3f0000 00000001 00000000 00000004 24000842 00000000 c00026d0 00000000 
>   GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000001 
>   GPR24: 00029002 7fef5140 30000000 00000000 00000000 00000040 00000001 00000000 
>   NIP [c002e950] smp_85xx_kick_cpu+0x120/0x410
>   LR [c002eb20] smp_85xx_kick_cpu+0x2f0/0x410
>   Call Trace:
>   [e804bdd0] [c002eb20] smp_85xx_kick_cpu+0x2f0/0x410 (unreliable)
>   [e804be20] [c0012e38] __cpu_up+0xc8/0x230
>   [e804be50] [c0040b34] bringup_cpu+0x34/0x110
>   [e804be70] [c00418a8] cpu_up+0x128/0x250
>   [e804beb0] [c0b84b14] smp_init+0xc4/0x10c
>   [e804bee0] [c0b75c1c] kernel_init_freeable+0xc8/0x250
>   [e804bf20] [c00026e8] kernel_init+0x18/0x120
>   [e804bf40] [c0011298] ret_from_kernel_thread+0x14/0x1c
>   Instruction dump:
>   7fb3e850 57bdd1be 2e1d0000 41d20250 57bd3032 393dffc0 7e6a9b78 5529d1be 
>   39290001 7d2903a6 60000000 60000000 <7c0050ac> 394a0040 4200fff8 7c0004ac 
>   ---[ end trace edcab2a1dfd5b38c ]---
>
>
> Which is obviously this hunk:
>
>> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
>> index 4fc77a99c9bf..68d204a45cd0 100644
>> --- a/arch/powerpc/mm/pgtable_32.c
>> +++ b/arch/powerpc/mm/pgtable_32.c
>> @@ -143,7 +143,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, pgprot_t prot, void *call
>>  	 * 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)) &&
>> +	if (slab_is_available() && virt_addr_valid(p) &&
>>  	    page_is_ram(__phys_to_pfn(p))) {
>>  		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
>>  		       (unsigned long long)p, __builtin_return_address(0));
>
>
> I'll try and come up with a fix tomorrow.

Actually I think that change is just wrong. virt_addr_valid() takes a
virtual address, but p is a physical address.

So I'll drop this hunk for now, which makes the patch a no-op when
DEBUG_VIRTUAL is n which is probably the way it should be.

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL
  2018-12-19  0:26   ` Michael Ellerman
@ 2018-12-19  6:57     ` Christophe Leroy
  2018-12-19  7:03       ` Christophe Leroy
  0 siblings, 1 reply; 10+ messages in thread
From: Christophe Leroy @ 2018-12-19  6:57 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linux-kernel, linuxppc-dev



Le 19/12/2018 à 01:26, Michael Ellerman a écrit :
> Michael Ellerman <mpe@ellerman.id.au> writes:
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>
>>> This patch implements CONFIG_DEBUG_VIRTUAL to warn about
>>> incorrect use of virt_to_phys() and page_to_phys()
>>
>> This commit is breaking my p5020ds booting a 32-bit kernel with:
>>
>>    smp: Bringing up secondary CPUs ...
>>    __ioremap(): phys addr 0x7fef5000 is RAM lr ioremap_coherent
>>    Unable to handle kernel paging request for data at address 0x00000000
>>    Faulting instruction address: 0xc002e950
>>    Oops: Kernel access of bad area, sig: 11 [#1]
>>    BE SMP NR_CPUS=24 CoreNet Generic
>>    Modules linked in:
>>    CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.20.0-rc2-gcc-7.0.1-00138-g9a0380d299e9 #148
>>    NIP:  c002e950 LR: c002eb20 CTR: 00000001
>>    REGS: e804bd20 TRAP: 0300   Not tainted  (4.20.0-rc2-gcc-7.0.1-00138-g9a0380d299e9)
>>    MSR:  00021002 <CE,ME>  CR: 28004222  XER: 00000000
>>    DEAR: 00000000 ESR: 00000000
>>    GPR00: c002eb20 e804bdd0 e8050000 00000000 00021002 00000000 00000050 00021002
>>    GPR08: 2d3f0000 00000001 00000000 00000004 24000842 00000000 c00026d0 00000000
>>    GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000001
>>    GPR24: 00029002 7fef5140 30000000 00000000 00000000 00000040 00000001 00000000
>>    NIP [c002e950] smp_85xx_kick_cpu+0x120/0x410
>>    LR [c002eb20] smp_85xx_kick_cpu+0x2f0/0x410
>>    Call Trace:
>>    [e804bdd0] [c002eb20] smp_85xx_kick_cpu+0x2f0/0x410 (unreliable)
>>    [e804be20] [c0012e38] __cpu_up+0xc8/0x230
>>    [e804be50] [c0040b34] bringup_cpu+0x34/0x110
>>    [e804be70] [c00418a8] cpu_up+0x128/0x250
>>    [e804beb0] [c0b84b14] smp_init+0xc4/0x10c
>>    [e804bee0] [c0b75c1c] kernel_init_freeable+0xc8/0x250
>>    [e804bf20] [c00026e8] kernel_init+0x18/0x120
>>    [e804bf40] [c0011298] ret_from_kernel_thread+0x14/0x1c
>>    Instruction dump:
>>    7fb3e850 57bdd1be 2e1d0000 41d20250 57bd3032 393dffc0 7e6a9b78 5529d1be
>>    39290001 7d2903a6 60000000 60000000 <7c0050ac> 394a0040 4200fff8 7c0004ac
>>    ---[ end trace edcab2a1dfd5b38c ]---
>>
>>
>> Which is obviously this hunk:
>>
>>> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
>>> index 4fc77a99c9bf..68d204a45cd0 100644
>>> --- a/arch/powerpc/mm/pgtable_32.c
>>> +++ b/arch/powerpc/mm/pgtable_32.c
>>> @@ -143,7 +143,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, pgprot_t prot, void *call
>>>   	 * 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)) &&
>>> +	if (slab_is_available() && virt_addr_valid(p) &&
>>>   	    page_is_ram(__phys_to_pfn(p))) {
>>>   		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
>>>   		       (unsigned long long)p, __builtin_return_address(0));
>>
>>
>> I'll try and come up with a fix tomorrow.
> 
> Actually I think that change is just wrong. virt_addr_valid() takes a
> virtual address, but p is a physical address.
> 
> So I'll drop this hunk for now, which makes the patch a no-op when
> DEBUG_VIRTUAL is n which is probably the way it should be.

The hunk is obviously wrong for sure. Anyway there's a problem, most 
likely high_memory is not a valid virtual address, so without this hunk 
I get the following warning at every ioremap():

[    0.000000] WARNING: CPU: 0 PID: 0 at 
./arch/powerpc/include/asm/io.h:809 __ioremap_caller+0x9c/0x180
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 
4.20.0-rc6-s3k-dev-00677-g9c98dcab6203-dirty #615
[    0.000000] NIP:  c000fcd0 LR: c000fc64 CTR: 00000000
[    0.000000] REGS: c073de50 TRAP: 0700   Not tainted 
(4.20.0-rc6-s3k-dev-00677-g9c98dcab6203-dirty)
[    0.000000] MSR:  00021032 <ME,IR,DR,RI>  CR: 28944422  XER: 0000f940
[    0.000000]
[    0.000000] GPR00: c000fe04 c073df00 c06e1450 00000001 00004023 
c073df38 c0018f50 00000001
[    0.000000] GPR08: 00002000 08000000 00002000 00000000 88944224 
00600000 00000000 07ff9580
[    0.000000] GPR16: 00000000 07ffb94c 00000000 00000000 00000000 
00000000 00000000 00000000
[    0.000000] GPR24: 00000000 c0760000 0000019f ff000000 ff000000 
c000fe04 00004000 c0018f50
[    0.000000] NIP [c000fcd0] __ioremap_caller+0x9c/0x180
[    0.000000] LR [c000fc64] __ioremap_caller+0x30/0x180
[    0.000000] Call Trace:
[    0.000000] [c073df00] [c02fc23c] of_address_to_resource+0x114/0x154 
(unreliable)
[    0.000000] [c073df30] [c000fe04] ioremap_wt+0x20/0x30
[    0.000000] [c073df40] [c0018f50] mpc8xx_pic_init+0x70/0xf8
[    0.000000] [c073df80] [c0655b84] mpc8xx_pics_init+0x10/0x6c
[    0.000000] [c073df90] [c0675080] cmpc885_pics_init+0x14/0x118
[    0.000000] [c073dfa0] [c0652eb0] init_IRQ+0x24/0x38
[    0.000000] [c073dfb0] [c0650b10] start_kernel+0x2a8/0x3d4
[    0.000000] [c073dff0] [c0002258] start_here+0x44/0x98
[    0.000000] Instruction dump:
[    0.000000] 419e00b8 7f83e378 480013fd 7c7d1b79 41820030 576304be 
7c63ea14 80010034
[    0.000000] bb410018 7c0803a6 38210030 4e800020 <0fe00000> 7f9c4840 
409cffc4 480000a8
[    0.000000] random: get_random_bytes called from 
print_oops_end_marker+0x60/0x84 with crng_init=0
[    0.000000] ---[ end trace 25d7f28ce013ad37 ]---

I'll try and come with solution during the day.

Christophe

> 
> cheers
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL
  2018-12-19  6:57     ` Christophe Leroy
@ 2018-12-19  7:03       ` Christophe Leroy
  2018-12-19  8:04         ` Michael Ellerman
  0 siblings, 1 reply; 10+ messages in thread
From: Christophe Leroy @ 2018-12-19  7:03 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linuxppc-dev, linux-kernel



On 12/19/2018 06:57 AM, Christophe Leroy wrote:
> 
> 
> Le 19/12/2018 à 01:26, Michael Ellerman a écrit :
>> Michael Ellerman <mpe@ellerman.id.au> writes:
>>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>>
>>>> This patch implements CONFIG_DEBUG_VIRTUAL to warn about
>>>> incorrect use of virt_to_phys() and page_to_phys()
>>>
>>> This commit is breaking my p5020ds booting a 32-bit kernel with:
>>>
>>>    smp: Bringing up secondary CPUs ...
>>>    __ioremap(): phys addr 0x7fef5000 is RAM lr ioremap_coherent
>>>    Unable to handle kernel paging request for data at address 0x00000000
>>>    Faulting instruction address: 0xc002e950
>>>    Oops: Kernel access of bad area, sig: 11 [#1]
>>>    BE SMP NR_CPUS=24 CoreNet Generic
>>>    Modules linked in:
>>>    CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
>>> 4.20.0-rc2-gcc-7.0.1-00138-g9a0380d299e9 #148
>>>    NIP:  c002e950 LR: c002eb20 CTR: 00000001
>>>    REGS: e804bd20 TRAP: 0300   Not tainted  
>>> (4.20.0-rc2-gcc-7.0.1-00138-g9a0380d299e9)
>>>    MSR:  00021002 <CE,ME>  CR: 28004222  XER: 00000000
>>>    DEAR: 00000000 ESR: 00000000
>>>    GPR00: c002eb20 e804bdd0 e8050000 00000000 00021002 00000000 
>>> 00000050 00021002
>>>    GPR08: 2d3f0000 00000001 00000000 00000004 24000842 00000000 
>>> c00026d0 00000000
>>>    GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 
>>> 00000000 00000001
>>>    GPR24: 00029002 7fef5140 30000000 00000000 00000000 00000040 
>>> 00000001 00000000
>>>    NIP [c002e950] smp_85xx_kick_cpu+0x120/0x410
>>>    LR [c002eb20] smp_85xx_kick_cpu+0x2f0/0x410
>>>    Call Trace:
>>>    [e804bdd0] [c002eb20] smp_85xx_kick_cpu+0x2f0/0x410 (unreliable)
>>>    [e804be20] [c0012e38] __cpu_up+0xc8/0x230
>>>    [e804be50] [c0040b34] bringup_cpu+0x34/0x110
>>>    [e804be70] [c00418a8] cpu_up+0x128/0x250
>>>    [e804beb0] [c0b84b14] smp_init+0xc4/0x10c
>>>    [e804bee0] [c0b75c1c] kernel_init_freeable+0xc8/0x250
>>>    [e804bf20] [c00026e8] kernel_init+0x18/0x120
>>>    [e804bf40] [c0011298] ret_from_kernel_thread+0x14/0x1c
>>>    Instruction dump:
>>>    7fb3e850 57bdd1be 2e1d0000 41d20250 57bd3032 393dffc0 7e6a9b78 
>>> 5529d1be
>>>    39290001 7d2903a6 60000000 60000000 <7c0050ac> 394a0040 4200fff8 
>>> 7c0004ac
>>>    ---[ end trace edcab2a1dfd5b38c ]---
>>>
>>>
>>> Which is obviously this hunk:
>>>
>>>> diff --git a/arch/powerpc/mm/pgtable_32.c 
>>>> b/arch/powerpc/mm/pgtable_32.c
>>>> index 4fc77a99c9bf..68d204a45cd0 100644
>>>> --- a/arch/powerpc/mm/pgtable_32.c
>>>> +++ b/arch/powerpc/mm/pgtable_32.c
>>>> @@ -143,7 +143,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long 
>>>> size, pgprot_t prot, void *call
>>>>        * 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)) &&
>>>> +    if (slab_is_available() && virt_addr_valid(p) &&
>>>>           page_is_ram(__phys_to_pfn(p))) {
>>>>           printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
>>>>                  (unsigned long long)p, __builtin_return_address(0));
>>>
>>>
>>> I'll try and come up with a fix tomorrow.
>>
>> Actually I think that change is just wrong. virt_addr_valid() takes a
>> virtual address, but p is a physical address.
>>
>> So I'll drop this hunk for now, which makes the patch a no-op when
>> DEBUG_VIRTUAL is n which is probably the way it should be.
> 
> The hunk is obviously wrong for sure. Anyway there's a problem, most 
> likely high_memory is not a valid virtual address, so without this hunk 
> I get the following warning at every ioremap():
> 
> [    0.000000] WARNING: CPU: 0 PID: 0 at 
> ./arch/powerpc/include/asm/io.h:809 __ioremap_caller+0x9c/0x180
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 
> 4.20.0-rc6-s3k-dev-00677-g9c98dcab6203-dirty #615
> [    0.000000] NIP:  c000fcd0 LR: c000fc64 CTR: 00000000
> [    0.000000] REGS: c073de50 TRAP: 0700   Not tainted 
> (4.20.0-rc6-s3k-dev-00677-g9c98dcab6203-dirty)
> [    0.000000] MSR:  00021032 <ME,IR,DR,RI>  CR: 28944422  XER: 0000f940
> [    0.000000]
> [    0.000000] GPR00: c000fe04 c073df00 c06e1450 00000001 00004023 
> c073df38 c0018f50 00000001
> [    0.000000] GPR08: 00002000 08000000 00002000 00000000 88944224 
> 00600000 00000000 07ff9580
> [    0.000000] GPR16: 00000000 07ffb94c 00000000 00000000 00000000 
> 00000000 00000000 00000000
> [    0.000000] GPR24: 00000000 c0760000 0000019f ff000000 ff000000 
> c000fe04 00004000 c0018f50
> [    0.000000] NIP [c000fcd0] __ioremap_caller+0x9c/0x180
> [    0.000000] LR [c000fc64] __ioremap_caller+0x30/0x180
> [    0.000000] Call Trace:
> [    0.000000] [c073df00] [c02fc23c] of_address_to_resource+0x114/0x154 
> (unreliable)
> [    0.000000] [c073df30] [c000fe04] ioremap_wt+0x20/0x30
> [    0.000000] [c073df40] [c0018f50] mpc8xx_pic_init+0x70/0xf8
> [    0.000000] [c073df80] [c0655b84] mpc8xx_pics_init+0x10/0x6c
> [    0.000000] [c073df90] [c0675080] cmpc885_pics_init+0x14/0x118
> [    0.000000] [c073dfa0] [c0652eb0] init_IRQ+0x24/0x38
> [    0.000000] [c073dfb0] [c0650b10] start_kernel+0x2a8/0x3d4
> [    0.000000] [c073dff0] [c0002258] start_here+0x44/0x98
> [    0.000000] Instruction dump:
> [    0.000000] 419e00b8 7f83e378 480013fd 7c7d1b79 41820030 576304be 
> 7c63ea14 80010034
> [    0.000000] bb410018 7c0803a6 38210030 4e800020 <0fe00000> 7f9c4840 
> 409cffc4 480000a8
> [    0.000000] random: get_random_bytes called from 
> print_oops_end_marker+0x60/0x84 with crng_init=0
> [    0.000000] ---[ end trace 25d7f28ce013ad37 ]---
> 
> I'll try and come with solution during the day.

In fact the solution is the following:

diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 4fc77a99c9bf..60401af2bc8f 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -143,7 +143,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long 
size, pgprot_t prot, void *call
  	 * 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)) &&
+	if (slab_is_available() && (p <= virt_to_phys(high_memory - 1)) &&
  	    page_is_ram(__phys_to_pfn(p))) {
  		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
  		       (unsigned long long)p, __builtin_return_address(0));


I'll send an updated patch in a few minutes.

Christophe

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL
  2018-12-19  7:03       ` Christophe Leroy
@ 2018-12-19  8:04         ` Michael Ellerman
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2018-12-19  8:04 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linuxppc-dev, linux-kernel

Christophe Leroy <christophe.leroy@c-s.fr> writes:
> On 12/19/2018 06:57 AM, Christophe Leroy wrote:
...
>
> In fact the solution is the following:
>
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index 4fc77a99c9bf..60401af2bc8f 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -143,7 +143,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long 
> size, pgprot_t prot, void *call
>   	 * 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)) &&
> +	if (slab_is_available() && (p <= virt_to_phys(high_memory - 1)) &&
>   	    page_is_ram(__phys_to_pfn(p))) {
>   		printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
>   		       (unsigned long long)p, __builtin_return_address(0));
>
>
> I'll send an updated patch in a few minutes.

Awesome, thanks. I'll take v4.

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v3] powerpc: implement CONFIG_DEBUG_VIRTUAL
  2018-12-11 22:40 [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL Christophe Leroy
  2018-12-12  0:23 ` Michael Ellerman
  2018-12-18 14:06 ` Michael Ellerman
@ 2018-12-22  9:54 ` Michael Ellerman
  2 siblings, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2018-12-22  9:54 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
  Cc: linuxppc-dev, linux-kernel

On Tue, 2018-12-11 at 22:40:53 UTC, Christophe Leroy wrote:
> This patch implements CONFIG_DEBUG_VIRTUAL to warn about
> incorrect use of virt_to_phys() and page_to_phys()
> 
> Below is the result of test_debug_virtual:
> 
> [    1.438746] WARNING: CPU: 0 PID: 1 at ./arch/powerpc/include/asm/io.h:808 test_debug_virtual_init+0x3c/0xd4
> [    1.448156] CPU: 0 PID: 1 Comm: swapper Not tainted 4.20.0-rc5-00560-g6bfb52e23a00-dirty #532
> [    1.457259] NIP:  c066c550 LR: c0650ccc CTR: c066c514
> [    1.462257] REGS: c900bdb0 TRAP: 0700   Not tainted  (4.20.0-rc5-00560-g6bfb52e23a00-dirty)
> [    1.471184] MSR:  00029032 <EE,ME,IR,DR,RI>  CR: 48000422  XER: 20000000
> [    1.477811]
> [    1.477811] GPR00: c0650ccc c900be60 c60d0000 00000000 006000c0 c9000000 00009032 c7fa0020
> [    1.477811] GPR08: 00002400 00000001 09000000 00000000 c07b5d04 00000000 c00037d8 00000000
> [    1.477811] GPR16: 00000000 00000000 00000000 00000000 c0760000 c0740000 00000092 c0685bb0
> [    1.477811] GPR24: c065042c c068a734 c0685b8c 00000006 00000000 c0760000 c075c3c0 ffffffff
> [    1.512711] NIP [c066c550] test_debug_virtual_init+0x3c/0xd4
> [    1.518315] LR [c0650ccc] do_one_initcall+0x8c/0x1cc
> [    1.523163] Call Trace:
> [    1.525595] [c900be60] [c0567340] 0xc0567340 (unreliable)
> [    1.530954] [c900be90] [c0650ccc] do_one_initcall+0x8c/0x1cc
> [    1.536551] [c900bef0] [c0651000] kernel_init_freeable+0x1f4/0x2cc
> [    1.542658] [c900bf30] [c00037ec] kernel_init+0x14/0x110
> [    1.547913] [c900bf40] [c000e1d0] ret_from_kernel_thread+0x14/0x1c
> [    1.553971] Instruction dump:
> [    1.556909] 3ca50100 bfa10024 54a5000e 3fa0c076 7c0802a6 3d454000 813dc204 554893be
> [    1.564566] 7d294010 7d294910 90010034 39290001 <0f090000> 7c3e0b78 955e0008 3fe0c062
> [    1.572425] ---[ end trace 6f6984225b280ad6 ]---
> [    1.577467] PA: 0x09000000 for VA: 0xc9000000
> [    1.581799] PA: 0x061e8f50 for VA: 0xc61e8f50
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/6bf752daca07c85c181159f75dcf65

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-12-22 17:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-11 22:40 [PATCH v3] powerpc: implement CONFIG_DEBUG_VIRTUAL Christophe Leroy
2018-12-12  0:23 ` Michael Ellerman
2018-12-12  7:19   ` Christophe Leroy
2018-12-12 10:35     ` Michael Ellerman
2018-12-18 14:06 ` Michael Ellerman
2018-12-19  0:26   ` Michael Ellerman
2018-12-19  6:57     ` Christophe Leroy
2018-12-19  7:03       ` Christophe Leroy
2018-12-19  8:04         ` Michael Ellerman
2018-12-22  9:54 ` [v3] " Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).