diff mbox

[1/2] monitor: Make sure SCR.NS=1 before trying to access hyp config regs

Message ID 1329330098-31636-2-git-send-email-dave.martin@linaro.org
State Not Applicable
Headers show

Commit Message

Dave Martin Feb. 15, 2012, 6:21 p.m. UTC
The architecture says that registers such as VTTBR and HVBAR are
only accessible from the Secure World if in Monitor mode and with
SCR.NS = 1.

Myabe some versions of the model don't enforce this, but the one I
have does, and it appears to be an architectural requirement.  This
patch avoids the resulting #undefs.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 monitor.S |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)

Comments

Rusty Russell Feb. 19, 2012, 11:20 p.m. UTC | #1
On Wed, 15 Feb 2012 18:21:37 +0000, Dave Martin <dave.martin@linaro.org> wrote:
> The architecture says that registers such as VTTBR and HVBAR are
> only accessible from the Secure World if in Monitor mode and with
> SCR.NS = 1.
> 
> Myabe some versions of the model don't enforce this, but the one I
> have does, and it appears to be an architectural requirement.  This
> patch avoids the resulting #undefs.

Damn, I wish I'd read this before I started debugging!  Thanks for doing
all the work though.

I've pushed your fixes to github for the moment.  I'll fold them
together into a nice bisectable commit for merge into the linaro
boot-wrapper tree.

This will be cleaner when I remove the old hvbar trap, but this works.

Thanks!
Rusty.
Dave Martin Feb. 20, 2012, 10:03 a.m. UTC | #2
On Mon, Feb 20, 2012 at 09:50:03AM +1030, Rusty Russell wrote:
> On Wed, 15 Feb 2012 18:21:37 +0000, Dave Martin <dave.martin@linaro.org> wrote:
> > The architecture says that registers such as VTTBR and HVBAR are
> > only accessible from the Secure World if in Monitor mode and with
> > SCR.NS = 1.
> > 
> > Myabe some versions of the model don't enforce this, but the one I
> > have does, and it appears to be an architectural requirement.  This
> > patch avoids the resulting #undefs.
> 
> Damn, I wish I'd read this before I started debugging!  Thanks for doing
> all the work though.

This wansn't too much of an issue, since I've encountered this kind of
thing before.  It's not that intuitive though... I can kind of see why
SCR.NS=1 is needed there, but the hypervisor system registers are not
actually banked between the Secure and Normal Worlds, so they could
have got away without it.

> I've pushed your fixes to github for the moment.  I'll fold them
> together into a nice bisectable commit for merge into the linaro
> boot-wrapper tree.
> 
> This will be cleaner when I remove the old hvbar trap, but this works.

My fix is a bodge really -- it could probably be done more neatly.

Thanks for merging it in, anyway.

Cheers
---Dave
diff mbox

Patch

diff --git a/monitor.S b/monitor.S
index 052ab1e..334186e 100644
--- a/monitor.S
+++ b/monitor.S
@@ -25,7 +25,7 @@ 
 	@
 1:
 	ldr	sp, =_monitor_stack
-	push	{r11, r12}
+	push	{r10-r12,lr}
 
 	cmp	r7, #0xffffffff
 	beq	_non_sec
@@ -36,16 +36,19 @@ 
 	movnes	pc, lr
 	and	r12, r7, #0xf
 	cmp	r12, #0x0
-	popgt	{r11, r12}
-	movgts	pc, lr
+	ldmgtfd	sp!, {r10-r12,pc}^
 
 	@ Check the VMID is 0
+	mrc	p15, 0, r10, c1, c1, 0		@ SCR
+	orr	r11, r10, #1			@ SCR.NS = 1
+	mcr	p15, 0, r11, c1, c1, 0
+	isb
 	mrrc	p15, 6, r12, r11, c2
+	mcr	p15, 0, r10, c1, c1, 0		@ Restore SCR
 	lsr	r11, r11, #16
 	and	r11, r11, #0xff
 	cmp	r11, #0
-	popne	{r11, r12}
-	movnes	pc, lr
+	ldmnefd	sp!, {r10-r12,pc}^
 
 	@ Jump to the right function
 	and	r12, r7, #0xf
@@ -68,16 +71,18 @@  _non_sec:
 	ldr	r11, =0x131
 	orr	r12, r12, r11
 	mcr	p15, 0, r12, c1, c1, 0
-	pop	{r11, r12}
-	movs	pc, lr
+	ldmfd	sp!, {r10-r12,pc}^
 
 	@
 	@ Read/Write HVBAR
 	@
 _write_hvbar:
+	orr	r11, r10, #1			@ SCR.NS = 1 (r10 already = SCR)
+	mcr	p15, 0, r11, c1, c1, 0
+	isb
 	mcr	p15, 4, r0, c12, c0, 0
-	pop	{r11, r12}
-	movs	pc, lr
+	mcr	p15, 0, r10, c1, c1, 0		@ Restore SCR
+	ldmfd	sp!, {r10-r12,pc}^
 
 	.ltorg