All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-01-26 11:10 ` Lorenzo Pieralisi
  0 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-26 11:10 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-acpi, Lorenzo Pieralisi, Will Deacon, Hanjun Guo, Loc Ho,
	Sudeep Holla, Catalin Marinas, Mark Rutland, Mark Salter,
	Al Stone

The SBBR and ACPI specifications allow ACPI based systems that do not
implement PSCI (eg systems with no EL3) to boot through the ACPI parking
protocol specification[1].

This patch implements the ACPI parking protocol CPU operations, and adds
code that eases parsing the parking protocol data structures to the
ARM64 SMP initializion carried out at the same time as cpus enumeration.

To wake-up the CPUs from the parked state, this patch implements a
wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
ARM one, so that a specific IPI is sent for wake-up purpose in order
to distinguish it from other IPI sources.

Given the current ACPI MADT parsing API, the patch implements a glue
layer that helps passing MADT GICC data structure from SMP initialization
code to the parking protocol implementation somewhat overriding the CPU
operations interfaces. This to avoid creating a completely trasparent
DT/ACPI CPU operations layer that would require creating opaque
structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
through static MADT table entries), which seems overkill given that ACPI
on ARM64 mandates only two booting protocols (PSCI and parking protocol),
so there is no need for further protocol additions.

Based on the original work by Mark Salter <msalter@redhat.com>

[1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Loc Ho <lho@apm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Al Stone <ahs3@redhat.com>
---
v2->v3

- Rebased against v4.5-rc1
- Moved Kconfig option under "Boot options"

v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/369121.html

v1->v2

- Rebased against v4.2
- Removed SMP dependency (it was removed from arm64)
- Updated some comments
- Clarified 64k page mailbox alignment and requested UEFI specs update

v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356750.html

 arch/arm64/Kconfig                        |   9 ++
 arch/arm64/include/asm/acpi.h             |  19 +++-
 arch/arm64/include/asm/hardirq.h          |   2 +-
 arch/arm64/include/asm/smp.h              |   9 ++
 arch/arm64/kernel/Makefile                |   1 +
 arch/arm64/kernel/acpi_parking_protocol.c | 153 ++++++++++++++++++++++++++++++
 arch/arm64/kernel/cpu_ops.c               |  27 +++++-
 arch/arm64/kernel/smp.c                   |  22 +++++
 8 files changed, 236 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm64/kernel/acpi_parking_protocol.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 8cc6228..53e48a6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -756,6 +756,15 @@ endmenu
 
 menu "Boot options"
 
+config ARM64_ACPI_PARKING_PROTOCOL
+	bool "Enable support for the ARM64 ACPI parking protocol"
+	depends on ACPI
+	help
+	  Enable support for the ARM64 ACPI parking protocol. If disabled
+	  the kernel will not allow booting through the ARM64 ACPI parking
+	  protocol even if the corresponding data is present in the ACPI
+	  MADT table.
+
 config CMDLINE
 	string "Default kernel command string"
 	default ""
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index caafd63..aee323b 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -87,9 +87,26 @@ void __init acpi_init_cpus(void);
 static inline void acpi_init_cpus(void) { }
 #endif /* CONFIG_ACPI */
 
+#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
+bool acpi_parking_protocol_valid(int cpu);
+void __init
+acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor);
+#else
+static inline bool acpi_parking_protocol_valid(int cpu) { return false; }
+static inline void
+acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
+{}
+#endif
+
 static inline const char *acpi_get_enable_method(int cpu)
 {
-	return acpi_psci_present() ? "psci" : NULL;
+	if (acpi_psci_present())
+		return "psci";
+
+	if (acpi_parking_protocol_valid(cpu))
+		return "parking-protocol";
+
+	return NULL;
 }
 
 #ifdef	CONFIG_ACPI_APEI
diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
index a57601f..8740297 100644
--- a/arch/arm64/include/asm/hardirq.h
+++ b/arch/arm64/include/asm/hardirq.h
@@ -20,7 +20,7 @@
 #include <linux/threads.h>
 #include <asm/irq.h>
 
-#define NR_IPI	5
+#define NR_IPI	6
 
 typedef struct {
 	unsigned int __softirq_pending;
diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
index d9c3d6a..2013a4d 100644
--- a/arch/arm64/include/asm/smp.h
+++ b/arch/arm64/include/asm/smp.h
@@ -64,6 +64,15 @@ extern void secondary_entry(void);
 extern void arch_send_call_function_single_ipi(int cpu);
 extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
 
+#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
+extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
+#else
+static inline void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
+{
+	BUILD_BUG();
+}
+#endif
+
 extern int __cpu_disable(void);
 
 extern void __cpu_die(unsigned int cpu);
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 83cd7e6..8a9c65c 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -41,6 +41,7 @@ arm64-obj-$(CONFIG_EFI)			+= efi.o efi-entry.stub.o
 arm64-obj-$(CONFIG_PCI)			+= pci.o
 arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
 arm64-obj-$(CONFIG_ACPI)		+= acpi.o
+arm64-obj-$(CONFIG_ARM64_ACPI_PARKING_PROTOCOL)	+= acpi_parking_protocol.o
 arm64-obj-$(CONFIG_PARAVIRT)		+= paravirt.o
 
 obj-y					+= $(arm64-obj-y) vdso/
diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
new file mode 100644
index 0000000..531c3ad
--- /dev/null
+++ b/arch/arm64/kernel/acpi_parking_protocol.c
@@ -0,0 +1,153 @@
+/*
+ * ARM64 ACPI Parking Protocol implementation
+ *
+ * Authors: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
+ *	    Mark Salter <msalter@redhat.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/acpi.h>
+#include <linux/types.h>
+
+#include <asm/cpu_ops.h>
+
+struct cpu_mailbox_entry {
+	phys_addr_t mailbox_addr;
+	u8 version;
+	u8 gic_cpu_id;
+};
+
+static struct cpu_mailbox_entry cpu_mailbox_entries[NR_CPUS];
+
+void __init acpi_set_mailbox_entry(int cpu,
+				   struct acpi_madt_generic_interrupt *p)
+{
+	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
+
+	cpu_entry->mailbox_addr = p->parked_address;
+	cpu_entry->version = p->parking_version;
+	cpu_entry->gic_cpu_id = p->cpu_interface_number;
+}
+
+bool __init acpi_parking_protocol_valid(int cpu)
+{
+	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
+
+	return cpu_entry->mailbox_addr && cpu_entry->version;
+}
+
+static int acpi_parking_protocol_cpu_init(unsigned int cpu)
+{
+	pr_debug("%s: ACPI parked addr=%llx\n", __func__,
+		  cpu_mailbox_entries[cpu].mailbox_addr);
+
+	return 0;
+}
+
+static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
+{
+	return 0;
+}
+
+struct parking_protocol_mailbox {
+	__le32 cpu_id;
+	__le32 reserved;
+	__le64 entry_point;
+};
+
+static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
+{
+	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
+	struct parking_protocol_mailbox __iomem *mailbox;
+	__le32 cpu_id;
+
+	/*
+	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
+	 * this deviates from the parking protocol specifications since
+	 * the mailboxes are required to be mapped nGnRnE; the attribute
+	 * discrepancy is harmless insofar as the protocol specification
+	 * is concerned).
+	 * If the mailbox is mistakenly allocated in the linear mapping
+	 * by FW ioremap will fail since the mapping will be prevented
+	 * by the kernel (it clashes with the linear mapping attributes
+	 * specifications).
+	 */
+	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
+	if (!mailbox)
+		return -EIO;
+
+	cpu_id = readl_relaxed(&mailbox->cpu_id);
+	/*
+	 * Check if firmware has set-up the mailbox entry properly
+	 * before kickstarting the respective cpu.
+	 */
+	if (cpu_id != ~0U) {
+		iounmap(mailbox);
+		return -ENXIO;
+	}
+
+	/*
+	 * We write the entry point and cpu id as LE regardless of the
+	 * native endianness of the kernel. Therefore, any boot-loaders
+	 * that read this address need to convert this address to the
+	 * Boot-Loader's endianness before jumping.
+	 */
+	writeq_relaxed(__pa(secondary_entry), &mailbox->entry_point);
+	writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id);
+
+	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
+
+	iounmap(mailbox);
+
+	return 0;
+}
+
+static void acpi_parking_protocol_cpu_postboot(void)
+{
+	int cpu = smp_processor_id();
+	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
+	struct parking_protocol_mailbox __iomem *mailbox;
+	__le64 entry_point;
+
+	/*
+	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
+	 * this deviates from the parking protocol specifications since
+	 * the mailboxes are required to be mapped nGnRnE; the attribute
+	 * discrepancy is harmless insofar as the protocol specification
+	 * is concerned).
+	 * If the mailbox is mistakenly allocated in the linear mapping
+	 * by FW ioremap will fail since the mapping will be prevented
+	 * by the kernel (it clashes with the linear mapping attributes
+	 * specifications).
+	 */
+	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
+	if (!mailbox)
+		return;
+
+	entry_point = readl_relaxed(&mailbox->entry_point);
+	/*
+	 * Check if firmware has cleared the entry_point as expected
+	 * by the protocol specification.
+	 */
+	WARN_ON(entry_point);
+
+	iounmap(mailbox);
+}
+
+const struct cpu_operations acpi_parking_protocol_ops = {
+	.name		= "parking-protocol",
+	.cpu_init	= acpi_parking_protocol_cpu_init,
+	.cpu_prepare	= acpi_parking_protocol_cpu_prepare,
+	.cpu_boot	= acpi_parking_protocol_cpu_boot,
+	.cpu_postboot	= acpi_parking_protocol_cpu_postboot
+};
diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
index b6bd7d4..c7cfb8f 100644
--- a/arch/arm64/kernel/cpu_ops.c
+++ b/arch/arm64/kernel/cpu_ops.c
@@ -25,19 +25,30 @@
 #include <asm/smp_plat.h>
 
 extern const struct cpu_operations smp_spin_table_ops;
+extern const struct cpu_operations acpi_parking_protocol_ops;
 extern const struct cpu_operations cpu_psci_ops;
 
 const struct cpu_operations *cpu_ops[NR_CPUS];
 
-static const struct cpu_operations *supported_cpu_ops[] __initconst = {
+static const struct cpu_operations *dt_supported_cpu_ops[] __initconst = {
 	&smp_spin_table_ops,
 	&cpu_psci_ops,
 	NULL,
 };
 
+static const struct cpu_operations *acpi_supported_cpu_ops[] __initconst = {
+#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
+	&acpi_parking_protocol_ops,
+#endif
+	&cpu_psci_ops,
+	NULL,
+};
+
 static const struct cpu_operations * __init cpu_get_ops(const char *name)
 {
-	const struct cpu_operations **ops = supported_cpu_ops;
+	const struct cpu_operations **ops;
+
+	ops = acpi_disabled ? dt_supported_cpu_ops : acpi_supported_cpu_ops;
 
 	while (*ops) {
 		if (!strcmp(name, (*ops)->name))
@@ -75,8 +86,16 @@ static const char *__init cpu_read_enable_method(int cpu)
 		}
 	} else {
 		enable_method = acpi_get_enable_method(cpu);
-		if (!enable_method)
-			pr_err("Unsupported ACPI enable-method\n");
+		if (!enable_method) {
+			/*
+			 * In ACPI systems the boot CPU does not require
+			 * checking the enable method since for some
+			 * boot protocol (ie parking protocol) it need not
+			 * be initialized. Don't warn spuriously.
+			 */
+			if (cpu != 0)
+				pr_err("Unsupported ACPI enable-method\n");
+		}
 	}
 
 	return enable_method;
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index b1adc51..6f608af 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -70,6 +70,7 @@ enum ipi_msg_type {
 	IPI_CPU_STOP,
 	IPI_TIMER,
 	IPI_IRQ_WORK,
+	IPI_WAKEUP
 };
 
 /*
@@ -445,6 +446,17 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
 	/* map the logical cpu id to cpu MPIDR */
 	cpu_logical_map(cpu_count) = hwid;
 
+	/*
+	 * Set-up the ACPI parking protocol cpu entries
+	 * while initializing the cpu_logical_map to
+	 * avoid parsing MADT entries multiple times for
+	 * nothing (ie a valid cpu_logical_map entry should
+	 * contain a valid parking protocol data set to
+	 * initialize the cpu if the parking protocol is
+	 * the only available enable method).
+	 */
+	acpi_set_mailbox_entry(cpu_count, processor);
+
 	cpu_count++;
 }
 
@@ -627,6 +639,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
 	S(IPI_CPU_STOP, "CPU stop interrupts"),
 	S(IPI_TIMER, "Timer broadcast interrupts"),
 	S(IPI_IRQ_WORK, "IRQ work interrupts"),
+	S(IPI_WAKEUP, "CPU wakeup interrupts"),
 };
 
 static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
@@ -670,6 +683,13 @@ void arch_send_call_function_single_ipi(int cpu)
 	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC);
 }
 
+#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
+void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
+{
+	smp_cross_call(mask, IPI_WAKEUP);
+}
+#endif
+
 #ifdef CONFIG_IRQ_WORK
 void arch_irq_work_raise(void)
 {
@@ -746,6 +766,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 		irq_exit();
 		break;
 #endif
+	case IPI_WAKEUP:
+		break;
 
 	default:
 		pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
-- 
2.5.1


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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-01-26 11:10 ` Lorenzo Pieralisi
  0 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-26 11:10 UTC (permalink / raw)
  To: linux-arm-kernel

The SBBR and ACPI specifications allow ACPI based systems that do not
implement PSCI (eg systems with no EL3) to boot through the ACPI parking
protocol specification[1].

This patch implements the ACPI parking protocol CPU operations, and adds
code that eases parsing the parking protocol data structures to the
ARM64 SMP initializion carried out at the same time as cpus enumeration.

To wake-up the CPUs from the parked state, this patch implements a
wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
ARM one, so that a specific IPI is sent for wake-up purpose in order
to distinguish it from other IPI sources.

Given the current ACPI MADT parsing API, the patch implements a glue
layer that helps passing MADT GICC data structure from SMP initialization
code to the parking protocol implementation somewhat overriding the CPU
operations interfaces. This to avoid creating a completely trasparent
DT/ACPI CPU operations layer that would require creating opaque
structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
through static MADT table entries), which seems overkill given that ACPI
on ARM64 mandates only two booting protocols (PSCI and parking protocol),
so there is no need for further protocol additions.

Based on the original work by Mark Salter <msalter@redhat.com>

[1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Loc Ho <lho@apm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Al Stone <ahs3@redhat.com>
---
v2->v3

- Rebased against v4.5-rc1
- Moved Kconfig option under "Boot options"

v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/369121.html

v1->v2

- Rebased against v4.2
- Removed SMP dependency (it was removed from arm64)
- Updated some comments
- Clarified 64k page mailbox alignment and requested UEFI specs update

v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356750.html

 arch/arm64/Kconfig                        |   9 ++
 arch/arm64/include/asm/acpi.h             |  19 +++-
 arch/arm64/include/asm/hardirq.h          |   2 +-
 arch/arm64/include/asm/smp.h              |   9 ++
 arch/arm64/kernel/Makefile                |   1 +
 arch/arm64/kernel/acpi_parking_protocol.c | 153 ++++++++++++++++++++++++++++++
 arch/arm64/kernel/cpu_ops.c               |  27 +++++-
 arch/arm64/kernel/smp.c                   |  22 +++++
 8 files changed, 236 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm64/kernel/acpi_parking_protocol.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 8cc6228..53e48a6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -756,6 +756,15 @@ endmenu
 
 menu "Boot options"
 
+config ARM64_ACPI_PARKING_PROTOCOL
+	bool "Enable support for the ARM64 ACPI parking protocol"
+	depends on ACPI
+	help
+	  Enable support for the ARM64 ACPI parking protocol. If disabled
+	  the kernel will not allow booting through the ARM64 ACPI parking
+	  protocol even if the corresponding data is present in the ACPI
+	  MADT table.
+
 config CMDLINE
 	string "Default kernel command string"
 	default ""
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index caafd63..aee323b 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -87,9 +87,26 @@ void __init acpi_init_cpus(void);
 static inline void acpi_init_cpus(void) { }
 #endif /* CONFIG_ACPI */
 
+#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
+bool acpi_parking_protocol_valid(int cpu);
+void __init
+acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor);
+#else
+static inline bool acpi_parking_protocol_valid(int cpu) { return false; }
+static inline void
+acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
+{}
+#endif
+
 static inline const char *acpi_get_enable_method(int cpu)
 {
-	return acpi_psci_present() ? "psci" : NULL;
+	if (acpi_psci_present())
+		return "psci";
+
+	if (acpi_parking_protocol_valid(cpu))
+		return "parking-protocol";
+
+	return NULL;
 }
 
 #ifdef	CONFIG_ACPI_APEI
diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
index a57601f..8740297 100644
--- a/arch/arm64/include/asm/hardirq.h
+++ b/arch/arm64/include/asm/hardirq.h
@@ -20,7 +20,7 @@
 #include <linux/threads.h>
 #include <asm/irq.h>
 
-#define NR_IPI	5
+#define NR_IPI	6
 
 typedef struct {
 	unsigned int __softirq_pending;
diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
index d9c3d6a..2013a4d 100644
--- a/arch/arm64/include/asm/smp.h
+++ b/arch/arm64/include/asm/smp.h
@@ -64,6 +64,15 @@ extern void secondary_entry(void);
 extern void arch_send_call_function_single_ipi(int cpu);
 extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
 
+#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
+extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
+#else
+static inline void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
+{
+	BUILD_BUG();
+}
+#endif
+
 extern int __cpu_disable(void);
 
 extern void __cpu_die(unsigned int cpu);
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 83cd7e6..8a9c65c 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -41,6 +41,7 @@ arm64-obj-$(CONFIG_EFI)			+= efi.o efi-entry.stub.o
 arm64-obj-$(CONFIG_PCI)			+= pci.o
 arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
 arm64-obj-$(CONFIG_ACPI)		+= acpi.o
+arm64-obj-$(CONFIG_ARM64_ACPI_PARKING_PROTOCOL)	+= acpi_parking_protocol.o
 arm64-obj-$(CONFIG_PARAVIRT)		+= paravirt.o
 
 obj-y					+= $(arm64-obj-y) vdso/
diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
new file mode 100644
index 0000000..531c3ad
--- /dev/null
+++ b/arch/arm64/kernel/acpi_parking_protocol.c
@@ -0,0 +1,153 @@
+/*
+ * ARM64 ACPI Parking Protocol implementation
+ *
+ * Authors: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
+ *	    Mark Salter <msalter@redhat.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/acpi.h>
+#include <linux/types.h>
+
+#include <asm/cpu_ops.h>
+
+struct cpu_mailbox_entry {
+	phys_addr_t mailbox_addr;
+	u8 version;
+	u8 gic_cpu_id;
+};
+
+static struct cpu_mailbox_entry cpu_mailbox_entries[NR_CPUS];
+
+void __init acpi_set_mailbox_entry(int cpu,
+				   struct acpi_madt_generic_interrupt *p)
+{
+	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
+
+	cpu_entry->mailbox_addr = p->parked_address;
+	cpu_entry->version = p->parking_version;
+	cpu_entry->gic_cpu_id = p->cpu_interface_number;
+}
+
+bool __init acpi_parking_protocol_valid(int cpu)
+{
+	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
+
+	return cpu_entry->mailbox_addr && cpu_entry->version;
+}
+
+static int acpi_parking_protocol_cpu_init(unsigned int cpu)
+{
+	pr_debug("%s: ACPI parked addr=%llx\n", __func__,
+		  cpu_mailbox_entries[cpu].mailbox_addr);
+
+	return 0;
+}
+
+static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
+{
+	return 0;
+}
+
+struct parking_protocol_mailbox {
+	__le32 cpu_id;
+	__le32 reserved;
+	__le64 entry_point;
+};
+
+static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
+{
+	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
+	struct parking_protocol_mailbox __iomem *mailbox;
+	__le32 cpu_id;
+
+	/*
+	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
+	 * this deviates from the parking protocol specifications since
+	 * the mailboxes are required to be mapped nGnRnE; the attribute
+	 * discrepancy is harmless insofar as the protocol specification
+	 * is concerned).
+	 * If the mailbox is mistakenly allocated in the linear mapping
+	 * by FW ioremap will fail since the mapping will be prevented
+	 * by the kernel (it clashes with the linear mapping attributes
+	 * specifications).
+	 */
+	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
+	if (!mailbox)
+		return -EIO;
+
+	cpu_id = readl_relaxed(&mailbox->cpu_id);
+	/*
+	 * Check if firmware has set-up the mailbox entry properly
+	 * before kickstarting the respective cpu.
+	 */
+	if (cpu_id != ~0U) {
+		iounmap(mailbox);
+		return -ENXIO;
+	}
+
+	/*
+	 * We write the entry point and cpu id as LE regardless of the
+	 * native endianness of the kernel. Therefore, any boot-loaders
+	 * that read this address need to convert this address to the
+	 * Boot-Loader's endianness before jumping.
+	 */
+	writeq_relaxed(__pa(secondary_entry), &mailbox->entry_point);
+	writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id);
+
+	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
+
+	iounmap(mailbox);
+
+	return 0;
+}
+
+static void acpi_parking_protocol_cpu_postboot(void)
+{
+	int cpu = smp_processor_id();
+	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
+	struct parking_protocol_mailbox __iomem *mailbox;
+	__le64 entry_point;
+
+	/*
+	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
+	 * this deviates from the parking protocol specifications since
+	 * the mailboxes are required to be mapped nGnRnE; the attribute
+	 * discrepancy is harmless insofar as the protocol specification
+	 * is concerned).
+	 * If the mailbox is mistakenly allocated in the linear mapping
+	 * by FW ioremap will fail since the mapping will be prevented
+	 * by the kernel (it clashes with the linear mapping attributes
+	 * specifications).
+	 */
+	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
+	if (!mailbox)
+		return;
+
+	entry_point = readl_relaxed(&mailbox->entry_point);
+	/*
+	 * Check if firmware has cleared the entry_point as expected
+	 * by the protocol specification.
+	 */
+	WARN_ON(entry_point);
+
+	iounmap(mailbox);
+}
+
+const struct cpu_operations acpi_parking_protocol_ops = {
+	.name		= "parking-protocol",
+	.cpu_init	= acpi_parking_protocol_cpu_init,
+	.cpu_prepare	= acpi_parking_protocol_cpu_prepare,
+	.cpu_boot	= acpi_parking_protocol_cpu_boot,
+	.cpu_postboot	= acpi_parking_protocol_cpu_postboot
+};
diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
index b6bd7d4..c7cfb8f 100644
--- a/arch/arm64/kernel/cpu_ops.c
+++ b/arch/arm64/kernel/cpu_ops.c
@@ -25,19 +25,30 @@
 #include <asm/smp_plat.h>
 
 extern const struct cpu_operations smp_spin_table_ops;
+extern const struct cpu_operations acpi_parking_protocol_ops;
 extern const struct cpu_operations cpu_psci_ops;
 
 const struct cpu_operations *cpu_ops[NR_CPUS];
 
-static const struct cpu_operations *supported_cpu_ops[] __initconst = {
+static const struct cpu_operations *dt_supported_cpu_ops[] __initconst = {
 	&smp_spin_table_ops,
 	&cpu_psci_ops,
 	NULL,
 };
 
+static const struct cpu_operations *acpi_supported_cpu_ops[] __initconst = {
+#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
+	&acpi_parking_protocol_ops,
+#endif
+	&cpu_psci_ops,
+	NULL,
+};
+
 static const struct cpu_operations * __init cpu_get_ops(const char *name)
 {
-	const struct cpu_operations **ops = supported_cpu_ops;
+	const struct cpu_operations **ops;
+
+	ops = acpi_disabled ? dt_supported_cpu_ops : acpi_supported_cpu_ops;
 
 	while (*ops) {
 		if (!strcmp(name, (*ops)->name))
@@ -75,8 +86,16 @@ static const char *__init cpu_read_enable_method(int cpu)
 		}
 	} else {
 		enable_method = acpi_get_enable_method(cpu);
-		if (!enable_method)
-			pr_err("Unsupported ACPI enable-method\n");
+		if (!enable_method) {
+			/*
+			 * In ACPI systems the boot CPU does not require
+			 * checking the enable method since for some
+			 * boot protocol (ie parking protocol) it need not
+			 * be initialized. Don't warn spuriously.
+			 */
+			if (cpu != 0)
+				pr_err("Unsupported ACPI enable-method\n");
+		}
 	}
 
 	return enable_method;
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index b1adc51..6f608af 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -70,6 +70,7 @@ enum ipi_msg_type {
 	IPI_CPU_STOP,
 	IPI_TIMER,
 	IPI_IRQ_WORK,
+	IPI_WAKEUP
 };
 
 /*
@@ -445,6 +446,17 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
 	/* map the logical cpu id to cpu MPIDR */
 	cpu_logical_map(cpu_count) = hwid;
 
+	/*
+	 * Set-up the ACPI parking protocol cpu entries
+	 * while initializing the cpu_logical_map to
+	 * avoid parsing MADT entries multiple times for
+	 * nothing (ie a valid cpu_logical_map entry should
+	 * contain a valid parking protocol data set to
+	 * initialize the cpu if the parking protocol is
+	 * the only available enable method).
+	 */
+	acpi_set_mailbox_entry(cpu_count, processor);
+
 	cpu_count++;
 }
 
@@ -627,6 +639,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
 	S(IPI_CPU_STOP, "CPU stop interrupts"),
 	S(IPI_TIMER, "Timer broadcast interrupts"),
 	S(IPI_IRQ_WORK, "IRQ work interrupts"),
+	S(IPI_WAKEUP, "CPU wakeup interrupts"),
 };
 
 static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
@@ -670,6 +683,13 @@ void arch_send_call_function_single_ipi(int cpu)
 	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC);
 }
 
+#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
+void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
+{
+	smp_cross_call(mask, IPI_WAKEUP);
+}
+#endif
+
 #ifdef CONFIG_IRQ_WORK
 void arch_irq_work_raise(void)
 {
@@ -746,6 +766,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 		irq_exit();
 		break;
 #endif
+	case IPI_WAKEUP:
+		break;
 
 	default:
 		pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
-- 
2.5.1

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-01-26 11:10 ` Lorenzo Pieralisi
@ 2016-01-26 17:47   ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-26 17:47 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-acpi, Will Deacon, Hanjun Guo, Loc Ho, Sudeep Holla,
	Catalin Marinas, Mark Rutland, Mark Salter, Al Stone,
	ard.biesheuvel, leif.lindholm

[+ Ard, Leif]

On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
> The SBBR and ACPI specifications allow ACPI based systems that do not
> implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> protocol specification[1].
> 
> This patch implements the ACPI parking protocol CPU operations, and adds
> code that eases parsing the parking protocol data structures to the
> ARM64 SMP initializion carried out at the same time as cpus enumeration.
> 
> To wake-up the CPUs from the parked state, this patch implements a
> wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> ARM one, so that a specific IPI is sent for wake-up purpose in order
> to distinguish it from other IPI sources.
> 
> Given the current ACPI MADT parsing API, the patch implements a glue
> layer that helps passing MADT GICC data structure from SMP initialization
> code to the parking protocol implementation somewhat overriding the CPU
> operations interfaces. This to avoid creating a completely trasparent
> DT/ACPI CPU operations layer that would require creating opaque
> structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> through static MADT table entries), which seems overkill given that ACPI
> on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> so there is no need for further protocol additions.
> 
> Based on the original work by Mark Salter <msalter@redhat.com>
> 
> [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Loc Ho <lho@apm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Al Stone <ahs3@redhat.com>
> ---
> v2->v3
> 
> - Rebased against v4.5-rc1

It is to report that this patch currently works on my AMD Seattle with
a v4.5-rc1 kernel whilst it was not working before owing to FW putting
the mailboxes in WB memory (that was part of linear mapping, I still have
to pinpoint the exact reason). I think it is related to recent changes in
EFI and memblock. The code in this patch is allowed to map mailboxes
through ioremap() (ie it was not before because IIUC pfn_valid was returning
true on the mailboxes addresses), which it looks like it changed with:

68709f45385a ("arm64: only consider memblocks with NOMAP cleared for
linear mapping")

Basically we end up mapping a memory area with ioremap with attributes
that may mismatch EFI descriptors, on my AMD Seattle the mailboxes are
part of runtime services data that is WB (see below).

I think the only way I can prevent this is by looking up the
mailboxes addresses in the EFI memory map and bail out if the
memory descriptor is marked as WB (according to the ACPI parking
protocol the mailboxes must be Device-nGnRnE).

AMD Seattle snip boot log with this patch applied and some debug output
below (ie mailbox address falls within "Runtime Data").

I will keep debugging, comments appreciated.

Thanks,
Lorenzo

[    0.000000] Processing EFI memory map:

<snip>

[    0.000000]   0x008028000000-0x0080280fffff [Runtime Data       |RUN|  |  |  |  |  |   |WB|WT|WC|UC]*
[    0.000000] acpi_parking_protocol_cpu_init: ACPI parked addr=80280c1000

> - Moved Kconfig option under "Boot options"
> 
> v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/369121.html
> 
> v1->v2
> 
> - Rebased against v4.2
> - Removed SMP dependency (it was removed from arm64)
> - Updated some comments
> - Clarified 64k page mailbox alignment and requested UEFI specs update
> 
> v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356750.html
> 
>  arch/arm64/Kconfig                        |   9 ++
>  arch/arm64/include/asm/acpi.h             |  19 +++-
>  arch/arm64/include/asm/hardirq.h          |   2 +-
>  arch/arm64/include/asm/smp.h              |   9 ++
>  arch/arm64/kernel/Makefile                |   1 +
>  arch/arm64/kernel/acpi_parking_protocol.c | 153 ++++++++++++++++++++++++++++++
>  arch/arm64/kernel/cpu_ops.c               |  27 +++++-
>  arch/arm64/kernel/smp.c                   |  22 +++++
>  8 files changed, 236 insertions(+), 6 deletions(-)
>  create mode 100644 arch/arm64/kernel/acpi_parking_protocol.c
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 8cc6228..53e48a6 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -756,6 +756,15 @@ endmenu
>  
>  menu "Boot options"
>  
> +config ARM64_ACPI_PARKING_PROTOCOL
> +	bool "Enable support for the ARM64 ACPI parking protocol"
> +	depends on ACPI
> +	help
> +	  Enable support for the ARM64 ACPI parking protocol. If disabled
> +	  the kernel will not allow booting through the ARM64 ACPI parking
> +	  protocol even if the corresponding data is present in the ACPI
> +	  MADT table.
> +
>  config CMDLINE
>  	string "Default kernel command string"
>  	default ""
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index caafd63..aee323b 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -87,9 +87,26 @@ void __init acpi_init_cpus(void);
>  static inline void acpi_init_cpus(void) { }
>  #endif /* CONFIG_ACPI */
>  
> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> +bool acpi_parking_protocol_valid(int cpu);
> +void __init
> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor);
> +#else
> +static inline bool acpi_parking_protocol_valid(int cpu) { return false; }
> +static inline void
> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
> +{}
> +#endif
> +
>  static inline const char *acpi_get_enable_method(int cpu)
>  {
> -	return acpi_psci_present() ? "psci" : NULL;
> +	if (acpi_psci_present())
> +		return "psci";
> +
> +	if (acpi_parking_protocol_valid(cpu))
> +		return "parking-protocol";
> +
> +	return NULL;
>  }
>  
>  #ifdef	CONFIG_ACPI_APEI
> diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
> index a57601f..8740297 100644
> --- a/arch/arm64/include/asm/hardirq.h
> +++ b/arch/arm64/include/asm/hardirq.h
> @@ -20,7 +20,7 @@
>  #include <linux/threads.h>
>  #include <asm/irq.h>
>  
> -#define NR_IPI	5
> +#define NR_IPI	6
>  
>  typedef struct {
>  	unsigned int __softirq_pending;
> diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
> index d9c3d6a..2013a4d 100644
> --- a/arch/arm64/include/asm/smp.h
> +++ b/arch/arm64/include/asm/smp.h
> @@ -64,6 +64,15 @@ extern void secondary_entry(void);
>  extern void arch_send_call_function_single_ipi(int cpu);
>  extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
>  
> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> +extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
> +#else
> +static inline void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
> +{
> +	BUILD_BUG();
> +}
> +#endif
> +
>  extern int __cpu_disable(void);
>  
>  extern void __cpu_die(unsigned int cpu);
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 83cd7e6..8a9c65c 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -41,6 +41,7 @@ arm64-obj-$(CONFIG_EFI)			+= efi.o efi-entry.stub.o
>  arm64-obj-$(CONFIG_PCI)			+= pci.o
>  arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
>  arm64-obj-$(CONFIG_ACPI)		+= acpi.o
> +arm64-obj-$(CONFIG_ARM64_ACPI_PARKING_PROTOCOL)	+= acpi_parking_protocol.o
>  arm64-obj-$(CONFIG_PARAVIRT)		+= paravirt.o
>  
>  obj-y					+= $(arm64-obj-y) vdso/
> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
> new file mode 100644
> index 0000000..531c3ad
> --- /dev/null
> +++ b/arch/arm64/kernel/acpi_parking_protocol.c
> @@ -0,0 +1,153 @@
> +/*
> + * ARM64 ACPI Parking Protocol implementation
> + *
> + * Authors: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> + *	    Mark Salter <msalter@redhat.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/acpi.h>
> +#include <linux/types.h>
> +
> +#include <asm/cpu_ops.h>
> +
> +struct cpu_mailbox_entry {
> +	phys_addr_t mailbox_addr;
> +	u8 version;
> +	u8 gic_cpu_id;
> +};
> +
> +static struct cpu_mailbox_entry cpu_mailbox_entries[NR_CPUS];
> +
> +void __init acpi_set_mailbox_entry(int cpu,
> +				   struct acpi_madt_generic_interrupt *p)
> +{
> +	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> +
> +	cpu_entry->mailbox_addr = p->parked_address;
> +	cpu_entry->version = p->parking_version;
> +	cpu_entry->gic_cpu_id = p->cpu_interface_number;
> +}
> +
> +bool __init acpi_parking_protocol_valid(int cpu)
> +{
> +	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> +
> +	return cpu_entry->mailbox_addr && cpu_entry->version;
> +}
> +
> +static int acpi_parking_protocol_cpu_init(unsigned int cpu)
> +{
> +	pr_debug("%s: ACPI parked addr=%llx\n", __func__,
> +		  cpu_mailbox_entries[cpu].mailbox_addr);
> +
> +	return 0;
> +}
> +
> +static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
> +{
> +	return 0;
> +}
> +
> +struct parking_protocol_mailbox {
> +	__le32 cpu_id;
> +	__le32 reserved;
> +	__le64 entry_point;
> +};
> +
> +static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
> +{
> +	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> +	struct parking_protocol_mailbox __iomem *mailbox;
> +	__le32 cpu_id;
> +
> +	/*
> +	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
> +	 * this deviates from the parking protocol specifications since
> +	 * the mailboxes are required to be mapped nGnRnE; the attribute
> +	 * discrepancy is harmless insofar as the protocol specification
> +	 * is concerned).
> +	 * If the mailbox is mistakenly allocated in the linear mapping
> +	 * by FW ioremap will fail since the mapping will be prevented
> +	 * by the kernel (it clashes with the linear mapping attributes
> +	 * specifications).
> +	 */
> +	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> +	if (!mailbox)
> +		return -EIO;
> +
> +	cpu_id = readl_relaxed(&mailbox->cpu_id);
> +	/*
> +	 * Check if firmware has set-up the mailbox entry properly
> +	 * before kickstarting the respective cpu.
> +	 */
> +	if (cpu_id != ~0U) {
> +		iounmap(mailbox);
> +		return -ENXIO;
> +	}
> +
> +	/*
> +	 * We write the entry point and cpu id as LE regardless of the
> +	 * native endianness of the kernel. Therefore, any boot-loaders
> +	 * that read this address need to convert this address to the
> +	 * Boot-Loader's endianness before jumping.
> +	 */
> +	writeq_relaxed(__pa(secondary_entry), &mailbox->entry_point);
> +	writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id);
> +
> +	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
> +
> +	iounmap(mailbox);
> +
> +	return 0;
> +}
> +
> +static void acpi_parking_protocol_cpu_postboot(void)
> +{
> +	int cpu = smp_processor_id();
> +	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> +	struct parking_protocol_mailbox __iomem *mailbox;
> +	__le64 entry_point;
> +
> +	/*
> +	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
> +	 * this deviates from the parking protocol specifications since
> +	 * the mailboxes are required to be mapped nGnRnE; the attribute
> +	 * discrepancy is harmless insofar as the protocol specification
> +	 * is concerned).
> +	 * If the mailbox is mistakenly allocated in the linear mapping
> +	 * by FW ioremap will fail since the mapping will be prevented
> +	 * by the kernel (it clashes with the linear mapping attributes
> +	 * specifications).
> +	 */
> +	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> +	if (!mailbox)
> +		return;
> +
> +	entry_point = readl_relaxed(&mailbox->entry_point);
> +	/*
> +	 * Check if firmware has cleared the entry_point as expected
> +	 * by the protocol specification.
> +	 */
> +	WARN_ON(entry_point);
> +
> +	iounmap(mailbox);
> +}
> +
> +const struct cpu_operations acpi_parking_protocol_ops = {
> +	.name		= "parking-protocol",
> +	.cpu_init	= acpi_parking_protocol_cpu_init,
> +	.cpu_prepare	= acpi_parking_protocol_cpu_prepare,
> +	.cpu_boot	= acpi_parking_protocol_cpu_boot,
> +	.cpu_postboot	= acpi_parking_protocol_cpu_postboot
> +};
> diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
> index b6bd7d4..c7cfb8f 100644
> --- a/arch/arm64/kernel/cpu_ops.c
> +++ b/arch/arm64/kernel/cpu_ops.c
> @@ -25,19 +25,30 @@
>  #include <asm/smp_plat.h>
>  
>  extern const struct cpu_operations smp_spin_table_ops;
> +extern const struct cpu_operations acpi_parking_protocol_ops;
>  extern const struct cpu_operations cpu_psci_ops;
>  
>  const struct cpu_operations *cpu_ops[NR_CPUS];
>  
> -static const struct cpu_operations *supported_cpu_ops[] __initconst = {
> +static const struct cpu_operations *dt_supported_cpu_ops[] __initconst = {
>  	&smp_spin_table_ops,
>  	&cpu_psci_ops,
>  	NULL,
>  };
>  
> +static const struct cpu_operations *acpi_supported_cpu_ops[] __initconst = {
> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> +	&acpi_parking_protocol_ops,
> +#endif
> +	&cpu_psci_ops,
> +	NULL,
> +};
> +
>  static const struct cpu_operations * __init cpu_get_ops(const char *name)
>  {
> -	const struct cpu_operations **ops = supported_cpu_ops;
> +	const struct cpu_operations **ops;
> +
> +	ops = acpi_disabled ? dt_supported_cpu_ops : acpi_supported_cpu_ops;
>  
>  	while (*ops) {
>  		if (!strcmp(name, (*ops)->name))
> @@ -75,8 +86,16 @@ static const char *__init cpu_read_enable_method(int cpu)
>  		}
>  	} else {
>  		enable_method = acpi_get_enable_method(cpu);
> -		if (!enable_method)
> -			pr_err("Unsupported ACPI enable-method\n");
> +		if (!enable_method) {
> +			/*
> +			 * In ACPI systems the boot CPU does not require
> +			 * checking the enable method since for some
> +			 * boot protocol (ie parking protocol) it need not
> +			 * be initialized. Don't warn spuriously.
> +			 */
> +			if (cpu != 0)
> +				pr_err("Unsupported ACPI enable-method\n");
> +		}
>  	}
>  
>  	return enable_method;
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index b1adc51..6f608af 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -70,6 +70,7 @@ enum ipi_msg_type {
>  	IPI_CPU_STOP,
>  	IPI_TIMER,
>  	IPI_IRQ_WORK,
> +	IPI_WAKEUP
>  };
>  
>  /*
> @@ -445,6 +446,17 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
>  	/* map the logical cpu id to cpu MPIDR */
>  	cpu_logical_map(cpu_count) = hwid;
>  
> +	/*
> +	 * Set-up the ACPI parking protocol cpu entries
> +	 * while initializing the cpu_logical_map to
> +	 * avoid parsing MADT entries multiple times for
> +	 * nothing (ie a valid cpu_logical_map entry should
> +	 * contain a valid parking protocol data set to
> +	 * initialize the cpu if the parking protocol is
> +	 * the only available enable method).
> +	 */
> +	acpi_set_mailbox_entry(cpu_count, processor);
> +
>  	cpu_count++;
>  }
>  
> @@ -627,6 +639,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
>  	S(IPI_CPU_STOP, "CPU stop interrupts"),
>  	S(IPI_TIMER, "Timer broadcast interrupts"),
>  	S(IPI_IRQ_WORK, "IRQ work interrupts"),
> +	S(IPI_WAKEUP, "CPU wakeup interrupts"),
>  };
>  
>  static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
> @@ -670,6 +683,13 @@ void arch_send_call_function_single_ipi(int cpu)
>  	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC);
>  }
>  
> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> +void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
> +{
> +	smp_cross_call(mask, IPI_WAKEUP);
> +}
> +#endif
> +
>  #ifdef CONFIG_IRQ_WORK
>  void arch_irq_work_raise(void)
>  {
> @@ -746,6 +766,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
>  		irq_exit();
>  		break;
>  #endif
> +	case IPI_WAKEUP:
> +		break;
>  
>  	default:
>  		pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
> -- 
> 2.5.1
> 

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-01-26 17:47   ` Lorenzo Pieralisi
  0 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-26 17:47 UTC (permalink / raw)
  To: linux-arm-kernel

[+ Ard, Leif]

On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
> The SBBR and ACPI specifications allow ACPI based systems that do not
> implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> protocol specification[1].
> 
> This patch implements the ACPI parking protocol CPU operations, and adds
> code that eases parsing the parking protocol data structures to the
> ARM64 SMP initializion carried out at the same time as cpus enumeration.
> 
> To wake-up the CPUs from the parked state, this patch implements a
> wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> ARM one, so that a specific IPI is sent for wake-up purpose in order
> to distinguish it from other IPI sources.
> 
> Given the current ACPI MADT parsing API, the patch implements a glue
> layer that helps passing MADT GICC data structure from SMP initialization
> code to the parking protocol implementation somewhat overriding the CPU
> operations interfaces. This to avoid creating a completely trasparent
> DT/ACPI CPU operations layer that would require creating opaque
> structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> through static MADT table entries), which seems overkill given that ACPI
> on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> so there is no need for further protocol additions.
> 
> Based on the original work by Mark Salter <msalter@redhat.com>
> 
> [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Loc Ho <lho@apm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Al Stone <ahs3@redhat.com>
> ---
> v2->v3
> 
> - Rebased against v4.5-rc1

It is to report that this patch currently works on my AMD Seattle with
a v4.5-rc1 kernel whilst it was not working before owing to FW putting
the mailboxes in WB memory (that was part of linear mapping, I still have
to pinpoint the exact reason). I think it is related to recent changes in
EFI and memblock. The code in this patch is allowed to map mailboxes
through ioremap() (ie it was not before because IIUC pfn_valid was returning
true on the mailboxes addresses), which it looks like it changed with:

68709f45385a ("arm64: only consider memblocks with NOMAP cleared for
linear mapping")

Basically we end up mapping a memory area with ioremap with attributes
that may mismatch EFI descriptors, on my AMD Seattle the mailboxes are
part of runtime services data that is WB (see below).

I think the only way I can prevent this is by looking up the
mailboxes addresses in the EFI memory map and bail out if the
memory descriptor is marked as WB (according to the ACPI parking
protocol the mailboxes must be Device-nGnRnE).

AMD Seattle snip boot log with this patch applied and some debug output
below (ie mailbox address falls within "Runtime Data").

I will keep debugging, comments appreciated.

Thanks,
Lorenzo

[    0.000000] Processing EFI memory map:

<snip>

[    0.000000]   0x008028000000-0x0080280fffff [Runtime Data       |RUN|  |  |  |  |  |   |WB|WT|WC|UC]*
[    0.000000] acpi_parking_protocol_cpu_init: ACPI parked addr=80280c1000

> - Moved Kconfig option under "Boot options"
> 
> v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/369121.html
> 
> v1->v2
> 
> - Rebased against v4.2
> - Removed SMP dependency (it was removed from arm64)
> - Updated some comments
> - Clarified 64k page mailbox alignment and requested UEFI specs update
> 
> v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356750.html
> 
>  arch/arm64/Kconfig                        |   9 ++
>  arch/arm64/include/asm/acpi.h             |  19 +++-
>  arch/arm64/include/asm/hardirq.h          |   2 +-
>  arch/arm64/include/asm/smp.h              |   9 ++
>  arch/arm64/kernel/Makefile                |   1 +
>  arch/arm64/kernel/acpi_parking_protocol.c | 153 ++++++++++++++++++++++++++++++
>  arch/arm64/kernel/cpu_ops.c               |  27 +++++-
>  arch/arm64/kernel/smp.c                   |  22 +++++
>  8 files changed, 236 insertions(+), 6 deletions(-)
>  create mode 100644 arch/arm64/kernel/acpi_parking_protocol.c
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 8cc6228..53e48a6 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -756,6 +756,15 @@ endmenu
>  
>  menu "Boot options"
>  
> +config ARM64_ACPI_PARKING_PROTOCOL
> +	bool "Enable support for the ARM64 ACPI parking protocol"
> +	depends on ACPI
> +	help
> +	  Enable support for the ARM64 ACPI parking protocol. If disabled
> +	  the kernel will not allow booting through the ARM64 ACPI parking
> +	  protocol even if the corresponding data is present in the ACPI
> +	  MADT table.
> +
>  config CMDLINE
>  	string "Default kernel command string"
>  	default ""
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index caafd63..aee323b 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -87,9 +87,26 @@ void __init acpi_init_cpus(void);
>  static inline void acpi_init_cpus(void) { }
>  #endif /* CONFIG_ACPI */
>  
> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> +bool acpi_parking_protocol_valid(int cpu);
> +void __init
> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor);
> +#else
> +static inline bool acpi_parking_protocol_valid(int cpu) { return false; }
> +static inline void
> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
> +{}
> +#endif
> +
>  static inline const char *acpi_get_enable_method(int cpu)
>  {
> -	return acpi_psci_present() ? "psci" : NULL;
> +	if (acpi_psci_present())
> +		return "psci";
> +
> +	if (acpi_parking_protocol_valid(cpu))
> +		return "parking-protocol";
> +
> +	return NULL;
>  }
>  
>  #ifdef	CONFIG_ACPI_APEI
> diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
> index a57601f..8740297 100644
> --- a/arch/arm64/include/asm/hardirq.h
> +++ b/arch/arm64/include/asm/hardirq.h
> @@ -20,7 +20,7 @@
>  #include <linux/threads.h>
>  #include <asm/irq.h>
>  
> -#define NR_IPI	5
> +#define NR_IPI	6
>  
>  typedef struct {
>  	unsigned int __softirq_pending;
> diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
> index d9c3d6a..2013a4d 100644
> --- a/arch/arm64/include/asm/smp.h
> +++ b/arch/arm64/include/asm/smp.h
> @@ -64,6 +64,15 @@ extern void secondary_entry(void);
>  extern void arch_send_call_function_single_ipi(int cpu);
>  extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
>  
> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> +extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
> +#else
> +static inline void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
> +{
> +	BUILD_BUG();
> +}
> +#endif
> +
>  extern int __cpu_disable(void);
>  
>  extern void __cpu_die(unsigned int cpu);
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 83cd7e6..8a9c65c 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -41,6 +41,7 @@ arm64-obj-$(CONFIG_EFI)			+= efi.o efi-entry.stub.o
>  arm64-obj-$(CONFIG_PCI)			+= pci.o
>  arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
>  arm64-obj-$(CONFIG_ACPI)		+= acpi.o
> +arm64-obj-$(CONFIG_ARM64_ACPI_PARKING_PROTOCOL)	+= acpi_parking_protocol.o
>  arm64-obj-$(CONFIG_PARAVIRT)		+= paravirt.o
>  
>  obj-y					+= $(arm64-obj-y) vdso/
> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
> new file mode 100644
> index 0000000..531c3ad
> --- /dev/null
> +++ b/arch/arm64/kernel/acpi_parking_protocol.c
> @@ -0,0 +1,153 @@
> +/*
> + * ARM64 ACPI Parking Protocol implementation
> + *
> + * Authors: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> + *	    Mark Salter <msalter@redhat.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/acpi.h>
> +#include <linux/types.h>
> +
> +#include <asm/cpu_ops.h>
> +
> +struct cpu_mailbox_entry {
> +	phys_addr_t mailbox_addr;
> +	u8 version;
> +	u8 gic_cpu_id;
> +};
> +
> +static struct cpu_mailbox_entry cpu_mailbox_entries[NR_CPUS];
> +
> +void __init acpi_set_mailbox_entry(int cpu,
> +				   struct acpi_madt_generic_interrupt *p)
> +{
> +	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> +
> +	cpu_entry->mailbox_addr = p->parked_address;
> +	cpu_entry->version = p->parking_version;
> +	cpu_entry->gic_cpu_id = p->cpu_interface_number;
> +}
> +
> +bool __init acpi_parking_protocol_valid(int cpu)
> +{
> +	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> +
> +	return cpu_entry->mailbox_addr && cpu_entry->version;
> +}
> +
> +static int acpi_parking_protocol_cpu_init(unsigned int cpu)
> +{
> +	pr_debug("%s: ACPI parked addr=%llx\n", __func__,
> +		  cpu_mailbox_entries[cpu].mailbox_addr);
> +
> +	return 0;
> +}
> +
> +static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
> +{
> +	return 0;
> +}
> +
> +struct parking_protocol_mailbox {
> +	__le32 cpu_id;
> +	__le32 reserved;
> +	__le64 entry_point;
> +};
> +
> +static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
> +{
> +	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> +	struct parking_protocol_mailbox __iomem *mailbox;
> +	__le32 cpu_id;
> +
> +	/*
> +	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
> +	 * this deviates from the parking protocol specifications since
> +	 * the mailboxes are required to be mapped nGnRnE; the attribute
> +	 * discrepancy is harmless insofar as the protocol specification
> +	 * is concerned).
> +	 * If the mailbox is mistakenly allocated in the linear mapping
> +	 * by FW ioremap will fail since the mapping will be prevented
> +	 * by the kernel (it clashes with the linear mapping attributes
> +	 * specifications).
> +	 */
> +	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> +	if (!mailbox)
> +		return -EIO;
> +
> +	cpu_id = readl_relaxed(&mailbox->cpu_id);
> +	/*
> +	 * Check if firmware has set-up the mailbox entry properly
> +	 * before kickstarting the respective cpu.
> +	 */
> +	if (cpu_id != ~0U) {
> +		iounmap(mailbox);
> +		return -ENXIO;
> +	}
> +
> +	/*
> +	 * We write the entry point and cpu id as LE regardless of the
> +	 * native endianness of the kernel. Therefore, any boot-loaders
> +	 * that read this address need to convert this address to the
> +	 * Boot-Loader's endianness before jumping.
> +	 */
> +	writeq_relaxed(__pa(secondary_entry), &mailbox->entry_point);
> +	writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id);
> +
> +	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
> +
> +	iounmap(mailbox);
> +
> +	return 0;
> +}
> +
> +static void acpi_parking_protocol_cpu_postboot(void)
> +{
> +	int cpu = smp_processor_id();
> +	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> +	struct parking_protocol_mailbox __iomem *mailbox;
> +	__le64 entry_point;
> +
> +	/*
> +	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
> +	 * this deviates from the parking protocol specifications since
> +	 * the mailboxes are required to be mapped nGnRnE; the attribute
> +	 * discrepancy is harmless insofar as the protocol specification
> +	 * is concerned).
> +	 * If the mailbox is mistakenly allocated in the linear mapping
> +	 * by FW ioremap will fail since the mapping will be prevented
> +	 * by the kernel (it clashes with the linear mapping attributes
> +	 * specifications).
> +	 */
> +	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> +	if (!mailbox)
> +		return;
> +
> +	entry_point = readl_relaxed(&mailbox->entry_point);
> +	/*
> +	 * Check if firmware has cleared the entry_point as expected
> +	 * by the protocol specification.
> +	 */
> +	WARN_ON(entry_point);
> +
> +	iounmap(mailbox);
> +}
> +
> +const struct cpu_operations acpi_parking_protocol_ops = {
> +	.name		= "parking-protocol",
> +	.cpu_init	= acpi_parking_protocol_cpu_init,
> +	.cpu_prepare	= acpi_parking_protocol_cpu_prepare,
> +	.cpu_boot	= acpi_parking_protocol_cpu_boot,
> +	.cpu_postboot	= acpi_parking_protocol_cpu_postboot
> +};
> diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
> index b6bd7d4..c7cfb8f 100644
> --- a/arch/arm64/kernel/cpu_ops.c
> +++ b/arch/arm64/kernel/cpu_ops.c
> @@ -25,19 +25,30 @@
>  #include <asm/smp_plat.h>
>  
>  extern const struct cpu_operations smp_spin_table_ops;
> +extern const struct cpu_operations acpi_parking_protocol_ops;
>  extern const struct cpu_operations cpu_psci_ops;
>  
>  const struct cpu_operations *cpu_ops[NR_CPUS];
>  
> -static const struct cpu_operations *supported_cpu_ops[] __initconst = {
> +static const struct cpu_operations *dt_supported_cpu_ops[] __initconst = {
>  	&smp_spin_table_ops,
>  	&cpu_psci_ops,
>  	NULL,
>  };
>  
> +static const struct cpu_operations *acpi_supported_cpu_ops[] __initconst = {
> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> +	&acpi_parking_protocol_ops,
> +#endif
> +	&cpu_psci_ops,
> +	NULL,
> +};
> +
>  static const struct cpu_operations * __init cpu_get_ops(const char *name)
>  {
> -	const struct cpu_operations **ops = supported_cpu_ops;
> +	const struct cpu_operations **ops;
> +
> +	ops = acpi_disabled ? dt_supported_cpu_ops : acpi_supported_cpu_ops;
>  
>  	while (*ops) {
>  		if (!strcmp(name, (*ops)->name))
> @@ -75,8 +86,16 @@ static const char *__init cpu_read_enable_method(int cpu)
>  		}
>  	} else {
>  		enable_method = acpi_get_enable_method(cpu);
> -		if (!enable_method)
> -			pr_err("Unsupported ACPI enable-method\n");
> +		if (!enable_method) {
> +			/*
> +			 * In ACPI systems the boot CPU does not require
> +			 * checking the enable method since for some
> +			 * boot protocol (ie parking protocol) it need not
> +			 * be initialized. Don't warn spuriously.
> +			 */
> +			if (cpu != 0)
> +				pr_err("Unsupported ACPI enable-method\n");
> +		}
>  	}
>  
>  	return enable_method;
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index b1adc51..6f608af 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -70,6 +70,7 @@ enum ipi_msg_type {
>  	IPI_CPU_STOP,
>  	IPI_TIMER,
>  	IPI_IRQ_WORK,
> +	IPI_WAKEUP
>  };
>  
>  /*
> @@ -445,6 +446,17 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
>  	/* map the logical cpu id to cpu MPIDR */
>  	cpu_logical_map(cpu_count) = hwid;
>  
> +	/*
> +	 * Set-up the ACPI parking protocol cpu entries
> +	 * while initializing the cpu_logical_map to
> +	 * avoid parsing MADT entries multiple times for
> +	 * nothing (ie a valid cpu_logical_map entry should
> +	 * contain a valid parking protocol data set to
> +	 * initialize the cpu if the parking protocol is
> +	 * the only available enable method).
> +	 */
> +	acpi_set_mailbox_entry(cpu_count, processor);
> +
>  	cpu_count++;
>  }
>  
> @@ -627,6 +639,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
>  	S(IPI_CPU_STOP, "CPU stop interrupts"),
>  	S(IPI_TIMER, "Timer broadcast interrupts"),
>  	S(IPI_IRQ_WORK, "IRQ work interrupts"),
> +	S(IPI_WAKEUP, "CPU wakeup interrupts"),
>  };
>  
>  static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
> @@ -670,6 +683,13 @@ void arch_send_call_function_single_ipi(int cpu)
>  	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC);
>  }
>  
> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> +void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
> +{
> +	smp_cross_call(mask, IPI_WAKEUP);
> +}
> +#endif
> +
>  #ifdef CONFIG_IRQ_WORK
>  void arch_irq_work_raise(void)
>  {
> @@ -746,6 +766,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
>  		irq_exit();
>  		break;
>  #endif
> +	case IPI_WAKEUP:
> +		break;
>  
>  	default:
>  		pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
> -- 
> 2.5.1
> 

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-01-26 11:10 ` Lorenzo Pieralisi
@ 2016-01-26 23:13   ` Loc Ho
  -1 siblings, 0 replies; 32+ messages in thread
From: Loc Ho @ 2016-01-26 23:13 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-arm-kernel, linux-acpi, Will Deacon, Hanjun Guo,
	Sudeep Holla, Catalin Marinas, Mark Rutland, Mark Salter,
	Al Stone

Hi Lorenzo,

On Tue, Jan 26, 2016 at 3:10 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> The SBBR and ACPI specifications allow ACPI based systems that do not
> implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> protocol specification[1].
>
> This patch implements the ACPI parking protocol CPU operations, and adds
> code that eases parsing the parking protocol data structures to the
> ARM64 SMP initializion carried out at the same time as cpus enumeration.
>
> To wake-up the CPUs from the parked state, this patch implements a
> wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> ARM one, so that a specific IPI is sent for wake-up purpose in order
> to distinguish it from other IPI sources.
>
> Given the current ACPI MADT parsing API, the patch implements a glue
> layer that helps passing MADT GICC data structure from SMP initialization
> code to the parking protocol implementation somewhat overriding the CPU
> operations interfaces. This to avoid creating a completely trasparent
> DT/ACPI CPU operations layer that would require creating opaque
> structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> through static MADT table entries), which seems overkill given that ACPI
> on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> so there is no need for further protocol additions.
>
> Based on the original work by Mark Salter <msalter@redhat.com>
>
> [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Loc Ho <lho@apm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Al Stone <ahs3@redhat.com>

I had tested this with X-Gene Mustang board. It applies cleanly
against 4.5.0-rc1 and detected all CPU's. For those of you want to try
this, you need an FW update if you are using APM Tianocore. Otherwise,
it will stuck in very early booting stage.

Thanks,
Loc

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-01-26 23:13   ` Loc Ho
  0 siblings, 0 replies; 32+ messages in thread
From: Loc Ho @ 2016-01-26 23:13 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Lorenzo,

On Tue, Jan 26, 2016 at 3:10 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> The SBBR and ACPI specifications allow ACPI based systems that do not
> implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> protocol specification[1].
>
> This patch implements the ACPI parking protocol CPU operations, and adds
> code that eases parsing the parking protocol data structures to the
> ARM64 SMP initializion carried out at the same time as cpus enumeration.
>
> To wake-up the CPUs from the parked state, this patch implements a
> wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> ARM one, so that a specific IPI is sent for wake-up purpose in order
> to distinguish it from other IPI sources.
>
> Given the current ACPI MADT parsing API, the patch implements a glue
> layer that helps passing MADT GICC data structure from SMP initialization
> code to the parking protocol implementation somewhat overriding the CPU
> operations interfaces. This to avoid creating a completely trasparent
> DT/ACPI CPU operations layer that would require creating opaque
> structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> through static MADT table entries), which seems overkill given that ACPI
> on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> so there is no need for further protocol additions.
>
> Based on the original work by Mark Salter <msalter@redhat.com>
>
> [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Loc Ho <lho@apm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Al Stone <ahs3@redhat.com>

I had tested this with X-Gene Mustang board. It applies cleanly
against 4.5.0-rc1 and detected all CPU's. For those of you want to try
this, you need an FW update if you are using APM Tianocore. Otherwise,
it will stuck in very early booting stage.

Thanks,
Loc

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-01-26 17:47   ` Lorenzo Pieralisi
@ 2016-01-27 10:23     ` Ard Biesheuvel
  -1 siblings, 0 replies; 32+ messages in thread
From: Ard Biesheuvel @ 2016-01-27 10:23 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-arm-kernel, linux-acpi, Will Deacon, Hanjun Guo, Loc Ho,
	Sudeep Holla, Catalin Marinas, Mark Rutland, Mark Salter,
	Al Stone, Leif Lindholm

On 26 January 2016 at 18:47, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> [+ Ard, Leif]
>
> On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
>> The SBBR and ACPI specifications allow ACPI based systems that do not
>> implement PSCI (eg systems with no EL3) to boot through the ACPI parking
>> protocol specification[1].
>>
>> This patch implements the ACPI parking protocol CPU operations, and adds
>> code that eases parsing the parking protocol data structures to the
>> ARM64 SMP initializion carried out at the same time as cpus enumeration.
>>
>> To wake-up the CPUs from the parked state, this patch implements a
>> wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
>> ARM one, so that a specific IPI is sent for wake-up purpose in order
>> to distinguish it from other IPI sources.
>>
>> Given the current ACPI MADT parsing API, the patch implements a glue
>> layer that helps passing MADT GICC data structure from SMP initialization
>> code to the parking protocol implementation somewhat overriding the CPU
>> operations interfaces. This to avoid creating a completely trasparent
>> DT/ACPI CPU operations layer that would require creating opaque
>> structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
>> through static MADT table entries), which seems overkill given that ACPI
>> on ARM64 mandates only two booting protocols (PSCI and parking protocol),
>> so there is no need for further protocol additions.
>>
>> Based on the original work by Mark Salter <msalter@redhat.com>
>>
>> [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
>>
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Hanjun Guo <hanjun.guo@linaro.org>
>> Cc: Loc Ho <lho@apm.com>
>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Mark Salter <msalter@redhat.com>
>> Cc: Al Stone <ahs3@redhat.com>
>> ---
>> v2->v3
>>
>> - Rebased against v4.5-rc1
>
> It is to report that this patch currently works on my AMD Seattle with
> a v4.5-rc1 kernel whilst it was not working before owing to FW putting
> the mailboxes in WB memory (that was part of linear mapping, I still have
> to pinpoint the exact reason). I think it is related to recent changes in
> EFI and memblock. The code in this patch is allowed to map mailboxes
> through ioremap() (ie it was not before because IIUC pfn_valid was returning
> true on the mailboxes addresses), which it looks like it changed with:
>
> 68709f45385a ("arm64: only consider memblocks with NOMAP cleared for
> linear mapping")
>
> Basically we end up mapping a memory area with ioremap with attributes
> that may mismatch EFI descriptors, on my AMD Seattle the mailboxes are
> part of runtime services data that is WB (see below).
>

The '|WB|WT|WC|UC]' means it can be mapped in various ways, and since
we dropped it from the linear mapping, it should tolerate being mapped
as device memory, with the caveat that, since it is tagged as
EFI_MEMORY_RUNTIME as well, it will be mapped cacheable during
invocations to EFI runtime services.

So the issue is not in the memory types it exposes, but in the fact
that it is listed as a region that is subject to VA remapping.

> I think the only way I can prevent this is by looking up the
> mailboxes addresses in the EFI memory map and bail out if the
> memory descriptor is marked as WB (according to the ACPI parking
> protocol the mailboxes must be Device-nGnRnE).
>

This is a firmware bug, and should be caught at the ACPI/UEFI
validation level. I am not sure we have to special case this in the
kernel.

-- 
Ard.



> AMD Seattle snip boot log with this patch applied and some debug output
> below (ie mailbox address falls within "Runtime Data").
>
> I will keep debugging, comments appreciated.
>
> Thanks,
> Lorenzo
>
> [    0.000000] Processing EFI memory map:
>
> <snip>
>
> [    0.000000]   0x008028000000-0x0080280fffff [Runtime Data       |RUN|  |  |  |  |  |   |WB|WT|WC|UC]*
> [    0.000000] acpi_parking_protocol_cpu_init: ACPI parked addr=80280c1000
>
>> - Moved Kconfig option under "Boot options"
>>
>> v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/369121.html
>>
>> v1->v2
>>
>> - Rebased against v4.2
>> - Removed SMP dependency (it was removed from arm64)
>> - Updated some comments
>> - Clarified 64k page mailbox alignment and requested UEFI specs update
>>
>> v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356750.html
>>
>>  arch/arm64/Kconfig                        |   9 ++
>>  arch/arm64/include/asm/acpi.h             |  19 +++-
>>  arch/arm64/include/asm/hardirq.h          |   2 +-
>>  arch/arm64/include/asm/smp.h              |   9 ++
>>  arch/arm64/kernel/Makefile                |   1 +
>>  arch/arm64/kernel/acpi_parking_protocol.c | 153 ++++++++++++++++++++++++++++++
>>  arch/arm64/kernel/cpu_ops.c               |  27 +++++-
>>  arch/arm64/kernel/smp.c                   |  22 +++++
>>  8 files changed, 236 insertions(+), 6 deletions(-)
>>  create mode 100644 arch/arm64/kernel/acpi_parking_protocol.c
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 8cc6228..53e48a6 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -756,6 +756,15 @@ endmenu
>>
>>  menu "Boot options"
>>
>> +config ARM64_ACPI_PARKING_PROTOCOL
>> +     bool "Enable support for the ARM64 ACPI parking protocol"
>> +     depends on ACPI
>> +     help
>> +       Enable support for the ARM64 ACPI parking protocol. If disabled
>> +       the kernel will not allow booting through the ARM64 ACPI parking
>> +       protocol even if the corresponding data is present in the ACPI
>> +       MADT table.
>> +
>>  config CMDLINE
>>       string "Default kernel command string"
>>       default ""
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index caafd63..aee323b 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -87,9 +87,26 @@ void __init acpi_init_cpus(void);
>>  static inline void acpi_init_cpus(void) { }
>>  #endif /* CONFIG_ACPI */
>>
>> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
>> +bool acpi_parking_protocol_valid(int cpu);
>> +void __init
>> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor);
>> +#else
>> +static inline bool acpi_parking_protocol_valid(int cpu) { return false; }
>> +static inline void
>> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
>> +{}
>> +#endif
>> +
>>  static inline const char *acpi_get_enable_method(int cpu)
>>  {
>> -     return acpi_psci_present() ? "psci" : NULL;
>> +     if (acpi_psci_present())
>> +             return "psci";
>> +
>> +     if (acpi_parking_protocol_valid(cpu))
>> +             return "parking-protocol";
>> +
>> +     return NULL;
>>  }
>>
>>  #ifdef       CONFIG_ACPI_APEI
>> diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
>> index a57601f..8740297 100644
>> --- a/arch/arm64/include/asm/hardirq.h
>> +++ b/arch/arm64/include/asm/hardirq.h
>> @@ -20,7 +20,7 @@
>>  #include <linux/threads.h>
>>  #include <asm/irq.h>
>>
>> -#define NR_IPI       5
>> +#define NR_IPI       6
>>
>>  typedef struct {
>>       unsigned int __softirq_pending;
>> diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
>> index d9c3d6a..2013a4d 100644
>> --- a/arch/arm64/include/asm/smp.h
>> +++ b/arch/arm64/include/asm/smp.h
>> @@ -64,6 +64,15 @@ extern void secondary_entry(void);
>>  extern void arch_send_call_function_single_ipi(int cpu);
>>  extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
>>
>> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
>> +extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
>> +#else
>> +static inline void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
>> +{
>> +     BUILD_BUG();
>> +}
>> +#endif
>> +
>>  extern int __cpu_disable(void);
>>
>>  extern void __cpu_die(unsigned int cpu);
>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>> index 83cd7e6..8a9c65c 100644
>> --- a/arch/arm64/kernel/Makefile
>> +++ b/arch/arm64/kernel/Makefile
>> @@ -41,6 +41,7 @@ arm64-obj-$(CONFIG_EFI)                     += efi.o efi-entry.stub.o
>>  arm64-obj-$(CONFIG_PCI)                      += pci.o
>>  arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o
>>  arm64-obj-$(CONFIG_ACPI)             += acpi.o
>> +arm64-obj-$(CONFIG_ARM64_ACPI_PARKING_PROTOCOL)      += acpi_parking_protocol.o
>>  arm64-obj-$(CONFIG_PARAVIRT)         += paravirt.o
>>
>>  obj-y                                        += $(arm64-obj-y) vdso/
>> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
>> new file mode 100644
>> index 0000000..531c3ad
>> --- /dev/null
>> +++ b/arch/arm64/kernel/acpi_parking_protocol.c
>> @@ -0,0 +1,153 @@
>> +/*
>> + * ARM64 ACPI Parking Protocol implementation
>> + *
>> + * Authors: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> + *       Mark Salter <msalter@redhat.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/acpi.h>
>> +#include <linux/types.h>
>> +
>> +#include <asm/cpu_ops.h>
>> +
>> +struct cpu_mailbox_entry {
>> +     phys_addr_t mailbox_addr;
>> +     u8 version;
>> +     u8 gic_cpu_id;
>> +};
>> +
>> +static struct cpu_mailbox_entry cpu_mailbox_entries[NR_CPUS];
>> +
>> +void __init acpi_set_mailbox_entry(int cpu,
>> +                                struct acpi_madt_generic_interrupt *p)
>> +{
>> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
>> +
>> +     cpu_entry->mailbox_addr = p->parked_address;
>> +     cpu_entry->version = p->parking_version;
>> +     cpu_entry->gic_cpu_id = p->cpu_interface_number;
>> +}
>> +
>> +bool __init acpi_parking_protocol_valid(int cpu)
>> +{
>> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
>> +
>> +     return cpu_entry->mailbox_addr && cpu_entry->version;
>> +}
>> +
>> +static int acpi_parking_protocol_cpu_init(unsigned int cpu)
>> +{
>> +     pr_debug("%s: ACPI parked addr=%llx\n", __func__,
>> +               cpu_mailbox_entries[cpu].mailbox_addr);
>> +
>> +     return 0;
>> +}
>> +
>> +static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
>> +{
>> +     return 0;
>> +}
>> +
>> +struct parking_protocol_mailbox {
>> +     __le32 cpu_id;
>> +     __le32 reserved;
>> +     __le64 entry_point;
>> +};
>> +
>> +static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>> +{
>> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
>> +     struct parking_protocol_mailbox __iomem *mailbox;
>> +     __le32 cpu_id;
>> +
>> +     /*
>> +      * Map mailbox memory with attribute device nGnRE (ie ioremap -
>> +      * this deviates from the parking protocol specifications since
>> +      * the mailboxes are required to be mapped nGnRnE; the attribute
>> +      * discrepancy is harmless insofar as the protocol specification
>> +      * is concerned).
>> +      * If the mailbox is mistakenly allocated in the linear mapping
>> +      * by FW ioremap will fail since the mapping will be prevented
>> +      * by the kernel (it clashes with the linear mapping attributes
>> +      * specifications).
>> +      */
>> +     mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
>> +     if (!mailbox)
>> +             return -EIO;
>> +
>> +     cpu_id = readl_relaxed(&mailbox->cpu_id);
>> +     /*
>> +      * Check if firmware has set-up the mailbox entry properly
>> +      * before kickstarting the respective cpu.
>> +      */
>> +     if (cpu_id != ~0U) {
>> +             iounmap(mailbox);
>> +             return -ENXIO;
>> +     }
>> +
>> +     /*
>> +      * We write the entry point and cpu id as LE regardless of the
>> +      * native endianness of the kernel. Therefore, any boot-loaders
>> +      * that read this address need to convert this address to the
>> +      * Boot-Loader's endianness before jumping.
>> +      */
>> +     writeq_relaxed(__pa(secondary_entry), &mailbox->entry_point);
>> +     writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id);
>> +
>> +     arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>> +
>> +     iounmap(mailbox);
>> +
>> +     return 0;
>> +}
>> +
>> +static void acpi_parking_protocol_cpu_postboot(void)
>> +{
>> +     int cpu = smp_processor_id();
>> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
>> +     struct parking_protocol_mailbox __iomem *mailbox;
>> +     __le64 entry_point;
>> +
>> +     /*
>> +      * Map mailbox memory with attribute device nGnRE (ie ioremap -
>> +      * this deviates from the parking protocol specifications since
>> +      * the mailboxes are required to be mapped nGnRnE; the attribute
>> +      * discrepancy is harmless insofar as the protocol specification
>> +      * is concerned).
>> +      * If the mailbox is mistakenly allocated in the linear mapping
>> +      * by FW ioremap will fail since the mapping will be prevented
>> +      * by the kernel (it clashes with the linear mapping attributes
>> +      * specifications).
>> +      */
>> +     mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
>> +     if (!mailbox)
>> +             return;
>> +
>> +     entry_point = readl_relaxed(&mailbox->entry_point);
>> +     /*
>> +      * Check if firmware has cleared the entry_point as expected
>> +      * by the protocol specification.
>> +      */
>> +     WARN_ON(entry_point);
>> +
>> +     iounmap(mailbox);
>> +}
>> +
>> +const struct cpu_operations acpi_parking_protocol_ops = {
>> +     .name           = "parking-protocol",
>> +     .cpu_init       = acpi_parking_protocol_cpu_init,
>> +     .cpu_prepare    = acpi_parking_protocol_cpu_prepare,
>> +     .cpu_boot       = acpi_parking_protocol_cpu_boot,
>> +     .cpu_postboot   = acpi_parking_protocol_cpu_postboot
>> +};
>> diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
>> index b6bd7d4..c7cfb8f 100644
>> --- a/arch/arm64/kernel/cpu_ops.c
>> +++ b/arch/arm64/kernel/cpu_ops.c
>> @@ -25,19 +25,30 @@
>>  #include <asm/smp_plat.h>
>>
>>  extern const struct cpu_operations smp_spin_table_ops;
>> +extern const struct cpu_operations acpi_parking_protocol_ops;
>>  extern const struct cpu_operations cpu_psci_ops;
>>
>>  const struct cpu_operations *cpu_ops[NR_CPUS];
>>
>> -static const struct cpu_operations *supported_cpu_ops[] __initconst = {
>> +static const struct cpu_operations *dt_supported_cpu_ops[] __initconst = {
>>       &smp_spin_table_ops,
>>       &cpu_psci_ops,
>>       NULL,
>>  };
>>
>> +static const struct cpu_operations *acpi_supported_cpu_ops[] __initconst = {
>> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
>> +     &acpi_parking_protocol_ops,
>> +#endif
>> +     &cpu_psci_ops,
>> +     NULL,
>> +};
>> +
>>  static const struct cpu_operations * __init cpu_get_ops(const char *name)
>>  {
>> -     const struct cpu_operations **ops = supported_cpu_ops;
>> +     const struct cpu_operations **ops;
>> +
>> +     ops = acpi_disabled ? dt_supported_cpu_ops : acpi_supported_cpu_ops;
>>
>>       while (*ops) {
>>               if (!strcmp(name, (*ops)->name))
>> @@ -75,8 +86,16 @@ static const char *__init cpu_read_enable_method(int cpu)
>>               }
>>       } else {
>>               enable_method = acpi_get_enable_method(cpu);
>> -             if (!enable_method)
>> -                     pr_err("Unsupported ACPI enable-method\n");
>> +             if (!enable_method) {
>> +                     /*
>> +                      * In ACPI systems the boot CPU does not require
>> +                      * checking the enable method since for some
>> +                      * boot protocol (ie parking protocol) it need not
>> +                      * be initialized. Don't warn spuriously.
>> +                      */
>> +                     if (cpu != 0)
>> +                             pr_err("Unsupported ACPI enable-method\n");
>> +             }
>>       }
>>
>>       return enable_method;
>> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
>> index b1adc51..6f608af 100644
>> --- a/arch/arm64/kernel/smp.c
>> +++ b/arch/arm64/kernel/smp.c
>> @@ -70,6 +70,7 @@ enum ipi_msg_type {
>>       IPI_CPU_STOP,
>>       IPI_TIMER,
>>       IPI_IRQ_WORK,
>> +     IPI_WAKEUP
>>  };
>>
>>  /*
>> @@ -445,6 +446,17 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
>>       /* map the logical cpu id to cpu MPIDR */
>>       cpu_logical_map(cpu_count) = hwid;
>>
>> +     /*
>> +      * Set-up the ACPI parking protocol cpu entries
>> +      * while initializing the cpu_logical_map to
>> +      * avoid parsing MADT entries multiple times for
>> +      * nothing (ie a valid cpu_logical_map entry should
>> +      * contain a valid parking protocol data set to
>> +      * initialize the cpu if the parking protocol is
>> +      * the only available enable method).
>> +      */
>> +     acpi_set_mailbox_entry(cpu_count, processor);
>> +
>>       cpu_count++;
>>  }
>>
>> @@ -627,6 +639,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
>>       S(IPI_CPU_STOP, "CPU stop interrupts"),
>>       S(IPI_TIMER, "Timer broadcast interrupts"),
>>       S(IPI_IRQ_WORK, "IRQ work interrupts"),
>> +     S(IPI_WAKEUP, "CPU wakeup interrupts"),
>>  };
>>
>>  static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
>> @@ -670,6 +683,13 @@ void arch_send_call_function_single_ipi(int cpu)
>>       smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC);
>>  }
>>
>> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
>> +void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
>> +{
>> +     smp_cross_call(mask, IPI_WAKEUP);
>> +}
>> +#endif
>> +
>>  #ifdef CONFIG_IRQ_WORK
>>  void arch_irq_work_raise(void)
>>  {
>> @@ -746,6 +766,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
>>               irq_exit();
>>               break;
>>  #endif
>> +     case IPI_WAKEUP:
>> +             break;
>>
>>       default:
>>               pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
>> --
>> 2.5.1
>>

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-01-27 10:23     ` Ard Biesheuvel
  0 siblings, 0 replies; 32+ messages in thread
From: Ard Biesheuvel @ 2016-01-27 10:23 UTC (permalink / raw)
  To: linux-arm-kernel

On 26 January 2016 at 18:47, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> [+ Ard, Leif]
>
> On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
>> The SBBR and ACPI specifications allow ACPI based systems that do not
>> implement PSCI (eg systems with no EL3) to boot through the ACPI parking
>> protocol specification[1].
>>
>> This patch implements the ACPI parking protocol CPU operations, and adds
>> code that eases parsing the parking protocol data structures to the
>> ARM64 SMP initializion carried out at the same time as cpus enumeration.
>>
>> To wake-up the CPUs from the parked state, this patch implements a
>> wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
>> ARM one, so that a specific IPI is sent for wake-up purpose in order
>> to distinguish it from other IPI sources.
>>
>> Given the current ACPI MADT parsing API, the patch implements a glue
>> layer that helps passing MADT GICC data structure from SMP initialization
>> code to the parking protocol implementation somewhat overriding the CPU
>> operations interfaces. This to avoid creating a completely trasparent
>> DT/ACPI CPU operations layer that would require creating opaque
>> structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
>> through static MADT table entries), which seems overkill given that ACPI
>> on ARM64 mandates only two booting protocols (PSCI and parking protocol),
>> so there is no need for further protocol additions.
>>
>> Based on the original work by Mark Salter <msalter@redhat.com>
>>
>> [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
>>
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Hanjun Guo <hanjun.guo@linaro.org>
>> Cc: Loc Ho <lho@apm.com>
>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Mark Salter <msalter@redhat.com>
>> Cc: Al Stone <ahs3@redhat.com>
>> ---
>> v2->v3
>>
>> - Rebased against v4.5-rc1
>
> It is to report that this patch currently works on my AMD Seattle with
> a v4.5-rc1 kernel whilst it was not working before owing to FW putting
> the mailboxes in WB memory (that was part of linear mapping, I still have
> to pinpoint the exact reason). I think it is related to recent changes in
> EFI and memblock. The code in this patch is allowed to map mailboxes
> through ioremap() (ie it was not before because IIUC pfn_valid was returning
> true on the mailboxes addresses), which it looks like it changed with:
>
> 68709f45385a ("arm64: only consider memblocks with NOMAP cleared for
> linear mapping")
>
> Basically we end up mapping a memory area with ioremap with attributes
> that may mismatch EFI descriptors, on my AMD Seattle the mailboxes are
> part of runtime services data that is WB (see below).
>

The '|WB|WT|WC|UC]' means it can be mapped in various ways, and since
we dropped it from the linear mapping, it should tolerate being mapped
as device memory, with the caveat that, since it is tagged as
EFI_MEMORY_RUNTIME as well, it will be mapped cacheable during
invocations to EFI runtime services.

So the issue is not in the memory types it exposes, but in the fact
that it is listed as a region that is subject to VA remapping.

> I think the only way I can prevent this is by looking up the
> mailboxes addresses in the EFI memory map and bail out if the
> memory descriptor is marked as WB (according to the ACPI parking
> protocol the mailboxes must be Device-nGnRnE).
>

This is a firmware bug, and should be caught at the ACPI/UEFI
validation level. I am not sure we have to special case this in the
kernel.

-- 
Ard.



> AMD Seattle snip boot log with this patch applied and some debug output
> below (ie mailbox address falls within "Runtime Data").
>
> I will keep debugging, comments appreciated.
>
> Thanks,
> Lorenzo
>
> [    0.000000] Processing EFI memory map:
>
> <snip>
>
> [    0.000000]   0x008028000000-0x0080280fffff [Runtime Data       |RUN|  |  |  |  |  |   |WB|WT|WC|UC]*
> [    0.000000] acpi_parking_protocol_cpu_init: ACPI parked addr=80280c1000
>
>> - Moved Kconfig option under "Boot options"
>>
>> v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/369121.html
>>
>> v1->v2
>>
>> - Rebased against v4.2
>> - Removed SMP dependency (it was removed from arm64)
>> - Updated some comments
>> - Clarified 64k page mailbox alignment and requested UEFI specs update
>>
>> v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356750.html
>>
>>  arch/arm64/Kconfig                        |   9 ++
>>  arch/arm64/include/asm/acpi.h             |  19 +++-
>>  arch/arm64/include/asm/hardirq.h          |   2 +-
>>  arch/arm64/include/asm/smp.h              |   9 ++
>>  arch/arm64/kernel/Makefile                |   1 +
>>  arch/arm64/kernel/acpi_parking_protocol.c | 153 ++++++++++++++++++++++++++++++
>>  arch/arm64/kernel/cpu_ops.c               |  27 +++++-
>>  arch/arm64/kernel/smp.c                   |  22 +++++
>>  8 files changed, 236 insertions(+), 6 deletions(-)
>>  create mode 100644 arch/arm64/kernel/acpi_parking_protocol.c
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 8cc6228..53e48a6 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -756,6 +756,15 @@ endmenu
>>
>>  menu "Boot options"
>>
>> +config ARM64_ACPI_PARKING_PROTOCOL
>> +     bool "Enable support for the ARM64 ACPI parking protocol"
>> +     depends on ACPI
>> +     help
>> +       Enable support for the ARM64 ACPI parking protocol. If disabled
>> +       the kernel will not allow booting through the ARM64 ACPI parking
>> +       protocol even if the corresponding data is present in the ACPI
>> +       MADT table.
>> +
>>  config CMDLINE
>>       string "Default kernel command string"
>>       default ""
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index caafd63..aee323b 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -87,9 +87,26 @@ void __init acpi_init_cpus(void);
>>  static inline void acpi_init_cpus(void) { }
>>  #endif /* CONFIG_ACPI */
>>
>> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
>> +bool acpi_parking_protocol_valid(int cpu);
>> +void __init
>> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor);
>> +#else
>> +static inline bool acpi_parking_protocol_valid(int cpu) { return false; }
>> +static inline void
>> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
>> +{}
>> +#endif
>> +
>>  static inline const char *acpi_get_enable_method(int cpu)
>>  {
>> -     return acpi_psci_present() ? "psci" : NULL;
>> +     if (acpi_psci_present())
>> +             return "psci";
>> +
>> +     if (acpi_parking_protocol_valid(cpu))
>> +             return "parking-protocol";
>> +
>> +     return NULL;
>>  }
>>
>>  #ifdef       CONFIG_ACPI_APEI
>> diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
>> index a57601f..8740297 100644
>> --- a/arch/arm64/include/asm/hardirq.h
>> +++ b/arch/arm64/include/asm/hardirq.h
>> @@ -20,7 +20,7 @@
>>  #include <linux/threads.h>
>>  #include <asm/irq.h>
>>
>> -#define NR_IPI       5
>> +#define NR_IPI       6
>>
>>  typedef struct {
>>       unsigned int __softirq_pending;
>> diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
>> index d9c3d6a..2013a4d 100644
>> --- a/arch/arm64/include/asm/smp.h
>> +++ b/arch/arm64/include/asm/smp.h
>> @@ -64,6 +64,15 @@ extern void secondary_entry(void);
>>  extern void arch_send_call_function_single_ipi(int cpu);
>>  extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
>>
>> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
>> +extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
>> +#else
>> +static inline void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
>> +{
>> +     BUILD_BUG();
>> +}
>> +#endif
>> +
>>  extern int __cpu_disable(void);
>>
>>  extern void __cpu_die(unsigned int cpu);
>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>> index 83cd7e6..8a9c65c 100644
>> --- a/arch/arm64/kernel/Makefile
>> +++ b/arch/arm64/kernel/Makefile
>> @@ -41,6 +41,7 @@ arm64-obj-$(CONFIG_EFI)                     += efi.o efi-entry.stub.o
>>  arm64-obj-$(CONFIG_PCI)                      += pci.o
>>  arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o
>>  arm64-obj-$(CONFIG_ACPI)             += acpi.o
>> +arm64-obj-$(CONFIG_ARM64_ACPI_PARKING_PROTOCOL)      += acpi_parking_protocol.o
>>  arm64-obj-$(CONFIG_PARAVIRT)         += paravirt.o
>>
>>  obj-y                                        += $(arm64-obj-y) vdso/
>> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
>> new file mode 100644
>> index 0000000..531c3ad
>> --- /dev/null
>> +++ b/arch/arm64/kernel/acpi_parking_protocol.c
>> @@ -0,0 +1,153 @@
>> +/*
>> + * ARM64 ACPI Parking Protocol implementation
>> + *
>> + * Authors: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> + *       Mark Salter <msalter@redhat.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/acpi.h>
>> +#include <linux/types.h>
>> +
>> +#include <asm/cpu_ops.h>
>> +
>> +struct cpu_mailbox_entry {
>> +     phys_addr_t mailbox_addr;
>> +     u8 version;
>> +     u8 gic_cpu_id;
>> +};
>> +
>> +static struct cpu_mailbox_entry cpu_mailbox_entries[NR_CPUS];
>> +
>> +void __init acpi_set_mailbox_entry(int cpu,
>> +                                struct acpi_madt_generic_interrupt *p)
>> +{
>> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
>> +
>> +     cpu_entry->mailbox_addr = p->parked_address;
>> +     cpu_entry->version = p->parking_version;
>> +     cpu_entry->gic_cpu_id = p->cpu_interface_number;
>> +}
>> +
>> +bool __init acpi_parking_protocol_valid(int cpu)
>> +{
>> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
>> +
>> +     return cpu_entry->mailbox_addr && cpu_entry->version;
>> +}
>> +
>> +static int acpi_parking_protocol_cpu_init(unsigned int cpu)
>> +{
>> +     pr_debug("%s: ACPI parked addr=%llx\n", __func__,
>> +               cpu_mailbox_entries[cpu].mailbox_addr);
>> +
>> +     return 0;
>> +}
>> +
>> +static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
>> +{
>> +     return 0;
>> +}
>> +
>> +struct parking_protocol_mailbox {
>> +     __le32 cpu_id;
>> +     __le32 reserved;
>> +     __le64 entry_point;
>> +};
>> +
>> +static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>> +{
>> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
>> +     struct parking_protocol_mailbox __iomem *mailbox;
>> +     __le32 cpu_id;
>> +
>> +     /*
>> +      * Map mailbox memory with attribute device nGnRE (ie ioremap -
>> +      * this deviates from the parking protocol specifications since
>> +      * the mailboxes are required to be mapped nGnRnE; the attribute
>> +      * discrepancy is harmless insofar as the protocol specification
>> +      * is concerned).
>> +      * If the mailbox is mistakenly allocated in the linear mapping
>> +      * by FW ioremap will fail since the mapping will be prevented
>> +      * by the kernel (it clashes with the linear mapping attributes
>> +      * specifications).
>> +      */
>> +     mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
>> +     if (!mailbox)
>> +             return -EIO;
>> +
>> +     cpu_id = readl_relaxed(&mailbox->cpu_id);
>> +     /*
>> +      * Check if firmware has set-up the mailbox entry properly
>> +      * before kickstarting the respective cpu.
>> +      */
>> +     if (cpu_id != ~0U) {
>> +             iounmap(mailbox);
>> +             return -ENXIO;
>> +     }
>> +
>> +     /*
>> +      * We write the entry point and cpu id as LE regardless of the
>> +      * native endianness of the kernel. Therefore, any boot-loaders
>> +      * that read this address need to convert this address to the
>> +      * Boot-Loader's endianness before jumping.
>> +      */
>> +     writeq_relaxed(__pa(secondary_entry), &mailbox->entry_point);
>> +     writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id);
>> +
>> +     arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>> +
>> +     iounmap(mailbox);
>> +
>> +     return 0;
>> +}
>> +
>> +static void acpi_parking_protocol_cpu_postboot(void)
>> +{
>> +     int cpu = smp_processor_id();
>> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
>> +     struct parking_protocol_mailbox __iomem *mailbox;
>> +     __le64 entry_point;
>> +
>> +     /*
>> +      * Map mailbox memory with attribute device nGnRE (ie ioremap -
>> +      * this deviates from the parking protocol specifications since
>> +      * the mailboxes are required to be mapped nGnRnE; the attribute
>> +      * discrepancy is harmless insofar as the protocol specification
>> +      * is concerned).
>> +      * If the mailbox is mistakenly allocated in the linear mapping
>> +      * by FW ioremap will fail since the mapping will be prevented
>> +      * by the kernel (it clashes with the linear mapping attributes
>> +      * specifications).
>> +      */
>> +     mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
>> +     if (!mailbox)
>> +             return;
>> +
>> +     entry_point = readl_relaxed(&mailbox->entry_point);
>> +     /*
>> +      * Check if firmware has cleared the entry_point as expected
>> +      * by the protocol specification.
>> +      */
>> +     WARN_ON(entry_point);
>> +
>> +     iounmap(mailbox);
>> +}
>> +
>> +const struct cpu_operations acpi_parking_protocol_ops = {
>> +     .name           = "parking-protocol",
>> +     .cpu_init       = acpi_parking_protocol_cpu_init,
>> +     .cpu_prepare    = acpi_parking_protocol_cpu_prepare,
>> +     .cpu_boot       = acpi_parking_protocol_cpu_boot,
>> +     .cpu_postboot   = acpi_parking_protocol_cpu_postboot
>> +};
>> diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
>> index b6bd7d4..c7cfb8f 100644
>> --- a/arch/arm64/kernel/cpu_ops.c
>> +++ b/arch/arm64/kernel/cpu_ops.c
>> @@ -25,19 +25,30 @@
>>  #include <asm/smp_plat.h>
>>
>>  extern const struct cpu_operations smp_spin_table_ops;
>> +extern const struct cpu_operations acpi_parking_protocol_ops;
>>  extern const struct cpu_operations cpu_psci_ops;
>>
>>  const struct cpu_operations *cpu_ops[NR_CPUS];
>>
>> -static const struct cpu_operations *supported_cpu_ops[] __initconst = {
>> +static const struct cpu_operations *dt_supported_cpu_ops[] __initconst = {
>>       &smp_spin_table_ops,
>>       &cpu_psci_ops,
>>       NULL,
>>  };
>>
>> +static const struct cpu_operations *acpi_supported_cpu_ops[] __initconst = {
>> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
>> +     &acpi_parking_protocol_ops,
>> +#endif
>> +     &cpu_psci_ops,
>> +     NULL,
>> +};
>> +
>>  static const struct cpu_operations * __init cpu_get_ops(const char *name)
>>  {
>> -     const struct cpu_operations **ops = supported_cpu_ops;
>> +     const struct cpu_operations **ops;
>> +
>> +     ops = acpi_disabled ? dt_supported_cpu_ops : acpi_supported_cpu_ops;
>>
>>       while (*ops) {
>>               if (!strcmp(name, (*ops)->name))
>> @@ -75,8 +86,16 @@ static const char *__init cpu_read_enable_method(int cpu)
>>               }
>>       } else {
>>               enable_method = acpi_get_enable_method(cpu);
>> -             if (!enable_method)
>> -                     pr_err("Unsupported ACPI enable-method\n");
>> +             if (!enable_method) {
>> +                     /*
>> +                      * In ACPI systems the boot CPU does not require
>> +                      * checking the enable method since for some
>> +                      * boot protocol (ie parking protocol) it need not
>> +                      * be initialized. Don't warn spuriously.
>> +                      */
>> +                     if (cpu != 0)
>> +                             pr_err("Unsupported ACPI enable-method\n");
>> +             }
>>       }
>>
>>       return enable_method;
>> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
>> index b1adc51..6f608af 100644
>> --- a/arch/arm64/kernel/smp.c
>> +++ b/arch/arm64/kernel/smp.c
>> @@ -70,6 +70,7 @@ enum ipi_msg_type {
>>       IPI_CPU_STOP,
>>       IPI_TIMER,
>>       IPI_IRQ_WORK,
>> +     IPI_WAKEUP
>>  };
>>
>>  /*
>> @@ -445,6 +446,17 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
>>       /* map the logical cpu id to cpu MPIDR */
>>       cpu_logical_map(cpu_count) = hwid;
>>
>> +     /*
>> +      * Set-up the ACPI parking protocol cpu entries
>> +      * while initializing the cpu_logical_map to
>> +      * avoid parsing MADT entries multiple times for
>> +      * nothing (ie a valid cpu_logical_map entry should
>> +      * contain a valid parking protocol data set to
>> +      * initialize the cpu if the parking protocol is
>> +      * the only available enable method).
>> +      */
>> +     acpi_set_mailbox_entry(cpu_count, processor);
>> +
>>       cpu_count++;
>>  }
>>
>> @@ -627,6 +639,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
>>       S(IPI_CPU_STOP, "CPU stop interrupts"),
>>       S(IPI_TIMER, "Timer broadcast interrupts"),
>>       S(IPI_IRQ_WORK, "IRQ work interrupts"),
>> +     S(IPI_WAKEUP, "CPU wakeup interrupts"),
>>  };
>>
>>  static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
>> @@ -670,6 +683,13 @@ void arch_send_call_function_single_ipi(int cpu)
>>       smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC);
>>  }
>>
>> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
>> +void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
>> +{
>> +     smp_cross_call(mask, IPI_WAKEUP);
>> +}
>> +#endif
>> +
>>  #ifdef CONFIG_IRQ_WORK
>>  void arch_irq_work_raise(void)
>>  {
>> @@ -746,6 +766,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
>>               irq_exit();
>>               break;
>>  #endif
>> +     case IPI_WAKEUP:
>> +             break;
>>
>>       default:
>>               pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
>> --
>> 2.5.1
>>

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-01-27 10:23     ` Ard Biesheuvel
@ 2016-01-27 11:46       ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-27 11:46 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, linux-acpi, Will Deacon, Hanjun Guo, Loc Ho,
	Sudeep Holla, Catalin Marinas, Mark Rutland, Mark Salter,
	Al Stone, Leif Lindholm

On Wed, Jan 27, 2016 at 11:23:41AM +0100, Ard Biesheuvel wrote:

[...]

> > It is to report that this patch currently works on my AMD Seattle with
> > a v4.5-rc1 kernel whilst it was not working before owing to FW putting
> > the mailboxes in WB memory (that was part of linear mapping, I still have
> > to pinpoint the exact reason). I think it is related to recent changes in
> > EFI and memblock. The code in this patch is allowed to map mailboxes
> > through ioremap() (ie it was not before because IIUC pfn_valid was returning
> > true on the mailboxes addresses), which it looks like it changed with:
> >
> > 68709f45385a ("arm64: only consider memblocks with NOMAP cleared for
> > linear mapping")
> >
> > Basically we end up mapping a memory area with ioremap with attributes
> > that may mismatch EFI descriptors, on my AMD Seattle the mailboxes are
> > part of runtime services data that is WB (see below).
> >
> 
> The '|WB|WT|WC|UC]' means it can be mapped in various ways, and since
> we dropped it from the linear mapping, it should tolerate being mapped
> as device memory, with the caveat that, since it is tagged as
> EFI_MEMORY_RUNTIME as well, it will be mapped cacheable during
> invocations to EFI runtime services.
> 
> So the issue is not in the memory types it exposes, but in the fact
> that it is listed as a region that is subject to VA remapping.

Thanks for clarifying and yes, I agree it is a firmware bug, I just
flagged this up to check with you whether we want to do something
about it (ie it is not just related to this patch).

> > I think the only way I can prevent this is by looking up the
> > mailboxes addresses in the EFI memory map and bail out if the
> > memory descriptor is marked as WB (according to the ACPI parking
> > protocol the mailboxes must be Device-nGnRnE).
> >
> 
> This is a firmware bug, and should be caught at the ACPI/UEFI
> validation level. I am not sure we have to special case this in the
> kernel.

I came to the same conclusion, I thought it was worth mentioning
it since with previous kernel versions this mismatch was caught in
the ioremap implementation and now it is not, if a warning has to be
put in place it has to be done separately, it is not an issue
specific to this patch only.

Thanks,
Lorenzo

> 
> -- 
> Ard.
> 
> 
> 
> > AMD Seattle snip boot log with this patch applied and some debug output
> > below (ie mailbox address falls within "Runtime Data").
> >
> > I will keep debugging, comments appreciated.
> >
> > Thanks,
> > Lorenzo
> >
> > [    0.000000] Processing EFI memory map:
> >
> > <snip>
> >
> > [    0.000000]   0x008028000000-0x0080280fffff [Runtime Data       |RUN|  |  |  |  |  |   |WB|WT|WC|UC]*
> > [    0.000000] acpi_parking_protocol_cpu_init: ACPI parked addr=80280c1000
> >
> >> - Moved Kconfig option under "Boot options"
> >>
> >> v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/369121.html
> >>
> >> v1->v2
> >>
> >> - Rebased against v4.2
> >> - Removed SMP dependency (it was removed from arm64)
> >> - Updated some comments
> >> - Clarified 64k page mailbox alignment and requested UEFI specs update
> >>
> >> v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356750.html
> >>
> >>  arch/arm64/Kconfig                        |   9 ++
> >>  arch/arm64/include/asm/acpi.h             |  19 +++-
> >>  arch/arm64/include/asm/hardirq.h          |   2 +-
> >>  arch/arm64/include/asm/smp.h              |   9 ++
> >>  arch/arm64/kernel/Makefile                |   1 +
> >>  arch/arm64/kernel/acpi_parking_protocol.c | 153 ++++++++++++++++++++++++++++++
> >>  arch/arm64/kernel/cpu_ops.c               |  27 +++++-
> >>  arch/arm64/kernel/smp.c                   |  22 +++++
> >>  8 files changed, 236 insertions(+), 6 deletions(-)
> >>  create mode 100644 arch/arm64/kernel/acpi_parking_protocol.c
> >>
> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> >> index 8cc6228..53e48a6 100644
> >> --- a/arch/arm64/Kconfig
> >> +++ b/arch/arm64/Kconfig
> >> @@ -756,6 +756,15 @@ endmenu
> >>
> >>  menu "Boot options"
> >>
> >> +config ARM64_ACPI_PARKING_PROTOCOL
> >> +     bool "Enable support for the ARM64 ACPI parking protocol"
> >> +     depends on ACPI
> >> +     help
> >> +       Enable support for the ARM64 ACPI parking protocol. If disabled
> >> +       the kernel will not allow booting through the ARM64 ACPI parking
> >> +       protocol even if the corresponding data is present in the ACPI
> >> +       MADT table.
> >> +
> >>  config CMDLINE
> >>       string "Default kernel command string"
> >>       default ""
> >> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> >> index caafd63..aee323b 100644
> >> --- a/arch/arm64/include/asm/acpi.h
> >> +++ b/arch/arm64/include/asm/acpi.h
> >> @@ -87,9 +87,26 @@ void __init acpi_init_cpus(void);
> >>  static inline void acpi_init_cpus(void) { }
> >>  #endif /* CONFIG_ACPI */
> >>
> >> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> >> +bool acpi_parking_protocol_valid(int cpu);
> >> +void __init
> >> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor);
> >> +#else
> >> +static inline bool acpi_parking_protocol_valid(int cpu) { return false; }
> >> +static inline void
> >> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
> >> +{}
> >> +#endif
> >> +
> >>  static inline const char *acpi_get_enable_method(int cpu)
> >>  {
> >> -     return acpi_psci_present() ? "psci" : NULL;
> >> +     if (acpi_psci_present())
> >> +             return "psci";
> >> +
> >> +     if (acpi_parking_protocol_valid(cpu))
> >> +             return "parking-protocol";
> >> +
> >> +     return NULL;
> >>  }
> >>
> >>  #ifdef       CONFIG_ACPI_APEI
> >> diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
> >> index a57601f..8740297 100644
> >> --- a/arch/arm64/include/asm/hardirq.h
> >> +++ b/arch/arm64/include/asm/hardirq.h
> >> @@ -20,7 +20,7 @@
> >>  #include <linux/threads.h>
> >>  #include <asm/irq.h>
> >>
> >> -#define NR_IPI       5
> >> +#define NR_IPI       6
> >>
> >>  typedef struct {
> >>       unsigned int __softirq_pending;
> >> diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
> >> index d9c3d6a..2013a4d 100644
> >> --- a/arch/arm64/include/asm/smp.h
> >> +++ b/arch/arm64/include/asm/smp.h
> >> @@ -64,6 +64,15 @@ extern void secondary_entry(void);
> >>  extern void arch_send_call_function_single_ipi(int cpu);
> >>  extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
> >>
> >> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> >> +extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
> >> +#else
> >> +static inline void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
> >> +{
> >> +     BUILD_BUG();
> >> +}
> >> +#endif
> >> +
> >>  extern int __cpu_disable(void);
> >>
> >>  extern void __cpu_die(unsigned int cpu);
> >> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> >> index 83cd7e6..8a9c65c 100644
> >> --- a/arch/arm64/kernel/Makefile
> >> +++ b/arch/arm64/kernel/Makefile
> >> @@ -41,6 +41,7 @@ arm64-obj-$(CONFIG_EFI)                     += efi.o efi-entry.stub.o
> >>  arm64-obj-$(CONFIG_PCI)                      += pci.o
> >>  arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o
> >>  arm64-obj-$(CONFIG_ACPI)             += acpi.o
> >> +arm64-obj-$(CONFIG_ARM64_ACPI_PARKING_PROTOCOL)      += acpi_parking_protocol.o
> >>  arm64-obj-$(CONFIG_PARAVIRT)         += paravirt.o
> >>
> >>  obj-y                                        += $(arm64-obj-y) vdso/
> >> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
> >> new file mode 100644
> >> index 0000000..531c3ad
> >> --- /dev/null
> >> +++ b/arch/arm64/kernel/acpi_parking_protocol.c
> >> @@ -0,0 +1,153 @@
> >> +/*
> >> + * ARM64 ACPI Parking Protocol implementation
> >> + *
> >> + * Authors: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >> + *       Mark Salter <msalter@redhat.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/acpi.h>
> >> +#include <linux/types.h>
> >> +
> >> +#include <asm/cpu_ops.h>
> >> +
> >> +struct cpu_mailbox_entry {
> >> +     phys_addr_t mailbox_addr;
> >> +     u8 version;
> >> +     u8 gic_cpu_id;
> >> +};
> >> +
> >> +static struct cpu_mailbox_entry cpu_mailbox_entries[NR_CPUS];
> >> +
> >> +void __init acpi_set_mailbox_entry(int cpu,
> >> +                                struct acpi_madt_generic_interrupt *p)
> >> +{
> >> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> >> +
> >> +     cpu_entry->mailbox_addr = p->parked_address;
> >> +     cpu_entry->version = p->parking_version;
> >> +     cpu_entry->gic_cpu_id = p->cpu_interface_number;
> >> +}
> >> +
> >> +bool __init acpi_parking_protocol_valid(int cpu)
> >> +{
> >> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> >> +
> >> +     return cpu_entry->mailbox_addr && cpu_entry->version;
> >> +}
> >> +
> >> +static int acpi_parking_protocol_cpu_init(unsigned int cpu)
> >> +{
> >> +     pr_debug("%s: ACPI parked addr=%llx\n", __func__,
> >> +               cpu_mailbox_entries[cpu].mailbox_addr);
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
> >> +{
> >> +     return 0;
> >> +}
> >> +
> >> +struct parking_protocol_mailbox {
> >> +     __le32 cpu_id;
> >> +     __le32 reserved;
> >> +     __le64 entry_point;
> >> +};
> >> +
> >> +static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
> >> +{
> >> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> >> +     struct parking_protocol_mailbox __iomem *mailbox;
> >> +     __le32 cpu_id;
> >> +
> >> +     /*
> >> +      * Map mailbox memory with attribute device nGnRE (ie ioremap -
> >> +      * this deviates from the parking protocol specifications since
> >> +      * the mailboxes are required to be mapped nGnRnE; the attribute
> >> +      * discrepancy is harmless insofar as the protocol specification
> >> +      * is concerned).
> >> +      * If the mailbox is mistakenly allocated in the linear mapping
> >> +      * by FW ioremap will fail since the mapping will be prevented
> >> +      * by the kernel (it clashes with the linear mapping attributes
> >> +      * specifications).
> >> +      */
> >> +     mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> >> +     if (!mailbox)
> >> +             return -EIO;
> >> +
> >> +     cpu_id = readl_relaxed(&mailbox->cpu_id);
> >> +     /*
> >> +      * Check if firmware has set-up the mailbox entry properly
> >> +      * before kickstarting the respective cpu.
> >> +      */
> >> +     if (cpu_id != ~0U) {
> >> +             iounmap(mailbox);
> >> +             return -ENXIO;
> >> +     }
> >> +
> >> +     /*
> >> +      * We write the entry point and cpu id as LE regardless of the
> >> +      * native endianness of the kernel. Therefore, any boot-loaders
> >> +      * that read this address need to convert this address to the
> >> +      * Boot-Loader's endianness before jumping.
> >> +      */
> >> +     writeq_relaxed(__pa(secondary_entry), &mailbox->entry_point);
> >> +     writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id);
> >> +
> >> +     arch_send_wakeup_ipi_mask(cpumask_of(cpu));
> >> +
> >> +     iounmap(mailbox);
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +static void acpi_parking_protocol_cpu_postboot(void)
> >> +{
> >> +     int cpu = smp_processor_id();
> >> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> >> +     struct parking_protocol_mailbox __iomem *mailbox;
> >> +     __le64 entry_point;
> >> +
> >> +     /*
> >> +      * Map mailbox memory with attribute device nGnRE (ie ioremap -
> >> +      * this deviates from the parking protocol specifications since
> >> +      * the mailboxes are required to be mapped nGnRnE; the attribute
> >> +      * discrepancy is harmless insofar as the protocol specification
> >> +      * is concerned).
> >> +      * If the mailbox is mistakenly allocated in the linear mapping
> >> +      * by FW ioremap will fail since the mapping will be prevented
> >> +      * by the kernel (it clashes with the linear mapping attributes
> >> +      * specifications).
> >> +      */
> >> +     mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> >> +     if (!mailbox)
> >> +             return;
> >> +
> >> +     entry_point = readl_relaxed(&mailbox->entry_point);
> >> +     /*
> >> +      * Check if firmware has cleared the entry_point as expected
> >> +      * by the protocol specification.
> >> +      */
> >> +     WARN_ON(entry_point);
> >> +
> >> +     iounmap(mailbox);
> >> +}
> >> +
> >> +const struct cpu_operations acpi_parking_protocol_ops = {
> >> +     .name           = "parking-protocol",
> >> +     .cpu_init       = acpi_parking_protocol_cpu_init,
> >> +     .cpu_prepare    = acpi_parking_protocol_cpu_prepare,
> >> +     .cpu_boot       = acpi_parking_protocol_cpu_boot,
> >> +     .cpu_postboot   = acpi_parking_protocol_cpu_postboot
> >> +};
> >> diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
> >> index b6bd7d4..c7cfb8f 100644
> >> --- a/arch/arm64/kernel/cpu_ops.c
> >> +++ b/arch/arm64/kernel/cpu_ops.c
> >> @@ -25,19 +25,30 @@
> >>  #include <asm/smp_plat.h>
> >>
> >>  extern const struct cpu_operations smp_spin_table_ops;
> >> +extern const struct cpu_operations acpi_parking_protocol_ops;
> >>  extern const struct cpu_operations cpu_psci_ops;
> >>
> >>  const struct cpu_operations *cpu_ops[NR_CPUS];
> >>
> >> -static const struct cpu_operations *supported_cpu_ops[] __initconst = {
> >> +static const struct cpu_operations *dt_supported_cpu_ops[] __initconst = {
> >>       &smp_spin_table_ops,
> >>       &cpu_psci_ops,
> >>       NULL,
> >>  };
> >>
> >> +static const struct cpu_operations *acpi_supported_cpu_ops[] __initconst = {
> >> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> >> +     &acpi_parking_protocol_ops,
> >> +#endif
> >> +     &cpu_psci_ops,
> >> +     NULL,
> >> +};
> >> +
> >>  static const struct cpu_operations * __init cpu_get_ops(const char *name)
> >>  {
> >> -     const struct cpu_operations **ops = supported_cpu_ops;
> >> +     const struct cpu_operations **ops;
> >> +
> >> +     ops = acpi_disabled ? dt_supported_cpu_ops : acpi_supported_cpu_ops;
> >>
> >>       while (*ops) {
> >>               if (!strcmp(name, (*ops)->name))
> >> @@ -75,8 +86,16 @@ static const char *__init cpu_read_enable_method(int cpu)
> >>               }
> >>       } else {
> >>               enable_method = acpi_get_enable_method(cpu);
> >> -             if (!enable_method)
> >> -                     pr_err("Unsupported ACPI enable-method\n");
> >> +             if (!enable_method) {
> >> +                     /*
> >> +                      * In ACPI systems the boot CPU does not require
> >> +                      * checking the enable method since for some
> >> +                      * boot protocol (ie parking protocol) it need not
> >> +                      * be initialized. Don't warn spuriously.
> >> +                      */
> >> +                     if (cpu != 0)
> >> +                             pr_err("Unsupported ACPI enable-method\n");
> >> +             }
> >>       }
> >>
> >>       return enable_method;
> >> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> >> index b1adc51..6f608af 100644
> >> --- a/arch/arm64/kernel/smp.c
> >> +++ b/arch/arm64/kernel/smp.c
> >> @@ -70,6 +70,7 @@ enum ipi_msg_type {
> >>       IPI_CPU_STOP,
> >>       IPI_TIMER,
> >>       IPI_IRQ_WORK,
> >> +     IPI_WAKEUP
> >>  };
> >>
> >>  /*
> >> @@ -445,6 +446,17 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
> >>       /* map the logical cpu id to cpu MPIDR */
> >>       cpu_logical_map(cpu_count) = hwid;
> >>
> >> +     /*
> >> +      * Set-up the ACPI parking protocol cpu entries
> >> +      * while initializing the cpu_logical_map to
> >> +      * avoid parsing MADT entries multiple times for
> >> +      * nothing (ie a valid cpu_logical_map entry should
> >> +      * contain a valid parking protocol data set to
> >> +      * initialize the cpu if the parking protocol is
> >> +      * the only available enable method).
> >> +      */
> >> +     acpi_set_mailbox_entry(cpu_count, processor);
> >> +
> >>       cpu_count++;
> >>  }
> >>
> >> @@ -627,6 +639,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
> >>       S(IPI_CPU_STOP, "CPU stop interrupts"),
> >>       S(IPI_TIMER, "Timer broadcast interrupts"),
> >>       S(IPI_IRQ_WORK, "IRQ work interrupts"),
> >> +     S(IPI_WAKEUP, "CPU wakeup interrupts"),
> >>  };
> >>
> >>  static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
> >> @@ -670,6 +683,13 @@ void arch_send_call_function_single_ipi(int cpu)
> >>       smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC);
> >>  }
> >>
> >> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> >> +void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
> >> +{
> >> +     smp_cross_call(mask, IPI_WAKEUP);
> >> +}
> >> +#endif
> >> +
> >>  #ifdef CONFIG_IRQ_WORK
> >>  void arch_irq_work_raise(void)
> >>  {
> >> @@ -746,6 +766,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
> >>               irq_exit();
> >>               break;
> >>  #endif
> >> +     case IPI_WAKEUP:
> >> +             break;
> >>
> >>       default:
> >>               pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
> >> --
> >> 2.5.1
> >>
> 

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-01-27 11:46       ` Lorenzo Pieralisi
  0 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-27 11:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jan 27, 2016 at 11:23:41AM +0100, Ard Biesheuvel wrote:

[...]

> > It is to report that this patch currently works on my AMD Seattle with
> > a v4.5-rc1 kernel whilst it was not working before owing to FW putting
> > the mailboxes in WB memory (that was part of linear mapping, I still have
> > to pinpoint the exact reason). I think it is related to recent changes in
> > EFI and memblock. The code in this patch is allowed to map mailboxes
> > through ioremap() (ie it was not before because IIUC pfn_valid was returning
> > true on the mailboxes addresses), which it looks like it changed with:
> >
> > 68709f45385a ("arm64: only consider memblocks with NOMAP cleared for
> > linear mapping")
> >
> > Basically we end up mapping a memory area with ioremap with attributes
> > that may mismatch EFI descriptors, on my AMD Seattle the mailboxes are
> > part of runtime services data that is WB (see below).
> >
> 
> The '|WB|WT|WC|UC]' means it can be mapped in various ways, and since
> we dropped it from the linear mapping, it should tolerate being mapped
> as device memory, with the caveat that, since it is tagged as
> EFI_MEMORY_RUNTIME as well, it will be mapped cacheable during
> invocations to EFI runtime services.
> 
> So the issue is not in the memory types it exposes, but in the fact
> that it is listed as a region that is subject to VA remapping.

Thanks for clarifying and yes, I agree it is a firmware bug, I just
flagged this up to check with you whether we want to do something
about it (ie it is not just related to this patch).

> > I think the only way I can prevent this is by looking up the
> > mailboxes addresses in the EFI memory map and bail out if the
> > memory descriptor is marked as WB (according to the ACPI parking
> > protocol the mailboxes must be Device-nGnRnE).
> >
> 
> This is a firmware bug, and should be caught at the ACPI/UEFI
> validation level. I am not sure we have to special case this in the
> kernel.

I came to the same conclusion, I thought it was worth mentioning
it since with previous kernel versions this mismatch was caught in
the ioremap implementation and now it is not, if a warning has to be
put in place it has to be done separately, it is not an issue
specific to this patch only.

Thanks,
Lorenzo

> 
> -- 
> Ard.
> 
> 
> 
> > AMD Seattle snip boot log with this patch applied and some debug output
> > below (ie mailbox address falls within "Runtime Data").
> >
> > I will keep debugging, comments appreciated.
> >
> > Thanks,
> > Lorenzo
> >
> > [    0.000000] Processing EFI memory map:
> >
> > <snip>
> >
> > [    0.000000]   0x008028000000-0x0080280fffff [Runtime Data       |RUN|  |  |  |  |  |   |WB|WT|WC|UC]*
> > [    0.000000] acpi_parking_protocol_cpu_init: ACPI parked addr=80280c1000
> >
> >> - Moved Kconfig option under "Boot options"
> >>
> >> v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-September/369121.html
> >>
> >> v1->v2
> >>
> >> - Rebased against v4.2
> >> - Removed SMP dependency (it was removed from arm64)
> >> - Updated some comments
> >> - Clarified 64k page mailbox alignment and requested UEFI specs update
> >>
> >> v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356750.html
> >>
> >>  arch/arm64/Kconfig                        |   9 ++
> >>  arch/arm64/include/asm/acpi.h             |  19 +++-
> >>  arch/arm64/include/asm/hardirq.h          |   2 +-
> >>  arch/arm64/include/asm/smp.h              |   9 ++
> >>  arch/arm64/kernel/Makefile                |   1 +
> >>  arch/arm64/kernel/acpi_parking_protocol.c | 153 ++++++++++++++++++++++++++++++
> >>  arch/arm64/kernel/cpu_ops.c               |  27 +++++-
> >>  arch/arm64/kernel/smp.c                   |  22 +++++
> >>  8 files changed, 236 insertions(+), 6 deletions(-)
> >>  create mode 100644 arch/arm64/kernel/acpi_parking_protocol.c
> >>
> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> >> index 8cc6228..53e48a6 100644
> >> --- a/arch/arm64/Kconfig
> >> +++ b/arch/arm64/Kconfig
> >> @@ -756,6 +756,15 @@ endmenu
> >>
> >>  menu "Boot options"
> >>
> >> +config ARM64_ACPI_PARKING_PROTOCOL
> >> +     bool "Enable support for the ARM64 ACPI parking protocol"
> >> +     depends on ACPI
> >> +     help
> >> +       Enable support for the ARM64 ACPI parking protocol. If disabled
> >> +       the kernel will not allow booting through the ARM64 ACPI parking
> >> +       protocol even if the corresponding data is present in the ACPI
> >> +       MADT table.
> >> +
> >>  config CMDLINE
> >>       string "Default kernel command string"
> >>       default ""
> >> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> >> index caafd63..aee323b 100644
> >> --- a/arch/arm64/include/asm/acpi.h
> >> +++ b/arch/arm64/include/asm/acpi.h
> >> @@ -87,9 +87,26 @@ void __init acpi_init_cpus(void);
> >>  static inline void acpi_init_cpus(void) { }
> >>  #endif /* CONFIG_ACPI */
> >>
> >> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> >> +bool acpi_parking_protocol_valid(int cpu);
> >> +void __init
> >> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor);
> >> +#else
> >> +static inline bool acpi_parking_protocol_valid(int cpu) { return false; }
> >> +static inline void
> >> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
> >> +{}
> >> +#endif
> >> +
> >>  static inline const char *acpi_get_enable_method(int cpu)
> >>  {
> >> -     return acpi_psci_present() ? "psci" : NULL;
> >> +     if (acpi_psci_present())
> >> +             return "psci";
> >> +
> >> +     if (acpi_parking_protocol_valid(cpu))
> >> +             return "parking-protocol";
> >> +
> >> +     return NULL;
> >>  }
> >>
> >>  #ifdef       CONFIG_ACPI_APEI
> >> diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
> >> index a57601f..8740297 100644
> >> --- a/arch/arm64/include/asm/hardirq.h
> >> +++ b/arch/arm64/include/asm/hardirq.h
> >> @@ -20,7 +20,7 @@
> >>  #include <linux/threads.h>
> >>  #include <asm/irq.h>
> >>
> >> -#define NR_IPI       5
> >> +#define NR_IPI       6
> >>
> >>  typedef struct {
> >>       unsigned int __softirq_pending;
> >> diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
> >> index d9c3d6a..2013a4d 100644
> >> --- a/arch/arm64/include/asm/smp.h
> >> +++ b/arch/arm64/include/asm/smp.h
> >> @@ -64,6 +64,15 @@ extern void secondary_entry(void);
> >>  extern void arch_send_call_function_single_ipi(int cpu);
> >>  extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
> >>
> >> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> >> +extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
> >> +#else
> >> +static inline void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
> >> +{
> >> +     BUILD_BUG();
> >> +}
> >> +#endif
> >> +
> >>  extern int __cpu_disable(void);
> >>
> >>  extern void __cpu_die(unsigned int cpu);
> >> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> >> index 83cd7e6..8a9c65c 100644
> >> --- a/arch/arm64/kernel/Makefile
> >> +++ b/arch/arm64/kernel/Makefile
> >> @@ -41,6 +41,7 @@ arm64-obj-$(CONFIG_EFI)                     += efi.o efi-entry.stub.o
> >>  arm64-obj-$(CONFIG_PCI)                      += pci.o
> >>  arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o
> >>  arm64-obj-$(CONFIG_ACPI)             += acpi.o
> >> +arm64-obj-$(CONFIG_ARM64_ACPI_PARKING_PROTOCOL)      += acpi_parking_protocol.o
> >>  arm64-obj-$(CONFIG_PARAVIRT)         += paravirt.o
> >>
> >>  obj-y                                        += $(arm64-obj-y) vdso/
> >> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
> >> new file mode 100644
> >> index 0000000..531c3ad
> >> --- /dev/null
> >> +++ b/arch/arm64/kernel/acpi_parking_protocol.c
> >> @@ -0,0 +1,153 @@
> >> +/*
> >> + * ARM64 ACPI Parking Protocol implementation
> >> + *
> >> + * Authors: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >> + *       Mark Salter <msalter@redhat.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/acpi.h>
> >> +#include <linux/types.h>
> >> +
> >> +#include <asm/cpu_ops.h>
> >> +
> >> +struct cpu_mailbox_entry {
> >> +     phys_addr_t mailbox_addr;
> >> +     u8 version;
> >> +     u8 gic_cpu_id;
> >> +};
> >> +
> >> +static struct cpu_mailbox_entry cpu_mailbox_entries[NR_CPUS];
> >> +
> >> +void __init acpi_set_mailbox_entry(int cpu,
> >> +                                struct acpi_madt_generic_interrupt *p)
> >> +{
> >> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> >> +
> >> +     cpu_entry->mailbox_addr = p->parked_address;
> >> +     cpu_entry->version = p->parking_version;
> >> +     cpu_entry->gic_cpu_id = p->cpu_interface_number;
> >> +}
> >> +
> >> +bool __init acpi_parking_protocol_valid(int cpu)
> >> +{
> >> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> >> +
> >> +     return cpu_entry->mailbox_addr && cpu_entry->version;
> >> +}
> >> +
> >> +static int acpi_parking_protocol_cpu_init(unsigned int cpu)
> >> +{
> >> +     pr_debug("%s: ACPI parked addr=%llx\n", __func__,
> >> +               cpu_mailbox_entries[cpu].mailbox_addr);
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
> >> +{
> >> +     return 0;
> >> +}
> >> +
> >> +struct parking_protocol_mailbox {
> >> +     __le32 cpu_id;
> >> +     __le32 reserved;
> >> +     __le64 entry_point;
> >> +};
> >> +
> >> +static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
> >> +{
> >> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> >> +     struct parking_protocol_mailbox __iomem *mailbox;
> >> +     __le32 cpu_id;
> >> +
> >> +     /*
> >> +      * Map mailbox memory with attribute device nGnRE (ie ioremap -
> >> +      * this deviates from the parking protocol specifications since
> >> +      * the mailboxes are required to be mapped nGnRnE; the attribute
> >> +      * discrepancy is harmless insofar as the protocol specification
> >> +      * is concerned).
> >> +      * If the mailbox is mistakenly allocated in the linear mapping
> >> +      * by FW ioremap will fail since the mapping will be prevented
> >> +      * by the kernel (it clashes with the linear mapping attributes
> >> +      * specifications).
> >> +      */
> >> +     mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> >> +     if (!mailbox)
> >> +             return -EIO;
> >> +
> >> +     cpu_id = readl_relaxed(&mailbox->cpu_id);
> >> +     /*
> >> +      * Check if firmware has set-up the mailbox entry properly
> >> +      * before kickstarting the respective cpu.
> >> +      */
> >> +     if (cpu_id != ~0U) {
> >> +             iounmap(mailbox);
> >> +             return -ENXIO;
> >> +     }
> >> +
> >> +     /*
> >> +      * We write the entry point and cpu id as LE regardless of the
> >> +      * native endianness of the kernel. Therefore, any boot-loaders
> >> +      * that read this address need to convert this address to the
> >> +      * Boot-Loader's endianness before jumping.
> >> +      */
> >> +     writeq_relaxed(__pa(secondary_entry), &mailbox->entry_point);
> >> +     writel_relaxed(cpu_entry->gic_cpu_id, &mailbox->cpu_id);
> >> +
> >> +     arch_send_wakeup_ipi_mask(cpumask_of(cpu));
> >> +
> >> +     iounmap(mailbox);
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> +static void acpi_parking_protocol_cpu_postboot(void)
> >> +{
> >> +     int cpu = smp_processor_id();
> >> +     struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> >> +     struct parking_protocol_mailbox __iomem *mailbox;
> >> +     __le64 entry_point;
> >> +
> >> +     /*
> >> +      * Map mailbox memory with attribute device nGnRE (ie ioremap -
> >> +      * this deviates from the parking protocol specifications since
> >> +      * the mailboxes are required to be mapped nGnRnE; the attribute
> >> +      * discrepancy is harmless insofar as the protocol specification
> >> +      * is concerned).
> >> +      * If the mailbox is mistakenly allocated in the linear mapping
> >> +      * by FW ioremap will fail since the mapping will be prevented
> >> +      * by the kernel (it clashes with the linear mapping attributes
> >> +      * specifications).
> >> +      */
> >> +     mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> >> +     if (!mailbox)
> >> +             return;
> >> +
> >> +     entry_point = readl_relaxed(&mailbox->entry_point);
> >> +     /*
> >> +      * Check if firmware has cleared the entry_point as expected
> >> +      * by the protocol specification.
> >> +      */
> >> +     WARN_ON(entry_point);
> >> +
> >> +     iounmap(mailbox);
> >> +}
> >> +
> >> +const struct cpu_operations acpi_parking_protocol_ops = {
> >> +     .name           = "parking-protocol",
> >> +     .cpu_init       = acpi_parking_protocol_cpu_init,
> >> +     .cpu_prepare    = acpi_parking_protocol_cpu_prepare,
> >> +     .cpu_boot       = acpi_parking_protocol_cpu_boot,
> >> +     .cpu_postboot   = acpi_parking_protocol_cpu_postboot
> >> +};
> >> diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
> >> index b6bd7d4..c7cfb8f 100644
> >> --- a/arch/arm64/kernel/cpu_ops.c
> >> +++ b/arch/arm64/kernel/cpu_ops.c
> >> @@ -25,19 +25,30 @@
> >>  #include <asm/smp_plat.h>
> >>
> >>  extern const struct cpu_operations smp_spin_table_ops;
> >> +extern const struct cpu_operations acpi_parking_protocol_ops;
> >>  extern const struct cpu_operations cpu_psci_ops;
> >>
> >>  const struct cpu_operations *cpu_ops[NR_CPUS];
> >>
> >> -static const struct cpu_operations *supported_cpu_ops[] __initconst = {
> >> +static const struct cpu_operations *dt_supported_cpu_ops[] __initconst = {
> >>       &smp_spin_table_ops,
> >>       &cpu_psci_ops,
> >>       NULL,
> >>  };
> >>
> >> +static const struct cpu_operations *acpi_supported_cpu_ops[] __initconst = {
> >> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> >> +     &acpi_parking_protocol_ops,
> >> +#endif
> >> +     &cpu_psci_ops,
> >> +     NULL,
> >> +};
> >> +
> >>  static const struct cpu_operations * __init cpu_get_ops(const char *name)
> >>  {
> >> -     const struct cpu_operations **ops = supported_cpu_ops;
> >> +     const struct cpu_operations **ops;
> >> +
> >> +     ops = acpi_disabled ? dt_supported_cpu_ops : acpi_supported_cpu_ops;
> >>
> >>       while (*ops) {
> >>               if (!strcmp(name, (*ops)->name))
> >> @@ -75,8 +86,16 @@ static const char *__init cpu_read_enable_method(int cpu)
> >>               }
> >>       } else {
> >>               enable_method = acpi_get_enable_method(cpu);
> >> -             if (!enable_method)
> >> -                     pr_err("Unsupported ACPI enable-method\n");
> >> +             if (!enable_method) {
> >> +                     /*
> >> +                      * In ACPI systems the boot CPU does not require
> >> +                      * checking the enable method since for some
> >> +                      * boot protocol (ie parking protocol) it need not
> >> +                      * be initialized. Don't warn spuriously.
> >> +                      */
> >> +                     if (cpu != 0)
> >> +                             pr_err("Unsupported ACPI enable-method\n");
> >> +             }
> >>       }
> >>
> >>       return enable_method;
> >> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> >> index b1adc51..6f608af 100644
> >> --- a/arch/arm64/kernel/smp.c
> >> +++ b/arch/arm64/kernel/smp.c
> >> @@ -70,6 +70,7 @@ enum ipi_msg_type {
> >>       IPI_CPU_STOP,
> >>       IPI_TIMER,
> >>       IPI_IRQ_WORK,
> >> +     IPI_WAKEUP
> >>  };
> >>
> >>  /*
> >> @@ -445,6 +446,17 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
> >>       /* map the logical cpu id to cpu MPIDR */
> >>       cpu_logical_map(cpu_count) = hwid;
> >>
> >> +     /*
> >> +      * Set-up the ACPI parking protocol cpu entries
> >> +      * while initializing the cpu_logical_map to
> >> +      * avoid parsing MADT entries multiple times for
> >> +      * nothing (ie a valid cpu_logical_map entry should
> >> +      * contain a valid parking protocol data set to
> >> +      * initialize the cpu if the parking protocol is
> >> +      * the only available enable method).
> >> +      */
> >> +     acpi_set_mailbox_entry(cpu_count, processor);
> >> +
> >>       cpu_count++;
> >>  }
> >>
> >> @@ -627,6 +639,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
> >>       S(IPI_CPU_STOP, "CPU stop interrupts"),
> >>       S(IPI_TIMER, "Timer broadcast interrupts"),
> >>       S(IPI_IRQ_WORK, "IRQ work interrupts"),
> >> +     S(IPI_WAKEUP, "CPU wakeup interrupts"),
> >>  };
> >>
> >>  static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
> >> @@ -670,6 +683,13 @@ void arch_send_call_function_single_ipi(int cpu)
> >>       smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC);
> >>  }
> >>
> >> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> >> +void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
> >> +{
> >> +     smp_cross_call(mask, IPI_WAKEUP);
> >> +}
> >> +#endif
> >> +
> >>  #ifdef CONFIG_IRQ_WORK
> >>  void arch_irq_work_raise(void)
> >>  {
> >> @@ -746,6 +766,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
> >>               irq_exit();
> >>               break;
> >>  #endif
> >> +     case IPI_WAKEUP:
> >> +             break;
> >>
> >>       default:
> >>               pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
> >> --
> >> 2.5.1
> >>
> 

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-01-26 23:13   ` Loc Ho
@ 2016-01-27 11:51     ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-27 11:51 UTC (permalink / raw)
  To: Loc Ho
  Cc: linux-arm-kernel, linux-acpi, Will Deacon, Hanjun Guo,
	Sudeep Holla, Catalin Marinas, Mark Rutland, Mark Salter,
	Al Stone

On Tue, Jan 26, 2016 at 03:13:30PM -0800, Loc Ho wrote:
> Hi Lorenzo,
> 
> On Tue, Jan 26, 2016 at 3:10 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > The SBBR and ACPI specifications allow ACPI based systems that do not
> > implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> > protocol specification[1].
> >
> > This patch implements the ACPI parking protocol CPU operations, and adds
> > code that eases parsing the parking protocol data structures to the
> > ARM64 SMP initializion carried out at the same time as cpus enumeration.
> >
> > To wake-up the CPUs from the parked state, this patch implements a
> > wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> > ARM one, so that a specific IPI is sent for wake-up purpose in order
> > to distinguish it from other IPI sources.
> >
> > Given the current ACPI MADT parsing API, the patch implements a glue
> > layer that helps passing MADT GICC data structure from SMP initialization
> > code to the parking protocol implementation somewhat overriding the CPU
> > operations interfaces. This to avoid creating a completely trasparent
> > DT/ACPI CPU operations layer that would require creating opaque
> > structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> > through static MADT table entries), which seems overkill given that ACPI
> > on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> > so there is no need for further protocol additions.
> >
> > Based on the original work by Mark Salter <msalter@redhat.com>
> >
> > [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > Cc: Loc Ho <lho@apm.com>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Mark Salter <msalter@redhat.com>
> > Cc: Al Stone <ahs3@redhat.com>
> 
> I had tested this with X-Gene Mustang board. It applies cleanly
> against 4.5.0-rc1 and detected all CPU's. For those of you want to try
> this, you need an FW update if you are using APM Tianocore. Otherwise,
> it will stuck in very early booting stage.

We can take this as a Tested-by, right ?

Thanks,
Lorenzo

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-01-27 11:51     ` Lorenzo Pieralisi
  0 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-27 11:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jan 26, 2016 at 03:13:30PM -0800, Loc Ho wrote:
> Hi Lorenzo,
> 
> On Tue, Jan 26, 2016 at 3:10 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > The SBBR and ACPI specifications allow ACPI based systems that do not
> > implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> > protocol specification[1].
> >
> > This patch implements the ACPI parking protocol CPU operations, and adds
> > code that eases parsing the parking protocol data structures to the
> > ARM64 SMP initializion carried out at the same time as cpus enumeration.
> >
> > To wake-up the CPUs from the parked state, this patch implements a
> > wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> > ARM one, so that a specific IPI is sent for wake-up purpose in order
> > to distinguish it from other IPI sources.
> >
> > Given the current ACPI MADT parsing API, the patch implements a glue
> > layer that helps passing MADT GICC data structure from SMP initialization
> > code to the parking protocol implementation somewhat overriding the CPU
> > operations interfaces. This to avoid creating a completely trasparent
> > DT/ACPI CPU operations layer that would require creating opaque
> > structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> > through static MADT table entries), which seems overkill given that ACPI
> > on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> > so there is no need for further protocol additions.
> >
> > Based on the original work by Mark Salter <msalter@redhat.com>
> >
> > [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > Cc: Loc Ho <lho@apm.com>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Mark Salter <msalter@redhat.com>
> > Cc: Al Stone <ahs3@redhat.com>
> 
> I had tested this with X-Gene Mustang board. It applies cleanly
> against 4.5.0-rc1 and detected all CPU's. For those of you want to try
> this, you need an FW update if you are using APM Tianocore. Otherwise,
> it will stuck in very early booting stage.

We can take this as a Tested-by, right ?

Thanks,
Lorenzo

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-01-27 11:51     ` Lorenzo Pieralisi
@ 2016-01-27 17:41       ` Loc Ho
  -1 siblings, 0 replies; 32+ messages in thread
From: Loc Ho @ 2016-01-27 17:41 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-arm-kernel, linux-acpi, Will Deacon, Hanjun Guo,
	Sudeep Holla, Catalin Marinas, Mark Rutland, Mark Salter,
	Al Stone

Hi,

> >
> > I had tested this with X-Gene Mustang board. It applies cleanly
> > against 4.5.0-rc1 and detected all CPU's. For those of you want to try
> > this, you need an FW update if you are using APM Tianocore. Otherwise,
> > it will stuck in very early booting stage.
>
> We can take this as a Tested-by, right ?


Yes... if you want one.

-Loc

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-01-27 17:41       ` Loc Ho
  0 siblings, 0 replies; 32+ messages in thread
From: Loc Ho @ 2016-01-27 17:41 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

> >
> > I had tested this with X-Gene Mustang board. It applies cleanly
> > against 4.5.0-rc1 and detected all CPU's. For those of you want to try
> > this, you need an FW update if you are using APM Tianocore. Otherwise,
> > it will stuck in very early booting stage.
>
> We can take this as a Tested-by, right ?


Yes... if you want one.

-Loc

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-01-26 11:10 ` Lorenzo Pieralisi
@ 2016-02-02 18:26   ` Catalin Marinas
  -1 siblings, 0 replies; 32+ messages in thread
From: Catalin Marinas @ 2016-02-02 18:26 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-arm-kernel, Mark Rutland, Mark Salter, Will Deacon,
	linux-acpi, Hanjun Guo, Sudeep Holla, Al Stone, Loc Ho

On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
> The SBBR and ACPI specifications allow ACPI based systems that do not
> implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> protocol specification[1].
> 
> This patch implements the ACPI parking protocol CPU operations, and adds
> code that eases parsing the parking protocol data structures to the
> ARM64 SMP initializion carried out at the same time as cpus enumeration.
> 
> To wake-up the CPUs from the parked state, this patch implements a
> wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> ARM one, so that a specific IPI is sent for wake-up purpose in order
> to distinguish it from other IPI sources.
> 
> Given the current ACPI MADT parsing API, the patch implements a glue
> layer that helps passing MADT GICC data structure from SMP initialization
> code to the parking protocol implementation somewhat overriding the CPU
> operations interfaces. This to avoid creating a completely trasparent
> DT/ACPI CPU operations layer that would require creating opaque
> structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> through static MADT table entries), which seems overkill given that ACPI
> on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> so there is no need for further protocol additions.
> 
> Based on the original work by Mark Salter <msalter@redhat.com>
> 
> [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Loc Ho <lho@apm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Al Stone <ahs3@redhat.com>

Applied, with a minor addition just to warn people from not using it in
other configurations (#ifdef still needed otherwise the
acpi_parking_protocol_valid symbol is not available; but I prefer uglier
code than people starting to use this IPI in their firmware):

--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -767,6 +767,12 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
                break;
 #endif
+
+#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
        case IPI_WAKEUP:
+               WARN_ONCE(!acpi_parking_protocol_valid(cpu),
+                         "CPU%u: Wake-up IPI outside the ACPI parking protocol\n",
+                         cpu);
                break;
+#endif
 
        default:

Thanks.

-- 
Catalin

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-02-02 18:26   ` Catalin Marinas
  0 siblings, 0 replies; 32+ messages in thread
From: Catalin Marinas @ 2016-02-02 18:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
> The SBBR and ACPI specifications allow ACPI based systems that do not
> implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> protocol specification[1].
> 
> This patch implements the ACPI parking protocol CPU operations, and adds
> code that eases parsing the parking protocol data structures to the
> ARM64 SMP initializion carried out at the same time as cpus enumeration.
> 
> To wake-up the CPUs from the parked state, this patch implements a
> wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> ARM one, so that a specific IPI is sent for wake-up purpose in order
> to distinguish it from other IPI sources.
> 
> Given the current ACPI MADT parsing API, the patch implements a glue
> layer that helps passing MADT GICC data structure from SMP initialization
> code to the parking protocol implementation somewhat overriding the CPU
> operations interfaces. This to avoid creating a completely trasparent
> DT/ACPI CPU operations layer that would require creating opaque
> structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> through static MADT table entries), which seems overkill given that ACPI
> on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> so there is no need for further protocol additions.
> 
> Based on the original work by Mark Salter <msalter@redhat.com>
> 
> [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Loc Ho <lho@apm.com>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Mark Salter <msalter@redhat.com>
> Cc: Al Stone <ahs3@redhat.com>

Applied, with a minor addition just to warn people from not using it in
other configurations (#ifdef still needed otherwise the
acpi_parking_protocol_valid symbol is not available; but I prefer uglier
code than people starting to use this IPI in their firmware):

--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -767,6 +767,12 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
                break;
 #endif
+
+#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
        case IPI_WAKEUP:
+               WARN_ONCE(!acpi_parking_protocol_valid(cpu),
+                         "CPU%u: Wake-up IPI outside the ACPI parking protocol\n",
+                         cpu);
                break;
+#endif
 
        default:

Thanks.

-- 
Catalin

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-02-02 18:26   ` Catalin Marinas
@ 2016-02-03 11:21     ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-02-03 11:21 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-arm-kernel, Mark Rutland, Mark Salter, Will Deacon,
	linux-acpi, Hanjun Guo, Sudeep Holla, Al Stone, Loc Ho

On Tue, Feb 02, 2016 at 06:26:58PM +0000, Catalin Marinas wrote:
> On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
> > The SBBR and ACPI specifications allow ACPI based systems that do not
> > implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> > protocol specification[1].
> > 
> > This patch implements the ACPI parking protocol CPU operations, and adds
> > code that eases parsing the parking protocol data structures to the
> > ARM64 SMP initializion carried out at the same time as cpus enumeration.
> > 
> > To wake-up the CPUs from the parked state, this patch implements a
> > wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> > ARM one, so that a specific IPI is sent for wake-up purpose in order
> > to distinguish it from other IPI sources.
> > 
> > Given the current ACPI MADT parsing API, the patch implements a glue
> > layer that helps passing MADT GICC data structure from SMP initialization
> > code to the parking protocol implementation somewhat overriding the CPU
> > operations interfaces. This to avoid creating a completely trasparent
> > DT/ACPI CPU operations layer that would require creating opaque
> > structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> > through static MADT table entries), which seems overkill given that ACPI
> > on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> > so there is no need for further protocol additions.
> > 
> > Based on the original work by Mark Salter <msalter@redhat.com>
> > 
> > [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > Cc: Loc Ho <lho@apm.com>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Mark Salter <msalter@redhat.com>
> > Cc: Al Stone <ahs3@redhat.com>
> 
> Applied, with a minor addition just to warn people from not using it in
> other configurations (#ifdef still needed otherwise the
> acpi_parking_protocol_valid symbol is not available; but I prefer uglier
> code than people starting to use this IPI in their firmware):

It makes sense, we could include asm/acpi.h in smp.c (which is not
included by linux/acpi.h if !CONFIG_ACPI) to pull in the symbol and
remove the ifdef if you think it is cleaner.

Thanks !
Lorenzo

> 
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -767,6 +767,12 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
>                 break;
>  #endif
> +
> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
>         case IPI_WAKEUP:
> +               WARN_ONCE(!acpi_parking_protocol_valid(cpu),
> +                         "CPU%u: Wake-up IPI outside the ACPI parking protocol\n",
> +                         cpu);
>                 break;
> +#endif
>  
>         default:
> 
> Thanks.
> 
> -- 
> Catalin
> 

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-02-03 11:21     ` Lorenzo Pieralisi
  0 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-02-03 11:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 02, 2016 at 06:26:58PM +0000, Catalin Marinas wrote:
> On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
> > The SBBR and ACPI specifications allow ACPI based systems that do not
> > implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> > protocol specification[1].
> > 
> > This patch implements the ACPI parking protocol CPU operations, and adds
> > code that eases parsing the parking protocol data structures to the
> > ARM64 SMP initializion carried out at the same time as cpus enumeration.
> > 
> > To wake-up the CPUs from the parked state, this patch implements a
> > wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> > ARM one, so that a specific IPI is sent for wake-up purpose in order
> > to distinguish it from other IPI sources.
> > 
> > Given the current ACPI MADT parsing API, the patch implements a glue
> > layer that helps passing MADT GICC data structure from SMP initialization
> > code to the parking protocol implementation somewhat overriding the CPU
> > operations interfaces. This to avoid creating a completely trasparent
> > DT/ACPI CPU operations layer that would require creating opaque
> > structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> > through static MADT table entries), which seems overkill given that ACPI
> > on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> > so there is no need for further protocol additions.
> > 
> > Based on the original work by Mark Salter <msalter@redhat.com>
> > 
> > [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > Cc: Loc Ho <lho@apm.com>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Mark Salter <msalter@redhat.com>
> > Cc: Al Stone <ahs3@redhat.com>
> 
> Applied, with a minor addition just to warn people from not using it in
> other configurations (#ifdef still needed otherwise the
> acpi_parking_protocol_valid symbol is not available; but I prefer uglier
> code than people starting to use this IPI in their firmware):

It makes sense, we could include asm/acpi.h in smp.c (which is not
included by linux/acpi.h if !CONFIG_ACPI) to pull in the symbol and
remove the ifdef if you think it is cleaner.

Thanks !
Lorenzo

> 
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -767,6 +767,12 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
>                 break;
>  #endif
> +
> +#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
>         case IPI_WAKEUP:
> +               WARN_ONCE(!acpi_parking_protocol_valid(cpu),
> +                         "CPU%u: Wake-up IPI outside the ACPI parking protocol\n",
> +                         cpu);
>                 break;
> +#endif
>  
>         default:
> 
> Thanks.
> 
> -- 
> Catalin
> 

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-02-03 11:21     ` Lorenzo Pieralisi
@ 2016-02-03 16:18       ` Catalin Marinas
  -1 siblings, 0 replies; 32+ messages in thread
From: Catalin Marinas @ 2016-02-03 16:18 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Mark Rutland, Sudeep Holla, Will Deacon, linux-acpi, Hanjun Guo,
	Mark Salter, Al Stone, linux-arm-kernel, Loc Ho

On Wed, Feb 03, 2016 at 11:21:12AM +0000, Lorenzo Pieralisi wrote:
> On Tue, Feb 02, 2016 at 06:26:58PM +0000, Catalin Marinas wrote:
> > On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
> > > The SBBR and ACPI specifications allow ACPI based systems that do not
> > > implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> > > protocol specification[1].
> > > 
> > > This patch implements the ACPI parking protocol CPU operations, and adds
> > > code that eases parsing the parking protocol data structures to the
> > > ARM64 SMP initializion carried out at the same time as cpus enumeration.
> > > 
> > > To wake-up the CPUs from the parked state, this patch implements a
> > > wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> > > ARM one, so that a specific IPI is sent for wake-up purpose in order
> > > to distinguish it from other IPI sources.
> > > 
> > > Given the current ACPI MADT parsing API, the patch implements a glue
> > > layer that helps passing MADT GICC data structure from SMP initialization
> > > code to the parking protocol implementation somewhat overriding the CPU
> > > operations interfaces. This to avoid creating a completely trasparent
> > > DT/ACPI CPU operations layer that would require creating opaque
> > > structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> > > through static MADT table entries), which seems overkill given that ACPI
> > > on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> > > so there is no need for further protocol additions.
> > > 
> > > Based on the original work by Mark Salter <msalter@redhat.com>
> > > 
> > > [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> > > 
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > > Cc: Loc Ho <lho@apm.com>
> > > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Mark Salter <msalter@redhat.com>
> > > Cc: Al Stone <ahs3@redhat.com>
> > 
> > Applied, with a minor addition just to warn people from not using it in
> > other configurations (#ifdef still needed otherwise the
> > acpi_parking_protocol_valid symbol is not available; but I prefer uglier
> > code than people starting to use this IPI in their firmware):
> 
> It makes sense, we could include asm/acpi.h in smp.c (which is not
> included by linux/acpi.h if !CONFIG_ACPI) to pull in the symbol and
> remove the ifdef if you think it is cleaner.

I don't think it's worth.

BTW, the acpi_parking_protocol_valid() definition has an __init
annotation while the declaration does not. I removed the __init
altogether since I get a section mismatch warning when being called from
handle_IPI.

-- 
Catalin

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-02-03 16:18       ` Catalin Marinas
  0 siblings, 0 replies; 32+ messages in thread
From: Catalin Marinas @ 2016-02-03 16:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 03, 2016 at 11:21:12AM +0000, Lorenzo Pieralisi wrote:
> On Tue, Feb 02, 2016 at 06:26:58PM +0000, Catalin Marinas wrote:
> > On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
> > > The SBBR and ACPI specifications allow ACPI based systems that do not
> > > implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> > > protocol specification[1].
> > > 
> > > This patch implements the ACPI parking protocol CPU operations, and adds
> > > code that eases parsing the parking protocol data structures to the
> > > ARM64 SMP initializion carried out at the same time as cpus enumeration.
> > > 
> > > To wake-up the CPUs from the parked state, this patch implements a
> > > wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> > > ARM one, so that a specific IPI is sent for wake-up purpose in order
> > > to distinguish it from other IPI sources.
> > > 
> > > Given the current ACPI MADT parsing API, the patch implements a glue
> > > layer that helps passing MADT GICC data structure from SMP initialization
> > > code to the parking protocol implementation somewhat overriding the CPU
> > > operations interfaces. This to avoid creating a completely trasparent
> > > DT/ACPI CPU operations layer that would require creating opaque
> > > structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> > > through static MADT table entries), which seems overkill given that ACPI
> > > on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> > > so there is no need for further protocol additions.
> > > 
> > > Based on the original work by Mark Salter <msalter@redhat.com>
> > > 
> > > [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> > > 
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > > Cc: Loc Ho <lho@apm.com>
> > > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Mark Salter <msalter@redhat.com>
> > > Cc: Al Stone <ahs3@redhat.com>
> > 
> > Applied, with a minor addition just to warn people from not using it in
> > other configurations (#ifdef still needed otherwise the
> > acpi_parking_protocol_valid symbol is not available; but I prefer uglier
> > code than people starting to use this IPI in their firmware):
> 
> It makes sense, we could include asm/acpi.h in smp.c (which is not
> included by linux/acpi.h if !CONFIG_ACPI) to pull in the symbol and
> remove the ifdef if you think it is cleaner.

I don't think it's worth.

BTW, the acpi_parking_protocol_valid() definition has an __init
annotation while the declaration does not. I removed the __init
altogether since I get a section mismatch warning when being called from
handle_IPI.

-- 
Catalin

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-02-03 16:18       ` Catalin Marinas
@ 2016-02-24 14:18         ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-02-24 14:18 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Mark Rutland, Sudeep Holla, Will Deacon, linux-acpi, Hanjun Guo,
	Mark Salter, Al Stone, linux-arm-kernel, Loc Ho, Itaru Kitayama

[+ Itaru]

On Wed, Feb 03, 2016 at 04:18:36PM +0000, Catalin Marinas wrote:
> On Wed, Feb 03, 2016 at 11:21:12AM +0000, Lorenzo Pieralisi wrote:
> > On Tue, Feb 02, 2016 at 06:26:58PM +0000, Catalin Marinas wrote:
> > > On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
> > > > The SBBR and ACPI specifications allow ACPI based systems that do not
> > > > implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> > > > protocol specification[1].
> > > > 
> > > > This patch implements the ACPI parking protocol CPU operations, and adds
> > > > code that eases parsing the parking protocol data structures to the
> > > > ARM64 SMP initializion carried out at the same time as cpus enumeration.
> > > > 
> > > > To wake-up the CPUs from the parked state, this patch implements a
> > > > wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> > > > ARM one, so that a specific IPI is sent for wake-up purpose in order
> > > > to distinguish it from other IPI sources.
> > > > 
> > > > Given the current ACPI MADT parsing API, the patch implements a glue
> > > > layer that helps passing MADT GICC data structure from SMP initialization
> > > > code to the parking protocol implementation somewhat overriding the CPU
> > > > operations interfaces. This to avoid creating a completely trasparent
> > > > DT/ACPI CPU operations layer that would require creating opaque
> > > > structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> > > > through static MADT table entries), which seems overkill given that ACPI
> > > > on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> > > > so there is no need for further protocol additions.
> > > > 
> > > > Based on the original work by Mark Salter <msalter@redhat.com>
> > > > 
> > > > [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> > > > 
> > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Cc: Will Deacon <will.deacon@arm.com>
> > > > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > > > Cc: Loc Ho <lho@apm.com>
> > > > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > Cc: Mark Salter <msalter@redhat.com>
> > > > Cc: Al Stone <ahs3@redhat.com>
> > > 
> > > Applied, with a minor addition just to warn people from not using it in
> > > other configurations (#ifdef still needed otherwise the
> > > acpi_parking_protocol_valid symbol is not available; but I prefer uglier
> > > code than people starting to use this IPI in their firmware):
> > 
> > It makes sense, we could include asm/acpi.h in smp.c (which is not
> > included by linux/acpi.h if !CONFIG_ACPI) to pull in the symbol and
> > remove the ifdef if you think it is cleaner.
> 
> I don't think it's worth.
> 
> BTW, the acpi_parking_protocol_valid() definition has an __init
> annotation while the declaration does not. I removed the __init
> altogether since I get a section mismatch warning when being called from
> handle_IPI.

Catalin,

Itaru spotted an issue related to ioremapping the mailbox in the
cpu_postboot method where I can't really use ioremap since irqs
are disabled on the secondaries coming up at that point, I missed
that, apologies (I wanted to avoid leaving the mailboxes mapped
after boot).

So, options to fix it:

(1) we leave the mailboxes mapped
(2) we remove the FW check in the postboot method (running on
    secondaries)
(3) I add a cpu_ops method to clean-up resources used for
    booting secondaries and there I can unmap the mailboxes

Frankly, they are all unappealing, I would go for (1), patch below, Itaru
can you give it a go please on Mustang against arm64 for-next/core ?

Thanks,
Lorenzo

-- >8 --
diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
index 4b1e5a7..b56fc0d 100644
--- a/arch/arm64/kernel/acpi_parking_protocol.c
+++ b/arch/arm64/kernel/acpi_parking_protocol.c
@@ -21,7 +21,14 @@
 
 #include <asm/cpu_ops.h>
 
+struct parking_protocol_mailbox {
+	__le32 cpu_id;
+	__le32 reserved;
+	__le64 entry_point;
+};
+
 struct cpu_mailbox_entry {
+	struct parking_protocol_mailbox __iomem *mailbox;
 	phys_addr_t mailbox_addr;
 	u8 version;
 	u8 gic_cpu_id;
@@ -59,12 +66,6 @@ static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
 	return 0;
 }
 
-struct parking_protocol_mailbox {
-	__le32 cpu_id;
-	__le32 reserved;
-	__le64 entry_point;
-};
-
 static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 {
 	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
@@ -107,7 +108,11 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 
-	iounmap(mailbox);
+	/*
+	 * stash the mailbox address mapping to use it for checks
+	 * in post boot method
+	 */
+	cpu_entry->mailbox = mailbox;
 
 	return 0;
 }
@@ -116,32 +121,15 @@ static void acpi_parking_protocol_cpu_postboot(void)
 {
 	int cpu = smp_processor_id();
 	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
-	struct parking_protocol_mailbox __iomem *mailbox;
+	struct parking_protocol_mailbox __iomem *mailbox = cpu_entry->mailbox;
 	__le64 entry_point;
 
-	/*
-	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
-	 * this deviates from the parking protocol specifications since
-	 * the mailboxes are required to be mapped nGnRnE; the attribute
-	 * discrepancy is harmless insofar as the protocol specification
-	 * is concerned).
-	 * If the mailbox is mistakenly allocated in the linear mapping
-	 * by FW ioremap will fail since the mapping will be prevented
-	 * by the kernel (it clashes with the linear mapping attributes
-	 * specifications).
-	 */
-	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
-	if (!mailbox)
-		return;
-
 	entry_point = readl_relaxed(&mailbox->entry_point);
 	/*
 	 * Check if firmware has cleared the entry_point as expected
 	 * by the protocol specification.
 	 */
 	WARN_ON(entry_point);
-
-	iounmap(mailbox);
 }
 
 const struct cpu_operations acpi_parking_protocol_ops = {

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-02-24 14:18         ` Lorenzo Pieralisi
  0 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-02-24 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

[+ Itaru]

On Wed, Feb 03, 2016 at 04:18:36PM +0000, Catalin Marinas wrote:
> On Wed, Feb 03, 2016 at 11:21:12AM +0000, Lorenzo Pieralisi wrote:
> > On Tue, Feb 02, 2016 at 06:26:58PM +0000, Catalin Marinas wrote:
> > > On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
> > > > The SBBR and ACPI specifications allow ACPI based systems that do not
> > > > implement PSCI (eg systems with no EL3) to boot through the ACPI parking
> > > > protocol specification[1].
> > > > 
> > > > This patch implements the ACPI parking protocol CPU operations, and adds
> > > > code that eases parsing the parking protocol data structures to the
> > > > ARM64 SMP initializion carried out at the same time as cpus enumeration.
> > > > 
> > > > To wake-up the CPUs from the parked state, this patch implements a
> > > > wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
> > > > ARM one, so that a specific IPI is sent for wake-up purpose in order
> > > > to distinguish it from other IPI sources.
> > > > 
> > > > Given the current ACPI MADT parsing API, the patch implements a glue
> > > > layer that helps passing MADT GICC data structure from SMP initialization
> > > > code to the parking protocol implementation somewhat overriding the CPU
> > > > operations interfaces. This to avoid creating a completely trasparent
> > > > DT/ACPI CPU operations layer that would require creating opaque
> > > > structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
> > > > through static MADT table entries), which seems overkill given that ACPI
> > > > on ARM64 mandates only two booting protocols (PSCI and parking protocol),
> > > > so there is no need for further protocol additions.
> > > > 
> > > > Based on the original work by Mark Salter <msalter@redhat.com>
> > > > 
> > > > [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
> > > > 
> > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Cc: Will Deacon <will.deacon@arm.com>
> > > > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > > > Cc: Loc Ho <lho@apm.com>
> > > > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > Cc: Mark Salter <msalter@redhat.com>
> > > > Cc: Al Stone <ahs3@redhat.com>
> > > 
> > > Applied, with a minor addition just to warn people from not using it in
> > > other configurations (#ifdef still needed otherwise the
> > > acpi_parking_protocol_valid symbol is not available; but I prefer uglier
> > > code than people starting to use this IPI in their firmware):
> > 
> > It makes sense, we could include asm/acpi.h in smp.c (which is not
> > included by linux/acpi.h if !CONFIG_ACPI) to pull in the symbol and
> > remove the ifdef if you think it is cleaner.
> 
> I don't think it's worth.
> 
> BTW, the acpi_parking_protocol_valid() definition has an __init
> annotation while the declaration does not. I removed the __init
> altogether since I get a section mismatch warning when being called from
> handle_IPI.

Catalin,

Itaru spotted an issue related to ioremapping the mailbox in the
cpu_postboot method where I can't really use ioremap since irqs
are disabled on the secondaries coming up at that point, I missed
that, apologies (I wanted to avoid leaving the mailboxes mapped
after boot).

So, options to fix it:

(1) we leave the mailboxes mapped
(2) we remove the FW check in the postboot method (running on
    secondaries)
(3) I add a cpu_ops method to clean-up resources used for
    booting secondaries and there I can unmap the mailboxes

Frankly, they are all unappealing, I would go for (1), patch below, Itaru
can you give it a go please on Mustang against arm64 for-next/core ?

Thanks,
Lorenzo

-- >8 --
diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
index 4b1e5a7..b56fc0d 100644
--- a/arch/arm64/kernel/acpi_parking_protocol.c
+++ b/arch/arm64/kernel/acpi_parking_protocol.c
@@ -21,7 +21,14 @@
 
 #include <asm/cpu_ops.h>
 
+struct parking_protocol_mailbox {
+	__le32 cpu_id;
+	__le32 reserved;
+	__le64 entry_point;
+};
+
 struct cpu_mailbox_entry {
+	struct parking_protocol_mailbox __iomem *mailbox;
 	phys_addr_t mailbox_addr;
 	u8 version;
 	u8 gic_cpu_id;
@@ -59,12 +66,6 @@ static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
 	return 0;
 }
 
-struct parking_protocol_mailbox {
-	__le32 cpu_id;
-	__le32 reserved;
-	__le64 entry_point;
-};
-
 static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 {
 	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
@@ -107,7 +108,11 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 
-	iounmap(mailbox);
+	/*
+	 * stash the mailbox address mapping to use it for checks
+	 * in post boot method
+	 */
+	cpu_entry->mailbox = mailbox;
 
 	return 0;
 }
@@ -116,32 +121,15 @@ static void acpi_parking_protocol_cpu_postboot(void)
 {
 	int cpu = smp_processor_id();
 	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
-	struct parking_protocol_mailbox __iomem *mailbox;
+	struct parking_protocol_mailbox __iomem *mailbox = cpu_entry->mailbox;
 	__le64 entry_point;
 
-	/*
-	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
-	 * this deviates from the parking protocol specifications since
-	 * the mailboxes are required to be mapped nGnRnE; the attribute
-	 * discrepancy is harmless insofar as the protocol specification
-	 * is concerned).
-	 * If the mailbox is mistakenly allocated in the linear mapping
-	 * by FW ioremap will fail since the mapping will be prevented
-	 * by the kernel (it clashes with the linear mapping attributes
-	 * specifications).
-	 */
-	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
-	if (!mailbox)
-		return;
-
 	entry_point = readl_relaxed(&mailbox->entry_point);
 	/*
 	 * Check if firmware has cleared the entry_point as expected
 	 * by the protocol specification.
 	 */
 	WARN_ON(entry_point);
-
-	iounmap(mailbox);
 }
 
 const struct cpu_operations acpi_parking_protocol_ops = {

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-02-24 14:18         ` Lorenzo Pieralisi
@ 2016-02-24 15:03           ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-02-24 15:03 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Mark Rutland, Sudeep Holla, Will Deacon, linux-acpi, Hanjun Guo,
	Mark Salter, Al Stone, linux-arm-kernel, Loc Ho, Itaru Kitayama

On Wed, Feb 24, 2016 at 02:18:32PM +0000, Lorenzo Pieralisi wrote:

[...]

>  static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>  {
>  	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> @@ -107,7 +108,11 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>  
>  	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>  
> -	iounmap(mailbox);
> +	/*
> +	 * stash the mailbox address mapping to use it for checks
> +	 * in post boot method
> +	 */
> +	cpu_entry->mailbox = mailbox;

The mailbox VA entry should be stashed before setting up the mailbox
and sending the IPI, anyway, if Loc and Itaru can test it I will wrap
this change into the final version with appropriate log.

Lorenzo

>  
>  	return 0;
>  }
> @@ -116,32 +121,15 @@ static void acpi_parking_protocol_cpu_postboot(void)
>  {
>  	int cpu = smp_processor_id();
>  	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> -	struct parking_protocol_mailbox __iomem *mailbox;
> +	struct parking_protocol_mailbox __iomem *mailbox = cpu_entry->mailbox;
>  	__le64 entry_point;
>  
> -	/*
> -	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
> -	 * this deviates from the parking protocol specifications since
> -	 * the mailboxes are required to be mapped nGnRnE; the attribute
> -	 * discrepancy is harmless insofar as the protocol specification
> -	 * is concerned).
> -	 * If the mailbox is mistakenly allocated in the linear mapping
> -	 * by FW ioremap will fail since the mapping will be prevented
> -	 * by the kernel (it clashes with the linear mapping attributes
> -	 * specifications).
> -	 */
> -	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> -	if (!mailbox)
> -		return;
> -
>  	entry_point = readl_relaxed(&mailbox->entry_point);
>  	/*
>  	 * Check if firmware has cleared the entry_point as expected
>  	 * by the protocol specification.
>  	 */
>  	WARN_ON(entry_point);
> -
> -	iounmap(mailbox);
>  }
>  
>  const struct cpu_operations acpi_parking_protocol_ops = {

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-02-24 15:03           ` Lorenzo Pieralisi
  0 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-02-24 15:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 24, 2016 at 02:18:32PM +0000, Lorenzo Pieralisi wrote:

[...]

>  static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>  {
>  	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> @@ -107,7 +108,11 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>  
>  	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>  
> -	iounmap(mailbox);
> +	/*
> +	 * stash the mailbox address mapping to use it for checks
> +	 * in post boot method
> +	 */
> +	cpu_entry->mailbox = mailbox;

The mailbox VA entry should be stashed before setting up the mailbox
and sending the IPI, anyway, if Loc and Itaru can test it I will wrap
this change into the final version with appropriate log.

Lorenzo

>  
>  	return 0;
>  }
> @@ -116,32 +121,15 @@ static void acpi_parking_protocol_cpu_postboot(void)
>  {
>  	int cpu = smp_processor_id();
>  	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> -	struct parking_protocol_mailbox __iomem *mailbox;
> +	struct parking_protocol_mailbox __iomem *mailbox = cpu_entry->mailbox;
>  	__le64 entry_point;
>  
> -	/*
> -	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
> -	 * this deviates from the parking protocol specifications since
> -	 * the mailboxes are required to be mapped nGnRnE; the attribute
> -	 * discrepancy is harmless insofar as the protocol specification
> -	 * is concerned).
> -	 * If the mailbox is mistakenly allocated in the linear mapping
> -	 * by FW ioremap will fail since the mapping will be prevented
> -	 * by the kernel (it clashes with the linear mapping attributes
> -	 * specifications).
> -	 */
> -	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> -	if (!mailbox)
> -		return;
> -
>  	entry_point = readl_relaxed(&mailbox->entry_point);
>  	/*
>  	 * Check if firmware has cleared the entry_point as expected
>  	 * by the protocol specification.
>  	 */
>  	WARN_ON(entry_point);
> -
> -	iounmap(mailbox);
>  }
>  
>  const struct cpu_operations acpi_parking_protocol_ops = {

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-02-24 14:18         ` Lorenzo Pieralisi
@ 2016-02-24 23:28           ` Itaru Kitayama
  -1 siblings, 0 replies; 32+ messages in thread
From: Itaru Kitayama @ 2016-02-24 23:28 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Mark Rutland, Catalin Marinas, Mark Salter, Will Deacon,
	linux-acpi, Hanjun Guo, Sudeep Holla, Al Stone, linux-arm-kernel,
	Loc Ho

Lorenzo,

On Mustang the lockdep warning I reported to you is disappeared and the 
system gets to
the prompt as you implemented, however only 7 CPUs out of 8 brought up.

[    0.069461] ACPI: Core revision 20160108
[    0.098175] ACPI: 4 ACPI AML tables successfully acquired and loaded

[    0.103899] Security Framework initialized
[    0.106883] Mount-cache hash table entries: 65536 (order: 7, 524288 
bytes)
[    0.112616] Mountpoint-cache hash table entries: 65536 (order: 7, 
524288 bytes)
[    0.133953] ftrace: allocating 25237 entries in 99 pages
[    0.228040] ASID allocator initialised with 65536 entries
[    0.253856] Remapping and enabling EFI services.
[    0.257328]   EFI remap 0x0000000010510000 => 0000000020000000
[    0.262000]   EFI remap 0x0000000010548000 => 0000000020018000
[    0.266702]   EFI remap 0x0000000017000000 => 0000000020020000
[    0.271403]   EFI remap 0x000000001c025000 => 0000000020035000
[    0.276060]   EFI remap 0x000000007c0c0000 => 0000000020040000
[    0.280786]   EFI remap 0x000000007c1c0000 => 0000000020050000
[    0.285495]   EFI remap 0x000000007c2c0000 => 0000000020060000
[    0.290204]   EFI remap 0x000000007c3c0000 => 0000000020070000
[    0.294861]   EFI remap 0x000000007c4c0000 => 0000000020080000
[    0.299535]   EFI remap 0x000000007c5c0000 => 0000000020090000
[    0.304244]   EFI remap 0x000000007c6c0000 => 00000000200a0000
[    0.308927]   EFI remap 0x000000007c7c0000 => 00000000200b0000
[    0.313671]   EFI remap 0x00000047fa80c000 => 00000000200cc000
[    0.318428]   EFI remap 0x00000047fa854000 => 00000000200d4000
[    0.323139]   EFI remap 0x00000047ff9fc000 => 00000000202cc000
[    0.327894]   EFI remap 0x00000047ffa10000 => 00000000202e0000
[    0.349223] Unable to handle kernel NULL pointer dereference at 
virtual address 00000008
[    0.349228] pgd = ffffff800a918000
[    0.349236] [00000008] *pgd=00000047ffffe003, *pud=00000047ffffe003, 
*pmd=0000000000000000
[    0.349242] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[    0.349248] Modules linked in:
[    0.349255] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.5.0-rc4-core+ #32
[    0.349257] Hardware name: APM X-Gene Mustang board (DT)
[    0.349261] task: ffffffc7fab6c600 ti: ffffffc7fab70000 task.ti: 
ffffffc7fab70000
[    0.349268] PC is at acpi_parking_protocol_cpu_postboot+0x38/0xa8
[    0.349271] LR is at acpi_parking_protocol_cpu_postboot+0x1c/0xa8
[    0.349274] pc : [<ffffff80080a0a98>] lr : [<ffffff80080a0a7c>] 
pstate: 600003c5
[    0.349275] sp : ffffffc7fab73f50
[    0.349280] x29: ffffffc7fab73f50 x28: 00000000fffebd7d
[    0.349285] x27: ffffff8008082d00 x26: 0000004002b18000
[    0.349289] x25: 0000004002b15000 x24: ffffff80097d1608
[    0.349293] x23: ffffff80096554a8 x22: ffffff80097d1000
[    0.349297] x21: ffffff800972d000 x20: 0000000000000001
[    0.349302] x19: 0000000000000008 x18: 0000000030d00800
[    0.349306] x17: 000000001d100000 x16: 00000000fef7ffed
[    0.349310] x15: dedffffebe7efcae x14: 00000000f2fdfbfd
[    0.349314] x13: 00000000ae7e9e7f x12: 00000000800000c9
[    0.349319] x11: 000000009df2cf7c x10: 0000000000000000
[    0.349323] x9 : 0000000000000000 x8 : ffffff80097cf740
[    0.349327] x7 : 0000000000000000 x6 : 00000000000066a7
[    0.349332] x5 : ffffff80097cf740 x4 : ffffff80098607e0
[    0.349336] x3 : 0000000000000003 x2 : 0000000000000004
[    0.349340] x1 : ffffff8009bfa810 x0 : 0000000000000003

[    0.349344] Process swapper/1 (pid: 0, stack limit = 0xffffffc7fab70020)
[    0.349347] Stack: (0xffffffc7fab73f50 to 0xffffffc7fab74000)
[    0.349351] 3f40: ffffffc7fab73f80 ffffff80080972f8
[    0.349355] 3f60: ffffff80096554a8 0000000000000001 ffffff800972d000 
ffffff80080972b8
[    0.349358] 3f80: 0000000000000000 0000004000282cec 000000001d100000 
0000000000000e12
[    0.349362] 3fa0: 000000001d100000 00000000b77ff5fd 000000001d100000 
00000000f77f7d7f
[    0.349365] 3fc0: 0000004002b15000 0000004002b18000 ffffff8008082d00 
00000000fffebd7d
[    0.349368] 3fe0: 0000000000000000 0000000000000000 0000000000000000 
0000000000000000
[    0.349370] Call trace:
[    0.349374] Exception stack(0xffffffc7fab73d90 to 0xffffffc7fab73eb0)
[    0.349377] 3d80: 0000000000000008 0000000000000001
[    0.349380] 3da0: ffffffc7fab73f50 ffffff80080a0a98 0000000000000000 
0000000000000000
[    0.349383] 3dc0: 0000000000000000 0000000000000000 0000000000000000 
0000000000000000
[    0.349387] 3de0: 0000000000000000 0000000000000000 0000000000000000 
0000000000000000
[    0.349390] 3e00: 0000000000000000 0000000000000000 0000000000000000 
0000000000000000
[    0.349393] 3e20: 0000000000000000 0000000000000000 0000000000000003 
ffffff8009bfa810
[    0.349397] 3e40: 0000000000000004 0000000000000003 ffffff80098607e0 
ffffff80097cf740
[    0.349400] 3e60: 00000000000066a7 0000000000000000 ffffff80097cf740 
0000000000000000
[    0.349404] 3e80: 0000000000000000 000000009df2cf7c 00000000800000c9 
00000000ae7e9e7f
[    0.349406] 3ea0: 00000000f2fdfbfd dedffffebe7efcae
[    0.349411] [<ffffff80080a0a98>] 
acpi_parking_protocol_cpu_postboot+0x38/0xa8
[    0.349415] [<ffffff80080972f8>] secondary_start_kernel+0x268/0x538
[    0.349419] [<0000004000282cec>] 0x4000282cec
[    0.349423] Code: 91204021 cb000040 f8607833 91002273 (b9400273)
[    0.349483] ---[ end trace 80a6d1ac53b24c3c ]---
[    0.349487] Kernel panic - not syncing: Fatal exception
[    0.349496] ---[ end Kernel panic - not syncing: Fatal exception
[    1.350177] CPU1: failed to come online
[    1.367354] Detected PIPT I-cache on CPU2
[    1.367398] CPU2: Booted secondary processor [500f0001]
[    1.379399] Detected PIPT I-cache on CPU3
[    1.379425] CPU3: Booted secondary processor [500f0001]
[    1.391487] Detected PIPT I-cache on CPU4
[    1.391521] CPU4: Booted secondary processor [500f0001]
[    1.403536] Detected PIPT I-cache on CPU5
[    1.403562] CPU5: Booted secondary processor [500f0001]
[    1.415590] Detected PIPT I-cache on CPU6
[    1.415625] CPU6: Booted secondary processor [500f0001]
[    1.427677] Detected PIPT I-cache on CPU7
[    1.427702] CPU7: Booted secondary processor [500f0001]
[    1.427823] Brought up 7 CPUs
[    1.470268] SMP: Total of 7 processors activated.
[    1.473801] CPU: All CPU(s) started at EL2


On 2/24/16 11:18 PM, Lorenzo Pieralisi wrote:
> [+ Itaru]
>
> On Wed, Feb 03, 2016 at 04:18:36PM +0000, Catalin Marinas wrote:
>> On Wed, Feb 03, 2016 at 11:21:12AM +0000, Lorenzo Pieralisi wrote:
>>> On Tue, Feb 02, 2016 at 06:26:58PM +0000, Catalin Marinas wrote:
>>>> On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
>>>>> The SBBR and ACPI specifications allow ACPI based systems that do not
>>>>> implement PSCI (eg systems with no EL3) to boot through the ACPI parking
>>>>> protocol specification[1].
>>>>>
>>>>> This patch implements the ACPI parking protocol CPU operations, and adds
>>>>> code that eases parsing the parking protocol data structures to the
>>>>> ARM64 SMP initializion carried out at the same time as cpus enumeration.
>>>>>
>>>>> To wake-up the CPUs from the parked state, this patch implements a
>>>>> wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
>>>>> ARM one, so that a specific IPI is sent for wake-up purpose in order
>>>>> to distinguish it from other IPI sources.
>>>>>
>>>>> Given the current ACPI MADT parsing API, the patch implements a glue
>>>>> layer that helps passing MADT GICC data structure from SMP initialization
>>>>> code to the parking protocol implementation somewhat overriding the CPU
>>>>> operations interfaces. This to avoid creating a completely trasparent
>>>>> DT/ACPI CPU operations layer that would require creating opaque
>>>>> structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
>>>>> through static MADT table entries), which seems overkill given that ACPI
>>>>> on ARM64 mandates only two booting protocols (PSCI and parking protocol),
>>>>> so there is no need for further protocol additions.
>>>>>
>>>>> Based on the original work by Mark Salter <msalter@redhat.com>
>>>>>
>>>>> [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
>>>>>
>>>>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>>> Cc: Will Deacon <will.deacon@arm.com>
>>>>> Cc: Hanjun Guo <hanjun.guo@linaro.org>
>>>>> Cc: Loc Ho <lho@apm.com>
>>>>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>>> Cc: Mark Salter <msalter@redhat.com>
>>>>> Cc: Al Stone <ahs3@redhat.com>
>>>> Applied, with a minor addition just to warn people from not using it in
>>>> other configurations (#ifdef still needed otherwise the
>>>> acpi_parking_protocol_valid symbol is not available; but I prefer uglier
>>>> code than people starting to use this IPI in their firmware):
>>> It makes sense, we could include asm/acpi.h in smp.c (which is not
>>> included by linux/acpi.h if !CONFIG_ACPI) to pull in the symbol and
>>> remove the ifdef if you think it is cleaner.
>> I don't think it's worth.
>>
>> BTW, the acpi_parking_protocol_valid() definition has an __init
>> annotation while the declaration does not. I removed the __init
>> altogether since I get a section mismatch warning when being called from
>> handle_IPI.
> Catalin,
>
> Itaru spotted an issue related to ioremapping the mailbox in the
> cpu_postboot method where I can't really use ioremap since irqs
> are disabled on the secondaries coming up at that point, I missed
> that, apologies (I wanted to avoid leaving the mailboxes mapped
> after boot).
>
> So, options to fix it:
>
> (1) we leave the mailboxes mapped
> (2) we remove the FW check in the postboot method (running on
>      secondaries)
> (3) I add a cpu_ops method to clean-up resources used for
>      booting secondaries and there I can unmap the mailboxes
>
> Frankly, they are all unappealing, I would go for (1), patch below, Itaru
> can you give it a go please on Mustang against arm64 for-next/core ?
>
> Thanks,
> Lorenzo
>
> -- >8 --
> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
> index 4b1e5a7..b56fc0d 100644
> --- a/arch/arm64/kernel/acpi_parking_protocol.c
> +++ b/arch/arm64/kernel/acpi_parking_protocol.c
> @@ -21,7 +21,14 @@
>   
>   #include <asm/cpu_ops.h>
>   
> +struct parking_protocol_mailbox {
> +	__le32 cpu_id;
> +	__le32 reserved;
> +	__le64 entry_point;
> +};
> +
>   struct cpu_mailbox_entry {
> +	struct parking_protocol_mailbox __iomem *mailbox;
>   	phys_addr_t mailbox_addr;
>   	u8 version;
>   	u8 gic_cpu_id;
> @@ -59,12 +66,6 @@ static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
>   	return 0;
>   }
>   
> -struct parking_protocol_mailbox {
> -	__le32 cpu_id;
> -	__le32 reserved;
> -	__le64 entry_point;
> -};
> -
>   static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>   {
>   	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> @@ -107,7 +108,11 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>   
>   	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>   
> -	iounmap(mailbox);
> +	/*
> +	 * stash the mailbox address mapping to use it for checks
> +	 * in post boot method
> +	 */
> +	cpu_entry->mailbox = mailbox;
>   
>   	return 0;
>   }
> @@ -116,32 +121,15 @@ static void acpi_parking_protocol_cpu_postboot(void)
>   {
>   	int cpu = smp_processor_id();
>   	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> -	struct parking_protocol_mailbox __iomem *mailbox;
> +	struct parking_protocol_mailbox __iomem *mailbox = cpu_entry->mailbox;
>   	__le64 entry_point;
>   
> -	/*
> -	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
> -	 * this deviates from the parking protocol specifications since
> -	 * the mailboxes are required to be mapped nGnRnE; the attribute
> -	 * discrepancy is harmless insofar as the protocol specification
> -	 * is concerned).
> -	 * If the mailbox is mistakenly allocated in the linear mapping
> -	 * by FW ioremap will fail since the mapping will be prevented
> -	 * by the kernel (it clashes with the linear mapping attributes
> -	 * specifications).
> -	 */
> -	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> -	if (!mailbox)
> -		return;
> -
>   	entry_point = readl_relaxed(&mailbox->entry_point);
>   	/*
>   	 * Check if firmware has cleared the entry_point as expected
>   	 * by the protocol specification.
>   	 */
>   	WARN_ON(entry_point);
> -
> -	iounmap(mailbox);
>   }
>   
>   const struct cpu_operations acpi_parking_protocol_ops = {

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-02-24 23:28           ` Itaru Kitayama
  0 siblings, 0 replies; 32+ messages in thread
From: Itaru Kitayama @ 2016-02-24 23:28 UTC (permalink / raw)
  To: linux-arm-kernel

Lorenzo,

On Mustang the lockdep warning I reported to you is disappeared and the 
system gets to
the prompt as you implemented, however only 7 CPUs out of 8 brought up.

[    0.069461] ACPI: Core revision 20160108
[    0.098175] ACPI: 4 ACPI AML tables successfully acquired and loaded

[    0.103899] Security Framework initialized
[    0.106883] Mount-cache hash table entries: 65536 (order: 7, 524288 
bytes)
[    0.112616] Mountpoint-cache hash table entries: 65536 (order: 7, 
524288 bytes)
[    0.133953] ftrace: allocating 25237 entries in 99 pages
[    0.228040] ASID allocator initialised with 65536 entries
[    0.253856] Remapping and enabling EFI services.
[    0.257328]   EFI remap 0x0000000010510000 => 0000000020000000
[    0.262000]   EFI remap 0x0000000010548000 => 0000000020018000
[    0.266702]   EFI remap 0x0000000017000000 => 0000000020020000
[    0.271403]   EFI remap 0x000000001c025000 => 0000000020035000
[    0.276060]   EFI remap 0x000000007c0c0000 => 0000000020040000
[    0.280786]   EFI remap 0x000000007c1c0000 => 0000000020050000
[    0.285495]   EFI remap 0x000000007c2c0000 => 0000000020060000
[    0.290204]   EFI remap 0x000000007c3c0000 => 0000000020070000
[    0.294861]   EFI remap 0x000000007c4c0000 => 0000000020080000
[    0.299535]   EFI remap 0x000000007c5c0000 => 0000000020090000
[    0.304244]   EFI remap 0x000000007c6c0000 => 00000000200a0000
[    0.308927]   EFI remap 0x000000007c7c0000 => 00000000200b0000
[    0.313671]   EFI remap 0x00000047fa80c000 => 00000000200cc000
[    0.318428]   EFI remap 0x00000047fa854000 => 00000000200d4000
[    0.323139]   EFI remap 0x00000047ff9fc000 => 00000000202cc000
[    0.327894]   EFI remap 0x00000047ffa10000 => 00000000202e0000
[    0.349223] Unable to handle kernel NULL pointer dereference at 
virtual address 00000008
[    0.349228] pgd = ffffff800a918000
[    0.349236] [00000008] *pgd=00000047ffffe003, *pud=00000047ffffe003, 
*pmd=0000000000000000
[    0.349242] Internal error: Oops: 96000005 [#1] PREEMPT SMP
[    0.349248] Modules linked in:
[    0.349255] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.5.0-rc4-core+ #32
[    0.349257] Hardware name: APM X-Gene Mustang board (DT)
[    0.349261] task: ffffffc7fab6c600 ti: ffffffc7fab70000 task.ti: 
ffffffc7fab70000
[    0.349268] PC is at acpi_parking_protocol_cpu_postboot+0x38/0xa8
[    0.349271] LR is at acpi_parking_protocol_cpu_postboot+0x1c/0xa8
[    0.349274] pc : [<ffffff80080a0a98>] lr : [<ffffff80080a0a7c>] 
pstate: 600003c5
[    0.349275] sp : ffffffc7fab73f50
[    0.349280] x29: ffffffc7fab73f50 x28: 00000000fffebd7d
[    0.349285] x27: ffffff8008082d00 x26: 0000004002b18000
[    0.349289] x25: 0000004002b15000 x24: ffffff80097d1608
[    0.349293] x23: ffffff80096554a8 x22: ffffff80097d1000
[    0.349297] x21: ffffff800972d000 x20: 0000000000000001
[    0.349302] x19: 0000000000000008 x18: 0000000030d00800
[    0.349306] x17: 000000001d100000 x16: 00000000fef7ffed
[    0.349310] x15: dedffffebe7efcae x14: 00000000f2fdfbfd
[    0.349314] x13: 00000000ae7e9e7f x12: 00000000800000c9
[    0.349319] x11: 000000009df2cf7c x10: 0000000000000000
[    0.349323] x9 : 0000000000000000 x8 : ffffff80097cf740
[    0.349327] x7 : 0000000000000000 x6 : 00000000000066a7
[    0.349332] x5 : ffffff80097cf740 x4 : ffffff80098607e0
[    0.349336] x3 : 0000000000000003 x2 : 0000000000000004
[    0.349340] x1 : ffffff8009bfa810 x0 : 0000000000000003

[    0.349344] Process swapper/1 (pid: 0, stack limit = 0xffffffc7fab70020)
[    0.349347] Stack: (0xffffffc7fab73f50 to 0xffffffc7fab74000)
[    0.349351] 3f40: ffffffc7fab73f80 ffffff80080972f8
[    0.349355] 3f60: ffffff80096554a8 0000000000000001 ffffff800972d000 
ffffff80080972b8
[    0.349358] 3f80: 0000000000000000 0000004000282cec 000000001d100000 
0000000000000e12
[    0.349362] 3fa0: 000000001d100000 00000000b77ff5fd 000000001d100000 
00000000f77f7d7f
[    0.349365] 3fc0: 0000004002b15000 0000004002b18000 ffffff8008082d00 
00000000fffebd7d
[    0.349368] 3fe0: 0000000000000000 0000000000000000 0000000000000000 
0000000000000000
[    0.349370] Call trace:
[    0.349374] Exception stack(0xffffffc7fab73d90 to 0xffffffc7fab73eb0)
[    0.349377] 3d80: 0000000000000008 0000000000000001
[    0.349380] 3da0: ffffffc7fab73f50 ffffff80080a0a98 0000000000000000 
0000000000000000
[    0.349383] 3dc0: 0000000000000000 0000000000000000 0000000000000000 
0000000000000000
[    0.349387] 3de0: 0000000000000000 0000000000000000 0000000000000000 
0000000000000000
[    0.349390] 3e00: 0000000000000000 0000000000000000 0000000000000000 
0000000000000000
[    0.349393] 3e20: 0000000000000000 0000000000000000 0000000000000003 
ffffff8009bfa810
[    0.349397] 3e40: 0000000000000004 0000000000000003 ffffff80098607e0 
ffffff80097cf740
[    0.349400] 3e60: 00000000000066a7 0000000000000000 ffffff80097cf740 
0000000000000000
[    0.349404] 3e80: 0000000000000000 000000009df2cf7c 00000000800000c9 
00000000ae7e9e7f
[    0.349406] 3ea0: 00000000f2fdfbfd dedffffebe7efcae
[    0.349411] [<ffffff80080a0a98>] 
acpi_parking_protocol_cpu_postboot+0x38/0xa8
[    0.349415] [<ffffff80080972f8>] secondary_start_kernel+0x268/0x538
[    0.349419] [<0000004000282cec>] 0x4000282cec
[    0.349423] Code: 91204021 cb000040 f8607833 91002273 (b9400273)
[    0.349483] ---[ end trace 80a6d1ac53b24c3c ]---
[    0.349487] Kernel panic - not syncing: Fatal exception
[    0.349496] ---[ end Kernel panic - not syncing: Fatal exception
[    1.350177] CPU1: failed to come online
[    1.367354] Detected PIPT I-cache on CPU2
[    1.367398] CPU2: Booted secondary processor [500f0001]
[    1.379399] Detected PIPT I-cache on CPU3
[    1.379425] CPU3: Booted secondary processor [500f0001]
[    1.391487] Detected PIPT I-cache on CPU4
[    1.391521] CPU4: Booted secondary processor [500f0001]
[    1.403536] Detected PIPT I-cache on CPU5
[    1.403562] CPU5: Booted secondary processor [500f0001]
[    1.415590] Detected PIPT I-cache on CPU6
[    1.415625] CPU6: Booted secondary processor [500f0001]
[    1.427677] Detected PIPT I-cache on CPU7
[    1.427702] CPU7: Booted secondary processor [500f0001]
[    1.427823] Brought up 7 CPUs
[    1.470268] SMP: Total of 7 processors activated.
[    1.473801] CPU: All CPU(s) started at EL2


On 2/24/16 11:18 PM, Lorenzo Pieralisi wrote:
> [+ Itaru]
>
> On Wed, Feb 03, 2016 at 04:18:36PM +0000, Catalin Marinas wrote:
>> On Wed, Feb 03, 2016 at 11:21:12AM +0000, Lorenzo Pieralisi wrote:
>>> On Tue, Feb 02, 2016 at 06:26:58PM +0000, Catalin Marinas wrote:
>>>> On Tue, Jan 26, 2016 at 11:10:38AM +0000, Lorenzo Pieralisi wrote:
>>>>> The SBBR and ACPI specifications allow ACPI based systems that do not
>>>>> implement PSCI (eg systems with no EL3) to boot through the ACPI parking
>>>>> protocol specification[1].
>>>>>
>>>>> This patch implements the ACPI parking protocol CPU operations, and adds
>>>>> code that eases parsing the parking protocol data structures to the
>>>>> ARM64 SMP initializion carried out at the same time as cpus enumeration.
>>>>>
>>>>> To wake-up the CPUs from the parked state, this patch implements a
>>>>> wakeup IPI for ARM64 (ie arch_send_wakeup_ipi_mask()) that mirrors the
>>>>> ARM one, so that a specific IPI is sent for wake-up purpose in order
>>>>> to distinguish it from other IPI sources.
>>>>>
>>>>> Given the current ACPI MADT parsing API, the patch implements a glue
>>>>> layer that helps passing MADT GICC data structure from SMP initialization
>>>>> code to the parking protocol implementation somewhat overriding the CPU
>>>>> operations interfaces. This to avoid creating a completely trasparent
>>>>> DT/ACPI CPU operations layer that would require creating opaque
>>>>> structure handling for CPUs data (DT represents CPU through DT nodes, ACPI
>>>>> through static MADT table entries), which seems overkill given that ACPI
>>>>> on ARM64 mandates only two booting protocols (PSCI and parking protocol),
>>>>> so there is no need for further protocol additions.
>>>>>
>>>>> Based on the original work by Mark Salter <msalter@redhat.com>
>>>>>
>>>>> [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx
>>>>>
>>>>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>>> Cc: Will Deacon <will.deacon@arm.com>
>>>>> Cc: Hanjun Guo <hanjun.guo@linaro.org>
>>>>> Cc: Loc Ho <lho@apm.com>
>>>>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>>> Cc: Mark Salter <msalter@redhat.com>
>>>>> Cc: Al Stone <ahs3@redhat.com>
>>>> Applied, with a minor addition just to warn people from not using it in
>>>> other configurations (#ifdef still needed otherwise the
>>>> acpi_parking_protocol_valid symbol is not available; but I prefer uglier
>>>> code than people starting to use this IPI in their firmware):
>>> It makes sense, we could include asm/acpi.h in smp.c (which is not
>>> included by linux/acpi.h if !CONFIG_ACPI) to pull in the symbol and
>>> remove the ifdef if you think it is cleaner.
>> I don't think it's worth.
>>
>> BTW, the acpi_parking_protocol_valid() definition has an __init
>> annotation while the declaration does not. I removed the __init
>> altogether since I get a section mismatch warning when being called from
>> handle_IPI.
> Catalin,
>
> Itaru spotted an issue related to ioremapping the mailbox in the
> cpu_postboot method where I can't really use ioremap since irqs
> are disabled on the secondaries coming up at that point, I missed
> that, apologies (I wanted to avoid leaving the mailboxes mapped
> after boot).
>
> So, options to fix it:
>
> (1) we leave the mailboxes mapped
> (2) we remove the FW check in the postboot method (running on
>      secondaries)
> (3) I add a cpu_ops method to clean-up resources used for
>      booting secondaries and there I can unmap the mailboxes
>
> Frankly, they are all unappealing, I would go for (1), patch below, Itaru
> can you give it a go please on Mustang against arm64 for-next/core ?
>
> Thanks,
> Lorenzo
>
> -- >8 --
> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
> index 4b1e5a7..b56fc0d 100644
> --- a/arch/arm64/kernel/acpi_parking_protocol.c
> +++ b/arch/arm64/kernel/acpi_parking_protocol.c
> @@ -21,7 +21,14 @@
>   
>   #include <asm/cpu_ops.h>
>   
> +struct parking_protocol_mailbox {
> +	__le32 cpu_id;
> +	__le32 reserved;
> +	__le64 entry_point;
> +};
> +
>   struct cpu_mailbox_entry {
> +	struct parking_protocol_mailbox __iomem *mailbox;
>   	phys_addr_t mailbox_addr;
>   	u8 version;
>   	u8 gic_cpu_id;
> @@ -59,12 +66,6 @@ static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
>   	return 0;
>   }
>   
> -struct parking_protocol_mailbox {
> -	__le32 cpu_id;
> -	__le32 reserved;
> -	__le64 entry_point;
> -};
> -
>   static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>   {
>   	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> @@ -107,7 +108,11 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>   
>   	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>   
> -	iounmap(mailbox);
> +	/*
> +	 * stash the mailbox address mapping to use it for checks
> +	 * in post boot method
> +	 */
> +	cpu_entry->mailbox = mailbox;
>   
>   	return 0;
>   }
> @@ -116,32 +121,15 @@ static void acpi_parking_protocol_cpu_postboot(void)
>   {
>   	int cpu = smp_processor_id();
>   	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> -	struct parking_protocol_mailbox __iomem *mailbox;
> +	struct parking_protocol_mailbox __iomem *mailbox = cpu_entry->mailbox;
>   	__le64 entry_point;
>   
> -	/*
> -	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
> -	 * this deviates from the parking protocol specifications since
> -	 * the mailboxes are required to be mapped nGnRnE; the attribute
> -	 * discrepancy is harmless insofar as the protocol specification
> -	 * is concerned).
> -	 * If the mailbox is mistakenly allocated in the linear mapping
> -	 * by FW ioremap will fail since the mapping will be prevented
> -	 * by the kernel (it clashes with the linear mapping attributes
> -	 * specifications).
> -	 */
> -	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> -	if (!mailbox)
> -		return;
> -
>   	entry_point = readl_relaxed(&mailbox->entry_point);
>   	/*
>   	 * Check if firmware has cleared the entry_point as expected
>   	 * by the protocol specification.
>   	 */
>   	WARN_ON(entry_point);
> -
> -	iounmap(mailbox);
>   }
>   
>   const struct cpu_operations acpi_parking_protocol_ops = {

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-02-24 23:28           ` Itaru Kitayama
@ 2016-02-25  9:24             ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-02-25  9:24 UTC (permalink / raw)
  To: Itaru Kitayama
  Cc: Catalin Marinas, Mark Rutland, Sudeep Holla, Will Deacon,
	linux-acpi, Hanjun Guo, Mark Salter, Al Stone, linux-arm-kernel,
	Loc Ho

On Thu, Feb 25, 2016 at 08:28:30AM +0900, Itaru Kitayama wrote:
> Lorenzo,
> 
> On Mustang the lockdep warning I reported to you is disappeared and
> the system gets to
> the prompt as you implemented, however only 7 CPUs out of 8 brought up.

It is because, as I mentioned in the other reply to this thread,
the VA should be stashed before setting up the mailboxes, patch below,
that should be final (I tested it on AMD Seattle too), please give it a go.

Thanks,
Lorenzo

-- >8 --
diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
index 4b1e5a7..dd671ef 100644
--- a/arch/arm64/kernel/acpi_parking_protocol.c
+++ b/arch/arm64/kernel/acpi_parking_protocol.c
@@ -21,7 +21,14 @@
 
 #include <asm/cpu_ops.h>
 
+struct parking_protocol_mailbox {
+	__le32 cpu_id;
+	__le32 reserved;
+	__le64 entry_point;
+};
+
 struct cpu_mailbox_entry {
+	struct parking_protocol_mailbox __iomem *mailbox;
 	phys_addr_t mailbox_addr;
 	u8 version;
 	u8 gic_cpu_id;
@@ -59,12 +66,6 @@ static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
 	return 0;
 }
 
-struct parking_protocol_mailbox {
-	__le32 cpu_id;
-	__le32 reserved;
-	__le64 entry_point;
-};
-
 static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 {
 	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
@@ -97,6 +98,12 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 	}
 
 	/*
+	 * stash the mailbox address mapping to use it for further checks
+	 * in postboot method
+	 */
+	cpu_entry->mailbox = mailbox;
+
+	/*
 	 * We write the entry point and cpu id as LE regardless of the
 	 * native endianness of the kernel. Therefore, any boot-loaders
 	 * that read this address need to convert this address to the
@@ -107,8 +114,6 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 
-	iounmap(mailbox);
-
 	return 0;
 }
 
@@ -116,32 +121,15 @@ static void acpi_parking_protocol_cpu_postboot(void)
 {
 	int cpu = smp_processor_id();
 	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
-	struct parking_protocol_mailbox __iomem *mailbox;
+	struct parking_protocol_mailbox __iomem *mailbox = cpu_entry->mailbox;
 	__le64 entry_point;
 
-	/*
-	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
-	 * this deviates from the parking protocol specifications since
-	 * the mailboxes are required to be mapped nGnRnE; the attribute
-	 * discrepancy is harmless insofar as the protocol specification
-	 * is concerned).
-	 * If the mailbox is mistakenly allocated in the linear mapping
-	 * by FW ioremap will fail since the mapping will be prevented
-	 * by the kernel (it clashes with the linear mapping attributes
-	 * specifications).
-	 */
-	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
-	if (!mailbox)
-		return;
-
 	entry_point = readl_relaxed(&mailbox->entry_point);
 	/*
 	 * Check if firmware has cleared the entry_point as expected
 	 * by the protocol specification.
 	 */
 	WARN_ON(entry_point);
-
-	iounmap(mailbox);
 }
 
 const struct cpu_operations acpi_parking_protocol_ops = {

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-02-25  9:24             ` Lorenzo Pieralisi
  0 siblings, 0 replies; 32+ messages in thread
From: Lorenzo Pieralisi @ 2016-02-25  9:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Feb 25, 2016 at 08:28:30AM +0900, Itaru Kitayama wrote:
> Lorenzo,
> 
> On Mustang the lockdep warning I reported to you is disappeared and
> the system gets to
> the prompt as you implemented, however only 7 CPUs out of 8 brought up.

It is because, as I mentioned in the other reply to this thread,
the VA should be stashed before setting up the mailboxes, patch below,
that should be final (I tested it on AMD Seattle too), please give it a go.

Thanks,
Lorenzo

-- >8 --
diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
index 4b1e5a7..dd671ef 100644
--- a/arch/arm64/kernel/acpi_parking_protocol.c
+++ b/arch/arm64/kernel/acpi_parking_protocol.c
@@ -21,7 +21,14 @@
 
 #include <asm/cpu_ops.h>
 
+struct parking_protocol_mailbox {
+	__le32 cpu_id;
+	__le32 reserved;
+	__le64 entry_point;
+};
+
 struct cpu_mailbox_entry {
+	struct parking_protocol_mailbox __iomem *mailbox;
 	phys_addr_t mailbox_addr;
 	u8 version;
 	u8 gic_cpu_id;
@@ -59,12 +66,6 @@ static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
 	return 0;
 }
 
-struct parking_protocol_mailbox {
-	__le32 cpu_id;
-	__le32 reserved;
-	__le64 entry_point;
-};
-
 static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 {
 	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
@@ -97,6 +98,12 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 	}
 
 	/*
+	 * stash the mailbox address mapping to use it for further checks
+	 * in postboot method
+	 */
+	cpu_entry->mailbox = mailbox;
+
+	/*
 	 * We write the entry point and cpu id as LE regardless of the
 	 * native endianness of the kernel. Therefore, any boot-loaders
 	 * that read this address need to convert this address to the
@@ -107,8 +114,6 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
 
 	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 
-	iounmap(mailbox);
-
 	return 0;
 }
 
@@ -116,32 +121,15 @@ static void acpi_parking_protocol_cpu_postboot(void)
 {
 	int cpu = smp_processor_id();
 	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
-	struct parking_protocol_mailbox __iomem *mailbox;
+	struct parking_protocol_mailbox __iomem *mailbox = cpu_entry->mailbox;
 	__le64 entry_point;
 
-	/*
-	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
-	 * this deviates from the parking protocol specifications since
-	 * the mailboxes are required to be mapped nGnRnE; the attribute
-	 * discrepancy is harmless insofar as the protocol specification
-	 * is concerned).
-	 * If the mailbox is mistakenly allocated in the linear mapping
-	 * by FW ioremap will fail since the mapping will be prevented
-	 * by the kernel (it clashes with the linear mapping attributes
-	 * specifications).
-	 */
-	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
-	if (!mailbox)
-		return;
-
 	entry_point = readl_relaxed(&mailbox->entry_point);
 	/*
 	 * Check if firmware has cleared the entry_point as expected
 	 * by the protocol specification.
 	 */
 	WARN_ON(entry_point);
-
-	iounmap(mailbox);
 }
 
 const struct cpu_operations acpi_parking_protocol_ops = {

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-02-25  9:24             ` Lorenzo Pieralisi
@ 2016-02-25 20:58               ` Loc Ho
  -1 siblings, 0 replies; 32+ messages in thread
From: Loc Ho @ 2016-02-25 20:58 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Itaru Kitayama, Catalin Marinas, Mark Rutland, Sudeep Holla,
	Will Deacon, linux-acpi, Hanjun Guo, Mark Salter, Al Stone,
	linux-arm-kernel

Hi,

> -- >8 --
> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c

I applies this to 4.5.0-rc5-next-20160225+ and boot just fine on
Mustang and Merlin with 8 CPU's.

Tested-by: Loc Ho <lho@apm.com>

-Loc

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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-02-25 20:58               ` Loc Ho
  0 siblings, 0 replies; 32+ messages in thread
From: Loc Ho @ 2016-02-25 20:58 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

> -- >8 --
> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c

I applies this to 4.5.0-rc5-next-20160225+ and boot just fine on
Mustang and Merlin with 8 CPU's.

Tested-by: Loc Ho <lho@apm.com>

-Loc

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

* Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol
  2016-02-25  9:24             ` Lorenzo Pieralisi
@ 2016-02-26  0:23               ` Itaru Kitayama
  -1 siblings, 0 replies; 32+ messages in thread
From: Itaru Kitayama @ 2016-02-26  0:23 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Catalin Marinas, Mark Rutland, Sudeep Holla, Will Deacon,
	linux-acpi, Hanjun Guo, Mark Salter, Al Stone, linux-arm-kernel,
	Loc Ho

Hi Lorenzo,

I have applied your latest patch on top of today's HEAD of for-next/core 
(cac4b8cd),
and confirmed the kernel boots fine with acpi=force. No lockdep warning 
was seen,
all 8 CPUs were successfully brought up.

Itaru

On 2/25/16 6:24 PM, Lorenzo Pieralisi wrote:
> On Thu, Feb 25, 2016 at 08:28:30AM +0900, Itaru Kitayama wrote:
>> Lorenzo,
>>
>> On Mustang the lockdep warning I reported to you is disappeared and
>> the system gets to
>> the prompt as you implemented, however only 7 CPUs out of 8 brought up.
> It is because, as I mentioned in the other reply to this thread,
> the VA should be stashed before setting up the mailboxes, patch below,
> that should be final (I tested it on AMD Seattle too), please give it a go.
>
> Thanks,
> Lorenzo
>
> -- >8 --
> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
> index 4b1e5a7..dd671ef 100644
> --- a/arch/arm64/kernel/acpi_parking_protocol.c
> +++ b/arch/arm64/kernel/acpi_parking_protocol.c
> @@ -21,7 +21,14 @@
>   
>   #include <asm/cpu_ops.h>
>   
> +struct parking_protocol_mailbox {
> +	__le32 cpu_id;
> +	__le32 reserved;
> +	__le64 entry_point;
> +};
> +
>   struct cpu_mailbox_entry {
> +	struct parking_protocol_mailbox __iomem *mailbox;
>   	phys_addr_t mailbox_addr;
>   	u8 version;
>   	u8 gic_cpu_id;
> @@ -59,12 +66,6 @@ static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
>   	return 0;
>   }
>   
> -struct parking_protocol_mailbox {
> -	__le32 cpu_id;
> -	__le32 reserved;
> -	__le64 entry_point;
> -};
> -
>   static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>   {
>   	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> @@ -97,6 +98,12 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>   	}
>   
>   	/*
> +	 * stash the mailbox address mapping to use it for further checks
> +	 * in postboot method
> +	 */
> +	cpu_entry->mailbox = mailbox;
> +
> +	/*
>   	 * We write the entry point and cpu id as LE regardless of the
>   	 * native endianness of the kernel. Therefore, any boot-loaders
>   	 * that read this address need to convert this address to the
> @@ -107,8 +114,6 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>   
>   	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>   
> -	iounmap(mailbox);
> -
>   	return 0;
>   }
>   
> @@ -116,32 +121,15 @@ static void acpi_parking_protocol_cpu_postboot(void)
>   {
>   	int cpu = smp_processor_id();
>   	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> -	struct parking_protocol_mailbox __iomem *mailbox;
> +	struct parking_protocol_mailbox __iomem *mailbox = cpu_entry->mailbox;
>   	__le64 entry_point;
>   
> -	/*
> -	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
> -	 * this deviates from the parking protocol specifications since
> -	 * the mailboxes are required to be mapped nGnRnE; the attribute
> -	 * discrepancy is harmless insofar as the protocol specification
> -	 * is concerned).
> -	 * If the mailbox is mistakenly allocated in the linear mapping
> -	 * by FW ioremap will fail since the mapping will be prevented
> -	 * by the kernel (it clashes with the linear mapping attributes
> -	 * specifications).
> -	 */
> -	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> -	if (!mailbox)
> -		return;
> -
>   	entry_point = readl_relaxed(&mailbox->entry_point);
>   	/*
>   	 * Check if firmware has cleared the entry_point as expected
>   	 * by the protocol specification.
>   	 */
>   	WARN_ON(entry_point);
> -
> -	iounmap(mailbox);
>   }
>   
>   const struct cpu_operations acpi_parking_protocol_ops = {


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

* [PATCH v3] ARM64: kernel: implement ACPI parking protocol
@ 2016-02-26  0:23               ` Itaru Kitayama
  0 siblings, 0 replies; 32+ messages in thread
From: Itaru Kitayama @ 2016-02-26  0:23 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Lorenzo,

I have applied your latest patch on top of today's HEAD of for-next/core 
(cac4b8cd),
and confirmed the kernel boots fine with acpi=force. No lockdep warning 
was seen,
all 8 CPUs were successfully brought up.

Itaru

On 2/25/16 6:24 PM, Lorenzo Pieralisi wrote:
> On Thu, Feb 25, 2016 at 08:28:30AM +0900, Itaru Kitayama wrote:
>> Lorenzo,
>>
>> On Mustang the lockdep warning I reported to you is disappeared and
>> the system gets to
>> the prompt as you implemented, however only 7 CPUs out of 8 brought up.
> It is because, as I mentioned in the other reply to this thread,
> the VA should be stashed before setting up the mailboxes, patch below,
> that should be final (I tested it on AMD Seattle too), please give it a go.
>
> Thanks,
> Lorenzo
>
> -- >8 --
> diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
> index 4b1e5a7..dd671ef 100644
> --- a/arch/arm64/kernel/acpi_parking_protocol.c
> +++ b/arch/arm64/kernel/acpi_parking_protocol.c
> @@ -21,7 +21,14 @@
>   
>   #include <asm/cpu_ops.h>
>   
> +struct parking_protocol_mailbox {
> +	__le32 cpu_id;
> +	__le32 reserved;
> +	__le64 entry_point;
> +};
> +
>   struct cpu_mailbox_entry {
> +	struct parking_protocol_mailbox __iomem *mailbox;
>   	phys_addr_t mailbox_addr;
>   	u8 version;
>   	u8 gic_cpu_id;
> @@ -59,12 +66,6 @@ static int acpi_parking_protocol_cpu_prepare(unsigned int cpu)
>   	return 0;
>   }
>   
> -struct parking_protocol_mailbox {
> -	__le32 cpu_id;
> -	__le32 reserved;
> -	__le64 entry_point;
> -};
> -
>   static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>   {
>   	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> @@ -97,6 +98,12 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>   	}
>   
>   	/*
> +	 * stash the mailbox address mapping to use it for further checks
> +	 * in postboot method
> +	 */
> +	cpu_entry->mailbox = mailbox;
> +
> +	/*
>   	 * We write the entry point and cpu id as LE regardless of the
>   	 * native endianness of the kernel. Therefore, any boot-loaders
>   	 * that read this address need to convert this address to the
> @@ -107,8 +114,6 @@ static int acpi_parking_protocol_cpu_boot(unsigned int cpu)
>   
>   	arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>   
> -	iounmap(mailbox);
> -
>   	return 0;
>   }
>   
> @@ -116,32 +121,15 @@ static void acpi_parking_protocol_cpu_postboot(void)
>   {
>   	int cpu = smp_processor_id();
>   	struct cpu_mailbox_entry *cpu_entry = &cpu_mailbox_entries[cpu];
> -	struct parking_protocol_mailbox __iomem *mailbox;
> +	struct parking_protocol_mailbox __iomem *mailbox = cpu_entry->mailbox;
>   	__le64 entry_point;
>   
> -	/*
> -	 * Map mailbox memory with attribute device nGnRE (ie ioremap -
> -	 * this deviates from the parking protocol specifications since
> -	 * the mailboxes are required to be mapped nGnRnE; the attribute
> -	 * discrepancy is harmless insofar as the protocol specification
> -	 * is concerned).
> -	 * If the mailbox is mistakenly allocated in the linear mapping
> -	 * by FW ioremap will fail since the mapping will be prevented
> -	 * by the kernel (it clashes with the linear mapping attributes
> -	 * specifications).
> -	 */
> -	mailbox = ioremap(cpu_entry->mailbox_addr, sizeof(*mailbox));
> -	if (!mailbox)
> -		return;
> -
>   	entry_point = readl_relaxed(&mailbox->entry_point);
>   	/*
>   	 * Check if firmware has cleared the entry_point as expected
>   	 * by the protocol specification.
>   	 */
>   	WARN_ON(entry_point);
> -
> -	iounmap(mailbox);
>   }
>   
>   const struct cpu_operations acpi_parking_protocol_ops = {

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

end of thread, other threads:[~2016-02-26  0:24 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-26 11:10 [PATCH v3] ARM64: kernel: implement ACPI parking protocol Lorenzo Pieralisi
2016-01-26 11:10 ` Lorenzo Pieralisi
2016-01-26 17:47 ` Lorenzo Pieralisi
2016-01-26 17:47   ` Lorenzo Pieralisi
2016-01-27 10:23   ` Ard Biesheuvel
2016-01-27 10:23     ` Ard Biesheuvel
2016-01-27 11:46     ` Lorenzo Pieralisi
2016-01-27 11:46       ` Lorenzo Pieralisi
2016-01-26 23:13 ` Loc Ho
2016-01-26 23:13   ` Loc Ho
2016-01-27 11:51   ` Lorenzo Pieralisi
2016-01-27 11:51     ` Lorenzo Pieralisi
2016-01-27 17:41     ` Loc Ho
2016-01-27 17:41       ` Loc Ho
2016-02-02 18:26 ` Catalin Marinas
2016-02-02 18:26   ` Catalin Marinas
2016-02-03 11:21   ` Lorenzo Pieralisi
2016-02-03 11:21     ` Lorenzo Pieralisi
2016-02-03 16:18     ` Catalin Marinas
2016-02-03 16:18       ` Catalin Marinas
2016-02-24 14:18       ` Lorenzo Pieralisi
2016-02-24 14:18         ` Lorenzo Pieralisi
2016-02-24 15:03         ` Lorenzo Pieralisi
2016-02-24 15:03           ` Lorenzo Pieralisi
2016-02-24 23:28         ` Itaru Kitayama
2016-02-24 23:28           ` Itaru Kitayama
2016-02-25  9:24           ` Lorenzo Pieralisi
2016-02-25  9:24             ` Lorenzo Pieralisi
2016-02-25 20:58             ` Loc Ho
2016-02-25 20:58               ` Loc Ho
2016-02-26  0:23             ` Itaru Kitayama
2016-02-26  0:23               ` Itaru Kitayama

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.