On Tue, Feb 13, 2024 at 01:41:45PM +0100, Ard Biesheuvel wrote:
> @@ -632,5 +616,5 @@ void __head startup_64_setup_env(unsigned long physbase)
> "movl %%eax, %%ss\n"
> "movl %%eax, %%es\n" : : "a"(__KERNEL_DS) : "memory");
>
> - startup_64_load_idt(physbase);
> + startup_64_load_idt(&RIP_REL_REF(vc_no_ghcb));
It took me a while to figure out that even if we pass in one of the two
GHCB handler pointers, we only set it if CONFIG_AMD_MEM_ENCRYPT.
I think this ontop of yours is a bit more readable as it makes it
perfectly clear *when* the pointer is valid.
Yeah, if handler is set, we set it for the X86_TRAP_VC vector
unconditionally but that can be changed later, if really needed.
But this way it is clear by having the callers select the pointer. I.e.,
it is a more common coding pattern this way, I'd say.
Thx.
---
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 9d7f12829f2d..e39114c348e2 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -582,8 +582,8 @@ static void __head startup_64_load_idt(void *handler)
};
gate_desc *idt = (gate_desc *)desc.address;
- if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT))
- /* VMM Communication Exception */
+ /* @handler is set only for a VMM Communication Exception */
+ if (handler)
set_bringup_idt_handler(idt, X86_TRAP_VC, handler);
native_load_idt(&desc);
@@ -592,10 +592,14 @@ static void __head startup_64_load_idt(void *handler)
/* This is used when running on kernel addresses */
void early_setup_idt(void)
{
- if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT))
+ void *handler = NULL;
+
+ if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT)) {
setup_ghcb();
+ handler = vc_boot_ghcb;
+ }
- startup_64_load_idt(vc_boot_ghcb);
+ startup_64_load_idt(handler);
}
/*
@@ -603,6 +607,8 @@ void early_setup_idt(void)
*/
void __head startup_64_setup_gdt_idt(void)
{
+ void *handler = NULL;
+
struct desc_ptr startup_gdt_descr = {
.address = (unsigned long)&RIP_REL_REF(startup_gdt),
.size = sizeof(startup_gdt) - 1,
@@ -616,5 +622,8 @@ void __head startup_64_setup_gdt_idt(void)
"movl %%eax, %%ss\n"
"movl %%eax, %%es\n" : : "a"(__KERNEL_DS) : "memory");
- startup_64_load_idt(&RIP_REL_REF(vc_no_ghcb));
+ if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT))
+ handler = &RIP_REL_REF(vc_no_ghcb);
+
+ startup_64_load_idt(handler);
}
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
