All of lore.kernel.org
 help / color / mirror / Atom feed
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [UPDATED] [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver
Date: Thu, 19 Mar 2015 17:25:31 +0000	[thread overview]
Message-ID: <20150319172531.GB10153@leverpostej> (raw)
In-Reply-To: <1426585901-19137-1-git-send-email-suzuki.poulose@arm.com>

On Tue, Mar 17, 2015 at 09:51:41AM +0000, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> A minor change, fixed missplled 'DEPRECATED' in the dev_warn().
> 
> Thanks
> Suzuki
> 
> ----8>----
> Avoid secure transactions while probing the CCI PMU. The
> existing code makes use of the Peripheral ID2 (PID2) register
> to determine the revision of the CCI400, which requires a
> secure transaction. This puts a limitation on the usage of the
> driver on systems running non-secure Linux(e.g, ARM64).
> 
> Updated the device-tree binding for cci pmu node to add the explicit
> revision number for the compatible field.
> 
> The supported strings are :
> 	arm,cci-400-pmu,r0
> 	arm,cci-400-pmu,r1
> 	arm,cci-400-pmu - DEPRECATED. See NOTE below
> 
> NOTE: If the revision is not mentioned, we need to probe the cci revision,
> which could be fatal on a platform running non-secure. We need a reliable way
> to know if we can poke the CCI registers at runtime on ARM32. We depend on
> 'mcpm_is_available()' when it is available. mcpm_is_available() returns true
> only when there is a registered driver for mcpm. Otherwise, we assume that we
> don't have secure access, and skips probing the revision number(ARM64 case).
> 
> The MCPM should figure out if it is safe to access the CCI. Unfortunately
> there isn't a reliable way to indicate the same via dtb. This patch doesn't
> address/change the current situation. It only deals with the CCI-PMU, leaving
> the assumptions about the secure access as it has been, prior to this patch.
> 
> Changes since V2:
>  - Use 'bool' instead of 'int' for platform_has_secure_cci_access().
>    (Suggested-by: Sudeep Holla)
> 
> Cc: devicetree at vger.kernel.org
> Cc: Punit Agrawal <punit.agrawal@arm.com>
> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> ---
>  Documentation/devicetree/bindings/arm/cci.txt |    7 +++--
>  arch/arm/include/asm/arm-cci.h                |   42 +++++++++++++++++++++++++
>  arch/arm64/include/asm/arm-cci.h              |   27 ++++++++++++++++
>  drivers/bus/arm-cci.c                         |   17 +++++++++-
>  include/linux/arm-cci.h                       |    2 ++
>  5 files changed, 92 insertions(+), 3 deletions(-)
>  create mode 100644 arch/arm/include/asm/arm-cci.h
>  create mode 100644 arch/arm64/include/asm/arm-cci.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/cci.txt b/Documentation/devicetree/bindings/arm/cci.txt
> index f28d82b..0e4b6a7 100644
> --- a/Documentation/devicetree/bindings/arm/cci.txt
> +++ b/Documentation/devicetree/bindings/arm/cci.txt
> @@ -94,8 +94,11 @@ specific to ARM.
>  		- compatible
>  			Usage: required
>  			Value type: <string>
> -			Definition: must be "arm,cci-400-pmu"
> -
> +			Definition: Supported strings are :

I'd prefer we said "Must contain one of:".

> +				 "arm,cci-400-pmu,r0"
> +				 "arm,cci-400-pmu,r1"
> +				 "arm,cci-400-pmu"  - DEPRECATED, permitted only where OS has
> +						      secure acces to CCI registers

Otherwise, the binding change looks fine to me, so:

Acked-by: Mark Rutland <mark.rutland@arm.com> [DT binding]

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
Cc: "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Nicolas Pitre <nico@linaro.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Kukjin Kim <kgene@kernel.org>,
	Abhilash Kesavan <a.kesavan@samsung.com>,
	Arnd Bergmann <arnd@arndb.de>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Liviu Dudau <Liviu.Dudau@arm.com>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
	Olof Johansson <olof@lixom.net>, Pawel Moll <Pawel.Moll@arm.com>,
	Punit Agrawal <Punit.Agrawal@arm.com>,
	Sudeep Holla <Sudeep.Holla@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	Catalin Marinas <Catalin.Marinas@arm.com>
Subject: Re: [UPDATED] [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver
Date: Thu, 19 Mar 2015 17:25:31 +0000	[thread overview]
Message-ID: <20150319172531.GB10153@leverpostej> (raw)
In-Reply-To: <1426585901-19137-1-git-send-email-suzuki.poulose@arm.com>

On Tue, Mar 17, 2015 at 09:51:41AM +0000, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> A minor change, fixed missplled 'DEPRECATED' in the dev_warn().
> 
> Thanks
> Suzuki
> 
> ----8>----
> Avoid secure transactions while probing the CCI PMU. The
> existing code makes use of the Peripheral ID2 (PID2) register
> to determine the revision of the CCI400, which requires a
> secure transaction. This puts a limitation on the usage of the
> driver on systems running non-secure Linux(e.g, ARM64).
> 
> Updated the device-tree binding for cci pmu node to add the explicit
> revision number for the compatible field.
> 
> The supported strings are :
> 	arm,cci-400-pmu,r0
> 	arm,cci-400-pmu,r1
> 	arm,cci-400-pmu - DEPRECATED. See NOTE below
> 
> NOTE: If the revision is not mentioned, we need to probe the cci revision,
> which could be fatal on a platform running non-secure. We need a reliable way
> to know if we can poke the CCI registers at runtime on ARM32. We depend on
> 'mcpm_is_available()' when it is available. mcpm_is_available() returns true
> only when there is a registered driver for mcpm. Otherwise, we assume that we
> don't have secure access, and skips probing the revision number(ARM64 case).
> 
> The MCPM should figure out if it is safe to access the CCI. Unfortunately
> there isn't a reliable way to indicate the same via dtb. This patch doesn't
> address/change the current situation. It only deals with the CCI-PMU, leaving
> the assumptions about the secure access as it has been, prior to this patch.
> 
> Changes since V2:
>  - Use 'bool' instead of 'int' for platform_has_secure_cci_access().
>    (Suggested-by: Sudeep Holla)
> 
> Cc: devicetree@vger.kernel.org
> Cc: Punit Agrawal <punit.agrawal@arm.com>
> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> ---
>  Documentation/devicetree/bindings/arm/cci.txt |    7 +++--
>  arch/arm/include/asm/arm-cci.h                |   42 +++++++++++++++++++++++++
>  arch/arm64/include/asm/arm-cci.h              |   27 ++++++++++++++++
>  drivers/bus/arm-cci.c                         |   17 +++++++++-
>  include/linux/arm-cci.h                       |    2 ++
>  5 files changed, 92 insertions(+), 3 deletions(-)
>  create mode 100644 arch/arm/include/asm/arm-cci.h
>  create mode 100644 arch/arm64/include/asm/arm-cci.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/cci.txt b/Documentation/devicetree/bindings/arm/cci.txt
> index f28d82b..0e4b6a7 100644
> --- a/Documentation/devicetree/bindings/arm/cci.txt
> +++ b/Documentation/devicetree/bindings/arm/cci.txt
> @@ -94,8 +94,11 @@ specific to ARM.
>  		- compatible
>  			Usage: required
>  			Value type: <string>
> -			Definition: must be "arm,cci-400-pmu"
> -
> +			Definition: Supported strings are :

I'd prefer we said "Must contain one of:".

> +				 "arm,cci-400-pmu,r0"
> +				 "arm,cci-400-pmu,r1"
> +				 "arm,cci-400-pmu"  - DEPRECATED, permitted only where OS has
> +						      secure acces to CCI registers

Otherwise, the binding change looks fine to me, so:

Acked-by: Mark Rutland <mark.rutland@arm.com> [DT binding]

  reply	other threads:[~2015-03-19 17:25 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-10 15:18 [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Suzuki K. Poulose
2015-03-10 15:18 ` Suzuki K. Poulose
2015-03-10 15:18 ` Suzuki K. Poulose
2015-03-10 15:18 ` [PATCH 1/5] arm-cci: Rearrange code for splitting PMU vs driver code Suzuki K. Poulose
2015-03-10 15:18   ` Suzuki K. Poulose
2015-03-10 15:18 ` [PATCH 2/5] arm-cci: Abstract the CCI400 PMU speicific definitions Suzuki K. Poulose
2015-03-10 15:18   ` Suzuki K. Poulose
2015-03-10 15:18   ` Suzuki K. Poulose
2015-03-17 18:49   ` Will Deacon
2015-03-17 18:49     ` Will Deacon
2015-03-17 18:49     ` Will Deacon
2015-03-10 15:18 ` [PATCH 3/5] arm-cci: Get rid of secure transactions for PMU driver Suzuki K. Poulose
2015-03-10 15:18   ` Suzuki K. Poulose
2015-03-10 15:18   ` Suzuki K. Poulose
2015-03-17  9:51   ` [UPDATED] " Suzuki K. Poulose
2015-03-17  9:51     ` Suzuki K. Poulose
2015-03-19 17:25     ` Mark Rutland [this message]
2015-03-19 17:25       ` Mark Rutland
2015-03-19 17:32     ` Mark Rutland
2015-03-19 17:32       ` Mark Rutland
2015-03-19 17:32       ` Mark Rutland
2015-03-19 17:38       ` Sudeep Holla
2015-03-19 17:38         ` Sudeep Holla
2015-03-19 17:52         ` Suzuki K. Poulose
2015-03-19 17:52           ` Suzuki K. Poulose
2015-03-19 17:54           ` Mark Rutland
2015-03-19 17:54             ` Mark Rutland
2015-03-19 17:54             ` Mark Rutland
2015-03-10 15:18 ` [PATCH 4/5] arm-cci: Split the code for PMU vs driver support Suzuki K. Poulose
2015-03-10 15:18   ` Suzuki K. Poulose
2015-03-10 16:24   ` Sudeep Holla
2015-03-10 16:24     ` Sudeep Holla
2015-03-10 16:24     ` Sudeep Holla
2015-03-10 15:18 ` [PATCH 5/5] arm-cci: Fix CCI PMU event validation Suzuki K. Poulose
2015-03-10 15:18   ` Suzuki K. Poulose
2015-03-17 18:52   ` Will Deacon
2015-03-17 18:52     ` Will Deacon
2015-03-17 18:52     ` Will Deacon
2015-03-10 16:09 ` [PATCHv3 0/5] arm-cci400: PMU monitoring support on ARM64 Nicolas Pitre
2015-03-10 16:09   ` Nicolas Pitre
2015-03-10 16:09   ` Nicolas Pitre
2015-03-10 16:11   ` Suzuki K. Poulose
2015-03-10 16:11     ` Suzuki K. Poulose
2015-03-10 16:21 ` Sudeep Holla
2015-03-10 16:21   ` Sudeep Holla
2015-03-10 16:24   ` Suzuki K. Poulose
2015-03-10 16:24     ` Suzuki K. Poulose
2015-03-11 11:40 ` Punit Agrawal
2015-03-11 11:40   ` Punit Agrawal
2015-03-11 11:40   ` Punit Agrawal
2015-03-17 18:54 ` Will Deacon
2015-03-17 18:54   ` Will Deacon
2015-03-18 10:09   ` Suzuki K. Poulose
2015-03-18 10:09     ` Suzuki K. Poulose

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150319172531.GB10153@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.