linux-kernel - Re: [PATCH] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)

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]

Message-ID: <51135F43.4030209@monstr.eu>
Date:	Thu, 07 Feb 2013 09:01:07 +0100
From:	Michal Simek <monstr@...str.eu>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
CC:	Arnd Bergmann <arnd@...db.de>,
	Grant Likely <grant.likely@...retlab.ca>,
	Alexey Brodkin <Alexey.Brodkin@...opsys.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Vineet Gupta <Vineet.Gupta1@...opsys.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	dahinds@...rs.sourceforge.net
Subject: Re: [PATCH] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16
 with generic iowrite(read)8/16(be)

On 02/07/2013 08:38 AM, Geert Uytterhoeven wrote:
> On Thu, Feb 7, 2013 at 8:23 AM, Michal Simek <monstr@...str.eu> wrote:
>>>> #define iowrite16be(v, addr)   iowrite16(be16_to_cpu(v), (addr))
>>>> #define iowrite16(v, addr)      writew((v), (addr))
>>>> #define writew(b,addr) __raw_writew(__cpu_to_le16(b),addr)
>>>>
>>>> static inline void __raw_writew(u16 b, volatile void __iomem *addr)
>>>> {
>>>>          *(volatile u16 __force *) addr = b;
>>>> }
>>>>
>>>> How is this suppose to work on Big Endian?
>>>> be16_to_cpu(v) is (v)
>>>> and
>>>> __cpu_to_le16(b) is swab16(v)
>>>
>>> Yes.
>>
>> But on native BE system ( I expect that v is in big endian)
>> iowrite16be(v, addr) should be just *(volatile u16 __force *) addr =
>> v; not *(volatile u16 __force *) addr = swab16(v);
>
>>>> What I would expect is
>>>> #define iowrite16be(v, addr)   __raw_writew(__cpu_to_be16(v), addr)
>>>
>>> Indeed, it should be "__cpu_to_be16(v)" instead of "be16_to_cpu(v)".
>>
>> What do you mean by that?
>
> Bummer, I missed that current iowrite16be() uses (the little endian)
> iowrite16(),
> not _raw_writew(), and thought the only difference between the original
> and your version was the endianness conversion macro.
>
> Yes,
>
>      #define iowrite16be(v, addr)   __raw_writew(__cpu_to_be16(v), addr)
>
> should be correct.

ok. Can you please confirm with me that the same problem is also for iowrite32be
ioread16be and ioread32be?

This description seems to me correct for BE and LE.
#define ioread16be(addr)       __be16_to_cpu(__raw_readw(addr))
#define ioread32be(addr)       __be32_to_cpu(__raw_readl(addr))
#define iowrite16be(v, addr)   __raw_writew(__cpu_to_be16(v), addr)
#define iowrite32be(v, addr)   __raw_writel(__cpu_to_be32(v), addr)

What do you think?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/