From mboxrd@z Thu Jan  1 00:00:00 1970
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Subject: Re: [PATCH V2 3/5] KVM: x86/vPMU: Create vPMU interface for VMX and
 SVM
Date: Thu, 9 Apr 2015 22:54:53 +0200
Message-ID: <20150409205453.GA17888@potion.brq.redhat.com>
References: <1428509905-32352-1-git-send-email-wei@redhat.com>
 <1428509905-32352-4-git-send-email-wei@redhat.com>
 <20150409194344.GC9729@potion.brq.redhat.com>
 <5526DB24.8020707@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: kvm@vger.kernel.org, pbonzini@redhat.com, gleb@kernel.org
To: Wei Huang <wei@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:58631 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754227AbbDIUy5 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 9 Apr 2015 16:54:57 -0400
Content-Disposition: inline
In-Reply-To: <5526DB24.8020707@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

2015-04-09 15:03-0500, Wei Huang:
> On 4/9/15 14:43, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote:
>> This patch introduces a lot of trailing whitespaces, please remove t=
hem.
>> (`git am` says 15.)
> Some of them were carried from original pmu.c file. I will purge them=
 in V3.

(Ah, I didn't see them in kvm_intel.c and didn't check further.)

>>>-	err =3D kvm_pmu_read_pmc(vcpu, ecx, &data);
>>>+	err =3D kvm_pmu_rdpmc(vcpu, ecx, &data);
>>
>>(What was the reason behind the new name?)
> Original kvm_pmu_read_pmc() was only used for RDPMC instruction. Such=
 change
> make it easier to be correlated with RDPMC directly; plus pmc is a co=
nfusing
> term in vPMU code (see "Design Note" in pmu.c file), which makes me t=
o think
> kvm_pmu_read_pmc() should read from "struct kvm_pmc" directly.

Makes sense,

Thanks.

>>>@@ -4918,13 +4919,13 @@ static int emulator_set_msr(struct x86_emula=
te_ctxt *ctxt,
>>>  static int emulator_check_pmc(struct x86_emulate_ctxt *ctxt,
>>>  			      u32 pmc)
>>>  {
>>>-	return kvm_pmu_check_pmc(emul_to_vcpu(ctxt), pmc);
>>>+	return kvm_pmu_check_msr_idx(emul_to_vcpu(ctxt), pmc);
>>
>>(Why not pmc?)
> See "Design Note" in pmu.c for a better explanation. I tried to use m=
sr as
> real x86 MSR; and msr_idx refers to MSR offset.

I skipped the comment as I thought it was there before, sorry ...

I wouldn't call it MSR index, MSR is just a related interface for PMC,
and MSR indices don't even have simple mapping to RDPMC ones.
We are indexing PMC without MSR, so index/pmc_idx/pmc seems better.