2014-11-06 16:29:31

by Aravind Gopalakrishnan

[permalink] [raw]
Subject: [PATCH] perf, amd, ibs: Update IBS MSRs and feature definitions

New Fam15h models carry extra feature bits and extend
the MSR register space for IBS ops. Adding them here.

While at it, add functionality to read IbsBrTarget and
OpData4 depending on their availability if user wants a
PERF_SAMPLE_RAW.

Cc: Paolo Bonzini <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Jan Kiszka <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Fenghua Yu <[email protected]>
Signed-off-by: Aravind Gopalakrishnan <[email protected]>
---
arch/x86/include/asm/perf_event.h | 3 +++
arch/x86/include/uapi/asm/msr-index.h | 2 ++
arch/x86/kernel/cpu/perf_event_amd_ibs.c | 15 +++++++++++++++
3 files changed, 20 insertions(+)

diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index 8dfc9fd..dc0f6ed 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -177,6 +177,9 @@ struct x86_pmu_capability {
#define IBS_CAPS_BRNTRGT (1U<<5)
#define IBS_CAPS_OPCNTEXT (1U<<6)
#define IBS_CAPS_RIPINVALIDCHK (1U<<7)
+#define IBS_CAPS_OPBRNFUSE (1U<<8)
+#define IBS_CAPS_FETCHCTLEXTD (1U<<9)
+#define IBS_CAPS_OPDATA4 (1U<<10)

#define IBS_CAPS_DEFAULT (IBS_CAPS_AVAIL \
| IBS_CAPS_FETCHSAM \
diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h
index e21331c..ba7b609 100644
--- a/arch/x86/include/uapi/asm/msr-index.h
+++ b/arch/x86/include/uapi/asm/msr-index.h
@@ -206,6 +206,8 @@
#define MSR_AMD64_IBSOP_REG_MASK ((1UL<<MSR_AMD64_IBSOP_REG_COUNT)-1)
#define MSR_AMD64_IBSCTL 0xc001103a
#define MSR_AMD64_IBSBRTARGET 0xc001103b
+#define MSR_AMD64_IBS_FETCH_EXTD_CTL 0xc001103c
+#define MSR_AMD64_IBSOPDATA4 0xc001103d
#define MSR_AMD64_IBS_REG_COUNT_MAX 8 /* includes MSR_AMD64_IBSBRTARGET */

/* Fam 16h MSRs */
diff --git a/arch/x86/kernel/cpu/perf_event_amd_ibs.c b/arch/x86/kernel/cpu/perf_event_amd_ibs.c
index cbb1be3e..a61f5c6 100644
--- a/arch/x86/kernel/cpu/perf_event_amd_ibs.c
+++ b/arch/x86/kernel/cpu/perf_event_amd_ibs.c
@@ -565,6 +565,21 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
perf_ibs->offset_max,
offset + 1);
} while (offset < offset_max);
+ if (event->attr.sample_type & PERF_SAMPLE_RAW) {
+ /*
+ * Read IbsBrTarget and IbsOpData4 separately
+ * depending on their availability.
+ * Can't add to offset_max as they are staggered
+ */
+ if (ibs_caps & IBS_CAPS_BRNTRGT) {
+ rdmsrl(MSR_AMD64_IBSBRTARGET, *buf++);
+ size++;
+ }
+ if (ibs_caps & IBS_CAPS_OPDATA4) {
+ rdmsrl(MSR_AMD64_IBSOPDATA4, *buf++);
+ size++;
+ }
+ }
ibs_data.size = sizeof(u64) * size;

regs = *iregs;
--
1.9.1


2014-11-06 16:34:37

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] perf, amd, ibs: Update IBS MSRs and feature definitions

On Thu, Nov 06, 2014 at 10:26:22AM -0600, Aravind Gopalakrishnan wrote:
> New Fam15h models carry extra feature bits and extend
> the MSR register space for IBS ops. Adding them here.
>
> While at it, add functionality to read IbsBrTarget and
> OpData4 depending on their availability if user wants a
> PERF_SAMPLE_RAW.
>
> Cc: Paolo Bonzini <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Jan Kiszka <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: Fenghua Yu <[email protected]>
> Signed-off-by: Aravind Gopalakrishnan <[email protected]>
> ---
> arch/x86/include/asm/perf_event.h | 3 +++
> arch/x86/include/uapi/asm/msr-index.h | 2 ++
> arch/x86/kernel/cpu/perf_event_amd_ibs.c | 15 +++++++++++++++
> 3 files changed, 20 insertions(+)
>
> diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
> index 8dfc9fd..dc0f6ed 100644
> --- a/arch/x86/include/asm/perf_event.h
> +++ b/arch/x86/include/asm/perf_event.h
> @@ -177,6 +177,9 @@ struct x86_pmu_capability {
> #define IBS_CAPS_BRNTRGT (1U<<5)
> #define IBS_CAPS_OPCNTEXT (1U<<6)
> #define IBS_CAPS_RIPINVALIDCHK (1U<<7)
> +#define IBS_CAPS_OPBRNFUSE (1U<<8)
> +#define IBS_CAPS_FETCHCTLEXTD (1U<<9)
> +#define IBS_CAPS_OPDATA4 (1U<<10)
>
> #define IBS_CAPS_DEFAULT (IBS_CAPS_AVAIL \
> | IBS_CAPS_FETCHSAM \
> diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h
> index e21331c..ba7b609 100644
> --- a/arch/x86/include/uapi/asm/msr-index.h
> +++ b/arch/x86/include/uapi/asm/msr-index.h
> @@ -206,6 +206,8 @@
> #define MSR_AMD64_IBSOP_REG_MASK ((1UL<<MSR_AMD64_IBSOP_REG_COUNT)-1)
> #define MSR_AMD64_IBSCTL 0xc001103a
> #define MSR_AMD64_IBSBRTARGET 0xc001103b
> +#define MSR_AMD64_IBS_FETCH_EXTD_CTL 0xc001103c
> +#define MSR_AMD64_IBSOPDATA4 0xc001103d
> #define MSR_AMD64_IBS_REG_COUNT_MAX 8 /* includes MSR_AMD64_IBSBRTARGET */
>
> /* Fam 16h MSRs */
> diff --git a/arch/x86/kernel/cpu/perf_event_amd_ibs.c b/arch/x86/kernel/cpu/perf_event_amd_ibs.c
> index cbb1be3e..a61f5c6 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd_ibs.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd_ibs.c
> @@ -565,6 +565,21 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
> perf_ibs->offset_max,
> offset + 1);
> } while (offset < offset_max);
> + if (event->attr.sample_type & PERF_SAMPLE_RAW) {
> + /*
> + * Read IbsBrTarget and IbsOpData4 separately
> + * depending on their availability.
> + * Can't add to offset_max as they are staggered
> + */
> + if (ibs_caps & IBS_CAPS_BRNTRGT) {
> + rdmsrl(MSR_AMD64_IBSBRTARGET, *buf++);

Are those MSRs present on everything that supports IBS and if not, you
probably should do rdmsr_safe() here instead and handle the error case.
Or do something with f/m/s checking or so...

> + size++;
> + }
> + if (ibs_caps & IBS_CAPS_OPDATA4) {
> + rdmsrl(MSR_AMD64_IBSOPDATA4, *buf++);
> + size++;
> + }
> + }
> ibs_data.size = sizeof(u64) * size;
>
> regs = *iregs;
> --
> 1.9.1
>

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

2014-11-06 16:57:51

by Aravind Gopalakrishnan

[permalink] [raw]
Subject: Re: [PATCH] perf, amd, ibs: Update IBS MSRs and feature definitions

On 11/6/2014 10:34 AM, Borislav Petkov wrote:
> On Thu, Nov 06, 2014 at 10:26:22AM -0600, Aravind Gopalakrishnan wrote:
>> diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h
>> index e21331c..ba7b609 100644
>> --- a/arch/x86/include/uapi/asm/msr-index.h
>> +++ b/arch/x86/include/uapi/asm/msr-index.h
>> @@ -206,6 +206,8 @@
>> #define MSR_AMD64_IBSOP_REG_MASK ((1UL<<MSR_AMD64_IBSOP_REG_COUNT)-1)
>> #define MSR_AMD64_IBSCTL 0xc001103a
>> #define MSR_AMD64_IBSBRTARGET 0xc001103b
>> +#define MSR_AMD64_IBS_FETCH_EXTD_CTL 0xc001103c
>> +#define MSR_AMD64_IBSOPDATA4 0xc001103d
>> #define MSR_AMD64_IBS_REG_COUNT_MAX 8 /* includes MSR_AMD64_IBSBRTARGET */
>>
>> /* Fam 16h MSRs */
>> diff --git a/arch/x86/kernel/cpu/perf_event_amd_ibs.c b/arch/x86/kernel/cpu/perf_event_amd_ibs.c
>> index cbb1be3e..a61f5c6 100644
>> --- a/arch/x86/kernel/cpu/perf_event_amd_ibs.c
>> +++ b/arch/x86/kernel/cpu/perf_event_amd_ibs.c
>> @@ -565,6 +565,21 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
>> perf_ibs->offset_max,
>> offset + 1);
>> } while (offset < offset_max);
>> + if (event->attr.sample_type & PERF_SAMPLE_RAW) {
>> + /*
>> + * Read IbsBrTarget and IbsOpData4 separately
>> + * depending on their availability.
>> + * Can't add to offset_max as they are staggered
>> + */
>> + if (ibs_caps & IBS_CAPS_BRNTRGT) {
>> + rdmsrl(MSR_AMD64_IBSBRTARGET, *buf++);
> Are those MSRs present on everything that supports IBS and if not, you
> probably should do rdmsr_safe() here instead and handle the error case.
> Or do something with f/m/s checking or so...

But Why?
IBS_CAPS_BRNTRGT and IBS_CAPS_OPDATA4 indicate support for the
respective MSRs and
I am only loading the MSR contents upon checking for their availability.
So it's not like an exception is
generated for a rdmsr command on an unimplemented/reserved MSR.

And the nice thing about the feature identifiers is that I don't have to
do f/m/s checks right?
I mean, if some other future processor decides to implement it, then we
don't have to revisit the code
to make a change to the f/m/s condition.
And if they don't want to use those MSRs then it's still OK as the
feature bits are not going to be set..


Thanks,
-Aravind.

>> + size++;
>> + }
>> + if (ibs_caps & IBS_CAPS_OPDATA4) {
>> + rdmsrl(MSR_AMD64_IBSOPDATA4, *buf++);
>> + size++;
>> + }
>> + }
>> ibs_data.size = sizeof(u64) * size;
>>
>> regs = *iregs;
>> --
>> 1.9.1
>>

2014-11-08 09:04:07

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] perf, amd, ibs: Update IBS MSRs and feature definitions

On Thu, Nov 06, 2014 at 10:57:40AM -0600, Aravind Gopalakrishnan wrote:
> On 11/6/2014 10:34 AM, Borislav Petkov wrote:
> >On Thu, Nov 06, 2014 at 10:26:22AM -0600, Aravind Gopalakrishnan wrote:
> >>diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h
> >>index e21331c..ba7b609 100644
> >>--- a/arch/x86/include/uapi/asm/msr-index.h
> >>+++ b/arch/x86/include/uapi/asm/msr-index.h
> >>@@ -206,6 +206,8 @@
> >> #define MSR_AMD64_IBSOP_REG_MASK ((1UL<<MSR_AMD64_IBSOP_REG_COUNT)-1)
> >> #define MSR_AMD64_IBSCTL 0xc001103a
> >> #define MSR_AMD64_IBSBRTARGET 0xc001103b
> >>+#define MSR_AMD64_IBS_FETCH_EXTD_CTL 0xc001103c
> >>+#define MSR_AMD64_IBSOPDATA4 0xc001103d
> >> #define MSR_AMD64_IBS_REG_COUNT_MAX 8 /* includes MSR_AMD64_IBSBRTARGET */
> >> /* Fam 16h MSRs */

> But Why?
> IBS_CAPS_BRNTRGT and IBS_CAPS_OPDATA4 indicate support for the respective
> MSRs

Ok, I was just making sure.

Btw, MSR_AMD64_IBS_FETCH_EXTD_CTL seems unused. Forgotten?

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

2014-11-10 16:01:17

by Aravind Gopalakrishnan

[permalink] [raw]
Subject: Re: [PATCH] perf, amd, ibs: Update IBS MSRs and feature definitions

On 11/8/2014 3:03 AM, Borislav Petkov wrote:
> On Thu, Nov 06, 2014 at 10:57:40AM -0600, Aravind Gopalakrishnan wrote:
>> On 11/6/2014 10:34 AM, Borislav Petkov wrote:
>>> On Thu, Nov 06, 2014 at 10:26:22AM -0600, Aravind Gopalakrishnan wrote:
>>>> diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h
>>>> index e21331c..ba7b609 100644
>>>> --- a/arch/x86/include/uapi/asm/msr-index.h
>>>> +++ b/arch/x86/include/uapi/asm/msr-index.h
>>>> @@ -206,6 +206,8 @@
>>>> #define MSR_AMD64_IBSOP_REG_MASK ((1UL<<MSR_AMD64_IBSOP_REG_COUNT)-1)
>>>> #define MSR_AMD64_IBSCTL 0xc001103a
>>>> #define MSR_AMD64_IBSBRTARGET 0xc001103b
>>>> +#define MSR_AMD64_IBS_FETCH_EXTD_CTL 0xc001103c
>>>> +#define MSR_AMD64_IBSOPDATA4 0xc001103d
>>>> #define MSR_AMD64_IBS_REG_COUNT_MAX 8 /* includes MSR_AMD64_IBSBRTARGET */
>>>> /* Fam 16h MSRs */
>> But Why?
>> IBS_CAPS_BRNTRGT and IBS_CAPS_OPDATA4 indicate support for the respective
>> MSRs
> Ok, I was just making sure.
>
> Btw, MSR_AMD64_IBS_FETCH_EXTD_CTL seems unused. Forgotten?
>

No, There aren't any bits in MSR_AMD64_IBS_FETCH_EXTD_CTL that we need
to configure right now.
So, have included it here mainly to keep the MSR listing consistent with
the manual.

Thanks,
-Aravind.

2014-11-10 16:03:51

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] perf, amd, ibs: Update IBS MSRs and feature definitions

On Mon, Nov 10, 2014 at 10:00:59AM -0600, Aravind Gopalakrishnan wrote:
> No, There aren't any bits in MSR_AMD64_IBS_FETCH_EXTD_CTL that we need to
> configure right now.
> So, have included it here mainly to keep the MSR listing consistent with the
> manual.

Please remove it then. We put MSRs in msr-index.h which are in use only.

Thanks.

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

2014-11-10 16:18:06

by Aravind Gopalakrishnan

[permalink] [raw]
Subject: Re: [PATCH] perf, amd, ibs: Update IBS MSRs and feature definitions

On 11/10/2014 10:03 AM, Borislav Petkov wrote:
> On Mon, Nov 10, 2014 at 10:00:59AM -0600, Aravind Gopalakrishnan wrote:
>> No, There aren't any bits in MSR_AMD64_IBS_FETCH_EXTD_CTL that we need to
>> configure right now.
>> So, have included it here mainly to keep the MSR listing consistent with the
>> manual.
> Please remove it then. We put MSRs in msr-index.h which are in use only.
>
>

Ok, Will do.

Thanks,
-Aravind.