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

Groups > linux.kernel > #1334779 > unrolled thread

Re: [PATCH v3 5/5] arm64/perf: Extend event mask for ARMv8.1

Back to article view | Back to linux.kernel

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

Contents

  Re: [PATCH v3 5/5] arm64/perf: Extend event mask for ARMv8.1 Will Deacon <will.deacon@arm.com> - 2016-02-15 21:10 +0100
    Re: [PATCH v3 5/5] arm64/perf: Extend event mask for ARMv8.1 Jan Glauber <jan.glauber@caviumnetworks.com> - 2016-02-16 09:20 +0100
      Re: [PATCH v3 5/5] arm64/perf: Extend event mask for ARMv8.1 Will Deacon <will.deacon@arm.com> - 2016-02-16 16:20 +0100
        Re: [PATCH v3 5/5] arm64/perf: Extend event mask for ARMv8.1 Jan Glauber <jan.glauber@caviumnetworks.com> - 2016-02-17 11:50 +0100

#1334779 — Re: [PATCH v3 5/5] arm64/perf: Extend event mask for ARMv8.1

On Wed, Feb 03, 2016 at 06:12:00PM +0100, Jan Glauber wrote:
> ARMv8.1 increases the PMU event number space. Detect the
> presence of this PMUv3 type and extend the event mask.
> 
> The event mask is moved to struct arm_pmu so different event masks
> can exist, depending on the PMU type.
> 
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> ---
>  arch/arm/kernel/perf_event_v6.c     |  6 ++++--
>  arch/arm/kernel/perf_event_v7.c     | 29 +++++++++++++++++++----------
>  arch/arm/kernel/perf_event_xscale.c |  4 +++-
>  arch/arm64/kernel/perf_event.c      | 33 +++++++++++++++++++--------------
>  drivers/perf/arm_pmu.c              |  5 +++--
>  include/linux/perf/arm_pmu.h        |  4 ++--
>  6 files changed, 50 insertions(+), 31 deletions(-)

[...]

>  static void armv8_pmu_init(struct arm_pmu *cpu_pmu)
>  {
> +	u64 id;
> +
>  	cpu_pmu->handle_irq		= armv8pmu_handle_irq,
>  	cpu_pmu->enable			= armv8pmu_enable_event,
>  	cpu_pmu->disable		= armv8pmu_disable_event,
> @@ -842,6 +840,13 @@ static void armv8_pmu_init(struct arm_pmu *cpu_pmu)
>  	cpu_pmu->reset			= armv8pmu_reset,
>  	cpu_pmu->max_period		= (1LLU << 32) - 1,
>  	cpu_pmu->set_event_filter	= armv8pmu_set_event_filter;
> +
> +	/* detect ARMv8.1 PMUv3 with extended event mask */
> +	id = read_cpuid(ID_AA64DFR0_EL1);
> +	if (((id >> 8) & 0xf) == 4)

We have helpers for this stuff (cpuid_feature_extract_field)...

> +		cpu_pmu->event_mask = 0xffff;	/* ARMv8.1 extended events */
> +	else
> +		cpu_pmu->event_mask = ARMV8_EVTYPE_EVENT;

... although can't we just update ARMV8_EVTYPE_EVENT to be 0xffff now?
AFAICT, that just eats into bits that used to be RES0, so we shouldn't
see any problems. That should make your patch *much* simpler!

Will

[toc] | [next] | [standalone]

#1335131

On Mon, Feb 15, 2016 at 08:04:04PM +0000, Will Deacon wrote:

[...]

> On Wed, Feb 03, 2016 at 06:12:00PM +0100, Jan Glauber wrote:
> > +		cpu_pmu->event_mask = 0xffff;	/* ARMv8.1 extended events */
> > +	else
> > +		cpu_pmu->event_mask = ARMV8_EVTYPE_EVENT;
> 
> ... although can't we just update ARMV8_EVTYPE_EVENT to be 0xffff now?
> AFAICT, that just eats into bits that used to be RES0, so we shouldn't
> see any problems. That should make your patch *much* simpler!

That would of course be easier, but I just can't assess the implications.

Probably I'm missing something but to me it looks like the event mask is the
only verification we do for the user-space selectable events. Is it safe for
implementations that only support 0x3ff events to allow access to the
whole 0xffff range? What memory would be accessed for non-existing
events?

Jan

> Will

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

#1335504

On Tue, Feb 16, 2016 at 09:00:15AM +0100, Jan Glauber wrote:
> On Mon, Feb 15, 2016 at 08:04:04PM +0000, Will Deacon wrote:
> 
> [...]
> 
> > On Wed, Feb 03, 2016 at 06:12:00PM +0100, Jan Glauber wrote:
> > > +		cpu_pmu->event_mask = 0xffff;	/* ARMv8.1 extended events */
> > > +	else
> > > +		cpu_pmu->event_mask = ARMV8_EVTYPE_EVENT;
> > 
> > ... although can't we just update ARMV8_EVTYPE_EVENT to be 0xffff now?
> > AFAICT, that just eats into bits that used to be RES0, so we shouldn't
> > see any problems. That should make your patch *much* simpler!
> 
> That would of course be easier, but I just can't assess the implications.
> 
> Probably I'm missing something but to me it looks like the event mask is the
> only verification we do for the user-space selectable events. Is it safe for
> implementations that only support 0x3ff events to allow access to the
> whole 0xffff range? What memory would be accessed for non-existing
> events?

Which memory? The worst-case is that we end up writing to some bits in
a register (e.g. PMXEVTYPER) that are RES0 in ARMv8 afaict.

Will

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

#1336242

On Tue, Feb 16, 2016 at 03:12:53PM +0000, Will Deacon wrote:
> On Tue, Feb 16, 2016 at 09:00:15AM +0100, Jan Glauber wrote:
> > On Mon, Feb 15, 2016 at 08:04:04PM +0000, Will Deacon wrote:
> > 
> > [...]
> > 
> > > On Wed, Feb 03, 2016 at 06:12:00PM +0100, Jan Glauber wrote:
> > > > +		cpu_pmu->event_mask = 0xffff;	/* ARMv8.1 extended events */
> > > > +	else
> > > > +		cpu_pmu->event_mask = ARMV8_EVTYPE_EVENT;
> > > 
> > > ... although can't we just update ARMV8_EVTYPE_EVENT to be 0xffff now?
> > > AFAICT, that just eats into bits that used to be RES0, so we shouldn't
> > > see any problems. That should make your patch *much* simpler!
> > 
> > That would of course be easier, but I just can't assess the implications.
> > 
> > Probably I'm missing something but to me it looks like the event mask is the
> > only verification we do for the user-space selectable events. Is it safe for
> > implementations that only support 0x3ff events to allow access to the
> > whole 0xffff range? What memory would be accessed for non-existing
> > events?
> 
> Which memory? The worst-case is that we end up writing to some bits in
> a register (e.g. PMXEVTYPER) that are RES0 in ARMv8 afaict.

OK, I see. Than I'm happy to drop 99% of that patch and just increase
the mask.

Jan

> Will

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web