Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <B183CDAA-DA88-4760-9C1B-F73A8F7840E7@lca.pw> <229E1896-0C06-418A-B7DE-40AEBFB44F85@lca.pw>
 <a4ae5c50-f317-4224-a5f2-6e1030e62d2b@csgroup.eu>
In-Reply-To: <a4ae5c50-f317-4224-a5f2-6e1030e62d2b@csgroup.eu>
From:   "Oliver O'Halloran" <oohall@gmail.com>
Date:   Sun, 10 May 2020 08:59:27 +1000
Message-ID: <CAOSf1CEe85Lev54g+hkEoDmmR1HRsF070oxLu8ok7fEtXUH7NQ@mail.gmail.com>
Subject: Re: ioremap() called early from pnv_pci_init_ioda_phb()
To:     Christophe Leroy <christophe.leroy@csgroup.eu>
Cc:     Qian Cai <cai@lca.pw>, Christophe Leroy <christophe.leroy@c-s.fr>,
        Michael Ellerman <mpe@ellerman.id.au>,
        linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sun, May 10, 2020 at 1:51 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 08/05/2020 =C3=A0 19:41, Qian Cai a =C3=A9crit :
> >
> >
> >> 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 ea=
rly_ioremap() instead=E2=80=9D
> >>
> >> but use the patch below will result in leaks because it will never cal=
l 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 stron=
g?
> >>
> >> --- 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 =3D r.start;
> >> -               phb->regs =3D ioremap(r.start, resource_size(&r));
> >> +               phb->regs =3D early_ioremap(r.start, resource_size(&r)=
);
> >>                 if (phb->regs =3D=3D NULL)
> >>                         pr_err("  Failed to map registers !\n=E2=80=9D=
);
> >
> > This will also trigger a panic with debugfs reads, so isn=E2=80=99t tha=
t this commit bogus at least for powerpc64?
> >
> > d538aadc2718 (=E2=80=9Cpowerpc/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.