All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Qian Cai <cai@lca.pw>, Christophe Leroy <christophe.leroy@c-s.fr>,
	Michael Ellerman <mpe@ellerman.id.au>
Cc: LKML <linux-kernel@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: ioremap() called early from pnv_pci_init_ioda_phb()
Date: Sat, 09 May 2020 18:38:36 +1000	[thread overview]
Message-ID: <1589013450.02gfmpktnp.astroid@bobo.none> (raw)
In-Reply-To: <229E1896-0C06-418A-B7DE-40AEBFB44F85@lca.pw>

Excerpts from Qian Cai's message of May 9, 2020 3:41 am:
> 
> 
>> On May 8, 2020, at 10:39 AM, Qian Cai <cai@lca.pw> wrote:
>> 
>> Booting POWER9 PowerNV has this message,
>> 
>> "ioremap() called early from pnv_pci_init_ioda_phb+0x420/0xdfc. Use early_ioremap() instead”
>> 
>> but use the patch below will result in leaks because it will never call early_iounmap() anywhere. However, it looks me it was by design that phb->regs mapping would be there forever where it would be used in pnv_ioda_get_inval_reg(), so is just that check_early_ioremap_leak() initcall too strong?
>> 
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -36,6 +36,7 @@
>> #include <asm/firmware.h>
>> #include <asm/pnv-pci.h>
>> #include <asm/mmzone.h>
>> +#include <asm/early_ioremap.h>
>> 
>> #include <misc/cxl-base.h>
>> 
>> @@ -3827,7 +3828,7 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
>>        /* Get registers */
>>        if (!of_address_to_resource(np, 0, &r)) {
>>                phb->regs_phys = r.start;
>> -               phb->regs = ioremap(r.start, resource_size(&r));
>> +               phb->regs = early_ioremap(r.start, resource_size(&r));
>>                if (phb->regs == NULL)
>>                        pr_err("  Failed to map registers !\n”);
> 
> This will also trigger a panic with debugfs reads, so isn’t that this commit bogus at least for powerpc64?

Your patch to use early_ioremap is faulting? I wonder why?

Thanks,
Nick

> 
> d538aadc2718 (“powerpc/ioremap: warn on early use of ioremap()")
> 
> 11017.617022][T122068] Faulting instruction address: 0xc0000000000db564
> [11017.617257][T122066] Faulting instruction address: 0xc0000000000db564
> [11017.617950][T122073] Faulting instruction address: 0xc0000000000db564
> [11017.618888][T122064] BUG: Unable to handle kernel data access on read at 0xffffffffffe20e10
> [11017.618935][T122064] Faulting instruction address: 0xc0000000000db564
> [11017.737996][T122072] 
> [11017.738010][T122073] Oops: Kernel access of bad area, sig: 11 [#2]
> [11017.738024][T122073] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
> [11017.738051][T122073] Modules linked in: brd ext4 crc16 mbcache jbd2 loop kvm_hv kvm ip_tables x_tables xfs sd_mod bnx2x ahci libahci tg3 mdio libata libphy firmware_class dm_mirror dm_region_hash dm_log dm_mod
> [11017.738110][T122073] CPU: 108 PID: 122073 Comm: read_all Tainted: G      D W         5.7.0-rc4-next-20200508+ #4
> [11017.738138][T122073] NIP:  c0000000000db564 LR: c00000000056f660 CTR: c0000000000db550
> [11017.738173][T122073] REGS: c000000374f6f980 TRAP: 0380   Tainted: G      D W          (5.7.0-rc4-next-20200508+)
> [11017.738234][T122073] MSR:  9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 22002282  XER: 20040000
> [11017.738278][T122073] CFAR: c00000000056f65c IRQMASK: 0 
> [11017.738278][T122073] GPR00: c00000000056f660 c000000374f6fc10 c000000001689400 c000201ffc41aa00 
> [11017.738278][T122073] GPR04: c000000374f6fc70 0000000000000000 0000000000000000 0000000000000001 
> [11017.738278][T122073] GPR08: 0000000000000000 ffffffffffe20000 0000000000000000 c0000008ee380080 
> [11017.738278][T122073] GPR12: c0000000000db550 c000201fff671280 0000000000000000 0000000000000000 
> [11017.738278][T122073] GPR16: 0000000000000002 0000000010040800 000000001001ccd8 000000001001cc80 
> [11017.738278][T122073] GPR20: 000000001001cc98 000000001001ccc8 000000001001cca8 000000001001cb48 
> [11017.738278][T122073] GPR24: 0000000000000000 0000000000000000 00000000000003ff 00007fffebb67390 
> [11017.738278][T122073] GPR28: c000000374f6fd90 c000200c0c6a7550 0000000000000000 c000200c0c6a7500 
> [11017.738542][T122073] NIP [c0000000000db564] pnv_eeh_dbgfs_get_inbB+0x14/0x30
> [11017.738579][T122073] LR [c00000000056f660] simple_attr_read+0xa0/0x180
> [11017.738613][T122073] Call Trace:
> [11017.738645][T122073] [c000000374f6fc10] [c00000000056f630] simple_attr_read+0x70/0x180 (unreliable)
> [11017.738672][T122073] [c000000374f6fcb0] [c00000000064a2e0] full_proxy_read+0x90/0xe0
> [11017.738686][T122073] [c000000374f6fd00] [c00000000051fe0c] __vfs_read+0x3c/0x70
> [11017.738722][T122073] [c000000374f6fd20] [c00000000051feec] vfs_read+0xac/0x170
> [11017.738757][T122073] [c000000374f6fd70] [c00000000052034c] ksys_read+0x7c/0x140
> [11017.738818][T122073] [c000000374f6fdc0] [c000000000038af4] system_call_exception+0x114/0x1e0
> [11017.738867][T122073] [c000000374f6fe20] [c00000000000c8f0] system_call_common+0xf0/0x278
> [11017.738916][T122073] Instruction dump:
> [11017.738948][T122073] 7c0004ac f9490d10 a14d0c78 38600000 b14d0c7a 4e800020 60000000 7c0802a6 
> [11017.739001][T122073] 60000000 e9230278 e9290028 7c0004ac <e9290e10> 0c090000 4c00012c 38600000 

WARNING: multiple messages have this Message-ID (diff)
From: Nicholas Piggin <npiggin@gmail.com>
To: Qian Cai <cai@lca.pw>, Christophe Leroy <christophe.leroy@c-s.fr>,
	Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: ioremap() called early from pnv_pci_init_ioda_phb()
Date: Sat, 09 May 2020 18:38:36 +1000	[thread overview]
Message-ID: <1589013450.02gfmpktnp.astroid@bobo.none> (raw)
In-Reply-To: <229E1896-0C06-418A-B7DE-40AEBFB44F85@lca.pw>

Excerpts from Qian Cai's message of May 9, 2020 3:41 am:
> 
> 
>> On May 8, 2020, at 10:39 AM, Qian Cai <cai@lca.pw> wrote:
>> 
>> Booting POWER9 PowerNV has this message,
>> 
>> "ioremap() called early from pnv_pci_init_ioda_phb+0x420/0xdfc. Use early_ioremap() instead”
>> 
>> but use the patch below will result in leaks because it will never call early_iounmap() anywhere. However, it looks me it was by design that phb->regs mapping would be there forever where it would be used in pnv_ioda_get_inval_reg(), so is just that check_early_ioremap_leak() initcall too strong?
>> 
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -36,6 +36,7 @@
>> #include <asm/firmware.h>
>> #include <asm/pnv-pci.h>
>> #include <asm/mmzone.h>
>> +#include <asm/early_ioremap.h>
>> 
>> #include <misc/cxl-base.h>
>> 
>> @@ -3827,7 +3828,7 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
>>        /* Get registers */
>>        if (!of_address_to_resource(np, 0, &r)) {
>>                phb->regs_phys = r.start;
>> -               phb->regs = ioremap(r.start, resource_size(&r));
>> +               phb->regs = early_ioremap(r.start, resource_size(&r));
>>                if (phb->regs == NULL)
>>                        pr_err("  Failed to map registers !\n”);
> 
> This will also trigger a panic with debugfs reads, so isn’t that this commit bogus at least for powerpc64?

Your patch to use early_ioremap is faulting? I wonder why?

Thanks,
Nick

> 
> d538aadc2718 (“powerpc/ioremap: warn on early use of ioremap()")
> 
> 11017.617022][T122068] Faulting instruction address: 0xc0000000000db564
> [11017.617257][T122066] Faulting instruction address: 0xc0000000000db564
> [11017.617950][T122073] Faulting instruction address: 0xc0000000000db564
> [11017.618888][T122064] BUG: Unable to handle kernel data access on read at 0xffffffffffe20e10
> [11017.618935][T122064] Faulting instruction address: 0xc0000000000db564
> [11017.737996][T122072] 
> [11017.738010][T122073] Oops: Kernel access of bad area, sig: 11 [#2]
> [11017.738024][T122073] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
> [11017.738051][T122073] Modules linked in: brd ext4 crc16 mbcache jbd2 loop kvm_hv kvm ip_tables x_tables xfs sd_mod bnx2x ahci libahci tg3 mdio libata libphy firmware_class dm_mirror dm_region_hash dm_log dm_mod
> [11017.738110][T122073] CPU: 108 PID: 122073 Comm: read_all Tainted: G      D W         5.7.0-rc4-next-20200508+ #4
> [11017.738138][T122073] NIP:  c0000000000db564 LR: c00000000056f660 CTR: c0000000000db550
> [11017.738173][T122073] REGS: c000000374f6f980 TRAP: 0380   Tainted: G      D W          (5.7.0-rc4-next-20200508+)
> [11017.738234][T122073] MSR:  9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 22002282  XER: 20040000
> [11017.738278][T122073] CFAR: c00000000056f65c IRQMASK: 0 
> [11017.738278][T122073] GPR00: c00000000056f660 c000000374f6fc10 c000000001689400 c000201ffc41aa00 
> [11017.738278][T122073] GPR04: c000000374f6fc70 0000000000000000 0000000000000000 0000000000000001 
> [11017.738278][T122073] GPR08: 0000000000000000 ffffffffffe20000 0000000000000000 c0000008ee380080 
> [11017.738278][T122073] GPR12: c0000000000db550 c000201fff671280 0000000000000000 0000000000000000 
> [11017.738278][T122073] GPR16: 0000000000000002 0000000010040800 000000001001ccd8 000000001001cc80 
> [11017.738278][T122073] GPR20: 000000001001cc98 000000001001ccc8 000000001001cca8 000000001001cb48 
> [11017.738278][T122073] GPR24: 0000000000000000 0000000000000000 00000000000003ff 00007fffebb67390 
> [11017.738278][T122073] GPR28: c000000374f6fd90 c000200c0c6a7550 0000000000000000 c000200c0c6a7500 
> [11017.738542][T122073] NIP [c0000000000db564] pnv_eeh_dbgfs_get_inbB+0x14/0x30
> [11017.738579][T122073] LR [c00000000056f660] simple_attr_read+0xa0/0x180
> [11017.738613][T122073] Call Trace:
> [11017.738645][T122073] [c000000374f6fc10] [c00000000056f630] simple_attr_read+0x70/0x180 (unreliable)
> [11017.738672][T122073] [c000000374f6fcb0] [c00000000064a2e0] full_proxy_read+0x90/0xe0
> [11017.738686][T122073] [c000000374f6fd00] [c00000000051fe0c] __vfs_read+0x3c/0x70
> [11017.738722][T122073] [c000000374f6fd20] [c00000000051feec] vfs_read+0xac/0x170
> [11017.738757][T122073] [c000000374f6fd70] [c00000000052034c] ksys_read+0x7c/0x140
> [11017.738818][T122073] [c000000374f6fdc0] [c000000000038af4] system_call_exception+0x114/0x1e0
> [11017.738867][T122073] [c000000374f6fe20] [c00000000000c8f0] system_call_common+0xf0/0x278
> [11017.738916][T122073] Instruction dump:
> [11017.738948][T122073] 7c0004ac f9490d10 a14d0c78 38600000 b14d0c7a 4e800020 60000000 7c0802a6 
> [11017.739001][T122073] 60000000 e9230278 e9290028 7c0004ac <e9290e10> 0c090000 4c00012c 38600000 

  reply	other threads:[~2020-05-09  8:38 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-08 14:39 ioremap() called early from pnv_pci_init_ioda_phb() Qian Cai
2020-05-08 17:41 ` Qian Cai
2020-05-09  8:38   ` Nicholas Piggin [this message]
2020-05-09  8:38     ` Nicholas Piggin
2020-05-09 15:37     ` Qian Cai
2020-05-09 15:37       ` Qian Cai
2020-05-09 15:49   ` Christophe Leroy
2020-05-09 22:59     ` Oliver O'Halloran
2020-05-09 22:59       ` Oliver O'Halloran
2020-05-09  8:11 ` Oliver O'Halloran
2020-05-09  8:11   ` Oliver O'Halloran
2020-05-09  8:43   ` Nicholas Piggin
2020-05-09  8:43     ` Nicholas Piggin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1589013450.02gfmpktnp.astroid@bobo.none \
    --to=npiggin@gmail.com \
    --cc=cai@lca.pw \
    --cc=christophe.leroy@c-s.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.