[libvirt PATCH v2 09/12] qemu: report max number of SEV guests

[Peter Krempa]

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 at redhat.com>
> ---
>  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.