2023-09-29 18:46:45

by Gatien CHEVALLIER

[permalink] [raw]
Subject: [PATCH v5 00/11] Introduce STM32 Firewall framework

Introduce STM32 Firewall framework for STM32MP1x and STM32MP2x
platforms. STM32MP1x(ETZPC) and STM32MP2x(RIFSC) Firewall controllers
register to the framework to offer firewall services such as access
granting.

This series of patches is a new approach on the previous STM32 system
bus, history is available here:
https://lore.kernel.org/lkml/20230127164040.1047583/

The need for such framework arises from the fact that there are now
multiple hardware firewalls implemented across multiple products.
Drivers are shared between different products, using the same code.
When it comes to firewalls, the purpose mostly stays the same: Protect
hardware resources. But the implementation differs, and there are
multiple types of firewalls: peripheral, memory, ...

Some hardware firewall controllers such as the RIFSC implemented on
STM32MP2x platforms may require to take ownership of a resource before
being able to use it, hence the requirement for firewall services to
take/release the ownership of such resources.

On the other hand, hardware firewall configurations are becoming
more and more complex. These mecanisms prevent platform crashes
or other firewall-related incoveniences by denying access to some
resources.

The stm32 firewall framework offers an API that is defined in
firewall controllers drivers to best fit the specificity of each
firewall.

For every peripherals protected by either the ETZPC or the RIFSC, the
firewall framework checks the firewall controlelr registers to see if
the peripheral's access is granted to the Linux kernel. If not, the
peripheral is configured as secure, the node is marked populated,
so that the driver is not probed for that device.

The firewall framework relies on the access-controller device tree
binding. It is used by peripherals to reference a domain access
controller. In this case a firewall controller. The bus uses the ID
referenced by the access-controller property to know where to look
in the firewall to get the security configuration for the peripheral.
This allows a device tree description rather than a hardcoded peripheral
table in the bus driver.

The STM32 ETZPC device is responsible for filtering accesses based on
security level, or co-processor isolation for any resource connected
to it.

The RIFSC is responsible for filtering accesses based on Compartment
ID / security level / privilege level for any resource connected to
it.

STM32MP13/15/25 SoC device tree files are updated in this series to
implement this mecanism.

Changes in V5:
- Integrate and rework the "feature-domains" binding patch in
this patchset. The binding is renamed to "access-controller"
- Rename every feature-domain* reference to access-control*
ones
- Correct loop bug and missing select STM32_FIREWALL in 32-bit
platform Kconfig


Changes in V4:
- Fix typo in commit message and YAML check errors in
"dt-bindings: Document common device controller bindings"
Note: This patch should be ignored as stated in the cover
letter. I've done this to avoid errors on this series of
patch
- Correct code syntax/style issues reported by Simon Horman
- Added Jonathan's tag for IIO on the treewide patch

Changes in V3:

Change incorrect ordering for bindings commits leading
to an error while running
"make DT_CHECKER_FLAGS=-m dt_binding_check"

Changes in V2:

generic:
- Add fw_devlink dependency for "feature-domains"
property.

bindings:
- Corrected YAMLS errors highlighted by Rob's robot
- Firewall controllers YAMLs no longer define the
maxItems for the "feature-domains" property
- Renamed st,stm32-rifsc.yaml to
st,stm32mp25-rifsc.yaml
- Fix examples in YAML files
- Change feature-domains maxItems to 2 in firewall
consumer files as there should not be more than
2 entries for now
- Declare "feature-domain-names" as an optional
property for firewall controllers child nodes.
- Add missing "feature-domains" property declaration
in bosch,m_can.yaml and st,stm32-cryp.yaml files

firewall framework:
- Support multiple entries for "feature-domains"
property
- Better handle the device-tree parsing using
phandle+args APIs
- Remove "resource firewall" type
- Add a field for the name of the firewall entry
- Fix licenses

RIFSC:
- Add controller name
- Driver is now a module_platform_driver
- Fix license

ETZPC:
- Add controller name
- Driver is now a module_platform_driver
- Fix license

Device trees:
- Fix rifsc node name
- Move the "ranges" property under the
"feature-domains" one

Gatien Chevallier (10):
dt-bindings: treewide: add access-controller description
dt-bindings: bus: document RIFSC
dt-bindings: bus: document ETZPC
firewall: introduce stm32_firewall framework
of: property: fw_devlink: Add support for "access-controller"
bus: rifsc: introduce RIFSC firewall controller driver
arm64: dts: st: add RIFSC as an access controller for STM32MP25x
boards
bus: etzpc: introduce ETZPC firewall controller driver
ARM: dts: stm32: add ETZPC as a system bus for STM32MP15x boards
ARM: dts: stm32: add ETZPC as a system bus for STM32MP13x boards

Oleksii Moisieiev (1):
dt-bindings: document generic access controller

.../access-controllers/access-controller.yaml | 90 +
.../bindings/bus/st,stm32-etzpc.yaml | 96 +
.../bindings/bus/st,stm32mp25-rifsc.yaml | 105 +
.../bindings/crypto/st,stm32-cryp.yaml | 4 +
.../bindings/crypto/st,stm32-hash.yaml | 4 +
.../devicetree/bindings/dma/st,stm32-dma.yaml | 4 +
.../bindings/dma/st,stm32-dmamux.yaml | 4 +
.../devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 +
.../bindings/iio/adc/st,stm32-adc.yaml | 4 +
.../bindings/iio/adc/st,stm32-dfsdm-adc.yaml | 4 +
.../bindings/iio/dac/st,stm32-dac.yaml | 4 +
.../bindings/media/cec/st,stm32-cec.yaml | 4 +
.../bindings/media/st,stm32-dcmi.yaml | 4 +
.../memory-controllers/st,stm32-fmc2-ebi.yaml | 4 +
.../bindings/mfd/st,stm32-lptimer.yaml | 4 +
.../bindings/mfd/st,stm32-timers.yaml | 4 +
.../devicetree/bindings/mmc/arm,pl18x.yaml | 4 +
.../bindings/net/can/bosch,m_can.yaml | 4 +
.../devicetree/bindings/net/stm32-dwmac.yaml | 4 +
.../bindings/phy/phy-stm32-usbphyc.yaml | 4 +
.../bindings/regulator/st,stm32-vrefbuf.yaml | 4 +
.../devicetree/bindings/rng/st,stm32-rng.yaml | 4 +
.../bindings/serial/st,stm32-uart.yaml | 4 +
.../bindings/sound/st,stm32-i2s.yaml | 4 +
.../bindings/sound/st,stm32-sai.yaml | 4 +
.../bindings/sound/st,stm32-spdifrx.yaml | 4 +
.../bindings/spi/st,stm32-qspi.yaml | 4 +
.../devicetree/bindings/spi/st,stm32-spi.yaml | 4 +
.../devicetree/bindings/usb/dwc2.yaml | 4 +
MAINTAINERS | 7 +
arch/arm/boot/dts/st/stm32mp131.dtsi | 1026 +++---
arch/arm/boot/dts/st/stm32mp133.dtsi | 51 +-
arch/arm/boot/dts/st/stm32mp13xc.dtsi | 19 +-
arch/arm/boot/dts/st/stm32mp13xf.dtsi | 19 +-
arch/arm/boot/dts/st/stm32mp151.dtsi | 2757 +++++++++--------
arch/arm/boot/dts/st/stm32mp153.dtsi | 52 +-
arch/arm/boot/dts/st/stm32mp15xc.dtsi | 19 +-
arch/arm/mach-stm32/Kconfig | 1 +
arch/arm64/Kconfig.platforms | 1 +
arch/arm64/boot/dts/st/stm32mp251.dtsi | 7 +-
drivers/bus/Kconfig | 9 +
drivers/bus/Makefile | 1 +
drivers/bus/stm32_etzpc.c | 141 +
drivers/bus/stm32_firewall.c | 294 ++
drivers/bus/stm32_firewall.h | 83 +
drivers/bus/stm32_rifsc.c | 252 ++
drivers/of/property.c | 2 +
include/linux/bus/stm32_firewall_device.h | 141 +
48 files changed, 3358 insertions(+), 1919 deletions(-)
create mode 100644 Documentation/devicetree/bindings/access-controllers/access-controller.yaml
create mode 100644 Documentation/devicetree/bindings/bus/st,stm32-etzpc.yaml
create mode 100644 Documentation/devicetree/bindings/bus/st,stm32mp25-rifsc.yaml
create mode 100644 drivers/bus/stm32_etzpc.c
create mode 100644 drivers/bus/stm32_firewall.c
create mode 100644 drivers/bus/stm32_firewall.h
create mode 100644 drivers/bus/stm32_rifsc.c
create mode 100644 include/linux/bus/stm32_firewall_device.h

--
2.25.1


2023-09-29 19:59:56

by Gatien CHEVALLIER

[permalink] [raw]
Subject: [PATCH v5 06/11] of: property: fw_devlink: Add support for "access-controller"

Allows tracking dependencies between devices and their access
controller.

Signed-off-by: Gatien Chevallier <[email protected]>
---

Changes in V5:
- Rename feature-domain* to access-control*

Patch not present in V1

drivers/of/property.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index cf8dacf3e3b8..c86521fac06d 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1267,6 +1267,7 @@ DEFINE_SIMPLE_PROP(resets, "resets", "#reset-cells")
DEFINE_SIMPLE_PROP(leds, "leds", NULL)
DEFINE_SIMPLE_PROP(backlight, "backlight", NULL)
DEFINE_SIMPLE_PROP(panel, "panel", NULL)
+DEFINE_SIMPLE_PROP(access_controller, "access-controller", "#access-controller-cells")
DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")

@@ -1361,6 +1362,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
{ .parse_prop = parse_regulators, },
{ .parse_prop = parse_gpio, },
{ .parse_prop = parse_gpios, },
+ { .parse_prop = parse_access_controller, },
{}
};

--
2.25.1

2023-09-29 20:38:47

by Gatien CHEVALLIER

[permalink] [raw]
Subject: [PATCH v5 05/11] firewall: introduce stm32_firewall framework

Introduce a STM32 firewall framework that offers to firewall consumers
different firewall services such as the ability to check their access
rights against their firewall controller(s).

The STM32 firewall framework offers a generic API for STM32 firewall
controllers that is defined in their drivers to best fit the
specificity of each firewall.

There are various types of firewalls:
-Peripheral firewalls that filter accesses to peripherals
-Memory firewalls that filter accesses to memories or memory regions
-No type for undefined type of firewall

Signed-off-by: Gatien Chevallier <[email protected]>
---

Changes in V5:
- Rename feature-domain* to access-control*
- Fix index out of range in stm32_firewall_get_firewall()
- Add mising "select STM32_FIREWALL" in config ARCH_STM32 in
arch/arm/mach-stm32/Kconfig

Changes in V4:
- Fix documentation syntax
- Put node in case of error in stm32_firewall_populate_bus()
- Check firewall controller presence before using it in
stm32_firewall_controller_register()

Changes in V2:
- Support multiple entries for "feature-domains"
property. Change stm32_firewall_get_firewall()
to do so.
- Better handle the device-tree parsing using
phandle+args APIs and phandle iterator.
- Remove "resource firewall" type
- Rephrase STM32_FIREWALL description in the bus
Kconfig and remove useless default value
- Add missing EXPORT_SYMBOL_GPL
- Remove useless stm32_firewall_get_id() API
- Add a field for the name of the firewall entry
in the stm32_firewall_controller structure
- Minor fixes on some traces level (err -> debug)
- Fix licenses to GPL-2.0-only
- Fix stm32_firewall_get_firewall() description
- Rephrase some sentences to better emphasize this
is STM32-specific
- Switch to list_for_each_entry() in
stm32_firewall_controller_unregister() as we only
expect one firewall controller instance per device.
The loop just breaks when found
- Do not register the firewall controller if it is
already registered to the framework

MAINTAINERS | 5 +
arch/arm/mach-stm32/Kconfig | 1 +
arch/arm64/Kconfig.platforms | 1 +
drivers/bus/Kconfig | 9 +
drivers/bus/Makefile | 1 +
drivers/bus/stm32_firewall.c | 294 ++++++++++++++++++++++
drivers/bus/stm32_firewall.h | 83 ++++++
include/linux/bus/stm32_firewall_device.h | 141 +++++++++++
8 files changed, 535 insertions(+)
create mode 100644 drivers/bus/stm32_firewall.c
create mode 100644 drivers/bus/stm32_firewall.h
create mode 100644 include/linux/bus/stm32_firewall_device.h

diff --git a/MAINTAINERS b/MAINTAINERS
index b19995690904..c2c362929673 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20299,6 +20299,11 @@ T: git git://linuxtv.org/media_tree.git
F: Documentation/devicetree/bindings/media/i2c/st,st-mipid02.yaml
F: drivers/media/i2c/st-mipid02.c

+ST STM32 FIREWALL
+M: Gatien Chevallier <[email protected]>
+S: Maintained
+F: drivers/bus/stm32_firewall.c
+
ST STM32 I2C/SMBUS DRIVER
M: Pierre-Yves MORDRET <[email protected]>
M: Alain Volmat <[email protected]>
diff --git a/arch/arm/mach-stm32/Kconfig b/arch/arm/mach-stm32/Kconfig
index 98145031586f..ae21a9f78f9c 100644
--- a/arch/arm/mach-stm32/Kconfig
+++ b/arch/arm/mach-stm32/Kconfig
@@ -12,6 +12,7 @@ menuconfig ARCH_STM32
select PINCTRL
select RESET_CONTROLLER
select STM32_EXTI
+ select STM32_FIREWALL
help
Support for STMicroelectronics STM32 processors.

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 6069120199bb..5a46e90f1e4e 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -293,6 +293,7 @@ config ARCH_STM32
select ARM_SMC_MBOX
select ARM_SCMI_PROTOCOL
select COMMON_CLK_SCMI
+ select STM32_FIREWALL
help
This enables support for ARMv8 based STMicroelectronics
STM32 family, including:
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index c98dd6ca2629..6c8d0725f193 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -163,6 +163,15 @@ config QCOM_SSC_BLOCK_BUS
i2c/spi/uart controllers, a hexagon core, and a clock controller
which provides clocks for the above.

+config STM32_FIREWALL
+ bool "STM32 Firewall framework"
+ depends on ARCH_STM32 || COMPILE_TEST
+ help
+ Say y to enable STM32 firewall framework and its services. Firewall
+ controllers will be able to register to the framework. Access for
+ hardware resources linked to a firewall controller can be requested
+ through this STM32 framework.
+
config SUN50I_DE2_BUS
bool "Allwinner A64 DE2 Bus Driver"
default ARM64
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index d90eed189a65..fc0511450ec2 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_OMAP_INTERCONNECT) += omap_l3_smx.o omap_l3_noc.o
obj-$(CONFIG_OMAP_OCP2SCP) += omap-ocp2scp.o
obj-$(CONFIG_QCOM_EBI2) += qcom-ebi2.o
obj-$(CONFIG_QCOM_SSC_BLOCK_BUS) += qcom-ssc-block-bus.o
+obj-$(CONFIG_STM32_FIREWALL) += stm32_firewall.o
obj-$(CONFIG_SUN50I_DE2_BUS) += sun50i-de2.o
obj-$(CONFIG_SUNXI_RSB) += sunxi-rsb.o
obj-$(CONFIG_OF) += simple-pm-bus.o
diff --git a/drivers/bus/stm32_firewall.c b/drivers/bus/stm32_firewall.c
new file mode 100644
index 000000000000..3cd0d3ff9f6c
--- /dev/null
+++ b/drivers/bus/stm32_firewall.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023, STMicroelectronics - All Rights Reserved
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/bus/stm32_firewall_device.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+
+#include "stm32_firewall.h"
+
+/* Corresponds to STM32_FIREWALL_MAX_EXTRA_ARGS + firewall ID */
+#define STM32_FIREWALL_MAX_ARGS (STM32_FIREWALL_MAX_EXTRA_ARGS + 1)
+
+static LIST_HEAD(firewall_controller_list);
+static DEFINE_MUTEX(firewall_controller_list_lock);
+
+/* Firewall device API */
+
+int stm32_firewall_get_firewall(struct device_node *np, struct stm32_firewall *firewall,
+ unsigned int nb_firewall)
+{
+ struct stm32_firewall_controller *ctrl;
+ struct of_phandle_iterator it;
+ unsigned int i, j = 0;
+ int err;
+
+ if (!firewall || !nb_firewall)
+ return -EINVAL;
+
+ /* Parse property with phandle parsed out */
+ of_for_each_phandle(&it, err, np, "access-controller", "#access-controller-cells", 0) {
+ struct of_phandle_args provider_args;
+ struct device_node *provider = it.node;
+ const char *fw_entry;
+ bool match = false;
+
+ if (err) {
+ pr_err("Unable to get access-controller property for node %s\n, err: %d",
+ np->full_name, err);
+ of_node_put(provider);
+ return err;
+ }
+
+ if (j > nb_firewall) {
+ pr_err("Too many firewall controllers");
+ of_node_put(provider);
+ return -EINVAL;
+ }
+
+ provider_args.args_count = of_phandle_iterator_args(&it, provider_args.args,
+ STM32_FIREWALL_MAX_ARGS);
+
+ /* Check if the parsed phandle corresponds to a registered firewall controller */
+ mutex_lock(&firewall_controller_list_lock);
+ list_for_each_entry(ctrl, &firewall_controller_list, entry) {
+ if (ctrl->dev->of_node->phandle == it.phandle) {
+ match = true;
+ firewall[j].firewall_ctrl = ctrl;
+ break;
+ }
+ }
+ mutex_unlock(&firewall_controller_list_lock);
+
+ if (!match) {
+ firewall[j].firewall_ctrl = NULL;
+ pr_err("No firewall controller registered for %s\n", np->full_name);
+ of_node_put(provider);
+ return -ENODEV;
+ }
+
+ err = of_property_read_string_index(np, "access-controller-names", j, &fw_entry);
+ if (err == 0)
+ firewall[j].entry = fw_entry;
+
+ /* Handle the case when there are no arguments given along with the phandle */
+ if (provider_args.args_count < 0 ||
+ provider_args.args_count > STM32_FIREWALL_MAX_ARGS) {
+ of_node_put(provider);
+ return -EINVAL;
+ } else if (provider_args.args_count == 0) {
+ firewall[j].extra_args_size = 0;
+ firewall[j].firewall_id = U32_MAX;
+ j++;
+ continue;
+ }
+
+ /* The firewall ID is always the first argument */
+ firewall[j].firewall_id = provider_args.args[0];
+
+ /* Extra args start at the second argument */
+ for (i = 0; i < provider_args.args_count - 1; i++)
+ firewall[j].extra_args[i] = provider_args.args[i + 1];
+
+ /* Remove the firewall ID arg that is not an extra argument */
+ firewall[j].extra_args_size = provider_args.args_count - 1;
+
+ j++;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(stm32_firewall_get_firewall);
+
+int stm32_firewall_grant_access(struct stm32_firewall *firewall)
+{
+ struct stm32_firewall_controller *firewall_controller;
+
+ if (!firewall || firewall->firewall_id == U32_MAX)
+ return -EINVAL;
+
+ firewall_controller = firewall->firewall_ctrl;
+
+ if (!firewall_controller)
+ return -ENODEV;
+
+ return firewall_controller->grant_access(firewall_controller, firewall->firewall_id);
+}
+EXPORT_SYMBOL_GPL(stm32_firewall_grant_access);
+
+int stm32_firewall_grant_access_by_id(struct stm32_firewall *firewall, u32 subsystem_id)
+{
+ struct stm32_firewall_controller *firewall_controller;
+
+ if (!firewall || subsystem_id == U32_MAX || firewall->firewall_id == U32_MAX)
+ return -EINVAL;
+
+ firewall_controller = firewall->firewall_ctrl;
+
+ if (!firewall_controller)
+ return -ENODEV;
+
+ return firewall_controller->grant_access(firewall_controller, subsystem_id);
+}
+EXPORT_SYMBOL_GPL(stm32_firewall_grant_access_by_id);
+
+void stm32_firewall_release_access(struct stm32_firewall *firewall)
+{
+ struct stm32_firewall_controller *firewall_controller;
+
+ if (!firewall || firewall->firewall_id == U32_MAX) {
+ pr_debug("Incorrect arguments when releasing a firewall access\n");
+ return;
+ }
+
+ firewall_controller = firewall->firewall_ctrl;
+
+ if (!firewall_controller) {
+ pr_debug("No firewall controller to release\n");
+ return;
+ }
+
+ firewall_controller->release_access(firewall_controller, firewall->firewall_id);
+}
+EXPORT_SYMBOL_GPL(stm32_firewall_release_access);
+
+void stm32_firewall_release_access_by_id(struct stm32_firewall *firewall, u32 subsystem_id)
+{
+ struct stm32_firewall_controller *firewall_controller;
+
+ if (!firewall || subsystem_id == U32_MAX || firewall->firewall_id == U32_MAX) {
+ pr_debug("Incorrect arguments when releasing a firewall access");
+ return;
+ }
+
+ firewall_controller = firewall->firewall_ctrl;
+
+ if (!firewall_controller) {
+ pr_debug("No firewall controller to release");
+ return;
+ }
+
+ firewall_controller->release_access(firewall_controller, subsystem_id);
+}
+EXPORT_SYMBOL_GPL(stm32_firewall_release_access_by_id);
+
+/* Firewall controller API */
+
+int stm32_firewall_controller_register(struct stm32_firewall_controller *firewall_controller)
+{
+ struct stm32_firewall_controller *ctrl;
+
+ if (!firewall_controller)
+ return -ENODEV;
+
+ pr_info("Registering %s firewall controller\n", firewall_controller->name);
+
+ mutex_lock(&firewall_controller_list_lock);
+ list_for_each_entry(ctrl, &firewall_controller_list, entry) {
+ if (ctrl == firewall_controller) {
+ pr_debug("%s firewall controller already registered\n",
+ firewall_controller->name);
+ mutex_unlock(&firewall_controller_list_lock);
+ return 0;
+ }
+ }
+ list_add_tail(&firewall_controller->entry, &firewall_controller_list);
+ mutex_unlock(&firewall_controller_list_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(stm32_firewall_controller_register);
+
+void stm32_firewall_controller_unregister(struct stm32_firewall_controller *firewall_controller)
+{
+ struct stm32_firewall_controller *ctrl;
+ bool controller_removed = false;
+
+ if (!firewall_controller) {
+ pr_debug("Null reference while unregistering firewall controller\n");
+ return;
+ }
+
+ mutex_lock(&firewall_controller_list_lock);
+ list_for_each_entry(ctrl, &firewall_controller_list, entry) {
+ if (ctrl == firewall_controller) {
+ controller_removed = true;
+ list_del_init(&ctrl->entry);
+ break;
+ }
+ }
+ mutex_unlock(&firewall_controller_list_lock);
+
+ if (!controller_removed)
+ pr_debug("There was no firewall controller named %s to unregister\n",
+ firewall_controller->name);
+}
+EXPORT_SYMBOL_GPL(stm32_firewall_controller_unregister);
+
+int stm32_firewall_populate_bus(struct stm32_firewall_controller *firewall_controller)
+{
+ struct stm32_firewall *firewalls;
+ struct device_node *child;
+ struct device *parent;
+ unsigned int i;
+ int len;
+ int err;
+
+ parent = firewall_controller->dev;
+
+ dev_dbg(parent, "Populating %s system bus\n", dev_name(firewall_controller->dev));
+
+ for_each_available_child_of_node(dev_of_node(parent), child) {
+ /* The access-controller property is mandatory for firewall bus devices */
+ len = of_count_phandle_with_args(child, "access-controller",
+ "#access-controller-cells");
+ if (len <= 0) {
+ of_node_put(child);
+ return -EINVAL;
+ }
+
+ firewalls = kcalloc(len, sizeof(*firewalls), GFP_KERNEL);
+ if (!firewalls) {
+ of_node_put(child);
+ return -ENOMEM;
+ }
+
+ err = stm32_firewall_get_firewall(child, firewalls, (unsigned int)len);
+ if (err) {
+ kfree(firewalls);
+ of_node_put(child);
+ return err;
+ }
+
+ for (i = 0; i < len; i++) {
+ if (firewall_controller->grant_access(firewall_controller,
+ firewalls[i].firewall_id)) {
+ /*
+ * Peripheral access not allowed or not defined.
+ * Mark the node as populated so platform bus won't probe it
+ */
+ of_node_set_flag(child, OF_POPULATED);
+ dev_err(parent, "%s: Device driver will not be probed\n",
+ child->full_name);
+ }
+ }
+
+ kfree(firewalls);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(stm32_firewall_populate_bus);
diff --git a/drivers/bus/stm32_firewall.h b/drivers/bus/stm32_firewall.h
new file mode 100644
index 000000000000..e5fac85fe346
--- /dev/null
+++ b/drivers/bus/stm32_firewall.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023, STMicroelectronics - All Rights Reserved
+ */
+
+#ifndef _STM32_FIREWALL_H
+#define _STM32_FIREWALL_H
+
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+/**
+ * STM32_PERIPHERAL_FIREWALL: This type of firewall protects peripherals
+ * STM32_MEMORY_FIREWALL: This type of firewall protects memories/subsets of memory
+ * zones
+ * STM32_NOTYPE_FIREWALL: Undefined firewall type
+ */
+
+#define STM32_PERIPHERAL_FIREWALL BIT(1)
+#define STM32_MEMORY_FIREWALL BIT(2)
+#define STM32_NOTYPE_FIREWALL BIT(3)
+
+/**
+ * struct stm32_firewall_controller - Information on firewall controller supplying services
+ *
+ * @name: Name of the firewall controller
+ * @dev: Device reference of the firewall controller
+ * @mmio: Base address of the firewall controller
+ * @entry: List entry of the firewall controller list
+ * @type: Type of firewall
+ * @max_entries: Number of entries covered by the firewall
+ * @grant_access: Callback used to grant access for a device access against a
+ * firewall controller
+ * @release_access: Callback used to release resources taken by a device when access was
+ * granted
+ * @grant_memory_range_access: Callback used to grant access for a device to a given memory region
+ */
+struct stm32_firewall_controller {
+ const char *name;
+ struct device *dev;
+ void __iomem *mmio;
+ struct list_head entry;
+ unsigned int type;
+ unsigned int max_entries;
+
+ int (*grant_access)(struct stm32_firewall_controller *ctrl, u32 id);
+ void (*release_access)(struct stm32_firewall_controller *ctrl, u32 id);
+ int (*grant_memory_range_access)(struct stm32_firewall_controller *ctrl, phys_addr_t paddr,
+ size_t size);
+};
+
+/**
+ * stm32_firewall_controller_register - Register a firewall controller to the STM32 firewall
+ * framework
+ * @firewall_controller: Firewall controller to register
+ *
+ * Returns 0 in case of success or -ENODEV if no controller was given.
+ */
+int stm32_firewall_controller_register(struct stm32_firewall_controller *firewall_controller);
+
+/**
+ * stm32_firewall_controller_unregister - Unregister a firewall controller from the STM32
+ * firewall framework
+ * @firewall_controller: Firewall controller to unregister
+ */
+void stm32_firewall_controller_unregister(struct stm32_firewall_controller *firewall_controller);
+
+/**
+ * stm32_firewall_populate_bus - Populate device tree nodes that have a correct firewall
+ * configuration. This is used at boot-time only, as a sanity check
+ * between device tree and firewalls hardware configurations to
+ * prevent a kernel crash when a device driver is not granted access
+ *
+ * @firewall_controller: Firewall controller which nodes will be populated or not
+ *
+ * Returns 0 in case of success or appropriate errno code if error occurred.
+ */
+int stm32_firewall_populate_bus(struct stm32_firewall_controller *firewall_controller);
+
+#endif /* _STM32_FIREWALL_H */
diff --git a/include/linux/bus/stm32_firewall_device.h b/include/linux/bus/stm32_firewall_device.h
new file mode 100644
index 000000000000..bbbee0b24ea8
--- /dev/null
+++ b/include/linux/bus/stm32_firewall_device.h
@@ -0,0 +1,141 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023, STMicroelectronics - All Rights Reserved
+ */
+
+#ifndef STM32_FIREWALL_DEVICE_H
+#define STM32_FIREWALL_DEVICE_H
+
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#define STM32_FIREWALL_MAX_EXTRA_ARGS 5
+
+/* Opaque reference to stm32_firewall_controller */
+struct stm32_firewall_controller;
+
+/**
+ * struct stm32_firewall - Information on a device's firewall. Each device can have more than one
+ * firewall.
+ *
+ * @firewall_ctrl: Pointer referencing a firewall controller of the device. It is
+ * opaque so a device cannot manipulate the controller's ops or access
+ * the controller's data
+ * @extra_args: Extra arguments that are implementation dependent
+ * @entry: Name of the firewall entry
+ * @extra_args_size: Number of extra arguments
+ * @firewall_id: Firewall ID associated the device for this firewall controller
+ */
+struct stm32_firewall {
+ struct stm32_firewall_controller *firewall_ctrl;
+ u32 extra_args[STM32_FIREWALL_MAX_EXTRA_ARGS];
+ const char *entry;
+ size_t extra_args_size;
+ u32 firewall_id;
+};
+
+#if IS_ENABLED(CONFIG_STM32_FIREWALL)
+/**
+ * stm32_firewall_get_firewall - Get the firewall(s) associated to given device.
+ * The firewall controller reference is always the first argument
+ * of each of the access-controller property entries.
+ * The firewall ID is always the second argument of each of the
+ * access-controller property entries.
+ * If there's no argument linked to the phandle, then the firewall ID
+ * field is set to U32_MAX, which is an invalid ID.
+ *
+ * @np: Device node to parse
+ * @firewall: Array of firewall references
+ * @nb_firewall: Number of firewall references to get. Must be at least 1.
+ *
+ * Returns 0 on success, -ENODEV if there's no match with a firewall controller or appropriate errno
+ * code if error occurred.
+ */
+int stm32_firewall_get_firewall(struct device_node *np, struct stm32_firewall *firewall,
+ unsigned int nb_firewall);
+
+/**
+ * stm32_firewall_grant_access - Request firewall access rights and grant access.
+ *
+ * @firewall: Firewall reference containing the ID to check against its firewall
+ * controller
+ *
+ * Returns 0 if access is granted, -EACCES if access is denied, -ENODEV if firewall is null or
+ * appropriate errno code if error occurred
+ */
+int stm32_firewall_grant_access(struct stm32_firewall *firewall);
+
+/**
+ * stm32_firewall_release_access - Release access granted from a call to
+ * stm32_firewall_grant_access().
+ *
+ * @firewall: Firewall reference containing the ID to check against its firewall
+ * controller
+ */
+void stm32_firewall_release_access(struct stm32_firewall *firewall);
+
+/**
+ * stm32_firewall_grant_access_by_id - Request firewall access rights of a given device
+ * based on a specific firewall ID
+ *
+ * Warnings:
+ * There is no way to ensure that the given ID will correspond to the firewall referenced in the
+ * device node if the ID did not come from stm32_firewall_get_firewall(). In that case, this
+ * function must be used with caution.
+ * This function should be used for subsystem resources that do not have the same firewall ID
+ * as their parent.
+ * U32_MAX is an invalid ID.
+ *
+ * @firewall: Firewall reference containing the firewall controller
+ * @subsystem_id: Firewall ID of the subsystem resource
+ *
+ * Returns 0 if access is granted, -EACCES if access is denied, -ENODEV if firewall is null or
+ * appropriate errno code if error occurred
+ */
+int stm32_firewall_grant_access_by_id(struct stm32_firewall *firewall, u32 subsystem_id);
+
+/**
+ * stm32_firewall_release_access_by_id - Release access granted from a call to
+ * stm32_firewall_grant_access_by_id().
+ *
+ * Warnings:
+ * There is no way to ensure that the given ID will correspond to the firewall referenced in the
+ * device node if the ID did not come from stm32_firewall_get_firewall(). In that case, this
+ * function must be used with caution.
+ * This function should be used for subsystem resources that do not have the same firewall ID
+ * as their parent.
+ * U32_MAX is an invalid ID.
+ *
+ * @firewall: Firewall reference containing the firewall controller
+ * @subsystem_id: Firewall ID of the subsystem resource
+ */
+void stm32_firewall_release_access_by_id(struct stm32_firewall *firewall, u32 subsystem_id);
+
+#else /* CONFIG_STM32_FIREWALL */
+
+int stm32_firewall_get_firewall(struct device_node *np, struct stm32_firewall *firewall)
+{
+ return -ENODEV;
+}
+
+int stm32_firewall_grant_access(struct stm32_firewall *firewall)
+{
+ return -ENODEV;
+}
+
+void stm32_firewall_release_access(struct stm32_firewall *firewall)
+{
+}
+
+int stm32_firewall_grant_access_by_id(struct stm32_firewall *firewall, u32 subsystem_id)
+{
+ return -ENODEV;
+}
+
+void stm32_firewall_release_access_by_id(struct stm32_firewall *firewall, u32 subsystem_id)
+{
+}
+
+#endif /* CONFIG_STM32_FIREWALL */
+#endif /* STM32_FIREWALL_DEVICE_H */
--
2.25.1

2023-09-29 21:56:43

by Gatien CHEVALLIER

[permalink] [raw]
Subject: [PATCH v5 01/11] dt-bindings: document generic access controller

From: Oleksii Moisieiev <[email protected]>

Introducing of the generic access controller bindings for the
access controller provider and consumer devices. Those bindings are
intended to allow a better handling of accesses to resources in a
hardware architecture supporting several compartments.

This patch is based on [1]. It is integrated in this patchset as it
provides a use-case for it.

Diffs with [1]:
- Rename feature-domain* properties to access-control* to narrow
down the scope of the binding
- YAML errors and typos corrected.
- Example updated
- Some rephrasing in the binding description

[1]: https://lore.kernel.org/lkml/0c0a82bb-18ae-d057-562b

Signed-off-by: Oleksii Moisieiev <[email protected]>
Signed-off-by: Gatien Chevallier <[email protected]>

---
Changes in V5:
- Diffs with [1]
- Discarded the [IGNORE] tag as the patch is now part of the
patchset

.../access-controllers/access-controller.yaml | 90 +++++++++++++++++++
1 file changed, 90 insertions(+)
create mode 100644 Documentation/devicetree/bindings/access-controllers/access-controller.yaml

diff --git a/Documentation/devicetree/bindings/access-controllers/access-controller.yaml b/Documentation/devicetree/bindings/access-controllers/access-controller.yaml
new file mode 100644
index 000000000000..9d305fccc333
--- /dev/null
+++ b/Documentation/devicetree/bindings/access-controllers/access-controller.yaml
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/access-controllers/access-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic Domain Access Controller
+
+maintainers:
+ - Oleksii Moisieiev <[email protected]>
+
+description: |+
+ Common access controllers properties
+
+ Access controllers are in charge of stating which of the hardware blocks under
+ their responsibility (their domain) can be accesssed by which compartment. A
+ compartment can be a cluster of CPUs (or coprocessors), a range of addresses
+ or a group of hardware blocks. An access controller's domain is the set of
+ resources covered by the access controller.
+
+ This device tree bindings can be used to bind devices to their access
+ controller provided by access-controller property. In this case, the device is
+ a consumer and the access controller is the provider.
+
+ An access controller can be represented by any node in the device tree and
+ can provide one or more configuration parameters, needed to control parameters
+ of the consumer device. A consumer node can refer to the provider by phandle
+ and a set of phandle arguments, specified by '#access-controller-cells'
+ property in the access controller node.
+
+ Access controllers are typically used to set/read the permissions of a
+ hardware block and grant access to it. Any of which depends on the access
+ controller. The capabilities of each access controller are defined by the
+ binding of the access controller device.
+
+ Each node can be a consumer for the several access controllers.
+
+# always select the core schema
+select: true
+
+properties:
+ "#access-controller-cells":
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Number of cells in a access-controller specifier;
+ Can be any value as specified by device tree binding documentation
+ of a particular provider.
+
+ access-control-provider:
+ description:
+ Indicates that the node is an access controller.
+
+ access-controller-names:
+ $ref: /schemas/types.yaml#/definitions/string-array
+ description:
+ A list of access-controller names, sorted in the same order as
+ access-controller entries. Consumer drivers will use
+ access-controller-names to match with existing access-controller entries.
+
+ access-controller:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description:
+ A list of access controller specifiers, as defined by the
+ bindings of the access-controller provider.
+
+additionalProperties: true
+
+examples:
+ - |
+ uart_controller: access-controller@50000 {
+ reg = <0x50000 0x10>;
+ access-control-provider;
+ #access-controller-cells = <2>;
+ };
+
+ bus_controller: bus@60000 {
+ reg = <0x60000 0x10000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ access-control-provider;
+ #access-controller-cells = <3>;
+
+ uart4: serial@60100 {
+ reg = <0x60100 0x400>;
+ access-controller = <&uart_controller 1 2>,
+ <&bus_controller 1 3 5>;
+ access-controller-names = "controller", "bus-controller";
+ };
+ };
--
2.25.1

2023-09-30 02:38:48

by Gatien CHEVALLIER

[permalink] [raw]
Subject: [PATCH v5 09/11] bus: etzpc: introduce ETZPC firewall controller driver

ETZPC is a peripheral and memory firewall controller that filter accesses
based on Arm TrustZone secure state and Arm CPU privilege execution level.
It handles MCU isolation as well.

Signed-off-by: Gatien Chevallier <[email protected]>
---

Changes in V2:
- Add controller name
- Driver is now a module_platform_driver
- Use error code returned by stm32_firewall_populate_bus()
- Fix license

MAINTAINERS | 1 +
drivers/bus/Makefile | 2 +-
drivers/bus/stm32_etzpc.c | 141 ++++++++++++++++++++++++++++++++++++++
3 files changed, 143 insertions(+), 1 deletion(-)
create mode 100644 drivers/bus/stm32_etzpc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 47e360fd715a..a2475be8fb89 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20302,6 +20302,7 @@ F: drivers/media/i2c/st-mipid02.c
ST STM32 FIREWALL
M: Gatien Chevallier <[email protected]>
S: Maintained
+F: drivers/bus/stm32_etzpc.c
F: drivers/bus/stm32_firewall.c
F: drivers/bus/stm32_rifsc.c

diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index e50d18e1d141..cddd4984d6af 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -26,7 +26,7 @@ obj-$(CONFIG_OMAP_INTERCONNECT) += omap_l3_smx.o omap_l3_noc.o
obj-$(CONFIG_OMAP_OCP2SCP) += omap-ocp2scp.o
obj-$(CONFIG_QCOM_EBI2) += qcom-ebi2.o
obj-$(CONFIG_QCOM_SSC_BLOCK_BUS) += qcom-ssc-block-bus.o
-obj-$(CONFIG_STM32_FIREWALL) += stm32_firewall.o stm32_rifsc.o
+obj-$(CONFIG_STM32_FIREWALL) += stm32_firewall.o stm32_rifsc.o stm32_etzpc.o
obj-$(CONFIG_SUN50I_DE2_BUS) += sun50i-de2.o
obj-$(CONFIG_SUNXI_RSB) += sunxi-rsb.o
obj-$(CONFIG_OF) += simple-pm-bus.o
diff --git a/drivers/bus/stm32_etzpc.c b/drivers/bus/stm32_etzpc.c
new file mode 100644
index 000000000000..7fc0f16960be
--- /dev/null
+++ b/drivers/bus/stm32_etzpc.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023, STMicroelectronics - All Rights Reserved
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#include "stm32_firewall.h"
+
+/*
+ * ETZPC registers
+ */
+#define ETZPC_DECPROT 0x10
+#define ETZPC_HWCFGR 0x3F0
+
+/*
+ * HWCFGR register
+ */
+#define ETZPC_HWCFGR_NUM_TZMA GENMASK(7, 0)
+#define ETZPC_HWCFGR_NUM_PER_SEC GENMASK(15, 8)
+#define ETZPC_HWCFGR_NUM_AHB_SEC GENMASK(23, 16)
+#define ETZPC_HWCFGR_CHUNKS1N4 GENMASK(31, 24)
+
+/*
+ * ETZPC miscellaneous
+ */
+#define ETZPC_PROT_MASK GENMASK(1, 0)
+#define ETZPC_PROT_A7NS 0x3
+#define ETZPC_DECPROT_SHIFT 1
+
+#define IDS_PER_DECPROT_REGS 16
+
+static int stm32_etzpc_grant_access(struct stm32_firewall_controller *ctrl, u32 firewall_id)
+{
+ u32 offset, reg_offset, sec_val;
+
+ if (firewall_id >= ctrl->max_entries) {
+ dev_err(ctrl->dev, "Invalid sys bus ID %u", firewall_id);
+ return -EINVAL;
+ }
+
+ /* Check access configuration, 16 peripherals per register */
+ reg_offset = ETZPC_DECPROT + 0x4 * (firewall_id / IDS_PER_DECPROT_REGS);
+ offset = (firewall_id % IDS_PER_DECPROT_REGS) << ETZPC_DECPROT_SHIFT;
+
+ /* Verify peripheral is non-secure and attributed to cortex A7 */
+ sec_val = (readl(ctrl->mmio + reg_offset) >> offset) & ETZPC_PROT_MASK;
+ if (sec_val != ETZPC_PROT_A7NS) {
+ dev_dbg(ctrl->dev, "Invalid bus configuration: reg_offset %#x, value %d\n",
+ reg_offset, sec_val);
+ return -EACCES;
+ }
+
+ return 0;
+}
+
+static void stm32_etzpc_release_access(struct stm32_firewall_controller *ctrl __maybe_unused,
+ u32 firewall_id __maybe_unused)
+{
+}
+
+static int stm32_etzpc_probe(struct platform_device *pdev)
+{
+ struct stm32_firewall_controller *etzpc_controller;
+ struct device_node *np = pdev->dev.of_node;
+ u32 nb_per, nb_master;
+ struct resource *res;
+ void __iomem *mmio;
+ int rc;
+
+ etzpc_controller = devm_kzalloc(&pdev->dev, sizeof(*etzpc_controller), GFP_KERNEL);
+ if (!etzpc_controller)
+ return -ENOMEM;
+
+ mmio = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+ if (IS_ERR(mmio))
+ return PTR_ERR(mmio);
+
+ etzpc_controller->dev = &pdev->dev;
+ etzpc_controller->mmio = mmio;
+ etzpc_controller->name = dev_driver_string(etzpc_controller->dev);
+ etzpc_controller->type = STM32_PERIPHERAL_FIREWALL | STM32_MEMORY_FIREWALL;
+ etzpc_controller->grant_access = stm32_etzpc_grant_access;
+ etzpc_controller->release_access = stm32_etzpc_release_access;
+
+ /* Get number of etzpc entries*/
+ nb_per = FIELD_GET(ETZPC_HWCFGR_NUM_PER_SEC,
+ readl(etzpc_controller->mmio + ETZPC_HWCFGR));
+ nb_master = FIELD_GET(ETZPC_HWCFGR_NUM_AHB_SEC,
+ readl(etzpc_controller->mmio + ETZPC_HWCFGR));
+ etzpc_controller->max_entries = nb_per + nb_master;
+
+ platform_set_drvdata(pdev, etzpc_controller);
+
+ rc = stm32_firewall_controller_register(etzpc_controller);
+ if (rc) {
+ dev_err(etzpc_controller->dev, "Couldn't register as a firewall controller: %d",
+ rc);
+ return rc;
+ }
+
+ rc = stm32_firewall_populate_bus(etzpc_controller);
+ if (rc) {
+ dev_err(etzpc_controller->dev, "Couldn't populate ETZPC bus: %d",
+ rc);
+ return rc;
+ }
+
+ /* Populate all allowed nodes */
+ return of_platform_populate(np, NULL, NULL, &pdev->dev);
+}
+
+static const struct of_device_id stm32_etzpc_of_match[] = {
+ { .compatible = "st,stm32-etzpc" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, stm32_etzpc_of_match);
+
+static struct platform_driver stm32_etzpc_driver = {
+ .probe = stm32_etzpc_probe,
+ .driver = {
+ .name = "stm32-etzpc",
+ .of_match_table = stm32_etzpc_of_match,
+ },
+};
+module_platform_driver(stm32_etzpc_driver);
+
+MODULE_AUTHOR("Gatien Chevallier <[email protected]>");
+MODULE_DESCRIPTION("STMicroelectronics ETZPC driver");
+MODULE_LICENSE("GPL");
--
2.25.1

2023-10-02 20:16:59

by Gatien CHEVALLIER

[permalink] [raw]
Subject: Re: [PATCH v5 01/11] dt-bindings: document generic access controller



On 9/29/23 17:35, Rob Herring wrote:
>
> On Fri, 29 Sep 2023 16:28:42 +0200, Gatien Chevallier wrote:
>> From: Oleksii Moisieiev <[email protected]>
>>
>> Introducing of the generic access controller bindings for the
>> access controller provider and consumer devices. Those bindings are
>> intended to allow a better handling of accesses to resources in a
>> hardware architecture supporting several compartments.
>>
>> This patch is based on [1]. It is integrated in this patchset as it
>> provides a use-case for it.
>>
>> Diffs with [1]:
>> - Rename feature-domain* properties to access-control* to narrow
>> down the scope of the binding
>> - YAML errors and typos corrected.
>> - Example updated
>> - Some rephrasing in the binding description
>>
>> [1]: https://lore.kernel.org/lkml/0c0a82bb-18ae-d057-562b
>>
>> Signed-off-by: Oleksii Moisieiev <[email protected]>
>> Signed-off-by: Gatien Chevallier <[email protected]>
>>
>> ---
>> Changes in V5:
>> - Diffs with [1]
>> - Discarded the [IGNORE] tag as the patch is now part of the
>> patchset
>>
>> .../access-controllers/access-controller.yaml | 90 +++++++++++++++++++
>> 1 file changed, 90 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/access-controllers/access-controller.yaml
>>
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/access-controllers/access-controller.yaml: access-control-provider: missing type definition
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>

Hi Rob,

Running:
1- make dt_binding_check | grep access-control
2- make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/access-controllers/access-controller.yaml
from Krzysztof's slideset

with

pip3 show dtschema
Name: dtschema
Version: 2023.9

and

pip3 show yamllint
Name: yamllint
Version: 1.32.0

I don't see any of the errors reported by the robot. I have to clone
your repository to reproduce it.

Should I resubmit with a clean dt-check using the latest dtschema?

***********
However, I get:
warning: ignoring duplicate '$id' value
'http://devicetree.org/schemas/reserved-memory/framebuffer.yaml#
warning: ignoring duplicate '$id' value
'http://devicetree.org/schemas/reserved-memory/memory-region.yaml#
warning: ignoring duplicate '$id' value
'http://devicetree.org/schemas/reserved-memory/shared-dma-pool.yaml#
warning: ignoring duplicate '$id' value
'http://devicetree.org/schemas/reserved-memory/reserved-memory.yaml

Above warnings disappears when switching to:
pip3 show dtschema
Name: dtschema
Version: 2023.7

The above YAMLs seem to be duplicated in dtschema's latest version.
I guess it's a synchro that needs to be done since:
https://lore.kernel.org/all/[email protected]/
***********

Best regards,
Gatien

2023-10-03 07:46:41

by Gatien CHEVALLIER

[permalink] [raw]
Subject: Re: [PATCH v5 01/11] dt-bindings: document generic access controller

Hi Rob,

On 10/2/23 19:30, Rob Herring wrote:
> On Fri, Sep 29, 2023 at 04:28:42PM +0200, Gatien Chevallier wrote:
>> From: Oleksii Moisieiev <[email protected]>
>>
>> Introducing of the generic access controller bindings for the
>> access controller provider and consumer devices. Those bindings are
>> intended to allow a better handling of accesses to resources in a
>> hardware architecture supporting several compartments.
>>
>> This patch is based on [1]. It is integrated in this patchset as it
>> provides a use-case for it.
>>
>> Diffs with [1]:
>> - Rename feature-domain* properties to access-control* to narrow
>> down the scope of the binding
>> - YAML errors and typos corrected.
>> - Example updated
>> - Some rephrasing in the binding description
>>
>> [1]: https://lore.kernel.org/lkml/0c0a82bb-18ae-d057-562b
>>
>> Signed-off-by: Oleksii Moisieiev <[email protected]>
>> Signed-off-by: Gatien Chevallier <[email protected]>
>>
>> ---
>> Changes in V5:
>> - Diffs with [1]
>> - Discarded the [IGNORE] tag as the patch is now part of the
>> patchset
>>
>> .../access-controllers/access-controller.yaml | 90 +++++++++++++++++++
>> 1 file changed, 90 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/access-controllers/access-controller.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/access-controllers/access-controller.yaml b/Documentation/devicetree/bindings/access-controllers/access-controller.yaml
>> new file mode 100644
>> index 000000000000..9d305fccc333
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/access-controllers/access-controller.yaml
>> @@ -0,0 +1,90 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/access-controllers/access-controller.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Generic Domain Access Controller
>> +
>> +maintainers:
>> + - Oleksii Moisieiev <[email protected]>
>> +
>> +description: |+
>> + Common access controllers properties
>> +
>> + Access controllers are in charge of stating which of the hardware blocks under
>> + their responsibility (their domain) can be accesssed by which compartment. A
>> + compartment can be a cluster of CPUs (or coprocessors), a range of addresses
>> + or a group of hardware blocks. An access controller's domain is the set of
>> + resources covered by the access controller.
>> +
>> + This device tree bindings can be used to bind devices to their access
>> + controller provided by access-controller property. In this case, the device is
>> + a consumer and the access controller is the provider.
>> +
>> + An access controller can be represented by any node in the device tree and
>> + can provide one or more configuration parameters, needed to control parameters
>> + of the consumer device. A consumer node can refer to the provider by phandle
>> + and a set of phandle arguments, specified by '#access-controller-cells'
>> + property in the access controller node.
>> +
>> + Access controllers are typically used to set/read the permissions of a
>> + hardware block and grant access to it. Any of which depends on the access
>> + controller. The capabilities of each access controller are defined by the
>> + binding of the access controller device.
>> +
>> + Each node can be a consumer for the several access controllers.
>> +
>> +# always select the core schema
>> +select: true
>> +
>> +properties:
>> + "#access-controller-cells":
>> + $ref: /schemas/types.yaml#/definitions/uint32
>
> Drop. "#.*-cells" already defines the type.
>

Ok, I will drop it for V6

>> + description:
>> + Number of cells in a access-controller specifier;
>> + Can be any value as specified by device tree binding documentation
>> + of a particular provider.
>> +
>> + access-control-provider:
>> + description:
>> + Indicates that the node is an access controller.
>
> Drop. The presence of "#access-controller-cells" is enough to do that.
>

Ok, I wasn't sure. I'll will drop it for V6

>> +
>> + access-controller-names:
>> + $ref: /schemas/types.yaml#/definitions/string-array
>> + description:
>> + A list of access-controller names, sorted in the same order as
>> + access-controller entries. Consumer drivers will use
>> + access-controller-names to match with existing access-controller entries.
>> +
>> + access-controller:
>
> For consistency with other provider bindings: access-controllers
>

Ack

>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description:
>> + A list of access controller specifiers, as defined by the
>> + bindings of the access-controller provider.
>> +
>> +additionalProperties: true
>> +
>> +examples:
>> + - |
>> + uart_controller: access-controller@50000 {
>> + reg = <0x50000 0x10>;
>> + access-control-provider;
>> + #access-controller-cells = <2>;
>> + };
>> +
>> + bus_controller: bus@60000 {
>> + reg = <0x60000 0x10000>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> + access-control-provider;
>> + #access-controller-cells = <3>;
>> +
>> + uart4: serial@60100 {
>> + reg = <0x60100 0x400>;
>> + access-controller = <&uart_controller 1 2>,
>> + <&bus_controller 1 3 5>;
>> + access-controller-names = "controller", "bus-controller";
>
> Not great names. It should indicate what access is being controlled
> locally. Perhaps "reg" for register access, "dma" or "bus" for bus
> master access. (Not sure what your uart_controller is controlling access
> to.)
>
> Rob

Yes, I agree it's poor naming. I'll come up with something more
adequate. Thank you for the input.

Best regards,
Gatien