linux-kernel - Re: [PATCH] m68k: io: Fix io{read,write}{16,32}be() for Coldfire peripherals

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] [day] [month] [year] [list]

Date:   Mon, 17 Jun 2019 22:23:45 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Greg Ungerer <gerg@...ux-m68k.org>,
        Angelo Dureghello <angelo@...am.it>,
        Logan Gunthorpe <logang@...tatee.com>,
        linux-m68k <linux-m68k@...ts.linux-m68k.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] m68k: io: Fix io{read,write}{16,32}be() for Coldfire peripherals

On Fri, Jun 14, 2019 at 4:52 PM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> On Mon, Apr 29, 2019 at 2:40 PM Arnd Bergmann <arnd@...db.de> wrote:
> > On Mon, Apr 29, 2019 at 10:19 AM Geert Uytterhoeven
> > This looks correct to me, but there are two points that stick out to me:
> >
> > - why do you only do this for mmio and not for pio?
>
> Because no one had a need for it? ;-)
>
> Now seriously, m68k only has MMIO, no PIO.  Any PIO, if used, is for ISA or
> PCMCIA I/O accesses, which are little endian.

I suppose the majority of PIO operations are single-byte only,
so endianess doesn't apply at all ;-). You are certainly right
that big-endian PIO access has close to zero uses in the
kernel (I could not find any either).

If you don't have ISA or PCMCIA devices, then using the
trivial readl/writel based wrappers is clearly the easiest way.

If you do have PIO based devices, then defining pio_read16be
etc would save a few bytes in lib/iomap.c.

> > - why do you even use the generic_iomap wrappers rather than
> >   the trivial asm-generic versions of those functions?
>
> Looking at git history, that was done to fix some link errors, which no
> longer seem to happen with the more mature asm-generic infrastructure we
> have now.
>
> So probably we can drop the selection of GENERIC_IOMAP and inclusion
> of <asm-generic/iomap.h>, after fixing a few compiler warnings like:
>
>     include/asm-generic/io.h: In function ‘ioread8_rep’
>     arch/m68k/include/asm/io_mm.h:371:44: warning: passing argument 1
> of ‘raw_insb’ discard ‘const’ qualifier from pointer target type
> [-Wdiscarded-qualifiers]
>      #define readsb(port, buf, nr)     raw_insb((port), (u8 *)(buf), (nr))
>     arch/m68k/include/asm/raw_io.h:101:50: note: expected ‘volatile u8
> *’ {aka ‘volatile unsigned char *’} but argument is of type ‘const
> volatile void *’
>     static inline void raw_insb(volatile u8 __iomem *port, u8 *buf,
> unsigned int len)

Those should just be missing 'volatile' and 'const' modifiers
on the arguments.

         Arnd