netdev - Re: [PATCH 2/3] net: socionext: Add Synquacer NetSec driver

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, 30 Nov 2017 17:48:44 +0000
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     David Miller <davem@...emloft.net>
Cc:     "Williams, Dan J" <dan.j.williams@...el.com>,
        Will Deacon <will.deacon@....com>,
        Jassi Brar <jassisinghbrar@...il.com>,
        "<netdev@...r.kernel.org>" <netdev@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Arnd Bergmann <arnd.bergmann@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Jaswinder Singh <jaswinder.singh@...aro.org>
Subject: Re: [PATCH 2/3] net: socionext: Add Synquacer NetSec driver

On 30 November 2017 at 17:42, David Miller <davem@...emloft.net> wrote:
> From: Ard Biesheuvel <ard.biesheuvel@...aro.org>
> Date: Thu, 30 Nov 2017 17:37:27 +0000
>
>> Well, the whole point of using memremap() instead of ioremap() is that
>> the region has memory semantics, i.e., we read the MAC address and the
>> DMA engine microcode from it. If memremap() itself is flawed in this
>> regard, I agree we should fix it. But as I understand it, this is
>> really an implementation problem in memremap() [the fact that it falls
>> back to ioremap()] and not a problem in this driver.
>>
>> So what alternative would you propose in this case?
>>
>> BTW, this should be IOREMAP_WC not IOREMAP_WT, because the EEPROM on
>> the platform in question does not tolerate cached mappings (or rather,
>> shareable mappings). ioremap_wt() happens to result in device mappings
>> rather than writethrough cacheable mappings, but this is another
>> problem in itself. Once arm64 fixes ioremap_wt(), this code will no
>> longer work on the SynQuacer SoC.
>
> It doesn't "fall back", it directly uses ioremap_wt() for non-RAM
> mappings.
>
> It you look, most architectures do a "#define iomrep_wt ioremap"

OK, but that still means the implementation of memremap() is flawed,
not its use in this driver.

memremap() exists to allow non-DRAM regions to be mapped with memory
semantics, and if we currently implement that incorrectly, we should
fix it. But that still does not mean we should have __iomem
annotations and special accessors in this case, precisely because it
has memory semantics, and so it is a virtual pointer that may be
dereferenced normally.

So if memremap() is unsuitable, how should we map memory that is not DRAM?