[PATCH] um: Turn HAS_IOPORT_MAP off

[PATCH] um: Turn HAS_IOPORT_MAP off

[Tiwei Bie]

Since we don't have port I/O support, we should turn HAS_IOPORT_MAP
off as well. It will fix clang warnings like below:

./include/asm-generic/io.h:1175:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
 1175 |         return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
      |                                                   ~~~~~~~~~~ ^

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506271049.MtzHqdSd-lkp@intel.com/
Fixes: b8c9c3b822fe ("um: stop using PCI port I/O")
Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
---
Strictly speaking, the -Wnull-pointer-arithmetic warnings reported
by lkp has been fixed by commit b8c9c3b822fe ("um: stop using PCI
port I/O"). But it introduced new -Wnull-pointer-arithmetic warnings,
and this patch fixes the newly introduced warnings.

 arch/um/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index f08e8a7fac93..76aa954520b1 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -54,6 +54,9 @@ config UML_IOMEM_EMULATION
 	select INDIRECT_IOMEM
 	select GENERIC_PCI_IOMAP
 
+config NO_IOPORT_MAP
+	def_bool y
+
 config ISA
 	bool
 
-- 
2.34.1

Re: [PATCH] um: Turn HAS_IOPORT_MAP off

[Geert Uytterhoeven]

CC arnd

On Sat, 28 Jun 2025 at 18:48, Tiwei Bie <tiwei.btw@antgroup.com> wrote:
> Since we don't have port I/O support, we should turn HAS_IOPORT_MAP
> off as well. It will fix clang warnings like below:
>
> ./include/asm-generic/io.h:1175:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
>  1175 |         return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
>       |                                                   ~~~~~~~~~~ ^
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202506271049.MtzHqdSd-lkp@intel.com/
> Fixes: b8c9c3b822fe ("um: stop using PCI port I/O")
> Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
> ---
> Strictly speaking, the -Wnull-pointer-arithmetic warnings reported
> by lkp has been fixed by commit b8c9c3b822fe ("um: stop using PCI
> port I/O"). But it introduced new -Wnull-pointer-arithmetic warnings,
> and this patch fixes the newly introduced warnings.
>
>  arch/um/Kconfig | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/um/Kconfig b/arch/um/Kconfig
> index f08e8a7fac93..76aa954520b1 100644
> --- a/arch/um/Kconfig
> +++ b/arch/um/Kconfig
> @@ -54,6 +54,9 @@ config UML_IOMEM_EMULATION
>         select INDIRECT_IOMEM
>         select GENERIC_PCI_IOMAP
>
> +config NO_IOPORT_MAP
> +       def_bool y
> +
>  config ISA
>         bool
>

Re: [PATCH] um: Turn HAS_IOPORT_MAP off

[Arnd Bergmann]

On Tue, Jul 1, 2025, at 10:26, Geert Uytterhoeven wrote:
> CC arnd
>
> On Sat, 28 Jun 2025 at 18:48, Tiwei Bie <tiwei.btw@antgroup.com> wrote:
>> Since we don't have port I/O support, we should turn HAS_IOPORT_MAP
>> off as well. It will fix clang warnings like below:
>>
>> ./include/asm-generic/io.h:1175:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
>>  1175 |         return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
>>       |                                                   ~~~~~~~~~~ ^
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202506271049.MtzHqdSd-lkp@intel.com/
>> Fixes: b8c9c3b822fe ("um: stop using PCI port I/O")
>> Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
>> ---

I think this one would be a more generic solution, does that work for you?

--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -381,7 +381,7 @@ config HAS_IOPORT
 config HAS_IOPORT_MAP
        bool
        depends on HAS_IOMEM && !NO_IOPORT_MAP
-       default y
+       default HAS_IOPORT
 
 source "kernel/dma/Kconfig"
 
It seems silly to turn off HAS_IOPORT_MAP on platforms that don't
have HAS_IOPORT to start with.

      Arnd

Re: [PATCH] um: Turn HAS_IOPORT_MAP off

[Tiwei Bie]

On 2025/7/1 16:43, Arnd Bergmann wrote:
> On Tue, Jul 1, 2025, at 10:26, Geert Uytterhoeven wrote:
>> CC arnd

Thanks for CCing Arnd. I missed that.

>>
>> On Sat, 28 Jun 2025 at 18:48, Tiwei Bie <tiwei.btw@antgroup.com> wrote:
>>> Since we don't have port I/O support, we should turn HAS_IOPORT_MAP
>>> off as well. It will fix clang warnings like below:
>>>
>>> ./include/asm-generic/io.h:1175:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
>>>  1175 |         return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
>>>       |                                                   ~~~~~~~~~~ ^
>>>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Closes: https://lore.kernel.org/oe-kbuild-all/202506271049.MtzHqdSd-lkp@intel.com/
>>> Fixes: b8c9c3b822fe ("um: stop using PCI port I/O")
>>> Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
>>> ---
> 
> I think this one would be a more generic solution,

+1

> does that work for you?

Yeah, that works for me. :)

> 
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -381,7 +381,7 @@ config HAS_IOPORT
>  config HAS_IOPORT_MAP
>         bool
>         depends on HAS_IOMEM && !NO_IOPORT_MAP
> -       default y
> +       default HAS_IOPORT
>  
>  source "kernel/dma/Kconfig"
>  
> It seems silly to turn off HAS_IOPORT_MAP on platforms that don't
> have HAS_IOPORT to start with.

I'm not entirely clear on the meaning of HAS_IOPORT though. It looks
HAS_IOPORT was introduced to support disabling compilation of the I/O
accessor functions inb()/outb() and friends [1]. And looking at the
code, xtensa supports IOPORT_MAP [2][3], but doesn't select HAS_IOPORT
(which is also stated in the commit log in [1]).

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fcbfe8121a45152a3cfbe1c28c96a3b611b7347d
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/xtensa/include/asm/io.h?h=v6.16-rc4#n25
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/xtensa/Kconfig?h=v6.16-rc4#n81

Regards,
Tiwei

Re: [PATCH] um: Turn HAS_IOPORT_MAP off

[Arnd Bergmann]

On Tue, Jul 1, 2025, at 14:22, Tiwei Bie wrote:
> On 2025/7/1 16:43, Arnd Bergmann wrote:
>> On Tue, Jul 1, 2025, at 10:26, Geert Uytterhoeven wrote:
>> --- a/lib/Kconfig
>> +++ b/lib/Kconfig
>> @@ -381,7 +381,7 @@ config HAS_IOPORT
>>  config HAS_IOPORT_MAP
>>         bool
>>         depends on HAS_IOMEM && !NO_IOPORT_MAP
>> -       default y
>> +       default HAS_IOPORT
>>  
>>  source "kernel/dma/Kconfig"
>>  
>> It seems silly to turn off HAS_IOPORT_MAP on platforms that don't
>> have HAS_IOPORT to start with.
>
> I'm not entirely clear on the meaning of HAS_IOPORT though. It looks
> HAS_IOPORT was introduced to support disabling compilation of the I/O
> accessor functions inb()/outb() and friends [1].

Correct

> And looking at the
> code, xtensa supports IOPORT_MAP [2][3], but doesn't select HAS_IOPORT
> (which is also stated in the commit log in [1]).

xtensa selects NO_IOPORT_MAP, which means that HAS_IOPORT_MAP
gets disabled here:

arch/xtensa/Kconfig:config NO_IOPORT_MAP
arch/xtensa/Kconfig-    def_bool n

The NO_IOPORT_MAP symbol is a bit confusing, but I think it
works as intended, at least on xtensa. There are probably
still some platforms that started selecting NO_IOPORT_MAP
because they have no actual I/O port implementation, but
that still need to be updated to not select HAS_IOPORT.

With my patch above, we should probably stop selecting
NO_IOPORT_MAP on platforms that don't also set HAS_IOPORT,
and keep the HAS_IOPORT_MAP/NO_IOPORT_MAP logic around
only for the few cases that cannot abstract inb()/outb()
through ioread8()/iowrite8(), i.e. Arm RiscPC, m68k q40,
etc.

      Arnd

Re: [PATCH] um: Turn HAS_IOPORT_MAP off

[Tiwei Bie]

On 2025/7/2 19:09, Arnd Bergmann wrote:
> On Tue, Jul 1, 2025, at 14:22, Tiwei Bie wrote:
>> On 2025/7/1 16:43, Arnd Bergmann wrote:
>>> On Tue, Jul 1, 2025, at 10:26, Geert Uytterhoeven wrote:
>>> --- a/lib/Kconfig
>>> +++ b/lib/Kconfig
>>> @@ -381,7 +381,7 @@ config HAS_IOPORT
>>>  config HAS_IOPORT_MAP
>>>         bool
>>>         depends on HAS_IOMEM && !NO_IOPORT_MAP
>>> -       default y
>>> +       default HAS_IOPORT
>>>  
>>>  source "kernel/dma/Kconfig"
>>>  
>>> It seems silly to turn off HAS_IOPORT_MAP on platforms that don't
>>> have HAS_IOPORT to start with.
>>
>> I'm not entirely clear on the meaning of HAS_IOPORT though. It looks
>> HAS_IOPORT was introduced to support disabling compilation of the I/O
>> accessor functions inb()/outb() and friends [1].
> 
> Correct
> 
>> And looking at the
>> code, xtensa supports IOPORT_MAP [2][3], but doesn't select HAS_IOPORT
>> (which is also stated in the commit log in [1]).
> 
> xtensa selects NO_IOPORT_MAP, which means that HAS_IOPORT_MAP
> gets disabled here:
> 
> arch/xtensa/Kconfig:config NO_IOPORT_MAP
> arch/xtensa/Kconfig-    def_bool n

The value of NO_IOPORT_MAP on xtensa is inverted compared to that
on most other platforms that explicitly defines NO_IOPORT_MAP.

When NO_IOPORT_MAP is explicitly defined, it is typically set to 'y',
which disables HAS_IOPORT_MAP [1]. However, on xtensa, NO_IOPORT_MAP
is set to 'n' [2], thereby enabling HAS_IOPORT_MAP.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/Kconfig?h=v6.16-rc4#n468
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/xtensa/Kconfig?h=v6.16-rc4#n82

Regards,
Tiwei

Re: [PATCH] um: Turn HAS_IOPORT_MAP off

[Arnd Bergmann]

On Wed, Jul 2, 2025, at 15:02, Tiwei Bie wrote:
> On 2025/7/2 19:09, Arnd Bergmann wrote:
>> On Tue, Jul 1, 2025, at 14:22, Tiwei Bie wrote:
>> xtensa selects NO_IOPORT_MAP, which means that HAS_IOPORT_MAP
>> gets disabled here:
>> 
>> arch/xtensa/Kconfig:config NO_IOPORT_MAP
>> arch/xtensa/Kconfig-    def_bool n
>
> The value of NO_IOPORT_MAP on xtensa is inverted compared to that
> on most other platforms that explicitly defines NO_IOPORT_MAP.

Ah of course, sorry for misunderstanding. Apparently this only
existed because it was selected by a long gone platform:

-config XTENSA_PLATFORM_S6105
-       bool "S6105"
-       select HAVE_IDE
-       select SERIAL_CONSOLE
-       select NO_IOPORT_MAP

Before commits

d38efc1f150f ("xtensa: adopt generic io routines")
d046f77ec4a4 ("xtensa: set NO_IOPORT to 'n' by default")

it was always-on, and I think the idea at the time was to
make it behave more like the generic PCI code, while it
was originally copied from sparc, which does things quite
differently.

It is rather clear that the I/O port implementation on
xtensa never actually worked, and some of the comments
on its PCI code suggest that it was never really used
either.

    Arnd

Re: [PATCH] um: Turn HAS_IOPORT_MAP off

[Tiwei Bie]

On 2025/7/2 21:39, Arnd Bergmann wrote:
> On Wed, Jul 2, 2025, at 15:02, Tiwei Bie wrote:
>> On 2025/7/2 19:09, Arnd Bergmann wrote:
>>> On Tue, Jul 1, 2025, at 14:22, Tiwei Bie wrote:
>>> xtensa selects NO_IOPORT_MAP, which means that HAS_IOPORT_MAP
>>> gets disabled here:
>>>
>>> arch/xtensa/Kconfig:config NO_IOPORT_MAP
>>> arch/xtensa/Kconfig-    def_bool n
>>
>> The value of NO_IOPORT_MAP on xtensa is inverted compared to that
>> on most other platforms that explicitly defines NO_IOPORT_MAP.
> 
> Ah of course, sorry for misunderstanding. Apparently this only
> existed because it was selected by a long gone platform:
> 
> -config XTENSA_PLATFORM_S6105
> -       bool "S6105"
> -       select HAVE_IDE
> -       select SERIAL_CONSOLE
> -       select NO_IOPORT_MAP

Cool, that makes sense!

> 
> Before commits
> 
> d38efc1f150f ("xtensa: adopt generic io routines")
> d046f77ec4a4 ("xtensa: set NO_IOPORT to 'n' by default")
> 
> it was always-on, and I think the idea at the time was to
> make it behave more like the generic PCI code, while it
> was originally copied from sparc, which does things quite
> differently.
> 
> It is rather clear that the I/O port implementation on
> xtensa never actually worked, and some of the comments
> on its PCI code suggest that it was never really used
> either.

Thanks for the details. I'm not really familiar with xtensa;
my understanding comes merely from reading the code. Please
feel free to proceed with your patch. I also think it is a
more generic solution. :)

Regards,
Tiwei