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 11:48:18 -0700
From:	Dan Williams <dan.j.williams@...el.com>
To:	Thomas Hellstrom <thellstrom@...are.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 Tue, Oct 13, 2015 at 11:37 AM, Thomas Hellstrom
<thellstrom@...are.com> wrote:
> 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,

Ah, sorry, I was looking at the default case...

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

Ok, I'll make local read_fifo() and write_fifo() macros to make this
explicit.  Are these names ok with you?
--
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/