linux-kernel - Re: [PATCH v5] arch/x86: port I/O tracing on x86

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <78787d18-0f30-4be0-9e7c-1b6dbf142fec@gmail.com>
Date:   Mon, 23 Oct 2023 15:29:53 -0600
From:   Dan Raymond <raymod2@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>,
        Greg KH <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        linux-serial <linux-serial@...r.kernel.org>, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
        hpa@...or.com, peterz@...radead.org,
        andriy.shevchenko@...ux.intel.com, quic_saipraka@...cinc.com
Subject: Re: [PATCH v5] arch/x86: port I/O tracing on x86

On 10/21/2023 2:15 PM, Steven Rostedt wrote:

>> Why are these needed to be changed at all?  What code changes with it,
>> and it's not mentioned in the changelog, so why is it required?
> 
> Agreed, if this has issues, it probably should be a separate patch.

As I mentioned to Greg, this fix is needed to avoid compiler warnings
triggered by this patch.  If I submitted this separately it would have
to be merged first.  Isn't it easier to combine them since this is
not a functional change (it just makes a cast explicit)?

>>> -#ifdef CONFIG_TRACEPOINTS
>>> +#if defined(CONFIG_TRACEPOINTS) && !defined(DISABLE_TRACEPOINTS)  
>>
>> Why this global change?
> 
> Yeah, DISABLE_TRACEPOINTS does not currently exist. If this is to be a
> new way to disable TRACEPOINTS it needs a separate patch and be able to
> disable tracepoints everywhere (maybe include/trace/*.h files also need
> to be modified?), and also be documented somewhere in Documentation/trace.

It's only needed to suppress compiler errors when building arch/x86/boot/*
and arch/x86/realmode/*.  Those source files include various x86 headers
such as <asm/msr.h> and <asm/shared/io.h>.  Those x86 headers include
<linux/tracepoint-defs.h> which references static_key_false() in
<linux/jump_label.h>.  DISABLE_TRACEPOINTS eliminates that reference and
hence suppresses the compiler error.

I didn't intend for this macro to be used by developers adding new
tracepoints so I didn't document it as such.  As far as creating a
separate patch: again this is a requirement for this patch and it doesn't
cause any functional changes so can't we combine them?