On Fri, Dec 10, 2021 at 11:37:32 +0000, Daniel P. Berrangé wrote:
> Different CPU generations have different limits on the number
> of SEV/SEV-ES guests that can be run. Since both limits come
> from the same overall set, there is typically also BIOS config
> to set the tradeoff betweeen SEV and SEV-ES guest limits.
>
> This is important information to expose for a mgmt application
> scheduling guests to hosts.
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
> src/qemu/qemu_capabilities.c | 38 +++++++++++++++++++
> src/qemu/qemu_driver.c | 10 +++++
> .../domaincapsdata/qemu_2.12.0-q35.x86_64.xml | 2 +-
> .../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 2 +-
> tests/domaincapsdata/qemu_2.12.0.x86_64.xml | 2 +-
> .../domaincapsdata/qemu_6.0.0-q35.x86_64.xml | 2 +-
> .../domaincapsdata/qemu_6.0.0-tcg.x86_64.xml | 2 +-
> tests/domaincapsdata/qemu_6.0.0.x86_64.xml | 2 +-
> 8 files changed, 54 insertions(+), 6 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 4ffd0a98a2..456ce1b72e 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -1897,6 +1897,8 @@ virQEMUCapsSEVInfoCopy(virSEVCapability **dst,
>
> tmp->cbitpos = src->cbitpos;
> tmp->reduced_phys_bits = src->reduced_phys_bits;
> + tmp->max_guests = src->max_guests;
> + tmp->max_es_guests = src->max_es_guests;
>
> *dst = g_steal_pointer(&tmp);
> return 0;
> @@ -3286,6 +3288,30 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCaps *qemuCaps,
> }
>
>
> +static void
> +virQEMUCapsGetSEVMaxGuests(virSEVCapability *caps)
> +{
> +# if __x86_64__
cppi: /home/pipo/libvirt/src/qemu/qemu_capabilities.c: line 3294: not properly indented
> + uint32_t eax, ebx, ecx, edx;
> + asm("xor %%ebx, %%ebx;" /* clear the other registers as some cpuid */
> + "xor %%edx, %%edx;" /* functions may use them as additional arguments */
> + "cpuid;"
> + : "=a" (eax),
> + "=b" (ebx),
Since the code doesn't use 'eax' and 'ebx' you can remove it and add it
to the clobbers section along with "cc".
> + "=c" (ecx),
> + "=d" (edx)
> + : "0" (0x8000001F),
> + "c" (0)
> + : "cc");
> +
A comment what the data in 'ecx' and 'edx' is would be helpful even when
it's obvious on a second look
> + caps->max_guests = ecx - edx + 1;
> + caps->max_es_guests = edx - 1;
> +# else
> + VIR_WARN("Unexpectedly asked for SEV guest count on non-x86_64 arch");
> + caps->max_guests = caps->max_es_guests = 0;
> +# endif
> +}
> +
> static int
> virQEMUCapsProbeQMPSEVCapabilities(virQEMUCaps *qemuCaps,
> qemuMonitor *mon)
> @@ -3305,6 +3331,8 @@ virQEMUCapsProbeQMPSEVCapabilities(virQEMUCaps *qemuCaps,
> return 0;
> }
>
> + virQEMUCapsGetSEVMaxGuests(caps);
This code ...
> +
> virSEVCapabilitiesFree(qemuCaps->sevCapabilities);
> qemuCaps->sevCapabilities = caps;
> return 0;
> @@ -4084,6 +4112,14 @@ virQEMUCapsParseSEVInfo(virQEMUCaps *qemuCaps, xmlXPathContextPtr ctxt)
> return -1;
> }
>
> +
> + /* We probe this every time because the values
> + * can change on every reboot via firmware
> + * config tunables. It is cheap to query so
> + * lack of caching is a non-issue
> + */
> + virQEMUCapsGetSEVMaxGuests(sev);
... and this code is executed also in context of e.g.
qemudomaincapstest, so the results are changing based on where you run
it, e.g. as on my box:
- <maxGuests>15</maxGuests>
+ <maxGuests>509</maxGuests>
So this method definitely is not suitable for implementation as in this
patch.
Since we already have code digging trhough 'cpuid' it feels that it
should be a better place to do so.
