Re: [RFC PATCH 5/6] i386/sev: add support to encrypt BIOS when SEV-SNP is enabled

[Brijesh Singh <brijesh.singh@amd.com>]

On 7/14/21 12:08 PM, Connor Kuehl wrote:
> On 7/9/21 3:55 PM, Brijesh Singh wrote:
>> The KVM_SEV_SNP_LAUNCH_UPDATE command is used for encrypting the bios
>> image used for booting the SEV-SNP guest.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   target/i386/sev.c        | 33 ++++++++++++++++++++++++++++++++-
>>   target/i386/trace-events |  1 +
>>   2 files changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>> index 259408a8f1..41dcb084d1 100644
>> --- a/target/i386/sev.c
>> +++ b/target/i386/sev.c
>> @@ -883,6 +883,30 @@ out:
>>       return ret;
>>   }
>>   
>> +static int
>> +sev_snp_launch_update(SevGuestState *sev, uint8_t *addr, uint64_t len, int type)
>> +{
>> +    int ret, fw_error;
>> +    struct kvm_sev_snp_launch_update update = {};
>> +
>> +    if (!addr || !len) {
>> +        return 1;
> 
> Should this be a -1? It looks like the caller checks if this function
> returns < 0, but doesn't check for res == 1.

Ah, it should be -1.

> 
> Alternatively, invoking error_report might provide more useful
> information that the preconditions to this function were violated.
> 

Sure, I will add error_report.

thanks