From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 36084C433F5 for ; Thu, 31 Mar 2022 06:53:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231515AbiCaGzn (ORCPT ); Thu, 31 Mar 2022 02:55:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55778 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231504AbiCaGzk (ORCPT ); Thu, 31 Mar 2022 02:55:40 -0400 Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6DB17AE56 for ; Wed, 30 Mar 2022 23:53:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1648709633; x=1680245633; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=WB64PwkyHlQUKysJVCgvUlIvhHf1QUAGXu7mKFwUKzM=; b=IKDWBKoM9xSH9A3e1CEU+QLL82wEbu9JjMuG2tqzggZ9Jc0oL6P94QJO G8PCwJdhAd/35aR2tmDddIAMI0sJ24cxQ9T4Y1akDVhOtxgL7ix1dyrLD plckn0krr+s04aEH/aAk80ccGb7UzS3VDDvXMpETn7I8OzKZDzVDK/z1I N+2PzmKeVNfXUlmsOb3vkZMgaVavzPjoT5DWZnaUQ4I5/8P+sU57bdHCt hRsVBP77EUBoatE3Vs71G5ZTKX98LGH7EIS3C2I5Npq6yk0GXb5mWNutN DXRqEABBDnM/sFIaCkys5Vc+S+qgUeqh0xn5kxYtaBts16BvF25UQIgnu g==; X-IronPort-AV: E=McAfee;i="6200,9189,10302"; a="320431804" X-IronPort-AV: E=Sophos;i="5.90,224,1643702400"; d="scan'208";a="320431804" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Mar 2022 23:53:52 -0700 X-IronPort-AV: E=Sophos;i="5.90,224,1643702400"; d="scan'208";a="650163722" Received: from obbruno-mobl.amr.corp.intel.com (HELO guptapa-desk) ([10.252.136.207]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Mar 2022 23:53:52 -0700 Date: Wed, 30 Mar 2022 23:53:50 -0700 From: Pawan Gupta To: Ricardo =?utf-8?Q?Ca=C3=B1uelo?= Cc: linux-kernel@vger.kernel.org, Borislav Petkov , Thadeu Lima de Souza Cascardo , Mark Gross , x86@kernel.org, "H. Peter Anvin" , Ingo Molnar , Thomas Gleixner , John Johansen , Steve Beattie , kernel@collabora.com Subject: Re: [PATCH v2] x86/speculation/srbds: do not try to turn mitigation off when not supported Message-ID: <20220331065350.nuii3ue4fagdsp52@guptapa-desk> References: <20220330082026.1549073-1-ricardo.canuelo@collabora.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20220330082026.1549073-1-ricardo.canuelo@collabora.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Mar 30, 2022 at 10:20:26AM +0200, Ricardo Cañuelo wrote: >When SRBDS is mitigated by TSX OFF, update_srbds_msr will still read and >write to MSR_IA32_MCU_OPT_CTRL even when that is not supported by the >microcode. > >Checking for X86_FEATURE_SRBDS_CTRL as a CPU feature available makes more >sense than checking for SRBDS_MITIGATION_UCODE_NEEDED as the found >"mitigation". > >Signed-off-by: Thadeu Lima de Souza Cascardo >Signed-off-by: Borislav Petkov >Signed-off-by: Ricardo Cañuelo >Tested-by: Ricardo Cañuelo >--- >Hi all, > >This patch was originally posted here: > >https://lore.kernel.org/all/20200609174313.2600320-1-cascardo@canonical.com/#t > >by Boris, based on the original patch by Cascardo, I didn't make any >changes to it. I didn't see it merged or discussed further and I can >still reproduce the issue on a Samsung Galaxy Chromebook 2 (Intel >Cometlake-U). When booted without the proper CPU u-codes, TSX is >disabled (so the CPU isn't vulnerable to SRDBS) but this code still >tries to access an unavailable MSR register so I get these two warning >messages: > >unchecked MSR access error: RDMSR from 0x123 at rIP: 0xffffffff8203707e (update_srbds_msr+0x2e/0xa0) >Call Trace: > > check_bugs+0x994/0xa6e > ? __get_locked_pte+0x8f/0x100 > start_kernel+0x630/0x664 > secondary_startup_64_no_verify+0xd5/0xdb > >unchecked MSR access error: WRMSR to 0x123 (tried to write 0x0000000000000001) at rIP: 0xffffffff820370a9 (update_srbds_msr+0x59/0xa0) >Call Trace: > > check_bugs+0x994/0xa6e > ? __get_locked_pte+0x8f/0x100 > start_kernel+0x630/0x664 > secondary_startup_64_no_verify+0xd5/0xdb > > >This patch avoids them. > > arch/x86/kernel/cpu/bugs.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > >diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c >index 6296e1ebed1d..9b14cb2ec693 100644 >--- a/arch/x86/kernel/cpu/bugs.c >+++ b/arch/x86/kernel/cpu/bugs.c >@@ -443,14 +443,14 @@ void update_srbds_msr(void) > if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) > return; > >- if (srbds_mitigation == SRBDS_MITIGATION_UCODE_NEEDED) >+ if (srbds_mitigation == SRBDS_MITIGATION_UCODE_NEEDED || >+ srbds_mitigation == SRBDS_MITIGATION_TSX_OFF) > return; Returning for TSX OFF case doesn't seem right. System is mitigated already and there is no need to incur performance loss due to microcode mitigation. So we need to write to the MSR for TSX OFF case to disable the microcode mitigation. IIUC root of the issue is the logic in srbds_select_mitigation() that gives precedence to TSX_OFF over UCODE_NEEDED: srbds_select_mitigation() { [...] if ((ia32_cap & ARCH_CAP_MDS_NO) && !boot_cpu_has(X86_FEATURE_RTM)) srbds_mitigation = SRBDS_MITIGATION_TSX_OFF; else if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) srbds_mitigation = SRBDS_MITIGATION_HYPERVISOR; else if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED; else if (cpu_mitigations_off() || srbds_off) srbds_mitigation = SRBDS_MITIGATION_OFF; If we don't want to touch this logic, below can be a simple fix: --- diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index 6296e1ebed1d..575817163354 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -437,7 +437,8 @@ void update_srbds_msr(void) { u64 mcu_ctrl; - if (!boot_cpu_has_bug(X86_BUG_SRBDS)) + if (!boot_cpu_has_bug(X86_BUG_SRBDS) || + !boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) return; if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) --- Thanks, Pawan