From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Rutland Date: Fri, 22 Aug 2014 12:38:05 +0100 Subject: [U-Boot] [Patch v2 3/5] armv8/fsl-lsch3: Release secondary cores from boot hold off with Boot Page In-Reply-To: References: <1408480082-4617-1-git-send-email-yorksun@freescale.com> <1408480082-4617-3-git-send-email-yorksun@freescale.com> <20140821134724.GM21734@leverpostej> <53F63B55.3040900@freescale.com> Message-ID: <20140822113805.GU21734@leverpostej> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Bhupesh, [...] > > >> diff --git a/arch/arm/cpu/armv8/fsl-lsch3/fdt.c > > >> b/arch/arm/cpu/armv8/fsl-lsch3/fdt.c > > >> new file mode 100644 > > >> index 0000000..2dbcdcb > > >> --- /dev/null > > >> +++ b/arch/arm/cpu/armv8/fsl-lsch3/fdt.c > > >> @@ -0,0 +1,56 @@ > > >> +/* > > >> + * Copyright 2014 Freescale Semiconductor, Inc. > > >> + * > > >> + * SPDX-License-Identifier: GPL-2.0+ > > >> + */ > > >> + > > >> +#include > > >> +#include > > >> +#include > > >> +#include "mp.h" > > >> + > > >> +#ifdef CONFIG_MP > > >> +void ft_fixup_cpu(void *blob) > > >> +{ > > >> + int off; > > >> + __maybe_unused u64 spin_tbl_addr = (u64)get_spin_tbl_addr(); > > >> + fdt32_t *reg; > > >> + int addr_cells; > > >> + u64 val; > > >> + size_t *boot_code_size = &(__secondary_boot_code_size); > > >> + > > >> + off = fdt_node_offset_by_prop_value(blob, -1, "device_type", > > >> + "cpus", 4); > > > > > > I didn't think /cpus had device_type = "cpus". I can't see any > > > instances in any DTs I have to hand. Can we not find /cpus by path? > > > > I will let Arnab to comment on this. He is coordinating with Linux device > > tree. > > Since I contribute to the DTS for FSL ARMv8 SoC, here is my rationale behind the same. > I have used the standard ARM cpu device-tree binding documentation as a reference (see [1]) > which defined the device_type which it mentions should be set to cpu. > > Please let me know if I am missing something. > > [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/cpus.txt Hi. As Arnab replied, it's only the CPU nodes themselves (e.g. /cpus/cpu at 0) that have device_type = "cpu". The /cpus node does not have a device_type at all. The /cpus node can always be found by path however, as the name is special. [...] > > >> --- a/arch/arm/cpu/armv8/fsl-lsch3/lowlevel.S > > >> +++ b/arch/arm/cpu/armv8/fsl-lsch3/lowlevel.S > > >> @@ -8,7 +8,9 @@ > > >> > > >> #include > > >> #include > > >> +#include > > >> #include > > >> +#include "mp.h" > > >> > > >> ENTRY(lowlevel_init) > > >> mov x29, lr /* Save LR */ > > >> @@ -37,29 +39,119 @@ ENTRY(lowlevel_init) > > >> > > >> branch_if_master x0, x1, 1f > > >> > > >> + ldr x0, =secondary_boot_func > > >> + blr x0 > > >> + > > >> +1: > > >> +2: > > > > > > Isn't the '2' label redundant? > > > > We have some internal code dealing with trust zone between the 1 and 2. It > > is not likely to be used in long term since we are trying to move them > > into security monitor. I can drop label 2 here. > > U-boot can still be booted in EL3, as it can be well booted w/o a ATF or EL3 capable > s/w running before the same. That's why we have CONFIG_EL3 and CONFIG_EL2 code legs > in the u-boot ARMv8 code. Ok. I was only confused by the fact the label didn't seem to be used anywhere, and it sounds like it can be dropped for now? [...] > > >> - /* > > >> - * All processors will enter EL2 and optionally EL1. > > >> +slave_cpu: > > >> + wfe > > >> +#ifdef CONFIG_FSL_SMP_RELEASE_ALL > > >> + /* All cores are released from the address in the 1st spin > > table > > >> + * element > > >> */ > > >> - bl armv8_switch_to_el2 > > >> -#ifdef CONFIG_ARMV8_SWITCH_TO_EL1 > > >> - bl armv8_switch_to_el1 > > >> + ldr x1, =__spin_table > > >> + ldr x0, [x1] > > >> +#else > > >> + ldr x0, [x11] > > >> +#endif > > >> + cbz x0, slave_cpu > > > > > > Similarly is there any reason to have the option of a single release > > > addr if we can support unique addresses? > > > > I think it was used by Linux for some ARM parts. I personally not a fun of > > using single release. But if it makes everyone happy, I can keep it. > > We followed the standard ARMv8 foundation model DTS initially which along with others > supported a single release address for all the cores. So, we wanted to comply to the same. As I mentioned elsewhere, the existing DTS aren't good examples. The fact that isn't clear is a problem on the Linux side, and I'm sorry that they have misled you. Using unique addresses is preferred. Sharing a single address should be discouraged. [...] > > >> diff --git a/arch/arm/include/asm/macro.h > > >> b/arch/arm/include/asm/macro.h index f77e4b8..0009c28 100644 > > >> --- a/arch/arm/include/asm/macro.h > > >> +++ b/arch/arm/include/asm/macro.h > > >> @@ -105,6 +105,98 @@ lr .req x30 > > >> cbz \xreg1, \master_label > > >> .endm > > >> > > >> +.macro armv8_switch_to_el2_m, xreg1 > > >> + mov \xreg1, #0x5b1 /* Non-secure EL0/EL1 | HVC | 64bit EL2 > > */ > > >> + msr scr_el3, \xreg1 > > > > > > When dropping to EL1 from EL2 we disable HVC via HCR_EL2; presumably > > > due to lack of a handler. Would it make sense to do similarly here and > > > disable SMC here until we have a user (e.g. PSCI)? > > > > I will let Arnab to comment here. > > In my view u-boot should be similar to UEFI and both should drop only to EL2 > before booting either Linux or Hypervisor. UEFI code doesn't switch to EL1 > before invoking Linux. Thoughts? That sounds ideal to me. In general I'd hope any bootloader would hand-off to the OS (Linux, Xen, whatever) at the highest-privileged non-secure mode. So EL2 if present, otherwise EL1N. We don't have any legacy to worry about with old kernels that cannot boot at EL1N, so I don't see a good reason to drop to EL1N if EL2 is available. If the option of booting at EL1 becomes necessary for some piece of software, I'd rather that were a run-time option and that piece of software were treated as the special case. Thanks, Mark.