Re: [PATCH] sev: check which processor the ASK/ARK chain should match

[Daniel P . Berrangé]

On Tue, Nov 16, 2021 at 04:38:59PM -0500, Tyler Fanelli wrote:
> The AMD ASK/ARK certificate chain differs between AMD SEV
> processor generations. SEV capabilities should provide
> which ASK/ARK certificate should be used based on the host
> processor.
> 
> Signed-off-by: Tyler Fanelli <tfanelli@redhat.com>
> ---
>  qapi/misc-target.json | 28 ++++++++++++++++++++++++++--
>  target/i386/sev.c     | 17 ++++++++++++++---
>  2 files changed, 40 insertions(+), 5 deletions(-)

> 
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index 5aa2b95b7d..c64aa3ff57 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -166,6 +166,24 @@
>  { 'command': 'query-sev-launch-measure', 'returns': 'SevLaunchMeasureInfo',
>    'if': 'TARGET_I386' }
>  
> +##
> +# @SevAskArkCertName:
> +#
> +# This enum describes which ASK/ARK certificate should be
> +# used based on the generation of an AMD Secure Encrypted
> +# Virtualization processor.
> +#
> +# @naples: AMD Naples processor (SEV 1st generation)
> +#
> +# @rome: AMD Rome processor (SEV 2nd generation)
> +#
> +# @milan: AMD Milan processor (SEV 3rd generation)
> +#
> +# Since: 7.0

I've found that many (all?)   Naples machines expose 'sev_es' in
their CPU flags, which is contrary to my understanding that SEV-ES
was only introduced in Zen2 / Rome. IOW, CPU flags don't seem to
provide a viable alternative to identify the generations, so this
info reported here is useful.


> @@ -534,9 +535,19 @@ static SevCapability *sev_get_capabilities(Error **errp)
>      cap->pdh = g_base64_encode(pdh_data, pdh_len);
>      cap->cert_chain = g_base64_encode(cert_chain_data, cert_chain_len);
>  
> -    host_cpuid(0x8000001F, 0, NULL, &ebx, NULL, NULL);
> +    host_cpuid(0x8000001F, 0, &eax, &ebx, NULL, NULL);
>      cap->cbitpos = ebx & 0x3f;
>  
> +    es = eax & 0x8;
> +    snp = eax & 0x10;
> +    if (!es && !snp) {
> +     cap->ask_ark_cert_name = SEV_ASK_ARK_CERT_NAME_NAPLES;
> +    } else if (es && !snp) {
> +     cap->ask_ark_cert_name = SEV_ASK_ARK_CERT_NAME_ROME;
> +    } else {
> +     cap->ask_ark_cert_name = SEV_ASK_ARK_CERT_NAME_MILAN;
> +    }

Ident appears off here - seems to have accidentally used tabs instead
of spaces. Since that's a trivial fix, feel free to add

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

when reposting.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|