| Message ID | 1379606806-439-3-git-send-email-andre.przywara@linaro.org |
|---|---|
| State | Accepted |
| Commit | 45b940d6f9a9d4989452ea67480e299bfa51ee19 |
| Headers | show |
Just checking, is the mcr p15,0,r1,c1,c1,0 in sync with the following text
. I could be wrong here, just checking
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
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.przywara@linaro.org>wrote:
> 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.przywara@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 0000000..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.przywara@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)
> +
> + movs pc, 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
>
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.przywara@linaro.org>wrote: > > > 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.przywara@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 0000000..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.przywara@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) > > + > > + movs pc, 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 > > > > > > -- > -mj
On Fri, Sep 20, 2013 at 6:12 AM, Christoffer Dall < christoffer.dall@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. > > > > > 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.przywara@linaro.org>wrote: > > > > > 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.przywara@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 0000000..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.przywara@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) > > > + > > > + movs pc, 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 > > > > > > > > > > > -- > > -mj > > -- > Christoffer >
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.dall@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
Hi Andre, On Thu, 19 Sep 2013 18:06:40 +0200, Andre Przywara <andre.przywara@linaro.org> wrote: > 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.przywara@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 BTW, this has an "empty line at EOF" warning when applying. Amicalement,
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 0000000..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.przywara@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) + + movs pc, lr @ return to non-secure SVC +
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.przywara@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