linux-kernel - Re: Page select register restrictions in regmap core

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: <e6733f56-014e-4ea0-aaf8-059334f2b27f@roeck-us.net>
Date: Mon, 17 Jun 2024 16:15:33 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Mark Brown <broonie@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 linux-kernel@...r.kernel.org, Armin Wolf <W_Armin@....de>
Subject: Re: Page select register restrictions in regmap core

On 6/17/24 15:47, Mark Brown wrote:
> On Mon, Jun 17, 2024 at 02:55:09PM -0700, Guenter Roeck wrote:
>> On 6/17/24 10:22, Mark Brown wrote:
> 
>>> The range is *entirely* defined within the driver, it is 100% a software
>>> construct, the hardware only influences our choice of range in that we
>>> can't place it on top of hardware registers.
> 
>> I _think_ what you are saying is that I'd have to model all registers
>> which are to be addressed through regmap as virtual registers with an offset
>> outside the range of real registers. Something like adding 0x100 to the
> 
> No, only registers that are accessed through a window need to be
> mapped into a range.  Any other registers can just be accessed.
> 
See below.

>> each register address and then accessing, say, the revision register
>> not as register 0x02 but as register 0x102. I would then define the matching
>> range from 0x100 .. 0x17f and the window from 0x00..0x7f.
> 
> That would make the range exactly the same size as the window so there'd
> be no paging going on and the registers could be accessed directly?  I
> guess that's another check that should be added...
> 

I tried to explain this before. The registers in address range 00..0x7f
are physical, but they are only accessible from page 0 with the exception
of the page select register. So, sure, the registers are not actually paged,
but page 0 must be selected to access them. That is the one and only reason
for specifying that first range and window. It ensures that page 0 is
selected when accessing the registers. If that wasn't the case, I could
define a single range for the actually paged addresses in the 0x80..0xff
window and be done with it.

>> Hmm, yes, I see that this should work. I don't think it is worth doing though
>> since I need to be able to access some registers outside regmap, and I'd have
>> to define two sets of addresses for all those registers. That would simplify
>> the code a bit but one would have to remember that register addresses through
>> regmap are different than register addresses when calling smbus functions
>> directly. I think we'll just stick with the current code and keep the paging
>> implementation in the driver.
> 
> Mixing regmap and non-regmap access to the same registers seems like a
> bad idea in general, you will have locking issues (especially around the
> paging).

The non-regmap access all happens in the probe function before regmap is
initialized. It is needed for basic chip identification, to prevent someone
from instantiating the driver on a random nvram/eeprom and messing it up
with attempts to write the page select register. I would not want to be
held responsible for someone with, say, DDR4 DIMMs force-instantiating
the spd5118 driver and then complaining about bricked DIMMs.

Thanks,
Guenter