Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On 05/22/2013 05:53 PM, Stephane Eranian wrote:
> Hi,
>
>
>
> On Wed, May 22, 2013 at 8:43 AM, Anshuman Khandual
> wrote:
>> On 05/21/2013 07:25 PM, Stephane Eranian wrote:
>>> On Thu, May 16, 2013 at 12:15 PM, Michael Neuling wrote:
Peter Zijlstra wrote:
> On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
>> On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra
>> wrote:
>>> We should always have proper privileges when requesting kernel data.
>>>
>>> Cc: Andi Kleen
>>> Cc: eran...@google.com
>>> Signed-off-by: Peter Zijlstra
>>> Link:
>>> http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
>>> ---
>>> arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
>>> +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
>>> @@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
>>> if (br_type & PERF_SAMPLE_BRANCH_USER)
>>> mask |= X86_BR_USER;
>>>
>>> - if (br_type & PERF_SAMPLE_BRANCH_KERNEL)
>>> + if (br_type & PERF_SAMPLE_BRANCH_KERNEL) {
>>> + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
>>> + return -EACCES;
>>> mask |= X86_BR_KERNEL;
>>> + }
>>>
>> This will prevent regular users from capturing kernel -> kernel branches.
>> But it won't prevent users from getting kernel -> user branches. Thus
>> some kernel address will still be captured. I guess they could be
>> eliminated
>> by the sw_filter.
>>
>> When using LBR priv level filtering, the filter applies to the branch
>> target
>> only.
>
> How about something like the below? It also adds the branch flags
> Mikey wanted for PowerPC.
Peter,
BTW PowerPC also has the ability to filter on conditional branches. Any
chance we could add something like the follow to perf also?
Mikey
diff --git a/include/uapi/linux/perf_event.h
b/include/uapi/linux/perf_event.h
index fb104e5..891c769 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -157,8 +157,9 @@ enum perf_branch_sample_type {
PERF_SAMPLE_BRANCH_ANY_CALL = 1U << 4, /* any call branch */
PERF_SAMPLE_BRANCH_ANY_RETURN = 1U << 5, /* any return branch */
PERF_SAMPLE_BRANCH_IND_CALL = 1U << 6, /* indirect calls */
+ PERF_SAMPLE_BRANCH_CONDITIONAL = 1U << 7, /* conditional branches
*/
>>> I would use PERF_SAMPLE_BRANCH_COND here.
>>>
- PERF_SAMPLE_BRANCH_MAX = 1U << 7, /* non-ABI */
+ PERF_SAMPLE_BRANCH_MAX = 1U << 8, /* non-ABI */
};
#define PERF_SAMPLE_BRANCH_PLM_ALL \
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cdf58ec..5b0b89d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL),
BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN),
BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL),
+ BRANCH_OPT("cnd", PERF_SAMPLE_BRANCH_CONDITIONAL),
>>>
>>> use "cond"
>>>
BRANCH_END
};
>>>
>>> And if you do this, you also need to update the x86
>>> perf_event_intel_lbr.c mapping
>>> tables to fill out the entries for PERF_SAMPLE_BRANCH_COND:
>>>
>>> [PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
>>>
>>> And you also need to update intel_pmu_setup_sw_lbr_filter()
>>> to handle the conversion to x86 instructions:
>>>
>>>if (br_type & PERF_SAMPLE_BRANCH_COND)
>>> mask |= X86_BR_JCC;
>>>
>>>
>>> You also need to update the perf-record.txt documentation to list cond
>>> as a possible
>>> branch filter.
>>
>> Hey Stephane,
>>
>> I have incorporated all the review comments into the patch series
>> https://lkml.org/lkml/2013/5/22/51.
>>
> I don't see how you can compile Patch 3/5:
>
> + BRANCH_OPT("cond", PERF_SAMPLE_BRANCH_CONDITIONAL),
>
> Needs to be PERF_SAMPLE_BRANCH_COND.
>
Ahh, sorry missed it, will fix it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Hi,
On Wed, May 22, 2013 at 8:43 AM, Anshuman Khandual
wrote:
> On 05/21/2013 07:25 PM, Stephane Eranian wrote:
>> On Thu, May 16, 2013 at 12:15 PM, Michael Neuling wrote:
>>> Peter Zijlstra wrote:
>>>
On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
> On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra
> wrote:
>> We should always have proper privileges when requesting kernel data.
>>
>> Cc: Andi Kleen
>> Cc: eran...@google.com
>> Signed-off-by: Peter Zijlstra
>> Link:
>> http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
>> ---
>> arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
>> +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
>> @@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
>> if (br_type & PERF_SAMPLE_BRANCH_USER)
>> mask |= X86_BR_USER;
>>
>> - if (br_type & PERF_SAMPLE_BRANCH_KERNEL)
>> + if (br_type & PERF_SAMPLE_BRANCH_KERNEL) {
>> + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
>> + return -EACCES;
>> mask |= X86_BR_KERNEL;
>> + }
>>
> This will prevent regular users from capturing kernel -> kernel branches.
> But it won't prevent users from getting kernel -> user branches. Thus
> some kernel address will still be captured. I guess they could be
> eliminated
> by the sw_filter.
>
> When using LBR priv level filtering, the filter applies to the branch
> target
> only.
How about something like the below? It also adds the branch flags
Mikey wanted for PowerPC.
>>>
>>> Peter,
>>>
>>> BTW PowerPC also has the ability to filter on conditional branches. Any
>>> chance we could add something like the follow to perf also?
>>>
>>> Mikey
>>>
>>> diff --git a/include/uapi/linux/perf_event.h
>>> b/include/uapi/linux/perf_event.h
>>> index fb104e5..891c769 100644
>>> --- a/include/uapi/linux/perf_event.h
>>> +++ b/include/uapi/linux/perf_event.h
>>> @@ -157,8 +157,9 @@ enum perf_branch_sample_type {
>>> PERF_SAMPLE_BRANCH_ANY_CALL = 1U << 4, /* any call branch */
>>> PERF_SAMPLE_BRANCH_ANY_RETURN = 1U << 5, /* any return branch */
>>> PERF_SAMPLE_BRANCH_IND_CALL = 1U << 6, /* indirect calls */
>>> + PERF_SAMPLE_BRANCH_CONDITIONAL = 1U << 7, /* conditional branches
>>> */
>>>
>> I would use PERF_SAMPLE_BRANCH_COND here.
>>
>>> - PERF_SAMPLE_BRANCH_MAX = 1U << 7, /* non-ABI */
>>> + PERF_SAMPLE_BRANCH_MAX = 1U << 8, /* non-ABI */
>>> };
>>>
>>> #define PERF_SAMPLE_BRANCH_PLM_ALL \
>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>> index cdf58ec..5b0b89d 100644
>>> --- a/tools/perf/builtin-record.c
>>> +++ b/tools/perf/builtin-record.c
>>> @@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
>>> BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL),
>>> BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN),
>>> BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL),
>>> + BRANCH_OPT("cnd", PERF_SAMPLE_BRANCH_CONDITIONAL),
>>
>> use "cond"
>>
>>> BRANCH_END
>>> };
>>>
>>
>> And if you do this, you also need to update the x86
>> perf_event_intel_lbr.c mapping
>> tables to fill out the entries for PERF_SAMPLE_BRANCH_COND:
>>
>> [PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
>>
>> And you also need to update intel_pmu_setup_sw_lbr_filter()
>> to handle the conversion to x86 instructions:
>>
>>if (br_type & PERF_SAMPLE_BRANCH_COND)
>> mask |= X86_BR_JCC;
>>
>>
>> You also need to update the perf-record.txt documentation to list cond
>> as a possible
>> branch filter.
>
> Hey Stephane,
>
> I have incorporated all the review comments into the patch series
> https://lkml.org/lkml/2013/5/22/51.
>
I don't see how you can compile Patch 3/5:
+ BRANCH_OPT("cond", PERF_SAMPLE_BRANCH_CONDITIONAL),
Needs to be PERF_SAMPLE_BRANCH_COND.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On 05/21/2013 07:25 PM, Stephane Eranian wrote:
> On Thu, May 16, 2013 at 12:15 PM, Michael Neuling wrote:
>> Peter Zijlstra wrote:
>>
>>> On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra
wrote:
> We should always have proper privileges when requesting kernel data.
>
> Cc: Andi Kleen
> Cc: eran...@google.com
> Signed-off-by: Peter Zijlstra
> Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
> ---
> arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> @@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
> if (br_type & PERF_SAMPLE_BRANCH_USER)
> mask |= X86_BR_USER;
>
> - if (br_type & PERF_SAMPLE_BRANCH_KERNEL)
> + if (br_type & PERF_SAMPLE_BRANCH_KERNEL) {
> + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
> + return -EACCES;
> mask |= X86_BR_KERNEL;
> + }
>
This will prevent regular users from capturing kernel -> kernel branches.
But it won't prevent users from getting kernel -> user branches. Thus
some kernel address will still be captured. I guess they could be
eliminated
by the sw_filter.
When using LBR priv level filtering, the filter applies to the branch
target
only.
>>>
>>> How about something like the below? It also adds the branch flags
>>> Mikey wanted for PowerPC.
>>
>> Peter,
>>
>> BTW PowerPC also has the ability to filter on conditional branches. Any
>> chance we could add something like the follow to perf also?
>>
>> Mikey
>>
>> diff --git a/include/uapi/linux/perf_event.h
>> b/include/uapi/linux/perf_event.h
>> index fb104e5..891c769 100644
>> --- a/include/uapi/linux/perf_event.h
>> +++ b/include/uapi/linux/perf_event.h
>> @@ -157,8 +157,9 @@ enum perf_branch_sample_type {
>> PERF_SAMPLE_BRANCH_ANY_CALL = 1U << 4, /* any call branch */
>> PERF_SAMPLE_BRANCH_ANY_RETURN = 1U << 5, /* any return branch */
>> PERF_SAMPLE_BRANCH_IND_CALL = 1U << 6, /* indirect calls */
>> + PERF_SAMPLE_BRANCH_CONDITIONAL = 1U << 7, /* conditional branches */
>>
> I would use PERF_SAMPLE_BRANCH_COND here.
>
>> - PERF_SAMPLE_BRANCH_MAX = 1U << 7, /* non-ABI */
>> + PERF_SAMPLE_BRANCH_MAX = 1U << 8, /* non-ABI */
>> };
>>
>> #define PERF_SAMPLE_BRANCH_PLM_ALL \
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index cdf58ec..5b0b89d 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
>> BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL),
>> BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN),
>> BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL),
>> + BRANCH_OPT("cnd", PERF_SAMPLE_BRANCH_CONDITIONAL),
>
> use "cond"
>
>> BRANCH_END
>> };
>>
>
> And if you do this, you also need to update the x86
> perf_event_intel_lbr.c mapping
> tables to fill out the entries for PERF_SAMPLE_BRANCH_COND:
>
> [PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
>
> And you also need to update intel_pmu_setup_sw_lbr_filter()
> to handle the conversion to x86 instructions:
>
>if (br_type & PERF_SAMPLE_BRANCH_COND)
> mask |= X86_BR_JCC;
>
>
> You also need to update the perf-record.txt documentation to list cond
> as a possible
> branch filter.
Hey Stephane,
I have incorporated all the review comments into the patch series
https://lkml.org/lkml/2013/5/22/51.
Regards
Anshuman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On 05/21/2013 07:25 PM, Stephane Eranian wrote:
On Thu, May 16, 2013 at 12:15 PM, Michael Neuling mi...@neuling.org wrote:
Peter Zijlstra pet...@infradead.org wrote:
On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra a.p.zijls...@chello.nl
wrote:
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen a...@linux.intel.com
Cc: eran...@google.com
Signed-off-by: Peter Zijlstra a.p.zijls...@chello.nl
Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
This will prevent regular users from capturing kernel - kernel branches.
But it won't prevent users from getting kernel - user branches. Thus
some kernel address will still be captured. I guess they could be
eliminated
by the sw_filter.
When using LBR priv level filtering, the filter applies to the branch
target
only.
How about something like the below? It also adds the branch flags
Mikey wanted for PowerPC.
Peter,
BTW PowerPC also has the ability to filter on conditional branches. Any
chance we could add something like the follow to perf also?
Mikey
diff --git a/include/uapi/linux/perf_event.h
b/include/uapi/linux/perf_event.h
index fb104e5..891c769 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -157,8 +157,9 @@ enum perf_branch_sample_type {
PERF_SAMPLE_BRANCH_ANY_CALL = 1U 4, /* any call branch */
PERF_SAMPLE_BRANCH_ANY_RETURN = 1U 5, /* any return branch */
PERF_SAMPLE_BRANCH_IND_CALL = 1U 6, /* indirect calls */
+ PERF_SAMPLE_BRANCH_CONDITIONAL = 1U 7, /* conditional branches */
I would use PERF_SAMPLE_BRANCH_COND here.
- PERF_SAMPLE_BRANCH_MAX = 1U 7, /* non-ABI */
+ PERF_SAMPLE_BRANCH_MAX = 1U 8, /* non-ABI */
};
#define PERF_SAMPLE_BRANCH_PLM_ALL \
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cdf58ec..5b0b89d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
BRANCH_OPT(any_call, PERF_SAMPLE_BRANCH_ANY_CALL),
BRANCH_OPT(any_ret, PERF_SAMPLE_BRANCH_ANY_RETURN),
BRANCH_OPT(ind_call, PERF_SAMPLE_BRANCH_IND_CALL),
+ BRANCH_OPT(cnd, PERF_SAMPLE_BRANCH_CONDITIONAL),
use cond
BRANCH_END
};
And if you do this, you also need to update the x86
perf_event_intel_lbr.c mapping
tables to fill out the entries for PERF_SAMPLE_BRANCH_COND:
[PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
And you also need to update intel_pmu_setup_sw_lbr_filter()
to handle the conversion to x86 instructions:
if (br_type PERF_SAMPLE_BRANCH_COND)
mask |= X86_BR_JCC;
You also need to update the perf-record.txt documentation to list cond
as a possible
branch filter.
Hey Stephane,
I have incorporated all the review comments into the patch series
https://lkml.org/lkml/2013/5/22/51.
Regards
Anshuman
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Hi,
On Wed, May 22, 2013 at 8:43 AM, Anshuman Khandual
khand...@linux.vnet.ibm.com wrote:
On 05/21/2013 07:25 PM, Stephane Eranian wrote:
On Thu, May 16, 2013 at 12:15 PM, Michael Neuling mi...@neuling.org wrote:
Peter Zijlstra pet...@infradead.org wrote:
On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra a.p.zijls...@chello.nl
wrote:
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen a...@linux.intel.com
Cc: eran...@google.com
Signed-off-by: Peter Zijlstra a.p.zijls...@chello.nl
Link:
http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
This will prevent regular users from capturing kernel - kernel branches.
But it won't prevent users from getting kernel - user branches. Thus
some kernel address will still be captured. I guess they could be
eliminated
by the sw_filter.
When using LBR priv level filtering, the filter applies to the branch
target
only.
How about something like the below? It also adds the branch flags
Mikey wanted for PowerPC.
Peter,
BTW PowerPC also has the ability to filter on conditional branches. Any
chance we could add something like the follow to perf also?
Mikey
diff --git a/include/uapi/linux/perf_event.h
b/include/uapi/linux/perf_event.h
index fb104e5..891c769 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -157,8 +157,9 @@ enum perf_branch_sample_type {
PERF_SAMPLE_BRANCH_ANY_CALL = 1U 4, /* any call branch */
PERF_SAMPLE_BRANCH_ANY_RETURN = 1U 5, /* any return branch */
PERF_SAMPLE_BRANCH_IND_CALL = 1U 6, /* indirect calls */
+ PERF_SAMPLE_BRANCH_CONDITIONAL = 1U 7, /* conditional branches
*/
I would use PERF_SAMPLE_BRANCH_COND here.
- PERF_SAMPLE_BRANCH_MAX = 1U 7, /* non-ABI */
+ PERF_SAMPLE_BRANCH_MAX = 1U 8, /* non-ABI */
};
#define PERF_SAMPLE_BRANCH_PLM_ALL \
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cdf58ec..5b0b89d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
BRANCH_OPT(any_call, PERF_SAMPLE_BRANCH_ANY_CALL),
BRANCH_OPT(any_ret, PERF_SAMPLE_BRANCH_ANY_RETURN),
BRANCH_OPT(ind_call, PERF_SAMPLE_BRANCH_IND_CALL),
+ BRANCH_OPT(cnd, PERF_SAMPLE_BRANCH_CONDITIONAL),
use cond
BRANCH_END
};
And if you do this, you also need to update the x86
perf_event_intel_lbr.c mapping
tables to fill out the entries for PERF_SAMPLE_BRANCH_COND:
[PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
And you also need to update intel_pmu_setup_sw_lbr_filter()
to handle the conversion to x86 instructions:
if (br_type PERF_SAMPLE_BRANCH_COND)
mask |= X86_BR_JCC;
You also need to update the perf-record.txt documentation to list cond
as a possible
branch filter.
Hey Stephane,
I have incorporated all the review comments into the patch series
https://lkml.org/lkml/2013/5/22/51.
I don't see how you can compile Patch 3/5:
+ BRANCH_OPT(cond, PERF_SAMPLE_BRANCH_CONDITIONAL),
Needs to be PERF_SAMPLE_BRANCH_COND.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On 05/22/2013 05:53 PM, Stephane Eranian wrote:
Hi,
On Wed, May 22, 2013 at 8:43 AM, Anshuman Khandual
khand...@linux.vnet.ibm.com wrote:
On 05/21/2013 07:25 PM, Stephane Eranian wrote:
On Thu, May 16, 2013 at 12:15 PM, Michael Neuling mi...@neuling.org wrote:
Peter Zijlstra pet...@infradead.org wrote:
On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra a.p.zijls...@chello.nl
wrote:
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen a...@linux.intel.com
Cc: eran...@google.com
Signed-off-by: Peter Zijlstra a.p.zijls...@chello.nl
Link:
http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
This will prevent regular users from capturing kernel - kernel branches.
But it won't prevent users from getting kernel - user branches. Thus
some kernel address will still be captured. I guess they could be
eliminated
by the sw_filter.
When using LBR priv level filtering, the filter applies to the branch
target
only.
How about something like the below? It also adds the branch flags
Mikey wanted for PowerPC.
Peter,
BTW PowerPC also has the ability to filter on conditional branches. Any
chance we could add something like the follow to perf also?
Mikey
diff --git a/include/uapi/linux/perf_event.h
b/include/uapi/linux/perf_event.h
index fb104e5..891c769 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -157,8 +157,9 @@ enum perf_branch_sample_type {
PERF_SAMPLE_BRANCH_ANY_CALL = 1U 4, /* any call branch */
PERF_SAMPLE_BRANCH_ANY_RETURN = 1U 5, /* any return branch */
PERF_SAMPLE_BRANCH_IND_CALL = 1U 6, /* indirect calls */
+ PERF_SAMPLE_BRANCH_CONDITIONAL = 1U 7, /* conditional branches
*/
I would use PERF_SAMPLE_BRANCH_COND here.
- PERF_SAMPLE_BRANCH_MAX = 1U 7, /* non-ABI */
+ PERF_SAMPLE_BRANCH_MAX = 1U 8, /* non-ABI */
};
#define PERF_SAMPLE_BRANCH_PLM_ALL \
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cdf58ec..5b0b89d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
BRANCH_OPT(any_call, PERF_SAMPLE_BRANCH_ANY_CALL),
BRANCH_OPT(any_ret, PERF_SAMPLE_BRANCH_ANY_RETURN),
BRANCH_OPT(ind_call, PERF_SAMPLE_BRANCH_IND_CALL),
+ BRANCH_OPT(cnd, PERF_SAMPLE_BRANCH_CONDITIONAL),
use cond
BRANCH_END
};
And if you do this, you also need to update the x86
perf_event_intel_lbr.c mapping
tables to fill out the entries for PERF_SAMPLE_BRANCH_COND:
[PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
And you also need to update intel_pmu_setup_sw_lbr_filter()
to handle the conversion to x86 instructions:
if (br_type PERF_SAMPLE_BRANCH_COND)
mask |= X86_BR_JCC;
You also need to update the perf-record.txt documentation to list cond
as a possible
branch filter.
Hey Stephane,
I have incorporated all the review comments into the patch series
https://lkml.org/lkml/2013/5/22/51.
I don't see how you can compile Patch 3/5:
+ BRANCH_OPT(cond, PERF_SAMPLE_BRANCH_CONDITIONAL),
Needs to be PERF_SAMPLE_BRANCH_COND.
Ahh, sorry missed it, will fix it.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Thu, May 16, 2013 at 12:15 PM, Michael Neuling wrote:
> Peter Zijlstra wrote:
>
>> On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
>> > On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra
>> > wrote:
>> > > We should always have proper privileges when requesting kernel data.
>> > >
>> > > Cc: Andi Kleen
>> > > Cc: eran...@google.com
>> > > Signed-off-by: Peter Zijlstra
>> > > Link:
>> > > http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
>> > > ---
>> > > arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
>> > > 1 file changed, 4 insertions(+), 1 deletion(-)
>> > >
>> > > --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
>> > > +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
>> > > @@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
>> > > if (br_type & PERF_SAMPLE_BRANCH_USER)
>> > > mask |= X86_BR_USER;
>> > >
>> > > - if (br_type & PERF_SAMPLE_BRANCH_KERNEL)
>> > > + if (br_type & PERF_SAMPLE_BRANCH_KERNEL) {
>> > > + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
>> > > + return -EACCES;
>> > > mask |= X86_BR_KERNEL;
>> > > + }
>> > >
>> > This will prevent regular users from capturing kernel -> kernel branches.
>> > But it won't prevent users from getting kernel -> user branches. Thus
>> > some kernel address will still be captured. I guess they could be
>> > eliminated
>> > by the sw_filter.
>> >
>> > When using LBR priv level filtering, the filter applies to the branch
>> > target
>> > only.
>>
>> How about something like the below? It also adds the branch flags
>> Mikey wanted for PowerPC.
>
> Peter,
>
> BTW PowerPC also has the ability to filter on conditional branches. Any
> chance we could add something like the follow to perf also?
>
> Mikey
>
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index fb104e5..891c769 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -157,8 +157,9 @@ enum perf_branch_sample_type {
> PERF_SAMPLE_BRANCH_ANY_CALL = 1U << 4, /* any call branch */
> PERF_SAMPLE_BRANCH_ANY_RETURN = 1U << 5, /* any return branch */
> PERF_SAMPLE_BRANCH_IND_CALL = 1U << 6, /* indirect calls */
> + PERF_SAMPLE_BRANCH_CONDITIONAL = 1U << 7, /* conditional branches */
>
I would use PERF_SAMPLE_BRANCH_COND here.
> - PERF_SAMPLE_BRANCH_MAX = 1U << 7, /* non-ABI */
> + PERF_SAMPLE_BRANCH_MAX = 1U << 8, /* non-ABI */
> };
>
> #define PERF_SAMPLE_BRANCH_PLM_ALL \
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index cdf58ec..5b0b89d 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
> BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL),
> BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN),
> BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL),
> + BRANCH_OPT("cnd", PERF_SAMPLE_BRANCH_CONDITIONAL),
use "cond"
> BRANCH_END
> };
>
And if you do this, you also need to update the x86
perf_event_intel_lbr.c mapping
tables to fill out the entries for PERF_SAMPLE_BRANCH_COND:
[PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
And you also need to update intel_pmu_setup_sw_lbr_filter()
to handle the conversion to x86 instructions:
if (br_type & PERF_SAMPLE_BRANCH_COND)
mask |= X86_BR_JCC;
You also need to update the perf-record.txt documentation to list cond
as a possible
branch filter.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Tue, May 21, 2013 at 10:50 AM, Peter Zijlstra wrote: > On Tue, May 21, 2013 at 03:41:35PM +1000, Michael Neuling wrote: >> Peter Zijlstra wrote: >> >> Can we add your signed-off-by on this? >> >> We are cleaning up our series for conditional branches and would like to >> add this as part of the post. > > Sure, but its completely untested.. I was hoping Stephane would say > somnething about it since he wrote all that magic ;-) > Let me take a look at it. > But yeah, feel free to add my SoB. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Tue, May 21, 2013 at 03:41:35PM +1000, Michael Neuling wrote: > Peter Zijlstra wrote: > > Can we add your signed-off-by on this? > > We are cleaning up our series for conditional branches and would like to > add this as part of the post. Sure, but its completely untested.. I was hoping Stephane would say somnething about it since he wrote all that magic ;-) But yeah, feel free to add my SoB. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Tue, May 21, 2013 at 03:41:35PM +1000, Michael Neuling wrote: Peter Zijlstra pet...@infradead.org wrote: Can we add your signed-off-by on this? We are cleaning up our series for conditional branches and would like to add this as part of the post. Sure, but its completely untested.. I was hoping Stephane would say somnething about it since he wrote all that magic ;-) But yeah, feel free to add my SoB. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Tue, May 21, 2013 at 10:50 AM, Peter Zijlstra pet...@infradead.org wrote: On Tue, May 21, 2013 at 03:41:35PM +1000, Michael Neuling wrote: Peter Zijlstra pet...@infradead.org wrote: Can we add your signed-off-by on this? We are cleaning up our series for conditional branches and would like to add this as part of the post. Sure, but its completely untested.. I was hoping Stephane would say somnething about it since he wrote all that magic ;-) Let me take a look at it. But yeah, feel free to add my SoB. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Thu, May 16, 2013 at 12:15 PM, Michael Neuling mi...@neuling.org wrote:
Peter Zijlstra pet...@infradead.org wrote:
On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra a.p.zijls...@chello.nl
wrote:
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen a...@linux.intel.com
Cc: eran...@google.com
Signed-off-by: Peter Zijlstra a.p.zijls...@chello.nl
Link:
http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
This will prevent regular users from capturing kernel - kernel branches.
But it won't prevent users from getting kernel - user branches. Thus
some kernel address will still be captured. I guess they could be
eliminated
by the sw_filter.
When using LBR priv level filtering, the filter applies to the branch
target
only.
How about something like the below? It also adds the branch flags
Mikey wanted for PowerPC.
Peter,
BTW PowerPC also has the ability to filter on conditional branches. Any
chance we could add something like the follow to perf also?
Mikey
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index fb104e5..891c769 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -157,8 +157,9 @@ enum perf_branch_sample_type {
PERF_SAMPLE_BRANCH_ANY_CALL = 1U 4, /* any call branch */
PERF_SAMPLE_BRANCH_ANY_RETURN = 1U 5, /* any return branch */
PERF_SAMPLE_BRANCH_IND_CALL = 1U 6, /* indirect calls */
+ PERF_SAMPLE_BRANCH_CONDITIONAL = 1U 7, /* conditional branches */
I would use PERF_SAMPLE_BRANCH_COND here.
- PERF_SAMPLE_BRANCH_MAX = 1U 7, /* non-ABI */
+ PERF_SAMPLE_BRANCH_MAX = 1U 8, /* non-ABI */
};
#define PERF_SAMPLE_BRANCH_PLM_ALL \
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cdf58ec..5b0b89d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
BRANCH_OPT(any_call, PERF_SAMPLE_BRANCH_ANY_CALL),
BRANCH_OPT(any_ret, PERF_SAMPLE_BRANCH_ANY_RETURN),
BRANCH_OPT(ind_call, PERF_SAMPLE_BRANCH_IND_CALL),
+ BRANCH_OPT(cnd, PERF_SAMPLE_BRANCH_CONDITIONAL),
use cond
BRANCH_END
};
And if you do this, you also need to update the x86
perf_event_intel_lbr.c mapping
tables to fill out the entries for PERF_SAMPLE_BRANCH_COND:
[PERF_SAMPLE_BRANCH_COND] = LBR_JCC,
And you also need to update intel_pmu_setup_sw_lbr_filter()
to handle the conversion to x86 instructions:
if (br_type PERF_SAMPLE_BRANCH_COND)
mask |= X86_BR_JCC;
You also need to update the perf-record.txt documentation to list cond
as a possible
branch filter.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Peter Zijlstra wrote: > On Thu, May 16, 2013 at 05:36:11PM +0200, Stephane Eranian wrote: > > On Thu, May 16, 2013 at 1:16 PM, Peter Zijlstra > > wrote: > > > On Thu, May 16, 2013 at 08:15:17PM +1000, Michael Neuling wrote: > > >> Peter, > > >> > > >> BTW PowerPC also has the ability to filter on conditional branches. Any > > >> chance we could add something like the follow to perf also? > > >> > > > > > > I don't see an immediate problem with that except that we on x86 need to > > > implement that in the software filter. Stephane do you see any > > > fundamental issue with that? > > > > > On X86, the LBR cannot filter on conditional in HW. Thus as Peter said, it > > would > > have to be done in SW. I did not add that because I think those branches are > > not necessarily useful for tools. > > Wouldn't it be mostly conditional branches that are the primary control flow > and can get predicted wrong? I mean, I'm sure someone will miss-predict an > unconditional branch but its not like we care about people with such > afflictions do we? > > Anyway, since PPC people thought it worth baking into hardware, presumably > they > have a compelling use case. Mikey could you see if you can retrieve that from > someone in the know? It might be interesting. > > Also, it looks like its trivial to add to x86, you seem to have already done > all the hard work by having X86_BR_JCC. > > The only missing piece would be: Peter, Can we add your signed-off-by on this? We are cleaning up our series for conditional branches and would like to add this as part of the post. Mikey > > --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c > +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c > @@ -337,6 +337,10 @@ static int intel_pmu_setup_sw_lbr_filter > > if (br_type & PERF_SAMPLE_BRANCH_IND_CALL) > mask |= X86_BR_IND_CALL; > + > + if (br_type & PERF_SAMPLE_BRANCH_CONDITIONAL) > + mask |= X86_BR_JCC; > + > /* >* stash actual user request into reg, it may >* be used by fixup code for some CPU > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Peter Zijlstra pet...@infradead.org wrote: On Thu, May 16, 2013 at 05:36:11PM +0200, Stephane Eranian wrote: On Thu, May 16, 2013 at 1:16 PM, Peter Zijlstra pet...@infradead.org wrote: On Thu, May 16, 2013 at 08:15:17PM +1000, Michael Neuling wrote: Peter, BTW PowerPC also has the ability to filter on conditional branches. Any chance we could add something like the follow to perf also? I don't see an immediate problem with that except that we on x86 need to implement that in the software filter. Stephane do you see any fundamental issue with that? On X86, the LBR cannot filter on conditional in HW. Thus as Peter said, it would have to be done in SW. I did not add that because I think those branches are not necessarily useful for tools. Wouldn't it be mostly conditional branches that are the primary control flow and can get predicted wrong? I mean, I'm sure someone will miss-predict an unconditional branch but its not like we care about people with such afflictions do we? Anyway, since PPC people thought it worth baking into hardware, presumably they have a compelling use case. Mikey could you see if you can retrieve that from someone in the know? It might be interesting. Also, it looks like its trivial to add to x86, you seem to have already done all the hard work by having X86_BR_JCC. The only missing piece would be: Peter, Can we add your signed-off-by on this? We are cleaning up our series for conditional branches and would like to add this as part of the post. Mikey --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c @@ -337,6 +337,10 @@ static int intel_pmu_setup_sw_lbr_filter if (br_type PERF_SAMPLE_BRANCH_IND_CALL) mask |= X86_BR_IND_CALL; + + if (br_type PERF_SAMPLE_BRANCH_CONDITIONAL) + mask |= X86_BR_JCC; + /* * stash actual user request into reg, it may * be used by fixup code for some CPU -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Sat, May 18, 2013 at 12:14 AM, Michael Neuling wrote: > Stephane Eranian wrote: > >> On Fri, May 17, 2013 at 1:39 PM, Peter Zijlstra wrote: >> > On Fri, May 17, 2013 at 09:32:08PM +1000, Michael Neuling wrote: >> >> Peter Zijlstra wrote: >> > >> >> > Wouldn't it be mostly conditional branches that are the primary control >> >> > flow >> >> > and can get predicted wrong? I mean, I'm sure someone will miss-predict >> >> > an >> >> > unconditional branch but its not like we care about people with such >> >> > afflictions do we? >> >> >> >> You could mispredict the target address of a computed goto. You'd know >> >> it was taken but not know target address until later in the pipeline. >> > >> > Oh right, computed targets could indeed be mis predicted. I was more >> > thinking >> > about jumps with immediate values. >> > >> >> On this, the POWER8 branch history buffer tells us two things about the >> >> prediction status. >> >> 1) if the branch was predicted taken/not taken correctly >> >> 2) if the target address was predicted correctly or not (for computed >> >> gotos only) >> >> So we'd actually like more prediction bits too :-D >> > >> > So if I understand this right, 1) maps to the predicted flags we have; 2) >> > would be new stuff? >> > >> > We don't really have anything like that on x86, but I suppose if you make >> > the >> > thing optional and present a 'useful' use-case implemented in userspace >> > code >> > we could take it :-) >> > >> >> > Anyway, since PPC people thought it worth baking into hardware, >> >> > presumably they have a compelling use case. Mikey could you see if you >> >> > can retrieve that from someone in the know? It might be interesting. >> >> >> >> I don't think we can mispredict a non-conditional non-computed but I'll >> >> have to check with the HW folks. >> > >> > I was mostly wondering about the use-case for the conditional filter. >> > Stephane >> > didn't think it useful, clearly your hardware guys thought different :-) >> >> From my experience talking with compiler people, they care about ALL >> the branches and not the conditional so much. They use LBR to do basic >> block profiling. > > OK. I don't have a good handle on what's useful for compilers or JITs > right now. I'm just plumbing through what's possible. > I understand. It is okay to extend the interface. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Stephane Eranian wrote: > On Fri, May 17, 2013 at 1:39 PM, Peter Zijlstra wrote: > > On Fri, May 17, 2013 at 09:32:08PM +1000, Michael Neuling wrote: > >> Peter Zijlstra wrote: > > > >> > Wouldn't it be mostly conditional branches that are the primary control > >> > flow > >> > and can get predicted wrong? I mean, I'm sure someone will miss-predict > >> > an > >> > unconditional branch but its not like we care about people with such > >> > afflictions do we? > >> > >> You could mispredict the target address of a computed goto. You'd know > >> it was taken but not know target address until later in the pipeline. > > > > Oh right, computed targets could indeed be mis predicted. I was more > > thinking > > about jumps with immediate values. > > > >> On this, the POWER8 branch history buffer tells us two things about the > >> prediction status. > >> 1) if the branch was predicted taken/not taken correctly > >> 2) if the target address was predicted correctly or not (for computed > >> gotos only) > >> So we'd actually like more prediction bits too :-D > > > > So if I understand this right, 1) maps to the predicted flags we have; 2) > > would be new stuff? > > > > We don't really have anything like that on x86, but I suppose if you make > > the > > thing optional and present a 'useful' use-case implemented in userspace code > > we could take it :-) > > > >> > Anyway, since PPC people thought it worth baking into hardware, > >> > presumably they have a compelling use case. Mikey could you see if you > >> > can retrieve that from someone in the know? It might be interesting. > >> > >> I don't think we can mispredict a non-conditional non-computed but I'll > >> have to check with the HW folks. > > > > I was mostly wondering about the use-case for the conditional filter. > > Stephane > > didn't think it useful, clearly your hardware guys thought different :-) > > From my experience talking with compiler people, they care about ALL > the branches and not the conditional so much. They use LBR to do basic > block profiling. OK. I don't have a good handle on what's useful for compilers or JITs right now. I'm just plumbing through what's possible. Mikey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Fri, May 17, 2013 at 1:39 PM, Peter Zijlstra wrote: > On Fri, May 17, 2013 at 09:32:08PM +1000, Michael Neuling wrote: >> Peter Zijlstra wrote: > >> > Wouldn't it be mostly conditional branches that are the primary control >> > flow >> > and can get predicted wrong? I mean, I'm sure someone will miss-predict an >> > unconditional branch but its not like we care about people with such >> > afflictions do we? >> >> You could mispredict the target address of a computed goto. You'd know >> it was taken but not know target address until later in the pipeline. > > Oh right, computed targets could indeed be mis predicted. I was more thinking > about jumps with immediate values. > >> On this, the POWER8 branch history buffer tells us two things about the >> prediction status. >> 1) if the branch was predicted taken/not taken correctly >> 2) if the target address was predicted correctly or not (for computed >> gotos only) >> So we'd actually like more prediction bits too :-D > > So if I understand this right, 1) maps to the predicted flags we have; 2) > would be new stuff? > > We don't really have anything like that on x86, but I suppose if you make the > thing optional and present a 'useful' use-case implemented in userspace code > we could take it :-) > >> > Anyway, since PPC people thought it worth baking into hardware, >> > presumably they have a compelling use case. Mikey could you see if you >> > can retrieve that from someone in the know? It might be interesting. >> >> I don't think we can mispredict a non-conditional non-computed but I'll >> have to check with the HW folks. > > I was mostly wondering about the use-case for the conditional filter. Stephane > didn't think it useful, clearly your hardware guys thought different :-) >From my experience talking with compiler people, they care about ALL the branches and not the conditional so much. They use LBR to do basic block profiling. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Fri, May 17, 2013 at 09:32:08PM +1000, Michael Neuling wrote: > Peter Zijlstra wrote: > > Wouldn't it be mostly conditional branches that are the primary control flow > > and can get predicted wrong? I mean, I'm sure someone will miss-predict an > > unconditional branch but its not like we care about people with such > > afflictions do we? > > You could mispredict the target address of a computed goto. You'd know > it was taken but not know target address until later in the pipeline. Oh right, computed targets could indeed be mis predicted. I was more thinking about jumps with immediate values. > On this, the POWER8 branch history buffer tells us two things about the > prediction status. > 1) if the branch was predicted taken/not taken correctly > 2) if the target address was predicted correctly or not (for computed > gotos only) > So we'd actually like more prediction bits too :-D So if I understand this right, 1) maps to the predicted flags we have; 2) would be new stuff? We don't really have anything like that on x86, but I suppose if you make the thing optional and present a 'useful' use-case implemented in userspace code we could take it :-) > > Anyway, since PPC people thought it worth baking into hardware, > > presumably they have a compelling use case. Mikey could you see if you > > can retrieve that from someone in the know? It might be interesting. > > I don't think we can mispredict a non-conditional non-computed but I'll > have to check with the HW folks. I was mostly wondering about the use-case for the conditional filter. Stephane didn't think it useful, clearly your hardware guys thought different :-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Peter Zijlstra wrote: > On Thu, May 16, 2013 at 05:36:11PM +0200, Stephane Eranian wrote: > > On Thu, May 16, 2013 at 1:16 PM, Peter Zijlstra > > wrote: > > > On Thu, May 16, 2013 at 08:15:17PM +1000, Michael Neuling wrote: > > >> Peter, > > >> > > >> BTW PowerPC also has the ability to filter on conditional branches. Any > > >> chance we could add something like the follow to perf also? > > >> > > > > > > I don't see an immediate problem with that except that we on x86 need to > > > implement that in the software filter. Stephane do you see any > > > fundamental issue with that? > > > > > On X86, the LBR cannot filter on conditional in HW. Thus as Peter said, it > > would > > have to be done in SW. I did not add that because I think those branches are > > not necessarily useful for tools. > > Wouldn't it be mostly conditional branches that are the primary control flow > and can get predicted wrong? I mean, I'm sure someone will miss-predict an > unconditional branch but its not like we care about people with such > afflictions do we? You could mispredict the target address of a computed goto. You'd know it was taken but not know target address until later in the pipeline. On this, the POWER8 branch history buffer tells us two things about the prediction status. 1) if the branch was predicted taken/not taken correctly 2) if the target address was predicted correctly or not (for computed gotos only) So we'd actually like more prediction bits too :-D > Anyway, since PPC people thought it worth baking into hardware, > presumably they have a compelling use case. Mikey could you see if you > can retrieve that from someone in the know? It might be interesting. I don't think we can mispredict a non-conditional non-computed but I'll have to check with the HW folks. Mikey > > Also, it looks like its trivial to add to x86, you seem to have already done > all the hard work by having X86_BR_JCC. > > The only missing piece would be: > > --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c > +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c > @@ -337,6 +337,10 @@ static int intel_pmu_setup_sw_lbr_filter > > if (br_type & PERF_SAMPLE_BRANCH_IND_CALL) > mask |= X86_BR_IND_CALL; > + > + if (br_type & PERF_SAMPLE_BRANCH_CONDITIONAL) > + mask |= X86_BR_JCC; > + > /* >* stash actual user request into reg, it may >* be used by fixup code for some CPU > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Thu, May 16, 2013 at 05:36:11PM +0200, Stephane Eranian wrote: > On Thu, May 16, 2013 at 1:16 PM, Peter Zijlstra wrote: > > On Thu, May 16, 2013 at 08:15:17PM +1000, Michael Neuling wrote: > >> Peter, > >> > >> BTW PowerPC also has the ability to filter on conditional branches. Any > >> chance we could add something like the follow to perf also? > >> > > > > I don't see an immediate problem with that except that we on x86 need to > > implement that in the software filter. Stephane do you see any > > fundamental issue with that? > > > On X86, the LBR cannot filter on conditional in HW. Thus as Peter said, it > would > have to be done in SW. I did not add that because I think those branches are > not necessarily useful for tools. Wouldn't it be mostly conditional branches that are the primary control flow and can get predicted wrong? I mean, I'm sure someone will miss-predict an unconditional branch but its not like we care about people with such afflictions do we? Anyway, since PPC people thought it worth baking into hardware, presumably they have a compelling use case. Mikey could you see if you can retrieve that from someone in the know? It might be interesting. Also, it looks like its trivial to add to x86, you seem to have already done all the hard work by having X86_BR_JCC. The only missing piece would be: --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c @@ -337,6 +337,10 @@ static int intel_pmu_setup_sw_lbr_filter if (br_type & PERF_SAMPLE_BRANCH_IND_CALL) mask |= X86_BR_IND_CALL; + + if (br_type & PERF_SAMPLE_BRANCH_CONDITIONAL) + mask |= X86_BR_JCC; + /* * stash actual user request into reg, it may * be used by fixup code for some CPU -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Thu, May 16, 2013 at 05:36:11PM +0200, Stephane Eranian wrote: On Thu, May 16, 2013 at 1:16 PM, Peter Zijlstra pet...@infradead.org wrote: On Thu, May 16, 2013 at 08:15:17PM +1000, Michael Neuling wrote: Peter, BTW PowerPC also has the ability to filter on conditional branches. Any chance we could add something like the follow to perf also? I don't see an immediate problem with that except that we on x86 need to implement that in the software filter. Stephane do you see any fundamental issue with that? On X86, the LBR cannot filter on conditional in HW. Thus as Peter said, it would have to be done in SW. I did not add that because I think those branches are not necessarily useful for tools. Wouldn't it be mostly conditional branches that are the primary control flow and can get predicted wrong? I mean, I'm sure someone will miss-predict an unconditional branch but its not like we care about people with such afflictions do we? Anyway, since PPC people thought it worth baking into hardware, presumably they have a compelling use case. Mikey could you see if you can retrieve that from someone in the know? It might be interesting. Also, it looks like its trivial to add to x86, you seem to have already done all the hard work by having X86_BR_JCC. The only missing piece would be: --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c @@ -337,6 +337,10 @@ static int intel_pmu_setup_sw_lbr_filter if (br_type PERF_SAMPLE_BRANCH_IND_CALL) mask |= X86_BR_IND_CALL; + + if (br_type PERF_SAMPLE_BRANCH_CONDITIONAL) + mask |= X86_BR_JCC; + /* * stash actual user request into reg, it may * be used by fixup code for some CPU -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Peter Zijlstra pet...@infradead.org wrote: On Thu, May 16, 2013 at 05:36:11PM +0200, Stephane Eranian wrote: On Thu, May 16, 2013 at 1:16 PM, Peter Zijlstra pet...@infradead.org wrote: On Thu, May 16, 2013 at 08:15:17PM +1000, Michael Neuling wrote: Peter, BTW PowerPC also has the ability to filter on conditional branches. Any chance we could add something like the follow to perf also? I don't see an immediate problem with that except that we on x86 need to implement that in the software filter. Stephane do you see any fundamental issue with that? On X86, the LBR cannot filter on conditional in HW. Thus as Peter said, it would have to be done in SW. I did not add that because I think those branches are not necessarily useful for tools. Wouldn't it be mostly conditional branches that are the primary control flow and can get predicted wrong? I mean, I'm sure someone will miss-predict an unconditional branch but its not like we care about people with such afflictions do we? You could mispredict the target address of a computed goto. You'd know it was taken but not know target address until later in the pipeline. On this, the POWER8 branch history buffer tells us two things about the prediction status. 1) if the branch was predicted taken/not taken correctly 2) if the target address was predicted correctly or not (for computed gotos only) So we'd actually like more prediction bits too :-D Anyway, since PPC people thought it worth baking into hardware, presumably they have a compelling use case. Mikey could you see if you can retrieve that from someone in the know? It might be interesting. I don't think we can mispredict a non-conditional non-computed but I'll have to check with the HW folks. Mikey Also, it looks like its trivial to add to x86, you seem to have already done all the hard work by having X86_BR_JCC. The only missing piece would be: --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c @@ -337,6 +337,10 @@ static int intel_pmu_setup_sw_lbr_filter if (br_type PERF_SAMPLE_BRANCH_IND_CALL) mask |= X86_BR_IND_CALL; + + if (br_type PERF_SAMPLE_BRANCH_CONDITIONAL) + mask |= X86_BR_JCC; + /* * stash actual user request into reg, it may * be used by fixup code for some CPU -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Fri, May 17, 2013 at 09:32:08PM +1000, Michael Neuling wrote: Peter Zijlstra pet...@infradead.org wrote: Wouldn't it be mostly conditional branches that are the primary control flow and can get predicted wrong? I mean, I'm sure someone will miss-predict an unconditional branch but its not like we care about people with such afflictions do we? You could mispredict the target address of a computed goto. You'd know it was taken but not know target address until later in the pipeline. Oh right, computed targets could indeed be mis predicted. I was more thinking about jumps with immediate values. On this, the POWER8 branch history buffer tells us two things about the prediction status. 1) if the branch was predicted taken/not taken correctly 2) if the target address was predicted correctly or not (for computed gotos only) So we'd actually like more prediction bits too :-D So if I understand this right, 1) maps to the predicted flags we have; 2) would be new stuff? We don't really have anything like that on x86, but I suppose if you make the thing optional and present a 'useful' use-case implemented in userspace code we could take it :-) Anyway, since PPC people thought it worth baking into hardware, presumably they have a compelling use case. Mikey could you see if you can retrieve that from someone in the know? It might be interesting. I don't think we can mispredict a non-conditional non-computed but I'll have to check with the HW folks. I was mostly wondering about the use-case for the conditional filter. Stephane didn't think it useful, clearly your hardware guys thought different :-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Fri, May 17, 2013 at 1:39 PM, Peter Zijlstra pet...@infradead.org wrote: On Fri, May 17, 2013 at 09:32:08PM +1000, Michael Neuling wrote: Peter Zijlstra pet...@infradead.org wrote: Wouldn't it be mostly conditional branches that are the primary control flow and can get predicted wrong? I mean, I'm sure someone will miss-predict an unconditional branch but its not like we care about people with such afflictions do we? You could mispredict the target address of a computed goto. You'd know it was taken but not know target address until later in the pipeline. Oh right, computed targets could indeed be mis predicted. I was more thinking about jumps with immediate values. On this, the POWER8 branch history buffer tells us two things about the prediction status. 1) if the branch was predicted taken/not taken correctly 2) if the target address was predicted correctly or not (for computed gotos only) So we'd actually like more prediction bits too :-D So if I understand this right, 1) maps to the predicted flags we have; 2) would be new stuff? We don't really have anything like that on x86, but I suppose if you make the thing optional and present a 'useful' use-case implemented in userspace code we could take it :-) Anyway, since PPC people thought it worth baking into hardware, presumably they have a compelling use case. Mikey could you see if you can retrieve that from someone in the know? It might be interesting. I don't think we can mispredict a non-conditional non-computed but I'll have to check with the HW folks. I was mostly wondering about the use-case for the conditional filter. Stephane didn't think it useful, clearly your hardware guys thought different :-) From my experience talking with compiler people, they care about ALL the branches and not the conditional so much. They use LBR to do basic block profiling. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Stephane Eranian eran...@google.com wrote: On Fri, May 17, 2013 at 1:39 PM, Peter Zijlstra pet...@infradead.org wrote: On Fri, May 17, 2013 at 09:32:08PM +1000, Michael Neuling wrote: Peter Zijlstra pet...@infradead.org wrote: Wouldn't it be mostly conditional branches that are the primary control flow and can get predicted wrong? I mean, I'm sure someone will miss-predict an unconditional branch but its not like we care about people with such afflictions do we? You could mispredict the target address of a computed goto. You'd know it was taken but not know target address until later in the pipeline. Oh right, computed targets could indeed be mis predicted. I was more thinking about jumps with immediate values. On this, the POWER8 branch history buffer tells us two things about the prediction status. 1) if the branch was predicted taken/not taken correctly 2) if the target address was predicted correctly or not (for computed gotos only) So we'd actually like more prediction bits too :-D So if I understand this right, 1) maps to the predicted flags we have; 2) would be new stuff? We don't really have anything like that on x86, but I suppose if you make the thing optional and present a 'useful' use-case implemented in userspace code we could take it :-) Anyway, since PPC people thought it worth baking into hardware, presumably they have a compelling use case. Mikey could you see if you can retrieve that from someone in the know? It might be interesting. I don't think we can mispredict a non-conditional non-computed but I'll have to check with the HW folks. I was mostly wondering about the use-case for the conditional filter. Stephane didn't think it useful, clearly your hardware guys thought different :-) From my experience talking with compiler people, they care about ALL the branches and not the conditional so much. They use LBR to do basic block profiling. OK. I don't have a good handle on what's useful for compilers or JITs right now. I'm just plumbing through what's possible. Mikey -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Sat, May 18, 2013 at 12:14 AM, Michael Neuling mi...@neuling.org wrote: Stephane Eranian eran...@google.com wrote: On Fri, May 17, 2013 at 1:39 PM, Peter Zijlstra pet...@infradead.org wrote: On Fri, May 17, 2013 at 09:32:08PM +1000, Michael Neuling wrote: Peter Zijlstra pet...@infradead.org wrote: Wouldn't it be mostly conditional branches that are the primary control flow and can get predicted wrong? I mean, I'm sure someone will miss-predict an unconditional branch but its not like we care about people with such afflictions do we? You could mispredict the target address of a computed goto. You'd know it was taken but not know target address until later in the pipeline. Oh right, computed targets could indeed be mis predicted. I was more thinking about jumps with immediate values. On this, the POWER8 branch history buffer tells us two things about the prediction status. 1) if the branch was predicted taken/not taken correctly 2) if the target address was predicted correctly or not (for computed gotos only) So we'd actually like more prediction bits too :-D So if I understand this right, 1) maps to the predicted flags we have; 2) would be new stuff? We don't really have anything like that on x86, but I suppose if you make the thing optional and present a 'useful' use-case implemented in userspace code we could take it :-) Anyway, since PPC people thought it worth baking into hardware, presumably they have a compelling use case. Mikey could you see if you can retrieve that from someone in the know? It might be interesting. I don't think we can mispredict a non-conditional non-computed but I'll have to check with the HW folks. I was mostly wondering about the use-case for the conditional filter. Stephane didn't think it useful, clearly your hardware guys thought different :-) From my experience talking with compiler people, they care about ALL the branches and not the conditional so much. They use LBR to do basic block profiling. OK. I don't have a good handle on what's useful for compilers or JITs right now. I'm just plumbing through what's possible. I understand. It is okay to extend the interface. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Thu, May 16, 2013 at 1:16 PM, Peter Zijlstra wrote:
> On Thu, May 16, 2013 at 08:15:17PM +1000, Michael Neuling wrote:
>> Peter,
>>
>> BTW PowerPC also has the ability to filter on conditional branches. Any
>> chance we could add something like the follow to perf also?
>>
>
> I don't see an immediate problem with that except that we on x86 need to
> implement that in the software filter. Stephane do you see any
> fundamental issue with that?
>
On X86, the LBR cannot filter on conditional in HW. Thus as Peter said, it would
have to be done in SW. I did not add that because I think those branches are
not necessarily useful for tools.
>>
>> diff --git a/include/uapi/linux/perf_event.h
>> b/include/uapi/linux/perf_event.h
>> index fb104e5..891c769 100644
>> --- a/include/uapi/linux/perf_event.h
>> +++ b/include/uapi/linux/perf_event.h
>> @@ -157,8 +157,9 @@ enum perf_branch_sample_type {
>> PERF_SAMPLE_BRANCH_ANY_CALL = 1U << 4, /* any call branch */
>> PERF_SAMPLE_BRANCH_ANY_RETURN = 1U << 5, /* any return branch */
>> PERF_SAMPLE_BRANCH_IND_CALL = 1U << 6, /* indirect calls */
>> + PERF_SAMPLE_BRANCH_CONDITIONAL = 1U << 7, /* conditional branches */
>>
>> - PERF_SAMPLE_BRANCH_MAX = 1U << 7, /* non-ABI */
>> + PERF_SAMPLE_BRANCH_MAX = 1U << 8, /* non-ABI */
>> };
>>
>> #define PERF_SAMPLE_BRANCH_PLM_ALL \
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index cdf58ec..5b0b89d 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
>> BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL),
>> BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN),
>> BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL),
>> + BRANCH_OPT("cnd", PERF_SAMPLE_BRANCH_CONDITIONAL),
>> BRANCH_END
>> };
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Thu, May 16, 2013 at 08:15:17PM +1000, Michael Neuling wrote:
> Peter,
>
> BTW PowerPC also has the ability to filter on conditional branches. Any
> chance we could add something like the follow to perf also?
>
I don't see an immediate problem with that except that we on x86 need to
implement that in the software filter. Stephane do you see any
fundamental issue with that?
>
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index fb104e5..891c769 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -157,8 +157,9 @@ enum perf_branch_sample_type {
> PERF_SAMPLE_BRANCH_ANY_CALL = 1U << 4, /* any call branch */
> PERF_SAMPLE_BRANCH_ANY_RETURN = 1U << 5, /* any return branch */
> PERF_SAMPLE_BRANCH_IND_CALL = 1U << 6, /* indirect calls */
> + PERF_SAMPLE_BRANCH_CONDITIONAL = 1U << 7, /* conditional branches */
>
> - PERF_SAMPLE_BRANCH_MAX = 1U << 7, /* non-ABI */
> + PERF_SAMPLE_BRANCH_MAX = 1U << 8, /* non-ABI */
> };
>
> #define PERF_SAMPLE_BRANCH_PLM_ALL \
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index cdf58ec..5b0b89d 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
> BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL),
> BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN),
> BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL),
> + BRANCH_OPT("cnd", PERF_SAMPLE_BRANCH_CONDITIONAL),
> BRANCH_END
> };
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Peter Zijlstra wrote:
> On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
> > On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra
> > wrote:
> > > We should always have proper privileges when requesting kernel data.
> > >
> > > Cc: Andi Kleen
> > > Cc: eran...@google.com
> > > Signed-off-by: Peter Zijlstra
> > > Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
> > > ---
> > > arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > > +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > > @@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
> > > if (br_type & PERF_SAMPLE_BRANCH_USER)
> > > mask |= X86_BR_USER;
> > >
> > > - if (br_type & PERF_SAMPLE_BRANCH_KERNEL)
> > > + if (br_type & PERF_SAMPLE_BRANCH_KERNEL) {
> > > + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
> > > + return -EACCES;
> > > mask |= X86_BR_KERNEL;
> > > + }
> > >
> > This will prevent regular users from capturing kernel -> kernel branches.
> > But it won't prevent users from getting kernel -> user branches. Thus
> > some kernel address will still be captured. I guess they could be eliminated
> > by the sw_filter.
> >
> > When using LBR priv level filtering, the filter applies to the branch target
> > only.
>
> How about something like the below? It also adds the branch flags
> Mikey wanted for PowerPC.
Peter,
BTW PowerPC also has the ability to filter on conditional branches. Any
chance we could add something like the follow to perf also?
Mikey
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index fb104e5..891c769 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -157,8 +157,9 @@ enum perf_branch_sample_type {
PERF_SAMPLE_BRANCH_ANY_CALL = 1U << 4, /* any call branch */
PERF_SAMPLE_BRANCH_ANY_RETURN = 1U << 5, /* any return branch */
PERF_SAMPLE_BRANCH_IND_CALL = 1U << 6, /* indirect calls */
+ PERF_SAMPLE_BRANCH_CONDITIONAL = 1U << 7, /* conditional branches */
- PERF_SAMPLE_BRANCH_MAX = 1U << 7, /* non-ABI */
+ PERF_SAMPLE_BRANCH_MAX = 1U << 8, /* non-ABI */
};
#define PERF_SAMPLE_BRANCH_PLM_ALL \
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cdf58ec..5b0b89d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL),
BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN),
BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL),
+ BRANCH_OPT("cnd", PERF_SAMPLE_BRANCH_CONDITIONAL),
BRANCH_END
};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Peter Zijlstra wrote:
> On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
> > On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra
> > wrote:
> > > We should always have proper privileges when requesting kernel data.
> > >
> > > Cc: Andi Kleen
> > > Cc: eran...@google.com
> > > Signed-off-by: Peter Zijlstra
> > > Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
> > > ---
> > > arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > > +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > > @@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
> > > if (br_type & PERF_SAMPLE_BRANCH_USER)
> > > mask |= X86_BR_USER;
> > >
> > > - if (br_type & PERF_SAMPLE_BRANCH_KERNEL)
> > > + if (br_type & PERF_SAMPLE_BRANCH_KERNEL) {
> > > + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
> > > + return -EACCES;
> > > mask |= X86_BR_KERNEL;
> > > + }
> > >
> > This will prevent regular users from capturing kernel -> kernel branches.
> > But it won't prevent users from getting kernel -> user branches. Thus
> > some kernel address will still be captured. I guess they could be eliminated
> > by the sw_filter.
> >
> > When using LBR priv level filtering, the filter applies to the branch target
> > only.
>
> How about something like the below? It also adds the branch flags
> Mikey wanted for PowerPC.
>
> ---
> arch/x86/kernel/cpu/perf_event_intel_lbr.c | 12 +---
> include/linux/perf_event.h | 10 +++---
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> index d978353..f44d635 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> @@ -585,17 +585,23 @@ intel_pmu_lbr_filter(struct cpu_hw_events *cpuc)
>
> /* if type does not correspond, then discard */
> if (type == X86_BR_NONE || (br_sel & type) != type) {
> - cpuc->lbr_entries[i].from = 0;
> + cpuc->lbr_entries[i].__delete = 1;
> compress = true;
> }
> +
> + /* hide kernel addresses if we're not privileged */
> + if (!(br_sel & X86_BR_KERNEL) && kernel_ip(from)) {
> + cpuc->lbr_entries[i].from = -1L;
> + cpuc->lbr_entries[i].invalid_from = 1;
> + }
> }
>
> if (!compress)
> return;
>
> - /* remove all entries with from=0 */
> + /* remove all entries with __delete */
> for (i = 0; i < cpuc->lbr_stack.nr; ) {
> - if (!cpuc->lbr_entries[i].from) {
> + if (cpuc->lbr_entries[i].__delete) {
> j = i;
> while (++j < cpuc->lbr_stack.nr)
> cpuc->lbr_entries[j-1] = cpuc->lbr_entries[j];
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index f463a46..7acf1c9 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -77,9 +77,13 @@ struct perf_raw_record {
> struct perf_branch_entry {
> __u64 from;
> __u64 to;
> - __u64 mispred:1, /* target mispredicted */
> - predicted:1,/* target predicted */
> - reserved:62;
> + __u64 mispred:1, /* target mispredicted */
> + predicted:1,/* target predicted */
> + invalid_to:1, /* @to isn't to be trusted */
> + invalid_from:1, /* @from isn't to be trusted*/
Thanks Peter. One possible issue...
When the kernel has to read the branch from memory, there is no way for
it to know that it's the same one that the HW actually executed. Hence
there's a possibility that the to address is invalid but we can't tell
for sure.
I'm happy to just ignore that and mark calculated to address as valid,
unless you think it would be worthwhile extra information to pass onto
the user?
If we wanted this extra fidelity we could add a possibly_invalid_to:1
flag to your patch but I'm not sure it's worth it to be honest.
mikey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Thu, May 16, 2013 at 11:09:16AM +0200, Peter Zijlstra wrote:
> How about something like the below? It also adds the branch flags Mikey wanted
> for PowerPC.
The asymmetry is unfortunate, but I think its more useful to have the from
kernel branch target than it is not to have it. This way you at least know
there was a kernel entry/exit and where you've continued.
Without either branch to kernel and branch from kernel entries you'd be
wondering WTF happend to your control flow.
> ---
> arch/x86/kernel/cpu/perf_event_intel_lbr.c | 12 +---
> include/linux/perf_event.h | 10 +++---
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> index d978353..f44d635 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> @@ -585,17 +585,23 @@ intel_pmu_lbr_filter(struct cpu_hw_events *cpuc)
>
> /* if type does not correspond, then discard */
> if (type == X86_BR_NONE || (br_sel & type) != type) {
> - cpuc->lbr_entries[i].from = 0;
> + cpuc->lbr_entries[i].__delete = 1;
> compress = true;
> }
> +
> + /* hide kernel addresses if we're not privileged */
> + if (!(br_sel & X86_BR_KERNEL) && kernel_ip(from)) {
> + cpuc->lbr_entries[i].from = -1L;
> + cpuc->lbr_entries[i].invalid_from = 1;
> + }
> }
>
> if (!compress)
> return;
>
> - /* remove all entries with from=0 */
> + /* remove all entries with __delete */
> for (i = 0; i < cpuc->lbr_stack.nr; ) {
> - if (!cpuc->lbr_entries[i].from) {
> + if (cpuc->lbr_entries[i].__delete) {
> j = i;
> while (++j < cpuc->lbr_stack.nr)
> cpuc->lbr_entries[j-1] = cpuc->lbr_entries[j];
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index f463a46..7acf1c9 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -77,9 +77,13 @@ struct perf_raw_record {
> struct perf_branch_entry {
> __u64 from;
> __u64 to;
> - __u64 mispred:1, /* target mispredicted */
> - predicted:1,/* target predicted */
> - reserved:62;
> + __u64 mispred:1, /* target mispredicted */
> + predicted:1,/* target predicted */
> + invalid_to:1, /* @to isn't to be trusted */
> + invalid_from:1, /* @from isn't to be trusted*/
> + reserved:59,
> + __delete:1; /* Implementation; userspace should
> +always see a 0 */
> };
>
> /*
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
> On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra wrote:
> > We should always have proper privileges when requesting kernel data.
> >
> > Cc: Andi Kleen
> > Cc: eran...@google.com
> > Signed-off-by: Peter Zijlstra
> > Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
> > ---
> > arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > @@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
> > if (br_type & PERF_SAMPLE_BRANCH_USER)
> > mask |= X86_BR_USER;
> >
> > - if (br_type & PERF_SAMPLE_BRANCH_KERNEL)
> > + if (br_type & PERF_SAMPLE_BRANCH_KERNEL) {
> > + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
> > + return -EACCES;
> > mask |= X86_BR_KERNEL;
> > + }
> >
> This will prevent regular users from capturing kernel -> kernel branches.
> But it won't prevent users from getting kernel -> user branches. Thus
> some kernel address will still be captured. I guess they could be eliminated
> by the sw_filter.
>
> When using LBR priv level filtering, the filter applies to the branch target
> only.
How about something like the below? It also adds the branch flags Mikey wanted
for PowerPC.
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c | 12 +---
include/linux/perf_event.h | 10 +++---
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
index d978353..f44d635 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -585,17 +585,23 @@ intel_pmu_lbr_filter(struct cpu_hw_events *cpuc)
/* if type does not correspond, then discard */
if (type == X86_BR_NONE || (br_sel & type) != type) {
- cpuc->lbr_entries[i].from = 0;
+ cpuc->lbr_entries[i].__delete = 1;
compress = true;
}
+
+ /* hide kernel addresses if we're not privileged */
+ if (!(br_sel & X86_BR_KERNEL) && kernel_ip(from)) {
+ cpuc->lbr_entries[i].from = -1L;
+ cpuc->lbr_entries[i].invalid_from = 1;
+ }
}
if (!compress)
return;
- /* remove all entries with from=0 */
+ /* remove all entries with __delete */
for (i = 0; i < cpuc->lbr_stack.nr; ) {
- if (!cpuc->lbr_entries[i].from) {
+ if (cpuc->lbr_entries[i].__delete) {
j = i;
while (++j < cpuc->lbr_stack.nr)
cpuc->lbr_entries[j-1] = cpuc->lbr_entries[j];
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index f463a46..7acf1c9 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -77,9 +77,13 @@ struct perf_raw_record {
struct perf_branch_entry {
__u64 from;
__u64 to;
- __u64 mispred:1, /* target mispredicted */
- predicted:1,/* target predicted */
- reserved:62;
+ __u64 mispred:1, /* target mispredicted */
+ predicted:1,/* target predicted */
+ invalid_to:1, /* @to isn't to be trusted */
+ invalid_from:1, /* @from isn't to be trusted*/
+ reserved:59,
+ __delete:1; /* Implementation; userspace should
+ always see a 0 */
};
/*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra a.p.zijls...@chello.nl wrote:
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen a...@linux.intel.com
Cc: eran...@google.com
Signed-off-by: Peter Zijlstra a.p.zijls...@chello.nl
Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
This will prevent regular users from capturing kernel - kernel branches.
But it won't prevent users from getting kernel - user branches. Thus
some kernel address will still be captured. I guess they could be eliminated
by the sw_filter.
When using LBR priv level filtering, the filter applies to the branch target
only.
How about something like the below? It also adds the branch flags Mikey wanted
for PowerPC.
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c | 12 +---
include/linux/perf_event.h | 10 +++---
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
index d978353..f44d635 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -585,17 +585,23 @@ intel_pmu_lbr_filter(struct cpu_hw_events *cpuc)
/* if type does not correspond, then discard */
if (type == X86_BR_NONE || (br_sel type) != type) {
- cpuc-lbr_entries[i].from = 0;
+ cpuc-lbr_entries[i].__delete = 1;
compress = true;
}
+
+ /* hide kernel addresses if we're not privileged */
+ if (!(br_sel X86_BR_KERNEL) kernel_ip(from)) {
+ cpuc-lbr_entries[i].from = -1L;
+ cpuc-lbr_entries[i].invalid_from = 1;
+ }
}
if (!compress)
return;
- /* remove all entries with from=0 */
+ /* remove all entries with __delete */
for (i = 0; i cpuc-lbr_stack.nr; ) {
- if (!cpuc-lbr_entries[i].from) {
+ if (cpuc-lbr_entries[i].__delete) {
j = i;
while (++j cpuc-lbr_stack.nr)
cpuc-lbr_entries[j-1] = cpuc-lbr_entries[j];
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index f463a46..7acf1c9 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -77,9 +77,13 @@ struct perf_raw_record {
struct perf_branch_entry {
__u64 from;
__u64 to;
- __u64 mispred:1, /* target mispredicted */
- predicted:1,/* target predicted */
- reserved:62;
+ __u64 mispred:1, /* target mispredicted */
+ predicted:1,/* target predicted */
+ invalid_to:1, /* @to isn't to be trusted */
+ invalid_from:1, /* @from isn't to be trusted*/
+ reserved:59,
+ __delete:1; /* Implementation; userspace should
+ always see a 0 */
};
/*
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Thu, May 16, 2013 at 11:09:16AM +0200, Peter Zijlstra wrote:
How about something like the below? It also adds the branch flags Mikey wanted
for PowerPC.
The asymmetry is unfortunate, but I think its more useful to have the from
kernel branch target than it is not to have it. This way you at least know
there was a kernel entry/exit and where you've continued.
Without either branch to kernel and branch from kernel entries you'd be
wondering WTF happend to your control flow.
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c | 12 +---
include/linux/perf_event.h | 10 +++---
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
index d978353..f44d635 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -585,17 +585,23 @@ intel_pmu_lbr_filter(struct cpu_hw_events *cpuc)
/* if type does not correspond, then discard */
if (type == X86_BR_NONE || (br_sel type) != type) {
- cpuc-lbr_entries[i].from = 0;
+ cpuc-lbr_entries[i].__delete = 1;
compress = true;
}
+
+ /* hide kernel addresses if we're not privileged */
+ if (!(br_sel X86_BR_KERNEL) kernel_ip(from)) {
+ cpuc-lbr_entries[i].from = -1L;
+ cpuc-lbr_entries[i].invalid_from = 1;
+ }
}
if (!compress)
return;
- /* remove all entries with from=0 */
+ /* remove all entries with __delete */
for (i = 0; i cpuc-lbr_stack.nr; ) {
- if (!cpuc-lbr_entries[i].from) {
+ if (cpuc-lbr_entries[i].__delete) {
j = i;
while (++j cpuc-lbr_stack.nr)
cpuc-lbr_entries[j-1] = cpuc-lbr_entries[j];
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index f463a46..7acf1c9 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -77,9 +77,13 @@ struct perf_raw_record {
struct perf_branch_entry {
__u64 from;
__u64 to;
- __u64 mispred:1, /* target mispredicted */
- predicted:1,/* target predicted */
- reserved:62;
+ __u64 mispred:1, /* target mispredicted */
+ predicted:1,/* target predicted */
+ invalid_to:1, /* @to isn't to be trusted */
+ invalid_from:1, /* @from isn't to be trusted*/
+ reserved:59,
+ __delete:1; /* Implementation; userspace should
+always see a 0 */
};
/*
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Peter Zijlstra pet...@infradead.org wrote:
On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra a.p.zijls...@chello.nl
wrote:
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen a...@linux.intel.com
Cc: eran...@google.com
Signed-off-by: Peter Zijlstra a.p.zijls...@chello.nl
Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
This will prevent regular users from capturing kernel - kernel branches.
But it won't prevent users from getting kernel - user branches. Thus
some kernel address will still be captured. I guess they could be eliminated
by the sw_filter.
When using LBR priv level filtering, the filter applies to the branch target
only.
How about something like the below? It also adds the branch flags
Mikey wanted for PowerPC.
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c | 12 +---
include/linux/perf_event.h | 10 +++---
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
index d978353..f44d635 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -585,17 +585,23 @@ intel_pmu_lbr_filter(struct cpu_hw_events *cpuc)
/* if type does not correspond, then discard */
if (type == X86_BR_NONE || (br_sel type) != type) {
- cpuc-lbr_entries[i].from = 0;
+ cpuc-lbr_entries[i].__delete = 1;
compress = true;
}
+
+ /* hide kernel addresses if we're not privileged */
+ if (!(br_sel X86_BR_KERNEL) kernel_ip(from)) {
+ cpuc-lbr_entries[i].from = -1L;
+ cpuc-lbr_entries[i].invalid_from = 1;
+ }
}
if (!compress)
return;
- /* remove all entries with from=0 */
+ /* remove all entries with __delete */
for (i = 0; i cpuc-lbr_stack.nr; ) {
- if (!cpuc-lbr_entries[i].from) {
+ if (cpuc-lbr_entries[i].__delete) {
j = i;
while (++j cpuc-lbr_stack.nr)
cpuc-lbr_entries[j-1] = cpuc-lbr_entries[j];
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index f463a46..7acf1c9 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -77,9 +77,13 @@ struct perf_raw_record {
struct perf_branch_entry {
__u64 from;
__u64 to;
- __u64 mispred:1, /* target mispredicted */
- predicted:1,/* target predicted */
- reserved:62;
+ __u64 mispred:1, /* target mispredicted */
+ predicted:1,/* target predicted */
+ invalid_to:1, /* @to isn't to be trusted */
+ invalid_from:1, /* @from isn't to be trusted*/
Thanks Peter. One possible issue...
When the kernel has to read the branch from memory, there is no way for
it to know that it's the same one that the HW actually executed. Hence
there's a possibility that the to address is invalid but we can't tell
for sure.
I'm happy to just ignore that and mark calculated to address as valid,
unless you think it would be worthwhile extra information to pass onto
the user?
If we wanted this extra fidelity we could add a possibly_invalid_to:1
flag to your patch but I'm not sure it's worth it to be honest.
mikey
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
Peter Zijlstra pet...@infradead.org wrote:
On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra a.p.zijls...@chello.nl
wrote:
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen a...@linux.intel.com
Cc: eran...@google.com
Signed-off-by: Peter Zijlstra a.p.zijls...@chello.nl
Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
This will prevent regular users from capturing kernel - kernel branches.
But it won't prevent users from getting kernel - user branches. Thus
some kernel address will still be captured. I guess they could be eliminated
by the sw_filter.
When using LBR priv level filtering, the filter applies to the branch target
only.
How about something like the below? It also adds the branch flags
Mikey wanted for PowerPC.
Peter,
BTW PowerPC also has the ability to filter on conditional branches. Any
chance we could add something like the follow to perf also?
Mikey
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index fb104e5..891c769 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -157,8 +157,9 @@ enum perf_branch_sample_type {
PERF_SAMPLE_BRANCH_ANY_CALL = 1U 4, /* any call branch */
PERF_SAMPLE_BRANCH_ANY_RETURN = 1U 5, /* any return branch */
PERF_SAMPLE_BRANCH_IND_CALL = 1U 6, /* indirect calls */
+ PERF_SAMPLE_BRANCH_CONDITIONAL = 1U 7, /* conditional branches */
- PERF_SAMPLE_BRANCH_MAX = 1U 7, /* non-ABI */
+ PERF_SAMPLE_BRANCH_MAX = 1U 8, /* non-ABI */
};
#define PERF_SAMPLE_BRANCH_PLM_ALL \
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cdf58ec..5b0b89d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
BRANCH_OPT(any_call, PERF_SAMPLE_BRANCH_ANY_CALL),
BRANCH_OPT(any_ret, PERF_SAMPLE_BRANCH_ANY_RETURN),
BRANCH_OPT(ind_call, PERF_SAMPLE_BRANCH_IND_CALL),
+ BRANCH_OPT(cnd, PERF_SAMPLE_BRANCH_CONDITIONAL),
BRANCH_END
};
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Thu, May 16, 2013 at 08:15:17PM +1000, Michael Neuling wrote:
Peter,
BTW PowerPC also has the ability to filter on conditional branches. Any
chance we could add something like the follow to perf also?
I don't see an immediate problem with that except that we on x86 need to
implement that in the software filter. Stephane do you see any
fundamental issue with that?
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index fb104e5..891c769 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -157,8 +157,9 @@ enum perf_branch_sample_type {
PERF_SAMPLE_BRANCH_ANY_CALL = 1U 4, /* any call branch */
PERF_SAMPLE_BRANCH_ANY_RETURN = 1U 5, /* any return branch */
PERF_SAMPLE_BRANCH_IND_CALL = 1U 6, /* indirect calls */
+ PERF_SAMPLE_BRANCH_CONDITIONAL = 1U 7, /* conditional branches */
- PERF_SAMPLE_BRANCH_MAX = 1U 7, /* non-ABI */
+ PERF_SAMPLE_BRANCH_MAX = 1U 8, /* non-ABI */
};
#define PERF_SAMPLE_BRANCH_PLM_ALL \
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cdf58ec..5b0b89d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
BRANCH_OPT(any_call, PERF_SAMPLE_BRANCH_ANY_CALL),
BRANCH_OPT(any_ret, PERF_SAMPLE_BRANCH_ANY_RETURN),
BRANCH_OPT(ind_call, PERF_SAMPLE_BRANCH_IND_CALL),
+ BRANCH_OPT(cnd, PERF_SAMPLE_BRANCH_CONDITIONAL),
BRANCH_END
};
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Thu, May 16, 2013 at 1:16 PM, Peter Zijlstra pet...@infradead.org wrote:
On Thu, May 16, 2013 at 08:15:17PM +1000, Michael Neuling wrote:
Peter,
BTW PowerPC also has the ability to filter on conditional branches. Any
chance we could add something like the follow to perf also?
I don't see an immediate problem with that except that we on x86 need to
implement that in the software filter. Stephane do you see any
fundamental issue with that?
On X86, the LBR cannot filter on conditional in HW. Thus as Peter said, it would
have to be done in SW. I did not add that because I think those branches are
not necessarily useful for tools.
diff --git a/include/uapi/linux/perf_event.h
b/include/uapi/linux/perf_event.h
index fb104e5..891c769 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -157,8 +157,9 @@ enum perf_branch_sample_type {
PERF_SAMPLE_BRANCH_ANY_CALL = 1U 4, /* any call branch */
PERF_SAMPLE_BRANCH_ANY_RETURN = 1U 5, /* any return branch */
PERF_SAMPLE_BRANCH_IND_CALL = 1U 6, /* indirect calls */
+ PERF_SAMPLE_BRANCH_CONDITIONAL = 1U 7, /* conditional branches */
- PERF_SAMPLE_BRANCH_MAX = 1U 7, /* non-ABI */
+ PERF_SAMPLE_BRANCH_MAX = 1U 8, /* non-ABI */
};
#define PERF_SAMPLE_BRANCH_PLM_ALL \
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cdf58ec..5b0b89d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -676,6 +676,7 @@ static const struct branch_mode branch_modes[] = {
BRANCH_OPT(any_call, PERF_SAMPLE_BRANCH_ANY_CALL),
BRANCH_OPT(any_ret, PERF_SAMPLE_BRANCH_ANY_RETURN),
BRANCH_OPT(ind_call, PERF_SAMPLE_BRANCH_IND_CALL),
+ BRANCH_OPT(cnd, PERF_SAMPLE_BRANCH_CONDITIONAL),
BRANCH_END
};
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
> On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra wrote:
> > We should always have proper privileges when requesting kernel data.
> >
> > Cc: Andi Kleen
> > Cc: eran...@google.com
> > Signed-off-by: Peter Zijlstra
> > Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
> > ---
> > arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> > @@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
> > if (br_type & PERF_SAMPLE_BRANCH_USER)
> > mask |= X86_BR_USER;
> >
> > - if (br_type & PERF_SAMPLE_BRANCH_KERNEL)
> > + if (br_type & PERF_SAMPLE_BRANCH_KERNEL) {
> > + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
> > + return -EACCES;
> > mask |= X86_BR_KERNEL;
> > + }
> >
> This will prevent regular users from capturing kernel -> kernel branches.
> But it won't prevent users from getting kernel -> user branches. Thus
> some kernel address will still be captured. I guess they could be eliminated
> by the sw_filter.
>
> When using LBR priv level filtering, the filter applies to the branch target
> only.
Ah, indeed. I'll try and whip up a patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra wrote:
> We should always have proper privileges when requesting kernel data.
>
> Cc: Andi Kleen
> Cc: eran...@google.com
> Signed-off-by: Peter Zijlstra
> Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
> ---
> arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> @@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
> if (br_type & PERF_SAMPLE_BRANCH_USER)
> mask |= X86_BR_USER;
>
> - if (br_type & PERF_SAMPLE_BRANCH_KERNEL)
> + if (br_type & PERF_SAMPLE_BRANCH_KERNEL) {
> + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
> + return -EACCES;
> mask |= X86_BR_KERNEL;
> + }
>
This will prevent regular users from capturing kernel -> kernel branches.
But it won't prevent users from getting kernel -> user branches. Thus
some kernel address will still be captured. I guess they could be eliminated
by the sw_filter.
When using LBR priv level filtering, the filter applies to the branch target
only.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra a.p.zijls...@chello.nl wrote:
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen a...@linux.intel.com
Cc: eran...@google.com
Signed-off-by: Peter Zijlstra a.p.zijls...@chello.nl
Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
This will prevent regular users from capturing kernel - kernel branches.
But it won't prevent users from getting kernel - user branches. Thus
some kernel address will still be captured. I guess they could be eliminated
by the sw_filter.
When using LBR priv level filtering, the filter applies to the branch target
only.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Wed, May 15, 2013 at 03:37:22PM +0200, Stephane Eranian wrote:
On Fri, May 3, 2013 at 2:11 PM, Peter Zijlstra a.p.zijls...@chello.nl wrote:
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen a...@linux.intel.com
Cc: eran...@google.com
Signed-off-by: Peter Zijlstra a.p.zijls...@chello.nl
Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
This will prevent regular users from capturing kernel - kernel branches.
But it won't prevent users from getting kernel - user branches. Thus
some kernel address will still be captured. I guess they could be eliminated
by the sw_filter.
When using LBR priv level filtering, the filter applies to the branch target
only.
Ah, indeed. I'll try and whip up a patch.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Fri, May 03, 2013 at 02:11:25PM +0200, Peter Zijlstra wrote:
> We should always have proper privileges when requesting kernel data.
>
> Cc: Andi Kleen
Acked-by: Andi Kleen
> --- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
> @@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
> if (br_type & PERF_SAMPLE_BRANCH_USER)
> mask |= X86_BR_USER;
>
> - if (br_type & PERF_SAMPLE_BRANCH_KERNEL)
> + if (br_type & PERF_SAMPLE_BRANCH_KERNEL) {
> + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
> + return -EACCES;
> mask |= X86_BR_KERNEL;
> + }
>
> /* we ignore BRANCH_HV here */
>
>
>
--
a...@linux.intel.com -- Speaking for myself only
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen
Cc: eran...@google.com
Signed-off-by: Peter Zijlstra
Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type & PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type & PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type & PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
/* we ignore BRANCH_HV here */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen a...@linux.intel.com
Cc: eran...@google.com
Signed-off-by: Peter Zijlstra a.p.zijls...@chello.nl
Link: http://lkml.kernel.org/n/tip-v0x9ky3ahzr6nm3c6ilwr...@git.kernel.org
---
arch/x86/kernel/cpu/perf_event_intel_lbr.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
/* we ignore BRANCH_HV here */
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] perf, x86, lbr: Demand proper privileges for PERF_SAMPLE_BRANCH_KERNEL
On Fri, May 03, 2013 at 02:11:25PM +0200, Peter Zijlstra wrote:
We should always have proper privileges when requesting kernel data.
Cc: Andi Kleen a...@linux.intel.com
Acked-by: Andi Kleen a...@linux.intel.com
--- a/arch/x86/kernel/cpu/perf_event_intel_lbr.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_lbr.c
@@ -318,8 +318,11 @@ static void intel_pmu_setup_sw_lbr_filte
if (br_type PERF_SAMPLE_BRANCH_USER)
mask |= X86_BR_USER;
- if (br_type PERF_SAMPLE_BRANCH_KERNEL)
+ if (br_type PERF_SAMPLE_BRANCH_KERNEL) {
+ if (perf_paranoid_kernel() !capable(CAP_SYS_ADMIN))
+ return -EACCES;
mask |= X86_BR_KERNEL;
+ }
/* we ignore BRANCH_HV here */
--
a...@linux.intel.com -- Speaking for myself only
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
