| 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: <YmiF6Rsy04pUHVQo@agluck-desk3.sc.intel.com>
Date: Tue, 26 Apr 2022 16:53:13 -0700
From: "Luck, Tony" <tony.luck@...el.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: "hdegoede@...hat.com" <hdegoede@...hat.com>,
"markgross@...nel.org" <markgross@...nel.org>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
"bp@...en8.de" <bp@...en8.de>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
"corbet@....net" <corbet@....net>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"andriy.shevchenko@...ux.intel.com"
<andriy.shevchenko@...ux.intel.com>,
"Joseph, Jithu" <jithu.joseph@...el.com>,
"Raj, Ashok" <ashok.raj@...el.com>,
"Williams, Dan J" <dan.j.williams@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"platform-driver-x86@...r.kernel.org"
<platform-driver-x86@...r.kernel.org>,
"patches@...ts.linux.dev" <patches@...ts.linux.dev>,
"Shankar, Ravi V" <ravi.v.shankar@...el.com>
Subject: Re: [PATCH v4 09/10] trace: platform/x86/intel/ifs: Add trace point
to track Intel IFS operations
On Mon, Apr 25, 2022 at 09:49:28PM -0400, Steven Rostedt wrote:
> On Mon, 25 Apr 2022 16:49:35 +0000
> "Luck, Tony" <tony.luck@...el.com> wrote:
>
> > I see two paths:
> >
> > 1) Create a new user friendly trace point for each new scan mode.
> > 2) Just provide a generic one that dumps both the 64-bit WRMSR and RDMSR values.
> >
> > Q: Are trace points "expensive" in some way ... so better to just have one than three?
> > Or are the cheap enough that decoding for the user is an OK thing?
>
> Yes, they are expensive as each TRACE_EVENT() can add a few KB of text and
> data. But you can add a DECLARE_EVENT_CLASS() and then add "printk"
> differences that are less memory heavy.
>
> See DEFINE_EVENT_PRINT().
I looked at the examples in samples/trace_events/trace-events-sample.h
and tried to use this. But I'm doing something wrong because the
compiler barfs on something defined but not used.
Maybe my problem is the TP_printk() in the DECLARE_EVENT_CLASS() that
is over-ridden by DEFINE_EVENT_PRINT(). I wasn't at all sure what to
put here ... or how to use the base tracepoint that doesn't have the
printk() over-ridden.
I think I need my class to just save both the u64 values to the trace
buffer. Then the different trace points will extract the bits they want
and print in a user friendly way. While this increases space used in
the trace buffer, these events are not crazy high frequency. Usually
one or two events per core with a gap 30 minutes or more between tests.
In my ".c" file the tracepoint looks like this using the name from
DEFINE_EVENT_PRINT(), and now passing the full u64 values:
trace_ifs_status_saf(activate.data, status.data);
and my #include file looks like this:
----------------------------------------------
/* SPDX-License-Identifier: GPL-2.0 */
#undef TRACE_SYSTEM
#define TRACE_SYSTEM intel_ifs
#if !defined(_TRACE_IFS_H) || defined(TRACE_HEADER_MULTI_READ)
#define _TRACE_IFS_H
#include <linux/ktime.h>
#include <linux/tracepoint.h>
DECLARE_EVENT_CLASS(ifs_status,
TP_PROTO(u64 activate, u64 status),
TP_ARGS(activate, status),
TP_STRUCT__entry(
__field( u64, activate )
__field( u64, status )
),
TP_fast_assign(
__entry->activate = activate;
__entry->status = status;
),
TP_printk("activate: %llx status: %llx",
__entry->activate,
__entry->status)
);
DEFINE_EVENT_PRINT(ifs_status, ifs_status_saf,
TP_PROTO(u64 activate, u64 status),
TP_ARGS(activate, status),
TP_printk("start: %.2x, stop: %.2x, status: %llx",
((union ifs_scan *)&(__entry->activate))->start,
((union ifs_scan *)&(__entry->activate))->stop,
__entry->status)
);
#endif /* _TRACE_IFS_H */
/* This part must be outside protection */
#include <trace/define_trace.h>
-----------------------------------------------------
GCC messages:
CC [M] drivers/platform/x86/intel/ifs/runtest.o
In file included from /home/agluck/GIT/mywork/include/trace/define_trace.h:102,
from /home/agluck/GIT/mywork/include/trace/events/intel_ifs.h:44,
from /home/agluck/GIT/mywork/drivers/platform/x86/intel/ifs/runtest.c:27:
/home/agluck/GIT/mywork/include/trace/trace_events.h:426:13: warning: ‘print_fmt_ifs_status’ defined but not used [-Wunused-variable]
426 | static char print_fmt_##call[] = print; \
| ^~~~~~~~~~
/home/agluck/GIT/mywork/include/trace/events/intel_ifs.h:11:1: note: in expansion of macro ‘DECLARE_EVENT_CLASS’
11 | DECLARE_EVENT_CLASS(ifs_status,
| ^~~~~~~~~~~~~~~~~~~
In file included from /home/agluck/GIT/mywork/include/trace/define_trace.h:102,
from /home/agluck/GIT/mywork/include/trace/events/intel_ifs.h:44,
from /home/agluck/GIT/mywork/drivers/platform/x86/intel/ifs/runtest.c:27:
/home/agluck/GIT/mywork/include/trace/trace_events.h:207:37: warning: ‘trace_event_type_funcs_ifs_status’ defined but not used [-Wunused-variable]
207 | static struct trace_event_functions trace_event_type_funcs_##call = { \
| ^~~~~~~~~~~~~~~~~~~~~~~
/home/agluck/GIT/mywork/include/trace/events/intel_ifs.h:11:1: note: in expansion of macro ‘DECLARE_EVENT_CLASS’
11 | DECLARE_EVENT_CLASS(ifs_status,
| ^~~~~~~~~~~~~~~~~~~
make[1]: Leaving directory '/home/agluck/GIT/mywork/build/ifsv5-rc1'
-Tony
Powered by blists - more mailing lists