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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 5 Apr 2023 12:41:16 +0530
From:   "Nikunj A. Dadhania" <nikunj@....com>
To:     Tom Lendacky <thomas.lendacky@....com>,
        linux-kernel@...r.kernel.org, x86@...nel.org
Cc:     bp@...en8.de, dionnaglaze@...gle.com, pgonda@...gle.com,
        seanjc@...gle.com, pbonzini@...hat.com, michael.roth@....com,
        ketanch@...k.ac.in
Subject: Re: [PATCH v2 05/11] x86/sev: Move and reorganize sev guest request
 api

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