Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 15 Feb 2019 05:10:14 -0800
From:   Andi Kleen <ak@linux.intel.com>
To:     "Wang, Wei W" <wei.w.wang@intel.com>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "pbonzini@redhat.com" <pbonzini@redhat.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "Liang, Kan" <kan.liang@intel.com>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "rkrcmar@redhat.com" <rkrcmar@redhat.com>,
        "Xu, Like" <like.xu@intel.com>,
        "jannh@google.com" <jannh@google.com>,
        "arei.gonglei@huawei.com" <arei.gonglei@huawei.com>,
        "jmattson@google.com" <jmattson@google.com>
Subject: Re: [PATCH v5 12/12] KVM/VMX/vPMU: support to report
 GLOBAL_STATUS_LBRS_FROZEN
Message-ID: <20190215131014.GC16922@tassilo.jf.intel.com>
References: <1550135174-5423-1-git-send-email-wei.w.wang@intel.com>
 <1550135174-5423-13-git-send-email-wei.w.wang@intel.com>
 <20190214163147.GL16922@tassilo.jf.intel.com>
 <286AC319A985734F985F78AFA26841F73DF71ED6@shsmsx102.ccr.corp.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <286AC319A985734F985F78AFA26841F73DF71ED6@shsmsx102.ccr.corp.intel.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Feb 15, 2019 at 08:56:02AM +0000, Wang, Wei W wrote:
> On Friday, February 15, 2019 12:32 AM, Andi Kleen wrote:
> > 
> > > +static void intel_pmu_get_global_status(struct kvm_pmu *pmu,
> > > +					struct msr_data *msr_info)
> > > +{
> > > +	u64 guest_debugctl, freeze_lbr_bits =
> > DEBUGCTLMSR_FREEZE_LBRS_ON_PMI |
> > > +					      DEBUGCTLMSR_LBR;
> > > +
> > > +	if (!pmu->global_status) {
> > > +		msr_info->data = 0;
> > > +		return;
> > > +	}
> > > +
> > > +	msr_info->data = pmu->global_status;
> > > +	if (pmu->version >= 4) {
> > > +		guest_debugctl = vmcs_read64(GUEST_IA32_DEBUGCTL);
> > > +		if ((guest_debugctl & freeze_lbr_bits) == freeze_lbr_bits)
> > 
> > It should only check for the freeze bit, the freeze bit can be set even when
> > LBRs are disabled.
> > 
> > Also you seem to set the bit unconditionally?
> > That doesn't seem right. It should only be set after an overflow.
> > 
> > So the PMI injection needs to set it.
> 
> OK. The freeze bits need to be cleared by IA32_PERF_GLOBAL_STATUS_RESET, which seems not supported by the perf code yet (thus guest won't clear them). Would handle_irq_v4 also need to be changed to support that?

In Arch Perfmon v4 it is  cleared by the MSR_CORE_PERF_GLOBAL_OVF_CTRL write
But the guest KVM pmu doesn't support v4 so far, so the only way to clear it is through DEBUGCTL.

STATUS_RESET would only be needed to set it from the guest, which is not necessary at least for now
(and would be also v4)

At some point the guest PMU should probably be updated for v4, but it can be done
separately from this.

-Andi