Hi Zenghui, On 18/07/2019 09:15, Zenghui Yu wrote: > We use "pmc->idx" and the "chained" bitmap to determine if the pmc is > chained, in kvm_pmu_pmc_is_chained(). But idx might be uninitialized > (and random) when we doing this decision, through a KVM_ARM_VCPU_INIT > ioctl -> kvm_pmu_vcpu_reset(). And the test_bit() against this random > idx will potentially hit a KASAN BUG [1]. > > In general, idx is the static property of a PMU counter that is not > expected to be modified across resets, as suggested by Julien. It > looks more reasonable if we can setup the PMU counter idx for a vcpu > in its creation time. Introduce a new function - kvm_pmu_vcpu_init() > for this basic setup. Oh, and the KASAN BUG will get fixed this way. > > [1] https://www.spinics.net/lists/kvm-arm/msg36700.html > > Fixes: 80f393a23be6 ("KVM: arm/arm64: Support chained PMU counters") > Suggested-by: Andrew Murray <andrew.murray@xxxxxxx> > Suggested-by: Julien Thierry <julien.thierry@xxxxxxx> > Cc: Marc Zyngier <maz@xxxxxxxxxx> > Signed-off-by: Zenghui Yu <yuzenghui@xxxxxxxxxx> > --- > > Changes since v1: > - Introduce kvm_pmu_vcpu_init() in vcpu's creation time, move the > assignment of pmc->idx into it. > - Thus change the subject. The old one is "KVM: arm/arm64: Assign > pmc->idx before kvm_pmu_stop_counter()". > > Julien, I haven't collected your Acked-by into this version. If you're > still happy with the change, please Ack again. Thanks! > Thanks for making the change. This looks good to me: Acked-by: Julien Thierry <julien.thierry@xxxxxxx> Thanks, Julien > include/kvm/arm_pmu.h | 2 ++ > virt/kvm/arm/arm.c | 2 ++ > virt/kvm/arm/pmu.c | 18 +++++++++++++++--- > 3 files changed, 19 insertions(+), 3 deletions(-) > > diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h > index 16c769a..6db0304 100644 > --- a/include/kvm/arm_pmu.h > +++ b/include/kvm/arm_pmu.h > @@ -34,6 +34,7 @@ struct kvm_pmu { > u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx); > void kvm_pmu_set_counter_value(struct kvm_vcpu *vcpu, u64 select_idx, u64 val); > u64 kvm_pmu_valid_counter_mask(struct kvm_vcpu *vcpu); > +void kvm_pmu_vcpu_init(struct kvm_vcpu *vcpu); > void kvm_pmu_vcpu_reset(struct kvm_vcpu *vcpu); > void kvm_pmu_vcpu_destroy(struct kvm_vcpu *vcpu); > void kvm_pmu_disable_counter_mask(struct kvm_vcpu *vcpu, u64 val); > @@ -71,6 +72,7 @@ static inline u64 kvm_pmu_valid_counter_mask(struct kvm_vcpu *vcpu) > { > return 0; > } > +static inline void kvm_pmu_vcpu_init(struct kvm_vcpu *vcpu) {} > static inline void kvm_pmu_vcpu_reset(struct kvm_vcpu *vcpu) {} > static inline void kvm_pmu_vcpu_destroy(struct kvm_vcpu *vcpu) {} > static inline void kvm_pmu_disable_counter_mask(struct kvm_vcpu *vcpu, u64 val) {} > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c > index f645c0f..c704fa6 100644 > --- a/virt/kvm/arm/arm.c > +++ b/virt/kvm/arm/arm.c > @@ -340,6 +340,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) > /* Set up the timer */ > kvm_timer_vcpu_init(vcpu); > > + kvm_pmu_vcpu_init(vcpu); > + > kvm_arm_reset_debug_ptr(vcpu); > > return kvm_vgic_vcpu_init(vcpu); > diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c > index 3dd8238..362a018 100644 > --- a/virt/kvm/arm/pmu.c > +++ b/virt/kvm/arm/pmu.c > @@ -215,6 +215,20 @@ static void kvm_pmu_stop_counter(struct kvm_vcpu *vcpu, struct kvm_pmc *pmc) > } > > /** > + * kvm_pmu_vcpu_init - assign pmu counter idx for cpu > + * @vcpu: The vcpu pointer > + * > + */ > +void kvm_pmu_vcpu_init(struct kvm_vcpu *vcpu) > +{ > + int i; > + struct kvm_pmu *pmu = &vcpu->arch.pmu; > + > + for (i = 0; i < ARMV8_PMU_MAX_COUNTERS; i++) > + pmu->pmc[i].idx = i; > +} > + > +/** > * kvm_pmu_vcpu_reset - reset pmu state for cpu > * @vcpu: The vcpu pointer > * > @@ -224,10 +238,8 @@ void kvm_pmu_vcpu_reset(struct kvm_vcpu *vcpu) > int i; > struct kvm_pmu *pmu = &vcpu->arch.pmu; > > - for (i = 0; i < ARMV8_PMU_MAX_COUNTERS; i++) { > + for (i = 0; i < ARMV8_PMU_MAX_COUNTERS; i++) > kvm_pmu_stop_counter(vcpu, &pmu->pmc[i]); > - pmu->pmc[i].idx = i; > - } > > bitmap_zero(vcpu->arch.pmu.chained, ARMV8_PMU_MAX_COUNTER_PAIRS); > } > -- Julien Thierry _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
