| 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
| ||
|
Message-ID: <d5644b18-8126-4dfe-1de1-a03c226bf790@quicinc.com>
Date: Tue, 17 May 2022 12:11:49 +0530
From: Sai Prakash Ranjan <quic_saipraka@...cinc.com>
To: <arnd@...db.de>, <rostedt@...dmis.org>
CC: <gregkh@...uxfoundation.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>,
<catalin.marinas@....com>
Subject: Re: [PATCHv14 5/9] lib: Add register read/write tracing support
On 5/11/2022 9:21 AM, Sai Prakash Ranjan wrote:
> On 5/4/2022 4:58 PM, Sai Prakash Ranjan wrote:
>> From: Prasad Sodagudi <psodagud@...eaurora.org>
>>
>> Generic MMIO read/write i.e., __raw_{read,write}{b,l,w,q} accessors
>> are typically used to read/write from/to memory mapped registers
>> and can cause hangs or some undefined behaviour in following few
>> cases,
>>
>> * If the access to the register space is unclocked, for example: if
>> there is an access to multimedia(MM) block registers without MM
>> clocks.
>>
>> * If the register space is protected and not set to be accessible from
>> non-secure world, for example: only EL3 (EL: Exception level) access
>> is allowed and any EL2/EL1 access is forbidden.
>>
>> * If xPU(memory/register protection units) is controlling access to
>> certain memory/register space for specific clients.
>>
>> and more...
>>
>> Such cases usually results in instant reboot/SErrors/NOC or interconnect
>> hangs and tracing these register accesses can be very helpful to debug
>> such issues during initial development stages and also in later stages.
>>
>> So use ftrace trace events to log such MMIO register accesses which
>> provides rich feature set such as early enablement of trace events,
>> filtering capability, dumping ftrace logs on console and many more.
>>
>> Sample output:
>>
>> rwmmio_write: __qcom_geni_serial_console_write+0x160/0x1e0 width=32 val=0xa0d5d addr=0xfffffbfffdbff700
>> rwmmio_post_write: __qcom_geni_serial_console_write+0x160/0x1e0 width=32 val=0xa0d5d addr=0xfffffbfffdbff700
>> rwmmio_read: qcom_geni_serial_poll_bit+0x94/0x138 width=32 addr=0xfffffbfffdbff610
>> rwmmio_post_read: qcom_geni_serial_poll_bit+0x94/0x138 width=32 val=0x0 addr=0xfffffbfffdbff610
>>
>> Signed-off-by: Prasad Sodagudi <psodagud@...eaurora.org>
>> Co-developed-by: Sai Prakash Ranjan <quic_saipraka@...cinc.com>
>> Signed-off-by: Sai Prakash Ranjan <quic_saipraka@...cinc.com>
>> ---
>> arch/Kconfig | 3 ++
>> arch/arm64/Kconfig | 1 +
>> include/trace/events/rwmmio.h | 97 +++++++++++++++++++++++++++++++++++
>> lib/Kconfig | 7 +++
>> lib/Makefile | 2 +
>> lib/trace_readwrite.c | 47 +++++++++++++++++
>> 6 files changed, 157 insertions(+)
>> create mode 100644 include/trace/events/rwmmio.h
>> create mode 100644 lib/trace_readwrite.c
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index 31c4fdc4a4ba..5e7aa17ed609 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -1369,6 +1369,9 @@ config ARCH_HAS_ELFCORE_COMPAT
>> config ARCH_HAS_PARANOID_L1D_FLUSH
>> bool
>> +config ARCH_HAVE_TRACE_MMIO_ACCESS
>> + bool
>> +
>> config DYNAMIC_SIGFRAME
>> bool
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 20ea89d9ac2f..926e1a252b6f 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -48,6 +48,7 @@ config ARM64
>> select ARCH_HAS_ZONE_DMA_SET if EXPERT
>> select ARCH_HAVE_ELF_PROT
>> select ARCH_HAVE_NMI_SAFE_CMPXCHG
>> + select ARCH_HAVE_TRACE_MMIO_ACCESS
>> select ARCH_INLINE_READ_LOCK if !PREEMPTION
>> select ARCH_INLINE_READ_LOCK_BH if !PREEMPTION
>> select ARCH_INLINE_READ_LOCK_IRQ if !PREEMPTION
>> diff --git a/include/trace/events/rwmmio.h b/include/trace/events/rwmmio.h
>> new file mode 100644
>> index 000000000000..82edee9bf716
>> --- /dev/null
>> +++ b/include/trace/events/rwmmio.h
>> @@ -0,0 +1,97 @@
>> +/* 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>
>> +
>> +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(unsigned long, caller)
>> + __field(unsigned long, addr)
>> + __field(u64, val)
>> + __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=%#lx",
>> + (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)
>> +);
>> +
>> +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(unsigned long, caller)
>> + __field(unsigned long, 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=%#lx",
>> + (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(unsigned long, caller)
>> + __field(unsigned long, addr)
>> + __field(u64, val)
>> + __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=%#lx",
>> + (void *)(unsigned long)__entry->caller, __entry->width,
>> + __entry->val, __entry->addr)
>> +);
>> +
>> +#endif /* _TRACE_RWMMIO_H */
>> +
>> +#include <trace/define_trace.h>
>> diff --git a/lib/Kconfig b/lib/Kconfig
>> index 087e06b4cdfd..5e2fd075724f 100644
>> --- a/lib/Kconfig
>> +++ b/lib/Kconfig
>> @@ -118,6 +118,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 6b9ffc1bd1ee..3df7d24e65d2 100644
>> --- a/lib/Makefile
>> +++ b/lib/Makefile
>> @@ -151,6 +151,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 */
>
> Hi Steve,
>
> Could you please review this patch, it is the main one left for this series to get merged. All your
> previous review comments have been incorporated, you can find them in the cover letter changelog.
>
> Thanks,
> Sai
Hi Steve,
Could we get your blessing for this patch?
Thanks,
Sai
Powered by blists - more mailing lists