From f240728b76c05ac507189a37375b120379eda650 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Wed, 10 Jul 2019 17:27:17 +0100 Subject: [PATCH 1/2] qemu: Move and generalise FDT PSCI fixup The QEMU platform port scans its device tree to advertise PSCI as the CPU enable method. It does this by scanning *every* node in the DT and check whether its compatible string starts with "arm,cortex-a". Then it sets the enable-method to PSCI, if it doesn't already have one. Other platforms might want to use this functionality as well, so let's move it out of the QEMU platform directory and make it more robust by fixing some shortcomings: - A compatible string starting with a certain prefix is not a good way to find the CPU nodes. For instance a "arm,cortex-a72-pmu" node will match as well and is in turn favoured with an enable-method. - If the DT already has an enable-method, we won't change this to PSCI. Those two issues will for instance fail on the Raspberry Pi 4 DT. To fix those problems, we adjust the scanning method: The DT spec says that all CPU nodes are subnodes of the mandatory /cpus node, which is a subnode of the root node. Also each CPU node has to have a device_type = "cpu" property. So we find the /cpus node, then scan for a subnode with the proper device_type, forcing the enable-method to "psci". We have to restart this search after a property has been patched, as the node offsets might have changed meanwhile. This allows this routine to be reused for the Raspberry Pi 4 later. Change-Id: I00cae16cc923d9f8bb96a9b2a2933b9a79b06139 Signed-off-by: Andre Przywara --- common/fdt_fixup.c | 126 +++++++++++++++++++++++++++++++++++++ include/common/fdt_fixup.h | 13 ++++ plat/qemu/dt.c | 99 ----------------------------- plat/qemu/platform.mk | 2 +- plat/qemu/qemu_bl2_setup.c | 3 +- plat/qemu/qemu_private.h | 3 - 6 files changed, 142 insertions(+), 104 deletions(-) create mode 100644 common/fdt_fixup.c create mode 100644 include/common/fdt_fixup.h delete mode 100644 plat/qemu/dt.c diff --git a/common/fdt_fixup.c b/common/fdt_fixup.c new file mode 100644 index 000000000..0ae0050c0 --- /dev/null +++ b/common/fdt_fixup.c @@ -0,0 +1,126 @@ +/* + * Copyright (c) 2016-2019, ARM Limited and Contributors. All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/* + * Contains generic routines to fix up the device tree blob passed on to + * payloads like BL32 and BL33 (and further down the boot chain). + * This allows to easily add PSCI nodes, when the original DT does not have + * it or advertises another method. + */ + +#include + +#include + +#include +#include +#include + +#include + +static int append_psci_compatible(void *fdt, int offs, const char *str) +{ + return fdt_appendprop(fdt, offs, "compatible", str, strlen(str) + 1); +} + +int dt_add_psci_node(void *fdt) +{ + int offs; + + if (fdt_path_offset(fdt, "/psci") >= 0) { + WARN("PSCI Device Tree node already exists!\n"); + return 0; + } + + offs = fdt_path_offset(fdt, "/"); + if (offs < 0) + return -1; + offs = fdt_add_subnode(fdt, offs, "psci"); + if (offs < 0) + return -1; + if (append_psci_compatible(fdt, offs, "arm,psci-1.0")) + return -1; + if (append_psci_compatible(fdt, offs, "arm,psci-0.2")) + return -1; + if (append_psci_compatible(fdt, offs, "arm,psci")) + return -1; + if (fdt_setprop_string(fdt, offs, "method", "smc")) + return -1; + if (fdt_setprop_u32(fdt, offs, "cpu_suspend", PSCI_CPU_SUSPEND_AARCH64)) + return -1; + if (fdt_setprop_u32(fdt, offs, "cpu_off", PSCI_CPU_OFF)) + return -1; + if (fdt_setprop_u32(fdt, offs, "cpu_on", PSCI_CPU_ON_AARCH64)) + return -1; + if (fdt_setprop_u32(fdt, offs, "sys_poweroff", PSCI_SYSTEM_OFF)) + return -1; + if (fdt_setprop_u32(fdt, offs, "sys_reset", PSCI_SYSTEM_RESET)) + return -1; + return 0; +} + +/* + * Find the first subnode that has a "device_type" property with the value + * "cpu" and which's enable-method is not "psci" (yet). + * Returns 0 if no such subnode is found, so all have already been patched + * or none have to be patched in the first place. + * Returns 1 if *one* such subnode has been found and successfully changed + * to "psci". + * Returns -1 on error. + * + * Call in a loop until it returns 0. Recalculate the node offset after + * it has returned 1. + */ +static int dt_update_one_cpu_node(void *fdt, int offset) +{ + int offs; + + /* Iterate over all subnodes to find those with device_type = "cpu". */ + for (offs = fdt_first_subnode(fdt, offset); offs >= 0; + offs = fdt_next_subnode(fdt, offs)) { + const char *prop; + int len; + + prop = fdt_getprop(fdt, offs, "device_type", &len); + if (!prop) + continue; + if (memcmp(prop, "cpu", 4) != 0 || len != 4) + continue; + + /* Ignore any nodes which already use "psci". */ + prop = fdt_getprop(fdt, offs, "enable-method", &len); + if (prop && memcmp(prop, "psci", 5) == 0 && len == 5) + continue; + + if (fdt_setprop_string(fdt, offs, "enable-method", "psci")) + return -1; + /* + * Subnode found and patched. + * Restart to accommodate potentially changed offsets. + */ + return 1; + } + + if (offs == -FDT_ERR_NOTFOUND) + return 0; + + return offs; +} + +int dt_add_psci_cpu_enable_methods(void *fdt) +{ + int offs, ret; + + do { + offs = fdt_path_offset(fdt, "/cpus"); + if (offs < 0) + return offs; + + ret = dt_update_one_cpu_node(fdt, offs); + } while (ret > 0); + + return ret; +} diff --git a/include/common/fdt_fixup.h b/include/common/fdt_fixup.h new file mode 100644 index 000000000..bb05bf5d0 --- /dev/null +++ b/include/common/fdt_fixup.h @@ -0,0 +1,13 @@ +/* + * Copyright (c) 2019, ARM Limited and Contributors. All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#ifndef FDT_FIXUP_H +#define FDT_FIXUP_H + +int dt_add_psci_node(void *fdt); +int dt_add_psci_cpu_enable_methods(void *fdt); + +#endif /* FDT_FIXUP_H */ diff --git a/plat/qemu/dt.c b/plat/qemu/dt.c deleted file mode 100644 index b1cd368cb..000000000 --- a/plat/qemu/dt.c +++ /dev/null @@ -1,99 +0,0 @@ -/* - * Copyright (c) 2016, ARM Limited and Contributors. All rights reserved. - * - * SPDX-License-Identifier: BSD-3-Clause - */ - -#include - -#include - -#include -#include -#include - -#include "qemu_private.h" - -static int append_psci_compatible(void *fdt, int offs, const char *str) -{ - return fdt_appendprop(fdt, offs, "compatible", str, strlen(str) + 1); -} - -int dt_add_psci_node(void *fdt) -{ - int offs; - - if (fdt_path_offset(fdt, "/psci") >= 0) { - WARN("PSCI Device Tree node already exists!\n"); - return 0; - } - - offs = fdt_path_offset(fdt, "/"); - if (offs < 0) - return -1; - offs = fdt_add_subnode(fdt, offs, "psci"); - if (offs < 0) - return -1; - if (append_psci_compatible(fdt, offs, "arm,psci-1.0")) - return -1; - if (append_psci_compatible(fdt, offs, "arm,psci-0.2")) - return -1; - if (append_psci_compatible(fdt, offs, "arm,psci")) - return -1; - if (fdt_setprop_string(fdt, offs, "method", "smc")) - return -1; - if (fdt_setprop_u32(fdt, offs, "cpu_suspend", PSCI_CPU_SUSPEND_AARCH64)) - return -1; - if (fdt_setprop_u32(fdt, offs, "cpu_off", PSCI_CPU_OFF)) - return -1; - if (fdt_setprop_u32(fdt, offs, "cpu_on", PSCI_CPU_ON_AARCH64)) - return -1; - if (fdt_setprop_u32(fdt, offs, "sys_poweroff", PSCI_SYSTEM_OFF)) - return -1; - if (fdt_setprop_u32(fdt, offs, "sys_reset", PSCI_SYSTEM_RESET)) - return -1; - return 0; -} - -static int check_node_compat_prefix(void *fdt, int offs, const char *prefix) -{ - const size_t prefix_len = strlen(prefix); - size_t l; - int plen; - const char *prop; - - prop = fdt_getprop(fdt, offs, "compatible", &plen); - if (!prop) - return -1; - - while (plen > 0) { - if (memcmp(prop, prefix, prefix_len) == 0) - return 0; /* match */ - - l = strlen(prop) + 1; - prop += l; - plen -= l; - } - - return -1; -} - -int dt_add_psci_cpu_enable_methods(void *fdt) -{ - int offs = 0; - - while (1) { - offs = fdt_next_node(fdt, offs, NULL); - if (offs < 0) - break; - if (fdt_getprop(fdt, offs, "enable-method", NULL)) - continue; /* already set */ - if (check_node_compat_prefix(fdt, offs, "arm,cortex-a")) - continue; /* no compatible */ - if (fdt_setprop_string(fdt, offs, "enable-method", "psci")) - return -1; - /* Need to restart scanning as offsets may have changed */ - offs = 0; - } - return 0; -} diff --git a/plat/qemu/platform.mk b/plat/qemu/platform.mk index 6b9749c79..bc4a21bc0 100644 --- a/plat/qemu/platform.mk +++ b/plat/qemu/platform.mk @@ -114,7 +114,7 @@ BL2_SOURCES += drivers/io/io_semihosting.c \ plat/qemu/qemu_io_storage.c \ plat/qemu/${ARCH}/plat_helpers.S \ plat/qemu/qemu_bl2_setup.c \ - plat/qemu/dt.c \ + common/fdt_fixup.c \ plat/qemu/qemu_bl2_mem_params_desc.c \ plat/qemu/qemu_image_load.c \ common/desc_image_load.c diff --git a/plat/qemu/qemu_bl2_setup.c b/plat/qemu/qemu_bl2_setup.c index 4c97c8dd6..166d2454e 100644 --- a/plat/qemu/qemu_bl2_setup.c +++ b/plat/qemu/qemu_bl2_setup.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015-2018, ARM Limited and Contributors. All rights reserved. + * Copyright (c) 2015-2019, ARM Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include diff --git a/plat/qemu/qemu_private.h b/plat/qemu/qemu_private.h index 46b1ca1e9..71ea4de10 100644 --- a/plat/qemu/qemu_private.h +++ b/plat/qemu/qemu_private.h @@ -28,9 +28,6 @@ void qemu_configure_mmu_el3(unsigned long total_base, unsigned long total_size, void plat_qemu_io_setup(void); unsigned int plat_qemu_calc_core_pos(u_register_t mpidr); -int dt_add_psci_node(void *fdt); -int dt_add_psci_cpu_enable_methods(void *fdt); - void qemu_console_init(void); void plat_qemu_gic_init(void); From 3ef45dda88c83413c2c554212956d7966fab2807 Mon Sep 17 00:00:00 2001 From: Andre Przywara Date: Mon, 15 Jul 2019 09:00:23 +0100 Subject: [PATCH 2/2] Add fdt_add_reserved_memory() helper function If a firmware component like TF-A reserves special memory regions for its own or secure payload services, it should announce the location and size of those regions to the non-secure world. This will avoid disappointment when some rich OS tries to acccess this memory, which will likely end in a crash. The traditional way of advertising reserved memory using device tree is using the special memreserve feature of the device tree blob (DTB). However by definition those regions mentioned there do not prevent the rich OS to map this memory, which may lead to speculative accesses to this memory and hence spurious bus errors. A safer way of carving out memory is to use the /reserved-memory node as part of the normal DT structure. Besides being easier to setup, this also defines an explicit "no-map" property to signify the secure-only nature of certain memory regions, which avoids the rich OS to accidentally step on it. Add a helper function to allow platform ports to easily add a region. Change-Id: I2b92676cf48fd3bdacda05b5c6b1c7952ebed68c Signed-off-by: Andre Przywara --- common/fdt_fixup.c | 30 ++++++++++++++++++++++++++++++ include/common/fdt_fixup.h | 2 ++ 2 files changed, 32 insertions(+) diff --git a/common/fdt_fixup.c b/common/fdt_fixup.c index 0ae0050c0..8843404df 100644 --- a/common/fdt_fixup.c +++ b/common/fdt_fixup.c @@ -9,6 +9,9 @@ * payloads like BL32 and BL33 (and further down the boot chain). * This allows to easily add PSCI nodes, when the original DT does not have * it or advertises another method. + * Also it supports to add reserved memory nodes to describe memory that + * is used by the secure world, so that non-secure software avoids using + * that. */ #include @@ -124,3 +127,30 @@ int dt_add_psci_cpu_enable_methods(void *fdt) return ret; } + +#define HIGH_BITS(x) ((sizeof(x) > 4) ? ((x) >> 32) : (typeof(x))0) + +int fdt_add_reserved_memory(void *dtb, const char *node_name, + uintptr_t base, size_t size) +{ + int offs = fdt_path_offset(dtb, "/reserved-memory"); + uint32_t addresses[3]; + + if (offs < 0) { /* create if not existing yet */ + offs = fdt_add_subnode(dtb, 0, "reserved-memory"); + if (offs < 0) + return offs; + fdt_setprop_u32(dtb, offs, "#address-cells", 2); + fdt_setprop_u32(dtb, offs, "#size-cells", 1); + fdt_setprop(dtb, offs, "ranges", NULL, 0); + } + + addresses[0] = cpu_to_fdt32(HIGH_BITS(base)); + addresses[1] = cpu_to_fdt32(base & 0xffffffff); + addresses[2] = cpu_to_fdt32(size & 0xffffffff); + offs = fdt_add_subnode(dtb, offs, node_name); + fdt_setprop(dtb, offs, "no-map", NULL, 0); + fdt_setprop(dtb, offs, "reg", addresses, 12); + + return 0; +} diff --git a/include/common/fdt_fixup.h b/include/common/fdt_fixup.h index bb05bf5d0..0248de9cf 100644 --- a/include/common/fdt_fixup.h +++ b/include/common/fdt_fixup.h @@ -9,5 +9,7 @@ int dt_add_psci_node(void *fdt); int dt_add_psci_cpu_enable_methods(void *fdt); +int fdt_add_reserved_memory(void *dtb, const char *node_name, + uintptr_t base, size_t size); #endif /* FDT_FIXUP_H */