Re: ioremap() called early from pnv_pci_init_ioda_phb()

[Oliver O'Halloran]

On Sun, May 10, 2020 at 1:51 AM Christophe Leroy
<christophe.leroy@xxxxxxxxxx> wrote:
>
>
>
> Le 08/05/2020 Ã 19:41, Qian Cai a Ãcrit :
> >
> >
> >> On May 8, 2020, at 10:39 AM, Qian Cai <cai@xxxxxx> 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?
> >
> > d538aadc2718 (âpowerpc/ioremap: warn on early use of ioremap()")
>
> No d538aadc2718 is not bogus. That's the point, we want to remove all
> early usages of ioremap() in order to remove the hack with the
> ioremap_bot stuff and all, and stick to the generic ioremap logic.
>
> In order to do so, all early use of ioremap() has to be converted to
> early_ioremap() or to fixmap or anything else that allows to do ioremaps
> before the slab is ready.
>
> early_ioremap() is for temporary mappings necessary at boottime. For
> long lasting mappings, another method is to be used.
>
> Now, the point is that other architectures like for instance x86 don't
> seem to have to use early_ioremap() much. Powerpc is for instance doing
> early mappings for PCI. Seems like x86 initialises PCI once slab is
> ready. Can't powerpc do the same ?

Patches welcome.