Re: [PATCH v2 05/11] x86/sev: Move and reorganize sev guest request api

[Nikunj A. Dadhania]

On 4/4/2023 2:31 AM, Tom Lendacky wrote:
> On 3/26/23 09:46, Nikunj A Dadhania wrote:
>> For enabling Secure TSC, SEV-SNP guests need to communicate with the
>> security coprocessor really early during boot. Lot of the required
> 
> s/security coprocessor really/AMD Secure Processor/
> 
> s/Lot/Many/
> 
>> functions are implemented in the sev-guest driver. Move the required
> 
> ... in the sev-guest driver and therefore not available at early boot.
> 
>> functions and provide API to the driver to assign VM communications
> 
> s/provide API/provide an API/
> 
> s/to assign.*//
> 
>> key and send guest request.

Sure will change.

>> @@ -96,6 +97,27 @@ struct snp_req_data {
>>     struct sev_guest_platform_data {
>>       u64 secrets_gpa;
>> +
>> +    void *certs_data;
>> +    struct aesgcm_ctx *ctx;
>> +    struct snp_guest_msg *req, *resp;
>> +    struct snp_secrets_page_layout *layout;
>> +    struct snp_req_data input;
>> +    u8 *vmpck0;
> 
> Isn't this unneeded? You have the vmpck and vmpck_id in the snp_guest_dev struct which will be set based on the module parameter, so vmpck0 and associated checks shouldn't be needed.

Yes, this can be removed.

>> +    platform_data = kzalloc(sizeof(*platform_data), GFP_KERNEL);
>> +    if (!platform_data)
>> +        return -ENOMEM;
>> +
>> +    if (snp_setup_psp_messaging(platform_data))
> 
> This shouldn't be done here (or yet) since you only moving the routines. The sev-guest driver should call this regardless of the vmpck_id value.

I am moving the routines and also making sure that sev-guest driver works with this change. So sev-guest driver will not need to call snp_setup_psp_messaging().

>> +    }
>> +
>> +    /* Skip VMPCK0 initialization as the key is already initialized during early boot */
>> +    if (vmpck_id && aesgcm_expandkey(pdata->ctx, snp_dev->vmpck, VMPCK_KEY_LEN, AUTHTAG_LEN)) {
> 
> See previous comment. The sev-guest driver should be setting up everything private to it no matter the vmpck to be used.

I will try this out, as for secure tsc the vmpck will be used once and the sequence number would have incremented. And VMPCK0 will be initialized twice.

Regards
Nikunj