ioremap() fail on physical address 0x0 in 3.4 kernel

DISCUSSION Murali Nalajala 5 participants 8 messages Aug 21, 2012 – Aug 22, 2012

Subsystems (2)

Arm (32-bit) Arm Qualcomm mailing list

lore

Share & tools

lore

Murali Nalajala

author

2012-08-21 14:28 UTC #0 lore raw

Hi All,
I am doing a below call in my driver to get the virtual address equivalent to physical address 0x0.

pdata->v_addr = ioremap(pdata->p_addr, PAGE_SIZE); /* pdata->p_addr=0 */

Above call returns me a valid virtual address i.e: 0xfa200000. After that when i try to access the address i am getting a kernel panic like below.

[   59.498141] Unable to handle kernel paging request at virtual address 
fa200000
[   59.498141] pgd = d4f80000
[   59.498141] [fa200000] *pgd=00000000
[   59.498141] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[   59.498141] Modules linked in:
[   59.498141] CPU: 0    Tainted: G        W 
(3.4.0-g3b5f728-00009-g638207a #13)
[   59.498141] PC is at msm_pm_config_rst_vector_before_pc+0x8/0x30
[   59.498141] LR is at msm_pm_boot_config_before_pc+0x18/0x20
[   59.498141] pc : [<c0078f84>]    lr : [<c007903c>]    psr: a0000093
[   59.498141] sp : c0837ef0  ip : cfe00000  fp : 0000000d
[   59.498141] r10: da7efc17  r9 : 225c4278  r8 : 00000006
[   59.498141] r7 : 0003c000  r6 : c085c824  r5 : 00000001  r4 : fa101000
[   59.498141] r3 : fa200000  r2 : c095080c  r1 : 002250fc  r0 : 00000000
[   59.498141] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM 
Segment kernel
[   59.498141] Control: 10c5387d  Table: 25180059  DAC: 00000015
[   59.498141] [<c0078f84>] 
(msm_pm_config_rst_vector_before_pc+0x8/0x30) from [<c007903c>] 
(msm_pm_boot_config_before_pc+0x18/0x20)
[   59.498141] [<c007903c>] (msm_pm_boot_config_before_pc+0x18/0x20) 
from [<c007a55c>] (msm_pm_power_collapse+0x410/0xb04)
[   59.498141] [<c007a55c>] (msm_pm_power_collapse+0x410/0xb04) from 
[<c007b17c>] (arch_idle+0x294/0x3e0)
[   59.498141] [<c007b17c>] (arch_idle+0x294/0x3e0) from [<c000eed8>] 
(default_idle+0x18/0x2c)
[   59.498141] [<c000eed8>] (default_idle+0x18/0x2c) from [<c000f254>] 
(cpu_idle+0x90/0xe4)
[   59.498141] [<c000f254>] (cpu_idle+0x90/0xe4) from [<c057231c>] 
(rest_init+0x88/0xa0)
[   59.498141] [<c057231c>] (rest_init+0x88/0xa0) from [<c07ff890>] 
(start_kernel+0x3a8/0x40c)
[   59.498141] Code: c0704256 e12fff1e e59f2020 e5923000 (e5930000)

I am observing the crash on 3.4 but not on the 3.0 kernel.I check the "arch/arch/mm/ioremap.c" code changes between 3.0 and 3.4. I came across few changes that are uploaded by "Nicolas Pitre" and few fixes made by RMK and Pawel Moll.

I don't see a crash after i commented out the below code in __arm_ioremap_pfn_caller().

read_lock()
---
loop
---
read_unlock()

Here i am pasting a few extra logs that i have added into "ioremap.c" & "mmu.c" file.

[    0.000000] memory pool 3 (start 2a300000 size 3800000) initialized
[    0.000000] Memory policy: ECC disabled, Data cache writeback
[    0.000000] vm_area_add_early: called with addr=fa100000 size=0x1000
[    0.000000] vm_area_add_early: called with addr=fa101000 size=0x1000
[    0.000000] vm_area_add_early: called with addr=fa000000 size=0x1000
[    0.000000] vm_area_add_early: called with addr=fa102000 size=0x1000
[    0.000000] vm_area_add_early: called with addr=fa103000 size=0x1000
[    0.000000] vm_area_add_early: called with addr=fa010000 size=0x1000
[    0.000000] vm_area_add_early: called with addr=fa900000 size=0xd00000
[    0.000000] vm_area_add_early: called with addr=fa400000 size=0x100000
[    0.000000] vm_area_add_early: called with addr=fa701000 size=0x1000
[    0.000000] vm_area_add_early: called with addr=fa300000 size=0x100000
[    0.000000] vm_area_add_early: called with addr=fa200000 size=0x100000
[    0.000000] vm_area_add_early: called with addr=fa500000 size=0x100000
[    0.000000] vm_area_add_early: called with addr=fa800000 size=0x100000
...
...
...
[ 0.161784] ioremap: pfn=0x0 phys=0x0 offset=0x0 size=0x1000
[ 0.161813] ioremap: area da0fbdc0: phys_addr=0xc0100000 pfn=0xc0100 
size=0x1000
[ 0.161838] ioremap: area da0fbe20: phys_addr=0xa8600000 pfn=0xa8600 
size=0x1000
[ 0.161861] ioremap: area da0fbd80: phys_addr=0xc0000000 pfn=0xc0000 
size=0x1000
[ 0.161886] ioremap: area da0fbda0: phys_addr=0xc0100000 pfn=0xc0100 
size=0x1000
[ 0.161909] ioremap: area da0fbde0: phys_addr=0xa9200000 pfn=0xa9200 
size=0x1000
[ 0.161933] ioremap: area da0fbe00: phys_addr=0xa9300000 pfn=0xa9300 
size=0x1000
[ 0.161956] ioremap: area da0fbd40: phys_addr=0x0 pfn=0x0 size=0x100000
[ 0.161979] ioremap: found: addr fa200000 => 0xfa200000 => 0xfa200000
[ 0.161999] *** reset_vector = 0xfa200000

Can someone know me what is wrong in ioremap call?
Why i am not seeing a crash after i commented out the loop above?

Thanks,
Murali N

Signature · 2 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Laura Abbott

2012-08-21 17:23 UTC #1 lore raw

Quoted 9 lines · Murali Nalajala

Hi All,
I am doing a below call in my driver to get the virtual address equivalent to physical address 0x0.
pdata->v_addr = ioremap(pdata->p_addr, PAGE_SIZE); /* pdata->p_addr=0 */
Above call returns me a valid virtual address i.e: 0xfa200000. After that when i try to access the address i am getting a kernel panic like below.

<snip>

Quoted 18 lines

[ 0.161784] ioremap: pfn=0x0 phys=0x0 offset=0x0 size=0x1000
[ 0.161813] ioremap: area da0fbdc0: phys_addr=0xc0100000 pfn=0xc0100
size=0x1000
[ 0.161838] ioremap: area da0fbe20: phys_addr=0xa8600000 pfn=0xa8600
size=0x1000
[ 0.161861] ioremap: area da0fbd80: phys_addr=0xc0000000 pfn=0xc0000
size=0x1000
[ 0.161886] ioremap: area da0fbda0: phys_addr=0xc0100000 pfn=0xc0100
size=0x1000
[ 0.161909] ioremap: area da0fbde0: phys_addr=0xa9200000 pfn=0xa9200
size=0x1000
[ 0.161933] ioremap: area da0fbe00: phys_addr=0xa9300000 pfn=0xa9300
size=0x1000
[ 0.161956] ioremap: area da0fbd40: phys_addr=0x0 pfn=0x0 size=0x100000
[ 0.161979] ioremap: found: addr fa200000 => 0xfa200000 => 0xfa200000
[ 0.161999] *** reset_vector = 0xfa200000

Quoted 3 lines

Can someone know me what is wrong in ioremap call?
Why i am not seeing a crash after i commented out the loop above?

Looks like you are hitting the empty section gap:

0xfa200000-0xfa300000 1048576 pmd_empty_section_gap+0x0/0x3c ioremap

pmd_empty_section_gap ends up with vm->phys_addr = 0x0 because it was never set. This section isn't actually mapped so when searching for the range in the static io map it finds this address and returns it but it isn't actually a valid address to return. Perhaps ioremap should not bother trying to re-use the static iomap if the address is zero and let pmd_empty_section_gap use 0x0 as a dummy value?

arch/arm/mm/ioremap.c +0 -0

@@ -228,7 +228,8 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned

long pfn,

          */
         read_lock(&vmlist_lock);
         for (area = vmlist; area; area = area->next) {
-               if (!size || (sizeof(phys_addr_t) == 4 && pfn >= 0x100000))
+               if (!size || !pfn || (sizeof(phys_addr_t) == 4 &&
+                   pfn >= 0x100000))
                         break;
                 if (!(area->flags & VM_ARM_STATIC_MAPPING))
                         continue;

Quoted 4 lines

Thanks,
Murali N

Thanks,
Laura

Signature · 2 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Murali Nalajala

author

2012-08-22 06:41 UTC #2 lore raw

Quoted 66 lines · Laura Abbott

Quoted 9 lines · Murali Nalajala
Hi All,
I am doing a below call in my driver to get the virtual address equivalent to physical address 0x0.
pdata->v_addr = ioremap(pdata->p_addr, PAGE_SIZE); /* pdata->p_addr=0 */
Above call returns me a valid virtual address i.e: 0xfa200000. After that when i try to access the address i am getting a kernel panic like below.
<snip>
Quoted 18 lines
[ 0.161784] ioremap: pfn=0x0 phys=0x0 offset=0x0 size=0x1000
[ 0.161813] ioremap: area da0fbdc0: phys_addr=0xc0100000 pfn=0xc0100
size=0x1000
[ 0.161838] ioremap: area da0fbe20: phys_addr=0xa8600000 pfn=0xa8600
size=0x1000
[ 0.161861] ioremap: area da0fbd80: phys_addr=0xc0000000 pfn=0xc0000
size=0x1000
[ 0.161886] ioremap: area da0fbda0: phys_addr=0xc0100000 pfn=0xc0100
size=0x1000
[ 0.161909] ioremap: area da0fbde0: phys_addr=0xa9200000 pfn=0xa9200
size=0x1000
[ 0.161933] ioremap: area da0fbe00: phys_addr=0xa9300000 pfn=0xa9300
size=0x1000
[ 0.161956] ioremap: area da0fbd40: phys_addr=0x0 pfn=0x0 size=0x100000
[ 0.161979] ioremap: found: addr fa200000 => 0xfa200000 => 0xfa200000
[ 0.161999] *** reset_vector = 0xfa200000
Quoted 3 lines
Can someone know me what is wrong in ioremap call?
Why i am not seeing a crash after i commented out the loop above?
Looks like you are hitting the empty section gap:
0xfa200000-0xfa300000 1048576 pmd_empty_section_gap+0x0/0x3c ioremap
pmd_empty_section_gap ends up with vm->phys_addr = 0x0 because it was never set. This section isn't actually mapped so when searching for the range in the static io map it finds this address and returns it but it isn't actually a valid address to return. Perhaps ioremap should not bother trying to re-use the static iomap if the address is zero and let pmd_empty_section_gap use 0x0 as a dummy value?
arch/arm/mm/ioremap.c +2 -1
@@ -228,7 +228,8 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned
 long pfn,
 */
 read_lock(&vmlist_lock);
 for (area = vmlist; area; area = area->next) {
- if (!size || (sizeof(phys_addr_t) == 4 && pfn >= 0x100000))
+ if (!size || !pfn || (sizeof(phys_addr_t) == 4 &&
+ pfn >= 0x100000))
 break;
 if (!(area->flags & VM_ARM_STATIC_MAPPING))
continue;
Quoted 1 line

Quoted 7 lines

Quoted 3 lines
Thanks,
Murali N
Thanks,
Laura

Hi Nicolas,
Laura's code helps me to resolve the issue. But not sure of side effects of this change and other implications.

If the above changes are ok, can you please provide the ack.

Thanks,
Murali N

Signature · 2 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Trilok Soni

2012-08-22 06:46 UTC #3 lore raw

In reply to Laura Abbott

Hi Laura,

Quoted 18 lines · Laura Abbott

Looks like you are hitting the empty section gap:
0xfa200000-0xfa300000 1048576 pmd_empty_section_gap+0x0/0x3c ioremap
pmd_empty_section_gap ends up with vm->phys_addr = 0x0 because it was never set. This section isn't actually mapped so when searching for the range in the static io map it finds this address and returns it but it isn't actually a valid address to return. Perhaps ioremap should not bother trying to re-use the static iomap if the address is zero and let pmd_empty_section_gap use 0x0 as a dummy value?

arch/arm/mm/ioremap.c void __iomem * __arm_ioremap_pfn_caller(unsigned · line 233

Quoted 7 lines

arch/arm/mm/ioremap.c +2 -1

@@ -228,7 +228,8 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned
 long pfn,
 */
 read_lock(&vmlist_lock);
 for (area = vmlist; area; area = area->next) {
- if (!size || (sizeof(phys_addr_t) == 4 && pfn >= 0x100000))
+ if (!size || !pfn || (sizeof(phys_addr_t) == 4 &&
+ pfn >= 0x100000))

Thanks for the patch, but how about just checking pmd_none on such addresses and since there won't be any mapping created for such gaps? We need to check if this is fine for all, since I remember that Nico added these functions for the OMAP boot issue I guess.

---Trilok Soni

Signature · 3 lines

--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Russell King - ARM Linux

2012-08-22 09:53 UTC #4 lore raw

Quoted 4 lines · Trilok Soni

Thanks for the patch, but how about just checking pmd_none on such addresses and since there won't be any mapping created for such gaps? We need to check if this is fine for all, since I remember that Nico added these functions for the OMAP boot issue I guess.

A better solution would be as below - this requires no changes to the existing ioremap code, we just mark these differently.

arch/arm/mm/mm.h +3 -0

index 2e8a1ef..87fa3f2 100644
@@ -55,6 +55,9 @@ extern void __flush_dcache_page(struct address_space *mapping, struct page *page
 /* permanent static mappings from iotable_init() */
 #define VM_ARM_STATIC_MAPPING	0x40000000
 
+/* empty mapping */
+#define VM_ARM_EMPTY_MAPPING	0x20000000
+
 /* mapping type (attributes) for permanent static mappings */
 #define VM_ARM_MTYPE(mt)		((mt) << 20)
 #define VM_ARM_MTYPE_MASK	(0x1f << 20)

arch/arm/mm/mmu.c +2 -2

index cf4528d..da90ef1 100644
@@ -813,7 +813,7 @@ static void __init pmd_empty_section_gap(unsigned long addr)
 	vm = early_alloc_aligned(sizeof(*vm), __alignof__(*vm));
 	vm->addr = (void *)addr;
 	vm->size = SECTION_SIZE;
-	vm->flags = VM_IOREMAP | VM_ARM_STATIC_MAPPING;
+	vm->flags = VM_IOREMAP | VM_ARM_EMPTY_MAPPING;
 	vm->caller = pmd_empty_section_gap;
 	vm_area_add_early(vm);
 }

@@ -826,7 +826,7 @@ static void __init fill_pmd_gaps(void)
 
 	/* we're still single threaded hence no lock needed here */
 	for (vm = vmlist; vm; vm = vm->next) {
-		if (!(vm->flags & VM_ARM_STATIC_MAPPING))
+		if (!(vm->flags & (VM_ARM_STATIC_MAPPING | VM_ARM_EMPTY_MAPPING))
 			continue;
 		addr = (unsigned long)vm->addr;
 		if (addr < next)

Russell King - ARM Linux

2012-08-22 11:23 UTC #5 lore raw

arch/arm/mm/mmu.c static void __init fill_pmd_gaps(void) · line 829

Quoted 6 lines · Russell King - ARM Linux

arch/arm/mm/mmu.c +1 -1

@@ -826,7 +826,7 @@ static void __init fill_pmd_gaps(void)
 
 	/* we're still single threaded hence no lock needed here */
 	for (vm = vmlist; vm; vm = vm->next) {
-		if (!(vm->flags & VM_ARM_STATIC_MAPPING))
+		if (!(vm->flags & (VM_ARM_STATIC_MAPPING | VM_ARM_EMPTY_MAPPING))

Not forgetting the additional close paren needed here...

Nicolas Pitre

Acked-by

2012-08-22 13:19 UTC #6 lore raw

Quoted 9 lines · Russell King - ARM Linux

arch/arm/mm/mmu.c static void __init fill_pmd_gaps(void) · line 829
Quoted 6 lines · Russell King - ARM Linux
arch/arm/mm/mmu.c +1 -1
@@ -826,7 +826,7 @@ static void __init fill_pmd_gaps(void)
 
 	/* we're still single threaded hence no lock needed here */
 	for (vm = vmlist; vm; vm = vm->next) {
-		if (!(vm->flags & VM_ARM_STATIC_MAPPING))
+		if (!(vm->flags & (VM_ARM_STATIC_MAPPING | VM_ARM_EMPTY_MAPPING))
Not forgetting the additional close paren needed here...

Acked-by Nicolas Pitre <[email protected]>

Nicolas

Murali Nalajala

author Tested-by

2012-08-22 12:03 UTC #7 lore raw

In reply to Russell King - ARM Linux

Quoted 38 lines · Russell King - ARM Linux

Quoted 4 lines · Trilok Soni
Thanks for the patch, but how about just checking pmd_none on such addresses and since there won't be any mapping created for such gaps? We need to check if this is fine for all, since I remember that Nico added these functions for the OMAP boot issue I guess.
A better solution would be as below - this requires no changes to the existing ioremap code, we just mark these differently.
arch/arm/mm/mm.h +3 -0
index 2e8a1ef..87fa3f2 100644
@@ -55,6 +55,9 @@ extern void __flush_dcache_page(struct address_space *mapping, struct page *page
  /* permanent static mappings from iotable_init() */
  #define VM_ARM_STATIC_MAPPING	0x40000000
 
+/* empty mapping */
+#define VM_ARM_EMPTY_MAPPING	0x20000000
+
  /* mapping type (attributes) for permanent static mappings */
  #define VM_ARM_MTYPE(mt)		((mt)<<  20)
  #define VM_ARM_MTYPE_MASK	(0x1f<<  20)
arch/arm/mm/mmu.c +1 -1
index cf4528d..da90ef1 100644
@@ -813,7 +813,7 @@ static void __init pmd_empty_section_gap(unsigned long addr)
  	vm = early_alloc_aligned(sizeof(*vm), __alignof__(*vm));
  	vm->addr = (void *)addr;
  	vm->size = SECTION_SIZE;
-	vm->flags = VM_IOREMAP | VM_ARM_STATIC_MAPPING;
+	vm->flags = VM_IOREMAP | VM_ARM_EMPTY_MAPPING;
  	vm->caller = pmd_empty_section_gap;
  	vm_area_add_early(vm);
  }

arch/arm/mm/mmu.c static void __init fill_pmd_gaps(void) · line 832

Quoted 8 lines

arch/arm/mm/mmu.c +1 -1

@@ -826,7 +826,7 @@ static void __init fill_pmd_gaps(void)
 
  	/* we're still single threaded hence no lock needed here */
  	for (vm = vmlist; vm; vm = vm->next) {
-		if (!(vm->flags&  VM_ARM_STATIC_MAPPING))
+		if (!(vm->flags&  (VM_ARM_STATIC_MAPPING | VM_ARM_EMPTY_MAPPING))
  			continue;
  		addr = (unsigned long)vm->addr;
  		if (addr<  next)

Tested on MSM platform

Tested-by Murali Nalajala <[email protected]>

Thanks,
Murali N

Signature · 2 lines

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.