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;
From:   "Wang, Wei W" <wei.w.wang@intel.com>
To:     'Andi Kleen' <ak@linux.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
Thread-Topic: [PATCH v5 12/12] KVM/VMX/vPMU: support to report
 GLOBAL_STATUS_LBRS_FROZEN
Thread-Index: AQHUxEm+n+zGVNvLLUW1jEFYo+dja6Xe9t6AgAE6N1A=
Date:   Fri, 15 Feb 2019 08:56:02 +0000
Message-ID: <286AC319A985734F985F78AFA26841F73DF71ED6@shsmsx102.ccr.corp.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>
In-Reply-To: <20190214163147.GL16922@tassilo.jf.intel.com>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 11.0.400.15
dlp-reaction: no-action
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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?

Best,
Wei