* [PATCH v2 1/7] dt-bindings: bus: Add firewall bindings
2020-01-28 15:37 [PATCH v2 0/7] Introduce bus firewall controller framework Benjamin Gaignard
@ 2020-01-28 15:38 ` Benjamin Gaignard
2020-01-28 15:38 ` [PATCH v2 2/7] bus: Introduce firewall controller framework Benjamin Gaignard
` (6 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Benjamin Gaignard @ 2020-01-28 15:38 UTC (permalink / raw)
To: broonie, robh, arnd, shawnguo, s.hauer, fabio.estevam,
sudeep.holla, lkml
Cc: devicetree, Benjamin Gaignard, loic.pallardy, gregkh,
linux-kernel, linux-imx, kernel, stefano.stabellini, system-dt,
linux-arm-kernel
Add schemas for firewall consumer and provider.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
version 2:
- describe bindings in yaml files
.../bindings/bus/firewall/firewall-consumer.yaml | 25 ++++++++++++++++++++++
.../bindings/bus/firewall/firewall-provider.yaml | 18 ++++++++++++++++
2 files changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/bus/firewall/firewall-consumer.yaml
create mode 100644 Documentation/devicetree/bindings/bus/firewall/firewall-provider.yaml
diff --git a/Documentation/devicetree/bindings/bus/firewall/firewall-consumer.yaml b/Documentation/devicetree/bindings/bus/firewall/firewall-consumer.yaml
new file mode 100644
index 000000000000..ea7963c600f7
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/firewall/firewall-consumer.yaml
@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/firewall/firewall-consumer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common Bus Firewall consumer binding
+
+maintainers:
+ - Benjamin Gaignard <benjamin.gaignard@st.com>
+
+# always select the core schema
+select: true
+
+properties:
+ firewall-0: true
+
+ firewall-names: true
+
+patternProperties:
+ "firewall-[0-9]":
+ $ref: "/schemas/types.yaml#/definitions/phandle-array"
+
+dependencies:
+ firewall-names: [ firewall-0 ]
diff --git a/Documentation/devicetree/bindings/bus/firewall/firewall-provider.yaml b/Documentation/devicetree/bindings/bus/firewall/firewall-provider.yaml
new file mode 100644
index 000000000000..0f9a38b63fbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/firewall/firewall-provider.yaml
@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/firewall/firewall-provider.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common Bus Firewall provider binding
+
+maintainers:
+ - Benjamin Gaignard <benjamin.gaignard@st.com>
+
+properties:
+ '#firewall-cells':
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Number of cells in a bus firewall specifier
+
+required:
+ - '#firewall-cells'
--
2.15.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-28 15:37 [PATCH v2 0/7] Introduce bus firewall controller framework Benjamin Gaignard
2020-01-28 15:38 ` [PATCH v2 1/7] dt-bindings: bus: Add firewall bindings Benjamin Gaignard
@ 2020-01-28 15:38 ` Benjamin Gaignard
2020-01-28 15:52 ` Greg KH
2020-01-28 15:38 ` [PATCH v2 3/7] base: Add calls to firewall controller Benjamin Gaignard
` (5 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Benjamin Gaignard @ 2020-01-28 15:38 UTC (permalink / raw)
To: broonie, robh, arnd, shawnguo, s.hauer, fabio.estevam,
sudeep.holla, lkml
Cc: devicetree, Benjamin Gaignard, loic.pallardy, gregkh,
linux-kernel, linux-imx, kernel, stefano.stabellini, system-dt,
linux-arm-kernel
The goal of this framework is to offer an interface for the
hardware blocks controlling bus accesses rights.
Bus firewall controllers are typically used to control if a
hardware block can perform read or write operations on bus.
Smarter firewall controllers could be able to define accesses
rights per hardware blocks to control where they can read
or write.
Firewall controller configurations are provided in device node,
parsed by the framework and send to the driver to apply them.
Each controller may need different number and type of inputs
to configure the firewall so device-tree properties size have to
be define by using "#firewall-cells".
Firewall configurations properties have to be named "firewall-X"
on device node.
"firewall-names" keyword can also be used to give a name to
a specific configuration.
Example of device-tree:
ctrl0: firewall@0 {
#firewall-cells = <2>;
};
foo: foo@0 {
firewall-names = "default", "setting1";
firewall-0 = <&ctrl0 1 2>;
firewall-1 = <&ctrl0 3 4>;
};
Configurations could be applied with functions like
firewall_set_config_by_index() or firewall_set_config_by_name().
firewall_set_default_config() function will apply the
configuration named "default" (if existing) or the configuration
with index 0 (i.e. firewall-0).
Drivers could register/unregister themselves be calling
firewall_register/firewall_unregister functions.
When a configuration has to be applied the driver callback,
provided in the ops at registration time, set_config is called
by the framework.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
version 2:
- rename the framework "firewall"
- rebased on top of v5.5
drivers/bus/Kconfig | 2 +
drivers/bus/Makefile | 2 +
drivers/bus/firewall/Kconfig | 7 ++
drivers/bus/firewall/Makefile | 1 +
drivers/bus/firewall/firewall.c | 264 ++++++++++++++++++++++++++++++++++++++++
include/linux/firewall.h | 70 +++++++++++
6 files changed, 346 insertions(+)
create mode 100644 drivers/bus/firewall/Kconfig
create mode 100644 drivers/bus/firewall/Makefile
create mode 100644 drivers/bus/firewall/firewall.c
create mode 100644 include/linux/firewall.h
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 50200d1c06ea..d3f636c64e1c 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -203,4 +203,6 @@ config DA8XX_MSTPRI
source "drivers/bus/fsl-mc/Kconfig"
+source "drivers/bus/firewall/Kconfig"
+
endmenu
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index 1320bcf9fa9d..278c27fd57cd 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -34,3 +34,5 @@ obj-$(CONFIG_UNIPHIER_SYSTEM_BUS) += uniphier-system-bus.o
obj-$(CONFIG_VEXPRESS_CONFIG) += vexpress-config.o
obj-$(CONFIG_DA8XX_MSTPRI) += da8xx-mstpri.o
+
+obj-$(CONFIG_FIREWALL_CONTROLLERS) += firewall/
diff --git a/drivers/bus/firewall/Kconfig b/drivers/bus/firewall/Kconfig
new file mode 100644
index 000000000000..893bacb955f5
--- /dev/null
+++ b/drivers/bus/firewall/Kconfig
@@ -0,0 +1,7 @@
+menu "Bus Firewall Controllers"
+
+config FIREWALL_CONTROLLERS
+ bool "Support of bus firewall controllers"
+ depends on OF
+
+endmenu
diff --git a/drivers/bus/firewall/Makefile b/drivers/bus/firewall/Makefile
new file mode 100644
index 000000000000..eb6b978d6450
--- /dev/null
+++ b/drivers/bus/firewall/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_FIREWALL_CONTROLLERS) += firewall.o
diff --git a/drivers/bus/firewall/firewall.c b/drivers/bus/firewall/firewall.c
new file mode 100644
index 000000000000..765105d29075
--- /dev/null
+++ b/drivers/bus/firewall/firewall.c
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
+ */
+
+#include <linux/device.h>
+#include <linux/firewall.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+
+/* Mutex taken to protect firewall_list */
+static DEFINE_MUTEX(firewall_list_mutex);
+
+/* Global list of firewall control devices */
+static LIST_HEAD(firewall_list);
+
+struct firewall_ctrl {
+ struct list_head node;
+ struct device *dev;
+ struct firewall_ops *ops;
+};
+
+static struct firewall_ctrl *get_firewallctrl_from_node(struct device_node *np)
+{
+ struct firewall_ctrl *ctrl;
+
+ mutex_lock(&firewall_list_mutex);
+
+ list_for_each_entry(ctrl, &firewall_list, node) {
+ if (ctrl->dev->of_node == np) {
+ mutex_unlock(&firewall_list_mutex);
+ return ctrl;
+ }
+ }
+
+ mutex_unlock(&firewall_list_mutex);
+
+ return NULL;
+}
+
+/**
+ * firewall_dt_has_default
+ *
+ * Check if the device node provide firewall configuration
+ *
+ * @dev: device with possible firewall configuration
+ *
+ * Return: true is firewall-0 property exist in the device node
+ */
+static bool firewall_dt_has_default(struct device *dev)
+{
+ struct device_node *np;
+ struct property *prop;
+ int size;
+
+ np = dev->of_node;
+ if (!np)
+ return false;
+
+ prop = of_find_property(np, "firewall-0", &size);
+
+ return prop ? true : false;
+}
+
+/**
+ * firewall_set_config_by_index
+ *
+ * Set a firewall controller configuration based on given index.
+ *
+ * @dev: device with firewall configuration to apply.
+ * @index: the index of the configuration in device node.
+ *
+ * Return: 0 if OK, -EPROBE_DEFER if waiting for firewall controller to be
+ * registered or negative value on other errors.
+ */
+int firewall_set_config_by_index(struct device *dev, int index)
+{
+ struct device_node *np = dev->of_node;
+ char *propname;
+ int configs, i, err = 0;
+
+ if (!np)
+ return 0;
+
+ propname = kasprintf(GFP_KERNEL, "firewall-%d", index);
+ configs = of_count_phandle_with_args(np, propname, "#firewall-cells");
+ if (configs < 0) {
+ err = -EINVAL;
+ goto error;
+ }
+
+ for (i = 0; i < configs; i++) {
+ struct firewall_ctrl *ctrl;
+ struct of_phandle_args args;
+
+ err = of_parse_phandle_with_args(np, propname,
+ "#firewall-cells",
+ i, &args);
+ if (err)
+ goto error;
+
+ /* Test if the controller is (or will be) available */
+ if (!of_device_is_available(args.np)) {
+ of_node_put(args.np);
+ continue;
+ }
+
+ ctrl = get_firewallctrl_from_node(args.np);
+ of_node_put(args.np);
+
+ /* Controller is not yet registered */
+ if (!ctrl) {
+ err = -EPROBE_DEFER;
+ goto error;
+ }
+
+ err = ctrl->ops->set_config(ctrl->dev, &args);
+ if (err)
+ goto error;
+ }
+
+error:
+ kfree(propname);
+ return err;
+}
+EXPORT_SYMBOL_GPL(firewall_set_config_by_index);
+
+/**
+ * firewall_set_config_by_name
+ *
+ * Set a firwall controller configuration based on given name.
+ *
+ * @dev: device with firewall configuration to apply.
+ * @name: the name of the configuration in device node.
+ *
+ * Return: 0 if OK, -EPROBE_DEFER if waiting for firewall controller to be
+ * registered or negative value on other errors.
+ */
+int firewall_set_config_by_name(struct device *dev, char *name)
+{
+ const char *configname;
+ int count, i;
+
+ count = of_property_count_strings(dev->of_node, "firewall-names");
+ for (i = 0; i < count; i++) {
+ int err;
+
+ err = of_property_read_string_index(dev->of_node,
+ "firewall-names",
+ i, &configname);
+ if (err)
+ return err;
+
+ if (strcmp(name, configname))
+ continue;
+
+ return firewall_set_config_by_index(dev, i);
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(firewall_set_config_by_name);
+
+/**
+ * firewall_set_default_config
+ *
+ * Set the default configuration for device.
+ * First try to apply configuration named "default", if it fails
+ * or doesn't exist, try to apply firewall-0 configuration.
+ *
+ * @dev: device with firewall configuration to apply.
+ *
+ * Return: 0 if OK, -EPROBE_DEFER if waiting for firewall controller to be
+ * registered or negative value on other errors.
+ */
+int firewall_set_default_config(struct device *dev)
+{
+ int ret;
+
+ /* Nothing to do if device node doesn't contain at least
+ * one configuration
+ */
+ if (!firewall_dt_has_default(dev))
+ return 0;
+
+ ret = firewall_set_config_by_name(dev, "default");
+ if (!ret || (ret == -EPROBE_DEFER))
+ return ret;
+
+ return firewall_set_config_by_index(dev, 0);
+}
+EXPORT_SYMBOL_GPL(firewall_set_default_config);
+
+/**
+ * firewall_register
+ *
+ * Register a firewall controller device.
+ *
+ * @dev: device implementing firewall controller.
+ * @ops: firewall controller operations.
+ *
+ * Return: 0 if OK or negative value on error.
+ */
+int firewall_register(struct device *dev,
+ struct firewall_ops *ops)
+{
+ struct firewall_ctrl *ctrl;
+
+ if (!dev || !ops || !ops->set_config)
+ return -EINVAL;
+
+ ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
+ if (!ctrl)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&ctrl->node);
+
+ ctrl->dev = dev;
+ ctrl->ops = ops;
+
+ mutex_lock(&firewall_list_mutex);
+ list_add_tail(&ctrl->node, &firewall_list);
+ mutex_unlock(&firewall_list_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(firewall_register);
+
+/**
+ * firewall_unregister
+ *
+ * Unregister a firewall controller device.
+ *
+ * @dev: device implementing firewall controller.
+ */
+void firewall_unregister(struct device *dev)
+{
+ struct firewall_ctrl *ctrl;
+
+ ctrl = get_firewallctrl_from_node(dev->of_node);
+ if (!ctrl)
+ return;
+
+ mutex_lock(&firewall_list_mutex);
+ list_del(&ctrl->node);
+ mutex_unlock(&firewall_list_mutex);
+
+ kfree(ctrl);
+}
+EXPORT_SYMBOL_GPL(firewall_unregister);
+
+static int __init firewall_init(void)
+{
+ pr_info("initialized bus firewall controller subsystem\n");
+ return 0;
+}
+
+/* Init early since drivers really need to configure firewall early */
+core_initcall(firewall_init);
diff --git a/include/linux/firewall.h b/include/linux/firewall.h
new file mode 100644
index 000000000000..67eb9985821c
--- /dev/null
+++ b/include/linux/firewall.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
+ */
+
+#ifndef _FIREWALL_H_
+#define _FIREWALL_H_
+
+#include <linux/device.h>
+#include <linux/of.h>
+
+/**
+ * struct firewall_ops
+ *
+ * Firewall controller operations structure to be filled by drivers.
+ */
+struct firewall_ops {
+ /**
+ * @set_config:
+ *
+ * Driver callback to set a firewall configuration on a controller.
+ * Configuration arguments are provided in out_args parameter.
+ *
+ * Return: 0 on success, a negative error code on failure.
+ */
+ int (*set_config)(struct device *dev, struct of_phandle_args *out_args);
+};
+
+#ifdef CONFIG_FIREWALL_CONTROLLERS
+
+int firewall_set_config_by_index(struct device *dev, int index);
+int firewall_set_config_by_name(struct device *dev, char *name);
+int firewall_set_default_config(struct device *dev);
+
+int firewall_register(struct device *dev, struct firewall_ops *ops);
+
+void firewall_unregister(struct device *dev);
+
+#else
+
+static inline int firewall_set_config_by_index(struct device *dev, int index)
+{
+ return 0;
+}
+
+static inline int firewall_set_config_by_name(struct device *dev, char *name)
+{
+ return 0;
+}
+
+static inline int firewall_set_default_config(struct device *dev)
+{
+ return 0;
+}
+
+static inline int firewall_register(struct device *dev,
+ struct firewall_ops *ops)
+{
+ return 0;
+}
+
+static inline void firewall_unregister(struct device *dev)
+{
+ /* Empty */
+}
+
+#endif
+
+#endif /* _FIREWALL_H_ */
--
2.15.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-28 15:38 ` [PATCH v2 2/7] bus: Introduce firewall controller framework Benjamin Gaignard
@ 2020-01-28 15:52 ` Greg KH
2020-01-28 16:41 ` Benjamin GAIGNARD
0 siblings, 1 reply; 34+ messages in thread
From: Greg KH @ 2020-01-28 15:52 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: robh, loic.pallardy, arnd, devicetree, system-dt, s.hauer,
linux-kernel, broonie, lkml, linux-imx, kernel, sudeep.holla,
fabio.estevam, stefano.stabellini, shawnguo, linux-arm-kernel
On Tue, Jan 28, 2020 at 04:38:01PM +0100, Benjamin Gaignard wrote:
> The goal of this framework is to offer an interface for the
> hardware blocks controlling bus accesses rights.
>
> Bus firewall controllers are typically used to control if a
> hardware block can perform read or write operations on bus.
So put this in the bus-specific code that controls the bus that these
devices live on. Why put it in the driver core when this is only on one
"bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?
And really, this should just be a totally new bus type, right? And any
devices on this bus should be changed to be on this new bus, and the
drivers changed to support them, instead of trying to overload the
platform bus with more stuff.
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-28 15:52 ` Greg KH
@ 2020-01-28 16:41 ` Benjamin GAIGNARD
2020-01-28 16:57 ` Greg KH
0 siblings, 1 reply; 34+ messages in thread
From: Benjamin GAIGNARD @ 2020-01-28 16:41 UTC (permalink / raw)
To: Greg KH
Cc: robh, Loic PALLARDY, arnd, devicetree, system-dt, s.hauer,
linux-kernel, broonie, lkml, linux-imx, kernel, sudeep.holla,
fabio.estevam, stefano.stabellini, shawnguo, linux-arm-kernel
On 1/28/20 4:52 PM, Greg KH wrote:
> On Tue, Jan 28, 2020 at 04:38:01PM +0100, Benjamin Gaignard wrote:
>> The goal of this framework is to offer an interface for the
>> hardware blocks controlling bus accesses rights.
>>
>> Bus firewall controllers are typically used to control if a
>> hardware block can perform read or write operations on bus.
> So put this in the bus-specific code that controls the bus that these
> devices live on. Why put it in the driver core when this is only on one
> "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?
It is really similar to what pin controller does, configuring an
hardware block given DT information.
I could argue that firewalls are not bus themselves they only interact
with it.
Bus firewalls exist on other SoC, I hope some others could be added in
this framework. ETZPC is only the first.
>
> And really, this should just be a totally new bus type, right? And any
> devices on this bus should be changed to be on this new bus, and the
> drivers changed to support them, instead of trying to overload the
> platform bus with more stuff.
I have tried to use the bus notifier to avoid to add this code at probe
time but without success:
https://lkml.org/lkml/2018/2/27/300
I have also tried to disable the nodes at runtime and Mark Rutland
explain me why it was wrong.
Benjamin
>
> thanks,
>
> greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-28 16:41 ` Benjamin GAIGNARD
@ 2020-01-28 16:57 ` Greg KH
2020-01-28 20:29 ` Benjamin GAIGNARD
0 siblings, 1 reply; 34+ messages in thread
From: Greg KH @ 2020-01-28 16:57 UTC (permalink / raw)
To: Benjamin GAIGNARD
Cc: robh, Loic PALLARDY, arnd, devicetree, system-dt, s.hauer,
linux-kernel, broonie, lkml, linux-imx, kernel, sudeep.holla,
fabio.estevam, stefano.stabellini, shawnguo, linux-arm-kernel
On Tue, Jan 28, 2020 at 04:41:29PM +0000, Benjamin GAIGNARD wrote:
>
> On 1/28/20 4:52 PM, Greg KH wrote:
> > On Tue, Jan 28, 2020 at 04:38:01PM +0100, Benjamin Gaignard wrote:
> >> The goal of this framework is to offer an interface for the
> >> hardware blocks controlling bus accesses rights.
> >>
> >> Bus firewall controllers are typically used to control if a
> >> hardware block can perform read or write operations on bus.
> > So put this in the bus-specific code that controls the bus that these
> > devices live on. Why put it in the driver core when this is only on one
> > "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?
>
> It is really similar to what pin controller does, configuring an
> hardware block given DT information.
Great, then use that instead :)
> I could argue that firewalls are not bus themselves they only interact
> with it.
They live on a bus, and do so in bus-specific ways, right?
> Bus firewalls exist on other SoC, I hope some others could be added in
> this framework. ETZPC is only the first.
Then put it on the bus it lives on, and the bus that the drivers for
that device are being controlled with. That sounds like the sane place
to do so, right?
> > And really, this should just be a totally new bus type, right? And any
> > devices on this bus should be changed to be on this new bus, and the
> > drivers changed to support them, instead of trying to overload the
> > platform bus with more stuff.
>
> I have tried to use the bus notifier to avoid to add this code at probe
> time but without success:
>
> https://lkml.org/lkml/2018/2/27/300
Almost 2 years ago? I can't remember something written 1 week ago...
Yes, don't abuse the notifier chain. I hate that thing as it is.
> I have also tried to disable the nodes at runtime and Mark Rutland
> explain me why it was wrong.
The bus controller should do this, right? Why not just do it there?
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-28 16:57 ` Greg KH
@ 2020-01-28 20:29 ` Benjamin GAIGNARD
2020-01-29 5:49 ` Greg KH
2020-01-29 9:42 ` Linus Walleij
0 siblings, 2 replies; 34+ messages in thread
From: Benjamin GAIGNARD @ 2020-01-28 20:29 UTC (permalink / raw)
To: Greg KH
Cc: robh, Loic PALLARDY, arnd, devicetree, system-dt, s.hauer,
linux-kernel, broonie, lkml, linux-imx, kernel, sudeep.holla,
fabio.estevam, stefano.stabellini, shawnguo, linux-arm-kernel
On 1/28/20 5:57 PM, Greg KH wrote:
> On Tue, Jan 28, 2020 at 04:41:29PM +0000, Benjamin GAIGNARD wrote:
>> On 1/28/20 4:52 PM, Greg KH wrote:
>>> On Tue, Jan 28, 2020 at 04:38:01PM +0100, Benjamin Gaignard wrote:
>>>> The goal of this framework is to offer an interface for the
>>>> hardware blocks controlling bus accesses rights.
>>>>
>>>> Bus firewall controllers are typically used to control if a
>>>> hardware block can perform read or write operations on bus.
>>> So put this in the bus-specific code that controls the bus that these
>>> devices live on. Why put it in the driver core when this is only on one
>>> "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?
>> It is really similar to what pin controller does, configuring an
>> hardware block given DT information.
> Great, then use that instead :)
I think that Linus W. will complain if I do that :)
>
>> I could argue that firewalls are not bus themselves they only interact
>> with it.
> They live on a bus, and do so in bus-specific ways, right?
>
>> Bus firewalls exist on other SoC, I hope some others could be added in
>> this framework. ETZPC is only the first.
> Then put it on the bus it lives on, and the bus that the drivers for
> that device are being controlled with. That sounds like the sane place
> to do so, right?
If that means that all drivers have to be modified it will be
problematic because not all
are specifics to the SoC.
>
>>> And really, this should just be a totally new bus type, right? And any
>>> devices on this bus should be changed to be on this new bus, and the
>>> drivers changed to support them, instead of trying to overload the
>>> platform bus with more stuff.
>> I have tried to use the bus notifier to avoid to add this code at probe
>> time but without success:
>>
>> https://lkml.org/lkml/2018/2/27/300
> Almost 2 years ago? I can't remember something written 1 week ago...
>
> Yes, don't abuse the notifier chain. I hate that thing as it is.
>
>> I have also tried to disable the nodes at runtime and Mark Rutland
>> explain me why it was wrong.
> The bus controller should do this, right? Why not just do it there?
The bus controller is a different hardware block.
>
> thanks,
>
> greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-28 20:29 ` Benjamin GAIGNARD
@ 2020-01-29 5:49 ` Greg KH
2020-01-29 9:42 ` Linus Walleij
1 sibling, 0 replies; 34+ messages in thread
From: Greg KH @ 2020-01-29 5:49 UTC (permalink / raw)
To: Benjamin GAIGNARD
Cc: robh, Loic PALLARDY, arnd, devicetree, system-dt, s.hauer,
linux-kernel, broonie, lkml, linux-imx, kernel, sudeep.holla,
fabio.estevam, stefano.stabellini, shawnguo, linux-arm-kernel
On Tue, Jan 28, 2020 at 08:29:45PM +0000, Benjamin GAIGNARD wrote:
>
> On 1/28/20 5:57 PM, Greg KH wrote:
> > On Tue, Jan 28, 2020 at 04:41:29PM +0000, Benjamin GAIGNARD wrote:
> >> On 1/28/20 4:52 PM, Greg KH wrote:
> >>> On Tue, Jan 28, 2020 at 04:38:01PM +0100, Benjamin Gaignard wrote:
> >>>> The goal of this framework is to offer an interface for the
> >>>> hardware blocks controlling bus accesses rights.
> >>>>
> >>>> Bus firewall controllers are typically used to control if a
> >>>> hardware block can perform read or write operations on bus.
> >>> So put this in the bus-specific code that controls the bus that these
> >>> devices live on. Why put it in the driver core when this is only on one
> >>> "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?
> >> It is really similar to what pin controller does, configuring an
> >> hardware block given DT information.
> > Great, then use that instead :)
> I think that Linus W. will complain if I do that :)
> >
> >> I could argue that firewalls are not bus themselves they only interact
> >> with it.
> > They live on a bus, and do so in bus-specific ways, right?
> >
> >> Bus firewalls exist on other SoC, I hope some others could be added in
> >> this framework. ETZPC is only the first.
> > Then put it on the bus it lives on, and the bus that the drivers for
> > that device are being controlled with. That sounds like the sane place
> > to do so, right?
>
> If that means that all drivers have to be modified it will be
> problematic because not all
>
> are specifics to the SoC.
That's fine, we have loads of drivers that work on different types of
busses.
Or, if this really is the "platform bus" then use that. (which is what
I was hinting at all along but no one seems to realize that, should have
been more obvious...)
> >>> And really, this should just be a totally new bus type, right? And any
> >>> devices on this bus should be changed to be on this new bus, and the
> >>> drivers changed to support them, instead of trying to overload the
> >>> platform bus with more stuff.
> >> I have tried to use the bus notifier to avoid to add this code at probe
> >> time but without success:
> >>
> >> https://lkml.org/lkml/2018/2/27/300
> > Almost 2 years ago? I can't remember something written 1 week ago...
> >
> > Yes, don't abuse the notifier chain. I hate that thing as it is.
> >
> >> I have also tried to disable the nodes at runtime and Mark Rutland
> >> explain me why it was wrong.
> > The bus controller should do this, right? Why not just do it there?
>
> The bus controller is a different hardware block.
Of course it is, but it controls a bus, and there are devices on that
bus, right? Don't circumvent things please.
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-28 20:29 ` Benjamin GAIGNARD
2020-01-29 5:49 ` Greg KH
@ 2020-01-29 9:42 ` Linus Walleij
2020-01-29 9:52 ` Greg KH
1 sibling, 1 reply; 34+ messages in thread
From: Linus Walleij @ 2020-01-29 9:42 UTC (permalink / raw)
To: Benjamin GAIGNARD
Cc: devicetree, Loic PALLARDY, arnd, robh, Greg KH, s.hauer,
linux-kernel, broonie, lkml, linux-imx, kernel, sudeep.holla,
fabio.estevam, shawnguo, stefano.stabellini, system-dt,
linux-arm-kernel
On Tue, Jan 28, 2020 at 9:30 PM Benjamin GAIGNARD
<benjamin.gaignard@st.com> wrote:
> On 1/28/20 5:57 PM, Greg KH wrote:
> > On Tue, Jan 28, 2020 at 04:41:29PM +0000, Benjamin GAIGNARD wrote:
> >> On 1/28/20 4:52 PM, Greg KH wrote:
> >>> So put this in the bus-specific code that controls the bus that these
> >>> devices live on. Why put it in the driver core when this is only on one
> >>> "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?
> >> It is really similar to what pin controller does, configuring an
> >> hardware block given DT information.
> > Great, then use that instead :)
> I think that Linus W. will complain if I do that :)
So the similarity would be something like the way that pin control
states are configured in the device tree and the pin control
handles are taken before probe in drivers/base/pinctrl.c embedding
a hook into dd.c.
Not that it in any way controls any hardware even remotely
similar to pin control. Pin control is an electronic thing,
this firewalling is about bus access.
IIUC this framework wants to discover at kernel boot time
whether certain devices are accessible to it or not by inspecting
the state of the firewalling hardware and then avoid probing
those that are inaccessible.
It needs the same deep hooks into dd.c to achieve this
I believe.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-29 9:42 ` Linus Walleij
@ 2020-01-29 9:52 ` Greg KH
2020-01-29 11:17 ` Mark Brown
0 siblings, 1 reply; 34+ messages in thread
From: Greg KH @ 2020-01-29 9:52 UTC (permalink / raw)
To: Linus Walleij
Cc: robh, stefano.stabellini, Benjamin GAIGNARD, arnd, devicetree,
shawnguo, s.hauer, linux-kernel, broonie, lkml, linux-imx,
kernel, sudeep.holla, fabio.estevam, Loic PALLARDY, system-dt,
linux-arm-kernel
On Wed, Jan 29, 2020 at 10:42:39AM +0100, Linus Walleij wrote:
> On Tue, Jan 28, 2020 at 9:30 PM Benjamin GAIGNARD
> <benjamin.gaignard@st.com> wrote:
> > On 1/28/20 5:57 PM, Greg KH wrote:
> > > On Tue, Jan 28, 2020 at 04:41:29PM +0000, Benjamin GAIGNARD wrote:
> > >> On 1/28/20 4:52 PM, Greg KH wrote:
>
> > >>> So put this in the bus-specific code that controls the bus that these
> > >>> devices live on. Why put it in the driver core when this is only on one
> > >>> "bus" (i.e. the catch-all-and-a-bag-of-chips platform bus)?
>
> > >> It is really similar to what pin controller does, configuring an
> > >> hardware block given DT information.
>
> > > Great, then use that instead :)
>
> > I think that Linus W. will complain if I do that :)
>
> So the similarity would be something like the way that pin control
> states are configured in the device tree and the pin control
> handles are taken before probe in drivers/base/pinctrl.c embedding
> a hook into dd.c.
>
> Not that it in any way controls any hardware even remotely
> similar to pin control. Pin control is an electronic thing,
> this firewalling is about bus access.
>
> IIUC this framework wants to discover at kernel boot time
> whether certain devices are accessible to it or not by inspecting
> the state of the firewalling hardware and then avoid probing
> those that are inaccessible.
>
> It needs the same deep hooks into dd.c to achieve this
> I believe.
It just needs to be part of the bus logic for the specific bus that this
"firewall" is on. Just like we do the same thing for USB or thunderbolt
devices. Put this in the bus-specific code please.
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-29 9:52 ` Greg KH
@ 2020-01-29 11:17 ` Mark Brown
2020-01-31 8:37 ` Benjamin GAIGNARD
0 siblings, 1 reply; 34+ messages in thread
From: Mark Brown @ 2020-01-29 11:17 UTC (permalink / raw)
To: Greg KH
Cc: robh, stefano.stabellini, Benjamin GAIGNARD, arnd, devicetree,
shawnguo, Linus Walleij, linux-kernel, lkml, linux-imx, kernel,
sudeep.holla, fabio.estevam, Loic PALLARDY, system-dt, s.hauer,
linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 337 bytes --]
On Wed, Jan 29, 2020 at 10:52:40AM +0100, Greg KH wrote:
> It just needs to be part of the bus logic for the specific bus that this
> "firewall" is on. Just like we do the same thing for USB or thunderbolt
> devices. Put this in the bus-specific code please.
I'd expect that this is going to affect at least platform and AMBA
buses.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-29 11:17 ` Mark Brown
@ 2020-01-31 8:37 ` Benjamin GAIGNARD
2020-01-31 9:06 ` Greg KH
0 siblings, 1 reply; 34+ messages in thread
From: Benjamin GAIGNARD @ 2020-01-31 8:37 UTC (permalink / raw)
To: Greg KH, Linus Walleij, robh, Loic PALLARDY, arnd, devicetree,
system-dt, s.hauer, linux-kernel, lkml, linux-imx, kernel,
sudeep.holla, fabio.estevam, stefano.stabellini, shawnguo,
linux-arm-kernel
On 1/29/20 12:17 PM, Mark Brown wrote:
> On Wed, Jan 29, 2020 at 10:52:40AM +0100, Greg KH wrote:
>
>> It just needs to be part of the bus logic for the specific bus that this
>> "firewall" is on. Just like we do the same thing for USB or thunderbolt
>> devices. Put this in the bus-specific code please.
> I'd expect that this is going to affect at least platform and AMBA
> buses.
Correct me if I'm wrong but creating a new type of bus would mean
that all the drivers living on this bus must be changed to register
themselves on this bus ?
Or does a solution exist to let them live on the platform bus and call
firewalled bus before been probed ?
All the impacted drivers could work on the existing bus with or without
the firewall so I don't want to break
that.
Benjamin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-31 8:37 ` Benjamin GAIGNARD
@ 2020-01-31 9:06 ` Greg KH
2020-02-14 16:05 ` Linus Walleij
0 siblings, 1 reply; 34+ messages in thread
From: Greg KH @ 2020-01-31 9:06 UTC (permalink / raw)
To: Benjamin GAIGNARD
Cc: robh, Loic PALLARDY, arnd, devicetree, shawnguo, Linus Walleij,
linux-kernel, lkml, linux-imx, kernel, sudeep.holla,
fabio.estevam, stefano.stabellini, system-dt, s.hauer,
linux-arm-kernel
On Fri, Jan 31, 2020 at 08:37:27AM +0000, Benjamin GAIGNARD wrote:
>
> On 1/29/20 12:17 PM, Mark Brown wrote:
> > On Wed, Jan 29, 2020 at 10:52:40AM +0100, Greg KH wrote:
> >
> >> It just needs to be part of the bus logic for the specific bus that this
> >> "firewall" is on. Just like we do the same thing for USB or thunderbolt
> >> devices. Put this in the bus-specific code please.
> > I'd expect that this is going to affect at least platform and AMBA
> > buses.
>
> Correct me if I'm wrong but creating a new type of bus would mean
> that all the drivers living on this bus must be changed to register
> themselves on this bus ?
Yes.
> Or does a solution exist to let them live on the platform bus and call
> firewalled bus before been probed ?
Why do people want to abuse the platform bus so much? If a device is on
a bus that can have such a controller, then it is on a real bus, use it!
> All the impacted drivers could work on the existing bus with or without
> the firewall so I don't want to break
break what?
>
> that.
Odd line-break :)
Just register the driver on both busses, no big deal.
Stop abusing the platform bus code for things that it is not for.
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-01-31 9:06 ` Greg KH
@ 2020-02-14 16:05 ` Linus Walleij
2020-02-14 21:40 ` Greg KH
0 siblings, 1 reply; 34+ messages in thread
From: Linus Walleij @ 2020-02-14 16:05 UTC (permalink / raw)
To: Greg KH, Stephen Rothwell, Grant Likely
Cc: robh, stefano.stabellini, Benjamin GAIGNARD, arnd, devicetree,
shawnguo, s.hauer, linux-kernel, lkml, linux-imx, kernel,
sudeep.holla, fabio.estevam, Loic PALLARDY, system-dt,
linux-arm-kernel
On Fri, Jan 31, 2020 at 10:06 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> Why do people want to abuse the platform bus so much? If a device is on
> a bus that can have such a controller, then it is on a real bus, use it!
I'm not saying it is a good thing, but the reason why it is (ab)used so
much can be found in:
drivers/of/platform.c
TL;DR: struct platform_device is the Device McDeviceFace and
platform bus the Bus McBusFace used by the device tree parser since
it is slightly to completely unaware of what devices it is actually
spawning.
And everything and its dog is using device tree in the embedded
world. (A quick glance in drivers/acpi gives me the impression
that ACPI is doing the very same thing but I am not a domain expert
there so I am not really sure.)
Whenever a device is created from a device tree it gets spawned
on either the platform bus or the amba bus. In 99 cases out of
100 it is going to be a platform_device.
In most device trees all devices ultimately spawn from the device
tree and the root of absolutely everything including irq chips on
the SoC, timers, PCI hosts and USB root hubs and whatnot is a
platform device, because that is how the core device tree parser has
chosen to spawn off devices.
This generic code goes back to
commit eca3930163ba8884060ce9d9ff5ef0d9b7c7b00f
"of: Merge of_platform_bus_type with platform_bus_type"
where the device tree-specific bus was replaced by the
platform bus. This code was then moved down to drivers/of
and used in multiple architectures. Grant's patch makes perfect
sense because at the time some devices were created using board
files (thus platform_device) and others using device tree and having
two different probe paths and driver files for this reason alone
was not reasonable. The same reasoning will apply to ACPI
vs device tree drivers.
What we *could* have done was to handle special devices
special, like happened for AMBA PrimeCells. Mea Culpa, I suppose
I am one of the guilty.
Supporting new bus types for root devices in systems described
in device tree would requiring patching drivers/of/platform.c
and people are afraid of that because the code there is pretty
complex.
Instead platform_device is (ab)used to carry stuff over from the
device tree to respective subsystem.
In some cases the struct platform_device from device tree is
discarded after use, it is just left dangling in memory with no other
purpose than to serve as .parent for whatever device on whatever
bus we were really creating.
For some devices such as root irq_chips they serve no purpose
whatsoever, they are just created and sitting around never
to be probed, because the code instantiating them parse the
device tree directly.
For the devices that actually probe to drive a piece of silicon,
arguably a different type of device on a different bus should be
created, such as (I am making this up) struct soc_device
on soc_bus. (Incidentally soc_bus exists, but its current use case
is not for this.)
I don't really see any better option for Benjamin or anyone else
though?
The reason why it is used so much should at least be clarified
now I think.
Yours,
Linus Walleij
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-02-14 16:05 ` Linus Walleij
@ 2020-02-14 21:40 ` Greg KH
2020-02-15 12:41 ` Benjamin GAIGNARD
0 siblings, 1 reply; 34+ messages in thread
From: Greg KH @ 2020-02-14 21:40 UTC (permalink / raw)
To: Linus Walleij
Cc: Stephen Rothwell, stefano.stabellini, Benjamin GAIGNARD, arnd,
robh, shawnguo, s.hauer, linux-kernel, devicetree, Grant Likely,
lkml, linux-imx, kernel, sudeep.holla, fabio.estevam,
Loic PALLARDY, system-dt, linux-arm-kernel
On Fri, Feb 14, 2020 at 05:05:07PM +0100, Linus Walleij wrote:
> On Fri, Jan 31, 2020 at 10:06 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > Why do people want to abuse the platform bus so much? If a device is on
> > a bus that can have such a controller, then it is on a real bus, use it!
>
> I'm not saying it is a good thing, but the reason why it is (ab)used so
> much can be found in:
> drivers/of/platform.c
>
> TL;DR: struct platform_device is the Device McDeviceFace and
> platform bus the Bus McBusFace used by the device tree parser since
> it is slightly to completely unaware of what devices it is actually
> spawning.
<snip>
Yeah, great explaination, and I understand. DT stuff really is ok to be
on a platform bus, as that's what almost all of them are.
But, when you try to start messing around with things like this
"firewall" says it is doing, it's then obvious that this really isn't a
DT like thing, but rather you do have a bus involved with a controller
so that should be used instead.
Or just filter away the DT stuff so that the kernel never even sees
those devices, which might just be simplest :)
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-02-14 21:40 ` Greg KH
@ 2020-02-15 12:41 ` Benjamin GAIGNARD
2020-02-16 7:21 ` Greg KH
0 siblings, 1 reply; 34+ messages in thread
From: Benjamin GAIGNARD @ 2020-02-15 12:41 UTC (permalink / raw)
To: Greg KH, Linus Walleij
Cc: Stephen Rothwell, Loic PALLARDY, arnd, devicetree, shawnguo,
s.hauer, linux-kernel, Grant Likely, lkml, linux-imx, kernel,
sudeep.holla, fabio.estevam, stefano.stabellini, system-dt, robh,
linux-arm-kernel
On 2/14/20 10:40 PM, Greg KH wrote:
> On Fri, Feb 14, 2020 at 05:05:07PM +0100, Linus Walleij wrote:
>> On Fri, Jan 31, 2020 at 10:06 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>>> Why do people want to abuse the platform bus so much? If a device is on
>>> a bus that can have such a controller, then it is on a real bus, use it!
>> I'm not saying it is a good thing, but the reason why it is (ab)used so
>> much can be found in:
>> drivers/of/platform.c
>>
>> TL;DR: struct platform_device is the Device McDeviceFace and
>> platform bus the Bus McBusFace used by the device tree parser since
>> it is slightly to completely unaware of what devices it is actually
>> spawning.
> <snip>
>
> Yeah, great explaination, and I understand. DT stuff really is ok to be
> on a platform bus, as that's what almost all of them are.
>
> But, when you try to start messing around with things like this
> "firewall" says it is doing, it's then obvious that this really isn't a
> DT like thing, but rather you do have a bus involved with a controller
> so that should be used instead.
Ok but how put in place a new bus while keeping the devices on platform
bus to avoid changing all the drivers ?
>
> Or just filter away the DT stuff so that the kernel never even sees
> those devices, which might just be simplest :)
yes but we lost the possibility to change the firewall configuration at
run time. I do expect to be able to describe in the DT firewall configuration
and to use them at run time. That could allow, for example, to handover
a HW block to the coprocessor when the main core is going to be suspended
to save power.
Benjamin
>
> thanks,
>
> greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/7] bus: Introduce firewall controller framework
2020-02-15 12:41 ` Benjamin GAIGNARD
@ 2020-02-16 7:21 ` Greg KH
0 siblings, 0 replies; 34+ messages in thread
From: Greg KH @ 2020-02-16 7:21 UTC (permalink / raw)
To: Benjamin GAIGNARD
Cc: Stephen Rothwell, Loic PALLARDY, arnd, robh, shawnguo,
Linus Walleij, linux-kernel, devicetree, Grant Likely, lkml,
linux-imx, kernel, sudeep.holla, fabio.estevam,
stefano.stabellini, system-dt, s.hauer, linux-arm-kernel
On Sat, Feb 15, 2020 at 12:41:07PM +0000, Benjamin GAIGNARD wrote:
>
> On 2/14/20 10:40 PM, Greg KH wrote:
> > On Fri, Feb 14, 2020 at 05:05:07PM +0100, Linus Walleij wrote:
> >> On Fri, Jan 31, 2020 at 10:06 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >>
> >>> Why do people want to abuse the platform bus so much? If a device is on
> >>> a bus that can have such a controller, then it is on a real bus, use it!
> >> I'm not saying it is a good thing, but the reason why it is (ab)used so
> >> much can be found in:
> >> drivers/of/platform.c
> >>
> >> TL;DR: struct platform_device is the Device McDeviceFace and
> >> platform bus the Bus McBusFace used by the device tree parser since
> >> it is slightly to completely unaware of what devices it is actually
> >> spawning.
> > <snip>
> >
> > Yeah, great explaination, and I understand. DT stuff really is ok to be
> > on a platform bus, as that's what almost all of them are.
> >
> > But, when you try to start messing around with things like this
> > "firewall" says it is doing, it's then obvious that this really isn't a
> > DT like thing, but rather you do have a bus involved with a controller
> > so that should be used instead.
>
> Ok but how put in place a new bus while keeping the devices on platform
> bus to avoid changing all the drivers ?
You don't, you put them all on your real bus, as that is what you now
have.
> > Or just filter away the DT stuff so that the kernel never even sees
> > those devices, which might just be simplest :)
>
> yes but we lost the possibility to change the firewall configuration at
> run time. I do expect to be able to describe in the DT firewall configuration
> and to use them at run time. That could allow, for example, to handover
> a HW block to the coprocessor when the main core is going to be suspended
> to save power.
Then use a real bus :)
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 3/7] base: Add calls to firewall controller
2020-01-28 15:37 [PATCH v2 0/7] Introduce bus firewall controller framework Benjamin Gaignard
2020-01-28 15:38 ` [PATCH v2 1/7] dt-bindings: bus: Add firewall bindings Benjamin Gaignard
2020-01-28 15:38 ` [PATCH v2 2/7] bus: Introduce firewall controller framework Benjamin Gaignard
@ 2020-01-28 15:38 ` Benjamin Gaignard
2020-01-28 15:38 ` [PATCH v2 4/7] dt-bindings: bus: Add STM32 ETZPC " Benjamin Gaignard
` (4 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Benjamin Gaignard @ 2020-01-28 15:38 UTC (permalink / raw)
To: broonie, robh, arnd, shawnguo, s.hauer, fabio.estevam,
sudeep.holla, lkml
Cc: devicetree, Benjamin Gaignard, loic.pallardy, gregkh,
linux-kernel, linux-imx, kernel, stefano.stabellini, system-dt,
linux-arm-kernel
To avoid modifying all the drivers call firewall_set_default_config
before probe to apply the configuration define in device node (if any).
When unbinding the device try to apply configuration named "unbind".
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
drivers/base/dd.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index d811e60610d3..6a2153f6b19b 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -20,6 +20,7 @@
#include <linux/device.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
+#include <linux/firewall.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/kthread.h>
@@ -521,6 +522,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
re_probe:
dev->driver = drv;
+ ret = firewall_set_default_config(dev);
+ if (ret)
+ goto firewall_failed;
+
/* If using pinctrl, bind pins now before probing */
ret = pinctrl_bind_pins(dev);
if (ret)
@@ -601,6 +606,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
pinctrl_bind_failed:
+ firewall_set_config_by_name(dev, "unbind");
+firewall_failed:
device_links_no_driver(dev);
devres_release_all(dev);
arch_teardown_dma_ops(dev);
@@ -1135,6 +1142,8 @@ static void __device_release_driver(struct device *dev, struct device *parent)
device_links_driver_cleanup(dev);
+ firewall_set_config_by_name(dev, "unbind");
+
devres_release_all(dev);
arch_teardown_dma_ops(dev);
dev->driver = NULL;
--
2.15.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v2 4/7] dt-bindings: bus: Add STM32 ETZPC firewall controller
2020-01-28 15:37 [PATCH v2 0/7] Introduce bus firewall controller framework Benjamin Gaignard
` (2 preceding siblings ...)
2020-01-28 15:38 ` [PATCH v2 3/7] base: Add calls to firewall controller Benjamin Gaignard
@ 2020-01-28 15:38 ` Benjamin Gaignard
2020-01-28 15:38 ` [PATCH v2 5/7] bus: firewall: Add driver for STM32 ETZPC controller Benjamin Gaignard
` (3 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Benjamin Gaignard @ 2020-01-28 15:38 UTC (permalink / raw)
To: broonie, robh, arnd, shawnguo, s.hauer, fabio.estevam,
sudeep.holla, lkml
Cc: devicetree, Benjamin Gaignard, loic.pallardy, gregkh,
linux-kernel, linux-imx, kernel, stefano.stabellini, system-dt,
linux-arm-kernel
Document STM32 ETZPC firewall controller bindings
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
version 2:
- use ymal file to describe the bindings
.../bindings/bus/firewall/st,stm32-etzpc.yaml | 41 ++++++++++++++++++++++
1 file changed, 41 insertions(+)
create mode 100644 Documentation/devicetree/bindings/bus/firewall/st,stm32-etzpc.yaml
diff --git a/Documentation/devicetree/bindings/bus/firewall/st,stm32-etzpc.yaml b/Documentation/devicetree/bindings/bus/firewall/st,stm32-etzpc.yaml
new file mode 100644
index 000000000000..a162e31e4529
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/firewall/st,stm32-etzpc.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/firewall/st,stm32-etzpc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STM32 Extended TrustZone Protection controller
+
+maintainers:
+ - Benjamin Gaignard <benjamin.gaignard@st.com>
+
+description: STMicroelectronics's STM32 firewall controller implementation
+
+allOf:
+ - $ref: "firewall-provider.yaml#"
+
+properties:
+ compatible:
+ enum:
+ - st,stm32-etzpc
+
+ reg:
+ maxItems: 1
+
+ '#firewall-cells':
+ const: 2
+
+required:
+ - compatible
+ - reg
+ - '#firewall-cells'
+
+examples:
+ - |
+ etzpc@5c007000 {
+ compatible = "st,stm32-etzpc";
+ reg = <0x5c007000 0x400>;
+ #firewall-cells = <2>;
+ };
+
+...
--
2.15.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v2 5/7] bus: firewall: Add driver for STM32 ETZPC controller
2020-01-28 15:37 [PATCH v2 0/7] Introduce bus firewall controller framework Benjamin Gaignard
` (3 preceding siblings ...)
2020-01-28 15:38 ` [PATCH v2 4/7] dt-bindings: bus: Add STM32 ETZPC " Benjamin Gaignard
@ 2020-01-28 15:38 ` Benjamin Gaignard
2020-01-28 15:38 ` [PATCH v2 6/7] ARM: dts: stm32: Add firewall node for stm32mp157 SoC Benjamin Gaignard
` (2 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Benjamin Gaignard @ 2020-01-28 15:38 UTC (permalink / raw)
To: broonie, robh, arnd, shawnguo, s.hauer, fabio.estevam,
sudeep.holla, lkml
Cc: devicetree, Benjamin Gaignard, loic.pallardy, gregkh,
linux-kernel, linux-imx, kernel, stefano.stabellini, system-dt,
linux-arm-kernel
STM32 Extended TrustZone Protection Controller (ETZPC) got 3 possible
configurations per hardware block:
- secure: hardware blocks are only accessible by software running on trust
zone (i.e op-tee firmware).
- non-secure: hardware blocks are accessible by non-secure software (i.e.
linux kernel).
- coprocessor: hardware blocks are only accessible by the coprocessor.
Each hardware block status is defined by a 2 bits field and all of
them are packed into 32 bits registers. ETZPC can manage up to 94
hardware blocks.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
version 2:
- provide the full list the hardware blocks
drivers/bus/firewall/Kconfig | 7 ++
drivers/bus/firewall/Makefile | 1 +
drivers/bus/firewall/stm32-etzpc.c | 140 +++++++++++++++++++++++++
include/dt-bindings/bus/firewall/stm32-etzpc.h | 90 ++++++++++++++++
4 files changed, 238 insertions(+)
create mode 100644 drivers/bus/firewall/stm32-etzpc.c
create mode 100644 include/dt-bindings/bus/firewall/stm32-etzpc.h
diff --git a/drivers/bus/firewall/Kconfig b/drivers/bus/firewall/Kconfig
index 893bacb955f5..f724c09801e0 100644
--- a/drivers/bus/firewall/Kconfig
+++ b/drivers/bus/firewall/Kconfig
@@ -4,4 +4,11 @@ config FIREWALL_CONTROLLERS
bool "Support of bus firewall controllers"
depends on OF
+config STM32_ETZPC
+ bool "STM32 ETZPC Domain Controller"
+ depends on FIREWALL_CONTROLLERS && MACH_STM32MP157
+ help
+ Select y to enable STM32 Extended TrustZone Protection
+ Controller (ETZPC)
+
endmenu
diff --git a/drivers/bus/firewall/Makefile b/drivers/bus/firewall/Makefile
index eb6b978d6450..d42e99b5865e 100644
--- a/drivers/bus/firewall/Makefile
+++ b/drivers/bus/firewall/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_FIREWALL_CONTROLLERS) += firewall.o
+obj-$(CONFIG_STM32_ETZPC) += stm32-etzpc.o
diff --git a/drivers/bus/firewall/stm32-etzpc.c b/drivers/bus/firewall/stm32-etzpc.c
new file mode 100644
index 000000000000..39999579fe92
--- /dev/null
+++ b/drivers/bus/firewall/stm32-etzpc.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
+ */
+
+#include <linux/device.h>
+#include <linux/firewall.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/bus/firewall/stm32-etzpc.h>
+
+#define ETZPC_DECPROT 0x010
+#define ETZPC_NUM_LOCKS 94
+
+struct stm32_etzpc {
+ struct regmap_field *fields[ETZPC_NUM_LOCKS];
+};
+
+static int stm32_etzpc_set_config(struct device *dev,
+ struct of_phandle_args *out_args)
+{
+ struct stm32_etzpc *etzpc = dev_get_drvdata(dev);
+ int index = out_args->args[0];
+ unsigned int value = out_args->args[1];
+ u32 status;
+
+ if (out_args->args_count != 2)
+ return -EINVAL;
+
+ if (index >= ETZPC_NUM_LOCKS)
+ return -EINVAL;
+
+ if (value > STM32_ETZPC_NON_SECURE)
+ return -EINVAL;
+
+ regmap_field_force_write(etzpc->fields[index], value);
+
+ /* Hardware could denied the new value, read it back to check it */
+ regmap_field_read(etzpc->fields[index], &status);
+
+ if (value != status) {
+ dev_info(dev, "failed to set configuration: index %d, value %d\n",
+ index, value);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static struct firewall_ops stm32_etzpc_ops = {
+ .set_config = stm32_etzpc_set_config,
+};
+
+static const struct regmap_config stm32_etzpc_regmap_cfg = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = sizeof(u32),
+ .max_register = 0x3FF,
+ .fast_io = true,
+};
+
+static int stm32_etzpc_probe(struct platform_device *pdev)
+{
+ struct stm32_etzpc *etzpc;
+ struct resource *res;
+ void __iomem *mmio;
+ struct regmap *regmap;
+ int i;
+
+ etzpc = devm_kzalloc(&pdev->dev, sizeof(*etzpc), GFP_KERNEL);
+ if (!etzpc)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ mmio = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(mmio))
+ return PTR_ERR(mmio);
+
+ regmap = devm_regmap_init_mmio(&pdev->dev, mmio,
+ &stm32_etzpc_regmap_cfg);
+
+ for (i = 0; i < ETZPC_NUM_LOCKS; i++) {
+ struct reg_field field;
+
+ /*
+ * Each hardware block status is defined by
+ * a 2 bits field and all of them are packed into
+ * 32 bits registers. Do some computation to get
+ * register offset and the shift.
+ */
+ field.reg = ETZPC_DECPROT + (i >> 4) * sizeof(u32);
+ field.lsb = (i % 0x10) << 1;
+ field.msb = field.lsb + 1;
+
+ etzpc->fields[i] = devm_regmap_field_alloc(&pdev->dev,
+ regmap, field);
+ }
+
+ platform_set_drvdata(pdev, etzpc);
+
+ return firewall_register(&pdev->dev, &stm32_etzpc_ops);
+}
+
+static int stm32_etzpc_remove(struct platform_device *pdev)
+{
+ firewall_unregister(&pdev->dev);
+
+ return 0;
+}
+
+static const struct of_device_id stm32_etzpc_of_match[] = {
+ { .compatible = "st,stm32-etzpc" },
+ { /* end node */ }
+};
+MODULE_DEVICE_TABLE(of, stm32_etzpc_of_match);
+
+static struct platform_driver stm32_etzpc_driver = {
+ .probe = stm32_etzpc_probe,
+ .remove = stm32_etzpc_remove,
+ .driver = {
+ .name = "stm32-etzpc",
+ .of_match_table = stm32_etzpc_of_match,
+ },
+};
+
+static int __init stm32_etzpc_init(void)
+{
+ return platform_driver_register(&stm32_etzpc_driver);
+}
+arch_initcall(stm32_etzpc_init);
+
+MODULE_AUTHOR("Benjamin Gaignard <benjamin.gaignard@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32 Bus Firewall Controller");
diff --git a/include/dt-bindings/bus/firewall/stm32-etzpc.h b/include/dt-bindings/bus/firewall/stm32-etzpc.h
new file mode 100644
index 000000000000..9c4783b9783c
--- /dev/null
+++ b/include/dt-bindings/bus/firewall/stm32-etzpc.h
@@ -0,0 +1,90 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) STMicroelectronics 2020 - All Rights Reserved
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
+ */
+
+#ifndef _STM32_ETZPC_H_
+#define _STM32_ETZPC_H_
+
+/* ETZPC configurations: trust-zone, non-secure or coprocessor*/
+#define STM32_ETZPC_TRUST 1
+#define STM32_ETPCZ_COPRO 2
+#define STM32_ETZPC_NON_SECURE 3
+
+/* ETZPC hard blocks index */
+#define STM32_ETZPC_USART1 3
+#define STM32_ETZPC_SPI6 4
+#define STM32_ETZPC_I2C4 5
+#define STM32_ETZPC_RNG1 7
+#define STM32_ETZPC_HASH1 8
+#define STM32_ETZPC_CRYP1 9
+#define STM32_ETZPC_I2C6 12
+#define STM32_ETZPC_TIM2 16
+#define STM32_ETZPC_TIM3 17
+#define STM32_ETZPC_TIM4 18
+#define STM32_ETZPC_TIM5 19
+#define STM32_ETZPC_TIM6 20
+#define STM32_ETZPC_TIM7 21
+#define STM32_ETZPC_TIM12 22
+#define STM32_ETZPC_TIM13 23
+#define STM32_ETZPC_TIM14 24
+#define STM32_ETZPC_LPTIM1 25
+#define STM32_ETZPC_SPI2 27
+#define STM32_ETZPC_SPI3 28
+#define STM32_ETZPC_USART2 30
+#define STM32_ETZPC_USART3 31
+#define STM32_ETZPC_USART4 32
+#define STM32_ETZPC_USART5 33
+#define STM32_ETZPC_I2C1 34
+#define STM32_ETZPC_I2C2 35
+#define STM32_ETZPC_I2C3 36
+#define STM32_ETZPC_I2C5 37
+#define STM32_ETZPC_CEC 38
+#define STM32_ETZPC_DAC 39
+#define STM32_ETZPC_UART7 40
+#define STM32_ETZPC_UART8 41
+#define STM32_ETZPC_MDIOS 44
+#define STM32_ETZPC_TIM1 48
+#define STM32_ETZPC_TIM8 49
+#define STM32_ETZPC_USART6 51
+#define STM32_ETZPC_SPI1 52
+#define STM32_ETZPC_SPI4 53
+#define STM32_ETZPC_TIM15 54
+#define STM32_ETZPC_TIM16 55
+#define STM32_ETZPC_TIM17 56
+#define STM32_ETZPC_SPI5 57
+#define STM32_ETZPC_SAI1 58
+#define STM32_ETZPC_SAI2 59
+#define STM32_ETZPC_SAI3 60
+#define STM32_ETZPC_DFSDM 61
+#define STM32_ETZPC_TT_FDCAN 62
+#define STM32_ETZPC_LPTIM2 64
+#define STM32_ETZPC_LPTIM3 65
+#define STM32_ETZPC_LPTIM4 66
+#define STM32_ETZPC_LPTIM5 67
+#define STM32_ETZPC_SAI4 68
+#define STM32_ETZPC_VREFBUF 69
+#define STM32_ETZPC_DCMI 70
+#define STM32_ETZPC_CRC2 71
+#define STM32_ETZPC_ADC 72
+#define STM32_ETZPC_HASH2 73
+#define STM32_ETZPC_RNG2 74
+#define STM32_ETZPC_CRYP2 75
+#define STM32_ETZPC_SRAM1 80
+#define STM32_ETZPC_SRAM2 81
+#define STM32_ETZPC_SRAM3 82
+#define STM32_ETZPC_SRAM4 83
+#define STM32_ETZPC_RETRAM 84
+#define STM32_ETZPC_OTG 85
+#define STM32_ETZPC_SDMMC3 86
+#define STM32_ETZPC_DLYBSD3 87
+#define STM32_ETZPC_DMA1 88
+#define STM32_ETZPC_DMA2 89
+#define STM32_ETZPC_DMAMUX 90
+#define STM32_ETZPC_FMC 91
+#define STM32_ETZPC_QSPI 92
+#define STM32_ETZPC_DLYBQ 93
+#define STM32_ETZPC_ETH1 94
+
+#endif /* _STM32_ETZPC_H_ */
--
2.15.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v2 6/7] ARM: dts: stm32: Add firewall node for stm32mp157 SoC
2020-01-28 15:37 [PATCH v2 0/7] Introduce bus firewall controller framework Benjamin Gaignard
` (4 preceding siblings ...)
2020-01-28 15:38 ` [PATCH v2 5/7] bus: firewall: Add driver for STM32 ETZPC controller Benjamin Gaignard
@ 2020-01-28 15:38 ` Benjamin Gaignard
2020-01-28 15:38 ` [PATCH v2 7/7] ARM: dts: stm32: enable firewall controller node on stm32mp157c-ed1 Benjamin Gaignard
2020-01-28 16:36 ` [PATCH v2 0/7] Introduce bus firewall controller framework Sudeep Holla
7 siblings, 0 replies; 34+ messages in thread
From: Benjamin Gaignard @ 2020-01-28 15:38 UTC (permalink / raw)
To: broonie, robh, arnd, shawnguo, s.hauer, fabio.estevam,
sudeep.holla, lkml
Cc: devicetree, Benjamin Gaignard, loic.pallardy, gregkh,
linux-kernel, linux-imx, kernel, stefano.stabellini, system-dt,
linux-arm-kernel
Declare ETZPC device as a firewall controller node for stm32mp157 SoC
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
arch/arm/boot/dts/stm32mp157c.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index ed8b258256d7..8a00dad9688e 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -1499,6 +1499,13 @@
};
};
+ etzpc: etzpc@5c007000 {
+ compatible = "st,stm32-etzpc";
+ reg = <0x5c007000 0x400>;
+ #firewall-cells = <2>;
+ status = "okay";
+ };
+
i2c6: i2c@5c009000 {
compatible = "st,stm32f7-i2c";
reg = <0x5c009000 0x400>;
--
2.15.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v2 7/7] ARM: dts: stm32: enable firewall controller node on stm32mp157c-ed1
2020-01-28 15:37 [PATCH v2 0/7] Introduce bus firewall controller framework Benjamin Gaignard
` (5 preceding siblings ...)
2020-01-28 15:38 ` [PATCH v2 6/7] ARM: dts: stm32: Add firewall node for stm32mp157 SoC Benjamin Gaignard
@ 2020-01-28 15:38 ` Benjamin Gaignard
2020-01-28 16:36 ` [PATCH v2 0/7] Introduce bus firewall controller framework Sudeep Holla
7 siblings, 0 replies; 34+ messages in thread
From: Benjamin Gaignard @ 2020-01-28 15:38 UTC (permalink / raw)
To: broonie, robh, arnd, shawnguo, s.hauer, fabio.estevam,
sudeep.holla, lkml
Cc: devicetree, Benjamin Gaignard, loic.pallardy, gregkh,
linux-kernel, linux-imx, kernel, stefano.stabellini, system-dt,
linux-arm-kernel
Enable ETZPC and set configuration for CEC node
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
arch/arm/boot/dts/stm32mp157c-ev1.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts b/arch/arm/boot/dts/stm32mp157c-ev1.dts
index 3789312c8539..5b72ef2a54df 100644
--- a/arch/arm/boot/dts/stm32mp157c-ev1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts
@@ -6,6 +6,7 @@
/dts-v1/;
#include "stm32mp157c-ed1.dts"
+#include <dt-bindings/bus/firewall/stm32-etzpc.h>
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/input/input.h>
@@ -77,6 +78,7 @@
&cec {
pinctrl-names = "default";
pinctrl-0 = <&cec_pins_a>;
+ firewall-0 = <&etzpc STM32_ETZPC_CEC STM32_ETZPC_NON_SECURE>;
status = "okay";
};
--
2.15.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-28 15:37 [PATCH v2 0/7] Introduce bus firewall controller framework Benjamin Gaignard
` (6 preceding siblings ...)
2020-01-28 15:38 ` [PATCH v2 7/7] ARM: dts: stm32: enable firewall controller node on stm32mp157c-ed1 Benjamin Gaignard
@ 2020-01-28 16:36 ` Sudeep Holla
2020-01-28 16:46 ` Benjamin GAIGNARD
7 siblings, 1 reply; 34+ messages in thread
From: Sudeep Holla @ 2020-01-28 16:36 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: robh, loic.pallardy, arnd, devicetree, gregkh, s.hauer,
linux-kernel, system-dt, broonie, lkml, linux-imx, kernel,
Sudeep Holla, fabio.estevam, stefano.stabellini, shawnguo,
linux-arm-kernel
On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote:
> Bus firewall framework aims to provide a kernel API to set the configuration
> of the harware blocks in charge of busses access control.
>
> Framework architecture is inspirated by pinctrl framework:
> - a default configuration could be applied before bind the driver.
> If a configuration could not be applied the driver is not bind
> to avoid doing accesses on prohibited regions.
> - configurations could be apllied dynamically by drivers.
> - device node provides the bus firewall configurations.
>
> An example of bus firewall controller is STM32 ETZPC hardware block
> which got 3 possible configurations:
> - trust: hardware blocks are only accessible by software running on trust
> zone (i.e op-tee firmware).
> - non-secure: hardware blocks are accessible by non-secure software (i.e.
> linux kernel).
> - coprocessor: hardware blocks are only accessible by the coprocessor.
> Up to 94 hardware blocks of the soc could be managed by ETZPC.
>
/me confused. Is ETZPC accessible from the non-secure kernel space to
begin with ? If so, is it allowed to configure hardware blocks as secure
or trusted ? I am failing to understand the overall design of a system
with ETZPC controller.
> At least two other hardware blocks can take benefits of this:
> - ARM TZC-400: http://infocenter.arm.com/help/topic/com.arm.doc.100325_0001_02_en/arm_corelink_tzc400_trustzone_address_space_controller_trm_100325_0001_02_en.pdf
> which is able to manage up to 8 regions in address space.
I strongly have to disagree with the above and NACK any patch trying
to do so. AFAIK, no system designed has TZC with non-secure access.
So we simply can't access this in the kernel and hence need no driver
for the same. Please avoid adding above misleading information in future.
--
Regards,
Sudeep
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-28 16:36 ` [PATCH v2 0/7] Introduce bus firewall controller framework Sudeep Holla
@ 2020-01-28 16:46 ` Benjamin GAIGNARD
2020-01-28 17:17 ` Sudeep Holla
0 siblings, 1 reply; 34+ messages in thread
From: Benjamin GAIGNARD @ 2020-01-28 16:46 UTC (permalink / raw)
To: Sudeep Holla
Cc: robh, Loic PALLARDY, arnd, devicetree, gregkh, s.hauer,
linux-kernel, system-dt, broonie, lkml, linux-imx, kernel,
fabio.estevam, stefano.stabellini, shawnguo, linux-arm-kernel
On 1/28/20 5:36 PM, Sudeep Holla wrote:
> On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote:
>> Bus firewall framework aims to provide a kernel API to set the configuration
>> of the harware blocks in charge of busses access control.
>>
>> Framework architecture is inspirated by pinctrl framework:
>> - a default configuration could be applied before bind the driver.
>> If a configuration could not be applied the driver is not bind
>> to avoid doing accesses on prohibited regions.
>> - configurations could be apllied dynamically by drivers.
>> - device node provides the bus firewall configurations.
>>
>> An example of bus firewall controller is STM32 ETZPC hardware block
>> which got 3 possible configurations:
>> - trust: hardware blocks are only accessible by software running on trust
>> zone (i.e op-tee firmware).
>> - non-secure: hardware blocks are accessible by non-secure software (i.e.
>> linux kernel).
>> - coprocessor: hardware blocks are only accessible by the coprocessor.
>> Up to 94 hardware blocks of the soc could be managed by ETZPC.
>>
> /me confused. Is ETZPC accessible from the non-secure kernel space to
> begin with ? If so, is it allowed to configure hardware blocks as secure
> or trusted ? I am failing to understand the overall design of a system
> with ETZPC controller.
Non-secure kernel could read the values set in ETZPC, if it doesn't match
with what is required by the device node the driver won't be probed.
Benjamin
>
>> At least two other hardware blocks can take benefits of this:
>> - ARM TZC-400: http://infocenter.arm.com/help/topic/com.arm.doc.100325_0001_02_en/arm_corelink_tzc400_trustzone_address_space_controller_trm_100325_0001_02_en.pdf
>> which is able to manage up to 8 regions in address space.
> I strongly have to disagree with the above and NACK any patch trying
> to do so. AFAIK, no system designed has TZC with non-secure access.
> So we simply can't access this in the kernel and hence need no driver
> for the same. Please avoid adding above misleading information in future.
>
> --
> Regards,
> Sudeep
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-28 16:46 ` Benjamin GAIGNARD
@ 2020-01-28 17:17 ` Sudeep Holla
2020-01-28 20:06 ` Benjamin GAIGNARD
0 siblings, 1 reply; 34+ messages in thread
From: Sudeep Holla @ 2020-01-28 17:17 UTC (permalink / raw)
To: Benjamin GAIGNARD
Cc: robh, Loic PALLARDY, arnd, devicetree, gregkh, s.hauer,
linux-kernel, system-dt, broonie, lkml, linux-imx, kernel,
Sudeep Holla, fabio.estevam, stefano.stabellini, shawnguo,
linux-arm-kernel
On Tue, Jan 28, 2020 at 04:46:41PM +0000, Benjamin GAIGNARD wrote:
>
> On 1/28/20 5:36 PM, Sudeep Holla wrote:
> > On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote:
> >> Bus firewall framework aims to provide a kernel API to set the configuration
> >> of the harware blocks in charge of busses access control.
> >>
> >> Framework architecture is inspirated by pinctrl framework:
> >> - a default configuration could be applied before bind the driver.
> >> If a configuration could not be applied the driver is not bind
> >> to avoid doing accesses on prohibited regions.
> >> - configurations could be apllied dynamically by drivers.
> >> - device node provides the bus firewall configurations.
> >>
> >> An example of bus firewall controller is STM32 ETZPC hardware block
> >> which got 3 possible configurations:
> >> - trust: hardware blocks are only accessible by software running on trust
> >> zone (i.e op-tee firmware).
> >> - non-secure: hardware blocks are accessible by non-secure software (i.e.
> >> linux kernel).
> >> - coprocessor: hardware blocks are only accessible by the coprocessor.
> >> Up to 94 hardware blocks of the soc could be managed by ETZPC.
> >>
> > /me confused. Is ETZPC accessible from the non-secure kernel space to
> > begin with ? If so, is it allowed to configure hardware blocks as secure
> > or trusted ? I am failing to understand the overall design of a system
> > with ETZPC controller.
>
> Non-secure kernel could read the values set in ETZPC, if it doesn't match
> with what is required by the device node the driver won't be probed.
>
OK, but I was under the impression that it was made clear that Linux is
not firmware validation suite. The firmware need to ensure all the devices
that are not accessible in the Linux kernel are marked as disabled and
this needs to happen before entering the kernel. So if this is what this
patch series achieves, then there is no need for it. Please stop pursuing
this any further or provide any other reasons(if any) to have it. Until
you have other reasons, NACK for this series.
Note you haven't cc-ed 2 people who has comments earlier[1][2]
--
Regards,
Sudeep
[1] https://lkml.org/lkml/2018/2/27/512
[2] https://lkml.org/lkml/2018/2/27/598
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-28 17:17 ` Sudeep Holla
@ 2020-01-28 20:06 ` Benjamin GAIGNARD
2020-01-28 22:06 ` Robin Murphy
2020-01-31 20:48 ` Florian Fainelli
0 siblings, 2 replies; 34+ messages in thread
From: Benjamin GAIGNARD @ 2020-01-28 20:06 UTC (permalink / raw)
To: Sudeep Holla
Cc: Mark Rutland, robh, Loic PALLARDY, arnd, devicetree, gregkh,
s.hauer, linux-kernel, system-dt, broonie, lkml, linux-imx,
kernel, fabio.estevam, stefano.stabellini, shawnguo,
linux-arm-kernel, Robin.Murphy
On 1/28/20 6:17 PM, Sudeep Holla wrote:
> On Tue, Jan 28, 2020 at 04:46:41PM +0000, Benjamin GAIGNARD wrote:
>> On 1/28/20 5:36 PM, Sudeep Holla wrote:
>>> On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote:
>>>> Bus firewall framework aims to provide a kernel API to set the configuration
>>>> of the harware blocks in charge of busses access control.
>>>>
>>>> Framework architecture is inspirated by pinctrl framework:
>>>> - a default configuration could be applied before bind the driver.
>>>> If a configuration could not be applied the driver is not bind
>>>> to avoid doing accesses on prohibited regions.
>>>> - configurations could be apllied dynamically by drivers.
>>>> - device node provides the bus firewall configurations.
>>>>
>>>> An example of bus firewall controller is STM32 ETZPC hardware block
>>>> which got 3 possible configurations:
>>>> - trust: hardware blocks are only accessible by software running on trust
>>>> zone (i.e op-tee firmware).
>>>> - non-secure: hardware blocks are accessible by non-secure software (i.e.
>>>> linux kernel).
>>>> - coprocessor: hardware blocks are only accessible by the coprocessor.
>>>> Up to 94 hardware blocks of the soc could be managed by ETZPC.
>>>>
>>> /me confused. Is ETZPC accessible from the non-secure kernel space to
>>> begin with ? If so, is it allowed to configure hardware blocks as secure
>>> or trusted ? I am failing to understand the overall design of a system
>>> with ETZPC controller.
>> Non-secure kernel could read the values set in ETZPC, if it doesn't match
>> with what is required by the device node the driver won't be probed.
>>
> OK, but I was under the impression that it was made clear that Linux is
> not firmware validation suite. The firmware need to ensure all the devices
> that are not accessible in the Linux kernel are marked as disabled and
> this needs to happen before entering the kernel. So if this is what this
> patch series achieves, then there is no need for it. Please stop pursuing
> this any further or provide any other reasons(if any) to have it. Until
> you have other reasons, NACK for this series.
No it doesn't disable the nodes.
When the firmware disable a node before the kernel that means it change
the DTB and that is a problem when you want to sign it. With my proposal
the DTB remains the same.
>
> Note you haven't cc-ed 2 people who has comments earlier[1][2]
I will cc them, thanks
> --
> Regards,
> Sudeep
>
> [1] https://lkml.org/lkml/2018/2/27/512
> [2] https://lkml.org/lkml/2018/2/27/598
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-28 20:06 ` Benjamin GAIGNARD
@ 2020-01-28 22:06 ` Robin Murphy
2020-01-29 13:40 ` Benjamin GAIGNARD
2020-01-31 20:48 ` Florian Fainelli
1 sibling, 1 reply; 34+ messages in thread
From: Robin Murphy @ 2020-01-28 22:06 UTC (permalink / raw)
To: Benjamin GAIGNARD, Sudeep Holla
Cc: Mark Rutland, robh, Loic PALLARDY, arnd, devicetree, gregkh,
s.hauer, linux-kernel, system-dt, broonie, lkml, linux-imx,
kernel, fabio.estevam, stefano.stabellini, shawnguo,
linux-arm-kernel
On 2020-01-28 8:06 pm, Benjamin GAIGNARD wrote:
>
> On 1/28/20 6:17 PM, Sudeep Holla wrote:
>> On Tue, Jan 28, 2020 at 04:46:41PM +0000, Benjamin GAIGNARD wrote:
>>> On 1/28/20 5:36 PM, Sudeep Holla wrote:
>>>> On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote:
>>>>> Bus firewall framework aims to provide a kernel API to set the configuration
>>>>> of the harware blocks in charge of busses access control.
>>>>>
>>>>> Framework architecture is inspirated by pinctrl framework:
>>>>> - a default configuration could be applied before bind the driver.
>>>>> If a configuration could not be applied the driver is not bind
>>>>> to avoid doing accesses on prohibited regions.
>>>>> - configurations could be apllied dynamically by drivers.
>>>>> - device node provides the bus firewall configurations.
>>>>>
>>>>> An example of bus firewall controller is STM32 ETZPC hardware block
>>>>> which got 3 possible configurations:
>>>>> - trust: hardware blocks are only accessible by software running on trust
>>>>> zone (i.e op-tee firmware).
>>>>> - non-secure: hardware blocks are accessible by non-secure software (i.e.
>>>>> linux kernel).
>>>>> - coprocessor: hardware blocks are only accessible by the coprocessor.
>>>>> Up to 94 hardware blocks of the soc could be managed by ETZPC.
>>>>>
>>>> /me confused. Is ETZPC accessible from the non-secure kernel space to
>>>> begin with ? If so, is it allowed to configure hardware blocks as secure
>>>> or trusted ? I am failing to understand the overall design of a system
>>>> with ETZPC controller.
>>> Non-secure kernel could read the values set in ETZPC, if it doesn't match
>>> with what is required by the device node the driver won't be probed.
>>>
>> OK, but I was under the impression that it was made clear that Linux is
>> not firmware validation suite. The firmware need to ensure all the devices
>> that are not accessible in the Linux kernel are marked as disabled and
>> this needs to happen before entering the kernel. So if this is what this
>> patch series achieves, then there is no need for it. Please stop pursuing
>> this any further or provide any other reasons(if any) to have it. Until
>> you have other reasons, NACK for this series.
>
> No it doesn't disable the nodes.
>
> When the firmware disable a node before the kernel that means it change
>
> the DTB and that is a problem when you want to sign it. With my proposal
>
> the DTB remains the same.
???
:/
The DTB is used to pass the kernel command line, memory reservations,
random seeds, and all manner of other things dynamically generated by
firmware at boot-time. Apologies for being blunt but if "changing the
DTB" is considered a problem then I can't help but think you're doing it
wrong.
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-28 22:06 ` Robin Murphy
@ 2020-01-29 13:40 ` Benjamin GAIGNARD
2020-01-31 18:25 ` Robin Murphy
2020-01-31 20:51 ` Florian Fainelli
0 siblings, 2 replies; 34+ messages in thread
From: Benjamin GAIGNARD @ 2020-01-29 13:40 UTC (permalink / raw)
To: Robin Murphy, Sudeep Holla
Cc: Mark Rutland, robh, Loic PALLARDY, arnd, devicetree, gregkh,
s.hauer, linux-kernel, system-dt, broonie, lkml, linux-imx,
kernel, fabio.estevam, stefano.stabellini, shawnguo,
linux-arm-kernel
On 1/28/20 11:06 PM, Robin Murphy wrote:
> On 2020-01-28 8:06 pm, Benjamin GAIGNARD wrote:
>>
>> On 1/28/20 6:17 PM, Sudeep Holla wrote:
>>> On Tue, Jan 28, 2020 at 04:46:41PM +0000, Benjamin GAIGNARD wrote:
>>>> On 1/28/20 5:36 PM, Sudeep Holla wrote:
>>>>> On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote:
>>>>>> Bus firewall framework aims to provide a kernel API to set the
>>>>>> configuration
>>>>>> of the harware blocks in charge of busses access control.
>>>>>>
>>>>>> Framework architecture is inspirated by pinctrl framework:
>>>>>> - a default configuration could be applied before bind the driver.
>>>>>> If a configuration could not be applied the driver is not bind
>>>>>> to avoid doing accesses on prohibited regions.
>>>>>> - configurations could be apllied dynamically by drivers.
>>>>>> - device node provides the bus firewall configurations.
>>>>>>
>>>>>> An example of bus firewall controller is STM32 ETZPC hardware block
>>>>>> which got 3 possible configurations:
>>>>>> - trust: hardware blocks are only accessible by software running
>>>>>> on trust
>>>>>> zone (i.e op-tee firmware).
>>>>>> - non-secure: hardware blocks are accessible by non-secure
>>>>>> software (i.e.
>>>>>> linux kernel).
>>>>>> - coprocessor: hardware blocks are only accessible by the
>>>>>> coprocessor.
>>>>>> Up to 94 hardware blocks of the soc could be managed by ETZPC.
>>>>>>
>>>>> /me confused. Is ETZPC accessible from the non-secure kernel space to
>>>>> begin with ? If so, is it allowed to configure hardware blocks as
>>>>> secure
>>>>> or trusted ? I am failing to understand the overall design of a
>>>>> system
>>>>> with ETZPC controller.
>>>> Non-secure kernel could read the values set in ETZPC, if it doesn't
>>>> match
>>>> with what is required by the device node the driver won't be probed.
>>>>
>>> OK, but I was under the impression that it was made clear that Linux is
>>> not firmware validation suite. The firmware need to ensure all the
>>> devices
>>> that are not accessible in the Linux kernel are marked as disabled and
>>> this needs to happen before entering the kernel. So if this is what
>>> this
>>> patch series achieves, then there is no need for it. Please stop
>>> pursuing
>>> this any further or provide any other reasons(if any) to have it. Until
>>> you have other reasons, NACK for this series.
>>
>> No it doesn't disable the nodes.
>>
>> When the firmware disable a node before the kernel that means it change
>>
>> the DTB and that is a problem when you want to sign it. With my proposal
>>
>> the DTB remains the same.
>
> ???
>
> :/
>
> The DTB is used to pass the kernel command line, memory reservations,
> random seeds, and all manner of other things dynamically generated by
> firmware at boot-time. Apologies for being blunt but if "changing the
> DTB" is considered a problem then I can't help but think you're doing
> it wrong.
Yes but I would like to limit the number of cases where a firmware has
to change the DTB.
With this proposal nodes remain the same and embedded the firewall
configuration(s).
Until now firewall configuration is "static", the firmware disable (or
remove) the nodes not accessible from Linux.
If Linux can rely on node's firewall information it could allow switch
dynamically an hardware block from Linux to a coprocessor.
For example Linux could manage the display pipe configuration and when
going to suspend handover the display hardware block to a coprocessor in
charge a refreshing only some pixels.
Benjamin
>
> Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-29 13:40 ` Benjamin GAIGNARD
@ 2020-01-31 18:25 ` Robin Murphy
2020-02-03 13:16 ` Benjamin GAIGNARD
2020-01-31 20:51 ` Florian Fainelli
1 sibling, 1 reply; 34+ messages in thread
From: Robin Murphy @ 2020-01-31 18:25 UTC (permalink / raw)
To: Benjamin GAIGNARD, Sudeep Holla
Cc: Mark Rutland, robh, Loic PALLARDY, arnd, devicetree, gregkh,
s.hauer, linux-kernel, system-dt, broonie, lkml, linux-imx,
kernel, fabio.estevam, stefano.stabellini, shawnguo,
linux-arm-kernel
On 29/01/2020 1:40 pm, Benjamin GAIGNARD wrote:
>
> On 1/28/20 11:06 PM, Robin Murphy wrote:
>> On 2020-01-28 8:06 pm, Benjamin GAIGNARD wrote:
>>>
>>> On 1/28/20 6:17 PM, Sudeep Holla wrote:
>>>> On Tue, Jan 28, 2020 at 04:46:41PM +0000, Benjamin GAIGNARD wrote:
>>>>> On 1/28/20 5:36 PM, Sudeep Holla wrote:
>>>>>> On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote:
>>>>>>> Bus firewall framework aims to provide a kernel API to set the
>>>>>>> configuration
>>>>>>> of the harware blocks in charge of busses access control.
>>>>>>>
>>>>>>> Framework architecture is inspirated by pinctrl framework:
>>>>>>> - a default configuration could be applied before bind the driver.
>>>>>>> If a configuration could not be applied the driver is not bind
>>>>>>> to avoid doing accesses on prohibited regions.
>>>>>>> - configurations could be apllied dynamically by drivers.
>>>>>>> - device node provides the bus firewall configurations.
>>>>>>>
>>>>>>> An example of bus firewall controller is STM32 ETZPC hardware block
>>>>>>> which got 3 possible configurations:
>>>>>>> - trust: hardware blocks are only accessible by software running
>>>>>>> on trust
>>>>>>> zone (i.e op-tee firmware).
>>>>>>> - non-secure: hardware blocks are accessible by non-secure
>>>>>>> software (i.e.
>>>>>>> linux kernel).
>>>>>>> - coprocessor: hardware blocks are only accessible by the
>>>>>>> coprocessor.
>>>>>>> Up to 94 hardware blocks of the soc could be managed by ETZPC.
>>>>>>>
>>>>>> /me confused. Is ETZPC accessible from the non-secure kernel space to
>>>>>> begin with ? If so, is it allowed to configure hardware blocks as
>>>>>> secure
>>>>>> or trusted ? I am failing to understand the overall design of a
>>>>>> system
>>>>>> with ETZPC controller.
>>>>> Non-secure kernel could read the values set in ETZPC, if it doesn't
>>>>> match
>>>>> with what is required by the device node the driver won't be probed.
>>>>>
>>>> OK, but I was under the impression that it was made clear that Linux is
>>>> not firmware validation suite. The firmware need to ensure all the
>>>> devices
>>>> that are not accessible in the Linux kernel are marked as disabled and
>>>> this needs to happen before entering the kernel. So if this is what
>>>> this
>>>> patch series achieves, then there is no need for it. Please stop
>>>> pursuing
>>>> this any further or provide any other reasons(if any) to have it. Until
>>>> you have other reasons, NACK for this series.
>>>
>>> No it doesn't disable the nodes.
>>>
>>> When the firmware disable a node before the kernel that means it change
>>>
>>> the DTB and that is a problem when you want to sign it. With my proposal
>>>
>>> the DTB remains the same.
>>
>> ???
>>
>> :/
>>
>> The DTB is used to pass the kernel command line, memory reservations,
>> random seeds, and all manner of other things dynamically generated by
>> firmware at boot-time. Apologies for being blunt but if "changing the
>> DTB" is considered a problem then I can't help but think you're doing
>> it wrong.
>
> Yes but I would like to limit the number of cases where a firmware has
> to change the DTB.
Sure, but unless you can limit that number to strictly zero, then
presumably the firmware must have the general capability to verify,
modify, and re-sign a DTB. At that point having it also tweak the status
of nodes that it wants for itself doesn't seem like a particularly big ask.
> With this proposal nodes remain the same and embedded the firewall
> configuration(s).
>
> Until now firewall configuration is "static", the firmware disable (or
> remove) the nodes not accessible from Linux.
>
> If Linux can rely on node's firewall information it could allow switch
> dynamically an hardware block from Linux to a coprocessor.
>
> For example Linux could manage the display pipe configuration and when
> going to suspend handover the display hardware block to a coprocessor in
> charge a refreshing only some pixels.
And like I'm sure I said before, the interface between Linux and the
Secure environment to ultimately achieve that will almost certainly make
inspecting a passive status bit in a register redundant anyway.
In the interest of being productive, though, there is another way of
looking at this. If we drop the pretence that it's in any way generic or
ever going to be relevant beyond certain configurations of certain
STMicro SoCs, then in plain terms it's just some block of MMIO registers
that have *something* to do with various other devices. At that point,
the answer is just to treat it as a syscon and make the relevant drivers
for those SoCs aware of it. I'm most familiar with the "General Register
File" on Rockchip SoCs as a prime example of "bunch of registers that
relate to the integration of various IP blocks", which manages to be
supported just fine without invasive hooks in the driver core.
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-31 18:25 ` Robin Murphy
@ 2020-02-03 13:16 ` Benjamin GAIGNARD
0 siblings, 0 replies; 34+ messages in thread
From: Benjamin GAIGNARD @ 2020-02-03 13:16 UTC (permalink / raw)
To: Robin Murphy, Sudeep Holla
Cc: Mark Rutland, robh, Loic PALLARDY, arnd, devicetree, gregkh,
s.hauer, linux-kernel, system-dt, broonie, lkml, linux-imx,
kernel, fabio.estevam, stefano.stabellini, shawnguo,
linux-arm-kernel
On 1/31/20 7:25 PM, Robin Murphy wrote:
> On 29/01/2020 1:40 pm, Benjamin GAIGNARD wrote:
>>
>> On 1/28/20 11:06 PM, Robin Murphy wrote:
>>> On 2020-01-28 8:06 pm, Benjamin GAIGNARD wrote:
>>>>
>>>> On 1/28/20 6:17 PM, Sudeep Holla wrote:
>>>>> On Tue, Jan 28, 2020 at 04:46:41PM +0000, Benjamin GAIGNARD wrote:
>>>>>> On 1/28/20 5:36 PM, Sudeep Holla wrote:
>>>>>>> On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote:
>>>>>>>> Bus firewall framework aims to provide a kernel API to set the
>>>>>>>> configuration
>>>>>>>> of the harware blocks in charge of busses access control.
>>>>>>>>
>>>>>>>> Framework architecture is inspirated by pinctrl framework:
>>>>>>>> - a default configuration could be applied before bind the driver.
>>>>>>>> If a configuration could not be applied the driver is not
>>>>>>>> bind
>>>>>>>> to avoid doing accesses on prohibited regions.
>>>>>>>> - configurations could be apllied dynamically by drivers.
>>>>>>>> - device node provides the bus firewall configurations.
>>>>>>>>
>>>>>>>> An example of bus firewall controller is STM32 ETZPC hardware
>>>>>>>> block
>>>>>>>> which got 3 possible configurations:
>>>>>>>> - trust: hardware blocks are only accessible by software running
>>>>>>>> on trust
>>>>>>>> zone (i.e op-tee firmware).
>>>>>>>> - non-secure: hardware blocks are accessible by non-secure
>>>>>>>> software (i.e.
>>>>>>>> linux kernel).
>>>>>>>> - coprocessor: hardware blocks are only accessible by the
>>>>>>>> coprocessor.
>>>>>>>> Up to 94 hardware blocks of the soc could be managed by ETZPC.
>>>>>>>>
>>>>>>> /me confused. Is ETZPC accessible from the non-secure kernel
>>>>>>> space to
>>>>>>> begin with ? If so, is it allowed to configure hardware blocks as
>>>>>>> secure
>>>>>>> or trusted ? I am failing to understand the overall design of a
>>>>>>> system
>>>>>>> with ETZPC controller.
>>>>>> Non-secure kernel could read the values set in ETZPC, if it doesn't
>>>>>> match
>>>>>> with what is required by the device node the driver won't be probed.
>>>>>>
>>>>> OK, but I was under the impression that it was made clear that
>>>>> Linux is
>>>>> not firmware validation suite. The firmware need to ensure all the
>>>>> devices
>>>>> that are not accessible in the Linux kernel are marked as disabled
>>>>> and
>>>>> this needs to happen before entering the kernel. So if this is what
>>>>> this
>>>>> patch series achieves, then there is no need for it. Please stop
>>>>> pursuing
>>>>> this any further or provide any other reasons(if any) to have it.
>>>>> Until
>>>>> you have other reasons, NACK for this series.
>>>>
>>>> No it doesn't disable the nodes.
>>>>
>>>> When the firmware disable a node before the kernel that means it
>>>> change
>>>>
>>>> the DTB and that is a problem when you want to sign it. With my
>>>> proposal
>>>>
>>>> the DTB remains the same.
>>>
>>> ???
>>>
>>> :/
>>>
>>> The DTB is used to pass the kernel command line, memory reservations,
>>> random seeds, and all manner of other things dynamically generated by
>>> firmware at boot-time. Apologies for being blunt but if "changing the
>>> DTB" is considered a problem then I can't help but think you're doing
>>> it wrong.
>>
>> Yes but I would like to limit the number of cases where a firmware has
>> to change the DTB.
>
> Sure, but unless you can limit that number to strictly zero, then
> presumably the firmware must have the general capability to verify,
> modify, and re-sign a DTB. At that point having it also tweak the
> status of nodes that it wants for itself doesn't seem like a
> particularly big ask.
>
>> With this proposal nodes remain the same and embedded the firewall
>> configuration(s).
>>
>> Until now firewall configuration is "static", the firmware disable (or
>> remove) the nodes not accessible from Linux.
>>
>> If Linux can rely on node's firewall information it could allow switch
>> dynamically an hardware block from Linux to a coprocessor.
>>
>> For example Linux could manage the display pipe configuration and when
>> going to suspend handover the display hardware block to a coprocessor in
>> charge a refreshing only some pixels.
>
> And like I'm sure I said before, the interface between Linux and the
> Secure environment to ultimately achieve that will almost certainly
> make inspecting a passive status bit in a register redundant anyway.
It is not only about secure and non secure hardware blocks split but
also about the split with the coprocessor.
The goal is to describe, in the device tree, these possible
configurations to be able to use them dynamically rather than
having a static configuration. It could also help to detect
misconfiguration between the firewall and the DT nodes.
>
> In the interest of being productive, though, there is another way of
> looking at this. If we drop the pretence that it's in any way generic
> or ever going to be relevant beyond certain configurations of certain
> STMicro SoCs, then in plain terms it's just some block of MMIO
> registers that have *something* to do with various other devices. At
> that point, the answer is just to treat it as a syscon and make the
> relevant drivers for those SoCs aware of it. I'm most familiar with
> the "General Register File" on Rockchip SoCs as a prime example of
> "bunch of registers that relate to the integration of various IP
> blocks", which manages to be supported just fine without invasive
> hooks in the driver core.
I had thought to use syscon but there is some problems with that way: if
the firewall hardware change the way it encode the information you have
change all it customers. That would mean add in each driver a test to
detect the firewall version and act accordingly. That doesn't sound
reasonable to me so I decide to create an interface to abstract the
firewall to avoid this problem.
I could reuse this interface in the ~40 impacted drivers (not all
dedicated to STMicro SoCs) but adding it in the driver core as a generic
service was making more sens for me. Note that I have take care of using
a compilation flag to not impact the others architectures.
Benjamin
>
> Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-29 13:40 ` Benjamin GAIGNARD
2020-01-31 18:25 ` Robin Murphy
@ 2020-01-31 20:51 ` Florian Fainelli
2020-02-03 13:41 ` Benjamin GAIGNARD
1 sibling, 1 reply; 34+ messages in thread
From: Florian Fainelli @ 2020-01-31 20:51 UTC (permalink / raw)
To: Benjamin GAIGNARD, Robin Murphy, Sudeep Holla
Cc: Mark Rutland, robh, Loic PALLARDY, arnd, devicetree, gregkh,
s.hauer, linux-kernel, broonie, lkml, linux-imx, kernel,
fabio.estevam, shawnguo, stefano.stabellini, system-dt,
linux-arm-kernel
On 1/29/20 5:40 AM, Benjamin GAIGNARD wrote:
>
> On 1/28/20 11:06 PM, Robin Murphy wrote:
>> On 2020-01-28 8:06 pm, Benjamin GAIGNARD wrote:
>>>
>>> On 1/28/20 6:17 PM, Sudeep Holla wrote:
>>>> On Tue, Jan 28, 2020 at 04:46:41PM +0000, Benjamin GAIGNARD wrote:
>>>>> On 1/28/20 5:36 PM, Sudeep Holla wrote:
>>>>>> On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote:
>>>>>>> Bus firewall framework aims to provide a kernel API to set the
>>>>>>> configuration
>>>>>>> of the harware blocks in charge of busses access control.
>>>>>>>
>>>>>>> Framework architecture is inspirated by pinctrl framework:
>>>>>>> - a default configuration could be applied before bind the driver.
>>>>>>> If a configuration could not be applied the driver is not bind
>>>>>>> to avoid doing accesses on prohibited regions.
>>>>>>> - configurations could be apllied dynamically by drivers.
>>>>>>> - device node provides the bus firewall configurations.
>>>>>>>
>>>>>>> An example of bus firewall controller is STM32 ETZPC hardware block
>>>>>>> which got 3 possible configurations:
>>>>>>> - trust: hardware blocks are only accessible by software running
>>>>>>> on trust
>>>>>>> zone (i.e op-tee firmware).
>>>>>>> - non-secure: hardware blocks are accessible by non-secure
>>>>>>> software (i.e.
>>>>>>> linux kernel).
>>>>>>> - coprocessor: hardware blocks are only accessible by the
>>>>>>> coprocessor.
>>>>>>> Up to 94 hardware blocks of the soc could be managed by ETZPC.
>>>>>>>
>>>>>> /me confused. Is ETZPC accessible from the non-secure kernel space to
>>>>>> begin with ? If so, is it allowed to configure hardware blocks as
>>>>>> secure
>>>>>> or trusted ? I am failing to understand the overall design of a
>>>>>> system
>>>>>> with ETZPC controller.
>>>>> Non-secure kernel could read the values set in ETZPC, if it doesn't
>>>>> match
>>>>> with what is required by the device node the driver won't be probed.
>>>>>
>>>> OK, but I was under the impression that it was made clear that Linux is
>>>> not firmware validation suite. The firmware need to ensure all the
>>>> devices
>>>> that are not accessible in the Linux kernel are marked as disabled and
>>>> this needs to happen before entering the kernel. So if this is what
>>>> this
>>>> patch series achieves, then there is no need for it. Please stop
>>>> pursuing
>>>> this any further or provide any other reasons(if any) to have it. Until
>>>> you have other reasons, NACK for this series.
>>>
>>> No it doesn't disable the nodes.
>>>
>>> When the firmware disable a node before the kernel that means it change
>>>
>>> the DTB and that is a problem when you want to sign it. With my proposal
>>>
>>> the DTB remains the same.
>>
>> ???
>>
>> :/
>>
>> The DTB is used to pass the kernel command line, memory reservations,
>> random seeds, and all manner of other things dynamically generated by
>> firmware at boot-time. Apologies for being blunt but if "changing the
>> DTB" is considered a problem then I can't help but think you're doing
>> it wrong.
>
> Yes but I would like to limit the number of cases where a firmware has
> to change the DTB.
>
> With this proposal nodes remain the same and embedded the firewall
> configuration(s).
>
> Until now firewall configuration is "static", the firmware disable (or
> remove) the nodes not accessible from Linux.
>
> If Linux can rely on node's firewall information it could allow switch
> dynamically an hardware block from Linux to a coprocessor.
>
> For example Linux could manage the display pipe configuration and when
> going to suspend handover the display hardware block to a coprocessor in
> charge a refreshing only some pixels.
OK, let's continue that example, would not it make sense then to just
steal the peripheral away from Linux by ensuring that Linux is no longer
running and the only thing that you need to make sure of is that either
you restore the HW in the exact same that you stole it from, or that
Linux is capable of refreshing its state against what the HW state was
left in?
If you have a set of display pipeline drivers, on your way to suspend,
you can define a protocol with the co-processor so as to signal an
ownership change, and the co-processor can take control from there.
In your example, it sounds like the firewall could be meant to detect
uncoordinated concurrent accesses to the same HW block between different
SW/FW entities. If that is the case, this is most likely a bug and you
can probably just get away with doing reporting instead of an entirely
new subsystem?
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-31 20:51 ` Florian Fainelli
@ 2020-02-03 13:41 ` Benjamin GAIGNARD
0 siblings, 0 replies; 34+ messages in thread
From: Benjamin GAIGNARD @ 2020-02-03 13:41 UTC (permalink / raw)
To: Florian Fainelli, Robin Murphy, Sudeep Holla
Cc: Mark Rutland, robh, Loic PALLARDY, arnd, devicetree, gregkh,
s.hauer, linux-kernel, broonie, lkml, linux-imx, kernel,
fabio.estevam, shawnguo, stefano.stabellini, system-dt,
linux-arm-kernel
On 1/31/20 9:51 PM, Florian Fainelli wrote:
> On 1/29/20 5:40 AM, Benjamin GAIGNARD wrote:
>> On 1/28/20 11:06 PM, Robin Murphy wrote:
>>> On 2020-01-28 8:06 pm, Benjamin GAIGNARD wrote:
>>>> On 1/28/20 6:17 PM, Sudeep Holla wrote:
>>>>> On Tue, Jan 28, 2020 at 04:46:41PM +0000, Benjamin GAIGNARD wrote:
>>>>>> On 1/28/20 5:36 PM, Sudeep Holla wrote:
>>>>>>> On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote:
>>>>>>>> Bus firewall framework aims to provide a kernel API to set the
>>>>>>>> configuration
>>>>>>>> of the harware blocks in charge of busses access control.
>>>>>>>>
>>>>>>>> Framework architecture is inspirated by pinctrl framework:
>>>>>>>> - a default configuration could be applied before bind the driver.
>>>>>>>> If a configuration could not be applied the driver is not bind
>>>>>>>> to avoid doing accesses on prohibited regions.
>>>>>>>> - configurations could be apllied dynamically by drivers.
>>>>>>>> - device node provides the bus firewall configurations.
>>>>>>>>
>>>>>>>> An example of bus firewall controller is STM32 ETZPC hardware block
>>>>>>>> which got 3 possible configurations:
>>>>>>>> - trust: hardware blocks are only accessible by software running
>>>>>>>> on trust
>>>>>>>> zone (i.e op-tee firmware).
>>>>>>>> - non-secure: hardware blocks are accessible by non-secure
>>>>>>>> software (i.e.
>>>>>>>> linux kernel).
>>>>>>>> - coprocessor: hardware blocks are only accessible by the
>>>>>>>> coprocessor.
>>>>>>>> Up to 94 hardware blocks of the soc could be managed by ETZPC.
>>>>>>>>
>>>>>>> /me confused. Is ETZPC accessible from the non-secure kernel space to
>>>>>>> begin with ? If so, is it allowed to configure hardware blocks as
>>>>>>> secure
>>>>>>> or trusted ? I am failing to understand the overall design of a
>>>>>>> system
>>>>>>> with ETZPC controller.
>>>>>> Non-secure kernel could read the values set in ETZPC, if it doesn't
>>>>>> match
>>>>>> with what is required by the device node the driver won't be probed.
>>>>>>
>>>>> OK, but I was under the impression that it was made clear that Linux is
>>>>> not firmware validation suite. The firmware need to ensure all the
>>>>> devices
>>>>> that are not accessible in the Linux kernel are marked as disabled and
>>>>> this needs to happen before entering the kernel. So if this is what
>>>>> this
>>>>> patch series achieves, then there is no need for it. Please stop
>>>>> pursuing
>>>>> this any further or provide any other reasons(if any) to have it. Until
>>>>> you have other reasons, NACK for this series.
>>>> No it doesn't disable the nodes.
>>>>
>>>> When the firmware disable a node before the kernel that means it change
>>>>
>>>> the DTB and that is a problem when you want to sign it. With my proposal
>>>>
>>>> the DTB remains the same.
>>> ???
>>>
>>> :/
>>>
>>> The DTB is used to pass the kernel command line, memory reservations,
>>> random seeds, and all manner of other things dynamically generated by
>>> firmware at boot-time. Apologies for being blunt but if "changing the
>>> DTB" is considered a problem then I can't help but think you're doing
>>> it wrong.
>> Yes but I would like to limit the number of cases where a firmware has
>> to change the DTB.
>>
>> With this proposal nodes remain the same and embedded the firewall
>> configuration(s).
>>
>> Until now firewall configuration is "static", the firmware disable (or
>> remove) the nodes not accessible from Linux.
>>
>> If Linux can rely on node's firewall information it could allow switch
>> dynamically an hardware block from Linux to a coprocessor.
>>
>> For example Linux could manage the display pipe configuration and when
>> going to suspend handover the display hardware block to a coprocessor in
>> charge a refreshing only some pixels.
> OK, let's continue that example, would not it make sense then to just
> steal the peripheral away from Linux by ensuring that Linux is no longer
> running and the only thing that you need to make sure of is that either
> you restore the HW in the exact same that you stole it from, or that
> Linux is capable of refreshing its state against what the HW state was
> left in?
>
> If you have a set of display pipeline drivers, on your way to suspend,
> you can define a protocol with the co-processor so as to signal an
> ownership change, and the co-processor can take control from there.
To handover a hardware block to the co-processor we need to inform it
and change the firewall configuration.
My proposal only aim to cover this last point by describing in the
device tree the possible configuration.
The example I had mind is how the pinctrl framework is working with it
states so doing something like changing firewall configuration and then
inform the co-processor in suspend function.
>
> In your example, it sounds like the firewall could be meant to detect
> uncoordinated concurrent accesses to the same HW block between different
> SW/FW entities. If that is the case, this is most likely a bug and you
> can probably just get away with doing reporting instead of an entirely
> new subsystem?
Prohibited accesses, most of the time, generate an abort on the bus so
your platform just crash and yes it is a bug.
This new subsystem won't change that, it only allow to describe and
dynamically set a configuration for DT information rather doing that for
type of firewall.
Benjamin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-28 20:06 ` Benjamin GAIGNARD
2020-01-28 22:06 ` Robin Murphy
@ 2020-01-31 20:48 ` Florian Fainelli
2020-02-02 12:23 ` Mark Brown
1 sibling, 1 reply; 34+ messages in thread
From: Florian Fainelli @ 2020-01-31 20:48 UTC (permalink / raw)
To: Benjamin GAIGNARD, Sudeep Holla
Cc: Mark Rutland, robh, Loic PALLARDY, arnd, devicetree, gregkh,
s.hauer, linux-kernel, broonie, lkml, linux-imx, kernel,
fabio.estevam, shawnguo, stefano.stabellini, system-dt,
linux-arm-kernel, Robin.Murphy
On 1/28/20 12:06 PM, Benjamin GAIGNARD wrote:
>
> On 1/28/20 6:17 PM, Sudeep Holla wrote:
>> On Tue, Jan 28, 2020 at 04:46:41PM +0000, Benjamin GAIGNARD wrote:
>>> On 1/28/20 5:36 PM, Sudeep Holla wrote:
>>>> On Tue, Jan 28, 2020 at 04:37:59PM +0100, Benjamin Gaignard wrote:
>>>>> Bus firewall framework aims to provide a kernel API to set the configuration
>>>>> of the harware blocks in charge of busses access control.
>>>>>
>>>>> Framework architecture is inspirated by pinctrl framework:
>>>>> - a default configuration could be applied before bind the driver.
>>>>> If a configuration could not be applied the driver is not bind
>>>>> to avoid doing accesses on prohibited regions.
>>>>> - configurations could be apllied dynamically by drivers.
>>>>> - device node provides the bus firewall configurations.
>>>>>
>>>>> An example of bus firewall controller is STM32 ETZPC hardware block
>>>>> which got 3 possible configurations:
>>>>> - trust: hardware blocks are only accessible by software running on trust
>>>>> zone (i.e op-tee firmware).
>>>>> - non-secure: hardware blocks are accessible by non-secure software (i.e.
>>>>> linux kernel).
>>>>> - coprocessor: hardware blocks are only accessible by the coprocessor.
>>>>> Up to 94 hardware blocks of the soc could be managed by ETZPC.
>>>>>
>>>> /me confused. Is ETZPC accessible from the non-secure kernel space to
>>>> begin with ? If so, is it allowed to configure hardware blocks as secure
>>>> or trusted ? I am failing to understand the overall design of a system
>>>> with ETZPC controller.
>>> Non-secure kernel could read the values set in ETZPC, if it doesn't match
>>> with what is required by the device node the driver won't be probed.
>>>
>> OK, but I was under the impression that it was made clear that Linux is
>> not firmware validation suite. The firmware need to ensure all the devices
>> that are not accessible in the Linux kernel are marked as disabled and
>> this needs to happen before entering the kernel. So if this is what this
>> patch series achieves, then there is no need for it. Please stop pursuing
>> this any further or provide any other reasons(if any) to have it. Until
>> you have other reasons, NACK for this series.
>
> No it doesn't disable the nodes.
>
> When the firmware disable a node before the kernel that means it change
>
> the DTB and that is a problem when you want to sign it. With my proposal
>
> the DTB remains the same.
Could you use an overlay then which is the result of the firewalling
results by your firewall block, which is smaller than the main SoC/board
DTB and can be easily audited not to accidentally enable blocks, but
only disable them by adding/changing the respective "status" property.
Worst case, your driver probes, has been firewalled and this is not
reflected in the DTB, you get a bus error, or a hang, or however it gets
implemented.
Like Robin and Sudeep here, I do not understand why the kernel should
have any business in this, let alone allowing blocks to change owners,
that sounds contrary to the purpose of a firewall being controlled under
an untrusted entity (Linux).
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/7] Introduce bus firewall controller framework
2020-01-31 20:48 ` Florian Fainelli
@ 2020-02-02 12:23 ` Mark Brown
0 siblings, 0 replies; 34+ messages in thread
From: Mark Brown @ 2020-02-02 12:23 UTC (permalink / raw)
To: Florian Fainelli
Cc: Mark Rutland, robh, stefano.stabellini, Benjamin GAIGNARD, arnd,
devicetree, gregkh, s.hauer, linux-kernel, lkml, linux-imx,
kernel, Sudeep Holla, fabio.estevam, shawnguo, Loic PALLARDY,
system-dt, linux-arm-kernel, Robin.Murphy
[-- Attachment #1.1: Type: text/plain, Size: 567 bytes --]
On Fri, Jan 31, 2020 at 12:48:33PM -0800, Florian Fainelli wrote:
> Like Robin and Sudeep here, I do not understand why the kernel should
> have any business in this, let alone allowing blocks to change owners,
> that sounds contrary to the purpose of a firewall being controlled under
> an untrusted entity (Linux).
Can we rely on there being a more trusted level of software than
Linux on a system? It wasn't standard to have anything on 32 bit
Arm systems as far as I remember so you could end up with some IP
blocks intended to support TrustZone sitting idle.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread