Re: [PATCH v3] target/i386/sev: add support to query the attestation report

[Eduardo Habkost]

On Mon, May 31, 2021 at 04:01:16PM -0400, Eduardo Habkost wrote:
> On Thu, Apr 29, 2021 at 12:07:28PM -0500, Brijesh Singh wrote:
> > The SEV FW >= 0.23 added a new command that can be used to query the
> > attestation report containing the SHA-256 digest of the guest memory
> > and VMSA encrypted with the LAUNCH_UPDATE and sign it with the PEK.
> > 
> > Note, we already have a command (LAUNCH_MEASURE) that can be used to
> > query the SHA-256 digest of the guest memory encrypted through the
> > LAUNCH_UPDATE. The main difference between previous and this command
> > is that the report is signed with the PEK and unlike the LAUNCH_MEASURE
> > command the ATTESATION_REPORT command can be called while the guest
> > is running.
> > 
> > Add a QMP interface "query-sev-attestation-report" that can be used
> > to get the report encoded in base64.
> > 
> > Cc: James Bottomley <jejb@linux.ibm.com>
> > Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
> > Cc: Eric Blake <eblake@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: kvm@vger.kernel.org
> > Reviewed-by: James Bottomley <jejb@linux.ibm.com>
> > Tested-by: James Bottomley <jejb@linux.ibm.com>
> > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> > ---
> [...]
> > +    gsize len;
> [...]
> > +    /* verify the input mnonce length */
> > +    if (len != sizeof(input.mnonce)) {
> > +        error_setg(errp, "SEV: mnonce must be %ld bytes (got %ld)",
> > +                sizeof(input.mnonce), len);
> 
> This breaks the build on i386.  Failed CI job:
> https://gitlab.com/ehabkost/qemu/-/jobs/1300032082
> 
> I'm applying the following fixup.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 12899a31736..0e135d56e53 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -517,7 +517,7 @@ sev_get_attestation_report(const char *mnonce, Error 
> **errp)
>  
>      /* verify the input mnonce length */
>      if (len != sizeof(input.mnonce)) {
> -        error_setg(errp, "SEV: mnonce must be %ld bytes (got %ld)",
> +        error_setg(errp, "SEV: mnonce must be %ld bytes (got %" 
> G_GSIZE_FORMAT ")",
>                  sizeof(input.mnonce), len);
>          g_free(buf);
>          return NULL;

The fix was incomplete, additional fixup was required.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
diff --git a/0e135d56e53 b/target/i386/sev.c
index 0e135d56e53..1a88f127035 100644
--- a/0e135d56e53
+++ b/target/i386/sev.c
@@ -517,7 +517,7 @@ sev_get_attestation_report(const char *mnonce, Error **errp)
 
     /* verify the input mnonce length */
     if (len != sizeof(input.mnonce)) {
-        error_setg(errp, "SEV: mnonce must be %ld bytes (got %" G_GSIZE_FORMAT 
")",
+        error_setg(errp, "SEV: mnonce must be %zu bytes (got %" G_GSIZE_FORMAT 
")",
                 sizeof(input.mnonce), len);
         g_free(buf);
         return NULL;

-- 
Eduardo