| Message ID | e82e93b090e004d2b9a679d656de96ca915e92da.1553935727.git.puwen@hygon.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for Hygon Dhyana Family 18h processor | expand |
>>> 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
On 2019/4/1 16:36, Jan Beulich wrote: > 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. Okay, will add the missing blank lines in next version patch series. >> --- 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 Okay, will add it in front of int. >> { >> unsigned int i; >> >> - switch ( current_cpu_data.x86 ) >> + if (!num_counters) > > Style (missing blanks). Will add the missing blanks. >> + 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. Will add a blank line on top of "case 0x10". > 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> Thanks.
>>> On 02.04.19 at 08:46, <puwen@hygon.cn> wrote: > On 2019/4/1 16:36, Jan Beulich wrote: >> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: >>> --- 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 > > Okay, will add it in front of int. Why in front of int? The old line of code here shows the canonical ordering we use. Jan
On 2019/4/2 18:21, Jan Beulich wrote: > On 02.04.19 at 08:46, <puwen@hygon.cn> wrote: >> On 2019/4/1 16:36, Jan Beulich wrote: >>> On 30.03.19 at 11:42, <puwen@hygon.cn> wrote: >>>> --- 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 >> >> Okay, will add it in front of int. > > Why in front of int? The old line of code here shows the canonical > ordering we use. Sorry, after int. :) By the way, how about the patch 01/15 of this series? If it's fine, could you please offer Acked-by tag for it?
>>> On 02.04.19 at 14:11, <puwen@hygon.cn> wrote: > By the way, how about the patch 01/15 of this series? > If it's fine, could you please offer Acked-by tag for it? I've yet to look at v4 of it. Jan
On 2019/4/2 23:50, Jan Beulich wrote: >>>> On 02.04.19 at 14:11, <puwen@hygon.cn> wrote: >> By the way, how about the patch 01/15 of this series? >> If it's fine, could you please offer Acked-by tag for it? > > I've yet to look at v4 of it. Andrew said the change of patch 01/15 should rebase over http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e72309ffbe7c4e507649c74749f130cda691131c . So I think I'll rework the patch first. Thx.
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c index 8f6daf1..93a27d8 100644 --- a/xen/arch/x86/cpu/vpmu.c +++ b/xen/arch/x86/cpu/vpmu.c @@ -456,6 +456,7 @@ static int vpmu_arch_initialise(struct vcpu *v) switch ( vendor ) { case X86_VENDOR_AMD: + case X86_VENDOR_HYGON: ret = svm_vpmu_initialise(v); break; @@ -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; diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c index 5efc39b..47911e5 100644 --- 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) { unsigned int i; - switch ( current_cpu_data.x86 ) + if (!num_counters) { - 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 0x16: - 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 %#x\n", current_cpu_data.x86); return -EINVAL; @@ -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: + case 0x12: + case 0x14: + case 0x16: + num_counters = F10H_NUM_COUNTERS; + counters = AMD_F10H_COUNTERS; + ctrls = AMD_F10H_CTRLS; + k7_counters_mirrored = 0; + break; + } + + return common_init(); +} + +int __init hygon_vpmu_init(void) +{ + switch ( current_cpu_data.x86 ) + { + case 0x18: + num_counters = F15H_NUM_COUNTERS; + counters = AMD_F15H_COUNTERS; + ctrls = AMD_F15H_CTRLS; + k7_counters_mirrored = 1; + break; + } + + return common_init(); +} + diff --git a/xen/include/asm-x86/vpmu.h b/xen/include/asm-x86/vpmu.h index 1287b9f..55f85ba 100644 --- a/xen/include/asm-x86/vpmu.h +++ b/xen/include/asm-x86/vpmu.h @@ -52,6 +52,7 @@ struct arch_vpmu_ops { int core2_vpmu_init(void); int vmx_vpmu_initialise(struct vcpu *); int amd_vpmu_init(void); +int hygon_vpmu_init(void); int svm_vpmu_initialise(struct vcpu *); struct vpmu_struct {
As Hygon Dhyana CPU share similar PMU architecture with AMD family 17h one, so add Hygon Dhyana support in vpmu_arch_initialise() and vpmu_init() by sharing AMD code path. Split the common part in amd_vpmu_init() to a static function _vpmu_init(), making AMD and Hygon to call the shared function to initialize vPMU. As current vPMU still not support AMD Zen(family 17h), add 0x17 support to amd_vpmu_init(). Also create a function hygon_vpmu_init() for Hygon vPMU initialization. Both of AMD 17h and Hygon 18h have the same performance event select and counter MSRs as AMD 15h has, so reuse the 15h definitions for them. Signed-off-by: Pu Wen <puwen@hygon.cn> --- xen/arch/x86/cpu/vpmu.c | 5 ++++ xen/arch/x86/cpu/vpmu_amd.c | 60 +++++++++++++++++++++++++++++++-------------- xen/include/asm-x86/vpmu.h | 1 + 3 files changed, 48 insertions(+), 18 deletions(-)