mbox series

[v4,00/11] Introduce STM32 Firewall framework

Message ID 20230811100731.108145-1-gatien.chevallier@foss.st.com
Headers show
Series Introduce STM32 Firewall framework | expand

Message

Gatien CHEVALLIER Aug. 11, 2023, 10:07 a.m. UTC
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 feature-domain-controller device
tree bindings: https://lore.kernel.org/lkml/0c0a82bb-18ae-d057-562b.
It is used by peripherals to reference a domain controller, in this
case a firewall feature domain. The bus uses the ID referenced by
the feature-domains 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 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

Oleksii Moisieiev (1):
  dt-bindings: Document common device controller bindings

Gatien Chevallier (10):
  dt-bindings: treewide: add feature-domains description
  dt-bindings: bus: document RIFSC
  dt-bindings: bus: document ETZPC
  firewall: introduce stm32_firewall framework
  of: property: fw_devlink: Add support for "feature-domains"
  bus: rifsc: introduce RIFSC firewall controller driver
  arm64: dts: st: add RIFSC as a domain 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

 .../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 +
 .../feature-domain-controller.yaml            |   84 +
 .../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         |    5 +
 .../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          | 1027 +++---
 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/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                  |  293 ++
 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 +
 47 files changed, 3352 insertions(+), 1919 deletions(-)
 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 Documentation/devicetree/bindings/feature-controllers/feature-domain-controller.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

Comments

Simon Horman Aug. 12, 2023, 2:09 p.m. UTC | #1
On Fri, Aug 11, 2023 at 12:07:25PM +0200, Gatien Chevallier wrote:

...

> diff --git a/drivers/bus/stm32_firewall.c b/drivers/bus/stm32_firewall.c
> new file mode 100644
> index 000000000000..900f3b052a66
> --- /dev/null
> +++ b/drivers/bus/stm32_firewall.c
> @@ -0,0 +1,293 @@
> +// 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, "feature-domains", "#feature-domain-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 feature-domains 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, "feature-domain-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 third argument */
> +		for (i = 0; i < provider_args.args_count; i++)
> +			firewall[j].extra_args[i] = provider_args.args[i + 1];

Hi Gatien,

Above it is checked that the maximum value of provider_args.args_count is
STM32_FIREWALL_MAX_ARGS.
So here the maximum value of i is STM32_FIREWALL_MAX_ARGS - 1.

STM32_FIREWALL_MAX_ARGS is defined as STM32_FIREWALL_MAX_EXTRA_ARGS + 1
And STM32_FIREWALL_MAX_EXTRA_ARGS is defined as 5.
So the maximum value of i is (5 + 1 - 1) = 5.

firewall[j] is of type struct stm32_firewall.
And its args field has STM32_FIREWALL_MAX_EXTRA_ARGS (5) elements.
Thus the maximum valid index is (5 - 1) = 4.

But the line above may access index 5.

Flagged by Smatch.

> +
> +		/* 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);

...

> diff --git a/include/linux/bus/stm32_firewall_device.h b/include/linux/bus/stm32_firewall_device.h
> new file mode 100644
> index 000000000000..7b4450a8ec15
> --- /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;
> +};

...
Gatien CHEVALLIER Sept. 28, 2023, 3:43 p.m. UTC | #2
On 8/12/23 16:09, Simon Horman wrote:
> On Fri, Aug 11, 2023 at 12:07:25PM +0200, Gatien Chevallier wrote:
> 
> ...
> 
>> diff --git a/drivers/bus/stm32_firewall.c b/drivers/bus/stm32_firewall.c
>> new file mode 100644
>> index 000000000000..900f3b052a66
>> --- /dev/null
>> +++ b/drivers/bus/stm32_firewall.c
>> @@ -0,0 +1,293 @@
>> +// 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, "feature-domains", "#feature-domain-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 feature-domains 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, "feature-domain-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 third argument */
>> +		for (i = 0; i < provider_args.args_count; i++)
>> +			firewall[j].extra_args[i] = provider_args.args[i + 1];
> 
> Hi Gatien,
> 
> Above it is checked that the maximum value of provider_args.args_count is
> STM32_FIREWALL_MAX_ARGS.
> So here the maximum value of i is STM32_FIREWALL_MAX_ARGS - 1.
> 
> STM32_FIREWALL_MAX_ARGS is defined as STM32_FIREWALL_MAX_EXTRA_ARGS + 1
> And STM32_FIREWALL_MAX_EXTRA_ARGS is defined as 5.
> So the maximum value of i is (5 + 1 - 1) = 5.
> 
> firewall[j] is of type struct stm32_firewall.
> And its args field has STM32_FIREWALL_MAX_EXTRA_ARGS (5) elements.
> Thus the maximum valid index is (5 - 1) = 4.
> 
> But the line above may access index 5.
> 
> Flagged by Smatch.
> 

Hi Simon,

Thank you for pointing this out.

I'll correct it for V5.

Best regards,
Gatien
>> +
>> +		/* 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);
> 
> ...
> 
>> diff --git a/include/linux/bus/stm32_firewall_device.h b/include/linux/bus/stm32_firewall_device.h
>> new file mode 100644
>> index 000000000000..7b4450a8ec15
>> --- /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;
>> +};
> 
> ...