mbox series

[v5,00/44] treewide: Remove I/O port accessors for HAS_IOPORT=n

Message ID 20230522105049.1467313-1-schnelle@linux.ibm.com
Headers show
Series treewide: Remove I/O port accessors for HAS_IOPORT=n | expand

Message

Niklas Schnelle May 22, 2023, 10:50 a.m. UTC
Hello Kernel Hackers,

Some platforms such as s390 do not support PCI I/O spaces. On such platforms
I/O space accessors like inb()/outb() are 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 be a NULL pointer access on these platforms. In
a previous patch we tried handling this with a run-time warning on access. This
approach however was rejected by Linus[0] 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.

This patch series does exactly that by utilizing the HAS_IOPORT Kconfig option
introduced in v6.4-rc1 via the spun out first patch[1] of a previous version. With
the final patch of this series HAS_IOPORT=n means that inb()/outb() and friends
are not defined. This is also the same approach originally planned by Uwe
Kleine-König as mentioned in commit ce816fa88cca ("Kconfig: rename HAS_IOPORT
to HAS_IOPORT_MAP"). With the HAS_IOPORT Kconfig option merged already
per-subsystem patches can now also be merged independently and only the last
patch needs to wait.

This series builds heavily on an original patch for demonstating the concept by
Arnd Bergmann[2] and incoporates feedback of previous versions [3][4][5][6].

A few patches have already been applied but I've kept those which are not yet
in v6.4-rc3.

This version is based on v6.4-rc3 and is also available on my kernel.org tree
in the has_ioport_v5:

https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git

Thanks,
Niklas Schnelle

Changes froom v4:
- Rebased on v6.4-rc3
- ata: Moved #ifdef CONFIG_PATA_ALI into ata_pci_bmdma_clear_simplex()
  (Damien Le Moal)
- char: Fixed ipmi typo and split the char subsystem patch into 3 parts for
  char, tpm, and ipmi (Greg K-H)
- rtc: Added missing "|| MACH_DECSTATION" (Maciej W. Rozycki)
- usb: Made UHCI_OUT() empty variant "do {} while (0)" and removed unneeded
  unconditional UHCI_IN() / UHCI_OUT() uses (Arnd, Alan Stern, Greg K-H)
- usb: pci-quirks: Added missing USB_PCI dependency
- video: Split into vgacon and fbdev patches
- Removed atyfb patch as it was merged in v6.4-rc3

Changes from v3:
- Rebased on v6.4-rc2 which includes the Kconfig HAS_IOPORT option
- Fixed some wrong #ifdef HAS_IOPORT to #ifdef CONFIG_HAS_IOPORT
- Split the usb subsystem patch into 3 parts (Arnd)
- Removed unneeded HAS_IOPORT dependency for SERIO_PARKBD (Geert Uytterhoeven)
- Removed no-op I/O access from FB_ATY (Ville Syrjälä)
- Simplified HAS_IOPORT dependencies for the sound subsystem (Takashi Iwai)
- Slightly reworded the subject line of the last patch which removes
  the I/O port accessor functions.

Changes from RFC v2:
- Rebased on v6.3-rc1
- Fixed a NULL pointer dereference in set_io_from_upio() due to accidentially
  expanded #ifdef CONFIG_SERIAL_8250_RT288X (kernel test robot)
- Dropped "ACPI: add dependency on HAS_IOPORT" (Bjorn Helgaas)
- Reworded commit message and moved ifdefs for "PCI/sysfs: Make I/O resource
  depend on HAS_IOPORT" (Bjorn Helgaas)
- Instead of complete removal inb() etc. are marked with __compiletime_error()
  when HAS_IOPORT is unset allowing for better error reporting (Ahmad Fatoum)
- Removed HAS_IOPORT dependency from PCMCIA as I/O port use is optional in at
  least PC Card. Instead added HAS_IOPORT on a per driver basis. (Bjorn
  Helgaas)
- Made uhci_has_pci_registers() constant 0 if HAS_IOPORT is not defined (Alan
  Stern)

Changes from RFC v1:
- Completely dropped the LEGACY_PCI option and replaced its dependencies with
  HAS_IOPORT as appropriate
- In the usb subsystem patch I incorporated the feedback from v1 by Alan Stern:
  - Used a local macro to nop in*()/out*() in the helpers
  - Removed an unnecessary further restriction on CONFIG_USB_UHCI_HCD
- Added a few more subsystems including wireless, ptp, and, mISDN that I had
  previously missed due to a blanket !S390.
- Removed blanket !S390 dependencies where they are added due to the I/O port
  problem
- In the sound system SND_OPL3_LIB needed to use "depends on" instead of
  "select" because of its added HAS_IOPORT dependency
- In the drm subsystem the bochs driver gets #ifdefs instead of a blanket
  dependency because its MMIO capable device variant should work without
  HAS_IOPORT.

[0] https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@mail.gmail.com/
[1] https://lore.kernel.org/lkml/20230323163354.1454196-1-schnelle@linux.ibm.com/
[2] https://lore.kernel.org/lkml/CAK8P3a0MNbx-iuzW_-=0ab6-TTZzwV-PT_6gAC1Gp5PgYyHcrA@mail.gmail.com/
[3] https://yhbt.net/lore/all/20211227164317.4146918-1-schnelle@linux.ibm.com/
[4] https://lore.kernel.org/all/20220429135108.2781579-1-schnelle@linux.ibm.com/
[5] https://lore.kernel.org/lkml/20230314121216.413434-1-schnelle@linux.ibm.com/
[6] https://lore.kernel.org/all/20230516110038.2413224-1-schnelle@linux.ibm.com/

Niklas Schnelle (44):
  kgdb: add HAS_IOPORT dependency
  ata: add HAS_IOPORT dependencies
  char: add HAS_IOPORT dependencies
  char: ipmi: handle HAS_IOPORT dependencies
  char: tpm: handle HAS_IOPORT dependencies
  comedi: add HAS_IOPORT dependencies
  counter: add HAS_IOPORT_MAP dependency
  /dev/port: don't compile file operations without CONFIG_DEVPORT
  drm: handle HAS_IOPORT dependencies
  firmware: dmi-sysfs: handle HAS_IOPORT=n
  gpio: add HAS_IOPORT dependencies
  hwmon: add HAS_IOPORT dependencies
  i2c: add HAS_IOPORT dependencies
  iio: ad7606: Kconfig: add HAS_IOPORT dependencies
  Input: add HAS_IOPORT dependencies
  Input: gameport: add ISA and HAS_IOPORT dependencies
  leds: add HAS_IOPORT dependencies
  media: add HAS_IOPORT dependencies
  misc: add HAS_IOPORT dependencies
  mISDN: add HAS_IOPORT dependencies
  mpt fusion: add HAS_IOPORT dependencies
  net: handle HAS_IOPORT dependencies
  parport: PC style parport depends on HAS_IOPORT
  PCI: Make quirk using inw() depend on HAS_IOPORT
  PCI/sysfs: Make I/O resource depend on HAS_IOPORT
  pcmcia: add HAS_IOPORT dependencies
  platform: add HAS_IOPORT dependencies
  pnp: add HAS_IOPORT dependencies
  power: add HAS_IOPORT dependencies
  rtc: add HAS_IOPORT dependencies
  scsi: add HAS_IOPORT dependencies
  sound: add HAS_IOPORT dependencies
  speakup: add HAS_IOPORT dependency for SPEAKUP_SERIALIO
  staging: add HAS_IOPORT dependencies
  tty: serial: handle HAS_IOPORT dependencies
  usb: add HAS_IOPORT dependencies
  usb: uhci: handle HAS_IOPORT dependencies
  usb: pci-quirks: handle HAS_IOPORT dependencies
  vgacon: add HAS_IOPORT dependencies
  fbdev: add HAS_IOPORT dependencies
  video: Handle HAS_IOPORT dependencies
  watchdog: add HAS_IOPORT dependencies
  wireless: add HAS_IOPORT dependencies
  asm-generic/io.h: Remove I/O port accessors for HAS_IOPORT=n

 drivers/accessibility/speakup/Kconfig        |   1 +
 drivers/ata/Kconfig                          |  28 ++---
 drivers/ata/libata-sff.c                     |   4 +
 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              |  16 ++-
 drivers/char/tpm/tpm_tis_core.c              |  19 ++-
 drivers/comedi/Kconfig                       | 103 +++++++++------
 drivers/counter/Kconfig                      |   1 +
 drivers/firmware/dmi-sysfs.c                 |   4 +
 drivers/gpio/Kconfig                         |  26 ++--
 drivers/gpu/drm/qxl/Kconfig                  |   1 +
 drivers/gpu/drm/tiny/bochs.c                 |  17 +++
 drivers/gpu/drm/tiny/cirrus.c                |   2 +
 drivers/hwmon/Kconfig                        |  21 +++-
 drivers/i2c/busses/Kconfig                   |  31 ++---
 drivers/iio/adc/Kconfig                      |   2 +-
 drivers/input/gameport/Kconfig               |   4 +-
 drivers/input/serio/Kconfig                  |   1 +
 drivers/input/touchscreen/Kconfig            |   1 +
 drivers/isdn/Kconfig                         |   1 -
 drivers/isdn/hardware/mISDN/Kconfig          |  12 +-
 drivers/leds/Kconfig                         |   2 +-
 drivers/media/pci/dm1105/Kconfig             |   2 +-
 drivers/media/radio/Kconfig                  |  14 ++-
 drivers/media/rc/Kconfig                     |   6 +
 drivers/message/fusion/Kconfig               |   2 +-
 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/3com/Kconfig            |   4 +-
 drivers/net/ethernet/8390/Kconfig            |   6 +-
 drivers/net/ethernet/amd/Kconfig             |   4 +-
 drivers/net/ethernet/fujitsu/Kconfig         |   2 +-
 drivers/net/ethernet/intel/Kconfig           |   2 +-
 drivers/net/ethernet/sis/Kconfig             |   4 +-
 drivers/net/ethernet/smsc/Kconfig            |   2 +-
 drivers/net/ethernet/ti/Kconfig              |   2 +-
 drivers/net/ethernet/via/Kconfig             |   1 +
 drivers/net/ethernet/xircom/Kconfig          |   2 +-
 drivers/net/fddi/defxx.c                     |   2 +-
 drivers/net/hamradio/Kconfig                 |   6 +-
 drivers/net/wan/Kconfig                      |   2 +-
 drivers/net/wireless/atmel/Kconfig           |   2 +-
 drivers/net/wireless/intersil/hostap/Kconfig |   2 +-
 drivers/parport/Kconfig                      |   3 +-
 drivers/pci/pci-sysfs.c                      |   4 +
 drivers/pci/quirks.c                         |   2 +
 drivers/pcmcia/Kconfig                       |   5 +-
 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                         |  25 ++--
 drivers/scsi/aic7xxx/Kconfig.aic79xx         |   2 +-
 drivers/scsi/aic7xxx/Kconfig.aic7xxx         |   2 +-
 drivers/scsi/aic94xx/Kconfig                 |   2 +-
 drivers/scsi/megaraid/Kconfig.megaraid       |   6 +-
 drivers/scsi/mvsas/Kconfig                   |   2 +-
 drivers/scsi/pcmcia/Kconfig                  |   6 +-
 drivers/scsi/qla2xxx/Kconfig                 |   2 +-
 drivers/staging/sm750fb/Kconfig              |   2 +-
 drivers/staging/vt6655/Kconfig               |   2 +-
 drivers/tty/Kconfig                          |   4 +-
 drivers/tty/serial/8250/8250_early.c         |   4 +
 drivers/tty/serial/8250/8250_pci.c           |  14 +++
 drivers/tty/serial/8250/8250_port.c          |  44 +++++--
 drivers/tty/serial/8250/Kconfig              |   5 +-
 drivers/tty/serial/Kconfig                   |   2 +-
 drivers/usb/Kconfig                          |  10 ++
 drivers/usb/core/hcd-pci.c                   |   2 +
 drivers/usb/host/Kconfig                     |   4 +-
 drivers/usb/host/pci-quirks.c                | 125 ++++++++++---------
 drivers/usb/host/pci-quirks.h                |  30 +++--
 drivers/usb/host/uhci-hcd.c                  |   2 +-
 drivers/usb/host/uhci-hcd.h                  |  24 ++--
 drivers/video/console/Kconfig                |   1 +
 drivers/video/fbdev/Kconfig                  |  22 ++--
 drivers/watchdog/Kconfig                     |   6 +-
 include/asm-generic/io.h                     |  60 +++++++++
 include/linux/gameport.h                     |   9 +-
 include/linux/parport.h                      |   2 +-
 include/video/vga.h                          |  35 ++++--
 lib/Kconfig.kgdb                             |   1 +
 net/ax25/Kconfig                             |   2 +-
 sound/drivers/Kconfig                        |   3 +
 sound/isa/Kconfig                            |   1 +
 sound/pci/Kconfig                            |  45 +++++--
 sound/pcmcia/Kconfig                         |   1 +
 97 files changed, 641 insertions(+), 297 deletions(-)


base-commit: 44c026a73be8038f03dbdeef028b642880cf1511

Comments

Arnd Bergmann May 22, 2023, 11:29 a.m. UTC | #1
On Mon, May 22, 2023, at 12:50, Niklas Schnelle wrote:

> A few patches have already been applied but I've kept those which are not yet
> in v6.4-rc3.
>
> This version is based on v6.4-rc3 and is also available on my kernel.org tree
> in the has_ioport_v5:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git

I think it would be best if as many patches as possible get merged
into v6.5 through the individidual subsystems, though I can take
whatever is left through the asm-generic tree.

Since the goal is to have maintainers pick up part of this, I would
recommend splitting the series per subsystem, having either a
separate patch or a small series for each maintainer that should
pick them up.

More importantly, I think you should rebase the series against
linux-next in order to find and drop the patches that are queued
up for 6.5 already. The patches will be applied into branches
that are based on 6.4-rc of course, but basing on linux-next
is usually the easiest when targeting multiple maintainer
trees.

Maybe let's give it another week to have more maintainers pick
up stuff from v5, and then send out a v6 as separate submissions.

    Arnd
Martin K. Petersen June 8, 2023, 1:42 a.m. UTC | #2
On Mon, 22 May 2023 12:50:05 +0200, Niklas Schnelle wrote:

> Some platforms such as s390 do not support PCI I/O spaces. On such platforms
> I/O space accessors like inb()/outb() are 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 be a NULL pointer access on these platforms. In
> a previous patch we tried handling this with a run-time warning on access. This
> approach however was rejected by Linus[0] 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.
> 
> [...]

Applied to 6.5/scsi-queue, thanks!

[21/44] mpt fusion: add HAS_IOPORT dependencies
        https://git.kernel.org/mkp/scsi/c/c3f903472ffa
[31/44] scsi: add HAS_IOPORT dependencies
        https://git.kernel.org/mkp/scsi/c/b58b2ba351b0
Niklas Schnelle June 27, 2023, 9:12 a.m. UTC | #3
On Mon, 2023-05-22 at 13:29 +0200, Arnd Bergmann wrote:
> On Mon, May 22, 2023, at 12:50, Niklas Schnelle wrote:
> 
> > A few patches have already been applied but I've kept those which are not yet
> > in v6.4-rc3.
> > 
> > This version is based on v6.4-rc3 and is also available on my kernel.org tree
> > in the has_ioport_v5:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git
> 
> I think it would be best if as many patches as possible get merged
> into v6.5 through the individidual subsystems, though I can take
> whatever is left through the asm-generic tree.
> 
> Since the goal is to have maintainers pick up part of this, I would
> recommend splitting the series per subsystem, having either a
> separate patch or a small series for each maintainer that should
> pick them up.
> 
> More importantly, I think you should rebase the series against
> linux-next in order to find and drop the patches that are queued
> up for 6.5 already. The patches will be applied into branches
> that are based on 6.4-rc of course, but basing on linux-next
> is usually the easiest when targeting multiple maintainer
> trees.
> 
> Maybe let's give it another week to have more maintainers pick
> up stuff from v5, and then send out a v6 as separate submissions.
> 
>     Arnd

Hi Arnd and All,

I'm sorry there hasn't been an updated in a long time and we're missing
v6.5. I've been quite busy with other work and life. Speaking of, I
will be mostly out for around a month starting some time mid to end
July as, if all goes well, I'm expecting to become a dad. That said, I
haven't forgotten about this and your overall plan of sending per-
subsystem patches sounds good, just haven't had the time to also
incorporate the feedback.

Thanks,
Niklas
Arnd Bergmann June 27, 2023, 12:53 p.m. UTC | #4
On Tue, Jun 27, 2023, at 11:12, Niklas Schnelle wrote:
> On Mon, 2023-05-22 at 13:29 +0200, Arnd Bergmann wrote:
>> 
>> Maybe let's give it another week to have more maintainers pick
>> up stuff from v5, and then send out a v6 as separate submissions.
>> 
>>     Arnd
>
> Hi Arnd and All,
>
> I'm sorry there hasn't been an updated in a long time and we're missing
> v6.5. I've been quite busy with other work and life. Speaking of, I
> will be mostly out for around a month starting some time mid to end
> July as, if all goes well, I'm expecting to become a dad. That said, I
> haven't forgotten about this and your overall plan of sending per-
> subsystem patches sounds good, just haven't had the time to also
> incorporate the feedback.

Ok, thanks for letting us know. I just checked to see that about half
of your series has already made it into linux-next and is likely to
be part of v6.5 or already in v6.4.

Maybe you can start out by taking a pass at just resending the ones
that don't need any changes and can just get picked up after -rc1,
and then I'll try to have a look at whatever remains after that.

    Arnd
Niklas Schnelle June 29, 2023, 1:26 p.m. UTC | #5
On Tue, 2023-06-27 at 14:53 +0200, Arnd Bergmann wrote:
> On Tue, Jun 27, 2023, at 11:12, Niklas Schnelle wrote:
> > On Mon, 2023-05-22 at 13:29 +0200, Arnd Bergmann wrote:
> > > 
> > > Maybe let's give it another week to have more maintainers pick
> > > up stuff from v5, and then send out a v6 as separate submissions.
> > > 
> > >     Arnd
> > 
> > Hi Arnd and All,
> > 
> > I'm sorry there hasn't been an updated in a long time and we're missing
> > v6.5. I've been quite busy with other work and life. Speaking of, I
> > will be mostly out for around a month starting some time mid to end
> > July as, if all goes well, I'm expecting to become a dad. That said, I
> > haven't forgotten about this and your overall plan of sending per-
> > subsystem patches sounds good, just haven't had the time to also
> > incorporate the feedback.
> 
> Ok, thanks for letting us know. I just checked to see that about half
> of your series has already made it into linux-next and is likely to
> be part of v6.5 or already in v6.4.
> 
> Maybe you can start out by taking a pass at just resending the ones
> that don't need any changes and can just get picked up after -rc1,
> and then I'll try to have a look at whatever remains after that.
> 
>     Arnd


Oh yeah looks better than I anticipated. I seem to have picked an odd
base commit for "tty: serial: .." because of which Greg couldn't apply
it so res-ending + rebase might be enough for that. By my count it
looks like only "usb: pci-quirksL ..." needs real work and possibly the
"drm: .." part though the discussion around cirrus doesn't look like it
would require much work. So I'll do rebase/re-send of the easy ones
tomorrow/next week.

Thanks,
Niklas