From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 64C333CBE9E for ; Mon, 23 Mar 2026 18:16:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.12 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774289818; cv=none; b=l4C1Ru0Rfgb55wzfLRmLDyjTRKdED8xl1BfKIR6jk2vpYTYzfYIZkFQvosp+mnlP2CCh+ZimoBpAeFt2nDOCRhm6/vaB79dAayC9+BND/8wOQmjkSYptR1e+/mhNabnxm9ssNvrqhl45qxkC8I6PAPcnhnpRq6yTzUZsOyboPXs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774289818; c=relaxed/simple; bh=vlM66C1vllDg2kPKCBE5FeOQd4j5E7r9a664v56RNG4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tza3Op8RwVyLoJW8eYOjmTBxYA3JrhQ7J9BU/Hbt6N5u7ATaqcxGEvvcVELc+QAubvqTpc+BVtLf7ckjtIXLEHpx/Yx3i22cFkc7zW9io6Px13qZ+4JabJYUh6IOJlC9DtRbJPFxgfMB0ozVePInnDP/HZ+hqguphoSsItfbYkU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=pass smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=SylQB3/S; arc=none smtp.client-ip=192.198.163.12 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="SylQB3/S" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1774289811; x=1805825811; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=vlM66C1vllDg2kPKCBE5FeOQd4j5E7r9a664v56RNG4=; b=SylQB3/S02R5t9paycmHeWMPEIWiW1oJUckz0zD/QN7NtSV/WTCuIW+y r2Z4J7D8qCxQPHgaJ6TSHKMGYjA2kbRelRVT7jLVVgkxo0x9XJtGoyd+p kM3q3JpnTM5UbFXL599NwOA3fQzTlVwusjEGTpFDN2LMcn2y1uehHtXeM J6MaOiCGuxHQqmUfySyLVqgLm62l6D8mvZF7lCFrq2y50mw0oEJj4m4z1 /Cq2M0eu5yrZgS9heudC5V5O4jYNiJaNxvD0wJ8pDlAECjBn5/rKbOqyM i0VLkgWluNJTKae6tKETNjGRfpMU0oG7BKC/ki0HGsIBiDqkYsSslZjBG Q==; X-CSE-ConnectionGUID: z6UkoIr2TZmYLry96ywBIA== X-CSE-MsgGUID: KGRBRg7qTj2DtitvNkdQVw== X-IronPort-AV: E=McAfee;i="6800,10657,11738"; a="79206272" X-IronPort-AV: E=Sophos;i="6.23,137,1770624000"; d="scan'208";a="79206272" Received: from orviesa001.jf.intel.com ([10.64.159.141]) by fmvoesa106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Mar 2026 11:16:49 -0700 X-CSE-ConnectionGUID: STfLZ+35RFi9GeTDUjTLrw== X-CSE-MsgGUID: ueRbeh9pS5+jFm0o/CVOCw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.23,137,1770624000"; d="scan'208";a="262018984" Received: from guptapa-desk.jf.intel.com (HELO desk) ([10.165.239.46]) by smtpauth.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Mar 2026 11:16:49 -0700 Date: Mon, 23 Mar 2026 11:16:43 -0700 From: Pawan Gupta To: Maciej =?utf-8?Q?Wiecz=C3=B3r-Retman?= Cc: tglx@kernel.org, peterz@infradead.org, xin@zytor.com, maciej.wieczor-retman@intel.com, babu.moger@amd.com, chang.seok.bae@intel.com, sohil.mehta@intel.com, dave.hansen@linux.intel.com, jpoimboe@kernel.org, elena.reshetova@intel.com, hpa@zytor.com, ak@linux.intel.com, darwi@linutronix.de, bp@alien8.de, mingo@redhat.com, x86@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v11 3/4] x86/cpu: Do a sanity check on required feature bits Message-ID: <20260323181528.ux3wslk42j2t6it7@desk> References: <7312c9f2feab8aea4612ed9a1841e8c22f5f69b1.1774008873.git.m.wieczorretman@pm.me> <20260321003015.4i7wrqmaunbljguw@desk> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Sat, Mar 21, 2026 at 05:58:18AM +0000, Maciej Wieczór-Retman wrote: > On 2026-03-20 at 17:31:27 -0700, Pawan Gupta wrote: > >On Fri, Mar 20, 2026 at 12:50:25PM +0000, Maciej Wieczor-Retman wrote: > >... > >> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c > >> index 0e318f3d56cb..92159a0963c8 100644 > >> --- a/arch/x86/kernel/cpu/common.c > >> +++ b/arch/x86/kernel/cpu/common.c > >> @@ -2005,6 +2005,38 @@ const char *x86_cap_name(unsigned int bit, char *buf) > >> return buf; > >> } > >> > >> +/* > >> + * As a sanity check compare the final x86_capability bitmask with the initial > >> + * predefined required feature bits. > >> + */ > >> +static void verify_required_features(const struct cpuinfo_x86 *c) > >> +{ > >> + u32 required_features[NCAPINTS] = REQUIRED_MASK_INIT; > >> + DECLARE_BITMAP(missing, NCAPINTS * 32); > >> + char cap_buf[X86_CAP_BUF_SIZE]; > >> + u32 *missing_u32; > >> + unsigned int i; > >> + u32 error = 0; > >> + > >> + memset(missing, 0, sizeof(missing)); > >> + missing_u32 = (u32 *)missing; > >> + > >> + for (i = 0; i < NCAPINTS; i++) { > >> + missing_u32[i] = ~c->x86_capability[i] & required_features[i]; > >> + error |= missing_u32[i]; > >> + } > >> + > >> + if (!error) > >> + return; > >> + > >> + /* At least one required feature is missing */ > >> + pr_warn("CPU %d: missing required feature(s):", c->cpu_index); > >> + for_each_set_bit(i, missing, NCAPINTS * 32) > >> + pr_cont(" %s", x86_cap_name(i, cap_buf)); > >> + pr_cont("\n"); > >> + add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK); > >> +} > > > >Do we need 2 loops? Can this be simplified as below: > > > >static void verify_required_features(const struct cpuinfo_x86 *c) > >{ > > u32 required_features[NCAPINTS + 1] = REQUIRED_MASK_INIT; > > char cap_buf[X86_CAP_BUF_SIZE]; > > int i, error = 0; > > > > for_each_set_bit(i, (unsigned long *)required_features, NCAPINTS * 32) { > > if (test_bit(i, (unsigned long *)c->x86_capability)) > > continue; > > if (!error) > > pr_warn("CPU %d: missing required feature(s):", c->cpu_index); > > pr_cont(" %s", x86_cap_name(i, cap_buf)); > > error = 1; > > } > > > > if (!error) > > return; > > > > pr_cont("\n"); > > add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK); > >} > > I'll have to test it but one concern I'd have is the pr_cont() in this > context? Since it can technically have asynchronous problems I would > think putting more code between subsequent calls to pr_cont() can > increase the chance of some race condition. But perhaps these two if > checks are not nearly enough for that to happen. You may be right, but relying on number of instructions in the loop for print syncronization seems flawed to begin with. What probably saves from output being garbled is the printk machinery caching the string until a '\n'. I am not fully sure. > Otherwise I liked in the previous approach the steps of setting up a > bitmask with simple bitwise logic operations, then checking the results > later. My main motivation for suggesting the change was to try and use the existing infrastructure for bit operations much as possible. Even in my suggestion test_bit(cap) can be replaced with test_cpu_cap(). > But the above code also works and I think it is easier to read. > So if there is no opposition I'll test it and switch to it for the next > version, thanks :) As I looked at it again, I see that cpu_has() helpers unconditionally returns true for all the required features. #define cpu_has(c, bit) \ (__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \ test_cpu_cap(c, bit)) So this seems inline with Boris's comment, system should crash and burn if any of the required features is missing.