[Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU

[Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU

[Wei Huang]

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

[Qemu-devel] [PATCH V3 1/2] arm64: Add an option to turn on/off vPMU support

[Wei Huang]

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

Re: [Qemu-devel] [PATCH V3 1/2] arm64: Add an option to turn on/off vPMU support

[Andrew Jones]

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
> 
> 

[Qemu-devel] [PATCH V3 2/2] arm: virt: add PMU property to mach-virt machine type

[Wei Huang]

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

Re: [Qemu-devel] [PATCH V3 2/2] arm: virt: add PMU property to mach-virt machine type

[Andrew Jones]

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>

Re: [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU

[Andrea Bolognani]

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

Re: [Qemu-devel] [PATCH V3 0/2] Add option to configure guest vPMU

[Wei Huang]

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
>