On Sat, Dec 30, 2023 at 10:19:41AM -0600, Michael Roth wrote: > From: Brijesh Singh <brijesh.singh@xxxxxxx> > > Before SNP VMs can be launched, the platform must be appropriately > configured and initialized. Platform initialization is accomplished via > the SNP_INIT command. Make sure to do a WBINVD and issue DF_FLUSH > command to prepare for the first SNP guest launch after INIT. ^^^^^^ Which "INIT"? Sounds like after hipervisor's init... > During the execution of SNP_INIT command, the firmware configures > and enables SNP security policy enforcement in many system components. > Some system components write to regions of memory reserved by early > x86 firmware (e.g. UEFI). Other system components write to regions > provided by the operation system, hypervisor, or x86 firmware. > Such system components can only write to HV-fixed pages or Default > pages. They will error when attempting to write to other page states "... to pages in other page states... " > after SNP_INIT enables their SNP enforcement. And yes, this version looks much better. Some text cleanups ontop: --- diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c index 85634d4f8cfe..7942ec730525 100644 --- a/drivers/crypto/ccp/sev-dev.c +++ b/drivers/crypto/ccp/sev-dev.c @@ -549,24 +549,22 @@ static int __sev_snp_init_locked(int *error) return 0; } - /* - * The SNP_INIT requires the MSR_VM_HSAVE_PA must be set to 0h - * across all cores. - */ + /* SNP_INIT requires MSR_VM_HSAVE_PA to be cleared on all CPUs. */ on_each_cpu(snp_set_hsave_pa, NULL, 1); /* - * Starting in SNP firmware v1.52, the SNP_INIT_EX command takes a list of - * system physical address ranges to convert into the HV-fixed page states - * during the RMP initialization. For instance, the memory that UEFI - * reserves should be included in the range list. This allows system + * Starting in SNP firmware v1.52, the SNP_INIT_EX command takes a list + * of system physical address ranges to convert into HV-fixed page + * states during the RMP initialization. For instance, the memory that + * UEFI reserves should be included in the that list. This allows system * components that occasionally write to memory (e.g. logging to UEFI - * reserved regions) to not fail due to RMP initialization and SNP enablement. + * reserved regions) to not fail due to RMP initialization and SNP + * enablement. */ if (sev_version_greater_or_equal(SNP_MIN_API_MAJOR, 52)) { /* * Firmware checks that the pages containing the ranges enumerated - * in the RANGES structure are either in the Default page state or in the + * in the RANGES structure are either in the default page state or in the * firmware page state. */ snp_range_list = kzalloc(PAGE_SIZE, GFP_KERNEL); @@ -577,7 +575,7 @@ static int __sev_snp_init_locked(int *error) } /* - * Retrieve all reserved memory regions setup by UEFI from the e820 memory map + * Retrieve all reserved memory regions from the e820 memory map * to be setup as HV-fixed pages. */ rc = walk_iomem_res_desc(IORES_DESC_NONE, IORESOURCE_MEM, 0, ~0, @@ -599,14 +597,13 @@ static int __sev_snp_init_locked(int *error) } /* - * The following sequence must be issued before launching the - * first SNP guest to ensure all dirty cache lines are flushed, - * including from updates to the RMP table itself via RMPUPDATE - * instructions: + * The following sequence must be issued before launching the first SNP + * guest to ensure all dirty cache lines are flushed, including from + * updates to the RMP table itself via the RMPUPDATE instruction: * - * - WBINDV on all running CPUs + * - WBINVD on all running CPUs * - SEV_CMD_SNP_INIT[_EX] firmware command - * - WBINDV on all running CPUs + * - WBINVD on all running CPUs * - SEV_CMD_SNP_DF_FLUSH firmware command */ wbinvd_on_all_cpus(); -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette