Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
MIME-Version: 1.0
References: <20230911061147.409152-1-mizhang@google.com> <ZP8r2CDsv3JkGYzX@google.com>
 <CAL715WKyS4sTH3yEOX2OyV+fxMLMOAV6tX-A7fvEAKEUGj8uxw@mail.gmail.com> <ZP9X7YvstWhS/pWn@google.com>
In-Reply-To: <ZP9X7YvstWhS/pWn@google.com>
From:   Mingwei Zhang <mizhang@google.com>
Date:   Mon, 11 Sep 2023 16:42:46 -0700
Message-ID: <CAL715WJwb8RhqdvOaurUxY5=ftixw7LHZWBT9PrOGRAzF5veqA@mail.gmail.com>
Subject: Re: [PATCH] KVM: vPMU: Use atomic bit operations for global_status
To:     Sean Christopherson <seanjc@google.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, Dapeng Mi <dapeng1.mi@intel.com>,
        Jim Mattson <jmattson@google.com>, Like Xu <likexu@tencent.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Sep 11, 2023 at 11:09=E2=80=AFAM Sean Christopherson <seanjc@google=
.com> wrote:
>
> On Mon, Sep 11, 2023, Mingwei Zhang wrote:
> > On Mon, Sep 11, 2023 at 8:01=E2=80=AFAM Sean Christopherson <seanjc@goo=
gle.com> wrote:
> > >
> > > On Mon, Sep 11, 2023, Mingwei Zhang wrote:
> > > > Use atomic bit operations for pmu->global_status because it may suf=
fer from
> > > > race conditions between emulated overflow in KVM vPMU and PEBS over=
flow in
> > > > host PMI handler.
> > >
> > > Only if the host PMI occurs on a different pCPU, and if that can happ=
en 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 eve=
nt, 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