On 11/22/2021 8:00 PM, Arnd Bergmann wrote:

On Mon, Nov 22, 2021 at 3:19 PM Sai Prakash Ranjan
[off-list ref] wrote:

quoted

On 11/22/2021 7:29 PM, Arnd Bergmann wrote:

quoted

I think this would be a lot less confusing to readers, as it is implemented
exactly in the place that has the normal definition, and it can also have
somewhat more logical semantics by only instrumenting the
normal/relaxed/ioport accessors but not the __raw_* versions that
are meant to be little more than a pointer dereference.

But how is this different from logic in atomic-instrumented.h which also
has asm-generic version?
Initial review few years back mentioned about having something similar
to atomic instrumentation
and hence it was implemented with the similar approach keeping
instrumentation out of arch specific details.

This is only a cosmetic difference. I usually prefer fewer indirections,
and I like the way that include/asm-generic/io.h only has all the
normal 'static inline' definitions spelled out, and calling the __raw_*
versions. Your version adds an extra layer with the arch_raw_readl(),
which I'd prefer to avoid.

I'm ok with your preference as long as we have some way to log these 
MMIO accesses.

quoted

And if we do move this instrumentation to asm-generic/io.h, how will
that be executed since
the arch specifc read{b,w,l,q} overrides this generic version?

As I understand it, your version also requires architecture specific 
changes, so that would be the same: it only works for architectures 
that get the definition of readl()/readl_relaxed()/inl()/... from 
include/asm-generic/io.h and only override the __raw version. Arnd

Sorry, I didn't get this part, so  I am trying this on ARM64:

arm64/include/asm/io.h has read{b,l,w,q} defined.
include/asm-generic/io.h has below:
   #ifndef readl
   #define readl readl
   static inline u32 readl(const volatile void __iomem *addr)

and we include asm-generic/io.h in arm64/include/asm/io.h at the end 
after the definitions for arm64 mmio accesors.
So arch implementation here overrides generic ones as I see it, am I 
missing something? I even confirmed this
with some trace_printk to generic and arch specific definitions of readl 
and I see arch specific ones being called.

Thanks,
Sai

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel