Skip to contentSkip to conversation

ARM: Highbank: Highbank: Add monitor support

PATCH Applied Peter Crosthwaite 2 participants 5 messages Nov 9, 2015

lore
Share & tools
lore

Peter Crosthwaite

author
#0 lore raw

v4 cover ARM: Highbank: Highbank: Add monitor support

Hi,

This adds dummy monitor support to the Highbank board. It is needed by the Highbank kernel which expects a monitor to be present.

A feature is added to arm/boot's board_setup feature, that allows the board_setup entry point to be entered in secure mode (which is needed to configure a monitor).

This feature does not play nice with -cpu override, but cpu override is not valid for well-defined ARM SoCs. So defeature -cpu for Highbank.

Regards,
Peter

See indiv. patches for detailed change logs.

Changed since v3:
Addressed PMM v2 review
Changed tcg_enabled() checks to kvm_enabled() Changed since v2:
Defeature -cpu for Highbank (new patch) Rework board_setup blob implementation (PMM review) Conditionalise feature on TCG
Dropped initial board_setup and Zynq patches (Merged) Changed since v1:
Addressed PMM review.
Added secure_board_setup flag (P4)
Added Zynq patch first, then Highbank

Peter Crosthwaite (3):
arm: boot: Add secure_board_setup flag
arm: highbank: Defeature CPU override
arm: highbank: Implement PSCI and dummy monitor

 hw/arm/boot.c        | 10 +++++-
 hw/arm/highbank.c    | 91 ++++++++++++++++++++++++++++++++++++++--------------
 include/hw/arm/arm.h |  6 ++++
 3 files changed, 82 insertions(+), 25 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v4 1/3 arm: boot: Add secure_board_setup flag

Add a flag that when set, will cause the primary CPU to start in secure mode, even if the overall boot is non-secure. This is useful for when there is a board-setup blob that needs to run from secure mode, but device and secondary CPU init should still be done as-normal for a non- secure boot.

Signed-off-by Peter Crosthwaite <[email protected]>

---
changed since v3:
Clarified comment for assertion.
Changed tcg_enabled() check to kvm_enabled(). changed since v2:
Assert if running KVM and board_setup_secure is set

 hw/arm/boot.c        | 10 +++++++++-
 include/hw/arm/arm.h |  6 ++++++
 2 files changed, 15 insertions(+), 1 deletion(-)
hw/arm/boot.c +9 -1
index b0879a5..75f69bf 100644
@@ -11,6 +11,7 @@
 #include "hw/hw.h"
 #include "hw/arm/arm.h"
 #include "hw/arm/linux-boot-if.h"
+#include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
 #include "hw/boards.h"
 #include "hw/loader.h"

@@ -495,7 +496,8 @@ static void do_cpu_reset(void *opaque)
                 }
 
                 /* Set to non-secure if not a secure boot */
-                if (!info->secure_boot) {
+                if (!info->secure_boot &&
+                    (cs != first_cpu || !info->secure_board_setup)) {
                     /* Linux expects non-secure state */
                     env->cp15.scr_el3 |= SCR_NS;
                 }

@@ -598,6 +600,12 @@ static void arm_load_kernel_notify(Notifier *notifier, void *data)
     struct arm_boot_info *info =
         container_of(n, struct arm_boot_info, load_kernel_notifier);
 
+    /* The board code is not supposed to set secure_board_setup unless
+     * running its code in secure mode is actually possible, and KVM
+     * doesn't support secure.
+     */
+    assert(!(info->secure_board_setup && kvm_enabled()));
+
     /* Load the kernel.  */
     if (!info->kernel_filename || info->firmware_loaded) {
include/hw/arm/arm.h +6 -0
index 67ba7db..c26b0e3 100644
@@ -97,6 +97,12 @@ struct arm_boot_info {
     hwaddr board_setup_addr;
     void (*write_board_setup)(ARMCPU *cpu,
                               const struct arm_boot_info *info);
+
+    /* If set, the board specific loader/setup blob will be run from secure
+     * mode, regardless of secure_boot. The blob becomes responsible for
+     * changing to non-secure state if implementing a non-secure boot
+     */
+    bool secure_board_setup;
 };
 
 /**
Signature · 1 line
1.9.1
author
#2 lore raw

PATCH v4 2/3 arm: highbank: Defeature CPU override

In reply to Peter Crosthwaite

This board should not support CPU model override. This allows for easier patching of the board with being able to rely on the CPU type being correct.

Reviewed-by Peter Maydell <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>

---

 hw/arm/highbank.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)
hw/arm/highbank.c +7 -14
index be04b27..f2e248b 100644
@@ -223,15 +223,13 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
     MemoryRegion *sysmem;
     char *sysboot_filename;
 
-    if (!cpu_model) {
-        switch (machine_id) {
-        case CALXEDA_HIGHBANK:
-            cpu_model = "cortex-a9";
-            break;
-        case CALXEDA_MIDWAY:
-            cpu_model = "cortex-a15";
-            break;
-        }
+    switch (machine_id) {
+    case CALXEDA_HIGHBANK:
+        cpu_model = "cortex-a9";
+        break;
+    case CALXEDA_MIDWAY:
+        cpu_model = "cortex-a15";
+        break;
     }
 
     for (n = 0; n < smp_cpus; n++) {

@@ -240,11 +238,6 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
         ARMCPU *cpu;
         Error *err = NULL;
 
-        if (!oc) {
-            error_report("Unable to find CPU definition");
-            exit(1);
-        }
-
         cpuobj = object_new(object_class_get_name(oc));
         cpu = ARM_CPU(cpuobj);
Signature · 1 line
1.9.1
author
#3 lore raw

PATCH v4 3/3 arm: highbank: Implement PSCI and dummy monitor

In reply to Peter Crosthwaite

Firstly, enable monitor mode and PSCI, both of which are features of this board.

In addition to PSCI, this board also uses SMC for cache maintenance ops. This means we need a secure monitor to catch these and nop them. Use the ARM boot board-setup feature to implement this. The SMC trap implements the needed nop while all other traps will pen the CPU.

As a KVM CPU cannot run in secure mode, do not do the board-setup if not running TCG. Report a warning explaining the limitation in this case.

Reviewed-by Peter Maydell <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>

---
Changed since v3:
Commit message typos.
Changed tcg_enabled() check to kvm_enabled()
Reworded error_report() message.
clarified non-smc trap behaviour in commit message Changed since v2:
Change to spinlock/movs trap table and drop fallthrough (PMM review) Do not do board-setup if KVM is enabled. Issue a warning. Changed since v1:
fallthrough all of trap table to nop implementation
use movw for table address
leave loader at 0
Move MVBAR (and blob to non-zero)
Split nop implementation from MVBAR setup
set secure boot for board
implement NS switch in blob
Changed since RFC:
Use bootloader callback to load blob.
Change "firmware" to "board-setup" for consistency.
Tweak commit message.

 hw/arm/highbank.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 10 deletions(-)
hw/arm/highbank.c +60 -10
index f2e248b..85ae69e 100644
@@ -22,6 +22,7 @@
 #include "hw/devices.h"
 #include "hw/loader.h"
 #include "net/net.h"
+#include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
 #include "hw/boards.h"
 #include "sysemu/block-backend.h"

@@ -32,10 +33,52 @@
 #define SMP_BOOT_REG            0x40
 #define MPCORE_PERIPHBASE       0xfff10000
 
+#define MVBAR_ADDR              0x200
+
 #define NIRQ_GIC                160
 
 /* Board init.  */
 
+/* MVBAR_ADDR is limited by precision of movw */
+
+QEMU_BUILD_BUG_ON(MVBAR_ADDR >= (1 << 16));
+
+#define ARMV7_IMM16(x) (extract32((x),  0, 12) | \
+                        extract32((x), 12,  4) << 16)
+
+static void hb_write_board_setup(ARMCPU *cpu,
+                                 const struct arm_boot_info *info)
+{
+    int n;
+    uint32_t board_setup_blob[] = {
+        /* MVBAR_ADDR */
+        /* Default unimplemented and unused vectors to spin. Makes it
+         * easier to debug (as opposed to the CPU running away).
+         */
+        0xeafffffe, /* notused1: b notused */
+        0xeafffffe, /* notused2: b notused */
+        0xe1b0f00e, /* smc: movs pc, lr - exception return */
+        0xeafffffe, /* prefetch_abort: b prefetch_abort */
+        0xeafffffe, /* data_abort: b data_abort */
+        0xeafffffe, /* notused3: b notused3 */
+        0xeafffffe, /* irq: b irq */
+        0xeafffffe, /* fiq: b fiq */
+#define BOARD_SETUP_ADDR (MVBAR_ADDR + 8 * sizeof(uint32_t))
+        0xe3000000 + ARMV7_IMM16(MVBAR_ADDR), /* movw r0, MVBAR_ADDR */
+        0xee0c0f30, /* mcr p15, 0, r0, c12, c0, 1 - set MVBAR */
+        0xee110f11, /* mrc p15, 0, r0, c1 , c1, 0 - get SCR */
+        0xe3810001, /* orr r0, #1 - set NS */
+        0xee010f11, /* mcr p15, 0, r0, c1 , c1, 0 - set SCR */
+        0xe1600070, /* smc - go to monitor mode to flush NS change */
+        0xe12fff1e, /* bx lr - return to caller */
+    };
+    for (n = 0; n < ARRAY_SIZE(board_setup_blob); n++) {
+        board_setup_blob[n] = tswap32(board_setup_blob[n]);
+    }
+    rom_add_blob_fixed("board-setup", board_setup_blob,
+                       sizeof(board_setup_blob), MVBAR_ADDR);
+}
+
 static void hb_write_secondary(ARMCPU *cpu, const struct arm_boot_info *info)
 {
     int n;

@@ -241,16 +284,13 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
         cpuobj = object_new(object_class_get_name(oc));
         cpu = ARM_CPU(cpuobj);
 
-        /* By default A9 and A15 CPUs have EL3 enabled.  This board does not
-         * currently support EL3 so the CPU EL3 property is disabled before
-         * realization.
-         */
-        if (object_property_find(cpuobj, "has_el3", NULL)) {
-            object_property_set_bool(cpuobj, false, "has_el3", &err);
-            if (err) {
-                error_report_err(err);
-                exit(1);
-            }
+        object_property_set_int(cpuobj, QEMU_PSCI_CONDUIT_SMC,
+                                "psci-conduit", &error_abort);
+
+        if (n) {
+            /* Secondary CPUs start in PSCI powered-down state */
+            object_property_set_bool(cpuobj, true,
+                                     "start-powered-off", &error_abort);
         }
 
         if (object_property_find(cpuobj, "reset-cbar", NULL)) {

@@ -371,6 +411,16 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
     highbank_binfo.loader_start = 0;
     highbank_binfo.write_secondary_boot = hb_write_secondary;
     highbank_binfo.secondary_cpu_reset_hook = hb_reset_secondary;
+    if (!kvm_enabled()) {
+        highbank_binfo.board_setup_addr = BOARD_SETUP_ADDR;
+        highbank_binfo.write_board_setup = hb_write_board_setup;
+        highbank_binfo.secure_board_setup = true;
+    } else {
+        error_report("WARNING: cannot load built-in Monitor support "
+                     "if KVM is enabled. Some guests (such as Linux) "
+                     "may not boot.");
+    }
+
     arm_load_kernel(ARM_CPU(first_cpu), &highbank_binfo);
 }
Signature · 1 line
1.9.1
Applied
#4 lore raw
In reply to Peter Crosthwaite
Quoted 9 lines · Peter Crosthwaite

Hi,

This adds dummy monitor support to the Highbank board. It is needed by the Highbank kernel which expects a monitor to be present.

A feature is added to arm/boot's board_setup feature, that allows the board_setup entry point to be entered in secure mode (which is needed to configure a monitor).

Quoted 7 lines

This feature does not play nice with -cpu override, but cpu override is not valid for well-defined ARM SoCs. So defeature -cpu for Highbank.

Regards,
Peter

See indiv. patches for detailed change logs.

Applied this version to target-arm.next, thanks.

-- PMM