[PATCH v3 0/5] target/arm/kvm: Report PMU unavailability

[PATCH v3 0/5] target/arm/kvm: Report PMU unavailability

[Akihiko Odaki]

target/arm/kvm.c checked PMU availability but claimed PMU is
available even if it is not. In fact, Asahi Linux supports KVM but lacks
PMU support. Only advertise PMU availability only when it is really
available.

Fixes: dc40d45ebd8e ("target/arm/kvm: Move kvm_arm_get_host_cpu_features and unexport")

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Changes in v3:
- Dropped patch "target/arm: Do not allow setting 'pmu' for hvf".
- Dropped patch "target/arm: Allow setting 'pmu' only for host and max".
- Dropped patch "target/arm/kvm: Report PMU unavailability".
- Added patch "target/arm/kvm: Fix PMU feature bit early".
- Added patch "hvf: arm: Do not advance PC when raising an exception".
- Added patch "hvf: arm: Properly disable PMU".
- Changed to check for Armv8 before adding PMU property.
- Link to v2: https://lore.kernel.org/r/20240716-pmu-v2-0-f3e3e4b2d3d5@daynix.com

Changes in v2:
- Restricted writes to 'pmu' to host and max.
- Prohibited writes to 'pmu' for hvf.
- Link to v1: https://lore.kernel.org/r/20240629-pmu-v1-0-7269123b88a4@daynix.com

---
Akihiko Odaki (5):
      tests/arm-cpu-features: Do not assume PMU availability
      target/arm/kvm: Fix PMU feature bit early
      target/arm: Always add pmu property for Armv8
      hvf: arm: Do not advance PC when raising an exception
      hvf: arm: Properly disable PMU

 target/arm/cpu.c               |   3 +-
 target/arm/hvf/hvf.c           | 318 +++++++++++++++++++++--------------------
 target/arm/kvm.c               |   7 +-
 tests/qtest/arm-cpu-features.c |  13 +-
 4 files changed, 175 insertions(+), 166 deletions(-)
---
base-commit: f2cb4026fccfe073f84a4b440e41d3ed0c3134f6
change-id: 20240629-pmu-ad5f67e2c5d0

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>

[PATCH v3 1/5] tests/arm-cpu-features: Do not assume PMU availability

[Akihiko Odaki]

Asahi Linux supports KVM but lacks PMU support.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/arm-cpu-features.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index 966c65d5c3e4..cfd6f7735354 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -509,6 +509,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
     assert_set_feature(qts, "host", "kvm-no-adjvtime", false);
 
     if (g_str_equal(qtest_get_arch(), "aarch64")) {
+        bool kvm_supports_pmu;
         bool kvm_supports_steal_time;
         bool kvm_supports_sve;
         char max_name[8], name[8];
@@ -537,11 +538,6 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
 
         assert_has_feature_enabled(qts, "host", "aarch64");
 
-        /* Enabling and disabling pmu should always work. */
-        assert_has_feature_enabled(qts, "host", "pmu");
-        assert_set_feature(qts, "host", "pmu", false);
-        assert_set_feature(qts, "host", "pmu", true);
-
         /*
          * Some features would be enabled by default, but they're disabled
          * because this instance of KVM doesn't support them. Test that the
@@ -551,11 +547,18 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
         assert_has_feature(qts, "host", "sve");
 
         resp = do_query_no_props(qts, "host");
+        kvm_supports_pmu = resp_get_feature(resp, "pmu");
         kvm_supports_steal_time = resp_get_feature(resp, "kvm-steal-time");
         kvm_supports_sve = resp_get_feature(resp, "sve");
         vls = resp_get_sve_vls(resp);
         qobject_unref(resp);
 
+        if (kvm_supports_pmu) {
+            /* If we have pmu then we should be able to toggle it. */
+            assert_set_feature(qts, "host", "pmu", false);
+            assert_set_feature(qts, "host", "pmu", true);
+        }
+
         if (kvm_supports_steal_time) {
             /* If we have steal-time then we should be able to toggle it. */
             assert_set_feature(qts, "host", "kvm-steal-time", false);

-- 
2.45.2

[PATCH v3 2/5] target/arm/kvm: Fix PMU feature bit early

[Akihiko Odaki]

kvm_arm_get_host_cpu_features() used to add the PMU feature
unconditionally, and kvm_arch_init_vcpu() removed it when it is actually
not available. Conditionally add the PMU feature in
kvm_arm_get_host_cpu_features() to save code.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 target/arm/kvm.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 70f79eda33cd..849e2e21b304 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -280,6 +280,7 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
     if (kvm_arm_pmu_supported()) {
         init.features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
         pmu_supported = true;
+        features |= 1ULL << ARM_FEATURE_PMU;
     }
 
     if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
@@ -448,7 +449,6 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
     features |= 1ULL << ARM_FEATURE_V8;
     features |= 1ULL << ARM_FEATURE_NEON;
     features |= 1ULL << ARM_FEATURE_AARCH64;
-    features |= 1ULL << ARM_FEATURE_PMU;
     features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
 
     ahcf->features = features;
@@ -1888,13 +1888,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
     if (!arm_feature(env, ARM_FEATURE_AARCH64)) {
         cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT;
     }
-    if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) {
-        cpu->has_pmu = false;
-    }
     if (cpu->has_pmu) {
         cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
-    } else {
-        env->features &= ~(1ULL << ARM_FEATURE_PMU);
     }
     if (cpu_isar_feature(aa64_sve, cpu)) {
         assert(kvm_arm_sve_supported());

-- 
2.45.2

[PATCH v3 3/5] target/arm: Always add pmu property for Armv8

[Akihiko Odaki]

kvm-steal-time and sve properties are added for KVM even if the
corresponding features are not available. Always add pmu property for
Armv8. Note that the property is added only for Armv8 as QEMU emulates
PMUv3, which is part of Armv8.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 target/arm/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 14d4eca12740..64038e26b2a9 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1744,6 +1744,8 @@ void arm_cpu_post_init(Object *obj)
     }
 
     if (arm_feature(&cpu->env, ARM_FEATURE_V8)) {
+        object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
+
         object_property_add_uint64_ptr(obj, "rvbar",
                                        &cpu->rvbar_prop,
                                        OBJ_PROP_FLAG_READWRITE);
@@ -1770,7 +1772,6 @@ void arm_cpu_post_init(Object *obj)
 
     if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
         cpu->has_pmu = true;
-        object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
     }
 
     /*

-- 
2.45.2

[PATCH v3 4/5] hvf: arm: Do not advance PC when raising an exception

[Akihiko Odaki]

hvf did not advance PC when raising an exception for most unhandled
system registers, but it mistakenly advanced PC when raising an
exception for GICv3 registers.

Fixes: a2260983c655 ("hvf: arm: Add support for GICv3")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 target/arm/hvf/hvf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index ef9bc42738d0..eb090e67a2f8 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1278,6 +1278,7 @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
         /* Call the TCG sysreg handler. This is only safe for GICv3 regs. */
         if (!hvf_sysreg_read_cp(cpu, reg, &val)) {
             hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
+            return 1;
         }
         break;
     case SYSREG_DBGBVR0_EL1:

-- 
2.45.2

[PATCH v3 5/5] hvf: arm: Properly disable PMU

[Akihiko Odaki]

Setting pmu property used to have no effect for hvf so fix it.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 target/arm/hvf/hvf.c | 317 ++++++++++++++++++++++++++-------------------------
 1 file changed, 163 insertions(+), 154 deletions(-)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index eb090e67a2f8..7c593c2d93de 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1199,57 +1199,23 @@ static bool hvf_sysreg_read_cp(CPUState *cpu, uint32_t reg, uint64_t *val)
     return false;
 }
 
-static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
+static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt,
+                           uint64_t *val)
 {
     ARMCPU *arm_cpu = ARM_CPU(cpu);
     CPUARMState *env = &arm_cpu->env;
-    uint64_t val = 0;
 
     switch (reg) {
     case SYSREG_CNTPCT_EL0:
-        val = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
-              gt_cntfrq_period_ns(arm_cpu);
-        break;
-    case SYSREG_PMCR_EL0:
-        val = env->cp15.c9_pmcr;
-        break;
-    case SYSREG_PMCCNTR_EL0:
-        pmu_op_start(env);
-        val = env->cp15.c15_ccnt;
-        pmu_op_finish(env);
-        break;
-    case SYSREG_PMCNTENCLR_EL0:
-        val = env->cp15.c9_pmcnten;
-        break;
-    case SYSREG_PMOVSCLR_EL0:
-        val = env->cp15.c9_pmovsr;
-        break;
-    case SYSREG_PMSELR_EL0:
-        val = env->cp15.c9_pmselr;
-        break;
-    case SYSREG_PMINTENCLR_EL1:
-        val = env->cp15.c9_pminten;
-        break;
-    case SYSREG_PMCCFILTR_EL0:
-        val = env->cp15.pmccfiltr_el0;
-        break;
-    case SYSREG_PMCNTENSET_EL0:
-        val = env->cp15.c9_pmcnten;
-        break;
-    case SYSREG_PMUSERENR_EL0:
-        val = env->cp15.c9_pmuserenr;
-        break;
-    case SYSREG_PMCEID0_EL0:
-    case SYSREG_PMCEID1_EL0:
-        /* We can't really count anything yet, declare all events invalid */
-        val = 0;
-        break;
+        *val = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
+               gt_cntfrq_period_ns(arm_cpu);
+        return 0;
     case SYSREG_OSLSR_EL1:
-        val = env->cp15.oslsr_el1;
-        break;
+        *val = env->cp15.oslsr_el1;
+        return 0;
     case SYSREG_OSDLR_EL1:
         /* Dummy register */
-        break;
+        return 0;
     case SYSREG_ICC_AP0R0_EL1:
     case SYSREG_ICC_AP0R1_EL1:
     case SYSREG_ICC_AP0R2_EL1:
@@ -1276,11 +1242,11 @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
     case SYSREG_ICC_SRE_EL1:
     case SYSREG_ICC_CTLR_EL1:
         /* Call the TCG sysreg handler. This is only safe for GICv3 regs. */
-        if (!hvf_sysreg_read_cp(cpu, reg, &val)) {
+        if (!hvf_sysreg_read_cp(cpu, reg, val)) {
             hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
             return 1;
         }
-        break;
+        return 0;
     case SYSREG_DBGBVR0_EL1:
     case SYSREG_DBGBVR1_EL1:
     case SYSREG_DBGBVR2_EL1:
@@ -1297,8 +1263,8 @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
     case SYSREG_DBGBVR13_EL1:
     case SYSREG_DBGBVR14_EL1:
     case SYSREG_DBGBVR15_EL1:
-        val = env->cp15.dbgbvr[SYSREG_CRM(reg)];
-        break;
+        *val = env->cp15.dbgbvr[SYSREG_CRM(reg)];
+        return 0;
     case SYSREG_DBGBCR0_EL1:
     case SYSREG_DBGBCR1_EL1:
     case SYSREG_DBGBCR2_EL1:
@@ -1315,8 +1281,8 @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
     case SYSREG_DBGBCR13_EL1:
     case SYSREG_DBGBCR14_EL1:
     case SYSREG_DBGBCR15_EL1:
-        val = env->cp15.dbgbcr[SYSREG_CRM(reg)];
-        break;
+        *val = env->cp15.dbgbcr[SYSREG_CRM(reg)];
+        return 0;
     case SYSREG_DBGWVR0_EL1:
     case SYSREG_DBGWVR1_EL1:
     case SYSREG_DBGWVR2_EL1:
@@ -1333,8 +1299,8 @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
     case SYSREG_DBGWVR13_EL1:
     case SYSREG_DBGWVR14_EL1:
     case SYSREG_DBGWVR15_EL1:
-        val = env->cp15.dbgwvr[SYSREG_CRM(reg)];
-        break;
+        *val = env->cp15.dbgwvr[SYSREG_CRM(reg)];
+        return 0;
     case SYSREG_DBGWCR0_EL1:
     case SYSREG_DBGWCR1_EL1:
     case SYSREG_DBGWCR2_EL1:
@@ -1351,35 +1317,64 @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
     case SYSREG_DBGWCR13_EL1:
     case SYSREG_DBGWCR14_EL1:
     case SYSREG_DBGWCR15_EL1:
-        val = env->cp15.dbgwcr[SYSREG_CRM(reg)];
-        break;
-    default:
-        if (is_id_sysreg(reg)) {
-            /* ID system registers read as RES0 */
-            val = 0;
-            break;
+        *val = env->cp15.dbgwcr[SYSREG_CRM(reg)];
+        return 0;
+    }
+
+    if (arm_feature(env, ARM_FEATURE_PMU)) {
+        switch (reg) {
+        case SYSREG_PMCR_EL0:
+            *val = env->cp15.c9_pmcr;
+            return 0;
+        case SYSREG_PMCCNTR_EL0:
+            pmu_op_start(env);
+            *val = env->cp15.c15_ccnt;
+            pmu_op_finish(env);
+            return 0;
+        case SYSREG_PMCNTENCLR_EL0:
+            *val = env->cp15.c9_pmcnten;
+            return 0;
+        case SYSREG_PMOVSCLR_EL0:
+            *val = env->cp15.c9_pmovsr;
+            return 0;
+        case SYSREG_PMSELR_EL0:
+            *val = env->cp15.c9_pmselr;
+            return 0;
+        case SYSREG_PMINTENCLR_EL1:
+            *val = env->cp15.c9_pminten;
+            return 0;
+        case SYSREG_PMCCFILTR_EL0:
+            *val = env->cp15.pmccfiltr_el0;
+            return 0;
+        case SYSREG_PMCNTENSET_EL0:
+            *val = env->cp15.c9_pmcnten;
+            return 0;
+        case SYSREG_PMUSERENR_EL0:
+            *val = env->cp15.c9_pmuserenr;
+            return 0;
+        case SYSREG_PMCEID0_EL0:
+        case SYSREG_PMCEID1_EL0:
+            /* We can't really count anything yet, declare all events invalid */
+            *val = 0;
+            return 0;
         }
-        cpu_synchronize_state(cpu);
-        trace_hvf_unhandled_sysreg_read(env->pc, reg,
-                                        SYSREG_OP0(reg),
-                                        SYSREG_OP1(reg),
-                                        SYSREG_CRN(reg),
-                                        SYSREG_CRM(reg),
-                                        SYSREG_OP2(reg));
-        hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
-        return 1;
-    }
-
-    trace_hvf_sysreg_read(reg,
-                          SYSREG_OP0(reg),
-                          SYSREG_OP1(reg),
-                          SYSREG_CRN(reg),
-                          SYSREG_CRM(reg),
-                          SYSREG_OP2(reg),
-                          val);
-    hvf_set_reg(cpu, rt, val);
+    }
 
-    return 0;
+    if (is_id_sysreg(reg)) {
+        /* ID system registers read as RES0 */
+        *val = 0;
+        return 0;
+    }
+
+    cpu_synchronize_state(cpu);
+    trace_hvf_unhandled_sysreg_read(env->pc, reg,
+                                    SYSREG_OP0(reg),
+                                    SYSREG_OP1(reg),
+                                    SYSREG_CRN(reg),
+                                    SYSREG_CRM(reg),
+                                    SYSREG_OP2(reg));
+    hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
+    return 1;
 }
 
 static void pmu_update_irq(CPUARMState *env)
@@ -1499,69 +1494,12 @@ static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
                            val);
 
     switch (reg) {
-    case SYSREG_PMCCNTR_EL0:
-        pmu_op_start(env);
-        env->cp15.c15_ccnt = val;
-        pmu_op_finish(env);
-        break;
-    case SYSREG_PMCR_EL0:
-        pmu_op_start(env);
-
-        if (val & PMCRC) {
-            /* The counter has been reset */
-            env->cp15.c15_ccnt = 0;
-        }
-
-        if (val & PMCRP) {
-            unsigned int i;
-            for (i = 0; i < pmu_num_counters(env); i++) {
-                env->cp15.c14_pmevcntr[i] = 0;
-            }
-        }
-
-        env->cp15.c9_pmcr &= ~PMCR_WRITABLE_MASK;
-        env->cp15.c9_pmcr |= (val & PMCR_WRITABLE_MASK);
-
-        pmu_op_finish(env);
-        break;
-    case SYSREG_PMUSERENR_EL0:
-        env->cp15.c9_pmuserenr = val & 0xf;
-        break;
-    case SYSREG_PMCNTENSET_EL0:
-        env->cp15.c9_pmcnten |= (val & pmu_counter_mask(env));
-        break;
-    case SYSREG_PMCNTENCLR_EL0:
-        env->cp15.c9_pmcnten &= ~(val & pmu_counter_mask(env));
-        break;
-    case SYSREG_PMINTENCLR_EL1:
-        pmu_op_start(env);
-        env->cp15.c9_pminten |= val;
-        pmu_op_finish(env);
-        break;
-    case SYSREG_PMOVSCLR_EL0:
-        pmu_op_start(env);
-        env->cp15.c9_pmovsr &= ~val;
-        pmu_op_finish(env);
-        break;
-    case SYSREG_PMSWINC_EL0:
-        pmu_op_start(env);
-        pmswinc_write(env, val);
-        pmu_op_finish(env);
-        break;
-    case SYSREG_PMSELR_EL0:
-        env->cp15.c9_pmselr = val & 0x1f;
-        break;
-    case SYSREG_PMCCFILTR_EL0:
-        pmu_op_start(env);
-        env->cp15.pmccfiltr_el0 = val & PMCCFILTR_EL0;
-        pmu_op_finish(env);
-        break;
     case SYSREG_OSLAR_EL1:
         env->cp15.oslsr_el1 = val & 1;
-        break;
+        return 0;
     case SYSREG_OSDLR_EL1:
         /* Dummy register */
-        break;
+        return 0;
     case SYSREG_ICC_AP0R0_EL1:
     case SYSREG_ICC_AP0R1_EL1:
     case SYSREG_ICC_AP0R2_EL1:
@@ -1591,10 +1529,10 @@ static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
         if (!hvf_sysreg_write_cp(cpu, reg, val)) {
             hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
         }
-        break;
+        return 0;
     case SYSREG_MDSCR_EL1:
         env->cp15.mdscr_el1 = val;
-        break;
+        return 0;
     case SYSREG_DBGBVR0_EL1:
     case SYSREG_DBGBVR1_EL1:
     case SYSREG_DBGBVR2_EL1:
@@ -1612,7 +1550,7 @@ static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
     case SYSREG_DBGBVR14_EL1:
     case SYSREG_DBGBVR15_EL1:
         env->cp15.dbgbvr[SYSREG_CRM(reg)] = val;
-        break;
+        return 0;
     case SYSREG_DBGBCR0_EL1:
     case SYSREG_DBGBCR1_EL1:
     case SYSREG_DBGBCR2_EL1:
@@ -1630,7 +1568,7 @@ static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
     case SYSREG_DBGBCR14_EL1:
     case SYSREG_DBGBCR15_EL1:
         env->cp15.dbgbcr[SYSREG_CRM(reg)] = val;
-        break;
+        return 0;
     case SYSREG_DBGWVR0_EL1:
     case SYSREG_DBGWVR1_EL1:
     case SYSREG_DBGWVR2_EL1:
@@ -1648,7 +1586,7 @@ static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
     case SYSREG_DBGWVR14_EL1:
     case SYSREG_DBGWVR15_EL1:
         env->cp15.dbgwvr[SYSREG_CRM(reg)] = val;
-        break;
+        return 0;
     case SYSREG_DBGWCR0_EL1:
     case SYSREG_DBGWCR1_EL1:
     case SYSREG_DBGWCR2_EL1:
@@ -1666,20 +1604,80 @@ static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val)
     case SYSREG_DBGWCR14_EL1:
     case SYSREG_DBGWCR15_EL1:
         env->cp15.dbgwcr[SYSREG_CRM(reg)] = val;
-        break;
-    default:
-        cpu_synchronize_state(cpu);
-        trace_hvf_unhandled_sysreg_write(env->pc, reg,
-                                         SYSREG_OP0(reg),
-                                         SYSREG_OP1(reg),
-                                         SYSREG_CRN(reg),
-                                         SYSREG_CRM(reg),
-                                         SYSREG_OP2(reg));
-        hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
-        return 1;
+        return 0;
     }
 
-    return 0;
+    if (arm_feature(env, ARM_FEATURE_PMU)) {
+        switch (reg) {
+        case SYSREG_PMCCNTR_EL0:
+            pmu_op_start(env);
+            env->cp15.c15_ccnt = val;
+            pmu_op_finish(env);
+            return 0;
+        case SYSREG_PMCR_EL0:
+            pmu_op_start(env);
+
+            if (val & PMCRC) {
+                /* The counter has been reset */
+                env->cp15.c15_ccnt = 0;
+            }
+
+            if (val & PMCRP) {
+                unsigned int i;
+                for (i = 0; i < pmu_num_counters(env); i++) {
+                    env->cp15.c14_pmevcntr[i] = 0;
+                }
+            }
+
+            env->cp15.c9_pmcr &= ~PMCR_WRITABLE_MASK;
+            env->cp15.c9_pmcr |= (val & PMCR_WRITABLE_MASK);
+
+            pmu_op_finish(env);
+            return 0;
+        case SYSREG_PMUSERENR_EL0:
+            env->cp15.c9_pmuserenr = val & 0xf;
+            return 0;
+        case SYSREG_PMCNTENSET_EL0:
+            env->cp15.c9_pmcnten |= (val & pmu_counter_mask(env));
+            return 0;
+        case SYSREG_PMCNTENCLR_EL0:
+            env->cp15.c9_pmcnten &= ~(val & pmu_counter_mask(env));
+            return 0;
+        case SYSREG_PMINTENCLR_EL1:
+            pmu_op_start(env);
+            env->cp15.c9_pminten |= val;
+            pmu_op_finish(env);
+            return 0;
+        case SYSREG_PMOVSCLR_EL0:
+            pmu_op_start(env);
+            env->cp15.c9_pmovsr &= ~val;
+            pmu_op_finish(env);
+            return 0;
+        case SYSREG_PMSWINC_EL0:
+            pmu_op_start(env);
+            pmswinc_write(env, val);
+            pmu_op_finish(env);
+            return 0;
+        case SYSREG_PMSELR_EL0:
+            env->cp15.c9_pmselr = val & 0x1f;
+            return 0;
+        case SYSREG_PMCCFILTR_EL0:
+            pmu_op_start(env);
+            env->cp15.pmccfiltr_el0 = val & PMCCFILTR_EL0;
+            pmu_op_finish(env);
+            return 0;
+        }
+    }
+
+    cpu_synchronize_state(cpu);
+    trace_hvf_unhandled_sysreg_write(env->pc, reg,
+                                     SYSREG_OP0(reg),
+                                     SYSREG_OP1(reg),
+                                     SYSREG_CRN(reg),
+                                     SYSREG_CRM(reg),
+                                     SYSREG_OP2(reg));
+    hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
+    return 1;
 }
 
 static int hvf_inject_interrupts(CPUState *cpu)
@@ -1944,7 +1942,18 @@ int hvf_vcpu_exec(CPUState *cpu)
         int sysreg_ret = 0;
 
         if (isread) {
-            sysreg_ret = hvf_sysreg_read(cpu, reg, rt);
+            sysreg_ret = hvf_sysreg_read(cpu, reg, rt, &val);
+
+            if (!sysreg_ret) {
+                trace_hvf_sysreg_read(reg,
+                                      SYSREG_OP0(reg),
+                                      SYSREG_OP1(reg),
+                                      SYSREG_CRN(reg),
+                                      SYSREG_CRM(reg),
+                                      SYSREG_OP2(reg),
+                                      val);
+                hvf_set_reg(cpu, rt, val);
+            }
         } else {
             val = hvf_get_reg(cpu, rt);
             sysreg_ret = hvf_sysreg_write(cpu, reg, val);

-- 
2.45.2

Re: [PATCH v3 2/5] target/arm/kvm: Fix PMU feature bit early

[Peter Maydell]

On Tue, 16 Jul 2024 at 13:50, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> kvm_arm_get_host_cpu_features() used to add the PMU feature
> unconditionally, and kvm_arch_init_vcpu() removed it when it is actually
> not available. Conditionally add the PMU feature in
> kvm_arm_get_host_cpu_features() to save code.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  target/arm/kvm.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 70f79eda33cd..849e2e21b304 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -280,6 +280,7 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>      if (kvm_arm_pmu_supported()) {
>          init.features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
>          pmu_supported = true;
> +        features |= 1ULL << ARM_FEATURE_PMU;
>      }
>
>      if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
> @@ -448,7 +449,6 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>      features |= 1ULL << ARM_FEATURE_V8;
>      features |= 1ULL << ARM_FEATURE_NEON;
>      features |= 1ULL << ARM_FEATURE_AARCH64;
> -    features |= 1ULL << ARM_FEATURE_PMU;
>      features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
>
>      ahcf->features = features;
> @@ -1888,13 +1888,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>      if (!arm_feature(env, ARM_FEATURE_AARCH64)) {
>          cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT;
>      }
> -    if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) {
> -        cpu->has_pmu = false;
> -    }
>      if (cpu->has_pmu) {
>          cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
> -    } else {
> -        env->features &= ~(1ULL << ARM_FEATURE_PMU);
>      }
>      if (cpu_isar_feature(aa64_sve, cpu)) {
>          assert(kvm_arm_sve_supported());

Not every KVM CPU is necessarily the "host" CPU type.
The "cortex-a57" and "cortex-a53" CPU types will work if you
happen to be on a host of that CPU type, and they don't go
through kvm_arm_get_host_cpu_features().

(Also, at some point in the future we're probably going to
want to support "tell the guest it has CPU type X via the
ID registers even when the host is CPU type Y". It seems
plausible that in that case also we'll end up wanting this
there too. But I don't put much weight on this because there's
probably a bunch of things we'll need to fix up if and when
we eventually try to implement this.)

thanks
-- PMM

Re: [PATCH v3 3/5] target/arm: Always add pmu property for Armv8

[Peter Maydell]

On Tue, 16 Jul 2024 at 13:50, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> kvm-steal-time and sve properties are added for KVM even if the
> corresponding features are not available. Always add pmu property for
> Armv8. Note that the property is added only for Armv8 as QEMU emulates
> PMUv3, which is part of Armv8.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  target/arm/cpu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 14d4eca12740..64038e26b2a9 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1744,6 +1744,8 @@ void arm_cpu_post_init(Object *obj)
>      }
>
>      if (arm_feature(&cpu->env, ARM_FEATURE_V8)) {
> +        object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
> +
>          object_property_add_uint64_ptr(obj, "rvbar",
>                                         &cpu->rvbar_prop,
>                                         OBJ_PROP_FLAG_READWRITE);
> @@ -1770,7 +1772,6 @@ void arm_cpu_post_init(Object *obj)
>
>      if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
>          cpu->has_pmu = true;
> -        object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
>      }

This regresses the ability to disable the PMU emulation on
CPUs like "cortex-a8", which are not v8 but still set
ARM_FEATURE_PMU.

thanks
-- PMM

Re: [PATCH v3 5/5] hvf: arm: Properly disable PMU

[Peter Maydell]

On Tue, 16 Jul 2024 at 13:51, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> Setting pmu property used to have no effect for hvf so fix it.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  target/arm/hvf/hvf.c | 317 ++++++++++++++++++++++++++-------------------------
>  1 file changed, 163 insertions(+), 154 deletions(-)

This patch is doing too much stuff at once. If you want to
change the API of hvf_sysreg_read(), please do that in its
own refactoring patch so it's easier to review.

thanks
-- PMM

Re: [PATCH v3 0/5] target/arm/kvm: Report PMU unavailability

[Peter Maydell]

On Tue, 16 Jul 2024 at 13:50, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> target/arm/kvm.c checked PMU availability but claimed PMU is
> available even if it is not. In fact, Asahi Linux supports KVM but lacks
> PMU support. Only advertise PMU availability only when it is really
> available.
>
> Fixes: dc40d45ebd8e ("target/arm/kvm: Move kvm_arm_get_host_cpu_features and unexport")
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> Changes in v3:
> - Dropped patch "target/arm: Do not allow setting 'pmu' for hvf".
> - Dropped patch "target/arm: Allow setting 'pmu' only for host and max".
> - Dropped patch "target/arm/kvm: Report PMU unavailability".
> - Added patch "target/arm/kvm: Fix PMU feature bit early".
> - Added patch "hvf: arm: Do not advance PC when raising an exception".
> - Added patch "hvf: arm: Properly disable PMU".
> - Changed to check for Armv8 before adding PMU property.
> - Link to v2: https://lore.kernel.org/r/20240716-pmu-v2-0-f3e3e4b2d3d5@daynix.com
>
> Changes in v2:
> - Restricted writes to 'pmu' to host and max.
> - Prohibited writes to 'pmu' for hvf.
> - Link to v1: https://lore.kernel.org/r/20240629-pmu-v1-0-7269123b88a4@daynix.com
>
> ---
> Akihiko Odaki (5):
>       tests/arm-cpu-features: Do not assume PMU availability
>       target/arm/kvm: Fix PMU feature bit early
>       target/arm: Always add pmu property for Armv8
>       hvf: arm: Do not advance PC when raising an exception
>       hvf: arm: Properly disable PMU

Hi; I've left reviews for some of these patches. I'm going to
apply "hvf: arm: Do not advance PC when raising an exception"
to my target-arm queue since I'm about to do a pullreq for
9.1 softfreeze.

thanks
-- PMM

Re: [PATCH v3 0/5] target/arm/kvm: Report PMU unavailability

[Peter Maydell]

On Thu, 18 Jul 2024 at 13:14, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 16 Jul 2024 at 13:50, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> >
> > target/arm/kvm.c checked PMU availability but claimed PMU is
> > available even if it is not. In fact, Asahi Linux supports KVM but lacks
> > PMU support. Only advertise PMU availability only when it is really
> > available.
> >
> > Fixes: dc40d45ebd8e ("target/arm/kvm: Move kvm_arm_get_host_cpu_features and unexport")
> >
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > ---
> > Changes in v3:
> > - Dropped patch "target/arm: Do not allow setting 'pmu' for hvf".
> > - Dropped patch "target/arm: Allow setting 'pmu' only for host and max".
> > - Dropped patch "target/arm/kvm: Report PMU unavailability".
> > - Added patch "target/arm/kvm: Fix PMU feature bit early".
> > - Added patch "hvf: arm: Do not advance PC when raising an exception".
> > - Added patch "hvf: arm: Properly disable PMU".
> > - Changed to check for Armv8 before adding PMU property.
> > - Link to v2: https://lore.kernel.org/r/20240716-pmu-v2-0-f3e3e4b2d3d5@daynix.com
> >
> > Changes in v2:
> > - Restricted writes to 'pmu' to host and max.
> > - Prohibited writes to 'pmu' for hvf.
> > - Link to v1: https://lore.kernel.org/r/20240629-pmu-v1-0-7269123b88a4@daynix.com
> >
> > ---
> > Akihiko Odaki (5):
> >       tests/arm-cpu-features: Do not assume PMU availability
> >       target/arm/kvm: Fix PMU feature bit early
> >       target/arm: Always add pmu property for Armv8
> >       hvf: arm: Do not advance PC when raising an exception
> >       hvf: arm: Properly disable PMU
>
> Hi; I've left reviews for some of these patches. I'm going to
> apply "hvf: arm: Do not advance PC when raising an exception"
> to my target-arm queue since I'm about to do a pullreq for
> 9.1 softfreeze.

...and I'll take patch 1 ("tests/arm-cpu-features: Do not assume PMU
availability") too, since it's been reviewed.

-- PMM

Re: [PATCH v3 2/5] target/arm/kvm: Fix PMU feature bit early

[Akihiko Odaki]

On 2024/07/18 21:07, Peter Maydell wrote:
> On Tue, 16 Jul 2024 at 13:50, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>
>> kvm_arm_get_host_cpu_features() used to add the PMU feature
>> unconditionally, and kvm_arch_init_vcpu() removed it when it is actually
>> not available. Conditionally add the PMU feature in
>> kvm_arm_get_host_cpu_features() to save code.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   target/arm/kvm.c | 7 +------
>>   1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>> index 70f79eda33cd..849e2e21b304 100644
>> --- a/target/arm/kvm.c
>> +++ b/target/arm/kvm.c
>> @@ -280,6 +280,7 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>       if (kvm_arm_pmu_supported()) {
>>           init.features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
>>           pmu_supported = true;
>> +        features |= 1ULL << ARM_FEATURE_PMU;
>>       }
>>
>>       if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
>> @@ -448,7 +449,6 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>       features |= 1ULL << ARM_FEATURE_V8;
>>       features |= 1ULL << ARM_FEATURE_NEON;
>>       features |= 1ULL << ARM_FEATURE_AARCH64;
>> -    features |= 1ULL << ARM_FEATURE_PMU;
>>       features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
>>
>>       ahcf->features = features;
>> @@ -1888,13 +1888,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>       if (!arm_feature(env, ARM_FEATURE_AARCH64)) {
>>           cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT;
>>       }
>> -    if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) {
>> -        cpu->has_pmu = false;
>> -    }
>>       if (cpu->has_pmu) {
>>           cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
>> -    } else {
>> -        env->features &= ~(1ULL << ARM_FEATURE_PMU);
>>       }
>>       if (cpu_isar_feature(aa64_sve, cpu)) {
>>           assert(kvm_arm_sve_supported());
> 
> Not every KVM CPU is necessarily the "host" CPU type.
> The "cortex-a57" and "cortex-a53" CPU types will work if you
> happen to be on a host of that CPU type, and they don't go
> through kvm_arm_get_host_cpu_features().

kvm_arm_vcpu_init() will emit an error in such a situation and I think 
it's better than silently removing a feature that the requested CPU type 
has. A user can still disable the feature if desired.

Regards,
Akihiko Odaki

Re: [PATCH v3 2/5] target/arm/kvm: Fix PMU feature bit early

[Cornelia Huck]

On Fri, Jul 19 2024, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:

> On 2024/07/18 21:07, Peter Maydell wrote:
>> On Tue, 16 Jul 2024 at 13:50, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>>
>>> kvm_arm_get_host_cpu_features() used to add the PMU feature
>>> unconditionally, and kvm_arch_init_vcpu() removed it when it is actually
>>> not available. Conditionally add the PMU feature in
>>> kvm_arm_get_host_cpu_features() to save code.
>>>
>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> ---
>>>   target/arm/kvm.c | 7 +------
>>>   1 file changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>>> index 70f79eda33cd..849e2e21b304 100644
>>> --- a/target/arm/kvm.c
>>> +++ b/target/arm/kvm.c
>>> @@ -280,6 +280,7 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>>       if (kvm_arm_pmu_supported()) {
>>>           init.features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
>>>           pmu_supported = true;
>>> +        features |= 1ULL << ARM_FEATURE_PMU;
>>>       }
>>>
>>>       if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
>>> @@ -448,7 +449,6 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>>       features |= 1ULL << ARM_FEATURE_V8;
>>>       features |= 1ULL << ARM_FEATURE_NEON;
>>>       features |= 1ULL << ARM_FEATURE_AARCH64;
>>> -    features |= 1ULL << ARM_FEATURE_PMU;
>>>       features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
>>>
>>>       ahcf->features = features;
>>> @@ -1888,13 +1888,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>>       if (!arm_feature(env, ARM_FEATURE_AARCH64)) {
>>>           cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT;
>>>       }
>>> -    if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) {
>>> -        cpu->has_pmu = false;
>>> -    }
>>>       if (cpu->has_pmu) {
>>>           cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
>>> -    } else {
>>> -        env->features &= ~(1ULL << ARM_FEATURE_PMU);
>>>       }
>>>       if (cpu_isar_feature(aa64_sve, cpu)) {
>>>           assert(kvm_arm_sve_supported());
>> 
>> Not every KVM CPU is necessarily the "host" CPU type.
>> The "cortex-a57" and "cortex-a53" CPU types will work if you
>> happen to be on a host of that CPU type, and they don't go
>> through kvm_arm_get_host_cpu_features().
>
> kvm_arm_vcpu_init() will emit an error in such a situation and I think 
> it's better than silently removing a feature that the requested CPU type 
> has. A user can still disable the feature if desired.

OTOH, if we fail for the named cpu models if the kernel does not provide
the cap, but silently disable for the host cpu model in that case, that
also seems inconsistent. I'd rather keep it as it is now.

Re: [PATCH v3 2/5] target/arm/kvm: Fix PMU feature bit early

[Akihiko Odaki]

On 2024/07/19 21:21, Cornelia Huck wrote:
> On Fri, Jul 19 2024, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> 
>> On 2024/07/18 21:07, Peter Maydell wrote:
>>> On Tue, 16 Jul 2024 at 13:50, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>>>
>>>> kvm_arm_get_host_cpu_features() used to add the PMU feature
>>>> unconditionally, and kvm_arch_init_vcpu() removed it when it is actually
>>>> not available. Conditionally add the PMU feature in
>>>> kvm_arm_get_host_cpu_features() to save code.
>>>>
>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>> ---
>>>>    target/arm/kvm.c | 7 +------
>>>>    1 file changed, 1 insertion(+), 6 deletions(-)
>>>>
>>>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>>>> index 70f79eda33cd..849e2e21b304 100644
>>>> --- a/target/arm/kvm.c
>>>> +++ b/target/arm/kvm.c
>>>> @@ -280,6 +280,7 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>>>        if (kvm_arm_pmu_supported()) {
>>>>            init.features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
>>>>            pmu_supported = true;
>>>> +        features |= 1ULL << ARM_FEATURE_PMU;
>>>>        }
>>>>
>>>>        if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, &init)) {
>>>> @@ -448,7 +449,6 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>>>        features |= 1ULL << ARM_FEATURE_V8;
>>>>        features |= 1ULL << ARM_FEATURE_NEON;
>>>>        features |= 1ULL << ARM_FEATURE_AARCH64;
>>>> -    features |= 1ULL << ARM_FEATURE_PMU;
>>>>        features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
>>>>
>>>>        ahcf->features = features;
>>>> @@ -1888,13 +1888,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>>>        if (!arm_feature(env, ARM_FEATURE_AARCH64)) {
>>>>            cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT;
>>>>        }
>>>> -    if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) {
>>>> -        cpu->has_pmu = false;
>>>> -    }
>>>>        if (cpu->has_pmu) {
>>>>            cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
>>>> -    } else {
>>>> -        env->features &= ~(1ULL << ARM_FEATURE_PMU);
>>>>        }
>>>>        if (cpu_isar_feature(aa64_sve, cpu)) {
>>>>            assert(kvm_arm_sve_supported());
>>>
>>> Not every KVM CPU is necessarily the "host" CPU type.
>>> The "cortex-a57" and "cortex-a53" CPU types will work if you
>>> happen to be on a host of that CPU type, and they don't go
>>> through kvm_arm_get_host_cpu_features().
>>
>> kvm_arm_vcpu_init() will emit an error in such a situation and I think
>> it's better than silently removing a feature that the requested CPU type
>> has. A user can still disable the feature if desired.
> 
> OTOH, if we fail for the named cpu models if the kernel does not provide
> the cap, but silently disable for the host cpu model in that case, that
> also seems inconsistent. I'd rather keep it as it is now.

There are two perspectives of consistency:
1) The initial value of pmu
2) The behavior with the pmu value

This change introduces inconsistency for 1); the host cpu model will 
have pmu=off by default and the other cpu models will keep default 
pmu=on value on a system that does not support PMU. It still keeps 
consistency for 2); it fails if the user sets pmu=on for any cpu model 
on such a system.

We should align 1) for better consistency, but I don't think such a 
change would be useful. It is likely that something is wrong with the 
system when the system reports a cpu model but it doesn't support its 
feature. I think that is the reason why we assert 
kvm_arm_sve_supported() for SVE; however I don't think such an assertion 
would help either because kvm_arm_vcpu_init() will fail anyway.

Regards,
Akihiko Odaki