[PATCH v2 0/6] Add support for a fake, para-virtualised machine

[PATCH v2 0/6] Add support for a fake, para-virtualised machine

[Will Deacon]

Hello again,

This is version two of the patches originally posted here:

  http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/135870.html

Given the lively discussion sparked in that thread, there have been a
fair number of changes since the RFC:

	* A client-side implementation of PSCI and proposed DT binding
	* Use of PSCI for SMP boot
	* Removal of the SMP pen code
	* Dropped the RFC tag

Marc hacked up the KVM side separately and this has been tested
successfully with his code using a magic build of kvmtool.

As usual, all feedback welcome.

Cheers,

Will


Marc Zyngier (1):
  ARM: Dummy Virtual Machine platform support

Will Deacon (5):
  ARM: opcodes: add missing include of linux/linkage.h
  ARM: opcodes: add opcodes definitions for ARM security extensions
  ARM: psci: add devicetree binding for describing PSCI firmware
  ARM: psci: add support for PSCI invocations from the kernel
  ARM: mach-virt: add SMP support using PSCI

 Documentation/devicetree/bindings/arm/psci.txt |  58 +++++++
 arch/arm/Kconfig                               |  12 ++
 arch/arm/Makefile                              |   1 +
 arch/arm/include/asm/opcodes-sec.h             |  24 +++
 arch/arm/include/asm/opcodes.h                 |   1 +
 arch/arm/include/asm/psci.h                    |  36 +++++
 arch/arm/kernel/Makefile                       |   1 +
 arch/arm/kernel/psci.c                         | 214 +++++++++++++++++++++++++
 arch/arm/mach-virt/Kconfig                     |  10 ++
 arch/arm/mach-virt/Makefile                    |   6 +
 arch/arm/mach-virt/platsmp.c                   |  76 +++++++++
 arch/arm/mach-virt/virt.c                      |  71 ++++++++
 12 files changed, 510 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/psci.txt
 create mode 100644 arch/arm/include/asm/opcodes-sec.h
 create mode 100644 arch/arm/include/asm/psci.h
 create mode 100644 arch/arm/kernel/psci.c
 create mode 100644 arch/arm/mach-virt/Kconfig
 create mode 100644 arch/arm/mach-virt/Makefile
 create mode 100644 arch/arm/mach-virt/platsmp.c
 create mode 100644 arch/arm/mach-virt/virt.c

-- 
1.8.0

[PATCH v2 1/6] ARM: opcodes: add missing include of linux/linkage.h

[Will Deacon]

opcodes.h wants to declare an asmlinkage function, so we need to include
linux/linkage.h

Acked-by: Dave Martin <dave.martin@linaro.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/opcodes.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/include/asm/opcodes.h b/arch/arm/include/asm/opcodes.h
index 74e211a..e796c59 100644
--- a/arch/arm/include/asm/opcodes.h
+++ b/arch/arm/include/asm/opcodes.h
@@ -10,6 +10,7 @@
 #define __ASM_ARM_OPCODES_H
 
 #ifndef __ASSEMBLY__
+#include <linux/linkage.h>
 extern asmlinkage unsigned int arm_check_condition(u32 opcode, u32 psr);
 #endif
 
-- 
1.8.0

[PATCH v2 2/6] ARM: opcodes: add opcodes definitions for ARM security extensions

[Will Deacon]

The ARM security extensions introduced the smc instruction, which is not
supported by all versions of GAS.

This patch introduces opcodes-sec.h, so that smc is made available in a
similar manner to hvc.

Acked-by: Dave Martin <dave.martin@linaro.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/opcodes-sec.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 arch/arm/include/asm/opcodes-sec.h

diff --git a/arch/arm/include/asm/opcodes-sec.h b/arch/arm/include/asm/opcodes-sec.h
new file mode 100644
index 0000000..bc3a917
--- /dev/null
+++ b/arch/arm/include/asm/opcodes-sec.h
@@ -0,0 +1,24 @@
+/*
+ * 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.
+ *
+ * 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.
+ *
+ * Copyright (C) 2012 ARM Limited
+ */
+
+#ifndef __ASM_ARM_OPCODES_SEC_H
+#define __ASM_ARM_OPCODES_SEC_H
+
+#include <asm/opcodes.h>
+
+#define __SMC(imm4) __inst_arm_thumb32(					\
+	0xE1600070 | (((imm4) & 0xF) << 0),				\
+	0xF7F08000 | (((imm4) & 0xF) << 16)				\
+)
+
+#endif /* __ASM_ARM_OPCODES_SEC_H */
-- 
1.8.0

[PATCH v2 3/6] ARM: psci: add devicetree binding for describing PSCI firmware

[Will Deacon]

This patch adds a new devicetree binding for describing PSCI firmware
to Linux.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 Documentation/devicetree/bindings/arm/psci.txt | 58 ++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/psci.txt

diff --git a/Documentation/devicetree/bindings/arm/psci.txt b/Documentation/devicetree/bindings/arm/psci.txt
new file mode 100644
index 0000000..904d0d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/psci.txt
@@ -0,0 +1,58 @@
+* Power State Coordination Interface (PSCI)
+
+Firmware implementing the PSCI functions described in ARM document number
+ARM DEN 0022A ("Power State Coordination Interface System Software on ARM
+processors") can be used by Linux to initiate various CPU-centric power
+operations.
+
+Issue A of the specification describes functions for CPU suspend, hotplug
+and migration of secure software.
+
+Functions are invoked by trapping to the privilege level of the PSCI
+firmware (specified as part of the binding below) and passing arguments
+in a manner similar to that specified by AAPCS:
+
+	 r0		=> 32-bit Function ID / return value
+	{r1 - r3}	=> Parameters
+
+Note that the immediate field of the trapping instruction must be set
+to #0.
+
+
+Main node required properties:
+
+ - compatible    : Must be "arm,psci"
+
+ - method        : The method of calling the PSCI firmware. Permitted
+                   values are:
+
+                   "smc" : SMC #0, with the register assignments specified
+		           in this binding.
+
+                   "hvc" : HVC #0, with the register assignments specified
+		           in this binding.
+
+ - function-base : The base ID from which the functions are offset.
+
+Main node optional properties:
+
+ - cpu_suspend   : Offset of CPU_SUSPEND ID from function-base
+
+ - cpu_off       : Offset of CPU_OFF ID from function-base
+
+ - cpu_on        : Offset of CPU_ON ID from function-base
+
+ - migrate       : Offset of MIGRATE ID from function-base
+
+
+Example:
+
+	psci {
+		compatible	= "arm,psci";
+		method		= "smc";
+		function-base	= <0x95c1ba5e>;
+		cpu_suspend	= <0>;
+		cpu_off		= <1>;
+		cpu_on		= <2>;
+		migrate		= <3>;
+	};
-- 
1.8.0

[PATCH v2 4/6] ARM: psci: add support for PSCI invocations from the kernel

[Will Deacon]

This patch adds support for the Power State Coordination Interface
defined by ARM, allowing Linux to request CPU-centric power-management
operations from firmware implementing the PSCI protocol.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/Kconfig            |  10 +++
 arch/arm/include/asm/psci.h |  36 ++++++++
 arch/arm/kernel/Makefile    |   1 +
 arch/arm/kernel/psci.c      | 214 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 261 insertions(+)
 create mode 100644 arch/arm/include/asm/psci.h
 create mode 100644 arch/arm/kernel/psci.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index e4017ea..2a04375 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1611,6 +1611,16 @@ config HOTPLUG_CPU
 	  Say Y here to experiment with turning CPUs off and on.  CPUs
 	  can be controlled through /sys/devices/system/cpu.
 
+config ARM_PSCI
+	bool "Support for the ARM Power State Coordination Interface (PSCI)"
+	depends on CPU_V7
+	help
+	  Say Y here if you want Linux to communicate with system firmware
+	  implementing the PSCI specification for CPU-centric power
+	  management operations described in ARM document number ARM DEN
+	  0022A ("Power State Coordination Interface System Software on
+	  ARM processors").
+
 config LOCAL_TIMERS
 	bool "Use local timer interrupts"
 	depends on SMP
diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
new file mode 100644
index 0000000..ce0dbe7
--- /dev/null
+++ b/arch/arm/include/asm/psci.h
@@ -0,0 +1,36 @@
+/*
+ * 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.
+ *
+ * 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.
+ *
+ * Copyright (C) 2012 ARM Limited
+ */
+
+#ifndef __ASM_ARM_PSCI_H
+#define __ASM_ARM_PSCI_H
+
+#define PSCI_POWER_STATE_TYPE_STANDBY		0
+#define PSCI_POWER_STATE_TYPE_POWER_DOWN	1
+
+struct psci_power_state {
+	u16	id;
+	u8	type;
+	u8	affinity_level;
+};
+
+struct psci_operations {
+	int (*cpu_suspend)(struct psci_power_state state,
+			   unsigned long entry_point);
+	int (*cpu_off)(struct psci_power_state state);
+	int (*cpu_on)(unsigned long cpuid, unsigned long entry_point);
+	int (*migrate)(unsigned long cpuid);
+};
+
+extern struct psci_operations psci_ops;
+
+#endif /* __ASM_ARM_PSCI_H */
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 5bbec7b..5f3338e 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -82,5 +82,6 @@ obj-$(CONFIG_DEBUG_LL)	+= debug.o
 obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
 
 obj-$(CONFIG_ARM_VIRT_EXT)	+= hyp-stub.o
+obj-$(CONFIG_ARM_PSCI)		+= psci.o
 
 extra-y := $(head-y) vmlinux.lds
diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c
new file mode 100644
index 0000000..e7fe909
--- /dev/null
+++ b/arch/arm/kernel/psci.c
@@ -0,0 +1,214 @@
+/*
+ * 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.
+ *
+ * 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.
+ *
+ * Copyright (C) 2012 ARM Limited
+ *
+ * Author: Will Deacon <will.deacon@arm.com>
+ */
+
+#define pr_fmt(fmt) "psci: " fmt
+
+#include <linux/init.h>
+#include <linux/of.h>
+
+#include <asm/compiler.h>
+#include <asm/errno.h>
+#include <asm/opcodes-sec.h>
+#include <asm/opcodes-virt.h>
+#include <asm/psci.h>
+
+struct psci_operations psci_ops;
+
+static int (*invoke_psci_fn)(u32, u32, u32, u32);
+
+enum psci_function {
+	PSCI_FN_CPU_SUSPEND,
+	PSCI_FN_CPU_ON,
+	PSCI_FN_CPU_OFF,
+	PSCI_FN_MIGRATE,
+	PSCI_FN_MAX,
+};
+
+static u32 psci_function_id[PSCI_FN_MAX];
+
+#define PSCI_RET_SUCCESS		0
+#define PSCI_RET_EOPNOTSUPP		-1
+#define PSCI_RET_EINVAL			-2
+#define PSCI_RET_EPERM			-3
+
+static int psci_to_linux_errno(int errno)
+{
+	switch (errno) {
+	case PSCI_RET_SUCCESS:
+		return 0;
+	case PSCI_RET_EOPNOTSUPP:
+		return -EOPNOTSUPP;
+	case PSCI_RET_EINVAL:
+		return -EINVAL;
+	case PSCI_RET_EPERM:
+		return -EPERM;
+	};
+
+	return -EINVAL;
+}
+
+#define PSCI_POWER_STATE_ID_MASK	0xffff
+#define PSCI_POWER_STATE_ID_SHIFT	0
+#define PSCI_POWER_STATE_TYPE_MASK	0x1
+#define PSCI_POWER_STATE_TYPE_SHIFT	16
+#define PSCI_POWER_STATE_AFFL_MASK	0x3
+#define PSCI_POWER_STATE_AFFL_SHIFT	24
+
+static u32 psci_power_state_pack(struct psci_power_state state)
+{
+	return	((state.id & PSCI_POWER_STATE_ID_MASK)
+			<< PSCI_POWER_STATE_ID_SHIFT)	|
+		((state.type & PSCI_POWER_STATE_TYPE_MASK)
+			<< PSCI_POWER_STATE_TYPE_SHIFT)	|
+		((state.affinity_level & PSCI_POWER_STATE_AFFL_MASK)
+			<< PSCI_POWER_STATE_AFFL_SHIFT);
+}
+
+/*
+ * The following two functions are invoked via the invoke_psci_fn pointer
+ * and will not be inlined, allowing us to piggyback on the AAPCS.
+ */
+static int __invoke_psci_fn_hvc(u32 function_id, u32 arg0, u32 arg1, u32 arg2)
+{
+	asm volatile(
+			__asmeq("%0", "r0")
+			__asmeq("%1", "r1")
+			__asmeq("%2", "r2")
+			__asmeq("%3", "r3")
+			__HVC(0)
+		: "+r" (function_id)
+		: "r" (arg0), "r" (arg1), "r" (arg2));
+
+	return function_id;
+}
+
+static int __invoke_psci_fn_smc(u32 function_id, u32 arg0, u32 arg1, u32 arg2)
+{
+	asm volatile(
+			__asmeq("%0", "r0")
+			__asmeq("%1", "r1")
+			__asmeq("%2", "r2")
+			__asmeq("%3", "r3")
+			__SMC(0)
+		: "+r" (function_id)
+		: "r" (arg0), "r" (arg1), "r" (arg2));
+
+	return function_id;
+}
+
+static int psci_cpu_suspend(struct psci_power_state state,
+			    unsigned long entry_point)
+{
+	int err;
+	u32 fn, power_state;
+
+	fn = psci_function_id[PSCI_FN_CPU_SUSPEND];
+	power_state = psci_power_state_pack(state);
+	err = invoke_psci_fn(fn, power_state, (u32)entry_point, 0);
+	return psci_to_linux_errno(err);
+}
+
+static int psci_cpu_off(struct psci_power_state state)
+{
+	int err;
+	u32 fn, power_state;
+
+	fn = psci_function_id[PSCI_FN_CPU_OFF];
+	power_state = psci_power_state_pack(state);
+	err = invoke_psci_fn(fn, power_state, 0, 0);
+	return psci_to_linux_errno(err);
+}
+
+static int psci_cpu_on(unsigned long cpuid, unsigned long entry_point)
+{
+	int err;
+	u32 fn;
+
+	fn = psci_function_id[PSCI_FN_CPU_ON];
+	err = invoke_psci_fn(fn, cpuid, entry_point, 0);
+	return psci_to_linux_errno(err);
+}
+
+static int psci_migrate(unsigned long cpuid)
+{
+	int err;
+	u32 fn;
+
+	fn = psci_function_id[PSCI_FN_MIGRATE];
+	err = invoke_psci_fn(fn, cpuid, 0, 0);
+	return psci_to_linux_errno(err);
+}
+
+static const struct of_device_id psci_of_match[] __initconst = {
+	{ .compatible = "arm,psci",	},
+	{},
+};
+
+static int __init psci_init(void)
+{
+	struct device_node *np;
+	const char *method;
+	u32 base, id;
+
+	np = of_find_matching_node(NULL, psci_of_match);
+	if (!np)
+		return 0;
+
+	pr_info("probing function IDs from device-tree\n");
+
+	if (of_property_read_u32(np, "function-base", &base)) {
+		pr_warning("missing \"function-base\" property\n");
+		goto out_put_node;
+	}
+
+	if (of_property_read_string(np, "method", &method)) {
+		pr_warning("missing \"method\" property\n");
+		goto out_put_node;
+	}
+
+	if (!strcmp("hvc", method)) {
+		invoke_psci_fn = __invoke_psci_fn_hvc;
+	} else if (!strcmp("smc", method)) {
+		invoke_psci_fn = __invoke_psci_fn_smc;
+	} else {
+		pr_warning("invalid \"method\" property: %s\n", method);
+		goto out_put_node;
+	}
+
+	if (!of_property_read_u32(np, "cpu_suspend", &id)) {
+		psci_function_id[PSCI_FN_CPU_SUSPEND] = base + id;
+		psci_ops.cpu_suspend = psci_cpu_suspend;
+	}
+
+	if (!of_property_read_u32(np, "cpu_off", &id)) {
+		psci_function_id[PSCI_FN_CPU_OFF] = base + id;
+		psci_ops.cpu_off = psci_cpu_off;
+	}
+
+	if (!of_property_read_u32(np, "cpu_on", &id)) {
+		psci_function_id[PSCI_FN_CPU_ON] = base + id;
+		psci_ops.cpu_on = psci_cpu_on;
+	}
+
+	if (!of_property_read_u32(np, "migrate", &id)) {
+		psci_function_id[PSCI_FN_MIGRATE] = base + id;
+		psci_ops.migrate = psci_migrate;
+	}
+
+out_put_node:
+	of_node_put(np);
+	return 0;
+}
+early_initcall(psci_init);
-- 
1.8.0

[PATCH v2 5/6] ARM: Dummy Virtual Machine platform support

[Will Deacon]

From: Marc Zyngier <marc.zyngier@arm.com>

Add support for the smallest, dumbest possible platform, to be
used as a guest for KVM or other hypervisors.

It only mandates a GIC and architected timers. Fits nicely with
a multiplatform zImage. Uses very little silicon area.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/Kconfig            |  2 ++
 arch/arm/Makefile           |  1 +
 arch/arm/mach-virt/Kconfig  |  9 +++++++
 arch/arm/mach-virt/Makefile |  5 ++++
 arch/arm/mach-virt/virt.c   | 65 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 82 insertions(+)
 create mode 100644 arch/arm/mach-virt/Kconfig
 create mode 100644 arch/arm/mach-virt/Makefile
 create mode 100644 arch/arm/mach-virt/virt.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2a04375..552d72e0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1127,6 +1127,8 @@ source "arch/arm/mach-versatile/Kconfig"
 source "arch/arm/mach-vexpress/Kconfig"
 source "arch/arm/plat-versatile/Kconfig"
 
+source "arch/arm/mach-virt/Kconfig"
+
 source "arch/arm/mach-w90x900/Kconfig"
 
 # Definitions to make life easier
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 5f914fc..e8232ad 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -192,6 +192,7 @@ machine-$(CONFIG_ARCH_SOCFPGA)		+= socfpga
 machine-$(CONFIG_ARCH_SPEAR13XX)	+= spear13xx
 machine-$(CONFIG_ARCH_SPEAR3XX)		+= spear3xx
 machine-$(CONFIG_MACH_SPEAR600)		+= spear6xx
+machine-$(CONFIG_ARCH_VIRT)		+= virt
 machine-$(CONFIG_ARCH_ZYNQ)		+= zynq
 
 # Platform directory name.  This list is sorted alphanumerically
diff --git a/arch/arm/mach-virt/Kconfig b/arch/arm/mach-virt/Kconfig
new file mode 100644
index 0000000..a568a2a
--- /dev/null
+++ b/arch/arm/mach-virt/Kconfig
@@ -0,0 +1,9 @@
+config ARCH_VIRT
+	bool "Dummy Virtual Machine" if ARCH_MULTI_V7
+	select ARCH_WANT_OPTIONAL_GPIOLIB
+	select ARM_GIC
+	select ARM_ARCH_TIMER
+	select HAVE_SMP
+	select CPU_V7
+	select SPARSE_IRQ
+	select USE_OF
diff --git a/arch/arm/mach-virt/Makefile b/arch/arm/mach-virt/Makefile
new file mode 100644
index 0000000..7ddbfa6
--- /dev/null
+++ b/arch/arm/mach-virt/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the linux kernel.
+#
+
+obj-y					:= virt.o
diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
new file mode 100644
index 0000000..174b9da
--- /dev/null
+++ b/arch/arm/mach-virt/virt.c
@@ -0,0 +1,65 @@
+/*
+ * Dummy Virtual Machine - does what it says on the tin.
+ *
+ * Copyright (C) 2012 ARM Ltd
+ * Authors: Will Deacon <will.deacon@arm.com>,
+ *          Marc Zyngier <marc.zyngier@arm.com>
+ *
+ * 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.
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+
+#include <asm/arch_timer.h>
+#include <asm/hardware/gic.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/time.h>
+
+const static struct of_device_id irq_match[] = {
+	{ .compatible = "arm,cortex-a15-gic", .data = gic_of_init, },
+	{}
+};
+
+static void __init gic_init_irq(void)
+{
+	of_irq_init(irq_match);
+}
+
+static void __init virt_init(void)
+{
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+}
+
+static void __init virt_timer_init(void)
+{
+	WARN_ON(arch_timer_of_register() != 0);
+	WARN_ON(arch_timer_sched_clock_init() != 0);
+}
+
+static const char *virt_dt_match[] = {
+	"linux,dummy-virt",
+	NULL
+};
+
+static struct sys_timer virt_timer = {
+	.init = virt_timer_init,
+};
+
+DT_MACHINE_START(VIRT, "Dummy Virtual Machine")
+	.init_irq	= gic_init_irq,
+	.handle_irq     = gic_handle_irq,
+	.timer		= &virt_timer,
+	.init_machine	= virt_init,
+	.dt_compat	= virt_dt_match,
+MACHINE_END
-- 
1.8.0

[PATCH v2 6/6] ARM: mach-virt: add SMP support using PSCI

[Will Deacon]

This patch adds support for SMP to mach-virt using the PSCI
infrastructure.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/mach-virt/Kconfig   |  1 +
 arch/arm/mach-virt/Makefile  |  1 +
 arch/arm/mach-virt/platsmp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-virt/virt.c    |  6 ++++
 4 files changed, 84 insertions(+)
 create mode 100644 arch/arm/mach-virt/platsmp.c

diff --git a/arch/arm/mach-virt/Kconfig b/arch/arm/mach-virt/Kconfig
index a568a2a..8958f0d 100644
--- a/arch/arm/mach-virt/Kconfig
+++ b/arch/arm/mach-virt/Kconfig
@@ -3,6 +3,7 @@ config ARCH_VIRT
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARM_GIC
 	select ARM_ARCH_TIMER
+	select ARM_PSCI
 	select HAVE_SMP
 	select CPU_V7
 	select SPARSE_IRQ
diff --git a/arch/arm/mach-virt/Makefile b/arch/arm/mach-virt/Makefile
index 7ddbfa6..042afc1 100644
--- a/arch/arm/mach-virt/Makefile
+++ b/arch/arm/mach-virt/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-y					:= virt.o
+obj-$(CONFIG_SMP)			+= platsmp.o
diff --git a/arch/arm/mach-virt/platsmp.c b/arch/arm/mach-virt/platsmp.c
new file mode 100644
index 0000000..930362b
--- /dev/null
+++ b/arch/arm/mach-virt/platsmp.c
@@ -0,0 +1,76 @@
+/*
+ * Dummy Virtual Machine - does what it says on the tin.
+ *
+ * Copyright (C) 2012 ARM Ltd
+ * Author: Will Deacon <will.deacon@arm.com>
+ *
+ * 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.
+ *
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/init.h>
+#include <linux/smp.h>
+#include <linux/of.h>
+
+#include <asm/psci.h>
+#include <asm/smp_plat.h>
+#include <asm/hardware/gic.h>
+
+extern void secondary_startup(void);
+
+/*
+ * Enumerate the possible CPU set from the device tree.
+ */
+static void __init virt_smp_init_cpus(void)
+{
+	struct device_node *dn = NULL;
+	int cpu = 0;
+
+	while ((dn = of_find_node_by_type(dn, "cpu"))) {
+		if (cpu < NR_CPUS)
+			set_cpu_possible(cpu, true);
+		cpu++;
+	}
+
+	/* sanity check */
+	if (cpu > NR_CPUS)
+		pr_warning("no. of cores (%d) greater than configured maximum "
+			   "of %d - clipping\n",
+			   cpu, NR_CPUS);
+
+	set_smp_cross_call(gic_raise_softirq);
+}
+
+static void __init virt_smp_prepare_cpus(unsigned int max_cpus)
+{
+}
+
+static int __cpuinit virt_boot_secondary(unsigned int cpu,
+					 struct task_struct *idle)
+{
+	if (psci_ops.cpu_on)
+		return psci_ops.cpu_on(cpu_logical_map(cpu),
+				       __pa(secondary_startup));
+	return -ENODEV;
+}
+
+static void __cpuinit virt_secondary_init(unsigned int cpu)
+{
+	gic_secondary_init(0);
+}
+
+struct smp_operations __initdata virt_smp_ops = {
+	.smp_init_cpus		= virt_smp_init_cpus,
+	.smp_prepare_cpus	= virt_smp_prepare_cpus,
+	.smp_secondary_init	= virt_secondary_init,
+	.smp_boot_secondary	= virt_boot_secondary,
+};
diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
index 174b9da..d764835 100644
--- a/arch/arm/mach-virt/virt.c
+++ b/arch/arm/mach-virt/virt.c
@@ -20,6 +20,7 @@
 
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
+#include <linux/smp.h>
 
 #include <asm/arch_timer.h>
 #include <asm/hardware/gic.h>
@@ -56,10 +57,15 @@ static struct sys_timer virt_timer = {
 	.init = virt_timer_init,
 };
 
+#ifdef CONFIG_SMP
+extern struct smp_operations virt_smp_ops;
+#endif
+
 DT_MACHINE_START(VIRT, "Dummy Virtual Machine")
 	.init_irq	= gic_init_irq,
 	.handle_irq     = gic_handle_irq,
 	.timer		= &virt_timer,
 	.init_machine	= virt_init,
+	.smp		= smp_ops(virt_smp_ops),
 	.dt_compat	= virt_dt_match,
 MACHINE_END
-- 
1.8.0

[PATCH v2 3/6] ARM: psci: add devicetree binding for describing PSCI firmware

[Arnd Bergmann]

On Monday 17 December 2012, Will Deacon wrote:
> +
> + - function-base : The base ID from which the functions are offset.
> +
> +Main node optional properties:
> +
> + - cpu_suspend   : Offset of CPU_SUSPEND ID from function-base
> +
> + - cpu_off       : Offset of CPU_OFF ID from function-base
> +
> + - cpu_on        : Offset of CPU_ON ID from function-base
> +
> + - migrate       : Offset of MIGRATE ID from function-base

What is the benefit of the "function-base" property over just having
32 bit IDs for each function. For all I can tell, the interface does
not rely on the numbers to be consecutive, so removing the function-base
attribute would make the binding simpler as well as more flexible.

	Arnd

[PATCH v2 4/6] ARM: psci: add support for PSCI invocations from the kernel

[Nicolas Pitre]

On Mon, 17 Dec 2012, Will Deacon wrote:

> This patch adds support for the Power State Coordination Interface
> defined by ARM, allowing Linux to request CPU-centric power-management
> operations from firmware implementing the PSCI protocol.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>

[...]

> +/*
> + * The following two functions are invoked via the invoke_psci_fn pointer
> + * and will not be inlined, allowing us to piggyback on the AAPCS.
> + */

To make sure the code is always in sync with the intent, you could mark 
those with  noinline as well.

> +static int __invoke_psci_fn_hvc(u32 function_id, u32 arg0, u32 arg1, u32 arg2)
> +{
> +	asm volatile(
> +			__asmeq("%0", "r0")
> +			__asmeq("%1", "r1")
> +			__asmeq("%2", "r2")
> +			__asmeq("%3", "r3")
> +			__HVC(0)
> +		: "+r" (function_id)
> +		: "r" (arg0), "r" (arg1), "r" (arg2));
> +
> +	return function_id;
> +}
> +
> +static int __invoke_psci_fn_smc(u32 function_id, u32 arg0, u32 arg1, u32 arg2)
> +{
> +	asm volatile(
> +			__asmeq("%0", "r0")
> +			__asmeq("%1", "r1")
> +			__asmeq("%2", "r2")
> +			__asmeq("%3", "r3")
> +			__SMC(0)
> +		: "+r" (function_id)
> +		: "r" (arg0), "r" (arg1), "r" (arg2));
> +
> +	return function_id;
> +}
> +
> +static int psci_cpu_suspend(struct psci_power_state state,
> +			    unsigned long entry_point)
> +{
> +	int err;
> +	u32 fn, power_state;
> +
> +	fn = psci_function_id[PSCI_FN_CPU_SUSPEND];
> +	power_state = psci_power_state_pack(state);
> +	err = invoke_psci_fn(fn, power_state, (u32)entry_point, 0);

Why do you need the u32 cast here?

> +	return psci_to_linux_errno(err);
> +}
> +
> +static int psci_cpu_off(struct psci_power_state state)
> +{
> +	int err;
> +	u32 fn, power_state;
> +
> +	fn = psci_function_id[PSCI_FN_CPU_OFF];
> +	power_state = psci_power_state_pack(state);
> +	err = invoke_psci_fn(fn, power_state, 0, 0);
> +	return psci_to_linux_errno(err);
> +}
> +
> +static int psci_cpu_on(unsigned long cpuid, unsigned long entry_point)
> +{
> +	int err;
> +	u32 fn;
> +
> +	fn = psci_function_id[PSCI_FN_CPU_ON];
> +	err = invoke_psci_fn(fn, cpuid, entry_point, 0);
> +	return psci_to_linux_errno(err);
> +}
> +
> +static int psci_migrate(unsigned long cpuid)
> +{
> +	int err;
> +	u32 fn;
> +
> +	fn = psci_function_id[PSCI_FN_MIGRATE];
> +	err = invoke_psci_fn(fn, cpuid, 0, 0);
> +	return psci_to_linux_errno(err);
> +}
> +
> +static const struct of_device_id psci_of_match[] __initconst = {
> +	{ .compatible = "arm,psci",	},
> +	{},
> +};
> +
> +static int __init psci_init(void)
> +{
> +	struct device_node *np;
> +	const char *method;
> +	u32 base, id;
> +
> +	np = of_find_matching_node(NULL, psci_of_match);
> +	if (!np)
> +		return 0;
> +
> +	pr_info("probing function IDs from device-tree\n");

Having "probing function IDs from device-tree" in the middle of a kernel 
log isn't very informative.  Better make this more useful or remove it.

> +
> +	if (of_property_read_u32(np, "function-base", &base)) {
> +		pr_warning("missing \"function-base\" property\n");

Same thing here: this lacks context in a kernel log.
And so on for the other occurrences.


Nicolas

[PATCH v2 6/6] ARM: mach-virt: add SMP support using PSCI

[Nicolas Pitre]

On Mon, 17 Dec 2012, Will Deacon wrote:

> This patch adds support for SMP to mach-virt using the PSCI
> infrastructure.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/mach-virt/Kconfig   |  1 +
>  arch/arm/mach-virt/Makefile  |  1 +
>  arch/arm/mach-virt/platsmp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-virt/virt.c    |  6 ++++
>  4 files changed, 84 insertions(+)
>  create mode 100644 arch/arm/mach-virt/platsmp.c
> 
> diff --git a/arch/arm/mach-virt/Kconfig b/arch/arm/mach-virt/Kconfig
> index a568a2a..8958f0d 100644
> --- a/arch/arm/mach-virt/Kconfig
> +++ b/arch/arm/mach-virt/Kconfig
> @@ -3,6 +3,7 @@ config ARCH_VIRT
>  	select ARCH_WANT_OPTIONAL_GPIOLIB
>  	select ARM_GIC
>  	select ARM_ARCH_TIMER
> +	select ARM_PSCI
>  	select HAVE_SMP
>  	select CPU_V7
>  	select SPARSE_IRQ
> diff --git a/arch/arm/mach-virt/Makefile b/arch/arm/mach-virt/Makefile
> index 7ddbfa6..042afc1 100644
> --- a/arch/arm/mach-virt/Makefile
> +++ b/arch/arm/mach-virt/Makefile
> @@ -3,3 +3,4 @@
>  #
>  
>  obj-y					:= virt.o
> +obj-$(CONFIG_SMP)			+= platsmp.o
> diff --git a/arch/arm/mach-virt/platsmp.c b/arch/arm/mach-virt/platsmp.c
> new file mode 100644
> index 0000000..930362b
> --- /dev/null
> +++ b/arch/arm/mach-virt/platsmp.c
> @@ -0,0 +1,76 @@
> +/*
> + * Dummy Virtual Machine - does what it says on the tin.
> + *
> + * Copyright (C) 2012 ARM Ltd
> + * Author: Will Deacon <will.deacon@arm.com>
> + *
> + * 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.
> + *
> + * 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, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/smp.h>
> +#include <linux/of.h>
> +
> +#include <asm/psci.h>
> +#include <asm/smp_plat.h>
> +#include <asm/hardware/gic.h>
> +
> +extern void secondary_startup(void);
> +
> +/*
> + * Enumerate the possible CPU set from the device tree.
> + */
> +static void __init virt_smp_init_cpus(void)
> +{
> +	struct device_node *dn = NULL;
> +	int cpu = 0;
> +
> +	while ((dn = of_find_node_by_type(dn, "cpu"))) {
> +		if (cpu < NR_CPUS)
> +			set_cpu_possible(cpu, true);
> +		cpu++;
> +	}
> +
> +	/* sanity check */
> +	if (cpu > NR_CPUS)
> +		pr_warning("no. of cores (%d) greater than configured maximum "
> +			   "of %d - clipping\n",
> +			   cpu, NR_CPUS);

Since commit 5587164eea you shouldn't need any of the above.

> +	set_smp_cross_call(gic_raise_softirq);
> +}
> +
> +static void __init virt_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +}
> +
> +static int __cpuinit virt_boot_secondary(unsigned int cpu,
> +					 struct task_struct *idle)
> +{
> +	if (psci_ops.cpu_on)
> +		return psci_ops.cpu_on(cpu_logical_map(cpu),
> +				       __pa(secondary_startup));
> +	return -ENODEV;
> +}
> +
> +static void __cpuinit virt_secondary_init(unsigned int cpu)
> +{
> +	gic_secondary_init(0);
> +}
> +
> +struct smp_operations __initdata virt_smp_ops = {
> +	.smp_init_cpus		= virt_smp_init_cpus,
> +	.smp_prepare_cpus	= virt_smp_prepare_cpus,
> +	.smp_secondary_init	= virt_secondary_init,
> +	.smp_boot_secondary	= virt_boot_secondary,
> +};
> diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
> index 174b9da..d764835 100644
> --- a/arch/arm/mach-virt/virt.c
> +++ b/arch/arm/mach-virt/virt.c
> @@ -20,6 +20,7 @@
>  
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
> +#include <linux/smp.h>
>  
>  #include <asm/arch_timer.h>
>  #include <asm/hardware/gic.h>
> @@ -56,10 +57,15 @@ static struct sys_timer virt_timer = {
>  	.init = virt_timer_init,
>  };
>  
> +#ifdef CONFIG_SMP
> +extern struct smp_operations virt_smp_ops;
> +#endif

You don't need to surround prototype declaration here, unless your goal 
was to define a dummy virt_smp_ops when CONFIG_SMP is not selected?
Otherwise the reference below would break compilation.

> +
>  DT_MACHINE_START(VIRT, "Dummy Virtual Machine")
>  	.init_irq	= gic_init_irq,
>  	.handle_irq     = gic_handle_irq,
>  	.timer		= &virt_timer,
>  	.init_machine	= virt_init,
> +	.smp		= smp_ops(virt_smp_ops),
>  	.dt_compat	= virt_dt_match,
>  MACHINE_END
> -- 
> 1.8.0
> 

[PATCH v2 3/6] ARM: psci: add devicetree binding for describing PSCI firmware

[Will Deacon]

Hi Arnd,

On Mon, Dec 17, 2012 at 08:00:11PM +0000, Arnd Bergmann wrote:
> On Monday 17 December 2012, Will Deacon wrote:
> > +
> > + - function-base : The base ID from which the functions are offset.
> > +
> > +Main node optional properties:
> > +
> > + - cpu_suspend   : Offset of CPU_SUSPEND ID from function-base
> > +
> > + - cpu_off       : Offset of CPU_OFF ID from function-base
> > +
> > + - cpu_on        : Offset of CPU_ON ID from function-base
> > +
> > + - migrate       : Offset of MIGRATE ID from function-base
> 
> What is the benefit of the "function-base" property over just having
> 32 bit IDs for each function. For all I can tell, the interface does
> not rely on the numbers to be consecutive, so removing the function-base
> attribute would make the binding simpler as well as more flexible.

Sure, happy to change that for v3.

Will

[PATCH v2 4/6] ARM: psci: add support for PSCI invocations from the kernel

[Will Deacon]

On Mon, Dec 17, 2012 at 08:51:27PM +0000, Nicolas Pitre wrote:
> On Mon, 17 Dec 2012, Will Deacon wrote:
> 
> > This patch adds support for the Power State Coordination Interface
> > defined by ARM, allowing Linux to request CPU-centric power-management
> > operations from firmware implementing the PSCI protocol.
> > 
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> [...]
> 
> > +/*
> > + * The following two functions are invoked via the invoke_psci_fn pointer
> > + * and will not be inlined, allowing us to piggyback on the AAPCS.
> > + */
> 
> To make sure the code is always in sync with the intent, you could mark 
> those with  noinline as well.

Can do.

> > +static int __invoke_psci_fn_hvc(u32 function_id, u32 arg0, u32 arg1, u32 arg2)
> > +{
> > +	asm volatile(
> > +			__asmeq("%0", "r0")
> > +			__asmeq("%1", "r1")
> > +			__asmeq("%2", "r2")
> > +			__asmeq("%3", "r3")
> > +			__HVC(0)
> > +		: "+r" (function_id)
> > +		: "r" (arg0), "r" (arg1), "r" (arg2));
> > +
> > +	return function_id;
> > +}
> > +
> > +static int __invoke_psci_fn_smc(u32 function_id, u32 arg0, u32 arg1, u32 arg2)
> > +{
> > +	asm volatile(
> > +			__asmeq("%0", "r0")
> > +			__asmeq("%1", "r1")
> > +			__asmeq("%2", "r2")
> > +			__asmeq("%3", "r3")
> > +			__SMC(0)
> > +		: "+r" (function_id)
> > +		: "r" (arg0), "r" (arg1), "r" (arg2));
> > +
> > +	return function_id;
> > +}
> > +
> > +static int psci_cpu_suspend(struct psci_power_state state,
> > +			    unsigned long entry_point)
> > +{
> > +	int err;
> > +	u32 fn, power_state;
> > +
> > +	fn = psci_function_id[PSCI_FN_CPU_SUSPEND];
> > +	power_state = psci_power_state_pack(state);
> > +	err = invoke_psci_fn(fn, power_state, (u32)entry_point, 0);
> 
> Why do you need the u32 cast here?

That's a hangover from when entry_point was a void *. I'll fix that, thanks.

> > +static int __init psci_init(void)
> > +{
> > +	struct device_node *np;
> > +	const char *method;
> > +	u32 base, id;
> > +
> > +	np = of_find_matching_node(NULL, psci_of_match);
> > +	if (!np)
> > +		return 0;
> > +
> > +	pr_info("probing function IDs from device-tree\n");
> 
> Having "probing function IDs from device-tree" in the middle of a kernel 
> log isn't very informative.  Better make this more useful or remove it.
> 
> > +
> > +	if (of_property_read_u32(np, "function-base", &base)) {
> > +		pr_warning("missing \"function-base\" property\n");
> 
> Same thing here: this lacks context in a kernel log.
> And so on for the other occurrences.

Actually, these are all prefixed with "psci: " thanks to the pr_fmt
definition at the top of the file. I can remove them if you like, but then
it's not obvious which parts of the PSCI code are available from looking at
a kernel boot log.

Cheers for the review,

Will

[PATCH v2 6/6] ARM: mach-virt: add SMP support using PSCI

[Will Deacon]

On Mon, Dec 17, 2012 at 09:45:52PM +0000, Nicolas Pitre wrote:
> On Mon, 17 Dec 2012, Will Deacon wrote:
> > This patch adds support for SMP to mach-virt using the PSCI
> > infrastructure.
> > 
> > Signed-off-by: Will Deacon <will.deacon@arm.com>

[...]

> > +/*
> > + * Enumerate the possible CPU set from the device tree.
> > + */
> > +static void __init virt_smp_init_cpus(void)
> > +{
> > +	struct device_node *dn = NULL;
> > +	int cpu = 0;
> > +
> > +	while ((dn = of_find_node_by_type(dn, "cpu"))) {
> > +		if (cpu < NR_CPUS)
> > +			set_cpu_possible(cpu, true);
> > +		cpu++;
> > +	}
> > +
> > +	/* sanity check */
> > +	if (cpu > NR_CPUS)
> > +		pr_warning("no. of cores (%d) greater than configured maximum "
> > +			   "of %d - clipping\n",
> > +			   cpu, NR_CPUS);
> 
> Since commit 5587164eea you shouldn't need any of the above.

There's going to be nothing left at this rate! Thanks.

> > +#ifdef CONFIG_SMP
> > +extern struct smp_operations virt_smp_ops;
> > +#endif
> 
> You don't need to surround prototype declaration here, unless your goal 
> was to define a dummy virt_smp_ops when CONFIG_SMP is not selected?
> Otherwise the reference below would break compilation.

Right you are, the smp_ops macro does the magic for us. I'll put together a
v3.

Cheers,

Will

> > +
> >  DT_MACHINE_START(VIRT, "Dummy Virtual Machine")
> >  	.init_irq	= gic_init_irq,
> >  	.handle_irq     = gic_handle_irq,
> >  	.timer		= &virt_timer,
> >  	.init_machine	= virt_init,
> > +	.smp		= smp_ops(virt_smp_ops),
> >  	.dt_compat	= virt_dt_match,
> >  MACHINE_END
> > -- 
> > 1.8.0
> > 
> 

[Xen-devel] [PATCH v2 5/6] ARM: Dummy Virtual Machine platform support

[Stefano Stabellini]

On Mon, 17 Dec 2012, Will Deacon wrote:
> From: Marc Zyngier <marc.zyngier@arm.com>
> 
> Add support for the smallest, dumbest possible platform, to be
> used as a guest for KVM or other hypervisors.
> 
> It only mandates a GIC and architected timers. Fits nicely with
> a multiplatform zImage. Uses very little silicon area.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/Kconfig            |  2 ++
>  arch/arm/Makefile           |  1 +
>  arch/arm/mach-virt/Kconfig  |  9 +++++++
>  arch/arm/mach-virt/Makefile |  5 ++++
>  arch/arm/mach-virt/virt.c   | 65 +++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 82 insertions(+)
>  create mode 100644 arch/arm/mach-virt/Kconfig
>  create mode 100644 arch/arm/mach-virt/Makefile
>  create mode 100644 arch/arm/mach-virt/virt.c

Should it come along with a DTS?


> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 2a04375..552d72e0 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1127,6 +1127,8 @@ source "arch/arm/mach-versatile/Kconfig"
>  source "arch/arm/mach-vexpress/Kconfig"
>  source "arch/arm/plat-versatile/Kconfig"
>  
> +source "arch/arm/mach-virt/Kconfig"
> +
>  source "arch/arm/mach-w90x900/Kconfig"
>  
>  # Definitions to make life easier
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 5f914fc..e8232ad 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -192,6 +192,7 @@ machine-$(CONFIG_ARCH_SOCFPGA)		+= socfpga
>  machine-$(CONFIG_ARCH_SPEAR13XX)	+= spear13xx
>  machine-$(CONFIG_ARCH_SPEAR3XX)		+= spear3xx
>  machine-$(CONFIG_MACH_SPEAR600)		+= spear6xx
> +machine-$(CONFIG_ARCH_VIRT)		+= virt
>  machine-$(CONFIG_ARCH_ZYNQ)		+= zynq
>  
>  # Platform directory name.  This list is sorted alphanumerically
> diff --git a/arch/arm/mach-virt/Kconfig b/arch/arm/mach-virt/Kconfig
> new file mode 100644
> index 0000000..a568a2a
> --- /dev/null
> +++ b/arch/arm/mach-virt/Kconfig
> @@ -0,0 +1,9 @@
> +config ARCH_VIRT
> +	bool "Dummy Virtual Machine" if ARCH_MULTI_V7
> +	select ARCH_WANT_OPTIONAL_GPIOLIB
> +	select ARM_GIC
> +	select ARM_ARCH_TIMER
> +	select HAVE_SMP
> +	select CPU_V7
> +	select SPARSE_IRQ
> +	select USE_OF
> diff --git a/arch/arm/mach-virt/Makefile b/arch/arm/mach-virt/Makefile
> new file mode 100644
> index 0000000..7ddbfa6
> --- /dev/null
> +++ b/arch/arm/mach-virt/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for the linux kernel.
> +#
> +
> +obj-y					:= virt.o
> diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
> new file mode 100644
> index 0000000..174b9da
> --- /dev/null
> +++ b/arch/arm/mach-virt/virt.c
> @@ -0,0 +1,65 @@
> +/*
> + * Dummy Virtual Machine - does what it says on the tin.
> + *
> + * Copyright (C) 2012 ARM Ltd
> + * Authors: Will Deacon <will.deacon@arm.com>,
> + *          Marc Zyngier <marc.zyngier@arm.com>
> + *
> + * 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.
> + *
> + * 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, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +
> +#include <asm/arch_timer.h>
> +#include <asm/hardware/gic.h>
> +#include <asm/mach/arch.h>
> +#include <asm/mach/time.h>
> +
> +const static struct of_device_id irq_match[] = {
> +	{ .compatible = "arm,cortex-a15-gic", .data = gic_of_init, },
> +	{}
> +};
> +
> +static void __init gic_init_irq(void)
> +{
> +	of_irq_init(irq_match);
> +}
> +
> +static void __init virt_init(void)
> +{
> +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> +}
> +
> +static void __init virt_timer_init(void)
> +{
> +	WARN_ON(arch_timer_of_register() != 0);
> +	WARN_ON(arch_timer_sched_clock_init() != 0);
> +}
> +
> +static const char *virt_dt_match[] = {
> +	"linux,dummy-virt",
> +	NULL
> +};
> +
> +static struct sys_timer virt_timer = {
> +	.init = virt_timer_init,
> +};
> +
> +DT_MACHINE_START(VIRT, "Dummy Virtual Machine")
> +	.init_irq	= gic_init_irq,
> +	.handle_irq     = gic_handle_irq,
> +	.timer		= &virt_timer,
> +	.init_machine	= virt_init,
> +	.dt_compat	= virt_dt_match,
> +MACHINE_END
> -- 
> 1.8.0
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel at lists.xen.org
> http://lists.xen.org/xen-devel
> 

[Xen-devel] [PATCH v2 6/6] ARM: mach-virt: add SMP support using PSCI

[Stefano Stabellini]

On Mon, 17 Dec 2012, Will Deacon wrote:
> This patch adds support for SMP to mach-virt using the PSCI
> infrastructure.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
>
>  arch/arm/mach-virt/Kconfig   |  1 +
>  arch/arm/mach-virt/Makefile  |  1 +
>  arch/arm/mach-virt/platsmp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-virt/virt.c    |  6 ++++
>  4 files changed, 84 insertions(+)
>  create mode 100644 arch/arm/mach-virt/platsmp.c
> 
> diff --git a/arch/arm/mach-virt/Kconfig b/arch/arm/mach-virt/Kconfig
> index a568a2a..8958f0d 100644
> --- a/arch/arm/mach-virt/Kconfig
> +++ b/arch/arm/mach-virt/Kconfig
> @@ -3,6 +3,7 @@ config ARCH_VIRT
>  	select ARCH_WANT_OPTIONAL_GPIOLIB
>  	select ARM_GIC
>  	select ARM_ARCH_TIMER
> +	select ARM_PSCI
>  	select HAVE_SMP
>  	select CPU_V7
>  	select SPARSE_IRQ

Considering that PSCI is actually needed only to boot secondary cpus,
maybe we want to select it if CONFIG_SMP is enabled?


> diff --git a/arch/arm/mach-virt/Makefile b/arch/arm/mach-virt/Makefile
> index 7ddbfa6..042afc1 100644
> --- a/arch/arm/mach-virt/Makefile
> +++ b/arch/arm/mach-virt/Makefile
> @@ -3,3 +3,4 @@
>  #
>  
>  obj-y					:= virt.o
> +obj-$(CONFIG_SMP)			+= platsmp.o
> diff --git a/arch/arm/mach-virt/platsmp.c b/arch/arm/mach-virt/platsmp.c
> new file mode 100644
> index 0000000..930362b
> --- /dev/null
> +++ b/arch/arm/mach-virt/platsmp.c
> @@ -0,0 +1,76 @@
> +/*
> + * Dummy Virtual Machine - does what it says on the tin.
> + *
> + * Copyright (C) 2012 ARM Ltd
> + * Author: Will Deacon <will.deacon@arm.com>
> + *
> + * 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.
> + *
> + * 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, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/smp.h>
> +#include <linux/of.h>
> +
> +#include <asm/psci.h>
> +#include <asm/smp_plat.h>
> +#include <asm/hardware/gic.h>
> +
> +extern void secondary_startup(void);
> +
> +/*
> + * Enumerate the possible CPU set from the device tree.
> + */
> +static void __init virt_smp_init_cpus(void)
> +{
> +	struct device_node *dn = NULL;
> +	int cpu = 0;
> +
> +	while ((dn = of_find_node_by_type(dn, "cpu"))) {
> +		if (cpu < NR_CPUS)
> +			set_cpu_possible(cpu, true);
> +		cpu++;
> +	}
> +
> +	/* sanity check */
> +	if (cpu > NR_CPUS)
> +		pr_warning("no. of cores (%d) greater than configured maximum "
> +			   "of %d - clipping\n",
> +			   cpu, NR_CPUS);
> +
> +	set_smp_cross_call(gic_raise_softirq);
> +}
> +
> +static void __init virt_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +}
> +
> +static int __cpuinit virt_boot_secondary(unsigned int cpu,
> +					 struct task_struct *idle)
> +{
> +	if (psci_ops.cpu_on)
> +		return psci_ops.cpu_on(cpu_logical_map(cpu),
> +				       __pa(secondary_startup));
> +	return -ENODEV;
> +}

Isn't there a better way to check whether PSCI is actually "enabled", as
in present in the device tree and initialized correctly?

Maybe we need a pcsi_enabled() static inline of some sort?


> +static void __cpuinit virt_secondary_init(unsigned int cpu)
> +{
> +	gic_secondary_init(0);
> +}
> +
> +struct smp_operations __initdata virt_smp_ops = {
> +	.smp_init_cpus		= virt_smp_init_cpus,
> +	.smp_prepare_cpus	= virt_smp_prepare_cpus,
> +	.smp_secondary_init	= virt_secondary_init,
> +	.smp_boot_secondary	= virt_boot_secondary,
> +};
> diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
> index 174b9da..d764835 100644
> --- a/arch/arm/mach-virt/virt.c
> +++ b/arch/arm/mach-virt/virt.c
> @@ -20,6 +20,7 @@
>  
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
> +#include <linux/smp.h>
>  
>  #include <asm/arch_timer.h>
>  #include <asm/hardware/gic.h>
> @@ -56,10 +57,15 @@ static struct sys_timer virt_timer = {
>  	.init = virt_timer_init,
>  };
>  
> +#ifdef CONFIG_SMP
> +extern struct smp_operations virt_smp_ops;
> +#endif
> +
>  DT_MACHINE_START(VIRT, "Dummy Virtual Machine")
>  	.init_irq	= gic_init_irq,
>  	.handle_irq     = gic_handle_irq,
>  	.timer		= &virt_timer,
>  	.init_machine	= virt_init,
> +	.smp		= smp_ops(virt_smp_ops),
>  	.dt_compat	= virt_dt_match,
>  MACHINE_END
> -- 
> 1.8.0

[Xen-devel] [PATCH v2 6/6] ARM: mach-virt: add SMP support using PSCI

[Marc Zyngier]

On 18/12/12 12:19, Stefano Stabellini wrote:
> On Mon, 17 Dec 2012, Will Deacon wrote:
>> This patch adds support for SMP to mach-virt using the PSCI
>> infrastructure.
>>
>> Signed-off-by: Will Deacon <will.deacon@arm.com>
>>
>>  arch/arm/mach-virt/Kconfig   |  1 +
>>  arch/arm/mach-virt/Makefile  |  1 +
>>  arch/arm/mach-virt/platsmp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++
>>  arch/arm/mach-virt/virt.c    |  6 ++++
>>  4 files changed, 84 insertions(+)
>>  create mode 100644 arch/arm/mach-virt/platsmp.c
>>
>> diff --git a/arch/arm/mach-virt/Kconfig b/arch/arm/mach-virt/Kconfig
>> index a568a2a..8958f0d 100644
>> --- a/arch/arm/mach-virt/Kconfig
>> +++ b/arch/arm/mach-virt/Kconfig
>> @@ -3,6 +3,7 @@ config ARCH_VIRT
>>  	select ARCH_WANT_OPTIONAL_GPIOLIB
>>  	select ARM_GIC
>>  	select ARM_ARCH_TIMER
>> +	select ARM_PSCI
>>  	select HAVE_SMP
>>  	select CPU_V7
>>  	select SPARSE_IRQ
> 
> Considering that PSCI is actually needed only to boot secondary cpus,
> maybe we want to select it if CONFIG_SMP is enabled?

Well, I was considering using it to "power-off" the VM when the last CPU
powers itself off, and this would apply to UP as well.

>> diff --git a/arch/arm/mach-virt/Makefile b/arch/arm/mach-virt/Makefile
>> index 7ddbfa6..042afc1 100644
>> --- a/arch/arm/mach-virt/Makefile
>> +++ b/arch/arm/mach-virt/Makefile
>> @@ -3,3 +3,4 @@
>>  #
>>  
>>  obj-y					:= virt.o
>> +obj-$(CONFIG_SMP)			+= platsmp.o
>> diff --git a/arch/arm/mach-virt/platsmp.c b/arch/arm/mach-virt/platsmp.c
>> new file mode 100644
>> index 0000000..930362b
>> --- /dev/null
>> +++ b/arch/arm/mach-virt/platsmp.c
>> @@ -0,0 +1,76 @@
>> +/*
>> + * Dummy Virtual Machine - does what it says on the tin.
>> + *
>> + * Copyright (C) 2012 ARM Ltd
>> + * Author: Will Deacon <will.deacon@arm.com>
>> + *
>> + * 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.
>> + *
>> + * 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, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/smp.h>
>> +#include <linux/of.h>
>> +
>> +#include <asm/psci.h>
>> +#include <asm/smp_plat.h>
>> +#include <asm/hardware/gic.h>
>> +
>> +extern void secondary_startup(void);
>> +
>> +/*
>> + * Enumerate the possible CPU set from the device tree.
>> + */
>> +static void __init virt_smp_init_cpus(void)
>> +{
>> +	struct device_node *dn = NULL;
>> +	int cpu = 0;
>> +
>> +	while ((dn = of_find_node_by_type(dn, "cpu"))) {
>> +		if (cpu < NR_CPUS)
>> +			set_cpu_possible(cpu, true);
>> +		cpu++;
>> +	}
>> +
>> +	/* sanity check */
>> +	if (cpu > NR_CPUS)
>> +		pr_warning("no. of cores (%d) greater than configured maximum "
>> +			   "of %d - clipping\n",
>> +			   cpu, NR_CPUS);
>> +
>> +	set_smp_cross_call(gic_raise_softirq);
>> +}
>> +
>> +static void __init virt_smp_prepare_cpus(unsigned int max_cpus)
>> +{
>> +}
>> +
>> +static int __cpuinit virt_boot_secondary(unsigned int cpu,
>> +					 struct task_struct *idle)
>> +{
>> +	if (psci_ops.cpu_on)
>> +		return psci_ops.cpu_on(cpu_logical_map(cpu),
>> +				       __pa(secondary_startup));
>> +	return -ENODEV;
>> +}
> 
> Isn't there a better way to check whether PSCI is actually "enabled", as
> in present in the device tree and initialized correctly?

All methods are optional, so I'm afraid you have to check for their
validity each time you want to access one.

> Maybe we need a pcsi_enabled() static inline of some sort?
> 
> 
>> +static void __cpuinit virt_secondary_init(unsigned int cpu)
>> +{
>> +	gic_secondary_init(0);
>> +}
>> +
>> +struct smp_operations __initdata virt_smp_ops = {
>> +	.smp_init_cpus		= virt_smp_init_cpus,
>> +	.smp_prepare_cpus	= virt_smp_prepare_cpus,
>> +	.smp_secondary_init	= virt_secondary_init,
>> +	.smp_boot_secondary	= virt_boot_secondary,
>> +};
>> diff --git a/arch/arm/mach-virt/virt.c b/arch/arm/mach-virt/virt.c
>> index 174b9da..d764835 100644
>> --- a/arch/arm/mach-virt/virt.c
>> +++ b/arch/arm/mach-virt/virt.c
>> @@ -20,6 +20,7 @@
>>  
>>  #include <linux/of_irq.h>
>>  #include <linux/of_platform.h>
>> +#include <linux/smp.h>
>>  
>>  #include <asm/arch_timer.h>
>>  #include <asm/hardware/gic.h>
>> @@ -56,10 +57,15 @@ static struct sys_timer virt_timer = {
>>  	.init = virt_timer_init,
>>  };
>>  
>> +#ifdef CONFIG_SMP
>> +extern struct smp_operations virt_smp_ops;
>> +#endif
>> +
>>  DT_MACHINE_START(VIRT, "Dummy Virtual Machine")
>>  	.init_irq	= gic_init_irq,
>>  	.handle_irq     = gic_handle_irq,
>>  	.timer		= &virt_timer,
>>  	.init_machine	= virt_init,
>> +	.smp		= smp_ops(virt_smp_ops),
>>  	.dt_compat	= virt_dt_match,
>>  MACHINE_END
>> -- 
>> 1.8.0
> 


-- 
Jazz is not dead. It just smells funny...

[Xen-devel] [PATCH v2 5/6] ARM: Dummy Virtual Machine platform support

[Will Deacon]

Hi Stefano,

On Tue, Dec 18, 2012 at 12:04:38PM +0000, Stefano Stabellini wrote:
> On Mon, 17 Dec 2012, Will Deacon wrote:
> > From: Marc Zyngier <marc.zyngier@arm.com>
> > 
> > Add support for the smallest, dumbest possible platform, to be
> > used as a guest for KVM or other hypervisors.
> > 
> > It only mandates a GIC and architected timers. Fits nicely with
> > a multiplatform zImage. Uses very little silicon area.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm/Kconfig            |  2 ++
> >  arch/arm/Makefile           |  1 +
> >  arch/arm/mach-virt/Kconfig  |  9 +++++++
> >  arch/arm/mach-virt/Makefile |  5 ++++
> >  arch/arm/mach-virt/virt.c   | 65 +++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 82 insertions(+)
> >  create mode 100644 arch/arm/mach-virt/Kconfig
> >  create mode 100644 arch/arm/mach-virt/Makefile
> >  create mode 100644 arch/arm/mach-virt/virt.c
> 
> Should it come along with a DTS?

The only things the platform needs are GIC, timers, memory and a CPU.
Furthermore, the location, size, frequency etc properties of these aren't
fixed, so a dts would be fairly useless because it will probably not match
the particular mach-virt instance you're targetting.

For kvmtool, I've been generating the device-tree at runtime based on how
kvmtool is invoked and it's been working pretty well so far.

Will

[Xen-devel] [PATCH v2 5/6] ARM: Dummy Virtual Machine platform support

[Stefano Stabellini]

On Tue, 18 Dec 2012, Will Deacon wrote:
> Hi Stefano,
> 
> On Tue, Dec 18, 2012 at 12:04:38PM +0000, Stefano Stabellini wrote:
> > On Mon, 17 Dec 2012, Will Deacon wrote:
> > > From: Marc Zyngier <marc.zyngier@arm.com>
> > > 
> > > Add support for the smallest, dumbest possible platform, to be
> > > used as a guest for KVM or other hypervisors.
> > > 
> > > It only mandates a GIC and architected timers. Fits nicely with
> > > a multiplatform zImage. Uses very little silicon area.
> > > 
> > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > ---
> > >  arch/arm/Kconfig            |  2 ++
> > >  arch/arm/Makefile           |  1 +
> > >  arch/arm/mach-virt/Kconfig  |  9 +++++++
> > >  arch/arm/mach-virt/Makefile |  5 ++++
> > >  arch/arm/mach-virt/virt.c   | 65 +++++++++++++++++++++++++++++++++++++++++++++
> > >  5 files changed, 82 insertions(+)
> > >  create mode 100644 arch/arm/mach-virt/Kconfig
> > >  create mode 100644 arch/arm/mach-virt/Makefile
> > >  create mode 100644 arch/arm/mach-virt/virt.c
> > 
> > Should it come along with a DTS?
> 
> The only things the platform needs are GIC, timers, memory and a CPU.
> Furthermore, the location, size, frequency etc properties of these aren't
> fixed, so a dts would be fairly useless because it will probably not match
> the particular mach-virt instance you're targetting.
> 
> For kvmtool, I've been generating the device-tree at runtime based on how
> kvmtool is invoked and it's been working pretty well so far.

I agree on the fact that it should be generated, but I personally think
that it would still be useful as an example.

[Xen-devel] [PATCH v2 5/6] ARM: Dummy Virtual Machine platform support

[Christopher Covington]

Hi Will,

On 12/18/2012 08:14 AM, Will Deacon wrote:
> Hi Stefano,
> 
> On Tue, Dec 18, 2012 at 12:04:38PM +0000, Stefano Stabellini wrote:
>> On Mon, 17 Dec 2012, Will Deacon wrote:
>>> From: Marc Zyngier <marc.zyngier@arm.com>
>>>
>>> Add support for the smallest, dumbest possible platform, to be
>>> used as a guest for KVM or other hypervisors.

[...]

>> Should it come along with a DTS?
> 
> The only things the platform needs are GIC, timers, memory and a CPU.

I assume multiple virtio-mmio peripherals are hiding behind what you seem to
advertising here as plain old memory?

> Furthermore, the location, size, frequency etc properties of these aren't
> fixed, so a dts would be fairly useless because it will probably not match
> the particular mach-virt instance you're targetting.

I disagree. I think an example DTS would be fairly useful, if only for the
full list of peripherals you're using on the platform.

> For kvmtool, I've been generating the device-tree at runtime based on how
> kvmtool is invoked and it's been working pretty well so far.

If you'd much prefer to post the command line, tools version, etc. that you're
using to generate the DTB, rather than the DTS, that'd be better than nothing.

It seems like Rob Herring's earlier question about whether the dummy platform
is really justified never got answered. I think sending a sample DTS out with
the patchset would help "highlight where we need to do more work on DT driving
the initialization."

Lastly, I'm somewhat curious, why virtio-mmio console rather than DCC?

Thanks,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
the Linux Foundation

[Xen-devel] [PATCH v2 5/6] ARM: Dummy Virtual Machine platform support

[Marc Zyngier]

Hi Christopher,

On 18/12/12 18:01, Christopher Covington wrote:
> Hi Will,
> 
> On 12/18/2012 08:14 AM, Will Deacon wrote:
>> Hi Stefano,
>>
>> On Tue, Dec 18, 2012 at 12:04:38PM +0000, Stefano Stabellini wrote:
>>> On Mon, 17 Dec 2012, Will Deacon wrote:
>>>> From: Marc Zyngier <marc.zyngier@arm.com>
>>>>
>>>> Add support for the smallest, dumbest possible platform, to be
>>>> used as a guest for KVM or other hypervisors.
> 
> [...]
> 
>>> Should it come along with a DTS?
>>
>> The only things the platform needs are GIC, timers, memory and a CPU.
> 
> I assume multiple virtio-mmio peripherals are hiding behind what you seem to
> advertising here as plain old memory?

No. Memory is memory. Virtio peripherals are created outside of the 
memory range. They end up having rings and descriptor in memory, but 
that's not any different from what you have with an fairly complicated 
DMA capable hardware device.

Furthermore, even if virtio-mmio is what we use with KVM, it could be 
something radically different. Xen uses something somewhat different.
It is not even required to boot the platform!

>> Furthermore, the location, size, frequency etc properties of these aren't
>> fixed, so a dts would be fairly useless because it will probably not match
>> the particular mach-virt instance you're targetting.
> 
> I disagree. I think an example DTS would be fairly useful, if only for the
> full list of peripherals you're using on the platform.

That's the whole point: we do not want to to specify anything, because 
there is no need to. You could have anything there, depending on your hypervisor.

>> For kvmtool, I've been generating the device-tree at runtime based on how
>> kvmtool is invoked and it's been working pretty well so far.
> 
> If you'd much prefer to post the command line, tools version, etc. that you're
> using to generate the DTB, rather than the DTS, that'd be better than nothing.

Here's what kvmtool has been seen to generate, with the parameters I used
a few minutes ago:

/dts-v1/;

/memreserve/	0x000000008fff0000 0x0000000000001000;
/ {
	interrupt-parent = <0x1>;
	compatible = "linux,dummy-virt";
	#address-cells = <0x2>;
	#size-cells = <0x2>;

	chosen {
		bootargs = "console=hvc0,38400 root=/dev/vda1";
	};

	memory {
		device_type = "memory";
		reg = <0x0 0x80000000 0x0 0x40000000>;
	};

	cpus {
		#address-cells = <0x1>;
		#size-cells = <0x0>;

		cpu at 0 {
			device_type = "cpu";
			compatible = "arm,cortex-a15";
			enable-method = "psci";
			reg = <0x0>;
		};

		cpu at 1 {
			device_type = "cpu";
			compatible = "arm,cortex-a15";
			enable-method = "psci";
			reg = <0x1>;
		};
	};

	intc {
		compatible = "arm,cortex-a15-gic";
		#interrupt-cells = <0x3>;
		interrupt-controller;
		reg = <0x0 0x3ffff000 0x0 0x1000 0x0 0x3fffd000 0x0 0x2000>;
		phandle = <0x1>;
	};

	timer {
		compatible = "arm,armv7-timer";
		interrupts = <0x1 0xd 0x301 0x1 0xe 0x301 0x1 0xb 0x301 0x1 0xa 0x301>;
	};

	virtio at 0 {
		compatible = "virtio,mmio";
		reg = <0x0 0x0 0x0 0x200>;
		interrupts = <0x0 0x0 0x1>;
	};

	virtio at 200 {
		compatible = "virtio,mmio";
		reg = <0x0 0x200 0x0 0x200>;
		interrupts = <0x0 0x1 0x1>;
	};

	virtio at 400 {
		compatible = "virtio,mmio";
		reg = <0x0 0x400 0x0 0x200>;
		interrupts = <0x0 0x2 0x1>;
	};

	virtio at 600 {
		compatible = "virtio,mmio";
		reg = <0x0 0x600 0x0 0x200>;
		interrupts = <0x0 0x3 0x1>;
	};

	psci {
		compatible = "arm,psci";
		method = "hvc";
		cpu_suspend = <0x95c1ba5e>;
		cpu_off = <0x95c1ba5f>;
		cpu_on = <0x95c1ba60>;
		migrate = <0x95c1ba61>;
	};
};

Does it help?

> It seems like Rob Herring's earlier question about whether the dummy platform
> is really justified never got answered. I think sending a sample DTS out with
> the patchset would help "highlight where we need to do more work on DT driving
> the initialization."
> 
> Lastly, I'm somewhat curious, why virtio-mmio console rather than DCC?

What would be the point of using DCC? We would have to trap on each access, and
then we'd have to invent yet another mechanism to channel the console to userspace.
Not to mention that I like to be able to actually input something on a console,
not just read from it.

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH v2 4/6] ARM: psci: add support for PSCI invocations from the kernel

[Nicolas Pitre]

On Tue, 18 Dec 2012, Will Deacon wrote:
> On Mon, Dec 17, 2012 at 08:51:27PM +0000, Nicolas Pitre wrote:
> > On Mon, 17 Dec 2012, Will Deacon wrote:
> > > +static int psci_cpu_suspend(struct psci_power_state state,
> > > +			    unsigned long entry_point)
> > > +{
> > > +	int err;
> > > +	u32 fn, power_state;
> > > +
> > > +	fn = psci_function_id[PSCI_FN_CPU_SUSPEND];
> > > +	power_state = psci_power_state_pack(state);
> > > +	err = invoke_psci_fn(fn, power_state, (u32)entry_point, 0);
> > 
> > Why do you need the u32 cast here?
> 
> That's a hangover from when entry_point was a void *. I'll fix that, thanks.

Hopefully you didn't pass virtual pointers to the PSCI call, did you?  :-)

> > > +static int __init psci_init(void)
> > > +{
> > > +	struct device_node *np;
> > > +	const char *method;
> > > +	u32 base, id;
> > > +
> > > +	np = of_find_matching_node(NULL, psci_of_match);
> > > +	if (!np)
> > > +		return 0;
> > > +
> > > +	pr_info("probing function IDs from device-tree\n");
> > 
> > Having "probing function IDs from device-tree" in the middle of a kernel 
> > log isn't very informative.  Better make this more useful or remove it.
> > 
> > > +
> > > +	if (of_property_read_u32(np, "function-base", &base)) {
> > > +		pr_warning("missing \"function-base\" property\n");
> > 
> > Same thing here: this lacks context in a kernel log.
> > And so on for the other occurrences.
> 
> Actually, these are all prefixed with "psci: " thanks to the pr_fmt
> definition at the top of the file.

Ah, goodie!  No more issue then.


Nicolas

[PATCH v2 4/6] ARM: psci: add support for PSCI invocations from the kernel

[Will Deacon]

On Tue, Dec 18, 2012 at 09:59:45PM +0000, Nicolas Pitre wrote:
> On Tue, 18 Dec 2012, Will Deacon wrote:
> > On Mon, Dec 17, 2012 at 08:51:27PM +0000, Nicolas Pitre wrote:
> > > On Mon, 17 Dec 2012, Will Deacon wrote:
> > > > +static int psci_cpu_suspend(struct psci_power_state state,
> > > > +			    unsigned long entry_point)
> > > > +{
> > > > +	int err;
> > > > +	u32 fn, power_state;
> > > > +
> > > > +	fn = psci_function_id[PSCI_FN_CPU_SUSPEND];
> > > > +	power_state = psci_power_state_pack(state);
> > > > +	err = invoke_psci_fn(fn, power_state, (u32)entry_point, 0);
> > > 
> > > Why do you need the u32 cast here?
> > 
> > That's a hangover from when entry_point was a void *. I'll fix that, thanks.
> 
> Hopefully you didn't pass virtual pointers to the PSCI call, did you?  :-)

...and I'd have gotten away with it if it wasn't for those meddling kids!

It was also made worse by Marc's code working first time too (after I blamed
the firmware like any sane kernel hacker would do :)

Will

[Xen-devel] [PATCH v2 5/6] ARM: Dummy Virtual Machine platform support

[Christopher Covington]

On 12/18/2012 01:18 PM, Marc Zyngier wrote:
> Hi Christopher,
> 
> On 18/12/12 18:01, Christopher Covington wrote:
>> Hi Will,
>>
>> On 12/18/2012 08:14 AM, Will Deacon wrote:
>>> Hi Stefano,
>>>

[...]

>>
>>> The only things the platform needs are GIC, timers, memory and a CPU.
>>
>> I assume multiple virtio-mmio peripherals are hiding behind what you seem to
>> be advertising here as plain old memory?
> 
> No. Memory is memory. Virtio peripherals are created outside of the 
> memory range. They end up having rings and descriptor in memory, but 
> that's not any different from what you have with an fairly complicated 
> DMA capable hardware device.

Sure, but I would consider such a device to be part of the platform (or
perhaps there's some better name to group together the set of devices that
are expected to modify memory?), and I was trying to fish for what additional
devices might be part of the platform on a regular basis, like what
console(s) and network interface(s).

> Here's what kvmtool has been seen to generate, with the parameters I used
> a few minutes ago:
>
> /dts-v1/;
> 
> /memreserve/	0x000000008fff0000 0x0000000000001000;
> / {
> 	interrupt-parent = <0x1>;
> 	compatible = "linux,dummy-virt";

Might it make sense to call this a generic ARM platform, using something
roughly in the direction of "linux,arm-generic" here and
s/mach-virt/mach-generic/ in the paths? Then any device-tree enabled ARMv7
platform using the generic timer and interrupt controller could reuse this
definition. This machine/platform seems like it could prove useful in
simulation and hardware scenarios. Would, for example, a fully-DT-enabled
Versatile Express machine converge on this definition? I wonder if it might
also be useful as a simple example with which to test out code sharing
between arch/arm and arch/arm64.

[...]
 
> Does it help?

Yes, thanks!

[...]
 
> What would be the point of using DCC?

It seemed like ARM-Ltd.-architected peripherals were picked for the timer and
interrupt controller, so I wondered why not for the console as well. As best
I'm aware, unless one ventures into the PrimeCell line with the PL011, DCC is
the closest match for an officially architected console mechanism.

> We would have to trap on each access...

Now that you point his out, this would indeed be fundamentally different than how
the coprocessor-register-accessed generic timer is handled, because the
virtualization extensions mean the hypervisor just needs to handle setup and
switching, but not intervention during normal operation.

This is drifting a bit off-topic to this particular patchset, but while I'm
on the topic of coprocessor register accesses in a virtualized environment,
is there a plan or are there existing mechanisms for handling the performance
counters? Do ID or cache or other register accesses need to be trapped?
Perhaps there are existing threads or portions of code on the topic that I've
overlooked.

> ...and then we'd have to invent yet another mechanism to channel the
> console to userspace.

What mechanism does virtio-mmio support in KVM use to channel the console to
userspace? Would there be a fundamental difference?

It seemed like Will was trying to frame the changes as _whether_ to support
various peripheral devices, because some of them aren't "needed" for an
artificial use case. I would rather frame the implementation decisions as
_which_ devices are going to be supported in the foreseeable future by your
work. Choosing a virtio-mmio console over DCC, semihosting, ram buffer, UART,
and whatever other alternatives there might be impacts someone trying to put
together a Linux image that boots on some combination of simulation,
virtualization, and hardware.

If one puts together a Linux image that uses a UART on hardware and a
virtio-mmio console in a virtualized environment, I would argue that this
image has some incrementally higher debug and maintenance cost than an image
that has the same console mechanism across all platforms. On the other hand,
perhaps the cost of implementing a uniform mechanism across all platforms is
higher still.

> Not to mention that I like to be able to actually input something on a console,
> not just read from it.

Perhaps there's some misunderstanding here. The Debug Communications Channel
that I'm familiar with is fully capable of both input and output.

http://infocenter.arm.com/help/topic/com.arm.doc.dui0471c/BEIHGIBB.html
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/tty/hvc/hvc_dcc.c

Thanks,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
the Linux Foundation

[Xen-devel] [PATCH v2 5/6] ARM: Dummy Virtual Machine platform support

[Stefano Stabellini]

On Wed, 19 Dec 2012, Christopher Covington wrote:
> > What would be the point of using DCC?
> 
> It seemed like ARM-Ltd.-architected peripherals were picked for the timer and
> interrupt controller, so I wondered why not for the console as well. As best
> I'm aware, unless one ventures into the PrimeCell line with the PL011, DCC is
> the closest match for an officially architected console mechanism.

I certainly wouldn't want to mandate the presence of any devices that
require emulation in the DT for a virtual machine, including DCC, as
simple as it might be.

[Xen-devel] [PATCH v2 5/6] ARM: Dummy Virtual Machine platform support

[Marc Zyngier]

On 20/12/12 13:12, Stefano Stabellini wrote:
> On Wed, 19 Dec 2012, Christopher Covington wrote:
>>> What would be the point of using DCC?
>>
>> It seemed like ARM-Ltd.-architected peripherals were picked for the timer and
>> interrupt controller, so I wondered why not for the console as well. As best
>> I'm aware, unless one ventures into the PrimeCell line with the PL011, DCC is
>> the closest match for an officially architected console mechanism.
> 
> I certainly wouldn't want to mandate the presence of any devices that
> require emulation in the DT for a virtual machine, including DCC, as
> simple as it might be.

Indeed. GIC and timers have explicit support for virtualization in
hardware. Emulating random IPs is up to the implementor, and given that
virtio already exists (and performs rather well), I don't feel the urge
to reinvent the wheel.

	M.
-- 
Jazz is not dead. It just smells funny...