xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0 of 6] ARM: various boot-time tidying
@ 2012-05-31 16:39 Tim Deegan
  2012-05-31 16:39 ` [PATCH 1 of 6] arm: More interrupt setup at start-of-day for secondary CPUs Tim Deegan
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Tim Deegan @ 2012-05-31 16:39 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, ian.campbell

These patches tinker with the ARM boot a little bit.  They shouldn't
clash with any of the other ARM work going on, since they only touch
very early code.

Cheers,

Tim.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1 of 6] arm: More interrupt setup at start-of-day for secondary CPUs
  2012-05-31 16:39 [PATCH 0 of 6] ARM: various boot-time tidying Tim Deegan
@ 2012-05-31 16:39 ` Tim Deegan
  2012-05-31 16:40 ` [PATCH 2 of 6] arm: Move hyp-mode entry code out of line Tim Deegan
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Tim Deegan @ 2012-05-31 16:39 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, ian.campbell

# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1338482127 -3600
# Node ID 25d389d891c5a2a3009258ef7261379a9ad97746
# Parent  d7318231cfe3498947bf75f09d6675407d7b4e76
arm: More interrupt setup at start-of-day for secondary CPUs.

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r d7318231cfe3 -r 25d389d891c5 xen/arch/arm/smpboot.c
--- a/xen/arch/arm/smpboot.c	Thu May 31 10:18:52 2012 +0200
+++ b/xen/arch/arm/smpboot.c	Thu May 31 17:35:27 2012 +0100
@@ -107,6 +107,8 @@ void __cpuinit start_secondary(unsigned 
 
     mmu_init_secondary_cpu();
     gic_init_secondary_cpu();
+    init_timer_interrupt();
+    gic_route_irqs();
 
     set_current(idle_vcpu[cpuid]);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2 of 6] arm: Move hyp-mode entry code out of line
  2012-05-31 16:39 [PATCH 0 of 6] ARM: various boot-time tidying Tim Deegan
  2012-05-31 16:39 ` [PATCH 1 of 6] arm: More interrupt setup at start-of-day for secondary CPUs Tim Deegan
@ 2012-05-31 16:40 ` Tim Deegan
  2012-05-31 16:54   ` David Vrabel
  2012-05-31 16:40 ` [PATCH 3 of 6] arm: avoid memory write in switch to Hyp mode Tim Deegan
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Tim Deegan @ 2012-05-31 16:40 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, ian.campbell

# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1338482127 -3600
# Node ID 05447f395c91029fb732142e36788cfa92374045
# Parent  25d389d891c5a2a3009258ef7261379a9ad97746
arm: Move hyp-mode entry code out of line.

This code is grottier than the rest of the start-of-day code and
very specific to the software model we're developing on.
Segregate it accordingly, by putting it in its own file.

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r 25d389d891c5 -r 05447f395c91 xen/arch/arm/Makefile
--- a/xen/arch/arm/Makefile	Thu May 31 17:35:27 2012 +0100
+++ b/xen/arch/arm/Makefile	Thu May 31 17:35:27 2012 +0100
@@ -12,6 +12,7 @@ obj-y += io.o
 obj-y += irq.o
 obj-y += kernel.o
 obj-y += mm.o
+obj-y += mode_switch.o
 obj-y += p2m.o
 obj-y += percpu.o
 obj-y += guestcopy.o
diff -r 25d389d891c5 -r 05447f395c91 xen/arch/arm/head.S
--- a/xen/arch/arm/head.S	Thu May 31 17:35:27 2012 +0100
+++ b/xen/arch/arm/head.S	Thu May 31 17:35:27 2012 +0100
@@ -122,50 +122,9 @@ 1:
 1:
 	/* OK, we're in Secure state. */
 	PRINT("- Started in Secure state -\r\n- Entering Hyp mode -\r\n")
-
-	/* Dance into Hyp mode */
-	cpsid aif, #0x16             /* Enter Monitor mode */
-	mrc   CP32(r0, SCR)
-	orr   r0, r0, #0x100         /* Set HCE */
-	orr   r0, r0, #0xb1          /* Set SCD, AW, FW and NS */
-	bic   r0, r0, #0xe           /* Clear EA, FIQ and IRQ */
-	mcr   CP32(r0, SCR)
-	/* Ugly: the system timer's frequency register is only
-	 * programmable in Secure state.  Since we don't know where its
-	 * memory-mapped control registers live, we can't find out the
-	 * right frequency.  Use the VE model's default frequency here. */
-	ldr   r0, =0x5f5e100         /* 100 MHz */
-	mcr   CP32(r0, CNTFRQ)
-	ldr   r0, =0x40c00           /* SMP, c11, c10 in non-secure mode */
-	mcr   CP32(r0, NSACR)
-	/* Continuing ugliness: Set up the GIC so NS state owns interrupts */
-	mov   r0, #GIC_BASE_ADDRESS
-	add   r0, r0, #GIC_DR_OFFSET
-	mov   r1, #0
-	str   r1, [r0]               /* Disable delivery in the distributor */
-	add   r0, r0, #0x80          /* GICD_IGROUP0 */
-	mov   r2, #0xffffffff        /* All interrupts to group 1 */
-	str   r2, [r0]
-	str   r2, [r0, #4]
-	str   r2, [r0, #8]
-	/* Must drop priority mask below 0x80 before entering NS state */
-	mov   r0, #GIC_BASE_ADDRESS
-	add   r0, r0, #GIC_CR_OFFSET
-	ldr   r1, =0xff
-	str   r1, [r0, #0x4]         /* -> GICC_PMR */
-	/* Reset a few config registers */
-	mov   r0, #0
-	mcr   CP32(r0, FCSEIDR)
-	mcr   CP32(r0, CONTEXTIDR)
-	/* FIXME: ought to reset some other NS control regs here */
-	adr   r1, 1f
-	adr   r0, hyp                /* Store paddr (hyp entry point) */
-	str   r0, [r1]               /* where we can use it for RFE */
-	isb                          /* Ensure we see the stored target address */
-	rfeia r1                     /* Enter Hyp mode */
-
-1:	.word 0                      /* PC to enter Hyp mode at */
-	.word 0x000001da             /* CPSR: LE, Abort/IRQ/FIQ off, Hyp */
+	ldr   r0, =enter_hyp_mode    /* VA of function */
+	adr   lr, hyp                /* Set return address for call */
+	add   pc, r0, r10            /* Call PA of function */
 
 hyp:
 	PRINT("- Setting up control registers -\r\n")
diff -r 25d389d891c5 -r 05447f395c91 xen/arch/arm/mode_switch.S
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/arch/arm/mode_switch.S	Thu May 31 17:35:27 2012 +0100
@@ -0,0 +1,76 @@
+/*
+ * xen/arch/arm/mode_switch.S
+ *
+ * Start-of day code to take a CPU from Secure mode to Hyp mode.
+ *
+ * Tim Deegan <tim@xen.org>
+ * Copyright (c) 2011-2012 Citrix Systems.
+ *
+ * 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.
+ */
+
+#include <asm/config.h>
+#include <asm/page.h>
+#include <asm/asm_defns.h>
+
+/* Get up a CPU into Hyp mode.  Clobbers r0-r3.
+ *
+ * This code is specific to the VE model, and not intended to be used
+ * on production systems.  As such it's a bit hackier than the main
+ * boot code in head.S.  In future it will be replaced by better
+ * integration with the bootloader/firmware so that Xen always starts
+ * in Hyp mode. */
+
+.globl enter_hyp_mode
+enter_hyp_mode:
+	mov   r3, lr                 /* Put return address in non-banked reg */
+	cpsid aif, #0x16             /* Enter Monitor mode */
+	mrc   CP32(r0, SCR)
+	orr   r0, r0, #0x100         /* Set HCE */
+	orr   r0, r0, #0xb1          /* Set SCD, AW, FW and NS */
+	bic   r0, r0, #0xe           /* Clear EA, FIQ and IRQ */
+	mcr   CP32(r0, SCR)
+	/* Ugly: the system timer's frequency register is only
+	 * programmable in Secure state.  Since we don't know where its
+	 * memory-mapped control registers live, we can't find out the
+	 * right frequency.  Use the VE model's default frequency here. */
+	ldr   r0, =0x5f5e100         /* 100 MHz */
+	mcr   CP32(r0, CNTFRQ)
+	ldr   r0, =0x40c00           /* SMP, c11, c10 in non-secure mode */
+	mcr   CP32(r0, NSACR)
+	/* Continuing ugliness: Set up the GIC so NS state owns interrupts */
+	mov   r0, #GIC_BASE_ADDRESS
+	add   r0, r0, #GIC_DR_OFFSET
+	mov   r1, #0
+	str   r1, [r0]               /* Disable delivery in the distributor */
+	add   r0, r0, #0x80          /* GICD_IGROUP0 */
+	mov   r2, #0xffffffff        /* All interrupts to group 1 */
+	str   r2, [r0]
+	str   r2, [r0, #4]
+	str   r2, [r0, #8]
+	/* Must drop priority mask below 0x80 before entering NS state */
+	mov   r0, #GIC_BASE_ADDRESS
+	add   r0, r0, #GIC_CR_OFFSET
+	ldr   r1, =0xff
+	str   r1, [r0, #0x4]         /* -> GICC_PMR */
+	/* Reset a few config registers */
+	mov   r0, #0
+	mcr   CP32(r0, FCSEIDR)
+	mcr   CP32(r0, CONTEXTIDR)
+	/* FIXME: ought to reset some other NS control regs here */
+	adr   r1, 1f                 /* Store return address */
+	str   r3, [r1]               /* where we can use it for RFE */
+	isb                          /* Ensure we see the stored address */
+	rfeia r1                     /* Enter Hyp mode */
+
+1:	.word 0                      /* PC to enter Hyp mode at */
+	.word 0x000001da             /* CPSR: LE, Abort/IRQ/FIQ off, Hyp */
+

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3 of 6] arm: avoid memory write in switch to Hyp mode
  2012-05-31 16:39 [PATCH 0 of 6] ARM: various boot-time tidying Tim Deegan
  2012-05-31 16:39 ` [PATCH 1 of 6] arm: More interrupt setup at start-of-day for secondary CPUs Tim Deegan
  2012-05-31 16:40 ` [PATCH 2 of 6] arm: Move hyp-mode entry code out of line Tim Deegan
@ 2012-05-31 16:40 ` Tim Deegan
  2012-06-01  9:07   ` Ian Campbell
  2012-05-31 16:40 ` [PATCH 4 of 6] arm: missing __init annotation Tim Deegan
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Tim Deegan @ 2012-05-31 16:40 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, ian.campbell

# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1338482127 -3600
# Node ID 8656509f24ea8b17d1c2dabd67c710662f66d17f
# Parent  05447f395c91029fb732142e36788cfa92374045
arm: avoid memory write in switch to Hyp mode.

Assemble the new CPSR in registers instead.  It's slightly cleaner,
And makes it possible to have a read-only text section.

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r 05447f395c91 -r 8656509f24ea xen/arch/arm/mode_switch.S
--- a/xen/arch/arm/mode_switch.S	Thu May 31 17:35:27 2012 +0100
+++ b/xen/arch/arm/mode_switch.S	Thu May 31 17:35:27 2012 +0100
@@ -66,11 +66,7 @@ enter_hyp_mode:
 	mcr   CP32(r0, FCSEIDR)
 	mcr   CP32(r0, CONTEXTIDR)
 	/* FIXME: ought to reset some other NS control regs here */
-	adr   r1, 1f                 /* Store return address */
-	str   r3, [r1]               /* where we can use it for RFE */
-	isb                          /* Ensure we see the stored address */
-	rfeia r1                     /* Enter Hyp mode */
-
-1:	.word 0                      /* PC to enter Hyp mode at */
-	.word 0x000001da             /* CPSR: LE, Abort/IRQ/FIQ off, Hyp */
-
+	mrs   r0, cpsr               /* Copy the CPSR */
+	add   r0, r0, #0x4           /* 0x16 (Monitor) -> 0x1a (Hyp) */
+	msr   spsr_cxsf, r0          /* into the SPSR */
+	movs  pc, r3                 /* Exception-return into Hyp mode */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4 of 6] arm: missing __init annotation
  2012-05-31 16:39 [PATCH 0 of 6] ARM: various boot-time tidying Tim Deegan
                   ` (2 preceding siblings ...)
  2012-05-31 16:40 ` [PATCH 3 of 6] arm: avoid memory write in switch to Hyp mode Tim Deegan
@ 2012-05-31 16:40 ` Tim Deegan
  2012-05-31 16:40 ` [PATCH 5 of 6] arm: allow ourselves access to all coprocessors in non-secure mode Tim Deegan
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Tim Deegan @ 2012-05-31 16:40 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, ian.campbell

# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1338482127 -3600
# Node ID 89f363564aa68dcb78d3aedd6a0b24f9bb1ffc13
# Parent  8656509f24ea8b17d1c2dabd67c710662f66d17f
arm: missing __init annotation

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r 8656509f24ea -r 89f363564aa6 xen/arch/arm/setup.c
--- a/xen/arch/arm/setup.c	Thu May 31 17:35:27 2012 +0100
+++ b/xen/arch/arm/setup.c	Thu May 31 17:35:27 2012 +0100
@@ -51,7 +51,7 @@ static void __init init_idle_domain(void
         /* TODO: setup_idle_pagetable(); */
 }
 
-static void processor_id(void)
+static void __init processor_id(void)
 {
     printk("Processor Features: %08x %08x\n",
            READ_CP32(ID_PFR0), READ_CP32(ID_PFR0));

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 5 of 6] arm: allow ourselves access to all coprocessors in non-secure mode
  2012-05-31 16:39 [PATCH 0 of 6] ARM: various boot-time tidying Tim Deegan
                   ` (3 preceding siblings ...)
  2012-05-31 16:40 ` [PATCH 4 of 6] arm: missing __init annotation Tim Deegan
@ 2012-05-31 16:40 ` Tim Deegan
  2012-05-31 16:40 ` [PATCH 6 of 6] arm: Enable VFP at boot Tim Deegan
  2012-06-01  9:31 ` [PATCH 0 of 6] ARM: various boot-time tidying Ian Campbell
  6 siblings, 0 replies; 15+ messages in thread
From: Tim Deegan @ 2012-05-31 16:40 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, ian.campbell

# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1338482127 -3600
# Node ID 9570c43c07f397d8d4a8e725698c17c10c034d80
# Parent  89f363564aa68dcb78d3aedd6a0b24f9bb1ffc13
arm: allow ourselves access to all coprocessors in non-secure mode

We'll need it to be able top use the VFP extensions, for example.

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r 89f363564aa6 -r 9570c43c07f3 xen/arch/arm/mode_switch.S
--- a/xen/arch/arm/mode_switch.S	Thu May 31 17:35:27 2012 +0100
+++ b/xen/arch/arm/mode_switch.S	Thu May 31 17:35:27 2012 +0100
@@ -65,7 +65,10 @@ enter_hyp_mode:
 	mov   r0, #0
 	mcr   CP32(r0, FCSEIDR)
 	mcr   CP32(r0, CONTEXTIDR)
-	/* FIXME: ought to reset some other NS control regs here */
+	/* Allow non-secure access to coprocessors, FIQs, VFP and NEON */
+	ldr   r1, =0x3fff            /* 14 CP bits set, all others clear */
+	mcr   CP32(r1, NSACR)
+
 	mrs   r0, cpsr               /* Copy the CPSR */
 	add   r0, r0, #0x4           /* 0x16 (Monitor) -> 0x1a (Hyp) */
 	msr   spsr_cxsf, r0          /* into the SPSR */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 6 of 6] arm: Enable VFP at boot
  2012-05-31 16:39 [PATCH 0 of 6] ARM: various boot-time tidying Tim Deegan
                   ` (4 preceding siblings ...)
  2012-05-31 16:40 ` [PATCH 5 of 6] arm: allow ourselves access to all coprocessors in non-secure mode Tim Deegan
@ 2012-05-31 16:40 ` Tim Deegan
  2012-06-01  9:20   ` Ian Campbell
  2012-06-01  9:31 ` [PATCH 0 of 6] ARM: various boot-time tidying Ian Campbell
  6 siblings, 1 reply; 15+ messages in thread
From: Tim Deegan @ 2012-05-31 16:40 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, ian.campbell

# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1338482127 -3600
# Node ID d12d5fcf152bffda41de51f9919b3211d69f955b
# Parent  9570c43c07f397d8d4a8e725698c17c10c034d80
arm: Enable VFP at boot.

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r 9570c43c07f3 -r d12d5fcf152b xen/arch/arm/Rules.mk
--- a/xen/arch/arm/Rules.mk	Thu May 31 17:35:27 2012 +0100
+++ b/xen/arch/arm/Rules.mk	Thu May 31 17:35:27 2012 +0100
@@ -24,7 +24,7 @@ ifneq ($(call cc-option,$(CC),-fvisibili
 CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
 endif
 
-CFLAGS += -march=armv7-a -mcpu=cortex-a15
+CFLAGS += -march=armv7-a -mcpu=cortex-a15 -mfpu=vfpv3 -mfloat-abi=softfp
 
 # Require GCC v3.4+ (to avoid issues with alignment constraints in Xen headers)
 check-$(gcc) = $(call cc-ver-check,CC,0x030400,"Xen requires at least gcc-3.4")
diff -r 9570c43c07f3 -r d12d5fcf152b xen/arch/arm/setup.c
--- a/xen/arch/arm/setup.c	Thu May 31 17:35:27 2012 +0100
+++ b/xen/arch/arm/setup.c	Thu May 31 17:35:27 2012 +0100
@@ -36,6 +36,7 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/setup.h>
+#include <asm/vfp.h>
 #include "gic.h"
 
 static __attribute_used__ void init_done(void)
@@ -192,6 +193,8 @@ void __init start_xen(unsigned long boot
 
     processor_id();
 
+    enable_vfp();
+
     softirq_init();
 
     tasklet_subsys_init();
diff -r 9570c43c07f3 -r d12d5fcf152b xen/arch/arm/smpboot.c
--- a/xen/arch/arm/smpboot.c	Thu May 31 17:35:27 2012 +0100
+++ b/xen/arch/arm/smpboot.c	Thu May 31 17:35:27 2012 +0100
@@ -26,6 +26,7 @@
 #include <xen/sched.h>
 #include <xen/smp.h>
 #include <xen/softirq.h>
+#include <asm/vfp.h>
 #include "gic.h"
 
 cpumask_t cpu_online_map;
@@ -106,6 +107,8 @@ void __cpuinit start_secondary(unsigned 
     WRITE_CP32((uint32_t) hyp_traps_vector, HVBAR);
 
     mmu_init_secondary_cpu();
+    enable_vfp();
+
     gic_init_secondary_cpu();
     init_timer_interrupt();
     gic_route_irqs();
diff -r 9570c43c07f3 -r d12d5fcf152b xen/include/asm-arm/vfp.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/xen/include/asm-arm/vfp.h	Thu May 31 17:35:27 2012 +0100
@@ -0,0 +1,35 @@
+#ifndef __ARM_VFP_H_
+#define __ARM_VFP_H_
+
+#include <xen/types.h>
+
+#define FPEXC_EN (1u << 30)
+
+/* Save and restore FP state.
+ * Ought to be using the new vmrs/vmsr names, but older binutils has a
+ * bug where it only allows them to target fpscr (and not, say, fpexc). */
+#define READ_FP(reg) ({                                 \
+    uint32_t val;                                       \
+    asm volatile ("fmrx %0, fp" #reg : "=r" (val));     \
+    val; })
+
+#define WRITE_FP(reg, val) do {                         \
+    asm volatile ("fmxr fp" #reg ", %0" : : "r" (val)); \
+} while (0)
+
+
+/* Start-of-day: Turn on VFP */
+static inline void enable_vfp(void)
+{
+    WRITE_FP(exc, READ_FP(exc) | FPEXC_EN);
+}
+
+#endif
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2 of 6] arm: Move hyp-mode entry code out of line
  2012-05-31 16:40 ` [PATCH 2 of 6] arm: Move hyp-mode entry code out of line Tim Deegan
@ 2012-05-31 16:54   ` David Vrabel
  2012-06-01  6:29     ` Ian Campbell
  0 siblings, 1 reply; 15+ messages in thread
From: David Vrabel @ 2012-05-31 16:54 UTC (permalink / raw)
  To: Tim Deegan; +Cc: stefano.stabellini, ian.campbell, xen-devel

On 31/05/12 17:40, Tim Deegan wrote:
> # HG changeset patch
> # User Tim Deegan <tim@xen.org>
> # Date 1338482127 -3600
> # Node ID 05447f395c91029fb732142e36788cfa92374045
> # Parent  25d389d891c5a2a3009258ef7261379a9ad97746
> arm: Move hyp-mode entry code out of line.
> 
> This code is grottier than the rest of the start-of-day code and
> very specific to the software model we're developing on.
> Segregate it accordingly, by putting it in its own file.

I've been vaguely thinking about writing a stub-bootloader to do this
and provide the DTB (instead of having to link it with Xen).  Is this
something that would be useful?

David

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2 of 6] arm: Move hyp-mode entry code out of line
  2012-05-31 16:54   ` David Vrabel
@ 2012-06-01  6:29     ` Ian Campbell
  0 siblings, 0 replies; 15+ messages in thread
From: Ian Campbell @ 2012-06-01  6:29 UTC (permalink / raw)
  To: David Vrabel; +Cc: Stefano Stabellini, Tim (Xen.org), xen-devel@lists.xen.org

On Thu, 2012-05-31 at 17:54 +0100, David Vrabel wrote:
> On 31/05/12 17:40, Tim Deegan wrote:
> > # HG changeset patch
> > # User Tim Deegan <tim@xen.org>
> > # Date 1338482127 -3600
> > # Node ID 05447f395c91029fb732142e36788cfa92374045
> > # Parent  25d389d891c5a2a3009258ef7261379a9ad97746
> > arm: Move hyp-mode entry code out of line.
> > 
> > This code is grottier than the rest of the start-of-day code and
> > very specific to the software model we're developing on.
> > Segregate it accordingly, by putting it in its own file.
> 
> I've been vaguely thinking about writing a stub-bootloader to do this
> and provide the DTB (instead of having to link it with Xen).  Is this
> something that would be useful?

Rather than a new stub-bootloader we should switch over to using the
same boot-wrapper.git as Linaro and KVM are using[0], and perhaps
enhancing it to meet our needs.

One benefit of that is that when they come to do stuff on real hardware
we will already be somewhat aligned with something which looks a bit
like a bootloader so it should be easier for us to transition too.

It also has other benefits like enabling us to use semi-hosting to pull
the files off the host filesystem.

Ian.

[0] http://git.linaro.org/gitweb?p=arm/models/boot-wrapper.git;a=summary

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3 of 6] arm: avoid memory write in switch to Hyp mode
  2012-05-31 16:40 ` [PATCH 3 of 6] arm: avoid memory write in switch to Hyp mode Tim Deegan
@ 2012-06-01  9:07   ` Ian Campbell
  2012-06-01  9:18     ` Tim Deegan
  0 siblings, 1 reply; 15+ messages in thread
From: Ian Campbell @ 2012-06-01  9:07 UTC (permalink / raw)
  To: Tim Deegan; +Cc: Stefano Stabellini, xen-devel@lists.xen.org

On Thu, 2012-05-31 at 17:40 +0100, Tim Deegan wrote:
> # HG changeset patch
> # User Tim Deegan <tim@xen.org>
> # Date 1338482127 -3600
> # Node ID 8656509f24ea8b17d1c2dabd67c710662f66d17f
> # Parent  05447f395c91029fb732142e36788cfa92374045
> arm: avoid memory write in switch to Hyp mode.
> 
> Assemble the new CPSR in registers instead.  It's slightly cleaner,
> And makes it possible to have a read-only text section.
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
> 
> diff -r 05447f395c91 -r 8656509f24ea xen/arch/arm/mode_switch.S
> --- a/xen/arch/arm/mode_switch.S	Thu May 31 17:35:27 2012 +0100
> +++ b/xen/arch/arm/mode_switch.S	Thu May 31 17:35:27 2012 +0100
> @@ -66,11 +66,7 @@ enter_hyp_mode:
>  	mcr   CP32(r0, FCSEIDR)
>  	mcr   CP32(r0, CONTEXTIDR)
>  	/* FIXME: ought to reset some other NS control regs here */
> -	adr   r1, 1f                 /* Store return address */
> -	str   r3, [r1]               /* where we can use it for RFE */
> -	isb                          /* Ensure we see the stored address */
> -	rfeia r1                     /* Enter Hyp mode */
> -
> -1:	.word 0                      /* PC to enter Hyp mode at */
> -	.word 0x000001da             /* CPSR: LE, Abort/IRQ/FIQ off, Hyp */
> -
> +	mrs   r0, cpsr               /* Copy the CPSR */
> +	add   r0, r0, #0x4           /* 0x16 (Monitor) -> 0x1a (Hyp) */
> +	msr   spsr_cxsf, r0          /* into the SPSR */

Not that it matters much but I think this could have been sprs_c to just
update spsr[0:7] (which is all you wanted to touch). In fact I reckon
you could have just done:
	mov r0, #0x1a|SPSR_I|SPSR_F
	msr spsr_c, r0

Anyway, I'm going to apply regardless because I'm just nitpicking...

> +	movs  pc, r3                 /* Exception-return into Hyp mode */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3 of 6] arm: avoid memory write in switch to Hyp mode
  2012-06-01  9:07   ` Ian Campbell
@ 2012-06-01  9:18     ` Tim Deegan
  0 siblings, 0 replies; 15+ messages in thread
From: Tim Deegan @ 2012-06-01  9:18 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Stefano Stabellini, xen-devel@lists.xen.org

At 10:07 +0100 on 01 Jun (1338545250), Ian Campbell wrote:
> > +	mrs   r0, cpsr               /* Copy the CPSR */
> > +	add   r0, r0, #0x4           /* 0x16 (Monitor) -> 0x1a (Hyp) */
> > +	msr   spsr_cxsf, r0          /* into the SPSR */
> 
> Not that it matters much but I think this could have been sprs_c to just
> update spsr[0:7] (which is all you wanted to touch). In fact I reckon
> you could have just done:
> 	mov r0, #0x1a|SPSR_I|SPSR_F
> 	msr spsr_c, r0

I considered that, but I don't think that anything in the path so far
will have initialised this mode's SPSR to a sensible value, and I didn't
want to accidentally turn on big-endian mode or some such.

Cheers,

Tim.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6 of 6] arm: Enable VFP at boot
  2012-05-31 16:40 ` [PATCH 6 of 6] arm: Enable VFP at boot Tim Deegan
@ 2012-06-01  9:20   ` Ian Campbell
  2012-06-01  9:29     ` Tim Deegan
  0 siblings, 1 reply; 15+ messages in thread
From: Ian Campbell @ 2012-06-01  9:20 UTC (permalink / raw)
  To: Tim Deegan; +Cc: Stefano Stabellini, xen-devel@lists.xen.org

On Thu, 2012-05-31 at 17:40 +0100, Tim Deegan wrote:
> # HG changeset patch
> # User Tim Deegan <tim@xen.org>
> # Date 1338482127 -3600
> # Node ID d12d5fcf152bffda41de51f9919b3211d69f955b
> # Parent  9570c43c07f397d8d4a8e725698c17c10c034d80
> arm: Enable VFP at boot.

Thanks, this should mean I can remove
"cluster.cpu0.vfp-enable_at_reset=1" from my fast model cfg file!

... tries it... yes it works, sweeet!

> 
> Signed-off-by: Tim Deegan <tim@xen.org>
> 
> diff -r 9570c43c07f3 -r d12d5fcf152b xen/arch/arm/Rules.mk
> --- a/xen/arch/arm/Rules.mk	Thu May 31 17:35:27 2012 +0100
> +++ b/xen/arch/arm/Rules.mk	Thu May 31 17:35:27 2012 +0100
> @@ -24,7 +24,7 @@ ifneq ($(call cc-option,$(CC),-fvisibili
>  CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
>  endif
>  
> -CFLAGS += -march=armv7-a -mcpu=cortex-a15
> +CFLAGS += -march=armv7-a -mcpu=cortex-a15 -mfpu=vfpv3 -mfloat-abi=softfp

This might have been worthy of a comment...

http://wiki.debian.org/ArmHardFloatPort has some discussion, including:
        * soft: Full software floating point. 
        * softfp: Use the FPU, but remain compatible with soft-float
        code. 
        * hard: Full hardware floating point. 

and "The combination of -mfpu=vfp and -mfloat-abi=hard is not available
in FSF GCC 4.4". (there's a reference to a TODO section which seems to
not exist anymore...). I think Debian's armhf does actually use hard
(the page seems to imply it)

Anyway softfp does seem like the right option, at least until
-mfloat-abi=hard. Since this is all internal to the hypervisor (guests
can do whatever they want, I'm happily running armhf binaries) I don't
think this is a big deal to change as and when the compiler will let us.

>  
>  # Require GCC v3.4+ (to avoid issues with alignment constraints in Xen headers)
>  check-$(gcc) = $(call cc-ver-check,CC,0x030400,"Xen requires at least gcc-3.4")
> diff -r 9570c43c07f3 -r d12d5fcf152b xen/arch/arm/setup.c
> --- a/xen/arch/arm/setup.c	Thu May 31 17:35:27 2012 +0100
> +++ b/xen/arch/arm/setup.c	Thu May 31 17:35:27 2012 +0100
> @@ -36,6 +36,7 @@
>  #include <asm/page.h>
>  #include <asm/current.h>
>  #include <asm/setup.h>
> +#include <asm/vfp.h>
>  #include "gic.h"
>  
>  static __attribute_used__ void init_done(void)
> @@ -192,6 +193,8 @@ void __init start_xen(unsigned long boot
>  
>      processor_id();
>  
> +    enable_vfp();
> +
>      softirq_init();
>  
>      tasklet_subsys_init();
> diff -r 9570c43c07f3 -r d12d5fcf152b xen/arch/arm/smpboot.c
> --- a/xen/arch/arm/smpboot.c	Thu May 31 17:35:27 2012 +0100
> +++ b/xen/arch/arm/smpboot.c	Thu May 31 17:35:27 2012 +0100
> @@ -26,6 +26,7 @@
>  #include <xen/sched.h>
>  #include <xen/smp.h>
>  #include <xen/softirq.h>
> +#include <asm/vfp.h>
>  #include "gic.h"
>  
>  cpumask_t cpu_online_map;
> @@ -106,6 +107,8 @@ void __cpuinit start_secondary(unsigned 
>      WRITE_CP32((uint32_t) hyp_traps_vector, HVBAR);
>  
>      mmu_init_secondary_cpu();
> +    enable_vfp();
> +
>      gic_init_secondary_cpu();
>      init_timer_interrupt();
>      gic_route_irqs();
> diff -r 9570c43c07f3 -r d12d5fcf152b xen/include/asm-arm/vfp.h
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/xen/include/asm-arm/vfp.h	Thu May 31 17:35:27 2012 +0100
> @@ -0,0 +1,35 @@
> +#ifndef __ARM_VFP_H_
> +#define __ARM_VFP_H_
> +
> +#include <xen/types.h>
> +
> +#define FPEXC_EN (1u << 30)
> +
> +/* Save and restore FP state.
> + * Ought to be using the new vmrs/vmsr names, but older binutils has a
> + * bug where it only allows them to target fpscr (and not, say, fpexc). */
> +#define READ_FP(reg) ({                                 \
> +    uint32_t val;                                       \
> +    asm volatile ("fmrx %0, fp" #reg : "=r" (val));     \
> +    val; })
> +
> +#define WRITE_FP(reg, val) do {                         \
> +    asm volatile ("fmxr fp" #reg ", %0" : : "r" (val)); \
> +} while (0)
> +
> +
> +/* Start-of-day: Turn on VFP */
> +static inline void enable_vfp(void)
> +{
> +    WRITE_FP(exc, READ_FP(exc) | FPEXC_EN);
> +}
> +
> +#endif
> +/*
> + * Local variables:
> + * mode: C
> + * c-set-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6 of 6] arm: Enable VFP at boot
  2012-06-01  9:20   ` Ian Campbell
@ 2012-06-01  9:29     ` Tim Deegan
  2012-06-01  9:37       ` Ian Campbell
  0 siblings, 1 reply; 15+ messages in thread
From: Tim Deegan @ 2012-06-01  9:29 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Stefano Stabellini, xen-devel@lists.xen.org

At 10:20 +0100 on 01 Jun (1338546010), Ian Campbell wrote:
> > -CFLAGS += -march=armv7-a -mcpu=cortex-a15
> > +CFLAGS += -march=armv7-a -mcpu=cortex-a15 -mfpu=vfpv3 -mfloat-abi=softfp
> 
> This might have been worthy of a comment...

Ah, yes it might.  Would you like me to add one and resubmit?

If I get time next week I'm going to look at VFP context switching; at
that point I might move all the VFP code into its own .c or .S file and
enable the fpu instructions just there, to be sure that gcc doesn't
start using the VFP registers in normal Xen code -- it will do that for
integer arithmetic with -mfpu=neon, for example.  OTOH it might be
OK to do that as long as there are no caller-save VFP registers; I'll
have a look at the calling conventions and see what can be done.

Tim.

> http://wiki.debian.org/ArmHardFloatPort has some discussion, including:
>         * soft: Full software floating point. 
>         * softfp: Use the FPU, but remain compatible with soft-float
>         code. 
>         * hard: Full hardware floating point. 
> 
> and "The combination of -mfpu=vfp and -mfloat-abi=hard is not available
> in FSF GCC 4.4". (there's a reference to a TODO section which seems to
> not exist anymore...). I think Debian's armhf does actually use hard
> (the page seems to imply it)
> 
> Anyway softfp does seem like the right option, at least until
> -mfloat-abi=hard. Since this is all internal to the hypervisor (guests
> can do whatever they want, I'm happily running armhf binaries) I don't
> think this is a big deal to change as and when the compiler will let us.
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0 of 6] ARM: various boot-time tidying
  2012-05-31 16:39 [PATCH 0 of 6] ARM: various boot-time tidying Tim Deegan
                   ` (5 preceding siblings ...)
  2012-05-31 16:40 ` [PATCH 6 of 6] arm: Enable VFP at boot Tim Deegan
@ 2012-06-01  9:31 ` Ian Campbell
  6 siblings, 0 replies; 15+ messages in thread
From: Ian Campbell @ 2012-06-01  9:31 UTC (permalink / raw)
  To: Tim Deegan; +Cc: Stefano Stabellini, xen-devel@lists.xen.org

On Thu, 2012-05-31 at 17:39 +0100, Tim Deegan wrote:
> These patches tinker with the ARM boot a little bit.  They shouldn't
> clash with any of the other ARM work going on, since they only touch
> very early code.

All applied, thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6 of 6] arm: Enable VFP at boot
  2012-06-01  9:29     ` Tim Deegan
@ 2012-06-01  9:37       ` Ian Campbell
  0 siblings, 0 replies; 15+ messages in thread
From: Ian Campbell @ 2012-06-01  9:37 UTC (permalink / raw)
  To: Tim Deegan; +Cc: Stefano Stabellini, xen-devel@lists.xen.org

On Fri, 2012-06-01 at 10:29 +0100, Tim Deegan wrote:
> At 10:20 +0100 on 01 Jun (1338546010), Ian Campbell wrote:
> > > -CFLAGS += -march=armv7-a -mcpu=cortex-a15
> > > +CFLAGS += -march=armv7-a -mcpu=cortex-a15 -mfpu=vfpv3 -mfloat-abi=softfp
> > 
> > This might have been worthy of a comment...
> 
> Ah, yes it might.  Would you like me to add one and resubmit?

I've already committed. If you feel like it then an incremental patch
might be nice...

> If I get time next week I'm going to look at VFP context switching; at
> that point I might move all the VFP code into its own .c or .S file and
> enable the fpu instructions just there, to be sure that gcc doesn't
> start using the VFP registers in normal Xen code -- it will do that for
> integer arithmetic with -mfpu=neon, for example.  OTOH it might be
> OK to do that as long as there are no caller-save VFP registers; I'll
> have a look at the calling conventions and see what can be done.

OK!

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-06-01  9:37 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-31 16:39 [PATCH 0 of 6] ARM: various boot-time tidying Tim Deegan
2012-05-31 16:39 ` [PATCH 1 of 6] arm: More interrupt setup at start-of-day for secondary CPUs Tim Deegan
2012-05-31 16:40 ` [PATCH 2 of 6] arm: Move hyp-mode entry code out of line Tim Deegan
2012-05-31 16:54   ` David Vrabel
2012-06-01  6:29     ` Ian Campbell
2012-05-31 16:40 ` [PATCH 3 of 6] arm: avoid memory write in switch to Hyp mode Tim Deegan
2012-06-01  9:07   ` Ian Campbell
2012-06-01  9:18     ` Tim Deegan
2012-05-31 16:40 ` [PATCH 4 of 6] arm: missing __init annotation Tim Deegan
2012-05-31 16:40 ` [PATCH 5 of 6] arm: allow ourselves access to all coprocessors in non-secure mode Tim Deegan
2012-05-31 16:40 ` [PATCH 6 of 6] arm: Enable VFP at boot Tim Deegan
2012-06-01  9:20   ` Ian Campbell
2012-06-01  9:29     ` Tim Deegan
2012-06-01  9:37       ` Ian Campbell
2012-06-01  9:31 ` [PATCH 0 of 6] ARM: various boot-time tidying Ian Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).