Re: [PATCH V2 3/5] KVM: x86/vPMU: Create vPMU interface for VMX and SVM

["Radim Krčmář" <rkrcmar@redhat.com>]

2015-04-09 15:03-0500, Wei Huang:
> On 4/9/15 14:43, Radim Krčmář wrote:
>> This patch introduces a lot of trailing whitespaces, please remove them.
>> (`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 = kvm_pmu_read_pmc(vcpu, ecx, &data);
>>>+	err = 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 confusing
> term in vPMU code (see "Design Note" in pmu.c file), which makes me to 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_emulate_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 msr 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.