[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z7rwA-qVauX7lY8G@gmail.com>
Date: Sun, 23 Feb 2025 10:53:07 +0100
From: Ingo Molnar <mingo@...nel.org>
To: David Woodhouse <dwmw2@...radead.org>
Cc: kexec@...ts.infradead.org, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H . Peter Anvin" <hpa@...or.com>,
David Woodhouse <dwmw@...zon.co.uk>,
"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
Kai Huang <kai.huang@...el.com>,
Nikolay Borisov <nik.borisov@...e.com>,
linux-kernel@...r.kernel.org, Simon Horman <horms@...nel.org>,
Dave Young <dyoung@...hat.com>,
Peter Zijlstra <peterz@...radead.org>, jpoimboe@...nel.org,
bsz@...zon.de
Subject: Re: [PATCH v6 2/7] x86/kexec: Debugging support: load a GDT
* David Woodhouse <dwmw2@...radead.org> wrote:
> From: David Woodhouse <dwmw@...zon.co.uk>
>
> There are some failure modes which lead to triple-faults in the
> relocate_kernel function, which is fairly much undebuggable for normal
> mortals.
>
> Adding a GDT in the relocate_kernel environment is step 1 towards being
> able to catch faults and do something more useful.
>
> Signed-off-by: David Woodhouse <dwmw@...zon.co.uk>
> ---
> arch/x86/kernel/relocate_kernel_64.S | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
> index af2cd06ff318..c62f03808f18 100644
> --- a/arch/x86/kernel/relocate_kernel_64.S
> +++ b/arch/x86/kernel/relocate_kernel_64.S
> @@ -39,6 +39,18 @@ SYM_DATA(kexec_pa_table_page, .quad 0)
> SYM_DATA(kexec_pa_swap_page, .quad 0)
> SYM_DATA_LOCAL(pa_backup_pages_map, .quad 0)
>
> +#ifdef CONFIG_KEXEC_DEBUG
> + .balign 16
> +SYM_DATA_START_LOCAL(kexec_debug_gdt)
> + .word kexec_debug_gdt_end - kexec_debug_gdt - 1
> + .long 0
> + .word 0
> + .quad 0x00cf9a000000ffff /* __KERNEL32_CS */
> + .quad 0x00af9a000000ffff /* __KERNEL_CS */
> + .quad 0x00cf92000000ffff /* __KERNEL_DS */
> +SYM_DATA_END_LABEL(kexec_debug_gdt, SYM_L_LOCAL, kexec_debug_gdt_end)
> +#endif /* CONFIG_KEXEC_DEBUG */
Yeah, so is there any reason (other than paranoia) why the early-early
GDT and IDT shouldn't be unconditional? There's many ways for such an
approach to bitrot, it's much better to not hide it behind a
default-disabled debug option...
Some of the other bits, like the hard-coded serial debugging
assumptions, probably need to be behind the debug option - but much of
the new debug mechanism looks safe and generic and can be always-on,
IMHO.
This would also throw regressions back into the face of whoever manages
to introduce them, ideally. ;-)
Thanks,
Ingo
Powered by blists - more mailing lists