Hello Kernel Hackers,
Some platforms such as s390 do not support legacy PCI devices nor PCI I/O
spaces. On such platforms I/O space accessors like inb()/outb() are merely
stubs that can never actually work. The way these stubs are implemented in
asm-generic/io.h leads to compiler warnings because any use will
essentially lead to a NULL pointer access. In a previous patch we tried
handling this case by generating a run-time warning on access. This
approach however was rejected by Linus in tha mail below with the argument
that this really should be a compile-time check and, though a much more
invasive change, we believe that is indeed the right approach.
https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/
This patch series aims to do exactly that by introducing a HAS_IOPORT
config option akin to the existing HAS_IOMEM. When this is unset
inb()/outb() and friends may not be defined. Now since I/O port access is
not only used in legacy PCI devices or with legacy I/O spaces for backwards
compatible PCI Express devices, but also for example in ACPI we also
introduce another config option LEGACY_PCI to specifically disable the
compilation of drivers for legacy PCI devices and legacy I/O space uses
while keeping I/O port accessors for non-legacy uses. This allows modern
systems which do not need legacy PCI support to skip building drivers for
legacy devices while keeping e.g. ACPI support.
This series builts heavily on an original patch for demonstating the
concept by Arnd Bergmann and was created in collaboration with him as
discussed in the follow up to his original patch here:
https://lore.kernel.org/lkml/CAK8P3a0MNbx-iuzW_-=0ab6-TTZzwV-PT_6gAC1Gp5PgYyHcrA@mail.gmail.com/
It rebases his patch on v5.16-rc7, adds the missing arch selects for
HAS_IOPORT, fixes a few trivial findings from the original patch discussion
and splits the patch into more manageable patches. One other thing that
came up during the discussion is the idea of adding a separate
HARDCODED_IOPORTS config option for drivers which use hardcoded I/O port
numbers, this is not currently implemented but could still be added if we
find enough drivers that should not be compiled on platforms where
HAS_IOPORT is set but these hardcoded I/O ports will not work. According to
the above discussion John Garry is looking into this but I wanted to get
the discussion going on this proposal.
The series is split up into multiple patches as follows:
- Patch 01: Adds the LEGACY_PCI config and selects it for all remaining
drivers for legacy PCI devices.
- Patch 02: Adds the HAS_IOPORT config option and selects it for those
architectures supporting the I/O space access. It is currently not
selected for s390, nds32, um, h8300, nios2, openrisc, hexagon, csky, and
xtensa
- Patches 03-26: Add HAS_IOPORT dependencies on a per subsystem basis.
These dependencies are either Kconfig "depends on" or ifdefs where I/O
port access is an alternative path or required e.g. for a sysfs file.
- Patches 27-31: Handle HAS_IOPORT dependencies for some special cases such
as sysfs files, PCI quirks and in USB code.
- Patch 32: Removes the inb()/outb() etc. definitions in asm-generic/io.h
when HAS_IOPORT is not selected e.g. on s390.
I performed the following testing:
- On s390 this series on top of v5.16-rc7 builds with allyesconfig i.e. the
HAS_IOPORT=n case. It also builds with defconfig and the resulting kernel
appears fully functional including tests with PCI devices.
- On x86_64 with a config based on Arch Linux' standard config and
LEGACY_PCI=n it builds and I've been running kernels with this
configuration for over a week without issue on my Ryzen 3990X based
workstation (initially based on v5.16-rc6). I also tested LEGACY_PCI=y
though I do not have any legacy PCI devices, I did confirm though that
the additional modules are built as expected.
- For ARM64 I cross-compiled based on the current Arch Linux ARM generic
kernel config and LEGACY_PCI=n and have been running a v5.16-rc6 based
version of this patch on my Raspberry Pi 4 (DT not UEFI) and checked that
the PCI based USB still works.
For easy consumption a branch on top of v5.16-rc7 is also available from my
Github here https://github.com/niklas88/linux/tree/has_ioport_rfc_v1
Thanks,
Niklas Schnelle
Niklas Schnelle (32):
Kconfig: introduce and depend on LEGACY_PCI
Kconfig: introduce HAS_IOPORT option and select it as necessary
ACPI: Kconfig: add HAS_IOPORT dependencies
parport: PC style parport depends on HAS_IOPORT
char: impi, tpm: depend on HAS_IOPORT
speakup: Kconfig: add HAS_IOPORT dependencies
Input: gameport: add ISA and HAS_IOPORT dependencies
comedi: Kconfig: add HAS_IOPORT dependencies
sound: Kconfig: add HAS_IOPORT dependencies
i2c: Kconfig: add HAS_IOPORT dependencies
Input: Kconfig: add HAS_IOPORT dependencies
iio: adc: Kconfig: add HAS_IOPORT dependencies
hwmon: Kconfig: add HAS_IOPORT dependencies
leds: Kconfig: add HAS_IOPORT dependencies
media: Kconfig: add HAS_IOPORT dependencies
misc: handle HAS_IOPORT dependencies
net: Kconfig: add HAS_IOPORT dependencies
pcmcia: Kconfig: add HAS_IOPORT dependencies
platform: Kconfig: add HAS_IOPORT dependencies
pnp: Kconfig: add HAS_IOPORT dependencies
power: Kconfig: add HAS_IOPORT dependencies
video: handle HAS_IOPORT dependencies
rtc: Kconfig: add HAS_IOPORT dependencies
scsi: Kconfig: add HAS_IOPORT dependencies
watchdog: Kconfig: add HAS_IOPORT dependencies
drm: handle HAS_IOPORT dependencies
PCI/sysfs: make I/O resource depend on HAS_IOPORT
PCI: make quirk using inw() depend on HAS_IOPORT
firmware: dmi-sysfs: handle HAS_IOPORT dependencies
/dev/port: don't compile file operations without CONFIG_DEVPORT
usb: handle HAS_IOPORT dependencies
asm-generic/io.h: drop inb() etc for HAS_IOPORT=n
arch/alpha/Kconfig | 1 +
arch/arc/Kconfig | 1 +
arch/arm/Kconfig | 1 +
arch/arm64/Kconfig | 1 +
arch/ia64/Kconfig | 1 +
arch/m68k/Kconfig | 1 +
arch/microblaze/Kconfig | 1 +
arch/mips/Kconfig | 1 +
arch/parisc/Kconfig | 1 +
arch/powerpc/Kconfig | 1 +
arch/riscv/Kconfig | 1 +
arch/sh/Kconfig | 1 +
arch/sparc/Kconfig | 1 +
arch/x86/Kconfig | 1 +
drivers/accessibility/speakup/Kconfig | 1 +
drivers/acpi/Kconfig | 1 +
drivers/ata/Kconfig | 34 ++---
drivers/ata/ata_generic.c | 3 +-
drivers/ata/libata-sff.c | 2 +
drivers/bus/Kconfig | 2 +-
drivers/char/Kconfig | 3 +-
drivers/char/ipmi/Makefile | 11 +-
drivers/char/ipmi/ipmi_si_intf.c | 3 +-
drivers/char/ipmi/ipmi_si_pci.c | 3 +
drivers/char/mem.c | 6 +-
drivers/char/tpm/Kconfig | 1 +
drivers/char/tpm/tpm_infineon.c | 14 +-
drivers/char/tpm/tpm_tis_core.c | 19 ++-
drivers/comedi/Kconfig | 53 ++++++++
drivers/firmware/dmi-sysfs.c | 4 +
drivers/gpio/Kconfig | 2 +-
drivers/gpu/drm/qxl/Kconfig | 1 +
drivers/gpu/drm/tiny/Kconfig | 1 +
drivers/gpu/drm/tiny/cirrus.c | 2 +
drivers/hwmon/Kconfig | 21 ++-
drivers/i2c/busses/Kconfig | 29 +++--
drivers/iio/adc/Kconfig | 2 +-
drivers/input/gameport/Kconfig | 6 +-
drivers/input/serio/Kconfig | 2 +
drivers/input/touchscreen/Kconfig | 1 +
drivers/isdn/hardware/mISDN/Kconfig | 14 +-
drivers/leds/Kconfig | 2 +-
drivers/media/cec/platform/Kconfig | 2 +-
drivers/media/pci/dm1105/Kconfig | 2 +-
drivers/media/radio/Kconfig | 15 ++-
drivers/media/rc/Kconfig | 6 +
drivers/message/fusion/Kconfig | 8 +-
drivers/misc/altera-stapl/Makefile | 3 +-
drivers/misc/altera-stapl/altera.c | 6 +-
drivers/net/Kconfig | 2 +-
drivers/net/arcnet/Kconfig | 2 +-
drivers/net/can/cc770/Kconfig | 1 +
drivers/net/can/sja1000/Kconfig | 1 +
drivers/net/ethernet/8390/Kconfig | 2 +-
drivers/net/ethernet/amd/Kconfig | 2 +-
drivers/net/ethernet/intel/Kconfig | 4 +-
drivers/net/ethernet/sis/Kconfig | 6 +-
drivers/net/ethernet/ti/Kconfig | 4 +-
drivers/net/ethernet/via/Kconfig | 5 +-
drivers/net/fddi/Kconfig | 4 +-
drivers/net/hamradio/Kconfig | 6 +-
drivers/net/wan/Kconfig | 2 +-
drivers/net/wireless/atmel/Kconfig | 4 +-
drivers/net/wireless/intersil/hostap/Kconfig | 4 +-
drivers/parport/Kconfig | 2 +-
drivers/pci/Kconfig | 11 ++
drivers/pci/pci-sysfs.c | 16 +++
drivers/pci/quirks.c | 2 +
drivers/pcmcia/Kconfig | 2 +-
drivers/platform/chrome/Kconfig | 1 +
drivers/platform/chrome/wilco_ec/Kconfig | 1 +
drivers/pnp/isapnp/Kconfig | 2 +-
drivers/power/reset/Kconfig | 1 +
drivers/rtc/Kconfig | 4 +-
drivers/scsi/Kconfig | 21 +--
drivers/scsi/aic7xxx/Kconfig.aic79xx | 2 +-
drivers/scsi/aic7xxx/Kconfig.aic7xxx | 2 +-
drivers/scsi/aic94xx/Kconfig | 2 +-
drivers/scsi/megaraid/Kconfig.megaraid | 2 +-
drivers/scsi/mvsas/Kconfig | 2 +-
drivers/scsi/qla2xxx/Kconfig | 2 +-
drivers/spi/Kconfig | 1 +
drivers/staging/sm750fb/Kconfig | 2 +-
drivers/staging/vt6655/Kconfig | 2 +-
drivers/tty/Kconfig | 2 +-
drivers/tty/serial/Kconfig | 2 +-
drivers/usb/core/hcd-pci.c | 3 +-
drivers/usb/host/Kconfig | 4 +-
drivers/usb/host/pci-quirks.c | 127 ++++++++++---------
drivers/usb/host/pci-quirks.h | 33 +++--
drivers/usb/host/uhci-hcd.c | 2 +-
drivers/usb/host/uhci-hcd.h | 77 +++++++----
drivers/video/fbdev/Kconfig | 23 ++--
drivers/watchdog/Kconfig | 6 +-
include/asm-generic/io.h | 5 +
include/linux/gameport.h | 9 +-
include/linux/parport.h | 2 +-
include/video/vga.h | 8 ++
lib/Kconfig | 4 +
lib/Kconfig.kgdb | 1 +
sound/drivers/Kconfig | 3 +
sound/pci/Kconfig | 43 +++++--
102 files changed, 532 insertions(+), 250 deletions(-)
--
2.32.0
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them.
Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Niklas Schnelle <[email protected]>
---
drivers/rtc/Kconfig | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 058e56a10ab8..fde2bdb7090d 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -951,6 +951,7 @@ comment "Platform RTC drivers"
config RTC_DRV_CMOS
tristate "PC-style 'CMOS'"
depends on X86 || ARM || PPC || MIPS || SPARC64
+ depends on HAS_IOPORT
default y if X86
select RTC_MC146818_LIB
help
@@ -971,6 +972,7 @@ config RTC_DRV_CMOS
config RTC_DRV_ALPHA
bool "Alpha PC-style CMOS"
depends on ALPHA
+ depends on HAS_IOPORT
select RTC_MC146818_LIB
default y
help
@@ -1188,7 +1190,7 @@ config RTC_DRV_MSM6242
config RTC_DRV_BQ4802
tristate "TI BQ4802"
- depends on HAS_IOMEM
+ depends on HAS_IOMEM && HAS_IOPORT
help
If you say Y here you will get support for the TI
BQ4802 RTC chip.
--
2.32.0
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them.
Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Niklas Schnelle <[email protected]>
---
drivers/hwmon/Kconfig | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 09397562c396..c1a2d8ac96fd 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -547,6 +547,7 @@ config SENSORS_SPARX5
config SENSORS_F71805F
tristate "Fintek F71805F/FG, F71806F/FG and F71872F/FG"
+ depends on HAS_IOPORT
depends on !PPC
help
If you say yes here you get support for hardware monitoring
@@ -558,6 +559,7 @@ config SENSORS_F71805F
config SENSORS_F71882FG
tristate "Fintek F71882FG and compatibles"
+ depends on HAS_IOPORT
depends on !PPC
help
If you say yes here you get support for hardware monitoring
@@ -761,6 +763,7 @@ config SENSORS_CORETEMP
config SENSORS_IT87
tristate "ITE IT87xx and compatibles"
+ depends on HAS_IOPORT
depends on !PPC
select HWMON_VID
help
@@ -1387,6 +1390,7 @@ config SENSORS_LM95245
config SENSORS_PC87360
tristate "National Semiconductor PC87360 family"
+ depends on HAS_IOPORT
depends on !PPC
select HWMON_VID
help
@@ -1401,6 +1405,7 @@ config SENSORS_PC87360
config SENSORS_PC87427
tristate "National Semiconductor PC87427"
+ depends on HAS_IOPORT
depends on !PPC
help
If you say yes here you get access to the hardware monitoring
@@ -1432,6 +1437,7 @@ config SENSORS_NTC_THERMISTOR
config SENSORS_NCT6683
tristate "Nuvoton NCT6683D"
+ depends on HAS_IOPORT
depends on !PPC
help
If you say yes here you get support for the hardware monitoring
@@ -1442,6 +1448,7 @@ config SENSORS_NCT6683
config SENSORS_NCT6775
tristate "Nuvoton NCT6775F and compatibles"
+ depends on HAS_IOPORT
depends on !PPC
depends on ACPI_WMI || ACPI_WMI=n
select HWMON_VID
@@ -1664,6 +1671,7 @@ config SENSORS_SIS5595
config SENSORS_DME1737
tristate "SMSC DME1737, SCH311x and compatibles"
+ depends on HAS_IOPORT
depends on I2C && !PPC
select HWMON_VID
help
@@ -1707,6 +1715,7 @@ config SENSORS_EMC6W201
config SENSORS_SMSC47M1
tristate "SMSC LPC47M10x and compatibles"
+ depends on HAS_IOPORT
depends on !PPC
help
If you say yes here you get support for the integrated fan
@@ -1741,6 +1750,7 @@ config SENSORS_SMSC47M192
config SENSORS_SMSC47B397
tristate "SMSC LPC47B397-NC"
+ depends on HAS_IOPORT
depends on !PPC
help
If you say yes here you get support for the SMSC LPC47B397-NC
@@ -1754,6 +1764,7 @@ config SENSORS_SCH56XX_COMMON
config SENSORS_SCH5627
tristate "SMSC SCH5627"
+ depends on HAS_IOPORT
depends on !PPC && WATCHDOG
select SENSORS_SCH56XX_COMMON
select WATCHDOG_CORE
@@ -1767,6 +1778,7 @@ config SENSORS_SCH5627
config SENSORS_SCH5636
tristate "SMSC SCH5636"
+ depends on HAS_IOPORT
depends on !PPC && WATCHDOG
select SENSORS_SCH56XX_COMMON
select WATCHDOG_CORE
@@ -1995,6 +2007,7 @@ config SENSORS_VIA686A
config SENSORS_VT1211
tristate "VIA VT1211"
+ depends on HAS_IOPORT
depends on !PPC
select HWMON_VID
help
@@ -2114,6 +2127,7 @@ config SENSORS_W83L786NG
config SENSORS_W83627HF
tristate "Winbond W83627HF, W83627THF, W83637HF, W83687THF, W83697HF"
+ depends on HAS_IOPORT
depends on !PPC
select HWMON_VID
help
@@ -2126,6 +2140,7 @@ config SENSORS_W83627HF
config SENSORS_W83627EHF
tristate "Winbond W83627EHF/EHG/DHG/UHG, W83667HG"
+ depends on HAS_IOPORT
depends on !PPC
select HWMON_VID
help
--
2.32.0
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. We thus need to add HAS_IOPORT as dependency for
those drivers using them.
Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Niklas Schnelle <[email protected]>
---
drivers/watchdog/Kconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 05258109bcc2..2e87a65bdc8b 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -452,6 +452,7 @@ config 21285_WATCHDOG
config 977_WATCHDOG
tristate "NetWinder WB83C977 watchdog"
depends on (FOOTBRIDGE && ARCH_NETWINDER) || (ARM && COMPILE_TEST)
+ depends on HAS_IOPORT
help
Say Y here to include support for the WB977 watchdog included in
NetWinder machines. Alternatively say M to compile the driver as
@@ -1200,6 +1201,7 @@ config ITCO_WDT
select WATCHDOG_CORE
depends on I2C || I2C=n
depends on MFD_INTEL_PMC_BXT || !MFD_INTEL_PMC_BXT
+ depends on HAS_IOPORT # for I2C_I801
select LPC_ICH if !EXPERT
select I2C_I801 if !EXPERT && I2C
help
--
2.32.0
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. SPEAKUP_SERIALIO thus needs to depend on HAS_IOPORT.
Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Niklas Schnelle <[email protected]>
---
drivers/accessibility/speakup/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/accessibility/speakup/Kconfig b/drivers/accessibility/speakup/Kconfig
index 07ecbbde0384..e84fb617acc4 100644
--- a/drivers/accessibility/speakup/Kconfig
+++ b/drivers/accessibility/speakup/Kconfig
@@ -46,6 +46,7 @@ if SPEAKUP
config SPEAKUP_SERIALIO
def_bool y
depends on ISA || COMPILE_TEST
+ depends on HAS_IOPORT
config SPEAKUP_SYNTH_ACNTSA
tristate "Accent SA synthesizer support"
--
2.32.0
We introduce a new HAS_IOPORT Kconfig option to gate support for
I/O port access. In a future patch HAS_IOPORT=n will disable compilation
of the I/O accessor functions inb()/outb() and friends on architectures
which can not meaningfully support legacy I/O spaces. On these platforms
inb()/outb() etc are currently just stubs in asm-generic/io.h which when
called will cause a NULL pointer access which some compilers actually
detect and warn about.
The dependencies on HAS_IOPORT in drivers as well as ifdefs for
HAS_IOPORT specific sections will be added in subsequent patches on
a per subsystem basis. Then a final patch will ifdef the I/O access
functions on HAS_IOPORT thus turning any use not gated by HAS_IOPORT
into a compile-time warning.
Link: https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/
Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Niklas Schnelle <[email protected]>
---
arch/alpha/Kconfig | 1 +
arch/arc/Kconfig | 1 +
arch/arm/Kconfig | 1 +
arch/arm64/Kconfig | 1 +
arch/ia64/Kconfig | 1 +
arch/m68k/Kconfig | 1 +
arch/microblaze/Kconfig | 1 +
arch/mips/Kconfig | 1 +
arch/parisc/Kconfig | 1 +
arch/powerpc/Kconfig | 1 +
arch/riscv/Kconfig | 1 +
arch/sh/Kconfig | 1 +
arch/sparc/Kconfig | 1 +
arch/x86/Kconfig | 1 +
drivers/bus/Kconfig | 2 +-
lib/Kconfig | 4 ++++
lib/Kconfig.kgdb | 1 +
17 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
index 4e87783c90ad..472a0c5e4c2f 100644
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -28,6 +28,7 @@ config ALPHA
select AUDIT_ARCH
select GENERIC_CPU_VULNERABILITIES
select GENERIC_SMP_IDLE_THREAD
+ select HAS_IOPORT
select HAVE_ARCH_AUDITSYSCALL
select HAVE_MOD_ARCH_SPECIFIC
select MODULES_USE_ELF_RELA
diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index b4ae6058902a..b3911ebbd237 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -27,6 +27,7 @@ config ARC
select GENERIC_PENDING_IRQ if SMP
select GENERIC_SCHED_CLOCK
select GENERIC_SMP_IDLE_THREAD
+ select HAS_IOPORT
select HAVE_ARCH_KGDB
select HAVE_ARCH_TRACEHOOK
select HAVE_ARCH_TRANSPARENT_HUGEPAGE if ARC_MMU_V4
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c2724d986fa0..605709b6eecb 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -66,6 +66,7 @@ config ARM
select GENERIC_SCHED_CLOCK
select GENERIC_SMP_IDLE_THREAD
select HARDIRQS_SW_RESEND
+ select HAS_IOPORT
select HAVE_ARCH_AUDITSYSCALL if AEABI && !OABI_COMPAT
select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c4207cf9bb17..a8b199a40c8f 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -135,6 +135,7 @@ config ARM64
select GENERIC_GETTIMEOFDAY
select GENERIC_VDSO_TIME_NS
select HARDIRQS_SW_RESEND
+ select HAS_IOPORT
select HAVE_MOVE_PMD
select HAVE_MOVE_PUD
select HAVE_PCI
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 1e33666fa679..672aa2a88b19 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -24,6 +24,7 @@ config IA64
select PCI_DOMAINS if PCI
select PCI_MSI
select PCI_SYSCALL if PCI
+ select HAS_IOPORT
select HAVE_ASM_MODVERSIONS
select HAVE_UNSTABLE_SCHED_CLOCK
select HAVE_EXIT_THREAD
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index 0b50da08a9c5..926d97c33828 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -16,6 +16,7 @@ config M68K
select GENERIC_CPU_DEVICES
select GENERIC_IOMAP
select GENERIC_IRQ_SHOW
+ select HAS_IOPORT
select HAVE_AOUT if MMU
select HAVE_ASM_MODVERSIONS
select HAVE_DEBUG_BUGVERBOSE
diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
index 59798e43cdb0..213ef2940079 100644
--- a/arch/microblaze/Kconfig
+++ b/arch/microblaze/Kconfig
@@ -21,6 +21,7 @@ config MICROBLAZE
select GENERIC_IRQ_SHOW
select GENERIC_PCI_IOMAP
select GENERIC_SCHED_CLOCK
+ select HAS_IOPORT if PCI
select HAVE_ARCH_HASH
select HAVE_ARCH_KGDB
select HAVE_ARCH_SECCOMP
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 0215dc1529e9..87e6e7c29493 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -47,6 +47,7 @@ config MIPS
select GENERIC_SMP_IDLE_THREAD
select GENERIC_TIME_VSYSCALL
select GUP_GET_PTE_LOW_HIGH if CPU_MIPS32 && PHYS_ADDR_T_64BIT
+ select HAS_IOPORT
select HAVE_ARCH_COMPILER_H
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_KGDB if MIPS_FP_SUPPORT
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 011dc32fdb4d..b352c6dbbead 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -43,6 +43,7 @@ config PARISC
select MODULES_USE_ELF_RELA
select CLONE_BACKWARDS
select TTY # Needed for pdc_cons.c
+ select HAS_IOPORT if PCI || EISA
select HAVE_DEBUG_STACKOVERFLOW
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_HASH
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index dea74d7717c0..d39ba34d839a 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -185,6 +185,7 @@ config PPC
select GENERIC_SMP_IDLE_THREAD
select GENERIC_TIME_VSYSCALL
select GENERIC_VDSO_TIME_NS
+ select HAS_IOPORT if PCI
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP
select HAVE_ARCH_HUGE_VMAP if PPC_RADIX_MMU || PPC_8xx
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 821252b65f89..b69cc86522fb 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -63,6 +63,7 @@ config RISCV
select GENERIC_SMP_IDLE_THREAD
select GENERIC_TIME_VSYSCALL if MMU && 64BIT
select GENERIC_VDSO_TIME_NS if HAVE_GENERIC_VDSO
+ select HAS_IOPORT if MMU
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
select HAVE_ARCH_JUMP_LABEL_RELATIVE if !XIP_KERNEL
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 70afb30e0b32..334a52535379 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -24,6 +24,7 @@ config SUPERH
select GENERIC_SCHED_CLOCK
select GENERIC_SMP_IDLE_THREAD
select GUP_GET_PTE_LOW_HIGH if X2TLB
+ select HAS_IOPORT if HAS_IOPORT_MAP
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_KGDB
select HAVE_ARCH_SECCOMP_FILTER
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 66fc08646be5..728598673724 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -32,6 +32,7 @@ config SPARC
select GENERIC_IRQ_SHOW
select ARCH_WANT_IPC_PARSE_VERSION
select GENERIC_PCI_IOMAP
+ select HAS_IOPORT
select HAVE_NMI_WATCHDOG if SPARC64
select HAVE_CBPF_JIT if SPARC32
select HAVE_EBPF_JIT if SPARC64
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5c2ccb85f2ef..8d3cfd693559 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -153,6 +153,7 @@ config X86
select GUP_GET_PTE_LOW_HIGH if X86_PAE
select HARDIRQS_SW_RESEND
select HARDLOCKUP_CHECK_TIMESTAMP if X86_64
+ select HAS_IOPORT
select HAVE_ACPI_APEI if ACPI
select HAVE_ACPI_APEI_NMI if ACPI
select HAVE_ALIGNED_STRUCT_PAGE if SLUB
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 3c68e174a113..a61285100224 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -81,7 +81,7 @@ config MOXTET
config HISILICON_LPC
bool "Support for ISA I/O space on HiSilicon Hip06/7"
depends on (ARM64 && ARCH_HISI) || (COMPILE_TEST && !ALPHA && !HEXAGON && !PARISC)
- depends on HAS_IOMEM
+ depends on HAS_IOPORT
select INDIRECT_PIO if ARM64
help
Driver to enable I/O access to devices attached to the Low Pin
diff --git a/lib/Kconfig b/lib/Kconfig
index 5e7165e6a346..e55746625762 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -95,6 +95,7 @@ config ARCH_USE_SYM_ANNOTATIONS
config INDIRECT_PIO
bool "Access I/O in non-MMIO mode"
depends on ARM64
+ depends on HAS_IOPORT
help
On some platforms where no separate I/O space exists, there are I/O
hosts which can not be accessed in MMIO mode. Using the logical PIO
@@ -486,6 +487,9 @@ config HAS_IOMEM
depends on !NO_IOMEM
default y
+config HAS_IOPORT
+ def_bool ISA || LEGACY_PCI
+
config HAS_IOPORT_MAP
bool
depends on HAS_IOMEM && !NO_IOPORT_MAP
diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb
index 05dae05b6cc9..c68e4d9dcecb 100644
--- a/lib/Kconfig.kgdb
+++ b/lib/Kconfig.kgdb
@@ -121,6 +121,7 @@ config KDB_DEFAULT_ENABLE
config KDB_KEYBOARD
bool "KGDB_KDB: keyboard as input device"
+ depends on HAS_IOPORT
depends on VT && KGDB_KDB
default n
help
--
2.32.0
Niklas Schnelle, le lun. 27 déc. 2021 17:42:51 +0100, a ecrit:
> In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> not being declared. SPEAKUP_SERIALIO thus needs to depend on HAS_IOPORT.
>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Niklas Schnelle <[email protected]>
Reviewed-by: Samuel Thibault <[email protected]>
> ---
> drivers/accessibility/speakup/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/accessibility/speakup/Kconfig b/drivers/accessibility/speakup/Kconfig
> index 07ecbbde0384..e84fb617acc4 100644
> --- a/drivers/accessibility/speakup/Kconfig
> +++ b/drivers/accessibility/speakup/Kconfig
> @@ -46,6 +46,7 @@ if SPEAKUP
> config SPEAKUP_SERIALIO
> def_bool y
> depends on ISA || COMPILE_TEST
> + depends on HAS_IOPORT
>
> config SPEAKUP_SYNTH_ACNTSA
> tristate "Accent SA synthesizer support"
> --
> 2.32.0
>
--
Samuel
/* Amuse the user. */
printk(
" \\|/ ____ \\|/\n"
" \"@'/ ,. \\`@\"\n"
" /_| \\__/ |_\\\n"
" \\__U_/\n");
(From linux/arch/sparc/kernel/traps.c:die_if_kernel())
On 12/27/21 8:43 AM, Niklas Schnelle wrote:
> In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> not being declared. We thus need to add HAS_IOPORT as dependency for
> those drivers using them.
>
How is the need for HAS_IOPORT determined, when exactly is it needed,
and when not ?
$ git grep -E "inb|inw" drivers/watchdog/ | cut -f1 -d: | sort -u
drivers/watchdog/acquirewdt.c
drivers/watchdog/advantechwdt.c
drivers/watchdog/cpu5wdt.c
drivers/watchdog/f71808e_wdt.c
drivers/watchdog/ibmasr.c
drivers/watchdog/ie6xx_wdt.c
drivers/watchdog/it8712f_wdt.c
drivers/watchdog/it87_wdt.c
drivers/watchdog/iTCO_vendor_support.c
drivers/watchdog/iTCO_wdt.c
drivers/watchdog/Kconfig
drivers/watchdog/machzwd.c
drivers/watchdog/mixcomwd.c
drivers/watchdog/ni903x_wdt.c
drivers/watchdog/nic7018_wdt.c
drivers/watchdog/nv_tco.c
drivers/watchdog/pc87413_wdt.c
drivers/watchdog/pcwd.c
drivers/watchdog/pcwd_pci.c
drivers/watchdog/sbc60xxwdt.c
drivers/watchdog/sbc7240_wdt.c
drivers/watchdog/sc1200wdt.c
drivers/watchdog/sch311x_wdt.c
drivers/watchdog/smsc37b787_wdt.c
drivers/watchdog/sp5100_tco.c
drivers/watchdog/w83627hf_wdt.c
drivers/watchdog/w83877f_wdt.c
drivers/watchdog/w83977f_wdt.c
drivers/watchdog/wafer5823wdt.c
drivers/watchdog/wdt977.c
drivers/watchdog/wdt.c
drivers/watchdog/wdt_pci.c
Guenter
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Niklas Schnelle <[email protected]>
> ---
> drivers/watchdog/Kconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 05258109bcc2..2e87a65bdc8b 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -452,6 +452,7 @@ config 21285_WATCHDOG
> config 977_WATCHDOG
> tristate "NetWinder WB83C977 watchdog"
> depends on (FOOTBRIDGE && ARCH_NETWINDER) || (ARM && COMPILE_TEST)
> + depends on HAS_IOPORT
> help
> Say Y here to include support for the WB977 watchdog included in
> NetWinder machines. Alternatively say M to compile the driver as
> @@ -1200,6 +1201,7 @@ config ITCO_WDT
> select WATCHDOG_CORE
> depends on I2C || I2C=n
> depends on MFD_INTEL_PMC_BXT || !MFD_INTEL_PMC_BXT
> + depends on HAS_IOPORT # for I2C_I801
> select LPC_ICH if !EXPERT
> select I2C_I801 if !EXPERT && I2C
> help
>
On 12/27/21 8:42 AM, Niklas Schnelle wrote:
> In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> not being declared. We thus need to add HAS_IOPORT as dependency for
> those drivers using them.
>
Similar to watchdog, I don't understand when HAS_IOPORT is needed and when not,
as there are more hwmon drivers using IO ports than the ones listed below.
Guenter
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Niklas Schnelle <[email protected]>
> ---
> drivers/hwmon/Kconfig | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 09397562c396..c1a2d8ac96fd 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -547,6 +547,7 @@ config SENSORS_SPARX5
>
> config SENSORS_F71805F
> tristate "Fintek F71805F/FG, F71806F/FG and F71872F/FG"
> + depends on HAS_IOPORT
> depends on !PPC
> help
> If you say yes here you get support for hardware monitoring
> @@ -558,6 +559,7 @@ config SENSORS_F71805F
>
> config SENSORS_F71882FG
> tristate "Fintek F71882FG and compatibles"
> + depends on HAS_IOPORT
> depends on !PPC
> help
> If you say yes here you get support for hardware monitoring
> @@ -761,6 +763,7 @@ config SENSORS_CORETEMP
>
> config SENSORS_IT87
> tristate "ITE IT87xx and compatibles"
> + depends on HAS_IOPORT
> depends on !PPC
> select HWMON_VID
> help
> @@ -1387,6 +1390,7 @@ config SENSORS_LM95245
>
> config SENSORS_PC87360
> tristate "National Semiconductor PC87360 family"
> + depends on HAS_IOPORT
> depends on !PPC
> select HWMON_VID
> help
> @@ -1401,6 +1405,7 @@ config SENSORS_PC87360
>
> config SENSORS_PC87427
> tristate "National Semiconductor PC87427"
> + depends on HAS_IOPORT
> depends on !PPC
> help
> If you say yes here you get access to the hardware monitoring
> @@ -1432,6 +1437,7 @@ config SENSORS_NTC_THERMISTOR
>
> config SENSORS_NCT6683
> tristate "Nuvoton NCT6683D"
> + depends on HAS_IOPORT
> depends on !PPC
> help
> If you say yes here you get support for the hardware monitoring
> @@ -1442,6 +1448,7 @@ config SENSORS_NCT6683
>
> config SENSORS_NCT6775
> tristate "Nuvoton NCT6775F and compatibles"
> + depends on HAS_IOPORT
> depends on !PPC
> depends on ACPI_WMI || ACPI_WMI=n
> select HWMON_VID
> @@ -1664,6 +1671,7 @@ config SENSORS_SIS5595
>
> config SENSORS_DME1737
> tristate "SMSC DME1737, SCH311x and compatibles"
> + depends on HAS_IOPORT
> depends on I2C && !PPC
> select HWMON_VID
> help
> @@ -1707,6 +1715,7 @@ config SENSORS_EMC6W201
>
> config SENSORS_SMSC47M1
> tristate "SMSC LPC47M10x and compatibles"
> + depends on HAS_IOPORT
> depends on !PPC
> help
> If you say yes here you get support for the integrated fan
> @@ -1741,6 +1750,7 @@ config SENSORS_SMSC47M192
>
> config SENSORS_SMSC47B397
> tristate "SMSC LPC47B397-NC"
> + depends on HAS_IOPORT
> depends on !PPC
> help
> If you say yes here you get support for the SMSC LPC47B397-NC
> @@ -1754,6 +1764,7 @@ config SENSORS_SCH56XX_COMMON
>
> config SENSORS_SCH5627
> tristate "SMSC SCH5627"
> + depends on HAS_IOPORT
> depends on !PPC && WATCHDOG
> select SENSORS_SCH56XX_COMMON
> select WATCHDOG_CORE
> @@ -1767,6 +1778,7 @@ config SENSORS_SCH5627
>
> config SENSORS_SCH5636
> tristate "SMSC SCH5636"
> + depends on HAS_IOPORT
> depends on !PPC && WATCHDOG
> select SENSORS_SCH56XX_COMMON
> select WATCHDOG_CORE
> @@ -1995,6 +2007,7 @@ config SENSORS_VIA686A
>
> config SENSORS_VT1211
> tristate "VIA VT1211"
> + depends on HAS_IOPORT
> depends on !PPC
> select HWMON_VID
> help
> @@ -2114,6 +2127,7 @@ config SENSORS_W83L786NG
>
> config SENSORS_W83627HF
> tristate "Winbond W83627HF, W83627THF, W83637HF, W83687THF, W83697HF"
> + depends on HAS_IOPORT
> depends on !PPC
> select HWMON_VID
> help
> @@ -2126,6 +2140,7 @@ config SENSORS_W83627HF
>
> config SENSORS_W83627EHF
> tristate "Winbond W83627EHF/EHG/DHG/UHG, W83667HG"
> + depends on HAS_IOPORT
> depends on !PPC
> select HWMON_VID
> help
>
On Mon, 2021-12-27 at 10:03 -0800, Guenter Roeck wrote:
> On 12/27/21 8:43 AM, Niklas Schnelle wrote:
> > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > not being declared. We thus need to add HAS_IOPORT as dependency for
> > those drivers using them.
> >
>
> How is the need for HAS_IOPORT determined, when exactly is it needed,
> and when not ?
I mostly used "make allyesconfig" on s390 with the accessors ifdeffed
out (i.e. patch 32) to find where the I/O port accesses were compiled
in. This means that it doesn't find drivers which pull in a HAS_IOPORT
dependency transitively e.g. by depending on x86 or ACPI. It should get
those that use e.g. "|| COMPILE_TEST" though. This might not be ideal
but on the other hand it should catch all those drivers that currently
built with known broken code.
>
> $ git grep -E "inb|inw" drivers/watchdog/ | cut -f1 -d: | sort -u
> drivers/watchdog/acquirewdt.c
> drivers/watchdog/advantechwdt.c
>
---8<---
Hi Niklas,
On Mon, Dec 27, 2021 at 5:44 PM Niklas Schnelle <[email protected]> wrote:
> We introduce a new HAS_IOPORT Kconfig option to gate support for
> I/O port access. In a future patch HAS_IOPORT=n will disable compilation
> of the I/O accessor functions inb()/outb() and friends on architectures
> which can not meaningfully support legacy I/O spaces. On these platforms
> inb()/outb() etc are currently just stubs in asm-generic/io.h which when
> called will cause a NULL pointer access which some compilers actually
> detect and warn about.
>
> The dependencies on HAS_IOPORT in drivers as well as ifdefs for
> HAS_IOPORT specific sections will be added in subsequent patches on
> a per subsystem basis. Then a final patch will ifdef the I/O access
> functions on HAS_IOPORT thus turning any use not gated by HAS_IOPORT
> into a compile-time warning.
>
> Link: https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Niklas Schnelle <[email protected]>
Thanks for your patch!
> --- a/arch/m68k/Kconfig
> +++ b/arch/m68k/Kconfig
> @@ -16,6 +16,7 @@ config M68K
> select GENERIC_CPU_DEVICES
> select GENERIC_IOMAP
> select GENERIC_IRQ_SHOW
> + select HAS_IOPORT
> select HAVE_AOUT if MMU
> select HAVE_ASM_MODVERSIONS
> select HAVE_DEBUG_BUGVERBOSE
This looks way too broad to me: most m68k platform do not have I/O
port access support.
My gut feeling says:
select HAS_IOPORT if PCI || ISA
but that might miss some intricate details...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Em Mon, 27 Dec 2021 17:42:47 +0100
Niklas Schnelle <[email protected]> escreveu:
> We introduce a new HAS_IOPORT Kconfig option to gate support for
> I/O port access. In a future patch HAS_IOPORT=n will disable compilation
> of the I/O accessor functions inb()/outb() and friends on architectures
> which can not meaningfully support legacy I/O spaces. On these platforms
> inb()/outb() etc are currently just stubs in asm-generic/io.h which when
> called will cause a NULL pointer access which some compilers actually
> detect and warn about.
>
> The dependencies on HAS_IOPORT in drivers as well as ifdefs for
> HAS_IOPORT specific sections will be added in subsequent patches on
> a per subsystem basis. Then a final patch will ifdef the I/O access
> functions on HAS_IOPORT thus turning any use not gated by HAS_IOPORT
> into a compile-time warning.
>
> Link: https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Niklas Schnelle <[email protected]>
...
> @@ -486,6 +487,9 @@ config HAS_IOMEM
> depends on !NO_IOMEM
> default y
>
> +config HAS_IOPORT
> + def_bool ISA || LEGACY_PCI
> +
That doesn't sound right.
The only dependency for LEGACY_PCI is PCI. If one selects LEGACY_PCI
on an architecture that doesn't support it, this will cause problems.
Instead, HAS_IOPORT should be selected at architecture level, and
the dependency here should be just the opposite: LEGACY_API should
depends on HAS_IOPORT.
Thanks,
Mauro
Hi Geert, Niklas,
Am 28.12.2021 um 23:08 schrieb Geert Uytterhoeven:
> Hi Niklas,
>
> On Mon, Dec 27, 2021 at 5:44 PM Niklas Schnelle <[email protected]> wrote:
>> We introduce a new HAS_IOPORT Kconfig option to gate support for
>> I/O port access. In a future patch HAS_IOPORT=n will disable compilation
>> of the I/O accessor functions inb()/outb() and friends on architectures
>> which can not meaningfully support legacy I/O spaces. On these platforms
>> inb()/outb() etc are currently just stubs in asm-generic/io.h which when
>> called will cause a NULL pointer access which some compilers actually
>> detect and warn about.
>>
>> The dependencies on HAS_IOPORT in drivers as well as ifdefs for
>> HAS_IOPORT specific sections will be added in subsequent patches on
>> a per subsystem basis. Then a final patch will ifdef the I/O access
>> functions on HAS_IOPORT thus turning any use not gated by HAS_IOPORT
>> into a compile-time warning.
>>
>> Link: https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/
>> Co-developed-by: Arnd Bergmann <[email protected]>
>> Signed-off-by: Arnd Bergmann <[email protected]>
>> Signed-off-by: Niklas Schnelle <[email protected]>
>
> Thanks for your patch!
>
>> --- a/arch/m68k/Kconfig
>> +++ b/arch/m68k/Kconfig
>> @@ -16,6 +16,7 @@ config M68K
>> select GENERIC_CPU_DEVICES
>> select GENERIC_IOMAP
>> select GENERIC_IRQ_SHOW
>> + select HAS_IOPORT
>> select HAVE_AOUT if MMU
>> select HAVE_ASM_MODVERSIONS
>> select HAVE_DEBUG_BUGVERBOSE
>
> This looks way too broad to me: most m68k platform do not have I/O
> port access support.
>
> My gut feeling says:
>
> select HAS_IOPORT if PCI || ISA
>
> but that might miss some intricate details...
In particular, this misses the Atari ROM port ISA adapter case -
select HAS_IOPORT if PCI || ISA || ATARI_ROM_ISA
might do instead.
Cheers,
Michael
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
On Tue, Dec 28, 2021 at 8:20 PM Michael Schmitz <[email protected]> wrote:
> Am 28.12.2021 um 23:08 schrieb Geert Uytterhoeven:
> > On Mon, Dec 27, 2021 at 5:44 PM Niklas Schnelle <[email protected]> wrote:
> >> We introduce a new HAS_IOPORT Kconfig option to gate support for
> >> I/O port access. In a future patch HAS_IOPORT=n will disable compilation
> >> of the I/O accessor functions inb()/outb() and friends on architectures
> >> which can not meaningfully support legacy I/O spaces. On these platforms
> >> inb()/outb() etc are currently just stubs in asm-generic/io.h which when
> >> called will cause a NULL pointer access which some compilers actually
> >> detect and warn about.
> >>
> >> The dependencies on HAS_IOPORT in drivers as well as ifdefs for
> >> HAS_IOPORT specific sections will be added in subsequent patches on
> >> a per subsystem basis. Then a final patch will ifdef the I/O access
> >> functions on HAS_IOPORT thus turning any use not gated by HAS_IOPORT
> >> into a compile-time warning.
> >>
> >> Link: https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/
> >> Co-developed-by: Arnd Bergmann <[email protected]>
> >> Signed-off-by: Arnd Bergmann <[email protected]>
> >> Signed-off-by: Niklas Schnelle <[email protected]>
> >
> > Thanks for your patch!
> >
> >> --- a/arch/m68k/Kconfig
> >> +++ b/arch/m68k/Kconfig
> >> @@ -16,6 +16,7 @@ config M68K
> >> select GENERIC_CPU_DEVICES
> >> select GENERIC_IOMAP
> >> select GENERIC_IRQ_SHOW
> >> + select HAS_IOPORT
> >> select HAVE_AOUT if MMU
> >> select HAVE_ASM_MODVERSIONS
> >> select HAVE_DEBUG_BUGVERBOSE
> >
> > This looks way too broad to me: most m68k platform do not have I/O
> > port access support.
> >
> > My gut feeling says:
> >
> > select HAS_IOPORT if PCI || ISA
> >
> > but that might miss some intricate details...
>
> In particular, this misses the Atari ROM port ISA adapter case -
>
> select HAS_IOPORT if PCI || ISA || ATARI_ROM_ISA
>
> might do instead.
Right, makes sense. I had suggested to go the easy way and assume that
each architecture would select HAS_IOPORT if any configuration supports
it, but it looks like for m68k there is a clearly defined set of platforms that
do.
Note that for the platforms that don't set any of the three symbols, the
fallback makes inb() an alias for readb() with a different argument type,
so there may be m68k specific drivers that rely on this, but those would
already be broken if ATARI_ROM_ISA is set.
Arnd
Hi Arnd,
Am 29.12.2021 um 16:41 schrieb Arnd Bergmann:
> On Tue, Dec 28, 2021 at 8:20 PM Michael Schmitz <[email protected]> wrote:
>> Am 28.12.2021 um 23:08 schrieb Geert Uytterhoeven:
>>> On Mon, Dec 27, 2021 at 5:44 PM Niklas Schnelle <[email protected]> wrote:
>>>> We introduce a new HAS_IOPORT Kconfig option to gate support for
>>>> I/O port access. In a future patch HAS_IOPORT=n will disable compilation
>>>> of the I/O accessor functions inb()/outb() and friends on architectures
>>>> which can not meaningfully support legacy I/O spaces. On these platforms
>>>> inb()/outb() etc are currently just stubs in asm-generic/io.h which when
>>>> called will cause a NULL pointer access which some compilers actually
>>>> detect and warn about.
>>>>
>>>> The dependencies on HAS_IOPORT in drivers as well as ifdefs for
>>>> HAS_IOPORT specific sections will be added in subsequent patches on
>>>> a per subsystem basis. Then a final patch will ifdef the I/O access
>>>> functions on HAS_IOPORT thus turning any use not gated by HAS_IOPORT
>>>> into a compile-time warning.
>>>>
>>>> Link: https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/
>>>> Co-developed-by: Arnd Bergmann <[email protected]>
>>>> Signed-off-by: Arnd Bergmann <[email protected]>
>>>> Signed-off-by: Niklas Schnelle <[email protected]>
>>>
>>> Thanks for your patch!
>>>
>>>> --- a/arch/m68k/Kconfig
>>>> +++ b/arch/m68k/Kconfig
>>>> @@ -16,6 +16,7 @@ config M68K
>>>> select GENERIC_CPU_DEVICES
>>>> select GENERIC_IOMAP
>>>> select GENERIC_IRQ_SHOW
>>>> + select HAS_IOPORT
>>>> select HAVE_AOUT if MMU
>>>> select HAVE_ASM_MODVERSIONS
>>>> select HAVE_DEBUG_BUGVERBOSE
>>>
>>> This looks way too broad to me: most m68k platform do not have I/O
>>> port access support.
>>>
>>> My gut feeling says:
>>>
>>> select HAS_IOPORT if PCI || ISA
>>>
>>> but that might miss some intricate details...
>>
>> In particular, this misses the Atari ROM port ISA adapter case -
>>
>> select HAS_IOPORT if PCI || ISA || ATARI_ROM_ISA
>>
>> might do instead.
>
> Right, makes sense. I had suggested to go the easy way and assume that
> each architecture would select HAS_IOPORT if any configuration supports
> it, but it looks like for m68k there is a clearly defined set of platforms that
> do.
>
> Note that for the platforms that don't set any of the three symbols, the
> fallback makes inb() an alias for readb() with a different argument type,
> so there may be m68k specific drivers that rely on this, but those would
> already be broken if ATARI_ROM_ISA is set.
I'd hope not - we spent some effort to make sure setting ATARI_ROM_ISA
does not affect other m68k platforms when e.g. building multiplatform
kernels.
Replacing inb() by readb() without any address translation won't do much
good for m68k though - addresses in the traditional ISA I/O port range
would hit the (unmapped) zero page.
Cheers,
Michael
>
> Arnd
>
On Tue, Dec 28, 2021 at 11:15 PM Michael Schmitz <[email protected]> wrote:
> Am 29.12.2021 um 16:41 schrieb Arnd Bergmann:
> > On Tue, Dec 28, 2021 at 8:20 PM Michael Schmitz <[email protected]> wrote:
> I'd hope not - we spent some effort to make sure setting ATARI_ROM_ISA
> does not affect other m68k platforms when e.g. building multiplatform
> kernels.
Ok
> Replacing inb() by readb() without any address translation won't do much
> good for m68k though - addresses in the traditional ISA I/O port range
> would hit the (unmapped) zero page.
Correct, this is exactly the problem that Niklas is trying to solve here:
we do have drivers that hit this bug, and on s390 clang actually produces
a compile-time error for drivers that cause a NULL pointer dereference
this way.
What some other architectures do is to rely on inb()/outb() to have a
zero-based offset, and use an io_offset in PCI buses to ensure that a
low port number on the bus gets translated into a pointer value for the
virtual mapping in the kernel, which is then represented as an unsigned
int.
As this is indistinguishable from architectures that just don't have
a base address for I/O ports (we unfortunately picked 0 as the default
PCI_IOBASE value), my suggestion was to start marking architectures
that may have this problem as using HAS_IOPORT in order to keep
the existing behavior unchanged. If m68k does not suffer from this,
making HAS_IOPORT conditional on those config options that actually
need it would of course be best.
Arnd
Hi Arnd,
Am 30.12.2021 um 14:48 schrieb Arnd Bergmann:
> On Tue, Dec 28, 2021 at 11:15 PM Michael Schmitz <[email protected]> wrote:
>> Am 29.12.2021 um 16:41 schrieb Arnd Bergmann:
>>> On Tue, Dec 28, 2021 at 8:20 PM Michael Schmitz <[email protected]> wrote:
>> I'd hope not - we spent some effort to make sure setting ATARI_ROM_ISA
>> does not affect other m68k platforms when e.g. building multiplatform
>> kernels.
>
> Ok
>
>> Replacing inb() by readb() without any address translation won't do much
>> good for m68k though - addresses in the traditional ISA I/O port range
>> would hit the (unmapped) zero page.
>
> Correct, this is exactly the problem that Niklas is trying to solve here:
> we do have drivers that hit this bug, and on s390 clang actually produces
> a compile-time error for drivers that cause a NULL pointer dereference
> this way.
Thanks for clarifying - I only saw Geert's CC and failed to go back to
the start of the thread.
> What some other architectures do is to rely on inb()/outb() to have a
> zero-based offset, and use an io_offset in PCI buses to ensure that a
> low port number on the bus gets translated into a pointer value for the
> virtual mapping in the kernel, which is then represented as an unsigned
> int.
M54xx does just that for Coldfire:
arch/m68k/include/asm/io_no.h:
#define PCI_IO_PA 0xf8000000 /* Host physical address */
(used to set PCI BAR mappings, so matches your definition above).
All other (MMU) m68k users of inb()/outb() apply an io_offset in the
platform specific address translation:
arch/m68k/include/asm/io_mm.h:
#define q40_isa_io_base 0xff400000
#define enec_isa_read_base 0xfffa0000
#define enec_isa_write_base 0xfffb0000
arch/m68k/include/asm/amigayle.h:
#define GAYLE_IO (0xa20000+zTwoBase) /* 16bit and
even 8bit registers */
#define GAYLE_IO_8BITODD (0xa30000+zTwoBase) /* odd 8bit
registers */
(all constants used in address translation inlines that are used by the
m68k inb()/outb() macros - you can call that the poor man's version of
PCI BAR mappings ...).
So as long as support for any of the m68k PCI or ISA bridges is selected
in the kernel config, the appropriate IO space mapping is applied. If no
support for PCI or ISA bridges is selected, we already fall back to zero
offset mapping (but as far as I can tell, it shouldn't be possible to
build a kernel without bridge support but drivers that require it).
>
> As this is indistinguishable from architectures that just don't have
> a base address for I/O ports (we unfortunately picked 0 as the default
> PCI_IOBASE value), my suggestion was to start marking architectures
> that may have this problem as using HAS_IOPORT in order to keep
> the existing behavior unchanged. If m68k does not suffer from this,
> making HAS_IOPORT conditional on those config options that actually
> need it would of course be best.
Following your description, HAS_IOPORT would be required for neither of
PCI, ISA or ATARI_ROM_ISA ??
Cheers,
Michael
>
> Arnd
>
On Thu, 2021-12-30 at 16:44 +1300, Michael Schmitz wrote:
> Hi Arnd,
>
> Am 30.12.2021 um 14:48 schrieb Arnd Bergmann:
> > On Tue, Dec 28, 2021 at 11:15 PM Michael Schmitz <[email protected]> wrote:
> > > Am 29.12.2021 um 16:41 schrieb Arnd Bergmann:
> > > > On Tue, Dec 28, 2021 at 8:20 PM Michael Schmitz <[email protected]> wrote:
---8<---
>
> > What some other architectures do is to rely on inb()/outb() to have a
> > zero-based offset, and use an io_offset in PCI buses to ensure that a
> > low port number on the bus gets translated into a pointer value for the
> > virtual mapping in the kernel, which is then represented as an unsigned
> > int.
>
> M54xx does just that for Coldfire:
>
> arch/m68k/include/asm/io_no.h:
> #define PCI_IO_PA 0xf8000000 /* Host physical address */
>
> (used to set PCI BAR mappings, so matches your definition above).
>
> All other (MMU) m68k users of inb()/outb() apply an io_offset in the
> platform specific address translation:
>
>
---8<---
> So as long as support for any of the m68k PCI or ISA bridges is selected
> in the kernel config, the appropriate IO space mapping is applied. If no
> support for PCI or ISA bridges is selected, we already fall back to zero
> offset mapping (but as far as I can tell, it shouldn't be possible to
> build a kernel without bridge support but drivers that require it).
>
> > As this is indistinguishable from architectures that just don't have
> > a base address for I/O ports (we unfortunately picked 0 as the default
> > PCI_IOBASE value), my suggestion was to start marking architectures
> > that may have this problem as using HAS_IOPORT in order to keep
> > the existing behavior unchanged. If m68k does not suffer from this,
> > making HAS_IOPORT conditional on those config options that actually
> > need it would of course be best.
>
> Following your description, HAS_IOPORT would be required for neither of
> PCI, ISA or ATARI_ROM_ISA ??
>
No, HAS_IOPORT being set just means that inb() etc. exist and are
functional be it as special instructions like on x86 or via an I/O
address offset. As I understand it if you do have PCI, ISA or
ATARI_ROM_ISA they are functional. If none of them are set and your
zero offset mapping means these accessors can't actually be used you
could make the declerations ifdeffed on CONFIG_HAS_IOPORT to detect the
cases where somone managed to build drivers that require them and that
would result in a compile time error instead of silently, or with a
NULL pointer warning, compiling code that won't work.
On Wed, Dec 29, 2021 at 10:44 PM Michael Schmitz <[email protected]> wrote:
> Am 30.12.2021 um 14:48 schrieb Arnd Bergmann:
> > On Tue, Dec 28, 2021 at 11:15 PM Michael Schmitz <[email protected]> wrote:
> > What some other architectures do is to rely on inb()/outb() to have a
> > zero-based offset, and use an io_offset in PCI buses to ensure that a
> > low port number on the bus gets translated into a pointer value for the
> > virtual mapping in the kernel, which is then represented as an unsigned
> > int.
>
> M54xx does just that for Coldfire:
>
> arch/m68k/include/asm/io_no.h:
> #define PCI_IO_PA 0xf8000000 /* Host physical address */
>
> (used to set PCI BAR mappings, so matches your definition above).
I think coldfire gets it right here, using PCI_IOBASE to find the start of
the window and a zero io_offset:
#define PCI_IOBASE ((void __iomem *) PCI_IO_PA)
> All other (MMU) m68k users of inb()/outb() apply an io_offset in the
> platform specific address translation:
>
> arch/m68k/include/asm/io_mm.h:
>
> #define q40_isa_io_base 0xff400000
> #define enec_isa_read_base 0xfffa0000
> #define enec_isa_write_base 0xfffb0000
>
> arch/m68k/include/asm/amigayle.h:
>
> #define GAYLE_IO (0xa20000+zTwoBase) /* 16bit and
> even 8bit registers */
> #define GAYLE_IO_8BITODD (0xa30000+zTwoBase) /* odd 8bit
> registers */
>
> (all constants used in address translation inlines that are used by the
> m68k inb()/outb() macros - you can call that the poor man's version of
> PCI BAR mappings ...).
This still looks like the same thing to me, where you have inb() take a
zero-based port number, not a pointer. The effect is the same as the
coldfire version, it just uses a custom inline function instead of the
version from asm-generic/io.h.
> So as long as support for any of the m68k PCI or ISA bridges is selected
> in the kernel config, the appropriate IO space mapping is applied. If no
> support for PCI or ISA bridges is selected, we already fall back to zero
> offset mapping (but as far as I can tell, it shouldn't be possible to
> build a kernel without bridge support but drivers that require it).
Right.
> > As this is indistinguishable from architectures that just don't have
> > a base address for I/O ports (we unfortunately picked 0 as the default
> > PCI_IOBASE value), my suggestion was to start marking architectures
> > that may have this problem as using HAS_IOPORT in order to keep
> > the existing behavior unchanged. If m68k does not suffer from this,
> > making HAS_IOPORT conditional on those config options that actually
> > need it would of course be best.
>
> Following your description, HAS_IOPORT would be required for neither of
> PCI, ISA or ATARI_ROM_ISA ??
For these three options, we definitely need HAS_IOPORT, which would
imply that some version of inb()/outb() is provided. The difference between
using a custom PCI_IOBASE (or an open-coded equivalent) and using
a zero PCI_IOBASE in combination with registering PCI using a custom
io_offset is whether we can use drivers with hardcoded port numbers.
These should depend on a different Kconfig symbol to be introduced
(CONFIG_HARDCODED_IOPORT or similar) once we introduce them,
and you could decide for m68k whether to allow those or not, I would
assume you do want them in order to use certain legacy ISA drivers.
Arnd
Hi Arnd,
Am 01.01.2022 um 05:04 schrieb Arnd Bergmann:
> On Wed, Dec 29, 2021 at 10:44 PM Michael Schmitz <[email protected]> wrote:
>> Am 30.12.2021 um 14:48 schrieb Arnd Bergmann:
>>> On Tue, Dec 28, 2021 at 11:15 PM Michael Schmitz <[email protected]> wrote:
>>> What some other architectures do is to rely on inb()/outb() to have a
>>> zero-based offset, and use an io_offset in PCI buses to ensure that a
>>> low port number on the bus gets translated into a pointer value for the
>>> virtual mapping in the kernel, which is then represented as an unsigned
>>> int.
>>
>> M54xx does just that for Coldfire:
>>
>> arch/m68k/include/asm/io_no.h:
>> #define PCI_IO_PA 0xf8000000 /* Host physical address */
>>
>> (used to set PCI BAR mappings, so matches your definition above).
>
> I think coldfire gets it right here, using PCI_IOBASE to find the start of
> the window and a zero io_offset:
>
> #define PCI_IOBASE ((void __iomem *) PCI_IO_PA)
Good - I bear that in mind if I ever get around to reactivating my 060
accelerator and the PCI board for that.
>> All other (MMU) m68k users of inb()/outb() apply an io_offset in the
>> platform specific address translation:
>>
>> arch/m68k/include/asm/io_mm.h:
>>
>> #define q40_isa_io_base 0xff400000
>> #define enec_isa_read_base 0xfffa0000
>> #define enec_isa_write_base 0xfffb0000
>>
>> arch/m68k/include/asm/amigayle.h:
>>
>> #define GAYLE_IO (0xa20000+zTwoBase) /* 16bit and
>> even 8bit registers */
>> #define GAYLE_IO_8BITODD (0xa30000+zTwoBase) /* odd 8bit
>> registers */
>>
>> (all constants used in address translation inlines that are used by the
>> m68k inb()/outb() macros - you can call that the poor man's version of
>> PCI BAR mappings ...).
>
> This still looks like the same thing to me, where you have inb() take a
> zero-based port number, not a pointer. The effect is the same as the
> coldfire version, it just uses a custom inline function instead of the
> version from asm-generic/io.h.
Right.
>> So as long as support for any of the m68k PCI or ISA bridges is selected
>> in the kernel config, the appropriate IO space mapping is applied. If no
>> support for PCI or ISA bridges is selected, we already fall back to zero
>> offset mapping (but as far as I can tell, it shouldn't be possible to
>> build a kernel without bridge support but drivers that require it).
>
> Right.
>
>>> As this is indistinguishable from architectures that just don't have
>>> a base address for I/O ports (we unfortunately picked 0 as the default
>>> PCI_IOBASE value), my suggestion was to start marking architectures
>>> that may have this problem as using HAS_IOPORT in order to keep
>>> the existing behavior unchanged. If m68k does not suffer from this,
>>> making HAS_IOPORT conditional on those config options that actually
>>> need it would of course be best.
>>
>> Following your description, HAS_IOPORT would be required for neither of
>> PCI, ISA or ATARI_ROM_ISA ??
>
> For these three options, we definitely need HAS_IOPORT, which would
> imply that some version of inb()/outb() is provided. The difference between
Thanks for clarifying that (and to Niklas as well). Did sound a little
counter-intuitive to me...
> using a custom PCI_IOBASE (or an open-coded equivalent) and using
> a zero PCI_IOBASE in combination with registering PCI using a custom
> io_offset is whether we can use drivers with hardcoded port numbers.
> These should depend on a different Kconfig symbol to be introduced
> (CONFIG_HARDCODED_IOPORT or similar) once we introduce them,
> and you could decide for m68k whether to allow those or not, I would
> assume you do want them in order to use certain legacy ISA drivers.
That's exactly the purpose (though we're overmuch pushing the envelope
trying to accomodate legacy ISA drivers on too many platforms).
Cheers,
Michael
> Arnd
>
Hi Niklas,
On 27/12/2021 16:42, Niklas Schnelle wrote:
> I performed the following testing:
>
> - On s390 this series on top of v5.16-rc7 builds with allyesconfig i.e. the
> HAS_IOPORT=n case.
Are you sure that allyesconfig gives HAS_IOPORT=n? Indeed I see no
mechanism is always disallow HAS_IOPORT for s390 (which I think we would
want).
> It also builds with defconfig and the resulting kernel
> appears fully functional including tests with PCI devices.
Thanks,
Johnw
On Thu, 2022-01-06 at 17:45 +0000, John Garry wrote:
> Hi Niklas,
>
> On 27/12/2021 16:42, Niklas Schnelle wrote:
> > I performed the following testing:
> >
> > - On s390 this series on top of v5.16-rc7 builds with allyesconfig i.e. the
> > HAS_IOPORT=n case.
>
> Are you sure that allyesconfig gives HAS_IOPORT=n? Indeed I see no
> mechanism is always disallow HAS_IOPORT for s390 (which I think we would
> want).
>
> > It also builds with defconfig and the resulting kernel
> > appears fully functional including tests with PCI devices.
>
> Thanks,
> Johnw
>
I checked again by adding
config HAS_IOPORT
def_bool n
in arch/s390/Kconfig and that doesn't seem to make a difference,
allyesconfig builds all the same. Also checked for CONFIG_HAS_IOPORT in
my .config and that isn't listed with or without the above addition.
I think this is because without a help text there is no "config
question" and thus nothing that allyesconfig would set to yes. I do
agree though that it's better to be explicit and add the above to those
Kconfigs that don't support HAS_IOPORT.
Thanks,
Niklas
On 07/01/2022 07:21, Niklas Schnelle wrote:
> I checked again by adding
>
> config HAS_IOPORT
> def_bool n
>
> in arch/s390/Kconfig and that doesn't seem to make a difference,
> allyesconfig builds all the same. Also checked for CONFIG_HAS_IOPORT in
> my .config and that isn't listed with or without the above addition.
Strange, as I build your branch and I see it:
HEAD is now at 9f421b6580ed asm-generic/io.h: drop inb() etc for
HAS_IOPORT=n
john@localhost:~/kernel-dev5> export ARCH=s390
john@localhost:~/kernel-dev5> export
CROSS_COMPILE=/usr/bin/s390x-suse-linux-
john@localhost:~/kernel-dev5> make allyesconfig
#
# configuration written to .config
#
john@localhost:~/kernel-dev5> more .config | grep HAS_IOPORT
CONFIG_HAS_IOPORT=y
>
> I think this is because without a help text there is no "config
> question" and thus nothing that allyesconfig would set to yes. I do
> agree though that it's better to be explicit and add the above to those
> Kconfigs that don't support HAS_IOPORT.
So maybe something like:
config HAS_IOPORT
def_bool ISA || LEGACY_PCI
depends on !S390
Otherwise you can build inb et al from asm-generic/io.h and get the
original compile error about arithmetic on NULL pointer, right?
But I assume that there is a more elegant way of doing this...
Thanks,
John