TDX guests are not allowed to clear CR4.MCE. Attempt to clear it leads
to #VE.
Use alternatives to keep the flag during kexec for TDX guests.
The change doesn't affect non-TDX-guest environments.
Signed-off-by: Kirill A. Shutemov <[email protected]>
Reviewed-by: Kai Huang <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>
---
arch/x86/kernel/relocate_kernel_64.S | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
index 56cab1bb25f5..e144bcf60cbe 100644
--- a/arch/x86/kernel/relocate_kernel_64.S
+++ b/arch/x86/kernel/relocate_kernel_64.S
@@ -5,6 +5,8 @@
*/
#include <linux/linkage.h>
+#include <linux/stringify.h>
+#include <asm/alternative.h>
#include <asm/page_types.h>
#include <asm/kexec.h>
#include <asm/processor-flags.h>
@@ -145,12 +147,15 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
* Set cr4 to a known state:
* - physical address extension enabled
* - 5-level paging, if it was enabled before
+ * - Machine check exception on TDX guest. Clearing MCE is not allowed
+ * in TDX guests.
*/
movl $X86_CR4_PAE, %eax
testq $X86_CR4_LA57, %r13
jz 1f
orl $X86_CR4_LA57, %eax
1:
+ ALTERNATIVE "", __stringify(orl $X86_CR4_MCE, %eax), X86_FEATURE_TDX_GUEST
movq %rax, %cr4
jmp 1f
--
2.43.0
On 3/25/2024 6:38 PM, Kirill A. Shutemov wrote:
> TDX guests are not allowed to clear CR4.MCE. Attempt to clear it leads
> to #VE.
Will we consider making it more safe and compatible for future to guard
against X86_FEATURE_MCE as well?
If in the future, MCE becomes configurable for TD guest, then CR4.MCE
might not be fixed1.
> Use alternatives to keep the flag during kexec for TDX guests.
>
> The change doesn't affect non-TDX-guest environments.
>
> Signed-off-by: Kirill A. Shutemov <[email protected]>
> Reviewed-by: Kai Huang <[email protected]>
> Reviewed-by: Thomas Gleixner <[email protected]>
> ---
> arch/x86/kernel/relocate_kernel_64.S | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
> index 56cab1bb25f5..e144bcf60cbe 100644
> --- a/arch/x86/kernel/relocate_kernel_64.S
> +++ b/arch/x86/kernel/relocate_kernel_64.S
> @@ -5,6 +5,8 @@
> */
>
> #include <linux/linkage.h>
> +#include <linux/stringify.h>
> +#include <asm/alternative.h>
> #include <asm/page_types.h>
> #include <asm/kexec.h>
> #include <asm/processor-flags.h>
> @@ -145,12 +147,15 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
> * Set cr4 to a known state:
> * - physical address extension enabled
> * - 5-level paging, if it was enabled before
> + * - Machine check exception on TDX guest. Clearing MCE is not allowed
> + * in TDX guests.
> */
> movl $X86_CR4_PAE, %eax
> testq $X86_CR4_LA57, %r13
> jz 1f
> orl $X86_CR4_LA57, %eax
> 1:
> + ALTERNATIVE "", __stringify(orl $X86_CR4_MCE, %eax), X86_FEATURE_TDX_GUEST
> movq %rax, %cr4
>
> jmp 1f
On Fri, Mar 29, 2024 at 11:21:32PM +0800, Xiaoyao Li wrote:
> On 3/25/2024 6:38 PM, Kirill A. Shutemov wrote:
> > TDX guests are not allowed to clear CR4.MCE. Attempt to clear it leads
> > to #VE.
>
> Will we consider making it more safe and compatible for future to guard
> against X86_FEATURE_MCE as well?
>
> If in the future, MCE becomes configurable for TD guest, then CR4.MCE might
> not be fixed1.
Good point.
I guess we can leave it clear if it was clear. This should be easy
enough. But we might want to clear even if was set if clearing is allowed.
It would require some kind of indication that clearing MCE is fine. We
don't have such indication yet. Not sure we can reasonably future-proof
the code at this point.
But let me think more.
--
Kiryl Shutsemau / Kirill A. Shutemov
On Fri, Mar 29, 2024 at 06:48:21PM +0200, Kirill A. Shutemov wrote:
> On Fri, Mar 29, 2024 at 11:21:32PM +0800, Xiaoyao Li wrote:
> > On 3/25/2024 6:38 PM, Kirill A. Shutemov wrote:
> > > TDX guests are not allowed to clear CR4.MCE. Attempt to clear it leads
> > > to #VE.
> >
> > Will we consider making it more safe and compatible for future to guard
> > against X86_FEATURE_MCE as well?
> >
> > If in the future, MCE becomes configurable for TD guest, then CR4.MCE might
> > not be fixed1.
>
> Good point.
>
> I guess we can leave it clear if it was clear. This should be easy
> enough. But we might want to clear even if was set if clearing is allowed.
>
> It would require some kind of indication that clearing MCE is fine. We
> don't have such indication yet. Not sure we can reasonably future-proof
> the code at this point.
>
> But let me think more.
I think I will go with the variant below.
diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
index 56cab1bb25f5..8e2037d78a1f 100644
--- a/arch/x86/kernel/relocate_kernel_64.S
+++ b/arch/x86/kernel/relocate_kernel_64.S
@@ -5,6 +5,8 @@
*/
#include <linux/linkage.h>
+#include <linux/stringify.h>
+#include <asm/alternative.h>
#include <asm/page_types.h>
#include <asm/kexec.h>
#include <asm/processor-flags.h>
@@ -145,11 +147,17 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
* Set cr4 to a known state:
* - physical address extension enabled
* - 5-level paging, if it was enabled before
+ * - Machine check exception on TDX guest, if it was enabled before.
+ * Clearing MCE might not allowed in TDX guests, depending on setup.
*/
movl $X86_CR4_PAE, %eax
testq $X86_CR4_LA57, %r13
jz 1f
orl $X86_CR4_LA57, %eax
+1:
+ testq $X86_CR4_MCE, %r13
+ jz 1f
+ ALTERNATIVE "", __stringify(orl $X86_CR4_MCE, %eax), X86_FEATURE_TDX_GUEST
1:
movq %rax, %cr4
--
Kiryl Shutsemau / Kirill A. Shutemov
Depending on setup, TDX guests might be allowed to clear CR4.MCE.
Attempt to clear it leads to #VE.
Use alternatives to keep the flag during kexec for TDX guests.
The change doesn't affect non-TDX-guest environments.
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
v9.1:
- Preserve CR4.MCE instead of setting it unconditionally.
---
arch/x86/kernel/relocate_kernel_64.S | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
index 56cab1bb25f5..8e2037d78a1f 100644
--- a/arch/x86/kernel/relocate_kernel_64.S
+++ b/arch/x86/kernel/relocate_kernel_64.S
@@ -5,6 +5,8 @@
*/
#include <linux/linkage.h>
+#include <linux/stringify.h>
+#include <asm/alternative.h>
#include <asm/page_types.h>
#include <asm/kexec.h>
#include <asm/processor-flags.h>
@@ -145,11 +147,17 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
* Set cr4 to a known state:
* - physical address extension enabled
* - 5-level paging, if it was enabled before
+ * - Machine check exception on TDX guest, if it was enabled before.
+ * Clearing MCE might not allowed in TDX guests, depending on setup.
*/
movl $X86_CR4_PAE, %eax
testq $X86_CR4_LA57, %r13
jz 1f
orl $X86_CR4_LA57, %eax
+1:
+ testq $X86_CR4_MCE, %r13
+ jz 1f
+ ALTERNATIVE "", __stringify(orl $X86_CR4_MCE, %eax), X86_FEATURE_TDX_GUEST
1:
movq %rax, %cr4
--
2.43.0
On 3/04/2024 4:42 am, Kirill A. Shutemov wrote:
> On Fri, Mar 29, 2024 at 06:48:21PM +0200, Kirill A. Shutemov wrote:
>> On Fri, Mar 29, 2024 at 11:21:32PM +0800, Xiaoyao Li wrote:
>>> On 3/25/2024 6:38 PM, Kirill A. Shutemov wrote:
>>>> TDX guests are not allowed to clear CR4.MCE. Attempt to clear it leads
>>>> to #VE.
>>>
>>> Will we consider making it more safe and compatible for future to guard
>>> against X86_FEATURE_MCE as well?
>>>
>>> If in the future, MCE becomes configurable for TD guest, then CR4.MCE might
>>> not be fixed1.
>>
>> Good point.
>>
>> I guess we can leave it clear if it was clear. This should be easy
>> enough. But we might want to clear even if was set if clearing is allowed.
>>
>> It would require some kind of indication that clearing MCE is fine. We
>> don't have such indication yet. Not sure we can reasonably future-proof
>> the code at this point.
>>
>> But let me think more.
>
> I think I will go with the variant below.
>
> diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
> index 56cab1bb25f5..8e2037d78a1f 100644
> --- a/arch/x86/kernel/relocate_kernel_64.S
> +++ b/arch/x86/kernel/relocate_kernel_64.S
> @@ -5,6 +5,8 @@
> */
>
> #include <linux/linkage.h>
> +#include <linux/stringify.h>
> +#include <asm/alternative.h>
> #include <asm/page_types.h>
> #include <asm/kexec.h>
> #include <asm/processor-flags.h>
> @@ -145,11 +147,17 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
> * Set cr4 to a known state:
> * - physical address extension enabled
> * - 5-level paging, if it was enabled before
> + * - Machine check exception on TDX guest, if it was enabled before.
> + * Clearing MCE might not allowed in TDX guests, depending on setup.
Nit: Perhaps we can just call out:
Clearing MCE is not allowed if it _was_ enabled before.
Which is always true I suppose.
> */
> movl $X86_CR4_PAE, %eax
> testq $X86_CR4_LA57, %r13
> jz 1f
> orl $X86_CR4_LA57, %eax
> +1:
> + testq $X86_CR4_MCE, %r13
> + jz 1f
> + ALTERNATIVE "", __stringify(orl $X86_CR4_MCE, %eax), X86_FEATURE_TDX_GUEST
> 1:
> movq %rax, %cr4
>
Anyway,
Reviewed-by: Kai Huang <[email protected]>
On Thu, Apr 04, 2024 at 10:40:34AM +1300, Huang, Kai wrote:
>
>
> On 3/04/2024 4:42 am, Kirill A. Shutemov wrote:
> > On Fri, Mar 29, 2024 at 06:48:21PM +0200, Kirill A. Shutemov wrote:
> > > On Fri, Mar 29, 2024 at 11:21:32PM +0800, Xiaoyao Li wrote:
> > > > On 3/25/2024 6:38 PM, Kirill A. Shutemov wrote:
> > > > > TDX guests are not allowed to clear CR4.MCE. Attempt to clear it leads
> > > > > to #VE.
> > > >
> > > > Will we consider making it more safe and compatible for future to guard
> > > > against X86_FEATURE_MCE as well?
> > > >
> > > > If in the future, MCE becomes configurable for TD guest, then CR4.MCE might
> > > > not be fixed1.
> > >
> > > Good point.
> > >
> > > I guess we can leave it clear if it was clear. This should be easy
> > > enough. But we might want to clear even if was set if clearing is allowed.
> > >
> > > It would require some kind of indication that clearing MCE is fine. We
> > > don't have such indication yet. Not sure we can reasonably future-proof
> > > the code at this point.
> > >
> > > But let me think more.
> >
> > I think I will go with the variant below.
> >
> > diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
> > index 56cab1bb25f5..8e2037d78a1f 100644
> > --- a/arch/x86/kernel/relocate_kernel_64.S
> > +++ b/arch/x86/kernel/relocate_kernel_64.S
> > @@ -5,6 +5,8 @@
> > */
> > #include <linux/linkage.h>
> > +#include <linux/stringify.h>
> > +#include <asm/alternative.h>
> > #include <asm/page_types.h>
> > #include <asm/kexec.h>
> > #include <asm/processor-flags.h>
> > @@ -145,11 +147,17 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
> > * Set cr4 to a known state:
> > * - physical address extension enabled
> > * - 5-level paging, if it was enabled before
> > + * - Machine check exception on TDX guest, if it was enabled before.
> > + * Clearing MCE might not allowed in TDX guests, depending on setup.
>
> Nit: Perhaps we can just call out:
>
> Clearing MCE is not allowed if it _was_ enabled before.
>
> Which is always true I suppose.
It is true now. Future TDX will allow to clear CR4.MCE and we don't want
to flip it back on in this case.
--
Kiryl Shutsemau / Kirill A. Shutemov