[libvirt] [PATCH 1/2] Add support for EOI with APIC
New options is added to support EOI (End of Interrupt) exposure for guests. As it makes sense only when APIC is enabled, I added this into the <apic> element in <features> because this should be tri-state option (cannot be handled as standalone feature). --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 9 ++++++++- src/conf/domain_conf.c | 35 ++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 11 +++++++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 62 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 503685f..66319d0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1018,6 +1018,13 @@ <dd>ACPI is useful for power management, for example, with KVM guests it is required for graceful shutdown to work. </dd> + <dt><code>apic</code></dt> + <dd>APIC allows the use of programmable IRQ + management. <span class="since">Since 0.10.2 (QEMU only)</span> + there is an optional attribute <code>eoi</code> with values "on" + and "off" which toggle the availability of EOI (End of + Interrupt) for the guest. + </dd> <dt><code>hap</code></dt> <dd>Enable use of Hardware Assisted Paging if available in the hardware. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c2c6184..029c796 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2859,7 +2859,14 @@ </optional> <optional> <element name="apic"> - <empty/> + <optional> + <attribute name="eoi"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </attribute> + </optional> </element> </optional> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 292cc9a..89c08da 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -110,6 +110,11 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "viridian", "privnet") +VIR_ENUM_IMPL(virDomainApicEoi, VIR_DOMAIN_APIC_EOI_LAST, + "default", + "on", + "off") + VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST, "destroy", "restart", @@ -8621,6 +8626,28 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; } def->features |= (1 << val); + if (val == VIR_DOMAIN_FEATURE_APIC) { + char *attrstr = NULL; + if (virAsprintf(&attrstr, + "string(./features/%s/@eoi)", + nodes[i]->name) < 0) + goto no_memory; + + tmp = virXPathString(attrstr, ctxt); + if (tmp) { + int eoi; + if ((eoi = virDomainApicEoiTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown value for attribute eoi: %s"), + nodes[i]->name); + VIR_FREE(tmp); + goto error; + } + def->apic_eoi = eoi; + VIR_FREE(tmp); + } + VIR_FREE(attrstr); + } } VIR_FREE(nodes); } @@ -13413,7 +13440,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, _("unexpected feature %d"), i); goto cleanup; } - virBufferAsprintf(buf, " <%s/>\n", name); + virBufferAsprintf(buf, " <%s", name); + if (i == VIR_DOMAIN_FEATURE_APIC && def->apic_eoi) { + virBufferAsprintf(buf, + " eoi='%s'", + virDomainApicEoiTypeToString(def->apic_eoi)); + } + virBufferAsprintf(buf, "/>\n"); } } virBufferAddLit(buf, " </features>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3995c2d..86dae7d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1351,6 +1351,14 @@ enum virDomainFeature { VIR_DOMAIN_FEATURE_LAST }; +enum virDomainApicEoi { + VIR_DOMAIN_APIC_EOI_DEFAULT = 0, + VIR_DOMAIN_APIC_EOI_ON, + VIR_DOMAIN_APIC_EOI_OFF, + + VIR_DOMAIN_APIC_EOI_LAST, +}; + enum virDomainLifecycleAction { VIR_DOMAIN_LIFECYCLE_DESTROY, VIR_DOMAIN_LIFECYCLE_RESTART, @@ -1642,6 +1650,8 @@ struct _virDomainDef { virDomainOSDef os; char *emulator; int features; + /* enum virDomainApicEoi */ + int apic_eoi; virDomainClockDef clock; @@ -2104,6 +2114,7 @@ VIR_ENUM_DECL(virDomainTaint) VIR_ENUM_DECL(virDomainVirt) VIR_ENUM_DECL(virDomainBoot) VIR_ENUM_DECL(virDomainFeature) +VIR_ENUM_DECL(virDomainApicEoi) VIR_ENUM_DECL(virDomainLifecycle) VIR_ENUM_DECL(virDomainLifecycleCrash) VIR_ENUM_DECL(virDomainPMState) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0494e1f..5dd9313 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -263,6 +263,8 @@ virBlkioDeviceWeightArrayClear; virDiskNameToBusDeviceIndex; virDiskNameToIndex; virDomainActualNetDefFree; +virDomainApicEoiTypeFromString; +virDomainApicEoiTypeToString; virDomainAssignDef; virDomainBlockedReasonTypeFromString; virDomainBlockedReasonTypeToString; -- 1.7.12
This patch adds full support for EOI setting for domains. Because this is CPU feature (flag), the model needs to be added even when it's not specified. Fortunately this problem was already solved with kvmclock, so this patch simply abuses that. And due to the size of the patch (17 lines) I dared to include the tests. --- src/qemu/qemu_command.c | 17 +++++++++++++ .../qemuxml2argv-cpu-eoi-disabled.args | 4 ++++ .../qemuxml2argv-cpu-eoi-disabled.xml | 28 ++++++++++++++++++++++ .../qemuxml2argv-cpu-eoi-enabled.args | 4 ++++ .../qemuxml2argv-cpu-eoi-enabled.xml | 28 ++++++++++++++++++++++ .../qemuxml2argv-eoi-disabled.args | 4 ++++ .../qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml | 25 +++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-eoi-enabled.args | 4 ++++ .../qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml | 25 +++++++++++++++++++ tests/qemuxml2argvtest.c | 5 ++++ tests/qemuxml2xmltest.c | 6 +++++ 11 files changed, 150 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cd4ee93..4aed8f6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4193,6 +4193,18 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, } } + if (def->apic_eoi) { + char sign; + if (def->apic_eoi == VIR_DOMAIN_APIC_EOI_ON) + sign = '+'; + else + sign = '-'; + + virBufferAsprintf(&buf, "%s,%ckvm_pv_eoi", + have_cpu ? "" : default_model, + sign); + } + if (virBufferError(&buf)) goto no_memory; @@ -7650,6 +7662,11 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, } dom->clock.timers[i]->present = present; ret = 0; + } else if (STREQ(feature, "kvm_pv_eoi")) { + if (policy == VIR_CPU_FEATURE_REQUIRE) + dom->apic_eoi = VIR_DOMAIN_APIC_EOI_ON; + else + dom->apic_eoi = VIR_DOMAIN_APIC_EOI_OFF; } else { if (!cpu) { if (!(cpu = qemuInitGuestCPU(dom))) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.args new file mode 100644 index 0000000..6d57f91 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +-cpu qemu32,-kvm_pv_eoi -m 214 -smp 6 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -boot n -net none -serial none \ +-parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.xml new file mode 100644 index 0000000..467df30 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.xml @@ -0,0 +1,28 @@ +<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/> + <apic eoi='off'/> + <pae/> + </features> + <cpu mode='custom' match='exact'> + <model fallback='allow'>qemu32</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/./qemu.sh</emulator> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.args new file mode 100644 index 0000000..3dc4310 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +-cpu qemu32,+kvm_pv_eoi -m 214 -smp 6 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -boot n -net none -serial none \ +-parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.xml new file mode 100644 index 0000000..1ed630a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.xml @@ -0,0 +1,28 @@ +<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/> + <apic eoi='on'/> + <pae/> + </features> + <cpu mode='custom' match='exact'> + <model fallback='allow'>qemu32</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/./qemu.sh</emulator> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.args b/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.args new file mode 100644 index 0000000..93475bd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +-cpu qemu32,-kvm_pv_eoi -m 214 -smp 6 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -boot n -net none -serial \ +none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml new file mode 100644 index 0000000..f84570e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml @@ -0,0 +1,25 @@ +<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/> + <apic eoi='off'/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/./qemu.sh</emulator> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.args b/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.args new file mode 100644 index 0000000..13f570b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test ./qemu.sh -S -M pc \ +-cpu qemu32,+kvm_pv_eoi -m 214 -smp 6 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -boot n -net none -serial \ +none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml new file mode 100644 index 0000000..03b6b52 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml @@ -0,0 +1,25 @@ +<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/> + <apic eoi='on'/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/./qemu.sh</emulator> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 47c3f6c..f0478b1 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -385,6 +385,11 @@ mymain(void) DO_TEST("cpu-host-kvmclock", QEMU_CAPS_ENABLE_KVM, QEMU_CAPS_CPU_HOST); DO_TEST("kvmclock", QEMU_CAPS_KVM); + DO_TEST("cpu-eoi-disabled", QEMU_CAPS_ENABLE_KVM); + DO_TEST("cpu-eoi-enabled", QEMU_CAPS_ENABLE_KVM); + DO_TEST("eoi-disabled", NONE); + DO_TEST("eoi-enabled", NONE); + DO_TEST("hugepages", QEMU_CAPS_MEM_PATH); DO_TEST("disk-cdrom", NONE); DO_TEST("disk-cdrom-empty", QEMU_CAPS_DRIVE); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 87d9e77..0a6da98 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -138,6 +138,12 @@ mymain(void) DO_TEST("cpu-kvmclock"); DO_TEST("cpu-host-kvmclock"); DO_TEST("kvmclock"); + + DO_TEST("cpu-eoi-disabled"); + DO_TEST("cpu-eoi-enabled"); + DO_TEST("eoi-disabled"); + DO_TEST("eoi-enabled"); + DO_TEST("hugepages"); DO_TEST("disk-aio"); DO_TEST("disk-cdrom"); -- 1.7.12
On 13.09.2012 16:12, Martin Kletzander wrote:
This patch adds full support for EOI setting for domains. Because this is CPU feature (flag), the model needs to be added even when it's not specified. Fortunately this problem was already solved with kvmclock, so this patch simply abuses that.
And due to the size of the patch (17 lines) I dared to include the tests. --- src/qemu/qemu_command.c | 17 +++++++++++++ .../qemuxml2argv-cpu-eoi-disabled.args | 4 ++++ .../qemuxml2argv-cpu-eoi-disabled.xml | 28 ++++++++++++++++++++++ .../qemuxml2argv-cpu-eoi-enabled.args | 4 ++++ .../qemuxml2argv-cpu-eoi-enabled.xml | 28 ++++++++++++++++++++++ .../qemuxml2argv-eoi-disabled.args | 4 ++++ .../qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml | 25 +++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-eoi-enabled.args | 4 ++++ .../qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml | 25 +++++++++++++++++++ tests/qemuxml2argvtest.c | 5 ++++ tests/qemuxml2xmltest.c | 6 +++++ 11 files changed, 150 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cd4ee93..4aed8f6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4193,6 +4193,18 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, } }
+ if (def->apic_eoi) { + char sign; + if (def->apic_eoi == VIR_DOMAIN_APIC_EOI_ON) + sign = '+'; + else + sign = '-'; + + virBufferAsprintf(&buf, "%s,%ckvm_pv_eoi", + have_cpu ? "" : default_model, + sign); + } + if (virBufferError(&buf)) goto no_memory;
@@ -7650,6 +7662,11 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, } dom->clock.timers[i]->present = present; ret = 0; + } else if (STREQ(feature, "kvm_pv_eoi")) { + if (policy == VIR_CPU_FEATURE_REQUIRE) + dom->apic_eoi = VIR_DOMAIN_APIC_EOI_ON; + else + dom->apic_eoi = VIR_DOMAIN_APIC_EOI_OFF;
Hey, this is nice. I feel like we are forgetting this part sometimes.
} else { if (!cpu) { if (!(cpu = qemuInitGuestCPU(dom)))
ACK Michal
On 09/13/2012 04:52 PM, Michal Privoznik wrote:
On 13.09.2012 16:12, Martin Kletzander wrote:
This patch adds full support for EOI setting for domains. Because this is CPU feature (flag), the model needs to be added even when it's not specified. Fortunately this problem was already solved with kvmclock, so this patch simply abuses that.
And due to the size of the patch (17 lines) I dared to include the tests. --- src/qemu/qemu_command.c | 17 +++++++++++++ .../qemuxml2argv-cpu-eoi-disabled.args | 4 ++++ .../qemuxml2argv-cpu-eoi-disabled.xml | 28 ++++++++++++++++++++++ .../qemuxml2argv-cpu-eoi-enabled.args | 4 ++++ .../qemuxml2argv-cpu-eoi-enabled.xml | 28 ++++++++++++++++++++++ .../qemuxml2argv-eoi-disabled.args | 4 ++++ .../qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml | 25 +++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-eoi-enabled.args | 4 ++++ .../qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml | 25 +++++++++++++++++++ tests/qemuxml2argvtest.c | 5 ++++ tests/qemuxml2xmltest.c | 6 +++++ 11 files changed, 150 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-disabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-eoi-enabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-disabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-eoi-enabled.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cd4ee93..4aed8f6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4193,6 +4193,18 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, } }
+ if (def->apic_eoi) { + char sign; + if (def->apic_eoi == VIR_DOMAIN_APIC_EOI_ON) + sign = '+'; + else + sign = '-'; + + virBufferAsprintf(&buf, "%s,%ckvm_pv_eoi", + have_cpu ? "" : default_model, + sign); + } + if (virBufferError(&buf)) goto no_memory;
@@ -7650,6 +7662,11 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, } dom->clock.timers[i]->present = present; ret = 0; + } else if (STREQ(feature, "kvm_pv_eoi")) { + if (policy == VIR_CPU_FEATURE_REQUIRE) + dom->apic_eoi = VIR_DOMAIN_APIC_EOI_ON; + else + dom->apic_eoi = VIR_DOMAIN_APIC_EOI_OFF;
Hey, this is nice. I feel like we are forgetting this part sometimes.
} else { if (!cpu) { if (!(cpu = qemuInitGuestCPU(dom)))
ACK
Michal
Thanks, pushed. Martin
On 13.09.2012 16:12, Martin Kletzander wrote:
New options is added to support EOI (End of Interrupt) exposure for guests. As it makes sense only when APIC is enabled, I added this into the <apic> element in <features> because this should be tri-state option (cannot be handled as standalone feature). --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 9 ++++++++- src/conf/domain_conf.c | 35 ++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 11 +++++++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 503685f..66319d0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1018,6 +1018,13 @@ <dd>ACPI is useful for power management, for example, with KVM guests it is required for graceful shutdown to work. </dd> + <dt><code>apic</code></dt> + <dd>APIC allows the use of programmable IRQ + management. <span class="since">Since 0.10.2 (QEMU only)</span> + there is an optional attribute <code>eoi</code> with values "on" + and "off" which toggle the availability of EOI (End of
s/toggle/toggles/
+ Interrupt) for the guest. + </dd> <dt><code>hap</code></dt> <dd>Enable use of Hardware Assisted Paging if available in the hardware.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 292cc9a..89c08da 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -110,6 +110,11 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "viridian", "privnet")
+VIR_ENUM_IMPL(virDomainApicEoi, VIR_DOMAIN_APIC_EOI_LAST, + "default", + "on", + "off") + VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST, "destroy", "restart", @@ -8621,6 +8626,28 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; } def->features |= (1 << val); + if (val == VIR_DOMAIN_FEATURE_APIC) { + char *attrstr = NULL; + if (virAsprintf(&attrstr, + "string(./features/%s/@eoi)", + nodes[i]->name) < 0) + goto no_memory; +
Why do we want runtime constructed string esp. if it is static one?
+ tmp = virXPathString(attrstr, ctxt); + if (tmp) { + int eoi; + if ((eoi = virDomainApicEoiTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown value for attribute eoi: %s"), + nodes[i]->name);
s/nodes[i]->name/tmp/
+ VIR_FREE(tmp); + goto error; + } + def->apic_eoi = eoi; + VIR_FREE(tmp); + } + VIR_FREE(attrstr); + } } VIR_FREE(nodes); }
ACK with this squashed in: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d13f671..16afb15 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8846,26 +8846,19 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } def->features |= (1 << val); if (val == VIR_DOMAIN_FEATURE_APIC) { - char *attrstr = NULL; - if (virAsprintf(&attrstr, - "string(./features/%s/@eoi)", - nodes[i]->name) < 0) - goto no_memory; - - tmp = virXPathString(attrstr, ctxt); + tmp = virXPathString("string(./features/apic/@eoi)", ctxt); if (tmp) { int eoi; - if ((eoi = virDomainApicEoiTypeFromString(tmp)) < 0) { + if ((eoi = virDomainApicEoiTypeFromString(tmp)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown value for attribute eoi: %s"), - nodes[i]->name); + tmp); VIR_FREE(tmp); goto error; } def->apic_eoi = eoi; VIR_FREE(tmp); } - VIR_FREE(attrstr); } } VIR_FREE(nodes); Michal
On 09/13/2012 04:52 PM, Michal Privoznik wrote:
On 13.09.2012 16:12, Martin Kletzander wrote:
New options is added to support EOI (End of Interrupt) exposure for guests. As it makes sense only when APIC is enabled, I added this into the <apic> element in <features> because this should be tri-state option (cannot be handled as standalone feature). --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 9 ++++++++- src/conf/domain_conf.c | 35 ++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 11 +++++++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 503685f..66319d0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1018,6 +1018,13 @@ <dd>ACPI is useful for power management, for example, with KVM guests it is required for graceful shutdown to work. </dd> + <dt><code>apic</code></dt> + <dd>APIC allows the use of programmable IRQ + management. <span class="since">Since 0.10.2 (QEMU only)</span> + there is an optional attribute <code>eoi</code> with values "on" + and "off" which toggle the availability of EOI (End of
s/toggle/toggles/
+ Interrupt) for the guest. + </dd> <dt><code>hap</code></dt> <dd>Enable use of Hardware Assisted Paging if available in the hardware.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 292cc9a..89c08da 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -110,6 +110,11 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "viridian", "privnet")
+VIR_ENUM_IMPL(virDomainApicEoi, VIR_DOMAIN_APIC_EOI_LAST, + "default", + "on", + "off") + VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST, "destroy", "restart", @@ -8621,6 +8626,28 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; } def->features |= (1 << val); + if (val == VIR_DOMAIN_FEATURE_APIC) { + char *attrstr = NULL; + if (virAsprintf(&attrstr, + "string(./features/%s/@eoi)", + nodes[i]->name) < 0) + goto no_memory; +
Why do we want runtime constructed string esp. if it is static one?
+ tmp = virXPathString(attrstr, ctxt); + if (tmp) { + int eoi; + if ((eoi = virDomainApicEoiTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown value for attribute eoi: %s"), + nodes[i]->name);
s/nodes[i]->name/tmp/
+ VIR_FREE(tmp); + goto error; + } + def->apic_eoi = eoi; + VIR_FREE(tmp); + } + VIR_FREE(attrstr); + } } VIR_FREE(nodes); }
ACK with this squashed in:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d13f671..16afb15 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8846,26 +8846,19 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } def->features |= (1 << val); if (val == VIR_DOMAIN_FEATURE_APIC) { - char *attrstr = NULL; - if (virAsprintf(&attrstr, - "string(./features/%s/@eoi)", - nodes[i]->name) < 0) - goto no_memory; - - tmp = virXPathString(attrstr, ctxt); + tmp = virXPathString("string(./features/apic/@eoi)", ctxt); if (tmp) { int eoi; - if ((eoi = virDomainApicEoiTypeFromString(tmp)) < 0) { + if ((eoi = virDomainApicEoiTypeFromString(tmp)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown value for attribute eoi: %s"), - nodes[i]->name); + tmp); VIR_FREE(tmp); goto error; } def->apic_eoi = eoi; VIR_FREE(tmp); } - VIR_FREE(attrstr); } } VIR_FREE(nodes);
Michal
Thanks guys, fushed with the fixes, Martin
On Thu, Sep 13, 2012 at 04:12:01PM +0200, Martin Kletzander wrote:
New options is added to support EOI (End of Interrupt) exposure for guests. As it makes sense only when APIC is enabled, I added this into the <apic> element in <features> because this should be tri-state option (cannot be handled as standalone feature). --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 9 ++++++++- src/conf/domain_conf.c | 35 ++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 11 +++++++++++ src/libvirt_private.syms | 2 ++ 5 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 503685f..66319d0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1018,6 +1018,13 @@ <dd>ACPI is useful for power management, for example, with KVM guests it is required for graceful shutdown to work. </dd> + <dt><code>apic</code></dt> + <dd>APIC allows the use of programmable IRQ + management. <span class="since">Since 0.10.2 (QEMU only)</span> + there is an optional attribute <code>eoi</code> with values "on" + and "off" which toggle the availability of EOI (End of + Interrupt) for the guest. + </dd> <dt><code>hap</code></dt> <dd>Enable use of Hardware Assisted Paging if available in the hardware. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c2c6184..029c796 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2859,7 +2859,14 @@ </optional> <optional> <element name="apic"> - <empty/> + <optional> + <attribute name="eoi"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </attribute> + </optional> </element> </optional> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 292cc9a..89c08da 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -110,6 +110,11 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "viridian", "privnet")
+VIR_ENUM_IMPL(virDomainApicEoi, VIR_DOMAIN_APIC_EOI_LAST, + "default", + "on", + "off") + VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST, "destroy", "restart", @@ -8621,6 +8626,28 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; } def->features |= (1 << val); + if (val == VIR_DOMAIN_FEATURE_APIC) { + char *attrstr = NULL; + if (virAsprintf(&attrstr, + "string(./features/%s/@eoi)", + nodes[i]->name) < 0) + goto no_memory; + + tmp = virXPathString(attrstr, ctxt); + if (tmp) { + int eoi; + if ((eoi = virDomainApicEoiTypeFromString(tmp)) < 0) {
actually the test should be <= 0, because 0 maps to "default" which is not an external value, and not present in the RNG
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown value for attribute eoi: %s"), + nodes[i]->name); + VIR_FREE(tmp); + goto error; + } + def->apic_eoi = eoi; + VIR_FREE(tmp); + } + VIR_FREE(attrstr); + } } VIR_FREE(nodes); } @@ -13413,7 +13440,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, _("unexpected feature %d"), i); goto cleanup; } - virBufferAsprintf(buf, " <%s/>\n", name); + virBufferAsprintf(buf, " <%s", name); + if (i == VIR_DOMAIN_FEATURE_APIC && def->apic_eoi) { + virBufferAsprintf(buf, + " eoi='%s'", + virDomainApicEoiTypeToString(def->apic_eoi)); + } + virBufferAsprintf(buf, "/>\n"); } } virBufferAddLit(buf, " </features>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3995c2d..86dae7d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1351,6 +1351,14 @@ enum virDomainFeature { VIR_DOMAIN_FEATURE_LAST };
+enum virDomainApicEoi { + VIR_DOMAIN_APIC_EOI_DEFAULT = 0, + VIR_DOMAIN_APIC_EOI_ON, + VIR_DOMAIN_APIC_EOI_OFF, + + VIR_DOMAIN_APIC_EOI_LAST, +}; + enum virDomainLifecycleAction { VIR_DOMAIN_LIFECYCLE_DESTROY, VIR_DOMAIN_LIFECYCLE_RESTART, @@ -1642,6 +1650,8 @@ struct _virDomainDef { virDomainOSDef os; char *emulator; int features; + /* enum virDomainApicEoi */ + int apic_eoi;
virDomainClockDef clock;
@@ -2104,6 +2114,7 @@ VIR_ENUM_DECL(virDomainTaint) VIR_ENUM_DECL(virDomainVirt) VIR_ENUM_DECL(virDomainBoot) VIR_ENUM_DECL(virDomainFeature) +VIR_ENUM_DECL(virDomainApicEoi) VIR_ENUM_DECL(virDomainLifecycle) VIR_ENUM_DECL(virDomainLifecycleCrash) VIR_ENUM_DECL(virDomainPMState) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0494e1f..5dd9313 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -263,6 +263,8 @@ virBlkioDeviceWeightArrayClear; virDiskNameToBusDeviceIndex; virDiskNameToIndex; virDomainActualNetDefFree; +virDomainApicEoiTypeFromString; +virDomainApicEoiTypeToString; virDomainAssignDef; virDomainBlockedReasonTypeFromString; virDomainBlockedReasonTypeToString;
Except for that small glitch that looks good to me, ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel Veillard -
Martin Kletzander -
Michal Privoznik