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 ; 25 Sep 2019 22:48:29 -0000 Received: from mx1.redhat.com ([209.132.183.28]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1iDG5C-0003Bj-PS for speck@linutronix.de; Thu, 26 Sep 2019 00:48:27 +0200 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 05D61C0546D5 for ; Wed, 25 Sep 2019 22:48:20 +0000 (UTC) Received: from treble (ovpn-120-76.rdu2.redhat.com [10.10.120.76]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A9E6F5C1D4 for ; Wed, 25 Sep 2019 22:48:19 +0000 (UTC) Date: Wed, 25 Sep 2019 17:48:17 -0500 From: Josh Poimboeuf Subject: [MODERATED] Re: [PATCH v4 03/10] TAAv4 3 Message-ID: <20190925224817.zcmzswojw5tmymul@treble> References: =?utf-8?q?=3Cfd65e9c9614b1ad51b15c6f0ada5266a5e73235b=2E1567543894=2Egi?= =?utf-8?q?t=2Epawan=2Ekumar=2Egupta=40linux=2Eintel=2Ecom=3E?= <5b6df5ee-a5b7-c281-de29-af6544b8abb6@intel.com> <20190906074645.GE13480@guptapadev.amr> MIME-Version: 1.0 In-Reply-To: <20190906074645.GE13480@guptapadev.amr> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Fri, Sep 06, 2019 at 12:46:45AM -0700, speck for Pawan Gupta wrote: > On Wed, Sep 04, 2019 at 05:19:43AM -0700, speck for Dave Hansen wrote: > > > diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c > > > index e7b1fe929cab..cdfc6c3d11d1 100644 > > > --- a/arch/x86/kernel/cpu/tsx.c > > > +++ b/arch/x86/kernel/cpu/tsx.c > > > @@ -29,6 +29,18 @@ static void tsx_disable(void) > > > wrmsrl(MSR_IA32_TSX_CTRL, tsx); > > > } > > > > > > +static void tsx_enable(void) > > > +{ > > > + u64 tsx; > > > + > > > + rdmsrl(MSR_IA32_TSX_CTRL, tsx); > > > + > > > + tsx &= ~MSR_TSX_CTRL_RTM_DISABLE; > > > + tsx &= ~MSR_TSX_CTRL_CPUID_CLEAR; > > > + > > > + wrmsrl(MSR_IA32_TSX_CTRL, tsx); > > > +} > > > > OK, so in the last patch we went through all the steps to enumerate this > > sucker. Is that still being respected here? > > This doesn't affect the enumeration, only adds support for tsx=on case. > > > > > Also, how would these bits have gotten set so that we need to clear them > > here? kexec? > > Yes. So you're saying we need to support the case where we booted with tsx=off, but then want to kexec with tsx=on? I don't know, that sounds a little esoteric to me. Do we actually support that type of scenario for other cmdline options? > > > +static int __init tsx_cmdline(char *str) > > > +{ > > > + if (!str) > > > + return -EINVAL; > > > + > > > + if (!strcmp(str, "on")) > > > + tsx_ctrl_state = TSX_CTRL_ENABLE; > > > + else if (!strcmp(str, "off")) > > > + tsx_ctrl_state = TSX_CTRL_DISABLE; > > > + > > > + return 0; > > > +} > > > +early_param("tsx", tsx_cmdline); > > > > Hmm... Let's say I have a non-TSX system. I specify tsx=on. This will > > set tsx_ctrl_state=TSX_CTRL_ENABLE, then tsx_ctrl_check_support() will > > set it to tsx_ctrl_state=TSX_CTRL_NOT_SUPPORTED. > > > > I *think* this all works, but I'd love a comment or two about it. Maybe > > even something that makes it clear that tsx_en/disable() are both only > > called when TSX *AND* TSX_CTRL_MSR are supported. > > > > This is all kinda weird and confusing because we're doing this all > > without the X86_FEATURE bits for TSX. > > If we add another X86_FEATURE bit it will have exact same state as > X86_FEATURE_RTM. Do we really need this? How about moving the tsx_init() call to early_init_intel()? Then you'd be able to check the feature bit above. And that might make the ordering a little easier to follow. -- Josh