linux-kernel - Re: [PATCH] drm/vmwgfx: switch from ioremap

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:	Tue, 13 Oct 2015 20:37:25 +0200
From:	Thomas Hellstrom <thellstrom@...are.com>
To:	Dan Williams <dan.j.williams@...el.com>
CC:	David Airlie <airlied@...ux.ie>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	<dri-devel@...ts.freedesktop.org>, Sinclair Yeh <syeh@...are.com>
Subject: Re: [PATCH] drm/vmwgfx: switch from ioremap_cache to memremap

On 10/13/2015 06:35 PM, Dan Williams wrote:
> On Mon, Oct 12, 2015 at 10:18 PM, Thomas Hellstrom
> <thellstrom@...are.com> wrote:
>> Hi!
>>
>> On 10/13/2015 12:35 AM, Dan Williams wrote:
>>> Per commit 2e586a7e017a "drm/vmwgfx: Map the fifo as cached" the driver
>>> expects the fifo registers to be cacheable.  In preparation for
>>> deprecating ioremap_cache() convert its usage in vmwgfx to memremap().
>>>
>>> Cc: David Airlie <airlied@...ux.ie>
>>> Cc: Thomas Hellstrom <thellstrom@...are.com>
>>> Cc: Sinclair Yeh <syeh@...are.com>
>>> Cc: dri-devel@...ts.freedesktop.org
>>> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
>> While I have nothing against the conversion, what's stopping the
>> compiler from reordering writes on a generic architecture and caching
>> and reordering reads on x86 in particular? At the very least it looks to
>> me like the memory accesses of the memremap'd memory needs to be
>> encapsulated within READ_ONCE and WRITE_ONCE.
> Hmm, currently the code is using ioread32/iowrite32 which only do
> volatile accesses, whereas READ_ONCE / WRITE_ONCE have a memory
> clobber on entry and exit.  So, I'm assuming all you need is the
> guarantee of "no compiler re-ordering" and not the stronger
> READ_ONCE/WRITE_ONCE guarantees, but that still seems broken compared
> to explicit fencing where it matters.

I'm not quite sure I follow you here, it looks to me like READ_ONCE()
and WRITE_ONCE() are implemented as
volatile accesses,

http://lxr.free-electrons.com/source/include/linux/compiler.h#L215

just like ioread32 and iowrite32

http://lxr.free-electrons.com/source/include/asm-generic/io.h#L54

which would minimize any potential impact of this change.
IMO optimizing the memory accesses can be done as a later step.

/Thomas





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