[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