Skip to content

Commit 67bd641

Browse files
author
Marc Zyngier
committed
Merge branch kvm-arm64/nv-pmu-fixes into kvmarm-master/next
* kvm-arm64/nv-pmu-fixes: : . : Fixes for NV PMU emulation. From the cover letter: : : "Joey reports that some of his PMU tests do not behave quite as : expected: : : - MDCR_EL2.HPMN is set to 0 out of reset : : - PMCR_EL0.P should reset all the counters when written from EL2 : : Oliver points out that setting PMCR_EL0.N from userspace by writing to : the register is silly with NV, and that we need a new PMU attribute : instead. : : On top of that, I figured out that we had a number of little gotchas: : : - It is possible for a guest to write an HPMN value that is out of : bound, and it seems valuable to limit it : : - PMCR_EL0.N should be the maximum number of counters when read from : EL2, and MDCR_EL2.HPMN when read from EL0/EL1 : : - Prevent userspace from updating PMCR_EL0.N when EL2 is available" : . KVM: arm64: Let kvm_vcpu_read_pmcr() return an EL-dependent value for PMCR_EL0.N KVM: arm64: Handle out-of-bound write to MDCR_EL2.HPMN KVM: arm64: Don't let userspace write to PMCR_EL0.N when the vcpu has EL2 KVM: arm64: Allow userspace to limit the number of PMU counters for EL2 VMs KVM: arm64: Contextualise the handling of PMCR_EL0.P writes KVM: arm64: Fix MDCR_EL2.HPMN reset value KVM: arm64: Repaint pmcr_n into nr_pmu_counters Signed-off-by: Marc Zyngier <maz@kernel.org>
2 parents b443265 + 600f6fa commit 67bd641

File tree

5 files changed

+116
-23
lines changed

5 files changed

+116
-23
lines changed

Documentation/virt/kvm/devices/vcpu.rst

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,30 @@ exit_reason = KVM_EXIT_FAIL_ENTRY and populate the fail_entry struct by setting
137137
hardare_entry_failure_reason field to KVM_EXIT_FAIL_ENTRY_CPU_UNSUPPORTED and
138138
the cpu field to the processor id.
139139

140+
1.5 ATTRIBUTE: KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS
141+
--------------------------------------------------
142+
143+
:Parameters: in kvm_device_attr.addr the address to an unsigned int
144+
representing the maximum value taken by PMCR_EL0.N
145+
146+
:Returns:
147+
148+
======= ====================================================
149+
-EBUSY PMUv3 already initialized, a VCPU has already run or
150+
an event filter has already been set
151+
-EFAULT Error accessing the value pointed to by addr
152+
-ENODEV PMUv3 not supported or GIC not initialized
153+
-EINVAL No PMUv3 explicitly selected, or value of N out of
154+
range
155+
======= ====================================================
156+
157+
Set the number of implemented event counters in the virtual PMU. This
158+
mandates that a PMU has explicitly been selected via
159+
KVM_ARM_VCPU_PMU_V3_SET_PMU, and will fail when no PMU has been
160+
explicitly selected, or the number of counters is out of range for the
161+
selected PMU. Selecting a new PMU cancels the effect of setting this
162+
attribute.
163+
140164
2. GROUP: KVM_ARM_VCPU_TIMER_CTRL
141165
=================================
142166

arch/arm64/include/asm/kvm_host.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,8 +359,8 @@ struct kvm_arch {
359359

360360
cpumask_var_t supported_cpus;
361361

362-
/* PMCR_EL0.N value for the guest */
363-
u8 pmcr_n;
362+
/* Maximum number of counters for the guest */
363+
u8 nr_pmu_counters;
364364

365365
/* Iterator for idreg debugfs */
366366
u8 idreg_debugfs_iter;

arch/arm64/include/uapi/asm/kvm.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -431,10 +431,11 @@ enum {
431431

432432
/* Device Control API on vcpu fd */
433433
#define KVM_ARM_VCPU_PMU_V3_CTRL 0
434-
#define KVM_ARM_VCPU_PMU_V3_IRQ 0
435-
#define KVM_ARM_VCPU_PMU_V3_INIT 1
436-
#define KVM_ARM_VCPU_PMU_V3_FILTER 2
437-
#define KVM_ARM_VCPU_PMU_V3_SET_PMU 3
434+
#define KVM_ARM_VCPU_PMU_V3_IRQ 0
435+
#define KVM_ARM_VCPU_PMU_V3_INIT 1
436+
#define KVM_ARM_VCPU_PMU_V3_FILTER 2
437+
#define KVM_ARM_VCPU_PMU_V3_SET_PMU 3
438+
#define KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS 4
438439
#define KVM_ARM_VCPU_TIMER_CTRL 1
439440
#define KVM_ARM_VCPU_TIMER_IRQ_VTIMER 0
440441
#define KVM_ARM_VCPU_TIMER_IRQ_PTIMER 1

arch/arm64/kvm/pmu-emul.c

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ static u64 kvm_pmu_hyp_counter_mask(struct kvm_vcpu *vcpu)
280280
return 0;
281281

282282
hpmn = SYS_FIELD_GET(MDCR_EL2, HPMN, __vcpu_sys_reg(vcpu, MDCR_EL2));
283-
n = vcpu->kvm->arch.pmcr_n;
283+
n = vcpu->kvm->arch.nr_pmu_counters;
284284

285285
/*
286286
* Programming HPMN to a value greater than PMCR_EL0.N is
@@ -608,14 +608,12 @@ void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val)
608608
kvm_pmu_set_counter_value(vcpu, ARMV8_PMU_CYCLE_IDX, 0);
609609

610610
if (val & ARMV8_PMU_PMCR_P) {
611-
/*
612-
* Unlike other PMU sysregs, the controls in PMCR_EL0 always apply
613-
* to the 'guest' range of counters and never the 'hyp' range.
614-
*/
615611
unsigned long mask = kvm_pmu_implemented_counter_mask(vcpu) &
616-
~kvm_pmu_hyp_counter_mask(vcpu) &
617612
~BIT(ARMV8_PMU_CYCLE_IDX);
618613

614+
if (!vcpu_is_el2(vcpu))
615+
mask &= ~kvm_pmu_hyp_counter_mask(vcpu);
616+
619617
for_each_set_bit(i, &mask, 32)
620618
kvm_pmu_set_pmc_value(kvm_vcpu_idx_to_pmc(vcpu, i), 0, true);
621619
}
@@ -1027,12 +1025,30 @@ u8 kvm_arm_pmu_get_max_counters(struct kvm *kvm)
10271025
return bitmap_weight(arm_pmu->cntr_mask, ARMV8_PMU_MAX_GENERAL_COUNTERS);
10281026
}
10291027

1028+
static void kvm_arm_set_nr_counters(struct kvm *kvm, unsigned int nr)
1029+
{
1030+
kvm->arch.nr_pmu_counters = nr;
1031+
1032+
/* Reset MDCR_EL2.HPMN behind the vcpus' back... */
1033+
if (test_bit(KVM_ARM_VCPU_HAS_EL2, kvm->arch.vcpu_features)) {
1034+
struct kvm_vcpu *vcpu;
1035+
unsigned long i;
1036+
1037+
kvm_for_each_vcpu(i, vcpu, kvm) {
1038+
u64 val = __vcpu_sys_reg(vcpu, MDCR_EL2);
1039+
val &= ~MDCR_EL2_HPMN;
1040+
val |= FIELD_PREP(MDCR_EL2_HPMN, kvm->arch.nr_pmu_counters);
1041+
__vcpu_sys_reg(vcpu, MDCR_EL2) = val;
1042+
}
1043+
}
1044+
}
1045+
10301046
static void kvm_arm_set_pmu(struct kvm *kvm, struct arm_pmu *arm_pmu)
10311047
{
10321048
lockdep_assert_held(&kvm->arch.config_lock);
10331049

10341050
kvm->arch.arm_pmu = arm_pmu;
1035-
kvm->arch.pmcr_n = kvm_arm_pmu_get_max_counters(kvm);
1051+
kvm_arm_set_nr_counters(kvm, kvm_arm_pmu_get_max_counters(kvm));
10361052
}
10371053

10381054
/**
@@ -1088,6 +1104,20 @@ static int kvm_arm_pmu_v3_set_pmu(struct kvm_vcpu *vcpu, int pmu_id)
10881104
return ret;
10891105
}
10901106

1107+
static int kvm_arm_pmu_v3_set_nr_counters(struct kvm_vcpu *vcpu, unsigned int n)
1108+
{
1109+
struct kvm *kvm = vcpu->kvm;
1110+
1111+
if (!kvm->arch.arm_pmu)
1112+
return -EINVAL;
1113+
1114+
if (n > kvm_arm_pmu_get_max_counters(kvm))
1115+
return -EINVAL;
1116+
1117+
kvm_arm_set_nr_counters(kvm, n);
1118+
return 0;
1119+
}
1120+
10911121
int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
10921122
{
10931123
struct kvm *kvm = vcpu->kvm;
@@ -1184,6 +1214,15 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
11841214

11851215
return kvm_arm_pmu_v3_set_pmu(vcpu, pmu_id);
11861216
}
1217+
case KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS: {
1218+
unsigned int __user *uaddr = (unsigned int __user *)(long)attr->addr;
1219+
unsigned int n;
1220+
1221+
if (get_user(n, uaddr))
1222+
return -EFAULT;
1223+
1224+
return kvm_arm_pmu_v3_set_nr_counters(vcpu, n);
1225+
}
11871226
case KVM_ARM_VCPU_PMU_V3_INIT:
11881227
return kvm_arm_pmu_v3_init(vcpu);
11891228
}
@@ -1222,6 +1261,7 @@ int kvm_arm_pmu_v3_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
12221261
case KVM_ARM_VCPU_PMU_V3_INIT:
12231262
case KVM_ARM_VCPU_PMU_V3_FILTER:
12241263
case KVM_ARM_VCPU_PMU_V3_SET_PMU:
1264+
case KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS:
12251265
if (kvm_vcpu_has_pmu(vcpu))
12261266
return 0;
12271267
}
@@ -1260,8 +1300,12 @@ u8 kvm_arm_pmu_get_pmuver_limit(void)
12601300
u64 kvm_vcpu_read_pmcr(struct kvm_vcpu *vcpu)
12611301
{
12621302
u64 pmcr = __vcpu_sys_reg(vcpu, PMCR_EL0);
1303+
u64 n = vcpu->kvm->arch.nr_pmu_counters;
1304+
1305+
if (vcpu_has_nv(vcpu) && !vcpu_is_el2(vcpu))
1306+
n = FIELD_GET(MDCR_EL2_HPMN, __vcpu_sys_reg(vcpu, MDCR_EL2));
12631307

1264-
return u64_replace_bits(pmcr, vcpu->kvm->arch.pmcr_n, ARMV8_PMU_PMCR_N);
1308+
return u64_replace_bits(pmcr, n, ARMV8_PMU_PMCR_N);
12651309
}
12661310

12671311
void kvm_pmu_nested_transition(struct kvm_vcpu *vcpu)

arch/arm64/kvm/sys_regs.c

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,7 @@ static unsigned int pmu_visibility(const struct kvm_vcpu *vcpu,
785785
static u64 reset_pmu_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
786786
{
787787
u64 mask = BIT(ARMV8_PMU_CYCLE_IDX);
788-
u8 n = vcpu->kvm->arch.pmcr_n;
788+
u8 n = vcpu->kvm->arch.nr_pmu_counters;
789789

790790
if (n)
791791
mask |= GENMASK(n - 1, 0);
@@ -1216,8 +1216,9 @@ static int set_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
12161216
* with the existing KVM behavior.
12171217
*/
12181218
if (!kvm_vm_has_ran_once(kvm) &&
1219+
!vcpu_has_nv(vcpu) &&
12191220
new_n <= kvm_arm_pmu_get_max_counters(kvm))
1220-
kvm->arch.pmcr_n = new_n;
1221+
kvm->arch.nr_pmu_counters = new_n;
12211222

12221223
mutex_unlock(&kvm->arch.config_lock);
12231224

@@ -2570,16 +2571,33 @@ static bool access_mdcr(struct kvm_vcpu *vcpu,
25702571
struct sys_reg_params *p,
25712572
const struct sys_reg_desc *r)
25722573
{
2573-
u64 old = __vcpu_sys_reg(vcpu, MDCR_EL2);
2574+
u64 hpmn, val, old = __vcpu_sys_reg(vcpu, MDCR_EL2);
25742575

2575-
if (!access_rw(vcpu, p, r))
2576-
return false;
2576+
if (!p->is_write) {
2577+
p->regval = old;
2578+
return true;
2579+
}
2580+
2581+
val = p->regval;
2582+
hpmn = FIELD_GET(MDCR_EL2_HPMN, val);
25772583

25782584
/*
2579-
* Request a reload of the PMU to enable/disable the counters affected
2580-
* by HPME.
2585+
* If HPMN is out of bounds, limit it to what we actually
2586+
* support. This matches the UNKNOWN definition of the field
2587+
* in that case, and keeps the emulation simple. Sort of.
25812588
*/
2582-
if ((old ^ __vcpu_sys_reg(vcpu, MDCR_EL2)) & MDCR_EL2_HPME)
2589+
if (hpmn > vcpu->kvm->arch.nr_pmu_counters) {
2590+
hpmn = vcpu->kvm->arch.nr_pmu_counters;
2591+
u64_replace_bits(val, hpmn, MDCR_EL2_HPMN);
2592+
}
2593+
2594+
__vcpu_sys_reg(vcpu, MDCR_EL2) = val;
2595+
2596+
/*
2597+
* Request a reload of the PMU to enable/disable the counters
2598+
* affected by HPME.
2599+
*/
2600+
if ((old ^ val) & MDCR_EL2_HPME)
25832601
kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu);
25842602

25852603
return true;
@@ -2698,6 +2716,12 @@ static int set_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
26982716
.set_user = set_imp_id_reg, \
26992717
.reset = reset_imp_id_reg, \
27002718
.val = mask, \
2719+
}
2720+
2721+
static u64 reset_mdcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
2722+
{
2723+
__vcpu_sys_reg(vcpu, r->reg) = vcpu->kvm->arch.nr_pmu_counters;
2724+
return vcpu->kvm->arch.nr_pmu_counters;
27012725
}
27022726

27032727
/*
@@ -3243,7 +3267,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
32433267
EL2_REG(SCTLR_EL2, access_rw, reset_val, SCTLR_EL2_RES1),
32443268
EL2_REG(ACTLR_EL2, access_rw, reset_val, 0),
32453269
EL2_REG_VNCR(HCR_EL2, reset_hcr, 0),
3246-
EL2_REG(MDCR_EL2, access_mdcr, reset_val, 0),
3270+
EL2_REG(MDCR_EL2, access_mdcr, reset_mdcr, 0),
32473271
EL2_REG(CPTR_EL2, access_rw, reset_val, CPTR_NVHE_EL2_RES1),
32483272
EL2_REG_VNCR(HSTR_EL2, reset_val, 0),
32493273
EL2_REG_VNCR(HFGRTR_EL2, reset_val, 0),

0 commit comments

Comments
 (0)