Re: [U-Boot] [PATCH 3/6] ARM: switch to non-secure state during bootm execution
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 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 extensions...)
+ 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.
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 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.
