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 ; 14 Apr 2020 20:03:24 -0000 Received: from mga07.intel.com ([134.134.136.100]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1jORmE-00033d-77 for speck@linutronix.de; Tue, 14 Apr 2020 22:03:23 +0200 Received: from localhost (mtg-dev.jf.intel.com [10.54.74.10]) by smtp.ostc.intel.com (Postfix) with ESMTP id 581286363 for ; Tue, 14 Apr 2020 20:03:12 +0000 (UTC) Date: Tue, 14 Apr 2020 13:03:12 -0700 From: mark gross Subject: [MODERATED] Re: [PATCH 3/4] V7 more sampling fun 3 Message-ID: <20200414200312.GB29751@mtg-dev.jf.intel.com> Reply-To: mgross@linux.intel.com References: <20200414034843.GA5995@mtg-dev.jf.intel.com> <87h7xmniid.fsf@nanos.tec.linutronix.de> MIME-Version: 1.0 In-Reply-To: <87h7xmniid.fsf@nanos.tec.linutronix.de> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Tue, Apr 14, 2020 at 06:23:38PM +0200, speck for Thomas Gleixner wrote: > Mark, > > speck for mark gross writes: > > On Thu, Jan 16, 2020 at 02:16:07PM -0800, speck for mark gross wrote: > >> + /* > >> + * Check to see if this is one of the MDS_NO systems supporting > >> + * TSX that are only exposed to SRBDS when TSX is enabled. > >> + */ > >> + ia32_cap = x86_read_arch_cap_msr(); > >> + if ((ia32_cap & ARCH_CAP_MDS_NO) && !boot_cpu_has(X86_FEATURE_RTM)) { > >> + srbds_mitigation = SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF; > >> + goto out; > >> + } > >> + > >> + if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) { > >> + srbds_mitigation = SRBDS_MITIGATION_HYPERVISOR; > >> + goto out; > >> + } > >> + > >> + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) { > >> + srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED; > >> + goto out; > >> + } > >> + > >> + if (cpu_mitigations_off() || srbds_off) { > >> + if (srbds_mitigation != SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF) > >> + srbds_mitigation = SRBDS_MITIGATION_OFF; > >> + } > >> +out: > > The test for cpu_mitigations_off or srbds off needs to be after out: > > Otherwise when TSX is off and srbds=off will report the wrong answer. > > If the CPU has SRBDS_IF_TSX and TSX is disabled then the correct > answer is: Not affected (TSX off) correct. > That's what we do with other issues as well. If the CPU is not affected > then we print this even with mitigation disabled (all or particular). That is what I'm working toward. Now that I look at the code the right answer is to not move the out: lable and nuke the nested if int eht srbds_off branch. That is whats wrong. That said I plan to us your if/else construct on the next version and forget about the goto's. --mark