From: Alexander Shishkin <alexander.shishkin@linux.intel.com>

In order to map the EFI runtime services, set_virtual_address_map()
needs to be called, which resides in the lower half of the address
space. This means that LASS needs to be temporarily disabled around
this call. This can only be done before the CR pinning is set up.

Move CR pinning setup behind the EFI initialization.

Wrapping efi_enter_virtual_mode() into lass_disable/enable_enforcement()
is not enough because AC flag gates data accesses, but not instruction
fetch. Clearing the CR4 bit is required.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 arch/x86/kernel/cpu/common.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 4f430be285de..9918121e0adc 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2081,7 +2081,6 @@ static __init void identify_boot_cpu(void)
 	enable_sep_cpu();
 #endif
 	cpu_detect_tlb(&boot_cpu_data);
-	setup_cr_pinning();
 
 	tsx_init();
 	tdx_init();
@@ -2532,10 +2531,14 @@ void __init arch_cpu_finalize_init(void)
 
 	/*
 	 * This needs to follow the FPU initializtion, since EFI depends on it.
+	 *
+	 * EFI twiddles CR4.LASS. Do it before CR pinning.
 	 */
 	if (efi_enabled(EFI_RUNTIME_SERVICES))
 		efi_enter_virtual_mode();
 
+	setup_cr_pinning();
+
 	/*
 	 * Ensure that access to the per CPU representation has the initial
 	 * boot CPU configuration.
-- 
2.47.2

On 7/1/25 02:58, Kirill A. Shutemov wrote:
> Move CR pinning setup behind the EFI initialization.

I kinda grumble about these one-off solutions. Could we just do this
once and for all and defer CR pinning as long as possible? For instance,
could we do it in a late_initcall()?

Do we need pinning before userspace comes up?

On Tue, Jul 01, 2025 at 04:10:19PM -0700, Dave Hansen wrote:
> On 7/1/25 02:58, Kirill A. Shutemov wrote:
> > Move CR pinning setup behind the EFI initialization.
> 
> I kinda grumble about these one-off solutions. Could we just do this
> once and for all and defer CR pinning as long as possible? For instance,
> could we do it in a late_initcall()?
> 
> Do we need pinning before userspace comes up?

Hm. I operated from an assumption that we want to pin control registers as
early as possible to get most benefit from it.

I guess we can defer it until later. But I am not sure late_initcall() is
the right place. Do we want random driver to twiddle control registers?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

On Wed, Jul 02, 2025 at 01:05:23PM +0300, Kirill A. Shutemov wrote:
> On Tue, Jul 01, 2025 at 04:10:19PM -0700, Dave Hansen wrote:
> > On 7/1/25 02:58, Kirill A. Shutemov wrote:
> > > Move CR pinning setup behind the EFI initialization.
> > 
> > I kinda grumble about these one-off solutions. Could we just do this
> > once and for all and defer CR pinning as long as possible? For instance,
> > could we do it in a late_initcall()?
> > 
> > Do we need pinning before userspace comes up?
> 
> Hm. I operated from an assumption that we want to pin control registers as
> early as possible to get most benefit from it.
> 
> I guess we can defer it until later. But I am not sure late_initcall() is
> the right place. Do we want random driver to twiddle control registers?

I will do it in core_initcall().

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

On 7/1/2025 2:58 AM, Kirill A. Shutemov wrote:
> From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> 
> In order to map the EFI runtime services, set_virtual_address_map()
> needs to be called, which resides in the lower half of the address
> space. This means that LASS needs to be temporarily disabled around
> this call. This can only be done before the CR pinning is set up.
> 
> Move CR pinning setup behind the EFI initialization.
> 
> Wrapping efi_enter_virtual_mode() into lass_disable/enable_enforcement()

I believe this should be lass_stac()/clac() since we reverted to the
original naming.

> is not enough because AC flag gates data accesses, but not instruction
> fetch. Clearing the CR4 bit is required.
> 
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/common.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 4f430be285de..9918121e0adc 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -2081,7 +2081,6 @@ static __init void identify_boot_cpu(void)
>  	enable_sep_cpu();
>  #endif
>  	cpu_detect_tlb(&boot_cpu_data);
> -	setup_cr_pinning();
>  
>  	tsx_init();
>  	tdx_init();
> @@ -2532,10 +2531,14 @@ void __init arch_cpu_finalize_init(void)
>  
>  	/*
>  	 * This needs to follow the FPU initializtion, since EFI depends on it.
> +	 *
> +	 * EFI twiddles CR4.LASS. Do it before CR pinning.
>  	 */
>  	if (efi_enabled(EFI_RUNTIME_SERVICES))
>  		efi_enter_virtual_mode();
>  
> +	setup_cr_pinning();
> +

Instead of EFI toggling CR4.LASS, why not defer the first LASS
activation itself?

i.e.

	if (efi_enabled(EFI_RUNTIME_SERVICES))
		efi_enter_virtual_mode();

	setup_lass();

	setup_cr_pinning();


This way, we can avoid the following patch (#5) altogether.

On Tue, Jul 01, 2025 at 12:03:01PM -0700, Sohil Mehta wrote:
> On 7/1/2025 2:58 AM, Kirill A. Shutemov wrote:
> > From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > 
> > In order to map the EFI runtime services, set_virtual_address_map()
> > needs to be called, which resides in the lower half of the address
> > space. This means that LASS needs to be temporarily disabled around
> > this call. This can only be done before the CR pinning is set up.
> > 
> > Move CR pinning setup behind the EFI initialization.
> > 
> > Wrapping efi_enter_virtual_mode() into lass_disable/enable_enforcement()
> 
> I believe this should be lass_stac()/clac() since we reverted to the
> original naming.

Doh. Will fix.

> > is not enough because AC flag gates data accesses, but not instruction
> > fetch. Clearing the CR4 bit is required.
> > 
> > Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > ---
> >  arch/x86/kernel/cpu/common.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> > index 4f430be285de..9918121e0adc 100644
> > --- a/arch/x86/kernel/cpu/common.c
> > +++ b/arch/x86/kernel/cpu/common.c
> > @@ -2081,7 +2081,6 @@ static __init void identify_boot_cpu(void)
> >  	enable_sep_cpu();
> >  #endif
> >  	cpu_detect_tlb(&boot_cpu_data);
> > -	setup_cr_pinning();
> >  
> >  	tsx_init();
> >  	tdx_init();
> > @@ -2532,10 +2531,14 @@ void __init arch_cpu_finalize_init(void)
> >  
> >  	/*
> >  	 * This needs to follow the FPU initializtion, since EFI depends on it.
> > +	 *
> > +	 * EFI twiddles CR4.LASS. Do it before CR pinning.
> >  	 */
> >  	if (efi_enabled(EFI_RUNTIME_SERVICES))
> >  		efi_enter_virtual_mode();
> >  
> > +	setup_cr_pinning();
> > +
> 
> Instead of EFI toggling CR4.LASS, why not defer the first LASS
> activation itself?
> 
> i.e.
> 
> 	if (efi_enabled(EFI_RUNTIME_SERVICES))
> 		efi_enter_virtual_mode();
> 
> 	setup_lass();
> 
> 	setup_cr_pinning();
> 
> 
> This way, we can avoid the following patch (#5) altogether.

That's definitely an option.

The benefit of current approach is that the enforcement is enabled
earlier and cover more boot code, providing marginal protection
improvement.

I also like that related security features (SMEP/SMAP/UMIP/LASS) are
enabled in the same place.

In the end it is a judgement call.

Maintainers, any preference?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov