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

[Qemu-devel] [PATCH V6 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 this option matches the "pmu" option as
supported in libvirt. To make sure backward compatible, a PMU-related
property is added to mach-virt machine types.

The following are testing results with this patchset. Other combinations
should have similar results:
  CONFIG (qemu-system-aarch64)                       vPMU   WARNING
  -M virt-2.8/virt,accel=kvm -cpu host               NO     NO
  -M virt-2.8/virt,accel=kvm -cpu host,pmu=off       NO     NO
  -M virt-2.8/virt,accel=kvm -cpu host,pmu=on        YES    NO
  -M virt-2.7,accel=kvm -cpu host                    YES    NO
  -M virt-2.7,accel=kvm -cpu host,pmu=off            NO     NO
  -M virt-2.7,accel=kvm -cpu host,pmu=on             YES    NO
  -M virt-2.6,accel=kvm -cpu host                    NO     NO
  -M virt-2.6,accel=kvm -cpu host,pmu=off            NO     NO
  -M virt-2.6,accel=kvm -cpu host,pmu=on             YES    NO

  -M virt-2.8/virt,accel=tcg -cpu cortex-a57         NO     NO
  -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=off NO     NO
  -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=on  NO     "No KVM"
  -M virt-2.7,accel=tcg -cpu cortex-a57              NO     NO
  -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=off      NO     NO
  -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=on       NO     "No KVM"
  -M virt-2.6,accel=tcg -cpu cortex-a57              NO     NO
  -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=off      NO     NO
  -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=on       NO     "No KVM"

  -M virt-2.8/virt,accel=tcg -cpu cortex-a15         NO     NO
  -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=off NO     "No PMU property"
  -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=on  NO     "No PMU property"
  -M virt-2.7,accel=tcg -cpu cortex-a15              NO     NO
  -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=off      NO     "No PMU property"
  -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=on       NO     "No PMU property"
  -M virt-2.6,accel=tcg -cpu cortex-a15              NO     NO
  -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=off      NO     "No PMU property"
  -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=on       NO     "No PMU property"

  * "NO KVM" msg
    warning: pmu can't be enabled without KVM acceleration
  * "No PMU property" msg
    can't apply global cortex-a15-arm-cpu.pmu=off: Property '.pmu' not found

V5->V6:
  * adapt patches for new machine type 2.8

V4->V5:
  * remove comment change for has_pmu
  * remove warning msg when pmu_default_on=TRUE && has_pmu=AUTO && tcg=TRUE

V3->V4:
  * change has_pmu from Boolean to OnOffAuto to handle different cases
  * "pmu" property is re-defined as DEFINE_PROP_ON_OFF_AUTO

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.

Thanks,
-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            | 19 ++++++++++++++++++-
 target-arm/cpu.c         | 23 +++++++++++++++++++++++
 target-arm/cpu.h         |  3 ++-
 target-arm/cpu64.c       |  2 ++
 target-arm/kvm64.c       | 17 ++++++++++++++---
 6 files changed, 60 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH V6 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, if nothing specified,
the pmu option is set to AUTO as default, allowing machine-level PMU
property to override it. Also when pmu is turned on under non-KVM mode,
a warning message will be printed.

Signed-off-by: Wei Huang <wei@redhat.com>
---
 hw/arm/virt-acpi-build.c |  2 +-
 hw/arm/virt.c            |  2 +-
 target-arm/cpu.c         | 23 +++++++++++++++++++++++
 target-arm/cpu.h         |  3 ++-
 target-arm/cpu64.c       |  2 ++
 target-arm/kvm64.c       | 17 ++++++++++++++---
 6 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 7b39b1d..bf4a0b8 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -541,7 +541,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 0f6305d..d348b02 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -490,7 +490,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 1b9540e..01e8506 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"
@@ -31,6 +32,7 @@
 #include "hw/arm/arm.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
+#include "sysemu/qtest.h"
 #include "kvm_arm.h"
 
 static void arm_cpu_set_pc(CPUState *cs, vaddr value)
@@ -509,6 +511,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_ON_OFF_AUTO("pmu", ARMCPU, has_pmu, ON_OFF_AUTO_AUTO);
+
 static Property arm_cpu_has_mpu_property =
             DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
 
@@ -552,6 +558,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 +587,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 +660,17 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         cpu->id_aa64pfr0 &= ~0xf000;
     }
 
+    if (cpu->has_pmu == ON_OFF_AUTO_ON && !kvm_enabled()) {
+        cpu->has_pmu = ON_OFF_AUTO_OFF;
+        if (!pmu_warned && !qtest_enabled()) {
+            error_report("warning: pmu can't be enabled without KVM acceleration");
+            pmu_warned = true;
+        }
+    }
+    if (cpu->has_pmu == ON_OFF_AUTO_OFF) {
+        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..a2de272 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -580,7 +580,7 @@ struct ARMCPU {
     /* CPU has security extension */
     bool has_el3;
     /* CPU has PMU (Performance Monitor Unit) */
-    bool has_pmu;
+    OnOffAuto has_pmu;
 
     /* CPU has memory protection unit */
     bool has_mpu;
@@ -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..ff86550 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;
+    if (!kvm_irqchip_in_kernel() ||
+        !kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) {
+            cpu->has_pmu = ON_OFF_AUTO_OFF;
+    }
+    if (cpu->has_pmu == ON_OFF_AUTO_ON) {
         cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
+    } else {
+        unset_feature(&env->features, ARM_FEATURE_PMU);
     }
 
     /* Do KVM_ARM_VCPU_INIT ioctl */
-- 
1.8.3.1

[Qemu-devel] [PATCH V6 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 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d348b02..43832f3 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 {
@@ -1322,6 +1323,8 @@ static void machvirt_init(MachineState *machine)
 
     for (n = 0; n < smp_cpus; n++) {
         Object *cpuobj = object_new(typename);
+        ARMCPU *cpu = ARM_CPU(cpuobj);
+
         if (!vmc->disallow_affinity_adjustment) {
             /* Adjust MPIDR like 64-bit KVM hosts, which incorporate the
              * GIC's target-list limitations. 32-bit KVM hosts currently
@@ -1352,6 +1355,12 @@ static void machvirt_init(MachineState *machine)
             }
         }
 
+        if (cpu->has_pmu == ON_OFF_AUTO_AUTO &&
+            object_property_find(cpuobj, "pmu", NULL)) {
+            cpu->has_pmu = vmc->pmu_default_on && kvm_enabled() ?
+                ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
+        }
+
         if (object_property_find(cpuobj, "reset-cbar", NULL)) {
             object_property_set_int(cpuobj, vbi->memmap[VIRT_CPUPERIPHS].base,
                                     "reset-cbar", &error_abort);
@@ -1562,8 +1571,12 @@ 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));
+
     virt_machine_2_8_options(mc);
     SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_7);
+
+    vmc->pmu_default_on = true;
 }
 DEFINE_VIRT_MACHINE(2, 7)
 
@@ -1582,5 +1595,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] [Qemu-arm] [PATCH V6 0/2] Add option to configure guest vPMU

[Wei Huang]

On 10/04/2016 04:38 PM, 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 this option matches the "pmu" option as
> supported in libvirt. To make sure backward compatible, a PMU-related
> property is added to mach-virt machine types.
> 
> The following are testing results with this patchset. Other combinations
> should have similar results:
>   CONFIG (qemu-system-aarch64)                       vPMU   WARNING
>   -M virt-2.8/virt,accel=kvm -cpu host               NO     NO
>   -M virt-2.8/virt,accel=kvm -cpu host,pmu=off       NO     NO
>   -M virt-2.8/virt,accel=kvm -cpu host,pmu=on        YES    NO

Hi Peter,

I just updated this patchset to comply with the recent addition of 2.8
machine type. Please let me know if you have any comments.

>   -M virt-2.7,accel=kvm -cpu host                    YES    NO
>   -M virt-2.7,accel=kvm -cpu host,pmu=off            NO     NO
>   -M virt-2.7,accel=kvm -cpu host,pmu=on             YES    NO
>   -M virt-2.6,accel=kvm -cpu host                    NO     NO
>   -M virt-2.6,accel=kvm -cpu host,pmu=off            NO     NO
>   -M virt-2.6,accel=kvm -cpu host,pmu=on             YES    NO
> 
>   -M virt-2.8/virt,accel=tcg -cpu cortex-a57         NO     NO
>   -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=off NO     NO
>   -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=on  NO     "No KVM"
>   -M virt-2.7,accel=tcg -cpu cortex-a57              NO     NO
>   -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=off      NO     NO
>   -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=on       NO     "No KVM"
>   -M virt-2.6,accel=tcg -cpu cortex-a57              NO     NO
>   -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=off      NO     NO
>   -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=on       NO     "No KVM"
> 
>   -M virt-2.8/virt,accel=tcg -cpu cortex-a15         NO     NO
>   -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=off NO     "No PMU property"
>   -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=on  NO     "No PMU property"
>   -M virt-2.7,accel=tcg -cpu cortex-a15              NO     NO
>   -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=off      NO     "No PMU property"
>   -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=on       NO     "No PMU property"
>   -M virt-2.6,accel=tcg -cpu cortex-a15              NO     NO
>   -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=off      NO     "No PMU property"
>   -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=on       NO     "No PMU property"
> 
>   * "NO KVM" msg
>     warning: pmu can't be enabled without KVM acceleration
>   * "No PMU property" msg
>     can't apply global cortex-a15-arm-cpu.pmu=off: Property '.pmu' not found
> 
> V5->V6:
>   * adapt patches for new machine type 2.8
> 
> V4->V5:
>   * remove comment change for has_pmu
>   * remove warning msg when pmu_default_on=TRUE && has_pmu=AUTO && tcg=TRUE
> 
> V3->V4:
>   * change has_pmu from Boolean to OnOffAuto to handle different cases
>   * "pmu" property is re-defined as DEFINE_PROP_ON_OFF_AUTO
> 
> 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.
> 
> Thanks,
> -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            | 19 ++++++++++++++++++-
>  target-arm/cpu.c         | 23 +++++++++++++++++++++++
>  target-arm/cpu.h         |  3 ++-
>  target-arm/cpu64.c       |  2 ++
>  target-arm/kvm64.c       | 17 ++++++++++++++---
>  6 files changed, 60 insertions(+), 6 deletions(-)
> 

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

[Andrew Jones]

On Tue, Oct 04, 2016 at 05:38:53PM -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, if nothing specified,
> the pmu option is set to AUTO as default, allowing machine-level PMU
> property to override it. Also when pmu is turned on under non-KVM mode,
> a warning message will be printed.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  hw/arm/virt-acpi-build.c |  2 +-
>  hw/arm/virt.c            |  2 +-
>  target-arm/cpu.c         | 23 +++++++++++++++++++++++
>  target-arm/cpu.h         |  3 ++-
>  target-arm/cpu64.c       |  2 ++
>  target-arm/kvm64.c       | 17 ++++++++++++++---
>  6 files changed, 43 insertions(+), 6 deletions(-)
>

Please leave r-b's on patches that don't change. afaict this one
didn't change, so [again]

Reviewed-by: Andrew Jones <drjones@redhat.com> 

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

[Andrew Jones]

On Tue, Oct 04, 2016 at 05:38:54PM -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 | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index d348b02..43832f3 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 {
> @@ -1322,6 +1323,8 @@ static void machvirt_init(MachineState *machine)
>  
>      for (n = 0; n < smp_cpus; n++) {
>          Object *cpuobj = object_new(typename);
> +        ARMCPU *cpu = ARM_CPU(cpuobj);
> +
>          if (!vmc->disallow_affinity_adjustment) {
>              /* Adjust MPIDR like 64-bit KVM hosts, which incorporate the
>               * GIC's target-list limitations. 32-bit KVM hosts currently
> @@ -1352,6 +1355,12 @@ static void machvirt_init(MachineState *machine)
>              }
>          }
>  
> +        if (cpu->has_pmu == ON_OFF_AUTO_AUTO &&
> +            object_property_find(cpuobj, "pmu", NULL)) {
> +            cpu->has_pmu = vmc->pmu_default_on && kvm_enabled() ?
> +                ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
> +        }
> +
>          if (object_property_find(cpuobj, "reset-cbar", NULL)) {
>              object_property_set_int(cpuobj, vbi->memmap[VIRT_CPUPERIPHS].base,
>                                      "reset-cbar", &error_abort);
> @@ -1562,8 +1571,12 @@ 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));
> +
>      virt_machine_2_8_options(mc);
>      SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_7);
> +

nit: no need for the above blank line

> +    vmc->pmu_default_on = true;
>  }
>  DEFINE_VIRT_MACHINE(2, 7)
>  
> @@ -1582,5 +1595,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
> 
>

Besides the nit

Reviewed-by: Andrew Jones <drjones@redhat.com> 

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

[Andrea Bolognani]

On Tue, 2016-10-04 at 17:38 -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, if nothing specified,
> the pmu option is set to AUTO as default, allowing machine-level PMU
> property to override it. Also when pmu is turned on under non-KVM mode,
> a warning message will be printed.
> 
> Signed-off-by: Wei Huang <wei@redhat.com>
> ---
>  hw/arm/virt-acpi-build.c |  2 +-
>  hw/arm/virt.c            |  2 +-
>  target-arm/cpu.c         | 23 +++++++++++++++++++++++
>  target-arm/cpu.h         |  3 ++-
>  target-arm/cpu64.c       |  2 ++
>  target-arm/kvm64.c       | 17 ++++++++++++++---
>  6 files changed, 43 insertions(+), 6 deletions(-)

On both Moonshot and ThunderX,

Tested-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[Andrea Bolognani]

On Tue, 2016-10-04 at 17:38 -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 | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

On both Moonshot and ThunderX,

Tested-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[Peter Maydell]

On 4 October 2016 at 22:38, Wei Huang <wei@redhat.com> 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 this option matches the "pmu" option as
> supported in libvirt. To make sure backward compatible, a PMU-related
> property is added to mach-virt machine types.
>
> The following are testing results with this patchset. Other combinations
> should have similar results:
>   CONFIG (qemu-system-aarch64)                       vPMU   WARNING
>   -M virt-2.8/virt,accel=kvm -cpu host               NO     NO

What's the rationale for defaulting to no-pmu? Other cpu features
we have tend to default to present unless you ask not to have them.

>   -M virt-2.8/virt,accel=kvm -cpu host,pmu=off       NO     NO
>   -M virt-2.8/virt,accel=kvm -cpu host,pmu=on        YES    NO
>   -M virt-2.7,accel=kvm -cpu host                    YES    NO
>   -M virt-2.7,accel=kvm -cpu host,pmu=off            NO     NO
>   -M virt-2.7,accel=kvm -cpu host,pmu=on             YES    NO

in virt-2.7 you always got a pmu regardless, so why does pmu=off here give
you no pmu?

>   -M virt-2.6,accel=kvm -cpu host                    NO     NO
>   -M virt-2.6,accel=kvm -cpu host,pmu=off            NO     NO
>   -M virt-2.6,accel=kvm -cpu host,pmu=on             YES    NO
>
>   -M virt-2.8/virt,accel=tcg -cpu cortex-a57         NO     NO
>   -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=off NO     NO
>   -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=on  NO     "No KVM"
>   -M virt-2.7,accel=tcg -cpu cortex-a57              NO     NO
>   -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=off      NO     NO
>   -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=on       NO     "No KVM"
>   -M virt-2.6,accel=tcg -cpu cortex-a57              NO     NO
>   -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=off      NO     NO
>   -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=on       NO     "No KVM"

This patchset doesn't actually change whether the pmu is present
or not under TCG so this is all a little bit odd.

>   -M virt-2.8/virt,accel=tcg -cpu cortex-a15         NO     NO
>   -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=off NO     "No PMU property"
>   -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=on  NO     "No PMU property"
>   -M virt-2.7,accel=tcg -cpu cortex-a15              NO     NO
>   -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=off      NO     "No PMU property"
>   -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=on       NO     "No PMU property"
>   -M virt-2.6,accel=tcg -cpu cortex-a15              NO     NO
>   -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=off      NO     "No PMU property"
>   -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=on       NO     "No PMU property"
>
>   * "NO KVM" msg
>     warning: pmu can't be enabled without KVM acceleration
>   * "No PMU property" msg
>     can't apply global cortex-a15-arm-cpu.pmu=off: Property '.pmu' not found

I'm really not a fan of the tristate has_pmu, so I'm hoping
there's a setup which lets us avoid it...

thanks
-- PMM

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

[Andrew Jones]

On Thu, Oct 13, 2016 at 05:19:37PM +0100, Peter Maydell wrote:
> On 4 October 2016 at 22:38, Wei Huang <wei@redhat.com> 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 this option matches the "pmu" option as
> > supported in libvirt. To make sure backward compatible, a PMU-related
> > property is added to mach-virt machine types.
> >
> > The following are testing results with this patchset. Other combinations
> > should have similar results:
> >   CONFIG (qemu-system-aarch64)                       vPMU   WARNING
> >   -M virt-2.8/virt,accel=kvm -cpu host               NO     NO
> 
> What's the rationale for defaulting to no-pmu? Other cpu features
> we have tend to default to present unless you ask not to have them.

The feature needs to be managed in order for users to be able to trade-off
features exposed vs. migratability. If the lack of an explicit pmu=on
means either you get a PMU, when it's available, or you don't, when it's
not, then managing the feature becomes more difficult. Also, libvirt
already manages this cpu property for x86, and there it defaults off.

Maybe we should revisit the other features that default on, but may
be turned off, to make sure they make sense. Can you point them out?
I took a quick look, but it appears the cpu property defaults are all
pretty much determined by vms->secure, which defaults to false.

> 
> >   -M virt-2.8/virt,accel=kvm -cpu host,pmu=off       NO     NO
> >   -M virt-2.8/virt,accel=kvm -cpu host,pmu=on        YES    NO
> >   -M virt-2.7,accel=kvm -cpu host                    YES    NO
> >   -M virt-2.7,accel=kvm -cpu host,pmu=off            NO     NO
> >   -M virt-2.7,accel=kvm -cpu host,pmu=on             YES    NO
> 
> in virt-2.7 you always got a pmu regardless, so why does pmu=off here give
> you no pmu?

Hmm, I think you're right that the property should just be a nop, or
even invalid (causing an error) for 2.7 and down.

> 
> >   -M virt-2.6,accel=kvm -cpu host                    NO     NO
> >   -M virt-2.6,accel=kvm -cpu host,pmu=off            NO     NO
> >   -M virt-2.6,accel=kvm -cpu host,pmu=on             YES    NO
> >
> >   -M virt-2.8/virt,accel=tcg -cpu cortex-a57         NO     NO
> >   -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=off NO     NO
> >   -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=on  NO     "No KVM"
> >   -M virt-2.7,accel=tcg -cpu cortex-a57              NO     NO
> >   -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=off      NO     NO
> >   -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=on       NO     "No KVM"
> >   -M virt-2.6,accel=tcg -cpu cortex-a57              NO     NO
> >   -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=off      NO     NO
> >   -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=on       NO     "No KVM"
> 
> This patchset doesn't actually change whether the pmu is present
> or not under TCG so this is all a little bit odd.

The goal here is to allow the property on the command line, i.e. not
error and fail, when using tcg. Otherwise more than just s/kvm/tcg/
would be required when switching, and accel=kvm:tcg wouldn't work either.

> 
> >   -M virt-2.8/virt,accel=tcg -cpu cortex-a15         NO     NO
> >   -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=off NO     "No PMU property"
> >   -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=on  NO     "No PMU property"
> >   -M virt-2.7,accel=tcg -cpu cortex-a15              NO     NO
> >   -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=off      NO     "No PMU property"
> >   -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=on       NO     "No PMU property"
> >   -M virt-2.6,accel=tcg -cpu cortex-a15              NO     NO
> >   -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=off      NO     "No PMU property"
> >   -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=on       NO     "No PMU property"
> >
> >   * "NO KVM" msg
> >     warning: pmu can't be enabled without KVM acceleration
> >   * "No PMU property" msg
> >     can't apply global cortex-a15-arm-cpu.pmu=off: Property '.pmu' not found
> 
> I'm really not a fan of the tristate has_pmu, so I'm hoping
> there's a setup which lets us avoid it...

Probably keeping PMU on no matter what for 2.7 and down will allow
the removal of the tristate.

Thanks,
drew

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

[Andrew Jones]

On Fri, Oct 14, 2016 at 09:48:28AM +0200, Andrew Jones wrote:
> On Thu, Oct 13, 2016 at 05:19:37PM +0100, Peter Maydell wrote:
> > On 4 October 2016 at 22:38, Wei Huang <wei@redhat.com> 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 this option matches the "pmu" option as
> > > supported in libvirt. To make sure backward compatible, a PMU-related
> > > property is added to mach-virt machine types.
> > >
> > > The following are testing results with this patchset. Other combinations
> > > should have similar results:
> > >   CONFIG (qemu-system-aarch64)                       vPMU   WARNING
> > >   -M virt-2.8/virt,accel=kvm -cpu host               NO     NO
> > 
> > What's the rationale for defaulting to no-pmu? Other cpu features
> > we have tend to default to present unless you ask not to have them.
> 
> The feature needs to be managed in order for users to be able to trade-off
> features exposed vs. migratability. If the lack of an explicit pmu=on
> means either you get a PMU, when it's available, or you don't, when it's
> not, then managing the feature becomes more difficult. Also, libvirt
> already manages this cpu property for x86, and there it defaults off.

We discussed this today with Andrea. He says libvirt doesn't mind the
default being on. x86 defaults pmu off, ppc defaults pmu on. I guess
we might as well stick with on, it should make things easier.

> 
> Maybe we should revisit the other features that default on, but may
> be turned off, to make sure they make sense. Can you point them out?
> I took a quick look, but it appears the cpu property defaults are all
> pretty much determined by vms->secure, which defaults to false.
> 
> > 
> > >   -M virt-2.8/virt,accel=kvm -cpu host,pmu=off       NO     NO
> > >   -M virt-2.8/virt,accel=kvm -cpu host,pmu=on        YES    NO
> > >   -M virt-2.7,accel=kvm -cpu host                    YES    NO
> > >   -M virt-2.7,accel=kvm -cpu host,pmu=off            NO     NO
> > >   -M virt-2.7,accel=kvm -cpu host,pmu=on             YES    NO
> > 
> > in virt-2.7 you always got a pmu regardless, so why does pmu=off here give
> > you no pmu?
> 
> Hmm, I think you're right that the property should just be a nop, or
> even invalid (causing an error) for 2.7 and down.

Hmm... a no-op property wouldn't be exactly compatible. An error message
stating the property isn't allowed would be. That's not how QEMU currently
handles compat though. compat is only supported for going from an older-
version host to a newer-version host (and back). In those cases the
command line is older-version compatible and works in both places.
Starting on a newer-version, even when using a compat machine type,
doesn't guarantee we can go to an older, because, for example, the command
line may include properties that don't exist there.

We could address this, if we really want. We'd need to keep the auto
tri-state to detect that the user set the property. If they did, then
we should complain and fail to start, never allowing virt-2.7
instantiations to have the pmu property on the command line. I don't
know if it's worth it though.

> 
> > 
> > >   -M virt-2.6,accel=kvm -cpu host                    NO     NO
> > >   -M virt-2.6,accel=kvm -cpu host,pmu=off            NO     NO
> > >   -M virt-2.6,accel=kvm -cpu host,pmu=on             YES    NO
> > >
> > >   -M virt-2.8/virt,accel=tcg -cpu cortex-a57         NO     NO
> > >   -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=off NO     NO
> > >   -M virt-2.8/virt,accel=tcg -cpu cortex-a57,pmu=on  NO     "No KVM"
> > >   -M virt-2.7,accel=tcg -cpu cortex-a57              NO     NO
> > >   -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=off      NO     NO
> > >   -M virt-2.7,accel=tcg -cpu cortex-a57,pmu=on       NO     "No KVM"
> > >   -M virt-2.6,accel=tcg -cpu cortex-a57              NO     NO
> > >   -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=off      NO     NO
> > >   -M virt-2.6,accel=tcg -cpu cortex-a57,pmu=on       NO     "No KVM"
> > 
> > This patchset doesn't actually change whether the pmu is present
> > or not under TCG so this is all a little bit odd.
> 
> The goal here is to allow the property on the command line, i.e. not
> error and fail, when using tcg. Otherwise more than just s/kvm/tcg/
> would be required when switching, and accel=kvm:tcg wouldn't work either.
> 
> > 
> > >   -M virt-2.8/virt,accel=tcg -cpu cortex-a15         NO     NO
> > >   -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=off NO     "No PMU property"
> > >   -M virt-2.8/virt,accel=tcg -cpu cortex-a15,pmu=on  NO     "No PMU property"
> > >   -M virt-2.7,accel=tcg -cpu cortex-a15              NO     NO
> > >   -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=off      NO     "No PMU property"
> > >   -M virt-2.7,accel=tcg -cpu cortex-a15,pmu=on       NO     "No PMU property"
> > >   -M virt-2.6,accel=tcg -cpu cortex-a15              NO     NO
> > >   -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=off      NO     "No PMU property"
> > >   -M virt-2.6,accel=tcg -cpu cortex-a15,pmu=on       NO     "No PMU property"
> > >
> > >   * "NO KVM" msg
> > >     warning: pmu can't be enabled without KVM acceleration
> > >   * "No PMU property" msg
> > >     can't apply global cortex-a15-arm-cpu.pmu=off: Property '.pmu' not found
> > 
> > I'm really not a fan of the tristate has_pmu, so I'm hoping
> > there's a setup which lets us avoid it...
> 
> Probably keeping PMU on no matter what for 2.7 and down will allow
> the removal of the tristate.

If we switch to default on, then the only way to turn it off is with
pmu=off added to the command line. So we'll know if pmu=off is given
and we can just ignore it for virt-2.7. If we don't care about keeping
the property off the virt-2.7 command line then there's no need for a
tri-state.

Thanks,
drew

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

[Peter Maydell]

On 18 October 2016 at 18:16, Andrew Jones <drjones@redhat.com> wrote:
> On Fri, Oct 14, 2016 at 09:48:28AM +0200, Andrew Jones wrote:
>> The feature needs to be managed in order for users to be able to trade-off
>> features exposed vs. migratability. If the lack of an explicit pmu=on
>> means either you get a PMU, when it's available, or you don't, when it's
>> not, then managing the feature becomes more difficult. Also, libvirt
>> already manages this cpu property for x86, and there it defaults off.
>
> We discussed this today with Andrea. He says libvirt doesn't mind the
> default being on. x86 defaults pmu off, ppc defaults pmu on. I guess
> we might as well stick with on, it should make things easier.

If we don't have a clear architectural consensus then I think we
should stick with 'default is on'. That seems less confusing than
"it was off, then in 2.7 it was on, then in 2.8 it's off again".

> If we switch to default on, then the only way to turn it off is with
> pmu=off added to the command line. So we'll know if pmu=off is given
> and we can just ignore it for virt-2.7. If we don't care about keeping
> the property off the virt-2.7 command line then there's no need for a
> tri-state.

I don't much mind what virt-2.7 does with pmu=anything, I
just don't want a tri-state, and particularly I don't want
a tri-state just to get a particular behaviour out of a
machine=virt-2.7 command line that wouldn't have worked on the
real 2.7 release.

So we should arrange it whichever way is most convenient, I think.

thanks
-- PMM