Messages in this thread Patch in this message | | | Date | Tue, 24 Oct 2023 08:59:30 +0000 | From | Oliver Upton <> | Subject | Re: [PATCH v8 05/13] KVM: arm64: Add {get,set}_user for PM{C,I}NTEN{SET,CLR}, PMOVS{SET,CLR} |
| |
On Fri, Oct 20, 2023 at 09:40:45PM +0000, Raghavendra Rao Ananta wrote: > For unimplemented counters, the bits in PM{C,I}NTEN{SET,CLR} and > PMOVS{SET,CLR} registers are expected to RAZ. To honor this, > explicitly implement the {get,set}_user functions for these > registers to mask out unimplemented counters for userspace reads > and writes. > > Signed-off-by: Raghavendra Rao Ananta <rananta@google.com> > --- > arch/arm64/kvm/sys_regs.c | 91 ++++++++++++++++++++++++++++++++++++--- > 1 file changed, 85 insertions(+), 6 deletions(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index faf97878dfbbb..2e5d497596ef8 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -987,6 +987,45 @@ static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct sys_reg_params *p, > return true; > } > > +static void set_pmreg_for_valid_counters(struct kvm_vcpu *vcpu, > + u64 reg, u64 val, bool set) > +{ > + struct kvm *kvm = vcpu->kvm; > + > + mutex_lock(&kvm->arch.config_lock); > + > + /* Make the register immutable once the VM has started running */
This is a considerable change from the existing behavior and lacks justification. These registers, or rather the state that these aliases update, is mutable from the guest. I see no reason for excluding userspace from this behavior.
> + if (kvm_vm_has_ran_once(kvm)) { > + mutex_unlock(&kvm->arch.config_lock); > + return; > + } > + > + val &= kvm_pmu_valid_counter_mask(vcpu); > + mutex_unlock(&kvm->arch.config_lock);
I'm not entirely sold on taking the config_lock here.
- If userspace is doing these ioctls in parallel then it cannot guarantee ordering in the first place, even w/ locking under the hood. Any garbage values will be discarded by KVM_REQ_RELOAD_PMU. - If the VM has already started PMCR.N is immutable, so there is no race.
-- Thanks, Oliver
| |