From: "Limonciello, Mario" <mario.limonciello@amd.com>
To: Tom Lendacky <thomas.lendacky@amd.com>,
Borislav Petkov <bp@alien8.de>,
"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
<x86@kernel.org>
Cc: Kees Cook <keescook@chromium.org>,
hughsient@gmail.com,
Martin Fernandez <martin.fernandez@eclypsium.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] x86: Use CPUID 0x8000001f to confirm SME availability
Date: Fri, 11 Feb 2022 13:51:53 -0600 [thread overview]
Message-ID: <a52990c0-3e54-a5f2-22c2-3446e92f4294@amd.com> (raw)
In-Reply-To: <568d9945-ed0c-90a7-d9cf-a5662f1dbce2@amd.com>
On 2/11/2022 08:55, Tom Lendacky wrote:
> On 2/10/22 23:36, Mario Limonciello wrote:
>> An upcoming change will disable the X86 SME feature flag when the
>> kernel hasn't activated SME. Avoid relying upon that when determining
>> whether to call `native_wbinvd` by directly calling the CPUID that
>> indicates it.
>>
>> Suggested-by: Borislav Petkov <bp@alien8.de>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> arch/x86/kernel/process.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
>> index 81d8ef036637..e131d71b3cae 100644
>> --- a/arch/x86/kernel/process.c
>> +++ b/arch/x86/kernel/process.c
>> @@ -765,8 +765,11 @@ void stop_this_cpu(void *dummy)
>> * without the encryption bit, they don't race each other when
>> flushed
>> * and potentially end up with the wrong entry being committed to
>> * memory.
>> + *
>> + * Test the CPUID bit directly because the machine might've cleared
>> + * X86_FEATURE_SME due to cmdline options.
>> */
>> - if (boot_cpu_has(X86_FEATURE_SME))
>> + if (cpuid_eax(0x8000001f) & BIT(0))
>
> Please test this by running kexec and alternating between mem_encrypt=on
> boots of the kexec kernel and mem_encrypt=off boots of the kexec kernel
> to ensure there is no memory corruption in the newly booted kernel. This
> testing needs to be done on hardware that doesn't have the
> X86_FEATURE_SME_COHERENT feature.
>
> Or if you post the generated instructions in this area I should be able
> to determine if the change is safe.
From objdump on process.o, here is the function with this patch
applied, built using gcc 11.2.0:
00000000000011d0 <stop_this_cpu>:
11d0: e8 00 00 00 00 call 11d5 <stop_this_cpu+0x5>
11d5: 55 push %rbp
11d6: 48 89 e5 mov %rsp,%rbp
11d9: 41 54 push %r12
11db: 53 push %rbx
11dc: 48 83 ec 18 sub $0x18,%rsp
11e0: 65 48 8b 04 25 28 00 mov %gs:0x28,%rax
11e7: 00 00
11e9: 48 89 45 e8 mov %rax,-0x18(%rbp)
11ed: 31 c0 xor %eax,%eax
11ef: ff 14 25 00 00 00 00 call *0x0
11f6: e8 00 00 00 00 call 11fb <stop_this_cpu+0x2b>
11fb: 31 f6 xor %esi,%esi
11fd: 48 c7 c3 00 00 00 00 mov $0x0,%rbx
1204: 89 c7 mov %eax,%edi
1206: e8 00 00 00 00 call 120b <stop_this_cpu+0x3b>
120b: e8 00 00 00 00 call 1210 <stop_this_cpu+0x40>
1210: e8 00 00 00 00 call 1215 <stop_this_cpu+0x45>
1215: 41 89 c4 mov %eax,%r12d
1218: 3d ff 1f 00 00 cmp $0x1fff,%eax
121d: 77 49 ja 1268 <stop_this_cpu+0x98>
121f: 4a 03 1c e5 00 00 00 add 0x0(,%r12,8),%rbx
1226: 00
1227: 48 89 df mov %rbx,%rdi
122a: e8 00 00 00 00 call 122f <stop_this_cpu+0x5f>
122f: c7 45 d8 1f 00 00 80 movl $0x8000001f,-0x28(%rbp)
1236: 48 8d 7d d8 lea -0x28(%rbp),%rdi
123a: 48 8d 75 dc lea -0x24(%rbp),%rsi
123e: c7 45 e0 00 00 00 00 movl $0x0,-0x20(%rbp)
1245: 48 8d 55 e0 lea -0x20(%rbp),%rdx
1249: 48 8d 4d e4 lea -0x1c(%rbp),%rcx
124d: ff 14 25 00 00 00 00 call *0x0
1254: f6 45 d8 01 testb $0x1,-0x28(%rbp)
1258: 74 02 je 125c <stop_this_cpu+0x8c>
125a: 0f 09 wbinvd
125c: eb 07 jmp 1265 <stop_this_cpu+0x95>
125e: 0f 00 2d 00 00 00 00 verw 0x0(%rip) # 1265
<stop_this_cpu+0x95>
1265: f4 hlt
1266: eb f4 jmp 125c <stop_this_cpu+0x8c>
1268: 4c 89 e6 mov %r12,%rsi
126b: 48 c7 c7 00 00 00 00 mov $0x0,%rdi
1272: e8 00 00 00 00 call 1277 <stop_this_cpu+0xa7>
1277: eb a6 jmp 121f <stop_this_cpu+0x4f>
1279: 0f 1f 80 00 00 00 00 nopl 0x0(%rax)
>
> Thanks,
> Tom
>
>> native_wbinvd();
>> for (;;) {
>> /*
next prev parent reply other threads:[~2022-02-11 19:52 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-11 5:36 Mario Limonciello
2022-02-11 5:36 ` [PATCH 2/2] x86/cpu: clear SME/SEV/SEV_ES features when not in use Mario Limonciello
2022-02-11 8:31 ` Borislav Petkov
2022-02-11 19:43 ` Limonciello, Mario
2022-02-11 14:46 ` Tom Lendacky
2022-02-11 19:54 ` Limonciello, Mario
2022-02-11 20:42 ` Tom Lendacky
2022-02-11 8:28 ` [PATCH 1/2] x86: Use CPUID 0x8000001f to confirm SME availability Borislav Petkov
2022-02-11 14:55 ` Tom Lendacky
2022-02-11 19:51 ` Limonciello, Mario [this message]
2022-02-11 21:09 ` Tom Lendacky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a52990c0-3e54-a5f2-22c2-3446e92f4294@amd.com \
--to=mario.limonciello@amd.com \
--cc=bp@alien8.de \
--cc=hughsient@gmail.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.fernandez@eclypsium.com \
--cc=thomas.lendacky@amd.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®