On 2/25/2025 1:09 PM, Tom Lendacky wrote: > On 2/25/25 11:45, Pratik R. Sampat wrote: >> On 2/25/2025 10:41 AM, Pratik R. Sampat wrote: >>> Hi Tom, >>> >>> On 2/24/2025 3:28 PM, Tom Lendacky wrote: >>>> On 2/21/25 15:01, Pratik R. Sampat wrote: >>>>> During platform init, SNP initialization may fail for several reasons, >>>>> such as firmware command failures and incompatible versions. However, >>>>> the KVM capability may continue to advertise support for it. Export this >>>>> information to KVM and withdraw SEV-SNP support if has not been >>>>> successfully initialized. >>>> >>>> Hmmm... rather than creating a new API, can you just issue an >>>> SNP_PLATFORM_STATUS command and see if the SNP is not in the UNINIT state? >>>> >>> >>> Although reading sev->snp_initialized is probably cheaper to do, it is >>> cleaner to query the platform status. >>> >>> Querying SNP_PLATFORM_STATUS requires the pages to transition to >>> firmware-owned and back, and the helpers for it are implemented within >>> sev-dev.c. So, similar to sev_platform_status(), I'm thinking it is >>> probably better to create the snp_platform_status() API as well and use >>> that within KVM to check the state. >>> >> >> Although I am guessing the initial intent was to not have an API exposed >> at all from CCP and only make the SNP_PLATFORM_STATUS call instead? >> >> Since that may not be cleanly possible (we have helpers for page state >> conversions such as rmp_mark_pages_firmware() in ccp) without >> duplicating functionality in KVM as well, I guess the question really >> boils down to whether we export the cheaper snp_initialized() or the >> snp_platform_status() API instead? > > Taking a closer look, we do already have APIs that KVM uses to allocate > firmware pages (output pages for SNP APIs) that can be used: > snp_alloc_firmware_page() and snp_free_firmware_page(). > > I think that should be enough to use sev_do_cmd() to perform the > SEV_CMD_SNP_PLATFORM_STATUS command without exposing a new API. > Ah, I had missed that! Calling the SNP_PLATFORM_STATUS this way works. Pratik > Thanks, > Tom > >> >> Thanks again! >> Pratik
