> -----Original Message----- > From: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> > Sent: Thursday, September 17, 2020 3:00 PM > To: Catalin Marinas <catalin.marinas@xxxxxxx> > Cc: linux-kernel@xxxxxxxxxxxxxxx; George Cherian <gcherian@xxxxxxxxxxx>; > Arnd Bergmann <arnd@xxxxxxxx>; Will Deacon <will@xxxxxxxxxx>; Bjorn > Helgaas <bhelgaas@xxxxxxxxxx>; Yang Yingliang > <yangyingliang@xxxxxxxxxx>; linux-pci@xxxxxxxxxxxxxxx; linux- > arch@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; David S. Miller > <davem@xxxxxxxxxxxxx> > Subject: Re: [PATCH v2 3/3] asm-generic/io.h: Fix > !CONFIG_GENERIC_IOMAP pci_iounmap() implementation > > > ---------------------------------------------------------------------- > On Wed, Sep 16, 2020 at 03:51:11PM +0100, Catalin Marinas wrote: > > On Wed, Sep 16, 2020 at 12:06:58PM +0100, Lorenzo Pieralisi wrote: > > > For arches that do not select CONFIG_GENERIC_IOMAP, the current > > > pci_iounmap() function does nothing causing obvious memory leaks for > > > mapped regions that are backed by MMIO physical space. > > > > > > In order to detect if a mapped pointer is IO vs MMIO, a check must > > > made available to the pci_iounmap() function so that it can actually > > > detect whether the pointer has to be unmapped. > > > > > > In configurations where CONFIG_HAS_IOPORT_MAP && > > > !CONFIG_GENERIC_IOMAP, a mapped port is detected using an > > > ioport_map() stub defined in asm-generic/io.h. > > > > > > Use the same logic to implement a stub (ie __pci_ioport_unmap()) > > > that detects if the passed in pointer in pci_iounmap() is IO vs MMIO > > > to iounmap conditionally and call it in pci_iounmap() fixing the issue. > > > > > > Leave __pci_ioport_unmap() as a NOP for all other config options. > > > > > > Reported-by: George Cherian <george.cherian@xxxxxxxxxxx> > > > Link: > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org > > > _lkml_20200905024811.74701-2D1-2Dyangyingliang- > 40huawei.com&d=DwIBAg > > > > &c=nKjWec2b6R0mOyPaz7xtfQ&r=TjMsEFPc7dirkF6u2D3eSIS0cA8FeYpzRkk > Mzr4a > > > Cbk&m=UO5qU5LtNtCn6_gnT0rCkBxIm-w8jCaxHO6v7oK-U- > I&s=CSGHQpKoVdNiqb1e > > > DFuRUhka_Xv5o2PosWZ1rR8oOD4&e= > > > Link: > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org > > > _lkml_20200824132046.3114383-2D1-2Dgeorge.cherian- > 40marvell.com&d=Dw > > > > IBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=TjMsEFPc7dirkF6u2D3eSIS0cA8FeYpz > RkkM > > > zr4aCbk&m=UO5qU5LtNtCn6_gnT0rCkBxIm-w8jCaxHO6v7oK-U- > I&s=3B83oan7i1g3 > > > KaPgQmFK6PudR9GzvAPk33Z5Yyv-CMI&e= > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> > > > Cc: Arnd Bergmann <arnd@xxxxxxxx> > > > Cc: George Cherian <george.cherian@xxxxxxxxxxx> > > > Cc: Will Deacon <will@xxxxxxxxxx> > > > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > > Cc: Catalin Marinas <catalin.marinas@xxxxxxx> > > > Cc: Yang Yingliang <yangyingliang@xxxxxxxxxx> > > > --- > > > include/asm-generic/io.h | 39 > > > +++++++++++++++++++++++++++------------ > > > 1 file changed, 27 insertions(+), 12 deletions(-) > > > > This works for me. The only question I have is whether pci_iomap.h is > > better than io.h for __pci_ioport_unmap(). These headers are really > > confusing. > > Yes they are, in total honesty there is much more to do to make them sane, > this patch is just a band-aid. > > I thought about moving this stuff into pci_iomap.h, though that file is > included _independently_ from io.h from some arches so I tried to keep > everything in io.h to minimize disruption. > > We can merge this patch - since it is a fix after all - and then I can try to > improve the whole pci_iounmap() includes. > > > Either way: > > > > Reviewed-by: Catalin Marinas <catalin.marinas@xxxxxxx> > > Thanks a lot. I'd appreciate a tested-by from the George as he is the one who > reported the problem. Verified this patch and it works as expected. Tested-by: George Cherian <george.cherian@xxxxxxxxxxx> > Lorenzo
