| Message ID | 20170523212859.6519.69031.stgit@taos.amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/23/2017 05:28 PM, Gary R Hook wrote: > Signed-off-by: Gary R Hook <gary.hook@amd.com> > --- > xen/arch/x86/oprofile/nmi_int.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c > index 13534d491405..5ad48c12e515 100644 > --- a/xen/arch/x86/oprofile/nmi_int.c > +++ b/xen/arch/x86/oprofile/nmi_int.c > @@ -419,6 +419,10 @@ static int __init nmi_init(void) > model = &op_athlon_spec; > cpu_type = "x86-64/family16h"; > break; > + case 0x17: > + model = &op_amd_fam15h_spec; > + cpu_type = "x86-64/family17h"; > + break; > } > break; Have you actually tried this? I don't know whether oprofile still works since corresponding kernel patches that I am aware of are at least 5 years old. -boris
On 5/23/2017 4:46 PM, Boris Ostrovsky wrote: > On 05/23/2017 05:28 PM, Gary R Hook wrote: >> Signed-off-by: Gary R Hook <gary.hook@amd.com> >> --- >> xen/arch/x86/oprofile/nmi_int.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c >> index 13534d491405..5ad48c12e515 100644 >> --- a/xen/arch/x86/oprofile/nmi_int.c >> +++ b/xen/arch/x86/oprofile/nmi_int.c >> @@ -419,6 +419,10 @@ static int __init nmi_init(void) >> model = &op_athlon_spec; >> cpu_type = "x86-64/family16h"; >> break; >> + case 0x17: >> + model = &op_amd_fam15h_spec; >> + cpu_type = "x86-64/family17h"; >> + break; >> } >> break; > > > Have you actually tried this? I don't know whether oprofile still works > since corresponding kernel patches that I am aware of are at least 5 > years old. Yes, I was getting a complaint during boot. That's why I did it. Works a treat on my family 17 system :-) I don't know whether the code is relevant (and if not, it should be removed, right?) but I don't like complaints, no matter how spurious. Thus, I minor patch.
On 05/23/2017 05:51 PM, Gary R Hook wrote: > On 5/23/2017 4:46 PM, Boris Ostrovsky wrote: >> On 05/23/2017 05:28 PM, Gary R Hook wrote: >>> Signed-off-by: Gary R Hook <gary.hook@amd.com> >>> --- >>> xen/arch/x86/oprofile/nmi_int.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/xen/arch/x86/oprofile/nmi_int.c >>> b/xen/arch/x86/oprofile/nmi_int.c >>> index 13534d491405..5ad48c12e515 100644 >>> --- a/xen/arch/x86/oprofile/nmi_int.c >>> +++ b/xen/arch/x86/oprofile/nmi_int.c >>> @@ -419,6 +419,10 @@ static int __init nmi_init(void) >>> model = &op_athlon_spec; >>> cpu_type = "x86-64/family16h"; >>> break; >>> + case 0x17: >>> + model = &op_amd_fam15h_spec; >>> + cpu_type = "x86-64/family17h"; >>> + break; >>> } >>> break; >> >> >> Have you actually tried this? I don't know whether oprofile still works >> since corresponding kernel patches that I am aware of are at least 5 >> years old. > > Yes, I was getting a complaint during boot. That's why I did it. Works > a treat on my family 17 system :-) > > I don't know whether the code is relevant (and if not, it should be > removed, right?) but I don't like complaints, no matter how spurious. > Thus, I minor patch. FWIW, it looks like last meaningful patch to Linux oprofile code (baremetal) was around 2015. -boris
>>> On 23.05.17 at 23:51, <gary.hook@amd.com> wrote: > On 5/23/2017 4:46 PM, Boris Ostrovsky wrote: >> On 05/23/2017 05:28 PM, Gary R Hook wrote: >>> Signed-off-by: Gary R Hook <gary.hook@amd.com> >>> --- >>> xen/arch/x86/oprofile/nmi_int.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/xen/arch/x86/oprofile/nmi_int.c > b/xen/arch/x86/oprofile/nmi_int.c >>> index 13534d491405..5ad48c12e515 100644 >>> --- a/xen/arch/x86/oprofile/nmi_int.c >>> +++ b/xen/arch/x86/oprofile/nmi_int.c >>> @@ -419,6 +419,10 @@ static int __init nmi_init(void) >>> model = &op_athlon_spec; >>> cpu_type = "x86-64/family16h"; >>> break; >>> + case 0x17: >>> + model = &op_amd_fam15h_spec; >>> + cpu_type = "x86-64/family17h"; >>> + break; >>> } >>> break; >> >> >> Have you actually tried this? I don't know whether oprofile still works >> since corresponding kernel patches that I am aware of are at least 5 >> years old. > > Yes, I was getting a complaint during boot. That's why I did it. Works a > treat on my family 17 system :-) I think Boris meant more than just boot a system, i.e. whether you've actually used oprofile successfully with the change. Dealing with the "Initialization failed" message would not necessarily require properly installing handlers - we could also declare newer families unsupported and simply suppress the message in such cases. Note how on most Intel family 6 models code behaves in this very way. Btw, please also note the indentation issue your patch has (spaces vs tabs). Jan
On 5/24/2017 1:43 AM, Jan Beulich wrote: >>>> On 23.05.17 at 23:51, <gary.hook@amd.com> wrote: >> On 5/23/2017 4:46 PM, Boris Ostrovsky wrote: >>> On 05/23/2017 05:28 PM, Gary R Hook wrote: >>>> Signed-off-by: Gary R Hook <gary.hook@amd.com> >>>> --- >>>> xen/arch/x86/oprofile/nmi_int.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/xen/arch/x86/oprofile/nmi_int.c >> b/xen/arch/x86/oprofile/nmi_int.c >>>> index 13534d491405..5ad48c12e515 100644 >>>> --- a/xen/arch/x86/oprofile/nmi_int.c >>>> +++ b/xen/arch/x86/oprofile/nmi_int.c >>>> @@ -419,6 +419,10 @@ static int __init nmi_init(void) >>>> model = &op_athlon_spec; >>>> cpu_type = "x86-64/family16h"; >>>> break; >>>> + case 0x17: >>>> + model = &op_amd_fam15h_spec; >>>> + cpu_type = "x86-64/family17h"; >>>> + break; >>>> } >>>> break; >>> >>> >>> Have you actually tried this? I don't know whether oprofile still works >>> since corresponding kernel patches that I am aware of are at least 5 >>> years old. >> >> Yes, I was getting a complaint during boot. That's why I did it. Works a >> treat on my family 17 system :-) > > I think Boris meant more than just boot a system, i.e. whether you've > actually used oprofile successfully with the change. My interpretation of the state of oprofile is that it's stagnant. Fron what I can tell, the future is 'perf'. I looked around, but could find nothing current for a project. Where does that leave us? > Dealing with the > "Initialization failed" message would not necessarily require properly > installing handlers - we could also declare newer families unsupported > and simply suppress the message in such cases. Note how on most > Intel family 6 models code behaves in this very way. That would be even better, IMHO. What would we like to do? > Btw, please also note the indentation issue your patch has (spaces > vs tabs). I copied a line from above, which uses spaces. My bad, but the badly formatted code has been there for a while.
>>> On 24.05.17 at 17:00, <gary.hook@amd.com> wrote: > On 5/24/2017 1:43 AM, Jan Beulich wrote: >>>>> On 23.05.17 at 23:51, <gary.hook@amd.com> wrote: >>> On 5/23/2017 4:46 PM, Boris Ostrovsky wrote: >>>> On 05/23/2017 05:28 PM, Gary R Hook wrote: >>>>> Signed-off-by: Gary R Hook <gary.hook@amd.com> >>>>> --- >>>>> xen/arch/x86/oprofile/nmi_int.c | 4 ++++ >>>>> 1 file changed, 4 insertions(+) >>>>> >>>>> diff --git a/xen/arch/x86/oprofile/nmi_int.c >>> b/xen/arch/x86/oprofile/nmi_int.c >>>>> index 13534d491405..5ad48c12e515 100644 >>>>> --- a/xen/arch/x86/oprofile/nmi_int.c >>>>> +++ b/xen/arch/x86/oprofile/nmi_int.c >>>>> @@ -419,6 +419,10 @@ static int __init nmi_init(void) >>>>> model = &op_athlon_spec; >>>>> cpu_type = "x86-64/family16h"; >>>>> break; >>>>> + case 0x17: >>>>> + model = &op_amd_fam15h_spec; >>>>> + cpu_type = "x86-64/family17h"; >>>>> + break; >>>>> } >>>>> break; >>>> >>>> >>>> Have you actually tried this? I don't know whether oprofile still works >>>> since corresponding kernel patches that I am aware of are at least 5 >>>> years old. >>> >>> Yes, I was getting a complaint during boot. That's why I did it. Works a >>> treat on my family 17 system :-) >> >> I think Boris meant more than just boot a system, i.e. whether you've >> actually used oprofile successfully with the change. > > My interpretation of the state of oprofile is that it's stagnant. Fron > what I can tell, the future is 'perf'. I looked around, but could find > nothing current for a project. Where does that leave us? Well, in that case you should at the very least clearly state that you did not actually test this. >> Dealing with the >> "Initialization failed" message would not necessarily require properly >> installing handlers - we could also declare newer families unsupported >> and simply suppress the message in such cases. Note how on most >> Intel family 6 models code behaves in this very way. > > That would be even better, IMHO. What would we like to do? Short of verifying whether things actually work with the change, my preference would be to leave the code alone. What's wrong with it saying it doesn't support the family in question? Second best option is to silence it as suggested. Jan
diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c index 13534d491405..5ad48c12e515 100644 --- a/xen/arch/x86/oprofile/nmi_int.c +++ b/xen/arch/x86/oprofile/nmi_int.c @@ -419,6 +419,10 @@ static int __init nmi_init(void) model = &op_athlon_spec; cpu_type = "x86-64/family16h"; break; + case 0x17: + model = &op_amd_fam15h_spec; + cpu_type = "x86-64/family17h"; + break; } break;
Signed-off-by: Gary R Hook <gary.hook@amd.com> --- xen/arch/x86/oprofile/nmi_int.c | 4 ++++ 1 file changed, 4 insertions(+)