[libvirt PATCH v3 10/13] qemu: report max number of SEV guests

[Daniel P. Berrangé]

On Tue, Dec 14, 2021 at 11:44:00AM +0100, Peter Krempa wrote:
> On Fri, Dec 10, 2021 at 16:47:10 +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                  | 39 +++++++++++++++++++
> >  src/qemu/qemu_driver.c                        | 10 +++++
> >  .../domaincapsdata/qemu_2.12.0-q35.x86_64.xml |  4 +-
> >  .../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml |  4 +-
> >  tests/domaincapsdata/qemu_2.12.0.x86_64.xml   |  4 +-
> >  .../domaincapsdata/qemu_6.0.0-q35.x86_64.xml  |  4 +-
> >  .../domaincapsdata/qemu_6.0.0-tcg.x86_64.xml  |  4 +-
> >  tests/domaincapsdata/qemu_6.0.0.x86_64.xml    |  4 +-
> >  tests/testutilsqemu.c                         | 21 ++++++++++
> >  9 files changed, 82 insertions(+), 12 deletions(-)
> 
> [...]
> 
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index ee23e10543..8ee0939295 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -19918,6 +19918,16 @@ qemuGetSEVInfoToParams(virQEMUCaps *qemuCaps,
> >                      sev->reduced_phys_bits) < 0)
> >          goto cleanup;
> >  
> > +    if (virTypedParamsAddUInt(&sevParams, &n, &maxpar,
> > +                    VIR_NODE_SEV_MAX_GUESTS,
> > +                    sev->max_guests) < 0)
> > +        goto cleanup;
> > +
> > +    if (virTypedParamsAddUInt(&sevParams, &n, &maxpar,
> > +                    VIR_NODE_SEV_MAX_ES_GUESTS,
> > +                    sev->max_es_guests) < 0)
> > +        goto cleanup;
> 
> Both calls have broken alignment.

This is consistent with the alignment of all existing code
in this method.


> > diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
> > index 5bd1d40ad4..7f848f158e 100644
> > --- a/tests/testutilsqemu.c
> > +++ b/tests/testutilsqemu.c
> > @@ -143,6 +143,27 @@ virCapabilitiesHostNUMANewHost(void)
> >      return virTestCapsBuildNUMATopology(3);
> >  }
> >  
> > +void
> 
> This form of overriding functions looked a bit unorthodox but prior art
> is right above, so it's okay.

It is basically relying on the linker method resolution ordering
to have same effect as LD_PRELOAD, without having to jump through
the hoops of creating a preload .so library.

> 
> > +virHostCPUX86GetCPUID(uint32_t leaf,
> > +                      uint32_t extended,
> > +                      uint32_t *eax,
> > +                      uint32_t *ebx,
> > +                      uint32_t *ecx,
> > +                      uint32_t *edx)
> > +{
> > +    if (eax)
> > +        *eax = 0;
> > +    if (ebx)
> > +        *ebx = 0;
> > +    if (ecx)
> > +        *ecx = 0;
> > +    if (edx)
> > +        *edx = 0;
> > +    if (leaf == 0x8000001F && extended == 0) {
> > +        *ecx = 509;
> > +        *edx = 451;
> 
> ecx/edx are unconditionally dereferenced here. Okay at this point but
> possibly unextensible. Consider adding pointer checks at least to avoid
> coverity moaning.

Hmm, yes will do.

> Reviewed-by: Peter Krempa <pkrempa at redhat.com>

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