| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <81c3e14a-59a4-4aae-a0e3-c415f4c294be@zytor.com>
Date: Tue, 13 Aug 2024 08:57:47 -0700
From: Xin Li <xin@...or.com>
To: Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org
Cc: hpa@...or.com, mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
x86@...nel.org, peterz@...radead.org, andrew.cooper3@...rix.com,
nik.borisov@...e.com, houwenlong.hwl@...group.com
Subject: Re: [PATCH v2 3/3] x86/fred: Enable FRED right after
init_mem_mapping()
On 8/13/2024 5:45 AM, Thomas Gleixner wrote:
> On Tue, Jul 09 2024 at 08:40, Xin Li wrote:
>
> I'm really unhappy about sprinkling all these FRED conditionals all over
> the place:
Sigh, for the reason of a bad taste...
>> init_mem_mapping();
>>
>> - idt_setup_early_pf();
>> + /*
>> + * init_mem_mapping() uses early IDT to setup memory mappings, thus FRED
>> + * can't be enabled earlier than that, unless FRED adds support to setup
>> + * memory mappings.
>> + */
>> + if (cpu_feature_enabled(X86_FEATURE_FRED))
>> + cpu_init_fred_exceptions();
>> + else
>> + idt_setup_early_pf();
>
>> @@ -248,6 +249,11 @@ static void notrace start_secondary(void *unused)
>>
>> cpu_init_exception_handling();
>>
>> + if (cpu_feature_enabled(X86_FEATURE_FRED)) {
>> + cpu_init_fred_exceptions();
>> + cpu_init_fred_rsps();
>> + }
>
>> /* Init cpu_entry_area before IST entries are set up */
>> setup_cpu_entry_areas();
>>
>> + /* FRED RSPs pointing to memory from CPU entry areas */
>> + if (cpu_feature_enabled(X86_FEATURE_FRED))
>> + cpu_init_fred_rsps();
>> +
>> /* Init GHCB memory pages when running as an SEV-ES guest */
>> sev_es_init_vc_handling();
>
> This really can be encapsulated and kept in places which need to know
> about FRED already. See below. Can you please validate?
I reviewed your patch, and there is no reason it won't work; my tests
show no problem with it.
Thanks!
Xin
> ---
> From: "Xin Li (Intel)" <xin@...or.com>
> Subject: x86/fred: Enable FRED right after init_mem_mapping()
> Date: Tue, 09 Jul 2024 08:40:48 -0700
>
> From: "Xin Li (Intel)" <xin@...or.com>
>
> On 64-bit init_mem_mapping() relies on the minimal page fault handler
> provided by the early IDT mechanism. The real page fault handler is
> installed right afterwards into the IDT.
>
> This is problematic on CPUs which have X86_FEATURE_FRED set because the
> real page fault handler retrieves the faulting address from the FRED
> exception stack frame and not from CR2, but that does obviously not work
> when FRED is not yet enabled in the CPU.
>
> To prevent this enable FRED right after init_mem_mapping() without
> interrupt stacks. Those are enabled later in trap_init() after the CPU
> entry area is set up.
>
> [ tglx: Encapsulate the FRED details ]
>
> Fixes: 14619d912b65 ("x86/fred: FRED entry/exit and dispatch code")
> Reported-by: Hou Wenlong <houwenlong.hwl@...group.com>
> Suggested-by: Thomas Gleixner <tglx@...utronix.de>
> Signed-off-by: Xin Li (Intel) <xin@...or.com>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Link: https://lore.kernel.org/all/20240709154048.3543361-4-xin@zytor.com
> ---
> arch/x86/include/asm/processor.h | 3 ++-
> arch/x86/kernel/cpu/common.c | 15 +++++++++++++--
> arch/x86/kernel/setup.c | 7 ++++++-
> arch/x86/kernel/smpboot.c | 2 +-
> arch/x86/kernel/traps.c | 2 +-
> 5 files changed, 23 insertions(+), 6 deletions(-)
>
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -582,7 +582,8 @@ extern void switch_gdt_and_percpu_base(i
> extern void load_direct_gdt(int);
> extern void load_fixmap_gdt(int);
> extern void cpu_init(void);
> -extern void cpu_init_exception_handling(void);
> +extern void cpu_init_exception_handling(bool boot_cpu);
> +extern void cpu_init_replace_early_idt(void);
> extern void cr4_init(void);
>
> extern void set_task_blockstep(struct task_struct *task, bool on);
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -2176,7 +2176,7 @@ static inline void tss_setup_io_bitmap(s
> * Setup everything needed to handle exceptions from the IDT, including the IST
> * exceptions which use paranoid_entry().
> */
> -void cpu_init_exception_handling(void)
> +void cpu_init_exception_handling(bool boot_cpu)
> {
> struct tss_struct *tss = this_cpu_ptr(&cpu_tss_rw);
> int cpu = raw_smp_processor_id();
> @@ -2196,13 +2196,24 @@ void cpu_init_exception_handling(void)
> setup_ghcb();
>
> if (cpu_feature_enabled(X86_FEATURE_FRED)) {
> - cpu_init_fred_exceptions();
> + /* The boot CPU has enabled FRED during early boot */
> + if (!boot_cpu)
> + cpu_init_fred_exceptions();
> +
> cpu_init_fred_rsps();
> } else {
> load_current_idt();
> }
> }
>
> +void __init cpu_init_replace_early_idt(void)
> +{
> + if (cpu_feature_enabled(X86_FEATURE_FRED))
> + cpu_init_fred_exceptions();
> + else
> + idt_setup_early_pf();
> +}
> +
> /*
> * cpu_init() initializes state that is per-CPU. Some data is already
> * initialized (naturally) in the bootstrap process, such as the GDT. We
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1039,7 +1039,12 @@ void __init setup_arch(char **cmdline_p)
>
> init_mem_mapping();
>
> - idt_setup_early_pf();
> + /*
> + * init_mem_mapping() relies on the early IDT page fault handling.
> + * Now either enable FRED or install the real page fault handler
> + * for 64-bit in the IDT.
> + */
> + cpu_init_replace_early_idt();
>
> /*
> * Update mmu_cr4_features (and, indirectly, trampoline_cr4_features)
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -246,7 +246,7 @@ static void notrace start_secondary(void
> __flush_tlb_all();
> }
>
> - cpu_init_exception_handling();
> + cpu_init_exception_handling(false);
>
> /*
> * Load the microcode before reaching the AP alive synchronization
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -1411,7 +1411,7 @@ void __init trap_init(void)
> sev_es_init_vc_handling();
>
> /* Initialize TSS before setting up traps so ISTs work */
> - cpu_init_exception_handling();
> + cpu_init_exception_handling(true);
>
> /* Setup traps as cpu_init() might #GP */
> if (!cpu_feature_enabled(X86_FEATURE_FRED))
>
Powered by blists - more mailing lists