On Wed, Aug 28, 2019 at 04:41:30PM -0700, Oliver Upton wrote:
> Creating a helper function to check the validity of the
Changelogs should use imperative mood, e.g.:
Create a helper function to check...
> {HOST,GUEST}_IA32_PERF_GLOBAL_CTRL wrt the PMU's global_ctrl_mask.
As is, this needs the SDM quote from patch 4/7 as it's not clear what
global_ctrl_mask contains, e.g. the check looks inverted to the
uninitiated. Adding a helper without a user is also discouraged,
e.g. if the helper is broken then bisection would point at the next
patch, so this should really be folded in to patch 4/7 anyways.
That being said, if you tweak the prototype (see below) then you can
use it intel_pmu_set_msr(), in which case a standalone patch does make
sense.
> Suggested-by: Jim Mattson <jmattson@xxxxxxxxxx>
> Signed-off-by: Oliver Upton <oupton@xxxxxxxxxx>
> ---
> arch/x86/kvm/pmu.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
> index 58265f761c3b..b7d9efff208d 100644
> --- a/arch/x86/kvm/pmu.h
> +++ b/arch/x86/kvm/pmu.h
> @@ -79,6 +79,17 @@ static inline bool pmc_is_enabled(struct kvm_pmc *pmc)
> return kvm_x86_ops->pmu_ops->pmc_is_enabled(pmc);
> }
>
> +static inline bool kvm_is_valid_perf_global_ctrl(struct kvm_vcpu *vcpu,
If this takes 'struct kvm_pmu *pmu' instead of a vcpu then it can also
be used in intel_pmu_set_msr().
> + u64 global_ctrl)
> +{
> + struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> +
> + if (pmu->global_ctrl_mask & global_ctrl)
intel_pmu_set_msr() allows 'global_ctrl == data' regardless of the mask,
do we need simliar code here?
> + return false;
> +
> + return true;
Or simply
return !(pmu->global_ctrl_mask & global_ctrl);
> +}
> +
> /* returns general purpose PMC with the specified MSR. Note that it can be
> * used for both PERFCTRn and EVNTSELn; that is why it accepts base as a
> * paramenter to tell them apart.
> --
> 2.23.0.187.g17f5b7556c-goog
>
