On 05/09/2021 16:58, Brijesh Singh wrote:
> Hi Dov,
>
> On 9/5/21 2:07 AM, Dov Murik wrote:
> ...
>>
>>>
>>> uint64_t
>>> @@ -1074,6 +1083,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>>> uint32_t ebx;
>>> uint32_t host_cbitpos;
>>> struct sev_user_data_status status = {};
>>> + void *init_args = NULL;
>>>
>>> if (!sev_common) {
>>> return 0;
>>> @@ -1126,7 +1136,18 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>>> sev_common->api_major = status.api_major;
>>> sev_common->api_minor = status.api_minor;
>> Not visible here in the context: the code here is using the
>> SEV_PLATFORM_STATUS command to get the build_id, api_major, and api_minor.
>>
>> I see that SNP has a new command SNP_PLATFORM_STATUS, which fills a
>> struct sev_data_snp_platform_status (hmmm, I can't find the struct's
>> definition; I assume it should look like Table 38 in 8.3.2 in SNP FW ABI
>> document).
>
> The API version can be queries either through the SNP_PLATFORM_STATUS or
> SEV_PLATFORM_STATUS and they both report the same info. As the
> definition of the sev_data_platform_status is concerned it should be
> defined in the kernel include/linux/psp-sev.h.
>
>
>> My questions are:
>>
>> 1. Is it OK to call the "legacy" SEV_PLATFORM_STATUS when about to init
>> an SNP guest?
>
> Yes, the legacy platform status command can be called on the SNP
> initialized host.
>
> I choose not to new command because we only care about the verison
> string and that is available through either of these commands (SNP or
> SEV platform status).
>
>> 2. Do we want to save some info like installed TCB version and reported
>> TCB version, and maybe other fields from SNP platform status?
>
> If we decide to add a new QMP (query-sev-snp) then it makes sense to
> export those fields so that a hypervisor console can give additional
> information; But note that for the guest, all these are available in the
> attestation report.
>
We have new QMP response for SNP guests (SevSnpGuestProperties, patch 3
in this series). I think it would make sense to add the
installed+reported TCB versions there (read-only properties), for
debugging/observability purposes.
-Dov
