All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V4 0/2] Add option to configure guest vPMU
@ 2016-09-21  2:33 Wei Huang
  2016-09-21  2:33 ` [Qemu-devel] [PATCH V4 1/2] arm64: Add an option to turn on/off vPMU support Wei Huang
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Wei Huang @ 2016-09-21  2:33 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 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.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.7,accel=tcg -cpu cortex-a57            NO     "No KVM"
    -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.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

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

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH V4 1/2] arm64: Add an option to turn on/off vPMU support
  2016-09-21  2:33 [Qemu-devel] [PATCH V4 0/2] Add option to configure guest vPMU Wei Huang
@ 2016-09-21  2:33 ` Wei Huang
  2016-09-21  7:53   ` Andrew Jones
  2016-09-21  2:33 ` [Qemu-devel] [PATCH V4 2/2] arm: virt: add PMU property to mach-virt machine type Wei Huang
  2016-09-21  7:38 ` [Qemu-devel] [PATCH V4 0/2] Add option to configure guest vPMU Andrew Jones
  2 siblings, 1 reply; 8+ messages in thread
From: Wei Huang @ 2016-09-21  2:33 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, 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         |  5 +++--
 target-arm/cpu64.c       |  2 ++
 target-arm/kvm64.c       | 17 ++++++++++++++---
 6 files changed, 44 insertions(+), 7 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..ed7b884 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..c8625eb 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -579,8 +579,8 @@ struct ARMCPU {
     bool powered_off;
     /* CPU has security extension */
     bool has_el3;
-    /* CPU has PMU (Performance Monitor Unit) */
-    bool has_pmu;
+    /* CPU PMU (Performance Monitor Unit) config, tri-state */
+    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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH V4 2/2] arm: virt: add PMU property to mach-virt machine type
  2016-09-21  2:33 [Qemu-devel] [PATCH V4 0/2] Add option to configure guest vPMU Wei Huang
  2016-09-21  2:33 ` [Qemu-devel] [PATCH V4 1/2] arm64: Add an option to turn on/off vPMU support Wei Huang
@ 2016-09-21  2:33 ` Wei Huang
  2016-09-21  8:01   ` Andrew Jones
  2016-09-21  7:38 ` [Qemu-devel] [PATCH V4 0/2] Add option to configure guest vPMU Andrew Jones
  2 siblings, 1 reply; 8+ messages in thread
From: Wei Huang @ 2016-09-21  2:33 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 | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a781ad0..b25351c 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 {
@@ -1287,6 +1288,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
@@ -1317,6 +1320,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 ? 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);
@@ -1514,6 +1523,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 +1544,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] 8+ messages in thread

* Re: [Qemu-devel] [PATCH V4 0/2] Add option to configure guest vPMU
  2016-09-21  2:33 [Qemu-devel] [PATCH V4 0/2] Add option to configure guest vPMU Wei Huang
  2016-09-21  2:33 ` [Qemu-devel] [PATCH V4 1/2] arm64: Add an option to turn on/off vPMU support Wei Huang
  2016-09-21  2:33 ` [Qemu-devel] [PATCH V4 2/2] arm: virt: add PMU property to mach-virt machine type Wei Huang
@ 2016-09-21  7:38 ` Andrew Jones
  2 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2016-09-21  7:38 UTC (permalink / raw)
  To: Wei Huang; +Cc: qemu-arm, peter.maydell, qemu-devel, abologna, shannon.zhao

On Tue, Sep 20, 2016 at 10:33:52PM -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 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.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.7,accel=tcg -cpu cortex-a57            NO     "No KVM"

We don't want this one above to warn.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH V4 1/2] arm64: Add an option to turn on/off vPMU support
  2016-09-21  2:33 ` [Qemu-devel] [PATCH V4 1/2] arm64: Add an option to turn on/off vPMU support Wei Huang
@ 2016-09-21  7:53   ` Andrew Jones
  2016-09-21 13:35     ` Wei Huang
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Jones @ 2016-09-21  7:53 UTC (permalink / raw)
  To: Wei Huang; +Cc: qemu-arm, peter.maydell, qemu-devel, abologna, shannon.zhao

On Tue, Sep 20, 2016 at 10:33: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         |  5 +++--
>  target-arm/cpu64.c       |  2 ++
>  target-arm/kvm64.c       | 17 ++++++++++++++---
>  6 files changed, 44 insertions(+), 7 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..ed7b884 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()) {

Why do we need this qtest_enabled thing?

> +            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..c8625eb 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -579,8 +579,8 @@ struct ARMCPU {
>      bool powered_off;
>      /* CPU has security extension */
>      bool has_el3;
> -    /* CPU has PMU (Performance Monitor Unit) */
> -    bool has_pmu;
> +    /* CPU PMU (Performance Monitor Unit) config, tri-state */

The comment didn't need to be modified.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH V4 2/2] arm: virt: add PMU property to mach-virt machine type
  2016-09-21  2:33 ` [Qemu-devel] [PATCH V4 2/2] arm: virt: add PMU property to mach-virt machine type Wei Huang
@ 2016-09-21  8:01   ` Andrew Jones
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2016-09-21  8:01 UTC (permalink / raw)
  To: Wei Huang; +Cc: qemu-arm, peter.maydell, qemu-devel, abologna, shannon.zhao

On Tue, Sep 20, 2016 at 10:33: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 | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index a781ad0..b25351c 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 {
> @@ -1287,6 +1288,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
> @@ -1317,6 +1320,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 ? ON_OFF_AUTO_ON :
> +                ON_OFF_AUTO_OFF;

Here's where we fix the case I pointed out in the cover letter test
matrix. I think it should be

 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);
> @@ -1514,6 +1523,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 +1544,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
> 
>

Additionally, as I mentioned off-list, I think this series is a good
excuse to add the 2.8 machine type (but as a separate patch)

Thanks,
drew

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH V4 1/2] arm64: Add an option to turn on/off vPMU support
  2016-09-21  7:53   ` Andrew Jones
@ 2016-09-21 13:35     ` Wei Huang
  2016-09-21 13:51       ` Andrew Jones
  0 siblings, 1 reply; 8+ messages in thread
From: Wei Huang @ 2016-09-21 13:35 UTC (permalink / raw)
  To: Andrew Jones; +Cc: qemu-arm, peter.maydell, qemu-devel, abologna, shannon.zhao



On 09/21/2016 02:53 AM, Andrew Jones wrote:
> On Tue, Sep 20, 2016 at 10:33: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         |  5 +++--
>>  target-arm/cpu64.c       |  2 ++
>>  target-arm/kvm64.c       | 17 ++++++++++++++---
>>  6 files changed, 44 insertions(+), 7 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..ed7b884 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()) {
> 
> Why do we need this qtest_enabled thing?

Without this, "make check" will print out the warning message (using a
qtest machine type). This seems to be a common practice across QEMU code.

> 
>> +            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..c8625eb 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -579,8 +579,8 @@ struct ARMCPU {
>>      bool powered_off;
>>      /* CPU has security extension */
>>      bool has_el3;
>> -    /* CPU has PMU (Performance Monitor Unit) */
>> -    bool has_pmu;
>> +    /* CPU PMU (Performance Monitor Unit) config, tri-state */
> 
> The comment didn't need to be modified.
> 
>> +    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
>>
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH V4 1/2] arm64: Add an option to turn on/off vPMU support
  2016-09-21 13:35     ` Wei Huang
@ 2016-09-21 13:51       ` Andrew Jones
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2016-09-21 13:51 UTC (permalink / raw)
  To: Wei Huang
  Cc: qemu-arm, peter.maydell, qemu-devel, abologna, shannon.zhao, armbru

On Wed, Sep 21, 2016 at 08:35:41AM -0500, Wei Huang wrote:
> On 09/21/2016 02:53 AM, Andrew Jones wrote:
> > On Tue, Sep 20, 2016 at 10:33:53PM -0400, Wei Huang wrote:
[snip]
> >> +    if (cpu->has_pmu == ON_OFF_AUTO_ON && !kvm_enabled()) {
> >> +        cpu->has_pmu = ON_OFF_AUTO_OFF;
> >> +        if (!pmu_warned && !qtest_enabled()) {
> > 
> > Why do we need this qtest_enabled thing?
> 
> Without this, "make check" will print out the warning message (using a
> qtest machine type). This seems to be a common practice across QEMU code.

I see. Too bad it's so ugly. Maybe we need a new error_report that
hides that ugliness, i.e. an error_report that only reports when
!qtest_enabled(). CC'ing Markus to get his opinion, but I'm off topic
now and wouldn't expect anything like that to be worked out before
accepting this patch.

> 
> > 
> >> +            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);
> >> +    }
> >> +
[snip]

Thanks,
drew

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-09-21 13:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-21  2:33 [Qemu-devel] [PATCH V4 0/2] Add option to configure guest vPMU Wei Huang
2016-09-21  2:33 ` [Qemu-devel] [PATCH V4 1/2] arm64: Add an option to turn on/off vPMU support Wei Huang
2016-09-21  7:53   ` Andrew Jones
2016-09-21 13:35     ` Wei Huang
2016-09-21 13:51       ` Andrew Jones
2016-09-21  2:33 ` [Qemu-devel] [PATCH V4 2/2] arm: virt: add PMU property to mach-virt machine type Wei Huang
2016-09-21  8:01   ` Andrew Jones
2016-09-21  7:38 ` [Qemu-devel] [PATCH V4 0/2] Add option to configure guest vPMU Andrew Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.