linux-kernel - Re: [PATCHv12 7/9] asm-generic/io: Add logging support for MMIO accessors

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: <YmqCYZwHoRuKtFSN@kroah.com>
Date:   Thu, 28 Apr 2022 14:02:41 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Sai Prakash Ranjan <quic_saipraka@...cinc.com>
Cc:     arnd@...db.de, catalin.marinas@....com, rostedt@...dmis.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        maz@...nel.org, quic_psodagud@...cinc.com, quic_tsoni@...cinc.com,
        will@...nel.org
Subject: Re: [PATCHv12 7/9] asm-generic/io: Add logging support for MMIO
 accessors

On Thu, Apr 28, 2022 at 04:51:49PM +0530, Sai Prakash Ranjan wrote:
> On 4/28/2022 4:21 PM, Greg KH wrote:
> > On Thu, Apr 28, 2022 at 03:25:30PM +0530, Sai Prakash Ranjan wrote:
> > > Add logging support for MMIO high level accessors such as read{b,w,l,q}
> > > and their relaxed versions to aid in debugging unexpected crashes/hangs
> > > caused by the corresponding MMIO operation.
> > > 
> > > Signed-off-by: Sai Prakash Ranjan <quic_saipraka@...cinc.com>
> > > ---
> > >   include/asm-generic/io.h | 82 ++++++++++++++++++++++++++++++++++++++--
> > >   1 file changed, 78 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > index 7ce93aaf69f8..99090722cb4b 100644
> > > --- a/include/asm-generic/io.h
> > > +++ b/include/asm-generic/io.h
> > > @@ -10,6 +10,7 @@
> > >   #include <asm/page.h> /* I/O is all done through memory accesses */
> > >   #include <linux/string.h> /* for memset() and memcpy() */
> > >   #include <linux/types.h>
> > > +#include <linux/instruction_pointer.h>
> > >   #ifdef CONFIG_GENERIC_IOMAP
> > >   #include <asm-generic/iomap.h>
> > > @@ -61,6 +62,35 @@
> > >   #define __io_par(v)     __io_ar(v)
> > >   #endif
> > > +#if IS_ENABLED(CONFIG_TRACE_MMIO_ACCESS) && !(defined(__DISABLE_TRACE_MMIO__))
> > Shouldn't you document __DISABLE_TRACE_MMIO__ somewhere?  It's not even
> > in the changelog.  Put a big comment above this for what is is for and
> > how to use it.  Otherwise you will forget all about this in 6 months :)
> > 
> > thanks,
> > 
> > greg k-h
> 
> Didn't you ask me to split the patch to the one actually adding the flag and the one using it.

Yes, and isn't this the commit that adds the flag?  Or was that on an
earlier one that I missed?

Ah, it's in patch 6/9

But you should also document it here in the .h file, otherwise the only
place it is described is in some random kvm Makefile that no one will
ever notice :)

thanks,

greg k-h