From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1194F1DA24 for ; Wed, 29 Nov 2023 16:53:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="FqoWy209" Received: by smtp.kernel.org (Postfix) with ESMTPSA id AE4CFC433C9; Wed, 29 Nov 2023 16:53:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1701276787; bh=Wq314BYKXeWO/ild5QLd+lHLiRCms0SQlJmL6wGPBJM=; h=In-Reply-To:References:Date:From:To:Cc:Subject:From; b=FqoWy209TmDsyBgUFiqxVEPB2XWsdPHtmG/sotJQe/IEJYujb16IlSBJwZ0Qx0RTl 2MEGxlyw6xauYSO6C2cGaz8RJUd7WGiAX4s/qsJ8IbzvJGMi26k+/mTXTnyRbTIfUO QdPF337YzmTGdpJusTFQU3xDx3tLXYhdgvTuZ3y/0Vt2bqKvQVSCqFdLl4LltUfQTO T9cvwSWpKunFz4I5Jpvsn6QK4ARQx6hLxYgzYwX0YJDGqKHi5GMrkTTkrz94//uH4N IZM7mOniapsS/ULcIZOR7rvpEfhlQ1WKJnWOQK64DcsvO8pWxj3FZMmkN9ajUUShrQ e3CxBs0L5yyvg== Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailauth.nyi.internal (Postfix) with ESMTP id 8A00227C0054; Wed, 29 Nov 2023 11:53:05 -0500 (EST) Received: from imap51 ([10.202.2.101]) by compute5.internal (MEProxy); Wed, 29 Nov 2023 11:53:05 -0500 X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeihedgledvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepofgfggfkjghffffhvfevufgtgfesthhqredtreerjeenucfhrhhomhepfdet rhhnugcuuegvrhhgmhgrnhhnfdcuoegrrhhnugeskhgvrhhnvghlrdhorhhgqeenucggtf frrghtthgvrhhnpedvtddtffejfeeggefgleefgfeghfehfeefffetgffgleegudevveet hfefjeevkeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhroh hmpegrrhhnugdomhgvshhmthhprghuthhhphgvrhhsohhnrghlihhthidquddvkeehudej tddvgedqvdekjedttddvieegqdgrrhhnugeppehkvghrnhgvlhdrohhrghesrghrnhgusg druggv X-ME-Proxy: Feedback-ID: i36794607:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id A96E8B6008D; Wed, 29 Nov 2023 11:53:04 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.9.0-alpha0-1234-gac66594aae-fm-20231122.001-gac66594a Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: In-Reply-To: <10ef8eac12c01f52ec3b8c0bedf6cf34627d1ceb.camel@redhat.com> References: <20231120215945.52027-2-pstanner@redhat.com> <20231120215945.52027-6-pstanner@redhat.com> <10ef8eac12c01f52ec3b8c0bedf6cf34627d1ceb.camel@redhat.com> Date: Wed, 29 Nov 2023 17:52:43 +0100 From: "Arnd Bergmann" To: "Philipp Stanner" , "Bjorn Helgaas" , "Andrew Morton" , "Randy Dunlap" , "Jason Gunthorpe" , "Eric Auger" , "Kent Overstreet" , "Niklas Schnelle" , "Neil Brown" , "John Sanpe" , "Dave Jiang" , "Yury Norov" , "Kees Cook" , "Masami Hiramatsu" , "David Gow" , "Thomas Gleixner" , "wuqiang.matt" , "Jason Baron" , "Ben Dooks" , "Danilo Krummrich" Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org Subject: Re: [PATCH 4/4] lib/iomap.c: improve comment about pci anomaly Content-Type: text/plain;charset=utf-8 Content-Transfer-Encoding: quoted-printable On Wed, Nov 29, 2023, at 13:40, Philipp Stanner wrote: > On Tue, 2023-11-21 at 11:03 +0100, Arnd Bergmann wrote: >> On Mon, Nov 20, 2023, at 22:59, Philipp Stanner wrote: >> We should be able to define a generic version like >>=20 >> void pci_iounmap(struct pci_dev *dev, void __iomem * addr) >> { >> #ifdef CONFIG_HAS_IOPORT >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (iomem_is_ioport(addr)) { >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 ioport_unmap(addr); >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 return; >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >> #endif >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iounmap(addr) >> } > > ACK, I think this makes sense =E2=80=93 if we agree (as in the other t= hread) > that we never need an empty pci_iounmap(). > >>=20 >> and then define iomem_is_ioport() in lib/iomap.c for x86, > > Wait a minute. > lib/ should never contain architecture-specific code, should it? I > assume your argument is that we write a default version of > iomem_is_ioport(), that could, in theory, be used by many > architectures, but ultimately only x86 will use that default. I would hope that eventually we can build lib/iomap.c only on x86, as everything else can live without it. >> while defining it in asm-generic/io.h for the rest, > > So we're not talking about the function prototypes here, but about the > actual implementation that should reside in asm-generic/io.h, aye? > Because the prototype obviously always has to be identical. It could live in lib/pci_iomap.c or in include/asm-generic/pci_iomap.h, it doesn't really matter since the definition is trivial. asm-generic/io.h is probably not the right place, unless we want to merge all of asm-generic/pci_iomap.h into asm-generic/io.h. We could do that now that all architectures include asm-generic/io.h and that includes asm-generic/pci_iomap.h unconditionally. >> with an override in asm/io.h for those architectures >> that need a custom inb(). > > So like this in ARCH/include/asm/io.h: > > #define iomem_is_ioport iomem_is_ioport > bool iomem_is_ioport(...); > > and in include/asm-generic/io.h: > > #ifndef iomem_is_ioport > bool iomem_is_ioport(...); > > correct? Yes. =20 >> arch/powerpc/platforms/Kconfig: select GENERIC_IOMAP >>=20 >> This is only used for two platforms: cell and powernv, >> though on Cell it no longer does anything after the >> commit f4981a00636 ("powerpc: Remove the celleb support"); >> I think the entire io_workarounds code now be folded >> back into spider_pci.c if we wanted to. >>=20 >> The PowerNV LPC support does seem to still rely on it. >> This tries to do the exact same thing as lib/logic_pio.c >> for Huawei arm64 servers. I suspect that neither of them >> does it entirely correctly since the powerpc side appears >> to just override any non-LPC PIO support while the arm64 >> side is missing the ioread/iowrite support. > > I think by now I get what the issue with GENERIC_IOMAP is. But do you > want me to do something about GENERIC_IOMAP (besides the things > directly related to the PCI functionality I'm touching) for you to > approve of a modified version of this patch series? It would be nice to clean up some of the architectures that incorrectly select it at the moment, but that can be a separate series if you want to get this one done first, or I can take a look myself. Arnd