From patchwork Tue Mar 26 15:49:12 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 2339141 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id 06B85DF264 for ; Tue, 26 Mar 2013 15:52:05 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1UKW7q-0000bQ-Kw; Tue, 26 Mar 2013 15:49:27 +0000 Received: from smtp02.citrix.com ([66.165.176.63]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UKW7h-0000XS-Fa for linux-arm-kernel@lists.infradead.org; Tue, 26 Mar 2013 15:49:18 +0000 X-IronPort-AV: E=Sophos;i="4.84,913,1355097600"; d="scan'208";a="14920037" Received: from accessns.citrite.net (HELO FTLPEX01CL02.citrite.net) ([10.9.154.239]) by FTLPIPO02.CITRIX.COM with ESMTP/TLS/AES128-SHA; 26 Mar 2013 15:49:14 +0000 Received: from ukmail1.uk.xensource.com (10.80.16.128) by smtprelay.citrix.com (10.13.107.79) with Microsoft SMTP Server id 14.2.342.3; Tue, 26 Mar 2013 11:49:14 -0400 Received: from kaball.uk.xensource.com ([10.80.2.59]) by ukmail1.uk.xensource.com with esmtp (Exim 4.69) (envelope-from ) id 1UKW7e-0003oq-7E; Tue, 26 Mar 2013 15:49:14 +0000 Date: Tue, 26 Mar 2013 15:49:12 +0000 From: Stefano Stabellini X-X-Sender: sstabellini@kaball.uk.xensource.com To: Will Deacon Subject: Re: [PATCH v2 6/6] [RFC] arm: use PSCI if available In-Reply-To: <20130326153730.GA22368@mudshark.cambridge.arm.com> Message-ID: References: <1364308875-26484-6-git-send-email-stefano.stabellini@eu.citrix.com> <20130326150444.GI20252@mudshark.cambridge.arm.com> <20130326153730.GA22368@mudshark.cambridge.arm.com> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130326_114917_695022_C8E79E4E X-CRM114-Status: GOOD ( 36.20 ) X-Spam-Score: -3.9 (---) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-3.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [66.165.176.63 listed in list.dnswl.org] -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 SPF_PASS SPF: sender matches SPF record -1.3 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: "xen-devel@lists.xensource.com" , "linux@arm.linux.org.uk" , Ian Campbell , "arnd@arndb.de" , "konrad.wilk@oracle.com" , Marc Zyngier , Stefano Stabellini , "linux-kernel@vger.kernel.org" , "nico@linaro.org" , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Tue, 26 Mar 2013, Will Deacon wrote: > On Tue, Mar 26, 2013 at 03:25:55PM +0000, Stefano Stabellini wrote: > > On Tue, 26 Mar 2013, Will Deacon wrote: > > > On Tue, Mar 26, 2013 at 02:41:15PM +0000, Stefano Stabellini wrote: > > > > +struct smp_operations __initdata psci_smp_ops = { > > > > + .smp_init_cpus = psci_smp_init_cpus, > > > > + .smp_prepare_cpus = psci_smp_prepare_cpus, > > > > + .smp_secondary_init = psci_secondary_init, > > > > + .smp_boot_secondary = psci_boot_secondary, > > > > +}; > > > > > > Whilst I like the idea of this, I don't think things will pan out this > > > nicely in practice. There will almost always be a level of indirection > > > required between the internal Linux SMP operations and the expectations of > > > the PSCI firmware, whether this is in CPU numbering or other, > > > platform-specific fields in various parameters. > > > > > > Tying these two things together like this confuses the layering in my > > > opinion and will likely lead to potentially subtle breakages if platforms > > > start trying to adopt this. > > > > What you are saying is that psci could either be used directly, like we > > are doing, or it could just be the base of some higher level platform > > specific smp_ops. > > > > Honestly I think that psci is already high level enough that I would > > worry if somebody started to wrap it around something else. > > I don't agree. PSCI is a low-level firmware interface, which will naturally > have implementation-specific parts to it. For example, many of the CPU power > functions have platform-specific state ID parameters which we can't just > ignore. Furthermore, the method by which a CPU is identified needn't match > the value in our logical map. The purpose of the PSCI code in Linux is to > provide a basic abstraction on top of this interface, so that platforms can > incorporate them into higher-level power management functions, which > themselves might be plumbed into smp_operations structures. > > > However we still support that use case just fine: they can just avoid > > having a psci node on device tree and just keep using their machine > > specific smp_ops. It's up to them really. > > Why get rid of the node? That's there to initialise the PSCI backend > accordingly and shouldn't have anything to do with SMP. > > > They can even base the implementation of their smp_ops on the current > > psci code, in order to facilitate that I could get rid of psci_ops > > (which initialization is based on device tree) and export the psci_cpu_* > > functions instead, so that they can be called directly by other smp_ops. > > Again, I think this destroys the layering. The whole point is that the PSCI > functions are called from within something that understands precisely how to > talk to the firmware and what it is capable of. All right, I am not going to pretend to know better the final purpose of PSCI :-) Assuming that you are correct, I can't see any other options than have a Xen specific override setup.c, but it's really ugly: > > > If this can indeed work for the virtual platforms (Xen and KVM), then I > > > think it would be better expressed using `virt' smp_ops, which map directly > > > to PSCI, rather than putting them here. Even then, it's tying KVM and Xen > > > together on the firmware side of things... > > > > Keep in mind that dom0 on Xen boots as a native machine (versatile > > express or exynos5 for example) with a Xen hypervisor node on it. We > > would need to find a way to override the default machine smp_ops with > > a set of xen_smp_ops early at boot. > > I don't like this option very much, I think is fragile. > > Why can't dom0 use whatever smp ops the native machine would use? Because Xen doesn't export them (and it's not going to). diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index 3f6cbb2..7876865 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -768,7 +768,12 @@ void __init setup_arch(char **cmdline_p) arm_dt_init_cpu_maps(); #ifdef CONFIG_SMP if (is_smp()) { - smp_set_ops(mdesc->smp); + int rc = -ENODEV; +#ifdef CONFIG_XEN + rc = xen_init_smp(); +#endif + if (rc) + smp_set_ops(mdesc->smp); smp_init_cpus(); } #endif