* [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU
@ 2016-09-15 5:04 Wei Huang
2016-09-15 5:04 ` [Qemu-devel] [PATCH V3 1/2] arm64: Add an option to turn on/off vPMU support Wei Huang
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Wei Huang @ 2016-09-15 5:04 UTC (permalink / raw)
To: qemu-arm; +Cc: qemu-devel, peter.maydell, drjones, shannon.zhao, abologna
This patchset adds a pmu=[on/off] option to enable/disable vPMU support
for guest VM. There are several reasons to justify this option. First,
vPMU can be problematic for cross-migration between different SoC as perf
counters are architecture-dependent. It is more flexible to have an option
to turn it on/off. Secondly Secondly this option matches the "pmu" option
as supported in libvirt. To make sure backward compatible, a PMU property
is added to mach-virt machine types.
V2->V3:
* revise patch 1 commit msg and if-else statement (Drew)
* move property field into VirtMachineClass (Drew)
V1->V2:
* keep the original field name as "has_pmu"
* add a warning message when PMU is turned on without KVM
* use the feature bit to check PMU availability, instead of using has_pmu
* add PMU compat support to mach-virt machine type
RFC->V1:
* set default pmu=off
* change struct ARMCPU field name "has_pmu" ==> "has_host_pmu"
* like el3, add a new feature ARM_FEATURE_HOST_PMU
* "pmu" property becomes CPU dependent. Only cortex-a53/cortex-a57/host
running on kvm supports this option.
-Wei
Wei Huang (2):
arm64: Add an option to turn on/off vPMU support
arm: virt: add PMU property to mach-virt machine type
hw/arm/virt-acpi-build.c | 2 +-
hw/arm/virt.c | 15 ++++++++++++++-
target-arm/cpu.c | 22 ++++++++++++++++++++++
target-arm/cpu.h | 1 +
target-arm/cpu64.c | 2 ++
target-arm/kvm64.c | 19 +++++++++++++++----
6 files changed, 55 insertions(+), 6 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH V3 1/2] arm64: Add an option to turn on/off vPMU support
2016-09-15 5:04 [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU Wei Huang
@ 2016-09-15 5:04 ` Wei Huang
2016-09-15 7:13 ` Andrew Jones
2016-09-15 5:04 ` [Qemu-devel] [PATCH V3 2/2] arm: virt: add PMU property to mach-virt machine type Wei Huang
2016-09-16 12:29 ` [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU Andrea Bolognani
2 siblings, 1 reply; 7+ messages in thread
From: Wei Huang @ 2016-09-15 5:04 UTC (permalink / raw)
To: qemu-arm; +Cc: qemu-devel, peter.maydell, drjones, shannon.zhao, abologna
This patch adds a pmu=[on/off] option to enable/disable vPMU support
in guest vCPU. This option is only available for cortex-a57/cortex-53/
host under both TCG and KVM modes, but unavailable on ARMv7 and other
processors. It allows virt tools, such as libvirt, to determine the
exsitence of vPMU and configure it. Note that this option, turned off
by default, can only be turned on under KVM mode; otherwise a warning
message will be printed out.
Signed-off-by: Wei Huang <wei@redhat.com>
---
hw/arm/virt-acpi-build.c | 2 +-
hw/arm/virt.c | 2 +-
target-arm/cpu.c | 22 ++++++++++++++++++++++
target-arm/cpu.h | 1 +
target-arm/cpu64.c | 2 ++
target-arm/kvm64.c | 19 +++++++++++++++----
6 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 295ec86..8b3083e 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -540,7 +540,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
gicc->uid = i;
gicc->flags = cpu_to_le32(ACPI_GICC_ENABLED);
- if (armcpu->has_pmu) {
+ if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
}
}
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a193b5a..a781ad0 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -477,7 +477,7 @@ static void fdt_add_pmu_nodes(const VirtBoardInfo *vbi, int gictype)
CPU_FOREACH(cpu) {
armcpu = ARM_CPU(cpu);
- if (!armcpu->has_pmu ||
+ if (!arm_feature(&armcpu->env, ARM_FEATURE_PMU) ||
!kvm_arm_pmu_create(cpu, PPI(VIRTUAL_PMU_IRQ))) {
return;
}
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index ce8b8f4..d304597 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -19,6 +19,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "qapi/error.h"
#include "cpu.h"
#include "internals.h"
@@ -509,6 +510,10 @@ static Property arm_cpu_rvbar_property =
static Property arm_cpu_has_el3_property =
DEFINE_PROP_BOOL("has_el3", ARMCPU, has_el3, true);
+/* use property name "pmu" to match other archs and virt tools */
+static Property arm_cpu_has_pmu_property =
+ DEFINE_PROP_BOOL("pmu", ARMCPU, has_pmu, false);
+
static Property arm_cpu_has_mpu_property =
DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
@@ -552,6 +557,11 @@ static void arm_cpu_post_init(Object *obj)
#endif
}
+ if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
+ qdev_property_add_static(DEVICE(obj), &arm_cpu_has_pmu_property,
+ &error_abort);
+ }
+
if (arm_feature(&cpu->env, ARM_FEATURE_MPU)) {
qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
&error_abort);
@@ -576,6 +586,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
ARMCPU *cpu = ARM_CPU(dev);
ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
CPUARMState *env = &cpu->env;
+ static bool pmu_warned;
/* Some features automatically imply others: */
if (arm_feature(env, ARM_FEATURE_V8)) {
@@ -648,6 +659,17 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
cpu->id_aa64pfr0 &= ~0xf000;
}
+ if (cpu->has_pmu && !kvm_enabled()) {
+ cpu->has_pmu = false;
+ if (!pmu_warned) {
+ error_report("warning: pmu can't be enabled without KVM acceleration");
+ pmu_warned = true;
+ }
+ }
+ if (!cpu->has_pmu) {
+ unset_feature(env, ARM_FEATURE_PMU);
+ }
+
if (!arm_feature(env, ARM_FEATURE_EL2)) {
/* Disable the hypervisor feature bits in the processor feature
* registers if we don't have EL2. These are id_pfr1[15:12] and
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 76d824d..5d9e6e7 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1129,6 +1129,7 @@ enum arm_features {
ARM_FEATURE_V8_SHA256, /* implements SHA256 part of v8 Crypto Extensions */
ARM_FEATURE_V8_PMULL, /* implements PMULL part of v8 Crypto Extensions */
ARM_FEATURE_THUMB_DSP, /* DSP insns supported in the Thumb encodings */
+ ARM_FEATURE_PMU, /* has PMU support */
};
static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index 1635deb..549cb1e 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -111,6 +111,7 @@ static void aarch64_a57_initfn(Object *obj)
set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
set_feature(&cpu->env, ARM_FEATURE_CRC);
set_feature(&cpu->env, ARM_FEATURE_EL3);
+ set_feature(&cpu->env, ARM_FEATURE_PMU);
cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A57;
cpu->midr = 0x411fd070;
cpu->revidr = 0x00000000;
@@ -166,6 +167,7 @@ static void aarch64_a53_initfn(Object *obj)
set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
set_feature(&cpu->env, ARM_FEATURE_CRC);
set_feature(&cpu->env, ARM_FEATURE_EL3);
+ set_feature(&cpu->env, ARM_FEATURE_PMU);
cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A53;
cpu->midr = 0x410fd034;
cpu->revidr = 0x00000000;
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
index 5faa76c..4e0124c 100644
--- a/target-arm/kvm64.c
+++ b/target-arm/kvm64.c
@@ -428,6 +428,11 @@ static inline void set_feature(uint64_t *features, int feature)
*features |= 1ULL << feature;
}
+static inline void unset_feature(uint64_t *features, int feature)
+{
+ *features &= ~(1ULL << feature);
+}
+
bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
{
/* Identify the feature bits corresponding to the host CPU, and
@@ -469,6 +474,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
set_feature(&features, ARM_FEATURE_VFP4);
set_feature(&features, ARM_FEATURE_NEON);
set_feature(&features, ARM_FEATURE_AARCH64);
+ set_feature(&features, ARM_FEATURE_PMU);
ahcc->features = features;
@@ -482,6 +488,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
int ret;
uint64_t mpidr;
ARMCPU *cpu = ARM_CPU(cs);
+ CPUARMState *env = &cpu->env;
if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE ||
!object_dynamic_cast(OBJECT(cpu), TYPE_AARCH64_CPU)) {
@@ -501,10 +508,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT;
}
- if (kvm_irqchip_in_kernel() &&
- kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) {
- cpu->has_pmu = true;
- cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
+ /* enable PMU based on KVM mode, hw capability, and user setting */
+ cpu->has_pmu &= kvm_irqchip_in_kernel() &&
+ kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3);
+ cpu->kvm_init_features[0] |= cpu->has_pmu << KVM_ARM_VCPU_PMU_V3;
+ if (cpu->has_pmu) {
+ cpu->kvm_init_features[0] |= cpu->has_pmu << KVM_ARM_VCPU_PMU_V3;
+ } else {
+ unset_feature(&env->features, ARM_FEATURE_PMU);
}
/* Do KVM_ARM_VCPU_INIT ioctl */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH V3 2/2] arm: virt: add PMU property to mach-virt machine type
2016-09-15 5:04 [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU Wei Huang
2016-09-15 5:04 ` [Qemu-devel] [PATCH V3 1/2] arm64: Add an option to turn on/off vPMU support Wei Huang
@ 2016-09-15 5:04 ` Wei Huang
2016-09-15 7:18 ` Andrew Jones
2016-09-16 12:29 ` [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU Andrea Bolognani
2 siblings, 1 reply; 7+ messages in thread
From: Wei Huang @ 2016-09-15 5:04 UTC (permalink / raw)
To: qemu-arm; +Cc: qemu-devel, peter.maydell, drjones, shannon.zhao, abologna
CPU vPMU is now turned off by default, but it was ON in virt-2.7
machine type. To solve this problem, this patch adds a PMU option
in machine state, which is used to control CPU's vPMU status. This
PMU option is not exposed to command line and is turned on in
virt-2.7 machine type to make sure it is backward compatible.
Signed-off-by: Wei Huang <wei@redhat.com>
---
hw/arm/virt.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a781ad0..a3fc454 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -84,6 +84,7 @@ typedef struct {
MachineClass parent;
VirtBoardInfo *daughterboard;
bool disallow_affinity_adjustment;
+ bool pmu_default_on;
} VirtMachineClass;
typedef struct {
@@ -1317,6 +1318,11 @@ static void machvirt_init(MachineState *machine)
}
}
+ if (vmc->pmu_default_on) {
+ /* Property name is "pmu", defined in arm_cpu_has_pmu_property */
+ object_property_set_bool(cpuobj, true, "pmu", NULL);
+ }
+
if (object_property_find(cpuobj, "reset-cbar", NULL)) {
object_property_set_int(cpuobj, vbi->memmap[VIRT_CPUPERIPHS].base,
"reset-cbar", &error_abort);
@@ -1514,6 +1520,9 @@ static void virt_2_7_instance_init(Object *obj)
static void virt_machine_2_7_options(MachineClass *mc)
{
+ VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
+ vmc->pmu_default_on = true;
}
DEFINE_VIRT_MACHINE_AS_LATEST(2, 7)
@@ -1532,5 +1541,9 @@ static void virt_machine_2_6_options(MachineClass *mc)
virt_machine_2_7_options(mc);
SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_6);
vmc->disallow_affinity_adjustment = true;
+ /* Disable PMU for 2.6 and down as PMU support was first introduced
+ * and enabled in 2.7.
+ */
+ vmc->pmu_default_on = false;
}
DEFINE_VIRT_MACHINE(2, 6)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH V3 1/2] arm64: Add an option to turn on/off vPMU support
2016-09-15 5:04 ` [Qemu-devel] [PATCH V3 1/2] arm64: Add an option to turn on/off vPMU support Wei Huang
@ 2016-09-15 7:13 ` Andrew Jones
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Jones @ 2016-09-15 7:13 UTC (permalink / raw)
To: Wei Huang; +Cc: qemu-arm, peter.maydell, qemu-devel, abologna, shannon.zhao
On Thu, Sep 15, 2016 at 01:04:15AM -0400, Wei Huang wrote:
> This patch adds a pmu=[on/off] option to enable/disable vPMU support
> in guest vCPU. This option is only available for cortex-a57/cortex-53/
> host under both TCG and KVM modes, but unavailable on ARMv7 and other
> processors. It allows virt tools, such as libvirt, to determine the
> exsitence of vPMU and configure it. Note that this option, turned off
> by default, can only be turned on under KVM mode; otherwise a warning
> message will be printed out.
Nice improvement to the message.
>
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
> hw/arm/virt-acpi-build.c | 2 +-
> hw/arm/virt.c | 2 +-
> target-arm/cpu.c | 22 ++++++++++++++++++++++
> target-arm/cpu.h | 1 +
> target-arm/cpu64.c | 2 ++
> target-arm/kvm64.c | 19 +++++++++++++++----
> 6 files changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 295ec86..8b3083e 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -540,7 +540,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtGuestInfo *guest_info)
> gicc->uid = i;
> gicc->flags = cpu_to_le32(ACPI_GICC_ENABLED);
>
> - if (armcpu->has_pmu) {
> + if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
> gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
> }
> }
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index a193b5a..a781ad0 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -477,7 +477,7 @@ static void fdt_add_pmu_nodes(const VirtBoardInfo *vbi, int gictype)
>
> CPU_FOREACH(cpu) {
> armcpu = ARM_CPU(cpu);
> - if (!armcpu->has_pmu ||
> + if (!arm_feature(&armcpu->env, ARM_FEATURE_PMU) ||
> !kvm_arm_pmu_create(cpu, PPI(VIRTUAL_PMU_IRQ))) {
> return;
> }
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index ce8b8f4..d304597 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -19,6 +19,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> #include "qapi/error.h"
> #include "cpu.h"
> #include "internals.h"
> @@ -509,6 +510,10 @@ static Property arm_cpu_rvbar_property =
> static Property arm_cpu_has_el3_property =
> DEFINE_PROP_BOOL("has_el3", ARMCPU, has_el3, true);
>
> +/* use property name "pmu" to match other archs and virt tools */
> +static Property arm_cpu_has_pmu_property =
> + DEFINE_PROP_BOOL("pmu", ARMCPU, has_pmu, false);
> +
> static Property arm_cpu_has_mpu_property =
> DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
>
> @@ -552,6 +557,11 @@ static void arm_cpu_post_init(Object *obj)
> #endif
> }
>
> + if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
> + qdev_property_add_static(DEVICE(obj), &arm_cpu_has_pmu_property,
> + &error_abort);
> + }
> +
> if (arm_feature(&cpu->env, ARM_FEATURE_MPU)) {
> qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
> &error_abort);
> @@ -576,6 +586,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> ARMCPU *cpu = ARM_CPU(dev);
> ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
> CPUARMState *env = &cpu->env;
> + static bool pmu_warned;
>
> /* Some features automatically imply others: */
> if (arm_feature(env, ARM_FEATURE_V8)) {
> @@ -648,6 +659,17 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> cpu->id_aa64pfr0 &= ~0xf000;
> }
>
> + if (cpu->has_pmu && !kvm_enabled()) {
> + cpu->has_pmu = false;
> + if (!pmu_warned) {
> + error_report("warning: pmu can't be enabled without KVM acceleration");
> + pmu_warned = true;
> + }
> + }
> + if (!cpu->has_pmu) {
> + unset_feature(env, ARM_FEATURE_PMU);
> + }
> +
> if (!arm_feature(env, ARM_FEATURE_EL2)) {
> /* Disable the hypervisor feature bits in the processor feature
> * registers if we don't have EL2. These are id_pfr1[15:12] and
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 76d824d..5d9e6e7 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1129,6 +1129,7 @@ enum arm_features {
> ARM_FEATURE_V8_SHA256, /* implements SHA256 part of v8 Crypto Extensions */
> ARM_FEATURE_V8_PMULL, /* implements PMULL part of v8 Crypto Extensions */
> ARM_FEATURE_THUMB_DSP, /* DSP insns supported in the Thumb encodings */
> + ARM_FEATURE_PMU, /* has PMU support */
> };
>
> static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index 1635deb..549cb1e 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -111,6 +111,7 @@ static void aarch64_a57_initfn(Object *obj)
> set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> set_feature(&cpu->env, ARM_FEATURE_CRC);
> set_feature(&cpu->env, ARM_FEATURE_EL3);
> + set_feature(&cpu->env, ARM_FEATURE_PMU);
> cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A57;
> cpu->midr = 0x411fd070;
> cpu->revidr = 0x00000000;
> @@ -166,6 +167,7 @@ static void aarch64_a53_initfn(Object *obj)
> set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
> set_feature(&cpu->env, ARM_FEATURE_CRC);
> set_feature(&cpu->env, ARM_FEATURE_EL3);
> + set_feature(&cpu->env, ARM_FEATURE_PMU);
> cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A53;
> cpu->midr = 0x410fd034;
> cpu->revidr = 0x00000000;
> diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
> index 5faa76c..4e0124c 100644
> --- a/target-arm/kvm64.c
> +++ b/target-arm/kvm64.c
> @@ -428,6 +428,11 @@ static inline void set_feature(uint64_t *features, int feature)
> *features |= 1ULL << feature;
> }
>
> +static inline void unset_feature(uint64_t *features, int feature)
> +{
> + *features &= ~(1ULL << feature);
> +}
> +
> bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
> {
> /* Identify the feature bits corresponding to the host CPU, and
> @@ -469,6 +474,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
> set_feature(&features, ARM_FEATURE_VFP4);
> set_feature(&features, ARM_FEATURE_NEON);
> set_feature(&features, ARM_FEATURE_AARCH64);
> + set_feature(&features, ARM_FEATURE_PMU);
>
> ahcc->features = features;
>
> @@ -482,6 +488,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> int ret;
> uint64_t mpidr;
> ARMCPU *cpu = ARM_CPU(cs);
> + CPUARMState *env = &cpu->env;
>
> if (cpu->kvm_target == QEMU_KVM_ARM_TARGET_NONE ||
> !object_dynamic_cast(OBJECT(cpu), TYPE_AARCH64_CPU)) {
> @@ -501,10 +508,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
> if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
> cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT;
> }
> - if (kvm_irqchip_in_kernel() &&
> - kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) {
> - cpu->has_pmu = true;
> - cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
> + /* enable PMU based on KVM mode, hw capability, and user setting */
This comment is just stating what the code is doing. I'd drop it.
> + cpu->has_pmu &= kvm_irqchip_in_kernel() &&
> + kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3);
> + cpu->kvm_init_features[0] |= cpu->has_pmu << KVM_ARM_VCPU_PMU_V3;
The above line is no longer needed with the if-else below
> + if (cpu->has_pmu) {
> + cpu->kvm_init_features[0] |= cpu->has_pmu << KVM_ARM_VCPU_PMU_V3;
Here we know cpu->has_pmu is true so we should just shift 1.
> + } else {
> + unset_feature(&env->features, ARM_FEATURE_PMU);
> }
>
> /* Do KVM_ARM_VCPU_INIT ioctl */
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH V3 2/2] arm: virt: add PMU property to mach-virt machine type
2016-09-15 5:04 ` [Qemu-devel] [PATCH V3 2/2] arm: virt: add PMU property to mach-virt machine type Wei Huang
@ 2016-09-15 7:18 ` Andrew Jones
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Jones @ 2016-09-15 7:18 UTC (permalink / raw)
To: Wei Huang; +Cc: qemu-arm, peter.maydell, qemu-devel, abologna, shannon.zhao
On Thu, Sep 15, 2016 at 01:04:16AM -0400, Wei Huang wrote:
> CPU vPMU is now turned off by default, but it was ON in virt-2.7
> machine type. To solve this problem, this patch adds a PMU option
> in machine state, which is used to control CPU's vPMU status. This
> PMU option is not exposed to command line and is turned on in
> virt-2.7 machine type to make sure it is backward compatible.
>
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
> hw/arm/virt.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index a781ad0..a3fc454 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -84,6 +84,7 @@ typedef struct {
> MachineClass parent;
> VirtBoardInfo *daughterboard;
> bool disallow_affinity_adjustment;
> + bool pmu_default_on;
> } VirtMachineClass;
>
> typedef struct {
> @@ -1317,6 +1318,11 @@ static void machvirt_init(MachineState *machine)
> }
> }
>
> + if (vmc->pmu_default_on) {
> + /* Property name is "pmu", defined in arm_cpu_has_pmu_property */
I still don't see the point of this comment. The code below sets the
property named "pmu", so we know the property name is "pmu" based on
that.
> + object_property_set_bool(cpuobj, true, "pmu", NULL);
> + }
> +
> if (object_property_find(cpuobj, "reset-cbar", NULL)) {
> object_property_set_int(cpuobj, vbi->memmap[VIRT_CPUPERIPHS].base,
> "reset-cbar", &error_abort);
> @@ -1514,6 +1520,9 @@ static void virt_2_7_instance_init(Object *obj)
>
> static void virt_machine_2_7_options(MachineClass *mc)
> {
> + VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> +
> + vmc->pmu_default_on = true;
> }
> DEFINE_VIRT_MACHINE_AS_LATEST(2, 7)
>
> @@ -1532,5 +1541,9 @@ static void virt_machine_2_6_options(MachineClass *mc)
> virt_machine_2_7_options(mc);
> SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_6);
> vmc->disallow_affinity_adjustment = true;
> + /* Disable PMU for 2.6 and down as PMU support was first introduced
> + * and enabled in 2.7.
> + */
> + vmc->pmu_default_on = false;
> }
> DEFINE_VIRT_MACHINE(2, 6)
> --
> 1.8.3.1
>
>
Otherwise,
Reviewed-by: Andrew Jones <drjones@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU
2016-09-15 5:04 [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU Wei Huang
2016-09-15 5:04 ` [Qemu-devel] [PATCH V3 1/2] arm64: Add an option to turn on/off vPMU support Wei Huang
2016-09-15 5:04 ` [Qemu-devel] [PATCH V3 2/2] arm: virt: add PMU property to mach-virt machine type Wei Huang
@ 2016-09-16 12:29 ` Andrea Bolognani
2016-09-16 16:04 ` Wei Huang
2 siblings, 1 reply; 7+ messages in thread
From: Andrea Bolognani @ 2016-09-16 12:29 UTC (permalink / raw)
To: Wei Huang, qemu-arm; +Cc: qemu-devel, peter.maydell, drjones, shannon.zhao
On Thu, 2016-09-15 at 01:04 -0400, Wei Huang wrote:
> This patchset adds a pmu=[on/off] option to enable/disable vPMU support
> for guest VM. There are several reasons to justify this option. First,
> vPMU can be problematic for cross-migration between different SoC as perf
> counters are architecture-dependent. It is more flexible to have an option
> to turn it on/off. Secondly Secondly this option matches the "pmu" option
> as supported in libvirt. To make sure backward compatible, a PMU property
> is added to mach-virt machine types.
[...]
> Wei Huang (2):
> arm64: Add an option to turn on/off vPMU support
> arm: virt: add PMU property to mach-virt machine type
>
> hw/arm/virt-acpi-build.c | 2 +-
> hw/arm/virt.c | 15 ++++++++++++++-
> target-arm/cpu.c | 22 ++++++++++++++++++++++
> target-arm/cpu.h | 1 +
> target-arm/cpu64.c | 2 ++
> target-arm/kvm64.c | 19 +++++++++++++++----
> 6 files changed, 55 insertions(+), 6 deletions(-)
This doesn't seem to be working :(
My guest configuration looks like
<domain type='kvm'>
<name>abologna-rhel73</name>
<uuid>78f74104-65f2-4faf-9faa-26107c1071ef</uuid>
<memory unit='KiB'>2097152</memory>
<currentMemory unit='KiB'>2097152</currentMemory>
<vcpu placement='static'>4</vcpu>
<os>
<type arch='aarch64' machine='virt-2.7'>hvm</type>
<loader readonly='yes' type='pflash'>/usr/share/AAVMF/AAVMF_CODE.fd</loader>
<nvram>/var/lib/libvirt/qemu/nvram/abologna-rhel73_VARS.fd</nvram>
<boot dev='hd'/>
</os>
<features>
<pmu state='off'/>
<gic version='2'/>
</features>
<cpu mode='host-passthrough'/>
<clock offset='utc'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>restart</on_crash>
<devices>
<emulator>/usr/libexec/abologna-qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='/var/lib/libvirt/images/abologna-rhel73.qcow2'/>
<target dev='vda' bus='virtio'/>
<address type='virtio-mmio'/>
</disk>
<controller type='pci' index='0' model='pcie-root'/>
<controller type='virtio-serial' index='0'>
<address type='virtio-mmio'/>
</controller>
<interface type='network'>
<mac address='52:54:00:57:76:ad'/>
<source network='default'/>
<model type='virtio'/>
<address type='virtio-mmio'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<channel type='unix'>
<target type='virtio' name='org.qemu.guest_agent.0'/>
<address type='virtio-serial' controller='0' bus='0' port='1'/>
</channel>
</devices>
</domain>
and based on that configuration, libvirt creates a QEMU
command line that looks like
LC_ALL=C \
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin \
QEMU_AUDIO_DRV=none \
/usr/libexec/abologna-qemu-kvm \
-name guest=abologna-rhel73,debug-threads=on \
-S \
-object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-5-abologna-rhel73/master-key.aes \
-machine virt-2.7,accel=kvm,usb=off \
-cpu host,pmu=off \
-drive file=/usr/share/AAVMF/AAVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
-drive file=/var/lib/libvirt/qemu/nvram/abologna-rhel73_VARS.fd,if=pflash,format=raw,unit=1 \
-m 2048 \
-realtime mlock=off \
-smp 4,sockets=4,cores=1,threads=1 \
-uuid 78f74104-65f2-4faf-9faa-26107c1071ef \
-nographic \
-no-user-config \
-nodefaults \
-chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-5-abologna-rhel73/monitor.sock,server,nowait \
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=utc \
-no-shutdown \
-boot strict=on \
-device virtio-serial-device,id=virtio-serial0 \
-drive file=/var/lib/libvirt/images/abologna-rhel73.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \
-device virtio-blk-device,scsi=off,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 \
-netdev tap,fd=25,id=hostnet0,vhost=on,vhostfd=27 \
-device virtio-net-device,netdev=hostnet0,id=net0,mac=52:54:00:57:76:ad \
-serial pty \
-chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/domain-5-abologna-rhel73/org.qemu.guest_agent.0,server,nowait \
-device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0 \
-msg timestamp=on
Inside the guest, however, even with pmu=off, I can still see
the PMU being initialized during boot
# dmesg | grep -i pmu
[ 0.000000] ACPI-PMU: Assign CPU 0 girq 23 level 0
[ 0.000000] ACPI-PMU: Assign CPU 1 girq 23 level 0
[ 0.000000] ACPI-PMU: Assign CPU 2 girq 23 level 0
[ 0.000000] ACPI-PMU: Assign CPU 3 girq 23 level 0
[ 0.087277] ACPI-PMU: Setting up 4 PMUs for CPU type 0
[ 0.576255] hw perfevents: enabled with armv8_pmuv3 PMU driver,
5 counters available
and perf can list the counters
# perf list | grep Hardware
branch-misses [Hardware event]
cache-misses [Hardware event]
cache-references [Hardware event]
cpu-cycles OR cycles [Hardware event]
instructions [Hardware event]
L1-dcache-load-misses [Hardware cache event]
L1-dcache-loads [Hardware cache event]
L1-dcache-store-misses [Hardware cache event]
L1-dcache-stores [Hardware cache event]
branch-load-misses [Hardware cache event]
branch-loads [Hardware cache event]
mem:<addr>[/len][:access] [Hardware breakpoint]
and use them
# perf stat true
Performance counter stats for 'true':
0.610960 task-clock (msec) # 0.102 CPUs utilized
0 context-switches # 0.000 K/sec
0 cpu-migrations # 0.000 K/sec
24 page-faults # 0.039 M/sec
1,383,676 cycles # 2.265 GHz
<not supported> stalled-cycles-frontend
<not supported> stalled-cycles-backend
396,335 instructions # 0.29 insns per cycle
<not supported> branches
5,536 branch-misses # 0.00% of all branches
0.006013060 seconds time elapsed
same way it can when using pmu=on or not passing any pmu=
value to QEMU.
I've tested this both on Moonshot and on ThunderX, the
behavior is the same.
Am I doing something wrong?
--
Andrea Bolognani / Red Hat / Virtualization
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU
2016-09-16 12:29 ` [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU Andrea Bolognani
@ 2016-09-16 16:04 ` Wei Huang
0 siblings, 0 replies; 7+ messages in thread
From: Wei Huang @ 2016-09-16 16:04 UTC (permalink / raw)
To: Andrea Bolognani, qemu-arm
Cc: qemu-devel, peter.maydell, drjones, shannon.zhao
On 09/16/2016 07:29 AM, Andrea Bolognani wrote:
> On Thu, 2016-09-15 at 01:04 -0400, Wei Huang wrote:
>> This patchset adds a pmu=[on/off] option to enable/disable vPMU support
>> for guest VM. There are several reasons to justify this option. First,
>> vPMU can be problematic for cross-migration between different SoC as perf
>> counters are architecture-dependent. It is more flexible to have an option
>> to turn it on/off. Secondly Secondly this option matches the "pmu" option
>> as supported in libvirt. To make sure backward compatible, a PMU property
>> is added to mach-virt machine types.
>
> [...]
>
>> Wei Huang (2):
>> arm64: Add an option to turn on/off vPMU support
>> arm: virt: add PMU property to mach-virt machine type
>>
>> hw/arm/virt-acpi-build.c | 2 +-
>> hw/arm/virt.c | 15 ++++++++++++++-
>> target-arm/cpu.c | 22 ++++++++++++++++++++++
>> target-arm/cpu.h | 1 +
>> target-arm/cpu64.c | 2 ++
>> target-arm/kvm64.c | 19 +++++++++++++++----
>> 6 files changed, 55 insertions(+), 6 deletions(-)
>
> This doesn't seem to be working :(
>
> My guest configuration looks like
>
> <domain type='kvm'>
> <name>abologna-rhel73</name>
> <uuid>78f74104-65f2-4faf-9faa-26107c1071ef</uuid>
> <memory unit='KiB'>2097152</memory>
> <currentMemory unit='KiB'>2097152</currentMemory>
> <vcpu placement='static'>4</vcpu>
> <os>
> <type arch='aarch64' machine='virt-2.7'>hvm</type>
> <loader readonly='yes' type='pflash'>/usr/share/AAVMF/AAVMF_CODE.fd</loader>
> <nvram>/var/lib/libvirt/qemu/nvram/abologna-rhel73_VARS.fd</nvram>
> <boot dev='hd'/>
> </os>
> <features>
> <pmu state='off'/>
> <gic version='2'/>
> </features>
> <cpu mode='host-passthrough'/>
> <clock offset='utc'/>
> <on_poweroff>destroy</on_poweroff>
> <on_reboot>restart</on_reboot>
> <on_crash>restart</on_crash>
> <devices>
> <emulator>/usr/libexec/abologna-qemu-kvm</emulator>
> <disk type='file' device='disk'>
> <driver name='qemu' type='qcow2'/>
> <source file='/var/lib/libvirt/images/abologna-rhel73.qcow2'/>
> <target dev='vda' bus='virtio'/>
> <address type='virtio-mmio'/>
> </disk>
> <controller type='pci' index='0' model='pcie-root'/>
> <controller type='virtio-serial' index='0'>
> <address type='virtio-mmio'/>
> </controller>
> <interface type='network'>
> <mac address='52:54:00:57:76:ad'/>
> <source network='default'/>
> <model type='virtio'/>
> <address type='virtio-mmio'/>
> </interface>
> <serial type='pty'>
> <target port='0'/>
> </serial>
> <console type='pty'>
> <target type='serial' port='0'/>
> </console>
> <channel type='unix'>
> <target type='virtio' name='org.qemu.guest_agent.0'/>
> <address type='virtio-serial' controller='0' bus='0' port='1'/>
> </channel>
> </devices>
> </domain>
>
> and based on that configuration, libvirt creates a QEMU
> command line that looks like
>
> LC_ALL=C \
> PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin \
> QEMU_AUDIO_DRV=none \
> /usr/libexec/abologna-qemu-kvm \
> -name guest=abologna-rhel73,debug-threads=on \
> -S \
> -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-5-abologna-rhel73/master-key.aes \
> -machine virt-2.7,accel=kvm,usb=off \
> -cpu host,pmu=off \
> -drive file=/usr/share/AAVMF/AAVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
> -drive file=/var/lib/libvirt/qemu/nvram/abologna-rhel73_VARS.fd,if=pflash,format=raw,unit=1 \
> -m 2048 \
> -realtime mlock=off \
> -smp 4,sockets=4,cores=1,threads=1 \
> -uuid 78f74104-65f2-4faf-9faa-26107c1071ef \
> -nographic \
> -no-user-config \
> -nodefaults \
> -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-5-abologna-rhel73/monitor.sock,server,nowait \
> -mon chardev=charmonitor,id=monitor,mode=control \
> -rtc base=utc \
> -no-shutdown \
> -boot strict=on \
> -device virtio-serial-device,id=virtio-serial0 \
> -drive file=/var/lib/libvirt/images/abologna-rhel73.qcow2,format=qcow2,if=none,id=drive-virtio-disk0 \
> -device virtio-blk-device,scsi=off,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 \
> -netdev tap,fd=25,id=hostnet0,vhost=on,vhostfd=27 \
> -device virtio-net-device,netdev=hostnet0,id=net0,mac=52:54:00:57:76:ad \
> -serial pty \
> -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/domain-5-abologna-rhel73/org.qemu.guest_agent.0,server,nowait \
> -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0 \
> -msg timestamp=on
>
> Inside the guest, however, even with pmu=off, I can still see
> the PMU being initialized during boot
>
> # dmesg | grep -i pmu
> [ 0.000000] ACPI-PMU: Assign CPU 0 girq 23 level 0
> [ 0.000000] ACPI-PMU: Assign CPU 1 girq 23 level 0
> [ 0.000000] ACPI-PMU: Assign CPU 2 girq 23 level 0
> [ 0.000000] ACPI-PMU: Assign CPU 3 girq 23 level 0
> [ 0.087277] ACPI-PMU: Setting up 4 PMUs for CPU type 0
> [ 0.576255] hw perfevents: enabled with armv8_pmuv3 PMU driver,
> 5 counters available
>
> and perf can list the counters
>
> # perf list | grep Hardware
> branch-misses [Hardware event]
> cache-misses [Hardware event]
> cache-references [Hardware event]
> cpu-cycles OR cycles [Hardware event]
> instructions [Hardware event]
> L1-dcache-load-misses [Hardware cache event]
> L1-dcache-loads [Hardware cache event]
> L1-dcache-store-misses [Hardware cache event]
> L1-dcache-stores [Hardware cache event]
> branch-load-misses [Hardware cache event]
> branch-loads [Hardware cache event]
> mem:<addr>[/len][:access] [Hardware breakpoint]
>
> and use them
>
> # perf stat true
>
> Performance counter stats for 'true':
>
> 0.610960 task-clock (msec) # 0.102 CPUs utilized
> 0 context-switches # 0.000 K/sec
> 0 cpu-migrations # 0.000 K/sec
> 24 page-faults # 0.039 M/sec
> 1,383,676 cycles # 2.265 GHz
> <not supported> stalled-cycles-frontend
> <not supported> stalled-cycles-backend
> 396,335 instructions # 0.29 insns per cycle
> <not supported> branches
> 5,536 branch-misses # 0.00% of all branches
>
> 0.006013060 seconds time elapsed
>
> same way it can when using pmu=on or not passing any pmu=
> value to QEMU.
>
> I've tested this both on Moonshot and on ThunderX, the
> behavior is the same.
>
> Am I doing something wrong?
I can replicate the problem. To answer your question: No, I think it is
a bug caused in this patch. Following code path, it turns out that
machvirt_init() is called after ARMCPU->has_pmu property has been
configured. So in your example, machvirt->pmu_default_on will overwrite
the ARMCPU->has_pmu value to be TRUE (always).
Even worse, our desired PMU setting can't be fully covered by a Boolean
ARMCPU->has_pmu field because machvirt_init() can't tell ARMCPU->has_pmu
is default-OFF or forced-OFF. Here is the matrix of one example:
ARMCPU->has_pmu VMC->pmu_default_on vPMU
-M virt-2.7 -cpu host default-OFF TRUE ON
-M virt-2.7 -cpu host,pmu=off forced-OFF TRUE OFF*
-M virt-2.7 -cpu host,pmu=on forced-ON TRUE ON
-M virt-2.6 -cpu host default-OFF FALSE OFF
-M virt-2.6 -cpu host,pmu=off forced-OFF FALSE OFF
-M virt-2.6 -cpu host,pmu=on forced-ON FALSE ON
-M virt-2.8+ -cpu host default-OFF FALSE OFF
-M virt-2.8+ -cpu host,pmu=off forced-OFF FALSE OFF
-M virt-2.8+ -cpu host,pmu=on forced-ON FALSE ON
The problematic one is the second line (*). To solve the problem, we
might want to change ARMCPU->has_pmu to an int (or enum).
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-09-16 16:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-15 5:04 [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU Wei Huang
2016-09-15 5:04 ` [Qemu-devel] [PATCH V3 1/2] arm64: Add an option to turn on/off vPMU support Wei Huang
2016-09-15 7:13 ` Andrew Jones
2016-09-15 5:04 ` [Qemu-devel] [PATCH V3 2/2] arm: virt: add PMU property to mach-virt machine type Wei Huang
2016-09-15 7:18 ` Andrew Jones
2016-09-16 12:29 ` [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU Andrea Bolognani
2016-09-16 16:04 ` Wei Huang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).