[U-Boot] [PATCHV5 1/6] armv8: fsl-layerscape: add i/d-cache enable function to enable_caches

[U-Boot] [PATCHV5 1/6] armv8: fsl-layerscape: add i/d-cache enable function to enable_caches

[Zhiqiang Hou]

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

This function assume that the d-cache and MMU has been enabled earlier,
so it just created MMU table in main memory. But the assumption is not
always correct, for example, the early setup is done in EL3, while
enable_caches() is called when the PE has turned into another EL.

Define the function mmu_setup() for fsl-layerscape to cover the weak
one.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
V5:
 - no change

 arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
index d939900..672a453 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
@@ -425,15 +425,21 @@ int arch_cpu_init(void)
 	return 0;
 }
 
+void mmu_setup(void)
+{
+	final_mmu_setup();
+}
+
 /*
- * This function is called from lib/board.c.
- * It recreates MMU table in main memory. MMU and d-cache are enabled earlier.
- * There is no need to disable d-cache for this operation.
+ * This function is called from common/board_r.c.
+ * It recreates MMU table in main memory.
  */
 void enable_caches(void)
 {
-	final_mmu_setup();
+	mmu_setup();
 	__asm_invalidate_tlb_all();
+	icache_enable();
+	dcache_enable();
 }
 #endif
 
-- 
2.1.0.27.g96db324

[U-Boot] [PATCHV5 2/6] ARMv8: add the secure monitor firmware framework

[Zhiqiang Hou]

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

The sec_firmware.h is the common header file for secure monitor
firmware under ARMv8. The declaration of common APIs can be
added to this file. And the implementation of common APIs will
be abstracted to sec_firmware.c, up to now there are some weak
implementations.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
V5:
 - Added c file sec_firmware.c.
 - Added declaration of sec_firmware_init().
 - Renamed the func sec_firmware_validate().

V4:
 - Reordered this patch.
 - Removed the FSL PPA related items.

 arch/arm/cpu/armv8/Makefile               |  1 +
 arch/arm/cpu/armv8/sec_firmware.c         | 25 +++++++++++++++++++++++++
 arch/arm/include/asm/armv8/sec_firmware.h | 16 ++++++++++++++++
 3 files changed, 42 insertions(+)
 create mode 100644 arch/arm/cpu/armv8/sec_firmware.c
 create mode 100644 arch/arm/include/asm/armv8/sec_firmware.h

diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
index 1c85aa9..667572b 100644
--- a/arch/arm/cpu/armv8/Makefile
+++ b/arch/arm/cpu/armv8/Makefile
@@ -15,6 +15,7 @@ obj-y	+= cache.o
 obj-y	+= tlb.o
 obj-y	+= transition.o
 obj-y	+= fwcall.o
+obj-$(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) += sec_firmware.o
 
 obj-$(CONFIG_FSL_LAYERSCAPE) += fsl-layerscape/
 obj-$(CONFIG_ARCH_ZYNQMP) += zynqmp/
diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
new file mode 100644
index 0000000..90d89e7
--- /dev/null
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2016 NXP Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/armv8/sec_firmware.h>
+
+__weak int sec_firmware_init(void)
+{
+	return -1;
+}
+
+__weak bool sec_firmware_is_valid(void)
+{
+	return false;
+}
+
+#if defined(CONFIG_ARMV8_PSCI)
+__weak unsigned int sec_firmware_support_psci_version(void)
+{
+	return 0;
+}
+#endif
diff --git a/arch/arm/include/asm/armv8/sec_firmware.h b/arch/arm/include/asm/armv8/sec_firmware.h
new file mode 100644
index 0000000..e2591aa
--- /dev/null
+++ b/arch/arm/include/asm/armv8/sec_firmware.h
@@ -0,0 +1,16 @@
+/*
+ * Copyright 2016 NXP Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __SEC_FIRMWARE_H_
+#define __SEC_FIRMWARE_H_
+
+int sec_firmware_init(void);
+bool sec_firmware_is_valid(void);
+#ifdef CONFIG_ARMV8_PSCI
+unsigned int sec_firmware_support_psci_version(void);
+#endif
+
+#endif /* __SEC_FIRMWARE_H_ */
-- 
2.1.0.27.g96db324

[U-Boot] [PATCHV5 3/6] ARMv8/layerscape: Add FSL PPA support

[Zhiqiang Hou]

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

The FSL Primary Protected Application (PPA) is a software component
loaded during boot which runs in TrustZone and remains resident
after boot.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
V5:
 - Added API sec_firmware_init() implementation.

V4:
 - Moved secure firmware validation API to this patch.
 - Moved secure firmware getting supported PSCI version API to this patch.

V3:
 - Refactor the code.
 - Add PPA firmware version info output.

 arch/arm/cpu/armv8/fsl-layerscape/Makefile     |   1 +
 arch/arm/cpu/armv8/fsl-layerscape/ppa.c        | 311 +++++++++++++++++++++++++
 arch/arm/cpu/armv8/fsl-layerscape/ppa_entry.S  |  53 +++++
 arch/arm/include/asm/arch-fsl-layerscape/ppa.h |  14 ++
 arch/arm/include/asm/armv8/sec_firmware.h      |   4 +
 5 files changed, 383 insertions(+)
 create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ppa.c
 create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ppa_entry.S
 create mode 100644 arch/arm/include/asm/arch-fsl-layerscape/ppa.h

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Makefile b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
index 5f86ef9..1535fee 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
+++ b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
@@ -10,6 +10,7 @@ obj-y += soc.o
 obj-$(CONFIG_MP) += mp.o
 obj-$(CONFIG_OF_LIBFDT) += fdt.o
 obj-$(CONFIG_SPL) += spl.o
+obj-$(CONFIG_FSL_LS_PPA) += ppa.o ppa_entry.o
 
 ifneq ($(CONFIG_FSL_LSCH3),)
 obj-y += fsl_lsch3_speed.o
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
new file mode 100644
index 0000000..6a75960
--- /dev/null
+++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
@@ -0,0 +1,311 @@
+/*
+ * Copyright 2016 NXP Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#include <common.h>
+#include <config.h>
+#include <errno.h>
+#include <malloc.h>
+#include <asm/system.h>
+#include <asm/io.h>
+#include <asm/types.h>
+#include <asm/macro.h>
+#include <asm/arch/soc.h>
+#ifdef CONFIG_FSL_LSCH3
+#include <asm/arch/immap_lsch3.h>
+#elif defined(CONFIG_FSL_LSCH2)
+#include <asm/arch/immap_lsch2.h>
+#endif
+#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
+#include <asm/armv8/sec_firmware.h>
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;
+
+extern void c_runtime_cpu_setup(void);
+
+#define LS_PPA_FIT_FIRMWARE_IMAGE	"firmware"
+#define LS_PPA_FIT_CNF_NAME		"config at 1"
+#define PPA_MEM_SIZE_ENV_VAR		"ppamemsize"
+
+/*
+ * Return the actual size of the PPA private DRAM block.
+ */
+unsigned long ppa_get_dram_block_size(void)
+{
+	unsigned long dram_block_size = CONFIG_SYS_LS_PPA_DRAM_BLOCK_MIN_SIZE;
+
+	char *dram_block_size_env_var = getenv(PPA_MEM_SIZE_ENV_VAR);
+
+	if (dram_block_size_env_var) {
+		dram_block_size = simple_strtoul(dram_block_size_env_var, NULL,
+						 10);
+
+		if (dram_block_size < CONFIG_SYS_LS_PPA_DRAM_BLOCK_MIN_SIZE) {
+			printf("fsl-ppa: WARNING: Invalid value for \'"
+			       PPA_MEM_SIZE_ENV_VAR
+			       "\' environment variable: %lu\n",
+			       dram_block_size);
+
+			dram_block_size = CONFIG_SYS_LS_PPA_DRAM_BLOCK_MIN_SIZE;
+		}
+	}
+
+	return dram_block_size;
+}
+
+static bool ppa_firmware_is_valid(void *ppa_addr)
+{
+	void *fit_hdr;
+
+	fit_hdr = ppa_addr;
+
+	if (fdt_check_header(fit_hdr)) {
+		printf("fsl-ppa: Bad firmware image (not a FIT image)\n");
+		return false;
+	}
+
+	if (!fit_check_format(fit_hdr)) {
+		printf("fsl-ppa: Bad firmware image (bad FIT header)\n");
+		return false;
+	}
+
+	return true;
+}
+
+static int ppa_firmware_get_data(void *ppa_addr,
+				const void **data, size_t *size)
+{
+	void *fit_hdr;
+	int conf_node_off, fw_node_off;
+	char *conf_node_name = NULL;
+	char *desc;
+	int ret;
+
+	fit_hdr = ppa_addr;
+	conf_node_name = LS_PPA_FIT_CNF_NAME;
+
+	conf_node_off = fit_conf_get_node(fit_hdr, conf_node_name);
+	if (conf_node_off < 0) {
+		printf("fsl-ppa: %s: no such config\n", conf_node_name);
+		return -ENOENT;
+	}
+
+	fw_node_off = fit_conf_get_prop_node(fit_hdr, conf_node_off,
+			LS_PPA_FIT_FIRMWARE_IMAGE);
+	if (fw_node_off < 0) {
+		printf("fsl-ppa: No '%s' in config\n",
+				LS_PPA_FIT_FIRMWARE_IMAGE);
+		return -ENOLINK;
+	}
+
+	/* Verify PPA firmware image */
+	if (!(fit_image_verify(fit_hdr, fw_node_off))) {
+		printf("fsl-ppa: Bad firmware image (bad CRC)\n");
+		return -EINVAL;
+	}
+
+	if (fit_image_get_data(fit_hdr, fw_node_off, data, size)) {
+		printf("fsl-ppa: Can't get %s subimage data/size",
+				LS_PPA_FIT_FIRMWARE_IMAGE);
+		return -ENOENT;
+	}
+
+	ret = fit_get_desc(fit_hdr, fw_node_off, &desc);
+	if (ret)
+		printf("PPA Firmware unavailable\n");
+	else
+		printf("%s\n", desc);
+
+	return 0;
+}
+
+/*
+ * PPA firmware FIT image parser checks if the image is in FIT
+ * format, verifies integrity of the image and calculates raw
+ * image address and size values.
+ *
+ * Returns 0 on success and a negative errno on error task fail.
+ */
+static int ppa_parse_firmware_fit_image(const void **raw_image_addr,
+				size_t *raw_image_size)
+{
+	void *ppa_addr;
+	int ret;
+
+#ifdef CONFIG_SYS_LS_PPA_FW_IN_NOR
+	ppa_addr = (void *)CONFIG_SYS_LS_PPA_FW_ADDR;
+#else
+#error "No CONFIG_SYS_LS_PPA_FW_IN_xxx defined"
+#endif
+
+	if (!ppa_firmware_is_valid(ppa_addr))
+		return -1;
+
+	ret = ppa_firmware_get_data(ppa_addr, raw_image_addr, raw_image_size);
+	if (ret)
+		return ret;
+
+	debug("fsl-ppa: raw_image_addr = 0x%p, raw_image_size = 0x%lx\n",
+			*raw_image_addr, *raw_image_size);
+
+	return 0;
+}
+
+static int ppa_copy_image(const char *title,
+			 u64 image_addr, u32 image_size, u64 ppa_ram_addr)
+{
+	debug("%s copied to address 0x%p\n", title, (void *)ppa_ram_addr);
+	memcpy((void *)ppa_ram_addr, (void *)image_addr, image_size);
+	flush_dcache_range(ppa_ram_addr, ppa_ram_addr + image_size);
+
+	return 0;
+}
+
+static int ppa_init_pre(u64 *entry)
+{
+	u64 ppa_ram_addr;
+	const void *raw_image_addr;
+	size_t raw_image_size = 0;
+	size_t ppa_ram_size = ppa_get_dram_block_size();
+	int ret;
+
+	debug("fsl-ppa: ppa size(0x%lx)\n", ppa_ram_size);
+
+	/*
+	 * The Excetpion Level must be EL3 to prepare and initialize the PPA.
+	 */
+	if (current_el() != 3) {
+		ret = -EACCES;
+		goto out;
+	}
+
+#ifdef CONFIG_SYS_MEM_RESERVE_SECURE
+	/*
+	 * The PPA must be stored in secure memory.
+	 * Append PPA to secure mmu table.
+	 */
+	if (!(gd->secure_ram & MEM_RESERVE_SECURE_MAINTAINED)) {
+		ret = -ENXIO;
+		goto out;
+	}
+
+	ppa_ram_addr = (gd->secure_ram & MEM_RESERVE_SECURE_ADDR_MASK) +
+			gd->arch.tlb_size;
+#else
+#error "The CONFIG_SYS_MEM_RESERVE_SECURE must be defined when enabled PPA"
+#endif
+
+	/* Align PPA base address to 4K */
+	ppa_ram_addr = (ppa_ram_addr + 0xfff) & ~0xfff;
+	debug("fsl-ppa: PPA load address (0x%llx)\n", ppa_ram_addr);
+
+	ret = ppa_parse_firmware_fit_image(&raw_image_addr, &raw_image_size);
+	if (ret < 0)
+		goto out;
+
+	if (ppa_ram_size < raw_image_size) {
+		ret = -ENOSPC;
+		goto out;
+	}
+
+	/* TODO:
+	 * Check if the end addr of PPA has been extend the secure memory.
+	 */
+
+	ppa_copy_image("PPA firmware", (u64)raw_image_addr,
+			raw_image_size, ppa_ram_addr);
+
+	debug("fsl-ppa: PPA entry: 0x%llx\n", ppa_ram_addr);
+	*entry = ppa_ram_addr;
+
+	return 0;
+
+out:
+	printf("fsl-ppa: error (%d)\n", ret);
+	*entry = 0;
+
+	return ret;
+}
+
+static int ppa_init_entry(void *ppa_entry)
+{
+	int ret;
+	u32 *boot_loc_ptr_l, *boot_loc_ptr_h;
+
+#ifdef CONFIG_FSL_LSCH3
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+	boot_loc_ptr_l = &gur->bootlocptrl;
+	boot_loc_ptr_h = &gur->bootlocptrh;
+#elif defined(CONFIG_FSL_LSCH2)
+	struct ccsr_scfg __iomem *scfg = (void *)(CONFIG_SYS_FSL_SCFG_ADDR);
+	boot_loc_ptr_l = &scfg->scratchrw[1];
+	boot_loc_ptr_h = &scfg->scratchrw[0];
+#endif
+
+	debug("fsl-ppa: boot_loc_ptr_l = 0x%p, boot_loc_ptr_h =0x%p\n",
+			boot_loc_ptr_l, boot_loc_ptr_h);
+	ret = ppa_init(ppa_entry, boot_loc_ptr_l, boot_loc_ptr_h);
+	if (ret < 0)
+		return ret;
+
+	debug("fsl-ppa: Return from PPA: current_el = %d\n", current_el());
+
+	/* The PE will be turned into EL2 when run out of PPA. */
+	if (current_el() != 2)
+		return -EACCES;
+
+	/*
+	 * First, set vector for EL2.
+	 */
+	c_runtime_cpu_setup();
+
+	/* Enable caches and MMU for EL2. */
+	enable_caches();
+
+	return 0;
+}
+
+bool sec_firmware_is_valid(void)
+{
+	void *ppa_addr;
+
+#ifdef CONFIG_SYS_LS_PPA_FW_IN_NOR
+	ppa_addr = (void *)CONFIG_SYS_LS_PPA_FW_ADDR;
+#else
+#error "No CONFIG_SYS_LS_PPA_FW_IN_xxx defined"
+#endif
+
+	return ppa_firmware_is_valid(ppa_addr);
+}
+
+#ifdef CONFIG_ARMV8_PSCI
+unsigned int sec_firmware_support_psci_version(void)
+{
+	unsigned int psci_ver = 0;
+
+	if (sec_firmware_is_valid())
+		psci_ver = ppa_support_psci_version();
+
+	return psci_ver;
+}
+#endif
+
+int sec_firmware_init(void)
+{
+	u64 ppa_entry;
+	int ret;
+
+	ret = ppa_init_pre(&ppa_entry);
+
+	if (!ret && ppa_entry) {
+		ret = ppa_init_entry((void *)ppa_entry);
+		if (ret)
+			printf("fsl-ppa: failed to initialze PPA\n");
+	} else {
+		printf("fsl-ppa: failed to get the PPA entry\n");
+	}
+
+	return ret;
+}
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa_entry.S b/arch/arm/cpu/armv8/fsl-layerscape/ppa_entry.S
new file mode 100644
index 0000000..a17869f
--- /dev/null
+++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa_entry.S
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2016 NXP Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <config.h>
+#include <linux/linkage.h>
+#include <asm/system.h>
+#include <asm/macro.h>
+
+ENTRY(ppa_init)
+	/*
+	 * x0: PPA entry point
+	 * x1: Boot Location Pointer Low
+	 * x2: Boot Location Pointer High
+	 */
+
+	/* Save stack pointer for EL2 */
+	mov	x3, sp
+	msr	sp_el2, x3
+
+	/* Set boot loc pointer */
+        adr	x4, 1f
+        mov	x3, x4
+#if defined(CONFIG_FSL_LSCH2)
+        rev	w3, w3
+#endif
+        str	w3, [x1]
+        lsr	x3, x4, #32
+#if defined(CONFIG_FSL_LSCH2)
+        rev	w3, w3
+#endif
+        str	w3, [x2]
+
+/* Call PPA monitor */
+        br	x0
+
+1:
+        mov	x0, #0
+        ret
+ENDPROC(ppa_init)
+
+#ifdef CONFIG_ARMV8_PSCI
+ENTRY(ppa_support_psci_version)
+	mov	x0, 0x84000000
+	mov	x1, 0x0
+	mov	x2, 0x0
+	mov	x3, 0x0
+	smc	#0
+	ret
+ENDPROC(ppa_support_psci_version)
+#endif
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/ppa.h b/arch/arm/include/asm/arch-fsl-layerscape/ppa.h
new file mode 100644
index 0000000..041f858
--- /dev/null
+++ b/arch/arm/include/asm/arch-fsl-layerscape/ppa.h
@@ -0,0 +1,14 @@
+/*
+ * Copyright 2016 NXP Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __FSL_PPA_H_
+#define __FSL_PPA_H_
+
+int ppa_init(void *, u32*, u32*);
+unsigned long ppa_get_dram_block_size(void);
+unsigned int ppa_support_psci_version(void);
+
+#endif
diff --git a/arch/arm/include/asm/armv8/sec_firmware.h b/arch/arm/include/asm/armv8/sec_firmware.h
index e2591aa..29f2c38 100644
--- a/arch/arm/include/asm/armv8/sec_firmware.h
+++ b/arch/arm/include/asm/armv8/sec_firmware.h
@@ -7,6 +7,10 @@
 #ifndef __SEC_FIRMWARE_H_
 #define __SEC_FIRMWARE_H_
 
+#ifdef CONFIG_FSL_LS_PPA
+#include <asm/arch/ppa.h>
+#endif
+
 int sec_firmware_init(void);
 bool sec_firmware_is_valid(void);
 #ifdef CONFIG_ARMV8_PSCI
-- 
2.1.0.27.g96db324

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[Zhiqiang Hou]

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

If the PSCI and PPA is ready, skip the fixup for spin-table and
waking secondary cores. If not, change SMP method to spin-table,
and the device node of PSCI will be removed.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
V5:
 - Changed the checking if the PSCI feature is ready to read the psci version.

V4:
 - Reordered this patch.

 arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
 arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36 +++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
index 672a453..eb566cd 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
@@ -23,6 +23,9 @@
 #ifdef CONFIG_FSL_ESDHC
 #include <fsl_esdhc.h>
 #endif
+#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
+#include <asm/armv8/sec_firmware.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -618,6 +621,7 @@ int arch_early_init_r(void)
 {
 #ifdef CONFIG_MP
 	int rv = 1;
+	bool psci_support = false;
 #endif
 
 #ifdef CONFIG_SYS_FSL_ERRATUM_A009635
@@ -625,9 +629,16 @@ int arch_early_init_r(void)
 #endif
 
 #ifdef CONFIG_MP
-	rv = fsl_layerscape_wake_seconday_cores();
-	if (rv)
-		printf("Did not wake secondary cores\n");
+#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) && defined(CONFIG_ARMV8_PSCI)
+	/* Check the psci version to determine if the psci is supported */
+	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
+			true : false;
+#endif
+	if (!psci_support) {
+		rv = fsl_layerscape_wake_seconday_cores();
+		if (rv)
+			printf("Did not wake secondary cores\n");
+	}
 #endif
 
 #ifdef CONFIG_SYS_HAS_SERDES
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
index 1e875c4..7b57410 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
@@ -20,6 +20,9 @@
 #ifdef CONFIG_MP
 #include <asm/arch/mp.h>
 #endif
+#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
+#include <asm/armv8/sec_firmware.h>
+#endif
 
 int fdt_fixup_phy_connection(void *blob, int offset, phy_interface_t phyc)
 {
@@ -36,7 +39,40 @@ void ft_fixup_cpu(void *blob)
 	int addr_cells;
 	u64 val, core_id;
 	size_t *boot_code_size = &(__secondary_boot_code_size);
+#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) && defined(CONFIG_ARMV8_PSCI)
+	int node;
+	bool psci_support;
+#endif
+
+#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) && defined(CONFIG_ARMV8_PSCI)
+	/* Check the psci version to determine if the psci is supported */
+	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
+			true : false;
+	if (!psci_support) {
+		/* remove psci DT node */
+		node = fdt_path_offset(blob, "/psci");
+		if (node >= 0)
+			goto remove_psci_node;
+
+		node = fdt_node_offset_by_compatible(blob, -1, "arm,psci");
+		if (node >= 0)
+			goto remove_psci_node;
+
+		node = fdt_node_offset_by_compatible(blob, -1, "arm,psci-0.2");
+		if (node >= 0)
+			goto remove_psci_node;
 
+		node = fdt_node_offset_by_compatible(blob, -1, "arm,psci-1.0");
+		if (node >= 0)
+			goto remove_psci_node;
+
+remove_psci_node:
+		if (node >= 0)
+			fdt_del_node(blob, node);
+	} else {
+		return;
+	}
+#endif
 	off = fdt_path_offset(blob, "/cpus");
 	if (off < 0) {
 		puts("couldn't find /cpus node\n");
-- 
2.1.0.27.g96db324

[U-Boot] [PATCHV5 5/6] ARMv8/PSCI: Fixup the device tree for PSCI

[Zhiqiang Hou]

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

Set the enable-method in the cpu node to PSCI, and create device
node for PSCI.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
V5:
 - Moved the weak func sec_firmware_support_psci_version to sec_firmware.c.
 - Correct the PSCI version value in switch-case. The right version format is marjor[31:16]:minor[15:0].

 arch/arm/cpu/armv8/Makefile |   1 +
 arch/arm/cpu/armv8/cpu-dt.c | 125 ++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/lib/bootm-fdt.c    |   2 +-
 3 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/cpu/armv8/cpu-dt.c

diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
index 667572b..8a4f538 100644
--- a/arch/arm/cpu/armv8/Makefile
+++ b/arch/arm/cpu/armv8/Makefile
@@ -15,6 +15,7 @@ obj-y	+= cache.o
 obj-y	+= tlb.o
 obj-y	+= transition.o
 obj-y	+= fwcall.o
+obj-y	+= cpu-dt.o
 obj-$(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) += sec_firmware.o
 
 obj-$(CONFIG_FSL_LAYERSCAPE) += fsl-layerscape/
diff --git a/arch/arm/cpu/armv8/cpu-dt.c b/arch/arm/cpu/armv8/cpu-dt.c
new file mode 100644
index 0000000..db0fc35
--- /dev/null
+++ b/arch/arm/cpu/armv8/cpu-dt.c
@@ -0,0 +1,125 @@
+/*
+ * Copyright 2016 NXP Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <libfdt.h>
+#include <fdt_support.h>
+#include <linux/sizes.h>
+#include <linux/kernel.h>
+#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
+#include <asm/armv8/sec_firmware.h>
+#endif
+
+#ifdef CONFIG_MP
+DECLARE_GLOBAL_DATA_PTR;
+
+#if defined(CONFIG_ARMV8_PSCI)
+static int cpu_update_dt_psci(void *fdt)
+{
+	int nodeoff;
+	unsigned int psci_ver;
+	char *psci_compt;
+	int tmp;
+
+	nodeoff = fdt_path_offset(fdt, "/cpus");
+	if (nodeoff < 0) {
+		printf("couldn't find /cpus\n");
+		return nodeoff;
+	}
+
+	/* add 'enable-method = "psci"' to each cpu node */
+	for (tmp = fdt_first_subnode(fdt, nodeoff);
+	     tmp >= 0;
+	     tmp = fdt_next_subnode(fdt, tmp)) {
+		const struct fdt_property *prop;
+		int len;
+
+		prop = fdt_get_property(fdt, tmp, "device_type", &len);
+		if (!prop)
+			continue;
+		if (len < 4)
+			continue;
+		if (strcmp(prop->data, "cpu"))
+			continue;
+
+		/*
+		 * Not checking rv here, our approach is to skip over errors in
+		 * individual cpu nodes, hopefully some of the nodes are
+		 * processed correctly and those will boot
+		 */
+		fdt_setprop_string(fdt, tmp, "enable-method", "psci");
+	}
+
+	/*
+	 * The PSCI node might be called "/psci" or might be called something
+	 * else but contain either of the compatible strings
+	 * "arm,psci"/"arm,psci-0.2"
+	 */
+	nodeoff = fdt_path_offset(fdt, "/psci");
+	if (nodeoff >= 0)
+		goto init_psci_node;
+
+	nodeoff = fdt_node_offset_by_compatible(fdt, -1, "arm,psci");
+	if (nodeoff >= 0)
+		goto init_psci_node;
+
+	nodeoff = fdt_node_offset_by_compatible(fdt, -1, "arm,psci-0.2");
+	if (nodeoff >= 0)
+		goto init_psci_node;
+
+	nodeoff = fdt_node_offset_by_compatible(fdt, -1, "arm,psci-1.0");
+	if (nodeoff >= 0)
+		goto init_psci_node;
+
+	nodeoff = fdt_path_offset(fdt, "/");
+	if (nodeoff < 0)
+		return nodeoff;
+
+	nodeoff = fdt_add_subnode(fdt, nodeoff, "psci");
+	if (nodeoff < 0)
+		return nodeoff;
+
+init_psci_node:
+#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
+	psci_ver = sec_firmware_support_psci_version();
+#endif
+	switch (psci_ver) {
+	case 0x00010000:
+		psci_compt = "arm,psci-1.0";
+		break;
+	case 0x00000002:
+		psci_compt = "arm,psci-0.2";
+		break;
+	case 0x00000001:
+		psci_compt = "arm,psci";
+		break;
+	default:
+		psci_compt = "arm,psci-0.2";
+		break;
+	}
+
+	tmp = fdt_setprop_string(fdt, nodeoff, "compatible", psci_compt);
+	if (tmp)
+		return tmp;
+
+	tmp = fdt_setprop_string(fdt, nodeoff, "method", "smc");
+	if (tmp)
+		return tmp;
+
+	return 0;
+}
+#endif
+#endif
+
+int psci_update_dt(void *fdt)
+{
+#ifdef CONFIG_MP
+#if defined(CONFIG_ARMV8_PSCI)
+	cpu_update_dt_psci(fdt);
+#endif
+#endif
+	return 0;
+}
diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c
index 7677358..c642ff8 100644
--- a/arch/arm/lib/bootm-fdt.c
+++ b/arch/arm/lib/bootm-fdt.c
@@ -42,7 +42,7 @@ int arch_fixup_fdt(void *blob)
 	}
 
 	ret = fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
-#ifdef CONFIG_ARMV7_NONSEC
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV8_PSCI)
 	if (ret)
 		return ret;
 
-- 
2.1.0.27.g96db324

[U-Boot] [PATCHV5 6/6] ARMv8/ls1043ardb: Integrate FSL PPA

[Zhiqiang Hou]

From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>

So far, the PPA use PSCI to make secondary cores bootup. So when
PPA is enabled, add the CONFIG_ARMV8_PSCI to identify the SMP
boot-method between PSCI and spin-table.

Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
---
V5:
 - Merged the 7th patch of this patchset in v4 to this patch.
 - Enalbed secure monitor framework support.

V4:
 - Reordered this patch.
 - Added checking the returned value of func ppa_init_pre().

 board/freescale/ls1043ardb/ls1043ardb.c |  8 ++++++++
 include/configs/ls1043ardb.h            | 11 +++++++++++
 2 files changed, 19 insertions(+)

diff --git a/board/freescale/ls1043ardb/ls1043ardb.c b/board/freescale/ls1043ardb/ls1043ardb.c
index ec5fdbf..65ce062 100644
--- a/board/freescale/ls1043ardb/ls1043ardb.c
+++ b/board/freescale/ls1043ardb/ls1043ardb.c
@@ -9,6 +9,7 @@
 #include <asm/io.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/fsl_serdes.h>
+#include <asm/arch/ppa.h>
 #include <asm/arch/soc.h>
 #include <fdt_support.h>
 #include <hwconfig.h>
@@ -25,6 +26,9 @@
 #ifdef CONFIG_U_QE
 #include <fsl_qe.h>
 #endif
+#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
+#include <asm/armv8/sec_firmware.h>
+#endif
 
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -103,6 +107,10 @@ int board_init(void)
 	enable_layerscape_ns_access();
 #endif
 
+#ifdef CONFIG_FSL_LS_PPA
+	sec_firmware_init();
+#endif
+
 #ifdef CONFIG_U_QE
 	u_qe_init();
 #endif
diff --git a/include/configs/ls1043ardb.h b/include/configs/ls1043ardb.h
index 6d35be2..9808a59 100644
--- a/include/configs/ls1043ardb.h
+++ b/include/configs/ls1043ardb.h
@@ -9,6 +9,17 @@
 
 #include "ls1043a_common.h"
 
+#if defined(CONFIG_FSL_LS_PPA)
+#define CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
+#define CONFIG_ARMV8_PSCI
+#define CONFIG_SYS_LS_PPA_DRAM_BLOCK_MIN_SIZE		(1UL * 1024 * 1024)
+
+#define CONFIG_SYS_LS_PPA_FW_IN_NOR
+#ifdef CONFIG_SYS_LS_PPA_FW_IN_NOR
+#define	CONFIG_SYS_LS_PPA_FW_ADDR	0x60500000
+#endif
+#endif
+
 #define CONFIG_DISPLAY_CPUINFO
 #define CONFIG_DISPLAY_BOARDINFO
 
-- 
2.1.0.27.g96db324

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[York Sun]

On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> If the PSCI and PPA is ready, skip the fixup for spin-table and
> waking secondary cores. If not, change SMP method to spin-table,
> and the device node of PSCI will be removed.
> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
> V5:
>  - Changed the checking if the PSCI feature is ready to read the psci version.
> 
> V4:
>  - Reordered this patch.
> 
>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
>  arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36 +++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> index 672a453..eb566cd 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> @@ -23,6 +23,9 @@
>  #ifdef CONFIG_FSL_ESDHC
>  #include <fsl_esdhc.h>
>  #endif
> +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
> +#include <asm/armv8/sec_firmware.h>
> +#endif
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -618,6 +621,7 @@ int arch_early_init_r(void)
>  {
>  #ifdef CONFIG_MP
>  	int rv = 1;
> +	bool psci_support = false;
>  #endif
>  
>  #ifdef CONFIG_SYS_FSL_ERRATUM_A009635
> @@ -625,9 +629,16 @@ int arch_early_init_r(void)
>  #endif
>  
>  #ifdef CONFIG_MP
> -	rv = fsl_layerscape_wake_seconday_cores();
> -	if (rv)
> -		printf("Did not wake secondary cores\n");
> +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) && defined(CONFIG_ARMV8_PSCI)
> +	/* Check the psci version to determine if the psci is supported */
> +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
> +			true : false;
> +#endif
> +	if (!psci_support) {
> +		rv = fsl_layerscape_wake_seconday_cores();
> +		if (rv)
> +			printf("Did not wake secondary cores\n");
> +	}
>  #endif
>  

Zhiqiang,

Here you presume the psci version returned by
sec_firmware_support_psci_version() is a proof of a functional psci. I think
that is flawed. This sec_firmware_support_psci_version() can return a positive
number as far as the image in specified location is valid. It doesn't guarantee
the image is running. The only place you would know the result of loading such
image is by calling sec_firmware_init() in board_init() in your 6th patch. But
you didn't check the return. That means you blindly believe a valid image would
be successfully loaded and started to run. Later when you need to decide to use
spin-table or PSCI for secondary cores, you don't know if such image is running.

I wonder if you can make a psci call to return the version number.

York

[U-Boot] [PATCHV5 3/6] ARMv8/layerscape: Add FSL PPA support

[York Sun]

On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> The FSL Primary Protected Application (PPA) is a software component
> loaded during boot which runs in TrustZone and remains resident
> after boot.
> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
> V5:
>  - Added API sec_firmware_init() implementation.
> 
> V4:
>  - Moved secure firmware validation API to this patch.
>  - Moved secure firmware getting supported PSCI version API to this patch.
> 
> V3:
>  - Refactor the code.
>  - Add PPA firmware version info output.
> 
>  arch/arm/cpu/armv8/fsl-layerscape/Makefile     |   1 +
>  arch/arm/cpu/armv8/fsl-layerscape/ppa.c        | 311 +++++++++++++++++++++++++
>  arch/arm/cpu/armv8/fsl-layerscape/ppa_entry.S  |  53 +++++
>  arch/arm/include/asm/arch-fsl-layerscape/ppa.h |  14 ++
>  arch/arm/include/asm/armv8/sec_firmware.h      |   4 +
>  5 files changed, 383 insertions(+)
>  create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ppa.c
>  create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ppa_entry.S
>  create mode 100644 arch/arm/include/asm/arch-fsl-layerscape/ppa.h
> 
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Makefile b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
> index 5f86ef9..1535fee 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
> @@ -10,6 +10,7 @@ obj-y += soc.o
>  obj-$(CONFIG_MP) += mp.o
>  obj-$(CONFIG_OF_LIBFDT) += fdt.o
>  obj-$(CONFIG_SPL) += spl.o
> +obj-$(CONFIG_FSL_LS_PPA) += ppa.o ppa_entry.o
>  
>  ifneq ($(CONFIG_FSL_LSCH3),)
>  obj-y += fsl_lsch3_speed.o
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
> new file mode 100644
> index 0000000..6a75960
> --- /dev/null
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
> @@ -0,0 +1,311 @@
> +/*
> + * Copyright 2016 NXP Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +#include <common.h>
> +#include <config.h>
> +#include <errno.h>
> +#include <malloc.h>
> +#include <asm/system.h>
> +#include <asm/io.h>
> +#include <asm/types.h>
> +#include <asm/macro.h>
> +#include <asm/arch/soc.h>
> +#ifdef CONFIG_FSL_LSCH3
> +#include <asm/arch/immap_lsch3.h>
> +#elif defined(CONFIG_FSL_LSCH2)
> +#include <asm/arch/immap_lsch2.h>
> +#endif
> +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
> +#include <asm/armv8/sec_firmware.h>
> +#endif
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +extern void c_runtime_cpu_setup(void);
> +
> +#define LS_PPA_FIT_FIRMWARE_IMAGE	"firmware"
> +#define LS_PPA_FIT_CNF_NAME		"config at 1"
> +#define PPA_MEM_SIZE_ENV_VAR		"ppamemsize"
> +
> +/*
> + * Return the actual size of the PPA private DRAM block.
> + */
> +unsigned long ppa_get_dram_block_size(void)
> +{
> +	unsigned long dram_block_size = CONFIG_SYS_LS_PPA_DRAM_BLOCK_MIN_SIZE;
> +
> +	char *dram_block_size_env_var = getenv(PPA_MEM_SIZE_ENV_VAR);
> +
> +	if (dram_block_size_env_var) {
> +		dram_block_size = simple_strtoul(dram_block_size_env_var, NULL,
> +						 10);
> +
> +		if (dram_block_size < CONFIG_SYS_LS_PPA_DRAM_BLOCK_MIN_SIZE) {
> +			printf("fsl-ppa: WARNING: Invalid value for \'"
> +			       PPA_MEM_SIZE_ENV_VAR
> +			       "\' environment variable: %lu\n",
> +			       dram_block_size);
> +
> +			dram_block_size = CONFIG_SYS_LS_PPA_DRAM_BLOCK_MIN_SIZE;
> +		}
> +	}
> +
> +	return dram_block_size;
> +}
> +
> +static bool ppa_firmware_is_valid(void *ppa_addr)
> +{
> +	void *fit_hdr;
> +
> +	fit_hdr = ppa_addr;
> +
> +	if (fdt_check_header(fit_hdr)) {
> +		printf("fsl-ppa: Bad firmware image (not a FIT image)\n");
> +		return false;
> +	}
> +
> +	if (!fit_check_format(fit_hdr)) {
> +		printf("fsl-ppa: Bad firmware image (bad FIT header)\n");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static int ppa_firmware_get_data(void *ppa_addr,
> +				const void **data, size_t *size)
> +{
> +	void *fit_hdr;
> +	int conf_node_off, fw_node_off;
> +	char *conf_node_name = NULL;
> +	char *desc;
> +	int ret;
> +
> +	fit_hdr = ppa_addr;
> +	conf_node_name = LS_PPA_FIT_CNF_NAME;
> +
> +	conf_node_off = fit_conf_get_node(fit_hdr, conf_node_name);
> +	if (conf_node_off < 0) {
> +		printf("fsl-ppa: %s: no such config\n", conf_node_name);
> +		return -ENOENT;
> +	}
> +
> +	fw_node_off = fit_conf_get_prop_node(fit_hdr, conf_node_off,
> +			LS_PPA_FIT_FIRMWARE_IMAGE);
> +	if (fw_node_off < 0) {
> +		printf("fsl-ppa: No '%s' in config\n",
> +				LS_PPA_FIT_FIRMWARE_IMAGE);
> +		return -ENOLINK;
> +	}
> +
> +	/* Verify PPA firmware image */
> +	if (!(fit_image_verify(fit_hdr, fw_node_off))) {
> +		printf("fsl-ppa: Bad firmware image (bad CRC)\n");
> +		return -EINVAL;
> +	}
> +
> +	if (fit_image_get_data(fit_hdr, fw_node_off, data, size)) {
> +		printf("fsl-ppa: Can't get %s subimage data/size",
> +				LS_PPA_FIT_FIRMWARE_IMAGE);
> +		return -ENOENT;
> +	}
> +
> +	ret = fit_get_desc(fit_hdr, fw_node_off, &desc);
> +	if (ret)
> +		printf("PPA Firmware unavailable\n");
> +	else
> +		printf("%s\n", desc);
> +
> +	return 0;
> +}
> +
> +/*
> + * PPA firmware FIT image parser checks if the image is in FIT
> + * format, verifies integrity of the image and calculates raw
> + * image address and size values.
> + *
> + * Returns 0 on success and a negative errno on error task fail.
> + */
> +static int ppa_parse_firmware_fit_image(const void **raw_image_addr,
> +				size_t *raw_image_size)
> +{
> +	void *ppa_addr;
> +	int ret;
> +
> +#ifdef CONFIG_SYS_LS_PPA_FW_IN_NOR
> +	ppa_addr = (void *)CONFIG_SYS_LS_PPA_FW_ADDR;
> +#else
> +#error "No CONFIG_SYS_LS_PPA_FW_IN_xxx defined"
> +#endif
> +
> +	if (!ppa_firmware_is_valid(ppa_addr))
> +		return -1;
> +
> +	ret = ppa_firmware_get_data(ppa_addr, raw_image_addr, raw_image_size);
> +	if (ret)
> +		return ret;
> +
> +	debug("fsl-ppa: raw_image_addr = 0x%p, raw_image_size = 0x%lx\n",
> +			*raw_image_addr, *raw_image_size);
> +
> +	return 0;
> +}
> +
> +static int ppa_copy_image(const char *title,
> +			 u64 image_addr, u32 image_size, u64 ppa_ram_addr)
> +{
> +	debug("%s copied to address 0x%p\n", title, (void *)ppa_ram_addr);
> +	memcpy((void *)ppa_ram_addr, (void *)image_addr, image_size);
> +	flush_dcache_range(ppa_ram_addr, ppa_ram_addr + image_size);
> +
> +	return 0;
> +}
> +
> +static int ppa_init_pre(u64 *entry)
> +{
> +	u64 ppa_ram_addr;
> +	const void *raw_image_addr;
> +	size_t raw_image_size = 0;
> +	size_t ppa_ram_size = ppa_get_dram_block_size();
> +	int ret;
> +
> +	debug("fsl-ppa: ppa size(0x%lx)\n", ppa_ram_size);
> +
> +	/*
> +	 * The Excetpion Level must be EL3 to prepare and initialize the PPA.
> +	 */
> +	if (current_el() != 3) {
> +		ret = -EACCES;
> +		goto out;
> +	}
> +
> +#ifdef CONFIG_SYS_MEM_RESERVE_SECURE
> +	/*
> +	 * The PPA must be stored in secure memory.
> +	 * Append PPA to secure mmu table.
> +	 */
> +	if (!(gd->secure_ram & MEM_RESERVE_SECURE_MAINTAINED)) {
> +		ret = -ENXIO;
> +		goto out;
> +	}
> +
> +	ppa_ram_addr = (gd->secure_ram & MEM_RESERVE_SECURE_ADDR_MASK) +
> +			gd->arch.tlb_size;
> +#else
> +#error "The CONFIG_SYS_MEM_RESERVE_SECURE must be defined when enabled PPA"
> +#endif
> +
> +	/* Align PPA base address to 4K */
> +	ppa_ram_addr = (ppa_ram_addr + 0xfff) & ~0xfff;
> +	debug("fsl-ppa: PPA load address (0x%llx)\n", ppa_ram_addr);
> +
> +	ret = ppa_parse_firmware_fit_image(&raw_image_addr, &raw_image_size);
> +	if (ret < 0)
> +		goto out;
> +
> +	if (ppa_ram_size < raw_image_size) {
> +		ret = -ENOSPC;
> +		goto out;
> +	}
> +
> +	/* TODO:
> +	 * Check if the end addr of PPA has been extend the secure memory.
> +	 */
> +
> +	ppa_copy_image("PPA firmware", (u64)raw_image_addr,
> +			raw_image_size, ppa_ram_addr);
> +
> +	debug("fsl-ppa: PPA entry: 0x%llx\n", ppa_ram_addr);
> +	*entry = ppa_ram_addr;
> +
> +	return 0;
> +
> +out:
> +	printf("fsl-ppa: error (%d)\n", ret);
> +	*entry = 0;
> +
> +	return ret;
> +}
> +
> +static int ppa_init_entry(void *ppa_entry)
> +{
> +	int ret;
> +	u32 *boot_loc_ptr_l, *boot_loc_ptr_h;
> +
> +#ifdef CONFIG_FSL_LSCH3
> +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
> +	boot_loc_ptr_l = &gur->bootlocptrl;
> +	boot_loc_ptr_h = &gur->bootlocptrh;
> +#elif defined(CONFIG_FSL_LSCH2)
> +	struct ccsr_scfg __iomem *scfg = (void *)(CONFIG_SYS_FSL_SCFG_ADDR);
> +	boot_loc_ptr_l = &scfg->scratchrw[1];
> +	boot_loc_ptr_h = &scfg->scratchrw[0];
> +#endif
> +
> +	debug("fsl-ppa: boot_loc_ptr_l = 0x%p, boot_loc_ptr_h =0x%p\n",
> +			boot_loc_ptr_l, boot_loc_ptr_h);
> +	ret = ppa_init(ppa_entry, boot_loc_ptr_l, boot_loc_ptr_h);
> +	if (ret < 0)
> +		return ret;
> +
> +	debug("fsl-ppa: Return from PPA: current_el = %d\n", current_el());
> +
> +	/* The PE will be turned into EL2 when run out of PPA. */
> +	if (current_el() != 2)
> +		return -EACCES;
> +
> +	/*
> +	 * First, set vector for EL2.
> +	 */
> +	c_runtime_cpu_setup();
> +
> +	/* Enable caches and MMU for EL2. */
> +	enable_caches();
> +
> +	return 0;
> +}
> +
> +bool sec_firmware_is_valid(void)
> +{
> +	void *ppa_addr;
> +
> +#ifdef CONFIG_SYS_LS_PPA_FW_IN_NOR
> +	ppa_addr = (void *)CONFIG_SYS_LS_PPA_FW_ADDR;
> +#else
> +#error "No CONFIG_SYS_LS_PPA_FW_IN_xxx defined"
> +#endif
> +
> +	return ppa_firmware_is_valid(ppa_addr);
> +}
> +
> +#ifdef CONFIG_ARMV8_PSCI
> +unsigned int sec_firmware_support_psci_version(void)
> +{
> +	unsigned int psci_ver = 0;
> +
> +	if (sec_firmware_is_valid())
> +		psci_ver = ppa_support_psci_version();
> +
> +	return psci_ver;
> +}
> +#endif


Zhiqiang,

I am not convinced this function can be used to prove ppa is up and running, as
you used in your 4th patch in this set. See more comment there.

This function can return a psci version number based on the image. Is it
possible to make a PSCI call to return the version of running image?

York

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[York Sun]

On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> 
> If the PSCI and PPA is ready, skip the fixup for spin-table and
> waking secondary cores. If not, change SMP method to spin-table,
> and the device node of PSCI will be removed.
> 
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> ---
> V5:
>  - Changed the checking if the PSCI feature is ready to read the psci version.
> 
> V4:
>  - Reordered this patch.
> 
>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
>  arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36 +++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> index 672a453..eb566cd 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> @@ -23,6 +23,9 @@
>  #ifdef CONFIG_FSL_ESDHC
>  #include <fsl_esdhc.h>
>  #endif
> +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
> +#include <asm/armv8/sec_firmware.h>
> +#endif
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -618,6 +621,7 @@ int arch_early_init_r(void)
>  {
>  #ifdef CONFIG_MP
>  	int rv = 1;
> +	bool psci_support = false;
>  #endif
>  
>  #ifdef CONFIG_SYS_FSL_ERRATUM_A009635
> @@ -625,9 +629,16 @@ int arch_early_init_r(void)
>  #endif
>  
>  #ifdef CONFIG_MP
> -	rv = fsl_layerscape_wake_seconday_cores();
> -	if (rv)
> -		printf("Did not wake secondary cores\n");
> +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) && defined(CONFIG_ARMV8_PSCI)
> +	/* Check the psci version to determine if the psci is supported */
> +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
> +			true : false;

Another comment, even if the function can be used to indicate if psci is
available, do you have to cast it to (int)? I think this can be simplified as
	psci_support = sec_firmware_support_psci_version() > 0;

York

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[Zhiqiang Hou]

Hi York,

Thanks for your comments!

> -----Original Message-----
> From: York Sun [mailto:york.sun at nxp.com]
> Sent: 2016?6?8? 8:51
> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
> albert.u.boot at aribaud.net; scottwood at freescale.com;
> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
> prabhakar at freescale.com; bhupesh.sharma at freescale.com
> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly
> 
> On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > If the PSCI and PPA is ready, skip the fixup for spin-table and waking
> > secondary cores. If not, change SMP method to spin-table, and the
> > device node of PSCI will be removed.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> > V5:
> >  - Changed the checking if the PSCI feature is ready to read the psci version.
> >
> > V4:
> >  - Reordered this patch.
> >
> >  arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
> > arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36
> > +++++++++++++++++++++++++++++++++
> >  2 files changed, 50 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > index 672a453..eb566cd 100644
> > --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > @@ -23,6 +23,9 @@
> >  #ifdef CONFIG_FSL_ESDHC
> >  #include <fsl_esdhc.h>
> >  #endif
> > +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
> > +<asm/armv8/sec_firmware.h> #endif
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -618,6 +621,7 @@ int arch_early_init_r(void)  {  #ifdef CONFIG_MP
> >  	int rv = 1;
> > +	bool psci_support = false;
> >  #endif
> >
> >  #ifdef CONFIG_SYS_FSL_ERRATUM_A009635 @@ -625,9 +629,16 @@ int
> > arch_early_init_r(void)  #endif
> >
> >  #ifdef CONFIG_MP
> > -	rv = fsl_layerscape_wake_seconday_cores();
> > -	if (rv)
> > -		printf("Did not wake secondary cores\n");
> > +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) &&
> defined(CONFIG_ARMV8_PSCI)
> > +	/* Check the psci version to determine if the psci is supported */
> > +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
> > +			true : false;
> > +#endif
> > +	if (!psci_support) {
> > +		rv = fsl_layerscape_wake_seconday_cores();
> > +		if (rv)
> > +			printf("Did not wake secondary cores\n");
> > +	}
> >  #endif
> >
> 
> Zhiqiang,
> 
> Here you presume the psci version returned by
> sec_firmware_support_psci_version() is a proof of a functional psci. I think that is
> flawed. This sec_firmware_support_psci_version() can return a positive number as
> far as the image in specified location is valid. It doesn't guarantee the image is
> running. The only place you would know the result of loading such image is by
> calling sec_firmware_init() in board_init() in your 6th patch. But you didn't check
> the return. That means you blindly believe a valid image would be successfully
> loaded and started to run. Later when you need to decide to use spin-table or PSCI
> for secondary cores, you don't know if such image is running.
> 
> I wonder if you can make a psci call to return the version number.

So far, the function sec_firmware_support_psci_version() do the psci call to get the
supported psci version. And if the returned psci version is valid, then we assume the
PPA is running, otherwise if the returned value is an error number, we assume the
PPA isn't running regularly. The return value of the function was casted to type 'int',
because the psci_version call will return the version by pattern major[31:16]:minor[15:0]
in normal, but a negative error number while in exception.

Thanks,
Zhiqiang

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[york sun]

On 06/11/2016 08:49 PM, Zhiqiang Hou wrote:
> Hi York,
>
> Thanks for your comments!
>
>> -----Original Message-----
>> From: York Sun [mailto:york.sun at nxp.com]
>> Sent: 2016?6?8? 8:51
>> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
>> albert.u.boot at aribaud.net; scottwood at freescale.com;
>> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
>> prabhakar at freescale.com; bhupesh.sharma at freescale.com
>> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly
>>
>> On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
>>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>>>
>>> If the PSCI and PPA is ready, skip the fixup for spin-table and waking
>>> secondary cores. If not, change SMP method to spin-table, and the
>>> device node of PSCI will be removed.
>>>
>>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>>> ---
>>> V5:
>>>   - Changed the checking if the PSCI feature is ready to read the psci version.
>>>
>>> V4:
>>>   - Reordered this patch.
>>>
>>>   arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
>>> arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36
>>> +++++++++++++++++++++++++++++++++
>>>   2 files changed, 50 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> index 672a453..eb566cd 100644
>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> @@ -23,6 +23,9 @@
>>>   #ifdef CONFIG_FSL_ESDHC
>>>   #include <fsl_esdhc.h>
>>>   #endif
>>> +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
>>> +<asm/armv8/sec_firmware.h> #endif
>>>
>>>   DECLARE_GLOBAL_DATA_PTR;
>>>
>>> @@ -618,6 +621,7 @@ int arch_early_init_r(void)  {  #ifdef CONFIG_MP
>>>   	int rv = 1;
>>> +	bool psci_support = false;
>>>   #endif
>>>
>>>   #ifdef CONFIG_SYS_FSL_ERRATUM_A009635 @@ -625,9 +629,16 @@ int
>>> arch_early_init_r(void)  #endif
>>>
>>>   #ifdef CONFIG_MP
>>> -	rv = fsl_layerscape_wake_seconday_cores();
>>> -	if (rv)
>>> -		printf("Did not wake secondary cores\n");
>>> +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) &&
>> defined(CONFIG_ARMV8_PSCI)
>>> +	/* Check the psci version to determine if the psci is supported */
>>> +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
>>> +			true : false;
>>> +#endif
>>> +	if (!psci_support) {
>>> +		rv = fsl_layerscape_wake_seconday_cores();
>>> +		if (rv)
>>> +			printf("Did not wake secondary cores\n");
>>> +	}
>>>   #endif
>>>
>>
>> Zhiqiang,
>>
>> Here you presume the psci version returned by
>> sec_firmware_support_psci_version() is a proof of a functional psci. I think that is
>> flawed. This sec_firmware_support_psci_version() can return a positive number as
>> far as the image in specified location is valid. It doesn't guarantee the image is
>> running. The only place you would know the result of loading such image is by
>> calling sec_firmware_init() in board_init() in your 6th patch. But you didn't check
>> the return. That means you blindly believe a valid image would be successfully
>> loaded and started to run. Later when you need to decide to use spin-table or PSCI
>> for secondary cores, you don't know if such image is running.
>>
>> I wonder if you can make a psci call to return the version number.
>
> So far, the function sec_firmware_support_psci_version() do the psci call to get the
> supported psci version. And if the returned psci version is valid, then we assume the
> PPA is running, otherwise if the returned value is an error number, we assume the
> PPA isn't running regularly. The return value of the function was casted to type 'int',
> because the psci_version call will return the version by pattern major[31:16]:minor[15:0]
> in normal, but a negative error number while in exception.
>
I see. It verifies the image is valid first, then it makes an smc call. 
Thanks.

York

[U-Boot] [PATCHV5 3/6] ARMv8/layerscape: Add FSL PPA support

[Zhiqiang Hou]

Hi York,

Thanks for your comments!

> -----Original Message-----
> From: York Sun [mailto:york.sun at nxp.com]
> Sent: 2016?6?8? 8:51
> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
> albert.u.boot at aribaud.net; scottwood at freescale.com;
> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
> prabhakar at freescale.com; bhupesh.sharma at freescale.com
> Subject: Re: [PATCHV5 3/6] ARMv8/layerscape: Add FSL PPA support
> 
> On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > The FSL Primary Protected Application (PPA) is a software component
> > loaded during boot which runs in TrustZone and remains resident after
> > boot.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> > V5:
> >  - Added API sec_firmware_init() implementation.
> >
> > V4:
> >  - Moved secure firmware validation API to this patch.
> >  - Moved secure firmware getting supported PSCI version API to this patch.
> >
> > V3:
> >  - Refactor the code.
> >  - Add PPA firmware version info output.
> >
> >  arch/arm/cpu/armv8/fsl-layerscape/Makefile     |   1 +
> >  arch/arm/cpu/armv8/fsl-layerscape/ppa.c        | 311
> +++++++++++++++++++++++++
> >  arch/arm/cpu/armv8/fsl-layerscape/ppa_entry.S  |  53 +++++
> > arch/arm/include/asm/arch-fsl-layerscape/ppa.h |  14 ++
> >  arch/arm/include/asm/armv8/sec_firmware.h      |   4 +
> >  5 files changed, 383 insertions(+)
> >  create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ppa.c
> >  create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ppa_entry.S
> >  create mode 100644 arch/arm/include/asm/arch-fsl-layerscape/ppa.h
> >
> > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
> > b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
> > index 5f86ef9..1535fee 100644
> > --- a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
> > @@ -10,6 +10,7 @@ obj-y += soc.o
> >  obj-$(CONFIG_MP) += mp.o
> >  obj-$(CONFIG_OF_LIBFDT) += fdt.o
> >  obj-$(CONFIG_SPL) += spl.o
> > +obj-$(CONFIG_FSL_LS_PPA) += ppa.o ppa_entry.o
> >
> >  ifneq ($(CONFIG_FSL_LSCH3),)
> >  obj-y += fsl_lsch3_speed.o
> > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
> > b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
> > new file mode 100644
> > index 0000000..6a75960
> > --- /dev/null
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
> > @@ -0,0 +1,311 @@
> > +/*
> > + * Copyright 2016 NXP Semiconductor, Inc.
> > + *
> > + * SPDX-License-Identifier:	GPL-2.0+
> > + */
> > +#include <common.h>
> > +#include <config.h>
> > +#include <errno.h>
> > +#include <malloc.h>
> > +#include <asm/system.h>
> > +#include <asm/io.h>
> > +#include <asm/types.h>
> > +#include <asm/macro.h>
> > +#include <asm/arch/soc.h>
> > +#ifdef CONFIG_FSL_LSCH3
> > +#include <asm/arch/immap_lsch3.h>
> > +#elif defined(CONFIG_FSL_LSCH2)
> > +#include <asm/arch/immap_lsch2.h>
> > +#endif
> > +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
> > +<asm/armv8/sec_firmware.h> #endif
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +extern void c_runtime_cpu_setup(void);
> > +
> > +#define LS_PPA_FIT_FIRMWARE_IMAGE	"firmware"
> > +#define LS_PPA_FIT_CNF_NAME		"config at 1"
> > +#define PPA_MEM_SIZE_ENV_VAR		"ppamemsize"
> > +
> > +/*
> > + * Return the actual size of the PPA private DRAM block.
> > + */
> > +unsigned long ppa_get_dram_block_size(void) {
> > +	unsigned long dram_block_size =
> > +CONFIG_SYS_LS_PPA_DRAM_BLOCK_MIN_SIZE;
> > +
> > +	char *dram_block_size_env_var = getenv(PPA_MEM_SIZE_ENV_VAR);
> > +
> > +	if (dram_block_size_env_var) {
> > +		dram_block_size = simple_strtoul(dram_block_size_env_var,
> NULL,
> > +						 10);
> > +
> > +		if (dram_block_size <
> CONFIG_SYS_LS_PPA_DRAM_BLOCK_MIN_SIZE) {
> > +			printf("fsl-ppa: WARNING: Invalid value for \'"
> > +			       PPA_MEM_SIZE_ENV_VAR
> > +			       "\' environment variable: %lu\n",
> > +			       dram_block_size);
> > +
> > +			dram_block_size =
> CONFIG_SYS_LS_PPA_DRAM_BLOCK_MIN_SIZE;
> > +		}
> > +	}
> > +
> > +	return dram_block_size;
> > +}
> > +
> > +static bool ppa_firmware_is_valid(void *ppa_addr) {
> > +	void *fit_hdr;
> > +
> > +	fit_hdr = ppa_addr;
> > +
> > +	if (fdt_check_header(fit_hdr)) {
> > +		printf("fsl-ppa: Bad firmware image (not a FIT image)\n");
> > +		return false;
> > +	}
> > +
> > +	if (!fit_check_format(fit_hdr)) {
> > +		printf("fsl-ppa: Bad firmware image (bad FIT header)\n");
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static int ppa_firmware_get_data(void *ppa_addr,
> > +				const void **data, size_t *size)
> > +{
> > +	void *fit_hdr;
> > +	int conf_node_off, fw_node_off;
> > +	char *conf_node_name = NULL;
> > +	char *desc;
> > +	int ret;
> > +
> > +	fit_hdr = ppa_addr;
> > +	conf_node_name = LS_PPA_FIT_CNF_NAME;
> > +
> > +	conf_node_off = fit_conf_get_node(fit_hdr, conf_node_name);
> > +	if (conf_node_off < 0) {
> > +		printf("fsl-ppa: %s: no such config\n", conf_node_name);
> > +		return -ENOENT;
> > +	}
> > +
> > +	fw_node_off = fit_conf_get_prop_node(fit_hdr, conf_node_off,
> > +			LS_PPA_FIT_FIRMWARE_IMAGE);
> > +	if (fw_node_off < 0) {
> > +		printf("fsl-ppa: No '%s' in config\n",
> > +				LS_PPA_FIT_FIRMWARE_IMAGE);
> > +		return -ENOLINK;
> > +	}
> > +
> > +	/* Verify PPA firmware image */
> > +	if (!(fit_image_verify(fit_hdr, fw_node_off))) {
> > +		printf("fsl-ppa: Bad firmware image (bad CRC)\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (fit_image_get_data(fit_hdr, fw_node_off, data, size)) {
> > +		printf("fsl-ppa: Can't get %s subimage data/size",
> > +				LS_PPA_FIT_FIRMWARE_IMAGE);
> > +		return -ENOENT;
> > +	}
> > +
> > +	ret = fit_get_desc(fit_hdr, fw_node_off, &desc);
> > +	if (ret)
> > +		printf("PPA Firmware unavailable\n");
> > +	else
> > +		printf("%s\n", desc);
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * PPA firmware FIT image parser checks if the image is in FIT
> > + * format, verifies integrity of the image and calculates raw
> > + * image address and size values.
> > + *
> > + * Returns 0 on success and a negative errno on error task fail.
> > + */
> > +static int ppa_parse_firmware_fit_image(const void **raw_image_addr,
> > +				size_t *raw_image_size)
> > +{
> > +	void *ppa_addr;
> > +	int ret;
> > +
> > +#ifdef CONFIG_SYS_LS_PPA_FW_IN_NOR
> > +	ppa_addr = (void *)CONFIG_SYS_LS_PPA_FW_ADDR; #else #error "No
> > +CONFIG_SYS_LS_PPA_FW_IN_xxx defined"
> > +#endif
> > +
> > +	if (!ppa_firmware_is_valid(ppa_addr))
> > +		return -1;
> > +
> > +	ret = ppa_firmware_get_data(ppa_addr, raw_image_addr,
> raw_image_size);
> > +	if (ret)
> > +		return ret;
> > +
> > +	debug("fsl-ppa: raw_image_addr = 0x%p, raw_image_size = 0x%lx\n",
> > +			*raw_image_addr, *raw_image_size);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ppa_copy_image(const char *title,
> > +			 u64 image_addr, u32 image_size, u64 ppa_ram_addr) {
> > +	debug("%s copied to address 0x%p\n", title, (void *)ppa_ram_addr);
> > +	memcpy((void *)ppa_ram_addr, (void *)image_addr, image_size);
> > +	flush_dcache_range(ppa_ram_addr, ppa_ram_addr + image_size);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ppa_init_pre(u64 *entry)
> > +{
> > +	u64 ppa_ram_addr;
> > +	const void *raw_image_addr;
> > +	size_t raw_image_size = 0;
> > +	size_t ppa_ram_size = ppa_get_dram_block_size();
> > +	int ret;
> > +
> > +	debug("fsl-ppa: ppa size(0x%lx)\n", ppa_ram_size);
> > +
> > +	/*
> > +	 * The Excetpion Level must be EL3 to prepare and initialize the PPA.
> > +	 */
> > +	if (current_el() != 3) {
> > +		ret = -EACCES;
> > +		goto out;
> > +	}
> > +
> > +#ifdef CONFIG_SYS_MEM_RESERVE_SECURE
> > +	/*
> > +	 * The PPA must be stored in secure memory.
> > +	 * Append PPA to secure mmu table.
> > +	 */
> > +	if (!(gd->secure_ram & MEM_RESERVE_SECURE_MAINTAINED)) {
> > +		ret = -ENXIO;
> > +		goto out;
> > +	}
> > +
> > +	ppa_ram_addr = (gd->secure_ram &
> MEM_RESERVE_SECURE_ADDR_MASK) +
> > +			gd->arch.tlb_size;
> > +#else
> > +#error "The CONFIG_SYS_MEM_RESERVE_SECURE must be defined when
> enabled PPA"
> > +#endif
> > +
> > +	/* Align PPA base address to 4K */
> > +	ppa_ram_addr = (ppa_ram_addr + 0xfff) & ~0xfff;
> > +	debug("fsl-ppa: PPA load address (0x%llx)\n", ppa_ram_addr);
> > +
> > +	ret = ppa_parse_firmware_fit_image(&raw_image_addr,
> &raw_image_size);
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	if (ppa_ram_size < raw_image_size) {
> > +		ret = -ENOSPC;
> > +		goto out;
> > +	}
> > +
> > +	/* TODO:
> > +	 * Check if the end addr of PPA has been extend the secure memory.
> > +	 */
> > +
> > +	ppa_copy_image("PPA firmware", (u64)raw_image_addr,
> > +			raw_image_size, ppa_ram_addr);
> > +
> > +	debug("fsl-ppa: PPA entry: 0x%llx\n", ppa_ram_addr);
> > +	*entry = ppa_ram_addr;
> > +
> > +	return 0;
> > +
> > +out:
> > +	printf("fsl-ppa: error (%d)\n", ret);
> > +	*entry = 0;
> > +
> > +	return ret;
> > +}
> > +
> > +static int ppa_init_entry(void *ppa_entry) {
> > +	int ret;
> > +	u32 *boot_loc_ptr_l, *boot_loc_ptr_h;
> > +
> > +#ifdef CONFIG_FSL_LSCH3
> > +	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
> > +	boot_loc_ptr_l = &gur->bootlocptrl;
> > +	boot_loc_ptr_h = &gur->bootlocptrh;
> > +#elif defined(CONFIG_FSL_LSCH2)
> > +	struct ccsr_scfg __iomem *scfg = (void *)(CONFIG_SYS_FSL_SCFG_ADDR);
> > +	boot_loc_ptr_l = &scfg->scratchrw[1];
> > +	boot_loc_ptr_h = &scfg->scratchrw[0]; #endif
> > +
> > +	debug("fsl-ppa: boot_loc_ptr_l = 0x%p, boot_loc_ptr_h =0x%p\n",
> > +			boot_loc_ptr_l, boot_loc_ptr_h);
> > +	ret = ppa_init(ppa_entry, boot_loc_ptr_l, boot_loc_ptr_h);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	debug("fsl-ppa: Return from PPA: current_el = %d\n", current_el());
> > +
> > +	/* The PE will be turned into EL2 when run out of PPA. */
> > +	if (current_el() != 2)
> > +		return -EACCES;
> > +
> > +	/*
> > +	 * First, set vector for EL2.
> > +	 */
> > +	c_runtime_cpu_setup();
> > +
> > +	/* Enable caches and MMU for EL2. */
> > +	enable_caches();
> > +
> > +	return 0;
> > +}
> > +
> > +bool sec_firmware_is_valid(void)
> > +{
> > +	void *ppa_addr;
> > +
> > +#ifdef CONFIG_SYS_LS_PPA_FW_IN_NOR
> > +	ppa_addr = (void *)CONFIG_SYS_LS_PPA_FW_ADDR; #else #error "No
> > +CONFIG_SYS_LS_PPA_FW_IN_xxx defined"
> > +#endif
> > +
> > +	return ppa_firmware_is_valid(ppa_addr); }
> > +
> > +#ifdef CONFIG_ARMV8_PSCI
> > +unsigned int sec_firmware_support_psci_version(void)
> > +{
> > +	unsigned int psci_ver = 0;
> > +
> > +	if (sec_firmware_is_valid())
> > +		psci_ver = ppa_support_psci_version();
> > +
> > +	return psci_ver;
> > +}
> > +#endif
> 
> 
> Zhiqiang,
> 
> I am not convinced this function can be used to prove ppa is up and running, as you
> used in your 4th patch in this set. See more comment there.
> 
> This function can return a psci version number based on the image. Is it possible to
> make a PSCI call to return the version of running image?

The psci version isn't based on the image but psci_version call, see my feedback for the
4th patch comments.

Thanks,
Zhiqiang

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[Zhiqiang Hou]

Hi York,

Thanks for your comments!

> -----Original Message-----
> From: York Sun [mailto:york.sun at nxp.com]
> Sent: 2016?6?8? 8:56
> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
> albert.u.boot at aribaud.net; scottwood at freescale.com;
> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
> prabhakar at freescale.com; bhupesh.sharma at freescale.com
> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly
> 
> On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > If the PSCI and PPA is ready, skip the fixup for spin-table and waking
> > secondary cores. If not, change SMP method to spin-table, and the
> > device node of PSCI will be removed.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> > V5:
> >  - Changed the checking if the PSCI feature is ready to read the psci version.
> >
> > V4:
> >  - Reordered this patch.
> >
> >  arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
> > arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36
> > +++++++++++++++++++++++++++++++++
> >  2 files changed, 50 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > index 672a453..eb566cd 100644
> > --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > @@ -23,6 +23,9 @@
> >  #ifdef CONFIG_FSL_ESDHC
> >  #include <fsl_esdhc.h>
> >  #endif
> > +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
> > +<asm/armv8/sec_firmware.h> #endif
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -618,6 +621,7 @@ int arch_early_init_r(void)  {  #ifdef CONFIG_MP
> >  	int rv = 1;
> > +	bool psci_support = false;
> >  #endif
> >
> >  #ifdef CONFIG_SYS_FSL_ERRATUM_A009635 @@ -625,9 +629,16 @@ int
> > arch_early_init_r(void)  #endif
> >
> >  #ifdef CONFIG_MP
> > -	rv = fsl_layerscape_wake_seconday_cores();
> > -	if (rv)
> > -		printf("Did not wake secondary cores\n");
> > +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) &&
> defined(CONFIG_ARMV8_PSCI)
> > +	/* Check the psci version to determine if the psci is supported */
> > +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
> > +			true : false;
> 
> Another comment, even if the function can be used to indicate if psci is available,
> do you have to cast it to (int)? I think this can be simplified as
> 	psci_support = sec_firmware_support_psci_version() > 0;

The type of this func return value is 'unsigned int', so the cast is necessary.

Thanks,
Zhiqiang

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[york sun]

On 06/11/2016 08:58 PM, Zhiqiang Hou wrote:
> Hi York,
>
> Thanks for your comments!
>
>> -----Original Message-----
>> From: York Sun [mailto:york.sun at nxp.com]
>> Sent: 2016?6?8? 8:56
>> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
>> albert.u.boot at aribaud.net; scottwood at freescale.com;
>> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
>> prabhakar at freescale.com; bhupesh.sharma at freescale.com
>> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly
>>
>> On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
>>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>>>
>>> If the PSCI and PPA is ready, skip the fixup for spin-table and waking
>>> secondary cores. If not, change SMP method to spin-table, and the
>>> device node of PSCI will be removed.
>>>
>>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>>> ---
>>> V5:
>>>   - Changed the checking if the PSCI feature is ready to read the psci version.
>>>
>>> V4:
>>>   - Reordered this patch.
>>>
>>>   arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
>>> arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36
>>> +++++++++++++++++++++++++++++++++
>>>   2 files changed, 50 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> index 672a453..eb566cd 100644
>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> @@ -23,6 +23,9 @@
>>>   #ifdef CONFIG_FSL_ESDHC
>>>   #include <fsl_esdhc.h>
>>>   #endif
>>> +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
>>> +<asm/armv8/sec_firmware.h> #endif
>>>
>>>   DECLARE_GLOBAL_DATA_PTR;
>>>
>>> @@ -618,6 +621,7 @@ int arch_early_init_r(void)  {  #ifdef CONFIG_MP
>>>   	int rv = 1;
>>> +	bool psci_support = false;
>>>   #endif
>>>
>>>   #ifdef CONFIG_SYS_FSL_ERRATUM_A009635 @@ -625,9 +629,16 @@ int
>>> arch_early_init_r(void)  #endif
>>>
>>>   #ifdef CONFIG_MP
>>> -	rv = fsl_layerscape_wake_seconday_cores();
>>> -	if (rv)
>>> -		printf("Did not wake secondary cores\n");
>>> +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) &&
>> defined(CONFIG_ARMV8_PSCI)
>>> +	/* Check the psci version to determine if the psci is supported */
>>> +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
>>> +			true : false;
>>
>> Another comment, even if the function can be used to indicate if psci is available,
>> do you have to cast it to (int)? I think this can be simplified as
>> 	psci_support = sec_firmware_support_psci_version() > 0;
>
> The type of this func return value is 'unsigned int', so the cast is necessary.

The return value of function sec_firmware_support_psci_version() may 
need some work. It has three results

Positive numbers mean success (presuming bit 31 is not used by major 
number. Need to check with PPA code)
Zero means image is not valid
Negative numbers means errors

If the error code doesn't include -EINVAL, you can return -EINVAL in 
case of an invalid image. Then you can have 
sec_firmware_support_psci_version() return int.

York

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[Scott Wood]

On 06/11/2016 10:58 PM, Zhiqiang Hou wrote:
> Hi York,
> 
> Thanks for your comments!
> 
>> -----Original Message-----
>> From: York Sun [mailto:york.sun at nxp.com]
>> Sent: 2016?6?8? 8:56
>> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
>> albert.u.boot at aribaud.net; scottwood at freescale.com;
>> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
>> prabhakar at freescale.com; bhupesh.sharma at freescale.com
>> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly
>>
>> On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
>>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>>>
>>> If the PSCI and PPA is ready, skip the fixup for spin-table and waking
>>> secondary cores. If not, change SMP method to spin-table, and the
>>> device node of PSCI will be removed.
>>>
>>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>>> ---
>>> V5:
>>>  - Changed the checking if the PSCI feature is ready to read the psci version.
>>>
>>> V4:
>>>  - Reordered this patch.
>>>
>>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
>>> arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36
>>> +++++++++++++++++++++++++++++++++
>>>  2 files changed, 50 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> index 672a453..eb566cd 100644
>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>> @@ -23,6 +23,9 @@
>>>  #ifdef CONFIG_FSL_ESDHC
>>>  #include <fsl_esdhc.h>
>>>  #endif
>>> +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
>>> +<asm/armv8/sec_firmware.h> #endif
>>>
>>>  DECLARE_GLOBAL_DATA_PTR;
>>>
>>> @@ -618,6 +621,7 @@ int arch_early_init_r(void)  {  #ifdef CONFIG_MP
>>>  	int rv = 1;
>>> +	bool psci_support = false;
>>>  #endif
>>>
>>>  #ifdef CONFIG_SYS_FSL_ERRATUM_A009635 @@ -625,9 +629,16 @@ int
>>> arch_early_init_r(void)  #endif
>>>
>>>  #ifdef CONFIG_MP
>>> -	rv = fsl_layerscape_wake_seconday_cores();
>>> -	if (rv)
>>> -		printf("Did not wake secondary cores\n");
>>> +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) &&
>> defined(CONFIG_ARMV8_PSCI)
>>> +	/* Check the psci version to determine if the psci is supported */
>>> +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
>>> +			true : false;
>>
>> Another comment, even if the function can be used to indicate if psci is available,
>> do you have to cast it to (int)? I think this can be simplified as
>> 	psci_support = sec_firmware_support_psci_version() > 0;
> 
> The type of this func return value is 'unsigned int', so the cast is necessary.

If it can return negative values then change the function's return
value.  If the value is not really negative but you just want to test
the upper bit, then do that explicitly.  In any case, please explain
what the format of this return value is supposed to be.

-Scott

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[Zhiqiang Hou]

Hi York,

Thanks for your comments!

> -----Original Message-----
> From: york sun
> Sent: 2016?6?12? 12:07
> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
> albert.u.boot at aribaud.net; scottwood at freescale.com;
> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
> prabhakar at freescale.com; bhupesh.sharma at freescale.com
> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly
> 
> On 06/11/2016 08:58 PM, Zhiqiang Hou wrote:
> > Hi York,
> >
> > Thanks for your comments!
> >
> >> -----Original Message-----
> >> From: York Sun [mailto:york.sun at nxp.com]
> >> Sent: 2016?6?8? 8:56
> >> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
> >> albert.u.boot at aribaud.net; scottwood at freescale.com;
> >> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
> >> prabhakar at freescale.com; bhupesh.sharma at freescale.com
> >> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method
> >> accordingly
> >>
> >> On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
> >>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >>>
> >>> If the PSCI and PPA is ready, skip the fixup for spin-table and
> >>> waking secondary cores. If not, change SMP method to spin-table, and
> >>> the device node of PSCI will be removed.
> >>>
> >>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >>> ---
> >>> V5:
> >>>   - Changed the checking if the PSCI feature is ready to read the psci version.
> >>>
> >>> V4:
> >>>   - Reordered this patch.
> >>>
> >>>   arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
> >>> arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36
> >>> +++++++++++++++++++++++++++++++++
> >>>   2 files changed, 50 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >>> index 672a453..eb566cd 100644
> >>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >>> @@ -23,6 +23,9 @@
> >>>   #ifdef CONFIG_FSL_ESDHC
> >>>   #include <fsl_esdhc.h>
> >>>   #endif
> >>> +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
> >>> +<asm/armv8/sec_firmware.h> #endif
> >>>
> >>>   DECLARE_GLOBAL_DATA_PTR;
> >>>
> >>> @@ -618,6 +621,7 @@ int arch_early_init_r(void)  {  #ifdef CONFIG_MP
> >>>   	int rv = 1;
> >>> +	bool psci_support = false;
> >>>   #endif
> >>>
> >>>   #ifdef CONFIG_SYS_FSL_ERRATUM_A009635 @@ -625,9 +629,16 @@ int
> >>> arch_early_init_r(void)  #endif
> >>>
> >>>   #ifdef CONFIG_MP
> >>> -	rv = fsl_layerscape_wake_seconday_cores();
> >>> -	if (rv)
> >>> -		printf("Did not wake secondary cores\n");
> >>> +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) &&
> >> defined(CONFIG_ARMV8_PSCI)
> >>> +	/* Check the psci version to determine if the psci is supported */
> >>> +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
> >>> +			true : false;
> >>
> >> Another comment, even if the function can be used to indicate if psci
> >> is available, do you have to cast it to (int)? I think this can be simplified as
> >> 	psci_support = sec_firmware_support_psci_version() > 0;
> >
> > The type of this func return value is 'unsigned int', so the cast is necessary.
> 
> The return value of function sec_firmware_support_psci_version() may need some
> work. It has three results
> 
> Positive numbers mean success (presuming bit 31 is not used by major number.
> Need to check with PPA code) Zero means image is not valid Negative numbers
> means errors
 
In PSCI spec v1.0, the bit 31 is used by major number, and the type of the return
value is uint, but there isn't any other description for the return value. I don't know
why the PPA isn't consistent with PSCI spec.

> If the error code doesn't include -EINVAL, you can return -EINVAL in case of an
> invalid image. Then you can have
> sec_firmware_support_psci_version() return int.
> 

Thanks,
Zhiqiang

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[Zhiqiang Hou]

Hi Scott,

Thanks for your comments!

> -----Original Message-----
> From: Scott Wood
> Sent: 2016?6?12? 12:10
> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; york sun <york.sun@nxp.com>; u-
> boot at lists.denx.de; albert.u.boot at aribaud.net; scottwood at freescale.com;
> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
> prabhakar at freescale.com; bhupesh.sharma at freescale.com
> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly
> 
> On 06/11/2016 10:58 PM, Zhiqiang Hou wrote:
> > Hi York,
> >
> > Thanks for your comments!
> >
> >> -----Original Message-----
> >> From: York Sun [mailto:york.sun at nxp.com]
> >> Sent: 2016?6?8? 8:56
> >> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
> >> albert.u.boot at aribaud.net; scottwood at freescale.com;
> >> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
> >> prabhakar at freescale.com; bhupesh.sharma at freescale.com
> >> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method
> >> accordingly
> >>
> >> On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
> >>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >>>
> >>> If the PSCI and PPA is ready, skip the fixup for spin-table and
> >>> waking secondary cores. If not, change SMP method to spin-table, and
> >>> the device node of PSCI will be removed.
> >>>
> >>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >>> ---
> >>> V5:
> >>>  - Changed the checking if the PSCI feature is ready to read the psci version.
> >>>
> >>> V4:
> >>>  - Reordered this patch.
> >>>
> >>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
> >>> arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36
> >>> +++++++++++++++++++++++++++++++++
> >>>  2 files changed, 50 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >>> index 672a453..eb566cd 100644
> >>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> >>> @@ -23,6 +23,9 @@
> >>>  #ifdef CONFIG_FSL_ESDHC
> >>>  #include <fsl_esdhc.h>
> >>>  #endif
> >>> +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
> >>> +<asm/armv8/sec_firmware.h> #endif
> >>>
> >>>  DECLARE_GLOBAL_DATA_PTR;
> >>>
> >>> @@ -618,6 +621,7 @@ int arch_early_init_r(void)  {  #ifdef CONFIG_MP
> >>>  	int rv = 1;
> >>> +	bool psci_support = false;
> >>>  #endif
> >>>
> >>>  #ifdef CONFIG_SYS_FSL_ERRATUM_A009635 @@ -625,9 +629,16 @@ int
> >>> arch_early_init_r(void)  #endif
> >>>
> >>>  #ifdef CONFIG_MP
> >>> -	rv = fsl_layerscape_wake_seconday_cores();
> >>> -	if (rv)
> >>> -		printf("Did not wake secondary cores\n");
> >>> +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) &&
> >> defined(CONFIG_ARMV8_PSCI)
> >>> +	/* Check the psci version to determine if the psci is supported */
> >>> +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
> >>> +			true : false;
> >>
> >> Another comment, even if the function can be used to indicate if psci
> >> is available, do you have to cast it to (int)? I think this can be simplified as
> >> 	psci_support = sec_firmware_support_psci_version() > 0;
> >
> > The type of this func return value is 'unsigned int', so the cast is necessary.
> 
> If it can return negative values then change the function's return value.  If the
> value is not really negative but you just want to test the upper bit, then do that
> explicitly.  In any case, please explain what the format of this return value is
> supposed to be.

Yes, will check the PSCI spec and PPA to fix it.

Thanks,
Zhiqiang

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[Zhiqiang Hou]

Hi York,

> -----Original Message-----
> From: Zhiqiang Hou
> Sent: 2016?6?12? 12:31
> To: york sun <york.sun@nxp.com>; u-boot at lists.denx.de;
> albert.u.boot at aribaud.net; scottwood at freescale.com;
> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
> prabhakar at freescale.com; bhupesh.sharma at freescale.com
> Subject: RE: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly
> 
> Hi York,
> 
> Thanks for your comments!
> 
> > -----Original Message-----
> > From: york sun
> > Sent: 2016?6?12? 12:07
> > To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
> > albert.u.boot at aribaud.net; scottwood at freescale.com;
> > Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
> > prabhakar at freescale.com; bhupesh.sharma at freescale.com
> > Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method
> > accordingly
> >
> > On 06/11/2016 08:58 PM, Zhiqiang Hou wrote:
> > > Hi York,
> > >
> > > Thanks for your comments!
> > >
> > >> -----Original Message-----
> > >> From: York Sun [mailto:york.sun at nxp.com]
> > >> Sent: 2016?6?8? 8:56
> > >> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
> > >> albert.u.boot at aribaud.net; scottwood at freescale.com;
> > >> Mingkai.hu at freescale.com; yorksun at freescale.com;
> > >> leoli at freescale.com; prabhakar at freescale.com;
> > >> bhupesh.sharma at freescale.com
> > >> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method
> > >> accordingly
> > >>
> > >> On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
> > >>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > >>>
> > >>> If the PSCI and PPA is ready, skip the fixup for spin-table and
> > >>> waking secondary cores. If not, change SMP method to spin-table,
> > >>> and the device node of PSCI will be removed.
> > >>>
> > >>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > >>> ---
> > >>> V5:
> > >>>   - Changed the checking if the PSCI feature is ready to read the psci version.
> > >>>
> > >>> V4:
> > >>>   - Reordered this patch.
> > >>>
> > >>>   arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
> > >>> arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36
> > >>> +++++++++++++++++++++++++++++++++
> > >>>   2 files changed, 50 insertions(+), 3 deletions(-)
> > >>>
> > >>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > >>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > >>> index 672a453..eb566cd 100644
> > >>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > >>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > >>> @@ -23,6 +23,9 @@
> > >>>   #ifdef CONFIG_FSL_ESDHC
> > >>>   #include <fsl_esdhc.h>
> > >>>   #endif
> > >>> +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
> > >>> +<asm/armv8/sec_firmware.h> #endif
> > >>>
> > >>>   DECLARE_GLOBAL_DATA_PTR;
> > >>>
> > >>> @@ -618,6 +621,7 @@ int arch_early_init_r(void)  {  #ifdef CONFIG_MP
> > >>>   	int rv = 1;
> > >>> +	bool psci_support = false;
> > >>>   #endif
> > >>>
> > >>>   #ifdef CONFIG_SYS_FSL_ERRATUM_A009635 @@ -625,9 +629,16 @@ int
> > >>> arch_early_init_r(void)  #endif
> > >>>
> > >>>   #ifdef CONFIG_MP
> > >>> -	rv = fsl_layerscape_wake_seconday_cores();
> > >>> -	if (rv)
> > >>> -		printf("Did not wake secondary cores\n");
> > >>> +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) &&
> > >> defined(CONFIG_ARMV8_PSCI)
> > >>> +	/* Check the psci version to determine if the psci is supported */
> > >>> +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
> > >>> +			true : false;
> > >>
> > >> Another comment, even if the function can be used to indicate if
> > >> psci is available, do you have to cast it to (int)? I think this can be simplified as
> > >> 	psci_support = sec_firmware_support_psci_version() > 0;
> > >
> > > The type of this func return value is 'unsigned int', so the cast is necessary.
> >
> > The return value of function sec_firmware_support_psci_version() may
> > need some work. It has three results
> >
> > Positive numbers mean success (presuming bit 31 is not used by major number.
> > Need to check with PPA code) Zero means image is not valid Negative
> > numbers means errors
> 
> In PSCI spec v1.0, the bit 31 is used by major number, and the type of the return
> value is uint, but there isn't any other description for the return value. I don't know
> why the PPA isn't consistent with PSCI spec.

I misunderstand your words, and the PPA is consistent with PSCI spec.
Will take your suggestion that presuming bit 31 isn't used by major number to handle
the return value.
 
> > If the error code doesn't include -EINVAL, you can return -EINVAL in
> > case of an invalid image. Then you can have
> > sec_firmware_support_psci_version() return int.
> >

Thanks,
Zhiqiang

[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[york sun]

On 06/13/2016 01:30 AM, Zhiqiang Hou wrote:
> Hi York,
>
>> -----Original Message-----
>> From: Zhiqiang Hou
>> Sent: 2016?6?12? 12:31
>> To: york sun <york.sun@nxp.com>; u-boot at lists.denx.de;
>> albert.u.boot at aribaud.net; scottwood at freescale.com;
>> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
>> prabhakar at freescale.com; bhupesh.sharma at freescale.com
>> Subject: RE: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly
>>
>> Hi York,
>>
>> Thanks for your comments!
>>
>>> -----Original Message-----
>>> From: york sun
>>> Sent: 2016?6?12? 12:07
>>> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
>>> albert.u.boot at aribaud.net; scottwood at freescale.com;
>>> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
>>> prabhakar at freescale.com; bhupesh.sharma at freescale.com
>>> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method
>>> accordingly
>>>
>>> On 06/11/2016 08:58 PM, Zhiqiang Hou wrote:
>>>> Hi York,
>>>>
>>>> Thanks for your comments!
>>>>
>>>>> -----Original Message-----
>>>>> From: York Sun [mailto:york.sun at nxp.com]
>>>>> Sent: 2016?6?8? 8:56
>>>>> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
>>>>> albert.u.boot at aribaud.net; scottwood at freescale.com;
>>>>> Mingkai.hu at freescale.com; yorksun at freescale.com;
>>>>> leoli at freescale.com; prabhakar at freescale.com;
>>>>> bhupesh.sharma at freescale.com
>>>>> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method
>>>>> accordingly
>>>>>
>>>>> On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
>>>>>> From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>>>>>>
>>>>>> If the PSCI and PPA is ready, skip the fixup for spin-table and
>>>>>> waking secondary cores. If not, change SMP method to spin-table,
>>>>>> and the device node of PSCI will be removed.
>>>>>>
>>>>>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>>>>>> ---
>>>>>> V5:
>>>>>>    - Changed the checking if the PSCI feature is ready to read the psci version.
>>>>>>
>>>>>> V4:
>>>>>>    - Reordered this patch.
>>>>>>
>>>>>>    arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
>>>>>> arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36
>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>    2 files changed, 50 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>>>> b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>>>> index 672a453..eb566cd 100644
>>>>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>>>>>> @@ -23,6 +23,9 @@
>>>>>>    #ifdef CONFIG_FSL_ESDHC
>>>>>>    #include <fsl_esdhc.h>
>>>>>>    #endif
>>>>>> +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
>>>>>> +<asm/armv8/sec_firmware.h> #endif
>>>>>>
>>>>>>    DECLARE_GLOBAL_DATA_PTR;
>>>>>>
>>>>>> @@ -618,6 +621,7 @@ int arch_early_init_r(void)  {  #ifdef CONFIG_MP
>>>>>>    	int rv = 1;
>>>>>> +	bool psci_support = false;
>>>>>>    #endif
>>>>>>
>>>>>>    #ifdef CONFIG_SYS_FSL_ERRATUM_A009635 @@ -625,9 +629,16 @@ int
>>>>>> arch_early_init_r(void)  #endif
>>>>>>
>>>>>>    #ifdef CONFIG_MP
>>>>>> -	rv = fsl_layerscape_wake_seconday_cores();
>>>>>> -	if (rv)
>>>>>> -		printf("Did not wake secondary cores\n");
>>>>>> +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) &&
>>>>> defined(CONFIG_ARMV8_PSCI)
>>>>>> +	/* Check the psci version to determine if the psci is supported */
>>>>>> +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
>>>>>> +			true : false;
>>>>>
>>>>> Another comment, even if the function can be used to indicate if
>>>>> psci is available, do you have to cast it to (int)? I think this can be simplified as
>>>>> 	psci_support = sec_firmware_support_psci_version() > 0;
>>>>
>>>> The type of this func return value is 'unsigned int', so the cast is necessary.
>>>
>>> The return value of function sec_firmware_support_psci_version() may
>>> need some work. It has three results
>>>
>>> Positive numbers mean success (presuming bit 31 is not used by major number.
>>> Need to check with PPA code) Zero means image is not valid Negative
>>> numbers means errors
>>
>> In PSCI spec v1.0, the bit 31 is used by major number, and the type of the return
>> value is uint, but there isn't any other description for the return value. I don't know
>> why the PPA isn't consistent with PSCI spec.
>
> I misunderstand your words, and the PPA is consistent with PSCI spec.
> Will take your suggestion that presuming bit 31 isn't used by major number to handle
> the return value.

Zhiqiang,

I didn't suggest to discard bit 31. If PSCI uses bit 31, you can't 
presume it otherwise. In this case, you cannot return negative value 
directly. You may process the version number to fit in an int variable, 
or find another way to indicate an error.

York