From patchwork Thu Jan 11 08:55:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philipp Stanner X-Patchwork-Id: 13516969 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (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 7C767FC01 for ; Thu, 11 Jan 2024 08:56:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="YRb7sIXH" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1704963364; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=uRUU62WWJzjJ7HLfvfm0qPbImNGXrVHE/GZX0ITXdyI=; b=YRb7sIXHa/BWFvXd3pu4HrE26kH+eZBeS+j5tJsKU/apIeYjg3TXzro2T5V6T5cPpDo5+Z oxMyvI/ipDwmwyjCQJnP6ncQ0NG4bMOQ+BTo0aeA67fAsMDZnlHwUsPzPgkLao110zM61k WvUkRl8UshiOcUz3DG9lHoqanZE00Hs= Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-357-asTFc5GkNQmRdgZ9mH6p4w-1; Thu, 11 Jan 2024 03:56:03 -0500 X-MC-Unique: asTFc5GkNQmRdgZ9mH6p4w-1 Received: by mail-qv1-f71.google.com with SMTP id 6a1803df08f44-680fce72f68so12524186d6.0 for ; Thu, 11 Jan 2024 00:56:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704963362; x=1705568162; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=uRUU62WWJzjJ7HLfvfm0qPbImNGXrVHE/GZX0ITXdyI=; b=JCH9rpyH3ezwYkAkY/P2ezWT+QWVK0a4EMffpynLhTEo3xQXtbmx/JRlYElEfTXwm5 LoVn5MsTY1cSBtVyVI/4ewFWSZw+Ksy4iYS67zTMWxF7rxmkA3eF26+bkCR+3sSEe7Fo CqHEXaf/APsynr//CJX/wHoI8vTJtnMwmZ41JACiUprJYBomfERrwhkDLT2DU2fRG4z9 Pli6YdZNozBK679pVFxKUGHT60KXPmma/B0i0yiKL94Kmz8nKgSP9Kbdj2PRvfmh1tge OuNMjmdRihNS2+0abwKQ0Wt1sMlI3AQ+upFgw2yKpMedsny4cRNELXFCCmmoYrQgHoL1 Ckfw== X-Gm-Message-State: AOJu0YyCkIVgVNFt5N4+GE0iogdwjCwqettka+WOEaLcobwhAubfWXFR KDfIuzhc4fQ2L5vrejuvXNtJtw4yoL3WK98jb8gUeD5qHmZ3SfXMiyNKnoKV3OJBbTeVm8xY1eB oWJSkzI1qzVOk1swLCrCqa1cNV5FV X-Received: by 2002:ad4:4ee5:0:b0:67f:4c05:4d34 with SMTP id dv5-20020ad44ee5000000b0067f4c054d34mr1504507qvb.5.1704963362527; Thu, 11 Jan 2024 00:56:02 -0800 (PST) X-Google-Smtp-Source: AGHT+IFyuZ8PglreiRvC9wa34DuAIGeLOWyX02+l1+qB0JRpRRAKI9YlRwxbi7HtKifNxwMidOC/Ew== X-Received: by 2002:ad4:4ee5:0:b0:67f:4c05:4d34 with SMTP id dv5-20020ad44ee5000000b0067f4c054d34mr1504485qvb.5.1704963362063; Thu, 11 Jan 2024 00:56:02 -0800 (PST) Received: from pstanner-thinkpadt14sgen1.remote.csb (nat-pool-muc-t.redhat.com. [149.14.88.26]) by smtp.gmail.com with ESMTPSA id e16-20020a0cd650000000b0067f7d131256sm168341qvj.17.2024.01.11.00.55.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Jan 2024 00:56:01 -0800 (PST) From: Philipp Stanner To: Bjorn Helgaas , Arnd Bergmann , Johannes Berg , Randy Dunlap , NeilBrown , John Sanpe , Kent Overstreet , Niklas Schnelle , Philipp Stanner , Dave Jiang , Uladzislau Koshchanka , "Masami Hiramatsu (Google)" , David Gow , Kees Cook , Rae Moar , Geert Uytterhoeven , "wuqiang.matt" , Yury Norov , Jason Baron , Thomas Gleixner , Marco Elver , Andrew Morton , Ben Dooks , dakr@redhat.com Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, linux-arch@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH v5 RESEND 0/5] Regather scattered PCI-Code Date: Thu, 11 Jan 2024 09:55:35 +0100 Message-ID: <20240111085540.7740-1-pstanner@redhat.com> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Second Resend. Would be cool if someone could tell me what I'll have to do so we can get this merged. This is blocking the followup work I've got in the pipe --- @Stable-Kernel: You receive this patch series because its first patch fixes leaks in PCI. Changes in v5: - Add forgotten update to MAINTAINERS file. Changes in v4: - Apply Arnd's Reviewed-by's - Add ifdef CONFIG_HAS_IOPORT_MAP guard in drivers/pci/iomap.c (build error on openrisc) - Fix typo in patch no.5 Changes in v3: - Create a separate patch for the leaks in lib/iomap.c. Make it the series' first patch. (Arnd) - Turns out the aforementioned bug wasn't just accidentally removing iounmap() with the ifdef, it was also missing ioport_unmap() to begin with. Add it. - Move the ARCH_WANTS_GENERIC_IOMEM_IS_IOPORT-mechanism from asm-generic/io.h to asm-generic/ioport.h. (Arnd) - Adjust the implementation of iomem_is_ioport() in asm-generic/io.h so that it matches exactly what pci_iounmap() previously did in lib/pci_iomap.c. (Arnd) - Move the CONFIG_HAS_IOPORT guard in asm-generic/io.h so that iomem_is_ioport() will always be compiled and just returns false if there are no ports. - Add TODOs to several places informing about the generic iomem_is_ioport() in lib/iomap.c not being generic. - Add TODO about the followup work to make drivers/pci/iomap.c's pci_iounmap() actually generic. Changes in v2: - Replace patch 4, previously extending the comment about pci_iounmap() in lib/iomap.c, with a patch that moves pci_iounmap() from that file to drivers/pci/iomap.c, creating a unified version there. (Arnd) - Implement iomem_is_ioport() as a new helper in asm-generic/io.h and lib/iomap.c. (Arnd) - Move the build rule in drivers/pci/Makefile for iomap.o under the guard of #if PCI. This had to be done because when just checking for GENERIC_PCI_IOMAP being defined, the functions don't disappear, which was the case previously in lib/pci_iomap.c, where the entire file was made empty if PCI was not set by the guard #ifdef PCI. (Intel's Bots) - Rephares all patches' commit messages a little bit. Sooooooooo. I reworked v1. Please review this carefully, the IO-Ranges are obviously a bit tricky, as is the build-system / ifdef-ery. Arnd has suggested that architectures defining a custom inb() need their own iomem_is_ioport(), as well. I've grepped for inb() and found the following list of archs that define their own: - alpha - arm - m68k <-- - parisc - powerpc - sh - sparc - x86 <-- All of those have their own definitons of pci_iounmap(). Therefore, they don't need our generic version in the first place and, thus, also need no iomem_is_ioport(). The two exceptions are x86 and m68k. The former uses lib/iomap.c through CONFIG_GENERIC_IOMAP, as Arnd pointed out in the previous discussion (thus, CONFIG_GENERIC_IOMAP is not really generic in this regard). So as I see it, only m68k WOULD need its own custom definition of iomem_is_ioport(). But as I understand it it doesn't because it uses the one from asm-generic/pci_iomap.h ?? I wasn't entirely sure how to deal with the address ranges for the generic implementation in asm-generic/io.h. It's marked with a TODO. Input appreciated. I removed the guard around define pci_iounmap in asm-generic/io.h. An alternative would be to have it be guarded by CONFIG_GENERIC_IOMAP and CONFIG_GENERIC_PCI_IOMAP, both. Without such a guard, there is no collision however, because generic pci_iounmap() from drivers/pci/iomap.c will only get pulled in when CONFIG_GENERIC_PCI_IOMAP is actually set. I cross-built this for a variety of architectures, including the usual suspects (s390, m68k). So far successfully. But let's see what Intel's robots say :O P. Original cover letter: Hi! So it seems that since ca. 2007 the PCI code has been scattered a bit. PCI's devres code, which is only ever used by users of the entire PCI-subsystem anyways, resides in lib/devres.c and is guarded by an ifdef PCI, just as the content of lib/pci_iomap.c is. It, thus, seems reasonable to move all of that. As I were at it, I moved as much of the devres-specific code from pci.c to devres.c, too. The only exceptions are four functions that are currently difficult to move. More information about that can be read here [1]. I noticed these scattered files while working on (new) PCI-specific devres functions. If we can get this here merged, I'll soon send another patch series that addresses some API-inconsistencies and could move the devres-part of the four remaining functions. I don't want to do that in this series as this here is only about moving code, whereas the next series would have to actually change API behavior. I successfully (cross-)built this for x86, x86_64, AARCH64 and ARM (allyesconfig). I booted a kernel with it on x86_64, with a Fedora desktop environment as payload. The OS came up fine I hope this is OK. If we can get it in, we'd soon have a very consistent PCI API again. Regards, P. Philipp Stanner (5): lib/pci_iomap.c: fix cleanup bugs in pci_iounmap() lib: move pci_iomap.c to drivers/pci/ lib: move pci-specific devres code to drivers/pci/ pci: move devres code from pci.c to devres.c lib, pci: unify generic pci_iounmap() MAINTAINERS | 1 - drivers/pci/Kconfig | 5 + drivers/pci/Makefile | 3 +- drivers/pci/devres.c | 450 +++++++++++++++++++++++++ lib/pci_iomap.c => drivers/pci/iomap.c | 49 +-- drivers/pci/pci.c | 249 -------------- drivers/pci/pci.h | 24 ++ include/asm-generic/io.h | 27 +- include/asm-generic/iomap.h | 21 ++ lib/Kconfig | 3 - lib/Makefile | 1 - lib/devres.c | 208 +----------- lib/iomap.c | 28 +- 13 files changed, 566 insertions(+), 503 deletions(-) create mode 100644 drivers/pci/devres.c rename lib/pci_iomap.c => drivers/pci/iomap.c (75%)