Re: [Xen-devel] [PATCH v2 03/14] x86/cpu/vpmu: Add Hygon Dhyana and AMD Zen support for vPMU

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.03.19 at 12:32, <puwen@xxxxxxxx> wrote:
> On 2019/3/18 16:59, Jan Beulich wrote:
>>>>> On 16.03.19 at 11:11, <puwen@xxxxxxxx> wrote:
>>> On 2019/3/15 20:41, Jan Beulich wrote:
>>>>>>> On 21.02.19 at 10:50, <puwen@xxxxxxxx> wrote:
>>>>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>>>>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>>>>> @@ -545,6 +545,8 @@ int __init amd_vpmu_init(void)
>>>>>        switch ( current_cpu_data.x86 )
>>>>>        {
>>>>>        case 0x15:
>>>>> +    case 0x17:
>>>>> +    case 0x18:
>>>>>            num_counters = F15H_NUM_COUNTERS;
>>>>>            counters = AMD_F15H_COUNTERS;
>>>>>            ctrls = AMD_F15H_CTRLS;
>>>>
>>>> Unless you know what AMD Fam18 will look like, you can't do it
>>>> like this. Fam18 really needs to be further qualified by a vendor
>>>> check at this point in time.
>>>
>>> Hygon will negotiate with AMD to make sure that only Hygon should use
>>> Fam18h.
>> 
>> In the success case of which please state this in the description.
>> Until those negotiations have succeeded I'm afraid I'm going to
>> insist to see the extra check added.
> 
> How to check vendor? Maybe like this:
>      case 0x15:
>      case 0x17:
>      case 0x18:
>          if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
>              boot_cpu_data.x86 == 0x18)
>              return -EINVAL;
> 
>          num_counters = F15H_NUM_COUNTERS;
>          counters = AMD_F15H_COUNTERS;
>          ctrls = AMD_F15H_CTRLS;
> 
> or just add Hygon support at beginning of amd_vpmu_init():
>      if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) {
>          num_counters = F15H_NUM_COUNTERS;
>          counters = AMD_F15H_COUNTERS;
>          ctrls = AMD_F15H_CTRLS;
>          k7_counters_mirrored = 1;
>      }

A suitable variant of the latter or

int __init amd_vpmu_init(void)
{
    unsigned int i, fam = current_cpu_data.x86

    /* <suitable comment> */
    if ( current_cpu_data.x86_vendor == X86_VENDOR_HYGON && fam == 0x18 )
       fam = 0x17;

    switch ( fam )
    ...

or perhaps even better would be two separate switch()-es, one for
AMD and one for Hygon. Possibly even a separate hygon_vpmu_init().

Jan



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