Re: [PATCH v4 04/15] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU

From: "Jan Beulich" <JBeulich@suse.com>
To: Pu Wen <puwen@hygon.cn>
Cc: Wei Liu <wei.liu2@citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Brian Woods <brian.woods@amd.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v4 04/15] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU
Date: Mon, 01 Apr 2019 02:35:21 -0600	[thread overview]
Message-ID: <5CA1CD49020000780022376C@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <e82e93b090e004d2b9a679d656de96ca915e92da.1553935727.git.puwen@hygon.cn>

>>> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote:
> @@ -876,6 +877,10 @@ static int __init vpmu_init(void)
>          if ( amd_vpmu_init() )
>             vpmu_mode = XENPMU_MODE_OFF;
>          break;
> +    case X86_VENDOR_HYGON:
> +        if ( hygon_vpmu_init() )
> +           vpmu_mode = XENPMU_MODE_OFF;
> +        break;
>      case X86_VENDOR_INTEL:
>          if ( core2_vpmu_init() )
>             vpmu_mode = XENPMU_MODE_OFF;

While I realize they're missing right now, I would have appreciated
if you had taken the opportunity to add the missing blank lines
between case blocks here.

> --- a/xen/arch/x86/cpu/vpmu_amd.c
> +++ b/xen/arch/x86/cpu/vpmu_amd.c
> @@ -538,28 +538,12 @@ int svm_vpmu_initialise(struct vcpu *v)
>      return 0;
>  }
>  
> -int __init amd_vpmu_init(void)
> +static int common_init(void)

__init

>  {
>      unsigned int i;
>  
> -    switch ( current_cpu_data.x86 )
> +    if (!num_counters)

Style (missing blanks).

> @@ -586,3 +570,43 @@ int __init amd_vpmu_init(void)
>      return 0;
>  }
>  
> +int __init amd_vpmu_init(void)
> +{
> +    switch ( current_cpu_data.x86 )
> +    {
> +    case 0x15:
> +    case 0x17:
> +        num_counters = F15H_NUM_COUNTERS;
> +        counters = AMD_F15H_COUNTERS;
> +        ctrls = AMD_F15H_CTRLS;
> +        k7_counters_mirrored = 1;
> +        break;
> +    case 0x10:

Same as above.

I won't insist on the first item getting addressed, but the other
three should be. They're all mechanical, so could be done while
committing, and with them in place
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel