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

Wei Huang
2015-04-09T21:08:41Z

<snip>
>>>> @@ -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.
>
I can fix the name of this function (maybe back to kvm_pmu_check_pmc(),
let me think about it). In the meanwhile, do you have any comments on
the following names? They will impact the rest code:

* msr: MSR for x86
* msr_idx: offset of MSR registers (used by rdpmc)
* glb_idx: a unified index for both GP and fixed counters (should we
rename it to idx instead?)

Are they confusing to you? Maybe I should move "Design Note" to commit
message instead of real code?

Thanks,
-Wei