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

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

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

On 8/21/2012 7:28 AM, Murali Nalajala wrote:

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>

[ 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?

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?

diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c

@@ -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;

Thanks,
Murali N

Thanks,
Laura

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

On 8/21/2012 10:53 PM, Laura Abbott wrote:

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

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

Hi Laura,

On 8/21/2012 10:53 PM, Laura Abbott wrote:

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

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

On Wed, Aug 22, 2012 at 12:16:08PM +0530, Trilok Soni wrote:

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.

diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
index 2e8a1ef..87fa3f2 100644
--- a/arch/arm/mm/mm.h
+++ b/arch/arm/mm/mm.h

@@ -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)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index cf4528d..da90ef1 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c

@@ -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)

On Wed, Aug 22, 2012 at 10:53:02AM +0100, Russell King - ARM Linux wrote:

Not forgetting the additional close paren needed here...

On 8/22/2012 3:23 PM, Russell King - ARM Linux wrote:

Tested on MSM platform

Tested-by: Murali Nalajala <redacted>

Thanks,
Murali N

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

On Wed, 22 Aug 2012, Russell King - ARM Linux wrote:

On Wed, Aug 22, 2012 at 10:53:02AM +0100, Russell King - ARM Linux wrote:

quoted

@@ -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 <redacted>


Nicolas