Re: [PATCH v9 20/43] x86/sev: Use SEV-SNP AP creation to start secondary CPUs

[Borislav Petkov]

On Fri, Jan 28, 2022 at 11:17:41AM -0600, Brijesh Singh wrote:
> @@ -822,6 +842,236 @@ void snp_set_memory_private(unsigned long vaddr, unsigned int npages)
>  	pvalidate_pages(vaddr, npages, true);
>  }
>  
> +static int snp_set_vmsa(void *va, bool vmsa)
> +{
> +	u64 attrs;
> +
> +	/*
> +	 * Running at VMPL0 allows the kernel to change the VMSA bit for a page
> +	 * using the RMPADJUST instruction. However, for the instruction to
> +	 * succeed it must target the permissions of a lesser privileged

"lesser privileged/higher number VMPL level"

so that it is perfectly clear what this means.

> +	 * VMPL level, so use VMPL1 (refer to the RMPADJUST instruction in the
> +	 * AMD64 APM Volume 3).
> +	 */
> +	attrs = 1;
> +	if (vmsa)
> +		attrs |= RMPADJUST_VMSA_PAGE_BIT;
> +
> +	return rmpadjust((unsigned long)va, RMP_PG_SIZE_4K, attrs);
> +}

...

> +static int wakeup_cpu_via_vmgexit(int apic_id, unsigned long start_ip)
> +{
> +	struct sev_es_save_area *cur_vmsa, *vmsa;
> +	struct ghcb_state state;
> +	unsigned long flags;
> +	struct ghcb *ghcb;
> +	u8 sipi_vector;
> +	int cpu, ret;
> +	u64 cr4;
> +
> +	/*
> +	 * SNP-SNP AP creation requires that the hypervisor must support SEV-SNP
	   ^^^^^^^

See what I mean? :-)

That marketing has brainwashed y'all.

> +	 * feature. The SEV-SNP feature check is already performed, so just check
> +	 * for the AP_CREATION feature flag.
> +	 */

Let's clean this one:

	/*
	 * The hypervisor SNP feature support check has happened earlier, just check
	 * the AP_CREATION one here.
	 */


> +	if (!(sev_hv_features & GHCB_HV_FT_SNP_AP_CREATION))
> +		return -EOPNOTSUPP;
> +
> +	/*
> +	 * Verify the desired start IP against the known trampoline start IP
> +	 * to catch any future new trampolines that may be introduced that
> +	 * would require a new protected guest entry point.
> +	 */
> +	if (WARN_ONCE(start_ip != real_mode_header->trampoline_start,
> +		      "Unsupported SEV-SNP start_ip: %lx\n", start_ip))
> +		return -EINVAL;
> +
> +	/* Override start_ip with known protected guest start IP */
> +	start_ip = real_mode_header->sev_es_trampoline_start;

Yah, I'd like to get rid of that ->sev_es_trampoline_start and use the
normal ->trampoline_start. TDX is introducing a third one even and
they're all mutually-exclusive u32 values.

...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette