Messages in this thread | | | Date | Thu, 5 Jan 2023 16:54:23 -0600 | Subject | Re: [PATCH RFC v7 25/64] crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP | From | "Kalra, Ashish" <> |
| |
Hello Jarkko,
On 1/4/2023 6:12 AM, Jarkko Sakkinen wrote: > 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(). >
sev_init ?
As this is invoked during CCP module load/initialization, shouldn't this get invoked before any other call sites invoking __sev_platform_init_locked() ?
Thanks, Ashish
> 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 >
| |