Re: [Xen-devel] [PATCH v25 13/15] x86/VPMU: Merge vpmu_rdmsr and vpmu_wrmsr

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Boris Ostrovsky [mailto:boris.ostrovsky@xxxxxxxxxx]
> Sent: Saturday, June 20, 2015 2:45 AM
> 
> The two routines share most of their logic.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> Reviewed-by: Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx>

Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>

> ---
>  xen/arch/x86/hvm/vpmu.c        | 75
> ++++++++++++++++--------------------------
>  xen/include/asm-x86/hvm/vpmu.h | 14 ++++++--
>  2 files changed, 41 insertions(+), 48 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
> index c0a22fb..9d6ca93 100644
> --- a/xen/arch/x86/hvm/vpmu.c
> +++ b/xen/arch/x86/hvm/vpmu.c
> @@ -100,63 +100,46 @@ void vpmu_lvtpc_update(uint32_t val)
>          apic_write(APIC_LVTPC, vpmu->hw_lapic_lvtpc);
>  }
> 
> -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported)
> +int vpmu_do_msr(unsigned int msr, uint64_t *msr_content,
> +                uint64_t supported, bool_t is_write)
>  {
>      struct vcpu *curr = current;
>      struct vpmu_struct *vpmu;
> +    const struct arch_vpmu_ops *ops;
> +    int ret = 0;
> 
> -    if ( vpmu_mode == XENPMU_MODE_OFF )
> -        return 0;
> +    if ( likely(vpmu_mode == XENPMU_MODE_OFF) )
> +        goto nop;
> 
>      vpmu = vcpu_vpmu(curr);
> -    if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
> -    {
> -        int ret = vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content, supported);
> -
> -        /*
> -         * We may have received a PMU interrupt during WRMSR handling
> -         * and since do_wrmsr may load VPMU context we should save
> -         * (and unload) it again.
> -         */
> -        if ( !is_hvm_vcpu(curr) && vpmu->xenpmu_data &&
> -             vpmu_is_set(vpmu, VPMU_CACHED) )
> -        {
> -            vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
> -            vpmu->arch_vpmu_ops->arch_vpmu_save(curr, 0);
> -            vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | VPMU_CONTEXT_LOADED);
> -        }
> -        return ret;
> -    }
> -
> -    return 0;
> -}
> -
> -int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
> -{
> -    struct vcpu *curr = current;
> -    struct vpmu_struct *vpmu;
> +    ops = vpmu->arch_vpmu_ops;
> +    if ( !ops )
> +        goto nop;
> +
> +    if ( is_write && ops->do_wrmsr )
> +        ret = ops->do_wrmsr(msr, *msr_content, supported);
> +    else if ( !is_write && ops->do_rdmsr )
> +        ret = ops->do_rdmsr(msr, msr_content);
> +    else
> +        goto nop;
> 
> -    if ( vpmu_mode == XENPMU_MODE_OFF )
> +    /*
> +     * We may have received a PMU interrupt while handling MSR access
> +     * and since do_wr/rdmsr may load VPMU context we should save
> +     * (and unload) it again.
> +     */
> +    if ( !is_hvm_vcpu(curr) && vpmu->xenpmu_data &&
> +        vpmu_is_set(vpmu, VPMU_CACHED) )
>      {
> -        *msr_content = 0;
> -        return 0;
> +        vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
> +        ops->arch_vpmu_save(curr, 0);
> +        vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | VPMU_CONTEXT_LOADED);
>      }
> 
> -    vpmu = vcpu_vpmu(curr);
> -    if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_rdmsr )
> -    {
> -        int ret = vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content);
> +    return ret;
> 
> -        if ( !is_hvm_vcpu(curr) && vpmu->xenpmu_data &&
> -             vpmu_is_set(vpmu, VPMU_CACHED) )
> -        {
> -            vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
> -            vpmu->arch_vpmu_ops->arch_vpmu_save(curr, 0);
> -            vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | VPMU_CONTEXT_LOADED);
> -        }
> -        return ret;
> -    }
> -    else
> + nop:
> +    if ( !is_write )
>          *msr_content = 0;
> 
>      return 0;
> diff --git a/xen/include/asm-x86/hvm/vpmu.h b/xen/include/asm-x86/hvm/vpmu.h
> index f486d2f..212e496 100644
> --- a/xen/include/asm-x86/hvm/vpmu.h
> +++ b/xen/include/asm-x86/hvm/vpmu.h
> @@ -101,8 +101,8 @@ static inline bool_t vpmu_are_all_set(const struct 
> vpmu_struct
> *vpmu,
>  }
> 
>  void vpmu_lvtpc_update(uint32_t val);
> -int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t 
> supported);
> -int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content);
> +int vpmu_do_msr(unsigned int msr, uint64_t *msr_content,
> +                uint64_t supported, bool_t is_write);
>  void vpmu_do_interrupt(struct cpu_user_regs *regs);
>  void vpmu_do_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>                                         unsigned int *ecx, unsigned int *edx);
> @@ -112,6 +112,16 @@ void vpmu_save(struct vcpu *v);
>  int vpmu_load(struct vcpu *v, bool_t from_guest);
>  void vpmu_dump(struct vcpu *v);
> 
> +static inline int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content,
> +                                uint64_t supported)
> +{
> +    return vpmu_do_msr(msr, &msr_content, supported, 1);
> +}
> +static inline int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
> +{
> +    return vpmu_do_msr(msr, msr_content, 0, 0);
> +}
> +
>  extern int acquire_pmu_ownership(int pmu_ownership);
>  extern void release_pmu_ownership(int pmu_ownership);
> 
> --
> 1.8.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel