On Mon, Sep 11, 2023 at 11:09 AM Sean Christopherson <[email protected]> wrote:
>
> On Mon, Sep 11, 2023, Mingwei Zhang wrote:
> > On Mon, Sep 11, 2023 at 8:01 AM Sean Christopherson <[email protected]> wrote:
> > >
> > > On Mon, Sep 11, 2023, Mingwei Zhang wrote:
> > > > Use atomic bit operations for pmu->global_status because it may suffer from
> > > > race conditions between emulated overflow in KVM vPMU and PEBS overflow in
> > > > host PMI handler.
> > >
> > > Only if the host PMI occurs on a different pCPU, and if that can happen don't we
> > > have a much larger problem?
> >
> > Why on different pCPU? For vPMU, I think there is always contention
> > between the vCPU thread and the host PMI handler running on the same
> > pCPU, no?
>
> A non-atomic instruction can't be interrupted by an NMI, or any other event, so
> I don't see how switching to atomic operations fixes anything unless the NMI comes
> in on a different pCPU.
You are right. I realize that. The race condition has to happen
concurrently from two different pCPUs. This happens to
pmu->reprogram_nmi but not pmu->global_status. The critical stuff we
care about should be re-entrancy issues for __kvm_perf_overflow() and
some state maintenance issues like avoiding duplicate NMI injection.
That concludes that we don't need this change.
Thanks
-Mingwei