[U-Boot] [PATCH] ARM: enable ARMv7 virt support for the Arndale board
From: Andre Przywara andre.przyw...@linaro.org
To enable hypervisors utilizing the ARMv7 virtualization extension
on the Arndale board, add the simple SMP pen address writer function
and add the required configuration variables to switch all cores to
HYP mode before launching the OS.
This allows booting KVM and Xen directly from u-boot.
Reviewed-by: Christoffer Dall christoffer.d...@linaro.org
Signed-off-by: Andre Przywara andre.przyw...@linaro.org
---
Note that Andre previously reported a board reset on initial boot with
this patch, but I have applied this patch on upstream master of U-Boot
and tested on two separate Arndale boards and it boots perfectly fine
and lets you use KVM, so I recommend taking this patch as is.
board/samsung/arndale/arndale.c | 10 ++
include/configs/arndale.h | 8
2 files changed, 18 insertions(+)
diff --git a/board/samsung/arndale/arndale.c b/board/samsung/arndale/arndale.c
index ef88314..83fd3bd 100644
--- a/board/samsung/arndale/arndale.c
+++ b/board/samsung/arndale/arndale.c
@@ -117,3 +117,13 @@ int checkboard(void)
return 0;
}
#endif
+
+#ifdef CONFIG_S5P_PA_SYSRAM
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+ writel(addr, CONFIG_S5P_PA_SYSRAM);
+
+ /* make sure this write is really executed */
+ __asm__ volatile (dsb\n);
+}
+#endif
diff --git a/include/configs/arndale.h b/include/configs/arndale.h
index 64b54ab..75f9933 100644
--- a/include/configs/arndale.h
+++ b/include/configs/arndale.h
@@ -250,4 +250,12 @@
/* Enable Time Command */
#define CONFIG_CMD_TIME
+#define CONFIG_S5P_PA_SYSRAM 0x0202
+#define CONFIG_SMP_PEN_ADDRCONFIG_S5P_PA_SYSRAM
+
+/* The PERIPHBASE in the CBAR register is wrong on the Arndale, so override it
*/
+#define CONFIG_ARM_GIC_BASE_ADDRESS0x1048
+
+#define CONFIG_ARMV7_VIRT
+
#endif /* __CONFIG_H */
--
1.8.5.2
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] ARM: enable ARMv7 virt support for the Arndale board
On 1 August 2014 14:46, Andreas Färber afaer...@suse.de wrote: Hi, Am 01.08.2014 13:35, schrieb Christoffer Dall: From: Andre Przywara andre.przyw...@linaro.org To enable hypervisors utilizing the ARMv7 virtualization extension on the Arndale board, add the simple SMP pen address writer function and add the required configuration variables to switch all cores to HYP mode before launching the OS. This allows booting KVM and Xen directly from u-boot. Reviewed-by: Christoffer Dall christoffer.d...@linaro.org Signed-off-by: Andre Przywara andre.przyw...@linaro.org You forgot to sign off the patch. But nice to see progress on this! Can you just add when applying the patch? Signed-off-by: Christoffer Dall christoffer.d...@linaro.org ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] ARM: enable ARMv7 virt support for the Arndale board
On 1 August 2014 14:59, Andre Przywara andre.przyw...@arm.com wrote: On 01/08/14 13:53, Christoffer Dall wrote: On 1 August 2014 14:46, Andreas Färber afaer...@suse.de wrote: Hi, Am 01.08.2014 13:35, schrieb Christoffer Dall: From: Andre Przywara andre.przyw...@linaro.org To enable hypervisors utilizing the ARMv7 virtualization extension on the Arndale board, add the simple SMP pen address writer function and add the required configuration variables to switch all cores to HYP mode before launching the OS. This allows booting KVM and Xen directly from u-boot. Reviewed-by: Christoffer Dall christoffer.d...@linaro.org Signed-off-by: Andre Przywara andre.przyw...@linaro.org You forgot to sign off the patch. But nice to see progress on this! Can you just add when applying the patch? Signed-off-by: Christoffer Dall christoffer.d...@linaro.org Andreas, I wonder if you could change (or add) my new address, as my Linaro address does not work any more (just tested). Signed-off-by: Andre Przywara andre.przyw...@arm.com So I actually did not know the policy here, given that you did the work under your linaro.org address, so I decided to keep this as they were and just cc your arm address. If you're fine with using the @arm.com address I can respin the patch with the right addresses and signed-off-by's. -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] ARM: enable ARMv7 virt support for the Arndale board
On 1 August 2014 15:29, Andre Przywara andre.przyw...@arm.com wrote: On 01/08/14 14:02, Christoffer Dall wrote: On 1 August 2014 14:59, Andre Przywara andre.przyw...@arm.com wrote: On 01/08/14 13:53, Christoffer Dall wrote: On 1 August 2014 14:46, Andreas Färber afaer...@suse.de wrote: Hi, Am 01.08.2014 13:35, schrieb Christoffer Dall: From: Andre Przywara andre.przyw...@linaro.org To enable hypervisors utilizing the ARMv7 virtualization extension on the Arndale board, add the simple SMP pen address writer function and add the required configuration variables to switch all cores to HYP mode before launching the OS. This allows booting KVM and Xen directly from u-boot. Reviewed-by: Christoffer Dall christoffer.d...@linaro.org Signed-off-by: Andre Przywara andre.przyw...@linaro.org You forgot to sign off the patch. But nice to see progress on this! Can you just add when applying the patch? Signed-off-by: Christoffer Dall christoffer.d...@linaro.org Andreas, I wonder if you could change (or add) my new address, as my Linaro address does not work any more (just tested). Signed-off-by: Andre Przywara andre.przyw...@arm.com So I actually did not know the policy here, given that you did the work under your linaro.org address, so I decided to keep this as they were and just cc your arm address. If you're fine with using the @arm.com address I can respin the patch with the right addresses and signed-off-by's. I don't know about the policy either - actually I hoped you could help here ;-) I am perfectly fine with giving Linaro as a sponsor credit for this, but I am just concerned about potential feedback, which would bounce and at best annoy people. As long as a post goes to a list also I catch it, but private only email would be stuck. This said I am fine both ways - maybe we keep both addresses for reference in? Andreas, could you just add Christoffer's Signed-off-by and my Signed-off-by @arm.com without a respin? Andreas pointed out that he's not the maintainer, so let's see what the maintainer says :) all this policy stuff seems to be bikeshedding anyhow. -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/9] ARM: HYP/non-sec switch in bootm.c
On 29 December 2013 19:10, tiger...@viatech.com.cn wrote: 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? Yes, the aim is to be able to reuse as much of this code for as many platforms as possible. 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. 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/9] ARM: HYP/non-sec switch in bootm.c
On 29 December 2013 21:15, tiger...@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: 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). 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [linux-sunxi] Re: [PATCH 0/9] ARMv7: add PSCI support to u-boot
On Fri, Nov 22, 2013 at 02:30:27PM +0530, Anup Patel wrote: On Fri, Nov 22, 2013 at 2:12 PM, Ian Campbell i...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/9] ARM: HYP/non-sec: add a barrier after setting SCR.NS==1
On Fri, Nov 22, 2013 at 10:56:05AM +, Marc Zyngier wrote: On 22/11/13 01:51, Christoffer Dall wrote: On 21 November 2013 00:59, Marc Zyngier marc.zyng...@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.zyng...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/9] ARM: HYP/non-sec: add a barrier after setting SCR.NS==1
On 21 November 2013 00:59, Marc Zyngier marc.zyng...@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.zyng...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 0/9] ARMv7: add PSCI support to u-boot
On 21 November 2013 07:04, Marc Zyngier marc.zyng...@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.zyng...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Question re HYP mode and IRQ/FIQ stack setting
On 12 November 2013 22:22, Albert ARIBAUD albert.u.b...@aribaud.net wrote: Hi Christoffer, On Tue, 12 Nov 2013 14:34:00 -0800, Christoffer Dall christoffer.d...@linaro.org wrote: On 12 November 2013 13:29, Albert ARIBAUD albert.u.b...@aribaud.net wrote: Hi Christoffer, On Tue, 12 Nov 2013 09:09:23 -0800, Christoffer Dall christoffer.d...@linaro.org wrote: I suspect that if you are in Hyp mode, you should not worry about FIQ/IRQ mode, but just make sure to configure Hyp mode properly to handle interrupts. (it's a separate entry in the exception vector and you probably need to look at the HSR register whn you've taken an interrupt). So, as Andre suggests below, it depends on your use case. Ok, so les me try to sum that up from my perspective (IRQ and FIQ stacks in U-Boot): Some ARM U-Boot targets run in SVC32 mode, and some in HYP mode. For targets which run in SVC32 mode: aborts execute in abort mode, IRQs in IRQ mode, FIQs in FIQ mode, etc., each mode having its own stack. correct For targets which run in HYP mode: aborts, FIQ and IRQs all run in HYP mode, using always the same stack. correct. In both types of targers: prefetch and data aborts, FIQs and IRQs execute through the usual vectors; in SVC32 mode, because there's no other way; in HYP mode, because exceptions occurring while in HYP mode use the normal vector, not the HYP vector. Correct? no, this sounds fishy. Look at the ARM ARMv7 (DDI 0406C.b) page B1-1166. Table B1-3 will tell you what you need to know. Hyp mode has its own vectors, pointed to by the HVBAR control register. I strongly suggest you familiarize yourself with these parts of the ARM ARM before writing code to that effect. To clarify my meaning: I wasn't speaking about the exceptions base address, but about the exception vector offset. IOW, I did not mean to say that in HYP mode, aborts, FIQs, IRQs etc use the SVC32 table of vectors rather than the HYP table. I meant to say that e.g. an IRQ will fire at offset 0x18, whether it fires while from SVC32 into IRQ mode, or from HYP mode into HYP mode. This seemed consistent with figure B1-8 on page B1-1179 of DDI 0406C.b, respectively first and sixth exit box on the right. Is this clearer and more correct? yes, now I understand. It's correct. If so, then U-Boot stack requirements for SVC32 and HYP mode are as follows: in SVC32, we need separate IRQ and FIQ stacks, and the main (SVC32) stack does not have to accommodate for interrupt handler context storage. In HYP mode, the only stack is the HYP one and exception handlers will use it too, so it has to accommodate for their context storage. That is correct, your Hyp mode stack should always be valid (have a page or so, like the kernel) and then you can always push things on there, even when taking an exception. Thanks Christoffer! This should allow me to refactor interrupt stack settings as I intended. -Christoffer Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Question re HYP mode and IRQ/FIQ stack setting
On 12 November 2013 03:41, Albert ARIBAUD albert.u.b...@aribaud.net wrote: (Cc:ing Andre and Christoffer as they have discussed HYP on the ML.) Hello, I am working on changing the way IRQ/FIQ stacks are set up, from on-the-fly in a hurry while in the handler to during init, so that when entering the handler, the stack is already correct. Setting the stack then requires switching from the current mode (in most cases, SVC32, 0x13) to IRQ (0x11) or FIQ (0x12) mode, in order to set the right banked SP, then back into the original mode. However, in the first lines of reset in arch/arm/cpu/armv7/start.S, the possibility of U-Boot being started in HYP mode (0x1A) is considered and, if in HYP mode, no switch to SVC32 is performed. I understand that the problem here is, if we drop from HYP to SVC32, then we cannot go back to HYP, and we want to be able to remain in HYP. correct (not without setting up a trap handler in Hyp mode and trapping to Hyp mode) Does this also apply to dropping from HYP to IRQ or FIQ mode, i.e., if we do such a drop, are we prevented from rising back from IRQ or FIQ mode to HYP? I seem to remember such an issue, but I am no specialist in HYP, so any help is welcome. Yes, it also applies. Hyp is strictly more privileged (PL2) than all the PL1 modes (SVC, SYS, IRQ, FIQ, ABT, UND) and therefore requires a trap to go from PL1 to PL2 (basically this is how hardware protection works - just like with syscalls from user mode to PL1). You can use MSR and MRS instructions to access the IRQ and FIQ registers directly from Hyp mode though. -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Question re HYP mode and IRQ/FIQ stack setting
On 12 November 2013 08:53, Andre Przywara andre.przyw...@linaro.org wrote:
On 11/12/2013 05:28 PM, Christoffer Dall wrote:
On 12 November 2013 03:41, Albert ARIBAUD albert.u.b...@aribaud.net
wrote:
(Cc:ing Andre and Christoffer as they have discussed HYP on the ML.)
Hello,
I am working on changing the way IRQ/FIQ stacks are set up, from
on-the-fly in a hurry while in the handler to during init, so that
when entering the handler, the stack is already correct.
Setting the stack then requires switching from the current mode (in
most cases, SVC32, 0x13) to IRQ (0x11) or FIQ (0x12) mode, in order to
set the right banked SP, then back into the original mode.
However, in the first lines of reset in arch/arm/cpu/armv7/start.S, the
possibility of U-Boot being started in HYP mode (0x1A) is considered
and, if in HYP mode, no switch to SVC32 is performed.
I understand that the problem here is, if we drop from HYP to SVC32,
then we cannot go back to HYP, and we want to be able to remain in HYP.
Right, that is to keep the HYP mode in case the firmware already enabled it.
This is for instance the case on the new Calxeda Midway. Actually this
approach will become more widespread, since it is required to provide proper
PSCI support (which needs to run in secure state, so requires an even higher
privilege level than HYP: EL3 in the new ARM speak).
What Andre is referring to here is that the PSCI specs mandates the
CPUs that a are turned on using PSCI go through the highest privilege
level implemented in the non-secure state. I couldn't actually find
the place in the PSCI specs where this is also true for the boot CPU,
but granted, anything else will be quite strange.
So to stay in 32-bit ARMv7 terminology here, a likely case for systems
with PSCI support in the firmware is that all cores will enter U-Boot
in non-secure PL2 (hyp) mode and power management is done by calling
SMC calls to the firmware that sits in the secure monitor.
correct (not without setting up a trap handler in Hyp mode and
trapping to Hyp mode)
Does this also apply to dropping from HYP to IRQ or FIQ mode, i.e., if
we do such a drop, are we prevented from rising back from IRQ or FIQ
mode to HYP? I seem to remember such an issue, but I am no specialist
in HYP, so any help is welcome.
Yes, it also applies. Hyp is strictly more privileged (PL2) than all
the PL1 modes (SVC, SYS, IRQ, FIQ, ABT, UND) and therefore requires a
trap to go from PL1 to PL2 (basically this is how hardware protection
works - just like with syscalls from user mode to PL1).
Thanks Christoffer for clarifying this, I wasn't sure about FIQ, but of
course your explanation (EL1 vs. EL2) makes totally sense.
But I wonder what happens when we enter FIQ or IRQ due to an actual
interrupt. Will the CPU return into HYP mode when the handler returns?
That is subject to some HYP mode register configuration, right?
Not quite, interrupts in Hyp mode will always be taken to Hyp mode
(unless it's secure interrupts, which are taken to monitor mode). In
fact, when you have the virtualization extensions, you also have the
security extensions, and I think all FIQs go to the secure mode here,
so you can't ever see an FIQ in Hyp mode. I may be mistaken here,
someone should look it up and verify.
Now, you can configure Hyp mode to trap interrupts raised while in any
PL1 or PL0 mode to Hyp mode.
I suspect that if you are in Hyp mode, you should not worry about
FIQ/IRQ mode, but just make sure to configure Hyp mode properly to
handle interrupts. (it's a separate entry in the exception vector and
you probably need to look at the HSR register whn you've taken an
interrupt). So, as Andre suggests below, it depends on your use case.
You can use MSR and MRS instructions to access the IRQ and FIQ
registers directly from Hyp mode though.
Albert,
so does msr sp_{fiq,irq}, rn fix your problem? Or do you still need to
actually go into one of these modes for further setup?
Regards,
Andre.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Question re HYP mode and IRQ/FIQ stack setting
On 12 November 2013 13:29, Albert ARIBAUD albert.u.b...@aribaud.net wrote: Hi Christoffer, On Tue, 12 Nov 2013 09:09:23 -0800, Christoffer Dall christoffer.d...@linaro.org wrote: I suspect that if you are in Hyp mode, you should not worry about FIQ/IRQ mode, but just make sure to configure Hyp mode properly to handle interrupts. (it's a separate entry in the exception vector and you probably need to look at the HSR register whn you've taken an interrupt). So, as Andre suggests below, it depends on your use case. Ok, so les me try to sum that up from my perspective (IRQ and FIQ stacks in U-Boot): Some ARM U-Boot targets run in SVC32 mode, and some in HYP mode. For targets which run in SVC32 mode: aborts execute in abort mode, IRQs in IRQ mode, FIQs in FIQ mode, etc., each mode having its own stack. correct For targets which run in HYP mode: aborts, FIQ and IRQs all run in HYP mode, using always the same stack. correct. In both types of targers: prefetch and data aborts, FIQs and IRQs execute through the usual vectors; in SVC32 mode, because there's no other way; in HYP mode, because exceptions occurring while in HYP mode use the normal vector, not the HYP vector. Correct? no, this sounds fishy. Look at the ARM ARMv7 (DDI 0406C.b) page B1-1166. Table B1-3 will tell you what you need to know. Hyp mode has its own vectors, pointed to by the HVBAR control register. I strongly suggest you familiarize yourself with these parts of the ARM ARM before writing code to that effect. If so, then U-Boot stack requirements for SVC32 and HYP mode are as follows: in SVC32, we need separate IRQ and FIQ stacks, and the main (SVC32) stack does not have to accommodate for interrupt handler context storage. In HYP mode, the only stack is the HYP one and exception handlers will use it too, so it has to accommodate for their context storage. That is correct, your Hyp mode stack should always be valid (have a page or so, like the kernel) and then you can always push things on there, even when taking an exception. -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 7/8] ARM: extend non-secure switch to also go into HYP mode
On Thu, Oct 03, 2013 at 08:24:57AM +0200, Albert ARIBAUD wrote: Hi Andre, On Thu, 19 Sep 2013 18:06:45 +0200, Andre Przywara andre.przyw...@linaro.org wrote: For the KVM and XEN hypervisors to be usable, we need to enter the kernel in HYP mode. Now that we already are in non-secure state, HYP mode switching is within short reach. While doing the non-secure switch, we have to enable the HVC instruction and setup the HYP mode HVBAR (while still secure). The actual switch is done by dropping back from a HYP mode handler without actually leaving HYP mode, so we introduce a new handler routine in our new secure exception vector table. In the assembly switching routine we save and restore the banked LR and SP registers around the hypercall to do the actual HYP mode switch. The C routine first checks whether we are in HYP mode already and also whether the virtualization extensions are available. It also checks whether the HYP mode switch was finally successful. The bootm command part only calls the new function after the non-secure switch. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/Makefile | 2 +- arch/arm/cpu/armv7/nonsec_virt.S | 43 +++- arch/arm/cpu/armv7/virt-v7.c | 37 ++ arch/arm/include/asm/armv7.h | 6 -- arch/arm/lib/bootm.c | 7 ++- 5 files changed, 86 insertions(+), 9 deletions(-) Changes: v3..v4: w/s fixes, embed error output v4..v5: none Seems like Christoffer's comment was not addressed here but IIUC, it was in other files (Christoffer, feel free to comment). Any reason why the older asm comments form was not replaced in here? I think these comments are a bit superflous, but not exactly harmful, so I didn't raise the flag when they were not corrected. My thought was that if you're building for a board that has support for the virtualization extensions you should be using a toolchain that knows about them too, but Andre pointed out that his (I think Debian) still used an old enough cross toolchain not to have this support. In any case, I don't think this warrants holding back the patches but can be fixed as a follow-up if the community agrees that we need to support older toolchains by some define that encodes the hvc and eret instructions properly. Thanks, -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 3/8] ARM: add assembly routine to switch to non-secure state
On Thu, Sep 19, 2013 at 10:00:03PM +0530, Mj Embd wrote: Hi Andre, There is another approach taken in xen. (xen/arch/arm/mode_switch.S) Which do you think is the better approach Hi there, I'm not sure I completely understand your question. Do you think this patch series should be changed to take something from Xen? If so, can you please clarify why? For the record, this patch series has been reviewed and tested quite a lot for U-Boot and we would very much like to get this merged and avoid further churn, unless there's a compelling technical reason to change it. Thanks, -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 3/8] ARM: add assembly routine to switch to non-secure state
On Fri, Sep 20, 2013 at 01:27:48AM +0530, Mj Embd wrote: two quick points (a) xen already has a mode_switch code, so AFAIK xen might not use it (as suggested by comment in another patch in this patch set) For KVM the boot procedure for Hyp mode is quite clearly defined: the kernel must be booted in Hyp mode. I was under the impression that Xen wanted to use the same paradigm and rely on bootloaders to switch to Hyp mode and thereby get rid of the code in Xen. (b) There are 2 methods of switching from Secure to Hyp mode one you have proposed another implemented in xen. I was suggesting take the best approach Can you please be more specific? Not everyone here knows the Xen low-level mode switch details by heart. As far as I know, there is only one architecturally defined proper mode to switch from secure mode to non-secure mode, and the state that needs to be configured for Hyp-mode and NS-mode is well defined. Obviously two implementation can do things differently (different order, different programminge environment, etc.) but that doesn't mean one is better than the other. I think it would be more productive if you can simply look at this code and if you think some things are done more efficiently in Xen, please comment on that, which would be very helpful. I'm afraid there's no magic way to apply a block of Xen code into U-Boot wihtout manual adjustment anyway, or the other way around for that matter. -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 2/8] ARM: add secure monitor handler to switch to non-secure state
On Fri, Sep 20, 2013 at 03:20:15AM +0530, Mj Embd wrote: Just checking, is the mcr p15,0,r1,c1,c1,0 in sync with the following text . I could be wrong here, just checking In the future, if you can comment specifically inline on the lines of code you are targeting, it is easier for other people to address your concerns. B1.5.1 Arm Arch Ref Manual - To avoid security holes, software must not: - — Change from Secure to Non-secure state by using an MSR or CPS instruction to switch from Monitor The important part here is that we don't change from S to NS by modifying the SCR, because monitor mode is always in secure mode, so the change only happens on the exception return. So yes, it's safe. -Christoffer mode to some other mode while SCR.NS is 1. - — Use an MCR instruction that writes SCR.NS to change from Secure to Non-secure state. This means ARM recommends that software does not alter SCR.NS in any mode except Monitor mode. ARM deprecates changing SCR.NS in any other mode. On Thu, Sep 19, 2013 at 9:36 PM, Andre Przywara andre.przyw...@linaro.orgwrote: A prerequisite for using virtualization is to be in HYP mode, which requires the CPU to be in non-secure state first. Add a new file in arch/arm/cpu/armv7 to hold a monitor handler routine which switches the CPU to non-secure state by setting the NS and associated bits. According to the ARM architecture reference manual this should not be done in SVC mode, so we have to setup a SMC handler for this. We create a new vector table to avoid interference with other boards. The MVBAR register will be programmed later just before the smc call. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/Makefile | 4 +++ arch/arm/cpu/armv7/nonsec_virt.S | 54 2 files changed, 58 insertions(+) create mode 100644 arch/arm/cpu/armv7/nonsec_virt.S Changes: v3..v4: clarify comments, w/s fixes v4..v5: remove unneeded padding in the exception table diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile index b723e22..3466c7a 100644 --- a/arch/arm/cpu/armv7/Makefile +++ b/arch/arm/cpu/armv7/Makefile @@ -20,6 +20,10 @@ ifneq ($(CONFIG_AM43XX)$(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CON SOBJS += lowlevel_init.o endif +ifneq ($(CONFIG_ARMV7_NONSEC),) +SOBJS += nonsec_virt.o +endif + SRCS := $(START:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(COBJS) $(SOBJS)) START := $(addprefix $(obj),$(START)) diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S new file mode 100644 index 000..c21bca3 --- /dev/null +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -0,0 +1,54 @@ +/* + * code for switching cores into non-secure state + * + * Copyright (c) 2013 Andre Przywara andre.przyw...@linaro.org + * + * 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 + */ + +#include config.h + +/* the vector table for secure state */ +_monitor_vectors: + .word 0 /* reset */ + .word 0 /* undef */ + adr pc, _secure_monitor + .word 0 + .word 0 + .word 0 + .word 0 + .word 0 + +/* + * secure monitor handler + * U-boot calls this software interrupt in start.S + * This is executed on a smc instruction, we use a smc #0 to switch + * 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 + orr r1, r1, #0x31 @ enable NS, AW, FW bits + + mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set) + + movspc, lr @ return to non-secure SVC + -- 1.7.12.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 2/8] ARM: add secure monitor handler to switch to non-secure state
On Fri, Sep 20, 2013 at 08:08:45AM +0530, Mj Embd wrote: On Fri, Sep 20, 2013 at 6:12 AM, Christoffer Dall christoffer.d...@linaro.org wrote: On Fri, Sep 20, 2013 at 03:20:15AM +0530, Mj Embd wrote: Just checking, is the mcr p15,0,r1,c1,c1,0 in sync with the following text . I could be wrong here, just checking In the future, if you can comment specifically inline on the lines of code you are targeting, it is easier for other people to address your concerns. B1.5.1 Arm Arch Ref Manual - To avoid security holes, software must not: - — Change from Secure to Non-secure state by using an MSR or CPS instruction to switch from Monitor The important part here is that we don't change from S to NS by modifying the SCR, because monitor mode is always in secure mode, so the change only happens on the exception return. So yes, it's safe. -Christoffer Ok. Good Discussion. Thanks, PS: Gmail auto wraps the previous msg in 3 dots, so sometimes I miss inlining. Thanks for pointing out. No problem, thanks for looking at the code. -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 0/8] ARMv7: Add HYP mode switching support
On Fri, Aug 16, 2013 at 03:53:01PM +0200, Andre Przywara wrote: [...] Albert, Tom, do you need more ACKs or Reviewed-bys? Albert, Tom, Can you let us know if you will accept a pull request for these patches? They look to be in pretty good shape? Thanks! -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 0/8] ARMv7: Add HYP mode switching support
On Mon, Aug 26, 2013 at 05:30:14PM -0400, Tom Rini wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 08/26/2013 04:51 PM, Christoffer Dall wrote: On Fri, Aug 16, 2013 at 03:53:01PM +0200, Andre Przywara wrote: [...] Albert, Tom, do you need more ACKs or Reviewed-bys? Albert, Tom, Can you let us know if you will accept a pull request for these patches? They look to be in pretty good shape? It looks good to me, I just need Albert to pick them up. Thanks! Great! Do you need a pull request or do you just apply the patches from the mailing list? Thanks, -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 7/8] ARM: extend non-secure switch to also go into HYP mode
On Fri, Aug 09, 2013 at 05:03:11PM +0200, Andre Przywara wrote: For the KVM and XEN hypervisors to be usable, we need to enter the kernel in HYP mode. Now that we already are in non-secure state, HYP mode switching is within short reach. While doing the non-secure switch, we have to enable the HVC instruction and setup the HYP mode HVBAR (while still secure). The actual switch is done by dropping back from a HYP mode handler without actually leaving HYP mode, so we introduce a new handler routine in our new secure exception vector table. In the assembly switching routine we save and restore the banked LR and SP registers around the hypercall to do the actual HYP mode switch. The C routine first checks whether we are in HYP mode already and also whether the virtualization extensions are available. It also checks whether the HYP mode switch was finally successful. The bootm command part only adds and adjusts some error reporting. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/Makefile | 2 +- arch/arm/cpu/armv7/nonsec_virt.S | 43 +++- arch/arm/cpu/armv7/virt-v7.c | 37 ++ arch/arm/include/asm/armv7.h | 6 -- arch/arm/lib/bootm.c | 7 ++- 5 files changed, 86 insertions(+), 9 deletions(-) diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile index 5813e87..c20df3d 100644 --- a/arch/arm/cpu/armv7/Makefile +++ b/arch/arm/cpu/armv7/Makefile @@ -36,7 +36,7 @@ ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONF SOBJS+= lowlevel_init.o endif -ifneq ($(CONFIG_ARMV7_NONSEC),) +ifneq ($(CONFIG_ARMV7_NONSEC)$(CONFIG_ARMV7_VIRT),) SOBJS+= nonsec_virt.o COBJS+= virt-v7.o endif diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index a88fa6b..fb1651d 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -1,5 +1,5 @@ /* - * code for switching cores into non-secure state + * code for switching cores into non-secure state and into HYP mode * * Copyright (c) 2013Andre Przywara andre.przyw...@linaro.org * @@ -28,15 +28,16 @@ #include asm/armv7.h .arch_extension sec +.arch_extension virt -/* the vector table for secure state */ +/* the vector table for secure state and HYP mode */ _monitor_vectors: .word 0 /* reset */ .word 0 /* undef */ adr pc, _secure_monitor .word 0 .word 0 - .word 0 + adr pc, _hyp_trap .word 0 .word 0 .word 0 /* pad */ @@ -54,10 +55,27 @@ _secure_monitor: bic r1, r1, #0x4e @ clear IRQ, FIQ, EA, nET bits orr r1, r1, #0x31 @ enable NS, AW, FW bits +#ifdef CONFIG_ARMV7_VIRT + mrc p15, 0, r0, c0, c1, 1 @ read ID_PFR1 + and r0, r0, #CPUID_ARM_VIRT_MASK@ mask virtualization bits + cmp r0, #(1 CPUID_ARM_VIRT_SHIFT) + orreq r1, r1, #0x100 @ allow HVC instruction +#endif + mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set) +#ifdef CONFIG_ARMV7_VIRT + mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value + mcreq p15, 4, r0, c12, c0, 0 @ write HVBAR +#endif + movspc, lr @ return to non-secure SVC +_hyp_trap: + mrs lr, elr_hyp @ for older asm: .byte 0x00, 0xe3, 0x0e, 0xe1 I see you kep this as is, oh well. + mov pc, lr @ do no switch modes, but + @ return to caller + /* * 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 @@ -72,9 +90,13 @@ ENTRY(_smp_pen) mcr p15, 0, r1, c12, c0, 0 @ set VBAR bl _nonsec_init + mov r12, r0 @ save GICC address +#ifdef CONFIG_ARMV7_VIRT + bl _switch_to_hyp +#endif - ldr r1, [r0, #GICC_IAR] @ acknowledge IPI - str r1, [r0, #GICC_EOIR]@ signal end of interrupt + ldr r1, [r12, #GICC_IAR]@ acknowledge IPI + str r1, [r12, #GICC_EOIR] @ signal end of interrupt adr r0, _smp_pen@ do not use this address again b smp_waitloop@ wait for IPIs, board specific @@ -161,3 +183,14 @@ ENTRY(_nonsec_init) bx lr ENDPROC(_nonsec_init) + +ENTRY(_switch_to_hyp) + mov r0, lr + mov r1, sp @ save SVC copy of LR and SP + isb + hvc #0 @ for older asm: .byte 0x70, 0x00, 0x40, 0xe1 + mov sp, r1 + mov lr, r0
Re: [U-Boot] [PATCH v4 0/8] ARMv7: Add HYP mode switching support
On Fri, Aug 09, 2013 at 05:03:04PM +0200, Andre Przywara wrote: (for GIT URL and Changelog see below) ARM CPUs with the virtualization extension have a new mode called HYP mode, which allows hypervisors to safely control and monitor guests. The current hypervisor implementations (KVM and Xen) require the kernel to be entered in that HYP mode. This patch series introduces a configuration variable CONFIG_ARMV7_VIRT which enables code to switch all cores into HYP mode. This is done automatically during execution of the bootm command. The process of switching into HYP mode requires the CPU to be in secure state initially when entering u-boot, it will then setup some register and switch to non-secure state. This requires the GIC to be programmed properly first. Explanations about the details are in the commit messages of the respective patches. The patches are structured like this: 1/8: prepare header file 2/8: add monitor handler (assembly) 3/8: add per CPU non-secure switch routine (assembly) 4/8: add system wide non-secure setup (C) 5/8: trigger non-secure switch during bootm command 6/8: add generic SMP functionality 7/8: add HYP mode switching 8/8: board specific code for ARM Versatile Express TC2 Since up to patch 6/8 this code works on non-virtualization capable CPUs also and there has been a request, there is now a second configuration variable CONFIG_ARMV7_NONSEC, which omits the final HYP mode switch and just goes into non-secure SVC state. You can specify either (or none) of them, the code cares about the dependency. The code aims to be as generic as possible, though currently it has only been tested on the Versatile Express TC-2 board. The last patch thus enables the feature for that board and should serve as an example for supporting other boards. For convenience there is a GIT tree which you can pull these patches from (hypmode_v4 branch): git://git.linaro.org/people/aprzywara/u-boot.git Changes RFC..v1 * not a dedicated command anymore, code run by bootm friends * protecting code with #ifdefs to avoid unnecessary inclusion and accidental crashing (when accessing restricted registers) * moving prototypes to header file to meet checkpatch recommendation * adding comment as proposed by Christoffer Changes v1..v2 mostly style and code layout changes * restructure assembly code to live in a new file and not start.S * split smp, nonsec_init and hyp_init to be separate functions * used named constants from common header files * split C function to be more readable * extend comments to be more precise and elaborate * add provision to override GIC base address (needed for Arndale?) * add configuration variable to enable VExpress specific SMP code * use writel/readl for MMIO GIC accesses * remove superfluous isb instructions * more minor fixes Changes v2..v3 * fix clobbering of GICC address actually spoiling the stack * do CNTFRQ setup in assembly per core (and not only once per SoC) * moving the new code files into arch/arm/cpu/armv7 * add config variable for doing non-secure switch only * use actual HYP and secure instructions mnemonics instead of the encoded byte sequence. This requires more recent compilers. * make the identification of the CPU core more robust and saner * use enum for error codes and rename them * lots of smaller layout and style fixes Changes v3..v4 * mask reserved bits in CBAR register * move the VExpress board specific SMP code into the board directory * embed error reporting in the respective functions and getting rid of the error code enum at all (by popular demand ;-) * minor style fixes Please review and comment! Only had those very few nits sent separately, which can be fixed later if we want. Otherwise looks good. Reviewed-by: Christoffer Dall christoffer.d...@linaro.org Thanks, -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4 4/8] ARM: add C function to switch to non-secure state
On Fri, Aug 09, 2013 at 05:03:08PM +0200, Andre Przywara wrote:
The core specific part of the work is done in the assembly routine
in nonsec_virt.S, introduced with the previous patch, but for the full
glory we need to setup the GIC distributor interface once for the
whole system, which is done in C here.
The routine is placed in arch/arm/cpu/armv7 to allow easy access from
other ARMv7 boards.
We check the availability of the security extensions first.
Since we need a safe way to access the GIC, we use the PERIPHBASE
registers on Cortex-A15 and A7 CPUs and do some sanity checks.
Boards not implementing the CBAR can override this value via a
configuration file variable.
Then we actually do the GIC enablement:
a) enable the GIC distributor, both for non-secure and secure state
(GICD_CTLR[1:0] = 11b)
b) allow all interrupts to be handled from non-secure state
(GICD_IGROUPRn = 0x)
The core specific GIC setup is then done in the assembly routine.
Signed-off-by: Andre Przywara andre.przyw...@linaro.org
---
arch/arm/cpu/armv7/Makefile | 1 +
arch/arm/cpu/armv7/virt-v7.c | 121
+++
arch/arm/include/asm/armv7.h | 3 ++
3 files changed, 125 insertions(+)
create mode 100644 arch/arm/cpu/armv7/virt-v7.c
diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
index 11a8ad5..5813e87 100644
--- a/arch/arm/cpu/armv7/Makefile
+++ b/arch/arm/cpu/armv7/Makefile
@@ -38,6 +38,7 @@ endif
ifneq ($(CONFIG_ARMV7_NONSEC),)
SOBJS+= nonsec_virt.o
+COBJS+= virt-v7.o
endif
SRCS := $(START:.o=.S) $(COBJS:.o=.c)
diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
new file mode 100644
index 000..689023f
--- /dev/null
+++ b/arch/arm/cpu/armv7/virt-v7.c
@@ -0,0 +1,121 @@
+/*
+ * (C) Copyright 2013
+ * Andre Przywara, Linaro
+ *
+ * Routines to transition ARMv7 processors from secure into non-secure state
+ * needed to enable ARMv7 virtualization for current hypervisors
+ *
+ * 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
+ */
+
+#include common.h
+#include asm/armv7.h
+#include asm/gic.h
+#include asm/io.h
+
+unsigned long gic_dist_addr;
+
+static unsigned int read_id_pfr1(void)
+{
+ unsigned int reg;
+
+ asm(mrc p15, 0, %0, c0, c1, 1\n : =r(reg));
+ return reg;
+}
+
+static unsigned long get_gicd_base_address(void)
+{
+#ifdef CONFIG_ARM_GIC_BASE_ADDRESS
+ return CONFIG_ARM_GIC_BASE_ADDRESS + GIC_DIST_OFFSET;
+#else
+ unsigned midr;
+ unsigned periphbase;
+
+ /* check whether we are an Cortex-A15 or A7.
+ * The actual HYP switch should work with all CPUs supporting
+ * the virtualization extension, but we need the GIC address,
+ * which we know only for sure for those two CPUs.
+ */
+ asm(mrc p15, 0, %0, c0, c0, 0\n : =r(midr));
+ switch (midr MIDR_PRIMARY_PART_MASK) {
+ case MIDR_CORTEX_A9_R0P1:
+ case MIDR_CORTEX_A15_R0P0:
+ case MIDR_CORTEX_A7_R0P0:
+ break;
+ default:
+ printf(nonsec: could not determine GIC address.\n);
+ return -1;
+ }
+
+ /* get the GIC base address from the CBAR register */
+ asm(mrc p15, 4, %0, c15, c0, 0\n : =r (periphbase));
+
+ /* the PERIPHBASE can be mapped above 4 GB (lower 8 bits used to
+ * encode this). Bail out here since we cannot access this without
+ * enabling paging.
+ */
+ if ((periphbase 0xff) != 0) {
+ printf(nonsec: PERIPHBASE is above 4 GB, no access.\n);
+ return -1;
+ }
+
+ return (periphbase CBAR_MASK) + GIC_DIST_OFFSET;
+#endif
+}
+
+int armv7_switch_nonsec(void)
+{
+ unsigned int reg;
+ unsigned itlinesnr, i;
+
+ /* check whether the CPU supports the security extensions */
+ reg = read_id_pfr1();
+ if ((reg 0xF0) == 0) {
+ printf(nonsec: Security extensions not implemented.\n);
+ return -1;
+ }
+
+ /* the SCR register will be set directly in the monitor mode handler,
+ * according to the spec one should not tinker with it in secure state
+
Re: [U-Boot] [PATCH v3 5/7] ARM: add SMP support for non-secure switch
On Tue, Jul 30, 2013 at 01:51:33PM +0200, Andre Przywara wrote:
On 07/30/2013 12:02 AM, Christoffer Dall wrote:
On Wed, Jul 10, 2013 at 01:54:17AM +0200, Andre Przywara wrote:
Currently the non-secure switch is only done for the boot processor.
To enable full SMP support, we have to switch all secondary cores
into non-secure state also.
So we add an entry point for secondary CPUs coming out of low-power
state and make sure we put them into WFI again after having switched
to non-secure state.
For this we acknowledge and EOI the wake-up IPI, then go into WFI.
Once being kicked out of it later, we sanity check that the start
address has actually been changed (since another attempt to switch
to non-secure would block the core) and jump to the new address.
The actual CPU kick is done by sending an inter-processor interrupt
via the GIC to all CPU interfaces except the requesting processor.
The secondary cores will then setup their respective GIC CPU
interface.
The address secondary cores jump to is board specific, we provide
the value here for the Versatile Express board.
Signed-off-by: Andre Przywara andre.przyw...@linaro.org
---
arch/arm/cpu/armv7/nonsec_virt.S| 27 +++
arch/arm/cpu/armv7/virt-v7.c| 19 ++-
arch/arm/include/asm/armv7.h| 1 +
arch/arm/include/asm/gic.h | 2 ++
include/configs/vexpress_ca15_tc2.h | 3 +++
5 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S
b/arch/arm/cpu/armv7/nonsec_virt.S
index e9ee831..f9b6b39 100644
--- a/arch/arm/cpu/armv7/nonsec_virt.S
+++ b/arch/arm/cpu/armv7/nonsec_virt.S
@@ -58,6 +58,33 @@ _secure_monitor:
movspc, lr @ return to non-secure SVC
/*
+ * 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
+ * code has already been executed by a C function before.
+ * Then they go back to wfi and wait to be woken up by the kernel again.
+ */
+ENTRY(_smp_pen)
+ mrs r0, cpsr
+ orr r0, r0, #0xc0
+ msr cpsr, r0@ disable interrupts
+ ldr r1, =_start
+ mcr p15, 0, r1, c12, c0, 0 @ set VBAR
+
+ bl _nonsec_init
+
+ ldr r1, [r0, #GICC_IAR] @ acknowledge IPI
+ str r1, [r0, #GICC_EOIR]@ signal end of interrupt
+ adr r1, _smp_pen
+waitloop:
+ wfi
+ ldr r0, =CONFIG_SYSFLAGS_ADDR @ load start address
You seem to have ignored my comment about using the sysflags name?
As I understand, the sysflags name is a versatile express specific
register name that just happens to be used for the SMP boot address as
well...
Therefore, this should really be CONFIG_SMP_BOOT_ADDR or something like
that, at the very least.
+ ldr r0, [r0]
+ cmp r0, r1 @ make sure we dont execute this code
+ beq waitloop@ again (due to a spurious wakeup)
+ mov pc, r0
+ENDPROC(_smp_pen)
+
+/*
* Switch a core to non-secure state.
*
* 1. initialize the GIC per-core interface
diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
index 54f9746..a0d0b34 100644
--- a/arch/arm/cpu/armv7/virt-v7.c
+++ b/arch/arm/cpu/armv7/virt-v7.c
@@ -77,6 +77,21 @@ static int get_gicd_base_address(unsigned int *gicdaddr)
#endif
}
+static void kick_secondary_cpus(unsigned int gicdaddr)
+{
+ unsigned int *secondary_boot_addr;
+
+ secondary_boot_addr = (void *)CONFIG_SYSFLAGS_ADDR;
+#ifdef CONFIG_SYSFLAGS_NEED_CLEAR_BITS
+ secondary_boot_addr[1] = (unsigned)-1;
+#endif
again, if you disagreed with my previous comment, please comment on it
and rationalize your choice.
I am sorry, I thought I reasoned that already in an earlier mail:
I don't want to introduce abstraction prematurely, so I wanted to
refactor the code as soon as a second board gets supported. Which
will probably be the Arndale, and also probably done by me ...
Hmm, a big motivation for doing this is to make it clear what new boards
need to do to support Hyp mode, and make it as easy as possible. We
have already made the choice to make this code generic in virt-v7.c as
opposed to vexpress.c (or whatever that file would be called), so I
don't really understand your argument about being premature, to be
completely honest.
I still feel you're wrapping board specific logic into generic code, and
that you should call out to a more generic function. Imagine an SOC
that uses an implementation defined control register for this instead of
a memory address...
Well, I can imagine quite some ways to do this, but in fact I'd like
to focus on things that really exist ;-)
My fear is that any abstraction I introduce now will not suffice for
a future board and needs to be refactored then anyway.
Sorry
Re: [U-Boot] [PATCH v3 6/7] ARM: extend non-secure switch to also go into HYP mode
On Tue, Jul 30, 2013 at 01:59:29PM +0200, Andre Przywara wrote: On 07/30/2013 12:02 AM, Christoffer Dall wrote: On Wed, Jul 10, 2013 at 01:54:18AM +0200, Andre Przywara wrote: [...] +_hyp_trap: + mrs lr, elr_hyp @ for older asm: .byte 0x00, 0xe3, 0x0e, 0xe1 this comment just confuses: either make it intelligent to support an older compiler or just get rid of these byte encodings. You can always disassemble the file and lookup the byte code with a modern compiler to get back to the byte encoding. Well, I used a Debian 6 cross compiler before, which didn't support these instructions. After your remark I updated the system to Debian 7, but found it not appropriate to ask any user to do the same just to use a fixed, non-parametrized assembly instruction. I have the feeling that there are quite some users out there who cannot and don't want to easily update their compiler. So I decided to leave the workaround in the comment to give a hint to a quick fix. ok, so if Debian's built-in cross compilers are indeed that old and we want to support those (that's ok with me), then let's fix it properly. By making it intelligent you mean a macro which does some version checking and inserts the .byte sequence if needed? Are there any archetypes of such code? Yes, see arch/arm/include/asm/opcodes-*.h -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 4/7] ARM: switch to non-secure state during bootm execution
On Tue, Jul 30, 2013 at 01:32:14PM +0200, Andre Przywara wrote:
On 07/30/2013 12:02 AM, Christoffer Dall wrote:
On Wed, Jul 10, 2013 at 01:54:16AM +0200, Andre Przywara wrote:
[...]
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 1b6e0ac..7b0619e 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -34,6 +34,10 @@
#include asm/bootm.h
#include linux/compiler.h
+#ifdef CONFIG_ARMV7_NONSEC
+#include asm/armv7.h
+#endif
+
DECLARE_GLOBAL_DATA_PTR;
static struct tag *params;
@@ -186,6 +190,29 @@ static void setup_end_tag(bd_t *bd)
__weak void setup_board_tags(struct tag **in_params) {}
+static void do_nonsec_virt_switch(void)
+{
+#ifdef CONFIG_ARMV7_NONSEC
+ int ret;
+
+ ret = armv7_switch_nonsec();
+ switch (ret) {
+ case NONSEC_VIRT_SUCCESS:
+ debug(entered non-secure state\n);
+ break;
+ case NONSEC_ERR_NO_SEC_EXT:
+ printf(nonsec: Security extensions not implemented.\n);
+ break;
+ case NONSEC_ERR_NO_GIC_ADDRESS:
+ printf(nonsec: could not determine GIC address.\n);
+ break;
+ case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
+ printf(nonsec: PERIPHBASE is above 4 GB, no access.\n);
+ break;
+ }
+#endif
+}
I still don't get why you just don't make armv7_switch_nonsec a void and
print the error when they occur... ???
My apologies for not elaborating on these comments I didn't incorporate:
So, I don't like the idea of marrying a low-level routine with high
level output. I don't want to constraint the usage of the routine by
requiring an output channel. Also some parts may not be fatal for
all users - someone could just try to switch and then behave
differently if that failed - without bothering the user.
May seem a bit over-engineered, but I like it better this way ;-)
If that is a show-stopper for you, I can change it, of course.
I won't hold back my ack for the patch series based on this, but I do
think it's over-engineered. I think at least just returning -1 for
error and 0 for success (or even make it a bool) and just printing a
generic error message is cleaner - the level of details as to why the
switch to hyp/nonsec didn't work could then be debug statements that a
board developer could enable with a #define DEBUG 1 in the
corresponding file.
But ok, we've had the conversation, if you still feel this is better and
necessary, then I'll let it be.
-Christoffer
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 4/7] ARM: switch to non-secure state during bootm execution
On Wed, Jul 10, 2013 at 01:54:16AM +0200, Andre Przywara wrote:
[...]
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 1b6e0ac..7b0619e 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -34,6 +34,10 @@
#include asm/bootm.h
#include linux/compiler.h
+#ifdef CONFIG_ARMV7_NONSEC
+#include asm/armv7.h
+#endif
+
DECLARE_GLOBAL_DATA_PTR;
static struct tag *params;
@@ -186,6 +190,29 @@ static void setup_end_tag(bd_t *bd)
__weak void setup_board_tags(struct tag **in_params) {}
+static void do_nonsec_virt_switch(void)
+{
+#ifdef CONFIG_ARMV7_NONSEC
+ int ret;
+
+ ret = armv7_switch_nonsec();
+ switch (ret) {
+ case NONSEC_VIRT_SUCCESS:
+ debug(entered non-secure state\n);
+ break;
+ case NONSEC_ERR_NO_SEC_EXT:
+ printf(nonsec: Security extensions not implemented.\n);
+ break;
+ case NONSEC_ERR_NO_GIC_ADDRESS:
+ printf(nonsec: could not determine GIC address.\n);
+ break;
+ case NONSEC_ERR_GIC_ADDRESS_ABOVE_4GB:
+ printf(nonsec: PERIPHBASE is above 4 GB, no access.\n);
+ break;
+ }
+#endif
+}
I still don't get why you just don't make armv7_switch_nonsec a void and
print the error when they occur... ???
-Christoffer
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 6/7] ARM: extend non-secure switch to also go into HYP mode
On Wed, Jul 10, 2013 at 01:54:18AM +0200, Andre Przywara wrote: For the KVM and XEN hypervisors to be usable, we need to enter the kernel in HYP mode. Now that we already are in non-secure state, HYP mode switching is within short reach. While doing the non-secure switch, we have to enable the HVC instruction and setup the HYP mode HVBAR (while still secure). The actual switch is done by dropping back from a HYP mode handler without actually leaving HYP mode, so we introduce a new handler routine in our new secure exception vector table. In the assembly switching routine we save and restore the banked LR and SP registers around the hypercall to do the actual HYP mode switch. The C routine first checks whether we are in HYP mode already and also whether the virtualization extensions are available. It also checks whether the HYP mode switch was finally successful. The bootm command part only adds and adjusts some error reporting. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/Makefile | 2 +- arch/arm/cpu/armv7/nonsec_virt.S | 43 +++- arch/arm/cpu/armv7/virt-v7.c | 31 + arch/arm/include/asm/armv7.h | 9 +++-- arch/arm/lib/bootm.c | 19 +++--- 5 files changed, 93 insertions(+), 11 deletions(-) diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile index b59f59e..e5eaa56 100644 --- a/arch/arm/cpu/armv7/Makefile +++ b/arch/arm/cpu/armv7/Makefile @@ -36,7 +36,7 @@ ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONF SOBJS+= lowlevel_init.o endif -ifneq ($(CONFIG_ARMV7_NONSEC),) +ifneq ($(CONFIG_ARMV7_NONSEC)$(CONFIG_ARMV7_VIRT),) SOBJS += nonsec_virt.o COBJS+= virt-v7.o endif diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index f9b6b39..895c3b0 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -1,5 +1,5 @@ /* - * code for switching cores into non-secure state + * code for switching cores into non-secure state and into HYP mode * * Copyright (c) 2013Andre Przywara andre.przyw...@linaro.org * @@ -28,15 +28,16 @@ #include asm/armv7.h .arch_extension sec +.arch_extension virt -/* the vector table for secure state */ +/* the vector table for secure state and HYP mode */ _monitor_vectors: .word 0 /* reset */ .word 0 /* undef */ adr pc, _secure_monitor .word 0 .word 0 - .word 0 + adr pc, _hyp_trap .word 0 .word 0 .word 0 /* pad */ @@ -53,10 +54,27 @@ _secure_monitor: bic r1, r1, #0x4e @ clear IRQ, FIQ, EA, nET bits orr r1, r1, #0x31 @ enable NS, AW, FW bits +#ifdef CONFIG_ARMV7_VIRT + mrc p15, 0, r0, c0, c1, 1 @ read ID_PFR1 + and r0, r0, #CPUID_ARM_VIRT_MASK@ mask virtualization bits + cmp r0, #(1 CPUID_ARM_VIRT_SHIFT) + orreq r1, r1, #0x100 @ allow HVC instruction +#endif + mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set) +#ifdef CONFIG_ARMV7_VIRT + mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value + mcreq p15, 4, r0, c12, c0, 0 @ write HVBAR +#endif + movspc, lr @ return to non-secure SVC +_hyp_trap: + mrs lr, elr_hyp @ for older asm: .byte 0x00, 0xe3, 0x0e, 0xe1 this comment just confuses: either make it intelligent to support an older compiler or just get rid of these byte encodings. You can always disassemble the file and lookup the byte code with a modern compiler to get back to the byte encoding. + mov pc, lr @ do no switch modes, but + @ return to caller + /* * 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 @@ -71,9 +89,13 @@ ENTRY(_smp_pen) mcr p15, 0, r1, c12, c0, 0 @ set VBAR bl _nonsec_init + mov r12, r0 @ save GICC address +#ifdef CONFIG_ARMV7_VIRT + bl _switch_to_hyp +#endif - ldr r1, [r0, #GICC_IAR] @ acknowledge IPI - str r1, [r0, #GICC_EOIR]@ signal end of interrupt + ldr r1, [r12, #GICC_IAR]@ acknowledge IPI + str r1, [r12, #GICC_EOIR] @ signal end of interrupt adr r1, _smp_pen waitloop: wfi @@ -164,3 +186,14 @@ ENTRY(_nonsec_init) bx lr ENDPROC(_nonsec_init) + +ENTRY(_switch_to_hyp) + mov r0, lr + mov r1, sp @ save SVC copy of LR and SP + isb did you find out that this isb is indeed
Re: [U-Boot] [PATCH v3 5/7] ARM: add SMP support for non-secure switch
On Wed, Jul 10, 2013 at 01:54:17AM +0200, Andre Przywara wrote:
Currently the non-secure switch is only done for the boot processor.
To enable full SMP support, we have to switch all secondary cores
into non-secure state also.
So we add an entry point for secondary CPUs coming out of low-power
state and make sure we put them into WFI again after having switched
to non-secure state.
For this we acknowledge and EOI the wake-up IPI, then go into WFI.
Once being kicked out of it later, we sanity check that the start
address has actually been changed (since another attempt to switch
to non-secure would block the core) and jump to the new address.
The actual CPU kick is done by sending an inter-processor interrupt
via the GIC to all CPU interfaces except the requesting processor.
The secondary cores will then setup their respective GIC CPU
interface.
The address secondary cores jump to is board specific, we provide
the value here for the Versatile Express board.
Signed-off-by: Andre Przywara andre.przyw...@linaro.org
---
arch/arm/cpu/armv7/nonsec_virt.S| 27 +++
arch/arm/cpu/armv7/virt-v7.c| 19 ++-
arch/arm/include/asm/armv7.h| 1 +
arch/arm/include/asm/gic.h | 2 ++
include/configs/vexpress_ca15_tc2.h | 3 +++
5 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S
b/arch/arm/cpu/armv7/nonsec_virt.S
index e9ee831..f9b6b39 100644
--- a/arch/arm/cpu/armv7/nonsec_virt.S
+++ b/arch/arm/cpu/armv7/nonsec_virt.S
@@ -58,6 +58,33 @@ _secure_monitor:
movspc, lr @ return to non-secure SVC
/*
+ * 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
+ * code has already been executed by a C function before.
+ * Then they go back to wfi and wait to be woken up by the kernel again.
+ */
+ENTRY(_smp_pen)
+ mrs r0, cpsr
+ orr r0, r0, #0xc0
+ msr cpsr, r0@ disable interrupts
+ ldr r1, =_start
+ mcr p15, 0, r1, c12, c0, 0 @ set VBAR
+
+ bl _nonsec_init
+
+ ldr r1, [r0, #GICC_IAR] @ acknowledge IPI
+ str r1, [r0, #GICC_EOIR]@ signal end of interrupt
+ adr r1, _smp_pen
+waitloop:
+ wfi
+ ldr r0, =CONFIG_SYSFLAGS_ADDR @ load start address
You seem to have ignored my comment about using the sysflags name?
As I understand, the sysflags name is a versatile express specific
register name that just happens to be used for the SMP boot address as
well...
Therefore, this should really be CONFIG_SMP_BOOT_ADDR or something like
that, at the very least.
+ ldr r0, [r0]
+ cmp r0, r1 @ make sure we dont execute this code
+ beq waitloop@ again (due to a spurious wakeup)
+ mov pc, r0
+ENDPROC(_smp_pen)
+
+/*
* Switch a core to non-secure state.
*
* 1. initialize the GIC per-core interface
diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
index 54f9746..a0d0b34 100644
--- a/arch/arm/cpu/armv7/virt-v7.c
+++ b/arch/arm/cpu/armv7/virt-v7.c
@@ -77,6 +77,21 @@ static int get_gicd_base_address(unsigned int *gicdaddr)
#endif
}
+static void kick_secondary_cpus(unsigned int gicdaddr)
+{
+ unsigned int *secondary_boot_addr;
+
+ secondary_boot_addr = (void *)CONFIG_SYSFLAGS_ADDR;
+#ifdef CONFIG_SYSFLAGS_NEED_CLEAR_BITS
+ secondary_boot_addr[1] = (unsigned)-1;
+#endif
again, if you disagreed with my previous comment, please comment on it
and rationalize your choice.
I still feel you're wrapping board specific logic into generic code, and
that you should call out to a more generic function. Imagine an SOC
that uses an implementation defined control register for this instead of
a memory address...
perhaps what you need is:
void set_board_smp_boot_addr(unsigned long addr);
unsigned long get_board_smp_boot_addr(void);
and call these instead of your direct use of sysflags addr here...?
+ *secondary_boot_addr = (uintptr_t)_smp_pen;
+ dmb();
+
+ /* now kick all CPUs (except this one) by writing to GICD_SGIR */
+ writel(1U 24, gicdaddr + GICD_SGIR);
+}
+
enum nonsec_virt_errors armv7_switch_nonsec(void)
{
unsigned int reg, ret;
@@ -110,7 +125,9 @@ enum nonsec_virt_errors armv7_switch_nonsec(void)
for (i = 0; i = itlinesnr; i++)
writel((unsigned)-1, gicdaddr + GICD_IGROUPRn + 4 * i);
- /* call the non-sec switching code on this CPU */
+ kick_secondary_cpus(gicdaddr);
+
+ /* call the non-sec switching code on this CPU also */
_nonsec_init();
return NONSEC_VIRT_SUCCESS;
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index e5c0279..f6582a1 100644
---
Re: [U-Boot] [PATCH v3 2/7] ARM: add secure monitor handler to switch to non-secure state
n Wed, Jul 10, 2013 at 01:54:14AM +0200, Andre Przywara wrote: A prerequisite for using virtualization is to be in HYP mode, which requires the CPU to be in non-secure state first. Add new file in arch/arm/cpu/armv7 to hold a monitor handler routine which switches the CPU to non-secure state by setting the NS and associated bits. According to the ARM architecture reference manual this should not be done in SVC mode, so we have to setup a SMC handler for this. We create a new vector table to avoid interference with other boards. The MVBAR register will be programmed later just before the smc call. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/Makefile | 4 +++ arch/arm/cpu/armv7/nonsec_virt.S | 54 2 files changed, 58 insertions(+) create mode 100644 arch/arm/cpu/armv7/nonsec_virt.S diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile index 7a8c2d0..5d75077 100644 --- a/arch/arm/cpu/armv7/Makefile +++ b/arch/arm/cpu/armv7/Makefile @@ -36,6 +36,10 @@ ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA)$(CONF SOBJS+= lowlevel_init.o endif +ifneq ($(CONFIG_ARMV7_NONSEC),) +SOBJS += nonsec_virt.o +endif + SRCS := $(START:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(COBJS) $(SOBJS)) START:= $(addprefix $(obj),$(START)) diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S new file mode 100644 index 000..68a6b38 --- /dev/null +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -0,0 +1,54 @@ +/* + * code for switching cores into non-secure state + * + * Copyright (c) 2013Andre Przywara andre.przyw...@linaro.org + * + * 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 + */ + +#include config.h + +/* the vector table for secure state */ +_monitor_vectors: + .word 0 /* reset */ + .word 0 /* undef */ + adr pc, _secure_monitor + .word 0 + .word 0 + .word 0 + .word 0 + .word 0 + .word 0 /* pad */ + +/* + * software interrupt aka. secure monitor handler a software interrupt is not aka. a secure monitor handler, this is misleading, it's just the smc handler. + * This is executed on a smc instruction, we use a smc #0 to switch + * 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 + orr r1, r1, #0x31 @ enable NS, AW, FW bits + + mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set) + + movspc, lr @ return to non-secure SVC + -- 1.7.12.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 4/7] ARM: switch to non-secure state during bootm execution
On Thu, Jun 13, 2013 at 01:01:10PM +0200, Andre Przywara wrote:
To actually trigger the non-secure switch we just implemented, call
the switching routine from within the bootm command implementation.
This way we automatically enable this feature without further user
intervention.
The core specific part of the work is done in the assembly routine
in nonsec_virt.S, introduced with the previous patch, but for the full
glory we need to setup the GIC distributor interface once for the
whole system, which is done in C here.
The routine is placed in arch/arm/lib to allow easy access from
different boards or CPUs.
We check the availability of the security extensions first.
The generic timer base frequency register is only accessible from
secure state, so we have to program it now. Actually this should be
done from primary firmware before, but some boards seems to omit
this, so if needed we do this here with a board specific value.
The Versatile Express board does not need this, so we remove the
frequency from the configuration file here.
Since we need a safe way to access the GIC, we use the PERIPHBASE
registers on Cortex-A15 and A7 CPUs and do some sanity checks.
Board not implementing the CBAR can override this value via a
configuration file variable.
Then we actually do the GIC enablement:
a) enable the GIC distributor, both for non-secure and secure state
(GICD_CTLR[1:0] = 11b)
b) allow all interrupts to be handled from non-secure state
(GICD_IGROUPRn = 0x)
The core specific GIC setup is then done in the assembly routine.
The actual bootm trigger is pretty small: calling the routine and
doing some error reporting.
Signed-off-by: Andre Przywara andre.przyw...@linaro.org
---
arch/arm/include/asm/armv7.h| 7 ++
arch/arm/lib/Makefile | 2 +
arch/arm/lib/bootm.c| 20 ++
arch/arm/lib/virt-v7.c | 137
include/configs/vexpress_ca15_tc2.h | 2 -
5 files changed, 166 insertions(+), 2 deletions(-)
create mode 100644 arch/arm/lib/virt-v7.c
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index 989bb72..56d0dd0 100644
--- a/arch/arm/include/asm/armv7.h
+++ b/arch/arm/include/asm/armv7.h
@@ -88,6 +88,13 @@ void v7_outer_cache_flush_range(u32 start, u32 end);
void v7_outer_cache_inval_range(u32 start, u32 end);
#ifdef CONFIG_ARMV7_VIRT
+
+#define HYP_ERR_NO_SEC_EXT 2
+#define HYP_ERR_NO_GIC_ADDRESS 3
+#define HYP_ERR_GIC_ADDRESS_ABOVE_4GB4
enum?
+
+int armv7_switch_nonsec(void);
+
/* defined in cpu/armv7/nonsec_virt.S */
void _nonsec_init(void);
#endif /* CONFIG_ARMV7_VIRT */
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index 8ad9f66..1570ad5 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -60,6 +60,8 @@ COBJS-y += reset.o
COBJS-y += cache.o
COBJS-y += cache-cp15.o
+COBJS-$(CONFIG_ARMV7_VIRT) += virt-v7.o
+
SRCS := $(GLSOBJS:.o=.S) $(GLCOBJS:.o=.c) \
$(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 1b6e0ac..8251a89 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -34,6 +34,10 @@
#include asm/bootm.h
#include linux/compiler.h
+#ifdef CONFIG_ARMV7_VIRT
+#include asm/armv7.h
+#endif
+
DECLARE_GLOBAL_DATA_PTR;
static struct tag *params;
@@ -222,6 +226,22 @@ static void boot_prep_linux(bootm_headers_t *images)
printf(FDT and ATAGS support not compiled in - hanging\n);
hang();
}
+#ifdef CONFIG_ARMV7_VIRT
+ switch (armv7_switch_nonsec()) {
+ case 0:
+ debug(entered non-secure state\n);
+ break;
this is weird, why not have a define for the success case?
I still think the debug printing should be done inside
armv7_switch_nonsec instead, and you can just have it be a void();
+ case HYP_ERR_NO_SEC_EXT:
+ printf(HYP mode: Security extensions not implemented.\n);
+ break;
+ case HYP_ERR_NO_GIC_ADDRESS:
+ printf(HYP mode: could not determine GIC address.\n);
+ break;
+ case HYP_ERR_GIC_ADDRESS_ABOVE_4GB:
+ printf(HYP mode: PERIPHBASE is above 4 GB, cannot access
this.\n);
+ break;
+ }
+#endif
}
/* Subcommand: GO */
diff --git a/arch/arm/lib/virt-v7.c b/arch/arm/lib/virt-v7.c
new file mode 100644
index 000..7876a77
--- /dev/null
+++ b/arch/arm/lib/virt-v7.c
@@ -0,0 +1,137 @@
+/*
+ * (C) Copyright 2013
+ * Andre Przywara, Linaro
+ *
+ * Routines to transition ARMv7 processors from secure into non-secure state
+ * needed to enable ARMv7 virtualization for current hypervisors
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is
Re: [U-Boot] [PATCH v2 3/7] ARM: add assembly routine to switch to non-secure state
On Thu, Jun 13, 2013 at 01:01:09PM +0200, Andre Przywara wrote: While actually switching to non-secure state is one thing, the more important part of this process is to make sure that we still super nit: not sure it's more important - it's just another thing we need to do. have full access to the interrupt controller (GIC). The GIC is fully aware of secure vs. non-secure state, some registers are banked, others may be configured to be accessible from secure state only. To be as generic as possible, we get the GIC memory mapped address based on the PERIPHBASE value in the CBAR register. Since this register is not architecturally defined, we check the MIDR before to be from an A15 or A7. For CPUs not having the CBAR or boards with wrong information herein we allow providing the base address as a configuration variable. With the GIC accessible, we: With the GIC accessible ? a) allow private interrupts to be delivered to the core (GICD_IGROUPR0 = 0x) b) enable the CPU interface (GICC_CTLR[0] = 1) c) set the priority filter to allow non-secure interrupts (GICC_PMR = 0xFF) After having switched to non-secure state, we also enable the non-secure GIC CPU interface, since this register is banked. Also we allow access to all coprocessor interfaces from non-secure state by writing the appropriate bits in the NSACR register. super nit 2: move this last paragraph above the non-secure stuff, so there's no confusion that this is done from secure mode. Since we need to call this routine also directly from the smp_pen later (where we don't have any stack), we can only use caller saved registers r0-r3 and r12 to not disturb the compiler. the compiler certainly does seem to get cranky when we disturb it ;) Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/nonsec_virt.S | 66 arch/arm/include/asm/armv7.h | 16 ++ arch/arm/include/asm/gic.h | 17 +++ 3 files changed, 99 insertions(+) create mode 100644 arch/arm/include/asm/gic.h diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index f5572f5..656d99b 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -23,6 +23,8 @@ */ #include config.h +#include asm/gic.h +#include asm/armv7.h /* the vector table for secure state */ _secure_vectors: @@ -52,3 +54,67 @@ _secure_monitor: movspc, lr @ return to non-secure SVC +#define lo(x) ((x) 0x) +#define hi(x) ((x) 16) + +/* + * Routine to switch a core to non-secure state. + * Initializes the GIC per-core interface, allows coprocessor access in + * non-secure modes and uses smc #0 to do the non-secure transition. + * To be called by smp_pen for secondary cores and directly for the BSP. + * For those two cases to work we must not use any stack and thus are + * limited to the caller saved registers r0-r3. you also use r12 (ip) ? Also, I think you can rewrite this comment to make it a little nicer. May I propose something along the lines of: /* * Switch a core to non-secure state. * * 1. initialize the GIC per-core interface * 2. allow coprocessor access in non-secure modes * 3. switch the cpu mode (by calling smc #0) * * Called from smp_pen by non-primary cores and directly by the BSP. * Do not assume that the stack is available and only use registers * r0-r3. * * PERIPHBASE is used to get the GIC address. This could be 40 bits long, * though, but we check this in C before calling this function. */ (I only propose this to match the high standard of these patches) + * PERIPHBASE is used to get the GIC address. This could be 40 bits long, + * though, but we check this in C before calling this function. + */ +.globl _nonsec_init +_nonsec_init: +#ifdef CONFIG_ARM_GIC_BASE_ADDRESS + ldr r2, =CONFIG_ARM_GIC_BASE_ADDRESS +#else + mrc p15, 4, r2, c15, c0, 0 @ read CBAR +#endif + add r3, r2, #GIC_DIST_OFFSET@ GIC dist i/f offset + mvn r1, #0 @ all bits to 1 + str r1, [r3, #GICD_IGROUPRn]@ allow private interrupts + + mrc p15, 0, r0, c0, c0, 0 @ read MIDR + bfc r0, #20, #4 @ mask out variant + bfc r0, #0, #4 @ and revision + + movwr1, #lo(MIDR_CORTEX_A7_R0P0 MIDR_PRIMARY_PART_MASK) in the git repo branch you pointed me to in the cover e-mail, this refers to MIDR_CORTEX_A6_R0P0 ? Forgot to push the last revision? + movtr1, #hi(MIDR_CORTEX_A7_R0P0 MIDR_PRIMARY_PART_MASK) + cmp r0, r1 @ check for Cortex-A7 + + orr r1, #(MIDR_CORTEX_A15_R0P0 0xf0) + cmpne r0, r1 @ check for Cortex-A15 + + movne r1, #GIC_CPU_OFFSET_A9 @ GIC CPU
Re: [U-Boot] [PATCH v2 5/7] ARM: add SMP support for non-secure switch
On Thu, Jun 13, 2013 at 01:01:11PM +0200, Andre Przywara wrote:
Currently the non-secure switch is only done for the boot processor.
To enable full SMP support, we have to switch all secondary cores
into non-secure state also.
So we add an entry point for secondary CPUs coming out of low-power
state and make sure we put them into WFI again after having switched
to non-secure state.
For this we acknowledge and EOI the wake-up IPI, then go into WFI.
Once being kicked out of it later, we sanity check that the start
address has actually been changed (since another attempt to switch
to non-secure would block the core) and jump to the new address.
The actual CPU kick is done by sending an inter-processor interrupt
via the GIC to all CPU interfaces except the requesting processor.
The secondary cores will then setup their respective GIC CPU
interface.
The address secondary cores jump to is board specific, we provide
the value here for the Versatile Express board.
Signed-off-by: Andre Przywara andre.przyw...@linaro.org
---
arch/arm/cpu/armv7/nonsec_virt.S| 27 +++
arch/arm/include/asm/armv7.h| 1 +
arch/arm/lib/virt-v7.c | 19 ++-
include/configs/vexpress_ca15_tc2.h | 3 +++
4 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/nonsec_virt.S
b/arch/arm/cpu/armv7/nonsec_virt.S
index 656d99b..919f6e9 100644
--- a/arch/arm/cpu/armv7/nonsec_virt.S
+++ b/arch/arm/cpu/armv7/nonsec_virt.S
@@ -54,6 +54,33 @@ _secure_monitor:
movspc, lr @ return to non-secure SVC
+/*
+ * 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
+ * code has already been executed by a C function before.
+ * Then they go back to wfi and wait to be woken up by the kernel again.
+ */
+.globl _smp_pen
+_smp_pen:
+ mrs r0, cpsr
+ orr r0, r0, #0xc0
+ msr cpsr, r0@ disable interrupts
+ ldr r1, =_start
+ mcr p15, 0, r1, c12, c0, 0 @ set VBAR
+
+ bl _nonsec_init
+
+ ldr r1, [r3, #0x0c] @ read GICD acknowledge
+ str r1, [r3, #0x10] @ write GICD EOI
+ adr r1, _smp_pen
+waitloop:
+ wfi
+ ldr r0, =CONFIG_SYSFLAGS_ADDR @ load start address
the name sysflags addr is a very vexpress specific thingy.
I think you need to call this something like SMP_SECONDARY_BOOT_ADDR or
whatever, which is more generic.
+ ldr r0, [r0]
+ cmp r0, r1 @ make sure we dont execute this code
+ beq waitloop@ again (due to a spurious wakeup)
+ mov pc, r0
+
#define lo(x) ((x) 0x)
#define hi(x) ((x) 16)
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index 56d0dd0..04545b9 100644
--- a/arch/arm/include/asm/armv7.h
+++ b/arch/arm/include/asm/armv7.h
@@ -97,6 +97,7 @@ int armv7_switch_nonsec(void);
/* defined in cpu/armv7/nonsec_virt.S */
void _nonsec_init(void);
+void _smp_pen(void);
#endif /* CONFIG_ARMV7_VIRT */
#endif /* ! __ASSEMBLY__ */
diff --git a/arch/arm/lib/virt-v7.c b/arch/arm/lib/virt-v7.c
index 7876a77..6946e4d 100644
--- a/arch/arm/lib/virt-v7.c
+++ b/arch/arm/lib/virt-v7.c
@@ -95,6 +95,21 @@ static int get_gic_base_address(char **gicdptr)
#endif
}
+static void kick_secondary_cpus(char *gicdptr)
+{
+ unsigned int *sysflags;
again, I think the use of the name sysflags here is misunderstood.
+
+ sysflags = (void *)CONFIG_SYSFLAGS_ADDR;
+#ifdef CONFIG_SYSFLAGS_NEED_CLEAR_BITS
+ sysflags[1] = (unsigned)-1;
this feels like we're wrapping some vexpress-logic into some
pseudo-generic logic here. It feels like there should be a function
provided by the board code, that we simply call.
void set_board_smp_boot_addr(unsigned long addr);
+#endif
+ *sysflags = (uintptr_t)_smp_pen;
+ dmb();
+
+ /* now kick all CPUs (expect this one) by writing to GICD_SGIR */
s/expect/except/
+ writel(1U 24, gicdptr[GICD_SGIR]);
+}
+
int armv7_switch_nonsec(void)
{
unsigned int reg, ret;
@@ -130,7 +145,9 @@ int armv7_switch_nonsec(void)
for (i = 0; i = itlinesnr; i++)
writel((unsigned)-1, gicdptr[GICD_IGROUPRn + 4 * i]);
- /* call the non-sec switching code on this CPU */
+ kick_secondary_cpus(gicdptr);
+
+ /* call the non-sec switching code on this CPU also */
_nonsec_init();
return 0;
diff --git a/include/configs/vexpress_ca15_tc2.h
b/include/configs/vexpress_ca15_tc2.h
index 4f425ac..ade9e5b 100644
--- a/include/configs/vexpress_ca15_tc2.h
+++ b/include/configs/vexpress_ca15_tc2.h
@@ -31,4 +31,7 @@
#include vexpress_common.h
#define CONFIG_BOOTP_VCI_STRING
Re: [U-Boot] [PATCH v2 6/7] ARM: extend non-secure switch to also go into HYP mode
On Thu, Jun 13, 2013 at 01:01:12PM +0200, Andre Przywara wrote: For the KVM and XEN hypervisors to be usable, we need to enter the kernel in HYP mode. Now that we already are in non-secure state, HYP mode switching is within short reach. While doing the non-secure switch, we have to enable the HVC instruction and setup the HYP mode HVBAR (while still secure). The actual switch is done by dropping back from a HYP mode handler without actually leaving HYP mode, so we introduce a new handler routine in our new secure exception vector table. In the assembly switching routine we save and restore the banked LR and SP registers around the hypercall to do the actual HYP mode switch. The C routine first checks whether we are in HYP mode already and also whether the virtualization extensions are available. It also checks whether the HYP mode switch was finally successful. The bootm command part only adds and adjusts some error reporting. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/nonsec_virt.S | 31 --- arch/arm/include/asm/armv7.h | 7 +-- arch/arm/lib/bootm.c | 14 ++ arch/arm/lib/virt-v7.c | 27 ++- 4 files changed, 65 insertions(+), 14 deletions(-) diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S index 919f6e9..950da6f 100644 --- a/arch/arm/cpu/armv7/nonsec_virt.S +++ b/arch/arm/cpu/armv7/nonsec_virt.S @@ -1,5 +1,5 @@ /* - * code for switching cores into non-secure state + * code for switching cores into non-secure state and into HYP mode * * Copyright (c) 2013Andre Przywara andre.przyw...@linaro.org * @@ -26,14 +26,14 @@ #include asm/gic.h #include asm/armv7.h -/* the vector table for secure state */ +/* the vector table for secure state and HYP mode */ _secure_vectors: .word 0 /* reset */ .word 0 /* undef */ adr pc, _secure_monitor .word 0 .word 0 - .word 0 + adr pc, _hyp_trap .word 0 .word 0 .word 0 /* pad */ @@ -50,10 +50,23 @@ _secure_monitor: bic r1, r1, #0x4e @ clear IRQ, FIQ, EA, nET bits orr r1, r1, #0x31 @ enable NS, AW, FW bits + mrc p15, 0, r0, c0, c1, 1 @ read ID_PFR1 + and r0, r0, #CPUID_ARM_VIRT_MASK@ mask virtualization bits + cmp r0, #(1 CPUID_ARM_VIRT_SHIFT) + orreq r1, r1, #0x100 @ allow HVC instruction + mcr p15, 0, r1, c1, c1, 0 @ write SCR (with NS bit set) + mrceq p15, 0, r0, c12, c0, 1 @ get MVBAR value Why not just do load the address of _secure_vectors directly? I think it makes it more clear what happens. + mcreq p15, 4, r0, c12, c0, 0 @ write HVBAR + movspc, lr @ return to non-secure SVC +_hyp_trap: + .byte 0x00, 0xe3, 0x0e, 0xe1@ mrs lr, elr_hyp Again, why not just add the necessary .arch_extension or assembler directive in the makefile and use the instructions directly. The only reason I would see for performing this obscurity would be to support really old compilers, which I doubt we fill for future boards? + mov pc, lr @ do no switch modes, but + @ return to caller + /* * 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 @@ -69,6 +82,7 @@ _smp_pen: mcr p15, 0, r1, c12, c0, 0 @ set VBAR bl _nonsec_init + bl _hyp_init ldr r1, [r3, #0x0c] @ read GICD acknowledge str r1, [r3, #0x10] @ write GICD EOI @@ -145,3 +159,14 @@ _nonsec_init: str r1, [r2]@ allow private interrupts bx lr + +.globl _hyp_init +_hyp_init: nit: the naming here is a little misleading for someone not knowing what's going on. You're not really initializing the mode, but switching to it: _switch_to_hyp: ??? + mov r2, lr + mov r3, sp @ save SVC copy of LR and SP + isb I don't think this isb is necessary. + .byte 0x70, 0x00, 0x40, 0xe1@ hvc #0 again, this is really funky. If it's really necessary, can we please have a define somewhere so you can just do HVC(0) instead? + mov sp, r3 + mov lr, r2 @ fix HYP mode banked LR and SP nit: s/fix HYP mode/restore S-SVC mode/ + + bx lr diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h index 04545b9..8c3a85e 100644 --- a/arch/arm/include/asm/armv7.h +++ b/arch/arm/include/asm/armv7.h @@ -89,15 +89,18 @@ void v7_outer_cache_inval_range(u32 start, u32
Re: [U-Boot] [PATCH 0/6] ARMv7: Add HYP mode switching support
On Mon, May 06, 2013 at 03:17:44PM +0200, Andre Przywara wrote: (for GIT URL and Changelog see below) ARM CPUs with the virtualization extension have a new mode called HYP mode, which allows hypervisors to safely control and monitor guests. The current hypervisor (KVM and Xen) implementations require the kernel to be entered in that HYP mode. This patch series introduces a configuration variable CONFIG_ARMV7_VIRT which enables code to switch all cores into HYP mode. This is done automatically during execution of the bootm command (but could also be done earlier - U-Boot runs fine in HYP mode without MMU usage). I forget the u-boot specifics here, but if you boot over networking, for example, does that eventually end up calling bootm or would Hyp mode not be entered in this case? I'm only raising this issue because it's important to minimize the hoops and efforts for booting in Hyp mode, since it's harmless to do so regardless of the linux kernel we're booting. [...] -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/6] ARM: add assembly routine to switch to non-secure state
On Fri, May 31, 2013 at 11:26:06AM +0200, Andre Przywara wrote: On 05/31/2013 05:04 AM, Christoffer Dall wrote: On Mon, May 06, 2013 at 03:17:46PM +0200, Andre Przywara wrote: While actually switching to non-secure state is one thing, the more important part of this process is to make sure that we still have full access to the interrupt controller (GIC). The GIC is fully aware of secure vs. non-secure state, some registers are banked, others may be configured to be accessible from secure state only. To be as generic as possible, we get the GIC memory mapped address based on the PERIPHBASE register. We check explicitly for ARM Cortex-A7 and A15 cores, assuming an A9 otherwise, as for those cores we know the offsets for the GIC CPU interface from the PERIPHBASE content. Other cores could be added as needed. With the GIC accessible, we: a) allow private interrupts to be delivered to the core (GICD_IGROUPR0 = 0x) b) enable the CPU interface (GICC_CTLR[0] = 1) c) set the priority filter to allow non-secure interrupts (GICC_PMR = 0x80) After having switched to non-secure state, we also enable the non-secure GIC CPU interface, since this register is banked. Also we allow access to all coprocessor interfaces from non-secure state by writing the appropriate bits in the NSACR register. For reasons obvious later we only use caller saved registers r0-r3. You probably want to put that in a comment in the code, and it would also be super helpful to explain the obvious part here, because most readers don't look forward in time to understand this patch... Agreed. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/start.S | 47 ++ 1 file changed, 47 insertions(+) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index da48b36..e63e892 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -572,3 +572,50 @@ fiq: #endif /* CONFIG_USE_IRQ */ #endif /* CONFIG_SPL_BUILD */ + +#ifdef CONFIG_ARMV7_VIRT +/* Routine to initialize GIC CPU interface and switch to nonsecure state. + */ +.globl _nonsec_gic_switch +_nonsec_gic_switch: + mrc p15, 4, r2, c15, c0, 0 @ r2 = PERIPHBASE You should probably check if bits [7:0] == 0x00 here, otherwise you may end up doing some very strange stuff - if any of those bits are set you can just error out at this point, but it should be gracefully handled. Also since it's core specific, you'd probably want to check that before using it. As you found out later, I am doing this before calling this routine. But I will add a comment here to avoid confusion. yeah, or just expect that this address is in r0 upon calling the routine, then you're in the clear. + add r3, r2, #0x1000 @ GIC dist i/f offset Since this is core specific, could the offset please go in an appropriate header file? It will also eliminate the need for the comment if you just have a proper define (i.e. GIC_DIST_OFFSET ...) + mvn r1, #0 + str r1, [r3, #0x80] @ allow private interrupts Aren't you makeing an assumption about the number of available interrupts here? You should read the ITLinesNumber field from the GICD_TYPER if I'm not mistaking. This is the per core private interrupts. All bits should be implemented. From the GIC spec, chapter 4.3.4: In a multiprocessor implementation, GICD_IGROUPR0 is banked for each connected processor. This register holds the group status bits for interrupts 0-31. I understand it, but the comments or naming of the routine never suggested that this was the code that was called per-core. I really think that is the core objective of this function: The NS-init that each core must do, it's not really GIC specific, so I suggest you rename it. I also think it would be much cleaner if you again used a define for the 0x80 offset. Also, don't you need to set some enable fields on the GICD_CTLR here to enable group 1 interrupts? Since this a non-banked per-system register, I do this later in the C part. later in the patch series, before in the flow of the code, right? :) + + mrc p15, 0, r0, c0, c0, 0 @ MIDR + bfc r0, #16, #8 @ mask out variant, arch + bfc r0, #0, #4 @ and revision + movwr1, #0xc070 + movtr1, #0x4100 + cmp r0, r1 @ check for Cortex-A7 + orr r1, #0xf0 wow, nice bit fiddling. It may be quite a bit easier to read if you simply had defines for the bitmasks and real values and just did a load and placed a literal section accordingly. The sequence is necessary since we are short on registers. I agree it is a bit obfuscated, will make it more readable. yeah but: #define ARM_CORTEX_A15_ID 0x4100c070 .ltorg [...] ldr r0
Re: [U-Boot] [PATCH 1/6] ARM: add secure monitor handler to switch to non-secure state
On Fri, May 31, 2013 at 11:23:16AM +0200, Andre Przywara wrote: On 05/31/2013 03:02 AM, Christoffer Dall wrote: Christoffer, thanks a lot for the thorough review. Comments inline. On Mon, May 06, 2013 at 03:17:45PM +0200, Andre Przywara wrote: A prerequisite for using virtualization is to be in HYP mode, which requires the CPU to be in non-secure state. Introduce a monitor handler routine which switches the CPU to non-secure state by setting the NS and associated bits. According to the ARM ARM this should not be done in SVC mode, so we have to setup a SMC handler for this. We reuse the current vector table for this and make sure that we only access the MVBAR register if the CPU supports the security extension and only if we configured the board to use it, since boards entering u-boot already in non-secure mode would crash on accessing MVBAR otherwise. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/start.S | 31 --- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index e9e57e6..da48b36 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -155,6 +155,13 @@ reset: /* Set vector address in CP15 VBAR register */ ldr r0, =_start mcr p15, 0, r0, c12, c0, 0 @Set VBAR + +#ifdef CONFIG_ARMV7_VIRT + mrc p15, 0, r1, c0, c1, 1 @ check for security extension + andsr1, r1, #0x30 + mcrne p15, 0, r0, c12, c0, 1 @ Set secure monitor MVBAR Hmm, this smells a bit simplified to me. Support for ARMv7_VIRT should easy to integrate into u-boot even for platforms that do not boot U-boot directly into secure mode (OMAP5 GP platforms are such an example). In this case you cannot assume that you can write the secure monitor mvbar. Right, Albert kind of hinted on this already. I already fixed this in my private tree, totally removing these MVBAR writes here and instead copying the values from VBAR later just before we do the smc. Will send out a fixed version. +#endif + #endif /* the mask ROM code should have PLL and others stable */ @@ -257,6 +264,12 @@ ENTRY(c_runtime_cpu_setup) ldr r0, =_start mcr p15, 0, r0, c12, c0, 0 @Set VBAR +#ifdef CONFIG_ARMV7_VIRT + mrc p15, 0, r1, c0, c1, 1 @ check for security extension + andsr1, r1, #0x30 + mcrne p15, 0, r0, c12, c0, 1 @ Set secure monitor MVBAR +#endif + bx lr ENDPROC(c_runtime_cpu_setup) @@ -490,11 +503,23 @@ undefined_instruction: bad_save_user_regs bl do_undefined_instruction +/* + * software interrupt aka. secure monitor handler + * This is executed on a smc instruction, we use a smc #0 to switch + * to non-secure state + */ .align 5 software_interrupt: - get_bad_stack_swi - bad_save_user_regs - bl do_software_interrupt Why is the following block not conditional on CONFIG_ARMV7_VIRT? Again, it feels a bit funny to modify this generic mechanism to contain this code for boards that boot in NS mode but have a way to enter Hyp mode using an HVC or SMC instruction. software_interrupt is currently a panic routine. So it is not actually used by u-boot, it's just there to dump some state and eventually call reset_cpu(). Which is pretty useful to catch if something went wrong. So I feel that since I am now the only user of software_interrupt I don't need any special precautions and consider this routine now actually part of the HYP mode switcher. But of course I can retain the original functionality if CONFIG_ARMV7_VIRT is not defined. Yeah, I don't really think it's cool if a software interrupt happens somewhere completely else in the system that we do this dance, which could be super hard to detect, which is why I'm not even a fan of re-using the vector in the first place. + mrc p15, 0, r1, c1, c1, 0 @ read SCR + bic r1, r1, #0x07f + orr r1, r1, #0x31 @ enable NS, AW, FW Are you sure you want to always route FIQ to non-secure here? Since we actually don't install any secure software and just use the secure state to do the HYP switch I don't feel like we should restrict FIQ. Since we don't use it for our own purposes, no one would be able to use it then, right? So my idea was to allow as much as possible. Yeah, makes sense. Don't you need to set the HCE bit? The whole register resets to 0register resets to zero. This is added later in 5/6. For reviewing purposes I split the patches up to do the non-secure switch only first. Later I add the bits to actually go to HYP mode. The splitting of patches is fine, but it would be helpful to explain the scope a little more in the commit text perhaps, maybe I'm just being silly. + + mrc p15, 0, r0, c12, c0, 0 @ save secure copy of VBAR + mcr p15, 0, r1, c1, c1
Re: [U-Boot] [PATCH 0/6] ARMv7: Add HYP mode switching support
On Fri, May 31, 2013 at 08:36:12AM +0200, Andre Przywara wrote: On 05/31/2013 08:11 AM, Christoffer Dall wrote: On Mon, May 06, 2013 at 03:17:44PM +0200, Andre Przywara wrote: (for GIT URL and Changelog see below) ARM CPUs with the virtualization extension have a new mode called HYP mode, which allows hypervisors to safely control and monitor guests. The current hypervisor (KVM and Xen) implementations require the kernel to be entered in that HYP mode. This patch series introduces a configuration variable CONFIG_ARMV7_VIRT which enables code to switch all cores into HYP mode. This is done automatically during execution of the bootm command (but could also be done earlier - U-Boot runs fine in HYP mode without MMU usage). I forget the u-boot specifics here, but if you boot over networking, for example, does that eventually end up calling bootm or would Hyp mode not be entered in this case? Despite the naming bootm is eventually always called when booting Linux, even bootz is a wrapper around this. Same with network boot. So unless you show me a case where this isn't, I think this is fine. Cool, I have no idea, I just wanted to be sure. Thanks, -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/6] ARM: switch to non-secure state during bootm execution
On Fri, May 31, 2013 at 11:30:32AM +0200, Andre Przywara wrote:
On 05/31/2013 07:10 AM, Christoffer Dall wrote:
On Mon, May 06, 2013 at 03:17:47PM +0200, Andre Przywara wrote:
To actually trigger the non-secure switch we just implemented, call
the switching routine from within the bootm command implementation.
This way we automatically enable this feature without further user
intervention.
Some part of the work is done in the assembly routine in start.S,
introduced with the previous patch, but for the full glory we need
to setup the GIC distributor interface once for the whole system,
which is done in C here.
The routine is placed in arch/arm/lib to allow easy access from
different boards or CPUs.
I'm beginning to loose track of exactly which parts is in assembly and
which parts are in C, and why. We are fiddling with some gic dist.
settings in assembly, and some of them in C as well.
I fear I dropped the explanation some time during a patch split earlier.
So the assembly code is the per core part - including GICD_IGROUPR0,
which is banked per core. The reason this is in assembly is to make
it easily run right out of the SMP pen.
In C I do anything that needs to be only done once for the whole system.
More comments inline...
I think renaming the assembly routine will go a along way. Ordering
this patch before the assembly patch with just a stub function
implementation may also have been easier to read, but that's easy for me
to say at this point.
I think it's just the ordering or naming of the patches that is a little
confusing.
First we check for the availability of the security extensions.
The generic timer base frequency register is only accessible from
secure state, so we have to program it now. Actually this should be
done from primary firmware before, but some boards seems to omit
this, so if needed we do this here with a board specific value.
Since we need a safe way to access the GIC, we use the PERIPHBASE
registers on Cortex-A15 and A7 CPUs and do some sanity checks.
Then we actually do the GIC enablement:
a) enable the GIC distributor, both for non-secure and secure state
(GICD_CTLR[1:0] = 11b)
b) allow all interrupts to be handled from non-secure state
(GICD_IGROUPRn = 0x)
The core specific GIC setup is then done in the assembly routine.
The actual bootm trigger is pretty small: calling the routine and
doing some error reporting. A return value of 1 will be added later.
To enable the whole code we introduce the CONFIG_ARMV7_VIRT variable.
Signed-off-by: Andre Przywara andre.przyw...@linaro.org
---
arch/arm/include/asm/armv7.h | 7 +++
arch/arm/lib/Makefile| 2 +
arch/arm/lib/bootm.c | 20
arch/arm/lib/virt-v7.c | 113
+++
4 files changed, 142 insertions(+)
create mode 100644 arch/arm/lib/virt-v7.c
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index a73630b..25afffe 100644
--- a/arch/arm/include/asm/armv7.h
+++ b/arch/arm/include/asm/armv7.h
@@ -74,4 +74,11 @@ void v7_outer_cache_inval_all(void);
void v7_outer_cache_flush_range(u32 start, u32 end);
void v7_outer_cache_inval_range(u32 start, u32 end);
+#ifdef CONFIG_ARMV7_VIRT
+int armv7_switch_nonsec(void);
+
+/* defined in cpu/armv7/start.S */
+void _nonsec_gic_switch(void);
+#endif /* CONFIG_ARMV7_VIRT */
+
#endif
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index 6ae161a..37a0e71 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -58,6 +58,8 @@ COBJS-y += reset.o
COBJS-y += cache.o
COBJS-y += cache-cp15.o
+COBJS-$(CONFIG_ARMV7_VIRT) += virt-v7.o
+
SRCS := $(GLSOBJS:.o=.S) $(GLCOBJS:.o=.c) \
$(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index f3b30c5..a3d3aae 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -35,6 +35,10 @@
#include asm/bootm.h
#include linux/compiler.h
+#ifdef CONFIG_ARMV7_VIRT
+#include asm/armv7.h
+#endif
+
DECLARE_GLOBAL_DATA_PTR;
#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
@@ -319,6 +323,22 @@ static void boot_prep_linux(bootm_headers_t *images)
hang();
#endif /* all tags */
}
+#ifdef CONFIG_ARMV7_VIRT
+ switch (armv7_switch_nonsec()) {
+ case 0:
+ debug(entered non-secure state\n);
+ break;
+ case 2:
+ printf(HYP mode: Security extensions not implemented.\n);
+ break;
+ case 3:
+ printf(HYP mode: CPU not supported (must be Cortex-A15 or
A7).\n);
I would drop the specifics of what's supported here.
This is in particular here since I rely on the PERIPHBASE register,
which is A15/A7 implementation specific. This is different from case
2 (which will later be changed to Virtualization
Re: [U-Boot] [PATCH 5/6] ARM: extend non-secure switch to also go into HYP mode
On Fri, May 31, 2013 at 11:34:38AM +0200, Andre Przywara wrote: On 05/31/2013 07:43 AM, Christoffer Dall wrote: On Mon, May 06, 2013 at 03:17:49PM +0200, Andre Przywara wrote: For the KVM and XEN hypervisors to be usable, we need to enter the kernel in HYP mode. Now that we already are in non-secure state, HYP mode switching is within short reach. While doing the non-secure switch, we have to enable the HVC instruction and setup the HYP mode HVBAR (while still secure). The actual switch is done by dropping back from a HYP mode handler without actually leaving HYP mode, so we introduce a new handler routine in the exception vector table. In the assembly switching routine - which we rename to hyp_gic_switch on the way - we save and restore the banked LR and SP registers around the hypercall to do the actual HYP mode switch. The C routine first checks whether we are in HYP mode already and also whether the virtualization extensions are available. It also checks whether the HYP mode switch was finally successful. The bootm command part only adds and adjusts some error reporting. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/start.S | 34 +++--- arch/arm/include/asm/armv7.h | 4 ++-- arch/arm/lib/bootm.c | 12 +--- arch/arm/lib/virt-v7.c | 22 +++--- 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 02234c7..921e9d9 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -41,7 +41,7 @@ _start: b reset ldr pc, _software_interrupt ldr pc, _prefetch_abort ldr pc, _data_abort - ldr pc, _not_used + ldr pc, _hyp_trap ldr pc, _irq ldr pc, _fiq #ifdef CONFIG_SPL_BUILD @@ -49,7 +49,7 @@ _undefined_instruction: .word _undefined_instruction _software_interrupt: .word _software_interrupt _prefetch_abort: .word _prefetch_abort _data_abort: .word _data_abort -_not_used: .word _not_used +_hyp_trap: .word _hyp_trap _irq: .word _irq _fiq: .word _fiq _pad: .word 0x12345678 /* now 16*4=64 */ @@ -58,7 +58,7 @@ _undefined_instruction: .word undefined_instruction _software_interrupt: .word software_interrupt _prefetch_abort: .word prefetch_abort _data_abort: .word data_abort -_not_used: .word not_used +_hyp_trap: .word hyp_trap _irq: .word irq _fiq: .word fiq _pad: .word 0x12345678 /* now 16*4=64 */ @@ -513,12 +513,18 @@ software_interrupt: mrc p15, 0, r1, c1, c1, 0 @ read SCR bic r1, r1, #0x07f orr r1, r1, #0x31 @ enable NS, AW, FW + mrc p15, 0, r0, c0, c1, 1 @ check for Virt ext + and r0, r0, #0xf000 + cmp r0, #0x1000 you can just do ands r0, r0 ,#0xf000 and change the orreq below to orrne Can I? But I want to make sure that Virt ext is of version 1 only, anything beyond that I cannot reliably support, right? Or is there a guarantee that this is backwards compatible? In the ARMv7 specifications it mentions either 1 or 0 as the possible values of this field, and doesn't say anything about reserved values iirc, but maybe that shouldn't be taken that literally. Just keep the code as it is then. + orreq r1, r1, #0x100 @ allow HVC instruction mrc p15, 0, r0, c12, c0, 0 @ save secure copy of VBAR mcr p15, 0, r1, c1, c1, 0 @ write SCR, switch to non-sec isb mcr p15, 0, r0, c12, c0, 0 @ write non-secure copy of VBAR + mcreq p15, 4, r0, c12, c0, 0 @ write HYP mode HVBAR + nit: s/HYP mode// movspc, lr .align 5 @@ -534,10 +540,9 @@ data_abort: bl do_data_abort .align 5 -not_used: - get_bad_stack - bad_save_user_regs - bl do_not_used +hyp_trap: + .byte 0x00, 0xe3, 0x0e, 0xe1@ mrs lr, elr_hyp do we really need to support this on assemblers that old? What do you mean with old? I can check again, but I think it didn't work with my self-compiled binutils 2.23. Or do I need a directive to enable this? You need a directive, see the Makefile in arch/arm/kvm/Makefile and the corresponding source files in the kernel. + mov pc, lr #ifdef CONFIG_USE_IRQ @@ -574,21 +579,21 @@ fiq: #endif /* CONFIG_SPL_BUILD */ #ifdef CONFIG_ARMV7_VIRT -/* Routine to initialize GIC CPU interface and switch to nonsecure state. - * Will be executed directly by secondary CPUs after coming out of +/* Routine to initialize GIC CPU interface, switch to nonsecure and to HYP + * mode. Will be executed directly by secondary CPUs after coming out
Re: [U-Boot] [PATCH 4/6] ARM: add SMP support for non-secure switch
On Fri, May 31, 2013 at 11:32:40AM +0200, Andre Przywara wrote:
On 05/31/2013 07:32 AM, Christoffer Dall wrote:
On Mon, May 06, 2013 at 03:17:48PM +0200, Andre Przywara wrote:
Currently the non-secure switch is only done for the boot processor.
To later allow full SMP support, we have to switch all secondary
cores into non-secure state also.
So we add an entry point for secondary CPUs coming out of low-power
state and make sure we put them into WFI again after having switched
to non-secure state.
For this we acknowledge and EOI the wake-up IPI, then go into WFI.
Once being kicked out of it later, we sanity check that the start
address has actually been changed (since another attempt to switch
to non-secure would block the core) and jump to the new address.
The actual CPU kick is done by sending an inter-processor interrupt
via the GIC to all CPU interfaces except the requesting processor.
The secondary cores will then setup their respective GIC CPU
interface.
The address secondary cores jump to is board specific, we provide
the value here for the Versatile Express board.
Signed-off-by: Andre Przywara andre.przyw...@linaro.org
---
arch/arm/cpu/armv7/start.S | 27 ++-
arch/arm/include/asm/armv7.h| 1 +
arch/arm/lib/virt-v7.c | 9 -
include/configs/vexpress_ca15_tc2.h | 1 +
4 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index e63e892..02234c7 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -575,8 +575,19 @@ fiq:
#ifdef CONFIG_ARMV7_VIRT
/* Routine to initialize GIC CPU interface and switch to nonsecure state.
+ * Will be executed directly by secondary CPUs after coming out of
+ * WFI, or can be called directly by C code for CPU 0.
+ * Those two paths mandate to not use any stack and to only use registers
+ * r0-r3 to comply with both the C ABI and the requirement of SMP startup
+ * code.
*/
.globl _nonsec_gic_switch
+.globl _smp_pen
+_smp_pen:
+ mrs r0, cpsr
+ orr r0, r0, #0xc0
+ msr cpsr, r0@ disable interrupts
+ mov lr, #0 @ clear LR to mark secondary
instead of this subtle abuse of lr, why not make this routine simply
take a parameter?
How would this work if this is called out of SMP pen? Shall I rely on
the registers being zero, then? Not very stable, I guess.
I think this whole routine is special anyways, so I felt this subtle
abuse is OK.
An option would be to set r0 to 1 in the smp_pen path and pass 0 as the
first parameter when calling from C. But then I'd need to save this
value - possibly in the LR register ;-)
see my example in the previous patch comments, I don't see why using the
lr like this makes anything anymore clear or more stable?
I also slightly object against wrapping the _smp_pen around the
_nonsec_gic_switch, I really think these are separate routines, where
one can just call the other...?
The actual routine and the purpose are the same, just the entry and exit
code is different. So this fitted nicely in here. I can add a more
specific comment on the different entry points.
eh, to me it reads kind of like this C function (but maybe I just choked
on this somehow):
void do_magic(bool mode)
{
if (mode) {
pull_rabit_out_of_hat();
do_card_trick();
}
saw_a_woman_in_half(); /* we always do this */
linking_rings();
if (!mode)
return;
while (1) {
vanish_statue_of_liberty();
walk_through_great_wall_of_china();
}
}
_nonsec_gic_switch:
mrc p15, 4, r2, c15, c0, 0 @ r2 = PERIPHBASE
add r3, r2, #0x1000 @ GIC dist i/f offset
@@ -617,5 +628,19 @@ _nonsec_gic_switch:
add r2, r2, #0x1000 @ GIC dist i/f offset
str r1, [r2]@ allow private interrupts
- mov pc, lr
+ cmp lr, #0
+ movne pc, lr @ CPU 0 to return
+ @ all others: go to sleep
+_ack_int:
+ ldr r1, [r3, #0x0c] @ read GICD acknowledge
+ str r1, [r3, #0x10] @ write GICD EOI
+
+ adr r1, _smp_pen
+waitloop:
+ wfi
+ ldr r0, =CONFIG_SYSFLAGS_ADDR @ load start address
+ ldr r0, [r0]
+ cmp r0, r1 @ make sure we dont execute this code
I think I raised this issue previously, but this code is in a core
u-boot file, but I could imagine a board with a different crazy boot
protocol that required you to check two different fields and jump
through other hoops to wake up from the smp pen, so I really think the
whole smp pen belongs
Re: [U-Boot] [PATCH 1/6] ARM: add secure monitor handler to switch to non-secure state
On Mon, May 06, 2013 at 03:17:45PM +0200, Andre Przywara wrote: A prerequisite for using virtualization is to be in HYP mode, which requires the CPU to be in non-secure state. Introduce a monitor handler routine which switches the CPU to non-secure state by setting the NS and associated bits. According to the ARM ARM this should not be done in SVC mode, so we have to setup a SMC handler for this. We reuse the current vector table for this and make sure that we only access the MVBAR register if the CPU supports the security extension and only if we configured the board to use it, since boards entering u-boot already in non-secure mode would crash on accessing MVBAR otherwise. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/start.S | 31 --- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index e9e57e6..da48b36 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -155,6 +155,13 @@ reset: /* Set vector address in CP15 VBAR register */ ldr r0, =_start mcr p15, 0, r0, c12, c0, 0 @Set VBAR + +#ifdef CONFIG_ARMV7_VIRT + mrc p15, 0, r1, c0, c1, 1 @ check for security extension + andsr1, r1, #0x30 + mcrne p15, 0, r0, c12, c0, 1 @ Set secure monitor MVBAR Hmm, this smells a bit simplified to me. Support for ARMv7_VIRT should easy to integrate into u-boot even for platforms that do not boot U-boot directly into secure mode (OMAP5 GP platforms are such an example). In this case you cannot assume that you can write the secure monitor mvbar. +#endif + #endif /* the mask ROM code should have PLL and others stable */ @@ -257,6 +264,12 @@ ENTRY(c_runtime_cpu_setup) ldr r0, =_start mcr p15, 0, r0, c12, c0, 0 @Set VBAR +#ifdef CONFIG_ARMV7_VIRT + mrc p15, 0, r1, c0, c1, 1 @ check for security extension + andsr1, r1, #0x30 + mcrne p15, 0, r0, c12, c0, 1 @ Set secure monitor MVBAR +#endif + bx lr ENDPROC(c_runtime_cpu_setup) @@ -490,11 +503,23 @@ undefined_instruction: bad_save_user_regs bl do_undefined_instruction +/* + * software interrupt aka. secure monitor handler + * This is executed on a smc instruction, we use a smc #0 to switch + * to non-secure state + */ .align 5 software_interrupt: - get_bad_stack_swi - bad_save_user_regs - bl do_software_interrupt Why is the following block not conditional on CONFIG_ARMV7_VIRT? Again, it feels a bit funny to modify this generic mechanism to contain this code for boards that boot in NS mode but have a way to enter Hyp mode using an HVC or SMC instruction. + mrc p15, 0, r1, c1, c1, 0 @ read SCR + bic r1, r1, #0x07f + orr r1, r1, #0x31 @ enable NS, AW, FW Are you sure you want to always route FIQ to non-secure here? Don't you need to set the HCE bit? The whole register resets to 0register resets to zero. + + mrc p15, 0, r0, c12, c0, 0 @ save secure copy of VBAR + mcr p15, 0, r1, c1, c1, 0 @ write SCR, switch to non-sec Not quite a swith to non-sec; you're still in monitor mode. + isb + mcr p15, 0, r0, c12, c0, 0 @ write non-secure copy of VBAR I don't actually think that you are, I think you're writing the secure copy here. In that case, I'm also wondering if the isb is superflous, because we perform an exception return below, but we of course want to make damn sure that the write of the NS bit is set before the exception return, maybe some ARM guys have the right expertise here. + + movspc, lr This movs is pretty drastic, because it changes from secure to non-secure world, and yes, you can tell by looking at the orr instruction above, but I would prefer a (potentially big fat) comment here as well. .align 5 prefetch_abort: -- 1.7.12.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/6] ARM: add assembly routine to switch to non-secure state
On Mon, May 06, 2013 at 03:17:46PM +0200, Andre Przywara wrote: While actually switching to non-secure state is one thing, the more important part of this process is to make sure that we still have full access to the interrupt controller (GIC). The GIC is fully aware of secure vs. non-secure state, some registers are banked, others may be configured to be accessible from secure state only. To be as generic as possible, we get the GIC memory mapped address based on the PERIPHBASE register. We check explicitly for ARM Cortex-A7 and A15 cores, assuming an A9 otherwise, as for those cores we know the offsets for the GIC CPU interface from the PERIPHBASE content. Other cores could be added as needed. With the GIC accessible, we: a) allow private interrupts to be delivered to the core (GICD_IGROUPR0 = 0x) b) enable the CPU interface (GICC_CTLR[0] = 1) c) set the priority filter to allow non-secure interrupts (GICC_PMR = 0x80) After having switched to non-secure state, we also enable the non-secure GIC CPU interface, since this register is banked. Also we allow access to all coprocessor interfaces from non-secure state by writing the appropriate bits in the NSACR register. For reasons obvious later we only use caller saved registers r0-r3. You probably want to put that in a comment in the code, and it would also be super helpful to explain the obvious part here, because most readers don't look forward in time to understand this patch... Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/start.S | 47 ++ 1 file changed, 47 insertions(+) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index da48b36..e63e892 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -572,3 +572,50 @@ fiq: #endif /* CONFIG_USE_IRQ */ #endif /* CONFIG_SPL_BUILD */ + +#ifdef CONFIG_ARMV7_VIRT +/* Routine to initialize GIC CPU interface and switch to nonsecure state. + */ +.globl _nonsec_gic_switch +_nonsec_gic_switch: + mrc p15, 4, r2, c15, c0, 0 @ r2 = PERIPHBASE You should probably check if bits [7:0] == 0x00 here, otherwise you may end up doing some very strange stuff - if any of those bits are set you can just error out at this point, but it should be gracefully handled. Also since it's core specific, you'd probably want to check that before using it. + add r3, r2, #0x1000 @ GIC dist i/f offset Since this is core specific, could the offset please go in an appropriate header file? It will also eliminate the need for the comment if you just have a proper define (i.e. GIC_DIST_OFFSET ...) + mvn r1, #0 + str r1, [r3, #0x80] @ allow private interrupts Aren't you makeing an assumption about the number of available interrupts here? You should read the ITLinesNumber field from the GICD_TYPER if I'm not mistaking. I also think it would be much cleaner if you again used a define for the 0x80 offset. Also, don't you need to set some enable fields on the GICD_CTLR here to enable group 1 interrupts? + + mrc p15, 0, r0, c0, c0, 0 @ MIDR + bfc r0, #16, #8 @ mask out variant, arch + bfc r0, #0, #4 @ and revision + movwr1, #0xc070 + movtr1, #0x4100 + cmp r0, r1 @ check for Cortex-A7 + orr r1, #0xf0 wow, nice bit fiddling. It may be quite a bit easier to read if you simply had defines for the bitmasks and real values and just did a load and placed a literal section accordingly. + cmpne r0, r1 @ check for Cortex-A15 + movne r1, #0x100 @ GIC CPU offset for A9 So I read the ARMV7_VIRT config flag as something you can only enable on a board that actually supports the virtulization extensions, which the A9 doesn't so this should probably error out instead (or do an endless loop, or ignore the case in the code, ...). + moveq r1, #0x2000 @ GIC CPU offset for A15/A7 Again, I think defines for these are appropriate. + add r3, r2, r1 @ r3 = GIC CPU i/f addr + + mov r1, #1 + str r1, [r3, #0]@ set GICC_CTLR[enable] + mov r1, #0x80 Why are you not setting this to #0xff ? + str r1, [r3, #4]@ set GICC_PIMR[7] + here it seems we're moving into non-gic related stuff in a function called _nonsec_gic_switch + movwr1, #0x3fff + movtr1, #0x0006 + mcr p15, 0, r1, c1, c1, 2 @ NSACR = all copros to non-sec + + ldr r1, =_start + mcr p15, 0, r1, c12, c0, 0 @ set VBAR + mcr p15, 0, r1, c12, c0, 1 @ set MVBAR I thought you already took care of the MVBAR during init? + + isb +
Re: [U-Boot] [PATCH 3/6] ARM: switch to non-secure state during bootm execution
On Mon, May 06, 2013 at 03:17:47PM +0200, Andre Przywara wrote:
To actually trigger the non-secure switch we just implemented, call
the switching routine from within the bootm command implementation.
This way we automatically enable this feature without further user
intervention.
Some part of the work is done in the assembly routine in start.S,
introduced with the previous patch, but for the full glory we need
to setup the GIC distributor interface once for the whole system,
which is done in C here.
The routine is placed in arch/arm/lib to allow easy access from
different boards or CPUs.
I'm beginning to loose track of exactly which parts is in assembly and
which parts are in C, and why. We are fiddling with some gic dist.
settings in assembly, and some of them in C as well.
I think it's just the ordering or naming of the patches that is a little
confusing.
First we check for the availability of the security extensions.
The generic timer base frequency register is only accessible from
secure state, so we have to program it now. Actually this should be
done from primary firmware before, but some boards seems to omit
this, so if needed we do this here with a board specific value.
Since we need a safe way to access the GIC, we use the PERIPHBASE
registers on Cortex-A15 and A7 CPUs and do some sanity checks.
Then we actually do the GIC enablement:
a) enable the GIC distributor, both for non-secure and secure state
(GICD_CTLR[1:0] = 11b)
b) allow all interrupts to be handled from non-secure state
(GICD_IGROUPRn = 0x)
The core specific GIC setup is then done in the assembly routine.
The actual bootm trigger is pretty small: calling the routine and
doing some error reporting. A return value of 1 will be added later.
To enable the whole code we introduce the CONFIG_ARMV7_VIRT variable.
Signed-off-by: Andre Przywara andre.przyw...@linaro.org
---
arch/arm/include/asm/armv7.h | 7 +++
arch/arm/lib/Makefile| 2 +
arch/arm/lib/bootm.c | 20
arch/arm/lib/virt-v7.c | 113
+++
4 files changed, 142 insertions(+)
create mode 100644 arch/arm/lib/virt-v7.c
diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h
index a73630b..25afffe 100644
--- a/arch/arm/include/asm/armv7.h
+++ b/arch/arm/include/asm/armv7.h
@@ -74,4 +74,11 @@ void v7_outer_cache_inval_all(void);
void v7_outer_cache_flush_range(u32 start, u32 end);
void v7_outer_cache_inval_range(u32 start, u32 end);
+#ifdef CONFIG_ARMV7_VIRT
+int armv7_switch_nonsec(void);
+
+/* defined in cpu/armv7/start.S */
+void _nonsec_gic_switch(void);
+#endif /* CONFIG_ARMV7_VIRT */
+
#endif
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index 6ae161a..37a0e71 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -58,6 +58,8 @@ COBJS-y += reset.o
COBJS-y += cache.o
COBJS-y += cache-cp15.o
+COBJS-$(CONFIG_ARMV7_VIRT) += virt-v7.o
+
SRCS := $(GLSOBJS:.o=.S) $(GLCOBJS:.o=.c) \
$(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y))
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index f3b30c5..a3d3aae 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -35,6 +35,10 @@
#include asm/bootm.h
#include linux/compiler.h
+#ifdef CONFIG_ARMV7_VIRT
+#include asm/armv7.h
+#endif
+
DECLARE_GLOBAL_DATA_PTR;
#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
@@ -319,6 +323,22 @@ static void boot_prep_linux(bootm_headers_t *images)
hang();
#endif /* all tags */
}
+#ifdef CONFIG_ARMV7_VIRT
+ switch (armv7_switch_nonsec()) {
+ case 0:
+ debug(entered non-secure state\n);
+ break;
+ case 2:
+ printf(HYP mode: Security extensions not implemented.\n);
+ break;
+ case 3:
+ printf(HYP mode: CPU not supported (must be Cortex-A15 or
A7).\n);
I would drop the specifics of what's supported here.
+ break;
+ case 4:
+ printf(HYP mode: PERIPHBASE is above 4 GB, cannot access
this.\n);
+ break;
+ }
I think these hard-coded numbers are a bit ugly, either define an enum
or some defines somewhere, or just make the armv7_switch_nonsec return a
boolean value and put the prints in there.
That will also make it easier to read the other function and not go
return 2 hmmm, I wonder what that means ;)
+#endif
}
/* Subcommand: GO */
diff --git a/arch/arm/lib/virt-v7.c b/arch/arm/lib/virt-v7.c
new file mode 100644
index 000..3a48aee
--- /dev/null
+++ b/arch/arm/lib/virt-v7.c
@@ -0,0 +1,113 @@
+/*
+ * (C) Copyright 2013
+ * Andre Przywara, Linaro
+ *
+ * routines to push ARMv7 processors from secure into non-secure state
+ * needed to enable ARMv7 virtualization for current hypervisors
Nits: Routines should be capitalized.
Re: [U-Boot] [PATCH 5/6] ARM: extend non-secure switch to also go into HYP mode
On Mon, May 06, 2013 at 03:17:49PM +0200, Andre Przywara wrote: For the KVM and XEN hypervisors to be usable, we need to enter the kernel in HYP mode. Now that we already are in non-secure state, HYP mode switching is within short reach. While doing the non-secure switch, we have to enable the HVC instruction and setup the HYP mode HVBAR (while still secure). The actual switch is done by dropping back from a HYP mode handler without actually leaving HYP mode, so we introduce a new handler routine in the exception vector table. In the assembly switching routine - which we rename to hyp_gic_switch on the way - we save and restore the banked LR and SP registers around the hypercall to do the actual HYP mode switch. The C routine first checks whether we are in HYP mode already and also whether the virtualization extensions are available. It also checks whether the HYP mode switch was finally successful. The bootm command part only adds and adjusts some error reporting. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/start.S | 34 +++--- arch/arm/include/asm/armv7.h | 4 ++-- arch/arm/lib/bootm.c | 12 +--- arch/arm/lib/virt-v7.c | 22 +++--- 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 02234c7..921e9d9 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -41,7 +41,7 @@ _start: b reset ldr pc, _software_interrupt ldr pc, _prefetch_abort ldr pc, _data_abort - ldr pc, _not_used + ldr pc, _hyp_trap ldr pc, _irq ldr pc, _fiq #ifdef CONFIG_SPL_BUILD @@ -49,7 +49,7 @@ _undefined_instruction: .word _undefined_instruction _software_interrupt: .word _software_interrupt _prefetch_abort: .word _prefetch_abort _data_abort: .word _data_abort -_not_used: .word _not_used +_hyp_trap: .word _hyp_trap _irq:.word _irq _fiq:.word _fiq _pad:.word 0x12345678 /* now 16*4=64 */ @@ -58,7 +58,7 @@ _undefined_instruction: .word undefined_instruction _software_interrupt: .word software_interrupt _prefetch_abort: .word prefetch_abort _data_abort: .word data_abort -_not_used: .word not_used +_hyp_trap: .word hyp_trap _irq:.word irq _fiq:.word fiq _pad:.word 0x12345678 /* now 16*4=64 */ @@ -513,12 +513,18 @@ software_interrupt: mrc p15, 0, r1, c1, c1, 0 @ read SCR bic r1, r1, #0x07f orr r1, r1, #0x31 @ enable NS, AW, FW + mrc p15, 0, r0, c0, c1, 1 @ check for Virt ext + and r0, r0, #0xf000 + cmp r0, #0x1000 you can just do ands r0, r0 ,#0xf000 and change the orreq below to orrne + orreq r1, r1, #0x100 @ allow HVC instruction mrc p15, 0, r0, c12, c0, 0 @ save secure copy of VBAR mcr p15, 0, r1, c1, c1, 0 @ write SCR, switch to non-sec isb mcr p15, 0, r0, c12, c0, 0 @ write non-secure copy of VBAR + mcreq p15, 4, r0, c12, c0, 0 @ write HYP mode HVBAR + nit: s/HYP mode// movspc, lr .align 5 @@ -534,10 +540,9 @@ data_abort: bl do_data_abort .align 5 -not_used: - get_bad_stack - bad_save_user_regs - bl do_not_used +hyp_trap: + .byte 0x00, 0xe3, 0x0e, 0xe1@ mrs lr, elr_hyp do we really need to support this on assemblers that old? + mov pc, lr #ifdef CONFIG_USE_IRQ @@ -574,21 +579,21 @@ fiq: #endif /* CONFIG_SPL_BUILD */ #ifdef CONFIG_ARMV7_VIRT -/* Routine to initialize GIC CPU interface and switch to nonsecure state. - * Will be executed directly by secondary CPUs after coming out of +/* Routine to initialize GIC CPU interface, switch to nonsecure and to HYP + * mode. Will be executed directly by secondary CPUs after coming out of So now this routine does three different things in different context at once, why? * WFI, or can be called directly by C code for CPU 0. * Those two paths mandate to not use any stack and to only use registers * r0-r3 to comply with both the C ABI and the requirement of SMP startup * code. */ -.globl _nonsec_gic_switch +.globl _hyp_gic_switch .globl _smp_pen _smp_pen: mrs r0, cpsr orr r0, r0, #0xc0 msr cpsr, r0@ disable interrupts mov lr, #0 @ clear LR to mark secondary -_nonsec_gic_switch: +_hyp_gic_switch: mrc p15, 4, r2, c15, c0, 0 @ r2 = PERIPHBASE add r3, r2, #0x1000 @ GIC dist
Re: [U-Boot] [PATCH 4/6] ARM: add SMP support for non-secure switch
On Mon, May 06, 2013 at 03:17:48PM +0200, Andre Przywara wrote: Currently the non-secure switch is only done for the boot processor. To later allow full SMP support, we have to switch all secondary cores into non-secure state also. So we add an entry point for secondary CPUs coming out of low-power state and make sure we put them into WFI again after having switched to non-secure state. For this we acknowledge and EOI the wake-up IPI, then go into WFI. Once being kicked out of it later, we sanity check that the start address has actually been changed (since another attempt to switch to non-secure would block the core) and jump to the new address. The actual CPU kick is done by sending an inter-processor interrupt via the GIC to all CPU interfaces except the requesting processor. The secondary cores will then setup their respective GIC CPU interface. The address secondary cores jump to is board specific, we provide the value here for the Versatile Express board. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/start.S | 27 ++- arch/arm/include/asm/armv7.h| 1 + arch/arm/lib/virt-v7.c | 9 - include/configs/vexpress_ca15_tc2.h | 1 + 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index e63e892..02234c7 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -575,8 +575,19 @@ fiq: #ifdef CONFIG_ARMV7_VIRT /* Routine to initialize GIC CPU interface and switch to nonsecure state. + * Will be executed directly by secondary CPUs after coming out of + * WFI, or can be called directly by C code for CPU 0. + * Those two paths mandate to not use any stack and to only use registers + * r0-r3 to comply with both the C ABI and the requirement of SMP startup + * code. */ .globl _nonsec_gic_switch +.globl _smp_pen +_smp_pen: + mrs r0, cpsr + orr r0, r0, #0xc0 + msr cpsr, r0@ disable interrupts + mov lr, #0 @ clear LR to mark secondary instead of this subtle abuse of lr, why not make this routine simply take a parameter? I also slightly object against wrapping the _smp_pen around the _nonsec_gic_switch, I really think these are separate routines, where one can just call the other...? _nonsec_gic_switch: mrc p15, 4, r2, c15, c0, 0 @ r2 = PERIPHBASE add r3, r2, #0x1000 @ GIC dist i/f offset @@ -617,5 +628,19 @@ _nonsec_gic_switch: add r2, r2, #0x1000 @ GIC dist i/f offset str r1, [r2]@ allow private interrupts - mov pc, lr + cmp lr, #0 + movne pc, lr @ CPU 0 to return + @ all others: go to sleep +_ack_int: + ldr r1, [r3, #0x0c] @ read GICD acknowledge + str r1, [r3, #0x10] @ write GICD EOI + + adr r1, _smp_pen +waitloop: + wfi + ldr r0, =CONFIG_SYSFLAGS_ADDR @ load start address + ldr r0, [r0] + cmp r0, r1 @ make sure we dont execute this code I think I raised this issue previously, but this code is in a core u-boot file, but I could imagine a board with a different crazy boot protocol that required you to check two different fields and jump through other hoops to wake up from the smp pen, so I really think the whole smp pen belongs in a board specific place. Also, the boot-wrapper code used wfe instead, not sure if there are any users that just send an event and doesn't send an IPI? + beq waitloop@ again (due to a spurious wakeup) + mov pc, r0 #endif /* CONFIG_ARMV7_VIRT */ diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h index 25afffe..296dc92 100644 --- a/arch/arm/include/asm/armv7.h +++ b/arch/arm/include/asm/armv7.h @@ -78,6 +78,7 @@ void v7_outer_cache_inval_range(u32 start, u32 end); int armv7_switch_nonsec(void); /* defined in cpu/armv7/start.S */ +void _smp_pen(void); void _nonsec_gic_switch(void); #endif /* CONFIG_ARMV7_VIRT */ diff --git a/arch/arm/lib/virt-v7.c b/arch/arm/lib/virt-v7.c index 3a48aee..0248010 100644 --- a/arch/arm/lib/virt-v7.c +++ b/arch/arm/lib/virt-v7.c @@ -48,6 +48,7 @@ int armv7_switch_nonsec(void) unsigned int reg; volatile unsigned int *gicdptr; unsigned itlinesnr, i; + unsigned int *sysflags; /* check whether the CPU supports the security extensions */ asm(mrc p15, 0, %0, c0, c1, 1\n : =r(reg)); @@ -106,7 +107,13 @@ int armv7_switch_nonsec(void) for (i = 0; i = itlinesnr; i++) gicdptr[GICD_IGROUPR0 / 4 + i] = (unsigned)-1; - /* call the non-sec switching code on this CPU */ +
Re: [U-Boot] [RFC PATCH 4/6] ARM: add SMP support for non-secure switch
On Mon, May 06, 2013 at 03:19:40PM +0200, Andre Przywara wrote: On 04/27/2013 12:13 AM, Christoffer Dall wrote: On Fri, Apr 26, 2013 at 6:14 AM, Andre Przywara andre.przyw...@linaro.org wrote: Currently the non-secure switch is only done for the boot processor. To later allow full SMP support, we have to switch all secondary cores into non-secure state also. So we add an entry point for secondary CPUs coming out of low-power state and make sure we put them into WFI again after having switched to non-secure state. For this we acknowledge and EOI the wake-up IPI, then go into WFI. Once being kicked out of it later, we sanity check that the start address has actually been changed (since another attempt to switch to non-secure would block the core) and jump to the new address. The actual CPU kick is done by sending an inter-processor interrupt via the GIC to all CPU interfaces except the requesting processor. The secondary cores will then setup their respective GIC CPU interface. The address secondary cores jump to is board specific, we provide the value here for the Versatile Express board. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/start.S | 27 ++- arch/arm/lib/virt-v7.c | 10 +- include/configs/vexpress_ca15_tc2.h | 1 + 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 401b0eb..2b47881 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -563,8 +563,19 @@ fiq: #endif /* CONFIG_SPL_BUILD */ /* Routine to initialize GIC CPU interface and switch to nonsecure state. + * Will be executed directly by secondary CPUs after coming out of + * WFI, or can be called directly by C code for CPU 0. + * Those two paths mandate to not use any stack and to only use registers + * r0-r3 to comply with both the C ABI and the requirement of SMP startup + * code. */ .globl _nonsec_gic_switch +.globl _smp_pen +_smp_pen: + mrs r0, cpsr + orr r0, r0, #0xc0 + msr cpsr, r0@ disable interrupts + mov lr, #0 @ clear LR to mark secondary _nonsec_gic_switch: mrc p15, 4, r2, c15, c0, 0 @ r2 = PERIPHBASE add r3, r2, #0x1000 @ GIC dist i/f offset @@ -605,4 +616,18 @@ _nonsec_gic_switch: add r2, r2, #0x1000 @ GIC dist i/f offset str r1, [r2]@ allow private interrupts - mov pc, lr + cmp lr, #0 + movne pc, lr @ CPU 0 to return + @ all others: go to sleep +_ack_int: + ldr r1, [r3, #0x0c] @ read GICD acknowledge + str r1, [r3, #0x10] @ write GICD EOI + + adr r1, _smp_pen +waitloop: + wfi + ldr r0, =CONFIG_SYSFLAGS_ADDR @ load start address + ldr r0, [r0] + cmp r0, r1 @ make sure we dont execute this code + beq waitloop@ again (due to a spurious wakeup) + mov pc, r0 diff --git a/arch/arm/lib/virt-v7.c b/arch/arm/lib/virt-v7.c index 416ca29..5ca093a 100644 --- a/arch/arm/lib/virt-v7.c +++ b/arch/arm/lib/virt-v7.c @@ -29,6 +29,7 @@ /* the assembly routine doing the actual work in start.S */ void _nonsec_gic_switch(void); +void _smp_pen(void); #define GICD_CTLR 0x000 #define GICD_TYPER 0x004 @@ -51,6 +52,7 @@ int armv7_switch_nonsec(void) unsigned int reg; volatile unsigned int *gicdptr; unsigned itlinesnr, i; + unsigned int *sysflags; /* check whether the CPU supports the security extensions */ asm(mrc p15, 0, %0, c0, c1, 1\n : =r(reg)); @@ -109,7 +111,13 @@ int armv7_switch_nonsec(void) for (i = 0; i = itlinesnr; i++) gicdptr[GICD_IGROUPR0 / 4 + i] = (unsigned)-1; - /* call the non-sec switching code on this CPU */ + /* now kick all CPUs (expect this one) by writing to GICD_SIGR */ + sysflags = (void *)CONFIG_SYSFLAGS_ADDR; + sysflags[1] = (unsigned)-1; + sysflags[0] = (uintptr_t)_smp_pen; + gicdptr[GICD_SGIR / 4] = 1U 24; + + /* call the non-sec switching code on this CPU also */ _nonsec_gic_switch(); return 0; diff --git a/include/configs/vexpress_ca15_tc2.h b/include/configs/vexpress_ca15_tc2.h index 9e230ad..210a27c 100644 --- a/include/configs/vexpress_ca15_tc2.h +++ b/include/configs/vexpress_ca15_tc2.h @@ -32,5 +32,6 @@ #define CONFIG_BOOTP_VCI_STRING U-boot.armv7.vexpress_ca15x2_tc2 #define CONFIG_SYS_CLK_FREQ 2400 +#define CONFIG_SYSFLAGS_ADDR 0x1c010030 #endif -- hmm, is all
Re: [U-Boot] [RFC PATCH 0/6] ARMv7: Add HYP mode switching support
On Fri, Apr 26, 2013 at 7:14 AM, Andre Przywara andre.przyw...@linaro.org wrote: On 04/26/2013 03:42 PM, Peter Maydell wrote: On 26 April 2013 14:24, Andre Przywara andre.przyw...@linaro.org wrote: On 04/26/2013 03:18 PM, Peter Maydell wrote: The obvious question here is why do we need a new command?. The kernel booting specification says boot the kernel in Hyp mode so we should just always do that for booting Linux, surely? Because it avoids regressions. I kind of feel uneasy to do a lot of tinkering with secure state and the GIC unconditionally, especially if enabled on many boards with virt-capable CPUs. There aren't exactly very many of those out there, so if we default to boot in HYP mode then (a) KVM will just work out of the box and (b) people doing u-boot ports to their board will find any issues and be able to submit fixes. If we don't turn it on by default then we'll end up stuck in a world where KVM doesn't work on half the virt capable boards out there. OK, that's a point. I changed the code already and will include it in the next revision. I strongly agree with Peter, and in fact I think it should only be a compile time option, not a run-time option, only for those debugging u-boot and a kernel on some new platform. -Christoffer ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 1/6] ARM: add secure monitor handler to switch to non-secure state
On Fri, Apr 26, 2013 at 03:14:54PM +0200, Andre Przywara wrote: A prerequisite for using virtualization is to be in HYP mode, which requires the CPU to be in non-secure state. According to the ARM ARM this should not be done in SVC mode, so we have to setup a SMC handler for this. We reuse the current vector table for this and make sure that we only access the MVBAR register if the CPU supports the virtualization extensions. Introduce a monitor handler routine which switches the CPU to non-secure state by setting the NS bit (and associated bits). Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/start.S | 20 +--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index e9e57e6..7bfb19d 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -155,6 +155,10 @@ reset: /* Set vector address in CP15 VBAR register */ ldr r0, =_start mcr p15, 0, r0, c12, c0, 0 @Set VBAR + + mrc p15, 0, r1, c0, c1, 1 @ check for security extension + ands r1, r1, #0x30 + mcrne p15, 0, r0, c12, c0, 1 @ Set secure monitor MVBAR #endif /* the mask ROM code should have PLL and others stable */ @@ -256,6 +260,9 @@ ENTRY(c_runtime_cpu_setup) /* Set vector address in CP15 VBAR register */ ldr r0, =_start mcr p15, 0, r0, c12, c0, 0 @Set VBAR + mrc p15, 0, r1, c0, c1, 1 @ check for security extension + ands r1, r1, #0x30 + mcrne p15, 0, r0, c12, c0, 1 @ Set secure monitor MVBAR bx lr @@ -492,9 +499,16 @@ undefined_instruction: .align 5 software_interrupt: - get_bad_stack_swi - bad_save_user_regs - bl do_software_interrupt I think you should add a comment block here saying that this is handling an smc #0 call and the intent is to change to non-secure world. + mrc p15, 0, r1, c1, c1, 0 @ read SCR + bic r1, r1, #0x07f + orr r1, r1, #0x31 @ enable NS, AW, FW + + mrc p15, 0, r0, c12, c0, 0 @ save secure copy of VBAR + mcr p15, 0, r1, c1, c1, 0 @ write SCR, switch to non-sec + isb + mcr p15, 0, r0, c12, c0, 0 @ write non-secure copy of VBAR + + movs pc, lr .align 5 prefetch_abort: -- 1.7.12.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 4/6] ARM: add SMP support for non-secure switch
On Fri, Apr 26, 2013 at 6:14 AM, Andre Przywara andre.przyw...@linaro.org wrote: Currently the non-secure switch is only done for the boot processor. To later allow full SMP support, we have to switch all secondary cores into non-secure state also. So we add an entry point for secondary CPUs coming out of low-power state and make sure we put them into WFI again after having switched to non-secure state. For this we acknowledge and EOI the wake-up IPI, then go into WFI. Once being kicked out of it later, we sanity check that the start address has actually been changed (since another attempt to switch to non-secure would block the core) and jump to the new address. The actual CPU kick is done by sending an inter-processor interrupt via the GIC to all CPU interfaces except the requesting processor. The secondary cores will then setup their respective GIC CPU interface. The address secondary cores jump to is board specific, we provide the value here for the Versatile Express board. Signed-off-by: Andre Przywara andre.przyw...@linaro.org --- arch/arm/cpu/armv7/start.S | 27 ++- arch/arm/lib/virt-v7.c | 10 +- include/configs/vexpress_ca15_tc2.h | 1 + 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 401b0eb..2b47881 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -563,8 +563,19 @@ fiq: #endif /* CONFIG_SPL_BUILD */ /* Routine to initialize GIC CPU interface and switch to nonsecure state. + * Will be executed directly by secondary CPUs after coming out of + * WFI, or can be called directly by C code for CPU 0. + * Those two paths mandate to not use any stack and to only use registers + * r0-r3 to comply with both the C ABI and the requirement of SMP startup + * code. */ .globl _nonsec_gic_switch +.globl _smp_pen +_smp_pen: + mrs r0, cpsr + orr r0, r0, #0xc0 + msr cpsr, r0@ disable interrupts + mov lr, #0 @ clear LR to mark secondary _nonsec_gic_switch: mrc p15, 4, r2, c15, c0, 0 @ r2 = PERIPHBASE add r3, r2, #0x1000 @ GIC dist i/f offset @@ -605,4 +616,18 @@ _nonsec_gic_switch: add r2, r2, #0x1000 @ GIC dist i/f offset str r1, [r2]@ allow private interrupts - mov pc, lr + cmp lr, #0 + movne pc, lr @ CPU 0 to return + @ all others: go to sleep +_ack_int: + ldr r1, [r3, #0x0c] @ read GICD acknowledge + str r1, [r3, #0x10] @ write GICD EOI + + adr r1, _smp_pen +waitloop: + wfi + ldr r0, =CONFIG_SYSFLAGS_ADDR @ load start address + ldr r0, [r0] + cmp r0, r1 @ make sure we dont execute this code + beq waitloop@ again (due to a spurious wakeup) + mov pc, r0 diff --git a/arch/arm/lib/virt-v7.c b/arch/arm/lib/virt-v7.c index 416ca29..5ca093a 100644 --- a/arch/arm/lib/virt-v7.c +++ b/arch/arm/lib/virt-v7.c @@ -29,6 +29,7 @@ /* the assembly routine doing the actual work in start.S */ void _nonsec_gic_switch(void); +void _smp_pen(void); #define GICD_CTLR 0x000 #define GICD_TYPER 0x004 @@ -51,6 +52,7 @@ int armv7_switch_nonsec(void) unsigned int reg; volatile unsigned int *gicdptr; unsigned itlinesnr, i; + unsigned int *sysflags; /* check whether the CPU supports the security extensions */ asm(mrc p15, 0, %0, c0, c1, 1\n : =r(reg)); @@ -109,7 +111,13 @@ int armv7_switch_nonsec(void) for (i = 0; i = itlinesnr; i++) gicdptr[GICD_IGROUPR0 / 4 + i] = (unsigned)-1; - /* call the non-sec switching code on this CPU */ + /* now kick all CPUs (expect this one) by writing to GICD_SIGR */ + sysflags = (void *)CONFIG_SYSFLAGS_ADDR; + sysflags[1] = (unsigned)-1; + sysflags[0] = (uintptr_t)_smp_pen; + gicdptr[GICD_SGIR / 4] = 1U 24; + + /* call the non-sec switching code on this CPU also */ _nonsec_gic_switch(); return 0; diff --git a/include/configs/vexpress_ca15_tc2.h b/include/configs/vexpress_ca15_tc2.h index 9e230ad..210a27c 100644 --- a/include/configs/vexpress_ca15_tc2.h +++ b/include/configs/vexpress_ca15_tc2.h @@ -32,5 +32,6 @@ #define CONFIG_BOOTP_VCI_STRING U-boot.armv7.vexpress_ca15x2_tc2 #define CONFIG_SYS_CLK_FREQ 2400 +#define CONFIG_SYSFLAGS_ADDR 0x1c010030 #endif -- hmm, is all this likely to work across all armv7 cores? I imagined that the SMP boot protocol could be potentially vastly different on different implementations and this
