[PATCH 1/2] ARM: vexpress: refine MCPM smp operations override criteria

[Lorenzo Pieralisi]

On Mon, Oct 17, 2016 at 06:31:58PM +0100, Sudeep Holla wrote:
> 
> 
> On 23/09/16 15:03, Lorenzo Pieralisi wrote:
> >On Fri, Sep 23, 2016 at 02:09:06PM +0100, Lorenzo Pieralisi wrote:
> >>Current vexpress smp init code detects whether to override the
> >>default smp ops with MCPM smp ops by matching the "cci-400"
> >>compatible string, in that MCPM requires control over CCI ports
> >>to manage low-power states entry/exit.
> >>
> >>The "cci-400" compatible string check is a necessary but not
> >>sufficient condition for MCPM to work, because the cci-400
> >>can be made visible to the kernel, but firmware can nonetheless
> >>disable non-secure CCI ports control, while still allowing PMU
> >>access; if booted in non-secure world, the kernel would still
> >>blindly override smp operations with MCPM operations, resulting
> >>in kernel faults when the CCI ports programming interface is
> >>accessed from non-secure world.
> >>
> >>This means that the "cci-400" compatible string check would
> >>result in a false positive in systems that eg boot in HYP mode,
> >>where CCI ports non-secure access is explicitly not allowed,
> >>and it is reported in the respective device tree nodes with
> >>CCI ports marked as disabled.
> >>
> >>Refactor the smp operations initialization to make sure that
> >>the kernel is actually allowed to take control over CCI ports
> >>(by enabling MCPM smp operations) before overriding default
> >>vexpress smp operations.
> >>
> >>Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi at arm.com>
> >>Cc: Liviu Dudau <liviu.dudau at arm.com>
> >>Cc: Sudeep Holla <sudeep.holla at arm.com>
> >>Cc: Nicolas Pitre <nicolas.pitre at linaro.org>
> >>Cc: Marc Zyngier <marc.zyngier at arm.com>
> >>---
> >> arch/arm/mach-vexpress/platsmp.c | 31 ++++++++++++++++++++++++-------
> >> 1 file changed, 24 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/arch/arm/mach-vexpress/platsmp.c b/arch/arm/mach-vexpress/platsmp.c
> >>index 8b8d072..6cfd782 100644
> >>--- a/arch/arm/mach-vexpress/platsmp.c
> >>+++ b/arch/arm/mach-vexpress/platsmp.c
> >>@@ -26,17 +26,34 @@
> >> bool __init vexpress_smp_init_ops(void)
> >> {
> >> #ifdef CONFIG_MCPM
> >>+	int cpu;
> >>+	struct device_node *cpu_node, *cci_node;
> >>+
> >> 	/*
> >>-	 * The best way to detect a multi-cluster configuration at the moment
> >>-	 * is to look for the presence of a CCI in the system.
> >>+	 * The best way to detect a multi-cluster configuration
> >>+	 * is to detect if the kernel can take over CCI ports
> >>+	 * control. Loop over possible CPUs and check if CCI
> >>+	 * port control is available.
> >> 	 * Override the default vexpress_smp_ops if so.
> >> 	 */
> >>-	struct device_node *node;
> >>-	node = of_find_compatible_node(NULL, NULL, "arm,cci-400");
> >>-	if (node && of_device_is_available(node)) {
> >>-		mcpm_smp_set_ops();
> >>-		return true;
> >>+	for_each_possible_cpu(cpu) {
> >>+		bool available;
> >>+
> >>+		cpu_node = of_get_cpu_node(cpu, NULL);
> >>+		if (WARN(!cpu_node, "Missing cpu device node!"))
> >>+			return false;
> >>+
> >>+		cci_node = of_parse_phandle(cpu_node, "cci-control-port", 0);
> >>+		available = cci_node && of_device_is_available(cci_node);
> >>+		of_node_put(cci_node);
> >>+		of_node_put(cpu_node);
> >>+
> >>+		if (!available)
> >>+			return false;
> >> 	}
> >>+
> >>+	mcpm_smp_set_ops();
> >>+	return true;
> >> #endif
> >> 	return false;
> >
> >For the records, while moving the code around I missed I was ending
> >up with this idiotic double return, I have already reworked the patch
> >and will squash changes in the final version if we agree on the bulk of
> >the code.
> >
> 
> I applied both patches to [1] with the fix for the above issue. Let me
> know if that's fine. I have tested both hyp mode boot and SVC mode +
> MCPM boot with latest u-boot by just fliping a bit in the firmware
> (board.txt) without recompiling the kernel.

I updated the patch to remove the double return will send an updated
patch today.

Thanks !
Lorenzo

> 
> -- 
> Regards,
> Sudeep
> 
> [1] git.kernel.org/sudeep.holla/linux/h/vexpress/for-next
>