Hi Raghu, On Mon, Oct 09, 2023 at 11:08:48PM +0000, Raghavendra Rao Ananta wrote: > From: Reiji Watanabe <reijiw@xxxxxxxxxx> > > The following patches will use the number of counters information > from the arm_pmu and use this to set the PMCR.N for the guest > during vCPU reset. However, since the guest is not associated > with any arm_pmu until userspace configures the vPMU device > attributes, and a reset can happen before this event, assign a > default PMU to the guest just before doing the reset. > > Signed-off-by: Reiji Watanabe <reijiw@xxxxxxxxxx> > Signed-off-by: Raghavendra Rao Ananta <rananta@xxxxxxxxxx> > --- > arch/arm64/kvm/arm.c | 20 ++++++++++++++++++++ > arch/arm64/kvm/pmu-emul.c | 12 ++---------- > include/kvm/arm_pmu.h | 6 ++++++ > 3 files changed, 28 insertions(+), 10 deletions(-) > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 78b0970eb8e6..708a53b70a7b 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -1313,6 +1313,23 @@ static bool kvm_vcpu_init_changed(struct kvm_vcpu *vcpu, > KVM_VCPU_MAX_FEATURES); > } > > +static int kvm_vcpu_set_pmu(struct kvm_vcpu *vcpu) > +{ > + struct kvm *kvm = vcpu->kvm; > + > + if (!kvm_arm_support_pmu_v3()) > + return -EINVAL; This check is pointless; the vCPU feature flags have been sanitised at this point, and a requirement of having PMUv3 is that this predicate is true. > + /* > + * When the vCPU has a PMU, but no PMU is set for the guest > + * yet, set the default one. > + */ > + if (unlikely(!kvm->arch.arm_pmu)) > + return kvm_arm_set_default_pmu(kvm); > + > + return 0; > +} > + Apologies, I believe I was unclear last time around as to what I was wanting here. Let's call this thing kvm_setup_vcpu() such that we can add other one-time setup activities to it in the future. Something like: diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 96641e442039..4896a44108e0 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -1265,19 +1265,17 @@ static bool kvm_vcpu_init_changed(struct kvm_vcpu *vcpu, KVM_VCPU_MAX_FEATURES); } -static int kvm_vcpu_set_pmu(struct kvm_vcpu *vcpu) +static int kvm_setup_vcpu(struct kvm_vcpu *vcpu) { struct kvm *kvm = vcpu->kvm; - if (!kvm_arm_support_pmu_v3()) - return -EINVAL; - /* * When the vCPU has a PMU, but no PMU is set for the guest * yet, set the default one. */ - if (unlikely(!kvm->arch.arm_pmu)) - return kvm_arm_set_default_pmu(kvm); + if (kvm_vcpu_has_pmu(vcpu) && !kvm->arch.arm_pmu && + kvm_arm_set_default_pmu(kvm)) + return -EINVAL; return 0; } @@ -1297,7 +1295,8 @@ static int __kvm_vcpu_set_target(struct kvm_vcpu *vcpu, bitmap_copy(kvm->arch.vcpu_features, &features, KVM_VCPU_MAX_FEATURES); - if (kvm_vcpu_has_pmu(vcpu) && kvm_vcpu_set_pmu(vcpu)) + ret = kvm_setup_vcpu(vcpu); + if (ret) goto out_unlock; /* Now we know what it is, we can reset it. */ -- Thanks, Oliver