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:	Wed, 06 Feb 2013 10:07:15 +1100
From:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:	Alexey Brodkin <Alexey.Brodkin@...opsys.com>
Cc:	Michal Simek <monstr@...str.eu>, Arnd Bergmann <arnd@...db.de>,
	Vineet Gupta <Vineet.Gupta1@...opsys.com>,
	linux-kernel@...r.kernel.org, grant.likely@...retlab.ca,
	alan@...rguk.ukuu.org.uk, 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 01:25 +0400, Alexey Brodkin wrote:
> Sounds good but how should one tell which approach is correct? For 
> example here - is the one implemented by Xilinx is golden reference or
> not?

So I'm reading that PDF you pointed to. So far what I can see is:

 - In 8-bit mode you only do 8-bit accesses, so endianness should be
totally irrelevant (at least the pdf says so)

 - In 16-bit mode, that's where things become interesting... the doc
says:

  PLB Data Bus 		| System ACE Data Bus
  ----------------------+--------------------
  PLB_DBus[8 : 15]	| SysACE_MPD[15 : 8]
  PLB_DBus[0 : 7] 	| SysACE_MPD[7 : 0]

Now, I'm not 100% of the bit numbering used by Xilinx here but it smells
like PLB used ppc numbering and SystemACE use the classic numbering, in
which case the above would mean that the MSB of the PLB is connected to
the LSB of the SystemACE and vice-versa.

If that is the case then this is the *correct* wiring and means that the
data port (if any) doesn't need any byteswapping. It also means that the
registers need byteswap on BE, as expected for a LE device. IE. Just
always use ioread32 (or _rep variants for the data port if there's such
a thing on it).

So that looks good... unless I misunderstood the Xilinx spec, this looks
like the right way to do and the only one we should support.

The various other mentions in the text of "bit swapping" vs "byte swapping"
and reference to SW swapping are all just very confused which makes me
think that while the wiring up ended up being correct, this is in part
by accident (or whoever wrote the doc didn't understand what this is
all about).

So is there a known implementation that gets it backward ? Unless it's
a very popular and very useful one to support I would advocate just
not doing so.

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