linux-kernel - Re: [PATCH 4/4] sh: machvec: remove custom ioport

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 13 Sep 2023 14:32:38 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>,
        linux-sh@...r.kernel.org, Rich Felker <dalias@...c.org>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        linux-kernel@...r.kernel.org, Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH 4/4] sh: machvec: remove custom ioport_{un,}map()

Hi Arnd,

On Wed, Aug 2, 2023 at 8:49 PM Arnd Bergmann <arnd@...nel.org> wrote:
> From: Arnd Bergmann <arnd@...db.de>
>
> These functions were only used on the microdev
> board that is now gone, so remove them to simplify
> the ioport handling.
>
> This could be further simplified to use the generic
> I/O port accessors now.
>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>

Thanks for your patch!

> --- a/arch/sh/include/asm/io.h
> +++ b/arch/sh/include/asm/io.h
> @@ -181,7 +181,7 @@ static inline void pfx##out##bwlq##p(type val, unsigned long port)  \
>  {                                                                      \
>         volatile type *__addr;                                          \
>                                                                         \
> -       __addr = __ioport_map(port, sizeof(type));                      \
> +       __addr = (void __iomem *)sh_io_port_base + port;                \

Note that this adds unconditional users of sh_io_port_base, while
sh_io_port_base is provided by arch/sh/kernel/ioport.c, which is
currently only built if CONFIG_GENERIC_IOMAP=n.

This is not a problem yet, as the final part to enable GENERIC_IOMAP
on SH never made it upstream.  However, Sato-san's series enables
GENERIC_IOMAP for SH_DEVICE_TREE=y builds, leading to a link failure.

A quick fix would be to always build the relevant parts:

--- a/arch/sh/kernel/Makefile
+++ b/arch/sh/kernel/Makefile
@@ -23,8 +23,8 @@ obj-y := head_32.o debugtraps.o dumpstack.o
                 \

 ifndef CONFIG_GENERIC_IOMAP
 obj-y                          += iomap.o
-obj-$(CONFIG_HAS_IOPORT_MAP)   += ioport.o
 endif
+obj-$(CONFIG_HAS_IOPORT_MAP)   += ioport.o

 obj-y                          += sys_sh32.o
 obj-y                          += cpu/
--- a/arch/sh/kernel/ioport.c
+++ b/arch/sh/kernel/ioport.c
@@ -12,6 +12,7 @@
 unsigned long sh_io_port_base __read_mostly = -1;
 EXPORT_SYMBOL(sh_io_port_base);

+#ifndef CONFIG_GENERIC_IOMAP
 void __iomem *ioport_map(unsigned long port, unsigned int nr)
 {
        void __iomem *ret;
@@ -28,3 +29,4 @@ void ioport_unmap(void __iomem *addr)
 {
 }
 EXPORT_SYMBOL(ioport_unmap);
+#endif /* !CONFIG_GENERIC_IOMAP */

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

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