diff mbox

arm64: kernel: acpi: fix ioremap in ACPI parking protocol cpu_postboot

Message ID 1456487412-4833-1-git-send-email-lorenzo.pieralisi@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lorenzo Pieralisi Feb. 26, 2016, 11:50 a.m. UTC 
When secondary cpus are booted through the ACPI parking protocol, the
booted cpu should check that FW has correctly cleared its mailbox entry
point value to make sure the boot process was correctly executed.
The entry point check is carried in the cpu_ops->cpu_postboot method, that
is executed by secondary cpus when entering the kernel with irqs disabled.

The ACPI parking protocol cpu_ops maps/unmaps the mailboxes on the
primary CPU to trigger secondary boot in the cpu_ops->cpu_boot method
and on secondary processors to carry out FW checks on the booted CPU
to verify the boot protocol was successfully executed in the
cpu_ops->cpu_postboot method.

Therefore, the cpu_ops->cpu_postboot method is forced to ioremap/unmap the
mailboxes, which is wrong in that ioremap cannot be safely be carried out
with irqs disabled.

To fix this issue, this patch reshuffles the code so that the mailboxes
are still mapped after the boot processor executes the cpu_ops->cpu_boot
method for a given cpu, and the VA at which a mailbox is mapped for a given
cpu is stashed in the per-cpu data struct so that secondary cpus can
retrieve them in the cpu_ops->cpu_postboot and complete the required
FW checks.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reported-by: Itaru Kitayama <itaru.kitayama@riken.jp>
Tested-by: Loc Ho <lho@apm.com>
Tested-by: Itaru Kitayama <itaru.kitayama@riken.jp>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Loc Ho <lho@apm.com>
Cc: Itaru Kitayama <itaru.kitayama@riken.jp>
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>
---
It applies to the arm64 tree, for-next/core branch.

Tested on AMD (Seattle), APM (Mustang and Merlin), sorry for all this churn.

Thanks,
Lorenzo

 arch/arm64/kernel/acpi_parking_protocol.c | 40 +++++++++++--------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

Comments

Catalin Marinas Feb. 26, 2016, 3:43 p.m. UTC | #1 
On Fri, Feb 26, 2016 at 11:50:12AM +0000, Lorenzo Pieralisi wrote:
> When secondary cpus are booted through the ACPI parking protocol, the
> booted cpu should check that FW has correctly cleared its mailbox entry
> point value to make sure the boot process was correctly executed.
> The entry point check is carried in the cpu_ops->cpu_postboot method, that
> is executed by secondary cpus when entering the kernel with irqs disabled.
> 
> The ACPI parking protocol cpu_ops maps/unmaps the mailboxes on the
> primary CPU to trigger secondary boot in the cpu_ops->cpu_boot method
> and on secondary processors to carry out FW checks on the booted CPU
> to verify the boot protocol was successfully executed in the
> cpu_ops->cpu_postboot method.
> 
> Therefore, the cpu_ops->cpu_postboot method is forced to ioremap/unmap the
> mailboxes, which is wrong in that ioremap cannot be safely be carried out
> with irqs disabled.
> 
> To fix this issue, this patch reshuffles the code so that the mailboxes
> are still mapped after the boot processor executes the cpu_ops->cpu_boot
> method for a given cpu, and the VA at which a mailbox is mapped for a given
> cpu is stashed in the per-cpu data struct so that secondary cpus can
> retrieve them in the cpu_ops->cpu_postboot and complete the required
> FW checks.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Reported-by: Itaru Kitayama <itaru.kitayama@riken.jp>
> Tested-by: Loc Ho <lho@apm.com>
> Tested-by: Itaru Kitayama <itaru.kitayama@riken.jp>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Loc Ho <lho@apm.com>
> Cc: Itaru Kitayama <itaru.kitayama@riken.jp>
> 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. Thanks.
diff mbox

Patch

diff --git a/arch/arm64/kernel/acpi_parking_protocol.c b/arch/arm64/kernel/acpi_parking_protocol.c
index 4b1e5a7..a32b401 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 FW
+	 * checks in the 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 = {