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]

Date:	Thu, 07 Feb 2013 08:35:50 +1100
From:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:	Grant Likely <grant.likely@...retlab.ca>
Cc:	Alexey Brodkin <Alexey.Brodkin@...opsys.com>,
	Michal Simek <monstr@...str.eu>, Arnd Bergmann <arnd@...db.de>,
	Vineet Gupta <Vineet.Gupta1@...opsys.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	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 Wed, 2013-02-06 at 10:14 +0000, Grant Likely wrote:
> 
> Huh? That makes no sense. This device out in the wild with both big
> and little endian bus attachments. You can argue all day that one of
> them is wrong, but it doesn't matter. It exists, is used, and must be
> supported.

No. That's where you are VERY wrong. We don't have to support crap and
arguably shouldn't if that can give an incentive to vendors to fix their
stuff. If you don't believe me, ask Linus :-)

>  In fact, the driver already knows about this and figures
> out at runtime how the device is wired up to the bus. This is not the
> problem.

Except that this is very gross, especially when you observe that in the
busted "big endian" case, it has to byteswap the bloody data port.

So you end up having to do that gross hack with separate accessors for
registers vs. data and not able to use the _rep variants, which also
means that on platforms like ppc, you end up with a memory barrier on
every access (or more), which is going to slow things down enormously.

> BTW, that document describes only one of the systemace bus
> attachments. There is a different on used on Microblaze little-endian,
> and some boards have the SystemACE directly wired to the SoC external
> bus (no adapter IP).
> 
> The only problem that I see is that the ARM and Microblaze
> ioread16be/iowrite16be helpers are missing barriers which smells like
> a bug and should be fixed.
> 
> Michal, have you tested Alexey's patch? If it works for you then I'm
> comfortable with acking it. It looks correct to me.

No, the real problem is that the "big endian" wiring is totally busted
and the HW guys who came with it must be taught a lesson. Not supporting
that crap might be one way to do it.

Ben.

--
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/