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