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

[Brijesh Singh]

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