Messages in this thread | | | From | Martin Fernandez <> | Date | Tue, 5 Jul 2022 10:21:47 -0300 | Subject | Re: [PATCH v2] x86/cpuinfo: Clear X86_FEATURE_TME if TME/MKTME is disabled by BIOS |
| |
On 7/5/22, Kai Huang <kai.huang@intel.com> wrote: > On Mon, 2022-07-04 at 11:22 -0300, Martin Fernandez wrote: >> Right now the only way to check this is by greping the kernel logs, >> which is inconvenient. This is currently checked for fwupd for >> example. >> >> I understand that cpuinfo is supposed to report every feature in the >> cpu but since AMD is doing the same (and it also broke backwards >> compatibility [1]) for sme/sev I think it's good to have this for >> Intel too. >> >> Another option to prevent greping the logs would be a file in >> sysfs. I'm open to suggestions to where to place this infomartion. I >> saw a proposal about a firmware security filesystem [2]; that would >> fit perfectly. >> >> [1] >> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/patch/?id=08f253ec3767bcfafc5d32617a92cee57c63968e >> >> [2] >> https://lore.kernel.org/all/20220622215648.96723-3-nayna@linux.ibm.com/ > > Leave above to others, but... >> >> Changelog since v1 >> >> Clear the flag not only for BSP but for every cpu in the system. > > ... the changelog history shouldn't be in the commit message. > > You can put one additional '---' after your 'Signed-off-by' and add your > changelog after it. The content between two '---'s will be stripped away > by > 'git am' when maintainer takes the patch: > > Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com> > --- > v1->v2: > xxx > --- > arch/x86/kernel/cpu/intel.c | 1 + > 1 file changed, 1 insertion(+)'
Thanks!, didn't know about it, makes sense.
>> >> Signed-off-by: Martin Fernandez <martin.fernandez@eclypsium.com> >> --- >> arch/x86/kernel/cpu/intel.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c >> index fd5dead8371c..17f23e23f911 100644 >> --- a/arch/x86/kernel/cpu/intel.c >> +++ b/arch/x86/kernel/cpu/intel.c >> @@ -570,6 +570,7 @@ static void detect_tme(struct cpuinfo_x86 *c) >> >> if (!TME_ACTIVATE_LOCKED(tme_activate) || >> !TME_ACTIVATE_ENABLED(tme_activate)) { >> pr_info_once("x86/tme: not enabled by BIOS\n"); >> + clear_cpu_cap(c, X86_FEATURE_TME); >> mktme_status = MKTME_DISABLED; >> return; > > This code change itself looks good to me. > > But, TME actually supports bypassing TME encryption/decryption by setting 1 > to > bit 31 to IA32_TME_ACTIVATE MSR. See 'Table 4-2 IA32_TME_ACTIVATE MSR' in > MKTME > spec below: > > https://edc.intel.com/content/www/us/en/design/ipla/software-development-platforms/client/platforms/alder-lake-desktop/12th-generation-intel-core-processors-datasheet-volume-1-of-2/002/intel-multi-key-total-memory-encryption/ > > When bit 31 is set, the TME is bypassed (no encryption/decryption for KeyID > 0). > > So looks userspace also needs to check this if it wants to truly check > whether > "TME memory encryption" is active. > > But perhaps it's arguable whether we can also clear TME flag in this case.
Yep, that's what I thought.
> So: > > Acked-by: Kai Huang <kai.huang@intel.com> > > > -- > Thanks, > -Kai > > >
| |