Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1200005 > unrolled thread

RE: [tip:perf/core] perf/x86: Add an MSR PMU driver

Started by"Liang, Kan" <kan.liang@intel.com>
First post2015-08-04 17:10 +0200
Last post2015-08-06 19:50 +0200
Articles 13 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  RE: [tip:perf/core] perf/x86: Add an MSR PMU driver "Liang, Kan" <kan.liang@intel.com> - 2015-08-04 17:10 +0200
    Re: [tip:perf/core] perf/x86: Add an MSR PMU driver Andy Lutomirski <luto@amacapital.net> - 2015-08-04 20:10 +0200
      RE: [tip:perf/core] perf/x86: Add an MSR PMU driver "Liang, Kan" <kan.liang@intel.com> - 2015-08-04 20:20 +0200
        Re: [tip:perf/core] perf/x86: Add an MSR PMU driver Andy Lutomirski <luto@amacapital.net> - 2015-08-04 20:20 +0200
          RE: [tip:perf/core] perf/x86: Add an MSR PMU driver "Liang, Kan" <kan.liang@intel.com> - 2015-08-04 23:00 +0200
            Re: [tip:perf/core] perf/x86: Add an MSR PMU driver Peter Zijlstra <peterz@infradead.org> - 2015-08-06 17:30 +0200
              Re: [tip:perf/core] perf/x86: Add an MSR PMU driver Andy Lutomirski <luto@amacapital.net> - 2015-08-06 17:40 +0200
                Re: [tip:perf/core] perf/x86: Add an MSR PMU driver Peter Zijlstra <peterz@infradead.org> - 2015-08-06 18:00 +0200
                  Re: [tip:perf/core] perf/x86: Add an MSR PMU driver Peter Zijlstra <peterz@infradead.org> - 2015-08-07 10:40 +0200
                    Re: [tip:perf/core] perf/x86: Add an MSR PMU driver Andy Lutomirski <luto@amacapital.net> - 2015-08-10 04:20 +0200
                    RE: [tip:perf/core] perf/x86: Add an MSR PMU driver "Liang, Kan" <kan.liang@intel.com> - 2015-08-10 16:10 +0200
                      Re: [tip:perf/core] perf/x86: Add an MSR PMU driver Peter Zijlstra <peterz@infradead.org> - 2015-08-10 16:30 +0200
              RE: [tip:perf/core] perf/x86: Add an MSR PMU driver "Liang, Kan" <kan.liang@intel.com> - 2015-08-06 19:50 +0200

#1200005 — RE: [tip:perf/core] perf/x86: Add an MSR PMU driver

From"Liang, Kan" <kan.liang@intel.com>
Date2015-08-04 17:10 +0200
SubjectRE: [tip:perf/core] perf/x86: Add an MSR PMU driver
Message-ID<pTM0Q-5Ik-61@gated-at.bofh.it>
DQo+ID4gKw0KPiA+ICtlbnVtIHBlcmZfbXNyX2lkIHsNCj4gPiArICAgICAgIFBFUkZfTVNSX1RT
QyAgICAgICAgICAgICAgICAgICAgPSAwLA0KPiA+ICsgICAgICAgUEVSRl9NU1JfQVBFUkYgICAg
ICAgICAgICAgICAgICA9IDEsDQo+ID4gKyAgICAgICBQRVJGX01TUl9NUEVSRiAgICAgICAgICAg
ICAgICAgID0gMiwNCj4gPiArICAgICAgIFBFUkZfTVNSX1BQRVJGICAgICAgICAgICAgICAgICAg
PSAzLA0KPiA+ICsgICAgICAgUEVSRl9NU1JfU01JICAgICAgICAgICAgICAgICAgICA9IDQsDQo+
ID4gKw0KPiA+ICsgICAgICAgUEVSRl9NU1JfRVZFTlRfTUFYLA0KPiA+ICt9Ow0KPiA+ICsNCj4g
PiArc3RydWN0IHBlcmZfbXNyIHsNCj4gPiArICAgICAgIGludCAgICAgaWQ7DQo+ID4gKyAgICAg
ICB1NjQgICAgIG1zcjsNCj4gPiArfTsNCj4gPiArDQo+ID4gK3N0YXRpYyBzdHJ1Y3QgcGVyZl9t
c3IgbXNyW10gPSB7DQo+ID4gKyAgICAgICB7IFBFUkZfTVNSX1RTQywgMCB9LA0KPiA+ICsgICAg
ICAgeyBQRVJGX01TUl9BUEVSRiwgTVNSX0lBMzJfQVBFUkYgfSwNCj4gPiArICAgICAgIHsgUEVS
Rl9NU1JfTVBFUkYsIE1TUl9JQTMyX01QRVJGIH0sDQo+ID4gKyAgICAgICB7IFBFUkZfTVNSX1BQ
RVJGLCBNU1JfUFBFUkYgfSwNCj4gPiArICAgICAgIHsgUEVSRl9NU1JfU01JLCBNU1JfU01JX0NP
VU5UIH0sIH07DQo+IA0KPiBJIHRoaW5rIHRoaXMgY291bGQgYmUgZWFzaWVyIHRvIHdvcmsgd2l0
aCBpZiBpdCB3ZXJlIFtQRVJGX01TUl9UU0NdID0gey4uLn0sDQo+IGV0Yy4gIE5vIGJpZyBkZWFs
LCB0aG91Z2gsIHVudGlsIHRoZSBsaXN0IGdldHMgbG9uZy4gIEhvd2V2ZXIsIGl0IG1pZ2h0IG1h
a2UNCj4gZml4aW5nIHRoZSBhcHBhcmVudCBpc3N1ZSBiZWxvdyBlYXNpZXIuLi4NCj4gDQo+ID4g
K3N0YXRpYyBpbnQgbXNyX2V2ZW50X2luaXQoc3RydWN0IHBlcmZfZXZlbnQgKmV2ZW50KSB7DQo+
ID4gKyAgICAgICB1NjQgY2ZnID0gZXZlbnQtPmF0dHIuY29uZmlnOw0KPiANCj4gLi4uDQo+IA0K
PiA+ICsgICAgICAgZXZlbnQtPmh3LmV2ZW50X2Jhc2UgPSBtc3JbY2ZnXS5tc3I7DQo+IA0KPiBT
aG91bGRuJ3QgdGhpcyB2ZXJpZnkgdGhhdCB0aGUgZmFuY3kgZW51bWVyYXRpb24gY29kZSBhY3R1
YWxseSBiZWxpZXZlcyB0aGF0DQo+IG1zcltjZmddIGV4aXN0cyBvbiB0aGlzIHN5c3RlbT8gT3Ro
ZXJ3aXNlIHdlIG1pZ2h0IGhhdmUgYSB2ZXJ5IHNob3J0IHdhaXQNCj4gdW50aWwgdGhlIHBlcmYg
ZnV6emVyIG9vcHNlcyB0aGlzIHRoaW5nIDopDQo+IA0KDQpJIHRoaW5rIHdlIGFscmVhZHkgZGlk
IHRoZSBjaGVjayBiZWZvcmUgdXNpbmcgbXNyW2NmZ10uDQoNCj4gKw0KPiArCWlmIChjZmcgPj0g
UEVSRl9NU1JfRVZFTlRfTUFYKQ0KPiArCQlyZXR1cm4gLUVJTlZBTDsNCj4gKw0KDQpLYW4NCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1200202

FromAndy Lutomirski <luto@amacapital.net>
Date2015-08-04 20:10 +0200
Message-ID<pTOP0-1jD-31@gated-at.bofh.it>
In reply to#1200005
On Tue, Aug 4, 2015 at 8:03 AM, Liang, Kan <kan.liang@intel.com> wrote:
>
>> > +
>> > +enum perf_msr_id {
>> > +       PERF_MSR_TSC                    = 0,
>> > +       PERF_MSR_APERF                  = 1,
>> > +       PERF_MSR_MPERF                  = 2,
>> > +       PERF_MSR_PPERF                  = 3,
>> > +       PERF_MSR_SMI                    = 4,
>> > +
>> > +       PERF_MSR_EVENT_MAX,
>> > +};
>> > +
>> > +struct perf_msr {
>> > +       int     id;
>> > +       u64     msr;
>> > +};
>> > +
>> > +static struct perf_msr msr[] = {
>> > +       { PERF_MSR_TSC, 0 },
>> > +       { PERF_MSR_APERF, MSR_IA32_APERF },
>> > +       { PERF_MSR_MPERF, MSR_IA32_MPERF },
>> > +       { PERF_MSR_PPERF, MSR_PPERF },
>> > +       { PERF_MSR_SMI, MSR_SMI_COUNT }, };
>>
>> I think this could be easier to work with if it were [PERF_MSR_TSC] = {...},
>> etc.  No big deal, though, until the list gets long.  However, it might make
>> fixing the apparent issue below easier...
>>
>> > +static int msr_event_init(struct perf_event *event) {
>> > +       u64 cfg = event->attr.config;
>>
>> ...
>>
>> > +       event->hw.event_base = msr[cfg].msr;
>>
>> Shouldn't this verify that the fancy enumeration code actually believes that
>> msr[cfg] exists on this system? Otherwise we might have a very short wait
>> until the perf fuzzer oopses this thing :)
>>
>
> I think we already did the check before using msr[cfg].

Where?  All I see is:

+       if (cfg >= PERF_MSR_EVENT_MAX)
+               return -EINVAL;

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1200211

From"Liang, Kan" <kan.liang@intel.com>
Date2015-08-04 20:20 +0200
Message-ID<pTOYG-1uV-27@gated-at.bofh.it>
In reply to#1200202
DQoNCj4gDQo+IE9uIFR1ZSwgQXVnIDQsIDIwMTUgYXQgODowMyBBTSwgTGlhbmcsIEthbiA8a2Fu
LmxpYW5nQGludGVsLmNvbT4gd3JvdGU6DQo+ID4NCj4gPj4gPiArDQo+ID4+ID4gK2VudW0gcGVy
Zl9tc3JfaWQgew0KPiA+PiA+ICsgICAgICAgUEVSRl9NU1JfVFNDICAgICAgICAgICAgICAgICAg
ICA9IDAsDQo+ID4+ID4gKyAgICAgICBQRVJGX01TUl9BUEVSRiAgICAgICAgICAgICAgICAgID0g
MSwNCj4gPj4gPiArICAgICAgIFBFUkZfTVNSX01QRVJGICAgICAgICAgICAgICAgICAgPSAyLA0K
PiA+PiA+ICsgICAgICAgUEVSRl9NU1JfUFBFUkYgICAgICAgICAgICAgICAgICA9IDMsDQo+ID4+
ID4gKyAgICAgICBQRVJGX01TUl9TTUkgICAgICAgICAgICAgICAgICAgID0gNCwNCj4gPj4gPiAr
DQo+ID4+ID4gKyAgICAgICBQRVJGX01TUl9FVkVOVF9NQVgsDQo+ID4+ID4gK307DQo+ID4+ID4g
Kw0KPiA+PiA+ICtzdHJ1Y3QgcGVyZl9tc3Igew0KPiA+PiA+ICsgICAgICAgaW50ICAgICBpZDsN
Cj4gPj4gPiArICAgICAgIHU2NCAgICAgbXNyOw0KPiA+PiA+ICt9Ow0KPiA+PiA+ICsNCj4gPj4g
PiArc3RhdGljIHN0cnVjdCBwZXJmX21zciBtc3JbXSA9IHsNCj4gPj4gPiArICAgICAgIHsgUEVS
Rl9NU1JfVFNDLCAwIH0sDQo+ID4+ID4gKyAgICAgICB7IFBFUkZfTVNSX0FQRVJGLCBNU1JfSUEz
Ml9BUEVSRiB9LA0KPiA+PiA+ICsgICAgICAgeyBQRVJGX01TUl9NUEVSRiwgTVNSX0lBMzJfTVBF
UkYgfSwNCj4gPj4gPiArICAgICAgIHsgUEVSRl9NU1JfUFBFUkYsIE1TUl9QUEVSRiB9LA0KPiA+
PiA+ICsgICAgICAgeyBQRVJGX01TUl9TTUksIE1TUl9TTUlfQ09VTlQgfSwgfTsNCj4gPj4NCj4g
Pj4gSSB0aGluayB0aGlzIGNvdWxkIGJlIGVhc2llciB0byB3b3JrIHdpdGggaWYgaXQgd2VyZSBb
UEVSRl9NU1JfVFNDXSA9DQo+ID4+IHsuLi59LCBldGMuICBObyBiaWcgZGVhbCwgdGhvdWdoLCB1
bnRpbCB0aGUgbGlzdCBnZXRzIGxvbmcuICBIb3dldmVyLA0KPiA+PiBpdCBtaWdodCBtYWtlIGZp
eGluZyB0aGUgYXBwYXJlbnQgaXNzdWUgYmVsb3cgZWFzaWVyLi4uDQo+ID4+DQo+ID4+ID4gK3N0
YXRpYyBpbnQgbXNyX2V2ZW50X2luaXQoc3RydWN0IHBlcmZfZXZlbnQgKmV2ZW50KSB7DQo+ID4+
ID4gKyAgICAgICB1NjQgY2ZnID0gZXZlbnQtPmF0dHIuY29uZmlnOw0KPiA+Pg0KPiA+PiAuLi4N
Cj4gPj4NCj4gPj4gPiArICAgICAgIGV2ZW50LT5ody5ldmVudF9iYXNlID0gbXNyW2NmZ10ubXNy
Ow0KPiA+Pg0KPiA+PiBTaG91bGRuJ3QgdGhpcyB2ZXJpZnkgdGhhdCB0aGUgZmFuY3kgZW51bWVy
YXRpb24gY29kZSBhY3R1YWxseQ0KPiA+PiBiZWxpZXZlcyB0aGF0IG1zcltjZmddIGV4aXN0cyBv
biB0aGlzIHN5c3RlbT8gT3RoZXJ3aXNlIHdlIG1pZ2h0IGhhdmUNCj4gPj4gYSB2ZXJ5IHNob3J0
IHdhaXQgdW50aWwgdGhlIHBlcmYgZnV6emVyIG9vcHNlcyB0aGlzIHRoaW5nIDopDQo+ID4+DQo+
ID4NCj4gPiBJIHRoaW5rIHdlIGFscmVhZHkgZGlkIHRoZSBjaGVjayBiZWZvcmUgdXNpbmcgbXNy
W2NmZ10uDQo+IA0KPiBXaGVyZT8gIEFsbCBJIHNlZSBpczoNCj4gDQo+ICsgICAgICAgaWYgKGNm
ZyA+PSBQRVJGX01TUl9FVkVOVF9NQVgpDQo+ICsgICAgICAgICAgICAgICByZXR1cm4gLUVJTlZB
TDsNCg0KWWVzLCB3ZSBjaGVjayBjZmcgaGVyZS4gU28gbXNyW2NmZ10gc2hvdWxkIGJlIGFsd2F5
cyBhdmFpbGFibGUuDQoNCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1200213

FromAndy Lutomirski <luto@amacapital.net>
Date2015-08-04 20:20 +0200
Message-ID<pTOYG-1uV-25@gated-at.bofh.it>
In reply to#1200211
On Tue, Aug 4, 2015 at 11:11 AM, Liang, Kan <kan.liang@intel.com> wrote:
>
>
>>
>> On Tue, Aug 4, 2015 at 8:03 AM, Liang, Kan <kan.liang@intel.com> wrote:
>> >
>> >> > +
>> >> > +enum perf_msr_id {
>> >> > +       PERF_MSR_TSC                    = 0,
>> >> > +       PERF_MSR_APERF                  = 1,
>> >> > +       PERF_MSR_MPERF                  = 2,
>> >> > +       PERF_MSR_PPERF                  = 3,
>> >> > +       PERF_MSR_SMI                    = 4,
>> >> > +
>> >> > +       PERF_MSR_EVENT_MAX,
>> >> > +};
>> >> > +
>> >> > +struct perf_msr {
>> >> > +       int     id;
>> >> > +       u64     msr;
>> >> > +};
>> >> > +
>> >> > +static struct perf_msr msr[] = {
>> >> > +       { PERF_MSR_TSC, 0 },
>> >> > +       { PERF_MSR_APERF, MSR_IA32_APERF },
>> >> > +       { PERF_MSR_MPERF, MSR_IA32_MPERF },
>> >> > +       { PERF_MSR_PPERF, MSR_PPERF },
>> >> > +       { PERF_MSR_SMI, MSR_SMI_COUNT }, };
>> >>
>> >> I think this could be easier to work with if it were [PERF_MSR_TSC] =
>> >> {...}, etc.  No big deal, though, until the list gets long.  However,
>> >> it might make fixing the apparent issue below easier...
>> >>
>> >> > +static int msr_event_init(struct perf_event *event) {
>> >> > +       u64 cfg = event->attr.config;
>> >>
>> >> ...
>> >>
>> >> > +       event->hw.event_base = msr[cfg].msr;
>> >>
>> >> Shouldn't this verify that the fancy enumeration code actually
>> >> believes that msr[cfg] exists on this system? Otherwise we might have
>> >> a very short wait until the perf fuzzer oopses this thing :)
>> >>
>> >
>> > I think we already did the check before using msr[cfg].
>>
>> Where?  All I see is:
>>
>> +       if (cfg >= PERF_MSR_EVENT_MAX)
>> +               return -EINVAL;
>
> Yes, we check cfg here. So msr[cfg] should be always available.
>

PERF_MSR_EVENT_MAX is a constant.  If I run this thing on an AMD CPU
that supports TSC, APERF, MPERF, and nothing else, and someone asks
for PPERF, then the check will succeed and we'll oops, right?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1200276

From"Liang, Kan" <kan.liang@intel.com>
Date2015-08-04 23:00 +0200
Message-ID<pTRtw-55z-3@gated-at.bofh.it>
In reply to#1200213
DQoNCj4gT24gVHVlLCBBdWcgNCwgMjAxNSBhdCAxMToxMSBBTSwgTGlhbmcsIEthbiA8a2FuLmxp
YW5nQGludGVsLmNvbT4gd3JvdGU6DQo+ID4NCj4gPg0KPiA+Pg0KPiA+PiBPbiBUdWUsIEF1ZyA0
LCAyMDE1IGF0IDg6MDMgQU0sIExpYW5nLCBLYW4gPGthbi5saWFuZ0BpbnRlbC5jb20+DQo+IHdy
b3RlOg0KPiA+PiA+DQo+ID4+ID4+ID4gKw0KPiA+PiA+PiA+ICtlbnVtIHBlcmZfbXNyX2lkIHsN
Cj4gPj4gPj4gPiArICAgICAgIFBFUkZfTVNSX1RTQyAgICAgICAgICAgICAgICAgICAgPSAwLA0K
PiA+PiA+PiA+ICsgICAgICAgUEVSRl9NU1JfQVBFUkYgICAgICAgICAgICAgICAgICA9IDEsDQo+
ID4+ID4+ID4gKyAgICAgICBQRVJGX01TUl9NUEVSRiAgICAgICAgICAgICAgICAgID0gMiwNCj4g
Pj4gPj4gPiArICAgICAgIFBFUkZfTVNSX1BQRVJGICAgICAgICAgICAgICAgICAgPSAzLA0KPiA+
PiA+PiA+ICsgICAgICAgUEVSRl9NU1JfU01JICAgICAgICAgICAgICAgICAgICA9IDQsDQo+ID4+
ID4+ID4gKw0KPiA+PiA+PiA+ICsgICAgICAgUEVSRl9NU1JfRVZFTlRfTUFYLA0KPiA+PiA+PiA+
ICt9Ow0KPiA+PiA+PiA+ICsNCj4gPj4gPj4gPiArc3RydWN0IHBlcmZfbXNyIHsNCj4gPj4gPj4g
PiArICAgICAgIGludCAgICAgaWQ7DQo+ID4+ID4+ID4gKyAgICAgICB1NjQgICAgIG1zcjsNCj4g
Pj4gPj4gPiArfTsNCj4gPj4gPj4gPiArDQo+ID4+ID4+ID4gK3N0YXRpYyBzdHJ1Y3QgcGVyZl9t
c3IgbXNyW10gPSB7DQo+ID4+ID4+ID4gKyAgICAgICB7IFBFUkZfTVNSX1RTQywgMCB9LA0KPiA+
PiA+PiA+ICsgICAgICAgeyBQRVJGX01TUl9BUEVSRiwgTVNSX0lBMzJfQVBFUkYgfSwNCj4gPj4g
Pj4gPiArICAgICAgIHsgUEVSRl9NU1JfTVBFUkYsIE1TUl9JQTMyX01QRVJGIH0sDQo+ID4+ID4+
ID4gKyAgICAgICB7IFBFUkZfTVNSX1BQRVJGLCBNU1JfUFBFUkYgfSwNCj4gPj4gPj4gPiArICAg
ICAgIHsgUEVSRl9NU1JfU01JLCBNU1JfU01JX0NPVU5UIH0sIH07DQo+ID4+ID4+DQo+ID4+ID4+
IEkgdGhpbmsgdGhpcyBjb3VsZCBiZSBlYXNpZXIgdG8gd29yayB3aXRoIGlmIGl0IHdlcmUNCj4g
Pj4gPj4gW1BFUkZfTVNSX1RTQ10gPSB7Li4ufSwgZXRjLiAgTm8gYmlnIGRlYWwsIHRob3VnaCwg
dW50aWwgdGhlIGxpc3QNCj4gPj4gPj4gZ2V0cyBsb25nLiAgSG93ZXZlciwgaXQgbWlnaHQgbWFr
ZSBmaXhpbmcgdGhlIGFwcGFyZW50IGlzc3VlIGJlbG93DQo+IGVhc2llci4uLg0KPiA+PiA+Pg0K
PiA+PiA+PiA+ICtzdGF0aWMgaW50IG1zcl9ldmVudF9pbml0KHN0cnVjdCBwZXJmX2V2ZW50ICpl
dmVudCkgew0KPiA+PiA+PiA+ICsgICAgICAgdTY0IGNmZyA9IGV2ZW50LT5hdHRyLmNvbmZpZzsN
Cj4gPj4gPj4NCj4gPj4gPj4gLi4uDQo+ID4+ID4+DQo+ID4+ID4+ID4gKyAgICAgICBldmVudC0+
aHcuZXZlbnRfYmFzZSA9IG1zcltjZmddLm1zcjsNCj4gPj4gPj4NCj4gPj4gPj4gU2hvdWxkbid0
IHRoaXMgdmVyaWZ5IHRoYXQgdGhlIGZhbmN5IGVudW1lcmF0aW9uIGNvZGUgYWN0dWFsbHkNCj4g
Pj4gPj4gYmVsaWV2ZXMgdGhhdCBtc3JbY2ZnXSBleGlzdHMgb24gdGhpcyBzeXN0ZW0/IE90aGVy
d2lzZSB3ZSBtaWdodA0KPiA+PiA+PiBoYXZlIGEgdmVyeSBzaG9ydCB3YWl0IHVudGlsIHRoZSBw
ZXJmIGZ1enplciBvb3BzZXMgdGhpcyB0aGluZyA6KQ0KPiA+PiA+Pg0KPiA+PiA+DQo+ID4+ID4g
SSB0aGluayB3ZSBhbHJlYWR5IGRpZCB0aGUgY2hlY2sgYmVmb3JlIHVzaW5nIG1zcltjZmddLg0K
PiA+Pg0KPiA+PiBXaGVyZT8gIEFsbCBJIHNlZSBpczoNCj4gPj4NCj4gPj4gKyAgICAgICBpZiAo
Y2ZnID49IFBFUkZfTVNSX0VWRU5UX01BWCkNCj4gPj4gKyAgICAgICAgICAgICAgIHJldHVybiAt
RUlOVkFMOw0KPiA+DQo+ID4gWWVzLCB3ZSBjaGVjayBjZmcgaGVyZS4gU28gbXNyW2NmZ10gc2hv
dWxkIGJlIGFsd2F5cyBhdmFpbGFibGUuDQo+ID4NCj4gDQo+IFBFUkZfTVNSX0VWRU5UX01BWCBp
cyBhIGNvbnN0YW50LiAgSWYgSSBydW4gdGhpcyB0aGluZyBvbiBhbiBBTUQgQ1BVDQo+IHRoYXQg
c3VwcG9ydHMgVFNDLCBBUEVSRiwgTVBFUkYsIGFuZCBub3RoaW5nIGVsc2UsIGFuZCBzb21lb25l
IGFza3MgZm9yDQo+IFBQRVJGLCB0aGVuIHRoZSBjaGVjayB3aWxsIHN1Y2NlZWQgYW5kIHdlJ2xs
IG9vcHMsIHJpZ2h0Pw0KPiANCg0KUmlnaHQsIGl0IGNvdWxkIGJlIGEgcHJvYmxlbS4NCkhvdyBh
Ym91dCB0aGUgcGF0Y2ggYXMgYmVsb3c/DQoNCi0tLQ0KRnJvbSAwMjE3ZmZjOWEwZDJmYWM2NDE3
NTUyYjlmNjZmYWZjNTM4ZWY5MDY4IE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQ0KRGF0ZTogVHVl
LCA0IEF1ZyAyMDE1IDA4OjI3OjE5IC0wNDAwDQpTdWJqZWN0OiBbUEFUQ0ggMS8xXSBwZXJmL3g4
Ni9tc3I6IEZpeCBpc3N1ZSBvZiBhY2Nlc3NpbmcgdW5zdXBwb3J0ZWQgTVNSDQogZXZlbnRzDQoN
Ck1vc3Qgb2YgcGxhdGZvcm1zIG9ubHkgc3VwcG9ydCBwYXJ0IG9mIE1TUiBldmVudHMuIElmIHVu
c3VwcG9ydGVkIE1TUg0KZXZlbnRzIGFyZSBtaXN0YWtlbmx5IGFjY2Vzc2VkLCBpdCBtYXkgY2F1
c2Ugb29wcy4NCkludHJvZHVjaW5nIC5hdmFpbGFibGUgdG8gbWFyayB0aGUgc3VwcG9ydGVkIE1T
UiBldmVudHMsIGFuZCBjaGVjayBpdCBpbg0KZXZlbnQgaW5pdC4NCg0KUmVwb3J0ZWQtYnk6IEFu
ZHkgTHV0b21pcnNraSA8bHV0b0BrZXJuZWwub3JnPg0KU2lnbmVkLW9mZi1ieTogS2FuIExpYW5n
IDxrYW4ubGlhbmdAaW50ZWwuY29tPg0KLS0tDQogYXJjaC94ODYva2VybmVsL2NwdS9wZXJmX2V2
ZW50X21zci5jIHwgMjEgKysrKysrKysrKysrKysrKy0tLS0tDQogMSBmaWxlIGNoYW5nZWQsIDE2
IGluc2VydGlvbnMoKyksIDUgZGVsZXRpb25zKC0pDQoNCmRpZmYgLS1naXQgYS9hcmNoL3g4Ni9r
ZXJuZWwvY3B1L3BlcmZfZXZlbnRfbXNyLmMgYi9hcmNoL3g4Ni9rZXJuZWwvY3B1L3BlcmZfZXZl
bnRfbXNyLmMNCmluZGV4IGFmMjE2ZTkuLmMxNzQyNDIgMTAwNjQ0DQotLS0gYS9hcmNoL3g4Ni9r
ZXJuZWwvY3B1L3BlcmZfZXZlbnRfbXNyLmMNCisrKyBiL2FyY2gveDg2L2tlcm5lbC9jcHUvcGVy
Zl9ldmVudF9tc3IuYw0KQEAgLTEzLDE0ICsxMywxNSBAQCBlbnVtIHBlcmZfbXNyX2lkIHsNCiBz
dHJ1Y3QgcGVyZl9tc3Igew0KIAlpbnQJaWQ7DQogCXU2NAltc3I7DQorCWJvb2wJYXZhaWxhYmxl
Ow0KIH07DQogDQogc3RhdGljIHN0cnVjdCBwZXJmX21zciBtc3JbXSA9IHsNCi0JeyBQRVJGX01T
Ul9UU0MsIDAgfSwNCi0JeyBQRVJGX01TUl9BUEVSRiwgTVNSX0lBMzJfQVBFUkYgfSwNCi0JeyBQ
RVJGX01TUl9NUEVSRiwgTVNSX0lBMzJfTVBFUkYgfSwNCi0JeyBQRVJGX01TUl9QUEVSRiwgTVNS
X1BQRVJGIH0sDQotCXsgUEVSRl9NU1JfU01JLCBNU1JfU01JX0NPVU5UIH0sDQorCXsgUEVSRl9N
U1JfVFNDLCAwLCB0cnVlIH0sDQorCXsgUEVSRl9NU1JfQVBFUkYsIE1TUl9JQTMyX0FQRVJGLCBm
YWxzZSB9LA0KKwl7IFBFUkZfTVNSX01QRVJGLCBNU1JfSUEzMl9NUEVSRiwgZmFsc2UgfSwNCisJ
eyBQRVJGX01TUl9QUEVSRiwgTVNSX1BQRVJGLCBmYWxzZSB9LA0KKwl7IFBFUkZfTVNSX1NNSSwg
TVNSX1NNSV9DT1VOVCwgZmFsc2UgfSwNCiB9Ow0KIA0KIFBNVV9FVkVOVF9BVFRSX1NUUklORyh0
c2MsICAgZXZhdHRyX3RzYywgICAiZXZlbnQ9MHgwMCIpOw0KQEAgLTc0LDYgKzc1LDEwIEBAIHN0
YXRpYyBpbnQgbXNyX2V2ZW50X2luaXQoc3RydWN0IHBlcmZfZXZlbnQgKmV2ZW50KQ0KIAkgICAg
ZXZlbnQtPmF0dHIuc2FtcGxlX3BlcmlvZCkgLyogbm8gc2FtcGxpbmcgKi8NCiAJCXJldHVybiAt
RUlOVkFMOw0KIA0KKwkvKiBUaGUgcGxhdGZvcm0gbWF5IG5vdCBzdXBwb3J0IGFsbCBldmVudHMg
Ki8NCisJaWYgKCFtc3JbY2ZnXS5hdmFpbGFibGUpDQorCQlyZXR1cm4gLUVJTlZBTDsNCisNCiAJ
ZXZlbnQtPmh3LmlkeCA9IC0xOw0KIAlldmVudC0+aHcuZXZlbnRfYmFzZSA9IG1zcltjZmddLm1z
cjsNCiAJZXZlbnQtPmh3LmNvbmZpZyA9IGNmZzsNCkBAIC0xODEsMTggKzE4NiwyMiBAQCBzdGF0
aWMgaW50IF9faW5pdCBpbnRlbF9tc3JfaW5pdChpbnQgaWR4KQ0KIAljYXNlIDcxOiAvKiAxNG5t
IEJyb2Fkd2VsbCArIEdUM2UgKEludGVsIElyaXMgUHJvIGdyYXBoaWNzKSAqLw0KIAljYXNlIDc5
OiAvKiAxNG5tIEJyb2Fkd2VsbCBTZXJ2ZXIgKi8NCiAJCWV2ZW50c19hdHRyc1tpZHgrK10gPSAm
ZXZhdHRyX3NtaS5hdHRyLmF0dHI7DQorCQltc3JbUEVSRl9NU1JfU01JXS5hdmFpbGFibGUgPSB0
cnVlOw0KIAkJYnJlYWs7DQogDQogCWNhc2UgNzg6IC8qIDE0bm0gU2t5bGFrZSBNb2JpbGUgKi8N
CiAJY2FzZSA5NDogLyogMTRubSBTa3lsYWtlIERlc2t0b3AgKi8NCiAJCWV2ZW50c19hdHRyc1tp
ZHgrK10gPSAmZXZhdHRyX3BwZXJmLmF0dHIuYXR0cjsNCiAJCWV2ZW50c19hdHRyc1tpZHgrK10g
PSAmZXZhdHRyX3NtaS5hdHRyLmF0dHI7DQorCQltc3JbUEVSRl9NU1JfUFBFUkZdLmF2YWlsYWJs
ZSA9IHRydWU7DQorCQltc3JbUEVSRl9NU1JfU01JXS5hdmFpbGFibGUgPSB0cnVlOw0KIAkJYnJl
YWs7DQogDQogCWNhc2UgNTU6IC8qIDIybm0gQXRvbSAiU2lsdmVybW9udCIgICAgICAgICAgICAg
ICAgKi8NCiAJY2FzZSA3NjogLyogMTRubSBBdG9tICJBaXJtb250IiAgICAgICAgICAgICAgICAg
ICAqLw0KIAljYXNlIDc3OiAvKiAyMm5tIEF0b20gIlNpbHZlcm1vbnQgQXZvdG9uL1JhbmdlbHki
ICovDQogCQlldmVudHNfYXR0cnNbaWR4KytdID0gJmV2YXR0cl9zbWkuYXR0ci5hdHRyOw0KKwkJ
bXNyW1BFUkZfTVNSX1NNSV0uYXZhaWxhYmxlID0gdHJ1ZTsNCiAJCWJyZWFrOw0KIAl9DQogDQpA
QCAtMjE1LDYgKzIyNCw4IEBAIHN0YXRpYyBpbnQgX19pbml0IG1zcl9pbml0KHZvaWQpDQogCQll
dmVudHNfYXR0cnNbaWR4KytdID0gJmV2YXR0cl9hcGVyZi5hdHRyLmF0dHI7DQogCQlldmVudHNf
YXR0cnNbaWR4KytdID0gJmV2YXR0cl9tcGVyZi5hdHRyLmF0dHI7DQogCQlldmVudHNfYXR0cnNb
aWR4XSA9IE5VTEw7DQorCQltc3JbUEVSRl9NU1JfQVBFUkZdLmF2YWlsYWJsZSA9IHRydWU7DQor
CQltc3JbUEVSRl9NU1JfTVBFUkZdLmF2YWlsYWJsZSA9IHRydWU7DQogCX0NCiANCiAJc3dpdGNo
IChib290X2NwdV9kYXRhLng4Nl92ZW5kb3IpIHsNCi0tDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1201848

FromPeter Zijlstra <peterz@infradead.org>
Date2015-08-06 17:30 +0200
Message-ID<pUvhg-4zC-11@gated-at.bofh.it>
In reply to#1200276
On Tue, Aug 04, 2015 at 08:39:27PM +0000, Liang, Kan wrote:
> Right, it could be a problem.
> How about the patch as below?

X-MS-Has-Attach:
X-MS-TNEF-Correlator:
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64

Your outlook wrecked it, please use a less broken MUA.

How about we do the below instead? Its also virt proof by virtue of
probing if we can read the MSRs.

---
 arch/x86/kernel/cpu/perf_event_msr.c | 122 +++++++++++------------------------
 1 file changed, 39 insertions(+), 83 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_msr.c b/arch/x86/kernel/cpu/perf_event_msr.c
index af216e9223e8..e9b6a0058110 100644
--- a/arch/x86/kernel/cpu/perf_event_msr.c
+++ b/arch/x86/kernel/cpu/perf_event_msr.c
@@ -10,17 +10,21 @@ enum perf_msr_id {
 	PERF_MSR_EVENT_MAX,
 };
 
+bool test_aperfmperf(void)
+{
+	return boot_cpu_has(X86_FEATURE_APERFMPERF);
+}
+
+bool test_intel(void)
+{
+	return boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
+	       boot_cpu_data.x86 == 6;
+}
+
 struct perf_msr {
-	int	id;
 	u64	msr;
-};
-
-static struct perf_msr msr[] = {
-	{ PERF_MSR_TSC, 0 },
-	{ PERF_MSR_APERF, MSR_IA32_APERF },
-	{ PERF_MSR_MPERF, MSR_IA32_MPERF },
-	{ PERF_MSR_PPERF, MSR_PPERF },
-	{ PERF_MSR_SMI, MSR_SMI_COUNT },
+	struct	perf_pmu_events_attr *attr;
+	bool	(*test)(void);
 };
 
 PMU_EVENT_ATTR_STRING(tsc,   evattr_tsc,   "event=0x00");
@@ -29,8 +33,17 @@ PMU_EVENT_ATTR_STRING(mperf, evattr_mperf, "event=0x02");
 PMU_EVENT_ATTR_STRING(pperf, evattr_pperf, "event=0x03");
 PMU_EVENT_ATTR_STRING(smi,   evattr_smi,   "event=0x04");
 
+static struct perf_msr msr[] = {
+	[PERF_MSR_TSC]   = { 0,			&evattr_tsc,	NULL,			},
+	[PERF_MSR_APERF] = { MSR_IA32_APERF,	&evattr_aperf,	test_aperfmperf,	},
+	[PERF_MSR_MPERF] = { MSR_IA32_MPERF,	&evattr_mperf,	test_aperfmperf,	},
+	[PERF_MSR_PPERF] = { MSR_PPERF,		&evattr_pperf,	test_intel,		},
+	[PERF_MSR_SMI]   = { MSR_SMI_COUNT,	&evattr_smi,	test_intel,		},
+};
+
 static struct attribute *events_attrs[PERF_MSR_EVENT_MAX + 1] = {
 	&evattr_tsc.attr.attr,
+	NULL,
 };
 
 static struct attribute_group events_attr_group = {
@@ -74,6 +87,9 @@ static int msr_event_init(struct perf_event *event)
 	    event->attr.sample_period) /* no sampling */
 		return -EINVAL;
 
+	if (!msr[cfg].attr)
+		return -EINVAL;
+
 	event->hw.idx = -1;
 	event->hw.event_base = msr[cfg].msr;
 	event->hw.config = cfg;
@@ -151,89 +167,29 @@ static struct pmu pmu_msr = {
 	.capabilities	= PERF_PMU_CAP_NO_INTERRUPT,
 };
 
-static int __init intel_msr_init(int idx)
-{
-	if (boot_cpu_data.x86 != 6)
-		return 0;
-
-	switch (boot_cpu_data.x86_model) {
-	case 30: /* 45nm Nehalem    */
-	case 26: /* 45nm Nehalem-EP */
-	case 46: /* 45nm Nehalem-EX */
-
-	case 37: /* 32nm Westmere    */
-	case 44: /* 32nm Westmere-EP */
-	case 47: /* 32nm Westmere-EX */
-
-	case 42: /* 32nm SandyBridge         */
-	case 45: /* 32nm SandyBridge-E/EN/EP */
-
-	case 58: /* 22nm IvyBridge       */
-	case 62: /* 22nm IvyBridge-EP/EX */
-
-	case 60: /* 22nm Haswell Core */
-	case 63: /* 22nm Haswell Server */
-	case 69: /* 22nm Haswell ULT */
-	case 70: /* 22nm Haswell + GT3e (Intel Iris Pro graphics) */
-
-	case 61: /* 14nm Broadwell Core-M */
-	case 86: /* 14nm Broadwell Xeon D */
-	case 71: /* 14nm Broadwell + GT3e (Intel Iris Pro graphics) */
-	case 79: /* 14nm Broadwell Server */
-		events_attrs[idx++] = &evattr_smi.attr.attr;
-		break;
-
-	case 78: /* 14nm Skylake Mobile */
-	case 94: /* 14nm Skylake Desktop */
-		events_attrs[idx++] = &evattr_pperf.attr.attr;
-		events_attrs[idx++] = &evattr_smi.attr.attr;
-		break;
-
-	case 55: /* 22nm Atom "Silvermont"                */
-	case 76: /* 14nm Atom "Airmont"                   */
-	case 77: /* 22nm Atom "Silvermont Avoton/Rangely" */
-		events_attrs[idx++] = &evattr_smi.attr.attr;
-		break;
-	}
-
-	events_attrs[idx] = NULL;
-
-	return 0;
-}
-
-static int __init amd_msr_init(int idx)
-{
-	return 0;
-}
-
 static int __init msr_init(void)
 {
-	int err;
-	int idx = 1;
+	int i, j = 1;
 
-	if (boot_cpu_has(X86_FEATURE_APERFMPERF)) {
-		events_attrs[idx++] = &evattr_aperf.attr.attr;
-		events_attrs[idx++] = &evattr_mperf.attr.attr;
-		events_attrs[idx] = NULL;
+	if (!boot_cpu_has(X86_FEATURE_TSC)) {
+		pr_cont("no MSR PMU driver.\n");
+		return 0;
 	}
 
-	switch (boot_cpu_data.x86_vendor) {
-	case X86_VENDOR_INTEL:
-		err = intel_msr_init(idx);
-		break;
-
-	case X86_VENDOR_AMD:
-		err = amd_msr_init(idx);
-		break;
+	/* Probe the MSRs. */
+	for (i = PERF_MSR_TSC + 1; i < PERF_MSR_EVENT_MAX; i++) {
+		u64 val;
 
-	default:
-		err = -ENOTSUPP;
+		if (!msr[i].test() || rdmsrl_safe(msr[i].msr, &val))
+			msr[i].attr = NULL;
 	}
 
-	if (err != 0) {
-		pr_cont("no msr PMU driver.\n");
-		return 0;
+	/* List remaining MSRs in the sysfs attrs. */
+	for (i = 0; i < PERF_MSR_EVENT_MAX; i++) {
+		if (msr[i].attr)
+			events_attrs[j++] = &msr[i].attr->attr.attr;
 	}
+	events_attrs[j] = NULL;
 
 	perf_pmu_register(&pmu_msr, "msr", -1);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1201858

FromAndy Lutomirski <luto@amacapital.net>
Date2015-08-06 17:40 +0200
Message-ID<pUvqX-4KP-47@gated-at.bofh.it>
In reply to#1201848
On Thu, Aug 6, 2015 at 8:21 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Aug 04, 2015 at 08:39:27PM +0000, Liang, Kan wrote:
> -       default:
> -               err = -ENOTSUPP;
> +               if (!msr[i].test() || rdmsrl_safe(msr[i].msr, &val))
> +                       msr[i].attr = NULL;

IIRC rdmsrl_safe literally never fails under QEMU TCG, and I'm not
entirely sure what happens under KVM if emulation kicks in.  It might
pay to keep the model check for the non-architectural stuff, or at
least check for a nonzero return value.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1201872

FromPeter Zijlstra <peterz@infradead.org>
Date2015-08-06 18:00 +0200
Message-ID<pUvKi-57B-15@gated-at.bofh.it>
In reply to#1201858
On Thu, Aug 06, 2015 at 08:30:08AM -0700, Andy Lutomirski wrote:
> On Thu, Aug 6, 2015 at 8:21 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Tue, Aug 04, 2015 at 08:39:27PM +0000, Liang, Kan wrote:
> > -       default:
> > -               err = -ENOTSUPP;
> > +               if (!msr[i].test() || rdmsrl_safe(msr[i].msr, &val))
> > +                       msr[i].attr = NULL;
> 
> IIRC rdmsrl_safe literally never fails under QEMU TCG, and I'm not

*sigh* the borkage never stops does it :-(

> entirely sure what happens under KVM if emulation kicks in.  It might
> pay to keep the model check for the non-architectural stuff, or at
> least check for a nonzero return value.

Of course, 0 might be a valid value.. Esp. for the SMI counter.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1202452

FromPeter Zijlstra <peterz@infradead.org>
Date2015-08-07 10:40 +0200
Message-ID<pULm2-2zq-19@gated-at.bofh.it>
In reply to#1201872
On Thu, Aug 06, 2015 at 05:59:43PM +0200, Peter Zijlstra wrote:
> On Thu, Aug 06, 2015 at 08:30:08AM -0700, Andy Lutomirski wrote:
> > On Thu, Aug 6, 2015 at 8:21 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > > On Tue, Aug 04, 2015 at 08:39:27PM +0000, Liang, Kan wrote:
> > > -       default:
> > > -               err = -ENOTSUPP;
> > > +               if (!msr[i].test() || rdmsrl_safe(msr[i].msr, &val))
> > > +                       msr[i].attr = NULL;
> > 
> > IIRC rdmsrl_safe literally never fails under QEMU TCG, and I'm not
> 
> *sigh* the borkage never stops does it :-(
> 
> > entirely sure what happens under KVM if emulation kicks in.  It might
> > pay to keep the model check for the non-architectural stuff, or at
> > least check for a nonzero return value.
> 
> Of course, 0 might be a valid value.. Esp. for the SMI counter.

This then..

---
 arch/x86/kernel/cpu/perf_event_msr.c |  170 +++++++++++++++++------------------
 1 file changed, 85 insertions(+), 85 deletions(-)

--- a/arch/x86/kernel/cpu/perf_event_msr.c
+++ b/arch/x86/kernel/cpu/perf_event_msr.c
@@ -10,17 +10,63 @@ enum perf_msr_id {
 	PERF_MSR_EVENT_MAX,
 };
 
+bool test_aperfmperf(int idx)
+{
+	return boot_cpu_has(X86_FEATURE_APERFMPERF);
+}
+
+bool test_intel(int idx)
+{
+	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+	    boot_cpu_data.x86 != 6)
+		return false;
+
+	switch (boot_cpu_data.x86_model) {
+	case 30: /* 45nm Nehalem    */
+	case 26: /* 45nm Nehalem-EP */
+	case 46: /* 45nm Nehalem-EX */
+
+	case 37: /* 32nm Westmere    */
+	case 44: /* 32nm Westmere-EP */
+	case 47: /* 32nm Westmere-EX */
+
+	case 42: /* 32nm SandyBridge         */
+	case 45: /* 32nm SandyBridge-E/EN/EP */
+
+	case 58: /* 22nm IvyBridge       */
+	case 62: /* 22nm IvyBridge-EP/EX */
+
+	case 60: /* 22nm Haswell Core */
+	case 63: /* 22nm Haswell Server */
+	case 69: /* 22nm Haswell ULT */
+	case 70: /* 22nm Haswell + GT3e (Intel Iris Pro graphics) */
+
+	case 61: /* 14nm Broadwell Core-M */
+	case 86: /* 14nm Broadwell Xeon D */
+	case 71: /* 14nm Broadwell + GT3e (Intel Iris Pro graphics) */
+	case 79: /* 14nm Broadwell Server */
+
+	case 55: /* 22nm Atom "Silvermont"                */
+	case 77: /* 22nm Atom "Silvermont Avoton/Rangely" */
+	case 76: /* 14nm Atom "Airmont"                   */
+		if (idx == PERF_MSR_SMI)
+			return true;
+		break;
+
+	case 78: /* 14nm Skylake Mobile */
+	case 94: /* 14nm Skylake Desktop */
+		if (idx == PERF_MSR_SMI || idx = PERF_MSR_PPERF)
+			return true;
+		break;
+	}
+
+	return false;
+}
+
 struct perf_msr {
-	int	id;
 	u64	msr;
-};
-
-static struct perf_msr msr[] = {
-	{ PERF_MSR_TSC, 0 },
-	{ PERF_MSR_APERF, MSR_IA32_APERF },
-	{ PERF_MSR_MPERF, MSR_IA32_MPERF },
-	{ PERF_MSR_PPERF, MSR_PPERF },
-	{ PERF_MSR_SMI, MSR_SMI_COUNT },
+	struct	perf_pmu_events_attr *attr;
+	bool	(*test)(int idx);
 };
 
 PMU_EVENT_ATTR_STRING(tsc,   evattr_tsc,   "event=0x00");
@@ -29,8 +75,16 @@ PMU_EVENT_ATTR_STRING(mperf, evattr_mper
 PMU_EVENT_ATTR_STRING(pperf, evattr_pperf, "event=0x03");
 PMU_EVENT_ATTR_STRING(smi,   evattr_smi,   "event=0x04");
 
+static struct perf_msr msr[] = {
+	[PERF_MSR_TSC]   = { 0,			&evattr_tsc,	NULL,		 },
+	[PERF_MSR_APERF] = { MSR_IA32_APERF,	&evattr_aperf,	test_aperfmperf, },
+	[PERF_MSR_MPERF] = { MSR_IA32_MPERF,	&evattr_mperf,	test_aperfmperf, },
+	[PERF_MSR_PPERF] = { MSR_PPERF,		&evattr_pperf,	test_intel,	 },
+	[PERF_MSR_SMI]   = { MSR_SMI_COUNT,	&evattr_smi,	test_intel,	 },
+};
+
 static struct attribute *events_attrs[PERF_MSR_EVENT_MAX + 1] = {
-	&evattr_tsc.attr.attr,
+	NULL,
 };
 
 static struct attribute_group events_attr_group = {
@@ -74,6 +128,9 @@ static int msr_event_init(struct perf_ev
 	    event->attr.sample_period) /* no sampling */
 		return -EINVAL;
 
+	if (!msr[cfg].attr)
+		return -EINVAL;
+
 	event->hw.idx = -1;
 	event->hw.event_base = msr[cfg].msr;
 	event->hw.config = cfg;
@@ -151,89 +208,32 @@ static struct pmu pmu_msr = {
 	.capabilities	= PERF_PMU_CAP_NO_INTERRUPT,
 };
 
-static int __init intel_msr_init(int idx)
-{
-	if (boot_cpu_data.x86 != 6)
-		return 0;
-
-	switch (boot_cpu_data.x86_model) {
-	case 30: /* 45nm Nehalem    */
-	case 26: /* 45nm Nehalem-EP */
-	case 46: /* 45nm Nehalem-EX */
-
-	case 37: /* 32nm Westmere    */
-	case 44: /* 32nm Westmere-EP */
-	case 47: /* 32nm Westmere-EX */
-
-	case 42: /* 32nm SandyBridge         */
-	case 45: /* 32nm SandyBridge-E/EN/EP */
-
-	case 58: /* 22nm IvyBridge       */
-	case 62: /* 22nm IvyBridge-EP/EX */
-
-	case 60: /* 22nm Haswell Core */
-	case 63: /* 22nm Haswell Server */
-	case 69: /* 22nm Haswell ULT */
-	case 70: /* 22nm Haswell + GT3e (Intel Iris Pro graphics) */
-
-	case 61: /* 14nm Broadwell Core-M */
-	case 86: /* 14nm Broadwell Xeon D */
-	case 71: /* 14nm Broadwell + GT3e (Intel Iris Pro graphics) */
-	case 79: /* 14nm Broadwell Server */
-		events_attrs[idx++] = &evattr_smi.attr.attr;
-		break;
-
-	case 78: /* 14nm Skylake Mobile */
-	case 94: /* 14nm Skylake Desktop */
-		events_attrs[idx++] = &evattr_pperf.attr.attr;
-		events_attrs[idx++] = &evattr_smi.attr.attr;
-		break;
-
-	case 55: /* 22nm Atom "Silvermont"                */
-	case 76: /* 14nm Atom "Airmont"                   */
-	case 77: /* 22nm Atom "Silvermont Avoton/Rangely" */
-		events_attrs[idx++] = &evattr_smi.attr.attr;
-		break;
-	}
-
-	events_attrs[idx] = NULL;
-
-	return 0;
-}
-
-static int __init amd_msr_init(int idx)
-{
-	return 0;
-}
-
 static int __init msr_init(void)
 {
-	int err;
-	int idx = 1;
+	int i, j = 0;
 
-	if (boot_cpu_has(X86_FEATURE_APERFMPERF)) {
-		events_attrs[idx++] = &evattr_aperf.attr.attr;
-		events_attrs[idx++] = &evattr_mperf.attr.attr;
-		events_attrs[idx] = NULL;
+	if (!boot_cpu_has(X86_FEATURE_TSC)) {
+		pr_cont("no MSR PMU driver.\n");
+		return 0;
 	}
 
-	switch (boot_cpu_data.x86_vendor) {
-	case X86_VENDOR_INTEL:
-		err = intel_msr_init(idx);
-		break;
-
-	case X86_VENDOR_AMD:
-		err = amd_msr_init(idx);
-		break;
-
-	default:
-		err = -ENOTSUPP;
+	/* Probe the MSRs. */
+	for (i = PERF_MSR_TSC + 1; i < PERF_MSR_EVENT_MAX; i++) {
+		u64 val;
+
+		/*
+		 * Virt sucks arse; you cannot tell if a R/O MSR is present :/
+		 */
+		if (!msr[i].test(i) || rdmsrl_safe(msr[i].msr, &val))
+			msr[i].attr = NULL;
 	}
 
-	if (err != 0) {
-		pr_cont("no msr PMU driver.\n");
-		return 0;
+	/* List remaining MSRs in the sysfs attrs. */
+	for (i = 0; i < PERF_MSR_EVENT_MAX; i++) {
+		if (msr[i].attr)
+			events_attrs[j++] = &msr[i].attr->attr.attr;
 	}
+	events_attrs[j] = NULL;
 
 	perf_pmu_register(&pmu_msr, "msr", -1);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1203748

FromAndy Lutomirski <luto@amacapital.net>
Date2015-08-10 04:20 +0200
Message-ID<pVKQV-881-3@gated-at.bofh.it>
In reply to#1202452
On Fri, Aug 7, 2015 at 1:34 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Aug 06, 2015 at 05:59:43PM +0200, Peter Zijlstra wrote:
>> On Thu, Aug 06, 2015 at 08:30:08AM -0700, Andy Lutomirski wrote:
>> > On Thu, Aug 6, 2015 at 8:21 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>> > > On Tue, Aug 04, 2015 at 08:39:27PM +0000, Liang, Kan wrote:
>> > > -       default:
>> > > -               err = -ENOTSUPP;
>> > > +               if (!msr[i].test() || rdmsrl_safe(msr[i].msr, &val))
>> > > +                       msr[i].attr = NULL;
>> >
>> > IIRC rdmsrl_safe literally never fails under QEMU TCG, and I'm not
>>
>> *sigh* the borkage never stops does it :-(
>>
>> > entirely sure what happens under KVM if emulation kicks in.  It might
>> > pay to keep the model check for the non-architectural stuff, or at
>> > least check for a nonzero return value.
>>
>> Of course, 0 might be a valid value.. Esp. for the SMI counter.
>
> This then..
>

LGTM on brief inspection.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1204309

From"Liang, Kan" <kan.liang@intel.com>
Date2015-08-10 16:10 +0200
Message-ID<pVVW2-7v-27@gated-at.bofh.it>
In reply to#1202452

> On Thu, Aug 06, 2015 at 05:59:43PM +0200, Peter Zijlstra wrote:
> > On Thu, Aug 06, 2015 at 08:30:08AM -0700, Andy Lutomirski wrote:
> > > On Thu, Aug 6, 2015 at 8:21 AM, Peter Zijlstra <peterz@infradead.org>
> wrote:
> > > > On Tue, Aug 04, 2015 at 08:39:27PM +0000, Liang, Kan wrote:
> > > > -       default:
> > > > -               err = -ENOTSUPP;
> > > > +               if (!msr[i].test() || rdmsrl_safe(msr[i].msr, &val))
> > > > +                       msr[i].attr = NULL;
> > >
> > > IIRC rdmsrl_safe literally never fails under QEMU TCG, and I'm not
> >
> > *sigh* the borkage never stops does it :-(
> >
> > > entirely sure what happens under KVM if emulation kicks in.  It
> > > might pay to keep the model check for the non-architectural stuff,
> > > or at least check for a nonzero return value.
> >
> > Of course, 0 might be a valid value.. Esp. for the SMI counter.
> 
> This then..
> 
> ---
>  arch/x86/kernel/cpu/perf_event_msr.c |  170 +++++++++++++++++-------
> -----------
>  1 file changed, 85 insertions(+), 85 deletions(-)
> 
> --- a/arch/x86/kernel/cpu/perf_event_msr.c
> +++ b/arch/x86/kernel/cpu/perf_event_msr.c
> @@ -10,17 +10,63 @@ enum perf_msr_id {
>  	PERF_MSR_EVENT_MAX,
>  };
> 
> +bool test_aperfmperf(int idx)
> +{
> +	return boot_cpu_has(X86_FEATURE_APERFMPERF);
> +}
> +
> +bool test_intel(int idx)
> +{
> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
> +	    boot_cpu_data.x86 != 6)
> +		return false;
> +
> +	switch (boot_cpu_data.x86_model) {
> +	case 30: /* 45nm Nehalem    */
> +	case 26: /* 45nm Nehalem-EP */
> +	case 46: /* 45nm Nehalem-EX */
> +
> +	case 37: /* 32nm Westmere    */
> +	case 44: /* 32nm Westmere-EP */
> +	case 47: /* 32nm Westmere-EX */
> +
> +	case 42: /* 32nm SandyBridge         */
> +	case 45: /* 32nm SandyBridge-E/EN/EP */
> +
> +	case 58: /* 22nm IvyBridge       */
> +	case 62: /* 22nm IvyBridge-EP/EX */
> +
> +	case 60: /* 22nm Haswell Core */
> +	case 63: /* 22nm Haswell Server */
> +	case 69: /* 22nm Haswell ULT */
> +	case 70: /* 22nm Haswell + GT3e (Intel Iris Pro graphics) */
> +
> +	case 61: /* 14nm Broadwell Core-M */
> +	case 86: /* 14nm Broadwell Xeon D */
> +	case 71: /* 14nm Broadwell + GT3e (Intel Iris Pro graphics) */
> +	case 79: /* 14nm Broadwell Server */
> +
> +	case 55: /* 22nm Atom "Silvermont"                */
> +	case 77: /* 22nm Atom "Silvermont Avoton/Rangely" */
> +	case 76: /* 14nm Atom "Airmont"                   */
> +		if (idx == PERF_MSR_SMI)
> +			return true;
> +		break;
> +
> +	case 78: /* 14nm Skylake Mobile */
> +	case 94: /* 14nm Skylake Desktop */
> +		if (idx == PERF_MSR_SMI || idx = PERF_MSR_PPERF)

Here is a typo.  The rest is OK for me.
I did some simple test on HSX.

Tested-by: Kan Liang <kan.liang@intel.com>

> +			return true;
> +		break;
> +	}
> +
> +	return false;
> +}
> +
>  struct perf_msr {
> -	int	id;
>  	u64	msr;
> -};
> -
> -static struct perf_msr msr[] = {
> -	{ PERF_MSR_TSC, 0 },
> -	{ PERF_MSR_APERF, MSR_IA32_APERF },
> -	{ PERF_MSR_MPERF, MSR_IA32_MPERF },
> -	{ PERF_MSR_PPERF, MSR_PPERF },
> -	{ PERF_MSR_SMI, MSR_SMI_COUNT },
> +	struct	perf_pmu_events_attr *attr;
> +	bool	(*test)(int idx);
>  };
> 
>  PMU_EVENT_ATTR_STRING(tsc,   evattr_tsc,   "event=0x00");
> @@ -29,8 +75,16 @@ PMU_EVENT_ATTR_STRING(mperf, evattr_mper
> PMU_EVENT_ATTR_STRING(pperf, evattr_pperf, "event=0x03");
>  PMU_EVENT_ATTR_STRING(smi,   evattr_smi,   "event=0x04");
> 
> +static struct perf_msr msr[] = {
> +	[PERF_MSR_TSC]   = { 0,			&evattr_tsc,
> 	NULL,		 },
> +	[PERF_MSR_APERF] = { MSR_IA32_APERF,	&evattr_aperf,
> 	test_aperfmperf, },
> +	[PERF_MSR_MPERF] = { MSR_IA32_MPERF,	&evattr_mperf,
> 	test_aperfmperf, },
> +	[PERF_MSR_PPERF] = { MSR_PPERF,		&evattr_pperf,
> 	test_intel,	 },
> +	[PERF_MSR_SMI]   = { MSR_SMI_COUNT,	&evattr_smi,
> 	test_intel,	 },
> +};
> +
>  static struct attribute *events_attrs[PERF_MSR_EVENT_MAX + 1] = {
> -	&evattr_tsc.attr.attr,
> +	NULL,
>  };
> 
>  static struct attribute_group events_attr_group = { @@ -74,6 +128,9 @@
> static int msr_event_init(struct perf_ev
>  	    event->attr.sample_period) /* no sampling */
>  		return -EINVAL;
> 
> +	if (!msr[cfg].attr)
> +		return -EINVAL;
> +
>  	event->hw.idx = -1;
>  	event->hw.event_base = msr[cfg].msr;
>  	event->hw.config = cfg;
> @@ -151,89 +208,32 @@ static struct pmu pmu_msr = {
>  	.capabilities	= PERF_PMU_CAP_NO_INTERRUPT,
>  };
> 
> -static int __init intel_msr_init(int idx) -{
> -	if (boot_cpu_data.x86 != 6)
> -		return 0;
> -
> -	switch (boot_cpu_data.x86_model) {
> -	case 30: /* 45nm Nehalem    */
> -	case 26: /* 45nm Nehalem-EP */
> -	case 46: /* 45nm Nehalem-EX */
> -
> -	case 37: /* 32nm Westmere    */
> -	case 44: /* 32nm Westmere-EP */
> -	case 47: /* 32nm Westmere-EX */
> -
> -	case 42: /* 32nm SandyBridge         */
> -	case 45: /* 32nm SandyBridge-E/EN/EP */
> -
> -	case 58: /* 22nm IvyBridge       */
> -	case 62: /* 22nm IvyBridge-EP/EX */
> -
> -	case 60: /* 22nm Haswell Core */
> -	case 63: /* 22nm Haswell Server */
> -	case 69: /* 22nm Haswell ULT */
> -	case 70: /* 22nm Haswell + GT3e (Intel Iris Pro graphics) */
> -
> -	case 61: /* 14nm Broadwell Core-M */
> -	case 86: /* 14nm Broadwell Xeon D */
> -	case 71: /* 14nm Broadwell + GT3e (Intel Iris Pro graphics) */
> -	case 79: /* 14nm Broadwell Server */
> -		events_attrs[idx++] = &evattr_smi.attr.attr;
> -		break;
> -
> -	case 78: /* 14nm Skylake Mobile */
> -	case 94: /* 14nm Skylake Desktop */
> -		events_attrs[idx++] = &evattr_pperf.attr.attr;
> -		events_attrs[idx++] = &evattr_smi.attr.attr;
> -		break;
> -
> -	case 55: /* 22nm Atom "Silvermont"                */
> -	case 76: /* 14nm Atom "Airmont"                   */
> -	case 77: /* 22nm Atom "Silvermont Avoton/Rangely" */
> -		events_attrs[idx++] = &evattr_smi.attr.attr;
> -		break;
> -	}
> -
> -	events_attrs[idx] = NULL;
> -
> -	return 0;
> -}
> -
> -static int __init amd_msr_init(int idx) -{
> -	return 0;
> -}
> -
>  static int __init msr_init(void)
>  {
> -	int err;
> -	int idx = 1;
> +	int i, j = 0;
> 
> -	if (boot_cpu_has(X86_FEATURE_APERFMPERF)) {
> -		events_attrs[idx++] = &evattr_aperf.attr.attr;
> -		events_attrs[idx++] = &evattr_mperf.attr.attr;
> -		events_attrs[idx] = NULL;
> +	if (!boot_cpu_has(X86_FEATURE_TSC)) {
> +		pr_cont("no MSR PMU driver.\n");
> +		return 0;
>  	}
> 
> -	switch (boot_cpu_data.x86_vendor) {
> -	case X86_VENDOR_INTEL:
> -		err = intel_msr_init(idx);
> -		break;
> -
> -	case X86_VENDOR_AMD:
> -		err = amd_msr_init(idx);
> -		break;
> -
> -	default:
> -		err = -ENOTSUPP;
> +	/* Probe the MSRs. */
> +	for (i = PERF_MSR_TSC + 1; i < PERF_MSR_EVENT_MAX; i++) {
> +		u64 val;
> +
> +		/*
> +		 * Virt sucks arse; you cannot tell if a R/O MSR is present :/
> +		 */
> +		if (!msr[i].test(i) || rdmsrl_safe(msr[i].msr, &val))
> +			msr[i].attr = NULL;
>  	}
> 
> -	if (err != 0) {
> -		pr_cont("no msr PMU driver.\n");
> -		return 0;
> +	/* List remaining MSRs in the sysfs attrs. */
> +	for (i = 0; i < PERF_MSR_EVENT_MAX; i++) {
> +		if (msr[i].attr)
> +			events_attrs[j++] = &msr[i].attr->attr.attr;
>  	}
> +	events_attrs[j] = NULL;
> 
>  	perf_pmu_register(&pmu_msr, "msr", -1);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1204326

FromPeter Zijlstra <peterz@infradead.org>
Date2015-08-10 16:30 +0200
Message-ID<pVWfo-uE-23@gated-at.bofh.it>
In reply to#1204309
On Mon, Aug 10, 2015 at 02:02:17PM +0000, Liang, Kan wrote:
> > +	case 94: /* 14nm Skylake Desktop */
> > +		if (idx == PERF_MSR_SMI || idx = PERF_MSR_PPERF)
> 
> Here is a typo.  The rest is OK for me.
> I did some simple test on HSX.

Yep, fixed that already.

> Tested-by: Kan Liang <kan.liang@intel.com>

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1201938

From"Liang, Kan" <kan.liang@intel.com>
Date2015-08-06 19:50 +0200
Message-ID<pUxsJ-7Gk-7@gated-at.bofh.it>
In reply to#1201848

> +	/* Probe the MSRs. */
> +	for (i = PERF_MSR_TSC + 1; i < PERF_MSR_EVENT_MAX; i++) {
> +		u64 val;
> 
> -	default:
> -		err = -ENOTSUPP;
> +		if (!msr[i].test() || rdmsrl_safe(msr[i].msr, &val))
> +			msr[i].attr = NULL;
>  	}
> 
> -	if (err != 0) {
> -		pr_cont("no msr PMU driver.\n");
> -		return 0;
> +	/* List remaining MSRs in the sysfs attrs. */
> +	for (i = 0; i < PERF_MSR_EVENT_MAX; i++) {

i should start from PERF_MSR_TSC + 1. The tsc has already been
inserted into events_attrs by default.

> +		if (msr[i].attr)
> +			events_attrs[j++] = &msr[i].attr->attr.attr;
>  	}
> +	events_attrs[j] = NULL;
> 
>  	perf_pmu_register(&pmu_msr, "msr", -1);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web