Re: [Xen-devel] [PATCH v23 12/15] x86/VPMU: Handle PMU interrupts for PV(H) guests

["Jan Beulich" <JBeulich@xxxxxxxx>]

> @@ -201,27 +202,56 @@ static inline void context_load(struct vcpu *v)
>      }
>  }
>  
> -static void amd_vpmu_load(struct vcpu *v)
> +static int amd_vpmu_load(struct vcpu *v, bool_t from_guest)
>  {
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
> -    struct xen_pmu_amd_ctxt *ctxt = vpmu->context;
> -    uint64_t *ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
> +    struct xen_pmu_amd_ctxt *ctxt;
> +    uint64_t *ctrl_regs;
> +    unsigned int i;
>  
>      vpmu_reset(vpmu, VPMU_FROZEN);
>  
> -    if ( vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
> +    if ( !from_guest && vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
>      {
> -        unsigned int i;
> +        ctxt = vpmu->context;
> +        ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
>  
>          for ( i = 0; i < num_counters; i++ )
>              wrmsrl(ctrls[i], ctrl_regs[i]);
>  
> -        return;
> +        return 0;
> +    }
> +
> +    if ( from_guest )
> +    {
> +        unsigned int num_enabled = 0;
> +
> +        ASSERT(!is_hvm_vcpu(v));
> +
> +        ctxt = &vpmu->xenpmu_data->pmu.c.amd;
> +        ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
> +        for ( i = 0; i < num_counters; i++ )
> +        {
> +            if ( (ctrl_regs[i] & CTRL_RSVD_MASK) != ctrl_rsvd[i] )
> +                return -EINVAL;
> +
> +            if ( is_pmu_enabled(ctrl_regs[i]) )
> +                num_enabled++;
> +        }
> +
> +        if ( num_enabled )
> +            vpmu_set(vpmu, VPMU_RUNNING);
> +        else
> +            vpmu_reset(vpmu, VPMU_RUNNING);
> +
> +        memcpy(vpmu->context, &vpmu->xenpmu_data->pmu.c.amd, ctxt_sz);

For one, why don't you use ctxt here? And then I think I made very
clear that all shared data must be read exactly once during any
hypervisor operation. Yet afaict you do two reads (one in the
verification loop above, and another in the memcpy()).

Jan


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