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

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(a)redhat.com&gt; > --- > 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.

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

> +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.

Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;