linux-kernel - Re: Question about regmap_range_cfg and regmap

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]

Message-ID: <61cb178b-dc9b-12fb-0443-d38e0c43e046@plating.de>
Date:   Wed, 4 Mar 2020 14:28:25 +0100
From:   Lars Möllendorf <lars.moellendorf@...ting.de>
To:     Mark Brown <broonie@...nel.org>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: Question about regmap_range_cfg and regmap_mmio

On 04.03.20 13:03, Mark Brown wrote:
> On Wed, Mar 04, 2020 at 12:25:09PM +0100, Lars Möllendorf wrote:
> 
>> this mail is copied from internal issue written in markdown - I hope
>> this is still readable as mail.
> 
> Not really frankly.  
Ok, sorry. Here the same text without code snippets and links:

In `__regmap_init()` `_regmap_bus_reg_read()`is assigned to
`regmap.reg_read()` if there are no `bus->read/write` functions, else
`_regmap_bus_read()` is assigned.

`_regmap_bus_reg_read()` calls the `reg_read` function of the bus
directly, `_regmap_bus_read()` instead calls `_regmap_raw_read()`.

`_regmap_raw_read()` in turn calls `_regmap_range_lookup()` and
`_regmap_select_page()` which do the paging.

`regmap_mmio` does not contain `bus->read/write`, but does contain
`bus->reg_read/reg_write` only.

>> `regmap_i2c` does -contain both-.

Sorry, I have to correct myself here. It does only contain the
`bus->read/write` functions.

> I *think* you are saying that paging doesn't work
> due to relying on having register read and write operations?

If I understand correctly it is relying on having plain
`bus->read/write` operations. MMIO *has* `bus->reg_read/reg_write` but
is missing the former. But maybe I just mix up the wording here.

>> My assumption is that paging is not a common use case for Memory-mapped
>> I/O and thus has not been implemented for this case.
> 
>> - Are my assumptions correct?
>> - If so, what would you recommend me to do:
>>   - Continue using `regmap-mmio` and implement my custom paging
>> functions on top of that?
> 
> This will obviously work.

Yes, this works. But it comes at the cost of implementing (and
maintaining) something which already exists. And the existing
implementation which uses virtual memory addresses is much smarter than
my current implementation.

>>   - Enhance the current `regmap-mmio` implementation so it does paging
>> and submit a patch?
> 
> That's not really possible since MMIO never writes the register address
> to the bus

Sorry, but I do not get why this shouldn't work with MMIO? If I
understood the code correctly in  `_regmap_raw_read` the address is
checked before it is used anywhere. If
`_regmap_range_lookup()` returns a range it does the paging, i.e.
translates the virtual address into the real address
(`_regmap_select_page`). If so the real address is passed to
`bus->read/write`, else the given address is used directly. Do I miss
something here?

>>   - Write my own `better-regmap-mmio` implementation?
> 
> It's not clear what that would mean.

Maybe for some reason the current MMIO implementation should not be
touched, or paging for MMIO is not wanted?

> You could also look into making the paging code not rely on explicit
> register read and write operations.

Maybe it is sufficient to implement `bus->read/write` as a wrapper of
`bus->reg_read/reg_write` in regmap-mmio.c?