On Wed, Nov 29, 2023 at 08:13:52PM -0600, Kalra, Ashish wrote: > It surely seems hard to follow up, so i am anyway going to clean it up by: > > Adding the "probe" parameter to sev_platform_init() where the parameter > being true indicates that we only want to do SNP initialization on probe, > the same parameter will get passed on to > __sev_platform_init_locked(). That's exactly what you should *not* do - the probe parameter controls whether if (psp_init_on_probe) __sev_platform_init_locked(error); and so on should get executed or not. If it is unclear, lemme know and I'll do a diff to show you what I mean. > > > + /* Prepare for first SNP guest launch after INIT */ > > > + wbinvd_on_all_cpus(); > > > > Why is that WBINVD needed? > > As the comment above mentions, WBINVD + DF_FLUSH is needed before the first > guest launch. Lemme see if I get this straight. The correct order is: WBINVD SNP_INIT_* WBINVD DF_FLUSH If so, do a comment which goes like this: /* * The order of commands to execute before the first guest * launch is the following: * * bla... */ > In the latest code base, _sev_snp_shutdown_locked() is called from > __sev_firmware_shutdown(). Then carve that function out only when needed - do not do changes preemptively. This is not helping during review. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette