Failure to add the mapping created in debug_ll_io_init() can lead
to the BUG_ON() triggering in lib/ioremap.c:27 if the static
virtual address decided for the debug_ll mapping overlaps with
another mapping that is created later. This happens because the
generic ioremap code has no idea there is a mapping there and it
tries to place a mapping in the same location and blows up when
it sees that there is a pte already present.
kernel BUG at lib/ioremap.c:27!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.10.0-rc2-00042-g2af0c67-dirty #316
task: ef088000 ti: ef082000 task.ti: ef082000
PC is at ioremap_page_range+0x16c/0x198
LR is at ioremap_page_range+0xf0/0x198
pc : [<c04cb874>] lr : [<c04cb7f8>] psr: 20000113
sp : ef083e78 ip : af140000 fp : ef083ebc
r10: ef7fc100 r9 : ef7fc104 r8 : 000af174
r7 : 00000647 r6 : beffffff r5 : f004c000 r4 : f0040000
r3 : af173417 r2 : 16440653 r1 : af173e07 r0 : ef7fc8fc
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c5787d Table: 8020406a DAC: 00000015
Process swapper/0 (pid: 1, stack limit = 0xef082238)
Stack: (0xef083e78 to 0xef084000)
3e60: 00040000 ef083eec
3e80: bf134000 f004bfff c0207c00 f004c000 c02fc120 f000c000 c15e7800 00040000
3ea0: ef083eec 00000647 c098ba9c c0953544 ef083edc ef083ec0 c021b82c c04cb714
3ec0: c09cdc50 00000040 ef0f1e00 ef1003c0 ef083f14 ef083ee0 c09535bc c021b7bc
3ee0: c0953544 c04d0c6c c094e2cc c1600be4 c07440c4 c09a6888 00000002 c0a15f00
3f00: ef082000 00000000 ef083f54 ef083f18 c0208728 c0953550 00000002 c1600bfc
3f20: c08e3fac c0839918 ef083f54 c1600b80 c09a6888 c0a15f00 0000008b c094e2cc
3f40: c098ba9c c098bab8 ef083f94 ef083f58 c094ea0c c020865c 00000002 00000002
3f60: c094e2cc 00000000 c025b674 00000000 c06ff860 00000000 00000000 00000000
3f80: 00000000 00000000 ef083fac ef083f98 c06ff878 c094e910 00000000 00000000
3fa0: 00000000 ef083fb0 c020efe8 c06ff86c 00000000 00000000 00000000 00000000
3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 c0595108
[<c04cb874>] (ioremap_page_range+0x16c/0x198) from [<c021b82c>] (__alloc_remap_buffer.isra.18+0x7c/0xc4)
[<c021b82c>] (__alloc_remap_buffer.isra.18+0x7c/0xc4) from [<c09535bc>] (atomic_pool_init+0x78/0x128)
[<c09535bc>] (atomic_pool_init+0x78/0x128) from [<c0208728>] (do_one_initcall+0xd8/0x198)
[<c0208728>] (do_one_initcall+0xd8/0x198) from [<c094ea0c>] (kernel_init_freeable+0x108/0x1d0)
[<c094ea0c>] (kernel_init_freeable+0x108/0x1d0) from [<c06ff878>] (kernel_init+0x18/0xf4)
[<c06ff878>] (kernel_init+0x18/0xf4) from [<c020efe8>] (ret_from_fork+0x14/0x20)
Code: e50b0040 ebf54b2f e51b0040 eaffffee (e7f001f2)
Fix it by telling generic layers about the static mapping. This also
has the nice side effect of letting you see the mapping in procfs'
vmallocinfo file.
Cc: Rob Herring <[email protected]>
Cc: Stephen Warren <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm/mm/mmu.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index e0d8565..04fe160 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -938,8 +938,12 @@ static void __init pci_reserve_io(void)
#ifdef CONFIG_DEBUG_LL
void __init debug_ll_io_init(void)
{
+ struct vm_struct *vm;
+ struct static_vm *svm;
struct map_desc map;
+ svm = early_alloc_aligned(sizeof(*svm), __alignof__(*svm));
+
debug_ll_addr(&map.pfn, &map.virtual);
if (!map.pfn || !map.virtual)
return;
@@ -948,6 +952,15 @@ void __init debug_ll_io_init(void)
map.length = PAGE_SIZE;
map.type = MT_DEVICE;
create_mapping(&map);
+
+ vm = &svm->vm;
+ vm->addr = (void *)map.virtual;
+ vm->size = map.length;
+ vm->phys_addr = __pfn_to_phys(map.pfn);
+ vm->flags = VM_IOREMAP | VM_ARM_STATIC_MAPPING;
+ vm->flags |= VM_ARM_MTYPE(map.type);
+ vm->caller = debug_ll_io_init;
+ add_static_vm_early(svm);
}
#endif
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
On Tue, Jun 11, 2013 at 8:30 PM, Stephen Boyd <[email protected]> wrote:
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index e0d8565..04fe160 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -938,8 +938,12 @@ static void __init pci_reserve_io(void)
> #ifdef CONFIG_DEBUG_LL
> void __init debug_ll_io_init(void)
> {
> + struct vm_struct *vm;
> + struct static_vm *svm;
> struct map_desc map;
>
> + svm = early_alloc_aligned(sizeof(*svm), __alignof__(*svm));
> +
> debug_ll_addr(&map.pfn, &map.virtual);
> if (!map.pfn || !map.virtual)
> return;
> @@ -948,6 +952,15 @@ void __init debug_ll_io_init(void)
> map.length = PAGE_SIZE;
> map.type = MT_DEVICE;
> create_mapping(&map);
> +
> + vm = &svm->vm;
> + vm->addr = (void *)map.virtual;
> + vm->size = map.length;
> + vm->phys_addr = __pfn_to_phys(map.pfn);
> + vm->flags = VM_IOREMAP | VM_ARM_STATIC_MAPPING;
> + vm->flags |= VM_ARM_MTYPE(map.type);
> + vm->caller = debug_ll_io_init;
> + add_static_vm_early(svm);
Can you use vm_reserve_area_early here or perhaps just call
iotable_init instead of create_mapping directly? I don't recall if
there was some reason I didn't do that.
Rob
On 06/11, Rob Herring wrote:
> On Tue, Jun 11, 2013 at 8:30 PM, Stephen Boyd <[email protected]> wrote:
>
> > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > index e0d8565..04fe160 100644
> > --- a/arch/arm/mm/mmu.c
> > +++ b/arch/arm/mm/mmu.c
> > @@ -938,8 +938,12 @@ static void __init pci_reserve_io(void)
> > #ifdef CONFIG_DEBUG_LL
> > void __init debug_ll_io_init(void)
> > {
> > + struct vm_struct *vm;
> > + struct static_vm *svm;
> > struct map_desc map;
> >
> > + svm = early_alloc_aligned(sizeof(*svm), __alignof__(*svm));
> > +
> > debug_ll_addr(&map.pfn, &map.virtual);
> > if (!map.pfn || !map.virtual)
> > return;
> > @@ -948,6 +952,15 @@ void __init debug_ll_io_init(void)
> > map.length = PAGE_SIZE;
> > map.type = MT_DEVICE;
> > create_mapping(&map);
> > +
> > + vm = &svm->vm;
> > + vm->addr = (void *)map.virtual;
> > + vm->size = map.length;
> > + vm->phys_addr = __pfn_to_phys(map.pfn);
> > + vm->flags = VM_IOREMAP | VM_ARM_STATIC_MAPPING;
> > + vm->flags |= VM_ARM_MTYPE(map.type);
> > + vm->caller = debug_ll_io_init;
> > + add_static_vm_early(svm);
>
> Can you use vm_reserve_area_early here or perhaps just call
> iotable_init instead of create_mapping directly? I don't recall if
> there was some reason I didn't do that.
>
I had iotable_init() before but I thought it was better to have
the ->caller argument say debug_ll_io_init() instead of
iotable_init(). Shall I extract out the similar code?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
On 06/12/2013 12:21 PM, Stephen Boyd wrote:
> On 06/11, Rob Herring wrote:
>> On Tue, Jun 11, 2013 at 8:30 PM, Stephen Boyd <[email protected]> wrote:
>>
>>> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
>>> index e0d8565..04fe160 100644
>>> --- a/arch/arm/mm/mmu.c
>>> +++ b/arch/arm/mm/mmu.c
>>> @@ -938,8 +938,12 @@ static void __init pci_reserve_io(void)
>>> #ifdef CONFIG_DEBUG_LL
>>> void __init debug_ll_io_init(void)
>>> {
>>> + struct vm_struct *vm;
>>> + struct static_vm *svm;
>>> struct map_desc map;
>>>
>>> + svm = early_alloc_aligned(sizeof(*svm), __alignof__(*svm));
>>> +
>>> debug_ll_addr(&map.pfn, &map.virtual);
>>> if (!map.pfn || !map.virtual)
>>> return;
>>> @@ -948,6 +952,15 @@ void __init debug_ll_io_init(void)
>>> map.length = PAGE_SIZE;
>>> map.type = MT_DEVICE;
>>> create_mapping(&map);
>>> +
>>> + vm = &svm->vm;
>>> + vm->addr = (void *)map.virtual;
>>> + vm->size = map.length;
>>> + vm->phys_addr = __pfn_to_phys(map.pfn);
>>> + vm->flags = VM_IOREMAP | VM_ARM_STATIC_MAPPING;
>>> + vm->flags |= VM_ARM_MTYPE(map.type);
>>> + vm->caller = debug_ll_io_init;
>>> + add_static_vm_early(svm);
>>
>> Can you use vm_reserve_area_early here or perhaps just call
>> iotable_init instead of create_mapping directly? I don't recall if
>> there was some reason I didn't do that.
>>
>
> I had iotable_init() before but I thought it was better to have
> the ->caller argument say debug_ll_io_init() instead of
> iotable_init(). Shall I extract out the similar code?
Well, that's always welcome...
Another option would be use __builtin_return_address. That would change
vmallocinfo from showing iotable_init to the caller of iotable_init.
Arguably, knowing the caller would be better.
Rob
On 06/14, Rob Herring wrote:
> On 06/12/2013 12:21 PM, Stephen Boyd wrote:
> > On 06/11, Rob Herring wrote:
> >> On Tue, Jun 11, 2013 at 8:30 PM, Stephen Boyd <[email protected]> wrote:
> >>
> >>> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> >>> index e0d8565..04fe160 100644
> >>> --- a/arch/arm/mm/mmu.c
> >>> +++ b/arch/arm/mm/mmu.c
> >>> @@ -938,8 +938,12 @@ static void __init pci_reserve_io(void)
> >>> #ifdef CONFIG_DEBUG_LL
> >>> void __init debug_ll_io_init(void)
> >>> {
> >>> + struct vm_struct *vm;
> >>> + struct static_vm *svm;
> >>> struct map_desc map;
> >>>
> >>> + svm = early_alloc_aligned(sizeof(*svm), __alignof__(*svm));
> >>> +
> >>> debug_ll_addr(&map.pfn, &map.virtual);
> >>> if (!map.pfn || !map.virtual)
> >>> return;
> >>> @@ -948,6 +952,15 @@ void __init debug_ll_io_init(void)
> >>> map.length = PAGE_SIZE;
> >>> map.type = MT_DEVICE;
> >>> create_mapping(&map);
> >>> +
> >>> + vm = &svm->vm;
> >>> + vm->addr = (void *)map.virtual;
> >>> + vm->size = map.length;
> >>> + vm->phys_addr = __pfn_to_phys(map.pfn);
> >>> + vm->flags = VM_IOREMAP | VM_ARM_STATIC_MAPPING;
> >>> + vm->flags |= VM_ARM_MTYPE(map.type);
> >>> + vm->caller = debug_ll_io_init;
> >>> + add_static_vm_early(svm);
> >>
> >> Can you use vm_reserve_area_early here or perhaps just call
> >> iotable_init instead of create_mapping directly? I don't recall if
> >> there was some reason I didn't do that.
> >>
> >
> > I had iotable_init() before but I thought it was better to have
> > the ->caller argument say debug_ll_io_init() instead of
> > iotable_init(). Shall I extract out the similar code?
>
> Well, that's always welcome...
>
> Another option would be use __builtin_return_address. That would change
> vmallocinfo from showing iotable_init to the caller of iotable_init.
> Arguably, knowing the caller would be better.
>
Ok sounds fair. I'll use iotable_init() and send a follow-up
patch to make iotable_init() more informative.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
On 06/14, Stephen Boyd wrote:
> On 06/14, Rob Herring wrote:
> > On 06/12/2013 12:21 PM, Stephen Boyd wrote:
> > > On 06/11, Rob Herring wrote:
> > >> Can you use vm_reserve_area_early here or perhaps just call
> > >> iotable_init instead of create_mapping directly? I don't recall if
> > >> there was some reason I didn't do that.
> > >>
> > >
> > > I had iotable_init() before but I thought it was better to have
> > > the ->caller argument say debug_ll_io_init() instead of
> > > iotable_init(). Shall I extract out the similar code?
> >
> > Well, that's always welcome...
> >
> > Another option would be use __builtin_return_address. That would change
> > vmallocinfo from showing iotable_init to the caller of iotable_init.
> > Arguably, knowing the caller would be better.
> >
>
> Ok sounds fair. I'll use iotable_init() and send a follow-up
> patch to make iotable_init() more informative.
>
Well now I get paging_init() as the function because that's where
we're going to return to. That actually seems less informative.
I'll do the similar code extraction.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation