On Tue, Sep 24, 2013 at 3:43 AM, Peter Krempa <pkrempa@xxxxxxxxxx> wrote: > The linux kernel recently added support for paravirtual spinlock > handling to avoid performance regressions on overcomitted hosts. This > feature needs to be turned in the hypervisor so that the guest OS is > notified about the possible support. > > This patch adds a new feature "paravirt-spinlock" to the XML and > supporting code to enable the "kvm_pv_unhalt" pseudoCPU feature in qemu. > > https://bugzilla.redhat.com/show_bug.cgi?id=1008989 > --- > This patch is RFC as qemu didn't add this to the upstream repo yet: > > Pull request: > http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg03385.html > > Original thread: > http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg03095.html > > I'm sending it to get review feedback but will push it only after qemu will > have it. > > > docs/formatdomain.html.in | 7 ++++++ > docs/schemas/domaincommon.rng | 10 ++++++++- > src/conf/domain_conf.c | 20 ++++++++++++++++- > src/conf/domain_conf.h | 1 + > src/qemu/qemu_command.c | 14 ++++++++++++ > .../qemuxml2argv-pv-spinlock-disabled.args | 5 +++++ > .../qemuxml2argv-pv-spinlock-disabled.xml | 26 ++++++++++++++++++++++ > .../qemuxml2argv-pv-spinlock-enabled.args | 5 +++++ > .../qemuxml2argv-pv-spinlock-enabled.xml | 26 ++++++++++++++++++++++ > tests/qemuxml2argvtest.c | 2 ++ > tests/qemuxml2xmltest.c | 2 ++ > 11 files changed, 116 insertions(+), 2 deletions(-) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 3689399..aa0076c 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -1182,6 +1182,7 @@ > <vapic state='on'/> > <spinlocks state='on' retries='4096'/> > </hyperv> > + <paravirt-spinlock/> > > </features> > ...</pre> > @@ -1253,6 +1254,12 @@ > </tr> > </table> > </dd> > + <dt><code>paravirtual-spinlock</code></dt> You call it paravirt-spinlock everywhere else so this looks like a typo. > + <dd>Notify the guest that the host supports paravirtual spinlocks > + for example by exposing the pvticketlocks mechanism. This feature > + can be forced of by using a "state='off'" attribute. > + </dd> > + > </dl> > > <h3><a name="elementsTime">Time keeping</a></h3> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 94a72f8..4c494cb 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -3560,7 +3560,7 @@ > </define> > <!-- > A set of optional features: PAE, APIC, ACPI, > - HyperV Enlightenment and HAP support > + HyperV Enlightenment, paravirtual spinlocks and HAP support > --> > <define name="features"> > <optional> > @@ -3606,6 +3606,14 @@ > <empty/> > </element> > </optional> > + <optional> > + <element name="paravirt-spinlock"> > + <optional> > + <ref name="featurestate"/> > + </optional> > + <empty/> > + </element> > + </optional> > </interleave> > </element> > </optional> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index e1a1d6d..919adc1 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -142,7 +142,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, > "hap", > "viridian", > "privnet", > - "hyperv") > + "hyperv", > + "paravirt-spinlock") > > VIR_ENUM_IMPL(virDomainFeatureState, VIR_DOMAIN_FEATURE_STATE_LAST, > "default", > @@ -11394,6 +11395,22 @@ virDomainDefParseXML(xmlDocPtr xml, > def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON; > break; > > + case VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK: > + node = ctxt->node; > + ctxt->node = nodes[i]; > + if ((tmp = virXPathString("string(./@state)", ctxt))) { > + if ((def->features[val] = virDomainFeatureStateTypeFromString(tmp)) == -1) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("unknown state atribute '%s' of feature '%s'"), > + tmp, virDomainFeatureTypeToString(val)); > + goto error; > + } > + } else { > + def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON; > + } > + ctxt->node = node; > + break; > + > case VIR_DOMAIN_FEATURE_LAST: > break; > } > @@ -16742,6 +16759,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, > case VIR_DOMAIN_FEATURE_HAP: > case VIR_DOMAIN_FEATURE_VIRIDIAN: > case VIR_DOMAIN_FEATURE_PRIVNET: > + case VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK: > switch ((enum virDomainFeatureState) def->features[i]) { > case VIR_DOMAIN_FEATURE_STATE_ON: > /* output just the element */ > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 9ddd706..4d69c4f 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -1618,6 +1618,7 @@ enum virDomainFeature { > VIR_DOMAIN_FEATURE_VIRIDIAN, > VIR_DOMAIN_FEATURE_PRIVNET, > VIR_DOMAIN_FEATURE_HYPERV, > + VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK, > > VIR_DOMAIN_FEATURE_LAST > }; > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 6f3b3cb..01de440 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -6608,6 +6608,20 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, > have_cpu = true; > } > > + if (def->features[VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK]) { > + char sign; > + if (def->features[VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK] == > + VIR_DOMAIN_FEATURE_STATE_ON) > + sign = '+'; > + else > + sign = '-'; > + > + virBufferAsprintf(&buf, "%s,%ckvm_pv_unhalt", > + have_cpu ? "" : default_model, > + sign); > + have_cpu = true; > + } > + > if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_DOMAIN_FEATURE_STATE_ON) { > if (!have_cpu) { > virBufferAdd(&buf, default_model, -1); > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args > new file mode 100644 > index 0000000..80047f9 > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args > @@ -0,0 +1,5 @@ > +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ > +/usr/bin/qemu -S -M pc \ > +-cpu qemu32,-kvm_pv_unhalt -m 214 -smp 6 -nographic -monitor \ > +unix:/tmp/test-monitor,server,nowait -boot n -usb -net none -serial \ > +none -parallel none > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml > new file mode 100644 > index 0000000..afb0b41 > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml > @@ -0,0 +1,26 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219100</memory> > + <currentMemory unit='KiB'>219100</currentMemory> > + <vcpu placement='static'>6</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='network'/> > + </os> > + <features> > + <acpi/> > + <pae/> > + <paravirt-spinlock state='off'/> > + </features> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu</emulator> > + <controller type='usb' index='0'/> > + <controller type='pci' index='0' model='pci-root'/> > + <memballoon model='virtio'/> > + </devices> > +</domain> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args > new file mode 100644 > index 0000000..70db173 > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args > @@ -0,0 +1,5 @@ > +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ > +/usr/bin/qemu -S -M pc \ > +-cpu qemu32,+kvm_pv_unhalt -m 214 -smp 6 -nographic -monitor \ > +unix:/tmp/test-monitor,server,nowait -boot n -usb -net none -serial \ > +none -parallel none > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml > new file mode 100644 > index 0000000..f29cade > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml > @@ -0,0 +1,26 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>219100</memory> > + <currentMemory unit='KiB'>219100</currentMemory> > + <vcpu placement='static'>6</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='network'/> > + </os> > + <features> > + <acpi/> > + <pae/> > + <paravirt-spinlock/> > + </features> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu</emulator> > + <controller type='usb' index='0'/> > + <controller type='pci' index='0' model='pci-root'/> > + <memballoon model='virtio'/> > + </devices> > +</domain> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index ec4a6e5..72b6b2a 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -435,6 +435,8 @@ mymain(void) > QEMU_CAPS_CHARDEV_SPICEVMC, QEMU_CAPS_SPICE, QEMU_CAPS_HDA_DUPLEX); > DO_TEST("eoi-disabled", NONE); > DO_TEST("eoi-enabled", NONE); > + DO_TEST("pv-spinlock-disabled", NONE); > + DO_TEST("pv-spinlock-enabled", NONE); > DO_TEST("kvmclock+eoi-disabled", QEMU_CAPS_ENABLE_KVM); > > DO_TEST("hyperv", NONE); > diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c > index c661573..0e83dcf 100644 > --- a/tests/qemuxml2xmltest.c > +++ b/tests/qemuxml2xmltest.c > @@ -156,6 +156,8 @@ mymain(void) > DO_TEST("cpu-eoi-enabled"); > DO_TEST("eoi-disabled"); > DO_TEST("eoi-enabled"); > + DO_TEST("pv-spinlock-disabled"); > + DO_TEST("pv-spinlock-enabled"); > > DO_TEST("hyperv"); > DO_TEST("hyperv-off"); > -- > 1.8.3.2 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list Just one typo that in addition to Jan's review. -- Doug Goldstein -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list