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

Tyler Fanelli posted 1 patch 2 years, 5 months ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20211116213858.363583-1-tfanelli@redhat.com
Maintainers: Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>
qapi/misc-target.json | 28 ++++++++++++++++++++++++++--
target/i386/sev.c     | 17 ++++++++++++++---
2 files changed, 40 insertions(+), 5 deletions(-)
[PATCH] sev: check which processor the ASK/ARK chain should match
Posted by Tyler Fanelli 2 years, 5 months ago
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
+##
+{ 'enum': 'SevAskArkCertName',
+  'data': ['naples', 'rome', 'milan'],
+  'if': 'TARGET_I386' }
 
 ##
 # @SevCapability:
@@ -182,13 +200,18 @@
 # @reduced-phys-bits: Number of physical Address bit reduction when SEV is
 #                     enabled
 #
+# @ask-ark-cert-name: The generation in which the AMD
+#                     ARK/ASK should be derived from
+#                     (since 7.0)
+#
 # Since: 2.12
 ##
 { 'struct': 'SevCapability',
   'data': { 'pdh': 'str',
             'cert-chain': 'str',
             'cbitpos': 'int',
-            'reduced-phys-bits': 'int'},
+            'reduced-phys-bits': 'int',
+            'ask-ark-cert-name': 'SevAskArkCertName'},
   'if': 'TARGET_I386' }
 
 ##
@@ -205,7 +228,8 @@
 #
 # -> { "execute": "query-sev-capabilities" }
 # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-#                  "cbitpos": 47, "reduced-phys-bits": 5}}
+#                  "cbitpos": 47, "reduced-phys-bits": 5,
+#                  "ask-ark-cert-name": "naples"}}
 #
 ##
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
diff --git a/target/i386/sev.c b/target/i386/sev.c
index eede07f11d..f30171e5ba 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -506,8 +506,9 @@ static SevCapability *sev_get_capabilities(Error **errp)
     guchar *pdh_data = NULL;
     guchar *cert_chain_data = NULL;
     size_t pdh_len = 0, cert_chain_len = 0;
-    uint32_t ebx;
-    int fd;
+    uint32_t eax, ebx;
+    int fd, es, snp;
+
 
     if (!kvm_enabled()) {
         error_setg(errp, "KVM not enabled");
@@ -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;
+    }
+
     /*
      * When SEV feature is enabled, we loose one bit in guest physical
      * addressing.
-- 
2.31.1


Re: [PATCH] sev: check which processor the ASK/ARK chain should match
Posted by Daniel P. Berrangé 2 years, 4 months ago
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 :|