Re: [Xen-devel] [PATCH v13 for-xen-4.5 12/21] x86/VPMU: Initialize AND and Intel VPMU with __initcall

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.10.14 at 23:40, <boris.ostrovsky@xxxxxxxxxx> wrote:

Please fix the subject (you surely mean AMD).

> Move common VPMU initilization operations into __initcalls to avoid 
> performing
> same tests and calculations for each vcpu.

Both title and description imply that _all_ the initialization code gets
moved into some __initcall, which is neither reasonable nor does this
match the actual patch.

> @@ -480,30 +429,67 @@ struct arch_vpmu_ops amd_vpmu_ops = {
>  
>  int svm_vpmu_initialise(struct vcpu *v)
>  {
> +    struct xen_pmu_amd_ctxt *ctxt;
>      struct vpmu_struct *vpmu = vcpu_vpmu(v);
> -    uint8_t family = current_cpu_data.x86;
> -    int ret = 0;
>  
> -    /* vpmu enabled? */
> -    if ( vpmu_mode == XENPMU_MODE_OFF )
> +    if ( (vpmu_mode == XENPMU_MODE_OFF) ||
> +         vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
>          return 0;
>  
> -    switch ( family )
> +    if ( !counters )
> +        return -EINVAL;
> +
> +    ctxt = xzalloc_bytes(sizeof(struct xen_pmu_amd_ctxt) +
> +                         2 * sizeof(uint64_t) * num_counters);
> +    if ( !ctxt )
> +    {
> +        gdprintk(XENLOG_WARNING, "Insufficient memory for PMU, "
> +            " PMU feature is unavailable on domain %d vcpu %d.\n",
> +            v->vcpu_id, v->domain->domain_id);

Is this really usefully a gdprintk()? Also please make use of %pv.

> +static int __init amd_vpmu_init(void)
> +{
> +    if ( current_cpu_data.x86_vendor != X86_VENDOR_AMD )
> +        return 0;
> +
> +    switch ( current_cpu_data.x86 )
>      {
> +    case 0x15:
> +        num_counters = F15H_NUM_COUNTERS;
> +        counters = AMD_F15H_COUNTERS;
> +        ctrls = AMD_F15H_CTRLS;
> +        k7_counters_mirrored = 1;
> +        break;
>      case 0x10:
>      case 0x12:
>      case 0x14:
> -    case 0x15:
>      case 0x16:
> -        ret = amd_vpmu_initialise(v);
> -        if ( !ret )
> -            vpmu->arch_vpmu_ops = &amd_vpmu_ops;
> -        return ret;
> +        num_counters = F10H_NUM_COUNTERS;
> +        counters = AMD_F10H_COUNTERS;
> +        ctrls = AMD_F10H_CTRLS;
> +        k7_counters_mirrored = 0;
> +        break;
> +    default:
> +        printk(XENLOG_WARNING "VPMU: Unsupported CPU family 0x%x\n",

%#x please

> @@ -875,16 +877,21 @@ int vmx_vpmu_initialise(struct vcpu *v)
>          /* future: */
>          case 0x3d:
>          case 0x4e:
> -            ret = core2_vpmu_initialise(v);
> -            if ( !ret )
> -                vpmu->arch_vpmu_ops = &core2_vpmu_ops;
> -            return ret;
> -        }
> +            break;
> +    default:
> +        printk(XENLOG_WARNING "VPMU: Unsupported CPU model 0x%x\n",

Again.

Jan


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