[U-Boot] [PATCH v5 2/8] ARM: add secure monitor handler to switch to non-secure state

[Christoffer Dall]

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 at 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 at 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 at 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 at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> >
> 
> 
> 
> -- 
> -mj

-- 
Christoffer