linux-kernel - Re: [PATCH 1/1] tty: serial: handle HAS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ef2912910d006c573324bcf063cb76e843dc8267.camel@linux.ibm.com>
Date: Tue, 01 Oct 2024 13:21:16 +0200
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: "Maciej W. Rozycki" <macro@...am.me.uk>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby
 <jirislaby@...nel.org>,
        Ilpo Järvinen
 <ilpo.jarvinen@...ux.intel.com>,
        linux-serial@...r.kernel.org, Arnd
 Bergmann <arnd@...nel.org>,
        Heiko Carstens <hca@...ux.ibm.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] tty: serial: handle HAS_IOPORT dependencies

On Thu, 2024-05-23 at 03:11 +0100, Maciej W. Rozycki wrote:
> On Fri, 5 Apr 2024, Niklas Schnelle wrote:
> 
> > diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> > index 0d35c77fad9e..38ac5236d2ea 100644
> > --- a/drivers/tty/serial/8250/8250_pci.c
> > +++ b/drivers/tty/serial/8250/8250_pci.c
> > @@ -928,6 +928,7 @@ static int pci_netmos_init(struct pci_dev *dev)
> >  	return num_serial;
> >  }
> >  
> > +#ifdef CONFIG_HAS_IOPORT
> >  /*
> >   * These chips are available with optionally one parallel port and up to
> >   * two serial ports. Unfortunately they all have the same product id.
> [...]
> > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> > index 47ff50763c04..54bf98869abf 100644
> > --- a/drivers/tty/serial/8250/Kconfig
> > +++ b/drivers/tty/serial/8250/Kconfig
> > @@ -136,7 +135,7 @@ config SERIAL_8250_PCILIB
> >  
> >  config SERIAL_8250_PCI
> >  	tristate "8250/16550 PCI device support"
> > -	depends on SERIAL_8250 && PCI
> > +	depends on SERIAL_8250 && PCI && HAS_IOPORT
> >  	select SERIAL_8250_PCILIB
> >  	default SERIAL_8250
> >  	help
> 
>  This is clearly wrong, there is PCIe 8250 serial hardware that does MMIO 
> only, so the option has to stay possible to enable.  I have such hardware 
> as shown in this log:
> 
> Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> serial 0001:01:00.0: enabling device (0140 -> 0142)
> serial 0001:01:00.0: detected caps 00000700 should be 00000500
> 0001:01:00.0: ttyS0 at MMIO 0x600c080401000 (irq = 40, base_baud = 15625000) is a 16C950/954
> serial 0001:01:00.0: detected caps 00000700 should be 00000500
> 0001:01:00.0: ttyS1 at MMIO 0x600c080401200 (irq = 40, base_baud = 15625000) is a 16C950/954
> 
> which is from a POWER9 system.  Which as you may know has no PCI port I/O 
> support in hardware, so it is quite relevant here.  I'd like to keep this 
> PCIe serial option functional with my system.
> 
>  Also your change itself modifies 8250_pci.c (cited above for reference), 
> which would make no sense if SERIAL_8250_PCI was permanently disabled for 
> !HAS_IOPORT.  Shall I take it than that the Kconfig change I question has 
> been made merely by mistake?
> 
>   Maciej

Hi Maciej,

With 2 more HAS_IOPORT patches having gone into v6.12-rc1 I'm looking
at what's left and we're down to 4 prerequisite patches[0] before being
able to compile-time disable inb()/outb()/…. This one being by far the
largest of these. Looking at your suggestion it seems that to compile
8250_pci.c without HAS_IOPORT I'll have to add #ifdef CONFIG_HAS_IOPORT
around the MOXI section as that uses I/O ports unconditionally. The
rest seems fine and I guess would theoretically work on a system with
!HAS_IOPORT. I'll send a v2 with that included. 

Note however that even though your POWER9 system does not have I/O port
support in hardware we still have HAS_IOPORT enabled for arch/powerpc
if PCI is enabed so even with this patch as is your POWER9 system
should not be affected.

Thanks,
Niklas

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/log/?h=has_ioport