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: <78c93d6b-af0e-4d96-b213-e1e402524361@roeck-us.net>
Date: Mon, 17 Jun 2024 09:59:50 -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

Hi Mark,

On Mon, Jun 17, 2024 at 04:08:08PM +0100, Mark Brown wrote:
> On Mon, Jun 17, 2024 at 07:20:28AM -0700, Guenter Roeck wrote:
> 
> >         .window_start     = 0,
> >         .window_len       = SPD5118_PAGE_SIZE,			// 128
> >         .range_min        = 0,
> >         .range_max        = SPD5118_PAGE_SIZE - 1,		// 127
> 
> 
> >         .window_start     = SPD5118_PAGE_SIZE,			// 128
> >         .window_len       = SPD5118_PAGE_SIZE,			// 128
> >         .range_min        = SPD5118_PAGE_SIZE,			// 128
> >         .range_max        = 9 * SPD5118_PAGE_SIZE - 1,		// 0x47f
> 
> You appear to be trying to define ranges that overlap with the windows
> that you're trying to expose.  I can't understand what that's trying to
> represent or how that would work.  The window is the physical registers
> that the host can actually see, the range is the virtual addresses which
> users of the region should use to access registers behind the window.
> This should be a range of register values which don't physically exist
> on the device.  I really can't understand what a sensible handling of an

Can you point me to an example ? All examples I can find have overlapping
values for .range_min/.range_max and .window_start/.window_len, and pretty
much all of them start range_min with 0.

> overlap would be, any attempt to access the window should recursively
> trigger selection of the range so no actual register should work.  I
> can't tell what it's trying to model.
> 

page 0: 0x00-0x7f	Volatile registers, page selector at 0x0b
	0x80-0xff	page 0 of non-volatile memory
page 1:	0x0b		page selector register	<-- this is what trips the check
	0x80-0xff	page 1 of non-volatile memory
...
page 7:	0x0b		page selector register
	0x80-0xff	page 7 of non-volatile memory

> This configuration would also be rejected by the next test which
> verifies that the window does not overlap with the range.
> 

No, it isn't. The windows in the two ranges don't overlap, and neither
do the ranges. The only overlap is the selector register. The check you
refer to explicitly does not apply to a single range.

> > This works just fine if I comment out the select register validation in
> > the regmap core (the one which generates the error). Is there a reason
> > for having this restriction, or would it be possible to drop it ?
> 
> I'm very surprised this does anything useful TBH, possibly that's a bug
> of some kind.  A configuration with the selector within the range is
> nonsensical as far as I can see since the range can't be accessed
> without programming the selector, the range should be purely virtual
> registers and the selector must be a physical register.

Pretty much all drivers I looked at start the range with 0, having
the selector register within the range is explicitly accepted by the
regmap code, and pretty much all drivers using regmap for page
selection do that. The difference here is that the page selector
register is in the first range and visible from all pages, but the
other volatile registers are only visible in page 0.
Yes, I would agree that this doesn't make much sense, but it is what
the spd5118 standard calls for, and at least the Renesas/IDT spd5188
chip implements it that way.

Anyway, how should I model this ?

Thanks,
Guenter