diff mbox series

[v2,18/18] perf: arm_spe: Disallow userspace profiling when arm_kernel_unmapped_at_el0()

Message ID 1512059986-21325-19-git-send-email-will.deacon@arm.com
State New
Headers show
Series arm64: Unmap the kernel whilst running in userspace (KAISER) | expand

Commit Message

Will Deacon Nov. 30, 2017, 4:39 p.m. UTC
When running with the kernel unmapped whilst at EL0, the virtually-addressed
SPE buffer is also unmapped, which can lead to buffer faults if userspace
profiling is enabled.

This patch prohibits SPE profiling of userspace when
arm_kernel_unmapped_at_el0().

Signed-off-by: Will Deacon <will.deacon@arm.com>

---
 drivers/perf/arm_spe_pmu.c | 7 +++++++
 1 file changed, 7 insertions(+)

-- 
2.1.4

Comments

Mark Rutland Dec. 1, 2017, 12:15 p.m. UTC | #1
On Thu, Nov 30, 2017 at 04:39:46PM +0000, Will Deacon wrote:
> When running with the kernel unmapped whilst at EL0, the virtually-addressed

> SPE buffer is also unmapped, which can lead to buffer faults if userspace

> profiling is enabled.

> 

> This patch prohibits SPE profiling of userspace when

> arm_kernel_unmapped_at_el0().

> 

> Signed-off-by: Will Deacon <will.deacon@arm.com>

> ---

>  drivers/perf/arm_spe_pmu.c | 7 +++++++

>  1 file changed, 7 insertions(+)

> 

> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c

> index 8ce262fc2561..c028db8973a4 100644

> --- a/drivers/perf/arm_spe_pmu.c

> +++ b/drivers/perf/arm_spe_pmu.c

> @@ -675,6 +675,13 @@ static int arm_spe_pmu_event_init(struct perf_event *event)

>  		return -EOPNOTSUPP;

>  

>  	/*

> +	 * If kernelspace is unmapped when running at EL0, then the SPE

> +	 * buffer will fault and prematurely terminate the AUX session.

> +	 */

> +	if (arm64_kernel_unmapped_at_el0() && !attr->exclude_user)

> +		dev_warn_once(&spe_pmu->pdev->dev, "unable to write to profiling buffer from EL0. Try passing \"kaiser=off\" on the kernel command line");


The commit messages sats this prohibits profiling, but we simply log a
message.

I take it you meant to return an error code, too?

Thanks,
Mark.
Stephen Boyd Dec. 1, 2017, 4:26 p.m. UTC | #2
On 11/30, Will Deacon wrote:
> When running with the kernel unmapped whilst at EL0, the virtually-addressed

> SPE buffer is also unmapped, which can lead to buffer faults if userspace

> profiling is enabled.

> 

> This patch prohibits SPE profiling of userspace when

> arm_kernel_unmapped_at_el0().

> 

> Signed-off-by: Will Deacon <will.deacon@arm.com>

> ---

>  drivers/perf/arm_spe_pmu.c | 7 +++++++

>  1 file changed, 7 insertions(+)

> 

> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c

> index 8ce262fc2561..c028db8973a4 100644

> --- a/drivers/perf/arm_spe_pmu.c

> +++ b/drivers/perf/arm_spe_pmu.c

> @@ -675,6 +675,13 @@ static int arm_spe_pmu_event_init(struct perf_event *event)

>  		return -EOPNOTSUPP;

>  

>  	/*

> +	 * If kernelspace is unmapped when running at EL0, then the SPE

> +	 * buffer will fault and prematurely terminate the AUX session.

> +	 */

> +	if (arm64_kernel_unmapped_at_el0() && !attr->exclude_user)

> +		dev_warn_once(&spe_pmu->pdev->dev, "unable to write to profiling buffer from EL0. Try passing \"kaiser=off\" on the kernel command line");


Missing newline on that print?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Will Deacon Dec. 1, 2017, 4:49 p.m. UTC | #3
On Fri, Dec 01, 2017 at 12:15:06PM +0000, Mark Rutland wrote:
> On Thu, Nov 30, 2017 at 04:39:46PM +0000, Will Deacon wrote:

> > When running with the kernel unmapped whilst at EL0, the virtually-addressed

> > SPE buffer is also unmapped, which can lead to buffer faults if userspace

> > profiling is enabled.

> > 

> > This patch prohibits SPE profiling of userspace when

> > arm_kernel_unmapped_at_el0().

> > 

> > Signed-off-by: Will Deacon <will.deacon@arm.com>

> > ---

> >  drivers/perf/arm_spe_pmu.c | 7 +++++++

> >  1 file changed, 7 insertions(+)

> > 

> > diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c

> > index 8ce262fc2561..c028db8973a4 100644

> > --- a/drivers/perf/arm_spe_pmu.c

> > +++ b/drivers/perf/arm_spe_pmu.c

> > @@ -675,6 +675,13 @@ static int arm_spe_pmu_event_init(struct perf_event *event)

> >  		return -EOPNOTSUPP;

> >  

> >  	/*

> > +	 * If kernelspace is unmapped when running at EL0, then the SPE

> > +	 * buffer will fault and prematurely terminate the AUX session.

> > +	 */

> > +	if (arm64_kernel_unmapped_at_el0() && !attr->exclude_user)

> > +		dev_warn_once(&spe_pmu->pdev->dev, "unable to write to profiling buffer from EL0. Try passing \"kaiser=off\" on the kernel command line");

> 

> The commit messages sats this prohibits profiling, but we simply log a

> message.

> 

> I take it you meant to return an error code, too?


To be honest with you, I've been changing my mind a lot about what to do
here and the code has ended up being a bit of a mess after I've butchered
it repeatedly.

The fact remains that there aren't any SPE-capable CPUs shipping at the
moment, so I'm inclined just to fail the probe for now and we can look
at whether or not we can do better when we've got some hardware to play
with.

And I'll add the missing newline.

Thanks,

Will
diff mbox series

Patch

diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index 8ce262fc2561..c028db8973a4 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -675,6 +675,13 @@  static int arm_spe_pmu_event_init(struct perf_event *event)
 		return -EOPNOTSUPP;
 
 	/*
+	 * If kernelspace is unmapped when running at EL0, then the SPE
+	 * buffer will fault and prematurely terminate the AUX session.
+	 */
+	if (arm64_kernel_unmapped_at_el0() && !attr->exclude_user)
+		dev_warn_once(&spe_pmu->pdev->dev, "unable to write to profiling buffer from EL0. Try passing \"kaiser=off\" on the kernel command line");
+
+	/*
 	 * Feedback-directed frequency throttling doesn't work when we
 	 * have a buffer of samples. We'd need to manually count the
 	 * samples in the buffer when it fills up and adjust the event