Messages in this thread | | | Date | Tue, 12 Jul 2022 19:14:08 +0000 | From | Sean Christopherson <> | Subject | Re: [PATCH v2] x86/cpuinfo: Clear X86_FEATURE_TME if TME/MKTME is disabled by BIOS |
| |
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.
| |