Re: [PATCH 2/4] arm-cci: Get rid of secure transactions for PMU driver

2015-02-25 Thread Suzuki K. Poulose 

On 24/02/15 21:53, Nicolas Pitre wrote:

On Tue, 24 Feb 2015, Suzuki K. Poulose wrote:


From: "Suzuki K. Poulose" 

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.


This is an extensive commit log about secure access issues which is nice
and appreciated.  However the patch does quite some more code reorg not
mentioned here.  Could you please move this code reorg to a separate
patch and then have a patch on top introducing these probing changes?
This should make the implication of what is said above clearer.


Sure, I will do that in the next revision. What I missed in the commit
follows (which will be added in the next version):

"This patch abstracts the representation of the CCI400 chipset
 PMU specific definitions, so that we can avoid probing the
 revision for any details. The new device-tree bindings helps to
 get the revision, without poking the CCI, and initialises the pmu with
 specific model details."

Thanks
Suzuki




Cc: devicet...@vger.kernel.org
Cc: Punit Agrawal 
Signed-off-by: Suzuki K. Poulose 
---
  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 |  138 -
  include/linux/arm-cci.h   |2 +
  5 files changed, 166 insertions(+), 50 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: 
- Definition: must be "arm,cci-400-pmu"
-
+ Definition: Supported strings are :
+  "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
   - reg:
   Usage: required
   Value type: Integer cells. A register entry, expressed
diff --git a/arch/arm/include/asm/arm-cci.h b/arch/arm/include/asm/arm-cci.h
new file mode 100644
index 000..b828d7a
--- /dev/null
+++ b/arch/arm/include/asm/arm-cci.h
@@ -0,0 +1,42 @@
+/*
+ * arch/arm/include/asm/arm-cci.h
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#ifndef __ASM_ARM_CCI_H
+#define __ASM_ARM_CCI_H
+
+#ifdef CONFIG_MCPM
+#include 
+
+/*
+ * We don't have a reliable way of detecting, whether
+ * we have access to secure-only registers, unless
+ * mcpm is registered.
+ */
+static inline int platform_has_secure_cci_access(void)
+{
+ return mcpm_is_available();
+}
+
+#else
+static inline int platform_has_secure_cci_access(void)
+{
+ return 0;
+}
+#endif
+
+#endif
diff

Re: [PATCH 2/4] arm-cci: Get rid of secure transactions for PMU driver

2015-02-25 Thread Suzuki K. Poulose 

On 24/02/15 21:53, Nicolas Pitre wrote:

On Tue, 24 Feb 2015, Suzuki K. Poulose wrote:


From: Suzuki K. Poulose suzuki.poul...@arm.com

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.


This is an extensive commit log about secure access issues which is nice
and appreciated.  However the patch does quite some more code reorg not
mentioned here.  Could you please move this code reorg to a separate
patch and then have a patch on top introducing these probing changes?
This should make the implication of what is said above clearer.


Sure, I will do that in the next revision. What I missed in the commit
follows (which will be added in the next version):

This patch abstracts the representation of the CCI400 chipset
 PMU specific definitions, so that we can avoid probing the
 revision for any details. The new device-tree bindings helps to
 get the revision, without poking the CCI, and initialises the pmu with
 specific model details.

Thanks
Suzuki




Cc: devicet...@vger.kernel.org
Cc: Punit Agrawal punit.agra...@arm.com
Signed-off-by: Suzuki K. Poulose suzuki.poul...@arm.com
---
  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 |  138 -
  include/linux/arm-cci.h   |2 +
  5 files changed, 166 insertions(+), 50 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 :
+  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
   - reg:
   Usage: required
   Value type: Integer cells. A register entry, expressed
diff --git a/arch/arm/include/asm/arm-cci.h b/arch/arm/include/asm/arm-cci.h
new file mode 100644
index 000..b828d7a
--- /dev/null
+++ b/arch/arm/include/asm/arm-cci.h
@@ -0,0 +1,42 @@
+/*
+ * arch/arm/include/asm/arm-cci.h
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/.
+ */
+
+#ifndef __ASM_ARM_CCI_H
+#define __ASM_ARM_CCI_H
+
+#ifdef CONFIG_MCPM
+#include asm/mcpm.h
+
+/*
+ * We don't have a reliable way of detecting, whether
+ * we have access to secure-only registers, unless
+ * mcpm is registered.
+ */
+static inline int platform_has_secure_cci_access(void)
+{
+ return mcpm_is_available();
+}
+
+#else
+static inline int

Re: [PATCH 2/4] arm-cci: Get rid of secure transactions for PMU driver

2015-02-24 Thread Nicolas Pitre 
On Tue, 24 Feb 2015, Suzuki K. Poulose wrote:

> From: "Suzuki K. Poulose" 
> 
> 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.

This is an extensive commit log about secure access issues which is nice 
and appreciated.  However the patch does quite some more code reorg not 
mentioned here.  Could you please move this code reorg to a separate 
patch and then have a patch on top introducing these probing changes?  
This should make the implication of what is said above clearer.

> Cc: devicet...@vger.kernel.org
> Cc: Punit Agrawal 
> Signed-off-by: Suzuki K. Poulose 
> ---
>  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 |  138 
> -
>  include/linux/arm-cci.h   |2 +
>  5 files changed, 166 insertions(+), 50 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: 
> - Definition: must be "arm,cci-400-pmu"
> -
> + Definition: Supported strings are :
> +  "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
>   - reg:
>   Usage: required
>   Value type: Integer cells. A register entry, expressed
> diff --git a/arch/arm/include/asm/arm-cci.h b/arch/arm/include/asm/arm-cci.h
> new file mode 100644
> index 000..b828d7a
> --- /dev/null
> +++ b/arch/arm/include/asm/arm-cci.h
> @@ -0,0 +1,42 @@
> +/*
> + * arch/arm/include/asm/arm-cci.h
> + *
> + * Copyright (C) 2015 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#ifndef __ASM_ARM_CCI_H
> +#define __ASM_ARM_CCI_H
> +
> +#ifdef CONFIG_MCPM
> +#include 
> +
> +/*
> + * We don't have a reliable way of detecting, whether
> + * we have access to secure-only registers, unless
> + * mcpm is registered.
> + */
> +static inline int platform_has_secure_cci_access(void)
> +{
> + return mcpm_is_available();
> +}
> +
> +#else
> +static inline int platform_has_secure_cci_access(void)
> +{
> + return 0;
> +}
> +#endif
> +
> +#endif
> diff --git a/arch/arm64/include/asm/arm-cci.h 
> b/arch/arm64/include/asm/arm-cci.h
> new file mode 100644
> index 000..37bbe37
> --- /dev/null
> +++ b/arch/arm64/include/asm/arm-cci.h
> @@ -0,0 +1,27 @@
> +/*
> + * arch/arm64/include/asm/arm-cci.h
> + *
> + *

[PATCH 2/4] arm-cci: Get rid of secure transactions for PMU driver

2015-02-24 Thread Suzuki K. Poulose 
From: "Suzuki K. Poulose" 

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.

Cc: devicet...@vger.kernel.org
Cc: Punit Agrawal 
Signed-off-by: Suzuki K. Poulose 
---
 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 |  138 -
 include/linux/arm-cci.h   |2 +
 5 files changed, 166 insertions(+), 50 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: 
-   Definition: must be "arm,cci-400-pmu"
-
+   Definition: Supported strings are :
+"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
- reg:
Usage: required
Value type: Integer cells. A register entry, expressed
diff --git a/arch/arm/include/asm/arm-cci.h b/arch/arm/include/asm/arm-cci.h
new file mode 100644
index 000..b828d7a
--- /dev/null
+++ b/arch/arm/include/asm/arm-cci.h
@@ -0,0 +1,42 @@
+/*
+ * arch/arm/include/asm/arm-cci.h
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#ifndef __ASM_ARM_CCI_H
+#define __ASM_ARM_CCI_H
+
+#ifdef CONFIG_MCPM
+#include 
+
+/*
+ * We don't have a reliable way of detecting, whether
+ * we have access to secure-only registers, unless
+ * mcpm is registered.
+ */
+static inline int platform_has_secure_cci_access(void)
+{
+   return mcpm_is_available();
+}
+
+#else
+static inline int platform_has_secure_cci_access(void)
+{
+   return 0;
+}
+#endif
+
+#endif
diff --git a/arch/arm64/include/asm/arm-cci.h b/arch/arm64/include/asm/arm-cci.h
new file mode 100644
index 000..37bbe37
--- /dev/null
+++ b/arch/arm64/include/asm/arm-cci.h
@@ -0,0 +1,27 @@
+/*
+ * arch/arm64/include/asm/arm-cci.h
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see

[PATCH 2/4] arm-cci: Get rid of secure transactions for PMU driver

2015-02-24 Thread Suzuki K. Poulose 
From: Suzuki K. Poulose suzuki.poul...@arm.com

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.

Cc: devicet...@vger.kernel.org
Cc: Punit Agrawal punit.agra...@arm.com
Signed-off-by: Suzuki K. Poulose suzuki.poul...@arm.com
---
 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 |  138 -
 include/linux/arm-cci.h   |2 +
 5 files changed, 166 insertions(+), 50 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 :
+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
- reg:
Usage: required
Value type: Integer cells. A register entry, expressed
diff --git a/arch/arm/include/asm/arm-cci.h b/arch/arm/include/asm/arm-cci.h
new file mode 100644
index 000..b828d7a
--- /dev/null
+++ b/arch/arm/include/asm/arm-cci.h
@@ -0,0 +1,42 @@
+/*
+ * arch/arm/include/asm/arm-cci.h
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/.
+ */
+
+#ifndef __ASM_ARM_CCI_H
+#define __ASM_ARM_CCI_H
+
+#ifdef CONFIG_MCPM
+#include asm/mcpm.h
+
+/*
+ * We don't have a reliable way of detecting, whether
+ * we have access to secure-only registers, unless
+ * mcpm is registered.
+ */
+static inline int platform_has_secure_cci_access(void)
+{
+   return mcpm_is_available();
+}
+
+#else
+static inline int platform_has_secure_cci_access(void)
+{
+   return 0;
+}
+#endif
+
+#endif
diff --git a/arch/arm64/include/asm/arm-cci.h b/arch/arm64/include/asm/arm-cci.h
new file mode 100644
index 000..37bbe37
--- /dev/null
+++ b/arch/arm64/include/asm/arm-cci.h
@@ -0,0 +1,27 @@
+/*
+ * arch/arm64/include/asm/arm-cci.h
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public

Re: [PATCH 2/4] arm-cci: Get rid of secure transactions for PMU driver

2015-02-24 Thread Nicolas Pitre 
On Tue, 24 Feb 2015, Suzuki K. Poulose wrote:

 From: Suzuki K. Poulose suzuki.poul...@arm.com
 
 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.

This is an extensive commit log about secure access issues which is nice 
and appreciated.  However the patch does quite some more code reorg not 
mentioned here.  Could you please move this code reorg to a separate 
patch and then have a patch on top introducing these probing changes?  
This should make the implication of what is said above clearer.

 Cc: devicet...@vger.kernel.org
 Cc: Punit Agrawal punit.agra...@arm.com
 Signed-off-by: Suzuki K. Poulose suzuki.poul...@arm.com
 ---
  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 |  138 
 -
  include/linux/arm-cci.h   |2 +
  5 files changed, 166 insertions(+), 50 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 :
 +  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
   - reg:
   Usage: required
   Value type: Integer cells. A register entry, expressed
 diff --git a/arch/arm/include/asm/arm-cci.h b/arch/arm/include/asm/arm-cci.h
 new file mode 100644
 index 000..b828d7a
 --- /dev/null
 +++ b/arch/arm/include/asm/arm-cci.h
 @@ -0,0 +1,42 @@
 +/*
 + * arch/arm/include/asm/arm-cci.h
 + *
 + * Copyright (C) 2015 ARM Ltd.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.
 + */
 +
 +#ifndef __ASM_ARM_CCI_H
 +#define __ASM_ARM_CCI_H
 +
 +#ifdef CONFIG_MCPM
 +#include asm/mcpm.h
 +
 +/*
 + * We don't have a reliable way of detecting, whether
 + * we have access to secure-only registers, unless
 + * mcpm is registered.
 + */
 +static inline int platform_has_secure_cci_access(void)
 +{
 + return mcpm_is_available();
 +}
 +
 +#else
 +static inline int platform_has_secure_cci_access(void)
 +{
 + return 0;
 +}
 +#endif
 +
 +#endif
 diff --git a/arch/arm64/include/asm/arm-cci.h 
 b/arch/arm64/include/asm/arm-cci.h
 new file mode 100644
 index 000..37bbe37
 --- /dev/null
 +++ b/arch/arm64/include/asm/arm-cci.h
 @@ -0,0 +1,27 @@
 +/*
 + * arch/arm64/include/asm/arm-cci.h
 + *
 + * Copyright (C) 2015 ARM Ltd.
 + *
 + * This program is free