On Fri, Jun 19, 2020 at 06:04:33PM -0300, Daniel Henrique Barboza wrote: > PPC64 has two KVM modules: kvm_hv and kvm_pr. The official supported > module was always kvm_hv, while kvm_pr was used for internal testing > or for very niche cases in Power 8 hosts, always without official > IBM or distro support. > > Problem is, QMP will report KVM supportfor PPC64 if any of these > modules is loaded in the host, and kvm_pr is broken in everything > but Power8 (and will remain broken, since kvm_pr is unmaintained). > This can lead to situations like [1], where the tooling is misled to > believe that the host has KVM capabilities when in reality it > doesn't. > > The first reaction would be to simply forsake kvm_pr support entirely > and move on, but there is no reason for now to be disruptive with any > Power8 guests in the wild that are using kvm_pr (somehow). A more > subtle approach is to not claim QEMU_CAPS_KVM support in all cases > that we know it's completely broken, allowing Power8 users to take > their shot using kvm_pr in their VMs. We can remove kvm_pr support > completely when the module is removed from the kernel. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1843865 I'm sorry I don't have a way to test this patch. However it looks reasonable from here based on the description above and the changes to the code below, so ACK. Rich. > CC: Leonardo Augusto Guimarães Garcia <lagarcia@xxxxxxxxxx> > CC: Greg Kurz <groug@xxxxxxxx> > CC: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> > CC: Richard W.M. Jones <rjones@xxxxxxxxxx> > Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx> > --- > src/qemu/qemu_capabilities.c | 38 ++++++++++++++++++++++++++++++++++-- > 1 file changed, 36 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 484fff99e5..b1c1d4dd70 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -49,6 +49,7 @@ > #include "qemu_process.h" > #include "qemu_firmware.h" > #include "virutil.h" > +#include "virkmod.h" > > #include <fcntl.h> > #include <sys/stat.h> > @@ -3242,6 +3243,31 @@ virQEMUCapsProbeQMPTPM(virQEMUCapsPtr qemuCaps, > } > > > +static void > +virQEMUCapsSetPPC64KVMState(virQEMUCapsPtr qemuCaps, virArch hostArch) > +{ > + g_autoptr(virCPUDef) hostCPU = virCPUProbeHost(hostArch); > + > + /* At this moment, PPC64 has two KVM modules: kvm_hv and kvm_pr. > + * QEMU will return KVM present and enabled = true if any of these > + * is loaded in the host. Thing is, kvm_pr was never officially > + * supported by IBM or any other distro, but people still ended > + * up using it in Power8 hosts regardless. It is also currently > + * unmaintained and broken on Power9, and will be sunset in the > + * future. kvm_hv is the only KVM module that is and will be > + * supported. > + * > + * Until then, do not claim QEMU_CAPS_KVM if there is only kvm_pr > + * loaded in the host. There is a good chance that there are > + * unsupported kvm_pr Power8 guests running in the wild, so let's > + * cut some slack for this case, for now. */ > + if (STRNEQLEN(hostCPU->model, "POWER8", 6) && !virKModIsLoaded("kvm_hv")) > + return; > + > + virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM); > +} > + > + > static int > virQEMUCapsProbeQMPKVMState(virQEMUCapsPtr qemuCaps, > qemuMonitorPtr mon) > @@ -3252,8 +3278,16 @@ virQEMUCapsProbeQMPKVMState(virQEMUCapsPtr qemuCaps, > if (qemuMonitorGetKVMState(mon, &enabled, &present) < 0) > return -1; > > - if (present && enabled) > - virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM); > + if (present && enabled) { > + virArch hostArch = virArchFromHost(); > + > + if (ARCH_IS_PPC64(hostArch)) { > + virQEMUCapsSetPPC64KVMState(qemuCaps, hostArch); > + return 0; > + } > + > + virQEMUCapsSet(qemuCaps, QEMU_CAPS_KVM); > + } > > return 0; > } > -- > 2.26.2 -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
