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.


>> 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.

-boris


-boris

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

Reply via email to