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: <6e74f535-895d-405f-8048-ec89ddeefef7@intel.com>
Date: Sat, 7 Dec 2024 05:14:51 -0800
From: Sohil Mehta <sohil.mehta@...el.com>
To: Ingo Molnar <mingo@...nel.org>
CC: <x86@...nel.org>, Borislav Petkov <bp@...en8.de>, Dave Hansen
	<dave.hansen@...ux.intel.com>, Thomas Gleixner <tglx@...utronix.de>, "Ingo
 Molnar" <mingo@...hat.com>, "H . Peter Anvin" <hpa@...or.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 12/7/2024 12:35 AM, Ingo Molnar wrote:
>>  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);
> 
> So why not make this a pr_info() at minimum?
> 

I was hesitant because the feature disabling may be inconsequential to
the end user. Also, the printed numbers would not make sense unless they
have the kernel source handy. But maybe it's better to inform and let
the user decide.

> Since this new logic will disable certain feature bits on random 
> machines, I'm sure there will be some surprises. I'd sure like to see 
> this printed in the system log of my machine if it happens.
> 

Sure, will update the print as follows in the next (hopefully final)
version.

pr_info("CPU%d: Disabling feature %s due to missing feature %s\n",
	smp_processor_id(),
	x86_feature_name(d->feature, feature_buf),
	x86_feature_name(d->depends, depends_buf));

Since it seems almost certain that we are going in this direction will
merge this patch into the previous one and send a combined one next time.

> It might also inform firmware & distro testers that something wasn't 
> set up properly on the firmware (or virtualization) side.
> 

Sohil


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ