From 27c0f2b0197070c8e94bdf28686d4c63b959dea8 Mon Sep 17 00:00:00 2001 From: Rajan Vaja Date: Tue, 13 Nov 2018 23:46:49 -0800 Subject: [PATCH 1/7] clk: zynqmp: handle fixed factor param query error Return proper error code in case query for fixed factor parameter fails. This also fixes build warning for set but not used variable 'ret'. Signed-off-by: Rajan Vaja Fixes: 3fde0e16d016 ("drivers: clk: Add ZynqMP clock driver") Signed-off-by: Stephen Boyd --- drivers/clk/zynqmp/clkc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c index 9d7d297f0ea8..297f16a20bfc 100644 --- a/drivers/clk/zynqmp/clkc.c +++ b/drivers/clk/zynqmp/clkc.c @@ -279,6 +279,9 @@ struct clk_hw *zynqmp_clk_register_fixed_factor(const char *name, u32 clk_id, qdata.arg1 = clk_id; ret = eemi_ops->query_data(qdata, ret_payload); + if (ret) + return ERR_PTR(ret); + mult = ret_payload[1]; div = ret_payload[2]; From 48d7f160b10711f014bf07b574c73452646c9fdd Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Mon, 5 Nov 2018 11:40:10 -0800 Subject: [PATCH 2/7] dt-bindings: clk: Introduce 'protected-clocks' property Add a generic clk property for clks which are not intended to be used by the OS due to security restrictions put in place by firmware. For example, on some Qualcomm firmwares reading or writing certain clk registers causes the entire system to reboot, but on other firmwares reading and writing those same registers is required to make devices like QSPI work. Rather than adding one-off properties each time a new set of clks appears to be protected, let's add a generic clk property to describe any set of clks that shouldn't be touched by the OS. This way we never need to register the clks or use them in certain firmware configurations. Cc: Rob Herring Cc: Bjorn Andersson Cc: Taniya Das Signed-off-by: Stephen Boyd Reviewed-by: Bjorn Andersson Reviewed-by: Rob Herring Signed-off-by: Stephen Boyd --- .../devicetree/bindings/clock/clock-bindings.txt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt index 2ec489eebe72..b646bbcf7f92 100644 --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt @@ -168,3 +168,19 @@ a shared clock is forbidden. Configuration of common clocks, which affect multiple consumer devices can be similarly specified in the clock provider node. + +==Protected clocks== + +Some platforms or firmwares may not fully expose all the clocks to the OS, such +as in situations where those clks are used by drivers running in ARM secure +execution levels. Such a configuration can be specified in device tree with the +protected-clocks property in the form of a clock specifier list. This property should +only be specified in the node that is providing the clocks being protected: + + clock-controller@a000f000 { + compatible = "vendor,clk95; + reg = <0xa000f000 0x1000> + #clocks-cells = <1>; + ... + protected-clocks = , ; + }; From b181b3b801da8893c8eb706e448dd5111b02de60 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Mon, 5 Nov 2018 11:40:11 -0800 Subject: [PATCH 3/7] clk: qcom: Support 'protected-clocks' property Certain firmware configurations "protect" clks and cause the entire system to reboot when a non-secure OS such as Linux tries to read or write protected clk registers. But other firmware configurations allow reading or writing the same registers, and they may actually require that the OS use the otherwise locked down clks. Support the 'protected-clocks' property by never registering these protected clks with the common clk framework. This way, when firmware is protecting these clks we won't have the chance to ever read or write these registers and take down the entire system. Cc: Taniya Das Cc: Bjorn Andersson Signed-off-by: Stephen Boyd Reviewed-by: Bjorn Andersson Signed-off-by: Stephen Boyd --- drivers/clk/qcom/common.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c index db9b2471ac40..0a48ed56833b 100644 --- a/drivers/clk/qcom/common.c +++ b/drivers/clk/qcom/common.c @@ -191,6 +191,22 @@ int qcom_cc_register_sleep_clk(struct device *dev) } EXPORT_SYMBOL_GPL(qcom_cc_register_sleep_clk); +/* Drop 'protected-clocks' from the list of clocks to register */ +static void qcom_cc_drop_protected(struct device *dev, struct qcom_cc *cc) +{ + struct device_node *np = dev->of_node; + struct property *prop; + const __be32 *p; + u32 i; + + of_property_for_each_u32(np, "protected-clocks", prop, p, i) { + if (i >= cc->num_rclks) + continue; + + cc->rclks[i] = NULL; + } +} + static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec, void *data) { @@ -251,6 +267,8 @@ int qcom_cc_really_probe(struct platform_device *pdev, cc->rclks = rclks; cc->num_rclks = num_clks; + qcom_cc_drop_protected(dev, cc); + for (i = 0; i < num_clks; i++) { if (!rclks[i]) continue; From d206e6b7ea3fdc8ec8f6be9a2ecfe58142b49e37 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Mon, 5 Nov 2018 21:50:13 -0800 Subject: [PATCH 4/7] arm64: dts: qcom: sdm845-mtp: Mark protected gcc clocks As of v4.20-rc1 probing the GCC driver on a SDM845 device with the standard security implementation causes an access violation and an immediate system restart. Use the protected-clocks property to mark the offending clocks protected for the MTP, in order to allow it to boot. Cc: Stephen Boyd Signed-off-by: Bjorn Andersson Acked-by: Andy Gross Signed-off-by: Stephen Boyd --- arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts index eedfaf8922e2..ef2d059c2db1 100644 --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts @@ -343,6 +343,12 @@ }; }; +&gcc { + protected-clocks = , + , + ; +}; + &i2c10 { status = "okay"; clock-frequency = <400000>; From d9f5b7f5dd0fa74a89de5a7ac1e26366f211ccee Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 3 Dec 2018 17:50:55 +0300 Subject: [PATCH 5/7] clk: mvebu: Off by one bugs in cp110_of_clk_get() These > comparisons should be >= to prevent reading beyond the end of of the clk_data->hws[] buffer. The clk_data->hws[] array is allocated in cp110_syscon_common_probe() when we do: cp110_clk_data = devm_kzalloc(dev, sizeof(*cp110_clk_data) + sizeof(struct clk_hw *) * CP110_CLK_NUM, GFP_KERNEL); As you can see, it has CP110_CLK_NUM elements which is equivalent to CP110_MAX_CORE_CLOCKS + CP110_MAX_GATABLE_CLOCKS. Fixes: d3da3eaef7f4 ("clk: mvebu: new driver for Armada CP110 system controller") Signed-off-by: Dan Carpenter Signed-off-by: Stephen Boyd --- drivers/clk/mvebu/cp110-system-controller.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clk/mvebu/cp110-system-controller.c b/drivers/clk/mvebu/cp110-system-controller.c index 9781b1bf5998..9235a331b588 100644 --- a/drivers/clk/mvebu/cp110-system-controller.c +++ b/drivers/clk/mvebu/cp110-system-controller.c @@ -200,11 +200,11 @@ static struct clk_hw *cp110_of_clk_get(struct of_phandle_args *clkspec, unsigned int idx = clkspec->args[1]; if (type == CP110_CLK_TYPE_CORE) { - if (idx > CP110_MAX_CORE_CLOCKS) + if (idx >= CP110_MAX_CORE_CLOCKS) return ERR_PTR(-EINVAL); return clk_data->hws[idx]; } else if (type == CP110_CLK_TYPE_GATABLE) { - if (idx > CP110_MAX_GATABLE_CLOCKS) + if (idx >= CP110_MAX_GATABLE_CLOCKS) return ERR_PTR(-EINVAL); return clk_data->hws[CP110_MAX_CORE_CLOCKS + idx]; } From 2e85c57493e391b93445c1e0d530b36b95becc64 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 3 Dec 2018 17:51:43 +0300 Subject: [PATCH 6/7] clk: mmp: Off by one in mmp_clk_add() The > comparison should be >= or we write one element beyond the end of the unit->clk_table[] array. (The unit->clk_table[] array is allocated in the mmp_clk_init() function and it has unit->nr_clks elements). Fixes: 4661fda10f8b ("clk: mmp: add basic support functions for DT support") Signed-off-by: Dan Carpenter Signed-off-by: Stephen Boyd --- drivers/clk/mmp/clk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/mmp/clk.c b/drivers/clk/mmp/clk.c index ad8d483a35cd..ca7d37e2c7be 100644 --- a/drivers/clk/mmp/clk.c +++ b/drivers/clk/mmp/clk.c @@ -183,7 +183,7 @@ void mmp_clk_add(struct mmp_clk_unit *unit, unsigned int id, pr_err("CLK %d has invalid pointer %p\n", id, clk); return; } - if (id > unit->nr_clks) { + if (id >= unit->nr_clks) { pr_err("CLK %d is invalid\n", id); return; } From 9a43be9cedd516f188e6333d3b43402386723eff Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 3 Dec 2018 17:52:01 +0300 Subject: [PATCH 7/7] clk: zynqmp: Off by one in zynqmp_is_valid_clock() The > comparison should be >= to prevent reading beyond the end of the clock[] array. (The clock[] array is allocated in zynqmp_clk_setup() and has clock_max_idx elements.) Fixes: 3fde0e16d016 ("drivers: clk: Add ZynqMP clock driver") Signed-off-by: Dan Carpenter Signed-off-by: Stephen Boyd --- drivers/clk/zynqmp/clkc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c index 297f16a20bfc..f65cc0ff76ab 100644 --- a/drivers/clk/zynqmp/clkc.c +++ b/drivers/clk/zynqmp/clkc.c @@ -128,7 +128,7 @@ static const struct zynqmp_eemi_ops *eemi_ops; */ static inline int zynqmp_is_valid_clock(u32 clk_id) { - if (clk_id > clock_max_idx) + if (clk_id >= clock_max_idx) return -ENODEV; return clock[clk_id].valid;