Re: [PATCH 1/2] sev: add sev-inject-launch-secret

[Eric Blake <eblake@redhat.com>]

On 5/28/20 3:51 PM, Tobin Feldman-Fitzthum wrote:
> From: Tobin Feldman-Fitzthum <tobin@ibm.com>
> 
> AMD SEV allows a guest owner to inject a secret blob
> into the memory of a virtual machine. The secret is
> encrypted with the SEV Transport Encryption Key and
> integrity is guaranteed with the Transport Integrity
> Key. Although QEMU faciliates the injection of the
> launch secret, it cannot access the secret.
> 
> Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
> ---

> +++ b/qapi/misc-target.json
> @@ -200,6 +200,26 @@
>   { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
>     'if': 'defined(TARGET_I386)' }
>   
> +##
> +# @sev-inject-launch-secret:
> +#
> +# This command injects a secret blob into memory of SEV guest.
> +#
> +# @packet-header: the launch secret packet header encoded in base64
> +#
> +# @secret: the launch secret data to be injected encoded in base64
> +#
> +# @gpa: the guest physical address where secret will be injected.
> +        GPA provided here will be ignored if guest ROM specifies
> +        the a launch secret GPA.

Missing # on the wrapped lines.

> +#
> +# Since: 5.0.0

You've missed 5.0, and more sites tend to use x.y instead of x.y.z 
(although we aren't consistent); this should be 'Since: 5.1'

> +#
> +##
> +{ 'command': 'sev-inject-launch-secret',
> +  'data': { 'packet_hdr': 'str', 'secret': 'str', 'gpa': 'uint64' },

This does not match your documentation above, which named it 
'packet-header'.  Should 'gpa' be optional, to account for the case 
where ROM specifies it?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org