[Qemu-devel] [PATCH for-2.5 v4 0/3] ARM: Highbank: Highbank: Add monitor support

[Qemu-devel] [PATCH for-2.5 v4 0/3] ARM: Highbank: Highbank: Add monitor support

[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).

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(-)

-- 
1.9.1

[Qemu-devel] [PATCH for-2.5 v4 1/3] arm: boot: Add secure_board_setup flag

[Peter Crosthwaite]

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 <crosthwaite.peter@gmail.com>
---
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(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index b0879a5..75f69bf 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -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) {
 
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index 67ba7db..c26b0e3 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -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;
 };
 
 /**
-- 
1.9.1

[Qemu-devel] [PATCH for-2.5 v4 2/3] arm: highbank: Defeature CPU override

[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 <peter.maydell@linaro.org>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---

 hw/arm/highbank.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index be04b27..f2e248b 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -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);
 
-- 
1.9.1

[Qemu-devel] [PATCH for-2.5 v4 3/3] arm: highbank: Implement PSCI and dummy monitor

[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 <peter.maydell@linaro.org>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
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(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index f2e248b..85ae69e 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -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);
 }
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH for-2.5 v4 0/3] ARM: Highbank: Highbank: Add monitor support

[Peter Maydell]

On 9 November 2015 at 07:10, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> 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.

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

-- PMM