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 ; 08 Apr 2020 20:27:00 -0000 Received: from mga18.intel.com ([134.134.136.126]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1jMHHm-000414-Od for speck@linutronix.de; Wed, 08 Apr 2020 22:26:59 +0200 Received: from localhost (mtg-dev.jf.intel.com [10.54.74.10]) by smtp.ostc.intel.com (Postfix) with ESMTP id 6C8A46363 for ; Wed, 8 Apr 2020 20:26:52 +0000 (UTC) Date: Wed, 8 Apr 2020 13:26:52 -0700 From: mark gross Subject: [MODERATED] Re: [PATCH 2/3] V5 more sampling fun 2 Message-ID: <20200408202652.GA136636@mtg-dev.jf.intel.com> Reply-To: mgross@linux.intel.com References: <20200406220714.q2jzjftqlxqnhco7@treble> <20200407003456.GA58233@mtg-dev.jf.intel.com> <20200407123955.ejjkigwkgfm2v4dt@treble> MIME-Version: 1.0 In-Reply-To: <20200407123955.ejjkigwkgfm2v4dt@treble> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Tue, Apr 07, 2020 at 07:39:55AM -0500, speck for Josh Poimboeuf wrote: > On Mon, Apr 06, 2020 at 05:34:56PM -0700, speck for mark gross wrote: > > On Mon, Apr 06, 2020 at 05:07:14PM -0500, speck for Josh Poimboeuf 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) { > > > > + if (!boot_cpu_has(X86_FEATURE_RTM)) > > > > + srbds_mitigation = SRBDS_NOT_AFFECTED_TSX_OFF; > > > > + } > > > > > > A 'goto out' would be helpful here; then the TSX_OFF checks below aren't > > > needed and the flow is simplified. > > a goto out would mess up the hypervisor check but, I'll add the goto for the > > mid function returns that set the srbds_mitigation value. > > Just to clarify, I was thinking something like: > > ia32_cap = x86_read_arch_cap_msr(); > if ((ia32_cap & ARCH_CAP_MDS_NO) && !boot_cpu_has(X86_FEATURE_RTM)) { > srbds_mitigation = SRBDS_NOT_AFFECTED_TSX_OFF; > goto out; > } > > As far as I can tell, that doesn't mess up the hypervisor check, since > it only sets SRBDS_HYPERVISOR if TSX_OFF isn't set. after looking again I agree and using the goto allows simplification of the following if block. > > > > > > + > > > > + if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) { > > > > + if (srbds_mitigation != SRBDS_NOT_AFFECTED_TSX_OFF) > > > > + srbds_mitigation = SRBDS_HYPERVISOR; > > > > + return; with your goto out aboe I can simplify this if block by removing the nested if. Thanks! --mark > > > > + } > > > > + > > > > +/* > > > > + * List affected CPU's for issues that cannot be enumerated. > > > > + */ > > > > > > I don't understand the comment, SRBDS seems to be enumerated above. > > hmm, how about I remove the comment? > > Sounds good to me. > > -- > Josh