From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linutronix.de (193.142.43.55:993) by crypto-ml.lab.linutronix.de with IMAP4-SSL for ; 22 Oct 2019 05:59:23 -0000 Received: from mga14.intel.com ([192.55.52.115]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1iMnCT-0000Sl-Ne for speck@linutronix.de; Tue, 22 Oct 2019 07:59:22 +0200 Date: Mon, 21 Oct 2019 22:53:10 -0700 From: Pawan Gupta Subject: [MODERATED] Re: ... Message-ID: <20191022055310.GA12194@guptapadev.amr> References: <4f718d5b-b578-ddd9-9898-36c6088a4615@redhat.com> MIME-Version: 1.0 In-Reply-To: <4f718d5b-b578-ddd9-9898-36c6088a4615@redhat.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Tue, Oct 22, 2019 at 12:10:31AM -0400, speck for Jon Masters wrote: > Subject: Re: [PATCH v7 07/10] TAAv7 7 > On 10/21/19 4:29 PM, speck for Pawan Gupta wrote: > > From: Pawan Gupta > > Subject: [PATCH v7 07/10] x86/tsx: Add "auto" option to TSX cmdline parameter > > > > Platforms which are not affected by X86_BUG_TAA may want the TSX feature > > enabled. Add "auto" option to the TSX cmdline parameter. When tsx=auto > > disable TSX when X86_BUG_TAA is present, otherwise enable TSX. > > Earlier, you do this: > > + if (!(ia32_cap & ARCH_CAP_TAA_NO) && > + (boot_cpu_has(X86_FEATURE_RTM) || > + (ia32_cap & ARCH_CAP_TSX_CTRL_MSR))) > + setup_force_cpu_bug(X86_BUG_TAA); > > Per the other discussion, I think you want to double check if tsx=auto > is doing what folks want it to do, because currently I think auto still > has the semantics of turning off TSX on everything, rather than just > those cases where a VERW mitigation won't suffice/is in use. tsx=auto can only disable TSX on current CPUs(MDS_NO=1) because older CPUs(MDS_NO=0) wont get TSX_CTRL_MSR. On older CPUs tsx_init() would simply do nothing because of lack of TSX_CTRL. void __init tsx_init(void) { if (!tsx_ctrl_is_supported()) return; [...] > > (see Thomas's update to the "Re: [PATCH v5 08/11] TAAv5 8" diagram) MDS_NO MD_CLEAR TSX_CTRL_MSR 0 0 0 Vulnerable (needs ucode) 0 1 0 MDS and TAA mitigated via VERW 1 1 0 MDS fixed, TAA vulnerable if TSX enabled because MD_CLEAR has no meaning and VERW is not guaranteed to clear buffers 1 X 1 MDS fixed, TAA can be mitigated by VERW or TSX_CTRL_MSR Thomas's table sums up the mitigation status. > > It seems that it's a good time to double check if that's what everyone > on the distro side is expecting, namely "tsx=auto will disable TSX > automatically on those parts impacted by TAA for which VERW mitigation > is not available". Microcode update for MDS_NO=1 parts adds both TSX_CTRL and VERW mitigation at the same time. It is for the OS to choose(tsx=) which mitigation to deploy. For MDS_NO=0 CPUs: VERW mitigation will be deployed. tsx= is ineffective, for the lack of TSX_CTRL. For MDS_NO=1 CPUs: If OS chooses tsx=on, VERW mitigation will be deployed. If OS chooses tsx=auto, TSX will be disabled. If OS chooses tsx=off, TSX will be disabled (for TAA_NO=1 as well). More details on tsx=auto: +----------+----------+----------------+---------------+--------------+-------------------+ | MSR_IA32_ARCH_CAPABILITIES bits | Result with cmdline tsx=auto | +----------+----------+----------------+---------------+--------------+-------------------+ | TAA_NO | MDS_NO | TSX_CTRL_MSR | VERW clears | TSX state | TAA mitigation | | | | | CPU buffers | after bootup | | +==========+==========+================+===============+==============+===================+ | 0 | 0 | 0 | Yes | HW default | Same as MDS | +----------+----------+----------------+---------------+--------------+-------------------+ | 0 | 0 | 1 | Invalid case | Invalid case | Invalid case | +----------+----------+----------------+---------------+--------------+-------------------+ | 0 | 1 | 0 | No | HW default | Need ucode update | +----------+----------+----------------+---------------+--------------+-------------------+ | 0 | 1 | 1 | Yes | TSX disabled | TSX disabled | +----------+----------+----------------+---------------+--------------+-------------------+ | 1 | X | 1 | X | TSX enabled | None needed | +----------+----------+----------------+---------------+--------------+-------------------+ Note: MDS_NO=0 and TSX_CTRL_MSR=1 is an invalid case above, which prevents tsx=auto from disabling TSX in older CPUs. >This seems to reduce the set where we end up > disabling TSX essentially to a few very recent processors (e.g. > CascadeLake some second gen, Bx stepping?). If that's what we are all > expecting/are planning to do, I suspect Red Hat would go with tsx=auto > as a default since the set of impacted processors can be documented. > > Anyway, auto isn't right yet. Below change isn't required as per the above explanation, but I can add it if adds clarity. Or maybe cover with code comments. diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c index 0969e6e9dff3..7b567ca266c9 100644 --- a/arch/x86/kernel/cpu/tsx.c +++ b/arch/x86/kernel/cpu/tsx.c @@ -70,7 +70,8 @@ static bool __init tsx_ctrl_is_supported(void) static enum tsx_ctrl_states x86_get_tsx_auto_mode(void) { - if (boot_cpu_has_bug(X86_BUG_TAA)) + if (boot_cpu_has_bug(X86_BUG_TAA) && + (ARCH_CAP_MDS_NO & x86_read_arch_cap_msr())) return TSX_CTRL_DISABLE; return TSX_CTRL_ENABLE;