Skip to contentSkip to conversation

Verification For arm64: suspend/resume implementation

Question Leo Yan 5 participants 12 messages Sep 13, 2013 – Sep 27, 2013

lore
Share & tools
lore

Leo Yan

author
#0 lore raw

hi Lorenzo,

I have applied your ARM64's suspend/resume related patches and can built successfully. i want to verify these patches on foundation model firstly, so below are my questions:

"Code has been tested on AEM v8 models and a simple CPU idle driver that enables a C-state where CPUs are reset when wfi is hit."

1. Can u help share this simple cpu idle driver?
2. On the foundation model, if the core is placed into reset state, then if there have interrupt is routed to the core, the core still cannot be waken up anymore; because the reset bit cannot be released by h/w. So how can let the core return back from the reset state?

Signature · 2 lines
Thx,
Leo Yan
#1 lore raw

Hi Leo,

Quoted 6 lines · On Thu, 12 Sep 2013 19:53:53 -0700 Leo Yan <[email protected]> wrote:

hi Lorenzo,

I have applied your ARM64's suspend/resume related patches and can built successfully. i want to verify these patches on foundation model firstly, so below are my questions:

Quoted 9 lines

"Code has been tested on AEM v8 models and a simple CPU idle driver that enables a C-state where CPUs are reset when wfi is hit."

1. Can u help share this simple cpu idle driver?
2. On the foundation model, if the core is placed into reset state, then if there have interrupt is routed to the core, the core still cannot be waken up anymore; because the reset bit cannot be released by h/w. So how can let the core return back from the reset state?

Does marvell have Fast models' license? Where could I get such license? I'm using the evaluation version.

Thanks

#2 lore raw
In reply to Leo Yan
Quoted 10 lines · Leo Yan

hi Lorenzo,

I have applied your ARM64's suspend/resume related patches and can built successfully. i want to verify these patches on foundation model firstly, so below are my questions:

"Code has been tested on AEM v8 models and a simple CPU idle driver that enables a C-state where CPUs are reset when wfi is hit."

1. Can u help share this simple cpu idle driver?

Yes, I will post a simple skeleton driver, PSCI based (bootwrapper implementation), by the end of September.

Quoted 4 lines

2. On the foundation model, if the core is placed into reset state, then if there have interrupt is routed to the core, the core still cannot be waken up anymore; because the reset bit cannot be released by h/w. So how can let the core return back from the reset state?

Well, I am testing it with the AEM models power controller that is not publicly available yet, and _should_ be released with the new version of the foundation models.

As a first step, I will write a PSCI suspend implementation that just executes wfi and resumes through the reset vector to emulate a power down as a means to make the suspend/resume code path usable to everyone.

Lorenzo

author
#3 lore raw
Quoted 20 lines · Lorenzo Pieralisi
Quoted 10 lines · Leo Yan

hi Lorenzo,

I have applied your ARM64's suspend/resume related patches and can built successfully. i want to verify these patches on foundation model firstly, so below are my questions:

"Code has been tested on AEM v8 models and a simple CPU idle driver that enables a C-state where CPUs are reset when wfi is hit."

1. Can u help share this simple cpu idle driver?

Yes, I will post a simple skeleton driver, PSCI based (bootwrapper implementation), by the end of September.

Quoted 4 lines

2. On the foundation model, if the core is placed into reset state, then if there have interrupt is routed to the core, the core still cannot be waken up anymore; because the reset bit cannot be released by h/w. So how can let the core return back from the reset state?

Quoted 8 lines

Well, I am testing it with the AEM models power controller that is not publicly available yet, and _should_ be released with the new version of the foundation models.

As a first step, I will write a PSCI suspend implementation that just executes wfi and resumes through the reset vector to emulate a power down as a means to make the suspend/resume code path usable to everyone.

Looking forward the related implementation; After them are ready, i'm glad have a trying.

At my side, i'm warming up related doc and tried to debug related tear down opreations according to CA53's TRM; pls see enclosed two patches. Firstly clarify, these two patches are _ONLY_ for debugging purpose, i have no plan to commit them for Linux kernel.

0001-cpuidle-add-simple-driver-for-arm64.patch: it's a simple cpuidle driver; 0002-ARM64-add-cpu-tear-down-function-for-A53-s-power-mod.patch: i tried to wrote a tear down operations (disable D$/flush L1 cache/Disable SMP bit, etc);

But i found in the patch 2, if the core execute the instruction "mrs
x0, S3_1_C15_C2_1" to access CPUECTLR_EL1, the kernel will report the
illegal instructions. So just like before i saw the discussion on mailing list, On ARMv8 we need operate the SMP bit in EL2/EL3/Secure EL1 but not in non-secure EL1.

Here i tried two methods to try to fix this issue, but both of them were failure:
1. I tried to set the ACTLR_EL2 bit 1 in the boot wrapper code, but when in the non-secure world's kernel to access CPUECTLR_EL1 it still will report the panic for illegal instruction;
2. I tried to modify the boot wrapper code to let the kernel stay in secure world's EL1, but looks like it also failed;

So do u have any suggestion for this failure?

Thx,
Leo Yan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-cpuidle-add-simple-driver-for-arm64.patch
Type: text/x-patch
Size: 5866 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130923/1711d443/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-ARM64-add-cpu-tear-down-function-for-A53-s-power-mod.patch
Type: text/x-patch
Size: 4314 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130923/1711d443/attachment-0003.bin>

#4 lore raw

Hi Leo,

Quoted 58 lines · Leo Yan
Quoted 20 lines · Lorenzo Pieralisi
Quoted 10 lines · Leo Yan

hi Lorenzo,

I have applied your ARM64's suspend/resume related patches and can built successfully. i want to verify these patches on foundation model firstly, so below are my questions:

"Code has been tested on AEM v8 models and a simple CPU idle driver that enables a C-state where CPUs are reset when wfi is hit."

1. Can u help share this simple cpu idle driver?

Yes, I will post a simple skeleton driver, PSCI based (bootwrapper implementation), by the end of September.

Quoted 4 lines

2. On the foundation model, if the core is placed into reset state, then if there have interrupt is routed to the core, the core still cannot be waken up anymore; because the reset bit cannot be released by h/w. So how can let the core return back from the reset state?

Quoted 8 lines

Well, I am testing it with the AEM models power controller that is not publicly available yet, and _should_ be released with the new version of the foundation models.

As a first step, I will write a PSCI suspend implementation that just executes wfi and resumes through the reset vector to emulate a power down as a means to make the suspend/resume code path usable to everyone.

Looking forward the related implementation; After them are ready, i'm glad have a trying.

At my side, i'm warming up related doc and tried to debug related tear down opreations according to CA53's TRM; pls see enclosed two patches. Firstly clarify, these two patches are _ONLY_ for debugging purpose, i have no plan to commit them for Linux kernel.

0001-cpuidle-add-simple-driver-for-arm64.patch: it's a simple cpuidle driver; 0002-ARM64-add-cpu-tear-down-function-for-A53-s-power-mod.patch: i tried to wrote a tear down operations (disable D$/flush L1 cache/Disable SMP bit, etc);

But i found in the patch 2, if the core execute the instruction "mrs
x0, S3_1_C15_C2_1" to access CPUECTLR_EL1, the kernel will report the
illegal instructions. So just like before i saw the discussion on mailing list, On ARMv8 we need operate the SMP bit in EL2/EL3/Secure EL1 but not in non-secure EL1.

Here i tried two methods to try to fix this issue, but both of them were failure:
1. I tried to set the ACTLR_EL2 bit 1 in the boot wrapper code, but when in the non-secure world's kernel to access CPUECTLR_EL1 it still will report the panic for illegal instruction;
2. I tried to modify the boot wrapper code to let the kernel stay in secure world's EL1, but looks like it also failed;

Quoted 1 line

So do u have any suggestion for this failure?

The foundation model (if thats what you are using) does not model an ARM cpu implementation. The CPUECTLR is a cpu specific register (imp. def.) so it is not present. The caches on the Foundation Model are inherently coherent so you do not need to access this register. If you do then the access is treated as an illegal instruction.

hth,
Achin

Quoted 359 lines

Thx,
Leo Yan

From a1aa5dd0924b8c7493c6fec31c00b2feba7f5ad7 Mon Sep 17 00:00:00 2001
From: Leo Yan <[email protected]>
Date: Mon, 23 Sep 2013 17:05:42 +0800
Subject: [PATCH 1/2] cpuidle: add simple driver for arm64

---
 drivers/cpuidle/Kconfig         |    6 ++
 drivers/cpuidle/Makefile        |    1 +
 drivers/cpuidle/cpuidle-arm64.c |  180 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 187 insertions(+)
 create mode 100644 drivers/cpuidle/cpuidle-arm64.c
drivers/cpuidle/Kconfig +6 -0
index c4cc27e..361325e 100644
@@ -39,4 +39,10 @@ config CPU_IDLE_CALXEDA
 	help
 	  Select this to enable cpuidle on Calxeda processors.
 
+config CPU_IDLE_ARM64
+	bool "CPU Idle Driver for ARM64"
+	depends on ARM64
+	help
+	  Select this to enable cpuidle on ARM64 processors.
+
 endif
drivers/cpuidle/Makefile +1 -0
index 0d8bd55..954b67f 100644
@@ -7,3 +7,4 @@ obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
 
 obj-$(CONFIG_CPU_IDLE_CALXEDA) += cpuidle-calxeda.o
 obj-$(CONFIG_ARCH_KIRKWOOD) += cpuidle-kirkwood.o
+obj-$(CONFIG_CPU_IDLE_ARM64) += cpuidle-arm64.o
drivers/cpuidle/cpuidle-arm64.c new file +180 -1
new file mode 100644
index 0000000..7d376db
@@ -0,0 +1,180 @@
+/*
+ * ARM64 CPU idle driver.
+ *
+ * Copyright (C) 2012 ARM Ltd.
+ * Author: Lorenzo Pieralisi <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/bitmap.h>
+#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+#include <linux/clockchips.h>
+#include <linux/debugfs.h>
+#include <linux/hrtimer.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/tick.h>
+#include <asm/proc-fns.h>
+#include <asm/suspend.h>
+#include <asm/cacheflush.h>
+
+static int arm64_cpuidle_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{
+	ktime_t time_start, time_end;
+	s64 diff;
+
+	time_start = ktime_get();
+
+	cpu_do_idle();
+
+	time_end = ktime_get();
+
+	local_irq_enable();
+
+	diff = ktime_to_us(ktime_sub(time_end, time_start));
+	if (diff > INT_MAX)
+		diff = INT_MAX;
+
+	dev->last_residency = (int) diff;
+
+	return index;
+}
+
+static int arm64_enter_powerdown(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int idx);
+
+static struct cpuidle_state arm64_cpuidle_set[] __initdata = {
+	[0] = {
+		.enter                  = arm64_cpuidle_simple_enter,
+		.exit_latency           = 1,
+		.target_residency       = 1,
+		.power_usage		= UINT_MAX,
+		.flags                  = CPUIDLE_FLAG_TIME_VALID,
+		.name                   = "WFI",
+		.desc                   = "ARM64 WFI",
+	},
+#if 1
+	[1] = {
+		.enter			= arm64_enter_powerdown,
+		.exit_latency		= 300,
+		.target_residency	= 1000,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "C1",
+		.desc			= "ARM64 power down",
+	},
+#endif
+};
+
+struct cpuidle_driver arm64_idle_driver = {
+	.name = "arm64_idle",
+	.owner = THIS_MODULE,
+	.safe_state_index = 0
+};
+
+static DEFINE_PER_CPU(struct cpuidle_device, arm64_idle_dev);
+
+extern void arm64_cpu_tear_down(void);
+typedef void (*phys_reset_t)(unsigned long);
+
+static int notrace arm64_powerdown_finisher(unsigned long arg)
+{
+	phys_reset_t phys_reset;
+
+	setup_mm_for_reboot();
+
+	arm64_cpu_tear_down();
+	wfi();
+
+	phys_reset = (phys_reset_t)(unsigned long)virt_to_phys(cpu_reset);
+	phys_reset(virt_to_phys(cpu_resume));
+
+	/* should never get here */
+	BUG();
+}
+
+/*
+ * arm64_enter_powerdown - Programs CPU to enter the specified state
+ * @dev: cpuidle device
+ * @drv: The target state to be programmed
+ * @idx: state index
+ *
+ * Called from the CPUidle framework to program the device to the
+ * specified target state selected by the governor.
+ */
+static int arm64_enter_powerdown(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int idx)
+{
+	struct timespec ts_preidle, ts_postidle, ts_idle;
+	int ret;
+
+	/* Used to keep track of the total time in idle */
+	getnstimeofday(&ts_preidle);
+
+	BUG_ON(!irqs_disabled());
+
+	cpu_pm_enter();
+
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+
+	ret = cpu_suspend((unsigned long) dev, arm64_powerdown_finisher);
+	if (ret)
+		BUG();
+
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+
+	cpu_pm_exit();
+
+	getnstimeofday(&ts_postidle);
+	local_irq_enable();
+	ts_idle = timespec_sub(ts_postidle, ts_preidle);
+
+	dev->last_residency = ts_idle.tv_nsec / NSEC_PER_USEC +
+					ts_idle.tv_sec * USEC_PER_SEC;
+	return idx;
+}
+
+/*
+ * arm64_idle_init
+ *
+ * Registers the bl specific cpuidle driver with the cpuidle
+ * framework with the valid set of states.
+ */
+int __init arm64_idle_init(void)
+{
+	struct cpuidle_device *dev;
+	int i, cpu_id;
+	struct cpuidle_driver *drv = &arm64_idle_driver;
+
+	drv->state_count = (sizeof(arm64_cpuidle_set) /
+				       sizeof(struct cpuidle_state));
+
+	for (i = 0; i < drv->state_count; i++) {
+		memcpy(&drv->states[i], &arm64_cpuidle_set[i],
+				sizeof(struct cpuidle_state));
+	}
+
+	cpuidle_register_driver(drv);
+
+	for_each_cpu(cpu_id, cpu_online_mask) {
+		pr_err("CPUidle for CPU%d registered\n", cpu_id);
+		dev = &per_cpu(arm64_idle_dev, cpu_id);
+		dev->cpu = cpu_id;
+
+		dev->state_count = drv->state_count;
+
+		if (cpuidle_register_device(dev)) {
+			printk(KERN_ERR "%s: Cpuidle register device failed\n",
+			       __func__);
+			return -EIO;
+		}
+	}
+
+	return 0;
+}
+
+device_initcall(arm64_idle_init);
--

1.7.9.5

From e238ddfc426bd1f8b2ac3b5f396a31150cbe7e5c Mon Sep 17 00:00:00 2001
From: Leo Yan <[email protected]>
Date: Mon, 23 Sep 2013 17:06:20 +0800
Subject: [PATCH 2/2] ARM64: add cpu tear down function for A53's power mode

---
 arch/arm64/kernel/Makefile        |    2 +-
 arch/arm64/kernel/cpu_tear_down.S |   90 +++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/kernel/cpu_tear_down.S
arch/arm64/kernel/Makefile +1 -1
index 010550a..046fbf0 100644
@@ -18,7 +18,7 @@ arm64-obj-$(CONFIG_SMP)			+= smp.o smp_spin_table.o smp_psci.o
 arm64-obj-$(CONFIG_HW_PERF_EVENTS)	+= perf_event.o
 arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT)+= hw_breakpoint.o
 arm64-obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
-arm64-obj-$(CONFIG_ARM_CPU_SUSPEND)	+= sleep.o suspend.o
+arm64-obj-$(CONFIG_ARM_CPU_SUSPEND)	+= sleep.o suspend.o cpu_tear_down.o
 
 arm64-obj-$(CONFIG_SUSPEND)		+= fake_suspend.o
 obj-y					+= $(arm64-obj-y) vdso/
arch/arm64/kernel/cpu_tear_down.S new file +90 -1
new file mode 100644
index 0000000..9f3d5d0
@@ -0,0 +1,90 @@
+/*
+ * arch/arm64/mach-vexpress/sleep.S
+ *
+ * Copyright (c) 2013 Marvell Semiconductor Inc.
+ *
+ * Author: Leo Yan <[email protected]>
+ *
+ * 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.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+#include <linux/linkage.h>
+#include <linux/init.h>
+#include <asm/assembler.h>
+
+/*
+ * Exits SMP coherency.
+ */
+ENTRY(arm64_cpu_tear_down)
+	mov	x12, lr
+
+	mrs	x0, sctlr_el1
+	bic	x0, x0, #1 << 2			// clear SCTLR.C
+	msr	sctlr_el1, x0
+	isb
+
+	dsb	sy				// ensure ordering with previous memory accesses
+	mrs	x0, clidr_el1			// read clidr
+	and	x3, x0, #0x7000000		// extract loc from clidr
+	lsr	x3, x3, #23			// left align loc bit field
+	cbz	x3, finished			// if loc is 0, then no need to clean
+	mov	x10, #0				// start clean at cache level 0
+
+	add	x2, x10, x10, lsr #1		// work out 3x current cache level
+	lsr	x1, x0, x2			// extract cache type bits from clidr
+	and	x1, x1, #7			// mask of the bits for current cache only
+	cmp	x1, #2				// see what cache we have at this level
+	b.lt	skip				// skip if no cache, or just i-cache
+	save_and_disable_irqs x9		// make CSSELR and CCSIDR access atomic
+	msr	csselr_el1, x10			// select current cache level in csselr
+	isb					// isb to sych the new cssr&csidr
+	mrs	x1, ccsidr_el1			// read the new ccsidr
+	restore_irqs x9
+	and	x2, x1, #7			// extract the length of the cache lines
+	add	x2, x2, #4			// add 4 (line length offset)
+	mov	x4, #0x3ff
+	and	x4, x4, x1, lsr #3		// find maximum number on the way size
+	clz	w5, w4				// find bit position of way size increment
+	mov	x7, #0x7fff
+	and	x7, x7, x1, lsr #13		// extract max number of the index size
+loop2:
+	mov	x9, x4				// create working copy of max way size
+loop3:
+	lsl	x6, x9, x5
+	orr	x11, x10, x6			// factor way and cache number into x11
+	lsl	x6, x7, x2
+	orr	x11, x11, x6			// factor index number into x11
+	dc	cisw, x11			// clean & invalidate by set/way
+	subs	x9, x9, #1			// decrement the way
+	b.ge	loop3
+	subs	x7, x7, #1			// decrement the index
+	b.ge	loop2
+
+skip:
+finished:
+	mov	x10, #0				// swith back to cache level 0
+	msr	csselr_el1, x10			// select current cache level in csselr
+	dsb	sy
+	isb
+
+	mrs	x0, S3_1_C15_C2_1
+	bic	x0, x0, #0x1 << 6		// disable SMP bit
+	msr	S3_1_C15_C2_1, x0
+	dsb	sy
+	isb
+
+	ret	x12
+ENDPROC(arm64_cpu_tear_down)
+
--

1.7.9.5

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

author
#5 lore raw
Quoted 6 lines · Achin Gupta

The foundation model (if thats what you are using) does not model an ARM cpu implementation. The CPUECTLR is a cpu specific register (imp. def.) so it is not present. The caches on the Foundation Model are inherently coherent so you do not need to access this register. If you do then the access is treated as an illegal instruction.

Thx for the info. So do u mean i need use FVP Model for A53?

Here have another question, ARM have the example code for boot wrapper which will switch from EL3 to secure EL1 rather than non-secure's EL1?

Thx,
Leo Yan

#6 lore raw

Hi Leo,

Quoted 11 lines · Leo Yan
Quoted 6 lines · Achin Gupta

The foundation model (if thats what you are using) does not model an ARM cpu implementation. The CPUECTLR is a cpu specific register (imp. def.) so it is not present. The caches on the Foundation Model are inherently coherent so you do not need to access this register. If you do then the access is treated as an illegal instruction.

Thx for the info. So do u mean i need use FVP Model for A53?

I think you should use the dual cluster A57_A53 Base FVP models. They have the power controller and model the CPUECTLR.SMP bit behaviour as well.

Quoted 3 lines

Here have another question, ARM have the example code for boot wrapper which will switch from EL3 to secure EL1 rather than non-secure's EL1?

I dont' think we do but let me check. Switching to S-EL1 instead of NS-EL1 should be a matter of _not_ setting the SCR_EL3.NS bit before doing the exception level change (ERET).

hth,
Achin

Quoted 4 lines

Thx,
Leo Yan

author
#7 lore raw
Quoted 13 lines · Achin Gupta

Hi Leo,

Quoted 10 lines · Leo Yan
Quoted 6 lines · Achin Gupta

The foundation model (if thats what you are using) does not model an ARM cpu implementation. The CPUECTLR is a cpu specific register (imp. def.) so it is not present. The caches on the Foundation Model are inherently coherent so you do not need to access this register. If you do then the access is treated as an illegal instruction.

Quoted 6 lines
Quoted 1 line

Thx for the info. So do u mean i need use FVP Model for A53?

I think you should use the dual cluster A57_A53 Base FVP models. They have the power controller and model the CPUECTLR.SMP bit behaviour as well.

Hmm...So far i only focus on A53; actually i have built FVP model for A53x4 cores, but now are pending for the license. If this model can support power controller and SMP bit, it will let my work much clean.

Quoted 7 lines
Quoted 3 lines

Here have another question, ARM have the example code for boot wrapper which will switch from EL3 to secure EL1 rather than non-secure's EL1?

I dont' think we do but let me check. Switching to S-EL1 instead of NS-EL1 should be a matter of _not_ setting the SCR_EL3.NS bit before doing the exception level change (ERET).

I quick try with below patch for boot wrapper, i saw foundataion model cannot boot up successfully and there have no console output; suppose it's hang at some boot operations, but now foundation model cannot debug low level code, so i cannot debug further more. :-(

boot.S +0 -1
index a1f25e2..2f1b867 100644
@@ -19,7 +19,6 @@ _start:
         b.ne    start_ns                        // skip EL3 initialisation
 
         mov     x0, #0x30                       // RES1
-       orr     x0, x0, #(1 << 0)               // Non-secure EL1
         orr     x0, x0, #(1 << 8)               // HVC enable
         orr     x0, x0, #(1 << 10)              // 64-bit EL2
         msr     scr_el3, x0

Thx,
Leo Yan

#8 lore raw
Quoted 19 lines · Leo Yan
Quoted 7 lines · Achin Gupta
Quoted 2 lines · Leo Yan

Here have another question, ARM have the example code for boot wrapper which will switch from EL3 to secure EL1 rather than non-secure's EL1?

I dont' think we do but let me check. Switching to S-EL1 instead of NS-EL1 should be a matter of _not_ setting the SCR_EL3.NS bit before doing the exception level change (ERET).

I quick try with below patch for boot wrapper, i saw foundataion model cannot boot up successfully and there have no console output; suppose it's hang at some boot operations, but now foundation model cannot debug low level code, so i cannot debug further more. :-(

boot.S +0 -0
index a1f25e2..2f1b867 100644

boot.S _start: · line 24

Quoted 7 lines
boot.S +0 -1
@@ -19,7 +19,6 @@ _start:
         b.ne    start_ns                        // skip EL3 initialisation
 
         mov     x0, #0x30                       // RES1
-       orr     x0, x0, #(1 << 0)               // Non-secure EL1
         orr     x0, x0, #(1 << 8)               // HVC enable
         orr     x0, x0, #(1 << 10)              // 64-bit EL2
         msr     scr_el3, x0

You also need to make sure the boot wrapper starts the kernel in S-EL1 rather than NS-EL2.

BTW, why do you need this? There are no virtualisation features supported on the secure side.

Signature · 1 line
Catalin
author
#9 lore raw
Quoted 22 lines · Catalin Marinas
Quoted 21 lines · Leo Yan
Quoted 7 lines · Achin Gupta
Quoted 2 lines · Leo Yan

Here have another question, ARM have the example code for boot wrapper which will switch from EL3 to secure EL1 rather than non-secure's EL1?

I dont' think we do but let me check. Switching to S-EL1 instead of NS-EL1 should be a matter of _not_ setting the SCR_EL3.NS bit before doing the exception level change (ERET).

I quick try with below patch for boot wrapper, i saw foundataion model cannot boot up successfully and there have no console output; suppose it's hang at some boot operations, but now foundation model cannot debug low level code, so i cannot debug further more. :-(

boot.S +0 -0
index a1f25e2..2f1b867 100644
@@ -19,7 +19,6 @@ _start:
          b.ne    start_ns                        // skip EL3 initialisation
 
Quoted 9 lines

boot.S _start: · line 24

Quoted 5 lines
boot.S +0 -1
          mov     x0, #0x30                       // RES1
-       orr     x0, x0, #(1 << 0)               // Non-secure EL1
          orr     x0, x0, #(1 << 8)               // HVC enable
          orr     x0, x0, #(1 << 10)              // 64-bit EL2
          msr     scr_el3, x0

You also need to make sure the boot wrapper starts the kernel in S-EL1 rather than NS-EL2.

Hi Catalin,

I tried to use below modification, but it still failed to boot up on foundation model; pls help review if convienence.

Also, i reviewed the kernel's code, the kernel will check if the core is NS-EL2 and will switch to NS-EL1; do u think this code will conflict w/t the core run into the kernel with S-EL1 state?

boot.S +1 -2
index a1f25e2..f5683d6 100644
@@ -19,7 +19,6 @@ _start:
         b.ne    start_ns                        // skip EL3 initialisation
 
         mov     x0, #0x30                       // RES1
-       orr     x0, x0, #(1 << 0)               // Non-secure EL1
         orr     x0, x0, #(1 << 8)               // HVC enable
         orr     x0, x0, #(1 << 10)              // 64-bit EL2
         msr     scr_el3, x0
@@ -62,7 +61,7 @@ _start:
          * Prepare the switch to the EL2_SP1 mode from EL3
          */
         ldr     x0, =start_ns                   // Return after mode switch
-       mov     x1, #0x3c9                      // EL2_SP1 | D | A | I | F
+       mov     x1, #0x3c5                      // EL1_SP1 | D | A | I | F
         msr     elr_el3, x0
         msr     spsr_el3, x1
         eret
Quoted 2 lines

BTW, why do you need this? There are no virtualisation features supported on the secure side.

So far the virtualisation is not a mandatory feature in short term due now our product is mainly focusing on the mobile product, so firstly we want to enable Linux kernel directly on secure world; just like we do same as ARMv7.

From the previous discussion of MCPM, I understand this is not very
consolidate w/t ARM's original idea of generic firmware (including PSCI); but actually i also think this is not confict.

because even the kernel run in S-EL1, we also can call SMC to switch to monitor mode and call PSCI related APIs, so whatever the kernel run in which world, eventually the firmware and PSCI will both support them. pls correct me if i misunderstand for this.

Thx,
Leo Yan

#10 lore raw
Quoted 32 lines · Leo Yan
Quoted 22 lines · Catalin Marinas
Quoted 21 lines · Leo Yan
Quoted 7 lines · Achin Gupta
Quoted 2 lines · Leo Yan

Here have another question, ARM have the example code for boot wrapper which will switch from EL3 to secure EL1 rather than non-secure's EL1?

I dont' think we do but let me check. Switching to S-EL1 instead of NS-EL1 should be a matter of _not_ setting the SCR_EL3.NS bit before doing the exception level change (ERET).

I quick try with below patch for boot wrapper, i saw foundataion model cannot boot up successfully and there have no console output; suppose it's hang at some boot operations, but now foundation model cannot debug low level code, so i cannot debug further more. :-(

boot.S +0 -0
index a1f25e2..2f1b867 100644
@@ -19,7 +19,6 @@ _start:
          b.ne    start_ns                        // skip EL3 initialisation
 
Quoted 8 lines

boot.S _start: · line 24

Quoted 5 lines
boot.S +0 -1
          mov     x0, #0x30                       // RES1
-       orr     x0, x0, #(1 << 0)               // Non-secure EL1
          orr     x0, x0, #(1 << 8)               // HVC enable
          orr     x0, x0, #(1 << 10)              // 64-bit EL2
          msr     scr_el3, x0

You also need to make sure the boot wrapper starts the kernel in S-EL1 rather than NS-EL2.

Quoted 2 lines

I tried to use below modification, but it still failed to boot up on foundation model; pls help review if convienence.

You probably need group 0 interrupts as well.

Quoted 3 lines

Also, i reviewed the kernel's code, the kernel will check if the core is NS-EL2 and will switch to NS-EL1; do u think this code will conflict w/t the core run into the kernel with S-EL1 state?

The kernel, if started at EL1, doesn't know whether it's secure or non-secure. So this code wouldn't get in the way.

Quoted 7 lines
Quoted 2 lines

BTW, why do you need this? There are no virtualisation features supported on the secure side.

So far the virtualisation is not a mandatory feature in short term due now our product is mainly focusing on the mobile product, so firstly we want to enable Linux kernel directly on secure world; just like we do same as ARMv7.

But is there other reason than just being the same as on your ARMv7 platform?

Apart from virtualisation, there are other Linux developments that assume Linux running in non-secure mode (SMMU, future GIC). I *strongly* recommend that you run AArch64 Linux in non-secure mode from start (and for the early bring-up, just configure your hardware to allow non-secure access to all your peripherals/registers).

You also make life easier for your customers (mobile vendors) that are more likely to require a secure OS.

Quoted 8 lines

From the previous discussion of MCPM, I understand this is not very
consolidate w/t ARM's original idea of generic firmware (including PSCI); but actually i also think this is not confict.

because even the kernel run in S-EL1, we also can call SMC to switch to monitor mode and call PSCI related APIs, so whatever the kernel run in which world, eventually the firmware and PSCI will both support them. pls correct me if i misunderstand for this.

That's not in conflict with PSCI as this is handled at EL3 (the first mode an ARMv8 CPU runs in, unlike ARMv7 where it starts in secure EL1, a.k.a. SVC).

Signature · 1 line
Catalin
author
#11 lore raw
Quoted 33 lines · Catalin Marinas
Quoted 32 lines · Leo Yan
Quoted 30 lines · Catalin Marinas
Quoted 19 lines · Leo Yan
Quoted 7 lines · Achin Gupta
Quoted 2 lines · Leo Yan

Here have another question, ARM have the example code for boot wrapper which will switch from EL3 to secure EL1 rather than non-secure's EL1?

I dont' think we do but let me check. Switching to S-EL1 instead of NS-EL1 should be a matter of _not_ setting the SCR_EL3.NS bit before doing the exception level change (ERET).

I quick try with below patch for boot wrapper, i saw foundataion model cannot boot up successfully and there have no console output; suppose it's hang at some boot operations, but now foundation model cannot debug low level code, so i cannot debug further more. :-(

boot.S +0 -0
index a1f25e2..2f1b867 100644

boot.S _start: · line 24

Quoted 7 lines
boot.S +0 -1
@@ -19,7 +19,6 @@ _start:
           b.ne    start_ns                        // skip EL3 initialisation
 
           mov     x0, #0x30                       // RES1
-       orr     x0, x0, #(1 << 0)               // Non-secure EL1
           orr     x0, x0, #(1 << 8)               // HVC enable
           orr     x0, x0, #(1 << 10)              // 64-bit EL2
           msr     scr_el3, x0

You also need to make sure the boot wrapper starts the kernel in S-EL1 rather than NS-EL2.

Quoted 4 lines
Quoted 2 lines

I tried to use below modification, but it still failed to boot up on foundation model; pls help review if convienence.

You probably need group 0 interrupts as well.

Thanks a lot for help. After change the IRQs to group 0, then kernel can work well in S-EL1. :-)

boot.S +1 -2
index a1f25e2..ae14c66 100644
@@ -19,7 +19,6 @@ _start:
         b.ne    start_ns                        // skip EL3 initialisation
 
         mov     x0, #0x30                       // RES1
-       orr     x0, x0, #(1 << 0)               // Non-secure EL1
         orr     x0, x0, #(1 << 8)               // HVC enable
         orr     x0, x0, #(1 << 10)              // 64-bit EL2
         msr     scr_el3, x0
@@ -42,7 +41,7 @@ _start:
         str     w0, [x1]
 
  1:     ldr     x1, =GIC_DIST_BASE + 0x80       // GICD_IGROUPR
-       mov     w0, #~0                         // Grp1 interrupts
+       mov     w0, #0                          // Grp0 interrupts
         str     w0, [x1], #4
         b.ne    2f                              // Only local 

interrupts for secondary CPUs

         str     w0, [x1], #4

@@ -62,7 +61,7 @@ _start:
* Prepare the switch to the EL2_SP1 mode from EL3 */
ldr x0, =start_ns // Return after mode switch

-       mov     x1, #0x3c9                      // EL2_SP1 | D | A | I | F
+       mov     x1, #0x3c5                      // EL1_SP1 | D | A | I | F

msr elr_el3, x0
msr spsr_el3, x1
eret

Quoted 8 lines
Quoted 7 lines
Quoted 2 lines

BTW, why do you need this? There are no virtualisation features supported on the secure side.

So far the virtualisation is not a mandatory feature in short term due now our product is mainly focusing on the mobile product, so firstly we want to enable Linux kernel directly on secure world; just like we do same as ARMv7.

Quoted 9 lines

But is there other reason than just being the same as on your ARMv7 platform?

Apart from virtualisation, there are other Linux developments that assume Linux running in non-secure mode (SMMU, future GIC). I *strongly* recommend that you run AArch64 Linux in non-secure mode from start (and for the early bring-up, just configure your hardware to allow non-secure access to all your peripherals/registers).

I have no such whole picture as you arm guys for ARMv8 and related features; this is also what i have concern if here have show stopper issue if kernel run in S-EL1. So thanks for the reminding and more detailed info will always be welcomed.

After the generic firmware has been opened and we get the code, then we will study it carefully and estimate how to deploy it.

Thx,
Leo Yan