Re: [Xen-devel] [PATCH 03/15] x86/cpu/vpmu: Add Hygon Dhyana support for vPMU

[Pu Wen <puwen@xxxxxxxx>]

On 2018/12/21 21:34, Boris Ostrovsky wrote:
> On 12/21/18 5:02 AM, Pu Wen wrote:
>> On 2018/12/20 22:25, Boris Ostrovsky wrote:
>> ...
>>>> diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
>>>> index 5efc39b..e9f0a5c 100644
>>>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>>>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>>>> @@ -554,6 +554,8 @@ int __init amd_vpmu_init(void)
>>>>        case 0x12:
>>>>        case 0x14:
>>>>        case 0x16:
>>>> +    case 0x17:
>>>> +    case 0x18:
>>>
>>> This also enables VPMU support for Zen which goes beyond what the
>>> commit message claims to do.
>> Sorry for the not clear commit message. Will add modification description
>> in the commit message and make the changes complete.
>>
>> On the other hand, since current Xen vPMU still not support Zen. so in
>> this patch we enable 0x17 support. If this modification is not preferred,
>> will remove AMD Xen 0x17 support in next version.
> 
> Enabling 0x17 should be fine, I just thought commit message should be
> explicit about that.

OK, will explicit describe the enabling of 0x17 in the commit message in next
version patch set. Thanks for the suggestion.

>>> Also, why are you choosing to use legacy MSRs (and you did the same in
>>> Linux)? Doesn't Zen (which you are saying is similar to Hygon) support
>>> c001_020X bank?
>> In Linux, the Xen PMU driver use the default branch cases, which also use
>> the legacy MSRs way. So we choose to follow legacy MSRs here in Dhyana
>> cases.
>>
>> Since both of Zen and Dhyana support C001_020X MSRs. If use the C001_020X
>> is preferred, we will try to modify the related codes and create a patch.
> 
> 
> I don't have a Zen box available right now but from what I can see 0x17
> counters are compatible with 0x15 so I think switching to C001_020X
> should work. And looks like you are using those in Linux (non-Xen part) too.
>> Also the Linux Xen PMU driver may need to be updated to use these MSRs.
> 
> Yes, although Linux part is used only by PV guests.

Yes, I have tested the MSRs of the 0x15 ones by booting a Dom0 PV guest.
It works even when the Linux Xen PMU driver use the legacy MSRs. I'll test the
PV guest by using C001_020X in the Linux Xen part tomorrow.

Thx.

-- 
Regards,
Pu Wen

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