From: Thomas Gleixner <tglx@linutronix.de>
To: "Guilherme G. Piccoli" <gpiccoli@igalia.com>,
"H. Peter Anvin" <hpa@zytor.com>,
bp@alien8.de
Cc: x86@kernel.org, linux-kernel@vger.kernel.org, mingo@redhat.com,
dave.hansen@linux.intel.com, kernel@gpiccoli.net,
kernel-dev@igalia.com
Subject: Re: [PATCH] x86/tsc: Add debugfs entry to mark TSC as unstable after boot
Date: Fri, 21 Mar 2025 22:19:11 +0100 [thread overview]
Message-ID: <87iko213qo.ffs@tglx> (raw)
In-Reply-To: <b43e2353-41ff-f2de-881c-c9a3348552b7@igalia.com>
On Fri, Mar 21 2025 at 16:26, Guilherme G. Piccoli wrote:
> On 17/03/2025 15:42, H. Peter Anvin wrote:
>> To be honest I don't think this belongs in debugfs; rather it belongs in sysfs.
No.
>> Debugfs should not be necessarily in serious production systems – it
>> is way too large of an attack surface, which is a very good reason
>> why it is its own filesystem – but if this is a real issue on
>> hardware then it may be needed.
There is ZERO reason to do that on a production system.
If the in kernel detection does not work, then switching it over after
someone detected the problem five hours after the fact does not help at
all.
The admin can force the TSC to be removed from timekeeping, which is the
really crucial part, already today by changing the clocksource via sysfs.
> In other words, we have 2 options in my understanding:
>
> (a) Drop it;
>
> (b) Re-implement using sysfs entry instead of debugfs;
Neither (a) nor (b) nor the proposed implementation.
There is actually a good reason why a debug/validation mechanism of some
sort makes sense, i.e. testing:
1) The hardware, which exposed these issues frequently is starting to
get into museum or junkyard state, which reduces the test base
significantly.
2) Modern hardware, which exposes this issue in large fleets
occasionally due to aging and misdirected neutrons, is not really a
good testbed either.
So we have no real test coverage for something, which can be crucial in
the actual failure case.
Sure, it could be argued that this can be implemented in qemu, but that's
fundamentally the wrong approach.
This is something which must be easily available to developers and CI
and not require to have a special setup with debug nonsense enabled in
some external tool.
The proposed implementation is just an ad hoc band aid as well. Why?
1) It has zero relation to the actual failure detection code paths.
2) It covers only a small part of the problem space. On all modern
systems, which have TSC_ADJUST the clocksource watchdog is disabled
and just asynchronously invoking TSC unstable is a hack which only
tests the unstable logic.
So I rather want to see a more complete solution, which
1) lets the clocksource watchdog logic fail the test
2) lets the TSC sync (including TSC_ADJUST) logic on CPU hotplug fail
3) tweaks the TSC_ADJUST register and validates that the detection and
mitigation logic on systems w/o clocksource watchdog works
correctly.
Ideally that's a kunit test for CI integration plus a debugfs interface
for developers, which comes with a related selftest.
Thanks,
tglx
next prev parent reply other threads:[~2025-03-21 21:19 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-26 13:27 Guilherme G. Piccoli
2025-03-17 14:35 ` Guilherme G. Piccoli
2025-03-17 18:42 ` H. Peter Anvin
2025-03-21 19:26 ` Guilherme G. Piccoli
2025-03-21 21:19 ` Thomas Gleixner [this message]
2025-03-23 17:53 ` Guilherme G. Piccoli
2025-03-23 18:14 ` Borislav Petkov
2025-03-23 19:21 ` Guilherme G. Piccoli
2025-03-23 19:51 ` Borislav Petkov
2025-03-23 19:59 ` Guilherme G. Piccoli
2025-03-17 14:40 ` Borislav Petkov
2025-03-17 15:03 ` Guilherme G. Piccoli
2025-03-17 15:14 ` Borislav Petkov
2025-03-17 15:24 ` Guilherme G. Piccoli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87iko213qo.ffs@tglx \
--to=tglx@linutronix.de \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=gpiccoli@igalia.com \
--cc=hpa@zytor.com \
--cc=kernel-dev@igalia.com \
--cc=kernel@gpiccoli.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®