[Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Singh, Brijesh]

There are limited numbers of the SEV guests that can be run concurrently.
A management applications may need to know this limit so that it can place
SEV VMs on hosts which have suitable resources available.

Currently, this limit is not exposed to the application. Add a new
'sev-max-guest' field in the query-sev-capabilities to provide this
information.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 qapi/target.json  | 6 ++++--
 target/i386/sev.c | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/qapi/target.json b/qapi/target.json
index 1d4d54b600..b45121d30b 100644
--- a/qapi/target.json
+++ b/qapi/target.json
@@ -183,7 +183,8 @@
   'data': { 'pdh': 'str',
             'cert-chain': 'str',
             'cbitpos': 'int',
-            'reduced-phys-bits': 'int'},
+            'reduced-phys-bits': 'int',
+            'sev-max-guests': 'int'},
   'if': 'defined(TARGET_I386)' }
 
 ##
@@ -200,7 +201,8 @@
 #
 # -> { "execute": "query-sev-capabilities" }
 # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-#                  "cbitpos": 47, "reduced-phys-bits": 5}}
+#                  "cbitpos": 47, "reduced-phys-bits": 5,
+#                  "sev-max-guests" : 15}}
 #
 ##
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
diff --git a/target/i386/sev.c b/target/i386/sev.c
index cd77f6b5d4..bb0cd79acd 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -488,7 +488,7 @@ sev_get_capabilities(void)
     guchar *pdh_data = NULL;
     guchar *cert_chain_data = NULL;
     size_t pdh_len = 0, cert_chain_len = 0;
-    uint32_t ebx;
+    uint32_t ebx, ecx, edx;
     int fd;
 
     fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
@@ -507,7 +507,7 @@ sev_get_capabilities(void)
     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, NULL, &ebx, &ecx, &edx);
     cap->cbitpos = ebx & 0x3f;
 
     /*
@@ -516,6 +516,8 @@ sev_get_capabilities(void)
      */
     cap->reduced_phys_bits = 1;
 
+    /* the maximum number of SEV guests that can run simultaneously */
+    cap->sev_max_guests = ecx - edx + 1;
 out:
     g_free(pdh_data);
     g_free(cert_chain_data);
-- 
2.17.1

[Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Singh, Brijesh]

There are limited numbers of the SEV guests that can be run concurrently.
A management applications may need to know this limit so that it can place
SEV VMs on hosts which have suitable resources available.

Currently, this limit is not exposed to the application. Add a new
'sev-max-guest' field in the query-sev-capabilities to provide this
information.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 qapi/target.json  | 6 ++++--
 target/i386/sev.c | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/qapi/target.json b/qapi/target.json
index 1d4d54b600..b45121d30b 100644
--- a/qapi/target.json
+++ b/qapi/target.json
@@ -183,7 +183,8 @@
   'data': { 'pdh': 'str',
             'cert-chain': 'str',
             'cbitpos': 'int',
-            'reduced-phys-bits': 'int'},
+            'reduced-phys-bits': 'int',
+            'sev-max-guests': 'int'},
   'if': 'defined(TARGET_I386)' }
 
 ##
@@ -200,7 +201,8 @@
 #
 # -> { "execute": "query-sev-capabilities" }
 # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
-#                  "cbitpos": 47, "reduced-phys-bits": 5}}
+#                  "cbitpos": 47, "reduced-phys-bits": 5,
+#                  "sev-max-guests" : 15}}
 #
 ##
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
diff --git a/target/i386/sev.c b/target/i386/sev.c
index cd77f6b5d4..bb0cd79acd 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -488,7 +488,7 @@ sev_get_capabilities(void)
     guchar *pdh_data = NULL;
     guchar *cert_chain_data = NULL;
     size_t pdh_len = 0, cert_chain_len = 0;
-    uint32_t ebx;
+    uint32_t ebx, ecx, edx;
     int fd;
 
     fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
@@ -507,7 +507,7 @@ sev_get_capabilities(void)
     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, NULL, &ebx, &ecx, &edx);
     cap->cbitpos = ebx & 0x3f;
 
     /*
@@ -516,6 +516,8 @@ sev_get_capabilities(void)
      */
     cap->reduced_phys_bits = 1;
 
+    /* the maximum number of SEV guests that can run simultaneously */
+    cap->sev_max_guests = ecx - edx + 1;
 out:
     g_free(pdh_data);
     g_free(cert_chain_data);
-- 
2.17.1

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Daniel P. Berrangé]

On Thu, Apr 11, 2019 at 05:59:50PM +0000, Singh, Brijesh wrote:
> There are limited numbers of the SEV guests that can be run concurrently.
> A management applications may need to know this limit so that it can place
> SEV VMs on hosts which have suitable resources available.
> 
> Currently, this limit is not exposed to the application. Add a new
> 'sev-max-guest' field in the query-sev-capabilities to provide this
> information.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Erik Skultety <eskultet@redhat.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  qapi/target.json  | 6 ++++--
>  target/i386/sev.c | 6 ++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/qapi/target.json b/qapi/target.json
> index 1d4d54b600..b45121d30b 100644
> --- a/qapi/target.json
> +++ b/qapi/target.json
> @@ -183,7 +183,8 @@

A few lines above here you need to document the new field
with (since 4.1) annotation.

>    'data': { 'pdh': 'str',
>              'cert-chain': 'str',
>              'cbitpos': 'int',
> -            'reduced-phys-bits': 'int'},
> +            'reduced-phys-bits': 'int',
> +            'sev-max-guests': 'int'},
>    'if': 'defined(TARGET_I386)' }
>  
>  ##
> @@ -200,7 +201,8 @@
>  #
>  # -> { "execute": "query-sev-capabilities" }
>  # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
> -#                  "cbitpos": 47, "reduced-phys-bits": 5}}
> +#                  "cbitpos": 47, "reduced-phys-bits": 5,
> +#                  "sev-max-guests" : 15}}
>  #
>  ##
>  { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index cd77f6b5d4..bb0cd79acd 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -488,7 +488,7 @@ sev_get_capabilities(void)
>      guchar *pdh_data = NULL;
>      guchar *cert_chain_data = NULL;
>      size_t pdh_len = 0, cert_chain_len = 0;
> -    uint32_t ebx;
> +    uint32_t ebx, ecx, edx;
>      int fd;
>  
>      fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
> @@ -507,7 +507,7 @@ sev_get_capabilities(void)
>      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, NULL, &ebx, &ecx, &edx);
>      cap->cbitpos = ebx & 0x3f;
>  
>      /*
> @@ -516,6 +516,8 @@ sev_get_capabilities(void)
>       */
>      cap->reduced_phys_bits = 1;
>  
> +    /* the maximum number of SEV guests that can run simultaneously */
> +    cap->sev_max_guests = ecx - edx + 1;
>  out:
>      g_free(pdh_data);
>      g_free(cert_chain_data);

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 :|

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Daniel P. Berrangé]

On Thu, Apr 11, 2019 at 05:59:50PM +0000, Singh, Brijesh wrote:
> There are limited numbers of the SEV guests that can be run concurrently.
> A management applications may need to know this limit so that it can place
> SEV VMs on hosts which have suitable resources available.
> 
> Currently, this limit is not exposed to the application. Add a new
> 'sev-max-guest' field in the query-sev-capabilities to provide this
> information.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Erik Skultety <eskultet@redhat.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  qapi/target.json  | 6 ++++--
>  target/i386/sev.c | 6 ++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/qapi/target.json b/qapi/target.json
> index 1d4d54b600..b45121d30b 100644
> --- a/qapi/target.json
> +++ b/qapi/target.json
> @@ -183,7 +183,8 @@

A few lines above here you need to document the new field
with (since 4.1) annotation.

>    'data': { 'pdh': 'str',
>              'cert-chain': 'str',
>              'cbitpos': 'int',
> -            'reduced-phys-bits': 'int'},
> +            'reduced-phys-bits': 'int',
> +            'sev-max-guests': 'int'},
>    'if': 'defined(TARGET_I386)' }
>  
>  ##
> @@ -200,7 +201,8 @@
>  #
>  # -> { "execute": "query-sev-capabilities" }
>  # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
> -#                  "cbitpos": 47, "reduced-phys-bits": 5}}
> +#                  "cbitpos": 47, "reduced-phys-bits": 5,
> +#                  "sev-max-guests" : 15}}
>  #
>  ##
>  { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index cd77f6b5d4..bb0cd79acd 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -488,7 +488,7 @@ sev_get_capabilities(void)
>      guchar *pdh_data = NULL;
>      guchar *cert_chain_data = NULL;
>      size_t pdh_len = 0, cert_chain_len = 0;
> -    uint32_t ebx;
> +    uint32_t ebx, ecx, edx;
>      int fd;
>  
>      fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
> @@ -507,7 +507,7 @@ sev_get_capabilities(void)
>      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, NULL, &ebx, &ecx, &edx);
>      cap->cbitpos = ebx & 0x3f;
>  
>      /*
> @@ -516,6 +516,8 @@ sev_get_capabilities(void)
>       */
>      cap->reduced_phys_bits = 1;
>  
> +    /* the maximum number of SEV guests that can run simultaneously */
> +    cap->sev_max_guests = ecx - edx + 1;
>  out:
>      g_free(pdh_data);
>      g_free(cert_chain_data);

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 :|

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Laszlo Ersek]

On 04/11/19 19:59, Singh, Brijesh wrote:
> There are limited numbers of the SEV guests that can be run concurrently.
> A management applications may need to know this limit so that it can place
> SEV VMs on hosts which have suitable resources available.
> 
> Currently, this limit is not exposed to the application. Add a new
> 'sev-max-guest' field in the query-sev-capabilities to provide this
> information.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Erik Skultety <eskultet@redhat.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  qapi/target.json  | 6 ++++--
>  target/i386/sev.c | 6 ++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/qapi/target.json b/qapi/target.json
> index 1d4d54b600..b45121d30b 100644
> --- a/qapi/target.json
> +++ b/qapi/target.json
> @@ -183,7 +183,8 @@
>    'data': { 'pdh': 'str',
>              'cert-chain': 'str',
>              'cbitpos': 'int',
> -            'reduced-phys-bits': 'int'},
> +            'reduced-phys-bits': 'int',
> +            'sev-max-guests': 'int'},

Would it be useful to make this new field optional? E.g. if it was
missing, libvirtd could assume "no limit".

Again, not sure if that's useful, but it's not hard to introduce the
field as optional now. Removing mandatory fields later is impossible.

Thanks
Laszlo

>    'if': 'defined(TARGET_I386)' }
>  
>  ##
> @@ -200,7 +201,8 @@
>  #
>  # -> { "execute": "query-sev-capabilities" }
>  # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
> -#                  "cbitpos": 47, "reduced-phys-bits": 5}}
> +#                  "cbitpos": 47, "reduced-phys-bits": 5,
> +#                  "sev-max-guests" : 15}}
>  #
>  ##
>  { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index cd77f6b5d4..bb0cd79acd 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -488,7 +488,7 @@ sev_get_capabilities(void)
>      guchar *pdh_data = NULL;
>      guchar *cert_chain_data = NULL;
>      size_t pdh_len = 0, cert_chain_len = 0;
> -    uint32_t ebx;
> +    uint32_t ebx, ecx, edx;
>      int fd;
>  
>      fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
> @@ -507,7 +507,7 @@ sev_get_capabilities(void)
>      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, NULL, &ebx, &ecx, &edx);
>      cap->cbitpos = ebx & 0x3f;
>  
>      /*
> @@ -516,6 +516,8 @@ sev_get_capabilities(void)
>       */
>      cap->reduced_phys_bits = 1;
>  
> +    /* the maximum number of SEV guests that can run simultaneously */
> +    cap->sev_max_guests = ecx - edx + 1;
>  out:
>      g_free(pdh_data);
>      g_free(cert_chain_data);
> 

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Laszlo Ersek]

On 04/11/19 19:59, Singh, Brijesh wrote:
> There are limited numbers of the SEV guests that can be run concurrently.
> A management applications may need to know this limit so that it can place
> SEV VMs on hosts which have suitable resources available.
> 
> Currently, this limit is not exposed to the application. Add a new
> 'sev-max-guest' field in the query-sev-capabilities to provide this
> information.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Erik Skultety <eskultet@redhat.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  qapi/target.json  | 6 ++++--
>  target/i386/sev.c | 6 ++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/qapi/target.json b/qapi/target.json
> index 1d4d54b600..b45121d30b 100644
> --- a/qapi/target.json
> +++ b/qapi/target.json
> @@ -183,7 +183,8 @@
>    'data': { 'pdh': 'str',
>              'cert-chain': 'str',
>              'cbitpos': 'int',
> -            'reduced-phys-bits': 'int'},
> +            'reduced-phys-bits': 'int',
> +            'sev-max-guests': 'int'},

Would it be useful to make this new field optional? E.g. if it was
missing, libvirtd could assume "no limit".

Again, not sure if that's useful, but it's not hard to introduce the
field as optional now. Removing mandatory fields later is impossible.

Thanks
Laszlo

>    'if': 'defined(TARGET_I386)' }
>  
>  ##
> @@ -200,7 +201,8 @@
>  #
>  # -> { "execute": "query-sev-capabilities" }
>  # <- { "return": { "pdh": "8CCDD8DDD", "cert-chain": "888CCCDDDEE",
> -#                  "cbitpos": 47, "reduced-phys-bits": 5}}
> +#                  "cbitpos": 47, "reduced-phys-bits": 5,
> +#                  "sev-max-guests" : 15}}
>  #
>  ##
>  { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index cd77f6b5d4..bb0cd79acd 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -488,7 +488,7 @@ sev_get_capabilities(void)
>      guchar *pdh_data = NULL;
>      guchar *cert_chain_data = NULL;
>      size_t pdh_len = 0, cert_chain_len = 0;
> -    uint32_t ebx;
> +    uint32_t ebx, ecx, edx;
>      int fd;
>  
>      fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
> @@ -507,7 +507,7 @@ sev_get_capabilities(void)
>      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, NULL, &ebx, &ecx, &edx);
>      cap->cbitpos = ebx & 0x3f;
>  
>      /*
> @@ -516,6 +516,8 @@ sev_get_capabilities(void)
>       */
>      cap->reduced_phys_bits = 1;
>  
> +    /* the maximum number of SEV guests that can run simultaneously */
> +    cap->sev_max_guests = ecx - edx + 1;
>  out:
>      g_free(pdh_data);
>      g_free(cert_chain_data);
> 

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Singh, Brijesh]

On 4/11/19 1:05 PM, Daniel P. Berrangé wrote:
> On Thu, Apr 11, 2019 at 05:59:50PM +0000, Singh, Brijesh wrote:
>> There are limited numbers of the SEV guests that can be run concurrently.
>> A management applications may need to know this limit so that it can place
>> SEV VMs on hosts which have suitable resources available.
>>
>> Currently, this limit is not exposed to the application. Add a new
>> 'sev-max-guest' field in the query-sev-capabilities to provide this
>> information.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Eric Blake <eblake@redhat.com>
>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Erik Skultety <eskultet@redhat.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   qapi/target.json  | 6 ++++--
>>   target/i386/sev.c | 6 ++++--
>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/qapi/target.json b/qapi/target.json
>> index 1d4d54b600..b45121d30b 100644
>> --- a/qapi/target.json
>> +++ b/qapi/target.json
>> @@ -183,7 +183,8 @@
> 
> A few lines above here you need to document the new field
> with (since 4.1) annotation.
> 

noted. thanks

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Singh, Brijesh]

On 4/11/19 1:05 PM, Daniel P. Berrangé wrote:
> On Thu, Apr 11, 2019 at 05:59:50PM +0000, Singh, Brijesh wrote:
>> There are limited numbers of the SEV guests that can be run concurrently.
>> A management applications may need to know this limit so that it can place
>> SEV VMs on hosts which have suitable resources available.
>>
>> Currently, this limit is not exposed to the application. Add a new
>> 'sev-max-guest' field in the query-sev-capabilities to provide this
>> information.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Eric Blake <eblake@redhat.com>
>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Erik Skultety <eskultet@redhat.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   qapi/target.json  | 6 ++++--
>>   target/i386/sev.c | 6 ++++--
>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/qapi/target.json b/qapi/target.json
>> index 1d4d54b600..b45121d30b 100644
>> --- a/qapi/target.json
>> +++ b/qapi/target.json
>> @@ -183,7 +183,8 @@
> 
> A few lines above here you need to document the new field
> with (since 4.1) annotation.
> 

noted. thanks

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Laszlo Ersek]

On 04/11/19 20:10, Laszlo Ersek wrote:
> On 04/11/19 19:59, Singh, Brijesh wrote:
>> There are limited numbers of the SEV guests that can be run concurrently.
>> A management applications may need to know this limit so that it can place
>> SEV VMs on hosts which have suitable resources available.
>>
>> Currently, this limit is not exposed to the application. Add a new
>> 'sev-max-guest' field in the query-sev-capabilities to provide this
>> information.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Eric Blake <eblake@redhat.com>
>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Erik Skultety <eskultet@redhat.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>  qapi/target.json  | 6 ++++--
>>  target/i386/sev.c | 6 ++++--
>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/qapi/target.json b/qapi/target.json
>> index 1d4d54b600..b45121d30b 100644
>> --- a/qapi/target.json
>> +++ b/qapi/target.json
>> @@ -183,7 +183,8 @@
>>    'data': { 'pdh': 'str',
>>              'cert-chain': 'str',
>>              'cbitpos': 'int',
>> -            'reduced-phys-bits': 'int'},
>> +            'reduced-phys-bits': 'int',
>> +            'sev-max-guests': 'int'},
> 
> Would it be useful to make this new field optional? E.g. if it was
> missing, libvirtd could assume "no limit".
> 
> Again, not sure if that's useful, but it's not hard to introduce the
> field as optional now. Removing mandatory fields later is impossible.

On second thought, if we're sure the hardware / encryption engine will
always have this kind of limitation, then mandatory looks fine.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Laszlo Ersek]

On 04/11/19 20:10, Laszlo Ersek wrote:
> On 04/11/19 19:59, Singh, Brijesh wrote:
>> There are limited numbers of the SEV guests that can be run concurrently.
>> A management applications may need to know this limit so that it can place
>> SEV VMs on hosts which have suitable resources available.
>>
>> Currently, this limit is not exposed to the application. Add a new
>> 'sev-max-guest' field in the query-sev-capabilities to provide this
>> information.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Eric Blake <eblake@redhat.com>
>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Erik Skultety <eskultet@redhat.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>  qapi/target.json  | 6 ++++--
>>  target/i386/sev.c | 6 ++++--
>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/qapi/target.json b/qapi/target.json
>> index 1d4d54b600..b45121d30b 100644
>> --- a/qapi/target.json
>> +++ b/qapi/target.json
>> @@ -183,7 +183,8 @@
>>    'data': { 'pdh': 'str',
>>              'cert-chain': 'str',
>>              'cbitpos': 'int',
>> -            'reduced-phys-bits': 'int'},
>> +            'reduced-phys-bits': 'int',
>> +            'sev-max-guests': 'int'},
> 
> Would it be useful to make this new field optional? E.g. if it was
> missing, libvirtd could assume "no limit".
> 
> Again, not sure if that's useful, but it's not hard to introduce the
> field as optional now. Removing mandatory fields later is impossible.

On second thought, if we're sure the hardware / encryption engine will
always have this kind of limitation, then mandatory looks fine.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Singh, Brijesh]

On 4/11/19 1:10 PM, Laszlo Ersek wrote:
> On 04/11/19 19:59, Singh, Brijesh wrote:
>> There are limited numbers of the SEV guests that can be run concurrently.
>> A management applications may need to know this limit so that it can place
>> SEV VMs on hosts which have suitable resources available.
>>
>> Currently, this limit is not exposed to the application. Add a new
>> 'sev-max-guest' field in the query-sev-capabilities to provide this
>> information.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Eric Blake <eblake@redhat.com>
>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Erik Skultety <eskultet@redhat.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   qapi/target.json  | 6 ++++--
>>   target/i386/sev.c | 6 ++++--
>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/qapi/target.json b/qapi/target.json
>> index 1d4d54b600..b45121d30b 100644
>> --- a/qapi/target.json
>> +++ b/qapi/target.json
>> @@ -183,7 +183,8 @@
>>     'data': { 'pdh': 'str',
>>               'cert-chain': 'str',
>>               'cbitpos': 'int',
>> -            'reduced-phys-bits': 'int'},
>> +            'reduced-phys-bits': 'int',
>> +            'sev-max-guests': 'int'},
> 
> Would it be useful to make this new field optional? E.g. if it was
> missing, libvirtd could assume "no limit".
> 

I am not sure if we need to make this field optional - mainly because
in SEV context hardware will always have some limits (at least in
foreseeable future). The architecture provides us a CPUID to query
this capabilities so I am assuming that future CPUs will populate
some values in it.

> Again, not sure if that's useful, but it's not hard to introduce the
> field as optional now. Removing mandatory fields later is impossible.
> 

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Singh, Brijesh]

On 4/11/19 1:10 PM, Laszlo Ersek wrote:
> On 04/11/19 19:59, Singh, Brijesh wrote:
>> There are limited numbers of the SEV guests that can be run concurrently.
>> A management applications may need to know this limit so that it can place
>> SEV VMs on hosts which have suitable resources available.
>>
>> Currently, this limit is not exposed to the application. Add a new
>> 'sev-max-guest' field in the query-sev-capabilities to provide this
>> information.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Eric Blake <eblake@redhat.com>
>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Erik Skultety <eskultet@redhat.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>   qapi/target.json  | 6 ++++--
>>   target/i386/sev.c | 6 ++++--
>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/qapi/target.json b/qapi/target.json
>> index 1d4d54b600..b45121d30b 100644
>> --- a/qapi/target.json
>> +++ b/qapi/target.json
>> @@ -183,7 +183,8 @@
>>     'data': { 'pdh': 'str',
>>               'cert-chain': 'str',
>>               'cbitpos': 'int',
>> -            'reduced-phys-bits': 'int'},
>> +            'reduced-phys-bits': 'int',
>> +            'sev-max-guests': 'int'},
> 
> Would it be useful to make this new field optional? E.g. if it was
> missing, libvirtd could assume "no limit".
> 

I am not sure if we need to make this field optional - mainly because
in SEV context hardware will always have some limits (at least in
foreseeable future). The architecture provides us a CPUID to query
this capabilities so I am assuming that future CPUs will populate
some values in it.

> Again, not sure if that's useful, but it's not hard to introduce the
> field as optional now. Removing mandatory fields later is impossible.
> 

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Laszlo Ersek]

On 04/11/19 21:02, Singh, Brijesh wrote:
> 
> 
> On 4/11/19 1:10 PM, Laszlo Ersek wrote:
>> On 04/11/19 19:59, Singh, Brijesh wrote:
>>> There are limited numbers of the SEV guests that can be run concurrently.
>>> A management applications may need to know this limit so that it can place
>>> SEV VMs on hosts which have suitable resources available.
>>>
>>> Currently, this limit is not exposed to the application. Add a new
>>> 'sev-max-guest' field in the query-sev-capabilities to provide this
>>> information.
>>>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> Cc: Eric Blake <eblake@redhat.com>
>>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Erik Skultety <eskultet@redhat.com>
>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>> ---
>>>   qapi/target.json  | 6 ++++--
>>>   target/i386/sev.c | 6 ++++--
>>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/qapi/target.json b/qapi/target.json
>>> index 1d4d54b600..b45121d30b 100644
>>> --- a/qapi/target.json
>>> +++ b/qapi/target.json
>>> @@ -183,7 +183,8 @@
>>>     'data': { 'pdh': 'str',
>>>               'cert-chain': 'str',
>>>               'cbitpos': 'int',
>>> -            'reduced-phys-bits': 'int'},
>>> +            'reduced-phys-bits': 'int',
>>> +            'sev-max-guests': 'int'},
>>
>> Would it be useful to make this new field optional? E.g. if it was
>> missing, libvirtd could assume "no limit".
>>
> 
> I am not sure if we need to make this field optional - mainly because
> in SEV context hardware will always have some limits (at least in
> foreseeable future). The architecture provides us a CPUID to query
> this capabilities so I am assuming that future CPUs will populate
> some values in it.

Yup, sounds reasonable. Please resubmit with Daniel's request addressed
and I'll be happy to R-b.

Erik: can you please ACK too?

Thanks!
Laszlo

>> Again, not sure if that's useful, but it's not hard to introduce the
>> field as optional now. Removing mandatory fields later is impossible.
>>
> 

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Laszlo Ersek]

On 04/11/19 21:02, Singh, Brijesh wrote:
> 
> 
> On 4/11/19 1:10 PM, Laszlo Ersek wrote:
>> On 04/11/19 19:59, Singh, Brijesh wrote:
>>> There are limited numbers of the SEV guests that can be run concurrently.
>>> A management applications may need to know this limit so that it can place
>>> SEV VMs on hosts which have suitable resources available.
>>>
>>> Currently, this limit is not exposed to the application. Add a new
>>> 'sev-max-guest' field in the query-sev-capabilities to provide this
>>> information.
>>>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> Cc: Eric Blake <eblake@redhat.com>
>>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Erik Skultety <eskultet@redhat.com>
>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>> ---
>>>   qapi/target.json  | 6 ++++--
>>>   target/i386/sev.c | 6 ++++--
>>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/qapi/target.json b/qapi/target.json
>>> index 1d4d54b600..b45121d30b 100644
>>> --- a/qapi/target.json
>>> +++ b/qapi/target.json
>>> @@ -183,7 +183,8 @@
>>>     'data': { 'pdh': 'str',
>>>               'cert-chain': 'str',
>>>               'cbitpos': 'int',
>>> -            'reduced-phys-bits': 'int'},
>>> +            'reduced-phys-bits': 'int',
>>> +            'sev-max-guests': 'int'},
>>
>> Would it be useful to make this new field optional? E.g. if it was
>> missing, libvirtd could assume "no limit".
>>
> 
> I am not sure if we need to make this field optional - mainly because
> in SEV context hardware will always have some limits (at least in
> foreseeable future). The architecture provides us a CPUID to query
> this capabilities so I am assuming that future CPUs will populate
> some values in it.

Yup, sounds reasonable. Please resubmit with Daniel's request addressed
and I'll be happy to R-b.

Erik: can you please ACK too?

Thanks!
Laszlo

>> Again, not sure if that's useful, but it's not hard to introduce the
>> field as optional now. Removing mandatory fields later is impossible.
>>
> 

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Paolo Bonzini]

On 11/04/19 21:02, Singh, Brijesh wrote:
> 
> 
> On 4/11/19 1:10 PM, Laszlo Ersek wrote:
>> On 04/11/19 19:59, Singh, Brijesh wrote:
>>> There are limited numbers of the SEV guests that can be run concurrently.
>>> A management applications may need to know this limit so that it can place
>>> SEV VMs on hosts which have suitable resources available.
>>>
>>> Currently, this limit is not exposed to the application. Add a new
>>> 'sev-max-guest' field in the query-sev-capabilities to provide this
>>> information.
>>>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> Cc: Eric Blake <eblake@redhat.com>
>>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Erik Skultety <eskultet@redhat.com>
>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>> ---
>>>   qapi/target.json  | 6 ++++--
>>>   target/i386/sev.c | 6 ++++--
>>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/qapi/target.json b/qapi/target.json
>>> index 1d4d54b600..b45121d30b 100644
>>> --- a/qapi/target.json
>>> +++ b/qapi/target.json
>>> @@ -183,7 +183,8 @@
>>>     'data': { 'pdh': 'str',
>>>               'cert-chain': 'str',
>>>               'cbitpos': 'int',
>>> -            'reduced-phys-bits': 'int'},
>>> +            'reduced-phys-bits': 'int',
>>> +            'sev-max-guests': 'int'},
>>
>> Would it be useful to make this new field optional? E.g. if it was
>> missing, libvirtd could assume "no limit".
>>
> 
> I am not sure if we need to make this field optional - mainly because
> in SEV context hardware will always have some limits (at least in
> foreseeable future). The architecture provides us a CPUID to query
> this capabilities so I am assuming that future CPUs will populate
> some values in it.

Since this field is not specific to guest configuration, I don't think
it belongs in query-sev-capabilities; QEMU does not care about >1 guest.

Paolo

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Paolo Bonzini]

On 11/04/19 21:02, Singh, Brijesh wrote:
> 
> 
> On 4/11/19 1:10 PM, Laszlo Ersek wrote:
>> On 04/11/19 19:59, Singh, Brijesh wrote:
>>> There are limited numbers of the SEV guests that can be run concurrently.
>>> A management applications may need to know this limit so that it can place
>>> SEV VMs on hosts which have suitable resources available.
>>>
>>> Currently, this limit is not exposed to the application. Add a new
>>> 'sev-max-guest' field in the query-sev-capabilities to provide this
>>> information.
>>>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> Cc: Eric Blake <eblake@redhat.com>
>>> Cc: Daniel P. Berrangé <berrange@redhat.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Erik Skultety <eskultet@redhat.com>
>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>> ---
>>>   qapi/target.json  | 6 ++++--
>>>   target/i386/sev.c | 6 ++++--
>>>   2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/qapi/target.json b/qapi/target.json
>>> index 1d4d54b600..b45121d30b 100644
>>> --- a/qapi/target.json
>>> +++ b/qapi/target.json
>>> @@ -183,7 +183,8 @@
>>>     'data': { 'pdh': 'str',
>>>               'cert-chain': 'str',
>>>               'cbitpos': 'int',
>>> -            'reduced-phys-bits': 'int'},
>>> +            'reduced-phys-bits': 'int',
>>> +            'sev-max-guests': 'int'},
>>
>> Would it be useful to make this new field optional? E.g. if it was
>> missing, libvirtd could assume "no limit".
>>
> 
> I am not sure if we need to make this field optional - mainly because
> in SEV context hardware will always have some limits (at least in
> foreseeable future). The architecture provides us a CPUID to query
> this capabilities so I am assuming that future CPUs will populate
> some values in it.

Since this field is not specific to guest configuration, I don't think
it belongs in query-sev-capabilities; QEMU does not care about >1 guest.

Paolo

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Erik Skultety]

On Fri, Apr 12, 2019 at 09:45:02AM +0200, Paolo Bonzini wrote:
> On 11/04/19 21:02, Singh, Brijesh wrote:
> >
> >
> > On 4/11/19 1:10 PM, Laszlo Ersek wrote:
> >> On 04/11/19 19:59, Singh, Brijesh wrote:
> >>> There are limited numbers of the SEV guests that can be run concurrently.
> >>> A management applications may need to know this limit so that it can place
> >>> SEV VMs on hosts which have suitable resources available.
> >>>
> >>> Currently, this limit is not exposed to the application. Add a new
> >>> 'sev-max-guest' field in the query-sev-capabilities to provide this
> >>> information.
> >>>
> >>> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >>> Cc: Markus Armbruster <armbru@redhat.com>
> >>> Cc: Eric Blake <eblake@redhat.com>
> >>> Cc: Daniel P. Berrangé <berrange@redhat.com>
> >>> Cc: Laszlo Ersek <lersek@redhat.com>
> >>> Cc: Erik Skultety <eskultet@redhat.com>
> >>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> >>> ---
> >>>   qapi/target.json  | 6 ++++--
> >>>   target/i386/sev.c | 6 ++++--
> >>>   2 files changed, 8 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/qapi/target.json b/qapi/target.json
> >>> index 1d4d54b600..b45121d30b 100644
> >>> --- a/qapi/target.json
> >>> +++ b/qapi/target.json
> >>> @@ -183,7 +183,8 @@
> >>>     'data': { 'pdh': 'str',
> >>>               'cert-chain': 'str',
> >>>               'cbitpos': 'int',
> >>> -            'reduced-phys-bits': 'int'},
> >>> +            'reduced-phys-bits': 'int',
> >>> +            'sev-max-guests': 'int'},
> >>
> >> Would it be useful to make this new field optional? E.g. if it was
> >> missing, libvirtd could assume "no limit".
> >>
> >
> > I am not sure if we need to make this field optional - mainly because
> > in SEV context hardware will always have some limits (at least in
> > foreseeable future). The architecture provides us a CPUID to query
> > this capabilities so I am assuming that future CPUs will populate
> > some values in it.
>
> Since this field is not specific to guest configuration, I don't think
> it belongs in query-sev-capabilities; QEMU does not care about >1 guest.

Neither pdh nor cert-chain are specific to the guest config. I see why this
should be better suited for query-sev, the same goes for libvirt - I think we
shouldn't have gone with reporting the SEV platform caps in domain capabilities,
we should have IMHO report it both in the host capabilities (platform specific
stuff) and in domain capabilities to indicate that both libvirt and QEMU
support the SEV feature. Having said that, we have a precedent which I think
we might be better off with following rather than splitting the information
among multiple commands.

Regards,
Erik

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Erik Skultety]

On Fri, Apr 12, 2019 at 09:45:02AM +0200, Paolo Bonzini wrote:
> On 11/04/19 21:02, Singh, Brijesh wrote:
> >
> >
> > On 4/11/19 1:10 PM, Laszlo Ersek wrote:
> >> On 04/11/19 19:59, Singh, Brijesh wrote:
> >>> There are limited numbers of the SEV guests that can be run concurrently.
> >>> A management applications may need to know this limit so that it can place
> >>> SEV VMs on hosts which have suitable resources available.
> >>>
> >>> Currently, this limit is not exposed to the application. Add a new
> >>> 'sev-max-guest' field in the query-sev-capabilities to provide this
> >>> information.
> >>>
> >>> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >>> Cc: Markus Armbruster <armbru@redhat.com>
> >>> Cc: Eric Blake <eblake@redhat.com>
> >>> Cc: Daniel P. Berrangé <berrange@redhat.com>
> >>> Cc: Laszlo Ersek <lersek@redhat.com>
> >>> Cc: Erik Skultety <eskultet@redhat.com>
> >>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> >>> ---
> >>>   qapi/target.json  | 6 ++++--
> >>>   target/i386/sev.c | 6 ++++--
> >>>   2 files changed, 8 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/qapi/target.json b/qapi/target.json
> >>> index 1d4d54b600..b45121d30b 100644
> >>> --- a/qapi/target.json
> >>> +++ b/qapi/target.json
> >>> @@ -183,7 +183,8 @@
> >>>     'data': { 'pdh': 'str',
> >>>               'cert-chain': 'str',
> >>>               'cbitpos': 'int',
> >>> -            'reduced-phys-bits': 'int'},
> >>> +            'reduced-phys-bits': 'int',
> >>> +            'sev-max-guests': 'int'},
> >>
> >> Would it be useful to make this new field optional? E.g. if it was
> >> missing, libvirtd could assume "no limit".
> >>
> >
> > I am not sure if we need to make this field optional - mainly because
> > in SEV context hardware will always have some limits (at least in
> > foreseeable future). The architecture provides us a CPUID to query
> > this capabilities so I am assuming that future CPUs will populate
> > some values in it.
>
> Since this field is not specific to guest configuration, I don't think
> it belongs in query-sev-capabilities; QEMU does not care about >1 guest.

Neither pdh nor cert-chain are specific to the guest config. I see why this
should be better suited for query-sev, the same goes for libvirt - I think we
shouldn't have gone with reporting the SEV platform caps in domain capabilities,
we should have IMHO report it both in the host capabilities (platform specific
stuff) and in domain capabilities to indicate that both libvirt and QEMU
support the SEV feature. Having said that, we have a precedent which I think
we might be better off with following rather than splitting the information
among multiple commands.

Regards,
Erik

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Paolo Bonzini]

On 12/04/19 10:19, Erik Skultety wrote:
>> Since this field is not specific to guest configuration, I don't think
>> it belongs in query-sev-capabilities; QEMU does not care about >1 guest.
> Neither pdh nor cert-chain are specific to the guest config.

Sort of, they are required to start a guest, aren't they?  But the
number of guests is irrelevant.

> I see why this
> should be better suited for query-sev, the same goes for libvirt - I think we
> shouldn't have gone with reporting the SEV platform caps in domain capabilities,
> we should have IMHO report it both in the host capabilities (platform specific
> stuff) and in domain capabilities to indicate that both libvirt and QEMU
> support the SEV feature. Having said that, we have a precedent which I think
> we might be better off with following rather than splitting the information
> among multiple commands.

For Libvirt, sure.  But I think this doesn't belong in QEMU at all.
Libvirt should just use CPUID.

Paolo

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Paolo Bonzini]

On 12/04/19 10:19, Erik Skultety wrote:
>> Since this field is not specific to guest configuration, I don't think
>> it belongs in query-sev-capabilities; QEMU does not care about >1 guest.
> Neither pdh nor cert-chain are specific to the guest config.

Sort of, they are required to start a guest, aren't they?  But the
number of guests is irrelevant.

> I see why this
> should be better suited for query-sev, the same goes for libvirt - I think we
> shouldn't have gone with reporting the SEV platform caps in domain capabilities,
> we should have IMHO report it both in the host capabilities (platform specific
> stuff) and in domain capabilities to indicate that both libvirt and QEMU
> support the SEV feature. Having said that, we have a precedent which I think
> we might be better off with following rather than splitting the information
> among multiple commands.

For Libvirt, sure.  But I think this doesn't belong in QEMU at all.
Libvirt should just use CPUID.

Paolo

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Erik Skultety]

On Fri, Apr 12, 2019 at 10:26:45AM +0200, Paolo Bonzini wrote:
> On 12/04/19 10:19, Erik Skultety wrote:
> >> Since this field is not specific to guest configuration, I don't think
> >> it belongs in query-sev-capabilities; QEMU does not care about >1 guest.
> > Neither pdh nor cert-chain are specific to the guest config.
>
> Sort of, they are required to start a guest, aren't they?  But the

Unless you're interested in the measurement, aka attestation, I don't think
those are required in any way.

Erik

> number of guests is irrelevant.
>
> > I see why this
> > should be better suited for query-sev, the same goes for libvirt - I think we
> > shouldn't have gone with reporting the SEV platform caps in domain capabilities,
> > we should have IMHO report it both in the host capabilities (platform specific
> > stuff) and in domain capabilities to indicate that both libvirt and QEMU
> > support the SEV feature. Having said that, we have a precedent which I think
> > we might be better off with following rather than splitting the information
> > among multiple commands.
>
> For Libvirt, sure.  But I think this doesn't belong in QEMU at all.
> Libvirt should just use CPUID.
>
> Paolo

Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'

[Erik Skultety]

On Fri, Apr 12, 2019 at 10:26:45AM +0200, Paolo Bonzini wrote:
> On 12/04/19 10:19, Erik Skultety wrote:
> >> Since this field is not specific to guest configuration, I don't think
> >> it belongs in query-sev-capabilities; QEMU does not care about >1 guest.
> > Neither pdh nor cert-chain are specific to the guest config.
>
> Sort of, they are required to start a guest, aren't they?  But the

Unless you're interested in the measurement, aka attestation, I don't think
those are required in any way.

Erik

> number of guests is irrelevant.
>
> > I see why this
> > should be better suited for query-sev, the same goes for libvirt - I think we
> > shouldn't have gone with reporting the SEV platform caps in domain capabilities,
> > we should have IMHO report it both in the host capabilities (platform specific
> > stuff) and in domain capabilities to indicate that both libvirt and QEMU
> > support the SEV feature. Having said that, we have a precedent which I think
> > we might be better off with following rather than splitting the information
> > among multiple commands.
>
> For Libvirt, sure.  But I think this doesn't belong in QEMU at all.
> Libvirt should just use CPUID.
>
> Paolo