| Message ID | 20250409160106.6445-6-maz@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: arm64: EL2 PMU handling fixes | expand |
HDCR? I thought you gave up on 32-bit a loooong time ago ;-) On Wed, Apr 09, 2025 at 05:01:05PM +0100, Marc Zyngier wrote: > We don't really pay attention to what gets written to MDCR_EL2.HPMN, > and funky guests could play ugly games on us. > > Restrict what gets written there, and limit the number of counters > to what the PMU is allowed to have. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/kvm/sys_regs.c | 34 +++++++++++++++++++++++++--------- > 1 file changed, 25 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 00b5396492d51..e53b8f82ca7f8 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -2571,17 +2571,33 @@ static bool access_mdcr(struct kvm_vcpu *vcpu, > struct sys_reg_params *p, > const struct sys_reg_desc *r) > { > - u64 old = __vcpu_sys_reg(vcpu, MDCR_EL2); > + if (!p->is_write) { > + p->regval = __vcpu_sys_reg(vcpu, MDCR_EL2); > + } else { nit: you can do an early return for an emulated read and get rid of a level of indentation for the write case. > + u64 hpmn = FIELD_GET(MDCR_EL2_HPMN, p->regval); > + u64 old = __vcpu_sys_reg(vcpu, MDCR_EL2); > + u64 val = p->regval; > > - if (!access_rw(vcpu, p, r)) > - return false; > + /* > + * If HPMN is out of bounds, limit it to what we actually > + * support. This matches the UNKNOWN definition of the field > + * in that case, and keeps the emulation simple. Sort of. > + */ > + if (hpmn > vcpu->kvm->arch.pmcr_n) { > + hpmn = vcpu->kvm->arch.pmcr_n; > + u64_replace_bits(val, hpmn, MDCR_EL2_HPMN); > + } > > - /* > - * Request a reload of the PMU to enable/disable the counters affected > - * by HPME. > - */ > - if ((old ^ __vcpu_sys_reg(vcpu, MDCR_EL2)) & MDCR_EL2_HPME) > - kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu); > + vcpu_write_sys_reg(vcpu, val, r->reg); > + > + /* > + * Request a reload of the PMU to enable/disable the > + * counters affected by HPME. > + */ > + > + if ((old ^ __vcpu_sys_reg(vcpu, MDCR_EL2)) & MDCR_EL2_HPME) > + kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu); > + } > > return true; > } > -- > 2.39.2 > Thanks, Oliver
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 00b5396492d51..e53b8f82ca7f8 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2571,17 +2571,33 @@ static bool access_mdcr(struct kvm_vcpu *vcpu, struct sys_reg_params *p, const struct sys_reg_desc *r) { - u64 old = __vcpu_sys_reg(vcpu, MDCR_EL2); + if (!p->is_write) { + p->regval = __vcpu_sys_reg(vcpu, MDCR_EL2); + } else { + u64 hpmn = FIELD_GET(MDCR_EL2_HPMN, p->regval); + u64 old = __vcpu_sys_reg(vcpu, MDCR_EL2); + u64 val = p->regval; - if (!access_rw(vcpu, p, r)) - return false; + /* + * If HPMN is out of bounds, limit it to what we actually + * support. This matches the UNKNOWN definition of the field + * in that case, and keeps the emulation simple. Sort of. + */ + if (hpmn > vcpu->kvm->arch.pmcr_n) { + hpmn = vcpu->kvm->arch.pmcr_n; + u64_replace_bits(val, hpmn, MDCR_EL2_HPMN); + } - /* - * Request a reload of the PMU to enable/disable the counters affected - * by HPME. - */ - if ((old ^ __vcpu_sys_reg(vcpu, MDCR_EL2)) & MDCR_EL2_HPME) - kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu); + vcpu_write_sys_reg(vcpu, val, r->reg); + + /* + * Request a reload of the PMU to enable/disable the + * counters affected by HPME. + */ + + if ((old ^ __vcpu_sys_reg(vcpu, MDCR_EL2)) & MDCR_EL2_HPME) + kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu); + } return true; }
We don't really pay attention to what gets written to MDCR_EL2.HPMN, and funky guests could play ugly games on us. Restrict what gets written there, and limit the number of counters to what the PMU is allowed to have. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/kvm/sys_regs.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)