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;
MIME-Version: 1.0
In-Reply-To: <07ff13d590cf290a14232fb113ff4183a6fa352d.camel@intel.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>
From:   Martin Fernandez <martin.fernandez@eclypsium.com>
Date:   Tue, 12 Jul 2022 09:59:52 -0300
Message-ID: <CAKgze5bzeakLTMEADKiweL3fnrcVzd6K9oJzo7wWDxuriUfgvA@mail.gmail.com>
Subject: Re: [PATCH v2] x86/cpuinfo: Clear X86_FEATURE_TME if TME/MKTME is
 disabled by BIOS
To:     Kai Huang <kai.huang@intel.com>
Cc:     Sean Christopherson <seanjc@google.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
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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?

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.