Currently if the Foundation model is running ARM Trusted Firmware then
the kernel, which is configured to use spin tables, cannot start secondary
processors or "power off" the simulation.
Add a couple of labels to the include file, and introduce a new .dts
file that uses these to override the enable-method.
Signed-off-by: Daniel Thompson <[email protected]>
---
arch/arm64/boot/dts/arm/Makefile | 3 +-
.../boot/dts/arm/foundation-v8-gicv3-psci.dts | 51 ++++++++++++++++++++++
arch/arm64/boot/dts/arm/foundation-v8.dtsi | 8 ++--
3 files changed, 57 insertions(+), 5 deletions(-)
create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
diff --git a/arch/arm64/boot/dts/arm/Makefile b/arch/arm64/boot/dts/arm/Makefile
index 75cc2aa10101..c9ec88809f3d 100644
--- a/arch/arm64/boot/dts/arm/Makefile
+++ b/arch/arm64/boot/dts/arm/Makefile
@@ -1,4 +1,5 @@
-dtb-$(CONFIG_ARCH_VEXPRESS) += foundation-v8.dtb foundation-v8-gicv3.dtb
+dtb-$(CONFIG_ARCH_VEXPRESS) += \
+ foundation-v8.dtb foundation-v8-gicv3.dtb foundation-v8-gicv3-psci.dtb
dtb-$(CONFIG_ARCH_VEXPRESS) += juno.dtb juno-r1.dtb juno-r2.dtb
dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb
dtb-$(CONFIG_ARCH_VEXPRESS) += vexpress-v2f-1xv7-ca53x2.dtb
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts b/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
new file mode 100644
index 000000000000..94a249095104
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
@@ -0,0 +1,51 @@
+/*
+ * ARM Ltd.
+ *
+ * ARMv8 Foundation model DTS (GICv3 configuration)
+ */
+
+#include "foundation-v8.dtsi"
+
+/ {
+ psci {
+ compatible = "arm,psci-1.0";
+ method = "smc";
+ };
+
+ gic: interrupt-controller@2f000000 {
+ compatible = "arm,gic-v3";
+ #interrupt-cells = <3>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+ interrupt-controller;
+ reg = <0x0 0x2f000000 0x0 0x10000>,
+ <0x0 0x2f100000 0x0 0x200000>,
+ <0x0 0x2c000000 0x0 0x2000>,
+ <0x0 0x2c010000 0x0 0x2000>,
+ <0x0 0x2c02f000 0x0 0x2000>;
+ interrupts = <1 9 4>;
+
+ its: its@2f020000 {
+ compatible = "arm,gic-v3-its";
+ msi-controller;
+ reg = <0x0 0x2f020000 0x0 0x20000>;
+ };
+ };
+};
+
+&cpu0 {
+ enable-method = "psci";
+};
+
+&cpu1 {
+ enable-method = "psci";
+};
+
+&cpu2 {
+ enable-method = "psci";
+};
+
+&cpu3 {
+ enable-method = "psci";
+};
diff --git a/arch/arm64/boot/dts/arm/foundation-v8.dtsi b/arch/arm64/boot/dts/arm/foundation-v8.dtsi
index 8ecdd4331980..8c7f8c4f090a 100644
--- a/arch/arm64/boot/dts/arm/foundation-v8.dtsi
+++ b/arch/arm64/boot/dts/arm/foundation-v8.dtsi
@@ -28,7 +28,7 @@
#address-cells = <2>;
#size-cells = <0>;
- cpu@0 {
+ cpu0: cpu@0 {
device_type = "cpu";
compatible = "arm,armv8";
reg = <0x0 0x0>;
@@ -36,7 +36,7 @@
cpu-release-addr = <0x0 0x8000fff8>;
next-level-cache = <&L2_0>;
};
- cpu@1 {
+ cpu1: cpu@1 {
device_type = "cpu";
compatible = "arm,armv8";
reg = <0x0 0x1>;
@@ -44,7 +44,7 @@
cpu-release-addr = <0x0 0x8000fff8>;
next-level-cache = <&L2_0>;
};
- cpu@2 {
+ cpu2: cpu@2 {
device_type = "cpu";
compatible = "arm,armv8";
reg = <0x0 0x2>;
@@ -52,7 +52,7 @@
cpu-release-addr = <0x0 0x8000fff8>;
next-level-cache = <&L2_0>;
};
- cpu@3 {
+ cpu3: cpu@3 {
device_type = "cpu";
compatible = "arm,armv8";
reg = <0x0 0x3>;
--
2.9.5
Hi Daniel,
On Mon, Sep 18, 2017 at 04:38:32PM +0100, Daniel Thompson wrote:
> Currently if the Foundation model is running ARM Trusted Firmware then
> the kernel, which is configured to use spin tables, cannot start secondary
> processors or "power off" the simulation.
>
> Add a couple of labels to the include file, and introduce a new .dts
> file that uses these to override the enable-method.
>
> Signed-off-by: Daniel Thompson <[email protected]>
This looks good, but has the unfortunate effect of leaving the
(irrelevant) cpu-release-addr property in the PSCI dts files, as that's
in the underlying dtsi file.
Could we split spin-table / PSCI parts into separate dtsi files?
e.g. have:
* foundation-v8.dtsi
* foundation-v8-gicv{2,3}.dtsi
* foundation-v8-{psci,spin-table}.dtsi
... and then combine those to build the dts files we want.
FWIW, with that:
Acked-by: Mark Rutland <[email protected]>
Thanks,
Mark.
On 18/09/17 17:12, Mark Rutland wrote:
> Hi Daniel,
>
> On Mon, Sep 18, 2017 at 04:38:32PM +0100, Daniel Thompson wrote:
>> Currently if the Foundation model is running ARM Trusted Firmware then
>> the kernel, which is configured to use spin tables, cannot start secondary
>> processors or "power off" the simulation.
>>
>> Add a couple of labels to the include file, and introduce a new .dts
>> file that uses these to override the enable-method.
>>
>> Signed-off-by: Daniel Thompson <[email protected]>
>
> This looks good, but has the unfortunate effect of leaving the
> (irrelevant) cpu-release-addr property in the PSCI dts files, as that's
> in the underlying dtsi file.
>
> Could we split spin-table / PSCI parts into separate dtsi files?
>
> e.g. have:
>
> * foundation-v8.dtsi
> * foundation-v8-gicv{2,3}.dtsi
> * foundation-v8-{psci,spin-table}.dtsi
>
> ... and then combine those to build the dts files we want.
Will do.
>
> FWIW, with that:
>
> Acked-by: Mark Rutland <[email protected]>
Talking about what we want... if it's all split out I might as well add
a gicv2+psci DT as well. Will that still retain your Acked-by or do you
want to see it first ;-) ?
Daniel.
On Tue, Sep 19, 2017 at 04:59:15PM +0100, Daniel Thompson wrote:
> On 18/09/17 17:12, Mark Rutland wrote:
> >Could we split spin-table / PSCI parts into separate dtsi files?
> >
> >e.g. have:
> >
> >* foundation-v8.dtsi
> >* foundation-v8-gicv{2,3}.dtsi
> >* foundation-v8-{psci,spin-table}.dtsi
> >
> >... and then combine those to build the dts files we want.
>
> Will do.
>
> >FWIW, with that:
> >
> >Acked-by: Mark Rutland <[email protected]>
>
> Talking about what we want... if it's all split out I might as well
> add a gicv2+psci DT as well. Will that still retain your Acked-by or
> do you want to see it first ;-) ?
Sure; that's fine by me. :)
Thanks,
Mark.
Currently if the Foundation model is running ARM Trusted Firmware then
the kernel, which is configured to use spin tables, cannot start secondary
processors or "power off" the simulation.
After adding a couple of labels to the include file and splitting out the
spin-table configuration into a header, we add a couple of new headers
together with two new DTs (GICv2+PSCI and GICv3+PSCI).
The new GICv3+PSCI DT has been boot tested, the remaining three (two of
which existed prior to this patch) have been "tested" by decompiling the
blobs and comparing them against a reference.
Acked-by: Mark Rutland <[email protected]>
Signed-off-by: Daniel Thompson <[email protected]>
---
arch/arm64/boot/dts/arm/Makefile | 4 +++-
arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi | 19 +++++++++++++++
.../boot/dts/arm/foundation-v8-gicv3-psci.dts | 9 +++++++
arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts | 25 ++-----------------
arch/arm64/boot/dts/arm/foundation-v8-gicv3.dtsi | 28 ++++++++++++++++++++++
arch/arm64/boot/dts/arm/foundation-v8-psci.dts | 9 +++++++
arch/arm64/boot/dts/arm/foundation-v8-psci.dtsi | 28 ++++++++++++++++++++++
.../boot/dts/arm/foundation-v8-spin-table.dtsi | 25 +++++++++++++++++++
arch/arm64/boot/dts/arm/foundation-v8.dts | 16 ++-----------
arch/arm64/boot/dts/arm/foundation-v8.dtsi | 16 ++++---------
10 files changed, 129 insertions(+), 50 deletions(-)
create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-gicv3.dtsi
create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-psci.dts
create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-psci.dtsi
create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-spin-table.dtsi
diff --git a/arch/arm64/boot/dts/arm/Makefile b/arch/arm64/boot/dts/arm/Makefile
index 75cc2aa10101..25f82c377f67 100644
--- a/arch/arm64/boot/dts/arm/Makefile
+++ b/arch/arm64/boot/dts/arm/Makefile
@@ -1,4 +1,6 @@
-dtb-$(CONFIG_ARCH_VEXPRESS) += foundation-v8.dtb foundation-v8-gicv3.dtb
+dtb-$(CONFIG_ARCH_VEXPRESS) += \
+ foundation-v8.dtb foundation-v8-psci.dtb \
+ foundation-v8-gicv3.dtb foundation-v8-gicv3-psci.dtb
dtb-$(CONFIG_ARCH_VEXPRESS) += juno.dtb juno-r1.dtb juno-r2.dtb
dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb
dtb-$(CONFIG_ARCH_VEXPRESS) += vexpress-v2f-1xv7-ca53x2.dtb
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi b/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
new file mode 100644
index 000000000000..851abf34fc80
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
@@ -0,0 +1,19 @@
+/*
+ * ARM Ltd.
+ *
+ * ARMv8 Foundation model DTS (GICv2 configuration)
+ */
+
+/ {
+ gic: interrupt-controller@2c001000 {
+ compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
+ #interrupt-cells = <3>;
+ #address-cells = <2>;
+ interrupt-controller;
+ reg = <0x0 0x2c001000 0 0x1000>,
+ <0x0 0x2c002000 0 0x2000>,
+ <0x0 0x2c004000 0 0x2000>,
+ <0x0 0x2c006000 0 0x2000>;
+ interrupts = <1 9 0xf04>;
+ };
+};
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts b/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
new file mode 100644
index 000000000000..e096e670bec3
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
@@ -0,0 +1,9 @@
+/*
+ * ARM Ltd.
+ *
+ * ARMv8 Foundation model DTS (GICv3+PSCI configuration)
+ */
+
+#include "foundation-v8.dtsi"
+#include "foundation-v8-gicv3.dtsi"
+#include "foundation-v8-psci.dtsi"
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts b/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts
index 35588dfa095c..c5d834d7d0ba 100644
--- a/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts
+++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts
@@ -5,26 +5,5 @@
*/
#include "foundation-v8.dtsi"
-
-/ {
- gic: interrupt-controller@2f000000 {
- compatible = "arm,gic-v3";
- #interrupt-cells = <3>;
- #address-cells = <2>;
- #size-cells = <2>;
- ranges;
- interrupt-controller;
- reg = <0x0 0x2f000000 0x0 0x10000>,
- <0x0 0x2f100000 0x0 0x200000>,
- <0x0 0x2c000000 0x0 0x2000>,
- <0x0 0x2c010000 0x0 0x2000>,
- <0x0 0x2c02f000 0x0 0x2000>;
- interrupts = <1 9 4>;
-
- its: its@2f020000 {
- compatible = "arm,gic-v3-its";
- msi-controller;
- reg = <0x0 0x2f020000 0x0 0x20000>;
- };
- };
-};
+#include "foundation-v8-gicv3.dtsi"
+#include "foundation-v8-spin-table.dtsi"
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dtsi b/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dtsi
new file mode 100644
index 000000000000..91fc5c60d88b
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dtsi
@@ -0,0 +1,28 @@
+/*
+ * ARM Ltd.
+ *
+ * ARMv8 Foundation model DTS (GICv3 configuration)
+ */
+
+/ {
+ gic: interrupt-controller@2f000000 {
+ compatible = "arm,gic-v3";
+ #interrupt-cells = <3>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+ interrupt-controller;
+ reg = <0x0 0x2f000000 0x0 0x10000>,
+ <0x0 0x2f100000 0x0 0x200000>,
+ <0x0 0x2c000000 0x0 0x2000>,
+ <0x0 0x2c010000 0x0 0x2000>,
+ <0x0 0x2c02f000 0x0 0x2000>;
+ interrupts = <1 9 4>;
+
+ its: its@2f020000 {
+ compatible = "arm,gic-v3-its";
+ msi-controller;
+ reg = <0x0 0x2f020000 0x0 0x20000>;
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-psci.dts b/arch/arm64/boot/dts/arm/foundation-v8-psci.dts
new file mode 100644
index 000000000000..723f23c7cd31
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/foundation-v8-psci.dts
@@ -0,0 +1,9 @@
+/*
+ * ARM Ltd.
+ *
+ * ARMv8 Foundation model DTS (GICv2+PSCI configuration)
+ */
+
+#include "foundation-v8.dtsi"
+#include "foundation-v8-gicv2.dtsi"
+#include "foundation-v8-psci.dtsi"
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-psci.dtsi b/arch/arm64/boot/dts/arm/foundation-v8-psci.dtsi
new file mode 100644
index 000000000000..16cdf395728b
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/foundation-v8-psci.dtsi
@@ -0,0 +1,28 @@
+/*
+ * ARM Ltd.
+ *
+ * ARMv8 Foundation model DTS (PSCI configuration)
+ */
+
+/ {
+ psci {
+ compatible = "arm,psci-1.0";
+ method = "smc";
+ };
+};
+
+&cpu0 {
+ enable-method = "psci";
+};
+
+&cpu1 {
+ enable-method = "psci";
+};
+
+&cpu2 {
+ enable-method = "psci";
+};
+
+&cpu3 {
+ enable-method = "psci";
+};
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-spin-table.dtsi b/arch/arm64/boot/dts/arm/foundation-v8-spin-table.dtsi
new file mode 100644
index 000000000000..4d4186ba0e8c
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/foundation-v8-spin-table.dtsi
@@ -0,0 +1,25 @@
+/*
+ * ARM Ltd.
+ *
+ * ARMv8 Foundation model DTS (spin table configuration)
+ */
+
+&cpu0 {
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x8000fff8>;
+};
+
+&cpu1 {
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x8000fff8>;
+};
+
+&cpu2 {
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x8000fff8>;
+};
+
+&cpu3 {
+ enable-method = "spin-table";
+ cpu-release-addr = <0x0 0x8000fff8>;
+};
diff --git a/arch/arm64/boot/dts/arm/foundation-v8.dts b/arch/arm64/boot/dts/arm/foundation-v8.dts
index 71168077312d..8ff7c86fc929 100644
--- a/arch/arm64/boot/dts/arm/foundation-v8.dts
+++ b/arch/arm64/boot/dts/arm/foundation-v8.dts
@@ -5,17 +5,5 @@
*/
#include "foundation-v8.dtsi"
-
-/ {
- gic: interrupt-controller@2c001000 {
- compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
- #interrupt-cells = <3>;
- #address-cells = <2>;
- interrupt-controller;
- reg = <0x0 0x2c001000 0 0x1000>,
- <0x0 0x2c002000 0 0x2000>,
- <0x0 0x2c004000 0 0x2000>,
- <0x0 0x2c006000 0 0x2000>;
- interrupts = <1 9 0xf04>;
- };
-};
+#include "foundation-v8-gicv2.dtsi"
+#include "foundation-v8-spin-table.dtsi"
diff --git a/arch/arm64/boot/dts/arm/foundation-v8.dtsi b/arch/arm64/boot/dts/arm/foundation-v8.dtsi
index 8ecdd4331980..60f6ab920743 100644
--- a/arch/arm64/boot/dts/arm/foundation-v8.dtsi
+++ b/arch/arm64/boot/dts/arm/foundation-v8.dtsi
@@ -28,36 +28,28 @@
#address-cells = <2>;
#size-cells = <0>;
- cpu@0 {
+ cpu0: cpu@0 {
device_type = "cpu";
compatible = "arm,armv8";
reg = <0x0 0x0>;
- enable-method = "spin-table";
- cpu-release-addr = <0x0 0x8000fff8>;
next-level-cache = <&L2_0>;
};
- cpu@1 {
+ cpu1: cpu@1 {
device_type = "cpu";
compatible = "arm,armv8";
reg = <0x0 0x1>;
- enable-method = "spin-table";
- cpu-release-addr = <0x0 0x8000fff8>;
next-level-cache = <&L2_0>;
};
- cpu@2 {
+ cpu2: cpu@2 {
device_type = "cpu";
compatible = "arm,armv8";
reg = <0x0 0x2>;
- enable-method = "spin-table";
- cpu-release-addr = <0x0 0x8000fff8>;
next-level-cache = <&L2_0>;
};
- cpu@3 {
+ cpu3: cpu@3 {
device_type = "cpu";
compatible = "arm,armv8";
reg = <0x0 0x3>;
- enable-method = "spin-table";
- cpu-release-addr = <0x0 0x8000fff8>;
next-level-cache = <&L2_0>;
};
--
2.9.5
On 19/09/17 19:32, Daniel Thompson wrote:
> Currently if the Foundation model is running ARM Trusted Firmware then
> the kernel, which is configured to use spin tables, cannot start secondary
> processors or "power off" the simulation.
>
> After adding a couple of labels to the include file and splitting out the
> spin-table configuration into a header, we add a couple of new headers
> together with two new DTs (GICv2+PSCI and GICv3+PSCI).
>
> The new GICv3+PSCI DT has been boot tested, the remaining three (two of
> which existed prior to this patch) have been "tested" by decompiling the
> blobs and comparing them against a reference.
>
How different are these from the ones hosted in [1] ?
On argument is that we want to take the DTS out of device tree as
firmware is responsible for generating them. Alternatively, we may be
duplicating resulting in discrepancies over time by coping it into kernel.
Since users of ARM TF must be able to access these, I am not sure if it
makes sense to merge these. Or we remove it from ARM TF to avoid any
conflicts/discrepancies.
Thoughts ?
--
Regards,
Sudeep
[1] https://github.com/ARM-software/arm-trusted-firmware/tree/master/fdts
On 20/09/17 10:42, Sudeep Holla wrote:
>
>
> On 19/09/17 19:32, Daniel Thompson wrote:
>> Currently if the Foundation model is running ARM Trusted Firmware then
>> the kernel, which is configured to use spin tables, cannot start secondary
>> processors or "power off" the simulation.
>>
>> After adding a couple of labels to the include file and splitting out the
>> spin-table configuration into a header, we add a couple of new headers
>> together with two new DTs (GICv2+PSCI and GICv3+PSCI).
>>
>> The new GICv3+PSCI DT has been boot tested, the remaining three (two of
>> which existed prior to this patch) have been "tested" by decompiling the
>> blobs and comparing them against a reference.
>>
>
> How different are these from the ones hosted in [1] ?
They look like they were either independently written or diverged a long
time ago. The existing kernel DTs describe hardware absent from the ARM
TF ones and vice versa.
With specific reference to PSCI it looks like my patches could perhaps
be improved by adding idle-state support.
> On argument is that we want to take the DTS out of device tree as
> firmware is responsible for generating them. Alternatively, we may be
> duplicating resulting in discrepancies over time by coping it into kernel.
The general problem is copying from where?
The kernel DTs are a well maintained centralized repository which is
*really* useful. git grep across the kernel DTs is a hugely powerful
tool when trying to better understand an ecosystem as sprawling and
diverse as ARMs. In fact I've even seen those sort of searchs used as a
basis to clean up unused code. Seeing that centralized repository
splinter into separate per-vendor silos would be a huge loss for kernel
developers.
> Since users of ARM TF must be able to access these, I am not sure if it
> makes sense to merge these. Or we remove it from ARM TF to avoid any
> conflicts/discrepancies. >
> Thoughts ?
I kind of agree that maintaining DTs and DT documentation in the kernel
is a little odd given that the kernel is not the only player here
(FreeBSD, u-boot, etc). However it is sufficiently well maintained that
projects are content(ish) to regard the kernel as the canonical source
for these things (u-boot, for example, seeks to shadow kernel DTs
without modifying them).
However regardless of the above I'd say they should be removed from ARM
TF. ARM TF does not use, modify, pass on or in any way consume DT... it
has no skin in the game here. Why does it want to own a few of blobs for
a small subset of the platforms it supports? I'm afraid that makes no
sense to me, to the extent that it didn't even occur to me to *look* in
the ARM TF sources to find any DTs for FVP until you pointed them out.
In other words, whilst people could discuss alternative ways to manage
DTs[1], I can't see any universe where ARM TF would be a logical place
to keep them.
Daniel.
[1] ... and I'd further suggest that only perhaps people who are
prepared to put resources into fixing it should convene such a
discussion.