Re: [PATCH v2] KVM: arm/arm64: Introduce kvm_pmu_vcpu_init() to setup PMU counter idx

[Marc Zyngier]

On 2019-07-23 09:17, Julien Thierry wrote:
> 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>

Applied, thanks both.

        M.
--
Jazz is not dead. It just smells funny...