| 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: <20240130081152.GH32821@thinkpad>
Date: Tue, 30 Jan 2024 13:41:52 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Krishna chaitanya chundru <quic_krichai@...cinc.com>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
linux-kernel@...r.kernel.org, mhi@...ts.linux.dev,
linux-arm-msm@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
quic_vbadigan@...cinc.com, quic_ramkri@...cinc.com,
quic_nitegupt@...cinc.com, quic_skananth@...cinc.com,
quic_parass@...cinc.com
Subject: Re: [PATCH v9] bus: mhi: host: Add tracing support
On Fri, Jan 05, 2024 at 05:53:03PM +0530, Krishna chaitanya chundru wrote:
> This change adds ftrace support for following functions which
> helps in debugging the issues when there is Channel state & MHI
> state change and also when we receive data and control events:
> 1. mhi_intvec_mhi_states
> 2. mhi_process_data_event_ring
> 3. mhi_process_ctrl_ev_ring
> 4. mhi_gen_tre
> 5. mhi_update_channel_state
> 6. mhi_tryset_pm_state
> 7. mhi_pm_st_worker
>
> Change the implementation of the arrays which has enum to strings mapping
> to make it consistent in both trace header file and other files.
>
> Where ever the trace events are added, debug messages are removed.
>
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@...cinc.com>
Few nitpicks below.
> Reviewed-by: "Steven Rostedt (Google)" <rostedt@...dmis.org>
> ---
> Changes in v9:
> - Change the implementations of some array so that the strings to enum mapping
> - is same in both trace header and other files as suggested by steve.
> - Link to v8: https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558555@quicinc.com
>
> Changes in v8:
> - Pass the structure and derefernce the variables in TP_fast_assign as suggested by steve
> - Link to v7: https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a04268b@quicinc.com
>
> Changes in v7:
> - change log format as pointed by mani.
> - Link to v6: https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546dac2@quicinc.com
>
> Changes in v6:
> - use 'rp' directly as suggested by jeffrey.
> - Link to v5: https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead4f1@quicinc.com
>
> Changes in v5:
> - Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
> - Instead of converting to u64 to print address, use %px to print the address to avoid
> - warnings in some platforms.
> - Link to v4: https://lore.kernel.org/r/20231111-ftrace_support-v4-1-c83602399461@quicinc.com
>
> Changes in v4:
> - Fix compilation issues in previous patch which happended due to rebasing.
> - In the defconfig FTRACE config is not enabled due to that the compilation issue is not
> - seen in my workspace.
> - Link to v3: https://lore.kernel.org/r/20231111-ftrace_support-v3-1-f358d2911a74@quicinc.com
>
> Changes in v3:
> - move trace header file from include/trace/events to drivers/bus/mhi/host/ so that
> - we can include driver header files.
> - Use macros directly in the trace events as suggested Jeffrey Hugo.
> - Reorder the structure in the events as suggested by steve to avoid holes in the buffer.
> - removed the mhi_to_physical function as this can give security issues.
> - removed macros to define strings as we can get those from driver headers.
> - Link to v2: https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce010b5@quicinc.com
>
> Changes in v2:
> - Passing the raw state into the trace event and using __print_symbolic() as suggested by bjorn.
> - Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
> - Fixed the kernel test rebot issues.
> - Link to v1: https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394fa49@quicinc.com
> ---
> drivers/bus/mhi/common.h | 38 +++---
> drivers/bus/mhi/host/init.c | 63 +++++----
> drivers/bus/mhi/host/internal.h | 40 ++++++
> drivers/bus/mhi/host/main.c | 19 ++-
> drivers/bus/mhi/host/pm.c | 7 +-
> drivers/bus/mhi/host/trace.h | 275 ++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 378 insertions(+), 64 deletions(-)
>
[...]
> +TRACE_EVENT(mhi_gen_tre,
> +
> + TP_PROTO(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan,
> + struct mhi_ring_element *mhi_tre),
> +
> + TP_ARGS(mhi_cntrl, mhi_chan, mhi_tre),
> +
> + TP_STRUCT__entry(
> + __string(name, mhi_cntrl->mhi_dev->name)
> + __field(int, ch_num)
> + __field(void *, wp)
> + __field(__le64, tre_ptr)
> + __field(__le32, dword0)
> + __field(__le32, dword1)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(name, mhi_cntrl->mhi_dev->name);
> + __entry->ch_num = mhi_chan->chan;
> + __entry->wp = mhi_tre;
> + __entry->tre_ptr = mhi_tre->ptr;
> + __entry->dword0 = mhi_tre->dword[0];
> + __entry->dword1 = mhi_tre->dword[1];
> + ),
> +
> + TP_printk("%s: Chan: %d Tre: 0x%p Tre buf: 0x%llx dword0: 0x%08x dword1: 0x%08x\n",
Use caps for printing the acronyms everywhere. Like TRE, DWORD etc...
> + __get_str(name), __entry->ch_num, __entry->wp, __entry->tre_ptr,
> + __entry->dword0, __entry->dword1)
> +);
> +
> +TRACE_EVENT(mhi_intvec_states,
> +
> + TP_PROTO(struct mhi_controller *mhi_cntrl, int dev_ee, int dev_state),
> +
> + TP_ARGS(mhi_cntrl, dev_ee, dev_state),
> +
> + TP_STRUCT__entry(
> + __string(name, mhi_cntrl->mhi_dev->name)
> + __field(int, local_ee)
> + __field(int, state)
> + __field(int, dev_ee)
> + __field(int, dev_state)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(name, mhi_cntrl->mhi_dev->name);
> + __entry->local_ee = mhi_cntrl->ee;
> + __entry->state = mhi_cntrl->dev_state;
> + __entry->dev_ee = dev_ee;
> + __entry->dev_state = dev_state;
> + ),
> +
> + TP_printk("%s: local ee: %s state: %s device ee: %s state: %s\n",
"Local EE... State:...Device EE.."
> + __get_str(name),
> + __print_symbolic(__entry->local_ee, MHI_EE_LIST),
> + __print_symbolic(__entry->state, MHI_STATE_LIST),
> + __print_symbolic(__entry->dev_ee, MHI_EE_LIST),
> + __print_symbolic(__entry->state, MHI_STATE_LIST))
> +);
> +
[...]
> +DECLARE_EVENT_CLASS(mhi_update_channel_state,
> +
> + TP_PROTO(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan, int state),
> +
> + TP_ARGS(mhi_cntrl, mhi_chan, state),
> +
> + TP_STRUCT__entry(
> + __string(name, mhi_cntrl->mhi_dev->name)
> + __field(int, ch_num)
> + __field(int, state)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(name, mhi_cntrl->mhi_dev->name);
> + __entry->ch_num = mhi_chan->chan;
> + __entry->state = state;
> + ),
> +
> + TP_printk("%s: chan%d: Updating state to: %s\n",
> + __get_str(name), __entry->ch_num,
> + __print_symbolic(__entry->state, MHI_CH_STATE_TYPE_LIST))
So same trace will get printed for both mhi_channel_command_start() and
mhi_channel_command_end()?
- Mani
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists