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

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

[Aravind Gopalakrishnan]

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 <pbonzini@redhat.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
---
 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

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

[Borislav Petkov]

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 <pbonzini@redhat.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Signed-off-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
> ---
>  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.
--

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

[Aravind Gopalakrishnan]

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

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

[Borislav Petkov]

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

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

[Aravind Gopalakrishnan]

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.

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

[Borislav Petkov]

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

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

[Aravind Gopalakrishnan]

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.