linux-kernel - RE: [PATCH Part2 v6 11/49] crypto:ccp: Define the SEV-SNP commands

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:   Tue, 20 Sep 2022 13:46:25 +0000
From:   "Kalra, Ashish" <Ashish.Kalra@....com>
To:     Borislav Petkov <bp@...en8.de>
CC:     "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "jroedel@...e.de" <jroedel@...e.de>,
        "Lendacky, Thomas" <Thomas.Lendacky@....com>,
        "hpa@...or.com" <hpa@...or.com>,
        "ardb@...nel.org" <ardb@...nel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "seanjc@...gle.com" <seanjc@...gle.com>,
        "vkuznets@...hat.com" <vkuznets@...hat.com>,
        "jmattson@...gle.com" <jmattson@...gle.com>,
        "luto@...nel.org" <luto@...nel.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "slp@...hat.com" <slp@...hat.com>,
        "pgonda@...gle.com" <pgonda@...gle.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "srinivas.pandruvada@...ux.intel.com" 
        <srinivas.pandruvada@...ux.intel.com>,
        "rientjes@...gle.com" <rientjes@...gle.com>,
        "dovmurik@...ux.ibm.com" <dovmurik@...ux.ibm.com>,
        "tobin@....com" <tobin@....com>,
        "Roth, Michael" <Michael.Roth@....com>,
        "vbabka@...e.cz" <vbabka@...e.cz>,
        "kirill@...temov.name" <kirill@...temov.name>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "marcorr@...gle.com" <marcorr@...gle.com>,
        "sathyanarayanan.kuppuswamy@...ux.intel.com" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "alpergun@...gle.com" <alpergun@...gle.com>,
        "dgilbert@...hat.com" <dgilbert@...hat.com>,
        "jarkko@...nel.org" <jarkko@...nel.org>
Subject: RE: [PATCH Part2 v6 11/49] crypto:ccp: Define the SEV-SNP commands

[AMD Official Use Only - General]

Hello Boris,

>> +/**
>> + * struct sev_data_snp_platform_status_buf - SNP_PLATFORM_STATUS 
>> +command params
>> + *
>> + * @address: physical address where the status should be copied  */ 
>> +struct sev_data_snp_platform_status_buf {
>> +	u64 status_paddr;			/* In */
>> +} __packed;
>> +
>> +/**
>> + * struct sev_data_snp_download_firmware - SNP_DOWNLOAD_FIRMWARE 
>> +command params
>> + *
>> + * @address: physical address of firmware image
>> + * @len: len of the firmware image
>> + */
>> +struct sev_data_snp_download_firmware {
>> +	u64 address;				/* In */
>> +	u32 len;				/* In */
>> +} __packed;
>> +
>> +/**
>> + * struct sev_data_snp_gctx_create - SNP_GCTX_CREATE command params
>> + *
>> + * @gctx_paddr: system physical address of the page donated to firmware by
>> + *		the hypervisor to contain the guest context.
>> + */
>> +struct sev_data_snp_gctx_create {
>> +	u64 gctx_paddr;				/* In */
>> +} __packed;

>So some of those structs have the same layout. Let's unify them pls.
>I.e., for

>sev_data_send_finish, sev_data_send_cancel, sev_data_receive_finish

>you do

>struct sev_data_tx {
>	u32 handle;				/* In */
>} __packed;

>For sev_data_snp_platform_status_buf, sev_data_snp_gctx_create, sev_data_snp_decommission and all those others who are a single u64, you use a single

>struct sev_data_addr {
>	u64 gctx_paddr;				/* In */
>} __packed;

>so that we don't have gazillion structs all of different names but a lot of them identical in content.

These are structure definitions as per SNP Firmware API specifications, and they match the SNP Firmware commands and required arguments.

As an example below:

8.12 SNP_DECOMMISSION
This command destroys a guest context. After this command successfully completes, the guest 
will not long be runnable.
8.12.1 Parameters
Table 55. Layout of the CMDBUF_SNP_DECOMMISSION Structure
GCTX_PADDR Bits 63:12 of the sPA of the guest's 
context page

Isn't it better to have 1:1 mapping between specification and structure definitions here ?

Thanks,
Ashish