linux-kernel - Re: [PATCH v3 2/2] x86/cpufeature: Add a debug print for unmet dependencies

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 for Android: free password hash cracker in your pocket

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

Message-ID: <A62E8AE8-A7C8-4223-A914-CF5130F77E68@zytor.com>
Date: Sat, 07 Dec 2024 13:11:23 -0800
From: "H. Peter Anvin" <hpa@...or.com>
To: Sohil Mehta <sohil.mehta@...el.com>, x86@...nel.org,
        Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>
CC: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
        Uros Bizjak <ubizjak@...il.com>, Sandipan Das <sandipan.das@....com>,
        Sean Christopherson <seanjc@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Vegard Nossum <vegard.nossum@...cle.com>,
        Tony Luck <tony.luck@...el.com>,
        Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
        Nikolay Borisov <nik.borisov@...e.com>,
        Eric Biggers <ebiggers@...gle.com>, Xin Li <xin3.li@...el.com>,
        Alexander Shishkin <alexander.shishkin@...el.com>,
        Kirill Shutemov <kirill.shutemov@...ux.intel.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] x86/cpufeature: Add a debug print for unmet dependencies

On December 6, 2024 4:41:26 PM PST, Sohil Mehta <sohil.mehta@...el.com> wrote:
>Instead of silently disabling features, add a print which might be
>useful to users if their favorite feature gets unexpectedly disabled.
>
>Features are typically represented through unsigned integers though some
>of them have user friendly names if they are exposed via cpuinfo.  Show
>the friendlier name if available otherwise display the X86_FEATURE_*
>numerals to make it easier to identify the feature.
>
>Use pr_debug() to avoid spamming the kernel log and generating false
>alarms. Note that the print will occur once per disabled feature on
>every CPU. Show this information only when someone is really looking for
>it.
>
>Suggested-by: Tony Luck <tony.luck@...el.com>
>Signed-off-by: Sohil Mehta <sohil.mehta@...el.com>
>---
>Sent as a separate patch to make it easier to review and drop if it
>feels unnecessary.
>
>I can see both sides of the argument. The pr_debug() serves a
>compromise between the two.
>
>v3: New patch.
>
>---
> arch/x86/kernel/cpu/cpuid-deps.c | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
>diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
>index 8bea5c5e4fd2..c72f2dd77d72 100644
>--- a/arch/x86/kernel/cpu/cpuid-deps.c
>+++ b/arch/x86/kernel/cpu/cpuid-deps.c
>@@ -147,12 +147,32 @@ void setup_clear_cpu_cap(unsigned int feature)
> 	do_clear_cpu_cap(NULL, feature);
> }
> 
>+/*
>+ * Return the feature "name" if available otherwise return
>+ * the X86_FEATURE_* numerals to make it easier to identify
>+ * the feature.
>+ */
>+static const char *x86_feature_name(unsigned int feature, char *buf)
>+{
>+	if (x86_cap_flags[feature])
>+		return x86_cap_flags[feature];
>+
>+	snprintf(buf, 12, "%d*32+%2d", feature / 32, feature % 32);
>+
>+	return buf;
>+}
>+
> void filter_feature_dependencies(struct cpuinfo_x86 *c)
> {
>+	char feature_buf[12], depends_buf[12];
> 	const struct cpuid_dep *d;
> 
> 	for (d = cpuid_deps; d->feature; d++) {
>-		if (cpu_has(c, d->feature) && !cpu_has(c, d->depends))
>+		if (cpu_has(c, d->feature) && !cpu_has(c, d->depends)) {
>+			pr_debug("x86/cpu: Disabling feature %s since feature %s is missing\n",
>+				 x86_feature_name(d->feature, feature_buf),
>+				 x86_feature_name(d->depends, depends_buf));
> 			do_clear_cpu_cap(c, d->feature);
>+		}
> 	}
> }

Ok, I realize that the x86 maintainers **very legitimately** don't want more crap in /proc/cpuinfo, but perhaps we could include the strings for printing debug messages in cleartext? Add a bitmap for which entries should go into /proc/cpuinfo.