On Fri, Jan 28, 2022 at 11:17:31AM -0600, Brijesh Singh wrote:
> diff --git a/arch/x86/boot/compressed/idt_64.c b/arch/x86/boot/compressed/idt_64.c
> index 9b93567d663a..63e9044ab1d6 100644
> --- a/arch/x86/boot/compressed/idt_64.c
> +++ b/arch/x86/boot/compressed/idt_64.c
> @@ -39,7 +39,15 @@ void load_stage1_idt(void)
> load_boot_idt(&boot_idt_desc);
> }
>
> -/* Setup IDT after kernel jumping to .Lrelocated */
> +/*
> + * Setup IDT after kernel jumping to .Lrelocated
> + *
> + * initialize_identity_maps() needs a PF handler setup. The PF handler setup
> + * needs to happen in load_stage2_idt() where the IDT is loaded and there the
> + * VC IDT entry gets setup too in order to handle VCs, one needs a GHCB which
> + * gets setup with an already setup table which is done in
> + * initialize_identity_maps() and this is where the circle is complete.
> + */
I've beefed it up more, please use this one instead:
/*
* Setup IDT after kernel jumping to .Lrelocated.
*
* initialize_identity_maps() needs a #PF handler to be setup
* in order to be able to fault-in identity mapping ranges; see
* do_boot_page_fault().
*
* This #PF handler setup needs to happen in load_stage2_idt() where the
* IDT is loaded and there the #VC IDT entry gets setup too.
*
* In order to be able to handle #VCs, one needs a GHCB which
* gets setup with an already set up pagetable, which is done in
* initialize_identity_maps(). And there's the catch 22: the boot #VC
* handler do_boot_stage2_vc() needs to call early_setup_ghcb() itself
* (and, especially set_page_decrypted()) because the SEV-ES setup code
* cannot initialize a GHCB as there's no #PF handler yet...
*/
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index 19ad09712902..24df739c9c05 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -43,6 +43,9 @@ static struct ghcb boot_ghcb_page __bss_decrypted __aligned(PAGE_SIZE);
> */
> static struct ghcb __initdata *boot_ghcb;
>
> +/* Bitmap of SEV features supported by the hypervisor */
> +static u64 sev_hv_features __ro_after_init;
> +
> /* #VC handler runtime per-CPU data */
> struct sev_es_runtime_data {
> struct ghcb ghcb_page;
> @@ -766,6 +769,18 @@ void __init sev_es_init_vc_handling(void)
> if (!sev_es_check_cpu_features())
> panic("SEV-ES CPU Features missing");
>
> + /*
> + * SEV-SNP is supported in v2 of the GHCB spec which mandates support for HV
> + * features. If SEV-SNP is enabled, then check if the hypervisor supports
> + * the SEV-SNP features.
You guys have been completely brainwashed by marketing. I say:
"s/SEV-SNP/SNP/g
And please do that everywhere in sev-specific files."
and you go and slap that "SEV-" thing everywhere instead. Why? That file
is already called sev.c so it must be SEV-something. Lemme simplify that
comment for ya:
/*
* SNP is supported in v2 of the GHCB spec which mandates support for HV
* features.
*/
That's it, no more needed - the rest should be visible from the code.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
