[libvirt] [PATCH 3/3] qemu: Add support for guest CPU cache

[Jiri Denemark]

On Thu, Apr 27, 2017 at 17:09:41 +0200, Peter Krempa wrote:
> On Tue, Apr 25, 2017 at 20:48:45 +0200, Jiri Denemark wrote:
> > This patch maps /domain/cpu/cache element into -cpu parameters:
> > 
> > - <cache mode='passthrough'/> is translated to host-cache-info=on
> > - <cache level='3' mode='emulate'/> is transformed into l3-cache=on
> > - <cache mode='disable'/> is turned in host-cache-info=off,l3-cache=off
> 
> host-cache-info=on,l3-cache=off is specifically invalid from qemu's
> point of view?

No. Libvirt will automatically add l3-cache=off to host-cache-info=on
and host-cache-info=off to l3-cache=on if it detects QEMU supports these
properties. Otherwise it will just blindly add the one explicitly
requested and let QEMU fail if it doesn't support it.

> > Any other <cache> element is forbidden.
> > 
> > The tricky part is detecting whether QEMU supports the CPU properties.
> > 
> > The 'host-cache-info' property is introduced in v2.4.0-1389-ge265e3e480,
> > earlier QEMU releases enabled host-cache-info by default and had no way
> > to disable it. If the property is present, it defaults to 'off' for any
> > QEMU until at least 2.9.0.
> > 
> > The 'l3-cache' property was introduced later by v2.7.0-200-g14c985cffa.
> > Earlier versions worked as if l3-cache=off was passed. For any QEMU
> > until at least 2.9.0 l3-cache is 'off' by default.
> > 
> > QEMU 2.9.0 was the first release which supports probing both properties
> > by running device-list-properties with typename=host-x86_64-cpu. Older
> > QEMU releases did not support device-list-properties command for CPU
> > devices. Thus we can't really rely on probing them and we can just use
> > query-cpu-model-expansion QMP command as a witness.
> > 
> > Because the cache property probing is only reliable for QEMU >= 2.9.0
> > when both are already supported for quite a few releases, we let QEMU
> > report an error if a specific cache mode is explicitly requested. Thea
> > other mode (or both if a user requested CPU cache to be disabled) is
> > explicitly turned off for QEMU >= 2.9.0 to avoid any surprises in case
> > the QEMU defaults change. Any older QEMU already turns them off so not
> > doing so explicitly does not make any harm.
> > 
> > Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> > ---
> 
> [...]
> 
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index cc3e1f808..5dd18f3c5 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -4787,6 +4789,12 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
> >          virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM))
> >          virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM);
> >  
> > +    if (ARCH_IS_X86(qemuCaps->arch) &&
> > +        virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) {
> > +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_HOST_CACHE);
> > +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_L3_CACHE);
> 
> Is it really necessary to add both of them separately even if you set
> them both at once?

Hmm, I guess QEMU_CAPS_CPU_CACHE would be enough.

> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-cache-passthrough2.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-cache-passthrough2.args
> > new file mode 100644
> > index 000000000..cf4c55ca3
> > --- /dev/null
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-cache-passthrough2.args
> > @@ -0,0 +1,21 @@
> > +LC_ALL=C \
> > +PATH=/bin \
> > +HOME=/home/test \
> > +USER=test \
> > +LOGNAME=test \
> > +QEMU_AUDIO_DRV=none \
> > +/usr/bin/qemu-system-x86_64 \
> > +-name foo \
> > +-S \
> > +-M pc \
> > +-cpu host,host-cache-info=on \
> 
> This is impossible in real life since QEMU_CAPS_CPU_HOST_CACHE and
> QEMU_CAPS_CPU_L3_CACHE are always defined together.

$cache=on is always added even if QEMU_CAPS_CPU_CACHE capability is not
set. Only the $cache=off parameters are influenced by the capability. In
other words, this is a real world command line generated for QEMU older
than 2.9.0.

> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> > index c1b014b7d..15765fdfd 100644
> > --- a/tests/qemuxml2argvtest.c
> > +++ b/tests/qemuxml2argvtest.c
> > @@ -2513,6 +2513,22 @@ mymain(void)
> >      DO_TEST("cpu-check-default-partial", QEMU_CAPS_KVM);
> >      DO_TEST("cpu-check-default-partial2", QEMU_CAPS_KVM);
> >  
> > +    DO_TEST("cpu-cache-disable",
> > +            QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST_CACHE, QEMU_CAPS_CPU_L3_CACHE);
> > +    DO_TEST("cpu-cache-disable2",
> > +            QEMU_CAPS_KVM);
> > +    DO_TEST("cpu-cache-disable3",
> > +            QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST_CACHE, QEMU_CAPS_CPU_L3_CACHE);
> > +    DO_TEST("cpu-cache-passthrough",
> > +            QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST_CACHE, QEMU_CAPS_CPU_L3_CACHE);
> > +    DO_TEST("cpu-cache-passthrough2",
> > +            QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST_CACHE);
> 
> This test is misleading, you don't get a config like this with this
> code.

Oh right, only QEMU_CAPS_KVM should be set for this test.

> 
> > +    DO_TEST("cpu-cache-emulate-l3",
> > +            QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST_CACHE, QEMU_CAPS_CPU_L3_CACHE);
> > +    DO_TEST_PARSE_ERROR("cpu-cache-emulate-l2", QEMU_CAPS_KVM);
> > +    DO_TEST_PARSE_ERROR("cpu-cache-passthrough3", QEMU_CAPS_KVM);
> > +    DO_TEST_PARSE_ERROR("cpu-cache-passthrough-l3", QEMU_CAPS_KVM);
> > +
> >      qemuTestDriverFree(&driver);
> >  
> >      return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> 
> ACK with the above things sorted out.

Pushed with all the things sorted out. Thanks.

Jirka