Re: [RFC PATCH 4/6] i386/sev: add the SNP launch start context

Brijesh Singh
2021-07-19T15:32:47Z

On 7/19/21 7:34 AM, Dov Murik wrote:
> Hi Brijesh,
>
> On 10/07/2021 0:55, Brijesh Singh wrote:
>> The SNP_LAUNCH_START is called first to create a cryptographic launch
>> context within the firmware.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>> target/i386/sev.c | 30 +++++++++++++++++++++++++++++-
>> target/i386/trace-events | 1 +
>> 2 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>> index 84ae244af0..259408a8f1 100644
>> --- a/target/i386/sev.c
>> +++ b/target/i386/sev.c
>> @@ -812,6 +812,29 @@ sev_read_file_base64(const char *filename, guchar **data, gsize *len)
>> return 0;
>> }
>>
>> +static int
>> +sev_snp_launch_start(SevGuestState *sev)
>> +{
>> + int ret = 1;
>> + int fw_error, rc;
>> + struct kvm_sev_snp_launch_start *start = &sev->snp_config.start;
>> +
>> + trace_kvm_sev_snp_launch_start(start->policy);
>> +
>> + rc = sev_ioctl(sev->sev_fd, KVM_SEV_SNP_LAUNCH_START, start, &fw_error);
>> + if (rc < 0) {
>> + error_report("%s: SNP_LAUNCH_START ret=%d fw_error=%d '%s'",
>> + __func__, ret, fw_error, fw_error_to_str(fw_error));
>
> Did you mean to report the value of ret or rc?

Ah, I was meaning the rc.

>
>
>> + goto out;
>
> Suggestion:
>
> Remove the `ret` variable.
> Here: simply `return 1`.
> At the end: remove the `out:` label; simply `return 0`.
>

Noted.

thanks