From patchwork Wed Feb 24 14:18:32 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lorenzo Pieralisi X-Patchwork-Id: 8408261 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 8CF089F1D4 for ; Wed, 24 Feb 2016 14:22:38 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6513820361 for ; Wed, 24 Feb 2016 14:22:37 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 387D1202AE for ; Wed, 24 Feb 2016 14:22:36 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aYaIf-00070I-71; Wed, 24 Feb 2016 14:20:21 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aYaFC-0004gF-Cp for linux-arm-kernel@lists.infradead.org; Wed, 24 Feb 2016 14:16:47 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2731B49; Wed, 24 Feb 2016 06:15:32 -0800 (PST) Received: from red-moon (red-moon.cambridge.arm.com [10.1.203.137]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 267503F25E; Wed, 24 Feb 2016 06:16:23 -0800 (PST) Date: Wed, 24 Feb 2016 14:18:32 +0000 From: Lorenzo Pieralisi To: Catalin Marinas Subject: Re: [PATCH v3] ARM64: kernel: implement ACPI parking protocol Message-ID: <20160224141832.GA26630@red-moon> References: <1453806638-23167-1-git-send-email-lorenzo.pieralisi@arm.com> <20160202182657.GC15706@e104818-lin.cambridge.arm.com> <20160203112112.GA18387@red-moon> <20160203161834.GB26487@MBP.local> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160203161834.GB26487@MBP.local> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160224_061646_537456_0C827B65 X-CRM114-Status: GOOD ( 31.58 ) X-Spam-Score: -6.9 (------) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Itaru Kitayama , Sudeep Holla , Will Deacon , linux-acpi@vger.kernel.org, Hanjun Guo , Mark Salter , Al Stone , linux-arm-kernel@lists.infradead.org, Loc Ho Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP [+ 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 > > > > > > > > [1] https://acpica.org/sites/acpica/files/MP%20Startup%20for%20ARM%20platforms.docx > > > > > > > > Signed-off-by: Lorenzo Pieralisi > > > > Cc: Will Deacon > > > > Cc: Hanjun Guo > > > > Cc: Loc Ho > > > > Cc: Sudeep Holla > > > > Cc: Catalin Marinas > > > > Cc: Mark Rutland > > > > Cc: Mark Salter > > > > Cc: Al Stone > > > > > > 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 +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 = {