Re: [RFC PATCH v2 04/12] i386/sev: initialize SNP context

[Dov Murik]

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