Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 12 Jul 2022 19:14:08 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Martin Fernandez <martin.fernandez@eclypsium.com>
Cc:     Kai Huang <kai.huang@intel.com>, linux-kernel@vger.kernel.org,
        bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
        mingo@redhat.com, tglx@linutronix.de,
        kirill.shutemov@linux.intel.com, daniel.gutson@eclypsium.com,
        hughsient@gmail.com, alex.bazhaniuk@eclypsium.com
Subject: Re: [PATCH v2] x86/cpuinfo: Clear X86_FEATURE_TME if TME/MKTME is
 disabled by BIOS
Message-ID: <Ys3IACV0fuK/vwCq@google.com>
References: <20220704142250.1512726-1-martin.fernandez@eclypsium.com>
 <8d2a3175be8a3aff1d3fc535dd9ab6217cfe1e66.camel@intel.com>
 <CAKgze5azQG1mnOASbpcrs9jTejdXGkXYmezz9bTKuWQoZp5EFg@mail.gmail.com>
 <YsxZKGxVUY61zPEt@google.com>
 <ba321fad38d5f96a240f1e88a11943cea16bb4dd.camel@intel.com>
 <YszFkTZ7RtXS1rd7@google.com>
 <07ff13d590cf290a14232fb113ff4183a6fa352d.camel@intel.com>
 <CAKgze5bzeakLTMEADKiweL3fnrcVzd6K9oJzo7wWDxuriUfgvA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAKgze5bzeakLTMEADKiweL3fnrcVzd6K9oJzo7wWDxuriUfgvA@mail.gmail.com>
Precedence: bulk

On Tue, Jul 12, 2022, Martin Fernandez wrote:
> On 7/11/22, Kai Huang <kai.huang@intel.com> wrote:
> >
> >>
> >> > This patch basically tries to fix the issue that TME flag isn't cleared
> >> > when TME
> >> > is disabled by BIOS.  And fir this purpose, the code change in this
> >> > patch looks
> >> > reasonable to me.  Unless I am mistaken, detect_tme() will be called for
> >> > all
> >> > cpus if TME is supported in CPUID but isn't enabled by BIOS (either
> >> > LOCKED or
> >> > ENABLED bit isn't set).
> >>
> >> But this patch doesn't handle the bypass bit, which _does_ effectively
> >> disable
> >> TME when set.  E.g. the MKTME spec says:
> >>
> >>  Software must inspect the Hardware Encryption Enable (bit 1) and TME
> >> Encryption
> >>  Bypass Enable (bit 31) to determine if TME encryption is enabled.
> >
> > Yeah so my original reply said:
> >
> > "But perhaps it's arguable whether we can also clear TME flag in this
> > case."
> >
> > And I only gave my Acked-by.
> >
> > It completely depends on the purpose of this patch, or what does this patch
> > claim to do.  If it only claims to clear TME bit if BIOS doesn't enable it,
> > then
> > looks fine to me.  If it wants to achieve "clear TME feature flag if
> > encryption
> > isn't active", then yes you are right.
> >
> > But as I said perhaps "whether we should clear TME flag when bypass is
> > enabled"
> > is arguable.  After all, what does TME flag in /proc/cpuinfo imply?
> >
> 
> What we want with this patch is to check whether some kind of memory
> encryption is active. Right now we are doing it by checking the "tme
> active in BIOS" log, so we are not checking the bypass.
> 
> Can you change this bypass bit at runtime? ie, does it make sense to
> check it only once at boot time?

No, the MSR has write-once behavior.  The LOCK bit is set on the first succesful
WRMSR (or amusingly, on the first SMI).

> If no, then maybe it's ok to check that bit in detect_tme and consider
> it for cpuinfo,
> 
> If it can change, then probably it's ok to leave this patch as is, and
> for our use case maybe we can add a sysfs file that reads that msr.