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

Kalra, Ashish
2022-09-20T13:46:40Z

[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