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;
Message-ID: <07ff13d590cf290a14232fb113ff4183a6fa352d.camel@intel.com>
Subject: Re: [PATCH v2] x86/cpuinfo: Clear X86_FEATURE_TME if TME/MKTME is
 disabled by BIOS
From:   Kai Huang <kai.huang@intel.com>
To:     Sean Christopherson <seanjc@google.com>
Cc:     Martin Fernandez <martin.fernandez@eclypsium.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
Date:   Tue, 12 Jul 2022 13:39:06 +1200
In-Reply-To: <YszFkTZ7RtXS1rd7@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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.2 (3.44.2-1.fc36) 
MIME-Version: 1.0
Precedence: bulk


>=20
> > 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 pat=
ch looks
> > reasonable to me.  Unless I am mistaken, detect_tme() will be called fo=
r all
> > cpus if TME is supported in CPUID but isn't enabled by BIOS (either LOC=
KED or
> > ENABLED bit isn't set).
>=20
> But this patch doesn't handle the bypass bit, which _does_ effectively di=
sable
> TME when set.  E.g. the MKTME spec says:
>=20
>  Software must inspect the Hardware Encryption Enable (bit 1) and TME Enc=
ryption
>  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 encryp=
tion
isn't active", then yes you are right. =C2=A0

But as I said perhaps "whether we should clear TME flag when bypass is enab=
led"
is arguable.  After all, what does TME flag in /proc/cpuinfo imply?