Messages in this thread | | | Date | Mon, 17 Jan 2022 21:21:03 +0530 | Subject | Re: [PATCHv8 4/5] lib: Add register read/write tracing support | From | Sai Prakash Ranjan <> |
| |
On 1/17/2022 9:00 PM, Sai Prakash Ranjan wrote: > Hi Steve, > > On 1/17/2022 8:33 PM, Steven Rostedt wrote: >> On Mon, 17 Jan 2022 09:02:53 +0530 >> Sai Prakash Ranjan <quic_saipraka@quicinc.com> wrote: >> >>> diff --git a/include/trace/events/rwmmio.h >>> b/include/trace/events/rwmmio.h >>> new file mode 100644 >>> index 000000000000..798fbe1ac9f9 >>> --- /dev/null >>> +++ b/include/trace/events/rwmmio.h >>> @@ -0,0 +1,112 @@ >>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>> +/* >>> + * Copyright (c) 2021-2022 Qualcomm Innovation Center, Inc. All >>> rights reserved. >>> + */ >>> +#undef TRACE_SYSTEM >>> +#define TRACE_SYSTEM rwmmio >>> + >>> +#if !defined(_TRACE_RWMMIO_H) || defined(TRACE_HEADER_MULTI_READ) >>> +#define _TRACE_RWMMIO_H >>> + >>> +#include <linux/tracepoint.h> >>> + >>> +TRACE_EVENT(rwmmio_write, >>> + >>> + TP_PROTO(unsigned long caller, u64 val, u8 width, volatile void >>> __iomem *addr), >>> + >>> + TP_ARGS(caller, val, width, addr), >>> + >>> + TP_STRUCT__entry( >>> + __field(u64, caller) >>> + __field(u64, val) >>> + __field(u64, addr) >>> + __field(u8, width) >>> + ), >>> + >>> + TP_fast_assign( >>> + __entry->caller = caller; >>> + __entry->val = val; >>> + __entry->addr = (unsigned long)(void *)addr; >>> + __entry->width = width; >>> + ), >>> + >>> + TP_printk("%pS width=%d val=%#llx addr=%#llx", >>> + (void *)(unsigned long)__entry->caller, __entry->width, >>> + __entry->val, __entry->addr) >>> +); >>> + >>> +TRACE_EVENT(rwmmio_post_write, >>> + >>> + TP_PROTO(unsigned long caller, u64 val, u8 width, volatile void >>> __iomem *addr), >>> + >>> + TP_ARGS(caller, val, width, addr), >>> + >>> + TP_STRUCT__entry( >>> + __field(u64, caller) >>> + __field(u64, val) >>> + __field(u64, addr) >>> + __field(u8, width) >>> + ), >>> + >>> + TP_fast_assign( >>> + __entry->caller = caller; >>> + __entry->val = val; >>> + __entry->addr = (unsigned long)(void *)addr; >>> + __entry->width = width; >>> + ), >>> + >>> + TP_printk("%pS width=%d val=%#llx addr=%#llx", >>> + (void *)(unsigned long)__entry->caller, __entry->width, >>> + __entry->val, __entry->addr) >>> +); >>> + >>> +TRACE_EVENT(rwmmio_read, >>> + >>> + TP_PROTO(unsigned long caller, u8 width, const volatile void >>> __iomem *addr), >>> + >>> + TP_ARGS(caller, width, addr), >>> + >>> + TP_STRUCT__entry( >>> + __field(u64, caller) >>> + __field(u64, addr) >>> + __field(u8, width) >>> + ), >>> + >>> + TP_fast_assign( >>> + __entry->caller = caller; >>> + __entry->addr = (unsigned long)(void *)addr; >>> + __entry->width = width; >>> + ), >>> + >>> + TP_printk("%pS width=%d addr=%#llx", >>> + (void *)(unsigned long)__entry->caller, __entry->width, >>> __entry->addr) >>> +); >>> + >>> +TRACE_EVENT(rwmmio_post_read, >>> + >>> + TP_PROTO(unsigned long caller, u64 val, u8 width, const >>> volatile void __iomem *addr), >>> + >>> + TP_ARGS(caller, val, width, addr), >>> + >>> + TP_STRUCT__entry( >>> + __field(u64, caller) >>> + __field(u64, val) >>> + __field(u64, addr) >>> + __field(u8, width) >>> + ), >>> + >>> + TP_fast_assign( >>> + __entry->caller = caller; >>> + __entry->val = val; >>> + __entry->addr = (unsigned long)(void *)addr; >>> + __entry->width = width; >>> + ), >>> + >>> + TP_printk("%pS width=%d val=%#llx addr=%#llx", >>> + (void *)(unsigned long)__entry->caller, __entry->width, >>> + __entry->val, __entry->addr) >>> +); >> The above should be replaced with: >> >> DECLARE_EVENT_CLASS(rwmmio_rw_template, >> >> TP_PROTO(unsigned long caller, u64 val, u8 width, volatile void >> __iomem *addr), >> >> TP_ARGS(caller, val, width, addr), >> >> TP_STRUCT__entry( >> __field(u64, caller) >> __field(u64, val) >> __field(u64, addr) >> __field(u8, width) >> ), >> >> TP_fast_assign( >> __entry->caller = caller; >> __entry->val = val; >> __entry->addr = (unsigned long)(void *)addr; >> __entry->width = width; >> ), >> >> TP_printk("%pS width=%d val=%#llx addr=%#llx", >> (void *)(unsigned long)__entry->caller, __entry->width, >> __entry->val, __entry->addr) >> ); >> >> DEFINE_EVENT(rwmmio_rw_template, rwmmio_write, >> TP_PROTO(unsigned long caller, u64 val, u8 width, volatile void >> __iomem *addr), >> TP_ARGS(caller, val, width, addr) >> ); >> >> DEFINE_EVENT(rwmmio_rw_template, rwmmio_post_write, >> TP_PROTO(unsigned long caller, u64 val, u8 width, volatile void >> __iomem *addr), >> TP_ARGS(caller, val, width, addr) >> ); >> >> DEFINE_EVENT(rwmmio_rw_template, rwmmio_post_read, >> TP_PROTO(unsigned long caller, u64 val, u8 width, volatile void >> __iomem *addr), >> TP_ARGS(caller, val, width, addr) >> ); >> >> It will save around 15k in memory. >> >> And since rwmmio_read doesn't have a val field, it can stay a >> TRACE_EVENT. >> >> TRACE_EVENT(rwmmio_read, >> >> TP_PROTO(unsigned long caller, u8 width, const volatile void >> __iomem *addr), >> >> TP_ARGS(caller, width, addr), >> >> TP_STRUCT__entry( >> __field(u64, caller) >> __field(u64, addr) >> __field(u8, width) >> ), >> >> TP_fast_assign( >> __entry->caller = caller; >> __entry->addr = (unsigned long)(void *)addr; >> __entry->width = width; >> ), >> >> TP_printk("%pS width=%d addr=%#llx", >> (void *)(unsigned long)__entry->caller, __entry->width, >> __entry->addr) >> ); >> >> -- Steve > > Thanks for the review, will add these changes in next version. > >
Just one note that I need to keep rwmmio_post_read also as TRACE_EVENT because of const qualifier in post_read resulting in compiler warnings,
lib/trace_readwrite.c:43:50: warning: passing argument 4 of ‘trace_rwmmio_post_read’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
Thanks, Sai >>> + >>> +#endif /* _TRACE_RWMMIO_H */ >>> + >>> +#include <trace/define_trace.h> >>> diff --git a/lib/Kconfig b/lib/Kconfig >>> index c80fde816a7e..ea520c315c0f 100644 >>> --- a/lib/Kconfig >>> +++ b/lib/Kconfig >>> @@ -119,6 +119,13 @@ config INDIRECT_IOMEM_FALLBACK >>> mmio accesses when the IO memory address is not a registered >>> emulated region. >>> +config TRACE_MMIO_ACCESS >>> + bool "Register read/write tracing" >>> + depends on TRACING && ARCH_HAVE_TRACE_MMIO_ACCESS >>> + help >>> + Create tracepoints for MMIO read/write operations. These >>> trace events >>> + can be used for logging all MMIO read/write operations. >>> + >>> source "lib/crypto/Kconfig" >>> config CRC_CCITT >>> diff --git a/lib/Makefile b/lib/Makefile >>> index 300f569c626b..43813b0061cd 100644 >>> --- a/lib/Makefile >>> +++ b/lib/Makefile >>> @@ -152,6 +152,8 @@ lib-y += logic_pio.o >>> lib-$(CONFIG_INDIRECT_IOMEM) += logic_iomem.o >>> +obj-$(CONFIG_TRACE_MMIO_ACCESS) += trace_readwrite.o >>> + >>> obj-$(CONFIG_GENERIC_HWEIGHT) += hweight.o >>> obj-$(CONFIG_BTREE) += btree.o >>> diff --git a/lib/trace_readwrite.c b/lib/trace_readwrite.c >>> new file mode 100644 >>> index 000000000000..88637038b30c >>> --- /dev/null >>> +++ b/lib/trace_readwrite.c >>> @@ -0,0 +1,47 @@ >>> +// SPDX-License-Identifier: GPL-2.0-only >>> +/* >>> + * Register read and write tracepoints >>> + * >>> + * Copyright (c) 2021-2022 Qualcomm Innovation Center, Inc. All >>> rights reserved. >>> + */ >>> + >>> +#include <linux/ftrace.h> >>> +#include <linux/module.h> >>> +#include <asm-generic/io.h> >>> + >>> +#define CREATE_TRACE_POINTS >>> +#include <trace/events/rwmmio.h> >>> + >>> +#ifdef CONFIG_TRACE_MMIO_ACCESS >>> +void log_write_mmio(u64 val, u8 width, volatile void __iomem *addr, >>> + unsigned long caller_addr) >>> +{ >>> + trace_rwmmio_write(caller_addr, val, width, addr); >>> +} >>> +EXPORT_SYMBOL_GPL(log_write_mmio); >>> +EXPORT_TRACEPOINT_SYMBOL_GPL(rwmmio_write); >>> + >>> +void log_post_write_mmio(u64 val, u8 width, volatile void __iomem >>> *addr, >>> + unsigned long caller_addr) >>> +{ >>> + trace_rwmmio_post_write(caller_addr, val, width, addr); >>> +} >>> +EXPORT_SYMBOL_GPL(log_post_write_mmio); >>> +EXPORT_TRACEPOINT_SYMBOL_GPL(rwmmio_post_write); >>> + >>> +void log_read_mmio(u8 width, const volatile void __iomem *addr, >>> + unsigned long caller_addr) >>> +{ >>> + trace_rwmmio_read(caller_addr, width, addr); >>> +} >>> +EXPORT_SYMBOL_GPL(log_read_mmio); >>> +EXPORT_TRACEPOINT_SYMBOL_GPL(rwmmio_read); >>> + >>> +void log_post_read_mmio(u64 val, u8 width, const volatile void >>> __iomem *addr, >>> + unsigned long caller_addr) >>> +{ >>> + trace_rwmmio_post_read(caller_addr, val, width, addr); >>> +} >>> +EXPORT_SYMBOL_GPL(log_post_read_mmio); >>> +EXPORT_TRACEPOINT_SYMBOL_GPL(rwmmio_post_read); >>> +#endif /* CONFIG_TRACE_MMIO_ACCESS */
| |