On Thu, 26 Jul 2018, Luwei Kang wrote:
perf/x86/intel/pt: Introduce a new function to get capability of Intel PT
Subjects should be short and consice.
perf/x86/intel/pt: Introduce intel_pt_cap_decode()
Also the decsription in $subject is not what the description below
says. Please make sure that your subjects and changelogs are consistent.
> New function pt_cap_decode() will be invoked in KVM to check
> if a specific capability is available in KVM guest.
> Another function pt_cap_get() can only check the hardware
> capabilities but this may different with KVM guest because
> some features may not be exposed to guest.
Again it took me a while to figure out what you are trying to
say. Suggestion:
intel_pt_validate_cap() validates whether a given PT capability is
supported by the hardware. It checks the PT capability array which
reflects the capabilites of the hardware on which the code is executed.
For setting up PT for KVM guests this is not correct as the capability
array for the guest can be different from the host array.
Provide a new function to check against a given capability array.
> -u32 pt_cap_get(enum pt_capabilities cap)
> +u32 pt_cap_decode(u32 *caps, enum pt_capabilities capability)
> {
> - struct pt_cap_desc *cd = &pt_caps[cap];
> - u32 c = pt_pmu.caps[cd->leaf * PT_CPUID_REGS_NUM + cd->reg];
> + struct pt_cap_desc *cd = &pt_caps[capability];
> + u32 c = caps[cd->leaf * PT_CPUID_REGS_NUM + cd->reg];
> unsigned int shift = __ffs(cd->mask);
>
> return (c & cd->mask) >> shift;
> }
> +EXPORT_SYMBOL_GPL(pt_cap_decode);
> +
> +u32 pt_cap_get(enum pt_capabilities cap)
> +{
> + return pt_cap_decode(pt_pmu.caps, cap);
> +}
> EXPORT_SYMBOL_GPL(pt_cap_get);
pt_cap_decode() is yet another misnomer.
So what you really want is:
s/pt_cap_get()/intel_pt_validate_hw_cap()/
and the new function wants to be:
intel_pt_validate_cap()
So it's entirely clear from the naming that the former pt_cap_get() is
related to the hardware capabilities of the machine on which the code is
executing.
Thanks,
tglx
