[libvirt PATCH v2 0/8] qemu: Add support for the armvtimer timer

This new timer model can be used to control the behavior of the virtual timer for KVM ARM/virt guests. Changes from [v1]: * redesign the XML interface completely, notably moving the configuration knob from <cpu> to <clock>. [v1] https://www.redhat.com/archives/libvir-list/2020-January/msg01475.html Andrea Bolognani (8): qemu: Use switch statement in qemuBuildCpuCommandLine() qemu: Add the QEMU_CAPS_CPU_KVM_NO_ADJVTIME capability conf: Introduce VIR_DOMAIN_TIMER_NAME_ARMVTIMER qemu: Validate configuration for the armvtimer timer qemu: Format the armvtimer timer on the command line tests: Add test case for the armvtimer timer docs: List the armvtimer timer among all others news: Mention the armvtimer timer docs/formatdomain.html.in | 6 +-- docs/news.xml | 10 ++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/libxl/libxl_conf.c | 1 + src/libxl/xen_common.c | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 49 +++++++++++++++---- src/qemu/qemu_domain.c | 36 ++++++++++++++ .../caps_5.0.0.aarch64.xml | 1 + .../clock-timer-armvtimer.aarch64-latest.args | 32 ++++++++++++ .../clock-timer-armvtimer.xml | 27 ++++++++++ tests/qemuxml2argvtest.c | 2 + .../clock-timer-armvtimer.aarch64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + 17 files changed, 160 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/clock-timer-armvtimer.aarch64-latest.args create mode 100644 tests/qemuxml2argvdata/clock-timer-armvtimer.xml create mode 120000 tests/qemuxml2xmloutdata/clock-timer-armvtimer.aarch64-latest.xml -- 2.24.1

Make sure we are taking all possible virDomainTimerNameType values into account. This will make upcoming changes easier. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0dbd78124b..52a74c7acf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6594,16 +6594,30 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, for (i = 0; i < def->clock.ntimers; i++) { virDomainTimerDefPtr timer = def->clock.timers[i]; - if (timer->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK && - timer->present != -1) { - qemuBuildCpuFeature(qemuCaps, &buf, "kvmclock", - !!timer->present); - } else if (timer->name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK && - timer->present == 1) { - virBufferAddLit(&buf, ",hv-time"); - } else if (timer->name == VIR_DOMAIN_TIMER_NAME_TSC && - timer->frequency > 0) { - virBufferAsprintf(&buf, ",tsc-frequency=%lu", timer->frequency); + switch ((virDomainTimerNameType)timer->name) { + case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: + if (timer->present != -1) { + qemuBuildCpuFeature(qemuCaps, &buf, "kvmclock", + !!timer->present); + } + break; + case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + if (timer->present == 1) + virBufferAddLit(&buf, ",hv-time"); + break; + case VIR_DOMAIN_TIMER_NAME_TSC: + if (timer->frequency > 0) + virBufferAsprintf(&buf, ",tsc-frequency=%lu", timer->frequency); + break; + case VIR_DOMAIN_TIMER_NAME_PLATFORM: + case VIR_DOMAIN_TIMER_NAME_PIT: + case VIR_DOMAIN_TIMER_NAME_RTC: + case VIR_DOMAIN_TIMER_NAME_HPET: + break; + case VIR_DOMAIN_TIMER_NAME_LAST: + default: + virReportEnumRangeError(virDomainTimerNameType, timer->name); + return -1; } } -- 2.24.1

On Fri, Feb 07, 2020 at 03:27:01PM +0100, Andrea Bolognani wrote:
Make sure we are taking all possible virDomainTimerNameType values into account. This will make upcoming changes easier.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0dbd78124b..52a74c7acf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6594,16 +6594,30 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, for (i = 0; i < def->clock.ntimers; i++) { virDomainTimerDefPtr timer = def->clock.timers[i];
- if (timer->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK && - timer->present != -1) { - qemuBuildCpuFeature(qemuCaps, &buf, "kvmclock", - !!timer->present); - } else if (timer->name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK && - timer->present == 1) { - virBufferAddLit(&buf, ",hv-time"); - } else if (timer->name == VIR_DOMAIN_TIMER_NAME_TSC && - timer->frequency > 0) { - virBufferAsprintf(&buf, ",tsc-frequency=%lu", timer->frequency); + switch ((virDomainTimerNameType)timer->name) { + case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: + if (timer->present != -1) { + qemuBuildCpuFeature(qemuCaps, &buf, "kvmclock", + !!timer->present); + } + break; + case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + if (timer->present == 1) + virBufferAddLit(&buf, ",hv-time"); + break; + case VIR_DOMAIN_TIMER_NAME_TSC: + if (timer->frequency > 0) + virBufferAsprintf(&buf, ",tsc-frequency=%lu", timer->frequency); + break; + case VIR_DOMAIN_TIMER_NAME_PLATFORM: + case VIR_DOMAIN_TIMER_NAME_PIT: + case VIR_DOMAIN_TIMER_NAME_RTC: + case VIR_DOMAIN_TIMER_NAME_HPET: + break; + case VIR_DOMAIN_TIMER_NAME_LAST: + default: + virReportEnumRangeError(virDomainTimerNameType, timer->name); + return -1; } }
--
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

On Fri, Feb 07, 2020 at 03:27:01PM +0100, Andrea Bolognani wrote:
Make sure we are taking all possible virDomainTimerNameType values into account. This will make upcoming changes easier.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

We will use this capability to detect whether the QEMU binary supports the kvm-no-adjvtime CPU feature. Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 + 3 files changed, 4 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index dd2311cfa9..0e727093bc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -559,6 +559,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "rng-builtin", "virtio-net.failover", "tpm-spapr", + "cpu.kvm-no-adjvtime", ); @@ -1562,6 +1563,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMemoryBackendMemfd[] static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMaxCPU[] = { { "unavailable-features", QEMU_CAPS_CPU_UNAVAILABLE_FEATURES }, + { "kvm-no-adjvtime", QEMU_CAPS_CPU_KVM_NO_ADJVTIME }, }; static virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 2473e64654..e6c5725a76 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -541,6 +541,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_OBJECT_RNG_BUILTIN, /* -object rng-builtin */ QEMU_CAPS_VIRTIO_NET_FAILOVER, /* virtio-net-*.failover */ QEMU_CAPS_DEVICE_TPM_SPAPR, /* -device tpm-spapr */ + QEMU_CAPS_CPU_KVM_NO_ADJVTIME, /* -cpu ...,kvm-no-adjvtime */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml index 538b40dd5b..c05cea2eb7 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml @@ -179,6 +179,7 @@ <flag name='smp-dies'/> <flag name='rng-builtin'/> <flag name='virtio-net.failover'/> + <flag name='cpu.kvm-no-adjvtime'/> <version>4002050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700241</microcodeVersion> -- 2.24.1

On Fri, Feb 07, 2020 at 03:27:02PM +0100, Andrea Bolognani wrote:
We will use this capability to detect whether the QEMU binary supports the kvm-no-adjvtime CPU feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 + 3 files changed, 4 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index dd2311cfa9..0e727093bc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -559,6 +559,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "rng-builtin", "virtio-net.failover", "tpm-spapr", + "cpu.kvm-no-adjvtime", );
@@ -1562,6 +1563,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMemoryBackendMemfd[]
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMaxCPU[] = { { "unavailable-features", QEMU_CAPS_CPU_UNAVAILABLE_FEATURES }, + { "kvm-no-adjvtime", QEMU_CAPS_CPU_KVM_NO_ADJVTIME }, };
static virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 2473e64654..e6c5725a76 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -541,6 +541,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_OBJECT_RNG_BUILTIN, /* -object rng-builtin */ QEMU_CAPS_VIRTIO_NET_FAILOVER, /* virtio-net-*.failover */ QEMU_CAPS_DEVICE_TPM_SPAPR, /* -device tpm-spapr */ + QEMU_CAPS_CPU_KVM_NO_ADJVTIME, /* -cpu ...,kvm-no-adjvtime */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml index 538b40dd5b..c05cea2eb7 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml @@ -179,6 +179,7 @@ <flag name='smp-dies'/> <flag name='rng-builtin'/> <flag name='virtio-net.failover'/> + <flag name='cpu.kvm-no-adjvtime'/> <version>4002050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700241</microcodeVersion> -- 2.24.1
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

On Fri, Feb 07, 2020 at 03:27:02PM +0100, Andrea Bolognani wrote:
We will use this capability to detect whether the QEMU binary supports the kvm-no-adjvtime CPU feature.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 + 3 files changed, 4 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index dd2311cfa9..0e727093bc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -559,6 +559,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "rng-builtin", "virtio-net.failover", "tpm-spapr", + "cpu.kvm-no-adjvtime", );
@@ -1562,6 +1563,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMemoryBackendMemfd[]
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMaxCPU[] = { { "unavailable-features", QEMU_CAPS_CPU_UNAVAILABLE_FEATURES }, + { "kvm-no-adjvtime", QEMU_CAPS_CPU_KVM_NO_ADJVTIME }, };
static virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 2473e64654..e6c5725a76 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -541,6 +541,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_OBJECT_RNG_BUILTIN, /* -object rng-builtin */ QEMU_CAPS_VIRTIO_NET_FAILOVER, /* virtio-net-*.failover */ QEMU_CAPS_DEVICE_TPM_SPAPR, /* -device tpm-spapr */ + QEMU_CAPS_CPU_KVM_NO_ADJVTIME, /* -cpu ...,kvm-no-adjvtime */
/* cpu.kvm-no-adjvtime */ No need to summon pac-man. Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

This new timer model will be used to control the behavior of the virtual timer for KVM ARM/virt guests. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/libxl/libxl_conf.c | 1 + src/libxl/xen_common.c | 1 + src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_domain.c | 3 +++ 7 files changed, 10 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9577d26c2a..29b6b95357 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1239,6 +1239,7 @@ <choice> <value>hpet</value> <value>pit</value> + <value>armvtimer</value> </choice> </attribute> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 51ae520897..78d964ed9e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1063,6 +1063,7 @@ VIR_ENUM_IMPL(virDomainTimerName, "tsc", "kvmclock", "hypervclock", + "armvtimer", ); VIR_ENUM_IMPL(virDomainTimerTrack, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2db3c19473..867a9c7661 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1993,6 +1993,7 @@ typedef enum { VIR_DOMAIN_TIMER_NAME_TSC, VIR_DOMAIN_TIMER_NAME_KVMCLOCK, VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK, + VIR_DOMAIN_TIMER_NAME_ARMVTIMER, VIR_DOMAIN_TIMER_NAME_LAST } virDomainTimerNameType; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index ee6b23895c..56deca7b7c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -359,6 +359,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: case VIR_DOMAIN_TIMER_NAME_RTC: case VIR_DOMAIN_TIMER_NAME_PIT: + case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported timer type (name) '%s'"), virDomainTimerNameTypeToString(clock.timers[i]->name)); diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 415549a42c..9a385eba0d 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -2182,6 +2182,7 @@ xenFormatCPUFeatures(virConfPtr conf, virDomainDefPtr def) case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: case VIR_DOMAIN_TIMER_NAME_RTC: case VIR_DOMAIN_TIMER_NAME_PIT: + case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported timer type (name) '%s'"), virDomainTimerNameTypeToString(def->clock.timers[i]->name)); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 52a74c7acf..71ae1f72e5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6199,6 +6199,7 @@ qemuBuildClockCommandLine(virCommandPtr cmd, case VIR_DOMAIN_TIMER_NAME_TSC: case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: /* Timers above are handled when building -cpu. */ case VIR_DOMAIN_TIMER_NAME_LAST: break; @@ -6609,6 +6610,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, if (timer->frequency > 0) virBufferAsprintf(&buf, ",tsc-frequency=%lu", timer->frequency); break; + case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: case VIR_DOMAIN_TIMER_NAME_PLATFORM: case VIR_DOMAIN_TIMER_NAME_PIT: case VIR_DOMAIN_TIMER_NAME_RTC: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1b4825a539..68348464a8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5428,6 +5428,9 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def, return -1; } break; + + case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: + break; } } -- 2.24.1

On Fri, Feb 07, 2020 at 03:27:03PM +0100, Andrea Bolognani wrote:
This new timer model will be used to control the behavior of the virtual timer for KVM ARM/virt guests.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/libxl/libxl_conf.c | 1 + src/libxl/xen_common.c | 1 + src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_domain.c | 3 +++ 7 files changed, 10 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9577d26c2a..29b6b95357 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1239,6 +1239,7 @@ <choice> <value>hpet</value> <value>pit</value> + <value>armvtimer</value> </choice> </attribute> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 51ae520897..78d964ed9e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1063,6 +1063,7 @@ VIR_ENUM_IMPL(virDomainTimerName, "tsc", "kvmclock", "hypervclock", + "armvtimer", );
VIR_ENUM_IMPL(virDomainTimerTrack, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2db3c19473..867a9c7661 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1993,6 +1993,7 @@ typedef enum { VIR_DOMAIN_TIMER_NAME_TSC, VIR_DOMAIN_TIMER_NAME_KVMCLOCK, VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK, + VIR_DOMAIN_TIMER_NAME_ARMVTIMER,
VIR_DOMAIN_TIMER_NAME_LAST } virDomainTimerNameType; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index ee6b23895c..56deca7b7c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -359,6 +359,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: case VIR_DOMAIN_TIMER_NAME_RTC: case VIR_DOMAIN_TIMER_NAME_PIT: + case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported timer type (name) '%s'"), virDomainTimerNameTypeToString(clock.timers[i]->name)); diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 415549a42c..9a385eba0d 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -2182,6 +2182,7 @@ xenFormatCPUFeatures(virConfPtr conf, virDomainDefPtr def) case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: case VIR_DOMAIN_TIMER_NAME_RTC: case VIR_DOMAIN_TIMER_NAME_PIT: + case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported timer type (name) '%s'"), virDomainTimerNameTypeToString(def->clock.timers[i]->name)); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 52a74c7acf..71ae1f72e5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6199,6 +6199,7 @@ qemuBuildClockCommandLine(virCommandPtr cmd, case VIR_DOMAIN_TIMER_NAME_TSC: case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: /* Timers above are handled when building -cpu. */ case VIR_DOMAIN_TIMER_NAME_LAST: break; @@ -6609,6 +6610,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, if (timer->frequency > 0) virBufferAsprintf(&buf, ",tsc-frequency=%lu", timer->frequency); break; + case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: case VIR_DOMAIN_TIMER_NAME_PLATFORM: case VIR_DOMAIN_TIMER_NAME_PIT: case VIR_DOMAIN_TIMER_NAME_RTC: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1b4825a539..68348464a8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5428,6 +5428,9 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def, return -1; } break; + + case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: + break; } }
-- 2.24.1
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

On Fri, Feb 07, 2020 at 03:27:03PM +0100, Andrea Bolognani wrote:
This new timer model will be used to control the behavior of the virtual timer for KVM ARM/virt guests.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/libxl/libxl_conf.c | 1 + src/libxl/xen_common.c | 1 + src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_domain.c | 3 +++ 7 files changed, 10 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9577d26c2a..29b6b95357 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1239,6 +1239,7 @@ <choice> <value>hpet</value> <value>pit</value> + <value>armvtimer</value> </choice> </attribute> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 51ae520897..78d964ed9e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1063,6 +1063,7 @@ VIR_ENUM_IMPL(virDomainTimerName, "tsc", "kvmclock", "hypervclock", + "armvtimer", );
Okay, so this name is a libvirt invention. And the timer itself is present on all ARM/virt guests and cannot be disabled, correct? Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Thu, 2020-02-13 at 14:26 +0100, Ján Tomko wrote:
On Fri, Feb 07, 2020 at 03:27:03PM +0100, Andrea Bolognani wrote:
+++ b/src/conf/domain_conf.c @@ -1063,6 +1063,7 @@ VIR_ENUM_IMPL(virDomainTimerName, "tsc", "kvmclock", "hypervclock", + "armvtimer", );
Okay, so this name is a libvirt invention.
I believe "ARM virtual timer" is the official name for the device; the name "armvtimer" is an abbreviation that me and Drew (CC'd) have agreed upon, and I understand that's fairly commonly used too. Drew can confirm whether this is actually the case.
And the timer itself is present on all ARM/virt guests and cannot be disabled, correct?
I believe so but, once again, it'd be better if Drew confirmed it :) -- Andrea Bolognani / Red Hat / Virtualization

On Thu, Feb 13, 2020 at 05:30:21PM +0100, Andrea Bolognani wrote:
On Thu, 2020-02-13 at 14:26 +0100, Ján Tomko wrote:
On Fri, Feb 07, 2020 at 03:27:03PM +0100, Andrea Bolognani wrote:
+++ b/src/conf/domain_conf.c @@ -1063,6 +1063,7 @@ VIR_ENUM_IMPL(virDomainTimerName, "tsc", "kvmclock", "hypervclock", + "armvtimer", );
Okay, so this name is a libvirt invention.
I believe "ARM virtual timer" is the official name for the device; the name "armvtimer" is an abbreviation that me and Drew (CC'd) have agreed upon, and I understand that's fairly commonly used too.
Drew can confirm whether this is actually the case.
To be precise the ARM architecture reference manual refers to the CPU's builtin timer as the "Generic Timer". All CPUs which the QEMU arm 'virt' machine type supports implement the Generic Timer, and all implementations of the Generic Timer must include the virtual counter. The virtual counter is used to indicate virtual time. In both QEMU and KVM when we want to refer to the virtual counter based timer we call it the "vtimer". So, while libvirt is indeed inventing the name "armvtimer", I think it's appropriate.
And the timer itself is present on all ARM/virt guests and cannot be disabled, correct?
I believe so but, once again, it'd be better if Drew confirmed it :)
Correct. While the Generic Timer is an optional feature of the CPU, all CPUs that the ARM/virt machine type support have it and it cannot be removed. So, all ARM/virt guests have vtimers, but not all ARM guests must have vtimers. If libvirt can start non-'virt' ARM guests, which have different CPUs, then there's a chance that the guest will not have a vtimer. I'm not sure if we want to try and incorporate that much specificity into the libvirt name, though. I think if a machine/CPU doesn't have a vtimer than this XML element should just not be valid. Thanks, drew

On Fri, 2020-02-14 at 11:19 +0100, Andrew Jones wrote:
On Thu, Feb 13, 2020 at 05:30:21PM +0100, Andrea Bolognani wrote:
On Thu, 2020-02-13 at 14:26 +0100, Ján Tomko wrote:
Okay, so this name is a libvirt invention.
I believe "ARM virtual timer" is the official name for the device; the name "armvtimer" is an abbreviation that me and Drew (CC'd) have agreed upon, and I understand that's fairly commonly used too.
Drew can confirm whether this is actually the case.
To be precise the ARM architecture reference manual refers to the CPU's builtin timer as the "Generic Timer". All CPUs which the QEMU arm 'virt' machine type supports implement the Generic Timer, and all implementations of the Generic Timer must include the virtual counter. The virtual counter is used to indicate virtual time. In both QEMU and KVM when we want to refer to the virtual counter based timer we call it the "vtimer".
So, while libvirt is indeed inventing the name "armvtimer", I think it's appropriate.
There's plenty of precedents for carrying a name used in KVM/QEMU over to libvirt, so we're good on that front.
And the timer itself is present on all ARM/virt guests and cannot be disabled, correct?
I believe so but, once again, it'd be better if Drew confirmed it :)
Correct. While the Generic Timer is an optional feature of the CPU, all CPUs that the ARM/virt machine type support have it and it cannot be removed.
So, all ARM/virt guests have vtimers, but not all ARM guests must have vtimers. If libvirt can start non-'virt' ARM guests, which have different CPUs, then there's a chance that the guest will not have a vtimer. I'm not sure if we want to try and incorporate that much specificity into the libvirt name, though. I think if a machine/CPU doesn't have a vtimer than this XML element should just not be valid.
We're explicitly preventing non-virt guests to configure the timer, so no problem here. Since all outstanding questions have been answered, I'm going to push the series now. Thanks Drew for providing these last nuggets of information, and of course thanks Jano for the review :) -- Andrea Bolognani / Red Hat / Virtualization

Its use is limited to certain guest types, and it only supports a subset of all possible tick policies. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 68348464a8..8036886508 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5430,6 +5430,39 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def, break; case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: + if (def->virtType != VIR_DOMAIN_VIRT_KVM || + !qemuDomainIsARMVirt(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Configuring the '%s' timer is not supported " + "for virtType=%s arch=%s machine=%s guests"), + virDomainTimerNameTypeToString(timer->name), + virDomainVirtTypeToString(def->virtType), + virArchToString(def->os.arch), + def->os.machine); + return -1; + } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_KVM_NO_ADJVTIME)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Configuring the '%s' timer is not supported " + "with this QEMU binary"), + virDomainTimerNameTypeToString(timer->name)); + return -1; + } + + switch (timer->tickpolicy) { + case -1: + case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: + case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The '%s' timer does not support " + "tickpolicy '%s'"), + virDomainTimerNameTypeToString(timer->name), + virDomainTimerTickpolicyTypeToString(timer->tickpolicy)); + return -1; + } break; } } -- 2.24.1

On Fri, Feb 07, 2020 at 03:27:04PM +0100, Andrea Bolognani wrote:
Its use is limited to certain guest types, and it only supports a subset of all possible tick policies.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 68348464a8..8036886508 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5430,6 +5430,39 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def, break;
case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: + if (def->virtType != VIR_DOMAIN_VIRT_KVM || + !qemuDomainIsARMVirt(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Configuring the '%s' timer is not supported " + "for virtType=%s arch=%s machine=%s guests"), + virDomainTimerNameTypeToString(timer->name), + virDomainVirtTypeToString(def->virtType), + virArchToString(def->os.arch), + def->os.machine); + return -1; + } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_KVM_NO_ADJVTIME)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Configuring the '%s' timer is not supported " + "with this QEMU binary"), + virDomainTimerNameTypeToString(timer->name)); + return -1; + } + + switch (timer->tickpolicy) { + case -1: + case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: + case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The '%s' timer does not support " + "tickpolicy '%s'"), + virDomainTimerNameTypeToString(timer->name), + virDomainTimerTickpolicyTypeToString(timer->tickpolicy)); + return -1; + } break; } } -- 2.24.1
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

On Fri, Feb 07, 2020 at 03:27:04PM +0100, Andrea Bolognani wrote:
Its use is limited to certain guest types, and it only supports a subset of all possible tick policies.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 68348464a8..8036886508 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5430,6 +5430,39 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def, break;
case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
Missing check for present == 0.
+ if (def->virtType != VIR_DOMAIN_VIRT_KVM || + !qemuDomainIsARMVirt(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Configuring the '%s' timer is not supported " + "for virtType=%s arch=%s machine=%s guests"), + virDomainTimerNameTypeToString(timer->name), + virDomainVirtTypeToString(def->virtType), + virArchToString(def->os.arch), + def->os.machine); + return -1; + } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_KVM_NO_ADJVTIME)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Configuring the '%s' timer is not supported " + "with this QEMU binary"), + virDomainTimerNameTypeToString(timer->name)); + return -1; + } + + switch (timer->tickpolicy) { + case -1: + case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: + case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The '%s' timer does not support " + "tickpolicy '%s'"),
Please join the last two lines.
+ virDomainTimerNameTypeToString(timer->name), + virDomainTimerTickpolicyTypeToString(timer->tickpolicy)); + return -1;
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Thu, 2020-02-13 at 14:28 +0100, Ján Tomko wrote:
On Fri, Feb 07, 2020 at 03:27:04PM +0100, Andrea Bolognani wrote:
+++ b/src/qemu/qemu_domain.c @@ -5430,6 +5430,39 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def, break;
case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
Missing check for present == 0.
Good catch! Assuming Drew confirms the timer can't be disabled, then we should definitely error out for present='no'. Do you want me to respin, or can I just squash in the diff below? diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 29ec203413..143ddc508e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5441,6 +5441,12 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def, def->os.machine); return -1; } + if (timer->present == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The '%s' timer can't be disabled"), + virDomainTimerNameTypeToString(timer->name)); + return -1; + } if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_KVM_NO_ADJVTIME)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Configuring the '%s' timer is not supported " -- Andrea Bolognani / Red Hat / Virtualization

On Thu, Feb 13, 2020 at 05:45:14PM +0100, Andrea Bolognani wrote:
On Thu, 2020-02-13 at 14:28 +0100, Ján Tomko wrote:
On Fri, Feb 07, 2020 at 03:27:04PM +0100, Andrea Bolognani wrote:
+++ b/src/qemu/qemu_domain.c @@ -5430,6 +5430,39 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def, break;
case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
Missing check for present == 0.
Good catch! Assuming Drew confirms the timer can't be disabled, then we should definitely error out for present='no'.
Do you want me to respin, or can I just squash in the diff below?
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 29ec203413..143ddc508e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5441,6 +5441,12 @@ qemuDomainDefValidateClockTimers(const virDomainDef *def, def->os.machine); return -1; } + if (timer->present == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The '%s' timer can't be disabled"), + virDomainTimerNameTypeToString(timer->name)); + return -1; + } if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_KVM_NO_ADJVTIME)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Configuring the '%s' timer is not supported "
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Its behavior is controlled by a KVM-specific CPU feature. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71ae1f72e5..60f8820c64 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6611,6 +6611,19 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAsprintf(&buf, ",tsc-frequency=%lu", timer->frequency); break; case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: + switch (timer->tickpolicy) { + case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: + virBufferAddLit(&buf, ",kvm-no-adjvtime=off"); + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + virBufferAddLit(&buf, ",kvm-no-adjvtime=on"); + break; + case -1: + case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: + break; + } + break; case VIR_DOMAIN_TIMER_NAME_PLATFORM: case VIR_DOMAIN_TIMER_NAME_PIT: case VIR_DOMAIN_TIMER_NAME_RTC: -- 2.24.1

On Fri, Feb 07, 2020 at 03:27:05PM +0100, Andrea Bolognani wrote:
Its behavior is controlled by a KVM-specific CPU feature.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71ae1f72e5..60f8820c64 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6611,6 +6611,19 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAsprintf(&buf, ",tsc-frequency=%lu", timer->frequency); break; case VIR_DOMAIN_TIMER_NAME_ARMVTIMER: + switch (timer->tickpolicy) { + case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: + virBufferAddLit(&buf, ",kvm-no-adjvtime=off"); + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + virBufferAddLit(&buf, ",kvm-no-adjvtime=on"); + break; + case -1: + case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: + break; + } + break; case VIR_DOMAIN_TIMER_NAME_PLATFORM: case VIR_DOMAIN_TIMER_NAME_PIT: case VIR_DOMAIN_TIMER_NAME_RTC: -- 2.24.1
Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

On Fri, Feb 07, 2020 at 03:27:05PM +0100, Andrea Bolognani wrote:
Its behavior is controlled by a KVM-specific CPU feature.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_command.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .../clock-timer-armvtimer.aarch64-latest.args | 32 +++++++++++++++++++ .../clock-timer-armvtimer.xml | 27 ++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ .../clock-timer-armvtimer.aarch64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + 5 files changed, 63 insertions(+) create mode 100644 tests/qemuxml2argvdata/clock-timer-armvtimer.aarch64-latest.args create mode 100644 tests/qemuxml2argvdata/clock-timer-armvtimer.xml create mode 120000 tests/qemuxml2xmloutdata/clock-timer-armvtimer.aarch64-latest.xml diff --git a/tests/qemuxml2argvdata/clock-timer-armvtimer.aarch64-latest.args b/tests/qemuxml2argvdata/clock-timer-armvtimer.aarch64-latest.args new file mode 100644 index 0000000000..a1faa97b9f --- /dev/null +++ b/tests/qemuxml2argvdata/clock-timer-armvtimer.aarch64-latest.args @@ -0,0 +1,32 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-guest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-guest/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-guest/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-aarch64 \ +-name guest=guest,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-guest/master-key.aes \ +-machine virt,accel=kvm,usb=off,dump-guest-core=off,gic-version=3 \ +-cpu host,kvm-no-adjvtime=on \ +-m 4096 \ +-overcommit mem-lock=off \ +-smp 4,sockets=4,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/clock-timer-armvtimer.xml b/tests/qemuxml2argvdata/clock-timer-armvtimer.xml new file mode 100644 index 0000000000..295ab64d75 --- /dev/null +++ b/tests/qemuxml2argvdata/clock-timer-armvtimer.xml @@ -0,0 +1,27 @@ +<domain type='kvm'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>4194304</memory> + <currentMemory unit='KiB'>4194304</currentMemory> + <vcpu placement='static'>4</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='3'/> + </features> + <cpu mode='host-passthrough' check='none'/> + <clock offset='utc'> + <timer name='armvtimer' tickpolicy='discard'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pcie-root'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 028364a06c..efa0ec9477 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2729,6 +2729,8 @@ mymain(void) /* SVE aarch64 CPU features work on modern QEMU */ DO_TEST_CAPS_ARCH_LATEST("aarch64-features-sve", "aarch64"); + DO_TEST_CAPS_ARCH_LATEST("clock-timer-armvtimer", "aarch64"); + qemuTestSetHostArch(&driver, VIR_ARCH_NONE); DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY); diff --git a/tests/qemuxml2xmloutdata/clock-timer-armvtimer.aarch64-latest.xml b/tests/qemuxml2xmloutdata/clock-timer-armvtimer.aarch64-latest.xml new file mode 120000 index 0000000000..4bfddd6573 --- /dev/null +++ b/tests/qemuxml2xmloutdata/clock-timer-armvtimer.aarch64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/clock-timer-armvtimer.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index ecd12c3d30..4b1699db7e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -269,6 +269,7 @@ mymain(void) DO_TEST("clock-catchup", QEMU_CAPS_KVM_PIT_TICK_POLICY); DO_TEST("kvmclock", NONE); DO_TEST("clock-timer-hyperv-rtc", NONE); + DO_TEST_CAPS_ARCH_LATEST("clock-timer-armvtimer", "aarch64"); DO_TEST("cpu-eoi-disabled", NONE); DO_TEST("cpu-eoi-enabled", NONE); -- 2.24.1

On Fri, Feb 07, 2020 at 03:27:06PM +0100, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .../clock-timer-armvtimer.aarch64-latest.args | 32 +++++++++++++++++++ .../clock-timer-armvtimer.xml | 27 ++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ .../clock-timer-armvtimer.aarch64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + 5 files changed, 63 insertions(+) create mode 100644 tests/qemuxml2argvdata/clock-timer-armvtimer.aarch64-latest.args create mode 100644 tests/qemuxml2argvdata/clock-timer-armvtimer.xml create mode 120000 tests/qemuxml2xmloutdata/clock-timer-armvtimer.aarch64-latest.xml
Personally, having the XML test squashed with the XML parser/formatter and the ->argv conversion with the QEMU command line formatter feels nicer. Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Thu, 2020-02-13 at 14:30 +0100, Ján Tomko wrote:
On Fri, Feb 07, 2020 at 03:27:06PM +0100, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .../clock-timer-armvtimer.aarch64-latest.args | 32 +++++++++++++++++++ .../clock-timer-armvtimer.xml | 27 ++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ .../clock-timer-armvtimer.aarch64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + 5 files changed, 63 insertions(+) create mode 100644 tests/qemuxml2argvdata/clock-timer-armvtimer.aarch64-latest.args create mode 100644 tests/qemuxml2argvdata/clock-timer-armvtimer.xml create mode 120000 tests/qemuxml2xmloutdata/clock-timer-armvtimer.aarch64-latest.xml
Personally, having the XML test squashed with the XML parser/formatter and the ->argv conversion with the QEMU command line formatter feels nicer.
I tend to agree, and that's how I had it initially, but in this particular case it ended up being more confusing IMHO. I can split it again if you want me to. Your call entirely :) -- Andrea Bolognani / Red Hat / Virtualization

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/formatdomain.html.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 44e2062d01..3e899ffe4e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2466,9 +2466,9 @@ "platform" (currently unsupported), "hpet" (libxl, xen, qemu), "kvmclock" (qemu), "pit" (qemu), "rtc" (qemu), "tsc" (libxl, qemu - - <span class="since">since 3.2.0</span>) - or "hypervclock" - (qemu - <span class="since">since 1.2.2</span>). + <span class="since">since 3.2.0</span>), "hypervclock" + (qemu - <span class="since">since 1.2.2</span>) or + "armvtimer" (qemu - <span class="since">since 6.1.0</span>). The <code>hypervclock</code> timer adds support for the reference time counter and the reference page for iTSC -- 2.24.1

On Fri, Feb 07, 2020 at 03:27:07PM +0100, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/formatdomain.html.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 44e2062d01..3e899ffe4e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2466,9 +2466,9 @@ "platform" (currently unsupported), "hpet" (libxl, xen, qemu), "kvmclock" (qemu), "pit" (qemu), "rtc" (qemu), "tsc" (libxl, qemu - - <span class="since">since 3.2.0</span>) - or "hypervclock" - (qemu - <span class="since">since 1.2.2</span>). + <span class="since">since 3.2.0</span>), "hypervclock" + (qemu - <span class="since">since 1.2.2</span>) or + "armvtimer" (qemu - <span class="since">since 6.1.0</span>).
The <code>hypervclock</code> timer adds support for the reference time counter and the reference page for iTSC
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index f567a1182e..5aa9d081a7 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -82,6 +82,16 @@ "type" and "persistent" attributes. </description> </change> + <change> + <summary> + qemu: Introduce the 'armvtimer' timer type + </summary> + <description> + QEMU 5.0 introduces the ability to control the behavior of the + virtual timer for KVM ARM/virt guests, and this new timer type + exposes the same capability to libvirt users. + </description> + </change> </section> <section title="Improvements"> </section> -- 2.24.1

On Fri, Feb 07, 2020 at 03:27:08PM +0100, Andrea Bolognani wrote:
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (4)
-
Andrea Bolognani
-
Andrew Jones
-
Ján Tomko
-
Masayoshi Mizuma