[U-Boot] [PATCH 0/9] ARMv7: add PSCI support to u-boot
PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations: - Core idle management - CPU hotplug - big.LITTLE migration models - System shutdown and reset
It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code.
More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.
The main idea here is to reuse some of the existing u-boot code to create a separate blob that can live in SRAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when u-boot is long gone, hence the need for a piece of memory that will not be touched by the OS.
This patch series contains 3 parts: - the first four patches are just bug fixes - the next three contain the generic PSCI code and build infrastructure - the last two implement the CPU_ON method of the Allwinner A20 (aka sun7i).
I realize the A20 u-boot code is not upstream yet (BTW is anyone actively working on that?), but hopefully that should give a good idea of how things are structured so far. The patches are against a merge of u-boot mainline and the sunxi tree as of ten days ago.
As for using this code, it goes like this: sun7i# ext2load mmc 0:1 0x40000000 /boot/sunxi-psci.bin 908 bytes read in 18 ms (48.8 KiB/s) sun7i# cp 0x40000000 0x4000 0x1000 sun7i# ext2load mmc 0:1 40008000 /boot/zImage 3415087 bytes read in 184 ms (17.7 MiB/s) sun7i# setenv bootargs console=ttyS0,115200 earlyprintk root=/dev/nfs nfsroot=10.1.203.35:/export/roots/bobby-brown,tcp,v3 rw ip=dhcp sun7i# bootz 40008000
The kernel now boots in HYP mode, finds its secondary CPU without any additional SMP code, and runs KVM out of the box. Hopefully, the Xen/ARM guys can do the same fairly easily.
I'm wildly cross-posting this patch series, including to lists I'm not subscribed to. Please keep me on Cc for any comment you may have.
Cheers,
M.
Marc Zyngier (9): ARM: HYP/non-sec: fix alignment requirements for vectors ARM: HYP/non-sec: move switch to non-sec to the last boot phase ARM: HYP/non-sec: add a barrier after setting SCR.NS==1 ARM: non-sec: reset CNTVOFF to zero ARM: HYP/non-sec: add generic ARMv7 PSCI code ARM: HYP/non-sec: make pen code sections depend on !ARMV7_PSCI ARM: HYP/non-sec: add the option for a second-stage monitor sunxi: HYP/non-sec: add sun7i PSCI backend sunxi: HYP/non-sec: configure CNTFRQ on all CPUs
Makefile | 5 ++ arch/arm/cpu/armv7/Makefile | 4 ++ arch/arm/cpu/armv7/nonsec_virt.S | 21 +++++- arch/arm/cpu/armv7/psci.S | 109 ++++++++++++++++++++++++++++ arch/arm/cpu/armv7/sunxi/Makefile | 3 + arch/arm/cpu/armv7/sunxi/config.mk | 6 +- arch/arm/cpu/armv7/sunxi/psci.S | 119 +++++++++++++++++++++++++++++++ arch/arm/cpu/armv7/sunxi/u-boot-psci.lds | 63 ++++++++++++++++ arch/arm/cpu/armv7/virt-v7.c | 2 + arch/arm/lib/bootm.c | 5 +- include/configs/sun7i.h | 7 ++ psci/Makefile | 67 +++++++++++++++++ 12 files changed, 406 insertions(+), 5 deletions(-) create mode 100644 arch/arm/cpu/armv7/psci.S create mode 100644 arch/arm/cpu/armv7/sunxi/psci.S create mode 100644 arch/arm/cpu/armv7/sunxi/u-boot-psci.lds create mode 100644 psci/Makefile
Make sure the vectors are aligned on a 32 byte boundary, not the code that deals with it...
Signed-off-by: Marc Zyngier marc.zyngier@arm.com --- arch/arm/cpu/armv7/nonsec_virt.S | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 24b4c18..29987cd 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -14,6 +14,8 @@ .arch_extension sec .arch_extension virt
+ .align 5 @ Minimal alignment for vectors + /* the vector table for secure state and HYP mode */ _monitor_vectors: .word 0 /* reset */ @@ -32,7 +34,6 @@ _monitor_vectors: * to non-secure state. * We use only r0 and r1 here, due to constraints in the caller. */ - .align 5 _secure_monitor: mrc p15, 0, r1, c1, c1, 0 @ read SCR bic r1, r1, #0x4e @ clear IRQ, FIQ, EA, nET bits
Hello Marc
- .align 5 @ Minimal alignment for vectors
/* the vector table for secure state and HYP mode */ _monitor_vectors: .word 0 /* reset */ @@ -32,7 +34,6 @@ _monitor_vectors:
- to non-secure state.
- We use only r0 and r1 here, due to constraints in the caller.
*/
- .align 5
_secure_monitor: mrc p15, 0, r1, c1, c1, 0 @ read SCR bic r1, r1, #0x4e @ clear IRQ, FIQ, EA, nET bits
Correct.
I had posted a patch to fix this problem. http://patchwork.ozlabs.org/patch/280915/ But it have not been appiled yet.
Best Regards Masahiro Yamada
Hi Masahiro,
On 21/11/13 10:19, Masahiro Yamada wrote:
Hello Marc
- .align 5 @ Minimal alignment for vectors
/* the vector table for secure state and HYP mode */ _monitor_vectors: .word 0 /* reset */ @@ -32,7 +34,6 @@ _monitor_vectors:
- to non-secure state.
- We use only r0 and r1 here, due to constraints in the caller.
*/
- .align 5
_secure_monitor: mrc p15, 0, r1, c1, c1, 0 @ read SCR bic r1, r1, #0x4e @ clear IRQ, FIQ, EA, nET bits
Correct.
I had posted a patch to fix this problem. http://patchwork.ozlabs.org/patch/280915/ But it have not been appiled yet.
How comes nobody cares about it? This is really a nasty one to track down, as it completely depends on the surrounding objects.
/me puzzled.
M.
On 11/21/2013 09:59 AM, Marc Zyngier wrote:
Make sure the vectors are aligned on a 32 byte boundary, not the code that deals with it...
I think that patch was posted before, and I already acked it, but it didn't make it into some tree. Albert, can you please take this? Also a candidate for the stable tree.
Signed-off-by: Marc Zyngier marc.zyngier@arm.com
Acked-by: Andre Przywara andre.przywara@linaro.org
Regards, Andre.
arch/arm/cpu/armv7/nonsec_virt.S | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 24b4c18..29987cd 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -14,6 +14,8 @@ .arch_extension sec .arch_extension virt
- .align 5 @ Minimal alignment for vectors
- /* the vector table for secure state and HYP mode */ _monitor_vectors: .word 0 /* reset */
@@ -32,7 +34,6 @@ _monitor_vectors:
- to non-secure state.
- We use only r0 and r1 here, due to constraints in the caller.
*/
- .align 5 _secure_monitor: mrc p15, 0, r1, c1, c1, 0 @ read SCR bic r1, r1, #0x4e @ clear IRQ, FIQ, EA, nET bits
Having the switch to non-secure in the "prep" phase is causing all kind of troubles, as that stage can be called multiple times.
Instead, move the switch to non-secure to the last possible phase, when there is no turning back anymore.
Signed-off-by: Marc Zyngier marc.zyngier@arm.com --- arch/arm/lib/bootm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index f476a89..f3da634 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -234,7 +234,6 @@ static void boot_prep_linux(bootm_headers_t *images) printf("FDT and ATAGS support not compiled in - hanging\n"); hang(); } - do_nonsec_virt_switch(); }
/* Subcommand: GO */ @@ -264,8 +263,10 @@ static void boot_jump_linux(bootm_headers_t *images, int flag) else r2 = gd->bd->bi_boot_params;
- if (!fake) + if (!fake) { + do_nonsec_virt_switch(); kernel_entry(0, machid, r2); + } }
/* Main Entry point for arm bootm implementation
On 11/21/2013 09:59 AM, Marc Zyngier wrote:
Having the switch to non-secure in the "prep" phase is causing all kind of troubles, as that stage can be called multiple times.
Instead, move the switch to non-secure to the last possible phase, when there is no turning back anymore.
Tested on Versatile Express TC2.
Albert, Tom: please apply for v2014.01.
Acked-by: Andre Przywara andre.przywara@linaro.org
Signed-off-by: Marc Zyngier marc.zyngier@arm.com
arch/arm/lib/bootm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index f476a89..f3da634 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -234,7 +234,6 @@ static void boot_prep_linux(bootm_headers_t *images) printf("FDT and ATAGS support not compiled in - hanging\n"); hang(); }
do_nonsec_virt_switch(); }
/* Subcommand: GO */
@@ -264,8 +263,10 @@ static void boot_jump_linux(bootm_headers_t *images, int flag) else r2 = gd->bd->bi_boot_params;
- if (!fake)
if (!fake) {
do_nonsec_virt_switch();kernel_entry(0, machid, r2);
} }
/* Main Entry point for arm bootm implementation
A CP15 instruction execution can be reordered, requiring an isb to be sure it is executed in program order.
Signed-off-by: Marc Zyngier marc.zyngier@arm.com --- arch/arm/cpu/armv7/nonsec_virt.S | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 29987cd..648066f 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -47,6 +47,7 @@ _secure_monitor: #endif
mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set) + isb
#ifdef CONFIG_ARMV7_VIRT mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value
On 21 November 2013 00:59, Marc Zyngier marc.zyngier@arm.com wrote:
A CP15 instruction execution can be reordered, requiring an isb to be sure it is executed in program order.
Signed-off-by: Marc Zyngier marc.zyngier@arm.com
arch/arm/cpu/armv7/nonsec_virt.S | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 29987cd..648066f 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -47,6 +47,7 @@ _secure_monitor: #endif
mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set)
isb#ifdef CONFIG_ARMV7_VIRT mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value -- 1.8.2.3
Does this matter? Are we not still in monitor mode and therefore secure and the exception return below will surely be ordered by the cpu after the mcr, right? or no?
-Christoffer
On 22/11/13 01:51, Christoffer Dall wrote:
On 21 November 2013 00:59, Marc Zyngier marc.zyngier@arm.com wrote:
A CP15 instruction execution can be reordered, requiring an isb to be sure it is executed in program order.
Signed-off-by: Marc Zyngier marc.zyngier@arm.com
arch/arm/cpu/armv7/nonsec_virt.S | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 29987cd..648066f 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -47,6 +47,7 @@ _secure_monitor: #endif
mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set)
isb#ifdef CONFIG_ARMV7_VIRT mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value -- 1.8.2.3
Does this matter? Are we not still in monitor mode and therefore secure and the exception return below will surely be ordered by the cpu after the mcr, right? or no?
You need to look at what is between the SCR access and the exception return (and you cannot see that from the patch): There's a write to HVBAR that can only be executed if SCR.NS==1.
If the write to SCR is delayed, the whole thing will stop very quickly...
M.
On Fri, Nov 22, 2013 at 10:56:05AM +0000, Marc Zyngier wrote:
On 22/11/13 01:51, Christoffer Dall wrote:
On 21 November 2013 00:59, Marc Zyngier marc.zyngier@arm.com wrote:
A CP15 instruction execution can be reordered, requiring an isb to be sure it is executed in program order.
Signed-off-by: Marc Zyngier marc.zyngier@arm.com
arch/arm/cpu/armv7/nonsec_virt.S | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 29987cd..648066f 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -47,6 +47,7 @@ _secure_monitor: #endif
mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set)
isb#ifdef CONFIG_ARMV7_VIRT mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value -- 1.8.2.3
Does this matter? Are we not still in monitor mode and therefore secure and the exception return below will surely be ordered by the cpu after the mcr, right? or no?
You need to look at what is between the SCR access and the exception return (and you cannot see that from the patch): There's a write to HVBAR that can only be executed if SCR.NS==1.
If the write to SCR is delayed, the whole thing will stop very quickly...
Ah, I didn't realize this specific about PL2-mode system control registers and relied on the fact that we were just in monitor mode and that the setting of this bit essentially didn't matter; I obviously got this wrong, and I think to be fair that Andre had this isb in his original code and removed it after my review.
/my bad.
Thanks for the explanation.
-Christoffer
Hi, Dall: I have a few questions about switching cpu's state from secure to non-sec in uboot. 1. I found do_nonsec_virt_switch() function had been integrated in uboot_2014_01_RC2. This function would switch cpu from secure state to non-sec, even into hyp-state. So, my question is: If a SOC is based on ARMv7 architecture: Virt-v7.c / nonsec_virt.S are common files to all kinds of SOCs which are produced by different Vendors?
2. Does uboot need to switch its state to non-sec? Based on ARM company released doc: U-boot should run at non-sec state, so no need to swith to non-sec again.
Best wishes,
On 29 December 2013 19:10, TigerLiu@viatech.com.cn wrote:
Hi, Dall: I have a few questions about switching cpu's state from secure to non-sec in uboot.
- I found do_nonsec_virt_switch() function had been integrated in
uboot_2014_01_RC2. This function would switch cpu from secure state to non-sec, even into hyp-state. So, my question is: If a SOC is based on ARMv7 architecture: Virt-v7.c / nonsec_virt.S are common files to all kinds of SOCs which are produced by different Vendors?
Yes, the aim is to be able to reuse as much of this code for as many platforms as possible.
- Does uboot need to switch its state to non-sec? Based on ARM company released doc: U-boot should run at non-sec state, so no need to swith to non-sec
again.
It depends on the board. Which ARM doc are you referring to?
In general, there are three options for how u-boot is booted: 1. In secure mode 2. In non-secure hyp mode 3. in non-secure svc mode
for (1) you can just switch to non-secure hyp. for (2) you don't have to do anything. for (3) you're screwed, unless there's a backdoor call to enter Hyp mode (typically found on TI hardware).
If we are talking PSCI, that's a different story, and if that's provided by the board and not U-boot (see Marc's recent work), then I would expect that board to always boot U-boot in Hyp mode to be coherent with the documentation.
-Christoffer
Hi, Dall: Thanks for your quick response!
It depends on the board. Which ARM doc are you referring to?
ARM Security Technology : Building a Secure System using TrustZone Technology. (PRD29-GENC-009492C) Figure 5-2 in Chapter 5.2.1 Boot Sequence. Based on my understanding, U-boot is classfied as normal world boot loader. Maybe my understanding is wrong! :)
In general, there are three options for how u-boot is booted:
- In secure mode
- In non-secure hyp mode
- in non-secure svc mode
for (1) you can just switch to non-secure hyp. for (2) you don't have to do anything. for (3) you're screwed, unless there's a backdoor call to enter Hyp mode (typically found on TI hardware).
For (1), i didn't get it totally: On a platform with a CA7 supporting TZ tech, but this SOC not support VT, usually cpu is powered on in secure mode. So, i could only switch it to non-sec state? If this CA7 also supports VT, so i could select 2 goals: Switch to non-sec state, or swith to non-sec hyp mode?
I think non-sec state is not identical with non-sec hyp mode.
Best wishes,
-Christoffer
On 29 December 2013 21:15, TigerLiu@viatech.com.cn wrote:
Hi, Dall: Thanks for your quick response!
It depends on the board. Which ARM doc are you referring to?
ARM Security Technology : Building a Secure System using TrustZone Technology. (PRD29-GENC-009492C) Figure 5-2 in Chapter 5.2.1 Boot Sequence. Based on my understanding, U-boot is classfied as normal world boot loader. Maybe my understanding is wrong! :)
I am not an authority on classifying software according to some specification. All I know is how the architecture and hardware works to some limited degree. My take on this would be that it depends on your use of TrustZone.
In general, there are three options for how u-boot is booted:
- In secure mode
- In non-secure hyp mode
- in non-secure svc mode
for (1) you can just switch to non-secure hyp. for (2) you don't have to do anything. for (3) you're screwed, unless there's a backdoor call to enter Hyp mode (typically found on TI hardware).
For (1), i didn't get it totally: On a platform with a CA7 supporting TZ tech, but this SOC not support VT,
If it's a Cortex-A7 you have both virtualization extensions and the security extensions. If it does not, it's not a Cortex-A7.
usually cpu is powered on in secure mode.
yes, a CPU is powered on in secure mode, but it varies what the first piece of software that runs is. If we are talking u-boot:
So, i could only switch it to non-sec state?
if there is no virtualization support on your SoC, then there is no Hyp mode, and you can not switch to it.
If this CA7 also supports VT, so i could select 2 goals: Switch to non-sec state, or swith to non-sec hyp mode?
I think non-sec state is not identical with non-sec hyp mode.
I think you need to look at the ARM ARM more carefully:
non-secure *state* includes several non-secure CPU modes (usr, svc, und, abt, irq, fiq, hyp). You can choose to switch to either of them as you want, but if you want to boot Linux you should choose Hyp mode so KVM will work, unless you are writing your own hypervisor or use Hyp for something else, in which case you can fall back to booting Linux in svc mode.
-Christoffer
Hi, Dall: Thanks a lot!
non-secure *state* includes several non-secure CPU modes (usr, svc, und, abt, irq, fiq, hyp). You can choose to switch to either of them as you want, but if you want to boot Linux you should choose Hyp mode so KVM will work, unless you are writing your own hypervisor or use Hyp for something else, in which case you can fall back to booting Linux in svc mode.
Yes, you are right, i got it!
Best wishes,
On 11/21/2013 09:59 AM, Marc Zyngier wrote:
A CP15 instruction execution can be reordered, requiring an isb to be sure it is executed in program order.
Makes sense ;-) and works on the VExpress TC2.
Albert, Tom, please apply for v2014.01.
Acked-by: Andre Przywara andre.przywara@linaro.org
Signed-off-by: Marc Zyngier marc.zyngier@arm.com
arch/arm/cpu/armv7/nonsec_virt.S | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 29987cd..648066f 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -47,6 +47,7 @@ _secure_monitor: #endif
mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set)
isb
#ifdef CONFIG_ARMV7_VIRT mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value
Before switching to non-secure, make sure that CNTVOFF is set to zero on all CPUs. Otherwise, kernel running in non-secure without HYP enabled (hence using virtual timers) may observe timers that are not synchronized, effectively seeing time going backward...
Signed-off-by: Marc Zyngier marc.zyngier@arm.com --- arch/arm/cpu/armv7/nonsec_virt.S | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 648066f..bbacbce 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -53,7 +53,14 @@ _secure_monitor: mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value mcreq p15, 4, r0, c12, c0, 0 @ write HVBAR #endif + bne 1f
+ @ Reset CNTVOFF to 0 before leaving monitor mode + mrc p15, 0, r0, c0, c1, 1 @ read ID_PFR1 + ands r0, r0, #CPUID_ARM_GENTIMER_MASK @ test arch timer bits + movne r0, #0 + mcrrne p15, 4, r0, r0, c14 @ Reset CNTVOFF to zero +1: movs pc, lr @ return to non-secure SVC
_hyp_trap:
On 11/21/2013 09:59 AM, Marc Zyngier wrote:
Before switching to non-secure, make sure that CNTVOFF is set to zero on all CPUs. Otherwise, kernel running in non-secure without HYP enabled (hence using virtual timers) may observe timers that are not synchronized, effectively seeing time going backward...
Under what circumstances would native Linux use the virtual timers? When VIRT_EXT is not defined?
Regards, Andre.
Signed-off-by: Marc Zyngier marc.zyngier@arm.com
arch/arm/cpu/armv7/nonsec_virt.S | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 648066f..bbacbce 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -53,7 +53,14 @@ _secure_monitor: mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value mcreq p15, 4, r0, c12, c0, 0 @ write HVBAR #endif
bne 1f
@ Reset CNTVOFF to 0 before leaving monitor mode
mrc p15, 0, r0, c0, c1, 1 @ read ID_PFR1
ands r0, r0, #CPUID_ARM_GENTIMER_MASK @ test arch timer bits
movne r0, #0
mcrrne p15, 4, r0, r0, c14 @ Reset CNTVOFF to zero
+1: movs pc, lr @ return to non-secure SVC
_hyp_trap:
On 26/11/13 14:41, Andre Przywara wrote:
On 11/21/2013 09:59 AM, Marc Zyngier wrote:
Before switching to non-secure, make sure that CNTVOFF is set to zero on all CPUs. Otherwise, kernel running in non-secure without HYP enabled (hence using virtual timers) may observe timers that are not synchronized, effectively seeing time going backward...
Under what circumstances would native Linux use the virtual timers? When VIRT_EXT is not defined?
Yes. In general, when the kernel is not entered in HYP mode.
M.
Regards, Andre.
Signed-off-by: Marc Zyngier marc.zyngier@arm.com
arch/arm/cpu/armv7/nonsec_virt.S | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 648066f..bbacbce 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -53,7 +53,14 @@ _secure_monitor: mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value mcreq p15, 4, r0, c12, c0, 0 @ write HVBAR #endif
bne 1f
@ Reset CNTVOFF to 0 before leaving monitor mode
mrc p15, 0, r0, c0, c1, 1 @ read ID_PFR1
ands r0, r0, #CPUID_ARM_GENTIMER_MASK @ test arch timer bits
movne r0, #0
mcrrne p15, 4, r0, r0, c14 @ Reset CNTVOFF to zero
+1: movs pc, lr @ return to non-secure SVC
_hyp_trap:
Implement core support for PSCI. As this is generic code, it doesn't implement anything really useful (all the functions are returning Not Implemented).
Signed-off-by: Marc Zyngier marc.zyngier@arm.com --- arch/arm/cpu/armv7/Makefile | 4 ++ arch/arm/cpu/armv7/psci.S | 109 ++++++++++++++++++++++++++++++++++++++++++++ psci/Makefile | 67 +++++++++++++++++++++++++++ 3 files changed, 180 insertions(+) create mode 100644 arch/arm/cpu/armv7/psci.S create mode 100644 psci/Makefile
diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile index 8fac06c..24b6cb9 100644 --- a/arch/arm/cpu/armv7/Makefile +++ b/arch/arm/cpu/armv7/Makefile @@ -23,6 +23,10 @@ obj-y += nonsec_virt.o obj-y += virt-v7.o endif
+ifneq ($(CONFIG_PSCI_BUILD),) +obj-y += psci.o +endif + obj-$(CONFIG_OMAP_COMMON) += omap-common/ obj-$(CONFIG_TEGRA) += tegra-common/
diff --git a/arch/arm/cpu/armv7/psci.S b/arch/arm/cpu/armv7/psci.S new file mode 100644 index 0000000..3cfb147 --- /dev/null +++ b/arch/arm/cpu/armv7/psci.S @@ -0,0 +1,109 @@ +/* + * Copyright (C) 2013 - ARM Ltd + * Author: Marc Zyngier marc.zyngier@arm.com + * + * 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/. + */ + +#include <config.h> +#include <linux/linkage.h> + + .arch_extension sec + + .globl _start @ Keep linker happy... +_start: + .align 5 +_psci_vectors: + adr pc, . @ reset + adr pc, . @ undef + adr pc, _smc_psci @ smc + adr pc, . @ pabort + adr pc, . @ dabort + adr pc, . @ hyp + adr pc, . @ irq + adr pc, . @ fiq + +ENTRY(psci_cpu_suspend) +ENTRY(psci_cpu_off) +ENTRY(psci_cpu_on) +ENTRY(psci_migrate) + mvn r0, #0 @ Return -1 (Not Implemented) + mov pc, lr +ENDPROC(psci_migrate) +ENDPROC(psci_cpu_on) +ENDPROC(psci_cpu_off) +ENDPROC(psci_cpu_suspend) +.weak psci_cpu_suspend +.weak psci_cpu_off +.weak psci_cpu_on +.weak psci_migrate + +_psci_table: + .word 0x95c10000 + .word psci_cpu_suspend + .word 0x95c10001 + .word psci_cpu_off + .word 0x95c10002 + .word psci_cpu_on + .word 0x95c10003 + .word psci_migrate + .word 0 + .word 0 + +_secure_stacks: @ Enough to save 16 registers per CPU + .skip 16*4*CONFIG_ARMV7_PSCI_NR_CPUS +_secure_stack_base: + +_smc_psci: + @ Switch to secure mode + mrc p15, 0, sp, c1, c1, 0 + bic sp, sp, #1 + mcr p15, 0, sp, c1, c1, 0 + + adr sp, _secure_stack_base + mcr p15, 0, r0, c13, c0, 4 @ use TPIDRPRW as a tmp reg + mcr p15, 0, r1, c13, c0, 3 @ use TPIDRURO as a tmp reg + mrc p15, 0, r0, c0, c0, 5 @ MPIDR + and r1, r0, #3 @ cpu number in cluster + lsr r0, r0, #8 + and r0, r0, #3 @ cluster number + mul r1, r1, r0 @ absolute cpu nr + sbc sp, sp, r1, lsl #6 @ sp = sp_base - 64*cpunr + + mrc p15, 0, r0, c13, c0, 4 @ restore r0 + mrc p15, 0, r1, c13, c0, 3 @ restore r1 + + push {r4-r12,lr} + + adr r4, _psci_table +1: ldr r5, [r4] @ Load PSCI function ID + ldr r6, [r4, #4] @ Load target PC + cmp r5, #0 @ If reach the end, bail out + mvneq r0, #0 @ Return -1 (Not Implemented) + beq 2f + cmp r0, r5 @ If not matching, try next entry + addne r4, r4, #8 + bne 1b + cmp r6, #0 @ Not implemented + mvneq r0, #0 + beq 2f + + blx r6 @ Execute PSCI function + +2: pop {r4-r12, lr} + + @ Back to non-secure + mrc p15, 0, sp, c1, c1, 0 + orr sp, sp, #1 + mcr p15, 0, sp, c1, c1, 0 + movs pc, lr @ Return to the kernel diff --git a/psci/Makefile b/psci/Makefile new file mode 100644 index 0000000..7cc8b29 --- /dev/null +++ b/psci/Makefile @@ -0,0 +1,67 @@ +# +# (C) Copyright 2000-2011 +# Wolfgang Denk, DENX Software Engineering, wd@denx.de. +# +# (C) Copyright 2011 +# Daniel Schwierzeck, daniel.schwierzeck@googlemail.com. +# +# (C) Copyright 2011 +# Texas Instruments Incorporated - http://www.ti.com/ +# Aneesh V aneesh@ti.com +# +# (C) Copyright 2013 ARM Ltd +# Marc Zyngier marc.zyngier@arm.com +# +# SPDX-License-Identifier: GPL-2.0+ +# +# Based on top-level Makefile. +# + +CONFIG_PSCI_BUILD := y +export CONFIG_PSCI_BUILD + +PSCI_BIN := u-boot-psci + +include $(TOPDIR)/config.mk + +obj := $(OBJTREE)/psci/ + +PSCI_OBJS := $(CPUDIR)/psci.o +PSCI_OBJS += $(CPUDIR)/nonsec_virt.o +PSCI_OBJS += $(SOC_PSCI) + +# Linker Script +LDSCRIPT := $(TOPDIR)/$(CPUDIR)/$(SOC)/u-boot-psci.lds + +build := -f $(TOPDIR)/scripts/Makefile.build -C + +# Special flags for CPP when processing the linker script. +# Pass the version down so we can handle backwards compatibility +# on the fly. +LDPPFLAGS += \ + -include $(TOPDIR)/include/u-boot/u-boot.lds.h \ + -include $(OBJTREE)/include/config.h \ + -DCPUDIR=$(CPUDIR) \ + $(shell $(LD) --version | \ + sed -ne 's/GNU ld version ([0-9][0-9]*).([0-9][0-9]*).*/-DLD_MAJOR=\1 -DLD_MINOR=\2/p') + +all: $(PSCI_BIN) + +$(PSCI_BIN): depend $(PSCI_OBJS) u-boot-psci.lds + $(LD) $(LDFLAGS) $(LDFLAGS_$(@F)) \ + $(PSCI_OBJS) -T u-boot-psci.lds \ + -Map $(PSCI_BIN).map -o $(PSCI_BIN) + +$(PSCI_OBJS): depend + $(MAKE) $(build) $(SRCTREE)/$(dir $@) + mkdir -p $(dir $@) + mv $(SRCTREE)/$@ $@ + +u-boot-psci.lds: $(LDSCRIPT) depend + $(CPP) $(CPPFLAGS) $(LDPPFLAGS) -I$(obj). -ansi -D__ASSEMBLY__ -P - < $< > $@ + +depend: $(obj).depend +.PHONY: depend + +# defines $(obj).depend target +include $(SRCTREE)/rules.mk
PSCI doesn't need any pen-related code, as it interacts directly with the power controller.
Make these sections depend on CONFIG_ARMV7_PSCI not being set.
Signed-off-by: Marc Zyngier marc.zyngier@arm.com --- arch/arm/cpu/armv7/nonsec_virt.S | 2 ++ arch/arm/cpu/armv7/virt-v7.c | 2 ++ 2 files changed, 4 insertions(+)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index bbacbce..c5a8347 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -68,6 +68,7 @@ _hyp_trap: mov pc, lr @ do no switch modes, but @ return to caller
+#ifndef CONFIG_ARMV7_PSCI /* * Secondary CPUs start here and call the code for the core specific parts * of the non-secure and HYP mode transition. The GIC distributor specific @@ -93,6 +94,7 @@ ENTRY(_smp_pen) adr r0, _smp_pen @ do not use this address again b smp_waitloop @ wait for IPIs, board specific ENDPROC(_smp_pen) +#endif
/* * Switch a core to non-secure state. diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c index 2cd604f..55f0bbf 100644 --- a/arch/arm/cpu/armv7/virt-v7.c +++ b/arch/arm/cpu/armv7/virt-v7.c @@ -147,8 +147,10 @@ int armv7_switch_nonsec(void) for (i = 1; i <= itlinesnr; i++) writel((unsigned)-1, gic_dist_addr + GICD_IGROUPRn + 4 * i);
+#ifndef CONFIG_ARMV7_PSCI smp_set_core_boot_addr((unsigned long)_smp_pen, -1); smp_kick_all_cpus(); +#endif
/* call the non-sec switching code on this CPU also */ _nonsec_init();
Hi, Marc:
PSCI doesn't need any pen-related code, as it interacts directly with the power controller.
Make these sections depend on CONFIG_ARMV7_PSCI not being set.
Signed-off-by: Marc Zyngier marc.zyngier@arm.com
arch/arm/cpu/armv7/nonsec_virt.S | 2 ++ arch/arm/cpu/armv7/virt-v7.c | 2 ++ 2 files changed, 4 insertions(+) ......
Was this patch not merged into Uboot-2014.07-RC2 source code?
Best wishes,
Allow the switch to a second stage secure monitor just before switching to non-secure.
This allows a resident piece of firmware to be active once the kernel has been entered (the u-boot monitor is dead anyway, its pages being reused).
Signed-off-by: Marc Zyngier marc.zyngier@arm.com --- arch/arm/cpu/armv7/nonsec_virt.S | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index c5a8347..f07e3f7 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -35,6 +35,12 @@ _monitor_vectors: * We use only r0 and r1 here, due to constraints in the caller. */ _secure_monitor: +#ifdef CONFIG_ARMV7_PSCI_BASE + ldr r1, =CONFIG_ARMV7_PSCI_BASE @ Switch to the next monitor + mcr p15, 0, r1, c12, c0, 1 + isb +#endif + mrc p15, 0, r1, c1, c1, 0 @ read SCR bic r1, r1, #0x4e @ clear IRQ, FIQ, EA, nET bits orr r1, r1, #0x31 @ enable NS, AW, FW bits @@ -50,7 +56,7 @@ _secure_monitor: isb
#ifdef CONFIG_ARMV7_VIRT - mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value + adreq r0, _monitor_vectors mcreq p15, 4, r0, c12, c0, 0 @ write HVBAR #endif bne 1f
So far, only supporting the CPU_ON method. Other functions can be added later.
Signed-off-by: Marc Zyngier marc.zyngier@arm.com --- Makefile | 5 ++ arch/arm/cpu/armv7/sunxi/Makefile | 3 + arch/arm/cpu/armv7/sunxi/config.mk | 6 +- arch/arm/cpu/armv7/sunxi/psci.S | 119 +++++++++++++++++++++++++++++++ arch/arm/cpu/armv7/sunxi/u-boot-psci.lds | 63 ++++++++++++++++ include/configs/sun7i.h | 6 ++ 6 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 arch/arm/cpu/armv7/sunxi/psci.S create mode 100644 arch/arm/cpu/armv7/sunxi/u-boot-psci.lds
diff --git a/Makefile b/Makefile index 629299c..640b94e 100644 --- a/Makefile +++ b/Makefile @@ -606,6 +606,11 @@ $(obj)tpl/u-boot-tpl.bin: $(SUBDIR_TOOLS) depend $(obj)spl/sunxi-spl.bin: $(SUBDIR_TOOLS) depend $(MAKE) -C spl all
+# sunxi: PSCI binary blob +$(obj)sunxi-psci.bin: depend + $(MAKE) -C psci all + $(OBJCOPY) $(OBJCFLAGS) -O binary psci/u-boot-psci $@ + updater: $(MAKE) -C tools/updater all
diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile index 00efb65..dc4ca54 100644 --- a/arch/arm/cpu/armv7/sunxi/Makefile +++ b/arch/arm/cpu/armv7/sunxi/Makefile @@ -47,6 +47,9 @@ obj-y += cpu_info.o ifdef CONFIG_CMD_WATCHDOG obj-y += cmd_watchdog.o endif +ifdef CONFIG_PSCI_BUILD +obj-y += psci.o +endif endif
ifdef CONFIG_SPL_BUILD diff --git a/arch/arm/cpu/armv7/sunxi/config.mk b/arch/arm/cpu/armv7/sunxi/config.mk index 9ce48b4..83eb02b 100644 --- a/arch/arm/cpu/armv7/sunxi/config.mk +++ b/arch/arm/cpu/armv7/sunxi/config.mk @@ -2,7 +2,11 @@ ifdef CONFIG_SPL ifndef CONFIG_SPL_BUILD ifndef CONFIG_SPL_FEL -ALL-y = $(obj)u-boot-sunxi-with-spl.bin +ALL-y = $(obj)u-boot-sunxi-with-spl.bin $(obj)sunxi-psci.bin endif endif endif + +ifdef CONFIG_PSCI_BUILD +SOC_PSCI = arch/arm/cpu/armv7/sunxi/psci.o +endif diff --git a/arch/arm/cpu/armv7/sunxi/psci.S b/arch/arm/cpu/armv7/sunxi/psci.S new file mode 100644 index 0000000..f7db812 --- /dev/null +++ b/arch/arm/cpu/armv7/sunxi/psci.S @@ -0,0 +1,119 @@ +/* + * Copyright (C) 2013 - ARM Ltd + * Author: Marc Zyngier marc.zyngier@arm.com + * + * Based on code by Carl van Schaik carl@ok-labs.com. + * + * 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/. + */ + +#include <config.h> +#include <asm/arch/cpu.h> + +#define TEN_MS (10 * CONFIG_SYS_CLK_FREQ / 1000) + + @ r1 = target CPU + @ r2 = target PC +.globl psci_cpu_on +psci_cpu_on: + adr r0, _target_pc + str r2, [r0] + dsb + + movw r0, #(SUNXI_CPUCFG_BASE & 0xffff) + movt r0, #(SUNXI_CPUCFG_BASE >> 16) + + @ CPU mask + and r1, r1, #3 @ only care about first cluster + mov r4, #1 + lsl r4, r4, r1 + + adr r6, _sunxi_cpu_entry + str r6, [r0, #0x1a4] @ PRIVATE_REG (boot vector) + + @ Assert reset on target CPU + mov r6, #0 + lsl r5, r1, #6 @ 64 bytes per CPU + add r5, r5, #0x40 @ Offset from base + add r5, r5, r0 @ CPU control block + str r6, [r5] @ Reset CPU + + @ l1 invalidate + ldr r6, [r0, #0x184] + bic r6, r6, r4 + str r6, [r0, #0x184] + + @ Lock CPU + ldr r6, [r0, #0x1e4] + bic r6, r6, r4 + str r6, [r0, #0x1e4] + + @ Release power clamp + movw r6, #0x1ff + movt r6, #0 +1: lsrs r6, r6, #1 + str r6, [r0, #0x1b0] + bne 1b + + @ Write CNTP_TVAL : 10ms @ 24MHz (240000 cycles) + movw r1, #(TEN_MS & 0xffff) + movt r1, #(TEN_MS >> 16) + mcr p15, 0, r1, c14, c2, 0 + isb + @ Enable physical timer, mask interrupt + mov r1, #3 + mcr p15, 0, r1, c14, c2, 1 + @ Poll physical timer until ISTATUS is on +1: isb + mrc p15, 0, r1, c14, c2, 1 + ands r1, r1, #4 + bne 1b + @ Disable timer + mov r1, #0 + mcr p15, 0, r1, c14, c2, 1 + isb + + @ Clear power gating + ldr r6, [r0, #0x1b4] + bic r6, r6, #1 + str r6, [r0, #0x1b4] + + @ Deassert reset on target CPU + mov r6, #3 + str r6, [r5] + + @ Unlock CPU + ldr r6, [r0, #0x1e4] + orr r6, r6, r4 + str r6, [r0, #0x1e4] + + mov r0, #0 @ Return PSCI_RET_SUCCESS + mov pc, lr + +_target_pc: + .word 0 + +_sunxi_cpu_entry: + @ Set SMP bit + mrc p15, 0, r0, c1, c0, 1 + orr r0, r0, #0x40 + mcr p15, 0, r0, c1, c0, 1 + isb + + bl _nonsec_init + + bl _switch_to_hyp + + adr lr, _target_pc + ldr lr, [lr] + bx lr diff --git a/arch/arm/cpu/armv7/sunxi/u-boot-psci.lds b/arch/arm/cpu/armv7/sunxi/u-boot-psci.lds new file mode 100644 index 0000000..a0f166e --- /dev/null +++ b/arch/arm/cpu/armv7/sunxi/u-boot-psci.lds @@ -0,0 +1,63 @@ +/* + * (C) Copyright 2013 ARM Ltd + * Marc Zyngier marc.zyngier@arm.com + * + * Based on sunxi/u-boot-spl.lds: + * + * (C) Copyright 2012 + * Allwinner Technology Co., Ltd. <www.allwinnertech.com> + * Tom Cubie tangliang@allwinnertech.com + * + * Based on omap-common/u-boot-spl.lds: + * + * (C) Copyright 2002 + * Gary Jennejohn, DENX Software Engineering, garyj@denx.de + * + * (C) Copyright 2010 + * Texas Instruments, <www.ti.com> + * Aneesh V aneesh@ti.com + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * 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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +MEMORY { sram : ORIGIN = CONFIG_ARMV7_PSCI_BASE, LENGTH = 0x1000 } +OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") +OUTPUT_ARCH(arm) +ENTRY(_start) +SECTIONS +{ + .text : + { + _start = .; + *(.text*) + } > sram + + . = ALIGN(4); + .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } > sram + + . = ALIGN(4); + .data : { *(SORT_BY_ALIGNMENT(.data*)) } > sram + + . = ALIGN(4); + _end = .; + + /DISCARD/ : { + *(.bss*) + } +} diff --git a/include/configs/sun7i.h b/include/configs/sun7i.h index a6ede2a..1d6fd22 100644 --- a/include/configs/sun7i.h +++ b/include/configs/sun7i.h @@ -38,6 +38,12 @@ #define CONFIG_BOARD_POSTCLK_INIT 1 #endif
+#define CONFIG_ARMV7_VIRT 1 +#define CONFIG_ARMV7_NONSEC 1 +#define CONFIG_ARMV7_PSCI 1 +#define CONFIG_ARMV7_PSCI_NR_CPUS 2 +#define CONFIG_ARMV7_PSCI_BASE SUNXI_SRAM_A2_BASE + /* * Include common sunxi configuration where most the settings are */
CNTFRQ needs to be properly configured on all CPUs. Otherwise, virtual machines hoping to find valuable information on secondary CPUs will be disapointed...
Signed-off-by: Marc Zyngier marc.zyngier@arm.com --- include/configs/sun7i.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/configs/sun7i.h b/include/configs/sun7i.h index 1d6fd22..facbf92 100644 --- a/include/configs/sun7i.h +++ b/include/configs/sun7i.h @@ -43,6 +43,7 @@ #define CONFIG_ARMV7_PSCI 1 #define CONFIG_ARMV7_PSCI_NR_CPUS 2 #define CONFIG_ARMV7_PSCI_BASE SUNXI_SRAM_A2_BASE +#define CONFIG_SYS_CLK_FREQ 24000000
/* * Include common sunxi configuration where most the settings are
On Thu, Nov 21, 2013 at 2:59 AM, Marc Zyngier marc.zyngier@arm.com wrote:
PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset
It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code.
More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.
The main idea here is to reuse some of the existing u-boot code to create a separate blob that can live in SRAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when u-boot is long gone, hence the need for a piece of memory that will not be touched by the OS.
Interesting. As a separate binary, I'm not sure this belongs or benefits from being in u-boot. I would like to see this as a more generic secure firmware loader or PSCI code be a part of u-boot code directly. With the latter, you could extend it beyond PSCI to things like env variable access (basically equivalent to UEFI runtime services). I'm not saying we should do that though.
BTW, you will need to mark this region reserved in the dtb if in system RAM.
Rob
Hi Rob,
On 21/11/13 14:28, Rob Herring wrote:
On Thu, Nov 21, 2013 at 2:59 AM, Marc Zyngier marc.zyngier@arm.com wrote:
PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset
It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code.
More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.
The main idea here is to reuse some of the existing u-boot code to create a separate blob that can live in SRAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when u-boot is long gone, hence the need for a piece of memory that will not be touched by the OS.
Interesting. As a separate binary, I'm not sure this belongs or benefits from being in u-boot. I would like to see this as a more generic secure firmware loader or PSCI code be a part of u-boot code directly. With the latter, you could extend it beyond PSCI to things like env variable access (basically equivalent to UEFI runtime services). I'm not saying we should do that though.
So I started this by having something that was actually part of u-boot, and copying itself into SRAM, patching stuff as it went. The net result was that I was reinventing a runtime linker. Needless to say, I gave up quickly... ;-)
What could be done would be for u-boot to be at least partially linked to run from some other region. That would allow for the secure mode services to be both part of u-boot, and stay resident.
That'd probably be a good thing to have a look at.
BTW, you will need to mark this region reserved in the dtb if in system RAM.
Yes. Eventually, I'd like the psci mode to be entirely generated from u-boot, as well as the eventual RAM reserved.
M.
On 21 November 2013 07:04, Marc Zyngier marc.zyngier@arm.com wrote:
Hi Rob,
On 21/11/13 14:28, Rob Herring wrote:
On Thu, Nov 21, 2013 at 2:59 AM, Marc Zyngier marc.zyngier@arm.com wrote:
PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset
It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code.
More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.
The main idea here is to reuse some of the existing u-boot code to create a separate blob that can live in SRAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when u-boot is long gone, hence the need for a piece of memory that will not be touched by the OS.
Interesting. As a separate binary, I'm not sure this belongs or benefits from being in u-boot. I would like to see this as a more generic secure firmware loader or PSCI code be a part of u-boot code directly. With the latter, you could extend it beyond PSCI to things like env variable access (basically equivalent to UEFI runtime services). I'm not saying we should do that though.
So I started this by having something that was actually part of u-boot, and copying itself into SRAM, patching stuff as it went. The net result was that I was reinventing a runtime linker. Needless to say, I gave up quickly... ;-)
I'm curious; why did you need to reinvent a linker? This was all just assembly right? Could you not write it as position independent code and just copy a blob of code and be done with it?
(I'm sure it's not that simple, but I'm curious to know why).
-Christoffer
On 22 November 2013 07:24, Christoffer Dall christoffer.dall@linaro.org wrote:
On 21 November 2013 07:04, Marc Zyngier marc.zyngier@arm.com wrote:
Hi Rob,
On 21/11/13 14:28, Rob Herring wrote:
On Thu, Nov 21, 2013 at 2:59 AM, Marc Zyngier marc.zyngier@arm.com wrote:
PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset
It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code.
More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.
The main idea here is to reuse some of the existing u-boot code to create a separate blob that can live in SRAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when u-boot is long gone, hence the need for a piece of memory that will not be touched by the OS.
Interesting. As a separate binary, I'm not sure this belongs or benefits from being in u-boot. I would like to see this as a more generic secure firmware loader or PSCI code be a part of u-boot code directly. With the latter, you could extend it beyond PSCI to things like env variable access (basically equivalent to UEFI runtime services). I'm not saying we should do that though.
So I started this by having something that was actually part of u-boot, and copying itself into SRAM, patching stuff as it went. The net result was that I was reinventing a runtime linker. Needless to say, I gave up quickly... ;-)
I'm curious; why did you need to reinvent a linker? This was all just assembly right? Could you not write it as position independent code and just copy a blob of code and be done with it?
We really cannot assume that all power related programming sequence for SOCs will simple and easy to fit in position independent code. I am not saying it is impossible but it will not be easy to translate complex C code to position independent assembly code.
An Independent binary of a secured firmware makes more sense here. Also, if secured firmware is an independent binary then it need not be open source.
-- Anup
(I'm sure it's not that simple, but I'm curious to know why).
-Christoffer _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
On Fri, 2013-11-22 at 09:28 +0530, Anup Patel wrote:
An Independent binary of a secured firmware makes more sense here. Also, if secured firmware is an independent binary then it need not be open source.
In which case it should/can not have anything to do with u-boot nor reuse any GPL'd u-boot code. The platform should supply the PSCI service itself if you want to do this.
I for one don't see this as an advantage.
Ian.
On Fri, Nov 22, 2013 at 2:12 PM, Ian Campbell ijc@hellion.org.uk wrote:
On Fri, 2013-11-22 at 09:28 +0530, Anup Patel wrote:
An Independent binary of a secured firmware makes more sense here. Also, if secured firmware is an independent binary then it need not be open source.
In which case it should/can not have anything to do with u-boot nor reuse any GPL'd u-boot code. The platform should supply the PSCI service itself if you want to do this.
I for one don't see this as an advantage.
Further, independent secure firmware can be also used by UEFI or other bootloaders.
For now we just need secure firmware loading service from u-boot, which is what this patchset does.
-- Anup
Ian.
-- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
On Fri, Nov 22, 2013 at 02:30:27PM +0530, Anup Patel wrote:
On Fri, Nov 22, 2013 at 2:12 PM, Ian Campbell ijc@hellion.org.uk wrote:
On Fri, 2013-11-22 at 09:28 +0530, Anup Patel wrote:
An Independent binary of a secured firmware makes more sense here. Also, if secured firmware is an independent binary then it need not be open source.
In which case it should/can not have anything to do with u-boot nor reuse any GPL'd u-boot code. The platform should supply the PSCI service itself if you want to do this.
I for one don't see this as an advantage.
Further, independent secure firmware can be also used by UEFI or other bootloaders.
For now we just need secure firmware loading service from u-boot, which is what this patchset does.
As I see it this patchset seeks to provide (and does a good job of it) you with PSCI services on platforms where you don't already have this, so you avoid having to implement yet another platform-specific SMP boot-up sequence in the kernel. This is not about providing any generic support for secure firmware.
Ideally, the platform would just ship with PSCI support and boot U-Boot in Hyp mode and everyone would be happy - whichever way vendors wish to do that is a completely different discussion than this patch set.
-Christoffer
On 22/11/13 03:58, Anup Patel wrote:
On 22 November 2013 07:24, Christoffer Dall christoffer.dall@linaro.org wrote:
On 21 November 2013 07:04, Marc Zyngier marc.zyngier@arm.com wrote:
Hi Rob,
On 21/11/13 14:28, Rob Herring wrote:
On Thu, Nov 21, 2013 at 2:59 AM, Marc Zyngier marc.zyngier@arm.com wrote:
PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset
It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code.
More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.
The main idea here is to reuse some of the existing u-boot code to create a separate blob that can live in SRAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when u-boot is long gone, hence the need for a piece of memory that will not be touched by the OS.
Interesting. As a separate binary, I'm not sure this belongs or benefits from being in u-boot. I would like to see this as a more generic secure firmware loader or PSCI code be a part of u-boot code directly. With the latter, you could extend it beyond PSCI to things like env variable access (basically equivalent to UEFI runtime services). I'm not saying we should do that though.
So I started this by having something that was actually part of u-boot, and copying itself into SRAM, patching stuff as it went. The net result was that I was reinventing a runtime linker. Needless to say, I gave up quickly... ;-)
I'm curious; why did you need to reinvent a linker? This was all just assembly right? Could you not write it as position independent code and just copy a blob of code and be done with it?
We really cannot assume that all power related programming sequence for SOCs will simple and easy to fit in position independent code. I am not saying it is impossible but it will not be easy to translate complex C code to position independent assembly code.
An Independent binary of a secured firmware makes more sense here. Also, if secured firmware is an independent binary then it need not be open source.
If it is not open source, it has no purpose in u-boot, and I have strictly no intention to support such a thing. Quite the opposite, actually.
Eventually, I want to get completely rid of the "loading" bit, and just let u-boot relocate its secure monitor part into "secure memory" (irrespective of it being actually secure or not).
M.
On 22/11/13 01:54, Christoffer Dall wrote:
On 21 November 2013 07:04, Marc Zyngier marc.zyngier@arm.com wrote:
Hi Rob,
On 21/11/13 14:28, Rob Herring wrote:
On Thu, Nov 21, 2013 at 2:59 AM, Marc Zyngier marc.zyngier@arm.com wrote:
PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset
It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code.
More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.
The main idea here is to reuse some of the existing u-boot code to create a separate blob that can live in SRAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when u-boot is long gone, hence the need for a piece of memory that will not be touched by the OS.
Interesting. As a separate binary, I'm not sure this belongs or benefits from being in u-boot. I would like to see this as a more generic secure firmware loader or PSCI code be a part of u-boot code directly. With the latter, you could extend it beyond PSCI to things like env variable access (basically equivalent to UEFI runtime services). I'm not saying we should do that though.
So I started this by having something that was actually part of u-boot, and copying itself into SRAM, patching stuff as it went. The net result was that I was reinventing a runtime linker. Needless to say, I gave up quickly... ;-)
I'm curious; why did you need to reinvent a linker? This was all just assembly right? Could you not write it as position independent code and just copy a blob of code and be done with it?
There is more than just that. Some of the code is actually shared between u-boot and PSCI, so in the process of copying to SRAM, you have to patch some things there.
Some of it could be rewritten as being PIC, but there is more fundamental issues about the current approach: the way the code is structured, it expects to be able to switch directly from secure to non-secure in the same memory space. With secure memory, that's a killer (you're pulling the rug (and entire floor) from under your own feet).
So I'm now heading towards changing the "non-secure-switching" part of u-boot to be able to return directly into the payload, instead of carrying on within the same memory-space. This will then allow moving that code to secure memory, and then we'll be able to share it between u-boot and PSCI. Well, I think... ;-)
M.
On Thu, 2013-11-21 at 15:04 +0000, Marc Zyngier wrote:
Hi Rob,
On 21/11/13 14:28, Rob Herring wrote:
On Thu, Nov 21, 2013 at 2:59 AM, Marc Zyngier marc.zyngier@arm.com wrote:
PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset
It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code.
More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.
The main idea here is to reuse some of the existing u-boot code to create a separate blob that can live in SRAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when u-boot is long gone, hence the need for a piece of memory that will not be touched by the OS.
Interesting. As a separate binary, I'm not sure this belongs or benefits from being in u-boot. I would like to see this as a more generic secure firmware loader or PSCI code be a part of u-boot code directly. With the latter, you could extend it beyond PSCI to things like env variable access (basically equivalent to UEFI runtime services). I'm not saying we should do that though.
So I started this by having something that was actually part of u-boot, and copying itself into SRAM, patching stuff as it went. The net result was that I was reinventing a runtime linker. Needless to say, I gave up quickly... ;-)
What could be done would be for u-boot to be at least partially linked to run from some other region.
Can't you just build and link it as you do now and then link it into the .rodata section of the final u-boot image as a blob to copy out to the defined address at runtime?
That would allow for the secure mode services to be both part of u-boot, and stay resident.
That'd probably be a good thing to have a look at.
BTW, you will need to mark this region reserved in the dtb if in system RAM.
Yes. Eventually, I'd like the psci mode to be entirely generated from u-boot, as well as the eventual RAM reserved.
Did you mean "psci node" here? In which case yes please.
Ian.
On Fri, Nov 22, 2013 at 2:10 PM, Ian Campbell ijc@hellion.org.uk wrote:
On Thu, 2013-11-21 at 15:04 +0000, Marc Zyngier wrote:
Hi Rob,
On 21/11/13 14:28, Rob Herring wrote:
On Thu, Nov 21, 2013 at 2:59 AM, Marc Zyngier marc.zyngier@arm.com wrote:
PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset
It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code.
More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.
The main idea here is to reuse some of the existing u-boot code to create a separate blob that can live in SRAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when u-boot is long gone, hence the need for a piece of memory that will not be touched by the OS.
Interesting. As a separate binary, I'm not sure this belongs or benefits from being in u-boot. I would like to see this as a more generic secure firmware loader or PSCI code be a part of u-boot code directly. With the latter, you could extend it beyond PSCI to things like env variable access (basically equivalent to UEFI runtime services). I'm not saying we should do that though.
So I started this by having something that was actually part of u-boot, and copying itself into SRAM, patching stuff as it went. The net result was that I was reinventing a runtime linker. Needless to say, I gave up quickly... ;-)
What could be done would be for u-boot to be at least partially linked to run from some other region.
Can't you just build and link it as you do now and then link it into the .rodata section of the final u-boot image as a blob to copy out to the defined address at runtime?
If you link secure firmware inside u-boot image then we will have to reflash or update entire u-boot image whenever we want to update the secure firmware.
-- Anup
That would allow for the secure mode services to be both part of u-boot, and stay resident.
That'd probably be a good thing to have a look at.
BTW, you will need to mark this region reserved in the dtb if in system RAM.
Yes. Eventually, I'd like the psci mode to be entirely generated from u-boot, as well as the eventual RAM reserved.
Did you mean "psci node" here? In which case yes please.
Ian.
-- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
On Fri, 2013-11-22 at 14:26 +0530, Anup Patel wrote:
If you link secure firmware inside u-boot image then we will have to reflash or update entire u-boot image whenever we want to update the secure firmware.
Only if you make your secure firmware part of u-boot. Nothing says you have to. You could equally well integrate with some existing blob provided by lower level firmware.
Ian.
On 22/11/13 08:40, Ian Campbell wrote:
On Thu, 2013-11-21 at 15:04 +0000, Marc Zyngier wrote:
Hi Rob,
On 21/11/13 14:28, Rob Herring wrote:
On Thu, Nov 21, 2013 at 2:59 AM, Marc Zyngier marc.zyngier@arm.com wrote:
PSCI is an ARM standard that provides a generic interface that supervisory software can use to manage power in the following situations:
- Core idle management
- CPU hotplug
- big.LITTLE migration models
- System shutdown and reset
It basically allows the kernel to offload these tasks to the firmware, and rely on common kernel side code.
More importantly, it gives a way to ensure that CPUs enter the kernel at the appropriate exception level (ie HYP mode, to allow the use of the virtualization extensions), even across events like CPUs being powered off/on or suspended.
The main idea here is to reuse some of the existing u-boot code to create a separate blob that can live in SRAM (or a reserved page of memory), containing a secure monitor that will implement the PSCI operations. This code will still be alive when u-boot is long gone, hence the need for a piece of memory that will not be touched by the OS.
Interesting. As a separate binary, I'm not sure this belongs or benefits from being in u-boot. I would like to see this as a more generic secure firmware loader or PSCI code be a part of u-boot code directly. With the latter, you could extend it beyond PSCI to things like env variable access (basically equivalent to UEFI runtime services). I'm not saying we should do that though.
So I started this by having something that was actually part of u-boot, and copying itself into SRAM, patching stuff as it went. The net result was that I was reinventing a runtime linker. Needless to say, I gave up quickly... ;-)
What could be done would be for u-boot to be at least partially linked to run from some other region.
Can't you just build and link it as you do now and then link it into the .rodata section of the final u-boot image as a blob to copy out to the defined address at runtime?
That would allow for the secure mode services to be both part of u-boot, and stay resident.
That'd probably be a good thing to have a look at.
BTW, you will need to mark this region reserved in the dtb if in system RAM.
Yes. Eventually, I'd like the psci mode to be entirely generated from u-boot, as well as the eventual RAM reserved.
Did you mean "psci node" here? In which case yes please.
Yes, that's what I meant. More. Coffee.
M.
On 11/21/2013 09:59 AM, Marc Zyngier wrote:
PSCI is an ARM standard that provides a generic interface that ...
Thanks again for posting this, I like the idea of adding PSCI handlers to u-boot for several platforms very much.
This patch series contains 3 parts:
- the first four patches are just bug fixes
Those are fine, I already acked those patches.
- the next three contain the generic PSCI code and build infrastructure
As I heard you will rework these anyway, I will refrain from commenting in detail, just some generic comments on the approach:
* Is the creation of a top-level psci directory for holding the PSCI binaries really necessary? Should this mimic the spl approach? Can you consider to move this at least into the arch/arm directory, as PSCI is ARM specific? Or add it to the SPL directory, as this serves a similar purpose? But maybe your new approach renders this all moot.
* Can you keep the SMP bringup code in place and re-use it from the PSCI handlers instead of "#ifndef PSCI"ing it? So maybe rename arch/arm/cpu/armv7/sunxi/psci.S to .../sunxi/smp.S? My idea here is to make PSCI an option in addition to the current SMP HYP mode code. So that for instance on VExpress (or better: Arndale) you could either use the existing code using the kernel's platform SMP code or enable PSCI in u-boot and let the kernel use that, too. I maybe ask too much for the first incarnation of the code, but that is how I would like to eventually see it. AFAIK Linux prefers PSCI over platform-defined SMP code, so this should work out of the box.
* Is the use of TPIDRPRW & Co. really safe? It looks like as we seem to be the only secure user (and they are banked), but I am just curious whether there is any "prior art" in using those registers temporarily.
... The kernel now boots in HYP mode, finds its secondary CPU without any additional SMP code, and runs KVM out of the box. Hopefully, the Xen/ARM guys can do the same fairly easily.
BTW: Yesterday my PSCI host patches for Xen have been committed, so Xen should be able to use that feature just like the kernel does.
Thanks! Andre.
Hi Andre,
On 06/12/13 11:43, Andre Przywara wrote:
On 11/21/2013 09:59 AM, Marc Zyngier wrote:
PSCI is an ARM standard that provides a generic interface that ...
Thanks again for posting this, I like the idea of adding PSCI handlers to u-boot for several platforms very much.
This patch series contains 3 parts:
- the first four patches are just bug fixes
Those are fine, I already acked those patches.
- the next three contain the generic PSCI code and build infrastructure
As I heard you will rework these anyway, I will refrain from commenting in detail, just some generic comments on the approach:
- Is the creation of a top-level psci directory for holding the PSCI
binaries really necessary? Should this mimic the spl approach? Can you consider to move this at least into the arch/arm directory, as PSCI is ARM specific? Or add it to the SPL directory, as this serves a similar purpose? But maybe your new approach renders this all moot.
The code base I have at the moment completely gets rid of the psci directory, and make that code a separate section that can be relocated to secure memory or simply marked as reserved.
- Can you keep the SMP bringup code in place and re-use it from the PSCI
handlers instead of "#ifndef PSCI"ing it? So maybe rename arch/arm/cpu/armv7/sunxi/psci.S to .../sunxi/smp.S? My idea here is to make PSCI an option in addition to the current SMP HYP mode code. So that for instance on VExpress (or better: Arndale) you could either use the existing code using the kernel's platform SMP code or enable PSCI in u-boot and let the kernel use that, too.
The main problem is the SMP wake-up code in u-boot. With PSCI, you really don't want to wake up the secondaries at all, and you wait until the kernel does an SMC. The current code wakes up the CPUs unconditionnally, and put them in a separate pen, and I'd really like to avoid dealing with a pen in the PSCI case (stupid platforms like VExpress might still require it, but that's an orthogonal discussion).
I maybe ask too much for the first incarnation of the code, but that is how I would like to eventually see it. AFAIK Linux prefers PSCI over platform-defined SMP code, so this should work out of the box.
Not really. So far, it will use the the platform SMP code if it is defined, and fallback to PSCI otherwise. There were patches from Stephen Boyd to use the "enable-method" property in the cpu node to select PSCI though. I need to ping him on that.
- Is the use of TPIDRPRW & Co. really safe? It looks like as we seem to
be the only secure user (and they are banked), but I am just curious whether there is any "prior art" in using those registers temporarily.
As you said, we own secure mode entirely. So they really are scratch registers, free to be used.
... The kernel now boots in HYP mode, finds its secondary CPU without any additional SMP code, and runs KVM out of the box. Hopefully, the Xen/ARM guys can do the same fairly easily.
BTW: Yesterday my PSCI host patches for Xen have been committed, so Xen should be able to use that feature just like the kernel does.
Excellent! I really need to sort these patches out and repost the whole series...
Thanks,
M.
On Fri, 2013-12-06 at 12:12 +0000, Marc Zyngier wrote:
BTW: Yesterday my PSCI host patches for Xen have been committed, so Xen should be able to use that feature just like the kernel does.
Excellent! I really need to sort these patches out and repost the whole series...
I was hoping to give this a go on my cb2 this afternoon. Do you happen to have a public git tree of either this version or the upcoming new one handy?
Ian.
(damn linux-sunxi reply-to vs. evolution madness ate your copy Marc, sorry!)
On Fri, 2013-12-06 at 12:59 +0000, Ian Campbell wrote:
On Fri, 2013-12-06 at 12:12 +0000, Marc Zyngier wrote:
BTW: Yesterday my PSCI host patches for Xen have been committed, so Xen should be able to use that feature just like the kernel does.
Excellent! I really need to sort these patches out and repost the whole series...
I was hoping to give this a go on my cb2 this afternoon. Do you happen to have a public git tree of either this version or the upcoming new one handy?
Actually, since this series doesn't build ("no rule to make sunxi-psci.bin"...) I guess I'll wait for the upcoming one.
Ian.
On 12/06/2013 04:44 PM, Ian Campbell wrote:
(damn linux-sunxi reply-to vs. evolution madness ate your copy Marc, sorry!)
On Fri, 2013-12-06 at 12:59 +0000, Ian Campbell wrote:
On Fri, 2013-12-06 at 12:12 +0000, Marc Zyngier wrote:
BTW: Yesterday my PSCI host patches for Xen have been committed, so Xen should be able to use that feature just like the kernel does.
Excellent! I really need to sort these patches out and repost the whole series...
I was hoping to give this a go on my cb2 this afternoon. Do you happen to have a public git tree of either this version or the upcoming new one handy?
Actually, since this series doesn't build ("no rule to make sunxi-psci.bin"...) I guess I'll wait for the upcoming one.
Have you observed this sentence is Marc's mail?
"The patches are against a merge of u-boot mainline and the sunxi tree as of ten days ago."
Not sure if that really helps, though, I stopped at this point and just tried the first 7 patches.
But you may give this another shot as long as there are still the winds outside ;-) Or is it already over on the isles?
Regards, Andre.
On Fri, 2013-12-06 at 16:48 +0100, Andre Przywara wrote:
On 12/06/2013 04:44 PM, Ian Campbell wrote:
(damn linux-sunxi reply-to vs. evolution madness ate your copy Marc, sorry!)
On Fri, 2013-12-06 at 12:59 +0000, Ian Campbell wrote:
On Fri, 2013-12-06 at 12:12 +0000, Marc Zyngier wrote:
BTW: Yesterday my PSCI host patches for Xen have been committed, so Xen should be able to use that feature just like the kernel does.
Excellent! I really need to sort these patches out and repost the whole series...
I was hoping to give this a go on my cb2 this afternoon. Do you happen to have a public git tree of either this version or the upcoming new one handy?
Actually, since this series doesn't build ("no rule to make sunxi-psci.bin"...) I guess I'll wait for the upcoming one.
Have you observed this sentence is Marc's mail?
"The patches are against a merge of u-boot mainline and the sunxi tree as of ten days ago."
The sunxi tree today has mainline v2014.01-rc1 in it, which I'd assumed was new enough because it was tagged after Marc posted these patches.
Nothing between v2013.01-rc1 and the current mainline looks related.
Ian.
Not sure if that really helps, though, I stopped at this point and just tried the first 7 patches.
But you may give this another shot as long as there are still the winds outside ;-) Or is it already over on the isles?
Regards, Andre.
On 06/12/13 17:21, Ian Campbell wrote:
On Fri, 2013-12-06 at 16:48 +0100, Andre Przywara wrote:
On 12/06/2013 04:44 PM, Ian Campbell wrote:
(damn linux-sunxi reply-to vs. evolution madness ate your copy Marc, sorry!)
On Fri, 2013-12-06 at 12:59 +0000, Ian Campbell wrote:
On Fri, 2013-12-06 at 12:12 +0000, Marc Zyngier wrote:
BTW: Yesterday my PSCI host patches for Xen have been committed, so Xen should be able to use that feature just like the kernel does.
Excellent! I really need to sort these patches out and repost the whole series...
I was hoping to give this a go on my cb2 this afternoon. Do you happen to have a public git tree of either this version or the upcoming new one handy?
Actually, since this series doesn't build ("no rule to make sunxi-psci.bin"...) I guess I'll wait for the upcoming one.
Have you observed this sentence is Marc's mail?
"The patches are against a merge of u-boot mainline and the sunxi tree as of ten days ago."
The sunxi tree today has mainline v2014.01-rc1 in it, which I'd assumed was new enough because it was tagged after Marc posted these patches.
Nothing between v2013.01-rc1 and the current mainline looks related.
Don't underestimate my own capacity to screw things up in a devastating way... ;-)
I'll try to sort my patches tomorrow morning, and hopefully won't mess it up this time. And given that I'm heading for Devonshire in a couple of hours (hint, hint!), it is not a strong guarantee... :D
M.
On 12/06/2013 01:12 PM, Marc Zyngier wrote:
Hi Andre,
On 06/12/13 11:43, Andre Przywara wrote:
On 11/21/2013 09:59 AM, Marc Zyngier wrote:
PSCI is an ARM standard that provides a generic interface that ...
Thanks again for posting this, I like the idea of adding PSCI handlers to u-boot for several platforms very much.
This patch series contains 3 parts:
- the first four patches are just bug fixes
Those are fine, I already acked those patches.
- the next three contain the generic PSCI code and build infrastructure
As I heard you will rework these anyway, I will refrain from commenting in detail, just some generic comments on the approach:
- Is the creation of a top-level psci directory for holding the PSCI
binaries really necessary? Should this mimic the spl approach? Can you consider to move this at least into the arch/arm directory, as PSCI is ARM specific? Or add it to the SPL directory, as this serves a similar purpose? But maybe your new approach renders this all moot.
The code base I have at the moment completely gets rid of the psci directory, and make that code a separate section that can be relocated to secure memory or simply marked as reserved.
Nice!
- Can you keep the SMP bringup code in place and re-use it from the PSCI
handlers instead of "#ifndef PSCI"ing it? So maybe rename arch/arm/cpu/armv7/sunxi/psci.S to .../sunxi/smp.S? My idea here is to make PSCI an option in addition to the current SMP HYP mode code. So that for instance on VExpress (or better: Arndale) you could either use the existing code using the kernel's platform SMP code or enable PSCI in u-boot and let the kernel use that, too.
The main problem is the SMP wake-up code in u-boot. With PSCI, you really don't want to wake up the secondaries at all, and you wait until the kernel does an SMC. The current code wakes up the CPUs unconditionnally,
Yes, and I want to do away with this if PSCI is defined - as this is not needed. I just asked for keeping the SMP wakeup code as generic as possible and neither tie it to PSCI nor HYP mode, so that either of them can reference that. But, ...
and put them in a separate pen, and I'd really like to avoid dealing with a pen in the PSCI case (stupid platforms like VExpress might still require it, but that's an orthogonal discussion).
Yes, there is indeed this problem with the pen. Maybe one can use your upcoming relocation code to move the pen to a more secure place (defined per platform). With "avoid dealing with a pen" you mean to wakeup from the system's pen and immediately jump to the OS init code, without staying in a wfi loop in some special memory?
I maybe ask too much for the first incarnation of the code, but that is how I would like to eventually see it. AFAIK Linux prefers PSCI over platform-defined SMP code, so this should work out of the box.
Not really. So far, it will use the the platform SMP code if it is defined, and fallback to PSCI otherwise.
Indeed. So I was mistaken on this. I thought it worked this way once at least with Highbank/Midway (could have been an internal branch, though).
There were patches from Stephen Boyd to use the "enable-method" property in the cpu node to select PSCI though. I need to ping him on that.
That one that is mandatory on ARM64? Makes sense, then.
Regards, Andre.
- Is the use of TPIDRPRW & Co. really safe? It looks like as we seem to
be the only secure user (and they are banked), but I am just curious whether there is any "prior art" in using those registers temporarily.
As you said, we own secure mode entirely. So they really are scratch registers, free to be used.
... The kernel now boots in HYP mode, finds its secondary CPU without any additional SMP code, and runs KVM out of the box. Hopefully, the Xen/ARM guys can do the same fairly easily.
BTW: Yesterday my PSCI host patches for Xen have been committed, so Xen should be able to use that feature just like the kernel does.
Excellent! I really need to sort these patches out and repost the whole series...
Thanks,
M.
On 06/12/13 13:03, Andre Przywara wrote:
Yes, there is indeed this problem with the pen. Maybe one can use your upcoming relocation code to move the pen to a more secure place (defined per platform).
Yes, that's what I've done. Also rewritten some of it to be able to execute solely in secure mode (you can't do SMC+HVC there, as you'd try to run non-secure from secure RAM...).
With "avoid dealing with a pen" you mean to wakeup from the system's pen and immediately jump to the OS init code, without staying in a wfi loop in some special memory?
Even better, waking up the CPUs from power-off directly, configuring the per-cpu part of the GIC and jumping to the kernel. Almost nothing.
M.
participants (10)
-
Andre Przywara -
Anup Patel -
Anup Patel -
Christoffer Dall -
Ian Campbell -
Marc Zyngier -
Masahiro Yamada -
Rob Herring -
TigerLiu＠via-alliance.com -
TigerLiu＠viatech.com.cn