linux-kernel - Re: bug fix for registers debugfs file implementation [RFC]

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: <A6A78A0A-155B-42E2-B6CA-2182FD4C997B@gmail.com>
Date:   Sat, 22 Apr 2017 13:28:15 -0700
From:   Variksla <variksla@...il.com>
To:     Mark Brown <broonie@...nel.org>,
        ckeepax@...nsource.wolfsonmicro.com
Cc:     linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org
Subject: Re: bug fix for registers debugfs file implementation [RFC]



> On Apr 21, 2017, at 10:27 AM, Mark Brown <broonie@...nel.org> wrote:

Thanks for the response.
> 
>> On Sat, Apr 01, 2017 at 02:13:41AM -0700, noman pouigt wrote:
>> Current registers debugfs file implementation doesn't
>> handle if the size exceeds 4k. It just dumps 4k of registers.
>> Fix this by using the seq_file which already handles
>> the file offset logic instead of reinventing the wheel.
>> 
>> I am wondering if there is any issue is doing below which
>> I am not aware of?
> 
> If I remember correctly this is done the way it is because seq_file has
> to iterate through the entire file to get to the point being read by the
> application.  This is a *very* big overhead for some applications (like
> monitoring some registers to see what they're doing) on bigger devices,

Wondering why would the user space application be monitoring the registers?
> especially if there's lots of uncached stuff and the reads have to go to
> the hardware.  With the current code you can open the file, seek to the
> registers you're interested in and read only them.  You'll notice that
> the other debug files have been converted over to seq_file as they're
> pure software and there's less reason to repeatedly read them.

Yes. I noticed that and that is why I realized that I am doing something wrong.
> 
> I'm also very surprised that this is failing for you as I know this code
> has been fairly heavily exercised with devices with very large register
> maps much larger than 4k and my own testing is mainly doing cats which
> seemed to work last time I tried and should be iterating over the 4k
> boundary...  what's the actual failure mode?  I'm guessing it's

I have integrated Florida codec(wm8281) from cirrus and it has more than 4K registers. I could not dump more than 4K with the existing interface.

Charles, are you able to dump all the registers?

> something if we end a register exactly on a page boundary or something?
> 
> Also your patch is completely tab/space mangled so it's not appliable.
> 
>> +static void regmap_debugfs_stop(struct seq_file *s, void *v)
>> +{
>> +}
> 
> The need for empty functions is worrying...

In several other places in kernel code the usage is similar.