* [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-11 17:59 ` Singh, Brijesh
0 siblings, 0 replies; 22+ messages in thread
From: Singh, Brijesh @ 2019-04-11 17:59 UTC (permalink / raw)
To: qemu-devel
Cc: Erik Skultety, Laszlo Ersek, Markus Armbruster, Singh, Brijesh,
Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-11 18:05 ` Daniel P. Berrangé
0 siblings, 0 replies; 22+ messages in thread
From: Daniel P. Berrangé @ 2019-04-11 18:05 UTC (permalink / raw)
To: Singh, Brijesh
Cc: qemu-devel, Paolo Bonzini, Markus Armbruster, Eric Blake,
Laszlo Ersek, Erik Skultety
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 :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-11 18:05 ` Daniel P. Berrangé
0 siblings, 0 replies; 22+ messages in thread
From: Daniel P. Berrangé @ 2019-04-11 18:05 UTC (permalink / raw)
To: Singh, Brijesh
Cc: Erik Skultety, Laszlo Ersek, qemu-devel, Markus Armbruster,
Paolo Bonzini
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 :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-11 18:18 ` Singh, Brijesh
0 siblings, 0 replies; 22+ messages in thread
From: Singh, Brijesh @ 2019-04-11 18:18 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Singh, Brijesh, qemu-devel, Paolo Bonzini, Markus Armbruster,
Eric Blake, Laszlo Ersek, Erik Skultety
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-11 18:18 ` Singh, Brijesh
0 siblings, 0 replies; 22+ messages in thread
From: Singh, Brijesh @ 2019-04-11 18:18 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Singh, Brijesh, Erik Skultety, qemu-devel, Markus Armbruster,
Paolo Bonzini, Laszlo Ersek
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-11 18:10 ` Laszlo Ersek
0 siblings, 0 replies; 22+ messages in thread
From: Laszlo Ersek @ 2019-04-11 18:10 UTC (permalink / raw)
To: Singh, Brijesh, qemu-devel
Cc: Erik Skultety, Markus Armbruster, Paolo Bonzini
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);
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-11 18:10 ` Laszlo Ersek
0 siblings, 0 replies; 22+ messages in thread
From: Laszlo Ersek @ 2019-04-11 18:10 UTC (permalink / raw)
To: Singh, Brijesh, qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Erik Skultety
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);
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-11 19:01 ` Laszlo Ersek
0 siblings, 0 replies; 22+ messages in thread
From: Laszlo Ersek @ 2019-04-11 19:01 UTC (permalink / raw)
To: Singh, Brijesh, qemu-devel
Cc: Erik Skultety, Markus Armbruster, Paolo Bonzini
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-11 19:01 ` Laszlo Ersek
0 siblings, 0 replies; 22+ messages in thread
From: Laszlo Ersek @ 2019-04-11 19:01 UTC (permalink / raw)
To: Singh, Brijesh, qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Erik Skultety
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-11 19:02 ` Singh, Brijesh
0 siblings, 0 replies; 22+ messages in thread
From: Singh, Brijesh @ 2019-04-11 19:02 UTC (permalink / raw)
To: Laszlo Ersek, qemu-devel
Cc: Singh, Brijesh, Erik Skultety, Markus Armbruster, Paolo Bonzini
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.
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-11 19:02 ` Singh, Brijesh
0 siblings, 0 replies; 22+ messages in thread
From: Singh, Brijesh @ 2019-04-11 19:02 UTC (permalink / raw)
To: Laszlo Ersek, qemu-devel
Cc: Paolo Bonzini, Singh, Brijesh, Markus Armbruster, Erik Skultety
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.
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-12 7:44 ` Laszlo Ersek
0 siblings, 0 replies; 22+ messages in thread
From: Laszlo Ersek @ 2019-04-12 7:44 UTC (permalink / raw)
To: Singh, Brijesh, Erik Skultety
Cc: qemu-devel, Markus Armbruster, Paolo Bonzini
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.
>>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-12 7:44 ` Laszlo Ersek
0 siblings, 0 replies; 22+ messages in thread
From: Laszlo Ersek @ 2019-04-12 7:44 UTC (permalink / raw)
To: Singh, Brijesh, Erik Skultety
Cc: Paolo Bonzini, qemu-devel, Markus Armbruster
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.
>>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-12 7:45 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2019-04-12 7:45 UTC (permalink / raw)
To: Singh, Brijesh, Laszlo Ersek, qemu-devel; +Cc: Erik Skultety, Markus Armbruster
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-12 7:45 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2019-04-12 7:45 UTC (permalink / raw)
To: Singh, Brijesh, Laszlo Ersek, qemu-devel; +Cc: Markus Armbruster, Erik Skultety
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-12 8:19 ` Erik Skultety
0 siblings, 0 replies; 22+ messages in thread
From: Erik Skultety @ 2019-04-12 8:19 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Singh, Brijesh, Laszlo Ersek, qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-12 8:19 ` Erik Skultety
0 siblings, 0 replies; 22+ messages in thread
From: Erik Skultety @ 2019-04-12 8:19 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Laszlo Ersek, Singh, Brijesh, qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-12 8:26 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2019-04-12 8:26 UTC (permalink / raw)
To: Erik Skultety; +Cc: Singh, Brijesh, Laszlo Ersek, qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-12 8:26 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2019-04-12 8:26 UTC (permalink / raw)
To: Erik Skultety; +Cc: Laszlo Ersek, Singh, Brijesh, qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-12 9:13 ` Erik Skultety
0 siblings, 0 replies; 22+ messages in thread
From: Erik Skultety @ 2019-04-12 9:13 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Singh, Brijesh, Laszlo Ersek, qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] target/i386: sev: add 'sev-max-guests' field to 'query-sev-capabilities'
@ 2019-04-12 9:13 ` Erik Skultety
0 siblings, 0 replies; 22+ messages in thread
From: Erik Skultety @ 2019-04-12 9:13 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Laszlo Ersek, Singh, Brijesh, qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 22+ messages in thread