On Wed, Dec 14, 2022 at 01:40:17PM -0600, Michael Roth wrote: > + /* > + * If boot CPU supports SNP, then first attempt to initialize > + * the SNP firmware. > + */ > + if (cpu_feature_enabled(X86_FEATURE_SEV_SNP)) { > + if (!sev_version_greater_or_equal(SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR)) { > + dev_err(sev->dev, "SEV-SNP support requires firmware version >= %d:%d\n", > + SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR); > + } else { > + rc = sev_snp_init(&error, true); > + if (rc) { > + /* > + * Don't abort the probe if SNP INIT failed, > + * continue to initialize the legacy SEV firmware. > + */ > + dev_err(sev->dev, "SEV-SNP: failed to INIT error %#x\n", error); > + } > + } > + } I think this is not right as there is a dep between sev init and this, and there is about a dozen of call sites already __sev_platform_init_locked(). Instead there should be __sev_snp_init_locked() that would be called as part of __sev_platform_init_locked() flow. Also TMR allocation should be moved inside __sev_platform_init_locked, given that it needs to be marked into RMP after SNP init. BR, Jarkko