linux-kernel - Re: [PATCH 2/2] media: verisilicon: change confusingly named relaxed register access

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <300ecb3e-8747-4399-8452-77724f85330b@app.fastmail.com>
Date:   Mon, 19 Jun 2023 21:26:34 +0200
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Nicolas Dufresne" <nicolas.dufresne@...labora.com>,
        "Arnd Bergmann" <arnd@...nel.org>,
        "Ezequiel Garcia" <ezequiel@...guardiasur.com.ar>,
        "Philipp Zabel" <p.zabel@...gutronix.de>,
        "Mauro Carvalho Chehab" <mchehab@...nel.org>
Cc:     "Hans Verkuil" <hverkuil-cisco@...all.nl>,
        "Benjamin Gaignard" <benjamin.gaignard@...labora.com>,
        "Jernej Skrabec" <jernej.skrabec@...il.com>,
        linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] media: verisilicon: change confusingly named relaxed register
 access

On Mon, Jun 19, 2023, at 20:29, Nicolas Dufresne wrote:
> Le lundi 19 juin 2023 à 16:49 +0200, Arnd Bergmann a écrit :
>> > 
>> > In this text you spoke about potential performance side effects of existing code
>> > and your changes, but its left all very vague and theoretical. Have you done any
>> > measurement ? Do you need help with the manner ?
>> 
>> I don't have this hardware and have not done any measurements.
>> Obviously the only point of using relaxed accessors is to
>> improve performance in critical code paths, but from the way they
>> are used here it seems that this was instead just an accident
>> and nobody else did any comparisons either.
>> 
>> My guess would be that if one wanted to speed up the register
>> access, a better way would be to use a regmap cache to avoid
>> reading registers when the contents are already known.
>
> All I know is that for the majority of registers when programming stateless
> codecs, each 32bit word of registers are fully written too, the read value is
> not always meaningful (its a value from last time the HW has been triggered) and
> should be ignored, so better to not do that. As for regmap, there is folks that
> have reported regmap to be completely overkill for this type of hardware.

Right, most likely neither the cache nor avoiding the readl() is necessary,
and that was exactly my point to start with: don't add potentially dangerous
microoptimizations like relaxed accessors unless the obvious optimizations
are also needed and used.

Obviously, testing my patch would still be a good idea before applying it.

       Arnd